Skip to content

fix(supervise): honor Pi RPC terminal semantics - #660

Merged
drewstone merged 4 commits into
mainfrom
fix/pi-rpc-event-accounting
Jul 30, 2026
Merged

fix(supervise): honor Pi RPC terminal semantics#660
drewstone merged 4 commits into
mainfrom
fix/pi-rpc-event-accounting

Conversation

@drewstone

@drewstone drewstone commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Treat only agent_settled as idle. agent_end remains intermediate so Pi can run its own retry and compaction behavior.
  • Correlate every prompt with 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.
  • Build the result only from the terminal successful assistant turn_end. User messages and tool results can never become deliverables; terminal provider errors and aborts become typed failures.
  • Reject pre-aborted work before launch, and on active cancellation drain the final Pi receipt for up to two seconds before returning AbortError.
  • Count each assistant receipt once at turn_end, including cache traffic in Runtime input, while preserving unknown subscription dollar cost through UsageEvent, Spend, the scope journal, and public worker progress.
  • Close dollar-capped admission after unknown cost rather than treating an unknown subtotal as zero.
  • Preserve Pi tool correlation by toolCallId, including start arguments, end status, result, error, and timing.

Root cause

Pi 0.83 emits message_end for user, assistant, and tool-result messages. It may emit agent_end with willRetry: true; only agent_settled says the session is actually idle. The old adapter used generic message text as output and stopped at agent_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 as done; 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_settled was consumed, allowing that older event to end the session first. Pi request IDs and prompt acceptance responses are now the ordering barrier.

Regression proof

  • Fixture matches the Pi 0.83 RPC event and response shapes for user, assistant, parallel tool-result, retry, terminal error, prompt rejection, successful empty output, pre-abort, delayed abort receipt, clean abort exit, prompt correlation, and agent_settled ordering.
  • Focused runtime and kernel tests: 76 of 76 passed.
  • Formerly flaky idle-boundary test: 12 of 12 concurrent stress runs passed.
  • 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:package and pnpm run verify:primeintellect: passed.

Boundaries

  • Runtime has only input and output token channels, so Pi cache reads and writes remain conservatively folded into input.
  • The process fixture pins the observed Pi 0.83 wire. This correction did not spend another paid live session.

@drewstone drewstone changed the title fix(supervise): preserve Pi RPC receipts fix(supervise): honor Pi RPC terminal semantics Jul 30, 2026
@drewstone
drewstone marked this pull request as ready for review July 30, 2026 07:07

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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_settled as idle, keeping agent_end intermediate so Pi can auto-retry/compact; (2) correlates every prompt to its RPC response by 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 generic message_end text as output and stopped at agent_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 optional usdKnown field 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. piExecutor is registered in createExecutorRegistry() (runtime.ts:1672) and routed by createExecutor({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. The usdKnown propagation 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 Executor interface (types.ts:90) — the documented extension point — and this change only refines its internals. The usdKnown?: false on the UsageEvent cost variant extends a convention worktree-cli-executor.ts:186 and types.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_settled racing a newer accepted prompt) is closed by acceptedPromptSinceSettlement (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.

value-audit · 20260730T071141Z

@drewstone
drewstone merged commit bcc6239 into main Jul 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants