fix(shell): preserve lenient tokenization in chain segments

package.json

@@ -73,7 +73,6 @@
     "picomatch": "^4.0.4",
     "react": "^18.3.1",
     "react-reconciler": "^0.29.2",
-    "shell-quote": "^1.8.3",
     "string-width": "^7.2.0",
     "yoga-layout": "^3.2.1",
     "zod": "^4.4.1"
@@ -84,7 +83,6 @@
     "@types/picomatch": "^4.0.3",
     "@types/react": "^18.3.12",
     "@types/react-reconciler": "^0.28.9",
-    "@types/shell-quote": "^1.7.5",
     "esbuild": "^0.21.5",
     "highlight.js": "^11.10.0",
     "htm": "^3.1.1",

src/tools/shell-chain.ts

@@ -1,8 +1,13 @@
 /** Parse + spawn `cmd1 | cmd2 && cmd3` ourselves — never invoke a shell, sidestep PS5.1's `&&` parse error. */
 
 import { type ChildProcess, type SpawnOptions, spawn } from "node:child_process";
-import { parse as shellParse } from "shell-quote";
-import { killProcessTree, prepareSpawn, smartDecodeOutput } from "./shell.js";
+import {
+  detectShellOperator,
+  killProcessTree,
+  prepareSpawn,
+  smartDecodeOutput,
+  tokenizeCommand,
+} from "./shell.js";
 
 export type ChainOp = "|" | "||" | "&&" | ";";
 
@@ -16,64 +21,99 @@ export interface CommandChain {
   ops: ChainOp[];
 }
 
-const CHAIN_OPS = new Set<string>(["|", "||", "&&", ";"]);
-
 export class UnsupportedSyntaxError extends Error {
   constructor(detail: string) {
     super(`run_command: ${detail}`);
     this.name = "UnsupportedSyntaxError";
   }
 }
 
-/** Returns null on plain commands (caller takes the simple path); throws on unsupported syntax. */
-export function parseCommandChain(cmd: string): CommandChain | null {
-  // shell-quote calls env() with name="" for `$(...)` — defer that to the `(` op handler.
-  const tokens = shellParse(cmd, (name: string) =>
-    name === "" ? "$" : { op: "$VAR" as const, name },
-  );
-  const segments: ChainSegment[] = [];
+/** Whitespace-bounded splitter — chain ops only count when they begin a token, so `--flag=1&2` stays literal. */
+function splitOnChainOps(cmd: string): { segs: string[]; ops: ChainOp[] } {
+  const segs: string[] = [];
   const ops: ChainOp[] = [];
-  let cur: string[] = [];
-  let sawChainOp = false;
-  for (const t of tokens) {
-    if (typeof t === "string") {
-      cur.push(t);
+  let segStart = 0;
+  let i = 0;
+  let quote: '"' | "'" | null = null;
+  let atTokenStart = true;
+  while (i < cmd.length) {
+    const ch = cmd[i]!;
+    if (quote) {
+      if (ch === quote) quote = null;
+      else if (ch === "\\" && quote === '"' && i + 1 < cmd.length) i++;
+      i++;
+      atTokenStart = false;
       continue;
     }
-    if ("comment" in t) continue;
-    const op = (t as { op: string }).op;
-    if (CHAIN_OPS.has(op)) {
-      sawChainOp = true;
-      if (cur.length === 0) throw new UnsupportedSyntaxError(`empty segment before "${op}"`);
-      segments.push({ argv: cur });
-      ops.push(op as ChainOp);
-      cur = [];
+    if (ch === '"' || ch === "'") {
+      quote = ch;
+      i++;
+      atTokenStart = false;
       continue;
     }
-    if (op === "glob") {
-      cur.push((t as { pattern: string }).pattern);
+    if (ch === " " || ch === "\t") {
+      i++;
+      atTokenStart = true;
       continue;
     }
-    if (op === "$VAR") {
-      const name = (t as { name: string }).name;
+    if (atTokenStart) {
+      let op: ChainOp | null = null;
+      let opLen = 0;
+      const next = cmd[i + 1];
+      if (ch === "|" && next === "|") {
+        op = "||";
+        opLen = 2;
+      } else if (ch === "&" && next === "&") {
+        op = "&&";
+        opLen = 2;
+      } else if (ch === "|") {
+        op = "|";
+        opLen = 1;
+      } else if (ch === ";") {
+        op = ";";
+        opLen = 1;
+      }
+      if (op !== null) {
+        segs.push(cmd.slice(segStart, i));
+        ops.push(op);
+        i += opLen;
+        segStart = i;
+        atTokenStart = true;
+        continue;
+      }
+    }
+    i++;
+    atTokenStart = false;
+  }
+  segs.push(cmd.slice(segStart));
+  return { segs, ops };
+}
+
+/** Returns null on plain commands (caller takes the simple path); throws on unsupported syntax inside any segment. */
+export function parseCommandChain(cmd: string): CommandChain | null {
+  const { segs, ops } = splitOnChainOps(cmd);
+  if (ops.length === 0) return null;
+  const segments: ChainSegment[] = [];
+  for (let i = 0; i < segs.length; i++) {
+    const trimmed = segs[i]!.trim();
+    if (trimmed.length === 0) {
+      const op = i === 0 ? ops[0]! : ops[i - 1]!;
       throw new UnsupportedSyntaxError(
-        `\$${name} expansion is not supported — pass values as literals, or use the binary's own --env flag`,
+        i === 0
+          ? `empty segment before "${op}"`
+          : i === segs.length - 1
+            ? `chain ends with "${op}"`
+            : `empty segment between "${ops[i - 1]}" and "${ops[i]}"`,
       );
     }
-    if (op === "(" || op === ")") {
+    const segOp = detectShellOperator(trimmed);
+    if (segOp !== null) {
       throw new UnsupportedSyntaxError(
-        "command substitution / subshells are not supported — split into separate calls",
+        `shell operator "${segOp}" is not supported — only \`|\`, \`||\`, \`&&\`, \`;\` chain operators are spawned natively. Redirects (\`>\`, \`<\`, \`2>&1\`) and background (\`&\`) require splitting into separate run_command calls.`,
       );
     }
-    throw new UnsupportedSyntaxError(
-      `shell operator "${op}" is not supported — only \`|\`, \`||\`, \`&&\`, \`;\` chain operators work; redirects (\`>\`, \`<\`, \`2>&1\`) are rejected`,
-    );
-  }
-  if (!sawChainOp) return null;
-  if (cur.length === 0) {
-    throw new UnsupportedSyntaxError(`chain ends with "${ops[ops.length - 1]}"`);
+    segments.push({ argv: tokenizeCommand(trimmed) });
   }
-  segments.push({ argv: cur });
   return { segments, ops };
 }
 

tests/shell-chain.test.ts

@@ -33,48 +33,60 @@ describe("parseCommandChain", () => {
     expect(c!.ops).toEqual(["&&", "||", ";"]);
   });
 
-  it("handles `|` without surrounding spaces (`a|b`)", () => {
-    const c = parseCommandChain("git status|grep main");
-    expect(c!.segments.map((s) => s.argv)).toEqual([
-      ["git", "status"],
-      ["grep", "main"],
-    ]);
-  });
-
   it("preserves quoted operators as literal arguments", () => {
-    const c = parseCommandChain('grep "a|b" file');
-    expect(c).toBeNull();
+    expect(parseCommandChain('grep "a|b" file')).toBeNull();
+    expect(parseCommandChain("grep 'a|b' file")).toBeNull();
   });
 
   it("passes globs through as literal patterns (no shell expansion)", () => {
     const c = parseCommandChain("ls *.ts | grep test");
     expect(c!.segments[0]!.argv).toEqual(["ls", "*.ts"]);
   });
 
-  it("rejects redirects", () => {
-    expect(() => parseCommandChain("echo hi > out.txt")).toThrow(UnsupportedSyntaxError);
-    expect(() => parseCommandChain("sort < in.txt")).toThrow(/<.*not supported/);
-    expect(() => parseCommandChain("cmd 2>&1")).toThrow(/not supported/);
+  it("does NOT split on chain chars embedded inside larger tokens", () => {
+    // `--flag=1&2` is one POSIX token; the `&` is a literal byte. Tokens
+    // containing `&` / `|` / `;` chars but not at the start are passed
+    // through untouched, matching the lenient single-command tokenizer.
+    expect(parseCommandChain("cargo run -- --flag=1&2")).toBeNull();
+    expect(parseCommandChain("grep a|b file")).toBeNull();
+    expect(parseCommandChain("echo a;b")).toBeNull();
   });
 
-  it("rejects background `&`", () => {
-    expect(() => parseCommandChain("long_task &")).toThrow(/"&" is not supported/);
+  it("allows embedded chain chars inside chain segments", () => {
+    const c = parseCommandChain("git status ; cargo run -- --flag=1&2");
+    expect(c!.segments.map((s) => s.argv)).toEqual([
+      ["git", "status"],
+      ["cargo", "run", "--", "--flag=1&2"],
+    ]);
+    expect(c!.ops).toEqual([";"]);
   });
 
-  it("rejects env-var expansion", () => {
-    expect(() => parseCommandChain("echo $HOME")).toThrow(/\$HOME expansion/);
+  it("rejects redirects discovered inside a chain segment", () => {
+    expect(() => parseCommandChain("echo hi ; ls > out.txt")).toThrow(/">"/);
+    expect(() => parseCommandChain("git status | wc -l > out.txt")).toThrow(/">"/);
+    expect(() => parseCommandChain("a ; cmd 2>&1")).toThrow(/2>&1/);
   });
 
-  it("rejects command substitution", () => {
-    expect(() => parseCommandChain("echo $(date)")).toThrow(/command substitution/);
+  it("rejects background `&` discovered inside a chain segment", () => {
+    expect(() => parseCommandChain("git status ; long &")).toThrow(/"&" is not supported/);
   });
 
   it("rejects empty leading segments", () => {
     expect(() => parseCommandChain("; echo hi")).toThrow(/empty segment/);
+    expect(() => parseCommandChain("|| cat")).toThrow(/empty segment/);
   });
 
   it("rejects a chain ending with an operator", () => {
     expect(() => parseCommandChain("echo hi &&")).toThrow(/chain ends with/);
+    expect(() => parseCommandChain("echo hi ;")).toThrow(/chain ends with/);
+  });
+
+  it("rejects empty middle segments", () => {
+    expect(() => parseCommandChain("a ; ; b")).toThrow(/empty segment/);
+  });
+
+  it("rejects unclosed quotes inside a chain segment", () => {
+    expect(() => parseCommandChain('git status ; echo "open')).toThrow(/unclosed/);
   });
 });
 
@@ -104,7 +116,7 @@ describe("isCommandAllowed", () => {
 
   it("returns false for unsupported syntax (rather than throwing)", () => {
     expect(isCommandAllowed("echo hi > out.txt")).toBe(false);
-    expect(isCommandAllowed("echo $(date)")).toBe(false);
+    expect(isCommandAllowed("echo hi &")).toBe(false);
   });
 });
 

tests/shell-tools.test.ts

@@ -134,16 +134,6 @@ describe("runCommand redirect rejection", () => {
     );
   });
 
-  it("rejects `$VAR` env-var expansion", async () => {
-    await expect(runCommand("echo $HOME", { cwd: tmp })).rejects.toThrow(
-      /\$HOME expansion is not supported/,
-    );
-  });
-
-  it("rejects command substitution `$(…)`", async () => {
-    await expect(runCommand("echo $(date)", { cwd: tmp })).rejects.toThrow(/command substitution/);
-  });
-
   it("rejects an empty leading segment", async () => {
     await expect(runCommand("; echo hi", { cwd: tmp })).rejects.toThrow(/empty segment before/);
   });