PiLoop seam: abort() + typed event stream (M1-K5a) - #1
Merged
Conversation
…nt stream (M1-K5a) Unblocks the real Pi adapter and the deferred grants $7.3 hard budget layer without building either. abort() is cooperative Promise<void> matching Pi 0.80.10 ground truth: in-flight tools are awaited, idle aborts resolve immediately, repeats are idempotent, and the in-flight turn resolves ok: true / exitReason "aborted" — an aborted run is durably finalized (synthetic stopReason), so the resume cursor advances; the event-log $5.2 poisoned-cursor guard stays scoped to failed turns. onEvent carries our own union (tool_start/tool_end/settled for the budget watcher; message_start/delta/end + tool_progress for the surface $7 stream bridge), never Pi's raw event types; each admitted variant names its consumer and each exclusion its reason. Sync fan-out in subscription order; a throwing subscriber is swallowed (same rule as the onActivity push seam). FakePiLoop implements both and auto-emits around dispatch; Session gains abort() and an oracle proving aborted-turn checkpoint semantics against the real consumer. jsonl.test's helper now awaits loop.start — the fake defers a microtask like the real async prompt(), so nothing may rely on synchronous JSONL writes. biome now skips src/surface/generated (the DO-NOT-EDIT emitted module tripped format checks; generated code is guarded by its generator and the round-trip oracle, not the linter).
…odel/route/compactions/retries
Session.abort() was pure delegation to loop.abort(), so a foreground
session parked in awaiting_approval hung forever on abort: the dispatch
promise never settled, the loop never reached idle, and the task stayed
waiting_for_user with an unpaired approval.requested — a silent
divergence from grants §6.1 ('cancelled fires on: session abort … a
parked call cannot outlive its session') and the §10.19 oracle.
ApprovalMachine.cancelAll now, per parked record: appends
approval.resolved {decision: cancelled, decidedBy: abort} (the row shape
recovery.ts already emits for the restart producer), drops the record,
and settles the held promise as the §6.1 error-result-to-model arrow.
One task.resumed closes the task.waiting the first park opened so the
aborted turn's accounting rows fold from a coherent state. Session.abort
cancels FIRST, then awaits loop.abort() — the reverse order would still
deadlock, since the cooperative abort resolves only at idle.
Deleting before settling is what makes §10.19's late-decision clause
hold: a decision for a cancelled toolCallId hits resolve()'s unknown-id
guard, is stale-logged, executes nothing, and mints nothing.
The piloop.ts/runtime.ts contract comments had codified the hang as
intended behavior; rewritten so the real Pi adapter isn't built against
a spec-wrong contract. SPEC-NOTE in cancelAll flags the one amendment
question: event-log §3.1's decidedBy vocabulary lacks a tag for the
abort producer grants §6.1 requires — 'abort' extends it rather than
mislabeling a live abort as the boot recovery pass.
Oracle: abort.test.ts pins §10.19 end to end — cancelled row appended,
promise settles as error, pendingApprovals empties, abort() resolves
(raced against a timeout so a regression fails loudly instead of
hanging the suite), late decision ignored with no ledger or store
writes.
…(K5a fixer 2) Correctness fixes from the two K5a adversarial reviews: - FakePiLoop's concurrent branch now emits tool_end in completion order (per-call .then()), not source order — the double was silently safer than real Pi, which inverts against a watcher that pairs start/end by index. - runTurn refuses re-entry synchronously, before touching `inFlight`, so a second overlapping turn can never clobber the identity the .finally guard depends on; driveTurn also refuses re-entry as a backstop. - abort() now drains the queued-steer table (mirrors Pi's clearQueue()), so a steer queued mid-turn can't fire a budget-gate-skipping turn after the abort. `steered` stays as the append-only audit record. - onEvent listeners are now token-keyed (Map) instead of a Set<listener>, so two subscriptions of the same function get independent unsubscribes. - Added a fan-out subscription-order oracle (previously unpinned). - Renamed tool_progress -> tool_update and added messageId to the message_start/delta/end variants (surface-protocol §7), documenting that the adapter mints and holds the id stable since Pi's AssistantMessage has none. Comment corrections (rationale was misleading the future adapter author, not the behavior): - The compaction/auto_retry exclusion cited a stats field (model/route/compactions/retries) that real SessionStats does not have — corrected against specs/recon/pi-types-0.80.10.md. - Documented that Pi binds tool execution at session construction, not per-turn, and that the seam's Dispatch is a mutable current-dispatch swap that only works because turns never overlap. - Added PiLoop.dispose() (interface + fake only) and stopped calling abort() "teardown" in both piloop.ts and runtime.ts. - Flagged the followUp() name collision with Pi's queueing method. - Reworded abort()'s idempotency/never-rejects clause as an adapter obligation, not a fact Pi documents. - Corrected budget.ts's SPEC-NOTE: budgetGate() has no bypass today, so the DESIGN §4 salvage call would be dropped, not "land with the adapter" for free. - Noted that FakePiLoop's settled fires once per turn while real agent_settled does not fire between a turn and a queued follow-up. Gate: dropped the biome.json generated-code exclusion (it silenced lint and assist, not just formatting) and instead run gen-protocol.mjs's TS output through biome's own formatter before writing it, so the committed file stays under the same gate as hand-written code. Regenerated core/src/surface/generated/protocol.ts through the new formatting step; parity test and self-test both still pass.
…rnResult naming, idle-steer hole) - Fix an unhandled rejection in the concurrent dispatch branch of FakePiLoop.driveTurn: the per-call .then() had no onRejected, so a rejecting dispatch (decideCall()/append() can both throw) leaked an unhandled rejection on the derived promise even though Promise.all still awaited and propagated it correctly. Regression test asserts the turn still rejects and zero unhandledRejection events fire. - Rename TurnResult.ok to checkpointSafe across every producer/consumer (piloop.ts, runtime.ts, budget.ts, FakePiLoop, all session tests) so the field name carries what it means (a durable, checkpoint-safe cursor - not "the model succeeded"); shrink the justifying comment now that the name does the work. Narrow exitReason to a closed ExitReason union and export it. - Close the steer-while-idle hole: document on PiLoop.steer that steering is only legal while a turn is in flight (an idle steer would mint a turn through real sendUserMessage that skips budgetGate() and the audit trail entirely - the same hole the abort/clearQueue fix closed for queued steers, left open for idle). FakePiLoop.steer now throws when idle instead of shelving the steer inertly. Tests pin both the idle-throw and that mid-turn steering still queues as before. - Comment coherence: reword runtime.ts's checkpoint comment to the checkpoint-safe framing (aborted included, failed excluded); rescope the settled-undercount warning to the hypothetical it actually is under this seam's own rules; delete the unreachable driving guard in driveTurn (runTurn already refuses re-entry synchronously before it would run) rather than keep two guards for one invariant. - Add invariant 14 (turn != ambient) to INVARIANTS.md covering the steer-idle hole, queue drain, and the future salvage bypass, guarded by the new steer-while-idle contract test plus the existing abort-drains-queue test.
…sk.resumed cause) grants §6.1 mandates that session abort produce cancelled approval rows, but §3.1's decidedBy vocabulary was a closed list with no tag for that producer, and task.resumed — the only legal transition out of waiting_for_user — reads as "the user answered" when an abort ends the wait. Both amendments approved 2026-07-19 after the K5a seam review; cause is optional so every existing row stays valid. Code and the abort oracle now cite the amendment instead of flagging the gap.
… in session/ Row 14 named three guards, none of which covered its main sentence: nothing stopped a future module from holding the loop and calling start/followUp directly, minting a turn that skips budgetGate on the way in and finishTurn on the way out. Confining the seam's import to core/src/session/ makes that drift fail the gate instead of passing it. Three self-test cases; 10 rules now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extends the
PiLoopseam so the real Pi adapter and the deferred grants §7.3hard budget layer can be built next. Interface, test double, and oracles only —
no adapter code.
Doing this before the adapter was deliberate: the seam is the shape every later
consumer codes against, and it is cheap to change now and expensive later.
What lands
abort(): Promise<void>— cooperative, matching Pi 0.80.10 exactly:in-flight tools are awaited rather than discarded, idle aborts resolve
immediately, repeats are idempotent. The aborted run is durably finalized, so
its turn is checkpoint-safe and the resume cursor advances; the event-log §5.2
poisoned-cursor guard stays scoped to failed turns.
onEvent(listener) => Unsubscribe— a 7-variant union of our own types,never Pi's raw
AgentSessionEvent(Pi minors are breaking; the wrapper is theinsulation).
tool_start/tool_end/settledserve the §7.3 budget watcher;message_start/message_delta/message_end/tool_updatemap 1:1 ontosurface-protocol §7's stream bridge. Every admitted variant names its
consumer; every exclusion names its reason.
dispose()— teardown is a distinct operation from abort, as it is in Pi.a session parked in
awaiting_approvaldeadlocked — the parked dispatchpromise never settled, so the loop never reached idle — and left
approval.requestedwith no partner row.FakePiLoopimplements all of it, andSessiongainsabort()so theseam is proven against its real consumer rather than only the double.
Spec amendment (reviewed and approved 2026-07-19)
event-log.md§3.1 gains two members. grants §6.1 mandates that session abortproduce
cancelledapproval rows, butdecidedBywas a closed list with no tagfor that producer, and
task.resumed— the only legal transition out ofwaiting_for_user— reads as "the user answered" when an abort ended the wait.decidedBygainsabort;task.resumedgainscause?: answered|cancelled.causeis optional, so every pre-amendment row stays valid.INVARIANTS.mdgains #14 turn ≠ ambient: every Pi run is driven throughSession.runTurn/followUp, so no loop-, queue-, or steer-originated turn canexist outside the budget gate and the audit trail.
Also in scope
corerequires ≥22 (Pi needs ≥22.19) — bumped, so thegate actually matches the engine the sidecar runs on.
scripts/gen-protocol.mjsnow formats its emitted TypeScript, so generatedprotocol types stay fully under the lint gate instead of being excluded from
it. Verified formatting-only: a renamed schema field still produces 36
differing lines through the same formatter, so invariant 9 still catches real
drift.
specs/recon/pi-types-0.80.10.mdtranscribes the SDK's actual.d.ts,including the four fields
getSessionStats()does not provide(
model,route,compactions,retries) — the adapter must supply allfour itself.
Oracle
336 tests (up from 317). New oracles pin: cooperative abort ordering, abort
idempotency, aborted-turn checkpoint semantics, the §10.19 cancel-on-abort
sequence,
tool_endcompletion ordering, turn re-entry refusal, queue drain onabort, subscription-order fan-out, throwing-subscriber isolation, token-keyed
subscriptions, and the idle-steer refusal that guards invariant 14.
Review found four defects that tests alone did not: a spec violation written
into the seam's own contract comments, a double that was safer than real Pi on
tool_endordering, an unhandled rejection introduced by a fix, and theidle-steer hole. Each is closed here.