From ef5703c5e962986178ec30e0dcb1adcf9f7bc39d Mon Sep 17 00:00:00 2001 From: SB Yoon <44089734+yansigit@users.noreply.github.com> Date: Thu, 13 Aug 2026 04:52:37 -0600 Subject: [PATCH] fix(mcp): move pack validation out of unit suite (#10065) --- changelog.d/fixes/9821-mcp-pack-unit-stall.md | 1 + scripts/build/mcpPublishedFilesClosure.ts | 132 +++++++++++++++++ scripts/build/pack-artifact-policy.ts | 53 +++++++ scripts/build/validate-pack-artifact.ts | 117 ++++++++++++--- .../mcp-published-files-closure-3578.test.ts | 137 +++--------------- ...cp-published-files-closure-helpers.test.ts | 70 +++++++++ tests/unit/pack-artifact-policy.test.ts | 35 +++++ 7 files changed, 410 insertions(+), 135 deletions(-) create mode 100644 changelog.d/fixes/9821-mcp-pack-unit-stall.md create mode 100644 scripts/build/mcpPublishedFilesClosure.ts create mode 100644 tests/unit/mcp-published-files-closure-helpers.test.ts diff --git a/changelog.d/fixes/9821-mcp-pack-unit-stall.md b/changelog.d/fixes/9821-mcp-pack-unit-stall.md new file mode 100644 index 0000000000..c8f677535b --- /dev/null +++ b/changelog.d/fixes/9821-mcp-pack-unit-stall.md @@ -0,0 +1 @@ +- **fix(test):** remove live `npm pack` from MCP files unit test (it stalled concurrent `test:unit` via prepare→husky + monorepo pack walk); keep the static #3578 `files` allowlist + negation guards in unit and fold #3821 pack assertions into `check:pack-artifact` / `check:pack-policy` (already `--ignore-scripts`). diff --git a/scripts/build/mcpPublishedFilesClosure.ts b/scripts/build/mcpPublishedFilesClosure.ts new file mode 100644 index 0000000000..4eda8cf592 --- /dev/null +++ b/scripts/build/mcpPublishedFilesClosure.ts @@ -0,0 +1,132 @@ +/** + * Shared MCP publish-path helpers (#3578 / #3821). + * + * Unit tests use the static `files` allowlist walker (no subprocess). + * The pack-artifact gate uses the same helpers against a real + * `npm pack --dry-run --ignore-scripts` file list so concurrent unit + * suites never shell out to `npm pack`. + */ + +import fs from "node:fs"; +import path from "node:path"; + +import { normalizeArtifactPath } from "./pack-artifact-policy.ts"; + +/** Co-located test / spec paths that must never ship in the npm tarball. */ +export const PACK_ARTIFACT_TEST_FILE_RE = /(?:^|\/)__tests__\/|\.(?:test|spec)\.[cm]?[jt]sx?$/; + +/** Negations that must stay in package.json `files` (static unit guard). */ +export const REQUIRED_PACKAGE_FILES_TEST_NEGATIONS: readonly string[] = [ + "!**/__tests__/**", + "!**/*.test.ts", + "!**/*.test.tsx", + "!**/*.test.js", + "!**/*.test.mjs", + "!**/*.spec.ts", + "!**/*.spec.tsx", +]; + +/** Spot-check file from the original #3578 bug report. */ +export const MCP_CLOSURE_SPOT_CHECK_PATH = "src/lib/combos/steps.ts"; + +function resolveImport(root: string, fromFile: string, spec: string): string | null { + let base: string; + if (spec.startsWith("@/")) base = path.join("src", spec.slice(2)); + else if (spec.startsWith("@omniroute/open-sse/")) + base = path.join("open-sse", spec.slice("@omniroute/open-sse/".length)); + else if (spec === "@omniroute/open-sse") base = path.join("open-sse", "index"); + else if (spec.startsWith("./") || spec.startsWith("../")) + base = path.join(path.dirname(fromFile), spec); + else return null; // bare package — not our source + base = base.replace(/\.(ts|tsx|js|mjs)$/, ""); + const cands = [ + base + ".ts", + base + ".tsx", + path.join(base, "index.ts"), + path.join(base, "index.tsx"), + base + ".js", + base + ".mjs", + ]; + for (const c of cands) if (fs.existsSync(path.join(root, c))) return c; + return null; +} + +/** + * Transitive import closure of the MCP server entrypoints under `src/` + `open-sse/`. + */ +export function computeMcpClosure(root: string = process.cwd()): string[] { + const roots: string[] = []; + for (const f of fs.readdirSync(path.join(root, "open-sse/mcp-server"))) { + if (f.endsWith(".ts")) roots.push("open-sse/mcp-server/" + f); + } + for (const d of ["open-sse/mcp-server/tools", "open-sse/mcp-server/schemas"]) { + const abs = path.join(root, d); + if (fs.existsSync(abs)) + for (const f of fs.readdirSync(abs)) if (f.endsWith(".ts")) roots.push(d + "/" + f); + } + + const seen = new Set(); + const stack = [...roots]; + const importRe = + /(?:import|export)[^"']*?from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']\s*\)/g; + while (stack.length) { + const f = stack.pop() as string; + if (seen.has(f)) continue; + seen.add(f); + let src: string; + try { + src = fs.readFileSync(path.join(root, f), "utf8"); + } catch { + continue; + } + let m: RegExpExecArray | null; + while ((m = importRe.exec(src))) { + const spec = m[1] || m[2]; + if (!spec) continue; + const r = resolveImport(root, f, spec); + if (r && !seen.has(r)) stack.push(r); + } + } + return [...seen].filter((f) => f.startsWith("src/") || f.startsWith("open-sse/")); +} + +/** Whether `file` is covered by a package.json `files` allowlist entry. */ +export function isCoveredByFiles(file: string, filesEntries: string[]): boolean { + for (const entry of filesEntries) { + if (entry.startsWith("!")) continue; // negations are not positive coverage + if (entry.endsWith("/")) { + if (file === entry.slice(0, -1) || file.startsWith(entry)) return true; + } else if (file === entry || file.startsWith(entry + "/")) { + return true; + } + } + return false; +} + +/** Packed paths that look like test / spec files (over-inclusion). */ +export function findLeakedTestArtifactPaths(filePaths: string[]): string[] { + return filePaths + .map(normalizeArtifactPath) + .filter(Boolean) + .filter((filePath) => PACK_ARTIFACT_TEST_FILE_RE.test(filePath)) + .sort(); +} + +/** MCP closure members missing from a packed (or candidate) path set. */ +export function findMissingMcpClosurePaths( + packedPaths: string[], + closurePaths: string[] = computeMcpClosure() +): string[] { + const packed = new Set(packedPaths.map(normalizeArtifactPath).filter(Boolean)); + return closurePaths + .map(normalizeArtifactPath) + .filter(Boolean) + .filter((filePath) => !packed.has(filePath)) + .sort(); +} + +/** Required `files` negation entries that are absent from package.json. */ +export function findMissingPackageFilesTestNegations(filesEntries: string[]): string[] { + const present = new Set(filesEntries); + return REQUIRED_PACKAGE_FILES_TEST_NEGATIONS.filter((entry) => !present.has(entry)); +} diff --git a/scripts/build/pack-artifact-policy.ts b/scripts/build/pack-artifact-policy.ts index c066dd084e..5ccbe52a3e 100644 --- a/scripts/build/pack-artifact-policy.ts +++ b/scripts/build/pack-artifact-policy.ts @@ -228,6 +228,59 @@ export function normalizeArtifactPath(filePath: string): string { .replace(/\/{2,}/g, "/"); } +/** Extract complete JSON values from npm's mixed stdout/stderr-style output. */ +export function parseJsonValuesOutput(output: string): unknown[] { + const values: unknown[] = []; + for (let start = 0; start < output.length; start++) { + if (output[start] !== "[" && output[start] !== "{") continue; + + const stack: string[] = []; + let inString = false; + let escaped = false; + for (let end = start; end < output.length; end++) { + const char = output[end]; + if (inString) { + if (escaped) escaped = false; + else if (char === "\\") escaped = true; + else if (char === '"') inString = false; + continue; + } + if (char === '"') { + inString = true; + } else if (char === "[" || char === "{") { + stack.push(char); + } else if (char === "]" || char === "}") { + const expectedOpen = char === "]" ? "[" : "{"; + if (stack.at(-1) !== expectedOpen) break; + stack.pop(); + if (stack.length === 0) { + try { + const parsed: unknown = JSON.parse(output.slice(start, end + 1)); + values.push(parsed); + start = end; + } catch { + // This bracket pair was not a complete JSON value; continue scanning. + } + break; + } + } + } + } + return values; +} + +/** Extract the first matching JSON array from npm's mixed stdout/stderr-style output. */ +export function parseJsonArrayOutput( + output: string, + matches: (parsed: unknown[]) => boolean = () => true +): unknown[] { + const parsed = parseJsonValuesOutput(output).find( + (value): value is unknown[] => Array.isArray(value) && matches(value) + ); + if (!parsed) throw new Error("Expected a valid JSON array in command output."); + return parsed; +} + /** * Paths that are NEVER publishable, whatever the allowlist says. * diff --git a/scripts/build/validate-pack-artifact.ts b/scripts/build/validate-pack-artifact.ts index 202cb87b51..4c0085846a 100644 --- a/scripts/build/validate-pack-artifact.ts +++ b/scripts/build/validate-pack-artifact.ts @@ -1,16 +1,23 @@ #!/usr/bin/env node -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; +import { + MCP_CLOSURE_SPOT_CHECK_PATH, + computeMcpClosure, + findLeakedTestArtifactPaths, + findMissingMcpClosurePaths, +} from "./mcpPublishedFilesClosure.ts"; import { PACK_ARTIFACT_ALLOWED_EXACT_PATHS, PACK_ARTIFACT_ALLOWED_PATH_PREFIXES, PACK_ARTIFACT_REQUIRED_PATHS, findMissingArtifactPaths, findUnexpectedArtifactPaths, + parseJsonValuesOutput, } from "./pack-artifact-policy.ts"; const __filename: string = fileURLToPath(import.meta.url); @@ -24,12 +31,29 @@ function runNpm(args: string[], stdio: "inherit" | "pipe" = "pipe"): string { const command = npmExecPath && !isBunRuntime ? process.execPath : npmCommand; const commandArgs = npmExecPath && !isBunRuntime ? [npmExecPath, ...args] : args; - return execFileSync(command, commandArgs, { + if (stdio === "inherit") { + execFileSync(command, commandArgs, { + cwd: ROOT, + encoding: "utf8", + stdio: "inherit", + maxBuffer: 64 * 1024 * 1024, + }); + return ""; + } + + const result = spawnSync(command, commandArgs, { cwd: ROOT, encoding: "utf8", - stdio: stdio === "inherit" ? "inherit" : ["ignore", "pipe", "pipe"], + stdio: ["ignore", "pipe", "pipe"], maxBuffer: 64 * 1024 * 1024, }); + if (result.error) throw result.error; + if (result.status !== 0) { + throw new Error( + (result.stderr || result.stdout || `npm exited with status ${result.status}`).trim() + ); + } + return `${result.stdout || ""}\n${result.stderr || ""}`; } function ensureAppStagingReady(): void { @@ -43,15 +67,39 @@ function ensureAppStagingReady(): void { runNpm(["run", "build:cli"], "inherit"); } -function runPackDryRun(): any { +type PackReport = { + files: Array<{ path: string }>; + filename?: string; + entryCount?: number; + size?: number; + unpackedSize?: number; +}; + +function findPackReport(value: unknown): PackReport | null { + if (Array.isArray(value)) { + for (const item of value) { + const report = findPackReport(item); + if (report) return report; + } + return null; + } + if (typeof value !== "object" || value === null) return null; + + const record = value as Record; + if (Array.isArray(record.files)) return record as unknown as PackReport; + for (const child of Object.values(record)) { + const report = findPackReport(child); + if (report) return report; + } + return null; +} + +function runPackDryRun(): PackReport { const output = runNpm(["pack", "--dry-run", "--json", "--ignore-scripts"]); - const jsonStart = output.indexOf("["); - const jsonEnd = output.lastIndexOf("]"); - const jsonPayload = - jsonStart >= 0 && jsonEnd > jsonStart ? output.slice(jsonStart, jsonEnd + 1) : output; - const parsed = JSON.parse(jsonPayload); - const packReport = Array.isArray(parsed) ? parsed[0] : null; + const packReport = parseJsonValuesOutput(output) + .map(findPackReport) + .find((report): report is PackReport => report !== null); if (!packReport || !Array.isArray(packReport.files)) { throw new Error("npm pack --dry-run --json did not return the expected files[] payload."); @@ -78,17 +126,17 @@ function formatBytes(bytes: number): string { } // --policy-only: skip the build (ensureAppStagingReady → build:cli) and the -// required-runtime-files check (which needs the built dist/), running ONLY the -// unexpected-files allowlist check. The unexpected files (e.g. stray bin/*.sh) are -// SOURCE files that `npm pack --dry-run` lists regardless of build, so this catches -// the "new file leaked into the tarball" regression cheaply on the fast-path (PR→release), -// instead of only on the release PR's full Package Artifact job. See incident v3.8.36 (#5029). +// required-runtime-files check (which needs the built dist/). Source-side policy checks +// still run against the real `npm pack --dry-run` file list: unexpected files (e.g. stray +// bin/*.sh), test/spec leaks, and missing MCP closure files. This catches source regressions +// cheaply on the fast-path (PR→release), instead of only on the release PR's full Package +// Artifact job. See incident v3.8.36 (#5029). const POLICY_ONLY = process.argv.includes("--policy-only"); try { if (!POLICY_ONLY) ensureAppStagingReady(); const packReport = runPackDryRun(); - const artifactPaths: string[] = packReport.files.map((file: any) => file.path); + const artifactPaths: string[] = packReport.files.map((file) => file.path); const unexpectedPaths: string[] = findUnexpectedArtifactPaths(artifactPaths, { exactPaths: PACK_ARTIFACT_ALLOWED_EXACT_PATHS, prefixPaths: PACK_ARTIFACT_ALLOWED_PATH_PREFIXES, @@ -97,11 +145,20 @@ try { ? [] : findMissingArtifactPaths(artifactPaths, PACK_ARTIFACT_REQUIRED_PATHS); + // #3821 — broad `files` prefixes (open-sse/, src/lib/, ...) would otherwise allow + // co-located *.test.* / __tests__ leaks; ban them explicitly on the real pack list. + const leakedTestPaths: string[] = findLeakedTestArtifactPaths(artifactPaths); + + // #3578 — MCP runs from published TypeScript source; every reachable file must pack. + const mcpClosure: string[] = computeMcpClosure(ROOT); + const missingMcpPaths: string[] = findMissingMcpClosurePaths(artifactPaths, mcpClosure); + console.log("📦 npm pack artifact summary"); console.log(` File: ${packReport.filename}`); console.log(` Entry count: ${packReport.entryCount}`); console.log(` Packed size: ${formatBytes(packReport.size)}`); console.log(` Unpacked size: ${formatBytes(packReport.unpackedSize)}`); + console.log(` MCP closure: ${mcpClosure.length} source files checked`); if (unexpectedPaths.length > 0) { console.error("\n❌ Unexpected files were found in the npm publish artifact:"); @@ -117,7 +174,33 @@ try { } } - if (unexpectedPaths.length > 0 || missingRequiredPaths.length > 0) { + if (leakedTestPaths.length > 0) { + console.error( + "\n❌ Test/spec files leaked into the npm publish artifact (tighten package.json files negations):" + ); + for (const leakedPath of leakedTestPaths) { + console.error(` - ${leakedPath}`); + } + } + + if (missingMcpPaths.length > 0) { + console.error( + "\n❌ MCP-reachable source files are missing from the npm publish artifact (would 404 --mcp):" + ); + for (const missingPath of missingMcpPaths) { + console.error(` - ${missingPath}`); + } + if (missingMcpPaths.includes(MCP_CLOSURE_SPOT_CHECK_PATH)) { + console.error(` (includes the #3578 bug file ${MCP_CLOSURE_SPOT_CHECK_PATH})`); + } + } + + if ( + unexpectedPaths.length > 0 || + missingRequiredPaths.length > 0 || + leakedTestPaths.length > 0 || + missingMcpPaths.length > 0 + ) { process.exit(1); } diff --git a/tests/unit/mcp-published-files-closure-3578.test.ts b/tests/unit/mcp-published-files-closure-3578.test.ts index 84d342a40e..7dd90da301 100644 --- a/tests/unit/mcp-published-files-closure-3578.test.ts +++ b/tests/unit/mcp-published-files-closure-3578.test.ts @@ -2,7 +2,13 @@ import test from "node:test"; import assert from "node:assert/strict"; import fs from "node:fs"; import path from "node:path"; -import { execFileSync } from "node:child_process"; + +import { + MCP_CLOSURE_SPOT_CHECK_PATH, + computeMcpClosure, + findMissingPackageFilesTestNegations, + isCoveredByFiles, +} from "../../scripts/build/mcpPublishedFilesClosure.ts"; // #3578 — `omniroute --mcp` crashed on npm installs with ERR_MODULE_NOT_FOUND for // src/lib/combos/steps.ts: the MCP server runs from raw TypeScript source and imports @@ -10,87 +16,22 @@ import { execFileSync } from "node:child_process"; // cherry-picked paths. This gate computes the MCP server's transitive import closure // and asserts every reachable src/ + open-sse/ file is covered by a package.json // `files` entry, so a missing dir can never silently ship a broken --mcp again. +// +// Live `npm pack` over-inclusion checks (#3821) live in +// `scripts/build/validate-pack-artifact.ts` (already `--ignore-scripts`) so concurrent +// `test:unit` never stalls on a monorepo pack walk / prepare→husky lifecycle. const ROOT = process.cwd(); -function resolveImport(fromFile: string, spec: string): string | null { - let base: string; - if (spec.startsWith("@/")) base = path.join("src", spec.slice(2)); - else if (spec.startsWith("@omniroute/open-sse/")) - base = path.join("open-sse", spec.slice("@omniroute/open-sse/".length)); - else if (spec === "@omniroute/open-sse") base = path.join("open-sse", "index"); - else if (spec.startsWith("./") || spec.startsWith("../")) - base = path.join(path.dirname(fromFile), spec); - else return null; // bare package — not our source - base = base.replace(/\.(ts|tsx|js|mjs)$/, ""); - const cands = [ - base + ".ts", - base + ".tsx", - path.join(base, "index.ts"), - path.join(base, "index.tsx"), - base + ".js", - base + ".mjs", - ]; - for (const c of cands) if (fs.existsSync(path.join(ROOT, c))) return c; - return null; -} - -function computeMcpClosure(): string[] { - const roots: string[] = []; - for (const f of fs.readdirSync(path.join(ROOT, "open-sse/mcp-server"))) { - if (f.endsWith(".ts")) roots.push("open-sse/mcp-server/" + f); - } - for (const d of ["open-sse/mcp-server/tools", "open-sse/mcp-server/schemas"]) { - const abs = path.join(ROOT, d); - if (fs.existsSync(abs)) - for (const f of fs.readdirSync(abs)) if (f.endsWith(".ts")) roots.push(d + "/" + f); - } - - const seen = new Set(); - const stack = [...roots]; - const importRe = - /(?:import|export)[^"']*?from\s*["']([^"']+)["']|import\s*\(\s*["']([^"']+)["']\s*\)/g; - while (stack.length) { - const f = stack.pop() as string; - if (seen.has(f)) continue; - seen.add(f); - let src: string; - try { - src = fs.readFileSync(path.join(ROOT, f), "utf8"); - } catch { - continue; - } - let m: RegExpExecArray | null; - while ((m = importRe.exec(src))) { - const spec = m[1] || m[2]; - if (!spec) continue; - const r = resolveImport(f, spec); - if (r && !seen.has(r)) stack.push(r); - } - } - return [...seen].filter((f) => f.startsWith("src/") || f.startsWith("open-sse/")); -} - -function isCoveredByFiles(file: string, filesEntries: string[]): boolean { - for (const entry of filesEntries) { - if (entry.endsWith("/")) { - if (file === entry.slice(0, -1) || file.startsWith(entry)) return true; - } else if (file === entry || file.startsWith(entry + "/")) { - return true; - } - } - return false; -} - test("#3578 every MCP-server source file is covered by package.json files", () => { const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, "package.json"), "utf8")); const filesEntries: string[] = pkg.files || []; - const closure = computeMcpClosure(); + const closure = computeMcpClosure(ROOT); // Sanity: the closure must actually include the file the bug report hit. assert.ok( - closure.includes("src/lib/combos/steps.ts"), - "closure should include the file from the bug report (#3578)" + closure.includes(MCP_CLOSURE_SPOT_CHECK_PATH), + `closure should include the file from the bug report (#3578): ${MCP_CLOSURE_SPOT_CHECK_PATH}` ); const uncovered = closure.filter((f) => !isCoveredByFiles(f, filesEntries)); @@ -102,54 +43,14 @@ test("#3578 every MCP-server source file is covered by package.json files", () = ); }); -// #3821-review (LEDGER-1): the static `files` check above only guards UNDER-inclusion -// (every MCP file is allowlisted). It cannot see that the whole-directory entries -// (open-sse/, src/lib/, ...) also drag co-located test files into the tarball, nor that -// a future secret-bearing fixture under a shipped dir would publish. This test asserts -// the REAL `npm pack --dry-run` output in BOTH directions: the MCP closure is present AND -// no `__tests__` / `*.test.*` / `*.spec.*` file ships. It is the regression anchor for the -// `!**/*.test.*` negations in package.json `files`. -function packedFilePaths(): string[] { - // --dry-run writes no tarball; --json emits [{ files: [{ path }] }] on stdout. - const out = execFileSync("npm", ["pack", "--dry-run", "--json"], { - cwd: ROOT, - encoding: "utf8", - maxBuffer: 32 * 1024 * 1024, - stdio: ["ignore", "pipe", "ignore"], - }); - const parsed = JSON.parse(out) as Array<{ files?: Array<{ path: string }> }>; - const entry = parsed[0]; - assert.ok(entry?.files?.length, "npm pack --dry-run returned no files"); - return entry.files!.map((f) => f.path); -} - -const TEST_FILE_RE = /(?:^|\/)__tests__\/|\.(?:test|spec)\.[cm]?[jt]sx?$/; - -test("#3578/#3821 npm pack ships the MCP closure but no test files", () => { - const packed = packedFilePaths(); - const packedSet = new Set(packed); - - // Direction 1 — under-inclusion: every MCP-reachable source file is actually packed. - const closure = computeMcpClosure(); - const missing = closure.filter((f) => !packedSet.has(f)); +test("#3821 package.json files keeps test/spec negations (static)", () => { + const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, "package.json"), "utf8")); + const filesEntries: string[] = pkg.files || []; + const missing = findMissingPackageFilesTestNegations(filesEntries); assert.deepEqual( missing, [], - `MCP-reachable source files are missing from the published tarball (would 404 --mcp):\n` + + `These package.json "files" negations are missing — without them co-located tests can ship:\n` + missing.map((f) => " - " + f).join("\n") ); - // Spot-check the file from the original bug report. - assert.ok( - packedSet.has("src/lib/combos/steps.ts"), - "src/lib/combos/steps.ts (the #3578 bug file) must be in the tarball" - ); - - // Direction 2 — over-inclusion: no co-located test / spec file is published. - const shippedTests = packed.filter((f) => TEST_FILE_RE.test(f)); - assert.deepEqual( - shippedTests, - [], - `These test files leaked into the npm tarball — tighten package.json "files" negations:\n` + - shippedTests.map((f) => " - " + f).join("\n") - ); }); diff --git a/tests/unit/mcp-published-files-closure-helpers.test.ts b/tests/unit/mcp-published-files-closure-helpers.test.ts new file mode 100644 index 0000000000..62e8795b03 --- /dev/null +++ b/tests/unit/mcp-published-files-closure-helpers.test.ts @@ -0,0 +1,70 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + REQUIRED_PACKAGE_FILES_TEST_NEGATIONS, + findLeakedTestArtifactPaths, + findMissingMcpClosurePaths, + findMissingPackageFilesTestNegations, + isCoveredByFiles, +} from "../../scripts/build/mcpPublishedFilesClosure.ts"; + +test("findLeakedTestArtifactPaths flags co-located test and __tests__ paths", () => { + assert.deepEqual( + findLeakedTestArtifactPaths([ + "open-sse/mcp-server/server.ts", + "open-sse/mcp-server/__tests__/server.test.ts", + "src/lib/combos/steps.ts", + "src/lib/combos/steps.test.ts", + "src/lib/foo.spec.tsx", + "src/lib/foo.test.mjs", + ]), + [ + "open-sse/mcp-server/__tests__/server.test.ts", + "src/lib/combos/steps.test.ts", + "src/lib/foo.spec.tsx", + "src/lib/foo.test.mjs", + ] + ); +}); + +test("findLeakedTestArtifactPaths returns empty when no tests ship", () => { + assert.deepEqual( + findLeakedTestArtifactPaths(["open-sse/mcp-server/server.ts", "src/lib/combos/steps.ts"]), + [] + ); +}); + +test("findMissingMcpClosurePaths reports closure members absent from the pack list", () => { + assert.deepEqual( + findMissingMcpClosurePaths( + ["open-sse/mcp-server/server.ts", "src/lib/combos/other.ts"], + ["open-sse/mcp-server/server.ts", "src/lib/combos/steps.ts", "src/lib/combos/other.ts"] + ), + ["src/lib/combos/steps.ts"] + ); +}); + +test("findMissingMcpClosurePaths is empty when the full closure is packed", () => { + const closure = ["open-sse/mcp-server/server.ts", "src/lib/combos/steps.ts"]; + assert.deepEqual(findMissingMcpClosurePaths(closure, closure), []); +}); + +test("isCoveredByFiles honours directory and exact package.json files entries", () => { + assert.equal(isCoveredByFiles("src/lib/combos/steps.ts", ["src/lib/"]), true); + assert.equal(isCoveredByFiles("src/lib/combos/steps.ts", ["src/lib/combos/steps.ts"]), true); + assert.equal(isCoveredByFiles("src/domain/foo.ts", ["src/lib/"]), false); + // Negations must not count as positive coverage. + assert.equal(isCoveredByFiles("src/lib/combos/steps.ts", ["!**/*.test.ts"]), false); +}); + +test("findMissingPackageFilesTestNegations flags deleted negations", () => { + assert.deepEqual( + findMissingPackageFilesTestNegations([...REQUIRED_PACKAGE_FILES_TEST_NEGATIONS]), + [] + ); + assert.deepEqual( + findMissingPackageFilesTestNegations(["bin/", "open-sse/", "!**/*.test.ts"]), + REQUIRED_PACKAGE_FILES_TEST_NEGATIONS.filter((e) => e !== "!**/*.test.ts") + ); +}); diff --git a/tests/unit/pack-artifact-policy.test.ts b/tests/unit/pack-artifact-policy.test.ts index 4c5ae49968..d44f0c31e8 100644 --- a/tests/unit/pack-artifact-policy.test.ts +++ b/tests/unit/pack-artifact-policy.test.ts @@ -11,6 +11,8 @@ import { findMissingArtifactPaths, findUnexpectedArtifactPaths, normalizeArtifactPath, + parseJsonArrayOutput, + parseJsonValuesOutput, } from "../../scripts/build/pack-artifact-policy.ts"; test("normalizeArtifactPath normalizes slashes and leading relative markers", () => { @@ -20,6 +22,39 @@ test("normalizeArtifactPath normalizes slashes and leading relative markers", () ); }); +test("parseJsonArrayOutput extracts the first valid array from mixed command output", () => { + const output = [ + "notice [not-json]", + '[{"path":"src/[literal].ts","files":[["nested"]]}]', + "notice [second-array]", + ].join("\n"); + assert.deepEqual(parseJsonArrayOutput(output), [ + { path: "src/[literal].ts", files: [["nested"]] }, + ]); +}); + +test("parseJsonArrayOutput can skip valid arrays that are not the target payload", () => { + const output = `[] +[{"filename":"omniroute.tgz","files":[{"path":"src/index.ts"}]}]`; + assert.deepEqual( + parseJsonArrayOutput(output, (candidate) => + candidate.some( + (entry) => + typeof entry === "object" && + entry !== null && + Array.isArray((entry as { files?: unknown }).files) + ) + ), + [{ filename: "omniroute.tgz", files: [{ path: "src/index.ts" }] }] + ); +}); + +test("parseJsonValuesOutput extracts object reports as well as arrays", () => { + assert.deepEqual(parseJsonValuesOutput('notice\n{"files":[{"path":"src/index.ts"}]}'), [ + { files: [{ path: "src/index.ts" }] }, + ]); +}); + test("findUnexpectedArtifactPaths flags staged app files outside the allowlist", () => { const unexpectedPaths = findUnexpectedArtifactPaths( [