fix(app): reconverge session load with upstream to fix deep-link race#6
Closed
maschwenk wants to merge 1 commit into
Closed
fix(app): reconverge session load with upstream to fix deep-link race#6maschwenk wants to merge 1 commit into
maschwenk wants to merge 1 commit into
Conversation
Realign server-session loadMessages, timeline model, and server-errors to upstream/dev so the initial limit=2 page backfills its user-message parents (upstream's parent-hydration). This is what makes the small deep-link load sufficient; our fork had branched before upstream added it and papered over the gap with a fragile 5-attempt retry loop and a simplified readiness check, which lost the race when a fast limit=200 prefetch resolved before a throttled limit=2 initial load. Drop those hacks (retry loop, ready simplification, inline not-found error) and keep only the one still-needed divergence: release the meta.loading flag when the finishing load still owns the slot, since upstream still gates that solely on the generation and can wedge loading=true forever. Adds a regression test for the reversed-order race (prefetch resolves before the slow initial load) plus upstream's parent-hydration and isTimelineReady coverage.
Author
|
Superseded by #7 (cherry-pick approach instead of full reconvergence). |
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
1 participant
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.
What
Fixes the Forge deep-link blank-chat race (direct
/session/:idmount) by reconverging the session-load path withupstream/dev.Our fork branched from upstream before upstream added parent-message hydration to
loadMessages— the step that backfills the user-message parents of the newest assistant messages so the initiallimit=2page is renderable. Our fork papered over the gap with a 5-attempt retry loop in the timeline model + a simplifiedreadycheck. When a fast backgroundlimit=200prefetch won the sharedinflight/meta.loadinglocks, the throttledlimit=2load got deduped/no-oped, the retry loop burned out, anddata.message[id](the only thing the timeline renders from) never got set → permanently blank transcript.Changes
context/server-session.ts— restore upstream's parent-hydration inloadMessages+sessionNotFoundError.pages/session/timeline/model.ts— restore upstream'sisTimelineReady+ plainsync(id)resource; drop the 5-attempt retry-loop hack. Now identical to upstream.utils/server-errors.ts— restoresessionNotFoundError/isLocalSessionNotFoundError. Now identical to upstream.The only remaining divergence from upstream: release
meta.loadingwhen the finishing load still owns the slot (owns), since upstream still gates that solely on the generation and can wedgeloading=trueforever after a generation change. ~8 lines.Tests
server-session.test.ts: reversed-order race — a fastlimit=200prefetch lands before the slow initiallimit=2load; asserts the transcript stays populated.isTimelineReadycoverage.bun testinpackages/app: 230 pass across context + timeline dirs.Consumed by
owner/Owner#47224 bumps the
apps/forge-ui/vendor/opencodesubmodule pin to this commit.