feat(shell): support file redirects in run_command#235
Merged
Conversation
…2>`, `2>>`, `2>&1`, `&>`) Phase 2 of the chain-aware run_command rewrite. Each segment now parses trailing redirects via a single-pass walker (no shell), opens files with fs.openSync, and hands the resulting fds to spawn's stdio array. Pipe wiring still applies — `<` overrides pipe-stdin only when explicitly present; `>`/`>>`/`&>` override the pipe-out / capture-buffer wiring. Supported per segment (max one per fd): - `>` stdout truncate - `>>` stdout append - `<` stdin from file - `2>` stderr truncate - `2>>` stderr append - `2>&1` merge stderr → stdout (file fd, pipe, or our buffer, whatever stdout points to) - `&>` both → file (truncate) Targets resolve relative to the chain's cwd. Multiple redirects to the same fd are rejected with a clear conflict message. ChainSegment now carries `redirects: Redirect[]` alongside `argv`. The old `tokenizeCommand + detectShellOperator` per-segment combo is replaced with `parseSegment`, a quote-aware single-pass walker that emits both argv tokens and redirects at the same level — keeps the lenient embedded-`&`/`|` handling intact (the `cmd1 ; cmd2 -flag=1&2` regression fix from #234 still passes). Heredoc `<<` is rejected explicitly. Background `&` is rejected explicitly (use run_background, not run_command). `$VAR` and `$(…)` pass through as literal characters — no shell expansion, matching the existing single-command tokenizer. Adds 30 new tests in tests/shell-redirects.test.ts covering parser output and execution; updates the obsolete redirect-rejection tests in tests/shell-tools.test.ts to cover the new rejection set (background `&`, heredoc `<<`, missing target, multi-redirect-per-fd).
…pipe `2>&1` `cmd1 2>&1 | cmd2` was wiring prev.stdout into child.stdin with default end-on-source-end, then piping prev.stderr in with `end: false`. When stdout finished first, child.stdin closed and any subsequent stderr writes hit a closed stream — output got truncated. Both branches now pipe with `end: false` and a 2-counter closes stdin only after both source streams emit `end`. Adds a test that pipes stderr through to a downstream cat-like process and asserts both streams reach the destination.
esengine
added a commit
that referenced
this pull request
May 5, 2026
`run_command` learns the four common shell chain operators (`|`, `||`, `&&`, `;`) and the seven file redirect operators (`>`, `>>`, `<`, `2>`, `2>>`, `2>&1`, `&>`). Parsed and spawned natively — no shell is invoked, so semantics are identical on Windows / macOS / Linux; PowerShell 5.1's `&&` parse error and the object-vs-bytes pipe gap are sidestepped. Each chain segment is allowlist-checked independently: `git status | grep main` now auto-runs when both halves are individually allowed, the granular-approval behaviour wviana asked for in discussion #231. The chain parser stays consistent with the project's long-standing lenient tokenizer — `cargo run -- --flag=1&2` and similar embedded- operator args stay literal instead of getting POSIX-strict-rejected. shell-quote is no longer a dependency; `splitOnChainOps` is whitespace- bounded like the existing `detectShellOperator`. Mid-pipe `2>&1` correctly merges stderr into the next segment's stdin without truncating on stdout-end (counter-based close-when-both-ended). Targets resolve relative to the project root. Heredoc `<<` and background `&` remain explicitly rejected with clear errors. `\$VAR` and `\$(...)` pass through as literal characters — no shell expansion. Closes #232. Driven by discussion #231. PRs: #233 (chain ops), #234 (lenient-tokenizer regression fix), #235 (redirects + mid-pipe `2>&1`).
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.
Phase 2 of #232. Builds on #233 + #234.
What changes
run_commandnow parses and executes file redirects natively. Each segment of a chain (or a standalone command) can carry trailing redirects; the parser reads them, the runner opens the files withfs.openSyncand hands the fds to spawn's stdio. No shell is invoked — semantics are identical on Windows / macOS / Linux.>>><2>2>>2>&1&>Targets resolve relative to the chain's cwd. At most one redirect per fd per segment — multiple
>s in one segment are rejected with a clear conflict message.Examples
Why this didn't ship in #233
shell-quote's POSIX strictness conflicted with the project's lenient tokenizer (#234 fixed that by going back to whitespace-bounded chain detection). With the parser layer now consistent with the project's existing primitives, redirects fit cleanly: same single-pass walker, same quote awareness, same lenient handling of embedded
&/|/;chars.Implementation
ChainSegmentnow carriesredirects: Redirect[]alongsideargv. NewparseSegment(segStr)is a single-pass walker that emits both argv tokens and redirect ops at the same level. Old per-segmenttokenizeCommand + detectShellOperatorcombo is gone — replaced by the unified parser.runPipeGroupopens redirect fds per segment viafs.openSync(always closed infinally), then hands them to spawn's stdio. Pipe wiring still applies:<overrides pipe-in,>/>>/&>override pipe-out / capture-buffer.2>&1is implemented by setting stderr's stdio spec to whatever stdout's points at — works for buffer-capture, fd, and pipe equally.Compat notes
&still rejected — userun_background.<<still rejected — use<file or the binary's flag.\$VARand\$(...)pass through as literal characters (matches the simple-command tokenizer's pre-existing behavior).&/|/;behavior from fix(shell): preserve lenient tokenization in chain segments #234 is preserved (cargo run -- --flag=1&2still works).Tests
tests/shell-redirects.test.ts— 30 new tests covering parser output for every redirect kind, stuck-to-target form, quoted targets, conflict detection, and execution (truncate/append, stdin-from-file, stderr split,2>&1merge,&>both, redirects across pipe boundaries).tests/shell-tools.test.ts— old "rejects stdout redirect / stderr-merge" tests rotated out; added rejections for missing target, heredoc, multi-redirect-per-fd.tests/shell-chain.test.ts— old "rejects redirects inside a chain segment" test repurposed to assert successful parsing.npm run verify— 2228 tests pass, no biome / typecheck errors on changed files.