fix(supervise): honor Pi RPC terminal semantics - #660
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 460bd667
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-07-30T07:07:59Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 207.8s (2 bridge agents) |
| Total | 207.8s |
💰 Value — sound
Fixes the Pi adapter so only agent_settled ends a session, the deliverable comes solely from the terminal assistant turn, usage is counted once, and unknown dollar cost fails closed — a critical correctness fix built in the codebase's grain.
- What it does: Reworks the Pi 0.83 RPC adapter's terminal/accounting logic across 9 files: (1) treats only
agent_settledas idle, keepingagent_endintermediate so Pi can auto-retry/compact; (2) correlates everypromptto its RPCresponseby id, so a stale buffered settlement can't end a session after a newer prompt was accepted and a rejected prompt fails instead of hanging; (3) builds the result artifa - Goals it achieves: Data-integrity and accounting correctness. The root cause (confirmed in the PR body against a real run): 2/2 workers ended with assistant stopReason:'error' yet both were journaled as
done, with persisted blobs exactly matching the 3,765-char user prompt and a 45,915-char tool result — because the old adapter used genericmessage_endtext as output and stopped atagent_end. Usage was also do - Assessment: Strong, coherent fix that matches the codebase's load-bearing principles exactly: conservation (
total≡free+reserved+committed), fail-closed admission, content-addressing only real output, and honest 'unknown means unknown, not zero' accounting. It extends the EXISTING accounting pipeline (UsageEvent→Spend→foldStream→readWorkerProgress→scope journal) with one optionalusdKnownfield rather than - Better / existing approach: none — this is the right approach. I searched for an existing RPC-response-correlation helper or a second JSON-RPC-over-stdio adapter to reuse (git grep for awaitingResponse/pendingResponse/inflight across src/, and for agent_settled/--mode rpc). The pi-executor is the ONLY line-delimited JSON RPC adapter in the codebase; the other executors (router/inline = direct HTTP, sandbox = composes runAgen
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
A correctness fix to a live, registered executor that stops provider failures being journaled as successful deliverables, threads unknown-dollar accounting through the established Spend/UsageEvent pattern, and handles Pi 0.83's terminal/race semantics; no dead surface, no competing pattern.
- Integration: Fully reachable.
piExecutoris registered increateExecutorRegistry()(runtime.ts:1672) and routed bycreateExecutor({backend:'pi'})(runtime.ts:1630-1631); the 'pi' harness is selected in candidate-execution/system-prompt.ts:75 and exercised through supervisor-agent.test.ts:123. TheusdKnownpropagation threads through the established Spend/UsageEvent channel consumed by budget pool, scop - Fit with existing patterns: Follows the codebase's grain precisely. The pi executor implements the open
Executorinterface (types.ts:90) — the documented extension point — and this change only refines its internals. TheusdKnown?: falseon the UsageEvent cost variant extends a conventionworktree-cli-executor.ts:186andtypes.ts:259(Spend.usdKnown) already established before this PR. No competing pi executor exists; - Real-world viability: Targets the real hard edges, not just the happy path. The settlement race (older buffered
agent_settledracing a newer accepted prompt) is closed byacceptedPromptSinceSettlement(pi-executor.ts:284-285,329). Prompt rejection that previously hung now fails explicitly via RPC response correlation (pi-executor.ts:276-291). Abort gets a bounded 2s receipt drain before AbortError (pi-executor.ts:3 - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What changed
agent_settledas idle.agent_endremains intermediate so Pi can run its own retry and compaction behavior.promptwith the Pi 0.83 RPC response. A settlement older than a newly accepted prompt cannot end the session, and a rejected prompt now fails explicitly instead of hanging.turn_end. User messages and tool results can never become deliverables; terminal provider errors and aborts become typed failures.AbortError.turn_end, including cache traffic in Runtime input, while preserving unknown subscription dollar cost throughUsageEvent,Spend, the scope journal, and public worker progress.toolCallId, including start arguments, end status, result, error, and timing.Root cause
Pi 0.83 emits
message_endfor user, assistant, and tool-result messages. It may emitagent_endwithwillRetry: true; onlyagent_settledsays the session is actually idle. The old adapter used generic message text as output and stopped atagent_end, so a provider failure could be journaled as a successful deliverable containing either the original prompt or a tool result.The raw triggering run showed this directly: 2 of 2 workers ended with assistant
stopReason: "error", yet both were journaled asdone; the persisted blobs exactly matched a 3,765-character user prompt and a 45,915-character tool result. Pi tool correlation itself was sound and remains intact.A full-suite run also exposed an idle-boundary race: a delivered follow-up could be written just before an older buffered
agent_settledwas consumed, allowing that older event to end the session first. Pi request IDs and prompt acceptance responses are now the ordering barrier.Regression proof
agent_settledordering.GIT_ALLOW_TEST_IDENTITY=1 pnpm test: 1,876 of 1,876 runnable tests passed; 6 intentionally skipped.pnpm run lint: 502 files checked, 0 errors.pnpm run typecheck: package and examples passed.pnpm run docs:check: generated API docs and freshness checks passed.pnpm run verify:packageandpnpm run verify:primeintellect: passed.Boundaries