Files
msd-core/sdk/src/query/decisions.test.ts
Tom Boucher f30da8326a feat: add gates ensuring discuss-phase decisions are translated to plans and verified (closes #2492) (#2611)
* feat(#2492): add gates ensuring discuss-phase decisions are translated and verified

Two gates close the loop between CONTEXT.md `<decisions>` and downstream
work, fixing #2492:

- Plan-phase **translation gate** (BLOCKING). After requirements
  coverage, refuses to mark a phase planned when a trackable decision
  is not cited (by id `D-NN` or by 6+-word phrase) in any plan's
  `must_haves`, `truths`, or body. Failure message names each missed
  decision with id, category, text, and remediation paths.

- Verify-phase **validation gate** (NON-BLOCKING). Searches plans,
  SUMMARY.md, files modified, and recent commit subjects for each
  trackable decision. Misses are written to VERIFICATION.md as a
  warning section but do not change verification status. Asymmetry is
  deliberate — fuzzy-match miss should not fail an otherwise green
  phase.

Shared helper `parseDecisions()` lives in `sdk/src/query/decisions.ts`
so #2493 can consume the same parser.

Decisions opt out of both gates via `### Claude's Discretion` heading
or `[informational]` / `[folded]` / `[deferred]` tags.

Both gates skip silently when `workflow.context_coverage_gate=false`
(default `true`).

Closes #2492

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(#2492): make plan-phase decision gate actually block (review F1, F8, F9, F10, F15)

- F1: replace `${context_path}` with `${CONTEXT_PATH}` in the plan-phase
  gate snippet so the BLOCKING gate receives a non-empty path. The
  variable was defined in Step 4 (`CONTEXT_PATH=$(_gsd_field "$INIT" ...)`)
  and the gate snippet referenced the lowercase form, leaving the gate to
  run with an empty path argument and silently skip.
- F15: wrap the SDK call with `jq -e '.data.passed == true' || exit 1` so
  failure halts the workflow instead of being printed and ignored. The
  verify-phase counterpart deliberately keeps no exit-1 (non-blocking by
  design) and now carries an inline note documenting the asymmetry.
- F10: tag the JSON example fence as `json` and the options-list fence as
  `text` (MD040).
- F8/F9: anchor the heading-presence test regexes to `^## 13[a-z]?\\.` so
  prose substrings like "Requirements Coverage Gate" mentioned in body
  text cannot satisfy the assertion. Added two new regression tests
  (variable-name match, exit-1 guard) so a future revert is caught.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(#2492): tighten decision-coverage gates against false positives and config drift (review F3,F4,F5,F6,F7,F16,F18,F19)

- F3: forward `workstream` arg through both gate handlers so workstream-scoped
  `workflow.context_coverage_gate=false` actually skips. Added negative test
  that creates a workstream config disabling the gate while the root config
  has it enabled and asserts the workstream call is skipped.
- F4: restrict the plan-phase haystack to designated sections — front-matter
  `must_haves` / `truths` / `objective` plus body sections under headings
  matching `must_haves|truths|tasks|objective`. HTML comments and fenced
  code blocks are stripped before extraction so a commented-out citation or
  a literal example never counts as coverage. Verify-phase keeps the broader
  artifact-wide haystack by design (non-blocking).
- F5: reject decisions with fewer than 6 normalized words from soft-matching
  (previously only rejected when the resulting phrase was under 12 chars
  AFTER slicing — too lenient). Short decisions now require an explicit
  `D-NN` citation, with regression tests for the boundary.
- F6: walk every `*-SUMMARY.md` independently and use `matchAll` with the
  `/g` flag so multiple `files_modified:` blocks across multiple summaries
  are all aggregated. Previously only the first block in the concatenated
  string was parsed, silently dropping later plans' files.
- F7: validate every `files_modified` path stays inside `projectDir` after
  resolution (rejects absolute paths, `../` traversal). Cap each file read
  at 256 KB. Skipped paths emit a stderr warning naming the entry.
- F16: validate `workflow.context_coverage_gate` is boolean in
  `loadGateConfig`; warn loudly on numeric or other-shaped values and
  default to ON. Mirrors the schema-vs-loadConfig validation gap from
  #2609.
- F18: bump verify-phase `git log -n` cap from 50 to 200 so longer-running
  phases are not undercounted. Documented as a precision-vs-recall tradeoff
  appropriate for a non-blocking gate.
- F19: tighten `QueryResult` / `QueryHandler` to be parameterized
  (`<T = unknown>`). Drops the `as unknown as Record<string, unknown>`
  casts in the gate handlers and surfaces shape mismatches at compile time
  for callers that pass a typed `data` value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(#2492): harden decisions parser and verify-phase glob (review F11,F12,F13,F14,F17,F20)

- F11: strip fenced code blocks from CONTEXT.md before searching for
  `<decisions>` so an example block inside ``` ``` is not mis-parsed.
- F12: accept tab-indented continuation lines (previously required a leading
  space) so decisions split with `\t` continue cleanly.
- F13: parse EVERY `<decisions>` block in the file via `matchAll`, not just
  the first. CONTEXT.md may legitimately carry more than one block.
- F14: `decisions.parse` handler now resolves a relative path against
  `projectDir` — symmetric with the gate handlers — and still accepts
  absolute paths.
- F17: replace `ls "${PHASE_DIR}"/*-CONTEXT.md | head -1` in verify-phase.md
  with a glob loop (ShellCheck SC2012 fix). Also avoids spawning an extra
  subprocess and survives filenames with whitespace.
- F20: extend the unicode quote-stripping in the discretion-heading match
  to cover U+2018/2019/201A/201B and the U+201C-F double-quote variants
  plus backtick, so any rendering of "Claude's Discretion" collapses to
  the same key.

Each fix has a regression test in `decisions.test.ts`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-23 00:26:53 -04:00

216 lines
7.1 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* Unit tests for CONTEXT.md `<decisions>` parser.
*
* Decision format (from `discuss-phase.md` lines 1035–1048):
*
* <decisions>
* ## Implementation Decisions
*
* ### Category A
* - **D-01:** First decision text
* - **D-02 [folded]:** Second decision text
*
* ### Claude's Discretion
* - free-form, never tracked
*
* ### Folded Todos
* - **D-03 [folded]:** ...
* </decisions>
*
* Issue #2492.
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { parseDecisions } from './decisions.js';
const MINIMAL = `# Phase 17 Context
<decisions>
## Implementation Decisions
### API Surface
- **D-01:** Use bit offsets, not byte offsets
- **D-02:** Display TArray element type alongside count
### Storage
- **D-03 [informational]:** Backing store is on disk
- **D-04:** Persist via SQLite WAL mode
### Claude's Discretion
- Naming of internal helpers is up to the implementer
- **D-99:** This should be ignored — it lives under Discretion
### Folded Todos
- **D-05 [folded]:** Add a CLI flag for verbose mode
</decisions>
`;
describe('parseDecisions (#2492)', () => {
it('extracts D-NN decisions with id, text, and category', () => {
const decisions = parseDecisions(MINIMAL);
const ids = decisions.map((d) => d.id);
expect(ids).toContain('D-01');
expect(ids).toContain('D-02');
expect(ids).toContain('D-04');
const d01 = decisions.find((d) => d.id === 'D-01');
expect(d01?.text).toBe('Use bit offsets, not byte offsets');
expect(d01?.category).toBe('API Surface');
});
it('captures bracketed tags', () => {
const decisions = parseDecisions(MINIMAL);
const d05 = decisions.find((d) => d.id === 'D-05');
expect(d05?.tags).toContain('folded');
const d03 = decisions.find((d) => d.id === 'D-03');
expect(d03?.tags).toContain('informational');
});
it('marks Claude\'s Discretion entries as non-trackable', () => {
const decisions = parseDecisions(MINIMAL);
const d99 = decisions.find((d) => d.id === 'D-99');
expect(d99).toBeDefined();
expect(d99?.trackable).toBe(false);
// And it must NOT appear in the trackable filter
const trackableIds = decisions.filter((d) => d.trackable).map((d) => d.id);
expect(trackableIds).not.toContain('D-99');
});
it('marks [informational] entries as opt-out (excluded from trackable by default)', () => {
const trackable = parseDecisions(MINIMAL).filter((d) => d.trackable);
const ids = trackable.map((d) => d.id);
expect(ids).toContain('D-01');
expect(ids).toContain('D-02');
expect(ids).toContain('D-04');
expect(ids).not.toContain('D-03'); // [informational] tag
expect(ids).not.toContain('D-05'); // [folded] tag — not user-facing decision
});
it('returns empty array when CONTEXT.md has no <decisions> block', () => {
expect(parseDecisions('# Phase 1\n\nNo decisions here.\n')).toEqual([]);
});
it('returns empty array when content is empty', () => {
expect(parseDecisions('')).toEqual([]);
});
it('returns empty array when <decisions> block is empty', () => {
expect(parseDecisions('<decisions>\n</decisions>')).toEqual([]);
});
it('does not crash on malformed bullet lines', () => {
const malformed = `<decisions>
- not a decision (no D-NN)
- **D-bogus:** wrong id format
- **D-7:** single digit allowed
- **D-10:** ten
</decisions>`;
const decisions = parseDecisions(malformed);
const ids = decisions.map((d) => d.id);
expect(ids).toContain('D-7');
expect(ids).toContain('D-10');
expect(ids).not.toContain('D-bogus');
});
it('preserves multi-line decision text continuations', () => {
const multi = `<decisions>
### Cat
- **D-01:** First line
continues here
- **D-02:** Second
</decisions>`;
const decisions = parseDecisions(multi);
const d01 = decisions.find((d) => d.id === 'D-01');
expect(d01?.text).toMatch(/First line/);
});
// ─── Adversarial-review regressions ────────────────────────────────────
it('ignores `<decisions>` blocks inside fenced code (review F11)', () => {
const content = `# Doc
\`\`\`
<decisions>
### Example
- **D-99:** Should not be parsed
</decisions>
\`\`\`
<decisions>
### Real
- **D-01:** Real decision text long enough to soft match
</decisions>`;
const decisions = parseDecisions(content);
const ids = decisions.map((d) => d.id);
expect(ids).toContain('D-01');
expect(ids).not.toContain('D-99');
});
it('captures continuation lines indented with TABS (review F12)', () => {
const content = '<decisions>\n### Cat\n- **D-07:** First line\n\tcontinued via tab\n</decisions>';
const decisions = parseDecisions(content);
const d07 = decisions.find((d) => d.id === 'D-07');
expect(d07?.text).toMatch(/continued via tab/);
});
it('parses ALL `<decisions>` blocks, not just the first (review F13)', () => {
const content = `<decisions>
### One
- **D-01:** First batch
</decisions>
Some prose.
<decisions>
### Two
- **D-02:** Second batch
</decisions>`;
const ids = parseDecisions(content).map((d) => d.id);
expect(ids).toContain('D-01');
expect(ids).toContain('D-02');
});
it('treats curly-quote variants of "Claude\u2019s Discretion" as non-trackable (review F20)', () => {
// U+201B (single high-reversed-9 quotation mark) — uncommon but legal unicode.
const content =
'<decisions>\n### Claude\u201Bs Discretion\n- **D-50:** Should be non-trackable\n</decisions>';
const decisions = parseDecisions(content);
const d50 = decisions.find((d) => d.id === 'D-50');
expect(d50?.trackable).toBe(false);
});
});
// ─── decisions.parse query handler ────────────────────────────────────────
import { decisionsParse } from './decisions.js';
import { mkdtemp, writeFile, rm, mkdir } from 'node:fs/promises';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
describe('decisionsParse handler (review F14 — accepts relative path via projectDir)', () => {
let tmp: string;
beforeEach(async () => {
tmp = await mkdtemp(join(tmpdir(), 'gsd-decparse-'));
});
afterEach(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('resolves a relative file path against projectDir', async () => {
await mkdir(join(tmp, '.planning', 'phases', '17'), { recursive: true });
await writeFile(
join(tmp, '.planning', 'phases', '17', '17-CONTEXT.md'),
'<decisions>\n### Cat\n- **D-01:** Hello\n</decisions>',
'utf-8',
);
const result = await decisionsParse(['.planning/phases/17/17-CONTEXT.md'], tmp);
expect((result.data as { trackable: number }).trackable).toBe(1);
expect((result.data as { missing: boolean }).missing).toBe(false);
});
it('still accepts an absolute path', async () => {
const abs = join(tmp, 'CONTEXT.md');
await writeFile(abs, '<decisions>\n### Cat\n- **D-02:** Bye\n</decisions>', 'utf-8');
const result = await decisionsParse([abs], tmp);
expect((result.data as { trackable: number }).trackable).toBe(1);
});
});