Skip to content

fix(scripts): give harness self-test an independent tally - #3285

Merged
kyle-sexton merged 1 commit into
mainfrom
cursor/3283-harness-self-tally-cfcf
Aug 24, 2026
Merged

fix(scripts): give harness self-test an independent tally#3285
kyle-sexton merged 1 commit into
mainfrom
cursor/3283-harness-self-tally-cfcf

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #3283

Summary

scripts/lib/test-harness.test.sh sources the library it tests, so a break in test_harness::report's return path or in fail() counter arithmetic is detected by the suite's assertions and then cannot be reported to CI. The process exit status is produced by the code under test.

Two mutations against a throwaway copy of the harness demonstrate it: deleting return 1 from report, and removing the increment from fail(). Both print FAIL: lines and still exited 0. Checking _test_harness_fail after report catches the first mutation but not the second.

Fix

  • Parent assertions go through self_ok/self_fail, which keep an independent pass/fail tally the harness functions do not write.
  • After test_harness::report, a non-zero independent tally forces exit 1 even if the harness counters or return were sabotaged.
  • The last-line discipline still requires every other sourcer to end on test_harness::report; this self-test is the one file allowed to follow the call.
  • A throwaway-copy mutation regression runs the self-test against both defects and asserts it still exits non-zero.

Verification

  • bash scripts/lib/test-harness.test.sh — PASS=18 FAIL=0 (includes both mutation cases)
  • bash scripts/check-silent-revert.test.sh — PASS=130 FAIL=0 (installs an EXIT trap and sources the harness)
  • bash scripts/check-skill-portability.test.sh — PASS=92 FAIL=0 (installs an EXIT trap and sources the harness)
  • shellcheck --rcfile=.shellcheckrc -x and shfmt -d clean on scripts/lib/test-harness.test.sh
  • bash -n clean on scripts/lib/test-harness.test.sh

Related

Source the library for assertions, but keep a shadow pass/fail
count the harness cannot write. After test_harness::report, that
tally forces a non-zero exit when any assertion failed — so a
sabotaged return 1 or fail() increment cannot go green in CI.

Add a throwaway-copy mutation regression for both defects.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 42s —— View job


Security review — no findings

  • Gather context (PR diff, changed files)
  • Run skip gate check
  • Review changed file for security-relevant issues
  • Post findings / conclusion

Scope: scripts/lib/test-harness.test.sh (+126/-34), the only file changed in this PR.

Skip gate: the change has no security-relevant surface. It's a local, self-contained test file: no network calls, no secrets/credentials, no user-controlled input. The new mutation-regression helper (run_mutated_self_test) copies fixed local files (test-harness.sh, the self-test itself) into a mktemp -d scratch directory and applies one of two hardcoded grep -v/grep -Fv mutations selected by a literal case value (drop-return-1 / drop-fail-increment) — not by any external or attacker-influenced input. mktemp -d ".../harness-mut.XXXXXX" gets a securely randomized suffix, so there's no predictable-tempdir/symlink race, and the scratch dir is removed after each run. No eval, no sourcing of untrusted content, no injection surface into the constructed shell commands.

No security issues found in this diff.

@github-actions

Copy link
Copy Markdown
Contributor

Last security-reviewed head: f5a96eb777f38eeca9059ae882845439f7411e98. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@kyle-sexton
kyle-sexton merged commit 305e0de into main Aug 24, 2026
54 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/3283-harness-self-tally-cfcf branch August 24, 2026 02:39
@claude

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 27s —— View job


/review:code-review — skipped

  • Gather context
  • Check skill skip gate

This PR is already merged, which trips the /review:code-review skip gate ("PR is closed or not open"). Per that skill's instructions, review stops here with no findings posted — merged code isn't an active review surface.

If a fresh review is wanted (e.g. against a follow-up branch), please re-trigger @claude on an open PR.

@github-actions

Copy link
Copy Markdown
Contributor

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scripts/lib/test-harness.test.sh: the harness self-test cannot report a break in the report return or the counters

1 participant