golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#4130): --context flag for check decision-coverage-plan + parseDecisions quadratic-backtracking hardening (#4374)

Browse Source 
* test(#4130): failing-first regressions for --context flag + parseDecisions hardening

Block A (flag): check decision-coverage-plan --context <path> must route
identically to the positional form; flag wins over positional context;
valueless --context falls through to the #2770 fail-closed caller error;
verify keeps its positional surface (flag is plan-only). RED on base:
the flag token lands in the args[2] phase slot (false uncovered) or the
args[3] context slot (silent CONTEXT.md-missing skip).

Block B (hardening): regex-lattice asserts pin the atomic-ID wrapper
(?=(X))\1 and the em-dash first-separator narrowing [^*—–]*[—–] plus the
no-adjacent-overlap property; a differential property compares the module
against a frozen copy of the pre-hardening grammars (reference validated
against the base build: 60k generated lines, 0 mismatches); 40k cliff
shapes assert correct outcomes with no wall-time asserts (repo rule).

A12: partitionPredicateArgs keeps one parser behind parsePredicateFlags.

* fix(#4130): --context flag for check decision-coverage-plan + quadratic-backtracking hardening in parseDecisions

(A) check decision-coverage-plan --context <path> — sibling convention
(check predicate, #2008): --flag value pairs parsed by the new shared
partitionPredicateArgs (parsePredicateFlags reimplemented as its flags
half — one parser, cannot diverge), the flag winning over a same-purpose
positional, positionals kept (no sibling deprecates them; the plan-phase
workflow caller passes positionals), valueless --context falls through
to the #2770 fail-closed caller error. Repair of the routing accident
where --context landed in the args[2] phase slot (false uncovered) or
the literal token in the args[3] context slot (silent green skip).

(B) parseDecisions regex seam hardened, byte-identical on all legal
inputs: the three bullet grammars consume the ID atomically via the
(?=(X))\1 lookahead emulation (kills the tail/[^:*]* O(n^2) re-split,
~1.1s @ 40k), and the em-dash first separator narrows [^*]*[—–] to
[^*—–]*[—–] (kills the dash-position O(n^2) retry, ~1.7s @ 40k). Group
indices unchanged (handlers untouched). Pinned by regex-lattice tests,
a differential fast-check property vs the frozen pre-hardening grammars,
and 40k cliff/legal-shape outcome tests (no wall-time asserts per repo
rule — no deterministic engine step counter exists in Node).

* docs+test(#4130): document --context invocation; harden lattice test tooling

- docs/CONFIGURATION.md Decision Coverage Gates: new 'Invoking the plan
  gate directly' block documenting both the positional and --context
  forms, flag precedence, and the valueless-flag fail-closed semantics
  (same place the gate's behavior is documented; sibling check predicate
  documents its flags the same way).
- Two changeset fragments per the maintainer brief (Added: flag; Fixed:
  hardening), PR numbers to be backfilled.
- tests/decisions.test.cjs review fixes: readRegExpTemplate template
  escaping (bare ')' SyntaxError), range-aware lattice checker with
  backreference skip and template unescape, honest A1 contract, lint
  escape warning.

* fix(#4130): valueless --context fails closed per #2770; A8 isolates flag-vs-positional context

Suite-caught fixes from the first verify run:
- cmdDecisionCoveragePlan now refuses a flag-shaped token as the
  positional context path: a bare valueless --context stays a positional
  (sibling parser semantics, unchanged) but reading it as a PATH would
  turn a caller mistake into a silent 'CONTEXT.md missing' green skip —
  exactly what #2770's fail-closed law forbids. Now falls through to
  the missing-context-argument error, as documented.
- A8 test compares decoy-positional+flag against flag-with-phase (phase
  held constant) so the row isolates WHICH context was read; the old
  form compared against a no-phase invocation that could never match.

* chore(#4130): backfill PR number in changeset fragments (PR #4374)

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-06 02:55:17 -04:00
committed by GitHub
parent 6adf3098ac
commit 7bb366e836
7 changed files with 708 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
src/check-command-router.cts
View File

@@ -289,9 +289,34 @@ function loadDecisionExtraction(contextPath: string): { trackable: Decision[]; o
};
}
/**
* `check decision-coverage-plan` — blocking plan-phase decision-coverage gate
* (#2492, #1365 fail-loud, #2770 empty-arg fail-closed).
*
* Invocation (the context path may be supplied EITHER way; #4130 follow-up):
* gsd_run check decision-coverage-plan <phase-dir> <context-path> (positional, the workflow caller's form)
* gsd_run check decision-coverage-plan --context <path> [<phase-dir>]
*
* `--context <path>` follows the sibling flag convention (`check predicate`,
* #2008): `--flag value` pairs parsed by the shared partitionPredicateArgs
* pass, the flag WINNING over a same-purpose positional when both appear,
* and a valueless `--context` counting as no context at all (it falls
* through to the #2770 caller-error branch, not to the "CONTEXT.md missing"
* green skip). The positional form keeps working unchanged — no sibling
* check verb deprecates positionals and the plan-phase workflow passes them.
*/
function cmdDecisionCoveragePlan(projectDir: string, args: string[], raw: boolean): void {
const phaseDir = args[2] ? resolvePath(args[2], projectDir) : '';
const contextArg = args[3];
// args[0]='check', args[1]=subcommand — partition the REST so flag tokens
// and their values never land in a positional slot.
const { flags, positionals } = partitionPredicateArgs(args.slice(2));
const phaseDir = positionals[0] ? resolvePath(positionals[0], projectDir) : '';
// A VALUELESS `--context` stays a bare token in the positionals (sibling
// parser semantics); it must not then be read as the context PATH — a
// `--`-prefixed "path" is a caller mistake, and #2770's law says a missing
// context argument fails CLOSED, never a silent "CONTEXT.md missing" green
// skip. So only a non-flag positional may serve as the context.
const positionalContext = positionals[1] && !positionals[1].startsWith('--') ? positionals[1] : '';
const contextArg = flags['context'] ?? positionalContext ?? '';
const contextPath = contextArg ? resolvePath(contextArg, projectDir) : '';
if (!gateEnabled(projectDir)) {
@@ -1268,21 +1293,41 @@ function buildPredicateDeps() {
};
}
/** Parse `--flag value` pairs from an args array into a map (last write wins). */
function parsePredicateFlags(args: string[]): Record<string, string> {
const out: Record<string, string> = {};
/**
* Split an args array into `--flag value` pairs and the leftover positional
* tokens, in ONE pass, with the semantics `check predicate` established
* (#2008): a `--flag` followed by a non-`--` token consumes it as the value
* (last write wins); a `--flag` with no value stays a bare token and moves to
* the positionals; everything else is positional. `parsePredicateFlags` is
* the flags half of this same pass — there is exactly one parser, so the
* flag-taking check verbs cannot drift apart (#4130 follow-up: `check
* decision-coverage-plan --context <path>` shares it).
*/
function partitionPredicateArgs(args: string[]): { flags: Record<string, string>; positionals: string[] } {
const flags: Record<string, string> = {};
const positionals: string[] = [];
for (let i = 0; i < args.length; i++) {
const a = args[i];
if (typeof a !== 'string') continue;
if (!a.startsWith('--')) continue;
if (!a.startsWith('--')) {
positionals.push(a);
continue;
}
const key = a.slice(2);
const next = args[i + 1];
if (key.length > 0 && typeof next === 'string' && !next.startsWith('--')) {
out[key] = next;
flags[key] = next;
i++;
} else {
positionals.push(a);
}
}
return out;
return { flags, positionals };
}
/** Parse `--flag value` pairs from an args array into a map (last write wins). */
function parsePredicateFlags(args: string[]): Record<string, string> {
return partitionPredicateArgs(args).flags;
}
/**
@@ -1821,7 +1866,9 @@ function routeCheckCommand({ args, cwd, raw }: RouteCheckCommandOptions): void {
// this for any gate whose `check` carries a `predicate` (instead of a `query`),
// passing the predicate object as --predicate '<json>'. NOTE: unlike the
// `check.query` subcommands above (which take positional phase args), this
// subcommand parses --flag value pairs.
// subcommand is flag-driven. `decision-coverage-plan` above now ALSO accepts
// `--context <path>` (its positionals still work) — both share
// partitionPredicateArgs, the one flag parser.
cmdCheckPredicate(cwd, args, raw);
return;
}
@@ -1850,6 +1897,7 @@ export = {
cmdCheckPredicate,
buildPredicateDeps,
parsePredicateFlags,
partitionPredicateArgs,
// Fail-closed phase-scope reader for the api-coverage gate — exported for
// in-process failure-injection tests (#2365 review).
readPhaseScope,

51
src/decisions.cts
View File

@@ -17,6 +17,11 @@
* - Outer bullet loop → seam's `iterateBullets` (for the header-fallback path)
*
* Resolves #1364 (markdown-header + em-dash recall) and #1365 (fail-loud gate).
*
* #4130 follow-up (hardening): the three bullet grammars below consume the
* decision ID atomically and narrow the em-dash first separator, eliminating
* the quadratic-backtracking cliff on pathological single bullets. Output is
* byte-identical on all legal inputs — see the notes at DECISION_ID_SOURCE.
*/
import {
@@ -74,6 +79,28 @@ const NON_TRACKABLE_TAGS = new Set(['informational', 'folded', 'deferred']);
*/
const DECISION_ID_SOURCE = 'D[0-9]*-[A-Za-z0-9][A-Za-z0-9_-]*';
/**
* #4130 follow-up (hardening): how the three grammars below CONSUME the ID —
* atomically, via the `(?=(X))\1` lookahead emulation (lookarounds are atomic
* in ECMAScript; the backreference must replay exactly what the lookahead
* captured, so the engine can never give the ID tail back one character at a
* time). That give-back was quadratic driver #1: the tail class
* `[A-Za-z0-9_-]*` overlaps the pre-separator class `[^:*]*` (every id char
* is also `[^:*]`), so on a FAILING bullet the base regex re-split the tail
* O(n) times with an O(n) scan after each — measured ~1.1s @ 40k chars on
* `- **D-` + `a-`×20k (the #4357 review's deferred cliff).
*
* Byte-identical on all legal inputs: a successful match always consumes the
* MAXIMAL id run (the lookahead's own match is exactly that maximal run), and
* the continuation's success depends only on the position of the first
* `:`/`*` (or `*` for the em-dash form) after the id boundary — id chars
* contain neither, so moving the boundary inside the run cannot change
* success or any capture. Group 1 stays the full id (the lookahead's capture
* IS group 1), so handlers keep reading match[1]/[2]/[3] untouched. Pinned by
* the differential property test against a frozen copy of the pre-hardening
* grammars and by the regex-lattice test in tests/decisions.test.cjs.
*/
/**
* #4130: the bold lead-in that ATTEMPTS the ID grammar above — used by the
* parse-miss guard and the #3939 join regexes, where recognising MORE shapes
@@ -90,9 +117,13 @@ const ID_ATTEMPT_SOURCE = 'D(?:[0-9][A-Za-z0-9]*)?-';
* Colon form: `- **D[phase]-NN[ [tags]]:** text`
* (#1343: `[^:*]*` subsumes any pre-colon prose, stops at `:**`)
* Group 1 captures the FULL id including any phase prefix (#4130).
* The ID is consumed atomically `(?=(…))\1` — see the hardening note above
* the constants (#4130 follow-up); with the tail unable to give back, the
* remaining `[^:*]*:` scan has a single viable split and the whole match is
* linear in line length.
*/
const bulletColonRe = new RegExp(
`^\\s*-\\s+\\*\\*(${DECISION_ID_SOURCE})(?:\\s*\\[([^\\]]+)\\])?[^:*]*:\\*\\*\\s*(.*)$`,
`^\\s*-\\s+\\*\\*(?=(${DECISION_ID_SOURCE}))\\1(?:\\s*\\[([^\\]]+)\\])?[^:*]*:\\*\\*\\s*(.*)$`,
);
/**
@@ -102,9 +133,20 @@ const bulletColonRe = new RegExp(
* outside the closing `**`. This form was not handled pre-T1 (bug #1364).
*
* Accepts both U+2014 em-dash (—) and U+2013 en-dash (–) for robustness.
*
* #4130 follow-up (hardening), quadratic driver #2: the first separator was
* `[^*]*[—–]`, whose leading class ALSO accepts the dash — on a failing
* dash-laden title the engine retried the separator at every dash position
* with an O(n) scan after each (~1.7s @ 40k). Narrowed to `[^*—–]*[—–]`:
* the leading class now excludes the dash, so the separator is the FIRST
* dash — one viable split, single pass. Behavior-preserving because every
* candidate dash lies before the first `*` (the leading class cannot cross
* a star), so the trailing `[^*]*` reaches that same first star from any
* candidate and `**` succeeds or fails identically; no capture involves the
* dash position. The ID is atomic like the other forms (driver #1).
*/
const bulletEmDashRe = new RegExp(
`^\\s*-\\s+\\*\\*(${DECISION_ID_SOURCE})(?:\\s*\\[([^\\]]+)\\])?[^*]*[—–][^*]*\\*\\*\\s*(.*)$`,
`^\\s*-\\s+\\*\\*(?=(${DECISION_ID_SOURCE}))\\1(?:\\s*\\[([^\\]]+)\\])?[^*—–]*[—–][^*]*\\*\\*\\s*(.*)$`,
);
/**
@@ -117,9 +159,12 @@ const bulletEmDashRe = new RegExp(
* (e.g. `D-07 ratio 3:1:**`) still fails the anchor and falls through to the parse-miss
* guard — matching bulletColonRe's `[^:*]*` discipline that the separator colon is the
* only colon permitted before `**`. (#1639)
*
* The ID is consumed atomically `(?=(…))\1` like the other forms — the
* hardening note above the constants explains why (#4130 follow-up).
*/
const bulletTitledColonRe = new RegExp(
`^\\s*-\\s+\\*\\*(${DECISION_ID_SOURCE})(?:\\s*\\[([^\\]]+)\\])?[^:*]*:[^:*]*\\*\\*\\s*(.*)$`,
`^\\s*-\\s+\\*\\*(?=(${DECISION_ID_SOURCE}))\\1(?:\\s*\\[([^\\]]+)\\])?[^:*]*:[^:*]*\\*\\*\\s*(.*)$`,
);
/**