* feat(#3907): gates report no-input instead of asserting a verdict they never reached The three stdin-reading gates bound 2 to a stdin read error only, with no arm for stdin closed at zero bytes - so empty input flowed into the detector, found nothing, and exited 1, which each module's own comment defines as a negative verdict. An unset PHASE_SECTION made the UI gate assert the phase has no UI. Empty and whitespace-only input now exit NO_INPUT, and a read error exits UNAVAILABLE rather than a locally-invented 2, both resolved through the registry and delivered by terminateNow. The exit code was only half of it: under --json the same input emitted {detected:false}, byte-identical to the fabricated payload #3909 exists to fix, and the blocking coverage gate reads that payload. Empty input now emits the in-tree {skipped:true,reason} form with no detected key at all. teams-status is excluded: it never reads stdin and has no invented 2, so the four-module framing in the issue and ADR is wrong. The dead root bin/lib/ui-safety-gate.cjs is deleted - no installer reference, no workflow invocation, and the live fallback chains are for other modules. Its removal restores the unit tests to the module that actually ships; they had been asserting the stale copy's two-field shape, which is why it drifted unnoticed. * fix(#3907): drive gate tests through the process seam, and make removed-but-needed basename-precise CONTRIBUTING requires every subprocess go through tests/helpers/process-seam.cjs; two of the three gate suites hand-rolled spawnSync while the third, added in the same change, used runNode correctly for the identical injection case. Converted the blocks this change added, leaving pre-existing ones alone. Deleting one of two files sharing a basename made lint-removed-but-needed report 14 references that were all to the surviving canonical module - the false-positive class its own docstring names. It now matches on the deleted file's full path when a surviving file shares its basename, which is more precise rather than weaker: a genuine full-path reference still fails, and behaviour is unchanged when no basename collides. It immediately caught a docstring on this branch that spelled the deleted path. * test(#3907): update the one existing assertion that pinned the old empty-stdin verdict A pre-existing test asserted exit 1 on empty stdin - the defect this phase removes - and was missed because the change added new blocks without auditing existing ones pinning the old contract. Audited the rest: the other three status-1 assertions in that file all feed real input and are the genuine-negative controls that must keep returning 1, so exactly one was stale. The retired 2 is gone from the describe's contract comment too. * chore(#3907): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -168,7 +168,9 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = {
|
||||
'phase.test.cjs': ['docs/adr/3524-...md', 'docs/adr/3524-cjs-sdk-hard-seam.md'],
|
||||
'pr-branch-planning-filter.test.cjs': ['docs/readme.md'],
|
||||
'precommit-alias-drift-hook.test.cjs': ['docs/adr/0174-...md', 'docs/adr/0174-retire-gsd-sdk-package-boundary.md'],
|
||||
'removed-but-needed-lint.test.cjs': ['docs/getting-started.md', 'docs/gsd-new-workspace.md', 'docs/setup.md'],
|
||||
'removed-but-needed-lint.test.cjs': [
|
||||
'docs/README.md', 'docs/getting-started.md', 'docs/gsd-new-workspace.md', 'docs/setup.md', 'docs/some-doc.md',
|
||||
],
|
||||
'repo-invariants.test.cjs': ['docs/FEATURES.md', 'docs/workflows/README'],
|
||||
'require-issue-link-policy.test.cjs': ['docs/-prefixed', 'docs/CONFIGURATION.md', 'docs/a.md', 'docs/b.md', 'docs/guide.md'],
|
||||
'reviewer-manifest-body.test.cjs': ['docs/how-to/ship-a-reviewer-lane.md'],
|
||||
|
||||
@@ -46,6 +46,38 @@
|
||||
* A common basename (`index.js`, `config.json`) can coincidentally match an
|
||||
* unrelated file, and this only catches LITERAL string references — not a
|
||||
* variable holding the filename or a glob that happened to match it.
|
||||
*
|
||||
* ## Basename-collision refinement (#3907)
|
||||
*
|
||||
* #3907 deleted `bin/lib/ui-safety-gate.cjs` while the SEPARATE, still-live
|
||||
* `gsd-core/bin/lib/ui-safety-gate.cjs` survives — every reference to the
|
||||
* survivor (including it referencing itself) was misattributed to the
|
||||
* deletion, 14 false violations on a correct tree. This epic is about
|
||||
* de-duplicating modules, so "delete one of two files sharing a basename"
|
||||
* recurs by construction.
|
||||
*
|
||||
* The fix is precision, not suppression: for each deleted file, if its
|
||||
* basename is ALSO the basename of a file that still exists in the
|
||||
* post-diff tree — `git ls-files` (tracked files) UNIONED with the
|
||||
* already filesystem-walked corpus/testsCorpus file lists, because
|
||||
* `git ls-files` alone misses gitignored BUILD ARTIFACTS such as
|
||||
* `gsd-core/bin/lib/*.cjs` (compiled from `.cts`, never committed) —
|
||||
* matching switches from the bare basename to the deleted file's full
|
||||
* repo-relative path. Because the deletion is already committed on this
|
||||
* branch, none of these sources can list the deleted file itself, so any
|
||||
* hit is necessarily a DIFFERENT file. A genuine
|
||||
* full-path reference to the deleted file is still caught (strictly more
|
||||
* precise, not weaker); a bare-basename reference becomes correctly
|
||||
* unattributable to the deleted file specifically and is no longer
|
||||
* blamed on it. When no surviving file shares the basename, nothing
|
||||
* changes — the original basename rule still applies exactly as before.
|
||||
*
|
||||
* Residual known limit: a bare-basename reference to a deleted file whose
|
||||
* basename survives elsewhere is now invisible to this guard (it cannot
|
||||
* tell whether the bare mention meant the deleted file or its
|
||||
* basename-twin). Same trade the docstring already makes for prose
|
||||
* mentions above — a false violation reds a correct tree; a missed
|
||||
* ambiguous mention only loses one detection channel.
|
||||
*/
|
||||
|
||||
const fs = require('node:fs');
|
||||
@@ -78,6 +110,40 @@ function referencesBasename(content, basename) {
|
||||
return re.test(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: does `content` contain a literal reference to the FULL
|
||||
* repo-relative path `relPath` (used instead of {@link referencesBasename}
|
||||
* when the deleted file's basename collides with a surviving file — #3907)?
|
||||
*
|
||||
* Same delimited-match idea as `referencesBasename`, but the left boundary
|
||||
* additionally excludes `/` — a `/` immediately to the left would mean the
|
||||
* matched text is really a SUFFIX of a longer path (e.g. content contains
|
||||
* `gsd-core/bin/lib/x.cjs` and `relPath` is `bin/lib/x.cjs`: without this,
|
||||
* the survivor's own path would be mistaken for a reference to the
|
||||
* deleted file it merely shares a basename with).
|
||||
* @param {string} content
|
||||
* @param {string} relPath - repo-relative path, forward-slash separated
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function referencesPath(content, relPath) {
|
||||
const re = new RegExp(`(^|[^\\w./-])${escapeRegex(relPath)}($|[^\\w.-])`);
|
||||
return re.test(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: given the post-diff tree's file list (repo-relative paths), build
|
||||
* the set of basenames that still exist. Used to decide, per deleted file,
|
||||
* whether basename matching would be ambiguous (#3907).
|
||||
* @param {string[]} survivingFiles - repo-relative paths of files present
|
||||
* in the post-diff tree (must NOT include the deleted files themselves)
|
||||
* @returns {Set<string>}
|
||||
*/
|
||||
function buildSurvivingBasenames(survivingFiles) {
|
||||
const set = new Set();
|
||||
for (const f of survivingFiles) set.add(path.basename(f));
|
||||
return set;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: given the deleted file's basename, does content contain a
|
||||
* lockfile-dependent idiom (`npm ci`, `cache: 'npm'` / `cache: "npm"`)?
|
||||
@@ -110,15 +176,26 @@ function walk(dir) {
|
||||
* of the post-diff tree, find every surviving reference.
|
||||
* @param {string[]} deletedFiles - repo-relative deleted paths
|
||||
* @param {{ file: string, content: string }[]} corpus
|
||||
* @param {Set<string>} [survivingBasenames] - basenames still present in the
|
||||
* post-diff tree (#3907); when a deleted file's basename is in this set,
|
||||
* matching switches from basename to the deleted file's full path
|
||||
* @returns {{ deletedFile: string, referencedIn: string, reason: string }[]}
|
||||
*/
|
||||
function findSurvivingReferences(deletedFiles, corpus) {
|
||||
function findSurvivingReferences(deletedFiles, corpus, survivingBasenames = new Set()) {
|
||||
const violations = [];
|
||||
for (const deletedFile of deletedFiles) {
|
||||
const basename = path.basename(deletedFile);
|
||||
const collides = survivingBasenames.has(basename);
|
||||
for (const { file, content } of corpus) {
|
||||
if (referencesBasename(content, basename)) {
|
||||
violations.push({ deletedFile, referencedIn: file, reason: `basename '${basename}' still referenced` });
|
||||
const matched = collides ? referencesPath(content, deletedFile) : referencesBasename(content, basename);
|
||||
if (matched) {
|
||||
violations.push({
|
||||
deletedFile,
|
||||
referencedIn: file,
|
||||
reason: collides
|
||||
? `full path '${deletedFile}' still referenced (basename '${basename}' also belongs to a surviving file, so matched by path — #3907)`
|
||||
: `basename '${basename}' still referenced`,
|
||||
});
|
||||
}
|
||||
}
|
||||
if (basename === 'package-lock.json') {
|
||||
@@ -154,9 +231,12 @@ function findSurvivingReferences(deletedFiles, corpus) {
|
||||
*
|
||||
* @param {string} line
|
||||
* @param {string} basename
|
||||
* @param {string} [matchTarget] - text to look for as a quoted object key
|
||||
* (#3907: the deleted file's full path when its basename collides with a
|
||||
* surviving file; defaults to `basename`, the original behaviour)
|
||||
* @returns {'asserts-absence'|'pins-existence'|null}
|
||||
*/
|
||||
function classifyTestReference(line, basename) {
|
||||
function classifyTestReference(line, basename, matchTarget = basename) {
|
||||
const negatedCheck =
|
||||
/!\s*[A-Za-z_$][\w.$]*\.(includes|indexOf|search|startsWith|endsWith|match)\s*\(/.test(line) ||
|
||||
/!\s*(fs\.)?(existsSync|statSync)\s*\(/.test(line) ||
|
||||
@@ -170,7 +250,7 @@ function classifyTestReference(line, basename) {
|
||||
// Template literal needs \\s so the RegExp constructor receives \s — a
|
||||
// bare \s in a template literal collapses to 's' and silently matches a
|
||||
// literal 's' instead of whitespace.
|
||||
if (new RegExp(`['"\`]${escapeRegex(basename)}['"\`]\\s*:`).test(line)) return 'pins-existence';
|
||||
if (new RegExp(`['"\`]${escapeRegex(matchTarget)}['"\`]\\s*:`).test(line)) return 'pins-existence';
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -183,24 +263,38 @@ function classifyTestReference(line, basename) {
|
||||
*
|
||||
* @param {string[]} deletedFiles - repo-relative deleted paths
|
||||
* @param {{ file: string, content: string }[]} testsCorpus
|
||||
* @param {Set<string>} [survivingBasenames] - basenames still present in the
|
||||
* post-diff tree (#3907); when a deleted file's basename is in this set,
|
||||
* matching switches from basename to the deleted file's full path, same
|
||||
* refinement as {@link findSurvivingReferences} so the two scanners
|
||||
* cannot disagree about what "a reference" means
|
||||
* @returns {{ deletedFile: string, referencedIn: string, reason: string }[]}
|
||||
*/
|
||||
function findSurvivingTestReferences(deletedFiles, testsCorpus) {
|
||||
function findSurvivingTestReferences(deletedFiles, testsCorpus, survivingBasenames = new Set()) {
|
||||
const violations = [];
|
||||
for (const deletedFile of deletedFiles) {
|
||||
const basename = path.basename(deletedFile);
|
||||
const refRe = new RegExp(`(^|[^\\w.-])${escapeRegex(basename)}($|[^\\w.-])`);
|
||||
const collides = survivingBasenames.has(basename);
|
||||
const matchTarget = collides ? deletedFile : basename;
|
||||
// The left-boundary class matches referencesPath's when collides
|
||||
// (excludes '/' so a survivor's own longer path isn't mistaken for a
|
||||
// reference to the deleted file it merely shares a basename with).
|
||||
const refRe = collides
|
||||
? new RegExp(`(^|[^\\w./-])${escapeRegex(matchTarget)}($|[^\\w.-])`)
|
||||
: new RegExp(`(^|[^\\w.-])${escapeRegex(matchTarget)}($|[^\\w.-])`);
|
||||
for (const { file, content } of testsCorpus) {
|
||||
for (const line of content.split(/\r?\n/)) {
|
||||
if (!refRe.test(line)) continue;
|
||||
const kind = classifyTestReference(line, basename);
|
||||
const kind = classifyTestReference(line, basename, matchTarget);
|
||||
if (kind === 'pins-existence') {
|
||||
violations.push({
|
||||
deletedFile,
|
||||
referencedIn: file,
|
||||
// control-char-stripped + capped: this text is echoed into lint
|
||||
// output that AI agents read as trusted instructions
|
||||
reason: `pins-existence: test depends on deleted basename '${basename}' — ${sanitizeEcho(line.trim())}`,
|
||||
reason: collides
|
||||
? `pins-existence: test depends on deleted path '${matchTarget}' — ${sanitizeEcho(line.trim())}`
|
||||
: `pins-existence: test depends on deleted basename '${basename}' — ${sanitizeEcho(line.trim())}`,
|
||||
});
|
||||
}
|
||||
// 'asserts-absence' is correct post-deletion state; null is a
|
||||
@@ -231,6 +325,28 @@ function getDeletedFiles(root, baseRef) {
|
||||
.map((line) => line.slice(2));
|
||||
}
|
||||
|
||||
/**
|
||||
* All tracked files at the current tree state (the post-diff tree, since
|
||||
* this scan runs against a branch where the deletion is already committed —
|
||||
* `git ls-files` therefore CANNOT list a deleted file, so any hit for a
|
||||
* deleted file's basename is necessarily a different, surviving file).
|
||||
* Used to build `survivingBasenames` for the #3907 collision refinement.
|
||||
* @param {string} root
|
||||
* @returns {string[]} repo-relative paths, forward-slash separated
|
||||
*/
|
||||
function getSurvivingFiles(root) {
|
||||
const out = cp.execFileSync('git', ['ls-files'], {
|
||||
cwd: root,
|
||||
encoding: 'utf8',
|
||||
timeout: 15000,
|
||||
});
|
||||
return out
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter(Boolean)
|
||||
.map((f) => f.replace(/\\/g, '/'));
|
||||
}
|
||||
|
||||
function buildCorpus(root) {
|
||||
const corpus = [];
|
||||
for (const rel of SCAN_ROOTS) {
|
||||
@@ -257,7 +373,6 @@ function scan(root, baseRef) {
|
||||
const deletedFiles = getDeletedFiles(root, baseRef);
|
||||
if (deletedFiles.length === 0) return [];
|
||||
const corpus = buildCorpus(root);
|
||||
const violations = findSurvivingReferences(deletedFiles, corpus);
|
||||
// tests/ arm (#3565): same deletions, discriminated references. A bare
|
||||
// mention that pins nothing is skipped; an absence assertion is the
|
||||
// correct post-deletion state; only a pin on a deleted file fails.
|
||||
@@ -272,7 +387,21 @@ function scan(root, baseRef) {
|
||||
// unreadable (broken symlink, binary that slipped past SKIP_EXT) — skip
|
||||
}
|
||||
}
|
||||
violations.push(...findSurvivingTestReferences(deletedFiles, testsCorpus));
|
||||
// #3907: per-deletion basename-collision detection — if a surviving file
|
||||
// shares the basename, matching switches from basename to full path (see
|
||||
// the header's "Basename-collision refinement" section) for BOTH arms.
|
||||
// `git ls-files` alone misses gitignored BUILD ARTIFACTS (e.g.
|
||||
// gsd-core/bin/lib/*.cjs, compiled from .cts and never committed) —
|
||||
// exactly the #3907 collision partner — so it is unioned with the
|
||||
// already filesystem-walked corpus/testsCorpus file lists, which do see
|
||||
// them.
|
||||
const survivingBasenames = buildSurvivingBasenames([
|
||||
...getSurvivingFiles(root),
|
||||
...corpus.map((c) => c.file),
|
||||
...testsCorpus.map((t) => t.file),
|
||||
]);
|
||||
const violations = findSurvivingReferences(deletedFiles, corpus, survivingBasenames);
|
||||
violations.push(...findSurvivingTestReferences(deletedFiles, testsCorpus, survivingBasenames));
|
||||
return violations;
|
||||
}
|
||||
|
||||
@@ -305,11 +434,14 @@ function main() {
|
||||
|
||||
module.exports = {
|
||||
referencesBasename,
|
||||
referencesPath,
|
||||
buildSurvivingBasenames,
|
||||
referencesNpmLockfileDependency,
|
||||
findSurvivingReferences,
|
||||
classifyTestReference,
|
||||
findSurvivingTestReferences,
|
||||
getDeletedFiles,
|
||||
getSurvivingFiles,
|
||||
buildCorpus,
|
||||
scan,
|
||||
SCAN_ROOTS,
|
||||
|
||||
Reference in New Issue
Block a user