Closes https://github.com/vercel-labs/just-bash/issues/634
Read jq input files in bounded batches and emit each evaluated input before reading later batches. Slurp still collects its input; parsing and evaluation are unchanged. File byte accounting is tracked separately in #650.
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
15🤖 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/13 |
| Changeset included | ✅ (.changeset/streaming-pipelines.md) |
Review panel: 🟡 medium highest severity
just-bash maintainer code review: 🟢 low
No actionable defects found in the complete base-to-head diff.
General code review: 🟢 low
No actionable correctness issues found in the complete base-to-head diff.
Adversarial security: 🟢 low
No actionable adversarial security issues found in the complete diff.
Adversarial security (second opinion): 🟢 low
Reviewed the full streaming-pipeline and jq batching diff from an adversarial security angle: the eligibility gate keeps attacker-reachable constructs (redirections, assignments, builtins, aliases, dynamic words, inherited stdin/FDs) on the buffered executor, stages still go through normal command dispatch so no resolution or expansion check is skipped, stdio is revocable and defense-context-wrapped like fs/env, output accounting nets to a single charge with forged-accounting credit capped, and the switch to runUntrustedAsync tightens rather than loosens the trust scope under concurrency. No dependency, install-script, CI, or network changes. No actionable security findings.
Standard Bash and host portability: 🟡 medium
The streaming path breaks standard pipeline behavior for common `cat` invocations involving options or mixed file/stdin operands.
- packages/just-bash/src/commands/cat/cat.ts:124 — Streaming `cat` eagerly drains all stdin before processing operands whenever any transform or file operand is present. Thus `seq 1000000000 | cat file - | head -n1` waits for or exhausts the producer instead of emitting `file` first and stopping it as ordinary Bash does. Consume stdin at its operand position and stream transformations incrementally.
Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.
Verified and fixed the local correctness cases in 569f488c:
- File operands are evaluated/emitted before a later
-operand reads stdin. Whole-input stdin decoding remains as documented. The regression exercises downstream closure while an earlier file is being emitted. jq . /first /missing | cat, with/firstcontaining1, preserves1\nbefore the missing-file diagnostic/status 2. The successful prefix is yielded before reporting the batch error.jq -r .a /inon{"a":""}emits\nboth standalone and in a pipeline. Native jq agrees; the buffered empty-output check was wrong.
The proposed rawfile/slurpfile fix assumes incorrect native behavior. jq 1.8.2 opens a literal file named - for these options. Given that file containing 1\n2\n and piped stdin 99\n:
jq -cn --rawfile v - '$v' # "1\n2\n"
jq -cn --slurpfile v - '$v' # [1,2]Without a file named -, jq reports a file-open error. The small fix is allowStdinMarker: false on the existing binding-file reader, rather than adding stdin-backed bindings. Both modes are covered.
The byte-accounting observation is real but inherited from the shared reader. I ran jq empty /f0 ... /f49 on 50 files of roughly 10 KB each with maxInputBytes: 64 and maxLiveBytes: 4096: both unchanged #453 (262a17b2) and this branch succeed. The batch refactor does not establish a byte bound, and its description only promises a 50-file bound. Correcting shared file-input accounting needs separate scope; I have flagged it rather than folding that change into this PR.
Also carried the shared trust-scope fix from #453 in 7bc7c9db. Three focused tests added; 388 jq/pipeline tests pass, plus type checking, formatting and banned-pattern checks.
The confirmed file-input/live-memory budget gap is now addressed separately in #650, based directly on main. It covers ordinary jq file operands and --rawfile/--slurpfile without bringing streaming changes into that fix. The PR links back here and to #634; three focused regressions and all 826 relevant tests pass locally.
⚠️ auto-maintain: review severity raised to 🔴 high
A new push changed this PR and the review now contains a higher-severity finding. See the updated review comment above.
Verified the latest findings against this PR's previous head.
- The deferred result-getter finding is real. Fixed in 6fcc99cb by keeping result accounting and copying inside the existing trusted/untrusted callback. The existing overlapping-stage regression now also exercises a returned getter; that case failed before this fix.
- The misplaced jq streaming flag is real. Fixed in 1619d621 by moving the property onto
jqCommand. The existing early-stop test now covers direct registration as well as the normal registry; direct registration previously read all 1,000 files. - I could not reproduce the output-accounting claim, including its suggested late-error case.
jq . /first /missing | cat, with/firstcontaining1, produces 2 stdout bytes and 40 diagnostic bytes. A 42-byte output budget permits the result; 41 bytes fails with status 126.accountResultonly credits explicitinternalOutputAccounting; the shared checkpoint caps existing credit and does not create credit from sibling output. jq returns no such marker. I have left that accounting design unchanged.
The mixed cat prefix - issue does reproduce, but originates in the shared #453 foundation. It remains separate from jq's operand handling, which already emits earlier files before reading stdin.
The jq and streaming-pipeline checks, the accounting reproduction, typecheck, and changed-file formatting pass. No additional review was requested.
Confirmed and fixed the plain mixed-operand cat case in the shared foundation: seq 1000000000 | cat /prefix - | head -n 1 outputs the file first and cancels the producer. Cat transformation options still use the existing buffered transformation path; a new incremental formatter is outside this jq batching fix. The shared trust-accounting, UTF-8 failure-output and bounded pipe-read fixes are included. Rebased onto current main; the jq batch change remains separate from #650’s input-budget fix.
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.