Closes https://github.com/vercel-labs/just-bash/issues/635
Stream completed statements from a first-stage for loop with static words and no outer redirections. Reuse the existing loop executor and pipeline, preserving control-flow status, diagnostics and isolated shell state.
Draft while #453 remains unmerged. This branch is rebased onto current main with the shared prerequisite fixes. Focused follow-up diff. The full GitHub diff includes the prerequisite; it should be rebased again once that prerequisite lands.
Validation: focused behavior tests, TypeScript, scoped Biome, knip and banned-pattern checks pass. No manual workflow rerun or review-bot request was made.
Activity
13🤖 auto-maintain review
Automated, advisory triage for @asafyish's PR. Facts below are read from the GitHub API.
| Check | Result |
|---|---|
| Author's merged PRs (this repo) | 1 |
| Account established | ✅ (age 5392d · 13 followers · 7 public repos) |
| Commits signed/verified | ⚠️ 0/12 |
| Changeset included | ✅ (.changeset/streaming-pipelines.md) |
Review panel: 🟡 medium highest severity
just-bash maintainer code review: 🟡 medium
Streaming works on success, but failure propagation corrupts partial non-ASCII output.
- packages/just-bash/src/interpreter/streaming-pipeline.ts:319 — Fatal pipeline errors lose the accumulator's `stdoutKind: "bytes"`: `prependTo` copies Latin-1-shaped bytes into `ControlFlowError.stdout`, which is returned as text. Thus partial non-ASCII output emitted before a streaming stage fails is mojibake (and its accounting uses the wrong UTF-8 length). Preserve/decode the byte kind when propagating failures.
General code review: 🟡 medium
Streaming overrides cannot safely delegate piped input to non-streaming original commands.
- packages/just-bash/src/interpreter/builtin-dispatch.ts:947 — A streaming custom override inherits `stdio` plus a `stdin` getter that always throws, and `origCommand` invokes the bundled command with that same context. Delegating to a non-streaming original that reads stdin (for example, a streaming `sort` override calling `ctx.origCommand(args)` in `seq | sort`) therefore fails instead of consuming the pipe; the original command needs a buffered adapter context.
Adversarial security: 🟢 low
No actionable adversarial security findings in the complete base-to-head diff.
Adversarial security (second opinion): 🟡 medium
No backdoor, exfiltration, or sandbox-escape path found; the streaming pipeline is well gated (bails on redirections, builtins, functions, fds, groupStdin, extraArgs) and resource-bounded, but concurrent stages leak DefenseInDepthBox trusted scope to sibling-stage builtin/interpreter code, which should be fixed or consciously accepted before merge.
- packages/just-bash/src/interpreter/streaming-pipeline.ts:180 — Concurrent streaming stages share one DefenseInDepthBox executionId, and `isTrustedContext()` consults the static per-executionId `trustedExecutionDepth` counter. The PR closes this for command bodies by wrapping untrusted `cmd.execute` in `runUntrustedAsync` (builtin-dispatch.ts:1031), but builtins dispatched via `dispatchBuiltin` and interpreter-level work (expansion, redirection, loop body execution in a `For` stage) are not wrapped. So while a trusted streaming stage (`defineCommand` defaults to `trusted: true`) is in flight, all sibling-stage builtin/interpreter work sees `shouldBlock() === false` and runs with global-access blocking and audit logging disabled for that window — a defense-in-depth regression that did not exist before intra-exec concurrency was introduced.
Standard Bash and host portability: 🟡 medium
Streaming cat breaks ordered operand processing and early pipeline termination for mixed file/stdin inputs.
- packages/just-bash/src/commands/cat/cat.ts:124 — Streaming `cat` eagerly drains stdin whenever any `-` operand exists, before processing earlier file operands. Thus `seq 1000000000 | cat /file - | head -n1` can hit limits or hang instead of immediately outputting `/file` as standard `cat` does. Consume stdin only when its `-` operand is reached.
Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.
Confirmed all three loop-specific findings with failing regressions and fixed them in 0be6ff4f:
- An 80-statement loop now completes with default defense enabled; its periodic yield uses the existing captured timer.
set -e; for i in a; do exit 7; done | cat; echo survivedprintssurvived, matching native Bash.falseis covered too. Streaming stages convert exit/errexit to stage results using the same approach as buffered pipelines.for i in a; do echo err >&2; echo out; done | head -n 0retainserron stderr. The loop keeps already-produced diagnostics when returning broken-pipe status.
The shared trusted-scope overlap was also reproduced and fixed by 691c1faa (the existing runUntrustedAsync wrapper at command dispatch). Details and the reusable foundation commit are posted on #453.
150 focused pipeline/control-flow tests pass, along with type checking, formatting and banned-pattern checks. Three loop regressions added; no new pipeline implementation.
Verified the control-flow/broken-pipe finding and fixed it in bab73de1. for i in a; do { echo err >&2; echo out; break; }; done | head -n 0 previously dropped err\n; the same happened with continue and exit 7. The existing diagnostic regression now covers those cases. The fix catches the final emit's broken pipe and lets already-populated control-flow errors propagate to the existing stage handler.
The macOS seq claim is incorrect for the supported host used to verify this: the system-provided /usr/bin/seq 1 3 runs successfully on macOS 15.7.3. These seq-based comparison cases also come from the shared #453 foundation; normal comparison runs use recorded fixtures. This finding does not justify a fixture rewrite.
Also carried the shared result-getter protection from #642 (679e6098 here). The mixed-file/stdin cat issue reproduces in #453 and remains a separate dependency concern.
154 focused loop, control-flow, and pipeline tests pass, along with typecheck and banned-pattern checks.
Confirmed and fixed the UTF-8 failure-output and mixed cat operand findings in the shared foundation. The failure regression uses a preceding text statement: printf ö; seq 1 | fail, where fail writes é\n then throws a limit error, now retains öé\n rather than mojibake.
For original-command delegation, a wrapper that only calls ctx.origCommand(args) should use the default buffered command contract. Verified a sort override in seq 3 | sort -r returns 3\n2\n1\n. Explicitly opting into streaming: true promises stdio consumption; delegating unchanged to a buffered original violates that contract and raises the existing explicit stdin error. I am not adding an implicit buffering adapter to this loop PR.
For the broader trusted-depth observation: interpreter/builtin code is repository-owned, and host hooks are trusted under THREAT_MODEL.md §1.1. Untrusted command bodies and result normalization are explicitly scoped with runUntrustedAsync; the newly verified frozen-result getter hole is fixed and tested. I did not reproduce an additional attacker-controlled callback bypass in the remaining interpreter work, so I am not expanding the trust model based only on the shared counter observation.
Rebased onto current main, preserving loop status handling; scoped pipeline/control-flow tests, typecheck, formatting, knip and banned-pattern checks pass.
This pull request is a draft
Merging is unavailable until it is marked ready for review.
@asafyish is attempting to deploy a commit to the Vercel Labs Team on Vercel.
A member of the Team first needs to authorize it.