Skip to content

fix: validated turn result outranks child exit code in headless work-on - #140

Merged
bborbe merged 7 commits into
masterfrom
fix/exit-code-vs-transcript
Sep 6, 2026
Merged

fix: validated turn result outranks child exit code in headless work-on#140
bborbe merged 7 commits into
masterfrom
fix/exit-code-vs-transcript

Conversation

@bborbe

@bborbe bborbe commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Fixes a defect where vault-cli work-on discarded headless turns that had actually succeeded.

The bug

runDetachedTurn captures the child's --output-format json blob to a temp file and validates it via validateSessionTurn — but only on the clean-exit path. The exitErr != nil branch returned before the read, so a turn that produced a complete, valid result was thrown away on the strength of the process exit code alone. The task's claude_session_id was then removed by the compensating clear, stranding the transcript where the Vault UI could no longer reach it.

Observed 2026-09-06: a turn ran ~2 minutes, ended stop_reason: end_turn with zero errors and zero permission denials, and the operator got a ~40-line Go stack whose only content was exit status 1.

Why this is a contract change, not a drift fix

docs/work-on-session-lifecycle.md documented the old behaviour as intended — "a non-zero child exit ... return[s] an error, so the caller persists nothing". The doc's own justification is what condemns it: it defends validation because offering a Resume that cannot work lies to the operator. Discarding a session that can be resumed is the same lie inverted, and costs more. The doc changes with the code.

The exit code is also the weaker signal by construction — stderr goes to os.DevNull, so it arrives with no explanation, while the result blob is structured and already validated.

What changed

  • The validated result is authoritative. A non-zero exit is no longer a failure when the result validates; the ignored signal is logged once rather than swallowed.
  • Parsed-but-rejected output leads with the child's own result text and names the failed predicate.
  • The exit status is reported only when the output is missing or unparseable — routed via a new errClaudeOutputUnparseable sentinel.
  • Write-back specs prove the clear-vs-retain consequence through Execute on real vault files, on both the task path and goal_workon.go's separate handleClaudeSession.

Regression lock

The obvious refactor — hoisting os.ReadFile above the select so both branches share it — would let a timed-out child's partial blob validate as success, persisting a resumable id for a turn that is still running. That is strictly worse than the bug being fixed. A test seeds a valid blob into the timeout path and asserts the error still fires; the read is pinned inside the child-exited branch.

Out of scope

Why a clean-end_turn child exits 1 at all is unresolved and deliberately excluded — the point of this fix is that vault-cli must not depend on the exit code being trustworthy. Ruled out during triage: router reachability, CLI flags, --max-turns (inert), and session-id collision (a different, instant failure).

Spec: specs/in-progress/045-bug-exit-code-outranks-validated-turn.md

…code on predicate rejection

The merge of origin/master folded this branch's Unreleased bullets into
v0.125.1, which is already tagged and released, leaving no Unreleased
section for the release watcher to cut. Split them back out; everything
from v0.125.1 down is byte-identical to master.

Also address two code-review findings in runDetachedTurn:
- a non-zero child exit is now logged when the output parsed but failed a
  predicate, mirroring the existing override log rather than dropping the
  signal silently
- document why the json.Unmarshal error is flattened into the message
  instead of chained, so the sentinel stays errors.Is-matchable
@bborbe
bborbe merged commit 2a22687 into master Sep 6, 2026
2 checks passed
@bborbe
bborbe deleted the fix/exit-code-vs-transcript branch September 6, 2026 17:53
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.

1 participant