test(ci): run the twelve orphaned vitest suites and stop new ones appearing

[murdore]

The problem

12 of the 28 test/*.test.ts files were referenced by no package.json script. Nothing ran them — not pnpm test, not test:unit, not a developer following the docs. They looked like coverage in review while executing zero assertions.

knowledgeGrounding · mcpResultCacheErrorSkip · promptRedaction
proxyAnalysis · proxyConfigHotReload · proxyObservabilityFoundation
proxyReliabilityHardening · proxyReplay · proxyRollingWorkerHandoff
proxyTerminalErrorAccounting · proxyUpdaterFallback · proxyUsageStatsPersistence

One of them had already rotted

proxyTerminalErrorAccounting.test.ts expects the Claude route to make two upstream attempts. It makes one, because 33bdd141 fix(proxy): bound account admission and fallback routing made translation-layer auto-fallback opt-in. That commit updated the three orphaned proxy suites its author knew about and missed this one — no run would have told them.

Bisected to confirm rather than assume: passes at 65b27b75, fails from 33bdd141 onward, and still fails on today's release.

The behaviour change is correct; the test was stale. Fixed by opting the mock router into auto-fallback, so the test still reaches the second-attempt path it was written to check (that exhaustion finalizes the terminal error exactly once, not twice — the thing its name is about). Added a companion test pinning the new default-off path, so the change that broke it can't happen silently again.

What this PR does

1. Wires all twelve in — test:proxy:vitest, test:knowledge-grounding:vitest, test:mcp-result-cache:vitest, test:prompt-redaction:vitest, all chained into test:unit. 300 previously-dead assertions now execute (234 + 44 + 5 + 17), all green.
2. Repairs the stale test and pins the behaviour that broke it.
3. Stops it recurring — checkOrphanedTestFiles() in scripts/build-validations.ts, which validate:all already runs in CI. A new unreferenced test file now fails the build with the file names and the fix.

Verification

• Guard catches the real thing: before wiring, 12 orphaned → VALIDATION FAILED; after, 0 orphaned → passed.
• Guard bites: deleting one filename from the scripts block reproduces 1 orphaned → VALIDATION FAILED; restored → passes.
• The guard reads the scripts block alone, so a filename appearing in some other package.json field can't count as wired without running anything.
• pnpm run check: 0 errors / 4,838 files. pnpm run lint: 0 errors.

Worth flagging separately

.github/workflows/ci.yml runs format:check, eslint, validate:all, build and build:cli — no test step at all. So this PR makes the suites runnable and enforced-as-wired, but CI still does not execute them. That is a larger decision (which suites, what runtime budget, which need credentials) and is deliberately not bundled here.

[coderabbitai]

Warning

Review limit reached

@murdore, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 84ffb001-4aed-49be-9c2a-0530d86a7633

📥 Commits

Reviewing files that changed from the base of the PR and between ce34fa0 and dce92a4.

📒 Files selected for processing (3)

• package.json
• scripts/build-validations.ts
• test/proxyTerminalErrorAccounting.test.ts

---

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

[github-actions]

✅ Single Commit Policy - COMPLIANT

Status: Policy requirements met • 1 commit • Valid format • Ready for merge

📊 View validation details

📝 Commit Details

• Hash: dce92a44a83fa09d5f69aa10d9d96ab4210ccc91
• Message: test(ci): run the twelve orphaned vitest suites and stop new ones appearing
• Author: Sachin Sharma

✅ Validation Results

• Single commit requirement met
• No merge commits in branch
• Semantic commit message format verified
• Ready for squash merge to release branch

---

🤖 Automated validation by NeuroLink Single Commit Enforcement

[github-actions]

🤖 AI Review & Build Compliance ✅

Status: AI analysis complete • Build rules validated • Ready for review

📊 View detailed analysis results

🛡️ Analysis Complete

• ✅ Security scan (vulnerabilities, API keys)
• ✅ TypeScript safety & code quality
• ✅ Error handling & best practices
• ✅ Build rule enforcement validated
• ✅ Commit format & compliance checks

📋 Ready for Merge When

• All CI checks passing
• Manual review approved
• Any AI-flagged issues resolved

---

🤖 AI analysis complete - check individual code comments for specific feedback

[Tara-ag]

Review Summary - PR #1267

Decision: APPROVED ✅

Files Changed (3 files, self-contained improvements)

1. package.json - Added 4 new test scripts for proxy, knowledge-grounding, mcp-result-cache, and prompt-redaction functionality
2. scripts/build-validations.ts - Added checkOrphanedTestFiles() to prevent drift where test files become orphaned (never executed)
3. test/proxyTerminalErrorAccounting.test.ts - Added regression test pinning auto-fallback disabled behavior

Impact on Existing Code

• Blast radius: 34 nodes changed, 500 nodes impacted within 2 hops
• Affected flows: Only the build validation script (run flow) - no production code affected
• Breaking changes: None - all changes are additive (new tests, new validation)
• Self-contained: Changes do not touch SDK or CLI functionality

Review Scope

Reviewed per file-by-file approach following NeuroLink conventions:

• ✅ No hardcoded secrets or credentials
• ✅ Proper TypeScript patterns (type over interface)
• ✅ Clear documentation/comments explaining test purpose
• ✅ Defensive error handling in build validation
• ✅ Test assertions cover expected behavior with clear edge cases
• ✅ Follows bootstrapped standards from recent merged PRs

Key Observations

• The new orphan detection prevents a real issue that had bitten before (12 orphaned files, one with failing assertion)
• Test comments explicitly explain why opt-in is needed for certain scenarios
• Build validation integrates seamlessly into existing workflow via pnpm run validate
• No circular dependencies introduced (configuration + build tool only)

Final Verdict

This is a safe, targeted improvement that enhances CI/CD reliability without touching production code. All changes are well-documented and follow established patterns. No blocking issues found.

No inline comments required - all findings are resolved/approved.

[Copilot]

🟡 Not ready to approve

The new orphan-test validation currently misses at least one existing unreferenced Vitest suite under test/ subdirectories, undermining the “stop new ones appearing” goal until the check and scripts are aligned.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR makes previously unreferenced Vitest suites runnable via package.json scripts, fixes a stale proxy terminal-error accounting test after an auto-fallback behavior change, and adds a build-time validation to prevent “dead” test files from accumulating.

Changes:

• Adds new test:*:vitest scripts and chains them into test:unit so the previously orphaned Vitest suites execute.
• Updates proxyTerminalErrorAccounting.test.ts to opt into auto-fallback for the legacy behavior test and adds a companion test for the default-off behavior.
• Introduces checkOrphanedTestFiles() in scripts/build-validations.ts and runs it as part of the validation runner.

File summaries

File	Description
test/proxyTerminalErrorAccounting.test.ts	Fixes a stale test by explicitly opting into auto-fallback and adds coverage for the default-off path.
scripts/build-validations.ts	Adds a CI-enforced validation intended to prevent unreferenced Vitest test files.
package.json	Wires the orphaned Vitest suites into runnable scripts and chains them into test:unit.

Review details

Suppressed comments (1)

package.json:225

• There is a Vitest suite at test/providers/dedupExecuteMap.test.ts that is not referenced by any package.json script, so it remains effectively dead. If the new orphan-test validation is expanded to include subdirectories (or if the intent is to ensure all Vitest suites are runnable), this file should be wired into the scripts block.

    "test:proxy:vitest": "pnpm exec vitest run test/proxyAnalysis.test.ts test/proxyConfigHotReload.test.ts test/proxyObservabilityFoundation.test.ts test/proxyReliabilityHardening.test.ts test/proxyReplay.test.ts test/proxyRollingWorkerHandoff.test.ts test/proxyTerminalErrorAccounting.test.ts test/proxyUpdaterFallback.test.ts test/proxyUsageStatsPersistence.test.ts",
    "test:knowledge-grounding:vitest": "pnpm exec vitest run test/knowledgeGrounding.test.ts",
    "test:mcp-result-cache:vitest": "pnpm exec vitest run test/mcpResultCacheErrorSkip.test.ts",
    "test:prompt-redaction:vitest": "pnpm exec vitest run test/promptRedaction.test.ts"

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[murdore]

Closing this — the approach is wrong for this repo.

This PR wires 12 Vitest files into test:unit and, worse, adds a build-validations check that makes CI fail unless every test/*.test.ts stays wired. That institutionalizes a pattern the repo doesn't use.

CLAUDE.md:190 is explicit: "all suites run via tsx; there is no vitest runner despite vitest.config.ts existing", and :258 says new coverage belongs in test/continuous-test-suite-*.ts. The repo is 82 continuous suites vs 28 Vitest files. My justification — that test:unit already chains 11 Vitest scripts — described drift, not the convention, and I used it to add more.

The finding underneath still holds and is being kept, since it is independent of the runner: 33bdd141 made translation-layer auto-fallback opt-in, so the Claude route now makes one upstream attempt where proxyTerminalErrorAccounting.test.ts still expects two. Bisected — passes at 65b27b75, fails from 33bdd141 onward, still fails on release. That coverage is moving to test/continuous-test-suite-bugfixes.ts, which is where 33bdd141 itself put its proxy tests and which test:unit already runs.

The related PRs have been corrected the same way: #1253, #1264 and #1265 no longer touch package.json or add Vitest files — their coverage now lives in the continuous suite.