[codesailor4]

Summary

Two security-sensitive gaps in areas the wave flagged but nothing tested.

SW-064 (#165) — createNpmInvocation had no direct coverage

This is the only place the CLI decides how to reach npm, and on Windows it does so
through cmd.exe /d /s /c npm <args> — a wrapper around a command interpreter.
The entire safety property is that the argument vector stays an argv: the moment a
caller-supplied string is joined into one command line, a value containing &,
|, ; or backticks stops being data and becomes a command fragment.

Nothing tested it. critical-path.test.ts:39 re-implements the same wrapper inline
in its own helper rather than importing the function, so it asserts a copy of the
behaviour, and npm.ts itself had no direct coverage at all.

The new cases run on both platforms (via the Object.defineProperty(process, 'platform', …) pattern already used in generator-reserved-names.test.ts) and pin:

• the fixed /d /s /c prefix always leads, so nothing can be prepended to it;
• a hostile argument — value & calc.exe, $(whoami), backticks, quotes,
  an embedded newline, value && echo pwned — arrives as exactly one inert argv
  element, byte-for-byte, on Windows and POSIX alike;
• command and args remain separate, which is what lets the spawner pass
  shell: false.

SW-065 (#166) — two supervisor cleanup properties unverified

The existing suite covers a one-child spawn failure and a two-child sibling exit.
Two properties of cleanup therefore had no real test:

No child is orphaned. With three children, a mid-loop failure must terminate
every already-started child. The two-child case cannot distinguish "all children
killed" from "the next one killed" — both produce the same assertion. This suite
also confirms a spec that was never spawned has nothing to clean up, so the test
cannot pass by over-killing.

The failure cause survives. fail() wraps the original error as the cause of
a SubprocessCliError so the underlying reason is not lost. The existing case
asserts only the error type; nothing checked the cause was actually attached.

Plus: exit code and signal both reach the message, handlers are removed after a
failure, and the child error event path still cleans up.

Issue references

Closes #165

Closes #166

Closes #167

Closes #169

What this PR actually changes

Implemented in full — #165 (npm-invocation-metacharacters.test.ts) and
#166 (dev-supervisor-cleanup.test.ts).

Not implemented here:

• SW-067 — Add npm test workflow exit-code propagation tests #167 (SW-067, npm exit-code propagation) is already covered twice:
  test-command.test.ts asserts a zero exit resolves and a code-9 exit rejects
  with SubprocessCliError, and test-orchestration-regression.test.ts also
  asserts the with code 9 message.
• SW-069 — Add test stdout/stderr streaming assertions #169 (SW-069, stdout/stderr streaming) is already covered:
  test-orchestration-regression.test.ts has dedicated streaming cases for both
  the Node and the Cargo workflow, and test-command.test.ts:44 asserts the
  successful child's output.

Both were verified against the code rather than assumed, so no duplicate coverage
was added for them.

Split from another PR in this wave

The repeated-signal half of cleanup idempotency is covered by a separate
contributor's dev-orchestrator-signal-idempotency.test.ts. This file stays on
failure paths so the two suites do not overlap.

Not run

node_modules was not installed and the suite was not executed, per your
constraint. The platform-stub cases use the same Object.defineProperty + restore
pattern as generator-reserved-names.test.ts, so they should behave, but a local
run is worth doing before merge — particularly on Windows, where the
cmd.exe branch actually executes.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: a5b75d2e-ab2f-4894-a186-bf3fb89f42f1

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}