* test(#3348): add failing-first coverage for the context-drift gate * feat(#3348): add context-drift pre-check gate for plan-phase Compares each phase's *-RESEARCH.md/*-PATTERNS.md/*-VALIDATION.md/*-SPEC.md effective last-changed time (git commit time, falling back to mtime for uncommitted edits) against *-CONTEXT.md's, so plan-phase no longer silently reuses an upstream artifact that predates a decision added to CONTEXT.md after that artifact was derived from it. Deterministic, no model call. New `gsd_run verify context-drift <phase>` command, sibling to the existing verify.codebase-drift/verify.schema-drift gates in the drift capability. Warn-only by default (workflow.context_drift_precheck), with an opt-in workflow.context_drift_action: block escape hatch. Wired at plan:pre in plan-phase.md, before both the RESEARCH.md and PATTERNS.md reuse decisions. * fix(#3348): address code-review findings — raw-text-match, stale comment, import placement, duplicated phase resolution * fix(#3859): pin the real commit's diff.ignoreSubmodules to match the empty-diff probe The #3859 empty-diff guard decides whether a submodule bump would land using `--ignore-submodules=dirty`, overriding the caller's `diff.ignoreSubmodules` config. The real `git commit -- <paths>` that follows was never given the same override, so under a bare `diff.ignoreSubmodules=all` repo config the two calculations disagree: driven on git 2.39.5 (Debian bookworm, the linux-node24 test-matrix image), the guard correctly stands aside but the scoped commit itself then silently fails (exit 1, no error text) for a gitlink bump it had just confirmed would be recorded, surfacing as commit_failed instead of committed:true. Pin `-c diff.ignoreSubmodules=dirty` onto the scoped commit call too, so the probe and the commit it protects can never diverge. Harmless when no submodule path is involved (driven: identical outcome on an ordinary scoped file, with and without the flag). * fix(#3348): guard resolvePhaseDirByToken's exact-match fallback against path traversal * fix(#3348): retarget phase-enumeration-drift exemption to the consolidated resolvePhaseDirByToken helper cmdVerifySchemaDrift's inline readdirSync was already function-scoped-exempt in lint-phase-enumeration-drift.cjs as a single-phase LOOKUP (not a current-milestone enumeration). This PR's refactor pass lifted that block into a shared helper, resolvePhaseDirByToken, also used by the new cmdVerifyContextDrift — the guard tracks exemptions by enclosing function name, so the readdirSync now lives in an unexempted function and started firing. Move the exemption to resolvePhaseDirByToken (same written reason, now covering both callers) instead of migrating to listAllPhaseDirs, which would introduce two real behavior deltas here: it catches readdirSync failures internally (old code let them throw) and sorts results by phase number before matchPhaseDirs picks matches[0] (old code used raw, OS-dependent readdirSync order). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3348): satisfy lint:ci — slash form, capability registry regen - docs/features/context-drift-gate.md used the deprecated /gsd: colon form; docs are never passed through the install-time slash-form converters, so lint-docs-command-form requires the hyphen form. Regenerated docs/FEATURES.md from the corrected fragment. - Regenerated gsd-core/bin/lib/capability-registry.cjs after editing capabilities/drift/capability.json (lint:generated-sync). * fix(#3859): pin the real commit's diff.ignoreSubmodules via env, not argv -c The prior fix pinned `-c diff.ignoreSubmodules=dirty` onto the scoped commit's argv via `commitArgs.unshift(...)`. `-c key=val` must precede the `commit` subcommand, so this shifted `commitArgs[0]` from `'commit'` to `'-c'` for every scoped commit call, breaking 17 position-based assertions in the commit-files pathspec regression suite that read `a[0] === 'commit'` to find the commit invocation among recorded git calls. `execGit` already accepts an `env` option merged onto `process.env` before spawning. Git honors `GIT_CONFIG_COUNT`/`GIT_CONFIG_KEY_0`/`GIT_CONFIG_VALUE_0` as a per-invocation config override functionally identical to `-c key=val`, expressed via env instead of argv. Passing that env alongside the existing commitArgs (still `['commit', ..., '--', ...stagedPaths]`, argv unchanged) fixes the real commit's effective diff.ignoreSubmodules to match the empty-diff guard's probe without moving anything in argv position 0. Scoped to exactly the canScope branch, matching the probe's own preconditions and leaving no behavior change for commits the probe never evaluated. No test file changes needed — the 17 previously-failing assertions test argv[0] against the array passed into execGit, which never changes. * fix(#3348): register verify-context-drift in the check subcommand router The drift capability's new plan:pre gate declares check.query "verify.context-drift", which normalizes to `check verify-context-drift`, but no such subcommand was routed — phase6-capstone-conformance's uniform-block-field test failed with "Unknown check subcommand" for every declared gate query. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3348): extend #1592's exact-key-list snapshot for the new context-drift config keys tests/capability-registry.test.cjs asserted an exact, hardcoded snapshot of the drift capability's config keys. #3348 legitimately adds two new keys (workflow.context_drift_precheck, workflow.context_drift_action) for its own plan:pre context-drift gate — extend the expected set (and clarify the assertion message) without weakening the test's exactness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3348): reconcile E2's exemption-migration pin with the resolvePhaseDirByToken extraction #3348 (an earlier commit on this branch, e4b80ad81) extracted cmdVerifySchemaDrift's inline phasesDir readdirSync/matchPhaseDirs block into the shared resolvePhaseDirByToken helper (also used by the new cmdVerifyContextDrift), and retargeted lint-phase-enumeration-drift.cjs's function-scoped exemption from cmdVerifySchemaDrift to resolvePhaseDirByToken accordingly — cmdVerifySchemaDrift no longer contains a line the guard's detectors match, so it needs no exemption. tests/phase-locator.test.cjs's E2 test still pinned the exemption to the old name (cmdVerifySchemaDrift), unaware of the migration. Update E2 to match the same "migrated call site's exemption must move, not duplicate" pattern the test already applies to cmdRoadmapAnalyze and cmdInitMilestoneOp just below it: drop cmdVerifySchemaDrift from the still-exempt list and add symmetric assertions that it no longer carries the exemption while resolvePhaseDirByToken now does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3348): fix two self-contradicting/nondeterministic tests in context-drift.test.cjs 'always exits 0 (query command contract)' included the no-phase-arg case, which contradicts the file's own earlier 'errors with usage message on missing phase arg' test (that case legitimately exits 1 via the Usage error) — drop it from the always-exits-0 cases. 'degrades to mtime comparison outside a git repo' and '...in a repo with no commits' relied on real wall-clock ordering between two back-to-back writeFileSync calls to prove CONTEXT.md is newer than RESEARCH.md; on a fast filesystem both can land in the same mtime tick, producing a tie that computeContextDrift's strict `<` correctly treats as not-stale, so stale_artifacts comes back empty. Make both tests deterministic via explicit fs.utimesSync instead of relying on timing (CONTRIBUTING.md: never assert elapsed wall-clock time). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3348): add context_drift_precheck:false to the plan:pre all-off fixture The "all plan:pre when-keys false" fixture explicitly disables every known workflow.* plan:pre toggle, but didn't yet know about the new workflow.context_drift_precheck key (defaults to true), so the new drift context-drift gate stayed active and broke the empty-activeHooks assertion. Emitted-Drift-Ack-Growth: plan-phase.md — adds the #3348 context-drift plan:pre pre-check section (new ## 4.6); this PR's own diff, not incidental drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(#3348): backfill changeset PR number (pr:0 -> 4147) --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -29,7 +29,7 @@ import { checkUiPresence } from './ui-safety-gate.cjs';
|
||||
import { hasStaticFrontendEvidence } from './ui-frontend-evidence.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import verifyModule = require('./verify.cjs');
|
||||
const { cmdVerifySchemaDrift, cmdVerifyCodebaseDrift } = verifyModule;
|
||||
const { cmdVerifySchemaDrift, cmdVerifyCodebaseDrift, cmdVerifyContextDrift } = verifyModule;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import roadmapModule = require('./roadmap.cjs');
|
||||
const { getRoadmapPhaseWithFallback } = roadmapModule;
|
||||
@@ -1720,6 +1720,13 @@ function routeCheckCommand({ args, cwd, raw }: RouteCheckCommandOptions): void {
|
||||
cmdVerifyCodebaseDrift(cwd, raw);
|
||||
return;
|
||||
}
|
||||
if (subcommand === 'verify-context-drift') {
|
||||
// Delegates to verify.context-drift — drift capability gate at plan:pre (non-blocking).
|
||||
// Dot-to-hyphen normalization means query "verify.context-drift" routes here.
|
||||
const phaseArg = typeof args[2] === 'string' ? args[2] : '';
|
||||
cmdVerifyContextDrift(cwd, phaseArg, raw);
|
||||
return;
|
||||
}
|
||||
if (subcommand === 'predicate') {
|
||||
// Generic gate-predicate evaluator (#2008). The workflow gate-dispatch calls
|
||||
// this for any gate whose `check` carries a `predicate` (instead of a `query`),
|
||||
@@ -1737,7 +1744,7 @@ function routeCheckCommand({ args, cwd, raw }: RouteCheckCommandOptions): void {
|
||||
routeProhibitionEnforcement(args, raw);
|
||||
return;
|
||||
}
|
||||
error('Unknown check subcommand. Available: api-coverage-verify-pre, auto-mode, decision-coverage-plan, decision-coverage-verify, gap-analysis-plan-post, predicate, prohibition-enforcement, tdd-review-checkpoint, ui-plan-gate, ui-safety-gate, verify-command-paths, verify-failure-directions, verify-schema-drift, verify-codebase-drift', ERROR_REASON.SDK_UNKNOWN_COMMAND);
|
||||
error('Unknown check subcommand. Available: api-coverage-verify-pre, auto-mode, decision-coverage-plan, decision-coverage-verify, gap-analysis-plan-post, predicate, prohibition-enforcement, tdd-review-checkpoint, ui-plan-gate, ui-safety-gate, verify-command-paths, verify-failure-directions, verify-schema-drift, verify-codebase-drift, verify-context-drift', ERROR_REASON.SDK_UNKNOWN_COMMAND);
|
||||
}
|
||||
|
||||
export = {
|
||||
|
||||
@@ -260,6 +260,14 @@ export const VERIFY_COMMAND_ALIASES: CommandAlias[] = [
|
||||
],
|
||||
"subcommand": "codebase-drift",
|
||||
"mutation": false
|
||||
},
|
||||
{
|
||||
"canonical": "verify.context-drift",
|
||||
"aliases": [
|
||||
"verify context-drift"
|
||||
],
|
||||
"subcommand": "context-drift",
|
||||
"mutation": false
|
||||
}
|
||||
];
|
||||
|
||||
|
||||
@@ -2084,7 +2084,7 @@ function cmdCommit(cwd: string, message: string | undefined, files: string[] | u
|
||||
// #3886: `git commit` runs pre-commit hooks (husky/lint-staged routinely
|
||||
// idles ~4s on Windows before any task) — 10s is too tight, and a timeout
|
||||
// kill is NOT an ordinary failure. Same band as the push call below.
|
||||
const commitResult = execGit(commitArgs, { cwd, timeout: COMMIT_TIMEOUT_MS, env: commitEnv });
|
||||
const commitResult = execGit(commitArgs, { cwd, env: commitEnv, timeout: COMMIT_TIMEOUT_MS });
|
||||
if (commitResult.exitCode !== 0) {
|
||||
// #3886: a SIGTERM'd git commit is a timeout, not commit_failed — the
|
||||
// partial stderr it flushed (often incidental CRLF warnings) is noise,
|
||||
|
||||
@@ -23,6 +23,7 @@ interface VerifyModule {
|
||||
cmdVerifyKeyLinks(cwd: string, phase: string | undefined, raw: boolean): void;
|
||||
cmdVerifySchemaDrift(cwd: string, phase: string | undefined, skip: boolean, raw: boolean): void;
|
||||
cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void;
|
||||
cmdVerifyContextDrift(cwd: string, phase: string | undefined, raw: boolean): void;
|
||||
}
|
||||
|
||||
interface RouteVerifyCommandOptions {
|
||||
@@ -59,6 +60,7 @@ function routeVerifyCommand({ verify, args, cwd, raw, error }: RouteVerifyComman
|
||||
// per ADR/PRD 3524 §3 / L160 (CJS-only by design). Routing through
|
||||
// recursive dispatch would re-enter this router path.
|
||||
'codebase-drift': () => verify.cmdVerifyCodebaseDrift(cwd, raw),
|
||||
'context-drift': () => verify.cmdVerifyContextDrift(cwd, args[2], raw),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
161
src/verify.cts
161
src/verify.cts
@@ -20,6 +20,8 @@ import stateMod = require('./state.cjs');
|
||||
import modelProfilesMod = require('./model-profiles.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module
|
||||
import planScanMod = require('./plan-scan.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- verification.cjs is an export= CommonJS module
|
||||
import verificationMod = require('./verification.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- core-utils.cjs is an export= CommonJS module
|
||||
import coreUtilsMod = require('./core-utils.cjs');
|
||||
const { findOrphanSummaries, findUnsummarizedPlans } = coreUtilsMod;
|
||||
@@ -56,6 +58,7 @@ import planningSnapshotMod = require('./planning-snapshot.cjs');
|
||||
const { buildPlanningSnapshot } = planningSnapshotMod;
|
||||
|
||||
const { planningDir } = planningWorkspace;
|
||||
const { defaultPhaseCleanCommitTimesMs } = verificationMod;
|
||||
const { extractFrontmatter, parseMustHavesBlock } = frontmatterMod;
|
||||
const { readStateHeadFreshness } = stateMod;
|
||||
|
||||
@@ -1744,6 +1747,150 @@ function cmdValidateAgents(cwd: string, raw: boolean): void {
|
||||
);
|
||||
}
|
||||
|
||||
// ─── Context drift (#3348) ───────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Resolve a phase directory under `phasesDir` from a user-supplied `phaseArg`,
|
||||
* via the canonical phase-directory matcher (phase-id.cjs::matchPhaseDirs) rather
|
||||
* than a naive substring test — a bare `.includes(phaseArg)` lets a non-existent
|
||||
* phase silently match a different phase whose directory name merely contains the
|
||||
* requested token (e.g. "1" matching "11-expansion"). Falls back to an exact
|
||||
* directory-name match. Returns null if neither resolves. (#1571, #2528)
|
||||
*/
|
||||
function resolvePhaseDirByToken(phasesDir: string, phaseArg: string): string | null {
|
||||
const normalizedPhase = normalizePhaseName(phaseArg);
|
||||
const dirEntries = fs.readdirSync(phasesDir, { withFileTypes: true });
|
||||
const dirNames = dirEntries.filter((e) => e.isDirectory()).map((e) => e.name);
|
||||
const matched = matchPhaseDirs(dirNames, normalizedPhase).matches[0];
|
||||
if (matched) return path.join(phasesDir, matched);
|
||||
const check = validatePath(phaseArg, phasesDir);
|
||||
if (check.safe && fs.existsSync(check.resolved)) return check.resolved;
|
||||
return null;
|
||||
}
|
||||
|
||||
interface ContextDriftEntry {
|
||||
file: string;
|
||||
effectiveMs: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure comparator: which of `entries` have an effective last-changed time
|
||||
* STRICTLY BEFORE `contextEffectiveMs` (CONTEXT.md's own effective time)? Strict
|
||||
* `<` is "stale" (matches findStaleVerificationSummary's own strict `>` convention
|
||||
* for "newer than" elsewhere in this codebase — an artifact committed in the SAME
|
||||
* commit/second as CONTEXT.md is in sync, not stale).
|
||||
*/
|
||||
function computeContextDrift(contextEffectiveMs: number, entries: ContextDriftEntry[]): string[] {
|
||||
return entries.filter((e) => e.effectiveMs < contextEffectiveMs).map((e) => e.file);
|
||||
}
|
||||
|
||||
function buildContextDriftMessage(staleArtifacts: string[], phaseArg: string): string {
|
||||
const parts = [`CONTEXT.md decisions are newer than: ${staleArtifacts.join(', ')}.`];
|
||||
if (staleArtifacts.some((f) => f.endsWith('-RESEARCH.md'))) {
|
||||
parts.push(`Regenerate research: /gsd:plan-phase ${phaseArg} --research.`);
|
||||
}
|
||||
if (staleArtifacts.some((f) => f.endsWith('-PATTERNS.md'))) {
|
||||
parts.push('Regenerate patterns: delete the PATTERNS.md file, then re-run /gsd:plan-phase.');
|
||||
}
|
||||
if (
|
||||
staleArtifacts.some(
|
||||
(f) => f.endsWith('-VALIDATION.md') || (f.endsWith('-SPEC.md') && !f.endsWith('-AI-SPEC.md') && !f.endsWith('-UI-SPEC.md')),
|
||||
)
|
||||
) {
|
||||
parts.push('Regenerate or manually reconcile VALIDATION.md / SPEC.md against the current decisions.');
|
||||
}
|
||||
parts.push('Do not hand-inject the newer decisions into a prompt as a substitute for regenerating — that carries the staleness forward.');
|
||||
return parts.join(' ');
|
||||
}
|
||||
|
||||
function cmdVerifyContextDrift(cwd: string, phaseArg: string | undefined, raw: boolean): void {
|
||||
if (!phaseArg) {
|
||||
error('Usage: verify context-drift <phase>');
|
||||
return;
|
||||
}
|
||||
|
||||
const pDir = planningDir(cwd);
|
||||
const phasesDir = path.join(pDir, 'phases');
|
||||
const emitSkip = (reason: string, message = ''): void => {
|
||||
output({ block: false, skipped: true, reason, stale_artifacts: [], message }, raw);
|
||||
};
|
||||
|
||||
if (!fs.existsSync(phasesDir)) {
|
||||
emitSkip('phase-not-found', `Phase directory not found: ${phaseArg}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Same phase-directory resolution rule cmdVerifySchemaDrift uses (#1571, #2528):
|
||||
// matchPhaseDirs, never a naive substring test.
|
||||
const phaseDir = resolvePhaseDirByToken(phasesDir, phaseArg);
|
||||
if (!phaseDir) {
|
||||
emitSkip('phase-not-found', `Phase directory not found: ${phaseArg}`);
|
||||
return;
|
||||
}
|
||||
|
||||
let phaseFiles: string[];
|
||||
try {
|
||||
phaseFiles = fs.readdirSync(phaseDir).slice().sort();
|
||||
} catch {
|
||||
emitSkip('phase-not-found', `Phase directory not found: ${phaseArg}`);
|
||||
return;
|
||||
}
|
||||
|
||||
const contextFile = phaseFiles.find((f) => f.endsWith('-CONTEXT.md'));
|
||||
if (!contextFile) {
|
||||
emitSkip('no-context-md');
|
||||
return;
|
||||
}
|
||||
|
||||
const researchFile = phaseFiles.find((f) => f.endsWith('-RESEARCH.md'));
|
||||
const patternsFile = phaseFiles.find((f) => f.endsWith('-PATTERNS.md'));
|
||||
const validationFile = phaseFiles.find((f) => f.endsWith('-VALIDATION.md'));
|
||||
const specFile = phaseFiles.find(
|
||||
(f) => f.endsWith('-SPEC.md') && !f.endsWith('-AI-SPEC.md') && !f.endsWith('-UI-SPEC.md'),
|
||||
);
|
||||
const upstreamFiles = [researchFile, patternsFile, validationFile, specFile].filter(
|
||||
(f): f is string => !!f,
|
||||
);
|
||||
|
||||
if (upstreamFiles.length === 0) {
|
||||
emitSkip('no-upstream-artifacts');
|
||||
return;
|
||||
}
|
||||
|
||||
const allFiles = [contextFile, ...upstreamFiles];
|
||||
const cleanCommitMs = defaultPhaseCleanCommitTimesMs(phaseDir, allFiles);
|
||||
const effectiveTimeMs = (file: string): number =>
|
||||
cleanCommitMs.has(file)
|
||||
? (cleanCommitMs.get(file) as number)
|
||||
: fs.statSync(path.join(phaseDir, file)).mtimeMs;
|
||||
|
||||
const contextMs = effectiveTimeMs(contextFile);
|
||||
const driftEntries: ContextDriftEntry[] = upstreamFiles.map((f) => ({ file: f, effectiveMs: effectiveTimeMs(f) }));
|
||||
const staleArtifacts = computeContextDrift(contextMs, driftEntries);
|
||||
|
||||
let wf: Record<string, unknown> | undefined;
|
||||
try {
|
||||
const rawCfg = JSON.parse(fs.readFileSync(path.join(pDir, 'config.json'), 'utf-8')) as Record<string, unknown>;
|
||||
wf = rawCfg['workflow'] as Record<string, unknown> | undefined;
|
||||
} catch {
|
||||
wf = undefined;
|
||||
}
|
||||
const action = wf?.context_drift_action === 'block' ? 'block' : 'warn';
|
||||
const block = staleArtifacts.length > 0 && action === 'block';
|
||||
const message = staleArtifacts.length > 0 ? buildContextDriftMessage(staleArtifacts, phaseArg) : '';
|
||||
|
||||
output(
|
||||
{
|
||||
block,
|
||||
skipped: false,
|
||||
stale_artifacts: staleArtifacts,
|
||||
action,
|
||||
message,
|
||||
},
|
||||
raw,
|
||||
);
|
||||
}
|
||||
|
||||
function cmdVerifySchemaDrift(
|
||||
cwd: string,
|
||||
phaseArg: string,
|
||||
@@ -1769,17 +1916,7 @@ function cmdVerifySchemaDrift(
|
||||
// matching "11-expansion"), making the drift gate inspect the wrong phase.
|
||||
// This shares the one selection rule with find-phase / verify
|
||||
// phase-completeness rather than restating it. (#1571, #2528)
|
||||
let phaseDir: string | null = null;
|
||||
const normalizedPhase = normalizePhaseName(phaseArg);
|
||||
const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
|
||||
const dirNames = entries.filter((e) => e.isDirectory()).map((e) => e.name);
|
||||
const drift = matchPhaseDirs(dirNames, normalizedPhase).matches[0];
|
||||
if (drift) phaseDir = path.join(phasesDir, drift);
|
||||
|
||||
if (!phaseDir) {
|
||||
const exact = path.join(phasesDir, phaseArg);
|
||||
if (fs.existsSync(exact)) phaseDir = exact;
|
||||
}
|
||||
const phaseDir = resolvePhaseDirByToken(phasesDir, phaseArg);
|
||||
|
||||
if (!phaseDir) {
|
||||
output(
|
||||
@@ -2000,5 +2137,7 @@ export = {
|
||||
cmdValidateAgents,
|
||||
cmdVerifySchemaDrift,
|
||||
cmdVerifyCodebaseDrift,
|
||||
computeContextDrift,
|
||||
cmdVerifyContextDrift,
|
||||
STATE_HEAD_ADVISORY_COMMITS,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user