* test(#1954): failing-first contract for plan-checker undeclared-coupling check * feat(#1954): flag undeclared coupling between same-wave plans * docs(#1954): backfill changeset pr number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/brave-geese-bark.md
Normal file
5
.changeset/brave-geese-bark.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Changed
|
||||
pr: 3237
|
||||
---
|
||||
**`gsd-plan-checker` now flags same-wave plans that are coupled but don't say so** — two plans in the same wave that share mutable state (a config key, table, migration, env var, singleton) or depend on each other's execution order, with no `depends_on` edge between them, are reported as an advisory Dimension 3 finding. The coupling gets settled at plan time instead of surfacing as an intermittent failure during parallel execution. `docs/AGENTS.md`'s plan-checker entry, which claimed eight verification dimensions and listed eight names matching none of the agent's actual fifteen, is corrected to the real list in the same change. (#1954)
|
||||
@@ -210,6 +210,42 @@ issue:
|
||||
fix_hint: "Plan 02 depends on 03, but 03 depends on 02"
|
||||
```
|
||||
|
||||
## Dimension 3b: Undeclared / Temporal Coupling
|
||||
|
||||
**Question:** Do two same-wave plans depend on each other through shared mutable state or
|
||||
execution order without declaring it? Dimension 3 checks *declared* edges and the wave guard
|
||||
checks `files_modified` overlap; neither sees an undeclared edge, which under parallel
|
||||
execution becomes an intermittent failure nobody can attribute.
|
||||
|
||||
**Scope: PLAN pairs, not tasks.** Tasks inside one plan run sequentially and cannot race.
|
||||
Compare same-wave plan pairs over the union of their tasks' `<files>` and `<action>`.
|
||||
|
||||
**FLAG only when ALL THREE hold** (coupling that is strong *and* non-local — Connascence of
|
||||
Execution; strong-but-local coupling inside one plan is fine):
|
||||
1. both plans sit in the same wave, and
|
||||
2. neither declares `depends_on` on the other, and
|
||||
3. their actions name a *specific* shared mutable resource (config key, table/row, migration,
|
||||
env var, singleton, cache) with at least one WRITER, or one names a prerequisite the other
|
||||
produces.
|
||||
|
||||
**Do NOT flag:** both sides only READ it, or it is immutable; the pair already overlaps in
|
||||
`files_modified` (report that once, on the file axis); the plans sit in a different wave, which
|
||||
already orders them; two tasks inside one plan; a vague same-subsystem claim naming no
|
||||
resource; incompatible *transformations* of one entity — that is Dimension 9.
|
||||
|
||||
**Severity: ALWAYS WARNING, never a blocker.** Coupling is sometimes intentional; the finding
|
||||
lets the planner declare the edge, move a plan to a later wave, or justify the pair.
|
||||
|
||||
```yaml
|
||||
issue:
|
||||
dimension: dependency_correctness
|
||||
severity: warning
|
||||
description: "Plans 02 and 03 are both Wave 1 with no depends_on, but 02 writes config key
|
||||
auth.session_ttl and 03 reads it"
|
||||
plans: ["02", "03"]
|
||||
fix_hint: "Declare depends_on, move 03 to a later wave, or justify either order"
|
||||
```
|
||||
|
||||
## Dimension 4: Key Links Planned
|
||||
|
||||
**Question:** Are artifacts wired together, not just created in isolation?
|
||||
@@ -1036,6 +1072,7 @@ Plan verification complete when:
|
||||
- [ ] Requirement coverage checked (all requirements have tasks)
|
||||
- [ ] Task completeness validated (all required fields present)
|
||||
- [ ] Dependency graph verified (no cycles, valid references)
|
||||
- [ ] Undeclared/temporal coupling checked (same-wave plan pairs, advisory)
|
||||
- [ ] Key links checked (wiring planned, not just artifacts)
|
||||
- [ ] Scope assessed (within context budget)
|
||||
- [ ] must_haves derivation verified (user-observable truths)
|
||||
|
||||
@@ -237,15 +237,28 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp
|
||||
| **Color** | Green |
|
||||
| **Produces** | PASS/FAIL verdict with specific feedback |
|
||||
|
||||
**8 Verification Dimensions:**
|
||||
1. Requirement coverage
|
||||
2. Task atomicity
|
||||
3. Dependency ordering
|
||||
4. File scope
|
||||
5. Verification commands
|
||||
6. Context fit
|
||||
7. Gap detection
|
||||
8. Nyquist compliance (when enabled)
|
||||
**Verification Dimensions** — labels match the agent's own `## Dimension <N>` headings:
|
||||
|
||||
| # | Dimension |
|
||||
|---|---|
|
||||
| 1 | Requirement coverage |
|
||||
| 2 | Task completeness |
|
||||
| 3 | Dependency correctness |
|
||||
| 3b | Undeclared / temporal coupling — advisory; flags same-wave plan pairs coupled through shared mutable state or execution order with no `depends_on` between them |
|
||||
| 4 | Key links planned |
|
||||
| 5 | Scope sanity |
|
||||
| 6 | Verification derivation |
|
||||
| 7 | Context compliance (when CONTEXT.md exists) |
|
||||
| 7b | Scope reduction detection |
|
||||
| 7c | Architectural tier compliance (when RESEARCH.md defines a responsibility map) |
|
||||
| 8 | Nyquist compliance (when enabled) |
|
||||
| 9 | Cross-plan data contracts |
|
||||
| 10 | CLAUDE.md compliance |
|
||||
| 11 | Research resolution |
|
||||
| 12 | Pattern compliance |
|
||||
|
||||
Two further dimensions carry no number: **Verify Command Format Sanity** and
|
||||
**Numeric/Factual Claim Authority**.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"gsd-plan-checker.md": "#1954: Dimension 3 gains sub-dimension 3b (undeclared / temporal coupling), plus one success-criteria line. Growth is the new sub-dimension only — no existing text was rewritten. The addition is deliberately dense 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 agent has no lazy-read seam; issue #1954's approved scope is also explicitly 'No new files'. After the change the file sits at 48810 bytes against the LARGE tier hard cap of 49152 (tests/agent-size-budget.test.cjs), i.e. 342 bytes of headroom. That is deliberate and disclosed: the cap is not crossed and is not raised, but the next contributor who needs room in this agent must do a lazy extraction rather than add prose. Content justification: Dimension 3 proves declared dependency edges resolve and are acyclic, and execute-phase's intra-wave guard proves same-wave plans do not overlap in files_modified — neither axis sees an undeclared edge, so two same-wave plans coupled through a shared config key, table, migration, env var, singleton or cache (or through one plan's produced state) pass plan-check and fail intermittently under parallel execution. 3b is advisory only (WARNING, never blocker, per the issue's rejected-alternatives list) and reuses the existing dependency_correctness finding key so no consumer sees a new dimension."
|
||||
}
|
||||
}
|
||||
@@ -2,7 +2,6 @@
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"gsd-integration-checker.md": "#2962: 1 bash block (line ~98 SUMMARY iteration) gained the nullglob shim for zsh portability of the for-glob loop.",
|
||||
"gsd-plan-checker.md": "#2962: 3 bash blocks (lines ~716, ~737, ~822) gained the nullglob shim for zsh portability of the for-glob loops.",
|
||||
"resume-project.md": "#2962: 1 bash block (line ~66 plans-without-summaries scan) gained the nullglob shim for zsh portability of the for-glob loop.",
|
||||
"complete-milestone.md": "#2962: 1 bash block (line ~221 summary one-liner extraction) gained the nullglob shim for zsh portability of the for-glob loop.",
|
||||
"audit-milestone.md": "#2962: 1 bash block (line ~126 requirements_completed extraction) gained the nullglob shim for zsh portability of the for-glob loop."
|
||||
|
||||
289
tests/plan-checker-coupling.test.cjs
Normal file
289
tests/plan-checker-coupling.test.cjs
Normal file
@@ -0,0 +1,289 @@
|
||||
// allow-test-rule: source-text-is-the-product — the plan-checker is a prompt; its .md text IS what the runtime loads (#1954)
|
||||
|
||||
/**
|
||||
* Dimension 3b — undeclared / temporal coupling between same-wave plans (#1954).
|
||||
*
|
||||
* `gsd-plan-checker` proves plan dependencies resolve and are acyclic (Dimension 3),
|
||||
* and `/gsd:execute-phase` separately proves same-wave plans do not overlap in
|
||||
* `files_modified`. Neither axis sees coupling that is real but undeclared — plan A
|
||||
* writes a config key / table / migration / global module that plan B reads, or B
|
||||
* only works if A ran first. In parallel execution that surfaces as an intermittent
|
||||
* failure the executor cannot attribute.
|
||||
*
|
||||
* ## What this suite locks
|
||||
*
|
||||
* The deployed contract AND the wiring between the three regions of the agent doc that
|
||||
* must agree: the sub-check body, its severity rule, and the `<success_criteria>`
|
||||
* checklist. A sub-check present in the body but absent from `success_criteria` is a
|
||||
* check the agent is never told to run; the reverse is a checklist item with no rubric.
|
||||
* Neither half is observable from the other, which is why both are asserted here.
|
||||
*
|
||||
* ## What it cannot prove
|
||||
*
|
||||
* That the model acts on the text. The subject is an LLM prompt — no test in this repo
|
||||
* can prove behavior for any of the agent's twelve existing dimensions either. Stated
|
||||
* so the coverage claim is honest rather than implied.
|
||||
*
|
||||
* Patterns are CRLF-tolerant (`\r?\n`): the runtime loads the file whole, including on
|
||||
* a checkout that produced CRLF, the same case `scripts/workflow-size.cjs` defends.
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const AGENT_PATH = path.join(ROOT, 'agents', 'gsd-plan-checker.md');
|
||||
const DOCS_AGENTS_PATH = path.join(ROOT, 'docs', 'AGENTS.md');
|
||||
|
||||
const agentDoc = fs.readFileSync(AGENT_PATH, 'utf-8');
|
||||
const docsAgents = fs.readFileSync(DOCS_AGENTS_PATH, 'utf-8');
|
||||
|
||||
// ── Span helpers ───────────────────────────────────────────────────
|
||||
// Offsets of the headings that bound each region. `indexOfHeading` returns -1 when
|
||||
// absent so a missing heading fails as a named assertion rather than an off-by-one.
|
||||
function indexOfHeading(content, pattern) {
|
||||
const m = content.match(pattern);
|
||||
return m && typeof m.index === 'number' ? m.index : -1;
|
||||
}
|
||||
|
||||
const D3_HEADING = /^## Dimension 3: /m;
|
||||
const D3B_HEADING = /^## Dimension 3b: /m;
|
||||
const D4_HEADING = /^## Dimension 4: /m;
|
||||
|
||||
function sliceBetween(content, startPattern, endPattern) {
|
||||
const start = indexOfHeading(content, startPattern);
|
||||
const end = indexOfHeading(content, endPattern);
|
||||
assert.ok(start >= 0, `start heading not found: ${startPattern}`);
|
||||
assert.ok(end >= 0, `end heading not found: ${endPattern}`);
|
||||
assert.ok(end > start, `end heading precedes start heading: ${startPattern} .. ${endPattern}`);
|
||||
return content.slice(start, end);
|
||||
}
|
||||
|
||||
/**
|
||||
* Count top-level ordered-list items in a span. This is the trigger gate's arity —
|
||||
* the "flag only when ALL N hold" conjunction. Widening it from 3 to 2 is what turns
|
||||
* a precise heuristic into a noise generator, so the count is asserted, not the prose.
|
||||
*/
|
||||
function countOrderedItems(span) {
|
||||
const matches = span.match(/^\d+\. /gm);
|
||||
return matches ? matches.length : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove fenced code blocks before scanning for headings. The agent's
|
||||
* `### Dimension 8 Output` section embeds a literal `## Dimension 8: ...` line inside a
|
||||
* fence as its output template, so a fence-blind scan reports Dimension 8 twice and any
|
||||
* uniqueness or completeness check built on it is wrong before it starts.
|
||||
*/
|
||||
function stripFences(content) {
|
||||
return content.replace(/^```[\s\S]*?^```/gm, '');
|
||||
}
|
||||
|
||||
describe('gsd-plan-checker Dimension 3b — undeclared/temporal coupling (#1954)', () => {
|
||||
describe('the sub-check exists and is scoped to Dimension 3', () => {
|
||||
test('Dimension 3 carries an undeclared-coupling sub-check', () => {
|
||||
const heading = agentDoc.match(/^## Dimension 3b: (.+)$/m);
|
||||
assert.ok(heading, 'agents/gsd-plan-checker.md must define a "## Dimension 3b:" heading');
|
||||
assert.match(
|
||||
heading[1],
|
||||
/coupling/i,
|
||||
`Dimension 3b must name coupling as its subject, got: ${heading[1]}`
|
||||
);
|
||||
});
|
||||
|
||||
test('the sub-check sits inside Dimension 3, not after it', () => {
|
||||
const d3 = indexOfHeading(agentDoc, D3_HEADING);
|
||||
const d3b = indexOfHeading(agentDoc, D3B_HEADING);
|
||||
const d4 = indexOfHeading(agentDoc, D4_HEADING);
|
||||
assert.ok(d3 >= 0 && d3b >= 0 && d4 >= 0, 'Dimensions 3, 3b and 4 must all be present');
|
||||
assert.ok(d3 < d3b, 'Dimension 3b must follow Dimension 3');
|
||||
assert.ok(d3b < d4, 'Dimension 3b must precede Dimension 4 — it extends Dimension 3');
|
||||
});
|
||||
|
||||
test('the sub-check scopes comparison to plan pairs', () => {
|
||||
// Parallelism is per PLAN (execute-phase spawns one executor per plan per wave),
|
||||
// so two tasks inside one plan run sequentially and cannot race. Scoping the
|
||||
// comparison to task pairs would flag orderings that are guaranteed by construction.
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(
|
||||
span,
|
||||
/Scope:\s*PLAN pairs, not tasks/,
|
||||
'Dimension 3b must state that the comparison is plan-pair scoped'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the trigger gate is a three-way conjunction', () => {
|
||||
test('the trigger gate enumerates exactly three conditions', () => {
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.strictEqual(
|
||||
countOrderedItems(span),
|
||||
3,
|
||||
'Dimension 3b must gate on exactly three AND-ed conditions ' +
|
||||
'(same wave, no declared edge, named shared mutable resource or produced-state ' +
|
||||
'prerequisite). Dropping one widens the heuristic into noise; 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 doc's arity, so its
|
||||
// inequality branch never executes. Exercise the counter at limit-1 / limit /
|
||||
// limit+1 (RULESET.TESTS.boundary-coverage) 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}`;
|
||||
for (const eol of ['\n', '\r\n']) {
|
||||
const spanOf = (count) =>
|
||||
['## Dimension 3b: heading', ...Array.from({ length: count }, (_, i) => item(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)})`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('severity is advisory, and stays advisory', () => {
|
||||
test('the sub-check finding is a warning', () => {
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(
|
||||
span,
|
||||
/severity:\s*warning/,
|
||||
'Dimension 3b\'s example issue must carry severity: warning'
|
||||
);
|
||||
});
|
||||
|
||||
test('the sub-check forbids escalating to blocker', () => {
|
||||
// The agent's own <adversarial_stance> penalises "issuing warnings for what are
|
||||
// actually blockers", which biases the model to escalate. Issue #1954 rejected the
|
||||
// hard-block alternative outright, so the prohibition has to be explicit in the span.
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(
|
||||
span,
|
||||
/never\s+(a\s+)?blocker/i,
|
||||
'Dimension 3b must state that the finding is never a blocker'
|
||||
);
|
||||
assert.doesNotMatch(
|
||||
span,
|
||||
/severity:\s*blocker/,
|
||||
'Dimension 3b must not contain a blocker-severity example — it is advisory only'
|
||||
);
|
||||
});
|
||||
|
||||
test('existing Dimension 3 blocker severities are unchanged', () => {
|
||||
// Independence: 3b is additive. The circular-dependency finding above it must
|
||||
// still block, or this change quietly downgraded a real gate.
|
||||
const d3Body = sliceBetween(agentDoc, D3_HEADING, D3B_HEADING);
|
||||
assert.match(
|
||||
d3Body,
|
||||
/severity:\s*blocker/,
|
||||
'Dimension 3\'s own example issue must still be severity: blocker'
|
||||
);
|
||||
assert.match(
|
||||
d3Body,
|
||||
/Circular dependency/,
|
||||
'Dimension 3 must still carry its circular-dependency example'
|
||||
);
|
||||
});
|
||||
|
||||
test('the finding reuses the dependency_correctness dimension key', () => {
|
||||
// Hyrum: anything consuming the checker's structured issues keys on `dimension`.
|
||||
// A new key would be a new observable contract; the issue asked for a finding
|
||||
// "under Dimension 3", not a new dimension.
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(
|
||||
span,
|
||||
/dimension:\s*dependency_correctness/,
|
||||
'Dimension 3b\'s example issue must reuse the dependency_correctness dimension key'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('negative space is enumerated', () => {
|
||||
test('the sub-check enumerates its non-triggering cases', () => {
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(span, /Do NOT flag/, 'Dimension 3b must carry an explicit non-triggering list');
|
||||
// Each token below is a distinct exclusion class from the design's negative space.
|
||||
// Their absence is what produced false positives in the alternatives considered.
|
||||
for (const [token, why] of [
|
||||
[/files_modified/, 'the file-overlap axis is already checked elsewhere — report it once'],
|
||||
[/depends_on/, 'a pair whose edge is already declared is not a finding'],
|
||||
[/different wave/i, 'the wave itself already orders the pair'],
|
||||
[/READ/, 'two readers of a shared resource are not coupled'],
|
||||
]) {
|
||||
assert.match(span, token, `Dimension 3b non-triggering list must cover: ${why}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('the sub-check defers transform conflicts to Dimension 9', () => {
|
||||
// Dimension 9 (Cross-Plan Data Contracts) owns incompatible transformations of a
|
||||
// shared entity. Without this boundary the same plan pair is reported twice.
|
||||
const span = sliceBetween(agentDoc, D3B_HEADING, D4_HEADING);
|
||||
assert.match(
|
||||
span,
|
||||
/Dimension 9/,
|
||||
'Dimension 3b must defer incompatible-transform findings to Dimension 9'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the sub-check is wired into the agent\'s completion checklist', () => {
|
||||
test('success_criteria includes the coupling check', () => {
|
||||
const span = sliceBetween(agentDoc, /<success_criteria>/, /<\/success_criteria>/);
|
||||
assert.match(
|
||||
span,
|
||||
/^- \[ \] .*coupling.*$/im,
|
||||
'the <success_criteria> checklist must carry a line for the coupling check — ' +
|
||||
'a rubric the agent is never told to run is not a check'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('docs parity', () => {
|
||||
// Parity against the agent's own headings is self-maintaining; a hand-typed count is
|
||||
// not. The section previously claimed "8 Verification Dimensions" while enumerating
|
||||
// names that matched no dimension in the agent at all — a stale count reads as
|
||||
// authoritative, which is worse than no count.
|
||||
function agentDimensionLabels() {
|
||||
return [...stripFences(agentDoc).matchAll(/^## Dimension ([0-9]+[a-z]?): /gm)].map((m) => m[1]);
|
||||
}
|
||||
|
||||
test('the agent defines a discoverable set of numbered dimensions', () => {
|
||||
const labels = agentDimensionLabels();
|
||||
assert.ok(
|
||||
labels.length >= 12,
|
||||
`expected the agent to define at least 12 numbered dimensions, found ${labels.length}`
|
||||
);
|
||||
assert.ok(labels.includes('3b'), 'Dimension 3b must be among the agent\'s numbered dimensions');
|
||||
assert.strictEqual(
|
||||
new Set(labels).size,
|
||||
labels.length,
|
||||
`duplicate dimension labels in the agent: ${labels.join(', ')}`
|
||||
);
|
||||
});
|
||||
|
||||
test('docs/AGENTS.md enumerates every dimension the agent defines', () => {
|
||||
const section = sliceBetween(docsAgents, /^### gsd-plan-checker$/m, /^### gsd-integration-checker$/m);
|
||||
const documented = new Set(
|
||||
[...section.matchAll(/^\| ([0-9]+[a-z]?) \| /gm)].map((m) => m[1])
|
||||
);
|
||||
const missing = agentDimensionLabels().filter((label) => !documented.has(label));
|
||||
assert.deepStrictEqual(
|
||||
missing,
|
||||
[],
|
||||
`docs/AGENTS.md omits dimension(s): ${missing.join(', ')}`
|
||||
);
|
||||
});
|
||||
|
||||
test('docs/AGENTS.md documents the coupling check', () => {
|
||||
const section = sliceBetween(docsAgents, /^### gsd-plan-checker$/m, /^### gsd-integration-checker$/m);
|
||||
assert.match(
|
||||
section,
|
||||
/coupling/i,
|
||||
'docs/AGENTS.md\'s gsd-plan-checker section must document the coupling check'
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user