fix(scripts): preserve test-harness counters on re-source

[kyle-sexton]

Closes #3268

Summary

scripts/lib/test-harness.sh owns the pass/fail exit contract for the scripts/*.test.sh suites. Its self-test did not cover EXIT-trap interaction, which is why the report is called rather than installed as a trap, and re-sourcing silently zeroed the counters so a recorded failure could exit 0.

Fix

• Add a TEST_HARNESS_SOURCED idempotency guard so a second source (direct or transitive) returns without resetting _test_harness_pass / _test_harness_fail.
• Extend scripts/lib/test-harness.test.sh to pin: pre-source and post-source cleanup traps still run and cannot turn a recorded failure into exit 0; a failing cleanup does not turn a clean suite red; a succeeding cleanup does not mask a failure; the summary prints before the caller's EXIT trap; printf format specifiers (%s, %d, %q) print verbatim; the summary prints once despite subshells and subshells do not disturb status; counters accumulate across repeated calls; an empty suite exits 0; re-sourcing preserves a recorded failure.

Verification

• bash scripts/lib/test-harness.test.sh — PASS=16 FAIL=0
• 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 both files
• bash -n clean on both files

Related

• Refs refactor(scripts): unify test-harness scaffolding in scripts/lib/test-harness.sh #3248 — the shared harness this holds to its EXIT-trap design
• Refs scripts/lib: unify the test-harness scaffolding re-declared across the shell suites #3160 — original unification brief

[claude]

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

---

Security review complete

• Gather context (PR diff, CLAUDE.md, changed files)
• Run /review:security-review criteria against the diff
• Post findings (or confirm no security issues)

No security issues found.

This PR touches only scripts/lib/test-harness.sh and its self-test scripts/lib/test-harness.test.sh. The change adds a TEST_HARNESS_SOURCED idempotency guard (test-harness.sh#L27-L32) so re-sourcing doesn't zero the pass/fail counters, plus new self-test cases covering EXIT-trap interaction, printf-verbatim output, subshell isolation, counter accumulation, and re-source idempotency.

Per the skip gate in /review:security-review, this change has no security-relevant surface:

• No untrusted/external input is processed — run_child's bash -c "$body" in the test file (test-harness.test.sh#L18-L24) is invoked only with static, hardcoded fixture strings authored in this file, not attacker-controlled data.
• No authentication, authorization, secrets, credentials, or GitHub Actions workflow files are touched.
• No injection surface (command/SQL/path/template) is introduced — this is self-contained test-harness plumbing.

[github-actions]

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

[claude]

Claude finished @kyle-sexton's task in 2m 9s —— View job

---

Code review complete

• Gather context (PR diff, CLAUDE.md, changed files)
• Review scripts/lib/test-harness.sh changes
• Review scripts/lib/test-harness.test.sh changes
• Post findings (inline + summary)

Scope: scripts/lib/test-harness.sh (+7/-0) and scripts/lib/test-harness.test.sh (+190/-2).

Findings

None. No correctness or maintainability issues that would block this PR.

Notes from the review (not blocking):

• The idempotency guard (TEST_HARNESS_SOURCED → early return 0) correctly addresses the described bug: re-sourcing the harness previously reset _test_harness_pass/_test_harness_fail to 0 unconditionally, so a suite that recorded a failure and was then re-sourced (directly or transitively) could exit 0. The guard variable is process-local (never exported), so it can't leak across the separate bash "$t" invocations each suite runner (run-plugin-tests.sh, affected-tests.sh) uses — confirmed by reading those runners, each test file executes in its own process.
• Traced the new EXIT-trap test cases (pre-source trap, post-source trap, failing cleanup, succeeding cleanup, summary-before-caller-trap) against actual bash semantics: a trap's own exit status never overrides the script's exit status unless the trap explicitly calls exit, and test_harness::report as the last statement means its return value is what propagates before the caller's trap fires afterward. The assertions match that behavior.
• Walked the run_child isolation used throughout: every new case spawns via bash -c "..." in a fresh process, so the re-source and trap-ordering tests are not contaminated by the outer test file's own TEST_HARNESS_SOURCED/trap state.
• Confirmed TEST_HARNESS_SOURCED doesn't collide with any other variable in the repo (only reference is this file).

Unverified by me (author-claimed only, not independently re-run in this review since this lane's tools don't include arbitrary script execution): the bash scripts/lib/test-harness.test.sh PASS=16/FAIL=0 result, the two consumer-suite runs, and the shellcheck/shfmt/bash -n cleanliness claims in the PR description. Static reading of the diff didn't surface anything that would contradict them.

[github-actions]

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