fix: surface Codex headless failures instead of awaiting input

[rpelevin]

Summary

Refs #311.

Codex headless sessions can report a sandbox or approval-policy write block in assistant output while the process otherwise completes. Construct then returned to awaiting_input without surfacing an actionable failure.

This PR:

• detects only assistant-authored lines beginning with Blocked:, excluding echoed user prompts and benign prose;
• recognizes the bare codex agent-section marker in real plain codex exec output;
• unwraps structured response_item.payload messages, preserves their roles, and emits clean text instead of raw JSON fallback;
• emits an actionable Error while keeping the adapter alive so the user can retry;
• avoids closing the adapter immediately after the error, removing the event/closed race observed in review;
• preserves the current routed-model ERROR: stderr handling from main;
• keeps the change scoped to failure signaling; SetApprovalMode translation remains a separate follow-up.

Validation

• cargo +1.88.0 test -p construct-adapter-codex -p construct-adapter-common
  • 37 Codex-adapter tests passed
  • 13 adapter-common tests passed
• cargo +1.88.0 check -p construct-adapter-codex -p construct-adapter-common passed
• cargo +1.88.0 build -p construct-adapter-codex -p construct-adapter-common --locked passed
• rustfmt +1.88.0 --edition 2021 --check crates/adapter-codex/src/lib.rs passed
• Targeted all-target Clippy passed with the pre-existing too_many_arguments lint allowed.
• The prior upstream CI run on 7c33cd0 reached the full suite; all then-current adapter tests passed and the sole failure was the unrelated construct-e2e split-layout timing assertion at split_layout.rs:584.

[edwin-zvs]

Thanks for this — and welcome! The problem is real, the diagnosis is right, and the plumbing (threading the child status through TurnOutcome, updating all six call sites, unit tests on the pure helpers) is clean.

I verified the branch end-to-end rather than just reading it: built 7de47c3 in a worktree and drove it against isolated daemons with a fake codex reproducing the real codex exec stdout shape (copied from an actual codex 0.146.0 run), with main @ 4c1459b as the baseline. Three findings below, then some smaller notes.

1. The detector fires on the user's own prompt

Real codex exec echoes the prompt verbatim on stdout:

--------
user
the sandbox is read-only, so write index.html anyway

blocked_write_reason runs on every raw stdout line, so that echo is scanned. A session whose prompt was "Please create index.html. Note the sandbox is read-only, so the write may fail." — pretty much what someone hitting #311 would type — produced this on a fully successful turn (exit 0, file written):

{"type":"error","message":"Codex headless turn was blocked by its sandbox or approval policy: Please create index.html. Note the sandbox is read-only, so the write may fail.. Configure Codex's own headless permissions and retry; …"}
{"type":"status","state":"errored", …}
{"type":"done","exit_code":1}

Benign agent prose does the same: "I checked whether the sandbox is read-only before writing index.html; it is writable, so the file was written." → session errored and terminated. main handles both correctly (awaiting input, session alive).

Suggestion: run detection on the parsed assistant text rather than the raw line (that also stops the 600-char excerpt from being raw JSON), and tighten the predicate — an anchored Blocked: beats a three-way keyword AND. The existing negative test only covers lines missing a keyword; the case that bites is a line that has all three innocuously.

2. Ending the session removes the user's ability to retry

After a triggered error, construct send <id> returns daemon error: session has no live adapter. That also applies to a plain nonzero exit with no sandbox involvement — a rate limit or dropped stream, which I simulated as exit 1 — where main returns to awaiting input and the user just resends. Spec 0009-transient-provider-errors-are-retryable argues against making those fatal, and adapter-hermes (same one-shot shape) emits Error and keeps looping.

Note SessionEvent::Error already drives the session to Errored in the daemon (crates/daemon/src/session/events.rs:496), so the explicit Status{Errored} emit is redundant.

3. The fix currently works ~40% of the time

Running the PR's intended case (Blocked: the workspace is read-only, so index.html could not be written, exit 0) ten times across two batches:

Outcome	Runs
errored + error event (fix works)	4/10
done ✓, zero error events, transcript ends at the blocked line	6/10

So most of the time the failure is still swallowed — now as a green done ✓, which reads as success.

The cause is a pre-existing daemon race that this PR's "emit, then exit the process immediately" shape walks straight into: the adapter's reader task (crates/daemon/src/adapter.rs:123) and the child-wait task that sends Closed (:200) are separate senders on one channel, and drain_adapter's Closed arm breaks — discarding events still queued behind it — then derives terminal state from the adapter process's exit code, which is Some(0) even on this error path.

Proof: adding a 300 ms sleep after the final Done emit makes it 6/6 correct; removing it brings the flakiness back. Filed separately as #1082.

Happily, fixing (2) mostly dissolves (3): if the adapter emits Error and keeps looping instead of exiting, there's no exit to race.

Smaller notes

• Fixes #311 will auto-close an issue this only partly addresses — SetApprovalMode is still ignored (crates/adapter-codex/src/lib.rs:1175), which is the part that makes writes actually work. Refs #311 would be more accurate.
• The TurnOutcome signature change isn't required: tokio caches the exit status (FusedChild::Done), so a second child.wait().await after drive_turn returns it again — adapter-hermes already relies on this. I confirmed with a scratch test (drive_turn to completion, then wait() → Some(7)). Not a defect, just FYI that the five-adapter churn was optional; the explicit payload is arguably clearer, so keep it if you prefer.
• .max(1) silently rewrites exit 0 → 1; worth a one-line comment.
• trimmed.chars().take(600) duplicates construct_adapter_common::short.
• First-blocked-line-wins sits right below the session_id logic that deliberately keeps the last value; a comment would keep the asymmetry from reading as accidental.
• The error strings explain construct's internals to the user ("the session is errored instead of awaiting input") — I'd cut that clause.
• spawn_stdout is generic over AsyncRead, so a test feeding it a fake reader would cover the diagnostics plumbing end of this cheaply.

For reproducing: point CONSTRUCT_CODEX_CMD at a script that prints the codex header block, echoes the prompt, then prints your chosen outcome line and exits — with CONSTRUCT_CODEX_MODE=headless and an isolated CONSTRUCT_*_DIR, that reproduces all of the above deterministically without touching the real CLI. Glad to share the exact scripts if useful.

Again, nice first contribution — the detection idea is sound, it's the input it's fed and what it does afterward that need adjusting.

[rpelevin]

CI reached the full suite. All 35 construct-adapter-codex tests passed; the only failure is the unrelated construct-e2e shared_split_layout_renders_wide_and_is_read_only_narrow assertion at split_layout.rs:584. This PR changes only crates/adapter-codex/src/lib.rs, and the same split-layout test passed on base f2ab6e6 and current main, so this appears to be a timing flake. I do not have upstream Actions rerun permission. Could a maintainer rerun the failed job?

[edwin-zvs]

It seems the first PR need to be merged to get Action rerun permission, let's see if your next PR triggers CI action.
And right. e2e test failure seems flaky and unrelated to this PR.

Just to make sure, could you check these two?

1. The detector can't fire on real codex output. Plain text labels the agent section codex with no -------- before it, so the section never leaves User; the structured path looks at the top level of a response_item envelope whose message, role, and content are all one level down under payload.
2. Non-assistant structured messages now render as raw JSON instead of clean text — in the try_emit_structured refactor.

[rpelevin]

Thanks — both were real, and they are fixed locally in 533af56.

• Plain mode now recognizes the bare codex marker that starts the agent section after the echoed user prompt, so an anchored assistant Blocked: line is detected on the real output shape.
• Structured mode now unwraps response_item.payload, preserves message roles, and emits recognized non-assistant messages as clean text instead of raw JSON.

I added fake-reader regressions for both shapes. They fail on 7c33cd0 for the reported reasons and pass on 533af56. The full focused verification passes: 37 adapter tests, 13 shared tests, package check/build, file-level rustfmt, and targeted all-target Clippy.

I kept the change scoped to the parser feedback; it does not touch SetApprovalMode or the unrelated split-layout test.

[edwin-zvs]

Thanks @rpelevin for the contribution!! let's go!