Add manual context compaction across providers - #1152
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. I will post one final review after the parallel checks finish. |
9a63de9 to
041b652
Compare
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: This change adds a context clean-up command for several providers. Some providers finish incorrectly, block other work, or expose the command too early.
I reviewed the requested SHA 9a63de9699e8a5beea001a336df9fb6fedcb87eb. The pull request later moved to 041b652e, but these code paths remain unchanged.
I found these defects:
- [P1] Non-Codex compaction takes a shared provider write lane. A stalled request can block unrelated sessions for 20 minutes.
- [P1] ACP prompt compaction hides tool activity but still permits file reads and writes.
- [P2] The daemon does not retain the runtime during compaction.
- [P2] Claude returns success when the SDK consumes
/compact, before compaction completes. - [P2] Pi emits
thread/compactedfor aborted and failed compactions. - [P2] New-thread composers offer
/compactbefore a thread exists. - [P2] Invalid mention ranges can replace a normal prompt with compaction.
- [P3] Failed UI compactions refresh broad, unrelated query groups.
The implementation also repeats compact detection and draft recovery in four UI paths. One shared action can reduce this duplication.
The provider adapters use different completion rules. A shared terminal contract and the existing compaction lifecycle can prevent these differences.
I ran the branch dev app and used the browser command surface. Codex returned 200, showed compaction, and reduced context use from 7% to 2%.
The source CLI also compacted the same thread. Ten changed packages passed type checks, and 129 focused tests passed.
The full local run passed nine package tasks. One unrelated machine-install test failed, while GitHub checks passed.
dc6f9f8 to
203228a
Compare
|
Review triage complete at 78396d4.
For the two general observations: compact detection is now centralized in isStandaloneBuiltinCompactCommand and shared by both submission controllers. Draft ownership remains controller-specific because the full thread composer and plugin/embedded composer have different persistence and callback lifecycles; combining those controllers is outside this feature. Provider-specific signals are already normalized into the shared compaction lifecycle, while command settlement consistently means accepted rather than terminally completed. Verification on the new tip: all affected typechecks pass; full suites pass for domain (129), agent-runtime (840), host-daemon (493), and app (2,419). |
78396d4 to
c8a320c
Compare
Summary
bb thread compact/compactcontrol through ACPpnpm dev:stopfor stopping this worktree's dev server and desktop processesProvider behavior
/compactcommand/compactmaintenance prompt/compressis not an ACP compaction controlRequest and lifecycle semantics
The compact endpoint returns after the provider accepts the maintenance operation; it does not wait for the provider to finish. The provider bridge emits the start event before acknowledging OpenCode/ACP, and both ACP and Pi report terminal status through their existing compaction lifecycle events. This keeps the HTTP result and timeline from contradicting each other when a provider rejects or interrupts work after it has started.
The composer therefore uses its normal submit behavior: accepted
/compactactions stay cleared, while genuine pre-acceptance failures restore the draft. The UI and CLI no longer claim that compaction completed based only on the request response.This branch does not summarize or reseed provider sessions.
Validation
/compactflowDiff shape after cleanup
The current diff is 2,204 additions / 119 deletions across 81 files:
The cleanup removed the unused public provider capability, the alternate compaction-through-threads.send path, repeated test setup, a stale 20-minute request timeout, and two-boolean ACP prompt state. The ACP bridge now uses one explicit turn / compaction / idle state.
Fixes #1103