fix(codex): inject inbound immediately — remove the idle gate (ADR-060)

[aterrylu]

What & why

Codex inbound queued every message and injected a turn/start only during a confirmed-idle window. That gate rested on an assumption ADR-057 itself recorded as "untested at the time of writing" — that a mid-turn injection would interleave and corrupt the thread.

It has now been measured, and it is false. Two things followed from believing it:

1. It duplicated a guarantee Codex already makes. Codex's followup_task contract delivers a queued message at a message boundary while sampling, or after the pending tool call completes. Codex owns mid-turn delivery safety; we re-implemented it from outside, with strictly less information.
2. It deadlocked. A thread blocked in collaboration.wait_agent reports active. So the gate queued instead of injecting — withholding the very message that would have released the agent. The agent waited on a message the gate was holding because the agent was waiting.

flowchart LR
  subgraph before["before — the gate"]
    A1["inbound"] --> B1["queue"] --> C1{"thread idle?"}
    C1 -->|"no (incl. wait_agent)"| B1
    C1 -->|yes| D1["turn/start"]
  end
  subgraph after["after — ADR-060"]
    A2["inbound"] --> B2["queue<br/><i>retry buffer</i>"] --> D2["turn/start<br/><i>immediately</i>"]
    D2 -.->|transport failure| B2
  end

Loading

How

deliverToCodex → enqueue → drain → turn/start immediately, into a busy thread. Deleted waitForIdle, idleDeadlineMs, queueWaitWarnMs, noteLongWait, the long-wait flag, QueuedInbound.queuedAt, and fmtDuration. Net deletion.

Kept, deliberately — the queue did not die:

• A best-effort skip while the last observed status is "compacting". Untested conservatism, not a measured requirement — we determined nothing about compaction, not that it's unsafe.
• The queue as a retry buffer for genuine TRANSPORT failures — socket down, no thread yet, a refused turn — with all of fix(codex): stop losing inbound messages in silence + kill the prompt-delivery false alarm #287's logging intact. Buffer and retry, never drop.
• thread/read for dashboard STATUS only. No longer a delivery gate.

The compacting skip is bounded, because the naive version was a new silent drop

Worth calling out, because review caught it and it inverts the justification I'd written. lastStatus is a cache, and nothing reset it. A missed "compaction finished" push — and we're a non-creator, so it is never replayed — combined with a failing thread/read would pin it at "compacting" forever. Every message to that agent would queue indefinitely, logging every 5s, with no operator notification, because the compacting branch was the only hold-back path in the file that never called noteFailure. The one notification the operator would get says the dashboard status may be stale — pointing at cosmetics while every message they send is swallowed.

That's the #287 class through a new door. So the skip now: clears lastStatus on teardown (it describes a socket that no longer exists), refreshes once when the status is unknown rather than injecting blind, bounds the hold at 5 minutes, and then fails open with a notification saying we stopped believing the status. Guarded by a test verified red against the unbounded version.

This is the ADR's own asymmetry principle applied honestly: I justified the skip as cheap conservatism, and it wasn't cheap until it was bounded.

Also in this PR

• The last true silent drop. shutdownAllAttachments now calls disposeCodexControl on the shutdown path, so a restart mid-queue logs its buffered inbound instead of discarding it in silence. Verified by a controlled before/after on two throwaway servers: identical steps, no output before, DROPPING 1 undelivered inbound message(s) after. The durable half is the log — dispose() also notifies, but that lands in an in-memory store the process destroys microseconds later, which the ADR records rather than papers over.
• A log that named a transport failure while naming nothing. An undici ErrorEvent carries message: "", and ?? doesn't rescue an empty string, so control socket error: printed no cause.
• Comment corrections. Seven sites still described idle-gating as current behavior. One was actively inverted — it told the reader that a dropped status notification "can never wedge inbound delivery", which after this change is precisely the wedge the bound now prevents. That's the comment a future maintainer would use to rule out the bug.
• fix(codex): stop losing inbound messages in silence + kill the prompt-delivery false alarm #287's unreleased changeset, which still asserted "inbound is idle-gated by design (injecting a turn mid-turn corrupts the thread)" and described two deleted timeouts. Both changesets publish into the same CHANGELOG; fixed in place before it ships.

Rejected — do not resurrect

• Suppress multi_agent — removes a working capability to dodge a bug in our own gate.
• A BASE_CONTEXT "don't use wait_agent" — trains agents off a working feature to preserve a workaround.
• Try-then-fallback on ActiveTurnNotSteerable — not reproducible; that error never fired for turn/start in 8 attempts.
• Classify busy-vs-waiting before injecting — provably impossible from a non-creator client: thread/read is byte-identical for a thread doing work and one blocked in wait_agent.

Testing

• make check exit 0; biome check (no --write) clean; tsc --build clean.
• The regression evidence: the invariant-level integration suite from test(codex): integration-test inbound delivery across the router→daemon seam #292 held across the removal with not one assertion moved. The only edit was dropping idlePollMs from its timings call, since that knob no longer exists. Test 2 is the nice one — it proved queue-then-inject-on-idle before and proves inject-immediately-into-a-busy-thread now, because what it actually asserts is "the message arrives."
• Unit tests: 3 asserting the gate's mechanics deleted rather than rewritten (they described behavior that no longer exists); 4 added — immediate-inject-while-busy, compacting delay-not-drop, stale-compacting bound, and the terminated-drop test re-based on a transport failure.

Live QA against real codex 0.144.6 on an isolated dev server:

scenario	result
agent blocked in wait_agent, then messaged	released ~4s, zero stall lines
8-step task, injected at step 3 (daemon independently reporting status=active)	all 8 files byte-correct, task completed, and the injected message answered
thread integrity after mid-turn injection	returned to idle; rollout 74 lines, 0 invalid JSON
restart with 1 message queued	before: silence · after: DROPPING 1 undelivered inbound message(s)

Checklist

• make check passes locally
• Conventional-commit title
• Architectural decision recorded — ADR-060, reversing ADR-057's idle-gate assumption (its capability-gating decision stands)
• make hero — no dashboard UI change

[nox-0x]

Approving — the gate removal is well-evidenced, the deletion is genuinely net-negative, and the compacting skip is bounded in the right direction; the three findings below are follow-ups, not blockers.

What I verified

• deliverToCodex → enqueue → drain → turn/start with no idle wait; the queue's remaining consumers are all transport-failure paths (ensureThread null, turn/start reject, drain catch), each of which retains noteFailure + scheduleRetry and never drops.
• The compacting guard fails open on lastStatus === null, and the single refresh is correctly scoped to "the guard would otherwise be blind" rather than reintroducing a per-delivery read.
• The turn/start-reply shift ordering is untouched, with the #287 rationale intact in the comment.
• The ErrorEvent fix is right: || over ?? is exactly what an empty-string message needs, and .error?.message is where undici puts the cause.
• shutdownAllAttachments disposes before live.clear(), in a synchronous loop that pty.kill()'s async onExit cannot preempt, and the new try/catch closes a real hole (a throw there would have skipped removePidFile/removeControlSocket). restartAllAttachments has its own kill loop and is unaffected, so no controller is disposed out from under a respawn.
• No stale references to waitForIdle/idleDeadlineMs/queueWaitWarnMs/idlePollMs remain outside the historical ADR-057 text, which is correctly left alone.
• ADR-060's UNTESTED section (compaction, byte equality, version skew 0.144.6 vs forge 0.145.0, sample size) is unusually honest about what the evidence does and does not cover, and the sleep methodology note is the kind of thing that saves the next person a day.

I could not execute the test suite in this environment, so the coverage assessment below is by reading.

Follow-ups (non-blocking)

1. 🟡 codexControl.ts — the compacting bound is per-message, not per-episode. compactingSince is cleared on fail-open and again on successful inject, but lastStatus is not, so under a genuinely stale compacting a backlog of N drains at one message per compactingMaxHoldMs with N "delivering anyway" notifications. Not a drop, but it narrows the bound the PR body sells as protecting the queue.
2. 🟡 codex-control.test.ts — the stale-compacting test's /may be stale/ assertion is also matched by statusLoop's "the dashboard status may be stale" notification, which fires first under statusPollMs: 10 + failThreadRead. The delivery assertion is the real (and genuinely red-against-unbounded) guard; the notification half currently passes for free.
3. 🟢 teardownSocket clears lastStatus but not compactingSince, so a reconnect can inherit an expired timestamp and fail open immediately on a genuinely new compaction, with a false "status likely stale" notification.

Version skew is worth a note for whoever operates this: the live QA is on 0.144.6 and forge runs 0.145.0. The ADR says so; just flagging that the first real-world signal will come from the newer daemon.

[aterrylu]

All three addressed in 29cb77b — the first was a real defect in the fix, thank you.

Per-message bound (codexControl.ts:643). Correct, and it undercut the framing. Clearing compactingSince on fail-open re-armed a full hold for each following message, so with lastStatus pinned the queue drained one message per bound with N identical notifications. The decision is about the reported status, so it now latches (compactingDisbelieved) until that status actually changes — a delivered message no longer ends an episode; leaving compacting does. Added a backlog test, and mutation-verified it: reverting to the per-message bound reds it.

compactingSince surviving teardown (:334). Fixed — both halves of the guard's state now reset together. Your sequence is exactly right: a genuinely new compaction after reconnect would have looked like it had already outlived the bound, producing a "status likely stale" log and notification that were simply false.

The test assertion (codex-control.test.ts:222). This is the sharpest one. /may be stale/ also matches statusLoop's "the dashboard status may be stale", which fires ~30ms in — before the 60ms bound expires — so the notification half was free regardless of whether the branch ran. Now matches /Delivering anyway/. Verified by removing the fail-open notifier: both tests go red, which they did not before.

Notably this is the same shape as your catch on #292 (an assertion satisfied by something other than the thing under test). Asserting on a substring of a message rather than on something unique to the branch is apparently my recurring way of writing a free assertion — worth me watching for directly.