* test(#1956): failing-first contract for cross-artifact fact-drift pass * feat(#1956): flag cross-artifact fact drift in the plan drift guard * fix(#1956): correct config-key assertion and bidirectional lifecycle-lag exemption * docs(#1956): document the cross-artifact axis in the architecture reference * feat(#1956): decide the phase-status drift axis deterministically * fix(#1956): scope the progress-table lookup, abstain without a position section, rank deferred * docs(#1956): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -25,6 +25,7 @@ const {
|
||||
AUTHORITY_RUNGS,
|
||||
getEffectiveAuthority,
|
||||
classifyDriftSeverity,
|
||||
comparePhaseStatus,
|
||||
} = require('../gsd-core/bin/lib/plan-drift-guard.cjs');
|
||||
|
||||
// ── 1. AUTHORITY_RUNGS sanity ────────────────────────────────────────────────
|
||||
@@ -326,3 +327,277 @@ describe('plan-review-convergence.md uses gsd_run drift-guard seam', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ── 6. comparePhaseStatus unit tests (#1956) ────────────────────────────────
|
||||
|
||||
describe('comparePhaseStatus', () => {
|
||||
test('equal ranks (STATE vocabulary vs ROADMAP vocabulary) → consistent', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'In progress', roadmapStatus: 'In Progress' });
|
||||
assert.equal(result.verdict, 'consistent');
|
||||
assert.equal(result.stateRank, result.roadmapRank);
|
||||
});
|
||||
|
||||
test('Phase complete vs In Progress → drifted, NOT lag (the issue\'s canonical case)', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Phase complete', roadmapStatus: 'In Progress' });
|
||||
assert.equal(result.verdict, 'drifted');
|
||||
assert.notEqual(result.verdict, 'lag', 'a completion disagreement must never be classified as lag, even though the ranks are only 1 apart');
|
||||
});
|
||||
|
||||
test('Phase complete vs Not started → drifted', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Phase complete', roadmapStatus: 'Not started' });
|
||||
assert.equal(result.verdict, 'drifted');
|
||||
});
|
||||
|
||||
test('Ready to plan vs In Progress → lag', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Ready to plan', roadmapStatus: 'In Progress' });
|
||||
assert.equal(result.verdict, 'lag');
|
||||
});
|
||||
|
||||
test('unknown status on either side → uncheckable, and the other side\'s rank still resolves', () => {
|
||||
const stateUnknown = comparePhaseStatus({ stateStatus: 'Frobnicating', roadmapStatus: 'In Progress' });
|
||||
assert.equal(stateUnknown.verdict, 'uncheckable');
|
||||
assert.equal(stateUnknown.stateRank, null);
|
||||
assert.equal(stateUnknown.roadmapRank, 1, 'the resolvable side must still be diagnosable even when the other is unknown');
|
||||
|
||||
const roadmapUnknown = comparePhaseStatus({ stateStatus: 'Phase complete', roadmapStatus: 'Frobnicating' });
|
||||
assert.equal(roadmapUnknown.verdict, 'uncheckable');
|
||||
assert.equal(roadmapUnknown.roadmapRank, null);
|
||||
assert.equal(roadmapUnknown.stateRank, 2, 'the resolvable side must still be diagnosable even when the other is unknown');
|
||||
});
|
||||
|
||||
test('null / undefined / empty-string on either side → uncheckable', () => {
|
||||
assert.equal(comparePhaseStatus({ stateStatus: null, roadmapStatus: 'In Progress' }).verdict, 'uncheckable');
|
||||
assert.equal(comparePhaseStatus({ stateStatus: undefined, roadmapStatus: 'In Progress' }).verdict, 'uncheckable');
|
||||
assert.equal(comparePhaseStatus({ stateStatus: '', roadmapStatus: 'In Progress' }).verdict, 'uncheckable');
|
||||
assert.equal(comparePhaseStatus({ stateStatus: 'In Progress', roadmapStatus: null }).verdict, 'uncheckable');
|
||||
assert.equal(comparePhaseStatus({ stateStatus: 'In Progress', roadmapStatus: undefined }).verdict, 'uncheckable');
|
||||
assert.equal(comparePhaseStatus({ stateStatus: 'In Progress', roadmapStatus: '' }).verdict, 'uncheckable');
|
||||
});
|
||||
|
||||
test('case and surrounding whitespace are ignored', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: ' PHASE COMPLETE ', roadmapStatus: 'Phase complete' });
|
||||
assert.equal(result.verdict, 'consistent');
|
||||
assert.equal(result.stateRank, 2);
|
||||
assert.equal(result.roadmapRank, 2);
|
||||
});
|
||||
|
||||
test('does not throw for unrecognized input (unlike classifyDriftSeverity)', () => {
|
||||
assert.doesNotThrow(() => comparePhaseStatus({ stateStatus: 'garbage', roadmapStatus: 'nonsense' }));
|
||||
assert.doesNotThrow(() => comparePhaseStatus({ stateStatus: undefined, roadmapStatus: undefined }));
|
||||
});
|
||||
|
||||
// #1956 review fix: 'Deferred' was missing from PHASE_STATUS_RANKS despite
|
||||
// gsd-core/templates/roadmap.md:133 declaring it as part of the full
|
||||
// ROADMAP Status vocabulary (`Not started | In progress | Complete |
|
||||
// Deferred`) — it silently always resolved 'uncheckable', losing real drift.
|
||||
test('Deferred vs Not started → consistent (both agree no work has happened)', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Not started', roadmapStatus: 'Deferred' });
|
||||
assert.equal(result.verdict, 'consistent');
|
||||
});
|
||||
|
||||
test('Deferred vs In progress → drifted (declared stopped vs declared happening)', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'In progress', roadmapStatus: 'Deferred' });
|
||||
assert.equal(result.verdict, 'drifted');
|
||||
});
|
||||
|
||||
test('Deferred vs Phase complete → drifted (declared stopped vs declared done)', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Phase complete', roadmapStatus: 'Deferred' });
|
||||
assert.equal(result.verdict, 'drifted');
|
||||
});
|
||||
|
||||
test('deferred resolves a real (non-null) rank on either side', () => {
|
||||
const result = comparePhaseStatus({ stateStatus: 'Not started', roadmapStatus: 'Deferred' });
|
||||
assert.notEqual(result.roadmapRank, null, "'deferred' must not be uncheckable — it is a declared vocabulary value");
|
||||
assert.equal(result.roadmapRank, 0);
|
||||
});
|
||||
});
|
||||
|
||||
// Shared ROADMAP.md "Progress" table fixture builder (#1956/#2012). Used by
|
||||
// BOTH the CLI acceptance tests below (via writeRoadmap, which writes it to
|
||||
// disk) and the findRoadmapProgressTable/deriveProgressFromRoadmap parity
|
||||
// test (section 8), so the CLI-level decoy fixture and the parity fixture
|
||||
// can never independently drift into slightly different shapes.
|
||||
//
|
||||
// `opts.decoy`, when true, prepends an earlier `## Archive Notes` section
|
||||
// carrying a table with the EXACT SAME four column headers
|
||||
// (`Phase | Plans Complete | Status | Completed`) as the real `## Progress`
|
||||
// table, with the same phase row reporting a DIFFERENT status ('Not
|
||||
// started') — the #2012 decoy shape a column-name-only lookup would pick up
|
||||
// first.
|
||||
function buildRoadmapProgressContent(phase, phaseName, roadmapStatus, opts = {}) {
|
||||
const decoySection = opts.decoy
|
||||
? `## Archive Notes
|
||||
|
||||
| Phase | Plans Complete | Status | Completed |
|
||||
|-------|----------------|--------|-----------|
|
||||
| ${phase}. ${phaseName} | 0/1 | Not started | - |
|
||||
|
||||
`
|
||||
: '';
|
||||
return `# Roadmap: Test Project
|
||||
|
||||
${decoySection}## Progress
|
||||
|
||||
| Phase | Plans Complete | Status | Completed |
|
||||
|-------|----------------|--------|-----------|
|
||||
| ${phase}. ${phaseName} | 0/1 | ${roadmapStatus} | - |
|
||||
`;
|
||||
}
|
||||
|
||||
// ── 7. #1956 acceptance — drifted phase status across STATE/ROADMAP, via the real CLI ──
|
||||
|
||||
describe('#1956 acceptance — a drifted phase status across STATE/ROADMAP yields a finding', () => {
|
||||
let tmpDir;
|
||||
let planningDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1956-acceptance-'));
|
||||
planningDir = path.join(tmpDir, '.planning');
|
||||
fs.mkdirSync(planningDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
const PHASE = 3;
|
||||
const PHASE_NAME = 'Convergence';
|
||||
|
||||
// Writes .planning/STATE.md carrying frontmatter + a `## Current Position`
|
||||
// section, matching gsd-core/templates/state.md.
|
||||
function writeState(stateStatus) {
|
||||
const content = `---
|
||||
gsd_state_version: '1.0'
|
||||
status: planning
|
||||
---
|
||||
|
||||
# Project State
|
||||
|
||||
## Current Position
|
||||
|
||||
Phase: ${PHASE} of 8 (${PHASE_NAME})
|
||||
Plan: 1 of 1 in current phase
|
||||
Status: ${stateStatus}
|
||||
Last activity: 2026-08-09 — test fixture
|
||||
|
||||
Progress: [░░░░░░░░░░] 0%
|
||||
`;
|
||||
fs.writeFileSync(path.join(planningDir, 'STATE.md'), content);
|
||||
}
|
||||
|
||||
// Writes .planning/ROADMAP.md carrying a `## Progress` section with the
|
||||
// table shape gsd-core/templates/roadmap.md declares. `opts.decoy` prepends
|
||||
// a same-headers decoy table under a different heading — see
|
||||
// buildRoadmapProgressContent above.
|
||||
function writeRoadmap(roadmapStatus, opts = {}) {
|
||||
fs.writeFileSync(
|
||||
path.join(planningDir, 'ROADMAP.md'),
|
||||
buildRoadmapProgressContent(PHASE, PHASE_NAME, roadmapStatus, opts),
|
||||
);
|
||||
}
|
||||
|
||||
test('an intentionally-drifted phase status yields a finding', () => {
|
||||
writeState('Phase complete');
|
||||
writeRoadmap('In Progress');
|
||||
const res = runGsdTools(['drift-guard', 'phase-status', '--phase', String(PHASE)], tmpDir);
|
||||
assert.ok(res.success, `Expected success, got: ${res.error}`);
|
||||
const result = JSON.parse(res.output);
|
||||
assert.equal(result.verdict, 'drifted');
|
||||
assert.equal(result.authority, 'STATE.md', 'the finding must name STATE.md as authority so the reviewer knows which side to keep');
|
||||
});
|
||||
|
||||
test('consistent artifacts do not', () => {
|
||||
writeState('Phase complete');
|
||||
writeRoadmap('Complete');
|
||||
const res = runGsdTools(['drift-guard', 'phase-status', '--phase', String(PHASE)], tmpDir);
|
||||
assert.ok(res.success, `Expected success, got: ${res.error}`);
|
||||
const result = JSON.parse(res.output);
|
||||
assert.equal(result.verdict, 'consistent');
|
||||
});
|
||||
|
||||
test('a missing ROADMAP.md is uncheckable, not consistent', () => {
|
||||
writeState('Phase complete');
|
||||
const res = runGsdTools(['drift-guard', 'phase-status', '--phase', String(PHASE)], tmpDir);
|
||||
assert.ok(res.success, `Expected success, got: ${res.error}`);
|
||||
const result = JSON.parse(res.output);
|
||||
assert.equal(result.verdict, 'uncheckable');
|
||||
assert.ok(result.reason && result.reason.length > 0, 'a skipped axis must be observable via a non-empty reason');
|
||||
});
|
||||
|
||||
// #1956 review Fix 1: the Progress-table lookup used to scan the WHOLE
|
||||
// ROADMAP for the first table matching the column headers, so an earlier
|
||||
// decoy table sharing those headers (e.g. an "Archive Notes" table) was
|
||||
// picked up instead of the real `## Progress` table (#2012 decoy
|
||||
// avoidance). The decoy's phase-3 row says 'Not started'; the real
|
||||
// `## Progress` table's phase-3 row says 'Complete', agreeing with STATE.md's
|
||||
// 'Phase complete' — the CLI must read the real table.
|
||||
test('a decoy table with the same headers is not mistaken for ## Progress', () => {
|
||||
writeState('Phase complete');
|
||||
writeRoadmap('Complete', { decoy: true });
|
||||
const res = runGsdTools(['drift-guard', 'phase-status', '--phase', String(PHASE)], tmpDir);
|
||||
assert.ok(res.success, `Expected success, got: ${res.error}`);
|
||||
const result = JSON.parse(res.output);
|
||||
assert.equal(result.verdict, 'consistent');
|
||||
assert.equal(result.roadmapStatus, 'Complete', 'must read the real ## Progress table\'s status, not the decoy\'s "Not started"');
|
||||
});
|
||||
|
||||
// #1956 review Fix 2: `stateCurrentPositionSlice(stateBody) ?? stateBody`
|
||||
// fell back to the whole STATE.md body when no `## Current Position`
|
||||
// heading was found, reintroducing the #2956 archive-shadowing bug — a
|
||||
// stray historical `Status:` line elsewhere in the body would be read as
|
||||
// if it were the real current status, fabricating a drift finding. A
|
||||
// STATE.md with NO Current Position heading, plus an earlier stray
|
||||
// `Status: Ready to plan` line, must abstain instead of guessing.
|
||||
test('a STATE.md with no ## Current Position heading is uncheckable, never a fabricated finding', () => {
|
||||
const stateContent = `---
|
||||
gsd_state_version: '1.0'
|
||||
status: planning
|
||||
---
|
||||
|
||||
# Project State
|
||||
|
||||
Status: Ready to plan
|
||||
|
||||
## Session
|
||||
|
||||
Some unrelated notes with no Current Position section.
|
||||
`;
|
||||
fs.writeFileSync(path.join(planningDir, 'STATE.md'), stateContent);
|
||||
writeRoadmap('Complete');
|
||||
const res = runGsdTools(['drift-guard', 'phase-status', '--phase', String(PHASE)], tmpDir);
|
||||
assert.ok(res.success, `Expected success, got: ${res.error}`);
|
||||
const result = JSON.parse(res.output);
|
||||
assert.equal(result.verdict, 'uncheckable');
|
||||
assert.equal(result.reason, 'no_current_position');
|
||||
assert.notEqual(result.verdict, 'drifted', 'a shadowed/absent Current Position must never fabricate a drift finding');
|
||||
});
|
||||
});
|
||||
|
||||
// ── 8. #1956/#2012 parity — findRoadmapProgressTable vs deriveProgressFromRoadmap ──
|
||||
//
|
||||
// Two SEPARATE implementations locate "the" ROADMAP Progress table:
|
||||
// roadmap-parser.cts's findRoadmapProgressTable (collectSection-based, used
|
||||
// by the drift-guard CLI) and phase-lifecycle.cts's deriveProgressFromRoadmap
|
||||
// (its own regex-based `## Progress` scope, used by the phase-lifecycle SDK
|
||||
// handler — deliberately NOT refactored onto the shared owner; its blast
|
||||
// radius is large). The repo requires a parity assertion whenever a parser is
|
||||
// expressed twice, so this test feeds both the SAME decoy-bearing ROADMAP
|
||||
// fixture from section 7 and asserts they agree about which table is real.
|
||||
|
||||
describe('#1956/#2012 parity — findRoadmapProgressTable vs deriveProgressFromRoadmap agree', () => {
|
||||
const { findRoadmapProgressTable } = require('../gsd-core/bin/lib/roadmap-parser.cjs');
|
||||
const { deriveProgressFromRoadmap } = require('../gsd-core/bin/lib/phase-lifecycle.cjs');
|
||||
|
||||
test('both locators pick the real ## Progress table, not the decoy', () => {
|
||||
const content = buildRoadmapProgressContent(3, 'Convergence', 'Complete', { decoy: true });
|
||||
|
||||
const table = findRoadmapProgressTable(content);
|
||||
assert.ok(table, 'findRoadmapProgressTable must find the real Progress table');
|
||||
const row = table.rows.find((r) => r.Phase.startsWith('3.'));
|
||||
assert.ok(row, 'expected a phase 3 row in the located table');
|
||||
assert.equal(row.Status, 'Complete', 'must read the real ## Progress table\'s phase-3 status, not the decoy\'s "Not started"');
|
||||
|
||||
const progress = deriveProgressFromRoadmap(content);
|
||||
assert.equal(progress.completedPhases, 1, 'deriveProgressFromRoadmap must count the real ## Progress table\'s Complete row, not be fooled by the decoy');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"plan-review-convergence.md": "#1956: the plan drift guard gains a second axis — a cross-artifact fact-drift pass sitting immediately after the existing source-grounding pass, under the SAME `plan_review.source_grounding` gate. Growth is the new section only (26195 -> 30672 bytes, +4477); no existing text was rewritten and no new config key was introduced. The addition is deliberately inline rather than extracted: ADR-1610 Decision 4 names eager `@`-import relocation as proxy-gaming (it shrinks the measured file while leaving loaded context unchanged or larger), legitimate extraction is Read-at-step lazy, and this pass has no lazy-read seam — it is prose the orchestrator must already hold when it runs the guard. The file remains far inside its DEFAULT tier hard cap of 40960 bytes (tests/workflow-size-budget.test.cjs), with ~11.4 KB of headroom. Content justification: source-grounding proves a plan's cited SYMBOLS exist in source; nothing proved that the same FACT stated in two planning artifacts still agrees. Because each phase runs in a fresh context an agent typically reads one artifact and trusts it, so a stale duplicate — a phase marked complete in STATE.md but in progress in ROADMAP.md, a success criterion the plan restates with a different outcome, a term used against its CONTEXT.md definition — silently steers it wrong. The pass keys on contradicting knowledge rather than similar-looking text (the false-positive mode the issue's own research comment names), gates on a three-way conjunction, defers the axes gsd-plan-checker already owns (Dimensions 1, 7b, 9) so nothing is reported twice, and is advisory only: it never sets hardBlock and contributes to neither HIGH_COUNT nor ACTIONABLE_COUNT, so a project carrying pre-existing drift can still converge."
|
||||
}
|
||||
}
|
||||
@@ -1502,3 +1502,412 @@ describe('bug-936 — plan-review-convergence runs plan-phase inline, not inside
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// #1956 — Cross-artifact fact-drift pass (second axis of the plan drift guard)
|
||||
//
|
||||
// The source-grounding pass answers "does this symbol exist in the source?".
|
||||
// This pass answers "does the project state the same FACT in two planning
|
||||
// artifacts, and do the two disagree?" — the DRY hazard the issue names, where
|
||||
// one copy is updated and the other silently steers a fresh-context agent wrong.
|
||||
//
|
||||
// ## What this suite locks
|
||||
//
|
||||
// The deployed contract, plus the two Hyrum contracts that are invisible from
|
||||
// the new section itself and would be silently broken by a plausible edit:
|
||||
// 1. the pass is ORCHESTRATOR-side, not inside the Agent(prompt=…) string —
|
||||
// the review agent's return message must end with its two "## " sections
|
||||
// and carry no others, because the workflow awk-parses them;
|
||||
// 2. the pass can never reach the convergence gate. Convergence is
|
||||
// HIGH_COUNT + ACTIONABLE_COUNT == 0, and the workflow's own ACTIONABLE
|
||||
// definition would otherwise swallow a fact-drift finding — which would
|
||||
// turn an advisory check into an infinite replan loop for any project that
|
||||
// already carries drift.
|
||||
//
|
||||
// ## What it cannot prove
|
||||
//
|
||||
// That the model acts on the text. The subject is an LLM prompt; no test in
|
||||
// this repo proves behavior for the source-grounding pass either. Stated so the
|
||||
// coverage claim is honest rather than implied.
|
||||
//
|
||||
// The file is read through readFileNormalized, the same LF-normalizing seam the
|
||||
// rest of this suite uses, so a CRLF checkout cannot skew the offsets.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('plan-review-convergence: cross-artifact fact-drift pass (#1956)', () => {
|
||||
const WORKFLOW = readFileNormalized(WORKFLOW_PATH);
|
||||
|
||||
const DRIFT_HEADING = /^### Cross-artifact fact-drift pass/m;
|
||||
const GROUNDING_HEADING = /^### Source-grounding pass/m;
|
||||
const AFTER_AGENT_LINE = /^After agent returns, verify REVIEWS\.md exists/m;
|
||||
|
||||
function offsetOf(content, pattern) {
|
||||
const m = content.match(pattern);
|
||||
return m && typeof m.index === 'number' ? m.index : -1;
|
||||
}
|
||||
|
||||
function driftSpan() {
|
||||
const start = offsetOf(WORKFLOW, DRIFT_HEADING);
|
||||
const end = offsetOf(WORKFLOW, AFTER_AGENT_LINE);
|
||||
assert.ok(start >= 0, 'workflow must define a "### Cross-artifact fact-drift pass" heading');
|
||||
assert.ok(end >= 0, 'workflow must retain the "After agent returns…" line that bounds the pass');
|
||||
assert.ok(end > start, 'the fact-drift pass must precede the "After agent returns…" line');
|
||||
return WORKFLOW.slice(start, end);
|
||||
}
|
||||
|
||||
/**
|
||||
* Top-level ordered-list items in a span — the trigger gate's arity, i.e. the
|
||||
* "flag only when ALL N hold" conjunction. Widening it from 3 to 2 is exactly
|
||||
* what turns a precise heuristic into a noise generator, so the COUNT is
|
||||
* asserted rather than the prose.
|
||||
*/
|
||||
function countOrderedItems(span) {
|
||||
const matches = span.match(/^\d+\. /gm);
|
||||
return matches ? matches.length : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Conjunction clauses — the indented sub-list under step 3's "FLAG only when
|
||||
* ALL THREE hold". Counted separately from the STEPS above because they are
|
||||
* different things: `countOrderedItems` measures the pass's procedure, this
|
||||
* measures the trigger gate's arity. Asserting one while claiming the other
|
||||
* is how a weakened gate slips through a green suite.
|
||||
*/
|
||||
function countConjunctionItems(span) {
|
||||
const matches = span.match(/^ {3}\d+\. /gm);
|
||||
return matches ? matches.length : 0;
|
||||
}
|
||||
|
||||
describe('the pass exists and extends the drift guard in place', () => {
|
||||
test('defines a cross-artifact fact-drift pass', () => {
|
||||
const heading = WORKFLOW.match(/^### Cross-artifact fact-drift pass(.*)$/m);
|
||||
assert.ok(heading, 'workflow must define a "### Cross-artifact fact-drift pass" heading');
|
||||
});
|
||||
|
||||
test('the pass extends the drift guard, in place', () => {
|
||||
const grounding = offsetOf(WORKFLOW, GROUNDING_HEADING);
|
||||
const drift = offsetOf(WORKFLOW, DRIFT_HEADING);
|
||||
const after = offsetOf(WORKFLOW, AFTER_AGENT_LINE);
|
||||
assert.ok(grounding >= 0 && drift >= 0 && after >= 0, 'all three anchors must be present');
|
||||
assert.ok(grounding < drift, 'the fact-drift pass must follow the source-grounding pass — it extends it');
|
||||
assert.ok(drift < after, 'the fact-drift pass must sit before the post-agent REVIEWS.md check');
|
||||
});
|
||||
|
||||
test('the pass region is outside the review-agent prompt', () => {
|
||||
// Anchor on the review-agent return contract itself — the sentence inside
|
||||
// the Agent(prompt=…) string that this test exists to protect — rather than
|
||||
// on a generic mode-argument literal that a later Agent() block could reuse
|
||||
// and thereby relocate the anchor past this section.
|
||||
const RETURN_CONTRACT = 'These two sections MUST be the final content of your response';
|
||||
const contractAt = WORKFLOW.indexOf(RETURN_CONTRACT);
|
||||
assert.ok(contractAt >= 0, 'the review agent prompt must still carry its return-message contract');
|
||||
assert.strictEqual(
|
||||
WORKFLOW.indexOf(RETURN_CONTRACT, contractAt + 1),
|
||||
-1,
|
||||
'the return-message contract must appear exactly once — a second copy makes this anchor ambiguous'
|
||||
);
|
||||
assert.ok(
|
||||
offsetOf(WORKFLOW, GROUNDING_HEADING) > contractAt,
|
||||
'source-grounding pass must remain orchestrator-side (after the Agent prompt)'
|
||||
);
|
||||
assert.ok(
|
||||
offsetOf(WORKFLOW, DRIFT_HEADING) > contractAt,
|
||||
'the fact-drift pass must be orchestrator-side — inside the Agent prompt its findings ' +
|
||||
'would break the "no additional ## headings" return contract the workflow parses'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('it adds no config surface', () => {
|
||||
test('the pass is gated on the existing drift-guard key', () => {
|
||||
assert.match(
|
||||
driftSpan(),
|
||||
/plan_review\.source_grounding/,
|
||||
'the fact-drift pass must name plan_review.source_grounding as its gate — issue #1956 ' +
|
||||
'scopes it as an extension of the existing guard, gated by the existing config'
|
||||
);
|
||||
});
|
||||
|
||||
test('the pass introduces no new config key', () => {
|
||||
// The issue's pre-submission checklist asserts the change adds no new
|
||||
// concept, and its breaking-change mitigation reads "gated behind the
|
||||
// EXISTING plan_review config". A third key would also force a 25th
|
||||
// setting into gsd-core/workflows/settings.md's six-section UX.
|
||||
//
|
||||
// Two halves, both required. The gate must still be NAMED — otherwise a
|
||||
// deleted or empty section would satisfy a bare "no novel keys" check
|
||||
// vacuously — and nothing beyond the two keys the drift guard already owns
|
||||
// may appear. (`source_grounding_authority` is resolved through
|
||||
// `gsd_run drift-guard authority` and is not spelled out in this file
|
||||
// today; it stays on the allowed list so naming it later is not a failure.)
|
||||
const keys = new Set([...WORKFLOW.matchAll(/plan_review\.([a-z_]+)/g)].map((m) => m[1]));
|
||||
assert.ok(
|
||||
keys.has('source_grounding'),
|
||||
'the workflow must still name plan_review.source_grounding as the drift-guard gate'
|
||||
);
|
||||
const novel = [...keys]
|
||||
.filter((k) => k !== 'source_grounding' && k !== 'source_grounding_authority')
|
||||
.sort();
|
||||
assert.deepStrictEqual(
|
||||
novel,
|
||||
[],
|
||||
`plan-review-convergence must introduce no new plan_review key, found: ${novel.join(', ')}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the trigger gate is a three-way conjunction', () => {
|
||||
test('the pass runs four procedure steps', () => {
|
||||
assert.strictEqual(
|
||||
countOrderedItems(driftSpan()),
|
||||
4,
|
||||
'the fact-drift pass must run four steps (deterministic phase-status, pair up judgment ' +
|
||||
'facts, judge them, record). This counts the PROCEDURE; the trigger gate arity is ' +
|
||||
'counted separately.'
|
||||
);
|
||||
});
|
||||
|
||||
test('the trigger gate enumerates exactly three conditions', () => {
|
||||
assert.strictEqual(
|
||||
countConjunctionItems(driftSpan()),
|
||||
3,
|
||||
'the fact-drift pass must gate on exactly three AND-ed conditions (same fact named on ' +
|
||||
'both sides, the two representations contradict, and the pair is one of the declared ' +
|
||||
'authority pairs). Dropping one widens it into a noise generator; adding one silently ' +
|
||||
'narrows what it can catch.'
|
||||
);
|
||||
});
|
||||
|
||||
test('condition counter fires at 2 / 3 / 4', () => {
|
||||
// The assertion above can only ever observe the real document's arity, so
|
||||
// its inequality branch never executes. Exercise the counter at
|
||||
// limit-1 / limit / limit+1 through the SAME function the guard uses, in
|
||||
// both LF and CRLF form, so a future edit cannot neuter it.
|
||||
const item = (n) => `${n}. condition ${n}`;
|
||||
const indentedItem = (n) => ` ${n}. condition ${n}`;
|
||||
for (const eol of ['\n', '\r\n']) {
|
||||
const spanOf = (count) =>
|
||||
['### Cross-artifact fact-drift pass', ...Array.from({ length: count }, (_, i) => item(i + 1))]
|
||||
.join(eol);
|
||||
const indentedSpanOf = (count) =>
|
||||
['### Cross-artifact fact-drift pass', ...Array.from({ length: count }, (_, i) => indentedItem(i + 1))]
|
||||
.join(eol);
|
||||
assert.strictEqual(countOrderedItems(spanOf(2)), 2, `2 items must count as 2 (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countOrderedItems(spanOf(3)), 3, `3 items must count as 3 (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countOrderedItems(spanOf(4)), 4, `4 items must count as 4 (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countConjunctionItems(indentedSpanOf(2)), 2, `2 indented items must count as 2 (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countConjunctionItems(indentedSpanOf(3)), 3, `3 indented items must count as 3 (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countConjunctionItems(indentedSpanOf(4)), 4, `4 indented items must count as 4 (eol=${JSON.stringify(eol)})`);
|
||||
// The two counters must not see each other's items — a column-0-only
|
||||
// span has no conjunction items, and an indented-only span has no
|
||||
// procedure items.
|
||||
assert.strictEqual(countOrderedItems(indentedSpanOf(3)), 0, `indented-only span must count 0 procedure items (eol=${JSON.stringify(eol)})`);
|
||||
assert.strictEqual(countConjunctionItems(spanOf(3)), 0, `column-0-only span must count 0 conjunction items (eol=${JSON.stringify(eol)})`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('severity is advisory, and can never gate convergence', () => {
|
||||
test('the finding is advisory and never a blocker', () => {
|
||||
assert.match(
|
||||
driftSpan(),
|
||||
/never\s+a\s+blocker/i,
|
||||
'the fact-drift pass must state that its finding is never a blocker — issue #1956 asks ' +
|
||||
'for an advisory finding, and blocking would strand every project carrying prior drift'
|
||||
);
|
||||
});
|
||||
|
||||
test('the pass can never gate convergence', () => {
|
||||
// Convergence is HIGH_COUNT + ACTIONABLE_COUNT == 0, and the workflow's
|
||||
// own ACTIONABLE definition ("a non-HIGH finding invisible to
|
||||
// execute-phase unless incorporated into PLAN.md") would otherwise
|
||||
// swallow a fact-drift finding — making pre-existing drift an infinite
|
||||
// replan loop. This has to be WRITTEN DOWN, not merely true today.
|
||||
const span = driftSpan();
|
||||
assert.match(span, /HIGH_COUNT/, 'the pass must name HIGH_COUNT when disclaiming the convergence gate');
|
||||
assert.match(span, /ACTIONABLE_COUNT/, 'the pass must name ACTIONABLE_COUNT when disclaiming the convergence gate');
|
||||
assert.match(
|
||||
span,
|
||||
/never sets\s+`?hardBlock/,
|
||||
'the pass must state that it never sets hardBlock — the source-grounding pass uses ' +
|
||||
'hardBlock to stop the review cycle, and this pass must not inherit that'
|
||||
);
|
||||
});
|
||||
|
||||
test('findings land in REVIEWS.md', () => {
|
||||
assert.match(
|
||||
driftSpan(),
|
||||
/REVIEWS\.md/,
|
||||
'the fact-drift pass must write to REVIEWS.md, matching the source-grounding pass — ' +
|
||||
'not to the review agent\'s return message'
|
||||
);
|
||||
});
|
||||
|
||||
test('the phase-status axis is delegated to the deterministic seam', () => {
|
||||
const span = driftSpan();
|
||||
assert.match(
|
||||
span,
|
||||
/gsd_run drift-guard phase-status/,
|
||||
'the phase-status axis must be decided by the drift-guard seam, not by model judgment — ' +
|
||||
'issue #1956 requires a drifted STATE/ROADMAP pair to yield a finding deterministically'
|
||||
);
|
||||
for (const verdict of [/\bdrifted\b/, /\blag\b/, /\buncheckable\b/]) {
|
||||
assert.match(span, verdict, `the pass must say how it treats the ${verdict} verdict`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('negative space is enumerated', () => {
|
||||
test('the pass keys on knowledge, not similar text', () => {
|
||||
// The maintainer's own research comment on #1956: "The check must key on
|
||||
// 'same knowledge, drifting representations,' not 'similar-looking text,'
|
||||
// or it will produce false positives."
|
||||
const span = driftSpan();
|
||||
assert.match(span, /knowledge/i, 'the pass must frame the check in terms of knowledge');
|
||||
assert.match(
|
||||
span,
|
||||
/contradict/i,
|
||||
'the pass must require a CONTRADICTION, not a resemblance — this is the rule that ' +
|
||||
'keeps it from firing on every restatement'
|
||||
);
|
||||
});
|
||||
|
||||
test('the pass enumerates its non-triggering cases', () => {
|
||||
assert.match(driftSpan(), /Do NOT flag/, 'the fact-drift pass must carry an explicit non-triggering list');
|
||||
});
|
||||
|
||||
test('non-triggering list covers every exclusion class', () => {
|
||||
const span = driftSpan();
|
||||
// Each token is a distinct exclusion class from the design's negative
|
||||
// space. Their absence is what produces the false positives the issue's
|
||||
// research comment warns about.
|
||||
for (const [token, why] of [
|
||||
[/wording/i, 'a wording-only difference asserting the same thing is not drift'],
|
||||
[/single-source/i, 'a fact held in one artifact only is the TARGET state, not a finding'],
|
||||
[/\bADDS\b/, 'a PLAN may ADD truths beyond the roadmap SCs — only subtraction/contradiction counts'],
|
||||
[/lifecycle/i, 'STATE trailing ROADMAP by one lifecycle step is lag, not drift'],
|
||||
[/Deferred Ideas/, 'CONTEXT.md non-authoritative sections must not be compared'],
|
||||
]) {
|
||||
assert.match(span, token, `the non-triggering list must cover: ${why}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('the pass defers overlapping axes to the plan checker', () => {
|
||||
// Report once, not twice. plan-checker already owns requirement coverage
|
||||
// (D1), scope reduction (D7b) and cross-plan data contracts (D9).
|
||||
const span = driftSpan();
|
||||
for (const dimension of [/Dimension 1\b/, /Dimension 7b\b/, /Dimension 9\b/]) {
|
||||
assert.match(span, dimension, `the pass must defer the overlapping axis to ${dimension}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('a completion disagreement is never exempted as lag', () => {
|
||||
// The issue's canonical example is "complete in STATE.md but in progress
|
||||
// in ROADMAP.md" — one lifecycle step apart, and exactly the case it wants
|
||||
// FLAGGED. An exemption phrased purely as "one step apart" would exempt it.
|
||||
const span = driftSpan();
|
||||
assert.match(
|
||||
span,
|
||||
/completion[^.]*never lag|never lag|Completeness is terminal/i,
|
||||
'the pass must state that a disagreement about completion is never lag'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('authority and coverage', () => {
|
||||
test('the pass names an authority for every artifact pair', () => {
|
||||
const span = driftSpan();
|
||||
for (const artifact of ['ROADMAP.md', 'PLAN.md', 'STATE.md', 'CONTEXT.md']) {
|
||||
assert.ok(
|
||||
span.includes(artifact),
|
||||
`the fact-drift pass must name ${artifact} — issue #1956 spans all four planning artifacts`
|
||||
);
|
||||
}
|
||||
assert.match(
|
||||
span,
|
||||
/Authority/i,
|
||||
'the pass must declare which side of each pair is the source of truth — a finding that ' +
|
||||
'names a divergence without naming the authority cannot be acted on'
|
||||
);
|
||||
});
|
||||
|
||||
test('a skipped axis is recorded, never silent', () => {
|
||||
const span = driftSpan();
|
||||
assert.match(span, /skip(ped)?/i, 'the pass must describe what happens when an artifact is absent');
|
||||
assert.match(
|
||||
span,
|
||||
/coverage/i,
|
||||
'a skipped axis must be recorded in the Verification coverage block — a clean pass must ' +
|
||||
'never silently mean "nothing was compared"'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('independence — the surrounding contracts are unchanged', () => {
|
||||
test('the source-grounding pass keeps its hard block', () => {
|
||||
const start = offsetOf(WORKFLOW, GROUNDING_HEADING);
|
||||
const end = offsetOf(WORKFLOW, DRIFT_HEADING);
|
||||
assert.ok(start >= 0 && end > start, 'source-grounding pass must still precede the fact-drift pass');
|
||||
const groundingSpan = WORKFLOW.slice(start, end);
|
||||
assert.match(
|
||||
groundingSpan,
|
||||
/hardBlock: true/,
|
||||
'the source-grounding pass must keep its hardBlock gating — #1956 is additive and must ' +
|
||||
'not downgrade the existing guard'
|
||||
);
|
||||
});
|
||||
|
||||
test('the review-agent return contract is unchanged', () => {
|
||||
assert.match(
|
||||
WORKFLOW,
|
||||
/no additional "## " headings after them/,
|
||||
'the review agent\'s return-message contract must survive — the workflow awk-parses ' +
|
||||
'those sections and a stray "## " heading breaks escalation-detail extraction'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('docs parity', () => {
|
||||
test('CONFIGURATION.md documents both drift-guard axes', () => {
|
||||
const configDoc = readFileNormalized(CONFIG_DOC_PATH);
|
||||
const row = configDoc
|
||||
.split('\n')
|
||||
.find((l) => l.startsWith('|') && l.includes('`plan_review.source_grounding`'));
|
||||
assert.ok(row, 'docs/CONFIGURATION.md must carry a table row for plan_review.source_grounding');
|
||||
assert.match(
|
||||
row,
|
||||
/cross-artifact|fact drift/i,
|
||||
'the plan_review.source_grounding row must document the second (fact-drift) axis — the ' +
|
||||
'key now gates two checks, and a reader disabling it must know what else goes dark'
|
||||
);
|
||||
});
|
||||
|
||||
test('USER-GUIDE.md documents the second axis', () => {
|
||||
const guide = readFileNormalized(path.join(__dirname, '..', 'docs', 'USER-GUIDE.md'));
|
||||
const start = guide.indexOf('plan_review.source_grounding: true');
|
||||
assert.ok(start >= 0, 'docs/USER-GUIDE.md must retain its Drift Guard section');
|
||||
const section = guide.slice(start, start + 4000);
|
||||
assert.match(
|
||||
section,
|
||||
/cross-artifact|fact drift/i,
|
||||
'the USER-GUIDE Drift Guard section must describe the cross-artifact fact-drift axis'
|
||||
);
|
||||
});
|
||||
|
||||
test('ARCHITECTURE.md documents the second axis', () => {
|
||||
// The issue's Scope of changes names ARCHITECTURE.md explicitly, and its
|
||||
// existing drift-guard paragraph is the one place the architecture doc
|
||||
// describes this guard at all — leaving it single-axis would state, in the
|
||||
// architecture reference, that the guard does less than it does.
|
||||
const arch = readFileNormalized(path.join(__dirname, '..', 'docs', 'ARCHITECTURE.md'));
|
||||
const start = arch.indexOf('The plan drift guard (`plan_review.source_grounding`)');
|
||||
assert.ok(start >= 0, 'docs/ARCHITECTURE.md must retain its plan drift guard paragraph');
|
||||
const section = arch.slice(start, start + 2000);
|
||||
assert.match(
|
||||
section,
|
||||
/cross-artifact|fact-drift/i,
|
||||
'the ARCHITECTURE.md drift-guard paragraph must describe the cross-artifact fact-drift axis'
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user