fix: send redirections and expansions to the real shell (#46)

[konard]

Fixes #46.

The problem

git push origin main 2>&1 could report exit code 0 with empty output while the push had actually failed. The report is a symptom of a broader class of bug: any redirection or expansion was silently swallowed whenever the command started with a built-in.

await $`echo hello > out.txt`;
// printed: hello > out.txt      (sh writes the file and prints nothing)
// out.txt: does not exist

Root cause

handleShellMode() in js/src/$.process-runner-execution.mjs only asked needsRealShell() when the command already contained a shell operator:

const requiresRealShell =
  (useShellOps && needsRealShell(command)) || hasShellEscapes(command);

hasShellOperators() recognises &&, ||, ;, & and ( — redirection characters are not in that set. So a command whose first word is a built-in (echo, cat, true, ls, seq, basename, …) went to the in-process implementation, whose arguments come from splitting on whitespace. > and out.txt arrived as two ordinary arguments; 2>&1 arrived as one. Nothing wrote a file, nothing redirected, and the exit status came from the built-in rather than from the real command.

The tell that this was accidental: echo $(echo hi) behaved correctly while echo $HOME did not — purely because ( happens to be in hasShellOperators().

Rust was worse: ProcessRunner dispatched to virtual commands before any real-shell check ran at all.

The fix

Both implementations now decide "does this need a real shell?" independently of which operators happen to be present, and both recognise redirection:

• js/src/$.process-runner-execution.mjs — needsRealShell(command) is consulted unconditionally.
• js/src/shell-parser.mjs — unquoted < and > join the unsupported-feature set. One rule covers >, >>, 1>, 2>, &>, >&, <, 0<, << and <<<; the existing 2>/&>/<< prefix checks became redundant and were dropped. The scan is quote-aware, so echo "a > b" still prints the literal text.
• rust/src/lib.rs — the real-shell check runs before virtual dispatch.
• rust/src/shell_parser.rs — needs_real_shell matches the same character set.

Why route to the shell instead of implementing redirection in the built-ins

Per the review feedback on this PR — "select behavior closer to how it would behave in sh scripts or with least surprise based on best practices from competitors" — /bin/sh is the contract. Partially reimplementing shell semantics in the built-in path is precisely what produced this bug class, and for >/>> both candidate implementations produce byte-identical observable results, so there is no user-visible choice worth a new config knob. The existing shellOperators and enableVirtualCommands/disableVirtualCommands options are unchanged.

Reproducing

node experiments/issue-46-redirection-parity.mjs

The harness runs 22 cases through both /bin/sh and command-stream and diffs stdout, exit code and the resulting directory contents. Before the fix: 16 divergences. After: 0.

Tests

Every case in both new files is asserted against /bin/sh rather than against a hand-written expectation, so the tests encode the parity contract directly.

• js/tests/redirection-silent-failure.test.mjs — 25 tests: 18 redirection cases, 5 expansion cases ($HOME, *, ~, backticks, $(...)), the two quoted cases that must not change (echo "a > b", echo 'a > b'), a needsRealShell unit test, and an offline git push regression test that pushes to a non-existent local path and asserts a non-zero code with fatal: on stdout.
• rust/tests/redirection_silent_failure.rs — 4 tests mirroring the same cases (#![cfg(unix)]), as required by .github/workflows/parity.yml.

Both files were confirmed to fail without the fix (JS: 16 failures; Rust: 3 of 4) and pass with it. The full JS suite shows no regressions against a stashed baseline; cargo test is green; ESLint, Prettier, cargo fmt and cargo clippy -D warnings are clean.

Housekeeping

• Merged main into the branch (resolves the reported conflicts — the branch predated the js/ + rust/ monorepo restructure).
• Removed six stale js/examples/test-git-push-*.mjs scripts and js/tests/git-push-silent-failure.test.mjs, written against the old flat layout; the old test also required network access.
• Release triggers added: js/.changeset/issue-46-redirection-sh-parity.md (patch) and rust/changelog.d/20260905_095834_redirection_sh_parity.md (patch).

[konard]

We need to get latest changes from default branch.

We should double check more cases similar like this, make sure we have test coverage similar to all our competitors, and select behavior closer to how it would behave in sh scripts or with least surprise based on best practices from competitors. If there multiple options we should allow to configure, and use closer to sh behavior by default.

[konard]

🤖 AI Work Session Started

Starting automated work session at 2026-09-05T09:43:11.989Z

The PR has been converted to draft mode while work is in progress.

This comment marks the beginning of an AI work session. Please wait for the session to finish, and provide your feedback.

[konard]

Addressed the review feedback:

"We need to get latest changes from default branch." — main is merged into the branch (commit 61da872). The conflicts came from the js/ + rust/ monorepo restructure that landed after this branch was cut; all files are now at their js/ paths, and src/$.mjs (deleted on main) was dropped.

"We should double check more cases similar like this" — the reported git push ... 2>&1 symptom was already fixed on main, but the defect class behind it was not. needsRealShell() was only consulted when the command also contained &&, ||, ;, & or (, and redirection characters are in none of those. Any command whose first word is a built-in and which contained a redirection or expansion was dispatched in-process with the operators passed as literal arguments. Sweeping the space found 16 of 22 cases diverging from /bin/sh — every redirection form (>, >>, 1>, 2>, 2>&1, <, 0<) plus $VAR, *, ~ and backticks. The giveaway was that echo $(echo hi) worked while echo $HOME did not, purely because ( is in hasShellOperators(). Rust was worse still: it dispatched to virtual commands before any real-shell check ran.

"make sure we have test coverage similar to all our competitors" — 25 new JS tests and 4 new Rust tests. Rather than hand-writing expectations, every case runs the same command through /bin/sh and through command-stream and asserts stdout, exit code and the resulting directory contents match. That makes sh itself the oracle, so the coverage cannot drift from the reference implementation.

"select behavior closer to how it would behave in sh scripts ... use closer to sh behavior by default" — the fix routes redirections and expansions to the system shell rather than reimplementing them in the built-in path. Partial reimplementation of shell semantics is exactly what produced this bug class. Divergences are now 0 of 22.

"If there multiple options we should allow to configure" — I did not add a knob here, and want to flag the reasoning explicitly in case you disagree: for >/>> the two candidate implementations (a built-in redirect writer vs. the real shell) produce byte-identical observable results, so there is no user-visible behaviour to choose between — a flag would only select an invisible implementation detail. The existing shellOperators and enableVirtualCommands/disableVirtualCommands options are untouched. If you would rather have an opt-out that keeps redirections inside the built-in path anyway, say the word and I will add it.

Reproduction harness: node experiments/issue-46-redirection-parity.mjs (16 divergences before, 0 after). Both new test files were confirmed to fail without the fix. All five CI workflows pass on d4fbd50, including the language parity check.

[konard]

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost: $6.507652

📊 Context and tokens usage:

Claude Opus 5: (2 sub-sessions)

1. 117.2K / 1M (12%) input tokens, 37.4K / 128K (29%) output tokens
2. 47.2K / 1M (5%) input tokens, 9.4K / 128K (7%) output tokens

Total: (5.7K new + 149.1K cache writes + 7.1M cache reads) input tokens, 57.5K output tokens, $6.507652 cost

🤖 Models used:

• Tool: Anthropic Claude Code
• Requested: opus (claude-opus-5)
• Thinking level: medium (~15999 tokens)
• Model: Claude Opus 5 (claude-opus-5)

📎 Log file uploaded as Gist (2927KB)

• View complete solution draft log

---

Now working session is ended, feel free to review and add any feedback on the solution draft.

[konard]

🎉 Auto-merged

This pull request has been automatically merged by hive-mind.

• All CI checks have passed

---

Auto-merged by hive-mind with --auto-merge flag