Skip to content

Fix main's build, and run a negated body unpublished like a chain operand - #467

Merged
tobert merged 3 commits into
mainfrom
fix/negation-drain-ctx
Sep 22, 2026
Merged

tobert merged 3 commits into
mainfrom
fix/negation-drain-ctx

Conversation

@tobert

@tobert tobert commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Merging #465 and then #466 broke main's build. #465 gave drain_stderr_into a ctx parameter, and #466 added three call sites that use the old signature. Each PR was green against a main that did not have the other.

Passing ctx fixes the build, but it exposed two stderr bugs in background jobs, where a fault under ! interacts with #465's publishing model:

f() { ! [[ 1 -eq abc ]]; }; f &              # the diagnostic reached the job stream twice
f() { ! [[ 1 -eq abc ]]; }; timeout 5 f &    # "<diag>\ntimeout: \n", but the foreground printed "timeout: <diag>"

The ! arm ran its body through execute_stmt_flow, which publishes stderr, and it used all of result.err as the error message. The arm now does what the &&/|| arms do with their left side. The body runs through execute_stmt_flow_dispatch, which does not publish. A fault takes only its unpublished stderr as the message, and every other result is published before the drain.

New job-stderr corpus cases cover a negated builtin error, a negated pipeline, a negated fault with and without earlier published stderr, and a timeout-wrapped fault under ! and &&. Going back to the publishing call fails the timeout case, and using the whole err as the message fails the published-body case. No test pins the drain after the body; every enclosing context also drains per statement.

The fix does not cover a pre-existing ordering bug, which is pinned with an ignored test. When a command's $(…) fails, result.err lists the command's own stderr before the substitution's, while the job stream has them in the order they ran. The && arm has it too, so it gets its own PR.

Reviewed with kaibo (DeepSeek); its timeout and ordering findings reproduced, as above.

🤖 Generated with Claude Code

tobert and others added 3 commits September 22, 2026 09:04
#465 gave drain_stderr_into a ctx parameter; #466 added three call
sites under the old signature. Each PR was green against a main
without the other, and the merged main failed to build.

Passing ctx was not enough. Inside a background job, a fault under
`!` published its message twice: the body had already published it,
and the `!` arm took all of result.err as the error message, which
published it again when rendered. The arm now does what `&&`/`||`
do with a left-side fault: publish before draining, and keep only
the unpublished tail as the message.

Corpus cases cover a negated builtin error, a negated pipeline whose
non-last stage reaches the job stream through the drain, and a
negated fault with and without earlier published stderr in its body.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
execute_stmt_flow publishes every statement's stderr as it returns,
the `!` body included, so a second publish in the arm was a no-op.
Removing it fails no test, which is how it showed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A fault under `!` inside `timeout` in a background job rendered as
"<diagnostic>\ntimeout: \n" where the foreground gives
"timeout: <diagnostic>". The body went through execute_stmt_flow,
which publishes, so by the time the `!` arm split off the
unpublished tail it was empty, and `timeout` wrapped an empty error.

The `!` arm now mirrors the chain arms: the body runs through
execute_stmt_flow_dispatch, a fault takes its whole unpublished
stderr as the message, and every other result is published before
the drain.

A pre-existing ordering bug is pinned with an ignored test: a
command's own stderr comes before its substitution's in result.err,
while the job stream has them in run order. The `&&` arm has it too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tobert

tobert commented Sep 22, 2026

Copy link
Copy Markdown
Owner Author

kaibo review (cast deepseek, deepseek-flash) of the ! arm against the job-stderr publishing model:

  • No path through the arm publishes twice, drops stderr, or can trip the publish/drain asserts (split_off leaves stderr_published_len == err.len()).
  • Found: in a job, a negated fault's error message was empty, so timeout rendered a bare timeout: . Reproduced; fixed in d3e60f1 by running the body unpublished.
  • Found: result.err puts a command's own stderr before its $(…)'s, while the job stream keeps run order; && too. Reproduced; pinned as an ignored test for a follow-up PR.
  • Claimed the fault tests could not discriminate the message split; mutation shows reverting it fails the published-body case.

🤖

@tobert
tobert merged commit d25f154 into main Sep 22, 2026
3 checks passed
@tobert
tobert deleted the fix/negation-drain-ctx branch September 22, 2026 13:59
tobert added a commit that referenced this pull request Sep 23, 2026
The rename to allow_unwrapped_commands, the exec/spawn 127 refusal, the
live job stderr stream, the review residuals and statement-level `!`
landed without changelog entries, which kept CHANGELOG.md out of five
PRs' merge conflicts. This records them under Unreleased, in the style
of the entries already there. #467 fixed bugs that never shipped in a
release, so it has no entry.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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