From 48d74ba423e83b8d6ad3f207913e42bfa055c28f Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 10 Aug 2026 21:41:31 -0400 Subject: [PATCH] chore(deps): check each shared source contributed, not the aggregate count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From Copilot's eighth pass of #1962. The no-op guard tested only `files.length === 0`, which stopped meaning anything once `SHARED_SOURCE_FILES` was added: `vitest.shared.mts` alone keeps the total nonzero, so a moved or renamed `core/` would leave the guard deriving candidates from a near-empty set and passing on skew it exists to catch. `sourcesWithNoFiles` now requires every configured dir and every named file to have contributed, and names the ones that didn't. Directory matching is on a path separator, so a prefix sibling (`core-internal/`) can't vouch for a renamed `core/`. Also drops two test comments left stale by the move to `ts.preProcessFile` — they still described the deleted TRIVIA regex. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 --- scripts/verify-dep-lockstep.mjs | 31 +++++++++++++++++-- scripts/verify-dep-lockstep.test.mjs | 46 ++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/scripts/verify-dep-lockstep.mjs b/scripts/verify-dep-lockstep.mjs index 8d20c206..de4cccf0 100644 --- a/scripts/verify-dep-lockstep.mjs +++ b/scripts/verify-dep-lockstep.mjs @@ -279,6 +279,23 @@ export function isSharedSourceFile(file) { return SHARED_SOURCE_DIRS.some((dir) => file.startsWith(`${dir}/`)); } +/** + * Which configured shared sources contributed no file to `files`. Each dir and + * each individually-named file must be represented; an aggregate count can't + * see one of them going missing, because the others keep the total nonzero. + */ +export function sourcesWithNoFiles( + files, + dirs = SHARED_SOURCE_DIRS, + named = SHARED_SOURCE_FILES, +) { + const missingDirs = dirs.filter( + (dir) => !files.some((f) => f.startsWith(`${dir}/`)), + ); + const missingNamed = named.filter((name) => !files.includes(name)); + return [...missingDirs, ...missingNamed]; +} + /** Tracked TypeScript files under the shared first-party source trees. */ function sharedSourceFiles() { const out = execFileSync( @@ -334,9 +351,19 @@ export function main() { } const files = sharedSourceFiles(); - if (files.length === 0) { + // Per-source, not an aggregate count (Copilot, #1962): `vitest.shared.mts` + // alone keeps the total nonzero, so a moved or renamed `core/` would leave + // the guard checking a near-empty candidate set and passing. Every configured + // source must contribute, or the enumeration is broken. + const empty = sourcesWithNoFiles(files); + if (empty.length > 0) { console.error( - `verify:dep-lockstep — found no tracked sources under ${SHARED_SOURCE_DIRS.join(", ")}. The guard would check nothing; fix the enumeration.`, + `verify:dep-lockstep — ${empty.length} configured shared source(s) matched no tracked file:\n`, + ); + for (const source of empty) console.error(` ${source}`); + console.error( + "\nThe guard would derive its candidates from an incomplete set and pass on skew it should catch." + + "\nA path was moved or renamed — fix SHARED_SOURCE_DIRS / SHARED_SOURCE_FILES in this file.", ); process.exit(1); } diff --git a/scripts/verify-dep-lockstep.test.mjs b/scripts/verify-dep-lockstep.test.mjs index 6fd9ecac..89268ee4 100644 --- a/scripts/verify-dep-lockstep.test.mjs +++ b/scripts/verify-dep-lockstep.test.mjs @@ -13,6 +13,7 @@ import { majorOf, packageNameOf, partitionSkew, + sourcesWithNoFiles, topLevelLockVersions, } from "./verify-dep-lockstep.mjs"; @@ -47,6 +48,46 @@ test("isSharedSourceFile: non-TS files and other trees are excluded", () => { assert.equal(isSharedSourceFile(file), false, file); }); +test("sourcesWithNoFiles: each configured source must contribute (Copilot, #1962)", () => { + const dirs = ["core", "test-servers/src"]; + const named = ["vitest.shared.mts"]; + const complete = [ + "core/mcp/a.ts", + "test-servers/src/b.ts", + "vitest.shared.mts", + ]; + assert.deepEqual(sourcesWithNoFiles(complete, dirs, named), []); + + // The failure an aggregate count can't see: `core/` moved, but the other two + // sources keep `files.length` nonzero, so the guard would derive candidates + // from an incomplete set and pass on skew it should catch. + assert.deepEqual( + sourcesWithNoFiles( + ["test-servers/src/b.ts", "vitest.shared.mts"], + dirs, + named, + ), + ["core"], + ); + assert.deepEqual( + sourcesWithNoFiles(["core/mcp/a.ts", "test-servers/src/b.ts"], dirs, named), + ["vitest.shared.mts"], + ); + assert.deepEqual(sourcesWithNoFiles([], dirs, named), [ + "core", + "test-servers/src", + "vitest.shared.mts", + ]); +}); + +test("sourcesWithNoFiles: a prefix sibling does not vouch for a dir", () => { + // `core-internal/` starts with `core` but is not it — the boundary check has + // to be on a path separator, or a renamed dir would look present. + assert.deepEqual(sourcesWithNoFiles(["core-internal/a.ts"], ["core"], []), [ + "core", + ]); +}); + test("packageNameOf: bare names, scopes, and subpaths", () => { const cases = [ ["zod", "zod"], @@ -120,8 +161,7 @@ test("importedPackageNames: comment trivia between tokens (Copilot, #1962)", () test("importedPackageNames: line-comment trivia, not just block (Copilot, #1962)", () => { // `//` runs to end-of-line and is legal in every position a block comment is, - // so these are valid imports too. The newline is matched by TRIVIA's `\\s` - // branch rather than by the line-comment branch. + // so a specifier can sit on the next line and these are still valid imports. const source = [ "import { a } from // reason", ' "express";', @@ -137,7 +177,7 @@ test("importedPackageNames: line-comment trivia, not just block (Copilot, #1962) ]); }); -test("importedPackageNames: the three specifier forms that introduce types", () => { +test("importedPackageNames: static, side-effect, and dynamic forms; builtins and relatives dropped", () => { const source = ` import { z } from "zod/v4"; export type { Foo } from '@modelcontextprotocol/core';