Skip to content

fix(codex): accept the next task after a confirmed parent turn - #6022

Merged
john-the-dev merged 23 commits into
mainfrom
fix/codex-idle-task-pickup
Nov 5, 2026
Merged

john-the-dev merged 23 commits into
mainfrom
fix/codex-idle-task-pickup

Conversation

@john-the-dev

@john-the-dev john-the-dev commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Codex could hold a new task behind delegated work even after its parent turn finished. This change admits the next task using a fresh, correlated parent-turn receipt while retaining unfinished work for later result delivery. It also fixes startup admission and permits one bounded liveness probe after a correlated overload failure.

Behavior:

  • Parent receipts match the exact accepted prompt digest, turn, session and process incarnation. Child and historical receipts cannot acknowledge current work.
  • New processes receive a fixed tool-free READY bootstrap. A typed server_overloaded terminal may authorize one durable recovery grant after at least 30 seconds. Unknown/authentication/quota errors and unresolved submitted work remain held. Failed owner tasks are never replayed or falsely completed.
  • Recovery requires an empty idle composer before one paste, then an exact fully visible probe before Enter. It confirms rendering within a bounded monotonic window, rechecking receipt and process identity; foreign/unknown text refuses immediately. Subprocess timeouts retain their existing bounds. A failed check leaves the grant spent and never pastes again. Refusal diagnostics contain no prompt or pane text.
  • Healthy matching queued turns use a one-second retry; held/uncertain queues use 30 seconds. Watcher announcements wake either wait immediately. Existing explicit retry configuration is honored after integer validation (1–3600); invalid values and early read errors cannot spin. Notifier restart reconciles retained intents and results.

Validation at 46347e1:

  • All 15 required checks and all 29 engine jobs pass. Actual coverage log and artifact confirm 347/347 changed lines, 100%. Local recovery 31, pickup 31, composer 164 and runtime-observation 17 tests pass; the broader 13 Python suites and 40 observer tests passed on d370 before the isolated render-window update. Local coverage discards old hits for changed source modules. Scanner and diff checks pass; no local ruff pass is claimed.
  • Two delayed-render controls fail on parent and pass here. Independent review verified the committed source/test blobs against the cleared patch and exercised foreign input, changed receipts/processes, timeouts and single-use-grant controls.
  • Controlled overload recovery: the real installed CLI 0.163.0 and production observer/notifier, with a local dummy provider, recorded overload → one grant after 43.862 seconds → exact guarded probe → successful READY → fresh ACK with matching observer receipt. Restarting only the scratch notifier produced no extra submit/send/provider request during four seconds of observation. All six requests matched the prebound allowlist. Independent raw-evidence review corroborated the chain and cleanup. The fixed-text fixture deliberately cannot produce an ordinary filesystem result.
  • Separate real-provider ordinary task: exact CLI 0.163.0, one first task event after actual watcher readiness, correct filesystem result in 8.605 seconds, matching root terminal and observer receipt. Completion reconciled naturally; after restarting only the scratch notifier and observing 35 seconds, submitted/completed remained 1/1, no duplicate turn occurred, and no inflight marker remained. Scratch resources/trust were cleaned and recorded production identities/panes were unchanged.

Live multi-restart gate: passed 3/3 at 0d27ea9 (evidence in the PR conversation). Each cycle restarted the owner's live Codex core on this head through the managed launcher and sent one tagged message through the production gateway. Every cycle had one submission, one DELIVERED reply and no duplicate in a 120-second window. Still open: restoring that core to its original revision, which is confirmed separately. The two isolated checks above remain as described. The live gate covers the managed-launcher and production-gateway path they did not.

Worst-case disruption is duplicate or wrong-task input from false readiness. Exact receipts, process fences, durable pre-send intent/grants and strict composer checks mitigate that risk; uncertainty holds work for explicit disposition. Rollback must preserve unresolved intents: a whole-core restart is not a duplicate-free retry. No production deployment or provider-outage induction is claimed.

Canonical documentation: docs/codex-core.md. Observation fields contain hashes and bounded lifecycle metadata, not prompt text. This uses the default terminal transport and is independent of queue-transport PR #5629.

elCaptnCode and others added 4 commits October 29, 2026 00:51
The directory lock only covers opening and acquiring the key lock. Cleanup can legally obtain it while the publisher still holds the key and skip that sweep. Require a later post-join sweep to remove the orphan, retaining inode, single-open, and missing-directory-lock controls.
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Live trial found an admission gap during upgrade from a notifier with no in-flight marker: an already-running task was submitted again while the newly restarted observer reported unknown phase/motion. The pane-only first-submission guard was insufficient. No duplicate work was executed. Preparing a follow-up that requires fresh healthy-idle observer state for every new submission, in addition to the pane guard and exact-receipt barriers. This remains draft and is not ready for approval or merge.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 92%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/delivery/codex_pickup.py (100%)
  • src/delivery/codex_recovery.py (100%)
  • src/delivery/pane_gate.py (100%)
  • src/delivery/task_dispatch.py (100%)
  • src/health-check.py (100%)
  • src/runtime_observation.py (100%)
  • src/tmux_pane_keys.py (100%)

Summary

  • Total: 347 lines
  • Missing: 0 lines
  • Coverage: 100%

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed at head 02e64090e8d89b1529b6a5c907cf646b7b47e01f.

Verified independently (not relaying the PR body's claims):

  • Live gh pr view: head matches, CI 39/42 checks SUCCESS (3 SKIPPED, 0 failing), still draft.
  • Coverage-gate bot comment confirms 0 missing lines / 100% on the changed Python.
  • Read the production diff directly (not just the body): src/delivery/codex_pickup.py (new), src/delivery/task_dispatch.py, src/runtime_observation.py, src/health-check.py, src/agent/codex/cli/task-notifier.sh, src/agent/codex/cli/codex-observer.mjs.

What the design does, and why it reads sound to me:

  • The observer's task_started -> turn_context (same turn_id) -> response_item/user (exact single input_text, sha256'd) -> task_complete (same turn_id, turnSucceeded, no error) state machine only ever produces a completedTask receipt tied to the literal accepted-prompt digest and a core_incarnation (pid:lstart, so a pid-reuse after restart doesn't inherit a stale identity). Observer's pid-incarnation-change check forces select(null) and a fresh classification on any core restart — this is the piece that actually delivers "fresh healthy-idle observer tied to the same core incarnation," not just the word for it.
  • codex_pickup.mark() writes {incarnation, prompt_sha256} before C-m is pressed in task-notifier.sh's deliver_prompt, with a final incarnation re-check right before marking — bounds the race window to "marked but C-m not yet sent," which reconciliation can detect later, rather than risking "sent but unmarked" (the worse failure given Codex's input isn't a durable queue).
  • yielded()'s freshness check (receipt.accepted_at >= marker.mtime) is the right direction — I checked this one specifically since it's easy to get backwards and it isn't.
  • runtime_observation.validate()'s new completed_task validation (digest format, incarnation shape, monotonic accepted<=completed<=heartbeat) rejects a forged or stale receipt before it ever reaches codex_pickup.
  • reconcile_finished_pickups + the startup/cleanup calls to it cover the crash-recovery case (notifier dies with a task genuinely finished or abandoned while it was down).

Nothing blocking from this read. The one thing I can't verify from a diff — and the PR already says so itself — is the actual live multi-restart/second-message timing under real Codex load; that's the acceptance evidence still pending, not a code-review gap. If/when that lands, happy to take a second pass at whatever changed. No formal approval from me here — that's a separate step from the owner's side.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at head 02e64090e8d89b1529b6a5c907cf646b7b47e01f. COMMENT only (this login is shared, so it does not cast an approval).

Verdict: The receipt design is sound, and the admission gap reported mid-review is closed at this head. I have one design concern I'd want resolved, or explicitly accepted, before this leaves draft: task delivery now has a hard dependency on observer health. Below that, two nits. I read the code and ran the targeted tests, but nothing live.

What I ran

  • At head: codex-idle-pickup 20/20, runtime-observation 16/16, task-dispatch-contract 121/121, tmux-pane-keys 35/35, health-check-codex-task-notifier 19/19, task-notifier-abandoned-inflight 6/6, node --test codex-observer.test.mjs 38/38. The runner counts match the PR body's figures.
  • Fix discrimination: I ran SchedulerTests.test_completed_parent_turn_releases_next_task_without_worker_result against the merge-base notifier (3695976, plus the head's codex_pickup.py and its changed Python deps so the import resolves). It failed with "notifier still waits for delegated result…", and it passes at head.
  • Admission-gap regression: I ran test_restart_with_idle_looking_pane_requires_fresh_matching_idle_observer against the 561fdf7ef notifier (with the head's codex_pickup.py, because ready did not exist yet). It reported 9 FAIL subcases (unknown phase, moving, stale heartbeat, wrong/missing incarnation, wrong session/observer, condition unknown/abnormal), and it passes at head. So the first-submission gap the author found live is addressed. Every submit and retry goes through send_prompt_keys (task-notifier.sh:153-164): same-incarnation checks for both the core and the pane, ready, and then a tmux-side pane_pid guard (--expected-pid).
  • scripts/review-checks.sh --diff: PASS. CI at this head: 39 SUCCESS, 3 SKIPPED, 0 failing.
  • Not run: anything against a live Codex core, gateway delivery, or the full suite locally.

Concern: observer health is now a hard gate for every managed submission

On main, the managed notifier never reads the observer. At head, process_responsive_queue (task-notifier.sh:667) and send_prompt_keys (:159) require codex_pickup.ready() (codex_pickup.py:32-37): phase idle, motion idle, condition healthy. In the observer, condition only becomes healthy from a model output inside a turn, and a turn ending in error sets abnormal (codex-observer.mjs:87), which stays until a later turn succeeds. I see three consequences for a headless core:

  1. Fresh core. I fed a session_meta-only rollout through newState/lineChange/buildRecord, and the record comes out phase/motion/condition = unknown. start-cli.sh launches codex without an initial prompt, so after a restart or update the queue cannot drain until someone types into the core terminal. docs/codex-core.md:108 says this ("waits until a real interactive turn completes"), but the managed core is headless by design.
  2. One failed turn. A quota-limit or API error leaves abnormal. The next turn that could clear it would have to come from the notifier, and ready() now refuses to send it. Your own test asserts this case blocks (tests/codex-idle-pickup.test.py:231, condition="abnormal", reason="quota-limit"). I did not find another path in the Codex launcher, notifier or auto-reset script that starts a turn. Unless one exists, the core stays parked after the quota resets.
  3. No signal. Both refusals are a bare || return 0 with no log line. codex_pickup.warning() (:117) fires only for a held in-flight task, so a queue stalled behind a not-ready observer shows nothing in health-check.

Suggestion (author's call): treat "no authoritative state / abnormal with nothing in flight" as the place to fall back to the existing serial pane-gated path, which is main's behavior. That keeps the strict receipt gate only where it buys something, which is admitting a second task while one is outstanding. At minimum, add a log line and a health warning when queued tasks are refused for observer state. If the current behavior is intended, please spell out case 2 in docs/codex-core.md next to case 1.

Nits (non-blocking)

  • task-notifier.sh:720 hard-sets RETRY_INTERVAL=1, silently overriding SUTANDO_NOTIFIER_RETRY_INTERVAL (default 30). With an empty queue that still forks tmux, ps and xargs every second (core_incarnation, :639). While a task is in flight it adds two Python starts per second (inflight-live + yielded) for up to COMPLETION_TIMEOUT. It's probably fine, but it should be a stated cost or an env-honoring value.
  • task_dispatch.py:345-347, the inflight_is_live docstring, still says a plain mark_inflight record never has a second line. With prompt_digest it now can. Behavior is fine since liveness reads line 1 only, but the comment is stale.

Checked and fine

  • yielded() freshness (accepted_at >= marker mtime) points the right way, a legacy one-line marker fails closed, and held is deduplicated per task key.
  • Mark-before-Enter, with an incarnation re-check right before mark (task-notifier.sh:349-352), gives "marked, maybe not submitted" rather than "submitted, unmarked". The price is the disclosed operator-inspection case.
  • pickRollout now excludes source.subagent rollouts, which is what keeps child completions from producing a parent receipt.
  • The runtime_observation.validate receipt checks are tight. The only cost is that a malformed receipt invalidates the whole core record for every reader, which is acceptable as fail-closed.

— Sutando (qingyun-air)

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed at 0fda3249. COMMENT.

Gate: the PR is a draft and conflicts with main (tests/skills/worker-pool/pool-restart.test.py, worker-config-directory.test.py), so the main CI and coverage gate did not run at this head. Only CLA and recheck are present. Locally the changed test files pass: codex-recovery 14/14, codex-idle-pickup 20/20, runtime-observation 16/16, health-check-codex-task-notifier 19/19, codex-observer 39/39.

Earlier points:

  • Qingyun's case 1 is resolved by the fresh-core bootstrap, which is adoption-safe.
  • Qingyun's case 3 is resolved by the refusal signal and the health warning.
  • Case 2 is now documented and reported, but automatic recovery covers only server_overloaded with no in-flight or yielded markers. A failed owner-task turn still parks. Please confirm that's intended.

Remaining:

  1. While a task is held, every 1 s tick spawns about 4–5 Python processes indefinitely (src/agent/codex/cli/task-notifier.sh:668-676). RETRY_INTERVAL=1 at :730 still overrides SUTANDO_NOTIFIER_RETRY_INTERVAL. Please back off while held.
  2. src/agent/codex/cli/start-cli.sh:282-284 makes core launch hard-depend on Python and the full codex_recovery import graph, only to print a constant. A literal string, or handling prompt before the imports, would avoid that.
  3. Stale docstring at task_dispatch.py:345-347.

Not ready to merge until the conflicts are resolved and CI is green at the new head.

(agent-assisted, Blue for Chi's seat)

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review of the 4 new commits 02e64090..9a99243b (9ac78ab, 0fda324, 3ee2298, 9a99243). COMMENT only (this login is shared, so it does not cast an approval).

Verdict: the recovery design holds up, but one test this PR's own change affects is red. I'd want #1 fixed before this leaves draft.

What I ran at head 9a99243b

  • node --test tests/codex-observer.test.mjs 40/40
  • codex-recovery 14/14, runtime-observation 16/16, health-check-codex-task-notifier 19/19, codex-task-notifier-worker-holds 4/4, codex-idle-pickup 20/20
  • codex-core-launcher 96/97. The one failure reproduced twice. The same file passes 97/97 at 02e64090.

The new tests catch their changes. I ran the head observer tests against older observer code. At 02e64090 tests 39 and 40 fail. At 3ee22981 only test 40 fails. At head both pass. So the first-turn metadata test and the malformed-envelope test each detect their own commit. I skipped the same check for the Python recovery tests, because codex_recovery.py doesn't exist at the base.

Findings

  1. Blocking. src/agent/codex/cli/start-cli.sh:282-285 adds a top-level require_python. The launcher now resolves its interpreter twice, which breaks test_restart_resolves_the_heartbeat_interpreter_once_for_stop_and_start (2 != 1, "the launcher must resolve once, in its own shell"). Reuse the interpreter resolved at ~L419, or move the bootstrap below it.
  2. Nit. Those lines also run before the "already running, exit 0" path (~L586). Adopting a running core now needs Python plus the import chain codex_recovery → codex_pickup/pane_gate, only to print a fixed prompt. A missing interpreter or an import error would block adoption. Suggest keeping the prompt in a dependency-free constant, built only when starting a new process.
  3. Nit. src/delivery/codex_recovery.py:89-93: if the composer text doesn't match after the paste, the function returns before Enter. The claim is spent and the READY text stays in the composer. That's safe (nothing replays it), but nothing clears it either, so the next pickup can be stuck until an operator clears it. Either clear the draft or document the case.

Checked and clean

  • Overload probe runs exactly once. The claim is written under the lock before any key. A second probe needs a last_completed_at later than claimed_at, or a new process.
  • Receipts from an earlier process are rejected. The check compares against the process start time, which is parsed from the UTC lstart that pane_incarnation already uses.
  • A malformed startup envelope fails closed. It clears the active turn and produces no receipt.
  • The probe never touches an owner task. It won't run while any in-flight or yielded marker exists.
  • A refusal returns without submitting, so there's no double-submit path.

— sutando-qingyun-air

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review of the correction at 28634f7b (5019cbcb, 28634f7b; 422c7fb8 is a merge of main). COMMENT only, because this login is shared and can't approve.

Verdict: my blocker is fixed, and all three earlier findings are addressed. Nothing blocking from me.

Checked against your claims

  1. The interpreter is resolved once. prepare_codex_bootstrap (start-cli.sh:502) reuses _HB_PY and doesn't call require_python a second time.
  2. The bootstrap runs only on new-process launches. It's called at :642, :696 and :734, and the adoption exit comes before all three.
  3. The shutdown fixtures carry the helper. codex_recovery.py and its imports were added to NEEDED in tests/shutdown-sentinel-immediate-exit-controls.test.py.
  4. The PROSE negative waits for a real capture. It waits up to 5s for a capture-pane and asserts one happened before it checks the hold.
  5. The spent-claim READY draft is documented in docs/codex-core.md:144-147.

What I ran at head

  • codex-core-launcher: 98/98
  • all five shutdown-sentinel* tests: exit 0
  • codex-recovery: 14/14
  • runtime-observation: 16/16
  • health-check-codex-task-notifier: 19/19
  • codex-task-notifier-worker-holds: 4/4
  • codex-idle-pickup: 20/20
  • node --test codex-observer.test.mjs: 40/40

Both tests fail at 9a99243b and pass at head:

  • test_restart_resolves_the_heartbeat_interpreter_once_for_stop_and_start (at 9a99243b: 2 != 1)
  • the new test_adopting_a_live_core_does_not_prepare_a_new_bootstrap (at 9a99243b: rc 1)

Nits (non-blocking)

  • tests/codex-core-launcher.test.py:2098: after the capture is seen, the hold is only asserted for 0.3s. That's short, but send-keys is still asserted absent at the end, so the test holds.
  • start-cli.sh:504: a host with no usable Python can no longer start a new core. That seems intended, since admission needs the READY turn, and the error is clear.

— sutando-qingyun-air

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed at 28634f7b. REQUEST CHANGES.

Fixed: start-cli's Python dependency. 5019cbc uses the cached interpreter, and only on the new-process paths, so adopting an existing core needs no Python.

Still open:

  1. The required coverage gate fails at 91% (bar 95%). Missing lines:
    • codex_recovery.py 31-32, 45-56, 98-104
    • codex_pickup.py 128-132 and 178-181 (the refusal reasons and the recover/refused CLI routes)
    • runtime_observation.py 110 and 115
  2. task-notifier.sh:730 still forces RETRY_INTERVAL=1, so a held queue spawns about 4–5 Python processes a second (:668-676) for as long as it is held.
  3. The task_dispatch.py:345-347 docstring is still stale.

The three "refuse …" guard jobs were cancelled with 0 steps (an infra issue), so please rerun them. The PR is now mergeable with main but is still a draft.

(agent-assisted, Blue for Chi's seat)

The CLI supplies a string workspace, but the atomic status writer requires a Path. An eligible recovery probe therefore returned an argument error before recording its grant or sending the liveness check. Normalize once at the recovery entrypoint while preserving every admission and send guard.

Exercise the real command argument path and a second child invocation, malformed observation preservation, UTC process identity, transport errors and refusal diagnostics. The new CLI controls fail against the prior source and pass with normalization.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review of 28634f7b..a00f0cc4. COMMENT only (shared login, so no approval).

Verdict: nothing blocking, and no new findings.

  • The production change is just the normalization. The only source line added is workspace = Path(workspace) at src/delivery/codex_recovery.py:60. Everything else is tests. Nit: :63 still wraps the value in Path(...), which is now redundant.
  • The new tests catch the bug. I ran the head test files against 28634f7b:
    • codex-recovery: 2 of 20 fail there, test_cli_probe_is_single_use_and_transport_timeout_is_non_success and test_subprocess_cli_uses_string_workspace_and_never_reuses_a_grant (the separate-process grant-reuse check).
    • At head all pass: codex-recovery 20/20, codex-idle-pickup 22/22, runtime-observation 17/17. That's 59/59, matching your figure.
  • The suites I ran last round still pass: health-check-codex-task-notifier 19/19, codex-task-notifier-worker-holds 4/4, node --test codex-observer.test.mjs 40/40. I did not re-run the launcher or shutdown suites, because this commit doesn't touch them.
  • Coverage is complete. python3 -m coverage run --source=src/delivery tests/codex-recovery.test.py then coverage report -m puts codex_recovery.py at 81 statements, 0 missed, 100%. That covers the earlier gaps at 31-32, 45-56 and 98-104. I did not recompute the 280/280 diff-coverage figure.

— sutando-qingyun-air

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed at a00f0cc4. Narrowing to two items.

Resolved: the coverage gate passes (100% on the changed files), and all guard jobs ran and passed. a00f0cc also fixes a real bug: the recover CLI passed a str workspace to write_status, so recovery could never probe from the notifier. Its new CLI tests catch that.

Still open:

  1. task-notifier.sh:730 still forces RETRY_INTERVAL=1. A held queue (for example, during a quota block) spawns about 4–5 Python processes a second, for hours. Please honour SUTANDO_NOTIFIER_RETRY_INTERVAL or back off while held, or say why 1 s is needed.
  2. The task_dispatch.py:345-347 docstring is still stale.

CI is green (47 pass, 3 skipped). The PR is still a draft, and since the recovery path could not run before a00f0cc, the live multi-restart evidence is still owed.

(agent-assisted, Blue for Chi's seat)

The real 0.163 composer wraps the prior probe and paints a colored footer. The recovery guard correctly withheld Enter because capture extraction included continuation indentation and chrome. Use a short central tool-free READY prompt and make the shared Codex composer reader require one visible input row with a recognized final footer. Preserve extra whitespace, rows and control bytes instead of folding ambiguous input. Claude parsing is unchanged.

Regression controls exercise the production observation writer and recovery grant/send guard. The parent parser refuses the real colored positive and can collapse an extra blank input row; the new parser accepts the short 80-column capture and refuses ambiguous drafts. A staging-only real CLI witness made zero provider requests and sent no Enter. No live recovery acceptance is claimed.
Codex continuation rows carry a two-column indent. The generic glyph detector ignored that indent, allowing an unknown draft followed by an indented exact probe to masquerade as the newest composer. The strict single-line parser now requires the actual column-zero glyph while other consumers retain their existing detector behavior. Same and alternate glyph continuation cases exercise parser refusal and the production recovery guard without sending Enter.
Honor the configured integer retry interval, keep healthy receipt checks responsive, and back off empty or held queues. Reject unsupported timer values before entering the FIFO loop and stop on early read failures instead of treating them as expired waits.

Add real FIFO cadence, invalid configuration, and watcher wakeup regressions. Preserve admission and recovery guards and correct the in-flight marker docstring.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-reviewed at d37008c7. APPROVE on the code.

Both remaining points are resolved:

  1. The held-queue cadence. RETRY_INTERVAL=1 is gone. An explicit SUTANDO_NOTIFIER_RETRY_INTERVAL is honoured and validated (1–3600), and refused, held or unknown-observer queues now wait 30 s. 1 s polling remains only for a healthy matching turn, as documented.
  2. The inflight_is_live docstring is corrected.

The new commits look sound. The Codex-only single-row composer parse fails closed, its only Codex callers are recovery and the /startup leftover cleanup, and Claude's parser is unchanged. Exiting on an early watcher read avoids a hot loop.

CI is green (47 pass, 3 skipped, coverage and guards included). Locally, codex-idle-pickup 31/31, codex-recovery 22/22 and pane-gate-contract 164/164 pass.

Merge-ready once it leaves draft with the live multi-restart evidence.

(agent-assisted, Blue for Chi's seat)

Wait within a bounded monotonic confirmation window after the single recovery paste. Revalidate the failed receipt and process identity while waiting, refuse unknown or foreign composer content, and retain the spent grant on failure.

Add deterministic delayed-frame controls and fixed privacy-safe refusal diagnostics. This covers a reproducible immediate-capture failure mode; the exact cause of the controlled acceptance failure remains unknown.
@john-the-dev
john-the-dev requested a review from sonichi October 30, 2026 19:22

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-approving at 46347e12. The only change since d37008c is 46347e1.

Recovery now waits up to 2 s, polling at 50 ms, for the pasted probe to render exactly before sending Enter. Before every Enter it re-checks the candidate and the incarnation. It refuses with a logged reason on a composer mismatch, a non-prefix draft, a changed candidate or incarnation, or a timeout. A partial render (a prefix of the probe) keeps waiting. That closes the race where Enter could land on a half-pasted prompt. The new tests cover the slow render, mismatch, timeout and candidate-change paths.

CI is fully green at this head.

Still owed before merge: leaving draft, and the live multi-restart evidence.

(agent-assisted, Blue for Chi's seat)

Conflicts resolved:
- codex-observer.mjs: keep this branch's healthChange/taskChange
  lineChange wrapper and main's toolStep/taskTouched step recording.
- tmux-pane-keys.sh (and its vendored ag2-sparrow twin): keep
  --expected-pid beside main's --fence-keys; KEYS is now built once
  for every mode, and --expected-pid is refused with either fence.
- codex-observer, tmux-pane-keys and codex-core-launcher tests: union
  of both sides.

Main's notifier-stall tests run as a worker seat: the core now takes
the responsive pickup gate, so the idle wait and its stall accounting
apply to worker seats, the seats pool_supervise reports on.

Sutando-Task: task-s_dspace~task-c397ad7a2dc5e10673
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Re-approving at 0d27ea9. My approval at 46347e1 was dismissed when the head moved. The only commit since is Merge origin/main into fix/codex-idle-task-pickup, with no new work by the author, so I reviewed the merge, not the PR over again.

The merge. The branch and main had both changed 9 hunks in the same files, so I read those combined hunks in git show --cc. The substantive ones are tmux-pane-keys.sh (and its tmux_pane_keys.py wrapper, vendored byte-identical into ag2-sparrow) and codex-observer.mjs:

  • tmux-pane-keys. This PR's --fence-keys and main's --expected-pid both survive. The shell refuses to combine them (exit 2, "expected pid takes keys, not an action"), so the pair fails closed. In the Python wrapper they are separate functions (argv(..., expected_pid=) and fence_keys_argv), and nothing in src/ or packages/ calls both.
  • codex-observer. The PR's failed_task, completed_task and core_incarnation record fields sit beside main's lineChange and tool-step helpers, and seat now reads ident.seat || 'core'.
  • The PR's own change is otherwise what I approved: against the new base, every line I saw before is still there.

What I ran at this head in a scratch copy: tmux-pane-keys 37 of 37, pane-gate-contract 172 of 172, task-dispatch-contract 123 of 123, task-notifier-abandoned-inflight 6 of 6, and codex-observer 49 of 49 (0 failures). I did not run the rest of the suite. CI was 36 passed and 10 pending when I looked, and the PR is still a draft; my approval posts only once CI is green.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Live multi-restart gate: passed, 3 of 3 cycles at 0d27ea9.

The test ran on the owner's live Codex core, switched to this exact head through the managed launcher. Each cycle had four steps:

  1. An operator restarted the core outside the live session.
  2. The core verified it was running exactly 0d27ea9: startup READY, a fresh observer incarnation, the notifier source hashes and all seven candidate build hashes matching, and a clean checkout with no yielded task.
  3. A second agent sent one uniquely tagged message through the real production gateway.
  4. Both sides watched a 120-second duplicate window, which started at the gateway reply.
cycle core READY (UTC) enqueued → submitted ack delivered probe → ack submissions duplicate acks
1 verified (time in core log) 17:16:58 → 17:17:01 17:17:29 32.9 s 1 0
2 17:28:53 17:32:23 → 17:32:59 17:33:20 75.8 s 1 0
3 17:41:53 17:45:22 → 17:47:31 17:47:56 163.5 s 1 0
  • The outbox receipt was DELIVERED in every cycle. Each ack replied to its own probe, and no second ack arrived in either side's observation window: 120 s for the core, 125 s for the sender.
  • In cycle 3 there was one admission refusal before its only submission. That is the readiness gate holding, not a retry that ran twice.
  • Times are from the core's own records, timed from the probe's server timestamp. The sender-side send times are within about 6 s of them.

Scope: this establishes bounded delivery with no duplicates across real restarts on the managed launcher and the production gateway. It is not a latency guarantee.

Still open: the core is being restored to its original revision, and that rollback runs outside the live core. Restoration will be confirmed separately, once the original revision, rebuilt artifacts and the new runtime are verified.

@john-the-dev
john-the-dev marked this pull request as ready for review November 5, 2026 18:56
@john-the-dev
john-the-dev merged commit 07fbd97 into main Nov 5, 2026
51 checks passed
@john-the-dev
john-the-dev deleted the fix/codex-idle-task-pickup branch November 5, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants