Synced from GitHub
fix(acp): keep turn spending out of context usage#27927
Problem
ACP idle turn totals count tokens spent across model requests, but overwrote live context occupancy and discarded its window size. Long turns could therefore block provider handoffs with "Insufficient context allowance for the provider handoff". Fixes #22407.
Change
Keep usage_update as the source of context occupancy, window size, and cost. Preserve idle completion and child-session handling. This carries the original four-file fix unchanged onto current main: 3 insertions, 20 deletions.
Scope and approval
This is the eligible, narrowly scoped bug fix for #22407, per contribution triage policy cb9629e689. Current main still projects turn spending as occupancy; the existing regression assertions fail there. This replacement retains only the original correction and its two existing-test updates, as requested by Maria.
Verification
- Reproduced both regression assertion failures against current main; all 68 focused tests pass with the fix.
- Blacksmith
vp run check:changed: format, lint, both knip checks, GraphQL generation check, typecheck of 13 affected packages, and 69 tests passed. - Replayed the original captured Claude ACP frames through the actual parser and handoff budget: before
{usedTokens:94740}, budget1256; after{usedTokens:37347,maxTokens:1000000,cost:{amount:0.3145576,currency:"USD"}}, budget64000. - Two independent reviews approve the final tree with no findings.
- Unverified: a fresh live Codex-to-Claude ACP handoff through the client. No UI code changed; no browser evidence captured. Previously corrupted persisted usage remains until a new
usage_updatereplaces it.
Takeover
Replaces #22608, with thanks to otavio for the original implementation.
The original fork advertises maintainer edits, but GitHub rejected the push with HTTP 403. Compared the original diff against this replacement: no pieces dropped or moved. Author credit is preserved in the commit trailer. Original PR remains open.
Built with GPT-6.1-Sol in the Codex harness on T3 Code.
Checks
25 successful · 13 skipped
Added 1 commit
- fix(acp): keep turn spending out of context usage698e2e2
Synced from GitHub
Approvability
Verdict: Approved at 698e2e2
Macroscope's review found this PR approvable — Removes turn-spending totals from context-usage reporting while preserving dedicated usage updates and idle-state handling. The change is narrowly scoped to usage interpretation, with no evident material operational or product-policy risk.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | −29 B (−0.6%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | −29 B (−2.3%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 21.6 KiB | 21.5 KiB | −41 B (−0.2%) | 24.4 KiB | ✅ |
| Codex | Live turn messages | 2 | 1 | −1 (−50.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | 0 B (0.0%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.3 KiB | 1.3 KiB | 0 B (0.0%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.9 KiB | 21.9 KiB | 0 B (0.0%) | 24.4 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: d92c437 · PR result: 698e2e2 · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Synced from GitHub
No actionable comments were generated in the recent review.
External image Recent review info
-
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: maria/takeover-pr-22608
- File: .coderabbit.config.ts
- Commit: 698e2e2
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Advanced
- Run ID:
98268252-eeec-4080-9330-c4bb065150eb
External image Commits
Reviewing files that changed from the base of the PR and between d92c437baa19409f99416a44f748f148f4317c65 and 698e2e29976948b8833b4ba156c7f1b0ef5ee691.
External image Files selected for processing: 4
apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.test.tspackages/provider-acp/src/server/adapter.tspackages/provider-acp/src/server/runtimeModel.test.tspackages/provider-acp/src/server/runtimeModel.ts
External image Files with no reviewable changes: 2
- packages/provider-acp/src/server/runtimeModel.ts
- apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.test.ts
Limit details: You’ve used all 10 included reviews currently available.
Walkthrough
The ACP adapter continues to process usage and session-info notifications. Idle state updates no longer produce usage events, and tests now expect no usage event in that case.
Changes: ACP Usage Update Handling
Layer / File(s) Summary Usage event handling
packages/provider-acp/src/server/adapter.ts,packages/provider-acp/src/server/runtimeModel.ts,packages/provider-acp/src/server/runtimeModel.test.ts,apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.test.tsThe adapter handles usage and session-info notifications through the metadata path. The runtime model no longer emits UsageUpdatedfor idle state updates. Tests reflect the changed event and child-thread context-usage expectations.Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related issues
pingdotgg/t3code#22407— The changes retain handling forusage_updatenotifications while removing usage derived from idlestate_updatenotifications, which relates to the reported context-occupancy behavior.
Suggested labels:
bugfix,size:XSSuggested reviewers:
juliusmarminge,t3dotgg
Merge Risk: External image Minimal · up to 698e2
The change removes the incorrect idle usage event while preserving context and settlement behavior. No merge-blocking risk was confirmed.
Pre-merge checks: Passed 4
Check name Status Explanation Title check Passed The title "fix(acp): keep turn spending out of context usage" matches the main change. The changeset stops idle state_update usage totals from overwriting usage_update data, which carries context occu… Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request. Description check Passed The pull request description includes all required sections. It explains the problem, change, scope and approval basis, focused verification results, limitations, and the model and harness used. Full details: Title check
Explanation
The title "fix(acp): keep turn spending out of context usage" matches the main change. The changeset stops idle state_update usage totals from overwriting usage_update data, which carries context occupancy, window size, and cost. The title is short, uses a conventional commit prefix, and names the core behavior. It does not name the handoff failure, but it does not need to cover every detail.
Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Note
Written by gpt-6.1-sol on behalf of Maria
Verified in a real dev environment
Verified unchanged head 698e2e29976948b8833b4ba156c7f1b0ef5ee691 on Linux. Used repository Playwright with /usr/bin/google-chrome, the real dev web client, and real provider turns through the local proxy. Created the project and threads through the app. Ran main and the PR sequentially, with isolated task data and no mocks or injected protocol frames.
- Before,
github/mainate5909e53c3: a Claude ACP Haiku 4.5 turn made five separate read-only shell calls. Its lastusage_updatereported 38,593 / 200,000 context tokens and USD 0.08375305. Idle reported 229,206 spent tokens. The running server persisted{usedTokens:229206}, losing the window and cost. This reproduces the usage corruption. - After, this PR: repeated the five-call flow through Claude ACP
0.99.1. Last context was 39,655 / 200,000, cost USD 0.08376335; idle spending was 235,729. The server retained 39,655 / 200,000 plus cost after completion. - Client display: a fresh ACP-only thread completed two real shell calls. Idle spending was 65,888, while persisted context stayed 33,039 / 200,000. The web tooltip showed 33k of 200k tokens used.
- Nearby behavior: native Codex
gpt-6.1-sol, Codex ACP2.5.0, Codex-to-Claude ACP handoff, Claude ACP-to-native Codex handoff, and a native Codex follow-up all completed through the composer. The PR's return handoff reachedreadywith deliveryinjectedand returnedreturn handoff ready.The follow-up returnedfollow-up verified. - Checks: both changed existing test files passed on Blacksmith, 68 tests. Existing head CI reports 31 successful, 0 failing, 0 pending. No source changes or pushes were needed.
Sanitized command and protocol transcript
ACP-only thread retains the correct 33k of 200k context after completion
Real native Codex follow-up after the Claude ACP return handoff
Inspected the screenshot and exported recording. The inline recording is an animated GIF for GitHub compatibility; its MP4 export passed playback and seeking checks in T3's browser player.
Not verified
- The exact
Insufficient context allowance for the provider handofferror did not occur in this run. The underlying incorrect usage snapshot reproduced on main and was corrected on the PR; live handoffs completed. - Native desktop shells, iOS/Android, other ACP implementations, and real ACP child-session usage were not exercised. The existing adapter tests cover child-session handling.
- In the mixed-provider thread, the context tooltip could prefer an older native Codex turn count. That selection logic is unchanged by this PR. Verified the ACP-only tooltip and persisted ACP snapshots separately; this PR does not resolve that separate display behavior.