fix(run): don't fail the run on a failing todo or skip test

[kriszyp]

⊙ Problem

harper-integration-test-run's test:fail handler sets process.exitCode = 1 and records the file in failedFiles for every node:test failure, including tests marked todo or skip. node:test's own semantics are that a failing todo/skip test doesn't fail the run, so a deliberately-failing todo test (used to track a known issue without blocking CI) turns the whole run red — exactly the case the todo marker exists to avoid.

❓ Your call: As specified, yes — bringing the runner's exit status in line with node:test's own todo/skip semantics is the fix; there's no alternative framing to weigh here.

💡 Solution

The test:fail handler now returns early for a todo/skip failure, before it sets process.exitCode or adds the file to failedFiles. node:test emits the marker as reason ?? true (same code on Node 22, 24 and 26), so a failing todo/skip test carries true or the reason string, which can be empty. The guard accepts exactly those: data.todo || data.todo === '' || data.skip || data.skip === ''. A real failure is unaffected: each test:fail event is handled independently, so a todo/skip test can never suppress a later real failure's exitCode = 1.

⚠️ Look hardest: the guard is the whole behavior change. An empty reason ({ todo: '' }) must still count as todo, not as a real failure.

⚖️ Alternatives

❓ Your call: marker-predicate — the guard lists the accepted marker values explicitly (review request), instead of testing presence (data.todo !== undefined || data.skip !== undefined). Both agree on everything node emits from { todo/skip: … } options and string reasons. They differ only for something like t.skip(0) or t.skip(false): node counts that test as skipped, this guard counts it as a real failure (exit 1). That errs loud, and it is a one-line change either way.

• exit-status-source — kept a per-event predicate rather than taking process.exitCode from the final test:summary event's success field. failedFiles would still need the per-event handler, so the switch would not shrink the change. Reversible if node ever adds another non-failing failure kind.
• suppress-logs-for-todo-skip — a failing todo/skip test no longer adds its file to failedFiles, so its Harper hdb.log tail is not printed either. Low stakes: it only affects diagnosing a known-broken todo test.
• self-test-isolation-mode — the self-test runs the CLI with --isolation=none only. The production default (process isolation) is covered by code trace: the guard does not branch on isolation mode, and node sets todo/skip on the event the same way in either mode.

🔧 Changes

• src/run.ts — the test:fail guard described above.
• test/run.test.ts (new) — spawns the real CLI under --isolation=none against each fixture and checks the exit code. runCli passes each test's t.signal to the child, so a timeout kills it instead of leaking it, and fails if the child died from a signal. It also strips HARPER_INTEGRATION_TEST_* from the child's env, so an exported shard/only setting cannot change which fixture runs.
• test/fixtures/run/ (new) — one failing test per file, named without .test. so the npm test glob does not run them as suites:
  • todo-fail.ts — { todo: true }.
  • skip-fail.ts — calls t.skip() inside the body. The { skip: true } option would skip the body, so the test would never fail and never reach the data.skip branch.
  • empty-reason-todo-fail.ts — { todo: '' }.
  • real-fail.ts — the control: an unmarked failure.

✅ Verification

• npm run check (tsc) and npm run build — clean.
• npm test — 68 tests, 0 failures; the 4 new cases pass.
• Fails on base: with src/run.ts from origin/main, the todo, skip and empty-reason cases fail (exit 1) and the real-failure control still passes.
• Each clause is covered: removing the '' clauses fails only the empty-reason case; removing the skip clauses fails only the skip case.
• CI on 324c7f6: all jobs green (ubuntu/macOS/Windows × Node 22/24/26, plus commitlint).
• Pre-push cross-model review (codex, gemini, cursor-composer, Harper domain adjudication): LGTM on the final full round over this head, with nits only.

_{🤖 Generated by Claude Sonnet 5, review updates by Claude Opus 5.5; posted via @kriszyp.}

Dispatch-Task: integration-testing-todo-not-failure

Related PRs: none found

Complexity: easy

Origin — the dispatch brief this PR was written from

Runner: a failing {todo} test must not fail the run

In src/run.ts (~:127) of HarperFast/integration-testing, the test:fail handler sets process.exitCode = 1 and adds the file to failedFiles for every failure, including tests marked { todo }. node:test semantics are that a failing todo test does not fail the run, so the harper-integration-test-run CLI turns any todo-marked known failure into a red CI job. Fix: skip both the exitCode assignment and failedFiles.add when data.todo is set (also check data.skip handling for consistency). Add a self-test with a deliberately failing { todo } assertion that asserts the run exits 0, and a control with a real failure that exits 1. Bump the patch version per the repo's release convention.

Acceptance

A suite whose only failure is a todo test exits 0; a real failure still exits 1.

_{Dispatch: task integration-testing-todo-not-failure · queued by finding-triage/wave-29 · ran by claude/sonnet/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=5; full=3 @ 324c7f6}

_{Review-Attention: skim ~2m (decisions: marker-predicate, exit-status-source, self-test-isolation) @ 324c7f6}

[gemini-code-assist]

Code Review

This pull request updates the test runner in src/run.ts to ensure that failing todo or skip tests do not cause the test run to fail, aligning with node:test semantics. It also introduces integration tests in test/run.test.ts to verify this behavior. Feedback suggests propagating process.execArgv when spawning the child process in tests to ensure any necessary Node.js flags or loaders are correctly applied.