Repository navigation
Codex app-server: orchestrator deferred-first-turn source-claim sequence - #612
Merged
Merged
Conversation
The daemon driver that wires the hosted-Codex source-claim handshake into the launch lifecycle. Dormant until the activation slice sets deferFirstTurn on the factory (the production factory is unchanged; the fake test runtime exercises the whole sequence). - Factor StartAcpForwardingAsync's post-bind body (liveness/TOCTOU guards, local binding registration, forwarder build+run) into a shared StartForwarderAfterBind, so the source-claim path reuses it WITHOUT re-binding (the claim supersedes AcpSessionStarted). The daemon forwarder ignores the claim's AcceptedSeq — resume is server-driven via the AcpBatchAck, exactly as before. - StartEnvelopeSourcedSessionAsync (fired fire-and-forget after RegisterAgentAsync for a runtime whose RequiresSourceClaimBeforeFirstTurn is true): AcpSessionSourceClaim → (Rejected/method-not-found: coded launch failure + teardown | Bound: start forwarder without re-binding → BeginFirstTurnAsync → confirm). Every step gated on the agent's AcpCts so a finalize aborts cleanly; contains all its own faults. - FailEnvelopeSourcedLaunchAsync = CleanupAgentAsync (single-flight: disposes the runtime, releases the slot, unregisters) + LaunchFailedAsync — the proven outer-catch composition, never FinalizeAgentRunAsync (which classifies a still-live process by exit code). - ConfirmSessionLaunchLoopAsync: background confirm, retrying only transient failures on a paced cadence for the agent's lifetime and NEVER tearing it down (Confirmed/AlreadyConfirmed done; Superseded/NotFound stop; a lost confirm leaves the row provisional for the server's own recovery to settle). - HandleLaunchAgent branches on RequiresSourceClaimBeforeFirstTurn; Cursor keeps the single-phase bind-then-forward path. Tests: AgentOrchestratorSourceClaimTests (3 — happy path claims-then-forwards-without-rebind- then-first-turns-then-confirms, Rejected teardown, method-not-found teardown), asserting through the deterministic AcpCallOrder/signal seams. The 8 existing forwarding tests still green (the factoring is behaviour-preserving). CaptureServerConnection/FakeAcpRuntime/ SpyAcpHostedAgentRuntimeFactory gain the source-claim/confirm/deferred-first-turn doubles.
PR Summary by QodoOrchestrate deferred first turns with ACP source claims
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
…lts (Copilot review) It is awaited from the fire-and-forget source-claim task outside any try, so a teardown fault (a cleanup or a LaunchFailed hub error) would escape as an unobserved task exception. Wrap its body in try/catch + log; document the no-throw invariant.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…view) A BeginFirstTurnAsync failure after a successful claim previously only logged, leaving a zombie agent holding a slot while the server's Rule-2 expiry closed the provisional row. Tear the agent down instead (symmetric with the claim-failure path, immediate slot release); the provisional row is still closed by Rule 2. New test: first-turn failure → LaunchFailed + unregister + dispose, no confirm.
…reason (qodo) - FailEnvelopeSourcedLaunchAsync now SetAgentStatus(Failed) BEFORE CleanupAgentAsync disposes the runtime — disposal ends the read loop, whose FinalizeAgentRunAsync would otherwise classify by exit code and emit a Completed/Failed status that races and could MASK the coded launch failure. A pre-set Failed makes that classification a no-op (the same guard the launch-verdict path uses). qodo #3 (finalizer overwrites claim failure). - The rejected-claim reason is now vendor-neutral (no 'codex app-server:' prefix in shared orchestration code). qodo #1 (vendor/driver separation). - qodo #2 (first-turn leak) was already addressed in the prior commit (tear down on a first-turn dispatch failure). Tests 4/4 + 8 forwarding regression still green.
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
…turn (qodo) qodo reliability bug (latent in #612, armed by this activation): a launch that finalized during the source-claim / forwarder setup could still release the held first turn. StartForwarderAfterBind aborts silently when finalize cancelled acpCts and/or removed the agent, but StartEnvelopeSourcedSessionAsync then called BeginFirstTurnAsync unconditionally, and BeginFirstTurnAsync unsealed the dispatcher (sending turn/start) BEFORE observing its token — so turn/start could race the teardown. Two-part fix: BeginFirstTurnAsync now ThrowIfCancellationRequested BEFORE Unseal (the dispatch runs on CancellationToken.None and can't be recalled, so this is the only safe gate); and StartEnvelopeSourcedSessionAsync re-checks the SAME liveness StartForwarderAfterBind guards on (cancelled OR agent-removed) before dispatching, so the agent-removed-but-token-live arm is covered too. Both land in the OCE/return arms that already tear down or log-and-abort. Added Cancelled_BeginFirstTurn_does_not_ dispatch_the_held_turn. Also trimmed the activation test comment qodo flagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
…erver (#624) * [AI-1762] §2.6: activate envelope-sourced transcript for hosted app-server Flip the app-server path from the dormant hook-sourced transcript to the live envelope source. One envelopeSourced decision in StartAppServerAsync now drives all four coupled facts so they can never desync: the guard-1 marker (BuildEnv → KCAP_HOSTED_APPSERVER, stands the rollout watcher down), the runtime's envelope emission (emitEnvelopeTranscript), the deferred first turn behind the source claim (deferFirstTurn), and the forwarder wiring (Transcript: runtime on HostedRuntimeStart). Everything downstream was already built and merged and keys off these: the orchestrator drains the runtime's envelopes through AcpTranscriptForwarder and, seeing RequiresSourceClaimBeforeFirstTurn, runs source-claim → BeginFirstTurn → confirm (#612); guard-2 refuses any stray hook write server-side (#1547); death-close is the existing finalize → EndAgentSession, and the 60-min rollout-idle fallback disengages automatically because the stamped marker suppresses the hook's watcher (#617). So this is a four-value flip in one method, not new machinery. Scope: this activates the app-server path, which today is reviewers only (UsesAppServer stays `active && IsReviewFlow`) — the interactive routing flip is deliberately the LAST step, after §2.6 lifecycle + §2.7. Gated behind codex.transport=app-server (default pty), so a default daemon is byte-unchanged; requires a server carrying #1530/#1547 (server-before-CLI); and AI-2110's live cert validates it before app-server becomes the default. Only the dormant-marker factory test inverts (now asserts the marker is stamped, the Transcript is wired, and the first turn is deferred). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [AI-1762] §2.6: a cancelled source-claim must not dispatch the first turn (qodo) qodo reliability bug (latent in #612, armed by this activation): a launch that finalized during the source-claim / forwarder setup could still release the held first turn. StartForwarderAfterBind aborts silently when finalize cancelled acpCts and/or removed the agent, but StartEnvelopeSourcedSessionAsync then called BeginFirstTurnAsync unconditionally, and BeginFirstTurnAsync unsealed the dispatcher (sending turn/start) BEFORE observing its token — so turn/start could race the teardown. Two-part fix: BeginFirstTurnAsync now ThrowIfCancellationRequested BEFORE Unseal (the dispatch runs on CancellationToken.None and can't be recalled, so this is the only safe gate); and StartEnvelopeSourcedSessionAsync re-checks the SAME liveness StartForwarderAfterBind guards on (cancelled OR agent-removed) before dispatching, so the agent-removed-but-token-live arm is covered too. Both land in the OCE/return arms that already tear down or log-and-abort. Added Cancelled_BeginFirstTurn_does_not_ dispatch_the_held_turn. Also trimmed the activation test comment qodo flagged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Sep 24, 2026
This was referenced Oct 14, 2026
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.
What
The daemon driver that wires the hosted-Codex source-claim handshake (server
AcpSessionSourceClaim/ConfirmSessionLaunch, merged in kcap-server #1530) into the launch lifecycle. This is the next §2.5 piece after the runtime contract (#606).Dormant until the activation slice sets
deferFirstTurnonCodexHostedAgentRuntimeFactory— the production factory is unchanged, so no shipping launch is affected. The fake test runtime exercises the whole sequence.The sequence (fired fire-and-forget after
RegisterAgentAsync, for a runtime whoseRequiresSourceClaimBeforeFirstTurnis true)AcpSessionSourceClaim— durable claim (server binds + writes the ownership ledger row).Rejected(or a pre-source-claim server's method-not-foundHubException) → coded launch failure + teardown.Bound→ start the transcript forwarder without re-binding (the claim supersededAcpSessionStarted) →BeginFirstTurnAsync(dispatch the held first turn) → confirm in a background loop.Pieces
StartForwarderAfterBind— factored out ofStartAcpForwardingAsync(its post-bind body: liveness/TOCTOU guards, local reconnect-binding registration, forwarder build+run). The claim path reuses it verbatim without theAcpSessionStartedbind. The daemon forwarder ignores the claim'sAcceptedSeq— resume is server-driven via theAcpBatchAck, exactly as before. The 8 existing forwarding tests still pass, proving the factoring is behaviour-preserving.StartEnvelopeSourcedSessionAsync— the sequence above; every step gated on the agent'sAcpCtsso a finalize aborts cleanly; contains all its own faults (no outer catch).FailEnvelopeSourcedLaunchAsync=CleanupAgentAsync(single-flight — disposes the runtime, releases the slot, unregisters) +LaunchFailedAsync— the proven launch outer-catch composition, neverFinalizeAgentRunAsync(which classifies a still-live process by exit code).ConfirmSessionLaunchLoopAsync— background confirm, retrying only transient failures on a paced cadence for the agent's lifetime and never tearing it down (Confirmed/AlreadyConfirmeddone;Superseded/NotFoundstop; a lost confirm leaves the row provisional for the server's own recovery to settle).HandleLaunchAgentbranches onRequiresSourceClaimBeforeFirstTurn; Cursor keeps the single-phase bind-then-forward path.Tests
AgentOrchestratorSourceClaimTests(3), asserting through the deterministicAcpCallOrder/signal seams (neverTask.Delay):AcpSessionStartedCallsempty, nobind:) → first-turns once → confirms with the claim token;Rejectedclaim → teardown (unregister + dispose), no first turn, no confirm;Test doubles extended:
CaptureServerConnection(source-claim/confirm raw seams),FakeAcpRuntime+SpyAcpHostedAgentRuntimeFactory(deferred-first-turn).Next
The activation slice flips
deferFirstTurn/emitEnvelopeTranscripton the factory + wiresTranscript: runtime+ thecodex-appserver-unattended-v2capability + guard-1 env marker + the server guard-2 source-refusal — making this whole path live.🤖 Generated with Claude Code