fix(shell): actually pass spawn its arguments

[glennmichael123]

Closes #69.

craft.shell.spawn(id, cmd, args) has advertised an arguments array since it was written. The SDK's spawn(command, args?, options?) says the same. Native declared no args field, so ignore_unknown_fields parsed the array and dropped it without a word:

craft.shell.spawn('p', 'git', ['status'])   // ran `git` bare, reported success

Why the arguments had nowhere to go

spawn ran /bin/sh -c <command>. A shell takes one string, not an argv — so there was no slot to put them in.

The issue offers "append them to the argv", but the argv was {"/bin/sh", "-c", command}. Appending to that means appending to the command string, which means quoting every argument correctly, forever, on two platforms, against input supplied by a web page — in order to avoid handing that page a shell.

So the default stops using one:

	before	after
what runs	/bin/sh -c "git"	execve("git", ["git", "status"])
args	parsed, discarded	passed
; in an argument	shell syntax	a semicolon the program receives
injection surface	a blocklist between a page and sh -c	none — no shell exists

shell: true — and it was already documented

The SDK's SpawnOptions has declared shell?: boolean all along, saying "The Craft bridge translates this automatically". It was as dead as args — native had no field for it either. It's real now.

That path keeps the metacharacter blocklist, because it's the only path that needs one. Passing args alongside it is refused, not half-honoured — silently ignoring arguments is the exact bug being fixed, and the compatibility path must not reintroduce it.

The compatibility break, stated plainly

spawn('p', 'git status') used to work, because sh split the string. It now looks for a program named "git status".

That is the same break Node made deliberately, and the fix is spawn('p', 'git', ['status']) — what the signature asked for in the first place. Worth noting how little the shell was contributing: validateCommand already rejected ;, |, &, >, <, \n, $( and backticks, so it provided word-splitting and essentially nothing else. Callers who need more can pass shell: true.

Tests

bridge_shell.zig had no tests and no test artifact — which is how it shipped parsing an array and throwing it away. The argv decision is now a pure function with 10 tests, wired into zig build test.

Controlled: reintroducing the original behaviour — parsing the arguments and dropping them, compiling cleanly — fails 7 tests by name:

error: 'bridge_shell.test.arguments reach the program instead of being dropped' failed
error: 'bridge_shell.test.no shell is involved, so no shell metacharacters are interpreted' failed
error: 'bridge_shell.test.a program with no arguments is just the program' failed
...

(My first attempt at that control failed to compile rather than failing an assertion, which proves nothing about the test. Redone so the broken version builds.)

Also covered: an older payload with no args key still spawns the same program, empty-string arguments survive, argument order is pinned, and the JSON→argv seam is tested end to end with the struct the bridge actually decodes with.

Noted, not fixed

The SDK's ChildProcess.start() posts to process.spawn, and process is not a routed bridge type anywhere in the dispatcher — that path answers nothing at all. Separate from this issue; flagging rather than widening scope into it.

Verified

zig build · zig build test · zig build test-js · x86_64-windows cross-build · zig fmt --check · tsc --noEmit · 511 bun tests · pickier 38 warnings, unchanged from main.

[github-actions]

✅ Binary load time

rounds:    25 interleaved
base:      p50 16.1ms   p95 18.0ms   (15.3–18.1ms)
head:      p50 15.7ms   p95 17.8ms   (15.1–19.3ms)
delta:     -2.5%  (fails above +20.0%)

No binary load time regression.

What this measures

craft --help: process spawn, dynamic linking and argument parsing.
It never opens a window, so it cannot see a change in window or
webview startup — real startup is benchmarks/startup.bench.ts, which
needs a display.

Both binaries are measured interleaved on this runner and compared by
p50, rather than against a number recorded on another machine. On
byte-identical binaries that method reads within ~3.5%; the old one
swung 45%.

[github-actions]

✅ Binary Size Report

Metric	Value
Current Size	14231KB (13.89MB)
Change	0KB (0%) unchanged

Size limits

• Warning: 14.50MB
• Maximum: 16.00MB