Files
msd-core/sdk/src/query/decisions.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

193 lines
6.2 KiB
TypeScript

/**
* CONTEXT.md `<decisions>` parser — shared helper for issue #2492 (decision
* coverage gates) and #2493 (post-planning gap checker).
*
* Decision format (produced by `discuss-phase.md`):
*
* <decisions>
* ## Implementation Decisions
*
* ### Category Heading
* - **D-01:** Decision text
* - **D-02 [tag1, tag2]:** Tagged decision
*
* ### Claude's Discretion
* - free-form, never tracked
* </decisions>
*
* A decision is "trackable" when:
* - it has a valid D-NN id
* - it is NOT under the "Claude's Discretion" category
* - it is NOT tagged `informational` or `folded`
*
* Trackable decisions are the ones the plan-phase translation gate and the
* verify-phase validation gate enforce.
*/
import { readFile } from 'node:fs/promises';
import { isAbsolute, join } from 'node:path';
import type { QueryHandler } from './utils.js';
export interface ParsedDecision {
/** Stable id: `D-01`, `D-7`, `D-42`. */
id: string;
/** Body text (everything after `**D-NN[ tags]:**` up to next bullet/blank). */
text: string;
/** Most recent `### ` heading inside the decisions block. */
category: string;
/** Bracketed tags from `**D-NN [tag1, tag2]:**`. Lower-cased. */
tags: string[];
/**
* False when under "Claude's Discretion" or tagged `informational` /
* `folded`. Trackable decisions are subject to the coverage gates.
*/
trackable: boolean;
}
const DISCRETION_HEADINGS = new Set([
"claude's discretion",
'claudes discretion',
'claude discretion',
]);
const NON_TRACKABLE_TAGS = new Set(['informational', 'folded', 'deferred']);
/**
* Strip fenced code blocks from `content` so example `<decisions>` snippets
* inside ```` ``` ```` do not pollute the parser (review F11).
*/
function stripFencedCode(content: string): string {
return content.replace(/```[\s\S]*?```/g, ' ').replace(/~~~[\s\S]*?~~~/g, ' ');
}
/**
* Extract the inner text of EVERY `<decisions>...</decisions>` block in
* order, concatenated by `\n\n`. Returns null when no block is present.
*
* CONTEXT.md may legitimately contain more than one block (for example, a
* "current decisions" block plus a "carry-over from prior phase" block);
* dropping all-but-the-first silently lost the second batch (review F13).
*/
function extractDecisionsBlock(content: string): string | null {
const cleaned = stripFencedCode(content);
const matches = [...cleaned.matchAll(/<decisions>([\s\S]*?)<\/decisions>/g)];
if (matches.length === 0) return null;
return matches.map((m) => m[1]).join('\n\n');
}
/**
* Parse trackable decisions from CONTEXT.md content.
*
* Returns ALL D-NN decisions found inside `<decisions>` (including
* non-trackable ones, with `trackable: false`). Callers that only want the
* gate-enforced decisions should filter `.filter(d => d.trackable)`.
*/
export function parseDecisions(content: string): ParsedDecision[] {
if (!content || typeof content !== 'string') return [];
const block = extractDecisionsBlock(content);
if (block === null) return [];
const lines = block.split(/\r?\n/);
const out: ParsedDecision[] = [];
let category = '';
let inDiscretion = false;
// Bullet line: `- **D-NN[ [tags]]:** text`
const bulletRe = /^\s*-\s+\*\*D-(\d+)(?:\s*\[([^\]]+)\])?\s*:\*\*\s*(.*)$/;
let current: ParsedDecision | null = null;
const flush = () => {
if (current) {
current.text = current.text.trim();
out.push(current);
current = null;
}
};
for (const line of lines) {
const trimmed = line.trim();
// Track category headings (`### Heading`)
const headingMatch = trimmed.match(/^###\s+(.+?)\s*$/);
if (headingMatch) {
flush();
category = headingMatch[1];
// Strip the full unicode-quote family so any rendering of "Claude's
// Discretion" (ASCII apostrophe, curly U+2019, U+2018, U+201A, U+201B,
// double-quote variants U+201C/D/E/F, etc.) collapses to the same key
// (review F20).
const normalized = category
.toLowerCase()
.replace(/[\u2018\u2019\u201A\u201B\u201C\u201D\u201E\u201F'"`]/g, '')
.trim();
inDiscretion = DISCRETION_HEADINGS.has(normalized);
continue;
}
const bulletMatch = line.match(bulletRe);
if (bulletMatch) {
flush();
const id = `D-${bulletMatch[1]}`;
const tags = bulletMatch[2]
? bulletMatch[2]
.split(',')
.map((t) => t.trim().toLowerCase())
.filter(Boolean)
: [];
const trackable =
!inDiscretion && !tags.some((t) => NON_TRACKABLE_TAGS.has(t));
current = { id, text: bulletMatch[3], category, tags, trackable };
continue;
}
// Continuation line for current decision (indented with space OR tab,
// non-bullet, non-empty) — tab indentation must work too (review F12).
if (current && trimmed !== '' && !trimmed.startsWith('-') && /^[ \t]/.test(line)) {
current.text += ' ' + trimmed;
continue;
}
// Blank line or unrelated content terminates the current decision
if (trimmed === '') {
flush();
}
}
flush();
return out;
}
// ─── Query handler ────────────────────────────────────────────────────────
/**
* `decisions.parse <path>` — parse CONTEXT.md and return decisions array.
*
* Used by workflow shell snippets that need to enumerate decisions without
* spawning a full Node process. Accepts either an absolute path or a path
* relative to `projectDir` — symmetric with the gate handlers (review F14).
*/
export const decisionsParse: QueryHandler = async (args, projectDir) => {
const filePath = args[0];
if (!filePath) {
return { data: { decisions: [], trackable: 0, total: 0, missing: true } };
}
const resolved = isAbsolute(filePath) ? filePath : join(projectDir, filePath);
let raw = '';
try {
raw = await readFile(resolved, 'utf-8');
} catch {
return { data: { decisions: [], trackable: 0, total: 0, missing: true } };
}
const decisions = parseDecisions(raw);
const trackable = decisions.filter((d) => d.trackable);
return {
data: {
decisions,
trackable: trackable.length,
total: decisions.length,
missing: false,
},
};
};