Keep the inferred-completion timer referenced

[stevebooks]

Problem

codex-companion.mjs can exit 0 having written nothing at all — no stdout, no stderr. Any caller that expects JSON on stdout cannot distinguish this from success.

The visible symptom is in the stop-review gate: stop-review-gate-hook.mjs runs codex-companion.mjs task --json, gets zero bytes, and JSON.parse throws. The hook reports "the stop-time Codex review task returned invalid JSON" and blocks the turn, so a tooling failure is presented to the user as a failed code review.

I have 27 occurrences logged locally over ~2.5 months, all with an identical signature:

status=0 signal=null errorCode=
stdout (0 bytes):
stderr:

No signal, no error code, both streams empty.

Cause

scheduleInferredCompletion in plugins/codex/scripts/lib/codex.mjs schedules the 250ms timer that resolves state.completion when a turn arrives without an explicit final-turn marker. That timer was unref'd, so it does not hold the event loop open:

  }, 250);
  state.completionTimer.unref?.();

Meanwhile runAppServerTurn is parked on await state.completion. Once the app-server socket closes, the unref'd timer is the only handle left — and unref'd handles do not keep Node alive. The loop drains, the promise never settles, and Node exits 0 silently. An unresolved promise is not an error, so nothing is reported.

It is a race against socket teardown, which is why it is intermittent rather than constant.

Notably this is not a broker failure. I tested both broker failure modes against the unmodified client, and neither produces this signature:

• broker accepts but never replies → hangs indefinitely
• broker drops the connection mid-request → exits 1 with codex app-server connection closed on stderr (handleExit correctly rejects pending requests)

Only event-loop drain yields exit 0 with both streams empty.

Change

Drop the .unref?.() so the timer stays referenced. The timer is bounded at 250ms and is always cleared (clearCompletionTimer, plus it nulls itself in its own callback), so keeping it referenced cannot delay or hang shutdown. Its early-return branches only trigger when state.completed is already true or when work is still pending — in which case a later event reschedules.

Test

tests/inferred-completion.test.mjs covers the observable contract in a subprocess, since scheduleInferredCompletion is not exported:

• an unref'd completion timer exits 0 with empty stdout (pins the failure mode)
• a referenced timer resolves and emits parseable JSON
• scheduleInferredCompletion does not unref its timer (guards the regression)

The third test fails against main before the change and passes after.

Verified end-to-end against a live broker: task --json now returns 151 bytes of valid JSON with rawOutput populated, where it previously returned zero bytes.

Notes

tests/state.test.mjs → "resolveStateDir uses a temp-backed per-workspace directory" already fails on a clean checkout of main on macOS, before this change. It looks environment-specific and is untouched here. Excluding it, the surrounding suite is 20/21 with the 3 new tests passing. I did not run the full suite locally as it spawns real codex processes and exceeds my local timeout.