fix(CS-11508): code submode recovers from initial 404 after indexing (v2)#5278
Open
FadhlanR wants to merge 5 commits into
Open
fix(CS-11508): code submode recovers from initial 404 after indexing (v2)#5278FadhlanR wants to merge 5 commits into
FadhlanR wants to merge 5 commits into
Conversation
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>
…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>
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>
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) <noreply@anthropic.com>
FadhlanR
force-pushed
the
cs-11508-create-file-not-found-v2
branch
from
6fb9a05 to
f406050
Compare
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 1h 53m 45s ⏱️ Results for commit 8ef38cb. Realm Server Test Results 1 files ±0 1 suites ±0 12m 29s ⏱️ +7s Results for commit 8ef38cb. ± Comparison against earlier commit f406050. |
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) <noreply@anthropic.com>
![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: 8ef38cb0d9
ℹ️ 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".
| // 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 }) => { |
There was a problem hiding this comment.
When a second navigation calls modify() while a previous file fetch is still in flight, keepLatestTask lets the old fetch finish instead of canceling it. That stale response still calls updateState(), and because updateState() compares the old response URL to the now-current this._url, an old extensionless/canonicalizing request can invoke onRedirect() and replace the user's newer code path back to the stale file before the queued latest read runs. This regresses quick code-mode navigation that restartableTask previously protected; either capture/validate the requested URL before applying results or keep navigation reads cancelable while only queuing invalidation reloads.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR re-introduces (v2) the fix for code submode getting stuck in a “This resource does not exist” state when navigating to a newly-created file that initially 404s but becomes available after realm indexing emits an index/incremental invalidation event.
Changes:
- Update
FileResourceto subscribe to realm invalidation events earlier (including on the initial 404 path) and treatcreate-file:clientRequestIds as reload-worthy. - Switch
FileResource.readfromrestartableTasktokeepLatestTaskto avoid cancellation-driven state races. - Add acceptance coverage for recovering from initial 404s when a new file arrives via an
index/incrementalevent (both cold-start and navigate-from-ready flows).
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 | Hoists realm subscription into modify(), refactors invalidation handler, treats create-file: as reload-worthy, and changes read task semantics to keepLatestTask. |
| packages/host/tests/acceptance/code-submode/create-file-test.gts | Adds acceptance tests that reproduce the initial-404-then-recover behavior via realm index/incremental events. |
Comments suppressed due to low confidence (1)
packages/host/app/resources/file.ts:238
- With
keepLatestTask, an in-flight read is no longer cancelled whenmodify()is called again. Because this task readsthis._urlinside the try/catch, a URL change during the await can cause the previous request’s 404/error handling to update state for the new URL (or log the wrong URL). Capture the URL at the start of the task and ignore stale results whenthis._urlhas changed.
private read = keepLatestTask(async (opts?: { force?: boolean }) => {
let response;
try {
response = await this.network.authedFetch(this._url, {
headers: { Accept: SupportedMimeType.CardSource },
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
380
to
383
| 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), | ||
| ); |
jurgenwerk
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-opens the work from #5202, which was merged then reverted in #5277 because the new acceptance test hung the host CI shard. The hang is fixed here by replacing the gate-based race test with a simpler acceptance test, and by switching
readtokeepLatestTaskso the race the original test was pinning can no longer occur.Summary
FileResource.modify()viarealm.realmOf()so a 404-then-recover flow can react to the laterindex/incrementalinvalidation. The SSE callback is extracted to a class field so it can safely run whileinnerStateis stillloadingornot-found.create-file:clientRequestIds (the pathWriteTextFileCommand/ the AI assistant uses) as reload-worthy in the invalidation handler, alongsideinstance:,editor:*, andbot-patch:._urlandinnerState.urlso events for a freshly-requested URL aren't dropped during the window wheremodify()has updated_urlbut the new fetch hasn't settled yet.FileResource.readtokeepLatestTask. The in-flight read completes normally and the event-driven reload runs after it, so state writes are sequential and a cancelled task'snot-foundcan no longer land on top of a fresh task'sready.visitOperatorModehops with awaitForon the URL-bar error in between, so each navigation phase settles before the create-file write triggers the realm broadcast.Test plan
index/incrementalevent (covers Buck's repro).ready, notnot-found.