From a8e1ec8344c707a63c78542aaa5b0cecac5baf93 Mon Sep 17 00:00:00 2001 From: Aryam Goyal Date: Sun, 26 Jul 2026 16:27:20 +0530 Subject: [PATCH] fix: list only the tests each package command can actually run Every test route carried the same repository-wide related-files list, because findRelatedTests ran once and its result was assigned to all of them. A report therefore claimed that npm --prefix packages/core run test would exercise packages/action/test/runner.test.ts, which that command never reaches. On this repository all three routes listed an identical eight files spanning three packages. Related files are now scoped to the route's package directory. A repository root script has no such boundary and still covers everything. Found by an external reviewer as "numerous unrelated tests from other packages" and reproduced here, where it turned out every route was affected rather than one. Co-Authored-By: Claude Opus 5 --- examples/reports/workspace-api-discount.md | 6 +-- packages/action/dist/index.mjs | 9 +++- packages/core/src/report.ts | 17 ++++++- packages/core/test/report.test.ts | 53 ++++++++++++++++++++++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/examples/reports/workspace-api-discount.md b/examples/reports/workspace-api-discount.md index 72cb160..1c19ecf 100644 --- a/examples/reports/workspace-api-discount.md +++ b/examples/reports/workspace-api-discount.md @@ -10,9 +10,9 @@ FixMap found 3 context files and generated 3 test routes. ## Test Route -- `pnpm --dir apps/api run test`: nearest package (apps/api) script named test. Related: `apps/api/test/orders.test.ts`, `packages/utils/test/currency.test.ts`. -- `pnpm --dir packages/utils run test`: nearest package (packages/utils) script named test. Related: `apps/api/test/orders.test.ts`, `packages/utils/test/currency.test.ts`. -- `pnpm --dir apps/api run typecheck`: nearest package (apps/api) script named typecheck. Related: `apps/api/src/orders.ts`, `packages/utils/src/currency.ts`. +- `pnpm --dir apps/api run test`: nearest package (apps/api) script named test. Related: `apps/api/test/orders.test.ts`. +- `pnpm --dir packages/utils run test`: nearest package (packages/utils) script named test. Related: `packages/utils/test/currency.test.ts`. +- `pnpm --dir apps/api run typecheck`: nearest package (apps/api) script named typecheck. Related: `apps/api/src/orders.ts`. ## Risk Map diff --git a/packages/action/dist/index.mjs b/packages/action/dist/index.mjs index 4085851..2e216b7 100644 --- a/packages/action/dist/index.mjs +++ b/packages/action/dist/index.mjs @@ -1046,6 +1046,13 @@ function isNearbyChangedFile(path, changedFiles) { } // packages/core/dist/report.js +function scopeToPackage(paths, packageDir) { + if (!packageDir) { + return paths; + } + const prefix = `${packageDir}/`; + return paths.filter((path) => path.startsWith(prefix)); +} function buildTestRoutes(repo, contextPaths) { const codeContextPaths = contextPaths.filter((path) => repo.files.find((file) => file.path === path)?.kind === "code"); if (codeContextPaths.length === 0) { @@ -1068,7 +1075,7 @@ function buildTestRoutes(repo, contextPaths) { routes.push({ command, reason: `${script.packageDir ? `nearest package (${script.packageDir})` : "repository root"} script named ${script.name}`, - relatedFiles: script.name === "test" ? relatedTests : codeContextPaths + relatedFiles: scopeToPackage(script.name === "test" ? relatedTests : codeContextPaths, script.packageDir) }); if (routes.length === 3) break; diff --git a/packages/core/src/report.ts b/packages/core/src/report.ts index 9c72ee4..7f8666d 100644 --- a/packages/core/src/report.ts +++ b/packages/core/src/report.ts @@ -1,6 +1,18 @@ import { tokenizePath } from "./signals.js"; import type { FixMapReport, RepoMap, RiskNote, TestRoute } from "./types.js"; +// A route runs one package's script, so it can only exercise files inside that package. +// Listing a sibling package's tests beneath it claims something the command cannot do: +// `npm --prefix packages/core run test` never reaches packages/action/test. A root +// script has no such boundary and legitimately covers everything. +function scopeToPackage(paths: string[], packageDir: string): string[] { + if (!packageDir) { + return paths; + } + const prefix = `${packageDir}/`; + return paths.filter((path) => path.startsWith(prefix)); +} + export function buildTestRoutes(repo: RepoMap, contextPaths: string[]): TestRoute[] { const codeContextPaths = contextPaths.filter((path) => repo.files.find((file) => file.path === path)?.kind === "code"); if (codeContextPaths.length === 0) { @@ -28,7 +40,10 @@ export function buildTestRoutes(repo: RepoMap, contextPaths: string[]): TestRout routes.push({ command, reason: `${script.packageDir ? `nearest package (${script.packageDir})` : "repository root"} script named ${script.name}`, - relatedFiles: script.name === "test" ? relatedTests : codeContextPaths + relatedFiles: scopeToPackage( + script.name === "test" ? relatedTests : codeContextPaths, + script.packageDir + ) }); if (routes.length === 3) break; } diff --git a/packages/core/test/report.test.ts b/packages/core/test/report.test.ts index e6db8d4..1ac214c 100644 --- a/packages/core/test/report.test.ts +++ b/packages/core/test/report.test.ts @@ -140,4 +140,57 @@ describe("report rendering", () => { expect(routes[0]?.command).toBe("pnpm --dir apps/api run test"); expect(buildTestRoutes(repo, ["README.md"])).toEqual([]); }); + + it("lists only the tests each package command can actually reach", () => { + // Every route previously carried the same repository-wide list, so a core + // route advertised action tests that `--prefix packages/core` never runs. + const source = (path: string) => ({ + path, extension: ".ts", sizeBytes: 10, isSource: true, isTest: false, kind: "code" as const, textSample: "" + }); + const test = (path: string) => ({ ...source(path), isTest: true }); + const repo: RepoMap = { + root: "/repo", + packageManager: "npm", + changedFiles: [], + diffText: "", + diagnostics: [], + packageScripts: [ + { name: "test", command: "vitest run", packageDir: "packages/core" }, + { name: "test", command: "vitest run", packageDir: "packages/action" } + ], + files: [ + source("packages/core/src/rank.ts"), + source("packages/action/src/runner.ts"), + test("packages/core/test/rank.test.ts"), + test("packages/action/test/runner.test.ts") + ] + }; + + const routes = buildTestRoutes(repo, ["packages/core/src/rank.ts", "packages/action/src/runner.ts"]); + const core = routes.find((route) => route.command.includes("packages/core")); + const action = routes.find((route) => route.command.includes("packages/action")); + + expect(core?.relatedFiles).toEqual(["packages/core/test/rank.test.ts"]); + expect(action?.relatedFiles).toEqual(["packages/action/test/runner.test.ts"]); + }); + + it("lets a repository-root command keep every related test", () => { + const repo: RepoMap = { + root: "/repo", + packageManager: "npm", + changedFiles: [], + diffText: "", + diagnostics: [], + packageScripts: [{ name: "test", command: "vitest run", packageDir: "" }], + files: [ + { path: "packages/core/src/rank.ts", extension: ".ts", sizeBytes: 10, isSource: true, isTest: false, kind: "code", textSample: "" }, + { path: "packages/core/test/rank.test.ts", extension: ".ts", sizeBytes: 10, isSource: true, isTest: true, kind: "code", textSample: "" } + ] + }; + + const routes = buildTestRoutes(repo, ["packages/core/src/rank.ts"]); + + expect(routes[0]?.command).toBe("npm run test"); + expect(routes[0]?.relatedFiles).toContain("packages/core/test/rank.test.ts"); + }); });