From 5ecc31522547b44853ad54adaba311dc1613b03e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 17:21:19 +0200 Subject: [PATCH] test(coach): make route-inventory gate check skip pure-comment lines A `text.includes("requireAssistantSurface(...)")` match also flips true on a docstring that merely mentions the gate, so a contributor who deletes the actual call but leaves the comment would slip through the inventory walk. Split into a per-line check that ignores `//` and ` *` JSDoc lines, matching the same posture the coach-cascade discovery test uses for `flags.coach`. Drops a dead `repoRoot` variable from the stale-allowlist test along the way. --- .../coach-route-gate-inventory.test.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/app/api/insights/__tests__/coach-route-gate-inventory.test.ts b/src/app/api/insights/__tests__/coach-route-gate-inventory.test.ts index 31f95b82b..1632559c4 100644 --- a/src/app/api/insights/__tests__/coach-route-gate-inventory.test.ts +++ b/src/app/api/insights/__tests__/coach-route-gate-inventory.test.ts @@ -69,6 +69,23 @@ const NOT_COACH_OWNED_ROUTES: ReadonlyArray = [ const COACH_GATE_NEEDLE = 'requireAssistantSurface("coach")'; const ANY_GATE_NEEDLE = "requireAssistantSurface("; +/** + * Return true when the file contains the needle on a line that is NOT + * a pure comment. A documentation comment that mentions the gate + * (`// requireAssistantSurface("coach") protects this surface`) must + * not satisfy the gate-presence check — otherwise a contributor who + * deletes the actual call but leaves the docstring would slip through. + */ +function fileHasGateCall(text: string, needle: string): boolean { + return text.split("\n").some((line) => { + if (!line.includes(needle)) return false; + const trimmed = line.trim(); + if (trimmed.startsWith("//")) return false; + if (trimmed.startsWith("*")) return false; + return true; + }); +} + /** * Walk every `route.ts` under `src/app/api/insights/` (including * dynamic segments under `[id]`). Returns POSIX-style paths relative @@ -115,8 +132,8 @@ describe("Coach API route gate inventory", () => { for (const path of routes) { const full = resolve(repoRoot, path); const text = readFileSync(full, "utf8"); - const hasCoachGate = text.includes(COACH_GATE_NEEDLE); - const hasAnyGate = text.includes(ANY_GATE_NEEDLE); + const hasCoachGate = fileHasGateCall(text, COACH_GATE_NEEDLE); + const hasAnyGate = fileHasGateCall(text, ANY_GATE_NEEDLE); if (hasCoachGate) continue; @@ -153,7 +170,6 @@ describe("Coach API route gate inventory", () => { }); it("allowlists do not reference deleted route files", () => { - const repoRoot = resolve(__dirname, "..", "..", "..", "..", ".."); const known = new Set(findInsightsRouteFiles()); const stale = [...NON_COACH_GATED_ROUTES, ...NOT_COACH_OWNED_ROUTES].filter( @@ -169,9 +185,5 @@ describe("Coach API route gate inventory", () => { ...stale.map((p) => ` - ${p}`), ].join("\n"), ).toEqual([]); - - // Silence "repoRoot declared and not used" if the loop above - // returns empty — reference it explicitly. - expect(repoRoot.length).toBeGreaterThan(0); }); });