fix(CS-11508): code submode recovers from initial 404 after indexing#5202
fix(CS-11508): code submode recovers from initial 404 after indexing#5202FadhlanR wants to merge 3 commits into
Conversation
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 1h 51m 45s ⏱️ - 1m 17s Results for commit 15afa38. ± Comparison against earlier commit a34483f. Realm Server Test Results 1 files ± 0 1 suites ±0 10m 22s ⏱️ - 2m 52s Results for commit d416026. ± Comparison against earlier commit 15afa38. |
FadhlanR
force-pushed
the
cs-11508-create-file-not-found
branch
from
3e2d8a3 to
7b4c9cd
Compare
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) <noreply@anthropic.com>
FadhlanR
force-pushed
the
cs-11508-create-file-not-found
branch
from
7b4c9cd to
a34483f
Compare
|
Please ignore the failing test. It's unrelated to this PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a34483f933
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Object.keys(this.cardService.clientRequestIds), | ||
| ); | ||
| } | ||
| } else if (clientRequestId.startsWith('bot-patch:')) { |
There was a problem hiding this comment.
When the AI creates the missing file via WriteTextFileCommand, saveSource(finalUrl, ..., 'create-file') generates an X-Boxel-Client-Request-Id like create-file:<uuid>, and the realm echoes that id on the index/incremental event. This invalidation handler only reloads for no id, instance:, editor:* from another client, or bot-patch:, so the subscribed resource still ignores the actual create-file event and remains in not-found after the initial 404. Treat create-file: invalidations for the current URL as reload-worthy (or otherwise handle this save type) so the intended WriteTextFileCommand flow recovers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Good catch — verified by reading cardService.saveSource (packages/host/app/services/card-service.ts line 240: clientRequestId = options?.clientRequestId ?? \${type}:${uuidv4()}`). The AI assistant flow does generate create-file:` and the original handler dropped it on the floor.
Addressed in 15afa38:
- Added
create-file:to the reload-worthy branch alongsidebot-patch:. Comment captures the why — the clientRequestId being incardService.clientRequestIdsdoes NOT imply we already have the content (unlike theeditor:case, where Monaco already holds the source). - The acceptance test now drives the write via
cardService.saveSource(new URL(newFileUrl), newFileSource, 'create-file')instead ofrealm.write, so the test would also fail without this branch — the original test was passing for the wrong reason (no clientRequestId →!clientRequestIdbranch reloads).
| let resolvedURL: string = | ||
| this.innerState.state === 'ready' || | ||
| this.innerState.state === 'not-found' || | ||
| this.innerState.state === 'server-error' | ||
| ? this.innerState.url | ||
| : this._url; |
There was a problem hiding this comment.
If this resource is reused while it is already ready for file A, modify() updates _url to missing file B but leaves innerState as A until the fetch settles. An index/incremental event for B that arrives in that window is matched against innerState.url (A), so it is ignored; if the in-flight fetch then returns the original 404, there is no later event to recover from. Use the current requested URL when it differs from the loaded state (or transition to loading on modify) so early invalidations for the newly requested file are not lost.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Confirmed — the prior matching used a single resolved URL (innerState.url when in ready/not-found/server-error, falling back to _url only during loading). For a reused resource, the ready/not-found branch returned the stale URL and the new file's invalidation got dropped.
Addressed in 15afa38: replaced the single-URL pick with a candidate set { normalize(_url), normalize(innerState.url) } and the handler matches if any invalidation appears in that set. This keeps the redirect case working (canonical URL still lives in innerState.url after a successful read) while no longer dropping early invalidations for the newly requested URL during a transition.
…t the requested URL
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:<uuid>`, 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) <noreply@anthropic.com>
|
Trying this out in the preview deployment, it fails for the first file creation but works afterward: 
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a race in host code submode where navigating to a not-yet-indexed file (initial fetch returns 404) could leave FileResource stuck in not-found even after the realm later broadcasts an index/incremental invalidation for that URL.
Changes:
- Subscribe to realm events before the first fetch in
FileResource.modify()so a laterindex/incrementalcan trigger a reload after an initial 404. - Extract the realm invalidation callback into a class field and make it tolerant of being invoked while
innerStateis stillloading; treatcreate-file:clientRequestIds as reload-worthy. - Add an acceptance test that reproduces the end-to-end failure mode (initial 404 → realm write → incremental invalidation → UI recovery).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/host/app/resources/file.ts | Establish realm-event subscription earlier and refactor invalidation handling to recover from initial 404s after indexing completes. |
| packages/host/tests/acceptance/code-submode/create-file-test.gts | Add acceptance coverage for recovery when an external write creates a new file and the realm later emits index/incremental. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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), | ||
| ); |
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) <noreply@anthropic.com>
Background and Goal
When code submode navigates to a file that does not yet exist in the realm — for example, the AI assistant runs
WriteTextFileCommandto create a new.gtsand immediately updates the operator-modecodePathto that URL — the URL bar surfaces "This resource does not exist" and stays there until the user manually re-navigates.Root cause:
FileResourceonly wires its realm-event subscription on the read success branch (packages/host/app/resources/file.ts). The 404 path early-returns before the subscription is set up, so the realm's subsequentindex/incrementalinvalidation for the just-created URL has no listener and the resource stays stuck innot-found.Where to start
packages/host/app/resources/file.ts—modify()now resolves the realm viarealm.realmOf(rri(url))and callssetSubscriptionbefore the first fetch. The success-branchsetSubscriptioncall is preserved (it's idempotent for the same realm URL). The SSE callback is extracted into a class fieldonRealmInvalidationand made tolerant of being invoked whileinnerStateis stillloading.packages/host/tests/acceptance/code-submode/create-file-test.gts— new modulewhen an external write creates a new file. The test drives the failure mode end-to-end: navigate to a URL that does not exist, assert the URL bar shows the not-found error, performrealm.write(...), await the matchingindex/incrementalevent, then assert recovery.Test plan
pnpm -C packages/host test:ember --filter="recovers when a newly-created file arrives via a realm"→ 1 / 1 PASS (verified locally; pre-fix this same test fails withElement [data-test-card-url-bar-error] exists once / expected: does not exist)pnpm -C packages/host test:ember --filter="Acceptance | code submode | create-file tests"→ 37 / 37 PASSpnpm -C packages/host lint→ clean🤖 Generated with Claude Code