Correlate Codex turn dispatches with the turn the turn/start response names - #3503
Merged
SawyerHood merged 7 commits intoSep 16, 2026
Conversation
SawyerHood
added this pull request to stack #3504
September 11, 2026 17:30
SawyerHood
force-pushed
the
bb/2580-codex-turn-start-correlation-thr_vdv2rx6c7w
branch
2 times, most recently
from
September 11, 2026 17:34
7cc1ee3 to
54cafa4
Compare
SawyerHood
force-pushed
the
bb/2580-codex-turn-start-correlation-thr_vdv2rx6c7w
branch
2 times, most recently
from
September 11, 2026 17:48
96f54be to
4c2be65
Compare
SawyerHood
force-pushed
the
bb/2580-codex-turn-start-correlation-thr_vdv2rx6c7w
branch
2 times, most recently
from
September 11, 2026 23:03
df67e58 to
ebcd0e5
Compare
… names The bridge ignored the turn/start result and guessed at settlement with a 250 ms timer. When codex answered before emitting turn/started and the notification lagged past the grace window, the timer fabricated a completed zero-work turn and the real turn arrived later as a second lifecycle, so a parent could see its child complete with no output while the child was still working (#2580). Every supported codex (0.136.0 up) answers turn/start with the turn it opened. The bridge now opens that turn from the response, attaches the accepted input to it, and suppresses the provider's own later turn/started. A response reporting an already-terminal turn settles the full lifecycle at once, with the later notifications suppressed the same way. A dispatch codex steers into the already-running turn only acknowledges the input against that turn. The zero-work timer remains for results that name no turn (thread/compact/start's empty result and a malformed body). A crash between the answer and the notification now settles the real turn as failed instead of fabricating a completed one. Cherry-picked from #2639 (Ratul Sarna) onto the #3462 fix; the host-daemon protocol version moves from #3462's 204 to 205. Co-Authored-By: Claude Code <noreply@anthropic.com>
thread/compact/start answers with an empty result, and codex then opens the
compaction as a turn: the committed real recording shows turn/started 192 ms
after the {} result, and a real app-server opens a turn even when there is
nothing to compact (it ends as failed with a systemError status). The bridge
sent compaction through the 250 ms zero-work timer, so a turn/started that
lagged past the grace window produced a fabricated completed turn followed
by the real compaction as an unrequested turn, the #2580 pattern.
A compaction dispatch now stays unopened until its turn/started claims it.
It settles synthetically only on a signal from codex: a thread/status/changed
to idle or systemError while no turn is open (completed or failed), or the
app-server exiting before the turn starts (failed). The zero-work timer is
kept only for a turn/start result that names no turn.
The fake app-server now runs compaction as a delayed turn, with modes for an
idle report without a turn and an exit before the turn starts, and the
regressions cover a turn/started 600 ms after the empty result, both signal
paths, and the exit. The already-open branch of openTurnFromStartResponse no
longer records a turn id that no later notification would remove.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Opening a turn from the turn/start response exposes it to bb before codex
activates it. A real app-server answers turn/start with {turn: inProgress},
then emits turn/started a few milliseconds later, and a turn/interrupt sent
in that gap fails with "no active turn to interrupt". A stop that arrived
right after bb saw the turn therefore failed, which the agent-runtime codex
integration test "stops an active turn and recovers with a follow-up" hit
under load.
The bridge now remembers which turns it opened from a response and whether
codex's own turn/started has arrived. When an interrupt of such a turn is
rejected, it waits for that notification (or for the turn to settle or the
app-server to exit, within the interrupt settlement timeout) and interrupts
again; it does not retry any other failure. The fake app-server gains a
prompt that rejects an interrupt before turn/started like a real
app-server, and a regression covers the stop.
Co-Authored-By: Claude Code <noreply@anthropic.com>
SawyerHood
force-pushed
the
bb/2580-codex-turn-start-correlation-thr_vdv2rx6c7w
branch
from
September 16, 2026 20:20
ebcd0e5 to
cd67ddb
Compare
SawyerHood
deleted the
bb/2580-codex-turn-start-correlation-thr_vdv2rx6c7w
branch
September 16, 2026 21:44
SawyerHood
added a commit
that referenced
this pull request
Sep 16, 2026
## Human comments ## What was wrong When the server considered a thread idle or failed while its daemon still held a root turn, Stop sent a release that deliberately retained the turn. The daemon returned the same result as a completed release, so Stop did nothing and subsequent sends kept failing as competing turns (#3462). A competing-turn refusal could also fail the server run despite a stored root turn still running. ## What changed The daemon reports `activeTurnRetained` on a declined release and identifies competing-turn refusals with `competing_turn`. Explicit Stop reconciles the retained turn into the normal interrupt path. Concurrent awaited stops share the complete operation, including an escalated interrupt; machine-preservation callers requiring a stopped thread receive any failure. Context clear cannot proceed while the thread remains stopping. An explicit Stop also interrupts a turn that starts while its release is pending. A declined release no longer marks background commands stopped. A regression covers an interrupt failure followed by a successful Stop retry. Competing-turn refusals with a stored running root turn retain the active run; negative controls still fail ordinary refusals and refusals without a running root turn. Automatic release protection remains intact. Rebased onto main `c663ff1911496870026c779911ff874fb2a43f24`, preserving the shared daemon Stop/storage-deletion helper, lifecycle ownership, and writable queues during Stop. Protocol 211 → 212 and plugin SDK 0.4.98 → 0.4.99 cover the contract/declaration changes. SDK, thread guide, and CLI skill document the behavior. The scripted Codex test server also accepts main's new configuration arguments so this layer passes independently. Existing failure policy remains: an unsuccessful interrupt leaves the thread `stopping`; the public Stop request can still return success, so callers must inspect thread status before treating interruption as confirmed. This is explicitly documented. `requireStopped` callers reject on the command failure. This is the bottom layer of #3502 → #3503 → #3512. #3503 repairs the Codex response/notification correlation race; #3512 protects provider-session ownership. ## How you verified - Fresh Turbo tests/typechecks for server, daemon, runtime, daemon contract, SDK, and templates. Runtime 331, contract 59, SDK 113, and templates 55 passed. Server broad run passed 2,893 tests; three timeout suites passed a focused rerun, and both inherited Codex fixture failures passed after the argument parser fix. Stop/checkpoint suites passed 23 tests; the timeout/Stop rerun passed 108. Daemon passed 717 tests; its two repository-discovery failures came from the filesystem sandbox's synthetic `/tmp/.git` marker, and all 26 discovery tests passed outside that sandbox. - New retained-background-work regression fails before the settlement guard and passes after it, including successful retry. Existing coverage exercises idle/error retained turns, concurrent stops, turn-start/release races, context clear, and escalated interrupt failure for `requireStopped` callers. - Real Codex through the configured account pool, fresh isolated BB store and session home: with `sleep 90` running, injected the server's erroneous `error` status, then invoked CLI Stop. The thread became idle in under a second with an interrupted completion, and the next turn answered `STOP_RECOVERY_CONFIRMED`. In a separate turn, injected idle during a foreground wait: the competing send recorded `competing_turn` without `system/error`, the original turn completed to idle, and a follow-up answered `COMPETING_RECOVERY_CONFIRMED`. Test server and daemon were shut down; no production store or process was changed. This verifies provider behavior through normal pool routing, not a separate scratch login. Fixes #3462 > AGENT GENERATED --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
SawyerHood
added a commit
that referenced
this pull request
Sep 16, 2026
## Human comments ## What was wrong After the runtime identity mix-up in #3461, the server could resume a thread into whichever provider session its newest event named. Ordinary completion events and contaminated identity history could therefore redirect messages into another conversation. Current main still has this selector. The independent safety review also reproduced a second path: editing a message can delete the original owner's identity event, making a contaminated later claimant appear to own that session. ## What changed Third layer of #3502 → #3503 → #3512. Review this layer against #3503; it retains the lower layers' Stop and Codex correlation behavior. - Resolve sessions only from identity events after the last completed context clear. Compare claims within the provider and host; unknown hosts are treated conservatively. An earlier foreign identity is skipped in favor of an owned identity. Earliest claims tied in the same millisecond are ambiguous, regardless of insertion order. - Refuse foreign-only, ambiguous, and invalid (null or empty authoritative handle) resume attempts with 409 `provider_session_unavailable`. No turn is dispatched or history appended. Queued sends record the refusal. `/clear` or `bb thread clear` starts a fresh session on the next send while retaining history. - Check ownership on edit checkpoints and anchored/tip fork sources. Refuse an edit that would erase the earliest shared ownership evidence, both before provider rewind preparation and atomically before suffix deletion. If another claimant arrives while a rewind is being staged, discard that staged session and retain the original history. - Record identities for empty-input starts. Require start/fork identity announcements in the public conformance kit and document unique, durable provider session handles. Scripted multi-process fixtures mint unique handles. - Regenerate the identity index migration after current main as `0125_silent_guardian`. Preserve idempotent index creation for migration replay. Update pruning tests to keep their retention assertions while no longer treating ordinary event stamps as authoritative identities. - Resolve the real-provider integration manifest after test TMPDIR isolation so setup and workers use the same manifest. No daemon wire fields change in this layer. Existing stored history is not rewritten on upgrade. ## How you verified Independent review covered the complete layer, #3461 and original investigation history, current-main changes, ownership queries/index plans, context clear, provider/host isolation, tied claims, legacy checkpoint lineage, and fork/rewind paths. A new migrated-SQLite regression failed before the edit guard: removing the owner's identity changed the other thread from `foreign` to `owned`. Additional regressions fail before and pass after for null/empty identity handles; these cannot silently replace the conversation. Guard tests cover tied/strictly ordered claims, provider/host/unknown-host scoping, retained duplicate claims, clear-context retention, pre-rewind refusal, and a competing claim arriving before the atomic edit commit. Server tests assert no history mutation or replacement start after refusal, and successful staged-session discard. Final head: `55097953348f0d583e34317375ed693d4d130716`, based on #3503 `cd67ddbb5735e4153e506bceec1a8c6a665dc2ef`. Its tree exactly matches the reviewed local tree. Fresh forced Turbo checks ran on integrated implementation head `e23319817a80042971fcf1e40036dda9b11ae5e6`; the final follow-up changes only fixtures and formatting: - DB: 573 tests and typecheck passed, including the generated migration, replay, query plans, new edit guard and invalid-history regressions. - Codex: 317 tests and typecheck passed. The unit run excludes inherited account-pool environment variables because one missing-variable test assumes they are absent. - Server: typecheck passed. The full run passed 2,909 tests with one skipped; two failed. One new-main fork metadata fixture lacked an identity announcement and was corrected. The other, an unchanged machine-environment test, exceeded its five-second limit under full-suite load. All nine affected fixture tests then passed with default timeouts; the 57-test migration suite also passed after formatting. The final follow-up changes tests and formatting only. - Other unchanged surfaces were freshly verified before final lower-layer integration: protocol 282, SDK 291, Claude Code 364, ACP 319, Pi 167 (one skipped), echo 15, scripted echo 1, bb-guide 5, and server/bridge integration 82. Corresponding typechecks passed. - Real `codex-cli 0.154.0`: isolated session/workspace tests through the configured account pool. All 12 tests passed again with the final integrated implementation, serially: eight basic lifecycle and four cross-process resume tests. The final fixture-only commit does not change provider code. These are live provider calls, with no production BB process or imported store started. - The SDK npm version guard passes for unpublished 0.4.99. Changed-file formatting passes under repository configuration; generated snapshots were not manually edited. Commands: ```sh pnpm exec turbo run test typecheck --filter=@bb/db --filter=@bb/server --filter=bb-plugin-provider-codex --force --continue=always pnpm exec turbo run test --filter=@bb/integration-tests --force pnpm exec turbo run test:integration --filter=@bb/agent-runtime --force -- integration.provider-basic.test.ts integration.resume.test.ts -t '^codex' --maxConcurrency 1 ``` Local runs used a writable npm cache. Tests were forced rather than relying on prior CI or cache results. The full server run is not represented as wholly green; the two failures and focused confirmations are reported above. Final-head CI remains the landing gate. ## Limits - Ownership is inferred from retained records, not an independent durable ownership ledger. Purging all original-owner records still removes that protection. - Existing legacy fork/rewind checkpoints whose session was never announced remain usable when no conflicting claim exists. Missing announcement history is not proof of cross-thread ownership. - Falling back to an older owned session cannot recover context written into another rollout. - A late edit refusal leaves the source session and stored history intact. Discarding an unused staged rewind is best-effort; a provider cleanup failure can leave an unused staged session. - Third-party bridges must provide handles unique across that provider's sessions on a host. Fixes #3461 > AGENT GENERATED --------- Co-authored-by: Claude Code <noreply@anthropic.com>
Contributor
|
Yay! |
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
2 participants
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.
Human comments
What was wrong
Stacked on #3502. Codex can return
turn/startbefore emittingturn/started. The bridge discarded the response and fabricated a completed turn after 250 ms, attaching accepted input to that synthetic turn while the real work ran in a second, unaccepted turn (#2580 and consolidated #2634). Compaction's empty response has the same delayed-notification problem.What changed
Preserves Ratul Sarna's original #2639 implementation and authorship, with lifecycle and failure-path fixes:
turn/start; deduplicate its later native lifecycle notifications. Dedupe is scoped by provider thread, cleared on exit, and bounded to 256 pending lifecycle records per thread.-32600/no active turn to interruptrejection when that same response-opened turn lacked native activation at the initial attempt. Wait for activation, return the original error on timeout, and avoid retrying a turn that already settled.turn/startresult that names no turn.HOST_DAEMON_PROTOCOL_VERSIONfrom Reconcile daemon turns the server lost track of on explicit stop #3502's 212 to 213. Update lifecycle regressions to use the current test harness after rebasing onto main.How you verified
pnpm exec turbo run test typecheck --filter=bb-plugin-provider-codex --filter=@bb/provider-bridge-protocol --filter=@bb/provider-parity --filter=@bb/host-daemon-contractpasses on the rebased layer. Codex: 317 tests; protocol: 279; parity: 56; contract: 59.CODEX_HOMEdirectories and workspaces: a relay delays native start notifications 400 ms. Reverting this layer to Reconcile daemon turns the server lost track of on explicit stop #3502 reproduces two lifecycles for both an ordinary turn and compaction; the final layer produces one accepted lifecycle. Delaying completion notifications also proves the review fix: the prior bridge sends two interrupts after native activation/real completion; the fixed bridge sends one and preserves the real-32600rejection. No BB core or imported store was started, and no credentials were copied.Fixes #2580
Closes #2639