fix: propagate token usage from ACP provider responses#8181
fix: propagate token usage from ACP provider responses#8181xr843 wants to merge 1 commit intoblock:mainfrom
Conversation
The ACP client provider was discarding token usage data from PromptResponse, causing all-zero token counts for ACP-based providers. This broke cost reporting and context window monitoring. Three targeted changes: 1. Extend AcpUpdate::Complete to carry Option<AcpUsage> 2. Forward r.usage when sending the complete signal 3. Map ACP Usage to ProviderUsage and yield it on stream completion Agents that don't populate PromptResponse.usage (like codex-acp) degrade gracefully — r.usage is None and the existing fallback applies. Closes block#8132 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> Signed-off-by: Tim Ren <[email protected]>
xr843
force-pushed
the
fix/acp-provider-token-usage
branch
from
bae1624 to
f7102fe
Compare
![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: f7102fe3be
ℹ️ 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".
| input_tokens: Some(u.input_tokens as i32), | ||
| output_tokens: Some(u.output_tokens as i32), | ||
| total_tokens: Some(u.total_tokens as i32), |
There was a problem hiding this comment.
Casting ACP u64 usage values with as i32 can silently wrap for large counts, producing negative or otherwise corrupted token metrics. This affects downstream cost/context accounting whenever an ACP adapter (or a misbehaving endpoint) reports values above i32::MAX, and the bad values are then persisted and displayed as if valid. Use a checked/saturating conversion (for example, clamp to i32::MAX) before constructing Usage.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #8132 — ACP client provider discards token usage data from
PromptResponse, causing all-zero token counts for ACP-based providers (claude-acp, codex-acp). This breaks cost reporting and context window monitoring.Changes
Three targeted changes in
crates/goose/src/acp/provider.rs:AcpUpdate::Completeto carryOption<AcpUsage>alongsideStopReasonr.usagewhen sending the complete signal from the prompt response handlerUsagetoProviderUsageand yield it as the final stream item before breakingGraceful degradation
For ACP agents that don't yet populate
PromptResponse.usage(e.g. codex-acp, goose-acp server),r.usageisNone, noProviderUsageis yielded, and the existing"unknown"fallback incollect_stream()applies unchanged. As those adapters add usage reporting, it starts working automatically.Notes
AcpUsagefields areu64, Goose'sUsageusesOption<i32>— theas i32cast is safe for practical token countsAcpUsagealso has optionalthought_tokens,cached_read_tokens,cached_write_tokenswhich Goose'sUsagedoesn't model yet — these could be surfaced in a follow-upCredit to the issue reporter for the thorough root cause analysis and proposed fix.
Generated with Claude Code