Show context window usage for ACP clients - #1283
Conversation
|
🚨 SLOP COP 🚨 · I am the SlopCop. I am now reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5
BB now shows how full an ACP agent's memory box is. It works like a fuel gauge.
One restore path can show the old box level after BB starts a new box.
Finding
P2 — A failed session restore can publish stale context usage.
The bridge now forwards usage while session/load is still pending. If the load fails, BB starts a fresh session.
The old usage event remains on the timeline. The UI can then show the old session's usage for the new session.
The bridge also parses sessionId, but it does not check that value. I left one inline comment with the recommended fix and tests.
Other review results
- Security: I found no data leak, command injection, or permission problem.
- Code quality: The adapter validates the new fields and rejects malformed values.
- Architecture: The code uses the existing context-usage event path. The provider implementations have different source data, so a shared helper adds little value.
- Performance: Each update adds one small event. I found no new unbounded loop or expensive hot-path work.
- Protocol: The change correctly increases
HOST_DAEMON_PROTOCOL_VERSIONfrom 94 to 95.
End-to-end check
I started the source app and ran a real OpenCode ACP thread.
OpenCode reported 11,460 used tokens from a 200,000-token window. The UI showed 6% used and 94% left.
Checks
- ACP adapter and bridge: 95 tests passed.
- Host daemon contract: 35 tests passed.
- Server event ownership: 8 tests passed.
- GitHub CI: all required checks passed.
git diff --check: passed.
I posted this as a comment only. I did not approve the pull request or request changes.
|
🚨 SLOP COP 🚨 · I am SlopCop. I am starting the review now. I will check security, code quality, architecture, performance, and the main user path. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: ACP agents can now tell BB how much model memory they use. BB shows that value in the existing context meter.
I found no actionable findings.
- Security: The code validates usage data at the ACP boundary. It also rejects updates for a different restored session.
- Code quality: The split is clean. The wire layer validates, the adapter normalizes, and the domain defines event scope.
- Architecture: I found no stale names, material duplicate logic, or useful refactor for this change.
- Performance: Resume keeps one latest usage snapshot. Normal updates remain small, and existing event pruning bounds storage.
- Compatibility: The daemon protocol increases from 94 to 95 for the new cross-process behavior.
I tested exact SHA 361acba8. The focused ACP, contract, server, and type checks passed. All GitHub checks also passed.
I also ran a live OpenCode ACP thread through the exact-sha dev server. The browser showed 6% used and 11k / 200k tokens.
One small test opportunity remains. A future test could cover session/load through runtime identity and timeline storage in one automated path.
## Summary - parse stable ACP `usage_update` notifications - translate exact `used` and `size` values into BB context-window events - preserve usage reports during session restore and before the first turn - increment the host-daemon protocol version for the new event behavior ## Root cause The ACP bridge forwarded `usage_update` as an untyped session update. The adapter classified it as noise, so the server never received a context-window event. The bridge also discarded all session updates during `session/load`. ## Validation - reproduced the issue with a failing ACP adapter test: valid usage returned no events - ACP, domain, and host-daemon contract tests pass: 1,032 tests - server tests pass: 1,391 tests - focused server ingest and timeline test passes - affected package type checks pass - host-daemon and server builds pass Closes get-bb#1111
Summary
usage_updatenotificationsusedandsizevalues into BB context-window eventsRoot cause
The ACP bridge forwarded
usage_updateas an untyped session update. The adapter classified it as noise, so the server never received a context-window event. The bridge also discarded all session updates duringsession/load.Validation
Closes #1111