chore(deps): check each shared source contributed, not the aggregate count
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
This commit is contained in:
parent
3638185e0f
commit
48d74ba423
2 changed files with 72 additions and 5 deletions
|
|
@ -279,6 +279,23 @@ export function isSharedSourceFile(file) {
|
||||||
return SHARED_SOURCE_DIRS.some((dir) => file.startsWith(`${dir}/`));
|
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. */
|
/** Tracked TypeScript files under the shared first-party source trees. */
|
||||||
function sharedSourceFiles() {
|
function sharedSourceFiles() {
|
||||||
const out = execFileSync(
|
const out = execFileSync(
|
||||||
|
|
@ -334,9 +351,19 @@ export function main() {
|
||||||
}
|
}
|
||||||
|
|
||||||
const files = sharedSourceFiles();
|
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(
|
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);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ import {
|
||||||
majorOf,
|
majorOf,
|
||||||
packageNameOf,
|
packageNameOf,
|
||||||
partitionSkew,
|
partitionSkew,
|
||||||
|
sourcesWithNoFiles,
|
||||||
topLevelLockVersions,
|
topLevelLockVersions,
|
||||||
} from "./verify-dep-lockstep.mjs";
|
} 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);
|
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", () => {
|
test("packageNameOf: bare names, scopes, and subpaths", () => {
|
||||||
const cases = [
|
const cases = [
|
||||||
["zod", "zod"],
|
["zod", "zod"],
|
||||||
|
|
@ -120,8 +161,7 @@ test("importedPackageNames: comment trivia between tokens (Copilot, #1962)", ()
|
||||||
|
|
||||||
test("importedPackageNames: line-comment trivia, not just block (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,
|
// `//` 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`
|
// so a specifier can sit on the next line and these are still valid imports.
|
||||||
// branch rather than by the line-comment branch.
|
|
||||||
const source = [
|
const source = [
|
||||||
"import { a } from // reason",
|
"import { a } from // reason",
|
||||||
' "express";',
|
' "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 = `
|
const source = `
|
||||||
import { z } from "zod/v4";
|
import { z } from "zod/v4";
|
||||||
export type { Foo } from '@modelcontextprotocol/core';
|
export type { Foo } from '@modelcontextprotocol/core';
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue