From fe5594570319129b20e58cd48c94eabaf4f71b0a Mon Sep 17 00:00:00 2001 From: Fadhlan Ridhwanallah Date: Thu, 11 Jun 2026 19:58:51 +0700 Subject: [PATCH 1/5] fix(CS-11508): code submode recovers from initial 404 after indexing The FileResource only wires its realm-event subscription on the read success branch. When code submode navigates to a file that has not yet been indexed (AI assistant creates a .gts then immediately updates codePath), the first authedFetch 404s and `read` early-returns before the subscription is set up, so the subsequent `index/incremental` invalidation has no listener and the URL bar stays on "This resource does not exist". Hoist the subscription into `modify()` via realm.realmOf() so it is wired before the first fetch. The success-branch subscription is kept (setSubscription is idempotent on the same realmURL). The SSE callback is extracted to a class field so it can safely run while innerState is still `loading` or `not-found`. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/host/app/resources/file.ts | 162 ++++++++++-------- .../code-submode/create-file-test.gts | 108 +++++++++++- 2 files changed, 200 insertions(+), 70 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index 45756e190a..eefedbf473 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -20,6 +20,7 @@ import type CardService from '@cardstack/host/services/card-service'; import type { SaveType } from '@cardstack/host/services/card-service'; import type OperatorModeStateService from '@cardstack/host/services/operator-mode-state-service'; +import type RealmService from '@cardstack/host/services/realm'; import type RecentFilesService from '@cardstack/host/services/recent-files-service'; import type StoreService from '@cardstack/host/services/store'; @@ -143,6 +144,7 @@ class _FileResource extends Resource { @service declare private cardService: CardService; @service declare private recentFilesService: RecentFilesService; @service declare private operatorModeStateService: OperatorModeStateService; + @service declare private realm: RealmService; @service declare private store: StoreService; constructor(owner: Owner) { @@ -178,6 +180,20 @@ class _FileResource extends Resource { this._url = url; this.onStateChange = onStateChange; this.onRedirect = onRedirect; + + // Subscribe to realm events BEFORE the first fetch so a 404 result + // (e.g. the AI assistant navigates code-submode to a file it just + // created, before realm indexing has caught up) can still be recovered + // when the realm subsequently broadcasts an `index/incremental` event + // for this URL. Without this, the success-branch `setSubscription` + // below at the end of `read` is never reached on the 404 path, leaving + // the resource permanently in `not-found` despite the realm having + // since delivered the file. + let realmId = this.realm.realmOf(rri(url)); + if (realmId) { + this.setSubscription(String(realmId), this.onRealmInvalidation); + } + this.read.perform(); } @@ -285,86 +301,98 @@ class _FileResource extends Resource { }, }); - this.setSubscription(realmURL, (event: RealmEventContent) => { - if ( - event.eventName !== 'index' || - // we wait specifically for the index complete event ("incremental") so - // that the subsequent index read retrieves the latest contents of the file - event.indexType !== 'incremental' || - !Array.isArray(event.invalidations) - ) { - return; - } + this.setSubscription(realmURL, this.onRealmInvalidation); + }); - let { invalidations } = event as { invalidations: string[] }; - let normalizedURL = this.url.endsWith('.json') - ? this.url.replace(/\.json$/, '') - : this.url; + private onRealmInvalidation = (event: RealmEventContent): void => { + if ( + event.eventName !== 'index' || + // we wait specifically for the index complete event ("incremental") so + // that the subsequent index read retrieves the latest contents of the file + event.indexType !== 'incremental' || + !Array.isArray(event.invalidations) + ) { + return; + } - if (invalidations.includes(normalizedURL)) { - realmEventsLogger.trace( - `file resource ${normalizedURL} processing invalidation`, - event, - ); + let { invalidations } = event as { invalidations: string[] }; + // Fall back to the input URL when the file has not yet successfully + // loaded — the `url` getter reads from `innerState` which is only set + // by `read`. With the subscription now wired from `modify()`, an event + // can land before the first read completes. + let resolvedURL: string = + this.innerState.state === 'ready' || + this.innerState.state === 'not-found' || + this.innerState.state === 'server-error' + ? this.innerState.url + : this._url; + let normalizedURL = resolvedURL.endsWith('.json') + ? resolvedURL.replace(/\.json$/, '') + : resolvedURL; + + if (invalidations.includes(normalizedURL)) { + realmEventsLogger.trace( + `file resource ${normalizedURL} processing invalidation`, + event, + ); - let clientRequestId = event.clientRequestId; - let reloadFile = false; + let clientRequestId = event.clientRequestId; + let reloadFile = false; - if (!clientRequestId || clientRequestId.startsWith('instance:')) { - reloadFile = true; + if (!clientRequestId || clientRequestId.startsWith('instance:')) { + reloadFile = true; + realmEventsLogger.debug( + `reloading file resource ${normalizedURL} because realm event has ${!clientRequestId ? 'no clientRequestId' : 'clientRequestId from instance editor'}`, + ); + } else if ( + clientRequestId.startsWith('editor:') || + clientRequestId.startsWith('editor-with-instance:') + ) { + if (this.cardService.clientRequestIds.has(clientRequestId)) { realmEventsLogger.debug( - `reloading file resource ${normalizedURL} because realm event has ${!clientRequestId ? 'no clientRequestId' : 'clientRequestId from instance editor'}`, + `ignoring because request id is contained in known clientRequestIds`, + event.clientRequestId, ); - } else if ( - clientRequestId.startsWith('editor:') || - clientRequestId.startsWith('editor-with-instance:') - ) { - if (this.cardService.clientRequestIds.has(clientRequestId)) { - realmEventsLogger.debug( - `ignoring because request id is contained in known clientRequestIds`, - event.clientRequestId, - ); - } else { - reloadFile = true; - realmEventsLogger.debug( - `reloading file resource ${normalizedURL} because request id is ${clientRequestId}, not contained within known clientRequestIds`, - Object.keys(this.cardService.clientRequestIds), - ); - } - } else if (clientRequestId.startsWith('bot-patch:')) { + } else { reloadFile = true; realmEventsLogger.debug( - `reloading file resource ${normalizedURL} because request id is ${clientRequestId}`, + `reloading file resource ${normalizedURL} because request id is ${clientRequestId}, not contained within known clientRequestIds`, + Object.keys(this.cardService.clientRequestIds), ); } + } else if (clientRequestId.startsWith('bot-patch:')) { + reloadFile = true; + realmEventsLogger.debug( + `reloading file resource ${normalizedURL} because request id is ${clientRequestId}`, + ); + } - if (reloadFile) { - // Mirrors the store's invalidation path: only reset the loader when - // the rewritten module has actually been imported (which includes - // entries cached as `state: 'broken'`). Resetting unconditionally - // would clone the whole loader on every external write — including - // boxel-cli writes for modules the host never loaded — and drop - // unrelated loaded modules. clearFetchCache is required because - // the module endpoint's ETag is keyed on unix-second-granularity - // `lastModified`; without it, a write landing in the same second - // as the prior fetch can be served as a 304 with the old broken - // body. The store only covers realms it subscribed to (i.e. ones - // it loaded a card instance from), so code-mode-only browsing of - // a .gts whose realm has no loaded instance relies on this path. - if ( - hasExecutableExtension(normalizedURL) && - this.loaderService.loader.isModuleLoaded(normalizedURL) - ) { - this.loaderService.resetLoader({ - clearFetchCache: true, - reason: 'file-resource-external-invalidation', - }); - } - this.read.perform({ force: true }); + if (reloadFile) { + // Mirrors the store's invalidation path: only reset the loader when + // the rewritten module has actually been imported (which includes + // entries cached as `state: 'broken'`). Resetting unconditionally + // would clone the whole loader on every external write — including + // boxel-cli writes for modules the host never loaded — and drop + // unrelated loaded modules. clearFetchCache is required because + // the module endpoint's ETag is keyed on unix-second-granularity + // `lastModified`; without it, a write landing in the same second + // as the prior fetch can be served as a 304 with the old broken + // body. The store only covers realms it subscribed to (i.e. ones + // it loaded a card instance from), so code-mode-only browsing of + // a .gts whose realm has no loaded instance relies on this path. + if ( + hasExecutableExtension(normalizedURL) && + this.loaderService.loader.isModuleLoaded(normalizedURL) + ) { + this.loaderService.resetLoader({ + clearFetchCache: true, + reason: 'file-resource-external-invalidation', + }); } + this.read.perform({ force: true }); } - }); - }); + } + }; writeTask = restartableTask( async ( diff --git a/packages/host/tests/acceptance/code-submode/create-file-test.gts b/packages/host/tests/acceptance/code-submode/create-file-test.gts index 216ac62bdd..eccab38f82 100644 --- a/packages/host/tests/acceptance/code-submode/create-file-test.gts +++ b/packages/host/tests/acceptance/code-submode/create-file-test.gts @@ -1,12 +1,26 @@ -import { click, fillIn, waitFor, waitUntil } from '@ember/test-helpers'; +import { + click, + fillIn, + settled, + waitFor, + waitUntil, +} from '@ember/test-helpers'; import { getService } from '@universal-ember/test-support'; import QUnit, { module, test } from 'qunit'; -import { baseRealm, rri, baseRRI, Deferred } from '@cardstack/runtime-common'; +import { + baseRealm, + rri, + baseRRI, + Deferred, + type Realm, +} from '@cardstack/runtime-common'; import type FileUploadService from '@cardstack/host/services/file-upload'; +import type { RealmEventContent } from 'https://cardstack.com/base/matrix-event'; + import { percySnapshot, setupLocalIndexing, @@ -271,6 +285,7 @@ module('Acceptance | code submode | create-file tests', function (hooks) { } let adapter: TestRealmAdapter; + let realm: Realm; setupApplicationTest(hooks); setupLocalIndexing(hooks); @@ -301,7 +316,7 @@ module('Acceptance | code submode | create-file tests', function (hooks) { testRealmURL2, ); } - ({ adapter } = await withCachedRealmSetup(async () => { + ({ adapter, realm } = await withCachedRealmSetup(async () => { await setupAcceptanceTestRealm({ contents: { ...SYSTEM_CARD_FIXTURE_CONTENTS, ...filesB }, realmURL: testRealmURL2, @@ -1529,4 +1544,91 @@ export class TestCard extends Animal { }); }, ); + + // When the AI assistant (or any external writer) creates a new .gts and + // then updates the code-submode codePath to the just-written URL, the + // host's FileResource (packages/host/app/resources/file.ts) can lose the + // race against the realm's index pipeline. The first authedFetch returns + // 404 and `read` transitions into `state: 'not-found'`. The realm later + // broadcasts `index/incremental` for the new URL, and the FileResource + // must react to that event and recover — otherwise the URL bar stays + // stuck on "This resource does not exist" until the user re-navigates. + // + // This test simulates the external write by navigating to a non-existent + // URL, confirming the URL bar shows the not-found error, then performing + // the write via the realm directly (mirroring what the realm-server does + // when a card+source PUT lands). After the realm broadcasts the matching + // `index/incremental` event, the URL bar must recover. + module('when an external write creates a new file', function (hooks) { + hooks.beforeEach(function () { + setRealmPermissions({ + [baseRealm.url]: ['read'], + [testRealmURL]: ['read', 'write'], + }); + }); + + test('code submode recovers when a newly-created file arrives via a realm index/incremental event', async function (assert) { + let newFilePath = 'ai-created-card.gts'; + let newFileUrl = `${testRealmURL}${newFilePath}`; + let newFileSource = ` + import { CardDef } from 'https://cardstack.com/base/card-api'; + export default class AiCreatedCard extends CardDef { + static displayName = 'Ai Created Card'; + } + `; + + // Simulate the AI assistant updating the codePath to a file that does + // not yet exist in the realm. The host has not seen this URL before, + // so FileResource.read will hit 404. + await visitOperatorMode(newFileUrl); + + await waitFor('[data-test-card-url-bar-error]'); + assert + .dom('[data-test-card-url-bar-error]') + .containsText( + 'This resource does not exist', + 'URL bar surfaces the not-found error on initial 404', + ); + + // The realm broadcasts the incremental invalidation event over matrix + // once indexing of the newly-written file completes. Subscribe so we + // can await its arrival deterministically before asserting recovery. + let incrementalEvent = new Deferred(); + let unsubscribe = getService('message-service').subscribe( + testRealmURL, + (ev: RealmEventContent) => { + if ( + ev.eventName === 'index' && + ev.indexType === 'incremental' && + Array.isArray(ev.invalidations) && + (ev.invalidations as string[]).includes(newFileUrl) + ) { + unsubscribe(); + incrementalEvent.fulfill(); + } + }, + ); + + // realm.write mirrors what the realm-server does when + // WriteTextFileCommand's PUT lands: persist source, transpile, index, + // and broadcast the `index/incremental` event with the new URL in + // `invalidations`. No clientRequestId is passed — the same shape the + // bot uses when it patches/creates a card module. + await realm.write(newFilePath, newFileSource); + await incrementalEvent.promise; + await settled(); + + assert + .dom('[data-test-card-url-bar-error]') + .doesNotExist( + 'URL bar error clears after the realm broadcasts the index/incremental event for the new file', + ); + assert + .dom('[data-test-card-url-bar-input]') + .hasValue( + newFileUrl, + 'code submode stays on the new file URL after recovery', + ); + }); + }); }); From 1ab5e591cba5e99acfdff7295a2621b6ea94245d Mon Sep 17 00:00:00 2001 From: Fadhlan Ridhwanallah Date: Fri, 12 Jun 2026 12:47:20 +0700 Subject: [PATCH 2/5] Address review: reload on create-file: and match invalidations against the requested URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes layered on the previous commit: 1. Reload on `create-file:` clientRequestId. `cardService.saveSource(..., 'create-file')` — the path `WriteTextFileCommand` uses for the AI assistant — tags the request with `X-Boxel-Client-Request-Id: create-file:`, which the realm echoes on the matching `index/incremental` event. The previous handler recognized `instance:`, `editor:*`, and `bot-patch:` but ignored `create-file:`, so the AI assistant's actual flow stayed stuck in `not-found` even with the subscription wired correctly. Treat `create-file:` like `bot-patch:` (always reload) — the id being in `cardService.clientRequestIds` does NOT imply we already have the content the way it does for `editor:` writes. 2. Match invalidations against `_url` AND `innerState.url`. When the resource is reused for a new URL, `_url` updates immediately in `modify()` but `innerState.url` is the prior file's URL until the new fetch settles. An `index/incremental` event for the new URL arriving in that window was matched against the stale `innerState.url` and dropped — leaving the new file stranded if its own fetch 404'd. Build a candidate set { normalize(_url), normalize(innerState.url) } and match if any invalidation is in it. This keeps the redirect case working (innerState.url is the canonical form) without dropping events for the newly requested URL during a transition. The acceptance test is also updated to drive the create-file path via `cardService.saveSource(..., 'create-file')` so the test would fail without (1) as well as without the subscription-hoisting fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/host/app/resources/file.ts | 56 +++++++++++++------ .../code-submode/create-file-test.gts | 36 +++++++----- 2 files changed, 60 insertions(+), 32 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index eefedbf473..0c66cd6307 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -191,7 +191,15 @@ class _FileResource extends Resource { // since delivered the file. let realmId = this.realm.realmOf(rri(url)); if (realmId) { - this.setSubscription(String(realmId), this.onRealmInvalidation); + this.setSubscription(realmId, this.onRealmInvalidation); + } else { + // No early subscription possible — the realm service hasn't yet + // discovered the realm that owns this URL. Recovery from an initial + // 404 then depends on the success-branch `setSubscription` inside + // `read`, which only fires if the fetch eventually succeeds. + log.debug( + `FileResource: no known realm for ${url} at modify-time; deferring subscription to read-success branch`, + ); } this.read.perform(); @@ -316,21 +324,26 @@ class _FileResource extends Resource { } let { invalidations } = event as { invalidations: string[] }; - // Fall back to the input URL when the file has not yet successfully - // loaded — the `url` getter reads from `innerState` which is only set - // by `read`. With the subscription now wired from `modify()`, an event - // can land before the first read completes. - let resolvedURL: string = - this.innerState.state === 'ready' || - this.innerState.state === 'not-found' || - this.innerState.state === 'server-error' - ? this.innerState.url - : this._url; - let normalizedURL = resolvedURL.endsWith('.json') - ? resolvedURL.replace(/\.json$/, '') - : resolvedURL; - - if (invalidations.includes(normalizedURL)) { + // Match invalidations against both the currently-requested URL + // (`this._url`, kept current by `modify`) and the URL the resource + // most recently loaded into `innerState`. Both are necessary + // because: + // - `innerState.url` may be a realm-canonicalized form of `_url` + // (e.g. `experiments/author` redirects to `experiments/author.gts`) + // and the realm emits invalidations for the canonical form. + // - During a transition (modify called with a new URL while + // innerState still holds a prior file), `innerState.url` is + // stale; only `_url` reflects what the caller is asking for — + // dropping the event here would orphan the new file. + let normalize = (raw: string) => + raw.endsWith('.json') ? raw.replace(/\.json$/, '') : raw; + let candidates = new Set([normalize(this._url)]); + if (this.innerState.state !== 'loading') { + candidates.add(normalize(this.innerState.url)); + } + let normalizedURL = invalidations.find((inv) => candidates.has(inv)); + + if (normalizedURL) { realmEventsLogger.trace( `file resource ${normalizedURL} processing invalidation`, event, @@ -360,7 +373,16 @@ class _FileResource extends Resource { Object.keys(this.cardService.clientRequestIds), ); } - } else if (clientRequestId.startsWith('bot-patch:')) { + } else if ( + clientRequestId.startsWith('bot-patch:') || + // create-file writes originate from this host (cardService.saveSource + // with saveType 'create-file' — the path WriteTextFileCommand uses) + // but the FileResource may not yet have any content because its first + // fetch raced indexing and 404'd. The clientRequestId being in + // cardService.clientRequestIds does NOT imply we already have the + // content (unlike the editor: case), so we still need to reload. + clientRequestId.startsWith('create-file:') + ) { reloadFile = true; realmEventsLogger.debug( `reloading file resource ${normalizedURL} because request id is ${clientRequestId}`, diff --git a/packages/host/tests/acceptance/code-submode/create-file-test.gts b/packages/host/tests/acceptance/code-submode/create-file-test.gts index eccab38f82..27404f6c21 100644 --- a/packages/host/tests/acceptance/code-submode/create-file-test.gts +++ b/packages/host/tests/acceptance/code-submode/create-file-test.gts @@ -9,13 +9,7 @@ import { import { getService } from '@universal-ember/test-support'; import QUnit, { module, test } from 'qunit'; -import { - baseRealm, - rri, - baseRRI, - Deferred, - type Realm, -} from '@cardstack/runtime-common'; +import { baseRealm, rri, baseRRI, Deferred } from '@cardstack/runtime-common'; import type FileUploadService from '@cardstack/host/services/file-upload'; @@ -285,7 +279,6 @@ module('Acceptance | code submode | create-file tests', function (hooks) { } let adapter: TestRealmAdapter; - let realm: Realm; setupApplicationTest(hooks); setupLocalIndexing(hooks); @@ -316,7 +309,7 @@ module('Acceptance | code submode | create-file tests', function (hooks) { testRealmURL2, ); } - ({ adapter, realm } = await withCachedRealmSetup(async () => { + ({ adapter } = await withCachedRealmSetup(async () => { await setupAcceptanceTestRealm({ contents: { ...SYSTEM_CARD_FIXTURE_CONTENTS, ...filesB }, realmURL: testRealmURL2, @@ -1609,14 +1602,23 @@ export class TestCard extends Animal { }, ); - // realm.write mirrors what the realm-server does when - // WriteTextFileCommand's PUT lands: persist source, transpile, index, - // and broadcast the `index/incremental` event with the new URL in - // `invalidations`. No clientRequestId is passed — the same shape the - // bot uses when it patches/creates a card module. - await realm.write(newFilePath, newFileSource); + // Mirror WriteTextFileCommand exactly. `cardService.saveSource` with + // saveType 'create-file' POSTs the new source to the realm and tags + // the request with `X-Boxel-Client-Request-Id: create-file:`, + // which the realm echoes back in the `index/incremental` event. + // This shape — saveType 'create-file' and that clientRequestId + // prefix — is what the AI assistant produces and what the + // invalidation handler must treat as reload-worthy even though the + // id is in `cardService.clientRequestIds`. + let cardService = getService('card-service'); + await cardService.saveSource( + new URL(newFileUrl), + newFileSource, + 'create-file', + ); await incrementalEvent.promise; await settled(); + await waitFor('[data-test-code-mode][data-test-save-idle]'); assert .dom('[data-test-card-url-bar-error]') @@ -1629,6 +1631,10 @@ export class TestCard extends Animal { newFileUrl, 'code submode stays on the new file URL after recovery', ); + assert.ok( + getMonacoContent().includes('AiCreatedCard'), + 'monaco loads the recovered file body, not a stale buffer', + ); }); }); }); From a4570cb27a39f55cd95346e67a2fe35c166010fb Mon Sep 17 00:00:00 2001 From: Fadhlan Ridhwanallah Date: Mon, 15 Jun 2026 20:23:43 +0700 Subject: [PATCH 3/5] Guard read task catch from TaskCancelation overwriting fresh state When an invalidation event arrives while FileResource.read is in flight, the handler restarts the read; ember-concurrency raises TaskCancelation at the cancelled task's awaited fetch. The catch block treated that the same as a real network error and called updateState({ state: 'not-found' }) AFTER the restarted task had already landed state: 'ready', leaving the URL bar stuck. Filter cancellation via didCancel(err) at the top of the catch so only genuine fetch failures fall through to the not-found state update. Adds an acceptance test that gates the two fetches independently and releases the cancelled read after the fresh read has already set state to ready, pinning the bad ordering. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/host/app/resources/file.ts | 13 +- .../code-submode/create-file-test.gts | 145 +++++++++++++++++- 2 files changed, 156 insertions(+), 2 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index 0c66cd6307..f764604f43 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -5,7 +5,7 @@ import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { parse } from 'date-fns'; -import { restartableTask } from 'ember-concurrency'; +import { didCancel, restartableTask } from 'ember-concurrency'; import { Resource } from 'ember-modify-based-class-resource'; import { @@ -243,6 +243,17 @@ class _FileResource extends Resource { return; } } catch (err: any) { + // `read` is restartable: when an invalidation event arrives while + // a read is in flight, `read.perform({force: true})` cancels this + // instance and starts a fresh one. ember-concurrency surfaces the + // cancel as a TaskCancelation thrown at the awaited fetch. The + // fresh task's `updateState({ state: 'ready' })` can land before + // the cancelled task's awaited promise resolves; treating the + // cancellation as a real fetch failure would then overwrite that + // ready state with `not-found`. + if (didCancel(err)) { + return; + } log.error(`Could not get file ${this._url}, err: ${err.message}`); this.updateState({ state: 'not-found', url: rri(this._url) }); return; diff --git a/packages/host/tests/acceptance/code-submode/create-file-test.gts b/packages/host/tests/acceptance/code-submode/create-file-test.gts index 27404f6c21..b8191d5027 100644 --- a/packages/host/tests/acceptance/code-submode/create-file-test.gts +++ b/packages/host/tests/acceptance/code-submode/create-file-test.gts @@ -9,7 +9,13 @@ import { import { getService } from '@universal-ember/test-support'; import QUnit, { module, test } from 'qunit'; -import { baseRealm, rri, baseRRI, Deferred } from '@cardstack/runtime-common'; +import { + baseRealm, + rri, + baseRRI, + Deferred, + SupportedMimeType, +} from '@cardstack/runtime-common'; import type FileUploadService from '@cardstack/host/services/file-upload'; @@ -1636,5 +1642,142 @@ export class TestCard extends Animal { 'monaco loads the recovered file body, not a stale buffer', ); }); + + // When `updateCodePath` runs `FileResource.modify` for a URL whose + // file is about to be created, the modify-driven read can still be + // in flight when the realm broadcasts the matching `index/incremental` + // event. The handler then calls `read.perform({force: true})`, which + // restartableTask treats as cancel-and-restart. The cancelled task's + // awaited fetch eventually resolves, raising TaskCancelation at the + // `await`. If the catch block treated cancellation as a real fetch + // failure, it would call `updateState({ state: 'not-found' })` + // AFTER the restart already landed `state: 'ready'`, leaving the + // URL bar permanently stuck on the not-found error. + // + // This test gates the two fetches independently so the cancelled + // (older) read can be released AFTER the fresh read has already set + // state to ready — pinning the ordering that requires the + // `didCancel` guard in `file.ts` to survive. + test('cancelled read does not overwrite a fresh ready state with not-found', async function (assert) { + let newFilePath = 'cancellation-race-card.gts'; + let newFileUrl = `${testRealmURL}${newFilePath}`; + let newFileSource = ` + import { CardDef } from 'https://cardstack.com/base/card-api'; + export default class CancellationRaceCard extends CardDef { + static displayName = 'Cancellation Race Card'; + } + `; + + // Land on an EXISTING file first so FileResource is in state + // 'ready' with a realm subscription already established. This + // matches Buck's flow (he was viewing a prior file when he + // clicked New Card Definition) and is required for the race — + // the bug only fires when modify is called on an already-subscribed + // FileResource and the indexing event lands during the read. + await visitOperatorMode(`${testRealmURL}index.json`); + await waitFor('[data-test-code-mode][data-test-save-idle]'); + + // Per-call gating: each request for newFileUrl awaits its own + // Deferred so the test can release the cancelled read AFTER the + // fresh read has already updated state to 'ready'. A single + // shared gate would force both reads to resolve in mount order, + // which doesn't reproduce the race. + let pendingReads: Deferred[] = []; + let network = getService('network'); + let readGate = async (request: Request) => { + if ( + request.method === 'GET' && + request.url === newFileUrl && + request.headers.get('Accept') === SupportedMimeType.CardSource + ) { + let gate = new Deferred(); + pendingReads.push(gate); + await gate.promise; + } + return null; + }; + network.virtualNetwork.mount(readGate, { prepend: true }); + + try { + // Write the file BEFORE navigating so the realm broadcasts the + // incremental event during the modify-driven read. The handler + // matches the in-flight URL and triggers read.perform({force:true}), + // which cancels the in-flight task and starts a fresh one. + let incrementalEvent = new Deferred(); + let unsubscribe = getService('message-service').subscribe( + testRealmURL, + (ev: RealmEventContent) => { + if ( + ev.eventName === 'index' && + ev.indexType === 'incremental' && + Array.isArray(ev.invalidations) && + (ev.invalidations as string[]).includes(newFileUrl) + ) { + unsubscribe(); + incrementalEvent.fulfill(); + } + }, + ); + + let cardService = getService('card-service'); + let opState = getService('operator-mode-state-service'); + + // Fire updateCodePath without awaiting so the test can observe + // the modify-driven read sitting at the gate. Awaiting would + // block until the read resolves, defeating the point of the + // gate. + let codePathChange = opState.updateCodePath(new URL(newFileUrl)); + + // Wait until the modify-driven read (read#1) is parked behind + // the gate before triggering the write. Without this barrier + // the saveSource POST and its incremental event could land + // before modify fires — the handler would then run with the + // PRIOR `_url` and drop the event, never reaching the + // restart-driven cancellation that this test exists to pin. + await waitUntil(() => pendingReads.length >= 1); + + await cardService.saveSource( + new URL(newFileUrl), + newFileSource, + 'create-file', + ); + await incrementalEvent.promise; + + // Wait until the restart spawned by the incremental event has + // also parked at the gate; only with both reads in flight can + // we choose the release order that reproduces the race. + await waitUntil(() => pendingReads.length >= 2); + + // Release the FRESH read first. Its 200 response lands and + // sets state to 'ready'. Then release the ORIGINAL (now + // cancelled) read so its TaskCancelation throws at its `await` + // — without the didCancel guard, the catch block overwrites + // the ready state with 'not-found'. + pendingReads[1].fulfill(); + await settled(); + pendingReads[0].fulfill(); + await codePathChange; + await settled(); + await waitFor('[data-test-code-mode][data-test-save-idle]'); + + assert + .dom('[data-test-card-url-bar-error]') + .doesNotExist( + 'cancelled read must not overwrite the fresh ready state with not-found', + ); + assert + .dom('[data-test-card-url-bar-input]') + .hasValue( + newFileUrl, + 'code submode stays on the new file URL after the cancellation race', + ); + assert.ok( + getMonacoContent().includes('CancellationRaceCard'), + 'monaco shows the new file body once the fresh read wins', + ); + } finally { + network.virtualNetwork.unmount(readGate); + } + }); }); }); From f406050e9dd9bc18a9af908b2203269653f0fa98 Mon Sep 17 00:00:00 2001 From: Fadhlan Ridhwanallah Date: Thu, 18 Jun 2026 20:41:22 +0700 Subject: [PATCH 4/5] Use keepLatestTask for FileResource.read so reloads queue, not cancel The earlier restartableTask + didCancel approach had to compensate for a cancel-after-restart race: ember-concurrency raises TaskCancelation when the cancelled task's awaited fetch finally resolves, and the catch block treated that the same as a real fetch failure. The fresh task's state: 'ready' could be overwritten by the cancelled task's state: 'not-found' whenever the cancelled response landed last. Switch read to keepLatestTask. The in-flight read now completes normally and the event-driven reload runs after, so state writes are sequential and no TaskCancelation ever flows through the catch block. Drop the didCancel guard along with it. Replace the gate-based race test (it hung the CI shard) with a simpler acceptance test for the navigate-from-ready-to-newly-created path, which covers Buck's actual repro without trying to pin a race that can no longer occur. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/host/app/resources/file.ts | 24 ++- .../code-submode/create-file-test.gts | 184 +++++------------- 2 files changed, 65 insertions(+), 143 deletions(-) diff --git a/packages/host/app/resources/file.ts b/packages/host/app/resources/file.ts index f764604f43..f62f22f2fb 100644 --- a/packages/host/app/resources/file.ts +++ b/packages/host/app/resources/file.ts @@ -5,7 +5,7 @@ import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { parse } from 'date-fns'; -import { didCancel, restartableTask } from 'ember-concurrency'; +import { keepLatestTask, restartableTask } from 'ember-concurrency'; import { Resource } from 'ember-modify-based-class-resource'; import { @@ -222,7 +222,16 @@ class _FileResource extends Resource { } } - private read = restartableTask(async (opts?: { force?: boolean }) => { + // `keepLatestTask`, not `restartableTask`: when an invalidation event + // arrives while a read is in flight, we want the in-flight read to + // complete and the event-driven reload to run AFTER it. Restarting + // here would cancel the in-flight task, and the cancelled task's + // awaited fetch would later throw TaskCancelation at the `await` — + // catching that as a real fetch failure would overwrite the fresh + // task's `state: 'ready'` with `state: 'not-found'` whenever the + // cancelled response happened to land after the restart's response. + // Queuing the latest extra perform keeps state writes sequential. + private read = keepLatestTask(async (opts?: { force?: boolean }) => { let response; try { response = await this.network.authedFetch(this._url, { @@ -243,17 +252,6 @@ class _FileResource extends Resource { return; } } catch (err: any) { - // `read` is restartable: when an invalidation event arrives while - // a read is in flight, `read.perform({force: true})` cancels this - // instance and starts a fresh one. ember-concurrency surfaces the - // cancel as a TaskCancelation thrown at the awaited fetch. The - // fresh task's `updateState({ state: 'ready' })` can land before - // the cancelled task's awaited promise resolves; treating the - // cancellation as a real fetch failure would then overwrite that - // ready state with `not-found`. - if (didCancel(err)) { - return; - } log.error(`Could not get file ${this._url}, err: ${err.message}`); this.updateState({ state: 'not-found', url: rri(this._url) }); return; diff --git a/packages/host/tests/acceptance/code-submode/create-file-test.gts b/packages/host/tests/acceptance/code-submode/create-file-test.gts index b8191d5027..f2d2956e9d 100644 --- a/packages/host/tests/acceptance/code-submode/create-file-test.gts +++ b/packages/host/tests/acceptance/code-submode/create-file-test.gts @@ -9,13 +9,7 @@ import { import { getService } from '@universal-ember/test-support'; import QUnit, { module, test } from 'qunit'; -import { - baseRealm, - rri, - baseRRI, - Deferred, - SupportedMimeType, -} from '@cardstack/runtime-common'; +import { baseRealm, rri, baseRRI, Deferred } from '@cardstack/runtime-common'; import type FileUploadService from '@cardstack/host/services/file-upload'; @@ -1643,141 +1637,71 @@ export class TestCard extends Animal { ); }); - // When `updateCodePath` runs `FileResource.modify` for a URL whose - // file is about to be created, the modify-driven read can still be - // in flight when the realm broadcasts the matching `index/incremental` - // event. The handler then calls `read.perform({force: true})`, which - // restartableTask treats as cancel-and-restart. The cancelled task's - // awaited fetch eventually resolves, raising TaskCancelation at the - // `await`. If the catch block treated cancellation as a real fetch - // failure, it would call `updateState({ state: 'not-found' })` - // AFTER the restart already landed `state: 'ready'`, leaving the - // URL bar permanently stuck on the not-found error. - // - // This test gates the two fetches independently so the cancelled - // (older) read can be released AFTER the fresh read has already set - // state to ready — pinning the ordering that requires the - // `didCancel` guard in `file.ts` to survive. - test('cancelled read does not overwrite a fresh ready state with not-found', async function (assert) { - let newFilePath = 'cancellation-race-card.gts'; + // Buck's actual reproduction is navigation: he was viewing a prior + // file in code mode, then clicked New Card Definition, and the URL + // bar stayed stuck on "This resource does not exist" for the new + // file. The existing test above covers cold-start visit to an + // un-indexed URL; this one covers the navigate-from-ready path, + // because `FileResource.modify` runs with `innerState` already in + // `ready` and the realm subscription already established when the + // codePath changes. + test('navigating from a ready file to a newly-created file recovers via index/incremental', async function (assert) { + let newFilePath = 'navigated-to-card.gts'; let newFileUrl = `${testRealmURL}${newFilePath}`; let newFileSource = ` import { CardDef } from 'https://cardstack.com/base/card-api'; - export default class CancellationRaceCard extends CardDef { - static displayName = 'Cancellation Race Card'; + export default class NavigatedToCard extends CardDef { + static displayName = 'Navigated To Card'; } `; - // Land on an EXISTING file first so FileResource is in state - // 'ready' with a realm subscription already established. This - // matches Buck's flow (he was viewing a prior file when he - // clicked New Card Definition) and is required for the race — - // the bug only fires when modify is called on an already-subscribed - // FileResource and the indexing event lands during the read. await visitOperatorMode(`${testRealmURL}index.json`); await waitFor('[data-test-code-mode][data-test-save-idle]'); - // Per-call gating: each request for newFileUrl awaits its own - // Deferred so the test can release the cancelled read AFTER the - // fresh read has already updated state to 'ready'. A single - // shared gate would force both reads to resolve in mount order, - // which doesn't reproduce the race. - let pendingReads: Deferred[] = []; - let network = getService('network'); - let readGate = async (request: Request) => { - if ( - request.method === 'GET' && - request.url === newFileUrl && - request.headers.get('Accept') === SupportedMimeType.CardSource - ) { - let gate = new Deferred(); - pendingReads.push(gate); - await gate.promise; - } - return null; - }; - network.virtualNetwork.mount(readGate, { prepend: true }); - - try { - // Write the file BEFORE navigating so the realm broadcasts the - // incremental event during the modify-driven read. The handler - // matches the in-flight URL and triggers read.perform({force:true}), - // which cancels the in-flight task and starts a fresh one. - let incrementalEvent = new Deferred(); - let unsubscribe = getService('message-service').subscribe( - testRealmURL, - (ev: RealmEventContent) => { - if ( - ev.eventName === 'index' && - ev.indexType === 'incremental' && - Array.isArray(ev.invalidations) && - (ev.invalidations as string[]).includes(newFileUrl) - ) { - unsubscribe(); - incrementalEvent.fulfill(); - } - }, - ); + let incrementalEvent = new Deferred(); + let unsubscribe = getService('message-service').subscribe( + testRealmURL, + (ev: RealmEventContent) => { + if ( + ev.eventName === 'index' && + ev.indexType === 'incremental' && + Array.isArray(ev.invalidations) && + (ev.invalidations as string[]).includes(newFileUrl) + ) { + unsubscribe(); + incrementalEvent.fulfill(); + } + }, + ); - let cardService = getService('card-service'); - let opState = getService('operator-mode-state-service'); - - // Fire updateCodePath without awaiting so the test can observe - // the modify-driven read sitting at the gate. Awaiting would - // block until the read resolves, defeating the point of the - // gate. - let codePathChange = opState.updateCodePath(new URL(newFileUrl)); - - // Wait until the modify-driven read (read#1) is parked behind - // the gate before triggering the write. Without this barrier - // the saveSource POST and its incremental event could land - // before modify fires — the handler would then run with the - // PRIOR `_url` and drop the event, never reaching the - // restart-driven cancellation that this test exists to pin. - await waitUntil(() => pendingReads.length >= 1); - - await cardService.saveSource( - new URL(newFileUrl), - newFileSource, - 'create-file', - ); - await incrementalEvent.promise; - - // Wait until the restart spawned by the incremental event has - // also parked at the gate; only with both reads in flight can - // we choose the release order that reproduces the race. - await waitUntil(() => pendingReads.length >= 2); - - // Release the FRESH read first. Its 200 response lands and - // sets state to 'ready'. Then release the ORIGINAL (now - // cancelled) read so its TaskCancelation throws at its `await` - // — without the didCancel guard, the catch block overwrites - // the ready state with 'not-found'. - pendingReads[1].fulfill(); - await settled(); - pendingReads[0].fulfill(); - await codePathChange; - await settled(); - await waitFor('[data-test-code-mode][data-test-save-idle]'); + let cardService = getService('card-service'); + let opState = getService('operator-mode-state-service'); - assert - .dom('[data-test-card-url-bar-error]') - .doesNotExist( - 'cancelled read must not overwrite the fresh ready state with not-found', - ); - assert - .dom('[data-test-card-url-bar-input]') - .hasValue( - newFileUrl, - 'code submode stays on the new file URL after the cancellation race', - ); - assert.ok( - getMonacoContent().includes('CancellationRaceCard'), - 'monaco shows the new file body once the fresh read wins', + await opState.updateCodePath(new URL(newFileUrl)); + await cardService.saveSource( + new URL(newFileUrl), + newFileSource, + 'create-file', + ); + await incrementalEvent.promise; + await settled(); + await waitFor('[data-test-code-mode][data-test-save-idle]'); + + assert + .dom('[data-test-card-url-bar-error]') + .doesNotExist( + 'URL bar error clears after the realm broadcasts the index/incremental event for the new file', ); - } finally { - network.virtualNetwork.unmount(readGate); - } + assert + .dom('[data-test-card-url-bar-input]') + .hasValue( + newFileUrl, + 'code submode stays on the new file URL after recovery', + ); + assert.ok( + getMonacoContent().includes('NavigatedToCard'), + 'monaco loads the recovered file body after the navigate-then-create sequence', + ); }); }); }); From 8ef38cb0d9892a0796274bb6983b3a29b26381d1 Mon Sep 17 00:00:00 2001 From: Fadhlan Ridhwanallah Date: Thu, 18 Jun 2026 22:02:47 +0700 Subject: [PATCH 5/5] Settle each navigation phase before triggering the create-file write The first attempt used updateCodePath + a concurrent saveSource so the modify-driven read and the POST raced. The modify-driven read returned 404, then the test asserted before the event-driven reload settled, leaving the URL bar stuck on not-found. Switch to two visitOperatorMode hops with a waitFor on the URL-bar error between them. Each hop drains the runloop, so the modify-driven read for the new URL is guaranteed to complete (and surface the expected 404 error) before the create-file POST runs and the realm broadcasts the incremental invalidation that should clear the error. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../code-submode/create-file-test.gts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/host/tests/acceptance/code-submode/create-file-test.gts b/packages/host/tests/acceptance/code-submode/create-file-test.gts index f2d2956e9d..b52cfd7674 100644 --- a/packages/host/tests/acceptance/code-submode/create-file-test.gts +++ b/packages/host/tests/acceptance/code-submode/create-file-test.gts @@ -1655,9 +1655,24 @@ export class TestCard extends Animal { } `; + // Load an existing file first so FileResource is in state 'ready' + // and subscribed to the realm before the navigation that triggers + // the bug. await visitOperatorMode(`${testRealmURL}index.json`); await waitFor('[data-test-code-mode][data-test-save-idle]'); + // Re-visit the new (not-yet-existent) URL through code mode. This + // drives a second `FileResource.modify` from the already-ready + // state — the exact transition Buck reported. + await visitOperatorMode(newFileUrl); + await waitFor('[data-test-card-url-bar-error]'); + assert + .dom('[data-test-card-url-bar-error]') + .containsText( + 'This resource does not exist', + 'URL bar surfaces the not-found error on initial 404 after navigation', + ); + let incrementalEvent = new Deferred(); let unsubscribe = getService('message-service').subscribe( testRealmURL, @@ -1675,9 +1690,6 @@ export class TestCard extends Animal { ); let cardService = getService('card-service'); - let opState = getService('operator-mode-state-service'); - - await opState.updateCodePath(new URL(newFileUrl)); await cardService.saveSource( new URL(newFileUrl), newFileSource,