fix(ci): endpoint-coverage gate is method-aware, wired into CI (F-18) - #276
Merged
Conversation
Three concrete gaps from the assessment: - check-endpoint-coverage.ts concatenated every test file into one haystack and did fragment matching with no HTTP method check -- a GET-only test anywhere in the suite could make a route "covered" for every other method, and an unrelated file sharing a path fragment could do the same. Rewrote isReferenced to require the method AND every path fragment to appear in the SAME test file. Still no route- schema parsing (same stated design goal as before), just method- aware and per-file instead of method-blind and suite-wide. This is a stricter, more honest check: total uncovered routes went from 119 (old matcher) to 380 (this one) on the same test suite -- not a regression, the old number was wrong. Rebaselined accordingly; 3 baseline entries the new matcher DOES find covered were removed. Migrating 84 test files that exercise the legacy `routes/**` tree instead of the live `plugins/*` Fastify API (a separate, much larger, pre-existing problem this assessment also names) is out of scope here -- baselining today's true gap count is the honest alternative to either silently keeping a weaker check or taking on that rewrite in this PR. - Wired `npm run test:endpoints` into CI as a blocking step. It was only ever run manually before; a new endpoint could ship with no same-file, method-matched test and nothing would catch it. - client-rag-memory.test.ts asserts a tenant-wide (`projectId: undefined`) queryRag call as CORRECT -- true for the legacy handler it actually calls, but that handler is dead code (routes/**, not registered in plugin.ts), and the equivalent behavior in the LIVE plugin was a real cross-project data leak (F-02, fixed separately). Added a file-header note and an inline comment on that specific assertion so a reader can't mistake a green run here for evidence about the live API's project scoping. Not addressed here: the `fastify-api.ts` test helper's auth fidelity (touches 31 test files; the actual failure mode needs more scoping before changing something that widely used), `npm audit`'s `continue-on-error: true` (needs a time-boxed exception mechanism first, not just removing the flag), and vitest coverage thresholds (needs an actual coverage run to pick a real number, not a guess). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
P1 finding from the 2026-09-05 finance-institution assessment.
check-endpoint-coverage.tscouldn't tell coverage from coincidence. It concatenated every test file into one haystack and did fragment matching with no HTTP method check — a GET-only test anywhere in the suite could make a route "covered" for every other method on the same path, and an unrelated file sharing a path fragment could do the same. RewroteisReferencedto require the method and every path fragment to appear in the same test file. Same stated design goal as before (no route-schema parsing, still cheap) — just method-aware and per-file instead of method-blind and suite-wide.This is a stricter, more honest check: total uncovered routes went from 119 (old matcher) to 380 (this one) on the same test suite — not a regression, the old number was simply wrong. Rebaselined accordingly (3 entries the new matcher does find real coverage for were removed). Migrating the ~84 test files that exercise the legacy
routes/**tree instead of the liveplugins/*Fastify API — a separate, much larger, pre-existing problem this assessment also names — is out of scope here; baselining today's true gap count honestly is the alternative to either quietly keeping the weaker check or taking on that migration inside this PR.Wired
npm run test:endpointsinto CI as a blocking step. It existed but was never run automatically — a new endpoint could ship with zero same-file, method-matched test and nothing would catch it.client-rag-memory.test.tsasserted a tenant-wide (projectId: undefined)queryRagcall as correct. True for the legacy handler it actually calls — but that handler is dead code (routes/**, never registered inplugin.ts), and the equivalent behavior in the live plugin was a real cross-project data leak (F-02, fixed separately in a different PR). Added a file-header note and an inline comment on that specific assertion so a reader can't mistake a green run here for evidence about the live API's project scoping.Explicitly out of scope for this PR
fastify-api.tstest helper's auth fidelity — it's used by 31 test files; the actual failure mode needs more scoping before touching something this widely relied on.npm audit'scontinue-on-error: true— needs a time-boxed exception mechanism (an allowlist with expiry) before it's safe to make blocking, not just removing the flag.Test plan
npx tsx scripts/check-endpoint-coverage.tsexits 0 against the rebaselined filenpx tsc --noEmitcleannpx eslintclean on changed files (one pre-existing unrelated warning left untouched)vitest run: 5046 passed, 0 failed, 5 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD