Repository navigation
fix(shell_exec): stream rather than fail when delayed_stream's timed wait fails - #3881
Merged
Merged
Conversation
…wait fails Phase 1's `Err` arm returned to the caller after joining the reader threads, and those sit in `read_to_end` until the child closes its pipes -- so the caller waited out the child's full runtime and then got `Failed to wait for command` for a command that had already finished. The arm became reachable in #3857: `shared_child` allocates a pipe and registers a SIGCHLD handler on every timed wait, where `wait-timeout` set its self-pipe up once per process, so a sandbox or an fd limit now surfaces as `Err` where it used to surface as the abort of #3856. It now falls through to streaming, like an exceeded threshold. Phase 2's `wait()` is a bare `waitid(WNOWAIT)` loop with neither a pipe nor a signal registration, so whatever broke the timed wait can't reach it and the real exit status still comes back. The wall-clock sites tear the child down instead, because a wait they can't observe bounds nothing; the module docstring records that split. `test_cmd_delayed_stream_crosses_the_threshold` covers the fall-through, which no test reached before: the suite's thresholds are `0`, which streams without calling `wait_timeout`, and `-1`, which skips phase 1.
The first version asserted elapsed time, which a phase-1 `Ok(Some)` satisfies just as well: the child runs the same wall-clock either way, so the assertion held whether or not the arm under test was taken. A line the child writes after the switch is the discriminator. Streaming sends it to stderr, so the error's buffer is empty; had phase 1 returned a status, `late` would still be buffered. Raising the threshold above the child's runtime fails the assertion with `left: "late"`.
worktrunk-bot
approved these changes
Aug 23, 2026
worktrunk-bot
left a comment
Collaborator
There was a problem hiding this comment.
Two non-blocking notes; the change itself reads right, and the rewritten test that pins the switch by where the late output lands is a much stronger assertion than the elapsed-time one.
Cmd::delayed_stream's own docstring still glosses the progress message as printing "at the moment streaming starts (the delay threshold is crossed)". After this change a failed timed wait starts streaming too, so the parenthetical names one of two triggers — the module docstring above records the split, but the item docstring is what a caller reads. It's outside the diff, so no inline suggestion; happy to push the one-liner if you want it.
A failed timed wait now starts streaming too, so naming the threshold as the moment it prints describes one of two triggers. The module docstring above already records when each happens.
150 ms between the threshold and the child's echo races the scheduler, which tests/CLAUDE.md warns against; 450 ms covers a deschedule longer than the suite produces. The threshold stays at 50 ms rather than shrinking: a spent `remaining` skips the wait, and the test would then pass without reaching the arm it covers.
1 task
social4hyq
pushed a commit
to social4hyq/homebrew-core
that referenced
this pull request
Sep 20, 2026
worktrunk 0.76.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## Release Notes ### Improved - **`wt switch --execute` takes a program, not a shell string**: `-x` names one program, with everything after `--` passed as literal argv. Worktrunk spawns it as a child rather than running it in your interactive shell, so shell functions and its `cd` no longer reach you. `-x sh -- -c '…'` recovers shell syntax, not functions. (Breaking: existing `-x` strings, plus `WORKTRUNK_DIRECTIVE_EXEC_FILE` and `WORKTRUNK_SHELL`.) ([#3977](max-sixty/worktrunk#3977), closes [#2860](max-sixty/worktrunk#2860), fixes [#3944](max-sixty/worktrunk#3944), thanks @omgreenfield for testing the migration path) - **Worktrunk decides tracking for the branches it creates**: a new branch gets an upstream only when its name matches the remote branch it starts from, whatever `branch.autoSetupMerge` says: `--create release --base origin/release` tracks, `--create feature --base origin/release` does not. Under `autoSetupMerge = false` this previously exited 128 after creating the worktree. ([#3913](max-sixty/worktrunk#3913), [#3950](max-sixty/worktrunk#3950), fixes [#3937](max-sixty/worktrunk#3937), thanks @mjakl for reporting and diagnosing) - **Retired config keys are no longer read**: `commit.generation.template-file` and `squash-template-file` are gone — put the file's contents in `template` or `squash-template`. There is no migration: the key stays, warns as unknown on every load, and the prompt reverts to the built-in default. Setting both `template` and `template-file` used to fail the load; it now loads `template`. `switch.picker.timeout-ms` likewise warns rather than being stripped. (Breaking.) ([#3949](max-sixty/worktrunk#3949)) - **Project aliases and hooks can use `wt switch --execute`**: their bodies previously refused it. Worktrunk now starts the program itself instead of handing shell text to your shell, and the command-approval gate remains the control on project-defined commands. ([#3977](max-sixty/worktrunk#3977)) - **Established automation and LLM customization interfaces are now stable**: `wt step eval`, `wt step for-each`, `wt step prune`, LLM branch summaries, `wt config state vars`, and `commit.generation.template-append` are no longer marked experimental. ([#3949](max-sixty/worktrunk#3949)) - **`wt step prune` stages removals concurrently again**: only the `git worktree remove` teardowns serialize, so dirty checks, fsmonitor shutdown, and the trash rename overlap. The `prune_e2e/live` benchmark median went from 377 ms to 222 ms. ([#3954](max-sixty/worktrunk#3954)) - **`wt config approvals add` reads as a review rather than a warning**: it opens with a cyan `Review 1 command for repo:` instead of a yellow `▲ … needs approval to execute`. The execution-time gate is unchanged. ([#3953](max-sixty/worktrunk#3953)) ### Fixed - **The `wt switch` picker survives a preview longer than 65,535 lines**: skim keeps the pane's line count in a `u16`, so a larger diff aborted `wt` with exit 101 seconds after it painted — on screen the picker looked like it closed by itself. Panes now cap at 60,000 lines and say so. ([#3959](max-sixty/worktrunk#3959), fixes [#3958](max-sixty/worktrunk#3958), thanks @sandertammesoo for reporting and diagnosing) - **The nushell wrapper propagates exit codes without a POSIX shell**: it reported a failing `wt` by spawning `sh`, so on Windows every failing command printed a ``Command `sh` not found`` trace and exit 1 whatever the real code was. Nushell's wrapper is a static file, so rerun `wt config shell install` to pick this up. ([#3945](max-sixty/worktrunk#3945)) - **The picker no longer shows one detached worktree's diff for another**: previews were keyed by branch name, so every detached row shared the key `(detached)`. Rows now key by canonical identity, which also lets `wt remove feature ~/repo.feature` plan that worktree once instead of failing on the second. ([#3926](max-sixty/worktrunk#3926)) - **`wt switch pr:<n>` gives `pre-switch` hooks the real branch**: `branch` and `target` held the literal `pr:3933`, and `target_worktree_path` was unset even when that branch had a worktree. `pr_number` and `pr_url` are now set for same-repo PRs, not only forks. ([#3941](max-sixty/worktrunk#3941), fixes [#3934](max-sixty/worktrunk#3934), thanks @robsonpeixoto for reporting) - **`wt config shell install` no longer replaces an rc file created while it was checking**: the missing-file path refuses to clobber and fails with a rerun hint. A dangling rc symlink is preserved and rejected rather than overwritten. ([#3929](max-sixty/worktrunk#3929)) - **`wt switch --create X --base pr:<n>` works when the PR branch has no local branch**: the fetch writes only the remote-tracking ref, and git won't expand a bare name to one, so the command failed with `No branch, tag, or commit named …`. The base is now named under its remote. ([#3951](max-sixty/worktrunk#3951)) - **`wt switch --create` streams `git worktree add` instead of stalling**: where a sandbox denied the timed wait's pipe or `sigaction`, the command waited out git's entire run and then reported `Failed to wait for command` for work that had already succeeded. ([#3881](max-sixty/worktrunk#3881)) - **Claude Code activity markers stay on the session's launch worktree**: the plugin's marker hooks pass `-C "$CLAUDE_PROJECT_DIR"`, so a `cd` mid-session no longer marks another repository. A session launched outside a repository gets no marker. ([#3956](max-sixty/worktrunk#3956)) ### Documentation - **`--execute` documents how to get a variable into a shell body**: each position is substituted verbatim, so a variable spliced into `sh -c` text is re-parsed by that shell and splits on spaces. The help now shows passing it as a separate argument and referencing it positionally. ([#3977](max-sixty/worktrunk#3977)) - **The clone-local default-branch override is documented**: `wt config state default-branch set` writes `worktrunk.default-branch` to the clone's local git config — machine-local, never committed, and shared by every linked worktree. `set` warns when the branch isn't checked out locally, and `wt list` skips its comparisons until it is. ([#3947](max-sixty/worktrunk#3947), [#3948](max-sixty/worktrunk#3948), fixes [#3946](max-sixty/worktrunk#3946), thanks @danielo515 for the request) - **The docs site renders code with the CLI's own colors**: terminal examples reproduce each snapshot's exact ANSI roles, while command references and the homepage command comparison get clap and shell syntax color from paired light/dark themes. File excerpts gain a path tab, and code wraps instead of scrolling on mobile. ([#3936](max-sixty/worktrunk#3936), [#3938](max-sixty/worktrunk#3938), [#3939](max-sixty/worktrunk#3939), [#3942](max-sixty/worktrunk#3942)) - **The `pre-start` upstream example uses `--base`**: it showed `wt switch --create feature origin/feature`, which `wt switch` rejects. ([#3955](max-sixty/worktrunk#3955)) ### Internal - **Library API rework** (Breaking library API): `cargo-semver-checks` fails ten lints — `RemoteRefProvider` and its provider structs give way to `ForgeKind` dispatch, the `--execute` rework retires the fish and PowerShell `ShellEscapeMode` escapes and `WORKTRUNK_SHELL_ENV_VAR`, `BranchRef` swaps `full_ref`/`worktree_path` for `id`, and accessors including `Branch::unset_upstream` and `ProjectConfig::ci_platform` are gone. ([#3913](max-sixty/worktrunk#3913), [#3926](max-sixty/worktrunk#3926), [#3949](max-sixty/worktrunk#3949), [#3963](max-sixty/worktrunk#3963), [#3977](max-sixty/worktrunk#3977)) - **Hook pipelines and `wt list` collection lost two indirection layers**: the background pipeline carries `PreparedStep` directly, and 15 marker task types collapse into one exhaustive dispatch. The synthesized-`--force` submodule path keeps its dirty re-check and destructive command under one lock, so prune's narrower locking doesn't widen that window. ([#3964](max-sixty/worktrunk#3964), [#3954](max-sixty/worktrunk#3954)) ## Install worktrunk 0.76.0 ### Install prebuilt binaries via shell script ```sh curl --proto '=https' --tlsv1.2 -LsSf https://github.com/max-sixty/worktrunk/releases/download/v0.76.0/worktrunk-installer.sh | sh && wt config shell install ``` ### Install prebuilt binaries via powershell script ```sh powershell -ExecutionPolicy Bypass -c "irm https://github.com/max-sixty/worktrunk/releases/download/v0.76.0/worktrunk-installer.ps1 | iex"; git-wt config shell install ``` ### Install prebuilt binaries via Homebrew ```sh brew install worktrunk && wt config shell install ``` ## Download worktrunk 0.76.0 | File | Platform | Checksum | |--------|----------|----------| | [worktrunk-aarch64-apple-darwin.tar.xz](https://github.com/max-sixty/worktrunk/releases/download/v0.76.0/worktrunk-aarch64-apple-darwin.tar.xz) | Apple Silicon macOS | [checksum](https://github.com/max-sixty/worktrunk/releases/download/v0.76.0/worktrunk-aarch64-apple-darwin.tar.xz.sha256) | | [worktrunk-x86_64-ap See merge request: Harmonybrew/homebrew-core!18482
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Cmd::delayed_streamwaits out a delay threshold before it starts streaming a child's output. When that timed wait returned an error rather than a status, the function returned the error to the caller — but only after joining the two reader threads, and those sit inread_to_enduntil the child closes its pipes. So the caller waited out the child's full runtime and then gotFailed to wait for commandfor a command that had already finished.That arm became reachable in #3857.
shared_childallocates a pipe and registers aSIGCHLDhandler on every timed wait, wherewait-timeoutset its self-pipe up once per process. A sandbox or an fd limit that used to abort the process (#3856) now surfaces as anErr.Change
Phase 1's
Errfalls through to streaming, which is what an exceeded threshold already does. Phase 2's blockingwait()is a barewaitid(WNOWAIT)loop with neither a pipe nor a signal registration, so whatever broke the timed wait cannot reach it and the real exit status still comes back. A command whose deadline machinery fails now streams its output instead of failing.The two sites where the deadline bounds wall-clock,
run_with_timeout_impland the pager, foldErrinto their teardown arm instead, because a wait they cannot observe bounds nothing. The threshold here only decides when output starts streaming, so it streams. The module docstring records that split.Testing
test_cmd_delayed_stream_crosses_the_thresholdis new, and it covers an arm nothing reached before. Of the thresholds the suite used,0streams without ever callingwait_timeout,-1disables phase 1, and5_000against a fast child returnsOk(Some)— soOk(None)never happened.The child writes a line 450 ms after the threshold passes. That line is the discriminator: streaming sends it to stderr, so the error's buffer comes back empty, where a phase 1 that had returned a status would still have it buffered. Checked by raising the threshold above the child's runtime, which fails the assertion with
left: "late". Elapsed time would not have distinguished the two, since the child runs the same wall-clock either way.The
Errarm itself stays untested: reaching it needspipeorsigactionto fail inside the wait, and it shares every line with the fall-through the new test covers.Also run:
cargo run -- hook pre-merge --yes(4677 tests, lints, doctests) and both rustdoc variants with-D warnings.