Repository navigation
fix: follow runtime session identity after model fallback - #134
ddallabenetta merged 19 commits into
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 9917e1a070
ℹ️ 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".
|
Completed a full local review of the PR and affected consumers in 5c77c1c, with the fixes delivered in one additive commit. Successful commands now return the canonical session and runtime handle, including after expiry recovery. Historical IDs remain independent even across retained HMR instances. History/context, auto-name, sidebar completion, and reload callbacks follow the same identity ownership rules. Delayed creation/promotion/fork results, composer recovery, and SSE connection continuations stop affecting the UI after their hook unmounts; handoff cleanup uses the final identity. The complete final candidate received independent correctness and simplicity passes with no remaining concrete findings. TypeScript syntax parsing of all 25 PR files and git diff --check passed. Tests, builds, hooks, browser acceptance, and typechecking were not run; local dependencies are absent. The additional Codex CLI review was unavailable because its configured model is unsupported by the account; GitNexus has no usable index. These are static-review results, not runtime acceptance. Scope round 14: baseline implementation 3 files +29/-6; current implementation 19 files +585/-195, tests 6 files +281/-12. The newly touched context route preserves runtime-addressed historical reads after expiry. No new files or dependencies. Confirmed PR head 5c77c1c after normal push; no CI wait. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4da3e38ca
ℹ️ 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".
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fe183fdc3
ℹ️ 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".
Use a stable runtime handle for active commands, events and polling while keeping historical session IDs independent. Retry expired handles only after an explicit negative acknowledgement. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87c63e4773
ℹ️ 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".
Include stable runtime identities in activity snapshots so fallback relocations do not mark historical sessions unread or play premature completion tones. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6a37630b9
ℹ️ 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".
Ignore identity responses after the hook unmounts and rekey composer drafts before notifying the parent of a runtime identity change. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97a71f223f
ℹ️ 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".
Keep bounded runtime continuation records after teardown, retry only confirmed final identities, and preserve the hook and its pending handshake and completion state across runtime relocations. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1af9df7244
ℹ️ 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".
Share live handles with settings reloads, pass canonical IDs to completion callbacks, and preserve teardown relocations in sidebar activity snapshots. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da4e305b47
ℹ️ 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".
Remember the active runtime handle in sessionStorage for refresh restoration while explicit history selection and new-session actions clear it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b74a8ab69
ℹ️ 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".
Clear only the matching dead runtime record after final identity recovery and treat reselecting a row with a continuation as an explicit historical reopen. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b93b11b8fd
ℹ️ 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".
Publish cleared runtime identities before retrying, propagate external expiry safely to the parent and hook, and route subagent transcript polling through the live handle. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46b746fe52
ℹ️ 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".
Add missing runtime identity capabilities in place while retaining SDK state and listeners, and retire legacy startup-ID cleanup callbacks. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6840a4d209
ℹ️ 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".
Continue state recovery after detail redirects, suppress background completion for the selected runtime, and resolve auto-naming through live or final runtime identities. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 918c437621
ℹ️ 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".
Recheck reopened canonical state after runtime expiry and restore per-tab continuations through activity mappings before requiring an old session row. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1e823fcb6
ℹ️ 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".
Carry canonical identity through command success and expiry recovery. Keep historical lookups independent, and fence delayed UI, composer, and stream continuations to their owning session. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@ddallabenetta could you take a look at the latest revision, 5c77c1c? I reviewed the full session identity flow and batched the fixes for command identity binding, independent historical lookups, and delayed UI/SSE callbacks after navigation. The latest review comments are addressed. Independent static review, syntax parsing, and diff checks passed; tests, builds, and browser acceptance have not been run. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c77c1ccbc
ℹ️ 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".
| const completedInBackground = [...previous].flatMap((id) => { | ||
| const runtimeId = previousRuntimeIds.get(id); | ||
| const currentId = (runtimeId && activitySnapshot.sessionIdsByRuntimeId?.[runtimeId]) || id; | ||
| return !runningSessionIds.has(currentId) && currentId !== selectedCanonicalId |
There was a problem hiding this comment.
Fresh evidence beyond the earlier selected-runtime fix is the no-current-mapping path: if the server restarts after selected run R has relocated A→B but before the sidebar records B, the previous snapshot still maps R to A while the new snapshot has neither R nor running IDs. currentId therefore falls back to A and selectedCanonicalId to B, so this condition marks historical A unread and invokes background completion; the hook's subsequent state reconciliation settles selected B and emits the normal completion, producing two tones. Treat previousRuntimeIds.get(id) === selectedRuntimeId as selected when the current map cannot resolve R.
AGENTS.md reference: AGENTS.md:L249-L251
Useful? React with 👍 / 👎.
Keep the previously selected runtime out of background completion after a server restart, even when its transcript ID has relocated. Cover unread markers, notifications, and related runtime transitions with behavioral regressions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 722018e582
ℹ️ 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".
| if (!mountedRef.current || sessionIdRef.current !== requestedId) return true; | ||
| if (state?.sessionId) { | ||
| followSessionIdentity(state.sessionId, state.runtimeId ?? null); |
There was a problem hiding this comment.
When an expired handle R1 is being reconciled for session B, a state response can be produced before another request or tab reopens B as R2 but arrive after a newer response has installed R2. Because this guard checks only requestedId, the late {sessionId: B} tombstone reaches this line as runtimeId: null and clears R2 from the hook, AppShell, and sessionStorage; if R2 then relocates to C, subsequent polling and commands fall back to historical B. Include the runtime used for the request in the ownership check, as the command path already does.
AGENTS.md reference: AGENTS.md:L252-L252
Useful? React with 👍 / 👎.
|
|
||
| try { | ||
| const response = await fetch(`/api/sessions/${encodeURIComponent(sessionId)}/auto-name`, { | ||
| const response = await fetch(`/api/sessions/${encodeURIComponent(runtimeId ?? sessionId)}/auto-name`, { |
There was a problem hiding this comment.
When an A→B model fallback misses only its identity frame, AppShell still has selected ID A plus the stable runtime handle used here until the hook's next reconciliation. handleViewFullHistory still opens /api/sessions/A/export, and the export route only calls resolveSessionPath(id), so the History button renders the abandoned A transcript instead of the active or completed B transcript. Resolve the handle to the current/final ID for this action, or make export runtime-aware, just as auto-name is here.
AGENTS.md reference: AGENTS.md:L250-L251
Useful? React with 👍 / 👎.
Ignore state responses from superseded runtime handles across polling, settlement, history loading and event preflight. Preserve active recovery when an old response is discarded. Resolve full-history exports through live or final runtime identities, while keeping historical transcript exports independent. Add behavioral regressions for out-of-order state, lifecycle boundaries, and exported transcript identity.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2085d4ac17
ℹ️ 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".
| contextRequestRef.current += 1; | ||
| identityDraftKeyRef.current = nextId; | ||
| sessionIdRef.current = nextId; | ||
| eventSourceSessionIdRef.current = nextId; |
There was a problem hiding this comment.
When a suspended tab keeps its SSE open while the A→B wrapper reaches the 10-minute idle teardown, the events route keeps heartbeating after destroy(), and the tombstone poll follows B with a null runtime; this assignment then relabels the still-open R1 source as B. ensureEventsConnected consequently accepts that source as OPEN and sends the next prompt to a newly created B/R2 wrapper without subscribing to R2, so streaming and blocking extension-UI events are lost and an input-requiring prompt can hang indefinitely. Close or invalidate the source when following an expired runtime instead of treating it like a live same-runtime relocation.
AGENTS.md reference: AGENTS.md:L250-L250
Useful? React with 👍 / 👎.
| contextUsage?: ContextUsage; | ||
| }; | ||
| if (!ownsContext()) return false; | ||
| if (d.sessionId) followSessionIdentity(d.sessionId, d.runtimeId ?? null); |
There was a problem hiding this comment.
When handleNavigate sends a command through expired R1 while concurrently loading /context through R1, the context response can be built as {sessionId: B, runtimeId: null} before the command retry reopens B as R2 but arrive afterward. ownsContext() checks only the transcript and request generation, so this call clears the newer R2 from the hook, AppShell, and sessionStorage; if that live wrapper later relocates, the tab has lost the handle needed to recover it. Capture the runtime used for the context request and ignore the response once runtimeIdRef.current changes, as the state-response paths already do.
AGENTS.md reference: AGENTS.md:L252-L252
Useful? React with 👍 / 👎.
Bind event streams to runtime handles, retire streams and pending handshakes when that runtime expires or changes, and ignore queued retired callbacks. Require a connected handshake before reuse while preserving same-runtime session relocations. Ignore context responses after the request's runtime ownership changes. Add behavioral regressions for expired/replaced streams, prompt gating, streamed output and blocking UI delivery, and concurrent context recovery.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79d995d5e3
ℹ️ 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".
| if (runtimeId !== undefined && eventSourceRef.current | ||
| && eventSourceRuntimeIdRef.current !== runtimeId) closeEvents(); |
There was a problem hiding this comment.
When the first prompt opens /events for a canonical session with no runtime handle, the immediate refreshContextUsage effect can receive {runtimeId: R} after the server registers the wrapper but before that EventSource delivers its connected frame. At that point eventSourceRuntimeIdRef.current is still null, so this branch closes the very handshake ensureEventsConnected is awaiting; the promise settles as closed and the prompt is rejected/restored even though the new stream and wrapper are valid. Treat a null runtime on a matching pending source as unbound and adopt R (or suppress the competing poll) rather than invalidating it.
AGENTS.md reference: AGENTS.md:L250-L250
Useful? React with 👍 / 👎.
Adopt a discovered runtime only for the matching unbound canonical stream while its first handshake is pending. Keep awaiting the connected frame and continue retiring expired, replaced, or unrelated sources. Reject contradictory first-frame runtime identities even after handshake timeout, without changing later transport reconnect behavior. Add poll-before-handshake and identity-ordering regression coverage.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3628617311
ℹ️ 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".
| if (initialSessionId && !restoredRef.current) { | ||
| restoredRef.current = true; | ||
| const target = allSessions.find((s) => s.id === initialSessionId); | ||
| const target = restoreSessionContinuation(initialSessionId, allSessions, listedRuntimeIdsRef.current); |
There was a problem hiding this comment.
When ?session=A is restored from the saved {A, R} continuation and the activity map resolves R to B, this returns B but onSelectSession(..., true) deliberately leaves the URL at A. The mounted hook subsequently remembers {B, R}, so a second refresh can no longer match the A URL to the continuation and reopens historical A (or loses the session), allowing the next prompt to diverge from the recovered run. Replace the route with the canonical ID whenever restoration changes it.
AGENTS.md reference: AGENTS.md:L242-L243
Useful? React with 👍 / 👎.
| contextRequestRef.current += 1; | ||
| identityDraftKeyRef.current = nextId; | ||
| sessionIdRef.current = nextId; | ||
| if (eventSourceRef.current) eventSourceSessionIdRef.current = nextId; |
There was a problem hiding this comment.
Fresh evidence beyond the expired-source fix is the fatal-error timer: after a same-runtime A→B relocation retains this source, connectEvents's onerror closure still reconnects with its captured A. If that error schedules a retry and expiry reconciliation clears the runtime before the timer fires (for example while settlement is still loading a large history), the timer opens /events/A; its connected frame then follows a newly opened historical A wrapper and subsequent prompts can mutate the wrong transcript. Reconnect using sessionIdRef.current or invalidate the queued retry when identity changes.
AGENTS.md reference: AGENTS.md:L250-L250
Useful? React with 👍 / 👎.
| if (d.runtimeId || d.sessionId !== sid) { | ||
| followSessionIdentity(d.sessionId, d.runtimeId); |
There was a problem hiding this comment.
Fresh evidence beyond the state/context ownership guards is that history ownership ignores the runtime generation. If a detail request for B captures idle wrapper R1, B is reopened and adopted as R2, and the delayed R1 response then arrives, ownsHistory() still passes because the transcript ID is unchanged; this branch reinstalls R1 and closes R2's SSE, so an active R2 prompt loses streaming and blocking extension UI events. Capture the runtime used when the detail request starts and reject a response once that handle has been superseded.
AGENTS.md reference: AGENTS.md:L251-L252
Useful? React with 👍 / 👎.
2 participants
Summary
When the SDK changes a running session's ID during model fallback, the browser keeps using the original ID. In a resumed conversation on 0.5.0, the model replied successfully, but deferred thinking requests returned
404 Assistant message not foundand the model selector continued to show the previous model.Follow the runtime identity when loading history or refreshing state, and move the existing live wrapper to its current registry key. Existing event streams announce the new identity, while new streams report the actual ID on connection. Pending sends use that ID after the handshake. A stable runtime handle keeps active polling, commands and event streams attached even when the identity event is missed, and follows identity changes in place without interrupting pending handshakes or completion notifications. Earlier persisted session IDs reopen their own transcripts instead of remaining aliases for the active conversation. Teardown retains a bounded runtime-handle-to-final-ID record. Expired handles permit one retry to that confirmed final ID only after the server explicitly confirms the command was not accepted; unknown handles fail explicitly. Teardown removes the wrapper by identity so independently reopened sessions survive.
Sidebar activity snapshots also match the same runtime across identity changes, preventing premature completion audio and unread markers on historical IDs.
The history response uses the live model so a completed reply does not reset the selector to the persisted, outdated model.
Verification
pwd, directory reading, and two file reads), then returned the requested summary without HTTP 400/500 errors.git diff --checkpassed. Repository-requested GitNexus analysis was attempted, but indexing crashed in a native binding and change detection could not run without an index.This fixes the reproduced identity mismatch; it does not claim to fix the separately reported HTTP 400/500 errors, which were not reproduced.