chore(#1417): add resolution-provenance CI guard (Resolution Provenance P4) (#1428)

Adds scripts/lint-resolution-provenance.cjs — a registry + ratchet CI guard
that locks in the agent-skills configured_empty/not_configured contract tests
so they cannot be silently removed, and establishes a registration point for
future config-interpreting read verbs (ADR-1411 P4).

Design rationale:
- REGISTRY (one entry: agent-skills → src/init.cts → tests/agent-skills.test.cjs)
  is the canonical registration site; new verbs are added here.
- For each registered verb, the guard asserts its test file contains BOTH a
  `configured_empty` assertion AND a `not_configured` assertion — proving the
  configured-empty-vs-not-configured contract is explicitly tested.
- NOT a universal static detector (intractable / false positives) — mirrors the
  no-adhoc-markdown-parsing grandfather pattern.
- Uses scripts/lib/allowlist-ratchet.cjs (assertWithinAllowlist) so stale
  allowlist entries fail (ratchet-down) and novel offenders always fail.
- checkRegistry() is factored as a pure exported function tested in
  tests/lint-resolution-provenance.test.cjs without shelling out.
- Wired into lint:ci (package.json) and lint step name updated in test.yml.
- CONTEXT.md ### Resolution Convention extended with P4 guard sentence.
- Allowlist starts empty ([]) — agent-skills already has its tests.

Closes #1417
Part of #1411

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-18 08:50:22 -04:00
committed by GitHub
parent 2d406d2f17
commit 8040a6bac0
6 changed files with 371 additions and 3 deletions

View File

@@ -108,7 +108,7 @@ jobs:
# eslint invocation home; the --cache flag inside it is a no-op in CI
# (node_modules/.cache is never restored) but still speeds local runs.
# Each sub-lint prints its own banner, so a failure identifies itself.
- name: Lint — all (ESLint, skill deps, test-file count, command contract, PR checks, legacy name, regression-test names)
- name: Lint — all (ESLint, skill deps, test-file count, command contract, PR checks, legacy name, regression-test names, resolution-provenance)
run: npm run lint:ci
test:

View File

@@ -98,7 +98,7 @@ Module owning projection from project/workstream context to concrete `.planning`
Cross-seam principle (ADR-1411, epic #1411): context resolution — config loading, project-root anchoring, workstream resolution — must report its provenance, not fall open silently to defaults. A resolver anchors deterministically to the project root (one walk-up module, no dependence on an arbitrary descendant cwd), returns *what* it resolved **and** *where it came from* (`source`/`degraded`), and surfaces a diagnostic when a *configured* input resolves empty (`not configured` and `configured-but-empty` are distinguishable). The resolution-side analog of ADR-227 (input-validation shape). Target seams: Config Loader Module (`loadConfig` → `ConfigResolution { config, source, degraded }`), Project-Root Resolution Module (single nearest-`.planning/` walk-up, retiring ad-hoc resolvers like `resolvePlanningCwd`), I/O Module (`Resolution<T> { value, configured, reason, warnings }` output envelope). A configured input resolving empty without a reason is a CI-guarded regression. **P1 (nearest-.planning/ heuristic) shipped in #1413; P2 (loadConfigResolved + agent-skills diagnostic) shipped in #1415 / closes #1366**: `loadConfigResolved` now implements the Config Loader seam target; `cmdAgentSkills` uses `findProjectRoot` + `loadConfigResolved` and emits `configured`/`reason`/`source`/`degraded` in its `--json` IR.
### Resolution Convention
Diagnostic-output convention for the Resolution Provenance principle (ADR-1411 P3, #1416). Config-interpreting read verbs expose `Resolution<T> { value, configured, reason, warnings }` (`src/resolution.cts`); agent-skills is the first adopter, where `value = { block, skills_count }` and `source`/`degraded` remain config-provenance extras outside the envelope. Other read verbs expose at least `warnings[]` (e.g. capability-state `{ runtimeConfigDir, capabilities, warnings? }`) without `configured`/`reason`, which are meaningful only for config-interpreting verbs. Mutation verbs expose `warnings[]` (advisory) PLUS `errors[]` (operation-not-applied), e.g. capability-writer `{ capabilities, warnings, errors }`. The shared seam across all shapes is `warnings: string[]`; a single generic `Resolution<T>` across read+write verbs was rejected by the deletion test (`configured`/`reason` are meaningless for capability verbs; `errors[]` cannot fold into `warnings[]`) — ADR-1411 P3 amendment. Recurrence prevention is delivered by P4's CI guard (a configured input resolving empty must carry a `reason`), not by a shared envelope.
Diagnostic-output convention for the Resolution Provenance principle (ADR-1411 P3, #1416). Config-interpreting read verbs expose `Resolution<T> { value, configured, reason, warnings }` (`src/resolution.cts`); agent-skills is the first adopter, where `value = { block, skills_count }` and `source`/`degraded` remain config-provenance extras outside the envelope. Other read verbs expose at least `warnings[]` (e.g. capability-state `{ runtimeConfigDir, capabilities, warnings? }`) without `configured`/`reason`, which are meaningful only for config-interpreting verbs. Mutation verbs expose `warnings[]` (advisory) PLUS `errors[]` (operation-not-applied), e.g. capability-writer `{ capabilities, warnings, errors }`. The shared seam across all shapes is `warnings: string[]`; a single generic `Resolution<T>` across read+write verbs was rejected by the deletion test (`configured`/`reason` are meaningless for capability verbs; `errors[]` cannot fold into `warnings[]`) — ADR-1411 P3 amendment. Recurrence prevention is delivered by P4's CI guard (a configured input resolving empty must carry a `reason`), not by a shared envelope. A CI guard (`scripts/lint-resolution-provenance.cjs`, wired into `lint:ci`) enforces that every registered config-interpreting read verb keeps a `configured_empty`/`not_configured` contract test; the registry in that script is the registration point for future verbs (ADR-1411 P4 / #1417).
### Worktree Safety Policy Module
CJS Module owning worktree lifecycle safety policy for the GSD orchestration layer. Interface: `resolveWorktreeContext(cwd, deps) → WorktreeContext` (linked-worktree root mapping), `parseWorktreePorcelain(output) → WorktreeEntry[]` (porcelain parser, skips detached HEAD), `planWorktreePrune(repoRoot, opts, deps) → PrunePlan` (metadata-prune plan, never destructive by default), `executeWorktreePrunePlan(plan, deps) → PruneResult` (executes prune; degrades gracefully on git timeout), `listLinkedWorktreePaths(repoRoot, deps) → LinkedPathsResult`, `inspectWorktreeHealth(repoRoot, opts, deps) → HealthResult` (orphan + stale detection), `snapshotWorktreeInventory(repoRoot, opts, deps) → InventoryResult`, `planWorktreeWaveCleanup(repoRoot, manifest) → CleanupPlan` (manifest-scoped, fail-closed), `executeWorktreeWaveCleanupPlan(plan, deps) → CleanupResult`. Source of truth: `gsd-core/bin/lib/worktree-safety.cjs`. Timeout path: all git subprocess calls are bounded; callers receive `ok:false, reason:'git_timed_out'` rather than a thrown exception. Test anchor: `tests/worktree-safety.test.cjs`. The `core.cjs` re-export spine was retired in epic #1267: this module absorbed the two thin compositional wrappers that squatted in Core — `resolveWorktreeRoot(cwd, deps)` (a projection over `resolveWorktreeContext`) and `pruneOrphanedWorktrees(...)` (sequences `planWorktreePrune` + `executeWorktreePrunePlan` with a timeout warning) — so callers reach this single worktree-lifecycle seam directly. `gitWorktreeInfoInternal` did NOT move here — worktree-info detection belongs to the Git Query Module.

View File

@@ -92,7 +92,7 @@
"pretest:coverage": "npm run build:lib && npm run lint:skill-deps",
"lint": "eslint . --cache --cache-location node_modules/.cache/eslint/",
"lint:fix": "eslint . --fix",
"lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-windows-test-portability.cjs && node scripts/lint-allow-test-rule-refs.cjs",
"lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-windows-test-portability.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs",
"lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs",
"lint:windows-test-portability": "node scripts/lint-windows-test-portability.cjs",
"lint:regression-names": "node scripts/lint-regression-test-names.cjs",

View File

@@ -0,0 +1 @@
[]

View File

@@ -0,0 +1,192 @@
#!/usr/bin/env node
'use strict';
/**
* lint-resolution-provenance.cjs — CI guard for Resolution Provenance contracts.
*
* ## Purpose (ADR-1411 P4 / #1417)
*
* This is a REGRESSION-LOCK and REGISTRATION RATCHET, NOT a universal static
* detector (which is intractable given false positives from config-reading
* helpers that don't consume a `reason`).
*
* The guard maintains a REGISTRY of config-interpreting read verbs that MUST
* carry provenance — each entry names the verb, its source file, and its test
* file. For every registered verb, the guard asserts that its test file
* contains BOTH a `configured_empty` assertion AND a `not_configured`
* assertion, proving that the configured-empty-vs-not-configured contract is
* explicitly tested (not silently open-to-defaults).
*
* ## Registration protocol
*
* When adding a NEW config-interpreting read verb:
* 1. Add an entry to REGISTRY below: { verb, sourceFile, testFile }.
* 2. Add a `configured_empty` test and a `not_configured` test to testFile.
* 3. If the test coverage cannot land in the same PR, add the verb to
* scripts/lint-resolution-provenance.allowlist.json to grandfather it —
* but the allowlist MUST shrink over time (stale entries fail).
*
* See docs/adr/1411-resolution-provenance.md and #1417.
*/
const fs = require('fs');
const path = require('path');
const { assertWithinAllowlist } = require('./lib/allowlist-ratchet.cjs');
const { ExitError, runMain } = require('./lib/cli-exit.cjs');
const ROOT = path.join(__dirname, '..');
const ALLOWLIST_PATH = path.join(__dirname, 'lint-resolution-provenance.allowlist.json');
/**
* Registry of config-interpreting read verbs that must carry provenance.
* Each entry: { verb, sourceFile, testFile }
*
* - verb: Short stable name for this verb (used in error messages and the
* allowlist).
* - sourceFile: Path (relative to ROOT) to the source implementation.
* - testFile: Path (relative to ROOT) to the test file that MUST contain
* both a `configured_empty` assertion and a `not_configured`
* assertion.
*
* Seed: agent-skills (P2/P3 fix, #1415/#1416) is the founding member.
*/
const REGISTRY = [
{
verb: 'agent-skills',
sourceFile: 'src/init.cts',
testFile: 'tests/agent-skills.test.cjs',
},
];
// Markers that MUST appear in every registered verb's test file.
const MARKER_CONFIGURED_EMPTY = 'configured_empty';
const MARKER_NOT_CONFIGURED = 'not_configured';
/**
* Pure check logic — factored out for unit testing without I/O.
*
* @param {object} opts
* @param {Array<{verb: string, sourceFile: string, testFile: string}>} opts.registry
* The REGISTRY to check (or an injected subset for tests).
* @param {string[]} opts.allowlist
* Array of verb names to grandfather (stale entries fail).
* @param {function(string): string} opts.readFile
* Reads a file path and returns its content. Injected for testability;
* callers pass `(p) => fs.readFileSync(p, 'utf8')`.
* @param {function(string): void} opts.fail
* Callback invoked with a descriptive failure message.
* @returns {{ ok: boolean }}
*/
function checkRegistry({ registry, allowlist, readFile, fail }) {
const allowlistSet = new Set(allowlist);
const offenders = []; // verbs that ARE failing (for ratchet: stale check)
let anyFail = false;
for (const entry of registry) {
const { verb, testFile } = entry;
const resolvedTestFile = path.isAbsolute(testFile) ? testFile : path.join(ROOT, testFile);
// Grandfathered? Check markers anyway to detect when it's been fixed.
let content;
try {
content = readFile(resolvedTestFile);
} catch (err) {
fail(
`[resolution-provenance] Cannot read test file for verb "${verb}" (${testFile}): ${err.message}\n` +
` Register the verb's test file correctly, or remove the registry entry.`
);
anyFail = true;
offenders.push(verb);
continue;
}
const hasConfiguredEmpty = content.includes(MARKER_CONFIGURED_EMPTY);
const hasNotConfigured = content.includes(MARKER_NOT_CONFIGURED);
if (!hasConfiguredEmpty || !hasNotConfigured) {
offenders.push(verb);
if (allowlistSet.has(verb)) {
// Grandfathered — tolerate but don't report.
continue;
}
const missing = [];
if (!hasConfiguredEmpty) missing.push(`\`configured_empty\``);
if (!hasNotConfigured) missing.push(`\`not_configured\``);
fail(
`[resolution-provenance] verb "${verb}" (${testFile}) is missing contract test marker(s):\n` +
` Missing: ${missing.join(', ')}\n` +
` Each registered config-interpreting read verb must have both a\n` +
` \`configured_empty\` assertion and a \`not_configured\` assertion in its\n` +
` test file to prove the configured-empty-vs-not-configured contract is\n` +
` tested (ADR-1411 P4 / #1417).\n` +
` Add the missing test(s) or grandfather the verb in\n` +
` scripts/lint-resolution-provenance.allowlist.json.`
);
anyFail = true;
}
}
// Ratchet: stale allowlist entries (verb is compliant but still grandfathered)
// must be pruned so the allowlist only ever shrinks.
const offenderSet = new Set(offenders);
const staleEntries = [];
for (const v of allowlistSet) {
if (!offenderSet.has(v)) {
staleEntries.push(v);
}
}
// Also verify stale entries via assertWithinAllowlist for consistent messaging.
const ratchetFailures = [];
assertWithinAllowlist({
label: 'resolution-provenance',
current: offenders,
known: allowlist,
fail: (msg) => ratchetFailures.push(msg),
pruneHint: 'edit scripts/lint-resolution-provenance.allowlist.json',
});
// Only report stale entries from the ratchet (novel offenders are already
// reported above with more actionable messages).
if (staleEntries.length > 0) {
for (const msg of ratchetFailures) {
// Only surface the stale-entry message (it contains "stale" or "no longer").
if (msg.includes('stale') || msg.includes('no longer')) {
fail(msg);
anyFail = true;
}
}
}
return { ok: !anyFail };
}
function main() {
const allowlist = JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf8'));
const failures = [];
const { ok } = checkRegistry({
registry: REGISTRY,
allowlist,
readFile: (filePath) => fs.readFileSync(filePath, 'utf8'),
fail: (msg) => failures.push(msg),
});
if (!ok) {
for (const msg of failures) process.stderr.write(`${msg}\n`);
throw new ExitError(1);
}
console.log(
`ok lint-resolution-provenance: ${REGISTRY.length} registered verb(s), all carry configured_empty + not_configured contract tests`
);
}
module.exports = { checkRegistry, REGISTRY };
// Only run the CLI check when executed directly, not when imported by tests
// (keeps the unit tests hermetic — importing checkRegistry must not run main).
if (require.main === module) runMain(main);

View File

@@ -0,0 +1,175 @@
'use strict';
/**
* Tests for scripts/lint-resolution-provenance.cjs — the registry-ratchet CI
* guard that locks in configured_empty / not_configured contract tests for
* every registered config-interpreting read verb (ADR-1411 P4 / #1417).
*
* Tests the PURE check logic (checkRegistry) directly, injecting fixture
* content rather than shelling out, so the suite is fast and hermetic.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
// Import the exported check function.
const { checkRegistry } = require('../scripts/lint-resolution-provenance.cjs');
/**
* Run checkRegistry with synthetic content injected as testFileContent so
* the test never reads from the real repo. Returns { ok, failures }.
*
* @param {object} opts
* @param {Array<{verb: string, sourceFile: string, testFile: string}>} opts.registry
* @param {string[]} opts.allowlist
* @param {string} opts.testFileContent - text that represents the test file's content
*/
function runCheck({ registry, allowlist, testFileContent }) {
const failures = [];
const { ok } = checkRegistry({
registry,
allowlist,
// Inject a content-reader so the test is I/O-free.
readFile: (_filePath) => testFileContent,
fail: (msg) => failures.push(msg),
});
return { ok, failures };
}
// ── suite ────────────────────────────────────────────────────────────────────
describe('lint-resolution-provenance: checkRegistry pure logic', () => {
const BOTH_MARKERS = `
test('configured_empty: agent_skills[X]=[] ...', () => {
assert.strictEqual(ir.reason, 'configured_empty');
});
test('not_configured: agent not in map ...', () => {
assert.strictEqual(ir.reason, 'not_configured');
});
`;
const MISSING_CONFIGURED_EMPTY = `
test('not_configured: agent not in map ...', () => {
assert.strictEqual(ir.reason, 'not_configured');
});
`;
const MISSING_NOT_CONFIGURED = `
test('configured_empty: agent_skills[X]=[] ...', () => {
assert.strictEqual(ir.reason, 'configured_empty');
});
`;
const sampleVerb = { verb: 'agent-skills', sourceFile: 'src/init.cts', testFile: 'tests/agent-skills.test.cjs' };
test('ok: registered verb whose test has both markers passes', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: [],
testFileContent: BOTH_MARKERS,
});
assert.strictEqual(failures.length, 0, `Unexpected failures: ${failures.join('\n')}`);
assert.ok(ok);
});
test('fail: missing configured_empty marker → fails with actionable message', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: [],
testFileContent: MISSING_CONFIGURED_EMPTY,
});
assert.ok(!ok);
assert.ok(failures.length > 0, 'Expected at least one failure');
assert.match(failures.join('\n'), /configured_empty/);
assert.match(failures.join('\n'), /agent-skills/);
});
test('fail: missing not_configured marker → fails with actionable message', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: [],
testFileContent: MISSING_NOT_CONFIGURED,
});
assert.ok(!ok);
assert.ok(failures.length > 0, 'Expected at least one failure');
assert.match(failures.join('\n'), /not_configured/);
assert.match(failures.join('\n'), /agent-skills/);
});
test('fail: missing both markers → fails mentioning both', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: [],
testFileContent: '// no relevant markers here',
});
assert.ok(!ok);
const msg = failures.join('\n');
assert.match(msg, /configured_empty/);
assert.match(msg, /not_configured/);
});
test('tolerated: allowlisted verb with missing markers is skipped', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: ['agent-skills'],
testFileContent: MISSING_CONFIGURED_EMPTY,
});
assert.strictEqual(failures.length, 0, `Unexpected failures: ${failures.join('\n')}`);
assert.ok(ok);
});
test('fail: stale allowlist entry (verb now compliant) must be pruned', () => {
const { ok, failures } = runCheck({
registry: [sampleVerb],
allowlist: ['agent-skills'],
testFileContent: BOTH_MARKERS,
});
assert.ok(!ok);
assert.match(failures.join('\n'), /agent-skills/);
assert.match(failures.join('\n'), /stale|prune|no longer/i);
});
test('ok: empty registry always passes', () => {
const { ok, failures } = runCheck({
registry: [],
allowlist: [],
testFileContent: '',
});
assert.strictEqual(failures.length, 0);
assert.ok(ok);
});
test('ok: multiple verbs all compliant passes', () => {
const verb2 = { verb: 'config-read', sourceFile: 'src/config.cts', testFile: 'tests/config.test.cjs' };
const { ok, failures } = runCheck({
registry: [sampleVerb, verb2],
allowlist: [],
testFileContent: BOTH_MARKERS,
});
assert.strictEqual(failures.length, 0, `Unexpected failures: ${failures.join('\n')}`);
assert.ok(ok);
});
});
describe('lint-resolution-provenance: real repo baseline', () => {
test('repo baseline passes (real registry vs real agent-skills.test.cjs)', () => {
// Run the actual check against the real registry and real test file.
// This is the regression lock: if agent-skills.test.cjs loses its markers
// the guard catches it here too.
const { checkRegistry: check, REGISTRY } = require('../scripts/lint-resolution-provenance.cjs');
const failures = [];
const { ok } = check({
registry: REGISTRY,
allowlist: [],
readFile: (filePath) => fs.readFileSync(filePath, 'utf8'),
fail: (msg) => failures.push(msg),
});
assert.strictEqual(
failures.length,
0,
`Real repo baseline failed:\n${failures.join('\n')}`
);
assert.ok(ok);
});
});