feat(shell): support |, &&, ||, ; chain operators in run_command#233
Merged
feat(shell): support |, &&, ||, ; chain operators in run_command#233
|, &&, ||, ; chain operators in run_command#233Conversation
…d-spawn Parse chain operators ourselves with shell-quote and spawn each segment as a separate process — never invoke a system shell. Sidesteps PowerShell 5.1's `&&` parse error and the object-vs-bytes pipe semantics gap on Windows. Each segment is allowlist-checked independently, so `git status | grep main` runs when both halves are individually allowed. Redirects (`>`, `<`, `2>&1`), background `&`, env-var expansion `$VAR`, command substitution `$(…)`, and subshells `(…)` remain rejected up-front with clear errors. Tracks #232, addresses #231.
esengine
added a commit
that referenced
this pull request
May 5, 2026
…t tokenization (#234) Phase 1 (#233) used shell-quote for chain segmentation, but its strict POSIX tokenization split `--flag=1&2` into three tokens with `&` as a background operator — a regression vs. pre-Phase-1 behavior, where the existing lenient `tokenizeCommand` left embedded `&`/`|`/`;` chars as literal bytes inside larger tokens. Replace with a small whitespace-bounded chain splitter that matches the existing `detectShellOperator` convention: chain ops only count when they begin a whitespace-separated token. Each segment then runs through the existing `tokenizeCommand` (lenient) and `detectShellOperator` (rejects `>`, `<`, `&`, `2>&1`, `&>`). Recovers: - `cargo run -- --flag=1&2` — runs (was rejected post-#233) - `git status ; cargo run -- --flag=1&2` — chain runs, second segment keeps `&` Drops: - `git status|grep main` (no spaces) — never worked pre-Phase-1, was a shell-quote-only side effect; align with the project's whitespace-bounded operator convention. Also drops `$VAR` / `$(...)` rejection — these now pass through as literal arguments, matching the existing single-command tokenizer's behavior. Models see the literal `$HOME` in echo's output and learn.
This was referenced May 5, 2026
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.
Closes #232. Addresses discussion #231.
What changes
run_commandnow supports the four most common chain operators:|,&&,||,;. Each segment is parsed viashell-quoteand spawned as a separate process — no system shell is ever invoked.Why not just
shell: trueThe four shells we'd hand the string to disagree on the basics:
|&&||;2>&1PowerShell 5.1 (the default on Windows 10/11) literally parse-errors on
&&, so a shell-passthrough path would breaknode -v && npm testfor every Windows user. Parsing ourselves and spawning each segment sidesteps the whole zoo — semantics are identical on every OS.Granular allowlist (the bit @wviana asked for)
Each segment is allowlist-checked independently:
git status | grep mainauto-runs because both halves are individually allowed. Previously the whole string was checked as one prefix and never matched.Out of scope (rejected with a clear error)
>,<,2>&1,&>) — Phase 2 will implement these as stream piping, still no shell.&— doesn't fitrun_command's synchronous model; userun_background.\$VAR— pass values literally or use the binary's own flags.\$(…)and subshells(…)— split into separate calls.*.ts— passed through as a literal argument; tools likegrep -r/rg/findalready glob themselves.Compatibility note
The strict POSIX parser disagrees with the previous lenient tokenizer in one edge case: an unquoted
&mid-token (cargo run -- --flag=1&2) used to pass through as a single argv. It now rejects with"&" is not supported. The fix is to quote:cargo run -- "--flag=1&2". This is the POSIX-correct form and matches what every real shell would do.Test plan
tests/shell-chain.test.tscovering parser, allowlist, pipe wiring,&&/||short-circuit,;sequence, exit-code propagation, stderr merge, timeouts, dispatch integration.tests/shell-tools.test.tsupdated to reflect chain ops being supported and redirects /&/\$VAR/\$(...)/ empty-segment / trailing-op being rejected.npm run verify— 2198 tests pass, no biome / typecheck errors on changed files.