Phase 3 of #3464, following #3465 and #3466. Those phases cut the exemption ceiling 305 -> 278 by removing vestigial markers and rewriting real source-greps behaviorally. This one addresses why the ratchet was weak in the first place: it counts markers, and markers correlated only loosely with violations, because the rule's implementation was far narrower than its intent. Four gaps, each measured against 833 files under tests/ before any code was written: A TEXT_METHODS omitted matchAll, split and replace, and never handled the regex-side form re.test(tracked) / /lit/.test(tracked) where the tracked value is the ARGUMENT rather than the callee object. B The extension test was /\.(?:cjs|js|ts)/, which does not match .cts, .mts or .mjs. Under ADR-457 this repo's production modules live in src/**/*.cts, so the rule has been structurally blind to the entire TypeScript source surface since that migration. Highest-value fix here. C Tracking stopped at one hop, so an intermediate transform (const b = strip(a); b.match(...)) escaped. D Variables were tracked by NAME in a flat Set<string>, with no scope resolution, so a name reused across describe/test blocks was conflated. D removes one verified false positive where an outer `const src = readFileSync(...)` was cross-attributed to a shadowed arrow-function parameter of the same name. Deliberately NOT implemented: flagging reads whose path cannot be statically resolved. Measured at 4255 sites across 255 files, 66 of them newly red, with a 6-of-6 false-positive rate in spot-checking -- every sampled site read a markdown workflow or fixture doc through a path variable, not JS source. The heuristic "no JS extension literal present" inverts to "not a source file" in this codebase. Shipping it would have manufactured exactly the marker-spam dynamic this epic exists to stop. The principled version needs real static resolution (constant-folding path.join and template literals) and is left to a future phase. The widening surfaced two genuinely-invisible violations, handled on their merits rather than uniformly: tests/verifier-behavior-unverified.test.cjs read src/verification.cts and regexed it for the VERIFIER_STATUSES array. Fixed BEHAVIORALLY with no marker: that constant is already exported, so the test now asserts the real runtime value -- strictly stronger, and immune to source formatting. Mutation-checked: injecting present_behavior_unverified into the exported array turns it red, restoring turns it green. tests/adr-index-gate.test.cjs scans src/plan-drift-guard.cts for docs/adr/<name>.md citations and asserts each cited ADR exists. Those citations live in COMMENTS, erased at compile time: no exported value, no runtime observable, and making it behavioral would mean contorting production code into exporting its own documentation citations. Irreducible, so it takes one marker, cited to #3502, stating exactly why. Documenting a real exemption beats leaving the violation invisible, which was the status quo. Two defects in this branch's own work, both found by review and fixed here rather than shipped: FALSE POSITIVE (adversarial review). Hop propagation walked every Identifier in a declarator init and treated any reference to a tracked variable as derivation, regardless of whether the derived VALUE still carried source text. So `const len = raw.length; /^\d+$/.test(len)` was reported as a source-grep. Propagation is now value-shape aware: it follows identity, string-returning string methods, split/join, template embedding, string concatenation, conditional branches and call arguments; it stops at .length, numeric methods (indexOf/search/charCodeAt), boolean methods (includes/startsWith/test), comparisons, negation, typeof, and Number/parseInt/Boolean coercions. Unrecognized shapes still propagate -- the conservative default for a linter is a rarer false positive over a silent false negative, and that choice is documented inline. Seven RuleTester rows now cover this axis, which was previously untested. SUPER-QUADRATIC SCAN (security review). resolveVariable() resolved each identifier with two linear Array.find passes over scope.references and scope.variables, once per identifier walked -- O(vars-in-scope) per lookup. On a synthetic single-scope file of N consts each referencing ~20 priors: 4.76s at N=3000 and 24.32s at N=6000 (~5.1x for 2x N). Replaced with a Map<IdentifierNode, Variable> built once per file, lazily, from the scope manager: 0.19s and 0.37s for the same inputs (~1.95x for 2x N, linear). Semantics unchanged. No measurable effect on the real repo either way, but this is exactly the bug class ADR-3212 / local/no-unbounded-quantifier exists to catch, and this repo has a prior incident where a rule written to catch complexity bugs shipped with one of its own. The per-widening measurement had a flaw worth recording: each widening was measured in isolation, so a site needing TWO at once appeared in neither column, and the UNION column was dominated by the rejected dynamic-path noise and never inspected for interactions. adr-index-gate needs both B (.cts) and A (.matchAll) and was missed for exactly that reason. Real newly-red count was 2, not the 1 predicted. Corrected on #3502 rather than quietly amended. Marker-bearing files 277 -> 278 against an unchanged ceiling of 278. The ceiling is NOT raised: the sole addition is the cited irreducible exemption. 27 RuleTester rows cover the change. The valid rows carry as much weight as the invalid ones -- shadowed same-name bindings, sibling block scopes, .md and .json literal reads, dynamic path variables, non-textual derivations, reads never text-searched, and require() of a .cjs must all stay valid. A widening that flagged those would be worse than the status quo, because it would push contributors toward adding markers to silence noise. Known limit, documented rather than papered over: a tracked value round-tripped through an array or object literal and read back via destructuring is still not tracked. Pre-existing, not introduced here, and deliberately not widened for -- closing it means tracking member identity, with its own false-positive surface. Closes #3502 Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
204 lines
10 KiB
JavaScript
204 lines
10 KiB
JavaScript
'use strict';
|
|
|
|
// Issue #966 — behavior-dependent must-haves must not pass on symbol presence.
|
|
// Content-assertion contract for the gsd-verifier agent: the
|
|
// PRESENT_BEHAVIOR_UNVERIFIED per-truth state, its routing to human_needed,
|
|
// the behavior-verified score split, and the parity invariant that the new
|
|
// per-truth state never leaks into the overall-status vocabulary.
|
|
|
|
const test = 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 verifierPath = path.join(ROOT, 'agents', 'gsd-verifier.md');
|
|
const verifier = fs.readFileSync(verifierPath, 'utf-8');
|
|
const standaloneTemplatePath = path.join(ROOT, 'gsd-core', 'templates', 'verification-report.md');
|
|
const standalone = fs.readFileSync(standaloneTemplatePath, 'utf-8');
|
|
|
|
test('Step 3 defines the PRESENT_BEHAVIOR_UNVERIFIED per-truth state', () => {
|
|
assert.match(verifier, /PRESENT_BEHAVIOR_UNVERIFIED/);
|
|
assert.match(verifier, /present[^\n]*wired|wired[^\n]*present/i);
|
|
});
|
|
|
|
test('behavior-dependent trigger names transition + cancellation/cleanup/ordering invariants', () => {
|
|
assert.match(verifier, /state transition/i);
|
|
assert.match(verifier, /cancellation|cleanup|ordering/i);
|
|
assert.match(verifier, /invariant/i);
|
|
});
|
|
|
|
test('PRESENT_BEHAVIOR_UNVERIFIED routes to human verification', () => {
|
|
assert.match(verifier, /PRESENT_BEHAVIOR_UNVERIFIED[\s\S]{0,400}?human/i);
|
|
});
|
|
|
|
test('PRESENT_BEHAVIOR_UNVERIFIED is the only truth excluded from the verified score', () => {
|
|
assert.match(
|
|
verifier,
|
|
/(do not count it toward the verified score)|(only[^\n]*excluded from `verified_truths`)|(only truths excluded)/i,
|
|
);
|
|
});
|
|
|
|
test('score still credits PASSED (override) truths (override contract preserved)', () => {
|
|
// The Step 9 score definition must count override-passed truths in verified_truths.
|
|
assert.match(
|
|
verifier,
|
|
/verified_truths[\s\S]{0,200}?PASSED \(override\)/,
|
|
'Step 9 score must count PASSED (override) truths in verified_truths',
|
|
);
|
|
});
|
|
|
|
test('behavior-unverified truths get a structured frontmatter list that survives gaps_found', () => {
|
|
assert.match(verifier, /behavior_unverified_items/);
|
|
// and it must NOT be gated only to human_needed (must mention it is emitted regardless of status / when count > 0)
|
|
assert.match(
|
|
verifier,
|
|
/behavior_unverified_items[\s\S]{0,160}?(regardless of (overall )?status|count > 0)/i,
|
|
);
|
|
});
|
|
|
|
test('Step 9 / template carry the behavior_unverified score-split field', () => {
|
|
assert.match(verifier, /behavior_unverified/);
|
|
});
|
|
|
|
test('critical_rules calibrates "presence is not behavior" without dropping the speed guard', () => {
|
|
assert.match(verifier, /presence is not behavior/i);
|
|
assert.match(verifier, /Keep verification fast/);
|
|
});
|
|
|
|
test('PARITY: per-truth state never leaks into the overall-status vocabulary', () => {
|
|
assert.doesNotMatch(verifier, /→ \*\*status:\s*present_behavior_unverified\*\*/i);
|
|
const unionLines = verifier.match(/^status:\s+[a-z_]+(?:\s*\|\s*[a-z_]+)+\s*$/gm) || [];
|
|
assert.ok(unionLines.length > 0, 'expected at least one status union line');
|
|
for (const line of unionLines) {
|
|
assert.doesNotMatch(line, /present_behavior_unverified/i);
|
|
// The real invariant is that the per-truth state is NOT in the union (above).
|
|
// Membership (order-independent) avoids brittleness on a future legitimate reorder.
|
|
for (const s of ['passed', 'gaps_found', 'human_needed']) {
|
|
assert.ok(line.includes(s), `status union must still contain ${s}: ${line}`);
|
|
}
|
|
}
|
|
});
|
|
|
|
test('overall-status enum in verification.cts is unchanged (no per-truth leak)', () => {
|
|
// Assert on the real exported runtime value rather than regexing the
|
|
// source text — strictly stronger (exercises the built module) and
|
|
// immune to source formatting changes.
|
|
const verificationLib = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'verification.cjs'));
|
|
const { VERIFIER_STATUSES } = verificationLib;
|
|
assert.ok(Array.isArray(VERIFIER_STATUSES), 'VERIFIER_STATUSES array must be present');
|
|
assert.ok(
|
|
!VERIFIER_STATUSES.includes('present_behavior_unverified'),
|
|
'VERIFIER_STATUSES must not leak the per-truth present_behavior_unverified state',
|
|
);
|
|
for (const s of ['passed', 'gaps_found', 'human_needed']) {
|
|
assert.ok(VERIFIER_STATUSES.includes(s), `VERIFIER_STATUSES must contain ${s}`);
|
|
}
|
|
});
|
|
|
|
test('VERIFICATION.md templates carry behavior_unverified + the new truth-state', () => {
|
|
assert.match(verifier, /behavior_unverified/);
|
|
assert.match(standalone, /PRESENT_BEHAVIOR_UNVERIFIED/);
|
|
assert.match(standalone, /behavior_unverified/);
|
|
assert.match(verifier, /behavior_unverified_items/);
|
|
assert.match(standalone, /behavior_unverified_items/);
|
|
});
|
|
|
|
const verifyPhase = fs.readFileSync(path.join(ROOT, 'gsd-core', 'references', 'verifier-phase-gates.md'), 'utf-8');
|
|
const planningArtifacts = fs.readFileSync(path.join(ROOT, 'docs', 'reference', 'planning-artifacts.md'), 'utf-8');
|
|
|
|
test('shipped verifier-phase-gates reference mirrors the behavior-unverified calibration', () => {
|
|
assert.match(verifyPhase, /PRESENT_BEHAVIOR_UNVERIFIED/);
|
|
assert.match(verifyPhase, /behavior_unverified/);
|
|
assert.match(verifyPhase, /state transition/i);
|
|
});
|
|
|
|
test('planning-artifacts reference documents the behavior-unverified calibration', () => {
|
|
assert.match(planningArtifacts, /PRESENT_BEHAVIOR_UNVERIFIED/);
|
|
assert.match(planningArtifacts, /behavior_unverified/);
|
|
});
|
|
|
|
test('Step 9 keeps gaps_found precedence and preserves behavior-unverified items', () => {
|
|
assert.match(verifier, /gaps_found's precedence|gaps_found[\s\S]{0,160}?precedence/i);
|
|
assert.match(verifier, /behavior_unverified_items[\s\S]{0,120}?(never lost|survive|regardless)/i);
|
|
});
|
|
|
|
test('shipped workflow flags behavior-unverified truths even on infrastructure phases', () => {
|
|
assert.match(
|
|
verifyPhase,
|
|
/PRESENT_BEHAVIOR_UNVERIFIED[\s\S]{0,400}?infrastructure|infrastructure[\s\S]{0,400}?PRESENT_BEHAVIOR_UNVERIFIED/i,
|
|
);
|
|
});
|
|
|
|
test('standalone template per-truth guideline respects gaps_found precedence', () => {
|
|
assert.match(standalone, /becomes `human_needed`[\s\S]{0,80}?gaps_found/i);
|
|
});
|
|
|
|
|
|
// ────────────────────────────────────────────────────────────────────────
|
|
// Folded from tests/bug-3321-verifier-runs-probes.test.cjs — consolidation epic #1969 (B7 #1976)
|
|
// ────────────────────────────────────────────────────────────────────────
|
|
{
|
|
const { describe: __foldDescribe } = require('node:test');
|
|
__foldDescribe("folded:bug-3321-verifier-runs-probes (consolidation epic #1969 B7 #1976)", () => {
|
|
'use strict';
|
|
|
|
const { describe, test } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('node:fs');
|
|
const path = require('node:path');
|
|
|
|
const REPO_ROOT = path.join(__dirname, '..');
|
|
const VERIFIER_AGENT = path.join(REPO_ROOT, 'agents', 'gsd-verifier.md');
|
|
|
|
function verifierProbeContract(content) {
|
|
const sectionStart = content.indexOf('## Step 7c: Probe Execution');
|
|
const sectionEnd = content.indexOf('## Step 8:', sectionStart);
|
|
assert.notEqual(sectionStart, -1, 'verifier must define Step 7c');
|
|
assert.notEqual(sectionEnd, -1, 'verifier must close Step 7c before Step 8');
|
|
|
|
const section = content.slice(sectionStart, sectionEnd);
|
|
const codeBlocks = [...section.matchAll(/```bash\r?\n([\s\S]*?)\r?\n```/g)].map((match) => match[1].split(/\r?\n/).join('\n'));
|
|
const executionSteps = [...section.matchAll(/^\d+\.\s+(.+)$/gm)].map((match) => match[1]);
|
|
return {
|
|
title: 'Step 7c: Probe Execution',
|
|
conventionalDiscoveryCommand: codeBlocks[0]?.split('\n').find((line) => line.startsWith('find scripts')) || null,
|
|
declaredDiscoveryCommand: codeBlocks[0]?.split('\n').find((line) => line.startsWith('grep -R')) || null,
|
|
executionCommand: codeBlocks[1] || '',
|
|
executionSteps,
|
|
statusRows: [...section.matchAll(/^\|\s*`([^`]+)`\s*\|\s*`([^`]+)`\s*\|[^|]+\|\s*([^|]+)\|$/gm)]
|
|
.map((match) => ({ probe: match[1], command: match[2], statuses: match[3].trim() })),
|
|
summaryClaimsRejected: section.includes('SUMMARY.md probe pass claims are not evidence'),
|
|
};
|
|
}
|
|
|
|
describe('bug #3321: gsd-verifier runs probes instead of trusting SUMMARY claims', () => {
|
|
test('verifier prompt requires direct probe discovery and execution', () => {
|
|
const content = fs.readFileSync(VERIFIER_AGENT, 'utf8');
|
|
const contract = verifierProbeContract(content);
|
|
|
|
assert.equal(contract.title, 'Step 7c: Probe Execution');
|
|
assert.equal(contract.conventionalDiscoveryCommand, "find scripts -path '*/tests/probe-*.sh' -type f 2>/dev/null | sort");
|
|
assert.equal(
|
|
contract.declaredDiscoveryCommand,
|
|
"grep -R -n -E 'probe-[^[:space:]]+\\.sh|scripts/.*/tests/probe-.*\\.sh' \"$PHASE_DIR\"/*-PLAN.md \"$PHASE_DIR\"/*-SUMMARY.md 2>/dev/null",
|
|
);
|
|
assert.deepEqual(contract.executionSteps, [
|
|
'Build the `PROBES` list from explicit PLAN declarations first; include conventional `scripts/*/tests/probe-*.sh` when the phase is a migration/tooling phase or the success criteria mention probes.',
|
|
'For every documented probe path, if the file is missing or unreadable, mark `MISSING_PROBE` and set `status: gaps_found`. Do not require the executable bit because probes run through `bash "$probe"`.',
|
|
'Run each probe from the built `PROBES` list from the repository root:',
|
|
'Exit code 0 is PASS. Any non-zero exit is FAILED and must include stdout/stderr evidence in VERIFICATION.md.',
|
|
'Do not substitute executor narration, SUMMARY.md PASS-marker counts, or a different dry-run driver command for the probe result.',
|
|
]);
|
|
assert.equal(contract.executionCommand, 'for probe in "${PROBES[@]}"; do\n gsd_run run-with-timeout 30 -- bash "$probe"\ndone');
|
|
assert.deepEqual(contract.statusRows, [{
|
|
probe: 'scripts/.../probe-name.sh',
|
|
command: 'bash "$probe"',
|
|
statuses: 'PASS / FAILED / MISSING_PROBE',
|
|
}]);
|
|
assert.equal(contract.summaryClaimsRejected, true);
|
|
});
|
|
});
|
|
});
|
|
}
|