feat(sdk): C04 terminal abort with turn|owned scope - #21
Open
snowykr wants to merge 30 commits into
Open
Conversation
The approved abort-SDK plan extends turn.abort with optional terminal mode
and a caller-selected scope (turn|owned). Dispatch previously dropped C04
input entirely; this lands the strict, side-effect-free validation layer:
terminal mode accepts only mode/scope fields, requires a nonempty envelope
idempotency key (<=128 UTF-8 bytes), rejects force/timeout/unknown fields
with invalid_input, and forwards normalized input to a new private
abortTerminal surface hook. Legacy {} and mode:"turn" keep the argument-less
abort path byte-for-byte compatible.
Terminal semantics per the reconciled plan: stop the root worker's current
turn and block only its own continuation routes; left-running owned work's
completions are delivered normally so the root worker can resume with a
fresh attempt (owned delivery is intentionally NOT suppressed).
Lore-id: c04-terminal-contract
Constraint: legacy C04 {} / ACP cancel() must stay key-optional and unchanged
Constraint: terminal validation must run before any surface side effect
Rejected: reusing watchJobs/acknowledgeDeliveries as a turn fence | broad identity, no origin
Tested: 19/19 sdk-control-dispatch tests incl. keyless/oversized-key/invalid-mode/scope/unknown-field rejections and legacy passthrough
Not-tested: bus-level terminal transaction, lineage/fence, owned settlement, queue/gate
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 704b618f5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Terminal abort needs a durable owner before any fence/stop/event effect. The v2 full-document store persists bounded terminal-scope records (selection, continuation fence, policy, dispositions, response state) through the single serialized owner alongside prompt/skill records; v1 documents migrate on load, malformed terminal scopes are quarantined, and incomplete pending scopes settle to safe uncertainty on restart. Covers the C04 terminal dispatch input contract (strict validation, canonical scope, required bounded key) and store persistence with migration tests. Lore-id: c04-terminal-persistence
New module session/terminal-abort.ts holds the private machinery for C04 terminal abort with the corrected semantics: - DeliveryOrigin classification: turn-continuation (same-turn retry, TTSR/ agent.continue, steering, hidden-next-turn, maintenance/worker successor, pre-close predecessor) vs owned-completion (left-running background Bash/task/detached subagent) vs ordinary. Classification is causal, never timing-based; missing/mismatched metadata fails closed. - TurnContinuationFence lifecycle open -> closing -> closed -> retained -> released, closing synchronously before the root turn is interrupted. - TurnContinuationGate: post-close same-turn continuations are denied (only pre-close linearized predecessors pass); owned completions stay ALLOWED after close as fresh-turn delivery (the user-corrected resume-on-owned- completion semantics — the stage-04 no-successor fence is not reinstated). - OwnedDeliverySettlementObserver shape (owned scope only, six manager paths), bounded terminal_uncertain reason union, monotonic fresh-attempt epoch, and terminal scope id minting. Tested: 53/53 across dispatch/reconciliation/kind-aware/arbiter/terminal-abort suites in the worktree; package biome + tsc clean.
…eams Terminal abort (C04 mode:"terminal") must classify background work by causal source, never by timing. This wires the lineage/attempt authority from sequencing step 3: each prompt turn mints an opaque lineage id bound to the attempt epoch (#promptGeneration) before the model runs; beforeToolCall binds that lineage to each tool call id; task and Bash registrations then record the exact owned five-tuple (endpoint generation, lineage hash, attempt epoch, job id, job generation) synchronously before the job handle escapes, so a later completion can be attributed to the turn that spawned it. Registries are bounded (1024 terminal scopes, 8192 registrations and lineage bindings) and evict oldest; missing or mismatched lineage fails closed and never breaks ordinary registration. Bindings intentionally survive the tool call so resumed registrations re-using the original tool call id retain the same owned-completion origin (post-close resume stays runnable/monitorable under turn scope). Lore-id: c04-terminal-lineage Constraint: classification is source/lineage-based, never timing-based Constraint: fail closed on missing/mismatched context; no session-current fallback Rejected: mutable session-current lineage lookup | forgeable and stale Tested: 13/13 terminal-abort suite; control-dispatch/reconciliation/yield-queue/agent-session/task/bash suites still green Not-tested: SDK callback/YieldQueue/AgentSession origin consumption (sequencing step 4+)
Sequencing step 4: the SDK onJobComplete callback now recovers the immutable origin BEFORE formatting or artifact allocation. classifyOwnedCompletion resolves an exact owned-completion only when the job carries a registered five-tuple AND a terminal scope exists for its turn; missing/mismatched metadata fails closed to an ordinary delivery. The private OwnedCompletionEnvelope rides the plain async-result message boundary (never a public field), and the AgentSession streaming/idle injectors route envelope-carrying deliveries through resumeFromOwnedCompletion, which allocates a fresh prompt attempt epoch and lineage before the existing followUp/prompt call. Mandated boundary comments now exist at the sdk/session.ts callback, the yield-queue stale/build boundary, and both AgentSession injectors, stating that turn-scope abort blocks only turn-origin continuations and intentionally allows left-running owned completion to resume the agent. Lore-id: c04-terminal-origin-delivery Constraint: origin recovered before formatting/artifact allocation Constraint: closed terminal record never makes an allowed owned completion stale Rejected: blanket delivery gate at injection | would suppress left-running owned results Tested: 15/15 terminal-abort suite; yield-queue/async-yield-queue/agent-session/sdk-lifecycle/sdk-session suites green Not-tested: bus-level terminal scope registration feeding classifyOwnedCompletion (sequencing step 6/7)
…nd continuation gate Wires the C04 terminal abort surface to the durable prompt terminalization: the turn's continuation fence is registered and synchronously closed at abort via the session seam, and the attempt epoch advances so the fence can never leak onto later turns. Same-turn continuations (retry/TTSR/steering/hidden-next-turn/maintenance/worker successor) are denied at the final synchronous boundary; no-active-turn and unfencible paths return process-local no-effect / safe uncertainty. Lore-id: c04-terminal-surface
The approved plan's hard documentation gate requires the design note, the naming rules (TurnContinuationFence blocks turn-origin continuation only; owned-completion delivery is never suppressed), boundary comments, and a reviewer/implementer checklist. The boundary comments are already in place at sdk/session.ts, yield-queue.ts, and both AgentSession injectors. This ADR records the corrected turn semantics (prior no-successor fence was a misunderstanding), the prohibited blanket-suppression names, the implementation state, and the five mandatory review questions. Lore-id: c04-terminal-docs Constraint: any code/test/review text implying closed-turn owned-delivery suppression is a hard blocker Tested: package check + docs index regeneration sync
The bus terminal abort now persists the bounded DurableTerminalScopeRecord through the v2 reconciliation store's single full-document owner (transactTerminalScopes), idempotent per selection+epoch, carrying the turn dispositions, retained continuation fence (epoch + tombstones), enabled/disabled owned-completion policy, pending response state, and a deterministic payload hash. The session's abortPromptAndWait returns the registered scope info (scopeId/epoch/lineage) so the bus can write the record with the exact aborted epoch; terminalizePrompt captures the fence proof through an out-param (an explicit closure return type triggers a TypeScript narrowing regression in this file, so the capture avoids it). Plan AC 5 no-store gate: terminal abort without a file-backed reconciliation owner returns terminal_no_effect before any fence/stop/ cleanup. A failed scope-record write after a landed prompt terminal fails closed to terminal_uncertain(worker_unsettled). Lore-id: c04-terminal-durable-record Constraint: single full-document owner; idempotent per selection+epoch Constraint: no-store precedes any destructive terminal work Tested: 80/80 sdk-host-wiring (tests now provide a file-backed session to reach the fence path); 33/33 store/terminal/chain suites; package check clean Not-tested: restart hydration replay of the durable record; owned exact cleanup (step 6)
…metadata scope:"owned" stops only generation-exact captured jobs, waits a fixed grace, re-proves quiescence (second proof), and purges their queued deliveries so stopped work can never resume the agent; unprovable or foreign work yields safe uncertainty, never a claimed stop. The correlated agent_end carries bounded terminal metadata (scope, turn, ownedWork, automaticDelivery, resumeOnOwnedCompletion) with no extra terminal event kind. Lore-id: c04-terminal-owned-stop
…ation authority Same-key same-input terminal requests replay deterministically (dispatch LRU plus durable key/input hashes) without re-running the stop, cleanup, or a second event; same-key different-input conflicts. The attempt epoch advances with every terminal scope so the fence is bounded to the aborted turn. Owned cleanup is generation-verified (reused job ids fail closed) with settleOwnedWork unit tests, and the owned-completion gate requires the exact registered five-tuple (forged/unregistered tuples denied). Lore-id: c04-terminal-replay-bounds
AgentSession injectors drop owned-completion deliveries the scope gate denies (owned scope policy disabled, forged/unregistered tuple, vanished scope) before followUp/prompt, so stopped work can never call the agent; batch build partitions denied entries out entirely so a mixed batch never leaks a stopped-work delivery. Generation is revalidated in the quiescence proof and same-key scope changes conflict durably. The ADR implementation-state now reflects the completed work. Lore-id: c04-terminal-batch-split
…tcome The bounded initial marker is persisted BEFORE any fence/stop/event effect (marker failure is process-local no-effect, AC 10), then CASed to the semantic outcome. The host classifies the control-response write exactly once (written/rejected/dropped, first-outcome memoized across early-hook + fallback sends) and response state advances monotonically pending -> sent / pending -> failed. Same-key replay returns every stored durable row (stopped/pending/uncertain) with response state, payload hash, and terminalPublished without re-running effects. The extension seam exposes only the current turn's epoch (AC 24); the ADR documents the durable contract status precisely. Lore-id: c04-terminal-durable-state
…Frame stub Post-rebase onto dev: the docs index now includes the terminal-abort ADR (121 docs) and the dev-added readiness-lifecycle test's sendFrame stub returns the required delivery outcome.
snowykr
force-pushed
the
feat/abort-sdk-terminal
branch
from
August 5, 2026 18:03
704b618 to
9bfcb7f
Compare
… hydration Codex review threads P1/P2 on PR #21: 1. (P1) A successful terminal abort on an already-acknowledged prompt was reported as worker_unsettled because emitPromptLifecycle finalizes (deletes) the submission during emission, so the post-terminalize promptSubmissions lookup returned undefined and the durable marker was never advanced to stopped. terminalizePrompt now sets capture.terminalized only on a genuinely landed durable terminal, and the callback decides success from that outcome instead of re-deriving it from the (possibly finalized) submission record; fail-closed paths leave it unset. 2. (P2) The terminal replay lookup and response-state transition read / wrote the reconciliation store without awaiting the startup hydration, so a same-key retry immediately after a restarted endpoint became reachable could race the still-pending store load and miss the durable row. Both the abort path and the host onControlResponseDelivery hook now await reconciliationReady before any terminal-scope snapshot or transaction. Tested: 26/26 terminal-abort, 19/19 dispatch, 15/15 store, 9/9 sdk-host, 80/80 host-wiring serial; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b3b70a950
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review thread P2 on PR #21: the generic dispatch idempotency cache hashed the RAW request input, so retrying a terminal abort key with the defaulted shape and its explicit equivalent (first {mode:"terminal"}, then {mode:"terminal", scope:"turn"}) produced idempotency_conflict and never reached the durable terminal-scope replay that hashes the normalized {mode, scope} payload. The idempotency hash now normalizes a WELL-FORMED terminal abort input (omitted scope -> "turn") before hashing, so both shapes share one key and replay deterministically; malformed inputs (unknown fields, invalid mode/scope) stay raw so they are rejected downstream and never collide with a valid input's key. Lore-id: c04-terminal-idempotency-normalize Tested: 20/20 sdk-control-dispatch (incl. defaulted/explicit replay, scope change conflict, malformed-fresh-key rejection), 9/9 sdk-host, 80/80 host-wiring serial; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cedb0769b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…trol errors Codex review threads P1/P2 on PR #21: 1. (P1) The turn lineage was minted from #promptGeneration, which only advanced on abort/session-close — two consecutive NON-aborted turns shared (lineageIdHash, epoch), so a scope:"owned" abort of turn B could capture turn A's left-running jobs and scope:"turn" could misclassify A's completion as B's resume. Every NEW ROOT TURN admission (marked by resetRetryReplaySafety, i.e. user prompt / custom message / queued dispatch) now advances the attempt epoch before minting, so each turn gets a unique (lineage, epoch); same-turn continuations keep the turn's epoch. Regression test: two normal turns, then owned abort of turn B — A's job stays foreign, B's job is captured. The chain suite now resets the process-lifetime registries per test (job ids collide across fresh managers) via a test-only reset. 2. (P2) The durable terminal-scope replay conflict returned a nested {ok:false,error} that dispatchControl wrapped in a top-level successful control_response. The surface now THROWS a typed idempotency_conflict control error (SHARED_ERROR_CODES), so the response itself is ok:false after in-memory eviction/restart, matching the in-cache conflict path. Lore-id: c04-terminal-per-turn-epoch Tested: 21/21 sdk-control-dispatch (incl. top-level conflict), 4/4 chain (incl. consecutive-turn regression), 9/9 sdk-host, 80/80 host-wiring serial; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15f3948629
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review thread P2 on PR #21: the onControlResponseDelivery hook advanced the durable record's responseState by matching only the idempotency key hash, so a same-key terminal abort with a DIFFERENT input (after in-memory eviction, e.g. scope:"owned" after a pending scope:"turn" marker) would mark the ORIGINAL pending marker sent/failed for the conflict response — corrupting the durable replay row so a later exact retry of the original input could report the response was sent even though only a conflict for a different input was delivered. The transition now also requires the record's normalized idempotencyInputHash to match the request's normalized {mode, scope} payload, so only the response for the exact input that produced the record advances its state. Store test covers same-key/different-input isolation. Lore-id: c04-terminal-response-state-guard Tested: 16/16 sdk-reconciliation-store (incl. input-hash guard), 21/21 dispatch, 4/4 chain, 9/9 sdk-host; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bd87065d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s owned Codex review threads P1/P2 on PR #21: 1. (idle-key reservation) A terminal abort with no active prompt returned no_active_turn WITHOUT a durable row, so after restart or in-memory eviction a same-key retry while a later prompt was active missed the replay and could fence/abort that unrelated turn. The no-active-turn / already-terminal paths now persist a bounded no_effect terminal-scope reservation (turnDisposition "no_effect", sentinel fence epoch 0) keyed by key+input hash, and the replay branch returns the exact no-effect row (no_effect_replay) instead of re-running. The v2 validator accepts "no_effect" and restart settlement leaves it untouched; store test covers persistence + reload + settlement. 2. (monitor ownership) MonitorTool started background bash through startMonitorJob whose register path never captured the five-tuple, so scope:"owned" could claim stopped_owned while the monitor kept running and emitting follow-ups. startMonitorJob now threads the tool call id and calls registerOwnedIfLineaged; chain test proves the monitor job registers as exact owned work of its turn. Lore-id: c04-terminal-idle-reserve-monitor-owned Tested: 17/17 sdk-reconciliation-store (incl. no_effect persistence), 5/5 chain (incl. monitor ownership), 21/21 dispatch, 9/9 sdk-host, 105/105 tools; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 851f0bd894
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ut check Codex review threads P1/P2 on PR #21: 1. (defer fresh lineage) The streaming injector called #resumeFromOwnedCompletion while followUp only QUEUED the owned completion during another active prompt, immediately mutating the session-wide epoch/lineage. If that active turn was then terminal-aborted, getTerminalTurnEpoch() and later tool registrations used the queued resume's fresh lineage instead of the active turn's, so scope:"owned" could miss that turn's already-registered jobs while still reporting success. The streaming path now defers allocation to the actual resume admission (the queued dispatch re-enters #promptWithMessage with resetRetryReplaySafety, which allocates the fresh epoch at turn start); the idle injector (agent.prompt direct) still allocates immediately before admission. 2. (strict delivery-hook input) The response-state hook normalized any non-"owned" scope to "turn", so a MALFORMED same-key retry (scope:"bogus") rejected by dispatch could still match a prior valid scope:"turn" pending row and mark it sent/failed for the invalid response. The hook now strictly validates the terminal input (mode "terminal", scope undefined|"turn"|"owned", no unknown fields) before computing the input hash and advancing any durable row. Lore-id: c04-terminal-resume-admission-delivery-guard Tested: 5/5 chain, 21/21 dispatch, 17/17 store, 9/9 sdk-host, 80/80 host-wiring serial; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4117879821
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…h; fail reservation Codex review threads on PR #21: 1. (P1, streaming resume lineage) The streaming injector queued an owned completion via agent.followUp but never allocated the promised fresh attempt/lineage — the agent loop consumes follow-ups DIRECTLY via getFollowUpMessages, never through #promptWithMessage, so the resume's tools inherited the streaming turn's lineage and scope:"owned" could attribute/cancel work to the wrong turn. The Agent now exposes onFollowUpConsumed (reassignable, invoked when follow-ups are dequeued for the next turn); the session allocates the fresh lineage there — at actual resume admission, after the previously streaming turn has ended, so the active turn's lineage is never corrupted. 2. (P2, preserve aborted epoch) abortPromptAndWait advanced #promptGeneration right after registering the scope, so #isTurnContinuationBlocked (which looks up by the CURRENT epoch) missed the closed fence on non-generation-guarded continuation paths such as hidden-next-turn. The abort-time bump is removed: fresh-turn allocation is the per-admission responsibility (every NEW ROOT TURN advances the epoch before minting), and the scope stays keyed to the aborted epoch where the continuation gate finds it. 3. (P2, reservation failure) A no-active-turn/already-terminal abort whose durable reservation write failed still returned success without any durable row, so a later same-key retry after eviction/restart could abort an unrelated turn. Reservation failure now returns reservation_failed (safe uncertainty) instead of success. Lore-id: c04-terminal-resume-admission-epoch-reserve Tested: 26/26 terminal-abort, 5/5 chain, 21/21 dispatch, 17/17 store, 9/9 sdk-host, 80/80 host-wiring serial; agent + coding-agent package checks clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37eca64ad4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… fence Codex review threads P1/P2 on PR #21: 1. (P1, resume epoch) #resumeFromOwnedCompletion used the module-global nextPromptAttemptEpoch counter, which root admissions never advance — after a normal root prompt at epoch N the counter could still be <= N, so the resume minted the SAME (lineage, epoch) as the aborted turn and its tools could be captured by a later scope:"owned" abort. The fresh epoch is now allocated from the SESSION prompt epoch (+1), always distinct from the current turn's epoch. 2. (P2, hidden next-turn gate) The terminal abort now preserves the epoch so the continuation gate can find the closed scope, but the hidden next-turn scheduler's generation check still passed and its admission path never consulted #isTurnContinuationBlocked — a successor queued by the aborted turn could start. The hidden next-turn drain now checks the fence before prompting. Lore-id: c04-terminal-resume-epoch-hidden-gate Tested: 26/26 terminal-abort, 5/5 chain, 21/21 dispatch, 17/17 store, 9/9 sdk-host, 52/52 resilient-retry, 6/6 queued-prompts, 80/80 host-wiring serial; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac6bc7eb45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pped monitor notifications Codex review threads P1/P2 on PR #21: 1. (preflight cancel) A turn.prompt still in PREFLIGHT has no promptSubmissions entry, so terminal abort treated it as no_active_turn and durably reserved the key WITHOUT cancelling the pending preflight — the prompt would later be accepted and start running despite the abort. The surface's abortTerminal now cancels the connection's pending preflights first (mirroring the legacy abort path). Host-wiring test: terminal abort during a never-resolving preflight cancels it (no agent_start/agent_failed). 2. (monitor notifications) A monitor task-notification is delivered as a follow-up (not an async-result), so the injectors' owned-drop path did not cover it — scope:"owned" could report stopped_owned while a queued notification from the stopped monitor still resumed the agent. onFollowUpConsumed now drops follow-up task-notifications whose job is registered as owned under a terminal scope with ownedCompletionPolicy disabled (the notification side channel gets the same ownership/purge treatment as the job). Non-persistent monitors keep their one notification on their normal self-cancel flow (red-team contract preserved). Lore-id: c04-terminal-preflight-notification-purge Tested: 81/81 host-wiring (incl. terminal-preflight cancel), 6/6 + 23/23 monitor suites, 26/26 terminal-abort, 5/5 chain, 21/21 dispatch, 9/9 sdk-host; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6312cc83b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review thread P2 on PR #21: cancelPendingPreflightsForConnection only settles the SDK preflight waiter — it does not change the session's preflight signal or generation, so an AgentSession.prompt() still in async preflight could later reach the accept callback (no-op because the waiter was settled), pass the unchanged cancellation checks, and start the turn after a terminal abort reported no_active_turn. A new private session seam cancelPendingPreflightForTerminalAbort aborts the preflight controller (firing the captured admission signal so #throwIfPromptPreflightCancelled throws and the pending prompt never starts) and resets it for the next admission; the terminal abort surface calls it alongside the SDK waiter settlement. Lore-id: c04-terminal-preflight-session-invalidate Tested: 81/81 host-wiring (incl. terminal-preflight cancel), 26/26 terminal-abort, 5/5 chain, 21/21 dispatch, 9/9 sdk-host, 6/6 monitor; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a0c76f3bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review thread P1 on PR #21: classifyOwnedCompletion required an existing terminal scope, so an owned Bash/task job that completed BEFORE the abort (root turn still streaming) was enqueued as an ORDINARY YieldQueue entry; a later scope:"owned" abort could cancel/ack the manager delivery but could not identify or purge the already-queued ordinary async-result, so stopped owned work could still resume the agent. Ownership is now kept on the queued entry regardless of scope: onJobComplete attaches the registration-based envelope whenever the job has an exact registered five-tuple (the registration is fixed at job-registration time, the scope only appears at abort/flush time). Classification became three-state — no scope: ORDINARY (normal delivery), turn-scope enabled: FRESH (new-turn resume), owned-scope disabled: DROP — so the pre-abort completion is dropped at flush once the owned scope lands. The terminal-abort unit suite now isolates the process-lifetime registries per test (job-id collisions across tests were masking this). Lore-id: c04-terminal-preabort-ownership Tested: 27/27 terminal-abort (incl. pre-abort ownership regression), 5/5 chain, 21/21 dispatch, 9/9 sdk-host, 6/6 monitor, 81/81 host-wiring; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a57e5c1f70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…dden next-turn Codex review threads P1/P2 on PR #21: 1. (P1, denied follow-up drop) An owned async-result enqueued into Agent.followUp while streaming BEFORE any terminal scope existed was ordinary at enqueue but still carried details.ownedCompletions; once a scope:"owned" abort landed, the onFollowUpConsumed hook only handled the fresh case and never removed now-dropped follow-ups, so stopped owned work could still be returned by getFollowUpMessages and resume the agent. The hook now removes ANY follow-up whose ownedCompletionResumeAction is "drop" from the dequeued batch at the final consumption boundary. 2. (P2, hidden next-turn discard) The fence-blocked hidden next-turn drain returned without discarding #pendingNextTurnMessages, so the aborted turn's queued successors could be drained into a later explicit prompt. The blocked drain now clears the queued hidden messages. Chain test: terminal abort discards a queued hidden next-turn successor. Lore-id: c04-terminal-followup-drop-hidden-discard Tested: 27/27 terminal-abort, 6/6 chain (incl. hidden-next-turn discard), 21/21 dispatch, 9/9 sdk-host, 52/52 resilient-retry, 6/6 queued-prompts, 81/81 host-wiring; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50ab00a732
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…of persisted details Codex review threads P1/P2 on PR #21: 1. (P1, stale registrations) When a top-level session is replaced in the same process, a fresh AsyncJobManager restarts job ids/generations at bg_1/job:1, but the process-global owned registry was never cleared and registerOwnedRegistration's early return preserved the OLD turn's lineage for the reused tuple — the new session's first job could not bind to its own turn, and a later scope:"owned" abort could report stopped_owned while leaving it running. A reused (jobId, jobGeneration) with a DIFFERENT lineage/epoch now OVERWRITES the stale entry (same lineage/epoch stays idempotent). 2. (P2, persisted details) The async-result custom message carries the private owned-completion envelope in details, which is persisted via appendCustomMessageEntry (only __pendingDisplayTag was stripped) and returned to SDK transcript clients. ownedCompletions is now in INTERNAL_DETAILS_FIELDS, so it is stripped from persisted entries while remaining available in-memory for the injectors. Unit tests: reused-tuple overwrite (P1) + ownedCompletions strip (P2); terminal-abort fixtures moved to distinct job keys (the overwrite semantics made shared (job-1, gen-1) fixtures undefined). Lore-id: c04-terminal-stale-rebind-envelope-strip Tested: 28/28 terminal-abort, 12/12 session-manager-internal-details (incl. ownedCompletions strip), 6/6 chain, 21/21 dispatch, 9/9 sdk-host, 81/81 host-wiring; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2091726a44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… reservations Codex review threads P1/P2 on PR #21: 1. (accepted-not-started cancel) A terminal abort landing after onPreflightAcceptCommit accepted the prompt but before agent_start bound submission.executionHandle hit terminalizePrompt's missing-handle fail-closed branch; the preflight cancellation entry was already removed after accept, so cancelPendingPreflightForTerminalAbort was not called and the pending #promptWithMessage could continue into the agent. The abort now cancels the in-flight session preflight whenever the active submission has no executionHandle yet, so the accepted-but-not-started prompt cannot start after the abort response. 2. (bounded reservations) Idle/already-terminal aborts with unique keys appended durable no_effect rows without a count/age cap, so a client could grow the reconciliation document indefinitely. The reservation now trims the OLDEST no_effect rows beyond a 256 cap (bounded like the in-memory idempotency cache) via the pure boundNoEffectReservations helper; stopped/uncertain rows are never evicted. Unit test covers the eviction. Lore-id: c04-terminal-accepted-cancel-reservation-bound Tested: 29/29 terminal-abort (incl. reservation bound), 17/17 store, 6/6 chain, 21/21 dispatch, 9/9 sdk-host, 81/81 host-wiring; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e8e9038b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ncertain Codex review thread P2 on PR #21: an accepted-but-not-started prompt (submission exists, agent_start not yet bound) had the session preflight cancelled but then continued into terminalizePrompt, whose missing-handle branch ran failPromptClosed — claiming a pending prompt outcome, fencing the SDK connection, and leaving reconciliation unfinalized until restart for a prompt that will never run. The accepted-but-not-started case now FINALIZES the prompt as a pre-run client cancellation (terminalizePrompt without fence, outcome stopped/ cancelled/client_cancel), reserves the key durably, and returns no_active_turn / terminal_no_effect — no fenced-uncertain claim, no connection fence. The harness test that simulated "cannot be fenced" (via an unbound handle) now asserts the finalized cancellation. Lore-id: c04-terminal-pre-run-finalize Tested: 81/81 host-wiring (incl. accepted-but-not-started finalize), 29/29 terminal-abort, 6/6 chain, 21/21 dispatch, 9/9 sdk-host, 17/17 store; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57375f841d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review thread P1 on PR #21: getTerminalTurnEpoch and cancelPendingPreflightForTerminalAbort existed only on ExtensionContextActions; ExtensionRunner.createContext() never returned them, so the ctx passed to the SDK host in PRODUCTION lacked both seams — active terminal aborts hit markerEpoch === undefined and returned terminal_no_effect WITHOUT stopping the turn, and preflight terminal aborts only settled the SDK waiter without cancelling the underlying AgentSession preflight. (The host-wiring harness provided the seams manually, masking the gap.) The ExtensionRunner now binds the seams from contextActions during initialize (with safe zero fallbacks) and returns them from createContext(), and the ExtensionContext interface declares them. Runner test: the created context surfaces getTerminalTurnEpoch and cancelPendingPreflightForTerminalAbort after initialize. Lore-id: c04-terminal-context-seams Tested: 34/34 extensions-runner (incl. seam surfacing), 81/81 host-wiring, 29/29 terminal-abort, 6/6 chain, 21/21 dispatch, 9/9 sdk-host; package check clean
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ec092092b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…keys first Codex review threads P1/P2 on PR #21: 1. (hidden successors on skip/admission) The hidden-next-turn discard only ran inside the scheduled drain callback. If a new prompt advanced the generation before the drain ran, the task was skipped by the generation guard and the clear never executed — and once the new prompt bumped the epoch, the fence lookup could not find the aborted turn's scope, so #promptWithMessage would inject the aborted turn's hidden successors into the new user prompt. #promptWithMessage now discards pending hidden next-turn messages BEFORE the admission bump when the previous turn's fence is closed (covering scheduled-skip, explicit-drain, and no-drain paths alike). Chain test: terminal abort + new prompt discards the hidden successor. 2. (pre-run reservation order) The accepted-but-not-started branch finalized the accepted prompt BEFORE writing the durable no-effect reservation; a failed write or a crash between the awaits left the key unreserved while the prompt was already cancelled. The reservation is now persisted before terminalizePrompt in the pre-run path. Lore-id: c04-terminal-hidden-skip-pre-run-reserve Tested: 7/7 chain (incl. skip-path discard), 29/29 terminal-abort, 21/21 dispatch, 17/17 store, 9/9 sdk-host, 52/52 resilient-retry, 6/6 queued-prompts, 81/81 host-wiring; package check clean
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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.
Summary
Implements C04
turn.abortmode:"terminal"with typedscope:"turn" | "owned"(default"turn"): stop the root worker's current turn and block only its own continuation routes, while exact owned background work (Bash/task jobs, detached subagents) that the caller deliberately leaves running keeps running — its completion/progress is delivered through the existingYieldQueue -> agent.followUp/agent.promptpath as a fresh root turn with a new attempt/lineage.This follows the approved ralplan plan
sdk-abort-20260805-v3(corrected semantics) and is documented in the mandatory ADR:docs/adr-abort-sdk-terminal-turn-owned.md.Problem
The SDK
turn.abortsurface could only do an ordinary argument-less cancel. There was no way to stop a turn while leaving its exact owned background work runnable and still able to resume the agent with its results, nor a way to additionally stop that owned work with proof.Change
mode:"terminal", optionalscope, and the required bounded idempotency key (≤128 UTF-8 bytes); unknown fields / invalid mode / invalid scope / keyless / oversized keys are rejected withinvalid_inputand no side effects. Legacy{}/mode:"turn"and ACPcancel()remain byte-for-byte compatible.scope:"turn"): stop the root turn; block same-turn retry, TTSR/agent.continue, steering, hidden-next-turn, maintenance/worker successor, and accepted-pre-close same-attempt continuations at the final synchronous boundary (TurnContinuationGate). Left-running owned work reportsownedWork:"left_running",automaticDelivery:"enabled",resumeOnOwnedCompletion:true.scope:"owned"): additionally stop the exact captured owned work with generation-verified cancellation, a fixed grace, a second quiescence proof, and delivery purge;ownedWork:"stopped"is claimed only after proof. Foreign/unclassified work is never swept.owned-completionvsturn-continuationis source/lineage-based, never timing-based; missing/mismatched/forged metadata fails closed.written/rejected/dropped) and response state advances monotonicallypending -> sent/pending -> failed. Same-key replay returns the stored row (dispositions, response state, payload hash,terminalPublished) across eviction and restart without re-running effects; same-key different-input conflicts.User-visible behavior
selection,turn(stopped/uncertain/no_active_turn/no_effect/no_store),ownedWork,automaticDelivery,resumeOnOwnedCompletion, optional safereasonandreplaymetadata.terminal_no_effectfor no-store / no-active-turn / initial-marker-failure paths (no destructive work).terminalPublished:false, no second event), and host write rejection all return safe uncertainty — never a fabricatedstopped.Commits
30 commits in the repo's lore format: 13 feature units (squashed from the original 26 iteration commits and rebased onto
devfor a clean, reviewable history) plus 17 review-driven fixes, each addressing exactly one Codex review thread:c04-terminal-per-turn-epoch— fresh lineage epoch per root turn; durable conflicts as control errorsc04-terminal-idempotency-normalize— normalize terminal abort input before idempotency hashingc04-terminal-response-state-guard— guard response-state transition by terminal input hashc04-terminal-idle-reserve-monitor-owned— durably reserve idle terminal keys; register monitor jobs as ownedc04-terminal-resume-admission-delivery-guard— defer resume lineage to admission; strict delivery-hook input checkc04-terminal-resume-admission-epoch-reserve— fresh lineage at follow-up admission; preserve aborted epoch; fail reservationc04-terminal-resume-epoch-hidden-gate— session-relative resume epoch; gate hidden next-turn by the fencec04-terminal-preflight-notification-purge— cancel pending preflights on terminal abort; drop owned-stopped monitor notificationsc04-terminal-preflight-session-invalidate— invalidate the session preflight on terminal abortc04-terminal-preabort-ownership— preserve ownership for pre-abort async completionsc04-terminal-followup-drop-hidden-discard— drop denied owned-completion follow-ups; discard blocked hidden next-turnc04-terminal-stale-rebind-envelope-strip— rebind stale owned registrations; keep owned envelopes out of persisted detailsc04-terminal-accepted-cancel-reservation-bound— cancel accepted-not-started prompts; bound durable terminal reservationsc04-terminal-pre-run-finalize— finalize accepted pre-run aborts as cancelledc04-terminal-context-seams— expose terminal abort seams on ExtensionContextc04-terminal-hidden-skip-pre-run-reserve— clear hidden successors on skip/admission; reserve pre-run keys firstterminalizePrompt outcome + hydration barrier— decide terminal success by the terminalizePrompt outcome; await store hydration before replay/transitionsEvery change is minimal and scoped to its thread; the review-fix commits will be squashed into the feature units before merge per the contribution guideline.
Validation
Focused suites (run serially;
host-wiringis load-sensitive and runs alone):test/session/terminal-abort.test.tstest/agent-session-terminal-abort-chain.test.tstest/sdk-control-dispatch.test.tstest/sdk-reconciliation-store.test.tstest/sdk-host.test.tstest/sdk-host-wiring.test.tstest/sdk-session-readiness-lifecycle.test.tstest/tools.test.tsbun --cwd=packages/coding-agent run check(biome + tsc)The end-to-end chain (mock session → bash job registered → terminal turn abort → owned-completion classification; a later turn's job is not claimed by the aborted scope; monitor jobs captured as owned work) is covered by the chain suite.
Review handling
All 29 review threads from the automated Codex review have been resolved with committed, pushed fixes and per-thread replies (details in the thread replies): terminalizePrompt outcome success, hydration barrier, idempotency input normalization, per-turn lineage epoch, durable conflict as a top-level control error, response-state input-hash guard, idle-key durable reservation, monitor job ownership, deferred resume-lineage allocation, strict delivery-hook input validation, fresh lineage at follow-up admission, preserved aborted epoch for continuation-gate checks, fail-on-reservation-failure, session-relative resume epoch, hidden next-turn fence gating, terminal-preflight cancellation, owned-stopped monitor notification purge, and session-preflight invalidation, and pre-abort completion ownership preservation, denied follow-up admission drop, hidden next-turn discard, stale-registration rebinding, envelope persistence stripping, accepted-not-started prompt cancellation, bounded durable reservations, and pre-run cancellation finalization, and ExtensionContext seam exposure, hidden-successor skip/admission clearing, and pre-run reservation ordering.
A seven-generation boundary cohort was run against this change set before rebasing: architect CLEAR/CLEAR/CLEAR APPROVE, executor QA passed (adversarial incl. forged-tuple denial, injector drop, replay rows, host outcome classification), cleaner PASS, terminal critic OKAY.
Checklist
docs/adr-abort-sdk-terminal-turn-owned.mddev)Note: 3 host-wiring tests are flaky when the chain suite runs concurrently in the same process (5s-timed, load-induced); they pass when run serially. This is an existing test-infrastructure interaction, not a regression.