Skip to content

Commit 36d29c0

Browse files
a0preethamclaude
andcommitted
refactor(interpreter): replace groupStdin string with StdinCursor class
Replaces the `groupStdin?: string` snapshot pattern in IOState with a `StdinCursor` object that tracks a position index into the original content. The cursor is shared by reference across commands in the same while-loop body, so any command that consumes stdin advances the position in-place without needing explicit snapshot/restore gymnastics. Key changes: - New `StdinCursor` class with `remaining`, `advance(n)`, `readAll()` - `read` and `mapfile` builtins call `advance` / `readAll` on the cursor - `file-reader.ts` calls `ctx.stdinCursor?.readAll()` so cat, grep, head, awk, sed etc. inside while loops all advance position correctly - `pipeline-execution.ts` simplified: save ref before pipeline, null it for non-first commands, restore after — the shared object carries any position advancement made by the first command - `executeGroup`, `executeSubshell`, `executeUserScript`, `eval` all install a fresh cursor only when they have their own stdin source; otherwise they share the outer cursor - `Bash.ts` constructs a StdinCursor from the top-level stdin option instead of passing a raw string Fixes the case where `cat | tr` (or any file-reader-based command) was used as the first command in a while-loop pipeline body and failed to advance the loop's stdin position. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyEbpGVknBW94w6XpvP462
1 parent 1ec5eec commit 36d29c0

15 files changed

Lines changed: 317 additions & 57 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"just-bash": patch
3+
---
4+
5+
interpreter: fix pipelines inside while-loop bodies eating loop stdin
6+
7+
Commands inside a `while` loop body that were the first stage of a pipeline
8+
(e.g. `cat | tr a-z A-Z`) or that read stdin directly (e.g. `grep pattern`)
9+
were silently consuming the loop's stdin without advancing the shared position,
10+
causing subsequent `read` calls in the loop condition to see stale or
11+
already-consumed input.
12+
13+
Replaces the `groupStdin?: string` snapshot in `IOState` with a `StdinCursor`
14+
class that tracks a byte offset into the original content. The cursor is shared
15+
by reference across all commands in a loop body, so any command that consumes
16+
stdin advances the position in-place. The loop's `read` builtin then resumes
17+
from the correct position on the next iteration without needing
18+
snapshot/restore logic.
19+
20+
**Before** — a pipeline in the while body would silently reset stdin:
21+
22+
```bash
23+
# Expected: A B C — Actual (before fix): infinite loop / wrong lines
24+
while IFS= read -r line; do
25+
echo "$line" | cat
26+
done <<'EOF'
27+
A
28+
B
29+
C
30+
EOF
31+
```
32+
33+
**After** — all commands share the cursor and advance it atomically:
34+
35+
```bash
36+
# Works correctly in all body patterns
37+
while IFS= read -r line; do echo "$line" | cat; done # pipeline body
38+
while IFS= read -r line; do cat | tr a-z A-Z; done # file-reader first
39+
while IFS= read -r line; do grep "pat"; done # direct stdin read
40+
```

packages/just-bash/src/Bash.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
SecurityViolationError,
6363
} from "./security/defense-in-depth-box.js";
6464
import type { DefenseInDepthConfig } from "./security/types.js";
65+
import { StdinCursor } from "./stdin-cursor.js";
6566
import { serialize } from "./transform/serialize.js";
6667
import type {
6768
BashTransformResult,
@@ -648,7 +649,11 @@ export class Bash {
648649
// UTF-8 bytes. Callers that already prepared a byte buffer (e.g.
649650
// `Buffer.from(buf).toString("latin1")`) opt into raw passthrough
650651
// via `stdinKind: "bytes"`.
651-
groupStdin: encodeStdinForPipeline(options?.stdin, options?.stdinKind),
652+
stdinCursor: options?.stdin
653+
? new StdinCursor(
654+
encodeStdinForPipeline(options.stdin, options.stdinKind) as string,
655+
)
656+
: undefined,
652657
// Cooperative cancellation signal (used by timeout command)
653658
signal: options?.signal,
654659
// Extra arguments injected directly into first command's arg list

packages/just-bash/src/commands/grep/grep.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,9 @@ export const grepCommand: Command = {
229229
// If no files and stdin is provided (including empty string), read from
230230
// stdin. grep runs regex over text — decode bytes to UTF-8 so multibyte
231231
// codepoints match `.` / character classes correctly.
232+
// Advance the cursor so while-loop iterations know stdin was consumed.
232233
if (files.length === 0 && ctx.stdin !== undefined) {
234+
ctx.stdinCursor?.readAll();
233235
const result = searchContent(decodeBytesToUtf8(ctx.stdin), regex, {
234236
invertMatch,
235237
showLineNumbers,

packages/just-bash/src/interpreter/builtin-dispatch.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -412,17 +412,15 @@ export async function executeExternalCommand(
412412
ctx.state.hashTable.set(commandName, cmdPath);
413413
}
414414

415-
// Use groupStdin as fallback if no stdin from redirections/pipeline —
416-
// needed for commands inside groups/functions that receive stdin via
417-
// heredoc. The pipeline glue (pipeline-execution.ts) and the
418-
// stdin-source sites (heredoc, here-string, `< file`, options.stdin)
419-
// are responsible for handing us a latin1-shaped byte buffer; we just
420-
// brand it. Commands that decode their input internally (sed, jq,
421-
// ...) return text via `textOutput()`, and the pipe / redirect layer
422-
// converts to bytes on their behalf.
423-
const effectiveStdin = unsafeBytesFromLatin1(
424-
stdin || ctx.state.groupStdin || "",
425-
);
415+
// Use the cursor's remaining content as fallback when no pipeline stdin.
416+
// The cursor is shared by reference: commands that consume stdin via
417+
// file-reader (cat, grep, head, …) call cursor.readAll() which advances
418+
// the position, so subsequent loop iterations start from the right place.
419+
// Only pass the cursor when stdin is empty (pipeline commands with explicit
420+
// stdin must not fall back to the loop's cursor).
421+
const cursorContent = !stdin ? ctx.state.stdinCursor?.remaining : undefined;
422+
const effectiveStdin = unsafeBytesFromLatin1(stdin || cursorContent || "");
423+
const stdinCursor = !stdin ? ctx.state.stdinCursor : undefined;
426424

427425
// Build exported environment for commands that need it (printenv, env, etc.)
428426
// Most builtins need access to the full env to modify state
@@ -434,6 +432,7 @@ export async function executeExternalCommand(
434432
env: ctx.state.env,
435433
exportedEnv,
436434
stdin: effectiveStdin,
435+
stdinCursor,
437436
limits: ctx.limits,
438437
exec: ctx.execFn,
439438
fetch: ctx.fetch,

packages/just-bash/src/interpreter/builtins/eval.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import { type ParseException, parse } from "../../parser/parser.js";
9+
import { StdinCursor } from "../../stdin-cursor.js";
910
import type { ExecResult } from "../../types.js";
1011
import {
1112
BreakError,
@@ -50,12 +51,12 @@ export async function handleEval(
5051
return OK;
5152
}
5253

53-
// Save and set groupStdin for piped eval commands
54-
// This allows stdin from the pipeline to flow to commands within eval
55-
const savedGroupStdin = ctx.state.groupStdin;
56-
const effectiveStdin = stdin ?? ctx.state.groupStdin;
57-
if (effectiveStdin !== undefined) {
58-
ctx.state.groupStdin = effectiveStdin;
54+
// If eval received stdin from a pipeline, install a fresh cursor for it so
55+
// read commands inside eval consume from that input. If no pipeline stdin,
56+
// eval shares the outer cursor — reads inside eval advance it normally.
57+
const savedCursor = ctx.state.stdinCursor;
58+
if (stdin) {
59+
ctx.state.stdinCursor = new StdinCursor(stdin);
5960
}
6061

6162
try {
@@ -77,7 +78,8 @@ export async function handleEval(
7778
}
7879
throw error;
7980
} finally {
80-
// Restore groupStdin
81-
ctx.state.groupStdin = savedGroupStdin;
81+
if (stdin) {
82+
ctx.state.stdinCursor = savedCursor;
83+
}
8284
}
8385
}

packages/just-bash/src/interpreter/builtins/mapfile.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ export function handleMapfile(
6262
}
6363
}
6464

65-
// Use stdin from parameter, or fall back to groupStdin
65+
// Use stdin from parameter, or fall back to cursor's remaining content.
6666
let effectiveStdin = stdin;
67-
if (!effectiveStdin && ctx.state.groupStdin !== undefined) {
68-
effectiveStdin = ctx.state.groupStdin;
67+
if (!effectiveStdin && ctx.state.stdinCursor !== undefined) {
68+
effectiveStdin = ctx.state.stdinCursor.remaining;
6969
}
7070

7171
// Split input by delimiter
@@ -162,9 +162,9 @@ export function handleMapfile(
162162
String(Math.max(existingLength, newEndIndex)),
163163
);
164164

165-
// Consume from groupStdin if we used it
166-
if (ctx.state.groupStdin !== undefined && !stdin) {
167-
ctx.state.groupStdin = "";
165+
// mapfile reads all stdin — exhaust the cursor so the loop advances.
166+
if (ctx.state.stdinCursor !== undefined && !stdin) {
167+
ctx.state.stdinCursor.readAll();
168168
}
169169

170170
return result("", "", 0);

packages/just-bash/src/interpreter/builtins/read.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ export function handleRead(
263263
return result("", "", 1);
264264
}
265265

266-
// Use stdin from parameter, or fall back to groupStdin (for piped groups/while loops)
266+
// Use stdin from parameter, or fall back to stdinCursor (for piped groups/while loops)
267267
// If -u is specified, use the file descriptor content instead
268268
let effectiveStdin = stdin;
269269

@@ -274,8 +274,8 @@ export function handleRead(
274274
} else {
275275
effectiveStdin = "";
276276
}
277-
} else if (!effectiveStdin && ctx.state.groupStdin !== undefined) {
278-
effectiveStdin = ctx.state.groupStdin;
277+
} else if (!effectiveStdin && ctx.state.stdinCursor !== undefined) {
278+
effectiveStdin = ctx.state.stdinCursor.remaining;
279279
}
280280

281281
// Handle -d '' (empty delimiter) - reads until NUL byte
@@ -308,8 +308,8 @@ export function handleRead(
308308
);
309309
}
310310
}
311-
} else if (ctx.state.groupStdin !== undefined && !stdin) {
312-
ctx.state.groupStdin = effectiveStdin.substring(bytesConsumed);
311+
} else if (ctx.state.stdinCursor !== undefined && !stdin) {
312+
ctx.state.stdinCursor.advance(bytesConsumed);
313313
}
314314
};
315315

packages/just-bash/src/interpreter/control-flow.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import type {
2121
WhileNode,
2222
WordNode,
2323
} from "../ast/types.js";
24+
import { StdinCursor } from "../stdin-cursor.js";
2425
import type { ExecResult } from "../types.js";
2526
import { evaluateArithmetic } from "./arithmetic.js";
2627
import { matchPattern } from "./conditionals.js";
@@ -295,10 +296,10 @@ export async function executeWhile(
295296
}
296297
}
297298

298-
// Save and set groupStdin for piped while loops
299-
const savedGroupStdin = ctx.state.groupStdin;
299+
// Install a fresh cursor for this loop's stdin, saving any outer cursor.
300+
const savedCursor = ctx.state.stdinCursor;
300301
if (effectiveStdin) {
301-
ctx.state.groupStdin = effectiveStdin;
302+
ctx.state.stdinCursor = new StdinCursor(effectiveStdin);
302303
}
303304

304305
ctx.state.loopDepth++;
@@ -390,7 +391,7 @@ export async function executeWhile(
390391
}
391392
} finally {
392393
ctx.state.loopDepth--;
393-
ctx.state.groupStdin = savedGroupStdin;
394+
ctx.state.stdinCursor = savedCursor;
394395
}
395396

396397
return result(stdout, stderr, exitCode);

packages/just-bash/src/interpreter/pipeline-execution.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ export async function executePipeline(
4747
const isMultiCommandPipeline = node.commands.length > 1;
4848
const savedLastArg = ctx.state.lastArg;
4949

50+
// Save the cursor so we can restore it after the pipeline. The first
51+
// command shares the cursor and may advance its position. Non-first
52+
// commands must not see it (they read from the pipe, not from the loop's
53+
// stdin). After the pipeline the cursor — with its potentially-advanced
54+
// position — is put back so the next loop iteration reads from the right
55+
// place.
56+
const savedCursor = ctx.state.stdinCursor;
57+
5058
for (let i = 0; i < node.commands.length; i++) {
5159
const command = node.commands[i];
5260
const isLast = i === node.commands.length - 1;
@@ -58,12 +66,10 @@ export async function executePipeline(
5866
// Clear $_ for each pipeline command - they each get fresh subshell context
5967
ctx.state.lastArg = "";
6068

61-
// After the first command, clear groupStdin so subsequent commands
62-
// only see stdin from the pipeline (even if empty), not the original groupStdin
63-
// This prevents commands like head from incorrectly falling back to groupStdin
64-
// when they receive empty output from a previous command (e.g., grep with no matches)
69+
// Non-first commands get their stdin from the pipe, not from the loop's
70+
// cursor. Clear it so they cannot accidentally fall back to it.
6571
if (!isFirst) {
66-
ctx.state.groupStdin = undefined;
72+
ctx.state.stdinCursor = undefined;
6773
}
6874
}
6975

@@ -238,5 +244,14 @@ export async function executePipeline(
238244
}
239245
// With lastpipe, the last command already updated $_ in the main shell context
240246

247+
// Restore the cursor ref for non-first pipeline commands. Those commands had
248+
// it cleared (they must only read from the pipe), but the savedCursor still
249+
// holds the shared object whose position may have been advanced by the first
250+
// command. Putting it back lets the next loop iteration read from the right
251+
// place.
252+
if (isMultiCommandPipeline) {
253+
ctx.state.stdinCursor = savedCursor;
254+
}
255+
241256
return lastResult;
242257
}

packages/just-bash/src/interpreter/subshell-group.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
} from "../ast/types.js";
1515
import { Parser } from "../parser/parser.js";
1616
import type { ParseException } from "../parser/types.js";
17+
import { StdinCursor } from "../stdin-cursor.js";
1718
import type { ExecResult } from "../types.js";
1819
import {
1920
BreakError,
@@ -109,10 +110,11 @@ export async function executeSubshell(
109110
const savedBashPid = ctx.state.bashPid;
110111
ctx.state.bashPid = ctx.state.nextVirtualPid++;
111112

112-
// Save any existing groupStdin and set new one from pipeline
113-
const savedGroupStdin = ctx.state.groupStdin;
113+
// Install a fresh cursor when subshell has its own stdin; otherwise share
114+
// the outer cursor so reads inside the subshell advance the outer position.
115+
const savedCursor = ctx.state.stdinCursor;
114116
if (stdin) {
115-
ctx.state.groupStdin = stdin;
117+
ctx.state.stdinCursor = new StdinCursor(stdin);
116118
}
117119

118120
let stdout = "";
@@ -130,7 +132,7 @@ export async function executeSubshell(
130132
ctx.state.fullyUnsetLocals = savedFullyUnsetLocals;
131133
ctx.state.loopDepth = savedLoopDepth;
132134
ctx.state.parentHasLoopContext = savedParentHasLoopContext;
133-
ctx.state.groupStdin = savedGroupStdin;
135+
if (stdin) ctx.state.stdinCursor = savedCursor;
134136
ctx.state.bashPid = savedBashPid;
135137
ctx.state.lastArg = savedLastArg;
136138
};
@@ -272,10 +274,11 @@ export async function executeGroup(
272274
}
273275
}
274276

275-
// Save any existing groupStdin and set new one from pipeline
276-
const savedGroupStdin = ctx.state.groupStdin;
277+
// Install a fresh cursor when the group has its own stdin source; otherwise
278+
// share the outer cursor so reads inside the group advance the outer position.
279+
const savedCursor = ctx.state.stdinCursor;
277280
if (effectiveStdin) {
278-
ctx.state.groupStdin = effectiveStdin;
281+
ctx.state.stdinCursor = new StdinCursor(effectiveStdin);
279282
}
280283

281284
try {
@@ -286,8 +289,7 @@ export async function executeGroup(
286289
exitCode = res.exitCode;
287290
}
288291
} catch (error) {
289-
// Restore groupStdin before handling error
290-
ctx.state.groupStdin = savedGroupStdin;
292+
if (effectiveStdin) ctx.state.stdinCursor = savedCursor;
291293
// ExecutionLimitError must always propagate - these are safety limits
292294
if (error instanceof ExecutionLimitError) {
293295
throw error;
@@ -303,8 +305,7 @@ export async function executeGroup(
303305
return result(stdout, `${stderr}${getErrorMessage(error)}\n`, 1);
304306
}
305307

306-
// Restore groupStdin
307-
ctx.state.groupStdin = savedGroupStdin;
308+
if (effectiveStdin) ctx.state.stdinCursor = savedCursor;
308309

309310
// Apply output redirections
310311
const bodyResult = result(stdout, stderr, exitCode);
@@ -353,15 +354,15 @@ export async function executeUserScript(
353354
const savedParentHasLoopContext = ctx.state.parentHasLoopContext;
354355
const savedLastArg = ctx.state.lastArg;
355356
const savedBashPid = ctx.state.bashPid;
356-
const savedGroupStdin = ctx.state.groupStdin;
357+
const savedCursor = ctx.state.stdinCursor;
357358
const savedSource = ctx.state.currentSource;
358359

359360
// Set up subshell-like environment
360361
ctx.state.parentHasLoopContext = savedLoopDepth > 0;
361362
ctx.state.loopDepth = 0;
362363
ctx.state.bashPid = ctx.state.nextVirtualPid++;
363364
if (stdin) {
364-
ctx.state.groupStdin = stdin;
365+
ctx.state.stdinCursor = new StdinCursor(stdin);
365366
}
366367
ctx.state.currentSource = scriptPath;
367368

@@ -387,7 +388,7 @@ export async function executeUserScript(
387388
ctx.state.parentHasLoopContext = savedParentHasLoopContext;
388389
ctx.state.lastArg = savedLastArg;
389390
ctx.state.bashPid = savedBashPid;
390-
ctx.state.groupStdin = savedGroupStdin;
391+
if (stdin) ctx.state.stdinCursor = savedCursor;
391392
ctx.state.currentSource = savedSource;
392393
};
393394

0 commit comments

Comments
 (0)