fix(subagent): retain failed review status after stream stall

[Yeachan-Heo]

What

Preserve a reviewer’s public structured yield when a terminal stream failure occurs, while retaining failed status and error evidence. Add deterministic xAI long-reasoning watchdog coverage and failed-subagent inspect/await coverage.

Why

Fixes #4747. The direct xAI OpenAI-completions 300-second default already exists at the immutable base; this PR verifies that contract and fixes the separate false-success evidence-promotion path.

Testing

• bun test packages/ai/test/idle-iterator.test.ts packages/ai/test/stream-timeout-defaults.test.ts packages/coding-agent/test/task/executor-warnings.test.ts packages/coding-agent/test/task/executor-review-findings.test.ts packages/coding-agent/test/tools/subagent.test.ts packages/coding-agent/test/tools/subagent-await-consumer-failure.test.ts
• bun --cwd=packages/coding-agent run check
• bun --cwd=packages/ai run check
• SSH-signed exact-head commit ad25c508aa3469f2e181aa2099f3d4de71e2d8f9, verified with git log --show-signature.

GJC verdict

---

• Target branch is dev
• Tested locally
• Verdict above matches the exact PR head, not an earlier commit
• Exact-head CI terminal
• Independent approval

Canonical verdict

gajae.pr-review-verdict.v1 merge-approved sha256:20f9f5e3470812c5e756ce614d0080add22d687cc004c065d468ebea005496d9 reviewer:human reviewer-id:probepark evidence:github-review-4979337021-exact-head-35f8210b42610fa5007832d4e3512211db6791b0-ssh-signature-verified

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Yeachan-Heo]

OWNER_CONFIRMATION_REQUIRED — exact head ad25c508aa3469f2e181aa2099f3d4de71e2d8f9 has one canonical needs-human verdict with the verified three-dot SHA-256 faadc5d118f1fe3a4acbc8cf7485b35fe97ee323a2c201089f96d60af4e7ae4e. git verify-commit verifies its SSH signature. All affected-path, native-build, test, type-check, virtual-integration, and state-gate jobs in Dev CI run 32328081510 are terminal green. The remaining failing checks are the base-owned PR-contract/bootstrap gates, correctly rejecting needs-human until an eligible independent exact-head approval changes the contract to merge-approved. Reviewer @probepark was requested and has write permission; no approval exists. No merge or issue close is authorized.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[probepark]

Independent maintainer review at exact head ad25c508 — approved.

the bug and the fix

finalizeSubprocessOutput unconditionally reset exitCode to 0 and cleared stderr on any valid structured yield. So a review that produced public output and then hit a terminal stream failure was promoted to success — the worst possible direction for a failure to be wrong in.

Head passes done.error || done.aborted as an explicit terminal fact and keeps the nonzero exit and error while still swapping private raw output for the policy-safe yield (task/executor.ts:620-689,2191-2197). That is the right shape: the yield governs what is shown, the terminal fact governs whether it succeeded, and they no longer overwrite each other.

I traced the retain-too-much direction, since that is the other way this goes wrong

A retained failure that outlives its cause would be just as bad. It does not:

• A valid yield on a healthy run still clears the provisional nonzero code and succeeds.
• A valid yield after done.error/done.aborted retains the terminal exit and error.
• Schema-invalid and placeholder yields stay failures under existing validation.
• Cancel after a settled failure is a no-op (only running/paused/queued are cancellable), so the status stands.
• Pause after failure returns not_running without mutating.
• Resume without a message deliberately leaves terminal status and returns guidance.
• Resume with a message registers a fresh job, moves the predecessor to historicalJobIds, clears terminalGeneration, and repoints currentJobId/status (async/job-manager.ts:1583-1744). A successful run then writes completed and resultText.

So stale failure does not survive a later success, and the failure propagates through the task receipt and async job mapping as failed/errorText. After job eviction the canonical failed status remains on the record, and real executor runs keep the public payload in the agent:// artifact even though inline errorText belonged to the evicted job.

nit — two of the three new tests are guards

test/task/executor-warnings.test.ts:139-156 is the genuine pin: without the source change the old code sets exit 0 and clears stderr, so its final assertions fail.

test/tools/subagent.test.ts:94-124 synthesizes { kind: "failed" } directly, so it passes on the parent — AsyncJobManager already retained an explicitly returned failed outcome. packages/ai/test/idle-iterator.test.ts:81-121 supplies 300,000 ms explicitly and also passes on the parent; the existing stream-timeout-defaults.test.ts is what actually pins the xAI/Grok 300-second default.

Worth adding the lifecycle case that is currently unpinned: drive a terminal failure through finalization, resume with a message, and assert the current snapshot becomes running/completed with result text rather than stale error text. That is the clearing behavior I checked by reading, and it deserves a test.

_{Reviewed by @probepark — method: detached worktree at ad25c508, read of the finalization change against the previous unconditional reset, enumeration of every lifecycle transition that could retain or clear the failure (cancel, pause, resume with and without message, eviction), per-test base-vs-head discrimination. Tests not executed.}

gajae.pr-review-verdict.v1 merge-approved sha256:faadc5d118f1fe3a4acbc8cf7485b35fe97ee323a2c201089f96d60af4e7ae4e reviewer:human reviewer-id:probepark evidence:exact-head-ad25c508-terminal-fact-no-longer-overwritten-by-yield-and-resume-clears-stale-failure

[probepark]

Re-approved at exact head fee06539. My earlier approval was bound to ad25c508; the contract correctly invalidated it when the head moved. The new commit is a strict improvement.

Failed awaits are now visible (subagent-render.ts): the title becomes Subagent failed and the summary reports N subagents failed rather than a neutral count. That matters for this PR specifically — retaining the failed status is only useful if the failure surfaces.

progress is removed from await receipts (subagent.ts):

// AgentProgress includes model-generated deltas, tool arguments, nested
// task details, and arbitrary tool output. None is an approved public
// subagent payload, so await receipts expose only liveness.
return { liveProgressAvailable: true };

That closes a leak I did not catch last round: AgentProgress carried model-generated deltas, tool arguments, nested task details and arbitrary tool output into a parent-visible receipt. Reducing it to a liveness bit is the right call, and terminal public output still flows through the bounded result/error receipt and agent://.

Verified locally at this head: bun test packages/coding-agent/test/tools/subagent.test.ts packages/coding-agent/test/tools/subagent-render.test.ts packages/coding-agent/test/task/executor-warnings.test.ts — 100 pass, 0 fail.

Everything from my prior review still holds: the terminal fact is no longer overwritten by a valid yield, and resume-with-message clears stale failure rather than retaining it.

_{Reviewed by @probepark — method: detached worktree at fee06539, diff of the delta against my previously approved ad25c508, read of the receipt reduction against the AgentProgress shape, focused test run in a worktree with natives built.}

gajae.pr-review-verdict.v1 merge-approved sha256:20f9f5e3470812c5e756ce614d0080add22d687cc004c065d468ebea005496d9 reviewer:human reviewer-id:probepark evidence:exact-head-fee06539-delta-surfaces-failed-awaits-and-removes-agentprogress-from-receipts-100-pass

[probepark]

Re-approved at exact head 35f8210b — rebase only.

I diffed the PR's own two commits at this head against the same two at my previously approved fee06539, ignoring index and hunk headers: byte-identical. The head moved only because the branch was rebased onto dev and picked up #4612 (d97b79ef), which I merged a few minutes ago.

Nothing to re-verify. The prior approval's reasoning carries: the terminal fact is no longer overwritten by a valid yield, resume-with-message clears stale failure rather than retaining it, failed awaits now surface in the renderer title and summary, and AgentProgress no longer rides into await receipts.

Focused suites were 100 pass / 0 fail at the pre-rebase content.

_{Reviewed by @probepark — method: content-equality check of the PR's commits at fee06539 and 35f8210b against their respective parents, confirming a pure rebase over the newly merged #4612.}

gajae.pr-review-verdict.v1 merge-approved sha256:20f9f5e3470812c5e756ce614d0080add22d687cc004c065d468ebea005496d9 reviewer:human reviewer-id:probepark evidence:exact-head-35f8210b-rebase-only-content-byte-identical-to-approved-fee06539

[Yeachan-Heo]

MERGE_READY — merged to dev as dadaf198804b13d5cff3a19767b5418e43dbf649. Exact reviewed head 35f8210b42610fa5007832d4e3512211db6791b0 carried canonical three-dot SHA-256 20f9f5e3470812c5e756ce614d0080add22d687cc004c065d468ebea005496d9; probepark (write) approved that exact head in review 4979337021, and its SSH signature was verified. Current-head Dev CI 32339420717 and exact-head PR contract 32339420913 passed. Fresh merged-dev bun run build passed. No release, tag, or publish action occurred.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]