fix(coding-agent): run resume budget check before session teardown - #1473
Tinycute00 wants to merge 16 commits into
Conversation
Resuming a session whose restored transcript exceeds the current model's context budget threw ModelUsabilityBudgetError only from createAgentSession, which runs after switchSession has already torn down the live session and invalidated its extension runner. A resume that was always going to be rejected therefore destroyed the session the user was still in, and the next input crashed with the stale-extension-context error. In interactive mode the error was routed to handleFatalRuntimeError -> process.exit(1), so the user was silently dropped to the shell. AgentSessionRuntime.switchSession now runs assertSessionAdmissible before teardownCurrent, mirroring the post-teardown admission check in createAgentSession against the current model. A rejected resume becomes a clean no-op that leaves the live session intact. InteractiveMode.handleResumeSession now catches ModelUsabilityBudgetError, shows it via showError, and returns cancelled instead of exiting the process.
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: 99e7984681
ℹ️ 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".
…ore the switch event Resolve the model a resume will actually restore (the destination session's stored model via resolveStoredModelReference when present and authorized, else the live model) and run the admission check against it, so a resume that the active model would fit but the restored model cannot is rejected before the live session is torn down. Move the non-mutating preflight (SessionManager.open, cwd existence, admission) ahead of the session_before_switch emit so a rejected resume is a true no-op and never lets handlers mutate live-session state. Give the cwd-override retry in handleResumeSession the same recoverable budget handling via a shared cancelResumeWithBudgetError helper instead of letting a second-attempt budget error escape as an unhandled rejection. Preserve the budget rejection's typed identity across the shared-host RPC boundary: connection-handler emits a model_usability_budget error code with the projection, and rpc-client reconstructs ModelUsabilityBudgetError so the TUI's instanceof check still holds client-side.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Addressed all four Codex findings plus the Changelog gate on this branch (commit
Tests: extended |
…reflight # Conflicts: # packages/coding-agent/CHANGELOG.md # packages/coding-agent/src/core/changes.md # packages/coding-agent/src/modes/interactive/changes.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 165ad7e536
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c3fa75de5
ℹ️ 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".
Defer writer reservations throughout candidate preparation, revalidate the accepted destination before switch handlers, and roll back new grants on cancellation or failure. Run candidate extension shutdown without disposing the live MCP singleton or provider resources. Integrate upstream/main through 8c90e72, preserving upstream lockfiles and released changelog sections.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43b6f8f66b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f7b567437
ℹ️ 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 materialized history until persistence, revalidate changed targets, retain typed recovery across TUI/RPC, normalize busy self-resume paths, and defer MCP native-search ownership until attachment. Admission ordering is unchanged. Verified with 26 named regressions, 10285 passing package tests, check/build, 10 real RPC scenarios and 12 xterm TUI scenarios. Ancillary text-tool-leak smoke failures reproduce identically at the unchanged base. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Preserve upstream mandatory resume compaction while disposing genuinely rejected candidates. Reconcile budget fixtures with the upstream admission policy and retain exact destination projection coverage. Verified check/build, 10294 passing coding-agent tests, 11 real RPC scenarios including first-turn compaction, and 12 xterm TUI scenarios. The lifecycle-order decision remains unresolved; no remote publication is included. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Run the cancellable check before destination trust, snapshot and factories while retaining exact SDK admission before outgoing shutdown. Preserve active btw work on cancellation or rejection and keep accepted replacement cleanup in session_shutdown. Record the approved event contract, keep post-snapshot conflicts recoverable, and cover async veto writes plus work beginning during veto or preparation. Verified check/build, 10302 package tests, 12 real RPC scenarios and 16 xterm TUI scenarios. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0672e9bf4b
ℹ️ 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".
Match local catalog ownership by installing provider-scoped catalogs only at session_start. Discarded candidates no longer replace live lazy-tool activation or native-injection diagnostics. Verified two faithful RED failures and three GREEN cases, 90 related controls, check/build, 10305 package tests, actual Node API behavior and both shared-worker TUI recovery paths. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Describe the cancellable handler before candidate snapshot and admission, and final revalidation before outgoing shutdown. This matches switchSession and the command-specific protocol section; no behavior or JSON examples change. Verified by source/document comparison and git diff --check. Code at parent 36c4da6 has green full local and GitHub CI; no prose-pinning test is added. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be41e0df4a
ℹ️ 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".
Stream content fingerprints instead of retaining a second full transcript buffer. Preserve same-size content conflict detection, metadata-only acceptance, synchronous writer checks and large legacy content. Verified faithful retention RED, 21 focused tests, a 65 MiB Node API probe, full check/build, 10311 package tests and the current RPC/TUI matrix. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Carry the existing lazy activator service into its owning AgentSession for unknown-tool resolution and native-injection diagnostics. Direct SDK sessions work without session_start while discarded candidates remain out of global catalogs. Verified three faithful RED cases, 101 focused tests, actual Node SDK dispatch and native recovery, full check/build, 10311 package tests and the final RPC/TUI matrix. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Distinguish SDK/factory admission from accepted-runtime startup mutations. Document ordinary budget sums, compaction-aware maxima and the separate compaction-required SDK fallback diagnostic. Verified against implementation and real startup probes; optional recommendation errors retain the admitted fallback. JSON examples and runtime behavior are unchanged. No prose-pinning tests. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Integrate 2d0fa41 and its stalled socket peer credit fix. Preserve both unreleased changelog and RPC tracker entries; no core implementation conflict or lifecycle policy change. Verified 176 related tests, check/build, 10315 package tests, fresh RPC12 and shared TUI2 with cleanup/auth traces. Other verified local surfaces exercise unchanged core paths. Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
2 participants
Summary
Keep the active session usable when
/resumeis rejected by destinationSDK/factory admission. Interactive and shared-host RPC clients retain typed
recoverable budget and preparation-conflict errors.
Lifecycle contract
session_before_switchcheck.This intentionally replaces the earlier PR promise of no before-switch callback
on budget rejection. The callback is a veto, not cleanup. Cleanup belongs in
session_shutdown; no new public event is introduced. Active/btwwork survivescancelled/rejected admission and is cleaned up on actual replacement.
A veto performs no destination trust prompt, trust persistence or factory
execution. Writes completed by an awaited veto enter the snapshot; later
preparation-time writes produce a recoverable conflict.
Admission covers the model, settings, prompt and active schemas assembled by the
SDK/factories at that point. Accepted
session_startand later arbitrary extensionmutations are not speculatively executed. The shipped recommendation's rejected
optional model change is reported as an extension error and retains the admitted
fallback; this is not a retroactive resume rejection. The PR does not promise
transactional validation of every later model/tool/prompt mutation.
Data and ownership
single or aggregate text over the existing 64 MiB resident budget. Restore
bounded caching only after persistence.
64 KiB scratch space, synchronous exact-content checks, missing/empty distinction,
same-size edit detection and metadata-only acceptance.
Discarded candidates invalidate only their own registrations, not live services.
only at accepted startup. Direct unbound SDK sessions separately retain their own
catalog/diagnostic owner through existing lazy-activator registration.
during awaited veto or preparation.
Verification
Final local head:
1d438ade7f875d5faac82b4d6eb0c23a4ae13c7d, integrating main2d0fa41c5without rewriting history. Only changelog/tracker additions conflicted;both sides and all upstream transport behavior were preserved.
bun run checkandbun run buildpassed.files / 40 skipped tests unchanged.
101 tests; snapshot/writer/large-legacy controls: 21 tests.
passed. A 68,157,731-byte session retained 142 bytes of fingerprint metadata,
preserved its durable digest, and preserved external bytes on typed conflict.
veto writes; shared TUI direct/cwd-retry cases both passed.
6
/btwoutcomes and 6 local budget flows. All critical screenshots were inspected.recorded. Interrupted guard runs remain preserved and are not labeled PASS.
separate mandatory-compaction SDK fallback diagnostic.
Classic LSP could not initialize with native TypeScript 7; full repository
compiler checks passed. One nonblocking Biome info is deliberate private-key
access in the resource-footprint test, not a suppression. Existing ancillary
Anthropic text-tool-leak failures reproduce on the unchanged base and were not
modified by this PR.
Summary by cubic
Fixes
/resumeso an over-budget, rejected, or cancelled resume no longer destroys the live session, drops active/btwwork, or exits the process. The cancellable before-switch veto now runs before destination snapshot, trust, and factory preparation, exact destination SDK budget admission (including upstream mandatory resume compaction) runs before outgoing shutdown, and only an accepted switch tears down the live runtime.Lifecycle contract
Ownership protections
session_start, preserving lazy-tool activation and native diagnostics for the live session./btw, tool-search and MCP ownership, and upstream compaction admission;docs/rpc.mdnow describes the veto/admission/shutdown ordering.Written for commit 1d438ad. Summary will update on new commits.