feat(#2128): phase-id anti-divergence guard — canonical token source + drift scanner + guards
Phase 4 of epic #2121 (ADR-2121 Decision 7), closing the recurrence loop that produced #2111 / #2114 / #2104: no module outside src/phase-id.cts may re-implement phase-ID parsing without failing CI. - phase-id.cts: add PHASE_NUMBER_TOKEN_SOURCE — the canonical phase-number-token grammar (\d+[A-Z]?(?:\.\d+)*) for enumeration/scan call sites, the ANY-phase counterpart to phaseMarkdownRegexSource(n)'s known-number lookup. Extend-only (never touches normalizePhaseName; blast radius 79 fns / CRITICAL). - scripts/lint-phase-id-drift.cjs: pure findPhaseIdRegexDrift(text) + scanRepo(root), wired to `npm run check:phase-id-drift`. Flags a literal re-derivation of the canonical token (both /\d/ and new-RegExp `\\d` escaping, plus the [A-Za-z] and [.-] near-variants) anywhere in src/** outside phase-id.cts, unless sanctioned with `// phase-id-owner: <reason>`. Narrow by design: bare \d+, digits-only captures, \w ids, status-message text and pipe-tables are not flagged. - tests/phase-id-drift-guard.test.cjs: fail-first drift cases (AC1) + live scanRepo(ROOT) zero-drift (AC3) + identity guard — phase-id.cjs exports the complete locked surface and no consumer re-exports a divergent copy (AC2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -78,6 +78,7 @@
|
||||
"check:env": "node scripts/check-env.cjs",
|
||||
"check:alias-drift": "node scripts/check-alias-drift.cjs",
|
||||
"check:identity-drift": "node scripts/lint-package-identity-drift.cjs",
|
||||
"check:phase-id-drift": "node scripts/lint-phase-id-drift.cjs",
|
||||
"check:integrity": "node scripts/check-npm-integrity.cjs",
|
||||
"build": "npm run generate:identity && npm run build:lib && npm run gen:plugin-skills && npm run gen:loop-host-contract && npm run gen:capability-registry && npm run build:hooks",
|
||||
"build:hooks": "node scripts/build-hooks.js",
|
||||
|
||||
133
scripts/lint-phase-id-drift.cjs
Normal file
133
scripts/lint-phase-id-drift.cjs
Normal file
@@ -0,0 +1,133 @@
|
||||
#!/usr/bin/env node
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Anti-divergence drift guard for the phase-identifier parsing seam
|
||||
* (epic #2121, Phase 4 / issue #2128, locked by ADR-2121 Decision 7).
|
||||
*
|
||||
* `src/phase-id.cts` is the SINGLE canonical owner of phase-ID parsing. Its
|
||||
* `PHASE_NUMBER_TOKEN_SOURCE` (and `phaseMarkdownRegexSource` for a known number)
|
||||
* is the one place the phase-number-token grammar `\d+[A-Z]?(?:\.\d+)*` is
|
||||
* defined. Every other module that scans/enumerates phase headings must build
|
||||
* its regex from that source rather than re-deriving the grammar as a literal —
|
||||
* otherwise the trio drifts again (the #2111 / #2114 / #2104 recurrence loop this
|
||||
* epic closes).
|
||||
*
|
||||
* This lint makes the invariant machine-enforced: it FAILS the moment a literal
|
||||
* re-derivation of the canonical token grammar is introduced anywhere in
|
||||
* `src/**` outside `phase-id.cts`, unless the site is deliberately sanctioned
|
||||
* with a `// phase-id-owner: <reason>` comment (on the same line or the line
|
||||
* directly above). Sites that build their regex from `PHASE_NUMBER_TOKEN_SOURCE`
|
||||
* carry no literal grammar and pass automatically.
|
||||
*
|
||||
* Detection is intentionally NARROW: only the contiguous canonical token
|
||||
* (`\d+[A-Z]?(?:\.\d+)*`, its `[A-Za-z]` and `[.-]` near-variants, in both
|
||||
* regex-literal `\d` and `new RegExp` template `\\d` escaping) is drift. Bare
|
||||
* `\d+` probes, `[\w][\w.-]*` ids, digits-only captures, status-message text
|
||||
* (`Phase\s+\d`), and pipe-table structures are NOT phase-token re-derivations
|
||||
* and are not flagged.
|
||||
*/
|
||||
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
// The canonical phase-number token as it appears in SOURCE TEXT:
|
||||
// \d+[A-Z]?(?:\.\d+)* in a regex literal -> one backslash before d/.
|
||||
// \\d+[A-Z]?(?:\\.\\d+)* in a template string -> two backslashes
|
||||
// Also tolerate the [A-Za-z] letter-class and the [.-] (dot-or-dash) separator
|
||||
// near-variants that a few enumeration call sites use.
|
||||
const TOKEN_DRIFT_RE = /\\{1,2}d\+\[A-Z(?:a-z)?\]\??\(\?:(?:\\{1,2}\.|\[\.-\])\\{1,2}d\+\)\*/;
|
||||
|
||||
const OWNER_MARK = 'phase-id-owner:';
|
||||
const CANON_REF = 'PHASE_NUMBER_TOKEN_SOURCE';
|
||||
|
||||
/**
|
||||
* Pure: find every literal re-derivation of the canonical phase-number token in
|
||||
* `text` that is NOT sanctioned. A site is sanctioned when its line — or the
|
||||
* line directly above it — contains `// phase-id-owner:`, or when the line
|
||||
* references `PHASE_NUMBER_TOKEN_SOURCE` (i.e. it is built from the canonical
|
||||
* source, not a literal). Returns [{ line, found }].
|
||||
*/
|
||||
function findPhaseIdRegexDrift(text) {
|
||||
const out = [];
|
||||
const lines = text.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
const m = TOKEN_DRIFT_RE.exec(line);
|
||||
if (!m) continue;
|
||||
if (line.includes(OWNER_MARK)) continue;
|
||||
if (i > 0 && lines[i - 1].includes(OWNER_MARK)) continue;
|
||||
if (line.includes(CANON_REF)) continue;
|
||||
out.push({ line: i + 1, found: m[0] });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// Authored TypeScript source only (the generated bin/lib/*.cjs mirror it).
|
||||
const SCAN_DIRS = ['src'];
|
||||
const SCAN_EXT = new Set(['.cts', '.ts', '.mts']);
|
||||
// The canonical owner defines the grammar; it is exempt by construction.
|
||||
const EXEMPT = new Set([path.join('src', 'phase-id.cts')]);
|
||||
|
||||
function walk(dir, acc) {
|
||||
let entries;
|
||||
try {
|
||||
entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return acc;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.git') continue;
|
||||
walk(full, acc);
|
||||
} else if (entry.isFile() && SCAN_EXT.has(path.extname(entry.name))) {
|
||||
acc.push(full);
|
||||
}
|
||||
}
|
||||
return acc;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan the authored source tree and return every unsanctioned phase-token
|
||||
* re-derivation, each annotated with the repo-relative file path.
|
||||
*/
|
||||
function scanRepo(root) {
|
||||
const violations = [];
|
||||
for (const dir of SCAN_DIRS) {
|
||||
for (const file of walk(path.join(root, dir), [])) {
|
||||
const rel = path.relative(root, file);
|
||||
if (EXEMPT.has(rel)) continue;
|
||||
let text;
|
||||
try {
|
||||
text = fs.readFileSync(file, 'utf8');
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
for (const d of findPhaseIdRegexDrift(text)) {
|
||||
violations.push({ file: rel, ...d });
|
||||
}
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
function main() {
|
||||
const root = path.join(__dirname, '..');
|
||||
const violations = scanRepo(root);
|
||||
if (violations.length === 0) {
|
||||
process.stdout.write('ok phase-id-drift: no unsanctioned phase-token re-derivations outside phase-id.cts\n');
|
||||
return;
|
||||
}
|
||||
process.stderr.write('phase-id-drift: literal re-derivation(s) of the canonical phase-number token found.\n');
|
||||
process.stderr.write('Build the regex from phase-id.cjs `PHASE_NUMBER_TOKEN_SOURCE` (or phaseMarkdownRegexSource for a\n');
|
||||
process.stderr.write('known number), or sanction the site with a `// phase-id-owner: <reason>` comment:\n');
|
||||
for (const d of violations) {
|
||||
process.stderr.write(` ${d.file}:${d.line} ${d.found}\n`);
|
||||
}
|
||||
process.exitCode = 1;
|
||||
}
|
||||
|
||||
if (require.main === module) main();
|
||||
|
||||
module.exports = { findPhaseIdRegexDrift, scanRepo, TOKEN_DRIFT_RE };
|
||||
@@ -41,6 +41,18 @@ const OPTIONAL_PROJECT_CODE_PREFIX_SOURCE = '(?:[A-Z][A-Z0-9_]*-)?';
|
||||
// source. Both forms must change together; see the #1729 regression test.
|
||||
const OPTIONAL_PHASE_TAG_SOURCE = '(?:\\s*\\([^)\\n]*\\))?';
|
||||
|
||||
// #2128: the canonical phase-NUMBER-TOKEN grammar — a phase number with an
|
||||
// optional single-letter variant suffix and optional dotted sub-phases
|
||||
// (1, 01, 12A, 12.1, 3.2.1). This is the ENUMERATION/scan counterpart to
|
||||
// phaseMarkdownRegexSource: use phaseMarkdownRegexSource(n) to build a source
|
||||
// for ONE KNOWN number; reference this constant when a call site must match ANY
|
||||
// phase and capture its token. Enumeration/parse sites inline this into a
|
||||
// `new RegExp(...)` instead of re-deriving the grammar as a literal, so every
|
||||
// phase-token producer shares one owner. The anti-divergence guard
|
||||
// (scripts/lint-phase-id-drift.cjs) fails CI if a literal re-derivation is
|
||||
// introduced outside this module without a `// phase-id-owner:` justification.
|
||||
const PHASE_NUMBER_TOKEN_SOURCE = '\\d+[A-Z]?(?:\\.\\d+)*';
|
||||
|
||||
function stripProjectCodePrefix(value: unknown, caseInsensitive = true): string {
|
||||
const input = String(value);
|
||||
const re = caseInsensitive ? PROJECT_CODE_PREFIX_STRIP_RE_I : PROJECT_CODE_PREFIX_STRIP_RE;
|
||||
@@ -350,6 +362,7 @@ export = {
|
||||
escapeRegex,
|
||||
OPTIONAL_PROJECT_CODE_PREFIX_SOURCE,
|
||||
OPTIONAL_PHASE_TAG_SOURCE,
|
||||
PHASE_NUMBER_TOKEN_SOURCE,
|
||||
stripProjectCodePrefix,
|
||||
normalizePhaseName,
|
||||
getMilestoneFromPhaseId,
|
||||
|
||||
143
tests/phase-id-drift-guard.test.cjs
Normal file
143
tests/phase-id-drift-guard.test.cjs
Normal file
@@ -0,0 +1,143 @@
|
||||
'use strict';
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
/**
|
||||
* Anti-divergence guard for the phase-identifier parsing seam
|
||||
* (epic #2121 Phase 4 / issue #2128, ADR-2121 Decision 7).
|
||||
*
|
||||
* `src/phase-id.cts` is the single canonical owner of phase-ID parsing. Two guards
|
||||
* keep it that way:
|
||||
* 1. DRIFT SCANNER (scripts/lint-phase-id-drift.cjs) — fails CI if any module
|
||||
* outside phase-id.cts re-derives the canonical phase-number token as a
|
||||
* literal without a `// phase-id-owner:` sanction.
|
||||
* 2. IDENTITY guard — phase-id.cjs exports the complete locked surface, and no
|
||||
* consumer re-exports a DIVERGENT copy of a canonical function (re-export,
|
||||
* never re-implement).
|
||||
*
|
||||
* Behavioral throughout: assertions drive `findPhaseIdRegexDrift` / `scanRepo`
|
||||
* and compare object identity — no `readFileSync().includes()` in a test body.
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const { findPhaseIdRegexDrift, scanRepo } = require(
|
||||
path.join(ROOT, 'scripts', 'lint-phase-id-drift.cjs'),
|
||||
);
|
||||
const phaseId = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'phase-id.cjs'));
|
||||
|
||||
// The locked canonical surface (ADR-2121 Decision 1/2; PHASE_NUMBER_TOKEN_SOURCE
|
||||
// added in Phase 4). Every name is exported by phase-id.cjs; the identity guard
|
||||
// forbids any other module from re-exporting a divergent copy of one.
|
||||
const CANONICAL = [
|
||||
'escapeRegex', 'OPTIONAL_PROJECT_CODE_PREFIX_SOURCE', 'OPTIONAL_PHASE_TAG_SOURCE',
|
||||
'PHASE_NUMBER_TOKEN_SOURCE', 'stripProjectCodePrefix', 'normalizePhaseName',
|
||||
'getMilestoneFromPhaseId', 'getPhaseDirFromPhaseId', 'phaseMarkdownRegexSource',
|
||||
'phaseMarkdownRegexSourceExact', 'comparePhaseNum', 'extractPhaseToken',
|
||||
'phaseTokenMatches', 'parsePhaseFromProse', 'stripConfiguredProjectCodePrefix',
|
||||
'isForeignPrefixedPhaseQuery', 'roadmapPhaseLookupSources',
|
||||
];
|
||||
|
||||
describe('#2128 phase-id drift scanner: findPhaseIdRegexDrift (pure)', () => {
|
||||
test('a regex built from PHASE_NUMBER_TOKEN_SOURCE is NOT drift', () => {
|
||||
assert.deepEqual(
|
||||
findPhaseIdRegexDrift('const re = new RegExp(`Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`);'),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test('a literal re-derivation of the canonical token IS flagged (fail-first)', () => {
|
||||
const v = findPhaseIdRegexDrift('const re = /Phase\\s+(\\d+[A-Z]?(?:\\.\\d+)*)/;');
|
||||
assert.equal(v.length, 1);
|
||||
assert.equal(v[0].found, '\\d+[A-Z]?(?:\\.\\d+)*');
|
||||
});
|
||||
|
||||
test('a re-derivation inside a new RegExp template (\\\\d escaping) IS flagged', () => {
|
||||
const v = findPhaseIdRegexDrift('new RegExp(`Phase\\\\s+(\\\\d+[A-Z]?(?:\\\\.\\\\d+)*)`)');
|
||||
assert.equal(v.length, 1);
|
||||
});
|
||||
|
||||
test('the [A-Za-z] and [.-] near-variants ARE flagged', () => {
|
||||
assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Za-z]?(?:\\.\\d+)*)/').length, 1);
|
||||
assert.equal(findPhaseIdRegexDrift('/(\\d+[A-Z]?(?:[.-]\\d+)*)/').length, 1);
|
||||
});
|
||||
|
||||
test('a same-line // phase-id-owner: sanction suppresses the flag', () => {
|
||||
assert.deepEqual(
|
||||
findPhaseIdRegexDrift('const re = /(\\d+[A-Z]?(?:\\.\\d+)*)/; // phase-id-owner: sanctioned exception'),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test('a preceding-line // phase-id-owner: sanction suppresses the flag', () => {
|
||||
assert.deepEqual(
|
||||
findPhaseIdRegexDrift('// phase-id-owner: sanctioned exception\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;'),
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test('non-token phase regexes are NOT flagged (no false positives)', () => {
|
||||
assert.deepEqual(findPhaseIdRegexDrift('/^Executing Phase\\s+\\d+/'), [], 'status-message bare \\d+');
|
||||
assert.deepEqual(findPhaseIdRegexDrift('/#{2,4}\\s*Phase\\s+(\\d+)[A-Z]?(?:\\.\\d+)*/'), [], 'digits-only capture is non-contiguous');
|
||||
assert.deepEqual(findPhaseIdRegexDrift('/Phase\\s+([\\w][\\w.-]*)/'), [], '\\w id grammar is not the canonical token');
|
||||
assert.deepEqual(findPhaseIdRegexDrift('/\\|\\s*Phase\\s*\\|\\s*Plans\\s*\\|/'), [], 'pipe-table structure');
|
||||
});
|
||||
|
||||
test('reports 1-based line numbers', () => {
|
||||
const v = findPhaseIdRegexDrift('line1\nconst re = /(\\d+[A-Z]?(?:\\.\\d+)*)/;\nline3');
|
||||
assert.equal(v[0].line, 2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#2128 phase-id drift scanner: the live repo is clean', () => {
|
||||
test('scanRepo finds zero unsanctioned phase-token re-derivations', () => {
|
||||
const violations = scanRepo(ROOT);
|
||||
assert.deepEqual(
|
||||
violations,
|
||||
[],
|
||||
'unsanctioned phase-token re-derivation(s) — build from PHASE_NUMBER_TOKEN_SOURCE or add // phase-id-owner:\n' +
|
||||
violations.map((d) => ` ${d.file}:${d.line} ${d.found}`).join('\n'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#2128 phase-id single-owner identity guard', () => {
|
||||
test('phase-id.cjs exports the complete locked canonical surface', () => {
|
||||
for (const name of CANONICAL) {
|
||||
assert.ok(name in phaseId, `phase-id.cjs must export the canonical member '${name}'`);
|
||||
}
|
||||
});
|
||||
|
||||
test('no consumer module re-exports a DIVERGENT copy of a canonical phase-id function', () => {
|
||||
// Forward guard: if any built lib module re-exports a name that phase-id.cjs
|
||||
// owns, it MUST be the identical reference — a re-export, never a local
|
||||
// re-implementation. All consumers pass today (none re-export); the guard
|
||||
// fails the moment a divergent copy ships.
|
||||
const libDir = path.join(ROOT, 'gsd-core', 'bin', 'lib');
|
||||
const consumers = fs.readdirSync(libDir).filter((f) => f.endsWith('.cjs') && f !== 'phase-id.cjs');
|
||||
let checked = 0;
|
||||
for (const f of consumers) {
|
||||
let mod;
|
||||
try {
|
||||
mod = require(path.join(libDir, f));
|
||||
} catch {
|
||||
continue; // a module that cannot be required in isolation can't re-export anything
|
||||
}
|
||||
if (!mod || typeof mod !== 'object') continue;
|
||||
checked++;
|
||||
for (const name of CANONICAL) {
|
||||
if (Object.prototype.hasOwnProperty.call(mod, name)) {
|
||||
assert.strictEqual(
|
||||
mod[name],
|
||||
phaseId[name],
|
||||
`${f} re-exports '${name}' but it is NOT the phase-id.cjs reference — re-export the canonical, do not re-implement`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
assert.ok(checked > 0, 'expected to inspect at least one consumer module');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user