diff --git a/.changeset/bold-goats-wave.md b/.changeset/bold-goats-wave.md new file mode 100644 index 000000000..42c21584f --- /dev/null +++ b/.changeset/bold-goats-wave.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2397 +--- +**api-coverage detector no longer false-positives non-API phases (and no longer fails open)** — the external-API-integration detector behind the blocking `verify:pre` seal gate required only same-line co-occurrence of an integration verb and an API noun, treated `/` as a word boundary (so first-party Next.js `src/app/api/…` route paths matched), and read any capitalized word before API/SDK/REST/GraphQL as a service name (so threat-model prose like "Resolver-only API" fired). It is now **fail-closed**: the compound rule requires the integration verb and API noun to share one clause (the clause boundary is the whole relationship test — no fragile word-gap cap that a genuine long integration clause would trip); fenced code, inline code spans, and path-shaped tokens are excluded before matching while external hosts like `api.stripe.com/v1` still count; and the ` API` surface rule rejects stopwords, locality/protocol descriptors ("Internal API", "REST API"), compound modifiers, and first-party-qualified services, so a real vendor name (`Stripe API`) fires from any clause position. A phase that integrates no external API can declare it first-class in `COVERAGE.md` — `No external API integration: ` — instead of fabricating a matrix row; when the detector still finds signals, the declaration overrides but the gate surfaces the overridden signals so the contradiction is visible. Because a false positive is cheaply dismissed by that declaration while a false negative silently slips a real API phase past the gate, the detector deliberately leans toward detecting. (#2365) diff --git a/CONTEXT.md b/CONTEXT.md index 9933d3381..6d3be56d8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -146,7 +146,7 @@ Primary installer for all runtimes. Single production file: `bin/install.js` (ge Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`). ### Markdown Sectionizer -Canonical markdown-structure parsing seam (`gsd-core/bin/lib/markdown-sectionizer.cjs`, generated from `src/markdown-sectionizer.cts`). Pure functions, Node built-ins only. Exports: `stripFencedCode(content) → { text, unterminatedFence }` (CommonMark-correct state machine, CRLF-safe, signals unterminated fences); `tokenizeHeadings(content) → HeadingToken[]` (ATX headings outside fenced blocks, `{ level, text, line, offset }`); `collectSections(content, stopPredicate) → Section[]` (line-by-line section collection driven by a heading predicate); `collectSection(content, headingPredicate, { levelBounded, stripFences }) → Section | null` (single named section with level-bounded stop); `iterateBullets(sectionText) → BulletItem[]` (dash/checkbox/numbered markers with indented continuation); `extractTaggedBlocks(content, tagName) → string[]` (inner text of every `…` block in document order, tagName regex-escaped, caller decides fence-stripping — generalises `decisions.cts`'s bespoke extractor for T1); `replaceSection(content, section, newBody) → string` (pure character-offset splice using `Section.bodyStart`/`bodyEnd` for read-modify-write callers — eliminates T6 `state.cts`'s 7× inline `content.replace` pattern); `withSection(content, target, edit) → string` (resolve the section whose heading matches `target` — exact heading text or a `HeadingToken` predicate — and run `edit(body)` against ONLY that section's body before splicing the result back; bounded no-op when no heading matches or `edit` returns the same/non-string body; ADR-2143 §4 structurally retires the #2130/#2067/#2080 boundary-crossing class by confining any regex the caller runs to the matched section). `Section` carries `bodyStart`/`bodyEnd` offsets for `replaceSection`. ADR-1372 (epic #1372) establishes this seam and a tiered migration plan (T0–T7) to retire the 8+ ad-hoc markdown parsers and ~20 inline section-collects across `src/*.cts`. New `src/*.cts` modules must import this seam instead of hand-rolling fence strippers or heading-regex section walks (enforced by the `no-adhoc-markdown-parsing` ESLint rule landing in tier T7). +Canonical markdown-structure parsing seam (`gsd-core/bin/lib/markdown-sectionizer.cjs`, generated from `src/markdown-sectionizer.cts`). Pure functions, Node built-ins only. Exports: `stripFencedCode(content) → { text, unterminatedFence }` (CommonMark-correct state machine, CRLF-safe, signals unterminated fences); `stripInlineCode(content) → string` (per-line CommonMark inline-code-span stripper — removes `` `code` `` spans while leaving fenced blocks to `stripFencedCode`; #2365); `scanInlineCodeSpans(content) → InlineCodeSpan[]` (locates every inline code span as `{ start, end, content }`, offsets into the full string and spans never crossing a `\n`; callers that need the span CONTENT (e.g. api-coverage's package-name evidence, #2365) use this, callers that just want spans gone use `stripInlineCode`); `tokenizeHeadings(content) → HeadingToken[]` (ATX headings outside fenced blocks, `{ level, text, line, offset }`); `collectSections(content, stopPredicate) → Section[]` (line-by-line section collection driven by a heading predicate); `collectSection(content, headingPredicate, { levelBounded, stripFences }) → Section | null` (single named section with level-bounded stop); `iterateBullets(sectionText) → BulletItem[]` (dash/checkbox/numbered markers with indented continuation); `extractTaggedBlocks(content, tagName) → string[]` (inner text of every `…` block in document order, tagName regex-escaped, caller decides fence-stripping — generalises `decisions.cts`'s bespoke extractor for T1); `replaceSection(content, section, newBody) → string` (pure character-offset splice using `Section.bodyStart`/`bodyEnd` for read-modify-write callers — eliminates T6 `state.cts`'s 7× inline `content.replace` pattern); `withSection(content, target, edit) → string` (resolve the section whose heading matches `target` — exact heading text or a `HeadingToken` predicate — and run `edit(body)` against ONLY that section's body before splicing the result back; bounded no-op when no heading matches or `edit` returns the same/non-string body; ADR-2143 §4 structurally retires the #2130/#2067/#2080 boundary-crossing class by confining any regex the caller runs to the matched section). `Section` carries `bodyStart`/`bodyEnd` offsets for `replaceSection`. ADR-1372 (epic #1372) establishes this seam and a tiered migration plan (T0–T7) to retire the 8+ ad-hoc markdown parsers and ~20 inline section-collects across `src/*.cts`. New `src/*.cts` modules must import this seam instead of hand-rolling fence strippers or heading-regex section walks (enforced by the `no-adhoc-markdown-parsing` ESLint rule landing in tier T7). ### Markdown Table Model Canonical GFM table parsing + schema registry seam (`gsd-core/bin/lib/markdown-table.cjs`, generated from `src/markdown-table.cts`; ADR-2143, epic #2143). Pure functions, Node built-ins only, string-in/value-out, no I/O. Exports: `parseMarkdownTable(sectionText) → Result` (parses the first GFM pipe table found; typed `{ok:false,reason}` parse errors for no-table, missing/misaligned delimiter row, and ragged data rows — never silently drops or coerces a malformed row); `MarkdownTable` (`{columns: string[], rows: Record[]}`, rows addressed by column name, not position); `Result` (`{ok:true,value}\|{ok:false,reason}` — re-exported from the Write-Set Module, the ADR-2143 §5 single source of truth for this shape, so existing importers of `Result` from `markdown-table.cjs` are unaffected; deliberately distinct from command-routing-hub's dispatch `Result` `{ok,data\|kind}`; the two never mix); `TABLE_SCHEMAS` (`Record` — the canonical column-header variants for every GFM table GSD parses or generates: `RoadmapProgress` flat/milestone-grouped, `RequirementsTraceability`, `QuickTasks` no-status/with-status, `Security` trust-boundaries/threat-register/accepted-risks/audit-trail); `matchTableSchema(columns) → {id,label}\|null` (resolves a parsed header back to its canonical schema by exact column-name/order match). This registry is the single source of truth for ROADMAP/STATE/SECURITY canonical tables — a parity test (`tests/markdown-table.test.cjs`) asserts every variant's header appears verbatim in the template/workflow file that generates it, so the registry and templates can never silently drift (ADR-2143 §3 Generative-Fix-Divergence guard). `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` is the first consumer: it locates the Progress section via the Markdown Sectionizer's `collectSection` and reads cells by column NAME through this seam, fixing #2137 (the prior position-anchored regex assumed `Status` was always the 3rd cell, which broke for the 5-column milestone-grouped `Milestone` variant). diff --git a/capabilities/ai-integration/fragments/api-coverage-plan-pre.md b/capabilities/ai-integration/fragments/api-coverage-plan-pre.md index ff280890a..d0a88af7b 100644 --- a/capabilities/ai-integration/fragments/api-coverage-plan-pre.md +++ b/capabilities/ai-integration/fragments/api-coverage-plan-pre.md @@ -34,6 +34,19 @@ the checkpoint entirely and continue planning. Do not raise it with the user. **If `detected` is `true`:** an external-API integration is in scope. You MUST produce a **coverage matrix** before the plan is finalized. +**If `detected` is `true` but the phase genuinely integrates no external API** +(the detector is deterministic, not infallible — confirm by re-reading the phase +scope, not by preference): do NOT fabricate a matrix row for a capability that +does not exist. Write a reasoned declaration to `${PHASE_DIR}/COVERAGE.md` +instead: + +```markdown +No external API integration: . +``` + +The reason is required, exactly like an `OPT-OUT` reason. The seal-time gate +accepts this declaration in place of a matrix. + ## Produce the coverage matrix Enumerate the external API's full **capability surface** — the verb/endpoint/method @@ -81,7 +94,8 @@ This checkpoint is enforced. At `verify:pre` the `api-coverage.verify-pre` gate runs `check api-coverage.verify-pre `: - If `COVERAGE.md` exists, it is validated — every row needs a valid decision and - every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**. + every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**. A + reasoned `No external API integration: …` declaration (and no rows) passes. - If `COVERAGE.md` is absent, the detector runs again over the phase scope. If a strong external-API-integration signal is found, the seal is **blocked** until a matrix is produced. If no signal is found, the phase is treated as a non-API diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index d6d2990cb..0fedbe85f 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -408,7 +408,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `active-workstream-store.cjs` | Workstream source precedence and selection (CLI `--ws` > `GSD_WORKSTREAM` env > stored pointer); name validation and environment propagation | | `adr-parser.cjs` | ADR decision parser for plan-phase ingest express path; normalizes section synonyms, parses status/decision/scope fences, and enforces status rejection gates | | `agent-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools agent` | -| `api-coverage.cjs` | API-coverage detector + matrix validator (#1562) — pure `detectApiIntegration` (compound verb+noun signal + ` API/SDK` surface; strips fenced code) and `validateCoverageMatrix`/`parseCoverageMatrix`/`renderCoverageMatrix` for the COVERAGE.md artifact; STDIN CLI (`echo "$SCOPE" \| node .../api-coverage.cjs [--json]`, exit 0=detected/1=none/2=error); consumed by the `ai-integration` capability's `plan:pre` contribution and blocking `verify:pre` gate (`check api-coverage.verify-pre`) | +| `api-coverage.cjs` | API-coverage detector + matrix validator (#1562, #2365) — pure `detectApiIntegration` (fail-closed: same-clause verb+noun signal + ` API/SDK` surface naming a real service; strips fenced code, inline code, and path-shaped tokens; external hosts count, first-party route paths do not) and `validateCoverageMatrix`/`parseCoverageMatrix`/`renderCoverageMatrix` for the COVERAGE.md artifact (incl. the `No external API integration: ` declaration); STDIN CLI (`echo "$SCOPE" \| node .../api-coverage.cjs [--json]`, exit 0=detected/1=none/2=error); consumed by the `ai-integration` capability's `plan:pre` contribution and blocking `verify:pre` gate (`check api-coverage.verify-pre`) | | `artifacts.cjs` | Canonical artifact registry — known `.planning/` root file names; used by `gsd-health` W019 lint | | `audit-command-router.cjs` | ADR-959 capability command router for `gsd-tools audit-uat` and `gsd-tools audit-open` — extracted from hardcoded cases in `gsd-tools.cjs`; dispatches to `uat.cjs:cmdAuditUat` and `audit.cjs:{auditOpenArtifacts,formatAuditReport}`; phase 4d-impl-3 | | `audit.cjs` | Audit dispatch, audit open sessions, audit storage helpers | @@ -475,7 +475,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `legacy-cleanup.cjs` | Detect and remove leftover get-shit-done-cc artifacts; exports `planLegacyCleanup` (pure scan) and `applyLegacyCleanup` (thin IO applier) that root out stale files from the old package across every GSD-managed runtime config directory (#607) | | `loop-host-contract.cjs` | Generated Loop Host Contract — 12 loop points, per-step agent roles, and core artifacts for the five-step pipeline (discuss/plan/execute/verify/ship); emitted by `scripts/gen-loop-host-contract.cjs --write` (ADR-894 §3); consumed by `gen-capability-registry.cjs` | | `loop-resolver.cjs` | Loop Extension Point resolver — ADR-857 phase 3c/6 registry-consuming query; given a canonical loop point, filters `byLoopPoint` by resolved Capability State plus config activation (`when` key traversal with prototype-pollution guard), returns `{ point, activeHooks, rendered }` envelope; `resolveLoopHooks` and `renderLoopHooks` are pure (no I/O); command surface: `gsd-tools loop render-hooks [--config-dir ]` | -| `markdown-sectionizer.cjs` | Canonical markdown-structure parsing seam (ADR-1372, epic #1372) — pure, Node built-ins only; exports `stripFencedCode` (CommonMark-correct fence stripper, CRLF-safe), `tokenizeHeadings` (ATX headings outside fenced blocks), `collectSections`/`collectSection` (line-by-line section collection with `bodyStart`/`bodyEnd` offsets), `iterateBullets` (dash/checkbox/numbered markers), `extractTaggedBlocks` (inner text of `…` blocks, caller decides fence-stripping), `replaceSection` (pure character-offset body splice for read-modify-write callers), and `withSection` (resolve a section by heading/predicate and run an edit callback against ONLY its body, splicing the result back — ADR-2143 §4 bounded mutation); foundation for T0–T7 migration tiers retiring 8+ ad-hoc parsers | +| `markdown-sectionizer.cjs` | Canonical markdown-structure parsing seam (ADR-1372, epic #1372) — pure, Node built-ins only; exports `stripFencedCode` (CommonMark-correct fence stripper, CRLF-safe), `stripInlineCode` (per-line CommonMark inline-code-span stripper, #2365), `tokenizeHeadings` (ATX headings outside fenced blocks), `collectSections`/`collectSection` (line-by-line section collection with `bodyStart`/`bodyEnd` offsets), `iterateBullets` (dash/checkbox/numbered markers), `extractTaggedBlocks` (inner text of `…` blocks, caller decides fence-stripping), `replaceSection` (pure character-offset body splice for read-modify-write callers), and `withSection` (resolve a section by heading/predicate and run an edit callback against ONLY its body, splicing the result back — ADR-2143 §4 bounded mutation); foundation for T0–T7 migration tiers retiring 8+ ad-hoc parsers | | `markdown-table.cjs` | Canonical GFM table model + `TABLE_SCHEMAS` registry seam (ADR-2143, epic #2143) — pure, Node built-ins only; exports `parseMarkdownTable(sectionText) → Result` (parses the first GFM pipe table, typed parse errors for ragged/malformed rows rather than silent coercion), `MarkdownTable` (`{columns, rows}`, rows addressed by column name), `Result` (`{ok:true,value}\|{ok:false,reason}` — distinct from command-routing-hub's dispatch `Result`), `TABLE_SCHEMAS` (canonical column-header variants for `RoadmapProgress`/`RequirementsTraceability`/`QuickTasks`/`Security` tables), and `matchTableSchema(columns) → {id,label}\|null` (resolves parsed headers back to a canonical schema); consumed by `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` (fixes #2137, the 5-column milestone-grouped Progress table) | | `milestone.cjs` | Milestone archival, requirements marking | | `model-catalog.cjs` | CJS adapter over the shared model catalog JSON; exports canonical runtime tier defaults, agent profile maps, alias maps, and routing metadata for all CLI consumers | diff --git a/gsd-core/bin/lib/api-coverage.cjs b/gsd-core/bin/lib/api-coverage.cjs index 3ee12dd74..e7e2aed6b 100644 --- a/gsd-core/bin/lib/api-coverage.cjs +++ b/gsd-core/bin/lib/api-coverage.cjs @@ -17,11 +17,20 @@ * (acceptance #2) are testable. Mirrors assumption-delta.cts (#1561). * - COMPOUND SIGNAL for low false positives. A bare word like "api" appears in * countless non-integration phases ("the public API of UserController"). The - * detector requires an INTEGRATION VERB co-occurring with an EXTERNAL-API - * NOUN (or an explicit " API/SDK" phrase). Single weak tokens do not - * fire. This is the issue's "low false-positive trigger" made mechanical. - * - FENCED CODE BLOCKS ARE STRIPPED first (markdown-sectionizer seam) so a - * trigger term inside a code snippet does not fire. + * detector requires an INTEGRATION VERB and an EXTERNAL-API NOUN in the SAME + * CLAUSE (#2365 — same-line co-occurrence across unrelated clauses over-fired; + * the clause boundary, not a word-gap cap, is the relationship test), or an + * explicit " API/SDK" phrase naming a real service. Single weak + * tokens do not fire. This is the issue's "low false-positive trigger" made + * mechanical. + * - CODE AND PATHS ARE NOT PROSE. Fenced code blocks and inline code spans are + * stripped first (markdown-sectionizer seam), and path-shaped tokens + * (`src/app/api/...`, URLs) are masked, so a trigger term inside code or a + * first-party route path does not fire (#2365). + * - NO-INTEGRATION DECLARATION (#2365 acceptance #5). A COVERAGE.md consisting + * of `No external API integration: ` is a valid, reasoned way for a + * phase to state that no external surface exists — the alternative to + * fabricating a matrix row when the detector is overruled by a human. * - THE DETECTOR IS A FALLBACK. The primary path is the plan:pre contribution * prompting COVERAGE.md creation. The detector runs only when COVERAGE.md is * ABSENT, to catch the "nobody decided" case (acceptance #1). Its precision @@ -171,19 +180,185 @@ function makeSnippet(line, anchor) { * `[A-Z]\w+ API` shape. Those are common English, not a service name, so they * are rejected before counting as a surface signal (acceptance #4 — low false * positives). */ -const SERVICE_SURFACE_API_RE = /\b([A-Z][A-Za-z0-9_-]{1,})\s+(API|SDK|REST|GraphQL)\b/; +// Service-name length is bounded ({1,40}) so a hostile "A-A-A-…-A-x" run cannot +// drive the greedy group into O(n^2) backtracking (#2365 review). Nearly all +// vendor names fit; a >41-char service token before API/SDK would be missed by +// this surface path (it would still fire via the compound verb+noun rule) — +// an accepted bound. +const SERVICE_SURFACE_API_RE = /\b([A-Z][A-Za-z0-9_-]{1,40})\s+(API|SDK|REST|GraphQL)\b/; const SERVICE_STOPWORDS = new Set([ 'the', 'an', 'a', 'our', 'this', 'these', 'that', 'those', 'new', 'add', 'use', 'your', 'my', 'no', 'some', 'any', 'all', 'each', 'every', 'both', 'if', 'when', 'while', 'with', 'via', 'using', 'into', 'its', 'their', 'we', 'you', 'they', 'it', ]); +/** #2365 — the detector is FAIL-CLOSED: it leans toward detecting, because a + * false positive is cheaply dismissed by a one-line COVERAGE.md "no external + * API integration" declaration, whereas a false NEGATIVE silently lets a real + * external-API phase past a BLOCKING gate. So the only prose the detector + * actively suppresses is the classes that are unambiguously NOT external + * integration: first-party route paths, verb/noun in unrelated clauses, and + * descriptive/protocol " API" prose with no named service. + * + * CLAUSE_BOUNDARY_RE: a verb and a noun form ONE compound action only inside + * one grammatical clause — sentence punctuation and table-cell walls (`|`) + * end a clause. `-` is deliberately absent (it would split hyphenated words). + * There is deliberately NO word-gap cap inside a clause: a cap cannot separate + * a genuine long integration clause (F4, 21 words) from a long internal-UI + * clause (18 words) — the clause boundary is the only sound signal, and the + * declaration handles the residual false positives. */ +const CLAUSE_BOUNDARY_RE = /[,;:.!?|()—–]/; +/** Same character class as CLAUSE_BOUNDARY_RE, as a set — for scanning a token's + * trailing punctuation without an unanchored `[…]+$` regex, whose backtracking + * is O(n^2) on a long punctuation run (#2365 review). */ +const CLAUSE_BOUNDARY_CHARS = new Set([',', ';', ':', '.', '!', '?', '|', '(', ')', '—', '–']); +/* DELIBERATELY NO cross-clause binding. Detection is same-clause only. Binding + * a verb in one clause to a noun in another ("Integrate Stripe, exposing its + * endpoints"; "Integrate Stripe; use its endpoints") requires knowing "Stripe" + * is a vendor and "its" refers to it — a vendor dictionary + coreference, which + * trek-e's brief rules out in principle. Every lexical cross-clause rule tried + * (word-gap cap, participle continuation) traded a false negative for a false + * positive across four review rounds. So a service named ONLY in a clause + * separate from its API noun, with no explicit ` API` surface, is a + * DOCUMENTED fail-open limitation — cheaply covered by the COVERAGE.md + * declaration and rare in real phase prose, which says "integrate the X API". */ +/** In the ` API|SDK` surface position, these capture words are NOT a + * named third-party service: locality/scope descriptors ("Internal API", + * "Public API") and bare protocol names ("REST API", "GraphQL API"). A real + * vendor name (Stripe, Shopify) is none of these, so rejecting them costs no + * true positives while killing the descriptive-prose false positives (#2365 + * acceptance #3, review F8). */ +const SURFACE_DESCRIPTOR_WORDS = new Set([ + 'internal', 'external', 'public', 'private', 'local', 'in-house', 'first-party', + 'generic', 'shared', 'common', 'legacy', 'rest', 'restful', 'graphql', 'grpc', + 'soap', 'rpc', 'http', 'https', 'json', 'xml', +]); +/** Locality qualifiers that, when they immediately precede a ` API`, + * mark it as first-party ("internal Payments API") — negative evidence for an + * EXTERNAL-API surface signal. Only unambiguously-internal words: "external" + * is deliberately absent (an external API IS external). */ +const INTERNAL_DESCRIPTORS = new Set(['internal', 'in-house', 'local', 'first-party', 'private']); +/** A capitalized compound modifier ("Resolver-only", "Read-only", "E-commerce" + * — lowercase letter right after the hyphen) is an adjective phrase, not a + * service name. Real hyphenated services capitalize the second segment + * ("T-Mobile"). */ +const COMPOUND_MODIFIER_RE = /^[A-Z][A-Za-z0-9]*-[a-z]/; +const URL_TOKEN_RE = /^[([<"'`]*[a-z][a-z0-9+.-]*:\/\//i; +const LOCAL_URL_RE = /^[([<"'`]*[a-z][a-z0-9+.-]*:\/\/(?:localhost|127(?:\.\d{1,3}){1,3}|0\.0\.0\.0|\[::1\])(?=[:/?#]|$)/i; +/** A scheme-less token that STARTS with a dotted hostname whose final label is + * alphabetic ("api.stripe.com/v1") — a bare external API host. A first-party + * route path ("src/app/api/…") has no dotted head, and an IP host ("127.1/…") + * has a numeric final label, so neither matches (#2365 review F2). */ +const DOMAIN_HEAD_RE = /^[([<"'`]*(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z]{2,}(?=[:/?#]|$)/i; +/** Mask whitespace-delimited tokens with an interior `/` — file paths, framework + * routes (`src/app/api/...`), URLs. They are references, not integration prose + * (#2365 root cause 2: `/` counted as a word boundary, so first-party route + * paths matched the noun vocabulary). Two carve-outs keep genuine signals: + * - a slashed token whose segments are ALL noun-vocabulary words ("API/SDK", + * "REST/GraphQL") is prose shorthand, not a path — left unmasked; + * - a non-local URL is masked, but noun terms inside it are collected as + * compound-rule evidence (the old detector caught "connect to + * https://api.stripe.com" via the `api` segment; losing that would + * fail-open). */ +function scanLineTokens(line, nounRe, nounSet) { + const urlNouns = []; + let masked = ''; + const tokenRe = /\S+/g; + let last = 0; + let m; + while ((m = tokenRe.exec(line)) !== null) { + const rawTok = m[0]; + masked += line.slice(last, m.index); + last = m.index + rawTok.length; + // Peel trailing clause-boundary punctuation off the token and keep it + // LITERAL in `masked` — masking it away would erase a clause split and pair + // unrelated verb/noun across it (#2365 review F6: "…example.com, document…"). + // A backward char scan (not a `[…]+$` regex) keeps this linear. + let trailLen = 0; + while (trailLen < rawTok.length && CLAUSE_BOUNDARY_CHARS.has(rawTok[rawTok.length - 1 - trailLen])) { + trailLen++; + } + const trail = trailLen ? rawTok.slice(rawTok.length - trailLen) : ''; + const tok = trailLen ? rawTok.slice(0, rawTok.length - trailLen) : rawTok; + if (!/\S[\\/]\S/.test(tok)) { + masked += rawTok; + continue; + } + const segments = tok.split(/[\\/]/).map((s) => s.replace(/[^A-Za-z0-9]/g, '')); + if (segments.every((s) => s.length > 0 && (nounSet.has(s.toLowerCase()) || /^v\d+$/i.test(s))) && + segments.some((s) => nounSet.has(s.toLowerCase()))) { + masked += rawTok; // "API/SDK", "API/v2" — noun shorthand, not a path + continue; + } + // A scheme URL or a bare external hostname is an external dependency + // reference: mask it from prose but keep it as compound-rule evidence. A + // first-party route path has neither a scheme nor a dotted host, so it is + // masked WITHOUT contributing nouns (#2365 root cause 2). + // A non-local URL that NAMES an API vocabulary word ("api.stripe.com/v1") + // is external-dependency evidence, so its vocab nouns feed the compound + // rule. We deliberately do NOT treat every path-bearing URL as an endpoint: + // that fired on ordinary asset/link URLs ("…/theme.css", "…?next=/x") and + // recreated routine UI-phase false positives (#2365 review). A bare external + // host that names no vocabulary word ("graph.microsoft.com") and is not + // written as " API" is therefore a DOCUMENTED fail-open limitation. + const isSchemeUrl = URL_TOKEN_RE.test(tok) && !LOCAL_URL_RE.test(tok); + const isDomainUrl = !URL_TOKEN_RE.test(tok) && DOMAIN_HEAD_RE.test(tok); + if (nounRe && (isSchemeUrl || isDomainUrl)) { + for (const f of collectTermMatches(nounRe, tok)) { + urlNouns.push({ term: f.term, start: m.index, end: m.index + tok.length }); + } + } + masked += ' '.repeat(tok.length) + trail; + } + masked += line.slice(last); + return { masked, urlNouns }; +} +/** All term matches in a clause, with offsets. `re` must be global with the + * term in group 2 and a consumed leading boundary in group 1. */ +function collectTermMatches(re, clause) { + const out = []; + re.lastIndex = 0; + let m; + while ((m = re.exec(clause)) !== null) { + const start = m.index + (m[1] || '').length; + out.push({ term: (m[2] || '').toLowerCase(), start, end: start + (m[2] || '').length }); + if (m[0].length === 0) + re.lastIndex++; + } + return out; +} +/** Split a line into clause segments, keeping each segment's start offset so + * line-level spans (masked URL tokens) can be mapped into their clause. */ +function splitClauses(masked) { + const out = []; + let start = 0; + for (let i = 0; i <= masked.length; i++) { + if (i === masked.length || CLAUSE_BOUNDARY_RE.test(masked[i])) { + out.push({ text: masked.slice(start, i), start }); + start = i + 1; + } + } + return out; +} /** * Detect whether phase-scope prose describes integrating an external API/SDK. * - * Fires when EITHER: - * (a) a compound verb+noun signal co-occurs on the same line, OR - * (b) an explicit ` API|SDK|REST|GraphQL` surface appears. + * FAIL-CLOSED: it leans toward detecting, because a false positive is dismissed + * by a one-line COVERAGE.md declaration while a false negative silently slips a + * real external-API phase past a blocking gate. It fires when EITHER: + * (a) an integration VERB and an API NOUN share one CLAUSE ("integrate the + * Stripe API", "Connect … to api.stripe.com") — the clause boundary is the + * whole relationship test, so verb/noun in DIFFERENT clauses do not pair + * (#2365 acceptance #2). There is NO cross-clause binding: a service named + * only in a clause separate from its API noun is a documented limitation. + * (b) an explicit ` API|SDK|REST|GraphQL` surface names a service + * that is not a stopword, a locality/protocol descriptor, a compound + * modifier, or first-party-qualified ("Stripe API", "Spotify SDK"). + * + * Fenced code, inline code spans, and path-shaped tokens are excluded before + * matching. A package-shaped inline span (`@stripe/stripe-js`, `stripe-sdk`) + * and a URL that NAMES an API vocab word ("api.stripe.com/v1") still count as + * noun/dependency evidence; a bare host that names none does not. * * Non-string inputs degrade to `{ detected: false }` without throwing. */ @@ -199,46 +374,137 @@ function detectApiIntegration(text, terms) { const signals = []; const seen = new Set(); const lines = stripped.split('\n'); - // (a) compound verb+noun on the same line. - if (effective.verbs.length > 0 && effective.nouns.length > 0) { - const verbRe = new RegExp('(^|[^a-zA-Z0-9])(' + effective.verbs.map(escapeRegex).join('|') + ')([^a-zA-Z0-9]|$)', 'gi'); - const nounRe = new RegExp('(^|[^a-zA-Z0-9])(' + effective.nouns.map(escapeRegex).join('|') + ')([^a-zA-Z0-9]|$)', 'gi'); - for (const line of lines) { - verbRe.lastIndex = 0; - nounRe.lastIndex = 0; - const vMatch = verbRe.exec(line); - if (!vMatch) - continue; - const nMatch = nounRe.exec(line); - if (!nMatch) - continue; - const verb = (vMatch[2] || '').toLowerCase(); - const noun = (nMatch[2] || '').toLowerCase(); - const key = `${verb}+${noun}`; - if (seen.has(key)) - continue; - seen.add(key); - signals.push({ verb, noun, snippet: makeSnippet(line, noun) }); - } - } - // (b) explicit API|SDK|REST|GraphQL surface. - for (const line of lines) { - SERVICE_SURFACE_API_RE.lastIndex = 0; - const m = SERVICE_SURFACE_API_RE.exec(line); - if (!m) - continue; - // Reject ordinary capitalized sentence starters ("The API …", "Our REST …"). - if (SERVICE_STOPWORDS.has((m[1] || '').toLowerCase())) - continue; - const noun = (m[2] || '').toLowerCase(); - const key = `surface+${noun}`; + const hasCompoundTerms = effective.verbs.length > 0 && effective.nouns.length > 0; + // Trailing boundary is a LOOKAHEAD (not consumed) so back-to-back terms + // separated by one boundary char are both found. + const verbRe = hasCompoundTerms + ? new RegExp('(^|[^a-zA-Z0-9])(' + effective.verbs.map(escapeRegex).join('|') + ')(?=[^a-zA-Z0-9]|$)', 'gi') + : null; + const nounRe = hasCompoundTerms + ? new RegExp('(^|[^a-zA-Z0-9])(' + effective.nouns.map(escapeRegex).join('|') + ')(?=[^a-zA-Z0-9]|$)', 'gi') + : null; + const surfaceRe = new RegExp(SERVICE_SURFACE_API_RE.source, 'g'); + const nounSet = new Set(effective.nouns); + const emitPair = (vTerm, nTerm, snippetLine) => { + const key = `${vTerm}+${nTerm}`; if (seen.has(key)) - continue; + return; seen.add(key); - signals.push({ verb: '(surface)', noun, snippet: makeSnippet(line, m[1]) }); + signals.push({ verb: vTerm, noun: nTerm, snippet: makeSnippet(snippetLine, nTerm) }); + }; + for (const rawLine of lines) { + // Inline code spans are code, not prose — mask them (length-preserving so + // offsets keep lining up), but keep package-shaped span content as noun + // evidence (#2365 review FN-4: `stripe-sdk` names a dependency). + const inlineSpans = (0, markdown_sectionizer_cjs_1.scanInlineCodeSpans)(rawLine); + let line = rawLine; + const spanNouns = []; + for (const s of inlineSpans) { + line = line.slice(0, s.start) + ' '.repeat(s.end - s.start) + line.slice(s.end); + const content = s.content.trim(); + if (content.length === 0 || /\s/.test(content)) + continue; + const segs = content.toLowerCase().split(/[^a-z0-9]+/).filter(Boolean); + if (segs.length < 2) + continue; // a bare `api` span is a code identifier + const hit = segs.find((seg) => nounSet.has(seg)); + if (hit) + spanNouns.push({ term: hit, start: s.start, end: s.end }); + } + // Path-shaped tokens (routes, file names, URLs) are references, not prose. + const { masked, urlNouns } = scanLineTokens(line, nounRe, nounSet); + const clauses = splitClauses(masked); + const extraNouns = urlNouns.concat(spanNouns); + // (a) compound verb+noun — SAME CLAUSE ONLY. There is no word-gap cap (a cap + // cannot tell a long genuine clause from a long internal one) and no + // cross-clause binding (see the note by CLAUSE_BOUNDARY_CHARS): the clause + // boundary is the whole relationship test. Nouns are NOT filtered on + // "internal" qualification here — "integrate the internal API" is a + // fail-closed positive; the declaration dismisses it if wrong. + if (verbRe && nounRe) { + for (const clause of clauses) { + const verbs = collectTermMatches(verbRe, clause.text); + if (verbs.length === 0) + continue; + const nouns = collectTermMatches(nounRe, clause.text); + const nounTerms = new Set(nouns.map((t) => t.term)); + for (const u of extraNouns) { + if (u.start >= clause.start && u.end <= clause.start + clause.text.length) { + nounTerms.add(u.term); + } + } + if (nounTerms.size === 0) + continue; + for (const vTerm of new Set(verbs.map((t) => t.term))) { + for (const nTerm of nounTerms) + emitPair(vTerm, nTerm, rawLine); + } + } + } + // (b) explicit API|SDK|REST|GraphQL surface — scan every candidate + // in every clause (a rejected first candidate must not shadow a later + // genuine service; #2365 review C-1). + for (const clause of clauses) { + surfaceRe.lastIndex = 0; + let m; + while ((m = surfaceRe.exec(clause.text)) !== null) { + const svc = m[1] || ''; + const svcLower = svc.toLowerCase(); + // Reject capitalized sentence starters ("The API"), locality/protocol + // descriptors ("Internal API", "REST API"), compound modifiers + // ("Resolver-only API"), and services qualified first-party + // ("internal Payments API"). A real vendor name is none of these. + if (SERVICE_STOPWORDS.has(svcLower)) + continue; + if (SURFACE_DESCRIPTOR_WORDS.has(svcLower)) + continue; + if (COMPOUND_MODIFIER_RE.test(svc)) + continue; + if (isInternallyQualified(masked, clause.start + m.index)) + continue; + const noun = (m[2] || '').toLowerCase(); + const key = `surface+${noun}`; + if (seen.has(key)) + continue; + seen.add(key); + signals.push({ verb: '(surface)', noun, snippet: makeSnippet(rawLine, svc) }); + } + } } return { detected: signals.length > 0, signals, terms: effective }; } +/** True when the word IMMEDIATELY ADJACENT before `offset` is a locality + * descriptor ("internal Payments API") — first-party qualification is negative + * evidence for an EXTERNAL-API signal. Only plain spaces/tabs may separate the + * descriptor from the service: any intervening punctuation means the descriptor + * belongs to a prior clause/sentence and must NOT qualify ("The cache is + * private. Stripe API …" — `private` is a different sentence; #2365 review). + * Looks back through a BOUNDED window, not the whole prefix, to stay linear. */ +const QUALIFIER_LOOKBACK = 24; // longest descriptor ("first-party") + separators +function isInternallyQualified(masked, offset) { + const from = offset > QUALIFIER_LOOKBACK ? offset - QUALIFIER_LOOKBACK : 0; + const window = masked.slice(from, offset); + // Only whitespace and markdown emphasis/wrapper markers (`*_~\`) may separate + // the descriptor from the service, so "The **internal** Payments API" still + // qualifies — but NOT a clause/sentence boundary, so "…is private. Stripe API" + // does not (the descriptor is a different sentence; #2365 review). + const m = /([A-Za-z0-9'-]+)[\s*_~`]*$/.exec(window); + if (!m) + return false; + // A word truncated by the window start is not a descriptor match (its real + // start lies before the window) — fail toward detection. + if (from > 0 && m.index === 0 && /[A-Za-z0-9'-]/.test(masked[from - 1])) + return false; + return INTERNAL_DESCRIPTORS.has(m[1].toLowerCase()); +} +/** Matches a declaration line such as + * `No external API integration: ` (also `**bold**` and em-dash + * separators). The reason is REQUIRED — a bare declaration does not parse. + * Deliberately NOT matched: blockquoted lines (`> No external …` is quoted + * text, not a declaration) and anything inside fenced code or HTML comments + * (both stripped before the scan; #2365 review C-3). */ +const NO_INTEGRATION_DECLARATION_RE = /^\s*(?:\*\*)?no external api integration(?:\*\*)?\s*(?:[:—–-]|--)\s*(\S[^\n]*)$/im; +const HTML_COMMENT_RE = //g; const VALID_DECISIONS = new Set(['INTEGRATE', 'OPT-OUT']); /** * Parse a coverage matrix from COVERAGE.md. Accepts two bijective formats: @@ -258,10 +524,17 @@ const VALID_DECISIONS = new Set(['INTEGRATE', 'OPT-OUT']); * `{ rows: [], errors: [], format: 'none' }` for empty/non-matrix input. */ function parseCoverageMatrix(text) { - const out = { rows: [], errors: [], format: 'none' }; + const out = { rows: [], errors: [], format: 'none', declaration: null }; if (typeof text !== 'string') return out; const src = text.replace(/\r\n/g, '\n'); + // #2365 acceptance #5: a "no external API integration" declaration. Scanned + // on fence-stripped, comment-stripped text so an example inside a code block + // or an HTML comment does not count. + const declMatch = NO_INTEGRATION_DECLARATION_RE.exec((0, markdown_sectionizer_cjs_1.stripFencedCode)(src).text.replace(HTML_COMMENT_RE, '')); + if (declMatch) { + out.declaration = { none: true, reason: (declMatch[1] || '').trim() }; + } // (1) fenced ```coverage JSON block takes precedence if present. // Case-insensitive info string (```coverage and ```Coverage are both legal CommonMark). const fenceBody = (0, markdown_sectionizer_cjs_1.extractFencedBlock)(src, 'coverage'); @@ -366,6 +639,26 @@ function validateCoverageMatrix(text) { const parsed = parseCoverageMatrix(text); const errors = [...parsed.errors]; const rows = parsed.rows; + // #2365 acceptance #5: a reasoned no-integration declaration with no rows + // satisfies the gate. A declaration ALONGSIDE rows is contradictory — the + // file must say one thing. + if (parsed.declaration) { + if (rows.length > 0) { + errors.push('declares "no external API integration" but also contains coverage rows — remove the declaration or the rows'); + } + else { + if (parsed.declaration.reason.length > REASON_MAX_LEN) { + errors.push(`declaration reason exceeds ${REASON_MAX_LEN} chars`); + } + const valid = errors.length === 0; + return { + valid, + errors, + counts: { surface: 0, integrate: 0, optout: 0 }, + none_declared: valid, + }; + } + } if (rows.length === 0) { if (errors.length === 0) errors.push('matrix is empty — no capabilities enumerated'); diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 82729df34..de78142e1 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -68,7 +68,7 @@ const capabilities = { "into": "planner", "fragment": { "path": "fragments/api-coverage-plan-pre.md", - "inline": "# API Coverage Decision Checkpoint\n\n> Full API Coverage by Default — Opt Out, Never Opt In. Fires when a phase\n> integrates an external API / SDK / service. Most non-API phases will not fire\n> it — that is the point.\n\n## Why this exists\n\n\"We integrated the API\" too often silently means \"we integrated whatever the\nfirst use case exercised.\" Every un-built capability is then an invisible hole,\ndiscovered later by a user who reasonably expected it to work. The phase sealed\ngreen because its tasks completed; nobody decided the gaps were acceptable,\nbecause nobody enumerated them. This checkpoint makes the surface **visible and\ndecided** before the phase can seal.\n\n## Detect whether this phase integrates an external API\n\nThe detector is a deterministic scan over the phase scope. It strips fenced\ncode blocks first, so a trigger term inside a code snippet does not fire. It\nreturns a typed result: `{ detected, signals[], terms }`. Run it on the phase\nscope (the concatenation of this phase's ROADMAP section + the PLAN body):\n\n```bash\nSCOPE=\"$(cat \"${PHASE_DIR}\"/*-PLAN.md 2>/dev/null) $(gsd_run query roadmap.get-phase \"${PHASE}\" 2>/dev/null || true)\"\nAPI_COVERAGE_JSON=$(printf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json 2>/dev/null || echo '{\"detected\":false,\"signals\":[]}')\n```\n\nRead `API_COVERAGE_JSON.detected`. Act on it only — do **not** pattern-match the\nprose yourself.\n\n**If `detected` is `false`:** this phase does not integrate an external API. Skip\nthe checkpoint entirely and continue planning. Do not raise it with the user.\n\n**If `detected` is `true`:** an external-API integration is in scope. You MUST\nproduce a **coverage matrix** before the plan is finalized.\n\n## Produce the coverage matrix\n\nEnumerate the external API's full **capability surface** — the verb/endpoint/method\nlist (e.g. for a music service: `search`, `play`, `pause`, `skip`, `set_volume`,\n`get_playlist`, `create_playlist`, `add_to_playlist`, …). For each capability\nrecord a decision, starting from **full coverage** as the default:\n\n| capability | decision | reason |\n|---|---|---|\n| `` | `INTEGRATE` \\| `OPT-OUT` | `` |\n\nRules:\n\n- **`INTEGRATE` is the default.** Every capability starts as INTEGRATE; the\n matrix is the *subtraction record*.\n- **Every `OPT-OUT` MUST carry a one-line reason** (`not needed`, `not needed\n yet`, `explicitly out of scope`, …). An opt-out without a reason is an\n un-decided hole — the exact failure mode this gate exists to close.\n- **A second integration against the same need** (e.g. a second platform for the\n same capability) starts from the **same full-coverage baseline** as the first.\n Do not carry over the first integration's opt-outs silently — re-decide each\n capability for the new surface, so a first-class/fallback asymmetry cannot\n accumulate.\n\nWrite the matrix to `${PHASE_DIR}/COVERAGE.md` (canonical markdown-table form):\n\n```markdown\n# API Coverage — \n\n> Full coverage by default. Opt-outs are explicit, reasoned decisions.\n\n| capability | decision | reason |\n|---|---|---|\n| search | INTEGRATE | |\n| playlists | INTEGRATE | |\n| skip | OPT-OUT | not needed yet — tracked for follow-up phase |\n```\n\nA fenced ` ```coverage ` JSON block is also accepted for machine-generated\nmatrices; the markdown table is preferred (human-editable, diff-friendly).\n\n## The seal-time gate\n\nThis checkpoint is enforced. At `verify:pre` the `api-coverage.verify-pre` gate\nruns `check api-coverage.verify-pre `:\n\n- If `COVERAGE.md` exists, it is validated — every row needs a valid decision and\n every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**.\n- If `COVERAGE.md` is absent, the detector runs again over the phase scope. If a\n strong external-API-integration signal is found, the seal is **blocked** until a\n matrix is produced. If no signal is found, the phase is treated as a non-API\n phase and the seal proceeds.\n\nSo: an API-integrating phase cannot seal without a decided matrix. Produce it at\nplan time; do not leave it for seal time.\n\n## Tuning the vocabulary (optional)\n\nThe trigger vocabulary is a curated, additive-only set in\n`gsd-core/bin/lib/api-coverage.cjs` (`DEFAULT_API_COVERAGE_TERMS`). To widen it\nfor a project, override at the call site:\n\n```bash\nprintf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json \\\n --verbs integrate,wrap,connect,embed --nouns api,sdk,rest,grpc,webhook,plugin\n```\n\nThe whole checkpoint is toggleable via `workflow.api_coverage_gate` in\n`.planning/config.json`.\n" + "inline": "# API Coverage Decision Checkpoint\n\n> Full API Coverage by Default — Opt Out, Never Opt In. Fires when a phase\n> integrates an external API / SDK / service. Most non-API phases will not fire\n> it — that is the point.\n\n## Why this exists\n\n\"We integrated the API\" too often silently means \"we integrated whatever the\nfirst use case exercised.\" Every un-built capability is then an invisible hole,\ndiscovered later by a user who reasonably expected it to work. The phase sealed\ngreen because its tasks completed; nobody decided the gaps were acceptable,\nbecause nobody enumerated them. This checkpoint makes the surface **visible and\ndecided** before the phase can seal.\n\n## Detect whether this phase integrates an external API\n\nThe detector is a deterministic scan over the phase scope. It strips fenced\ncode blocks first, so a trigger term inside a code snippet does not fire. It\nreturns a typed result: `{ detected, signals[], terms }`. Run it on the phase\nscope (the concatenation of this phase's ROADMAP section + the PLAN body):\n\n```bash\nSCOPE=\"$(cat \"${PHASE_DIR}\"/*-PLAN.md 2>/dev/null) $(gsd_run query roadmap.get-phase \"${PHASE}\" 2>/dev/null || true)\"\nAPI_COVERAGE_JSON=$(printf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json 2>/dev/null || echo '{\"detected\":false,\"signals\":[]}')\n```\n\nRead `API_COVERAGE_JSON.detected`. Act on it only — do **not** pattern-match the\nprose yourself.\n\n**If `detected` is `false`:** this phase does not integrate an external API. Skip\nthe checkpoint entirely and continue planning. Do not raise it with the user.\n\n**If `detected` is `true`:** an external-API integration is in scope. You MUST\nproduce a **coverage matrix** before the plan is finalized.\n\n**If `detected` is `true` but the phase genuinely integrates no external API**\n(the detector is deterministic, not infallible — confirm by re-reading the phase\nscope, not by preference): do NOT fabricate a matrix row for a capability that\ndoes not exist. Write a reasoned declaration to `${PHASE_DIR}/COVERAGE.md`\ninstead:\n\n```markdown\nNo external API integration: .\n```\n\nThe reason is required, exactly like an `OPT-OUT` reason. The seal-time gate\naccepts this declaration in place of a matrix.\n\n## Produce the coverage matrix\n\nEnumerate the external API's full **capability surface** — the verb/endpoint/method\nlist (e.g. for a music service: `search`, `play`, `pause`, `skip`, `set_volume`,\n`get_playlist`, `create_playlist`, `add_to_playlist`, …). For each capability\nrecord a decision, starting from **full coverage** as the default:\n\n| capability | decision | reason |\n|---|---|---|\n| `` | `INTEGRATE` \\| `OPT-OUT` | `` |\n\nRules:\n\n- **`INTEGRATE` is the default.** Every capability starts as INTEGRATE; the\n matrix is the *subtraction record*.\n- **Every `OPT-OUT` MUST carry a one-line reason** (`not needed`, `not needed\n yet`, `explicitly out of scope`, …). An opt-out without a reason is an\n un-decided hole — the exact failure mode this gate exists to close.\n- **A second integration against the same need** (e.g. a second platform for the\n same capability) starts from the **same full-coverage baseline** as the first.\n Do not carry over the first integration's opt-outs silently — re-decide each\n capability for the new surface, so a first-class/fallback asymmetry cannot\n accumulate.\n\nWrite the matrix to `${PHASE_DIR}/COVERAGE.md` (canonical markdown-table form):\n\n```markdown\n# API Coverage — \n\n> Full coverage by default. Opt-outs are explicit, reasoned decisions.\n\n| capability | decision | reason |\n|---|---|---|\n| search | INTEGRATE | |\n| playlists | INTEGRATE | |\n| skip | OPT-OUT | not needed yet — tracked for follow-up phase |\n```\n\nA fenced ` ```coverage ` JSON block is also accepted for machine-generated\nmatrices; the markdown table is preferred (human-editable, diff-friendly).\n\n## The seal-time gate\n\nThis checkpoint is enforced. At `verify:pre` the `api-coverage.verify-pre` gate\nruns `check api-coverage.verify-pre `:\n\n- If `COVERAGE.md` exists, it is validated — every row needs a valid decision and\n every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**. A\n reasoned `No external API integration: …` declaration (and no rows) passes.\n- If `COVERAGE.md` is absent, the detector runs again over the phase scope. If a\n strong external-API-integration signal is found, the seal is **blocked** until a\n matrix is produced. If no signal is found, the phase is treated as a non-API\n phase and the seal proceeds.\n\nSo: an API-integrating phase cannot seal without a decided matrix. Produce it at\nplan time; do not leave it for seal time.\n\n## Tuning the vocabulary (optional)\n\nThe trigger vocabulary is a curated, additive-only set in\n`gsd-core/bin/lib/api-coverage.cjs` (`DEFAULT_API_COVERAGE_TERMS`). To widen it\nfor a project, override at the call site:\n\n```bash\nprintf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json \\\n --verbs integrate,wrap,connect,embed --nouns api,sdk,rest,grpc,webhook,plugin\n```\n\nThe whole checkpoint is toggleable via `workflow.api_coverage_gate` in\n`.planning/config.json`.\n" }, "produces": [ "COVERAGE.md" @@ -3174,7 +3174,7 @@ const byLoopPoint = { "into": "planner", "fragment": { "path": "fragments/api-coverage-plan-pre.md", - "inline": "# API Coverage Decision Checkpoint\n\n> Full API Coverage by Default — Opt Out, Never Opt In. Fires when a phase\n> integrates an external API / SDK / service. Most non-API phases will not fire\n> it — that is the point.\n\n## Why this exists\n\n\"We integrated the API\" too often silently means \"we integrated whatever the\nfirst use case exercised.\" Every un-built capability is then an invisible hole,\ndiscovered later by a user who reasonably expected it to work. The phase sealed\ngreen because its tasks completed; nobody decided the gaps were acceptable,\nbecause nobody enumerated them. This checkpoint makes the surface **visible and\ndecided** before the phase can seal.\n\n## Detect whether this phase integrates an external API\n\nThe detector is a deterministic scan over the phase scope. It strips fenced\ncode blocks first, so a trigger term inside a code snippet does not fire. It\nreturns a typed result: `{ detected, signals[], terms }`. Run it on the phase\nscope (the concatenation of this phase's ROADMAP section + the PLAN body):\n\n```bash\nSCOPE=\"$(cat \"${PHASE_DIR}\"/*-PLAN.md 2>/dev/null) $(gsd_run query roadmap.get-phase \"${PHASE}\" 2>/dev/null || true)\"\nAPI_COVERAGE_JSON=$(printf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json 2>/dev/null || echo '{\"detected\":false,\"signals\":[]}')\n```\n\nRead `API_COVERAGE_JSON.detected`. Act on it only — do **not** pattern-match the\nprose yourself.\n\n**If `detected` is `false`:** this phase does not integrate an external API. Skip\nthe checkpoint entirely and continue planning. Do not raise it with the user.\n\n**If `detected` is `true`:** an external-API integration is in scope. You MUST\nproduce a **coverage matrix** before the plan is finalized.\n\n## Produce the coverage matrix\n\nEnumerate the external API's full **capability surface** — the verb/endpoint/method\nlist (e.g. for a music service: `search`, `play`, `pause`, `skip`, `set_volume`,\n`get_playlist`, `create_playlist`, `add_to_playlist`, …). For each capability\nrecord a decision, starting from **full coverage** as the default:\n\n| capability | decision | reason |\n|---|---|---|\n| `` | `INTEGRATE` \\| `OPT-OUT` | `` |\n\nRules:\n\n- **`INTEGRATE` is the default.** Every capability starts as INTEGRATE; the\n matrix is the *subtraction record*.\n- **Every `OPT-OUT` MUST carry a one-line reason** (`not needed`, `not needed\n yet`, `explicitly out of scope`, …). An opt-out without a reason is an\n un-decided hole — the exact failure mode this gate exists to close.\n- **A second integration against the same need** (e.g. a second platform for the\n same capability) starts from the **same full-coverage baseline** as the first.\n Do not carry over the first integration's opt-outs silently — re-decide each\n capability for the new surface, so a first-class/fallback asymmetry cannot\n accumulate.\n\nWrite the matrix to `${PHASE_DIR}/COVERAGE.md` (canonical markdown-table form):\n\n```markdown\n# API Coverage — \n\n> Full coverage by default. Opt-outs are explicit, reasoned decisions.\n\n| capability | decision | reason |\n|---|---|---|\n| search | INTEGRATE | |\n| playlists | INTEGRATE | |\n| skip | OPT-OUT | not needed yet — tracked for follow-up phase |\n```\n\nA fenced ` ```coverage ` JSON block is also accepted for machine-generated\nmatrices; the markdown table is preferred (human-editable, diff-friendly).\n\n## The seal-time gate\n\nThis checkpoint is enforced. At `verify:pre` the `api-coverage.verify-pre` gate\nruns `check api-coverage.verify-pre `:\n\n- If `COVERAGE.md` exists, it is validated — every row needs a valid decision and\n every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**.\n- If `COVERAGE.md` is absent, the detector runs again over the phase scope. If a\n strong external-API-integration signal is found, the seal is **blocked** until a\n matrix is produced. If no signal is found, the phase is treated as a non-API\n phase and the seal proceeds.\n\nSo: an API-integrating phase cannot seal without a decided matrix. Produce it at\nplan time; do not leave it for seal time.\n\n## Tuning the vocabulary (optional)\n\nThe trigger vocabulary is a curated, additive-only set in\n`gsd-core/bin/lib/api-coverage.cjs` (`DEFAULT_API_COVERAGE_TERMS`). To widen it\nfor a project, override at the call site:\n\n```bash\nprintf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json \\\n --verbs integrate,wrap,connect,embed --nouns api,sdk,rest,grpc,webhook,plugin\n```\n\nThe whole checkpoint is toggleable via `workflow.api_coverage_gate` in\n`.planning/config.json`.\n" + "inline": "# API Coverage Decision Checkpoint\n\n> Full API Coverage by Default — Opt Out, Never Opt In. Fires when a phase\n> integrates an external API / SDK / service. Most non-API phases will not fire\n> it — that is the point.\n\n## Why this exists\n\n\"We integrated the API\" too often silently means \"we integrated whatever the\nfirst use case exercised.\" Every un-built capability is then an invisible hole,\ndiscovered later by a user who reasonably expected it to work. The phase sealed\ngreen because its tasks completed; nobody decided the gaps were acceptable,\nbecause nobody enumerated them. This checkpoint makes the surface **visible and\ndecided** before the phase can seal.\n\n## Detect whether this phase integrates an external API\n\nThe detector is a deterministic scan over the phase scope. It strips fenced\ncode blocks first, so a trigger term inside a code snippet does not fire. It\nreturns a typed result: `{ detected, signals[], terms }`. Run it on the phase\nscope (the concatenation of this phase's ROADMAP section + the PLAN body):\n\n```bash\nSCOPE=\"$(cat \"${PHASE_DIR}\"/*-PLAN.md 2>/dev/null) $(gsd_run query roadmap.get-phase \"${PHASE}\" 2>/dev/null || true)\"\nAPI_COVERAGE_JSON=$(printf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json 2>/dev/null || echo '{\"detected\":false,\"signals\":[]}')\n```\n\nRead `API_COVERAGE_JSON.detected`. Act on it only — do **not** pattern-match the\nprose yourself.\n\n**If `detected` is `false`:** this phase does not integrate an external API. Skip\nthe checkpoint entirely and continue planning. Do not raise it with the user.\n\n**If `detected` is `true`:** an external-API integration is in scope. You MUST\nproduce a **coverage matrix** before the plan is finalized.\n\n**If `detected` is `true` but the phase genuinely integrates no external API**\n(the detector is deterministic, not infallible — confirm by re-reading the phase\nscope, not by preference): do NOT fabricate a matrix row for a capability that\ndoes not exist. Write a reasoned declaration to `${PHASE_DIR}/COVERAGE.md`\ninstead:\n\n```markdown\nNo external API integration: .\n```\n\nThe reason is required, exactly like an `OPT-OUT` reason. The seal-time gate\naccepts this declaration in place of a matrix.\n\n## Produce the coverage matrix\n\nEnumerate the external API's full **capability surface** — the verb/endpoint/method\nlist (e.g. for a music service: `search`, `play`, `pause`, `skip`, `set_volume`,\n`get_playlist`, `create_playlist`, `add_to_playlist`, …). For each capability\nrecord a decision, starting from **full coverage** as the default:\n\n| capability | decision | reason |\n|---|---|---|\n| `` | `INTEGRATE` \\| `OPT-OUT` | `` |\n\nRules:\n\n- **`INTEGRATE` is the default.** Every capability starts as INTEGRATE; the\n matrix is the *subtraction record*.\n- **Every `OPT-OUT` MUST carry a one-line reason** (`not needed`, `not needed\n yet`, `explicitly out of scope`, …). An opt-out without a reason is an\n un-decided hole — the exact failure mode this gate exists to close.\n- **A second integration against the same need** (e.g. a second platform for the\n same capability) starts from the **same full-coverage baseline** as the first.\n Do not carry over the first integration's opt-outs silently — re-decide each\n capability for the new surface, so a first-class/fallback asymmetry cannot\n accumulate.\n\nWrite the matrix to `${PHASE_DIR}/COVERAGE.md` (canonical markdown-table form):\n\n```markdown\n# API Coverage — \n\n> Full coverage by default. Opt-outs are explicit, reasoned decisions.\n\n| capability | decision | reason |\n|---|---|---|\n| search | INTEGRATE | |\n| playlists | INTEGRATE | |\n| skip | OPT-OUT | not needed yet — tracked for follow-up phase |\n```\n\nA fenced ` ```coverage ` JSON block is also accepted for machine-generated\nmatrices; the markdown table is preferred (human-editable, diff-friendly).\n\n## The seal-time gate\n\nThis checkpoint is enforced. At `verify:pre` the `api-coverage.verify-pre` gate\nruns `check api-coverage.verify-pre `:\n\n- If `COVERAGE.md` exists, it is validated — every row needs a valid decision and\n every `OPT-OUT` a reason. A malformed/partial matrix **blocks the seal**. A\n reasoned `No external API integration: …` declaration (and no rows) passes.\n- If `COVERAGE.md` is absent, the detector runs again over the phase scope. If a\n strong external-API-integration signal is found, the seal is **blocked** until a\n matrix is produced. If no signal is found, the phase is treated as a non-API\n phase and the seal proceeds.\n\nSo: an API-integrating phase cannot seal without a decided matrix. Produce it at\nplan time; do not leave it for seal time.\n\n## Tuning the vocabulary (optional)\n\nThe trigger vocabulary is a curated, additive-only set in\n`gsd-core/bin/lib/api-coverage.cjs` (`DEFAULT_API_COVERAGE_TERMS`). To widen it\nfor a project, override at the call site:\n\n```bash\nprintf '%s' \"$SCOPE\" | node gsd-core/bin/lib/api-coverage.cjs --json \\\n --verbs integrate,wrap,connect,embed --nouns api,sdk,rest,grpc,webhook,plugin\n```\n\nThe whole checkpoint is toggleable via `workflow.api_coverage_gate` in\n`.planning/config.json`.\n" }, "produces": [ "COVERAGE.md" diff --git a/gsd-core/references/api-coverage.md b/gsd-core/references/api-coverage.md index f5d238f9f..233a26444 100644 --- a/gsd-core/references/api-coverage.md +++ b/gsd-core/references/api-coverage.md @@ -24,13 +24,26 @@ treated as an external-API integration when **either**: 1. a `COVERAGE.md` matrix is present in the phase directory (the planner produced one at `plan:pre`), **or** 2. the phase scope shows a strong external-API-integration signal (an integration - verb co-occurring with an external-API noun, or an explicit ` - API|SDK|REST|GraphQL` surface) and no matrix yet exists. + verb and an external-API noun **in the same clause**, or an explicit + ` API|SDK|REST|GraphQL` surface naming a real service) and no matrix + yet exists. -Non-API phases (refactors, bug fixes, internal-only work, features that merely -*mention* an existing internal API) do **not** fire the gate — the trigger -requires a compound signal, so a bare word like "api" in "the public API of -UserController" is intentionally ignored. +The detector is deliberately **fail-closed**: it leans toward firing, because a +false positive is dismissed by a one-line `COVERAGE.md` "no external API +integration" declaration, whereas a false *negative* silently lets a real +external-API phase past this blocking gate — strictly worse. So it suppresses +only prose that is unambiguously not external integration. A bare word like +"api" in "the public API of UserController" is ignored (no integration verb + +named service); the clause boundary is the whole relationship test, so an +integration verb and an API noun in **different** clauses do not pair. Since +#2365 the detector also excludes non-prose spans before matching: fenced code +blocks, inline `` `code` `` spans, and path-shaped tokens (a first-party +`src/app/api/profile/route.ts` route is a file path, not an external API, while +an external host like `api.stripe.com/v1` still counts). In the +` API` surface position it rejects capitalized sentence starters +("The API"), locality/protocol descriptors ("Internal API", "REST API"), +compound modifiers ("Resolver-only API"), and first-party-qualified services +("internal Payments API") — a real vendor name is none of these. ## The two touch points @@ -70,6 +83,23 @@ must be non-empty and unique; every decision must be `INTEGRATE` or `OPT-OUT`; every `OPT-OUT` must have a reason. Violations block the seal with a precise error. +### Declaring "no external API integration" (#2365) + +A phase that integrates no external API/SDK/service — but was still asked for a +matrix (e.g. the detector over-fired, or a team wants the decision on record) — +declares it instead of fabricating a row: + +```markdown +No external API integration: UI-only phase, no third-party surface. +``` + +The reason is **required**, exactly like an `OPT-OUT` reason — the declaration +is a reasoned decision, not a bypass. A `COVERAGE.md` containing both the +declaration and coverage rows is contradictory and blocks the seal. When the +detector still finds integration signals in the phase scope, the declaration +wins (it is the human overrule for a fallible detector) but the gate output +surfaces the overridden signals so the contradiction is visible, not silent. + ## A second integration against the same need A second platform for an existing capability (e.g. adding YouTube alongside diff --git a/src/api-coverage.cts b/src/api-coverage.cts index f63b14ee8..5952aaae1 100644 --- a/src/api-coverage.cts +++ b/src/api-coverage.cts @@ -16,11 +16,20 @@ * (acceptance #2) are testable. Mirrors assumption-delta.cts (#1561). * - COMPOUND SIGNAL for low false positives. A bare word like "api" appears in * countless non-integration phases ("the public API of UserController"). The - * detector requires an INTEGRATION VERB co-occurring with an EXTERNAL-API - * NOUN (or an explicit " API/SDK" phrase). Single weak tokens do not - * fire. This is the issue's "low false-positive trigger" made mechanical. - * - FENCED CODE BLOCKS ARE STRIPPED first (markdown-sectionizer seam) so a - * trigger term inside a code snippet does not fire. + * detector requires an INTEGRATION VERB and an EXTERNAL-API NOUN in the SAME + * CLAUSE (#2365 — same-line co-occurrence across unrelated clauses over-fired; + * the clause boundary, not a word-gap cap, is the relationship test), or an + * explicit " API/SDK" phrase naming a real service. Single weak + * tokens do not fire. This is the issue's "low false-positive trigger" made + * mechanical. + * - CODE AND PATHS ARE NOT PROSE. Fenced code blocks and inline code spans are + * stripped first (markdown-sectionizer seam), and path-shaped tokens + * (`src/app/api/...`, URLs) are masked, so a trigger term inside code or a + * first-party route path does not fire (#2365). + * - NO-INTEGRATION DECLARATION (#2365 acceptance #5). A COVERAGE.md consisting + * of `No external API integration: ` is a valid, reasoned way for a + * phase to state that no external surface exists — the alternative to + * fabricating a matrix row when the detector is overruled by a human. * - THE DETECTOR IS A FALLBACK. The primary path is the plan:pre contribution * prompting COVERAGE.md creation. The detector runs only when COVERAGE.md is * ABSENT, to catch the "nobody decided" case (acceptance #1). Its precision @@ -47,7 +56,7 @@ * exit 0 = integration detected, 1 = none, 2 = startup error */ -import { stripFencedCode, extractFencedBlock } from './markdown-sectionizer.cjs'; +import { stripFencedCode, scanInlineCodeSpans, extractFencedBlock } from './markdown-sectionizer.cjs'; // ─── Integration-signal vocabulary ──────────────────────────────────────────── @@ -185,7 +194,12 @@ function makeSnippet(line: string, anchor: string): string { * `[A-Z]\w+ API` shape. Those are common English, not a service name, so they * are rejected before counting as a surface signal (acceptance #4 — low false * positives). */ -const SERVICE_SURFACE_API_RE = /\b([A-Z][A-Za-z0-9_-]{1,})\s+(API|SDK|REST|GraphQL)\b/; +// Service-name length is bounded ({1,40}) so a hostile "A-A-A-…-A-x" run cannot +// drive the greedy group into O(n^2) backtracking (#2365 review). Nearly all +// vendor names fit; a >41-char service token before API/SDK would be missed by +// this surface path (it would still fire via the compound verb+noun rule) — +// an accepted bound. +const SERVICE_SURFACE_API_RE = /\b([A-Z][A-Za-z0-9_-]{1,40})\s+(API|SDK|REST|GraphQL)\b/; const SERVICE_STOPWORDS = new Set([ 'the', 'an', 'a', 'our', 'this', 'these', 'that', 'those', 'new', 'add', 'use', 'your', 'my', 'no', 'some', 'any', 'all', 'each', 'every', 'both', @@ -193,12 +207,205 @@ const SERVICE_STOPWORDS = new Set([ 'we', 'you', 'they', 'it', ]); +/** #2365 — the detector is FAIL-CLOSED: it leans toward detecting, because a + * false positive is cheaply dismissed by a one-line COVERAGE.md "no external + * API integration" declaration, whereas a false NEGATIVE silently lets a real + * external-API phase past a BLOCKING gate. So the only prose the detector + * actively suppresses is the classes that are unambiguously NOT external + * integration: first-party route paths, verb/noun in unrelated clauses, and + * descriptive/protocol " API" prose with no named service. + * + * CLAUSE_BOUNDARY_RE: a verb and a noun form ONE compound action only inside + * one grammatical clause — sentence punctuation and table-cell walls (`|`) + * end a clause. `-` is deliberately absent (it would split hyphenated words). + * There is deliberately NO word-gap cap inside a clause: a cap cannot separate + * a genuine long integration clause (F4, 21 words) from a long internal-UI + * clause (18 words) — the clause boundary is the only sound signal, and the + * declaration handles the residual false positives. */ +const CLAUSE_BOUNDARY_RE = /[,;:.!?|()—–]/; +/** Same character class as CLAUSE_BOUNDARY_RE, as a set — for scanning a token's + * trailing punctuation without an unanchored `[…]+$` regex, whose backtracking + * is O(n^2) on a long punctuation run (#2365 review). */ +const CLAUSE_BOUNDARY_CHARS = new Set([',', ';', ':', '.', '!', '?', '|', '(', ')', '—', '–']); + +/* DELIBERATELY NO cross-clause binding. Detection is same-clause only. Binding + * a verb in one clause to a noun in another ("Integrate Stripe, exposing its + * endpoints"; "Integrate Stripe; use its endpoints") requires knowing "Stripe" + * is a vendor and "its" refers to it — a vendor dictionary + coreference, which + * trek-e's brief rules out in principle. Every lexical cross-clause rule tried + * (word-gap cap, participle continuation) traded a false negative for a false + * positive across four review rounds. So a service named ONLY in a clause + * separate from its API noun, with no explicit ` API` surface, is a + * DOCUMENTED fail-open limitation — cheaply covered by the COVERAGE.md + * declaration and rare in real phase prose, which says "integrate the X API". */ + +/** In the ` API|SDK` surface position, these capture words are NOT a + * named third-party service: locality/scope descriptors ("Internal API", + * "Public API") and bare protocol names ("REST API", "GraphQL API"). A real + * vendor name (Stripe, Shopify) is none of these, so rejecting them costs no + * true positives while killing the descriptive-prose false positives (#2365 + * acceptance #3, review F8). */ +const SURFACE_DESCRIPTOR_WORDS = new Set([ + 'internal', 'external', 'public', 'private', 'local', 'in-house', 'first-party', + 'generic', 'shared', 'common', 'legacy', 'rest', 'restful', 'graphql', 'grpc', + 'soap', 'rpc', 'http', 'https', 'json', 'xml', +]); + +/** Locality qualifiers that, when they immediately precede a ` API`, + * mark it as first-party ("internal Payments API") — negative evidence for an + * EXTERNAL-API surface signal. Only unambiguously-internal words: "external" + * is deliberately absent (an external API IS external). */ +const INTERNAL_DESCRIPTORS = new Set(['internal', 'in-house', 'local', 'first-party', 'private']); + +/** A capitalized compound modifier ("Resolver-only", "Read-only", "E-commerce" + * — lowercase letter right after the hyphen) is an adjective phrase, not a + * service name. Real hyphenated services capitalize the second segment + * ("T-Mobile"). */ +const COMPOUND_MODIFIER_RE = /^[A-Z][A-Za-z0-9]*-[a-z]/; + +interface TermMatch { + term: string; + start: number; + end: number; +} + +interface LineScan { + /** The line with path-shaped tokens replaced by same-length space padding + * (offsets preserved for the clause logic). */ + masked: string; + /** Noun-vocabulary terms found inside NON-LOCAL URLs (`https://api.stripe.com`) + * — a URL that itself names an API surface is external-dependency evidence, + * so it still feeds the compound rule even though the URL is masked from + * plain prose matching. */ + urlNouns: TermMatch[]; +} + +const URL_TOKEN_RE = /^[([<"'`]*[a-z][a-z0-9+.-]*:\/\//i; +const LOCAL_URL_RE = /^[([<"'`]*[a-z][a-z0-9+.-]*:\/\/(?:localhost|127(?:\.\d{1,3}){1,3}|0\.0\.0\.0|\[::1\])(?=[:/?#]|$)/i; +/** A scheme-less token that STARTS with a dotted hostname whose final label is + * alphabetic ("api.stripe.com/v1") — a bare external API host. A first-party + * route path ("src/app/api/…") has no dotted head, and an IP host ("127.1/…") + * has a numeric final label, so neither matches (#2365 review F2). */ +const DOMAIN_HEAD_RE = /^[([<"'`]*(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z]{2,}(?=[:/?#]|$)/i; + +/** Mask whitespace-delimited tokens with an interior `/` — file paths, framework + * routes (`src/app/api/...`), URLs. They are references, not integration prose + * (#2365 root cause 2: `/` counted as a word boundary, so first-party route + * paths matched the noun vocabulary). Two carve-outs keep genuine signals: + * - a slashed token whose segments are ALL noun-vocabulary words ("API/SDK", + * "REST/GraphQL") is prose shorthand, not a path — left unmasked; + * - a non-local URL is masked, but noun terms inside it are collected as + * compound-rule evidence (the old detector caught "connect to + * https://api.stripe.com" via the `api` segment; losing that would + * fail-open). */ +function scanLineTokens(line: string, nounRe: RegExp | null, nounSet: Set): LineScan { + const urlNouns: TermMatch[] = []; + let masked = ''; + const tokenRe = /\S+/g; + let last = 0; + let m: RegExpExecArray | null; + while ((m = tokenRe.exec(line)) !== null) { + const rawTok = m[0]; + masked += line.slice(last, m.index); + last = m.index + rawTok.length; + // Peel trailing clause-boundary punctuation off the token and keep it + // LITERAL in `masked` — masking it away would erase a clause split and pair + // unrelated verb/noun across it (#2365 review F6: "…example.com, document…"). + // A backward char scan (not a `[…]+$` regex) keeps this linear. + let trailLen = 0; + while (trailLen < rawTok.length && CLAUSE_BOUNDARY_CHARS.has(rawTok[rawTok.length - 1 - trailLen])) { + trailLen++; + } + const trail = trailLen ? rawTok.slice(rawTok.length - trailLen) : ''; + const tok = trailLen ? rawTok.slice(0, rawTok.length - trailLen) : rawTok; + if (!/\S[\\/]\S/.test(tok)) { + masked += rawTok; + continue; + } + const segments = tok.split(/[\\/]/).map((s) => s.replace(/[^A-Za-z0-9]/g, '')); + if ( + segments.every((s) => s.length > 0 && (nounSet.has(s.toLowerCase()) || /^v\d+$/i.test(s))) && + segments.some((s) => nounSet.has(s.toLowerCase())) + ) { + masked += rawTok; // "API/SDK", "API/v2" — noun shorthand, not a path + continue; + } + // A scheme URL or a bare external hostname is an external dependency + // reference: mask it from prose but keep it as compound-rule evidence. A + // first-party route path has neither a scheme nor a dotted host, so it is + // masked WITHOUT contributing nouns (#2365 root cause 2). + // A non-local URL that NAMES an API vocabulary word ("api.stripe.com/v1") + // is external-dependency evidence, so its vocab nouns feed the compound + // rule. We deliberately do NOT treat every path-bearing URL as an endpoint: + // that fired on ordinary asset/link URLs ("…/theme.css", "…?next=/x") and + // recreated routine UI-phase false positives (#2365 review). A bare external + // host that names no vocabulary word ("graph.microsoft.com") and is not + // written as " API" is therefore a DOCUMENTED fail-open limitation. + const isSchemeUrl = URL_TOKEN_RE.test(tok) && !LOCAL_URL_RE.test(tok); + const isDomainUrl = !URL_TOKEN_RE.test(tok) && DOMAIN_HEAD_RE.test(tok); + if (nounRe && (isSchemeUrl || isDomainUrl)) { + for (const f of collectTermMatches(nounRe, tok)) { + urlNouns.push({ term: f.term, start: m.index, end: m.index + tok.length }); + } + } + masked += ' '.repeat(tok.length) + trail; + } + masked += line.slice(last); + return { masked, urlNouns }; +} + +/** All term matches in a clause, with offsets. `re` must be global with the + * term in group 2 and a consumed leading boundary in group 1. */ +function collectTermMatches(re: RegExp, clause: string): TermMatch[] { + const out: TermMatch[] = []; + re.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = re.exec(clause)) !== null) { + const start = m.index + (m[1] || '').length; + out.push({ term: (m[2] || '').toLowerCase(), start, end: start + (m[2] || '').length }); + if (m[0].length === 0) re.lastIndex++; + } + return out; +} + +interface ClauseSpan { + text: string; + start: number; +} + +/** Split a line into clause segments, keeping each segment's start offset so + * line-level spans (masked URL tokens) can be mapped into their clause. */ +function splitClauses(masked: string): ClauseSpan[] { + const out: ClauseSpan[] = []; + let start = 0; + for (let i = 0; i <= masked.length; i++) { + if (i === masked.length || CLAUSE_BOUNDARY_RE.test(masked[i])) { + out.push({ text: masked.slice(start, i), start }); + start = i + 1; + } + } + return out; +} + /** * Detect whether phase-scope prose describes integrating an external API/SDK. * - * Fires when EITHER: - * (a) a compound verb+noun signal co-occurs on the same line, OR - * (b) an explicit ` API|SDK|REST|GraphQL` surface appears. + * FAIL-CLOSED: it leans toward detecting, because a false positive is dismissed + * by a one-line COVERAGE.md declaration while a false negative silently slips a + * real external-API phase past a blocking gate. It fires when EITHER: + * (a) an integration VERB and an API NOUN share one CLAUSE ("integrate the + * Stripe API", "Connect … to api.stripe.com") — the clause boundary is the + * whole relationship test, so verb/noun in DIFFERENT clauses do not pair + * (#2365 acceptance #2). There is NO cross-clause binding: a service named + * only in a clause separate from its API noun is a documented limitation. + * (b) an explicit ` API|SDK|REST|GraphQL` surface names a service + * that is not a stopword, a locality/protocol descriptor, a compound + * modifier, or first-party-qualified ("Stripe API", "Spotify SDK"). + * + * Fenced code, inline code spans, and path-shaped tokens are excluded before + * matching. A package-shaped inline span (`@stripe/stripe-js`, `stripe-sdk`) + * and a URL that NAMES an API vocab word ("api.stripe.com/v1") still count as + * noun/dependency evidence; a bare host that names none does not. * * Non-string inputs degrade to `{ detected: false }` without throwing. */ @@ -220,49 +427,131 @@ export function detectApiIntegration( const seen = new Set(); const lines = stripped.split('\n'); - // (a) compound verb+noun on the same line. - if (effective.verbs.length > 0 && effective.nouns.length > 0) { - const verbRe = new RegExp( - '(^|[^a-zA-Z0-9])(' + effective.verbs.map(escapeRegex).join('|') + ')([^a-zA-Z0-9]|$)', - 'gi', - ); - const nounRe = new RegExp( - '(^|[^a-zA-Z0-9])(' + effective.nouns.map(escapeRegex).join('|') + ')([^a-zA-Z0-9]|$)', - 'gi', - ); - for (const line of lines) { - verbRe.lastIndex = 0; - nounRe.lastIndex = 0; - const vMatch = verbRe.exec(line); - if (!vMatch) continue; - const nMatch = nounRe.exec(line); - if (!nMatch) continue; - const verb = (vMatch[2] || '').toLowerCase(); - const noun = (nMatch[2] || '').toLowerCase(); - const key = `${verb}+${noun}`; - if (seen.has(key)) continue; - seen.add(key); - signals.push({ verb, noun, snippet: makeSnippet(line, noun) }); + const hasCompoundTerms = effective.verbs.length > 0 && effective.nouns.length > 0; + // Trailing boundary is a LOOKAHEAD (not consumed) so back-to-back terms + // separated by one boundary char are both found. + const verbRe = hasCompoundTerms + ? new RegExp( + '(^|[^a-zA-Z0-9])(' + effective.verbs.map(escapeRegex).join('|') + ')(?=[^a-zA-Z0-9]|$)', + 'gi', + ) + : null; + const nounRe = hasCompoundTerms + ? new RegExp( + '(^|[^a-zA-Z0-9])(' + effective.nouns.map(escapeRegex).join('|') + ')(?=[^a-zA-Z0-9]|$)', + 'gi', + ) + : null; + const surfaceRe = new RegExp(SERVICE_SURFACE_API_RE.source, 'g'); + + const nounSet = new Set(effective.nouns); + + const emitPair = (vTerm: string, nTerm: string, snippetLine: string): void => { + const key = `${vTerm}+${nTerm}`; + if (seen.has(key)) return; + seen.add(key); + signals.push({ verb: vTerm, noun: nTerm, snippet: makeSnippet(snippetLine, nTerm) }); + }; + + for (const rawLine of lines) { + // Inline code spans are code, not prose — mask them (length-preserving so + // offsets keep lining up), but keep package-shaped span content as noun + // evidence (#2365 review FN-4: `stripe-sdk` names a dependency). + const inlineSpans = scanInlineCodeSpans(rawLine); + let line = rawLine; + const spanNouns: TermMatch[] = []; + for (const s of inlineSpans) { + line = line.slice(0, s.start) + ' '.repeat(s.end - s.start) + line.slice(s.end); + const content = s.content.trim(); + if (content.length === 0 || /\s/.test(content)) continue; + const segs = content.toLowerCase().split(/[^a-z0-9]+/).filter(Boolean); + if (segs.length < 2) continue; // a bare `api` span is a code identifier + const hit = segs.find((seg) => nounSet.has(seg)); + if (hit) spanNouns.push({ term: hit, start: s.start, end: s.end }); + } + + // Path-shaped tokens (routes, file names, URLs) are references, not prose. + const { masked, urlNouns } = scanLineTokens(line, nounRe, nounSet); + const clauses = splitClauses(masked); + const extraNouns = urlNouns.concat(spanNouns); + + // (a) compound verb+noun — SAME CLAUSE ONLY. There is no word-gap cap (a cap + // cannot tell a long genuine clause from a long internal one) and no + // cross-clause binding (see the note by CLAUSE_BOUNDARY_CHARS): the clause + // boundary is the whole relationship test. Nouns are NOT filtered on + // "internal" qualification here — "integrate the internal API" is a + // fail-closed positive; the declaration dismisses it if wrong. + if (verbRe && nounRe) { + for (const clause of clauses) { + const verbs = collectTermMatches(verbRe, clause.text); + if (verbs.length === 0) continue; + const nouns = collectTermMatches(nounRe, clause.text); + const nounTerms = new Set(nouns.map((t) => t.term)); + for (const u of extraNouns) { + if (u.start >= clause.start && u.end <= clause.start + clause.text.length) { + nounTerms.add(u.term); + } + } + if (nounTerms.size === 0) continue; + for (const vTerm of new Set(verbs.map((t) => t.term))) { + for (const nTerm of nounTerms) emitPair(vTerm, nTerm, rawLine); + } + } + } + + // (b) explicit API|SDK|REST|GraphQL surface — scan every candidate + // in every clause (a rejected first candidate must not shadow a later + // genuine service; #2365 review C-1). + for (const clause of clauses) { + surfaceRe.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = surfaceRe.exec(clause.text)) !== null) { + const svc = m[1] || ''; + const svcLower = svc.toLowerCase(); + // Reject capitalized sentence starters ("The API"), locality/protocol + // descriptors ("Internal API", "REST API"), compound modifiers + // ("Resolver-only API"), and services qualified first-party + // ("internal Payments API"). A real vendor name is none of these. + if (SERVICE_STOPWORDS.has(svcLower)) continue; + if (SURFACE_DESCRIPTOR_WORDS.has(svcLower)) continue; + if (COMPOUND_MODIFIER_RE.test(svc)) continue; + if (isInternallyQualified(masked, clause.start + m.index)) continue; + const noun = (m[2] || '').toLowerCase(); + const key = `surface+${noun}`; + if (seen.has(key)) continue; + seen.add(key); + signals.push({ verb: '(surface)', noun, snippet: makeSnippet(rawLine, svc) }); + } } } - // (b) explicit API|SDK|REST|GraphQL surface. - for (const line of lines) { - SERVICE_SURFACE_API_RE.lastIndex = 0; - const m = SERVICE_SURFACE_API_RE.exec(line); - if (!m) continue; - // Reject ordinary capitalized sentence starters ("The API …", "Our REST …"). - if (SERVICE_STOPWORDS.has((m[1] || '').toLowerCase())) continue; - const noun = (m[2] || '').toLowerCase(); - const key = `surface+${noun}`; - if (seen.has(key)) continue; - seen.add(key); - signals.push({ verb: '(surface)', noun, snippet: makeSnippet(line, m[1]) }); - } - return { detected: signals.length > 0, signals, terms: effective }; } + +/** True when the word IMMEDIATELY ADJACENT before `offset` is a locality + * descriptor ("internal Payments API") — first-party qualification is negative + * evidence for an EXTERNAL-API signal. Only plain spaces/tabs may separate the + * descriptor from the service: any intervening punctuation means the descriptor + * belongs to a prior clause/sentence and must NOT qualify ("The cache is + * private. Stripe API …" — `private` is a different sentence; #2365 review). + * Looks back through a BOUNDED window, not the whole prefix, to stay linear. */ +const QUALIFIER_LOOKBACK = 24; // longest descriptor ("first-party") + separators +function isInternallyQualified(masked: string, offset: number): boolean { + const from = offset > QUALIFIER_LOOKBACK ? offset - QUALIFIER_LOOKBACK : 0; + const window = masked.slice(from, offset); + // Only whitespace and markdown emphasis/wrapper markers (`*_~\`) may separate + // the descriptor from the service, so "The **internal** Payments API" still + // qualifies — but NOT a clause/sentence boundary, so "…is private. Stripe API" + // does not (the descriptor is a different sentence; #2365 review). + const m = /([A-Za-z0-9'-]+)[\s*_~`]*$/.exec(window); + if (!m) return false; + // A word truncated by the window start is not a descriptor match (its real + // start lies before the window) — fail toward detection. + if (from > 0 && m.index === 0 && /[A-Za-z0-9'-]/.test(masked[from - 1])) return false; + return INTERNAL_DESCRIPTORS.has(m[1].toLowerCase()); +} + // ─── Coverage matrix parse / validate / render ──────────────────────────────── export type CoverageDecision = 'INTEGRATE' | 'OPT-OUT'; @@ -273,18 +562,40 @@ export interface CoverageRow { reason: string; } +/** #2365 acceptance #5: a first-class "this phase integrates no external API" + * declaration — the legitimate alternative to fabricating a matrix row for a + * capability that does not exist. Like an OPT-OUT row, it must carry a + * reason: the declaration is a reasoned decision, not a bypass. */ +export interface CoverageNoneDeclaration { + none: true; + reason: string; +} + export interface CoverageParseResult { rows: CoverageRow[]; errors: string[]; format: 'table' | 'json' | 'none'; + declaration: CoverageNoneDeclaration | null; } export interface CoverageValidationResult { valid: boolean; errors: string[]; counts: { surface: number; integrate: number; optout: number }; + /** True when a valid no-integration declaration (and no rows) satisfied the gate. */ + none_declared?: boolean; } +/** Matches a declaration line such as + * `No external API integration: ` (also `**bold**` and em-dash + * separators). The reason is REQUIRED — a bare declaration does not parse. + * Deliberately NOT matched: blockquoted lines (`> No external …` is quoted + * text, not a declaration) and anything inside fenced code or HTML comments + * (both stripped before the scan; #2365 review C-3). */ +const NO_INTEGRATION_DECLARATION_RE = + /^\s*(?:\*\*)?no external api integration(?:\*\*)?\s*(?:[:—–-]|--)\s*(\S[^\n]*)$/im; +const HTML_COMMENT_RE = //g; + const VALID_DECISIONS = new Set(['INTEGRATE', 'OPT-OUT']); /** @@ -305,10 +616,20 @@ const VALID_DECISIONS = new Set(['INTEGRATE', 'OPT-OUT']); * `{ rows: [], errors: [], format: 'none' }` for empty/non-matrix input. */ export function parseCoverageMatrix(text: unknown): CoverageParseResult { - const out: CoverageParseResult = { rows: [], errors: [], format: 'none' }; + const out: CoverageParseResult = { rows: [], errors: [], format: 'none', declaration: null }; if (typeof text !== 'string') return out; const src = text.replace(/\r\n/g, '\n'); + // #2365 acceptance #5: a "no external API integration" declaration. Scanned + // on fence-stripped, comment-stripped text so an example inside a code block + // or an HTML comment does not count. + const declMatch = NO_INTEGRATION_DECLARATION_RE.exec( + stripFencedCode(src).text.replace(HTML_COMMENT_RE, ''), + ); + if (declMatch) { + out.declaration = { none: true, reason: (declMatch[1] || '').trim() }; + } + // (1) fenced ```coverage JSON block takes precedence if present. // Case-insensitive info string (```coverage and ```Coverage are both legal CommonMark). const fenceBody = extractFencedBlock(src, 'coverage'); @@ -410,6 +731,28 @@ export function validateCoverageMatrix(text: unknown): CoverageValidationResult const errors = [...parsed.errors]; const rows = parsed.rows; + // #2365 acceptance #5: a reasoned no-integration declaration with no rows + // satisfies the gate. A declaration ALONGSIDE rows is contradictory — the + // file must say one thing. + if (parsed.declaration) { + if (rows.length > 0) { + errors.push( + 'declares "no external API integration" but also contains coverage rows — remove the declaration or the rows', + ); + } else { + if (parsed.declaration.reason.length > REASON_MAX_LEN) { + errors.push(`declaration reason exceeds ${REASON_MAX_LEN} chars`); + } + const valid = errors.length === 0; + return { + valid, + errors, + counts: { surface: 0, integrate: 0, optout: 0 }, + none_declared: valid, + }; + } + } + if (rows.length === 0) { if (errors.length === 0) errors.push('matrix is empty — no capabilities enumerated'); return { valid: false, errors, counts: { surface: 0, integrate: 0, optout: 0 } }; diff --git a/src/check-command-router.cts b/src/check-command-router.cts index a180eb0cc..613ab1b77 100644 --- a/src/check-command-router.cts +++ b/src/check-command-router.cts @@ -1144,6 +1144,42 @@ function cmdApiCoverageVerifyPre(projectDir: string, args: string[], raw: boolea } const v = validateCoverageMatrix(matrixText); if (v.valid) { + if (v.none_declared) { + // The declaration is the human override for the detector — it PASSES + // even when detection fires (that is acceptance #5's point: the + // detector is fallible and the declaration is the reasoned overrule). + // But a contradiction must be VISIBLE, not silent: re-run detection + // over the phase scope and surface any signals it still finds + // (#2365 review S-1). + const declScope = readPhaseScope(projectDir, resolvedDir, phaseNumber); + const declDetection = detectApiIntegration(declScope.text); + const declSignals = declDetection.signals.map((s) => ({ verb: s.verb, noun: s.noun })); + // The declaration legitimately wins even over a read error (it is the + // human overrule), but if scope was incomplete we say so — the contract + // is that contradictions stay visible, not silent (#2365 review). + const baseMsg = declDetection.detected + ? `api-coverage: COVERAGE.md declares no external API integration, overriding ${declSignals.length} detected signal(s) — confirm the declaration is accurate` + : 'api-coverage: COVERAGE.md declares no external API integration — matrix not required'; + output( + { + block: false, + passed: true, + coverage_present: true, + matrix: coverageFile, + counts: v.counts, + none_declared: true, + detected: declDetection.detected, + ...(declDetection.detected ? { signals: declSignals } : {}), + ...(declScope.readError ? { scope_read_error: declScope.readError } : {}), + message: declScope.readError + ? `${baseMsg} (note: phase scope was incompletely read — ${declScope.readError})` + : baseMsg, + }, + raw, + undefined, + ); + return; + } output( { block: false, @@ -1191,8 +1227,27 @@ function cmdApiCoverageVerifyPre(projectDir: string, args: string[], raw: boolea } // (2) no matrix — detect whether this phase integrates an external API. - const scopeText = readPhaseScope(projectDir, resolvedDir, phaseNumber); - const detection = detectApiIntegration(scopeText); + const scope = readPhaseScope(projectDir, resolvedDir, phaseNumber); + if (scope.readError) { + // Fail-closed: an unreadable plan could be the one describing the + // integration, so we cannot certify "no integration" — block and surface it. + output( + { + block: true, + passed: false, + coverage_present: false, + detected: false, + message: + `api-coverage: could not read the phase scope (${scope.readError}); ` + + 'refusing to certify no external-API integration from incomplete scope. ' + + 'Fix the unreadable plan file, or add a COVERAGE.md declaration.', + }, + raw, + undefined, + ); + return; + } + const detection = detectApiIntegration(scope.text); if (detection.detected) { // Surface only verb/noun (typed, bounded) — NOT raw prose snippets — so the // gate output cannot relay injected PLAN.md instructions to the orchestrator. @@ -1235,8 +1290,27 @@ function cmdApiCoverageVerifyPre(projectDir: string, args: string[], raw: boolea * whole roadmap, which would cross-contaminate sibling phases). Strips nothing * here — detectApiIntegration strips fenced code itself. */ -function readPhaseScope(projectDir: string, phaseDir: string, phaseNumber: string): string { +interface PhaseScopeRead { + text: string; + /** Non-null when a plan file EXISTED but could not be read. The gate must not + * conclude "no external API integration" from provably incomplete scope — an + * unreadable plan could be the one describing the integration (#2365 review: + * the blocking consumer silently passed partially-read scope). A missing plan + * directory is NOT a read error (a phase may legitimately have no plans yet). */ + readError: string | null; +} + +/** A filesystem error that is NOT "does not exist" — i.e. a real read failure + * (EACCES/EIO/…) the gate must not swallow. `ENOENT` is a legitimate "not + * there yet" and is treated as absence, not error. */ +function isRealReadFailure(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + return err != null && code !== 'ENOENT'; +} + +function readPhaseScope(projectDir: string, phaseDir: string, phaseNumber: string): PhaseScopeRead { const chunks: string[] = []; + let readError: string | null = null; try { const entries = fs.readdirSync(phaseDir, { withFileTypes: true }); const plans = entries @@ -1244,25 +1318,47 @@ function readPhaseScope(projectDir: string, phaseDir: string, phaseNumber: strin .map((e) => e.name) .sort(); for (const p of plans) { - chunks.push(fs.readFileSync(path.join(phaseDir, p), 'utf8')); + try { + chunks.push(fs.readFileSync(path.join(phaseDir, p), 'utf8')); + } catch (err) { + // A plan file that exists but cannot be read — record it and keep + // reading the rest so the message names the first failure. + if (!readError) { + readError = `could not read ${p}: ${err instanceof Error ? err.message : String(err)}`; + } + } + } + } catch (err) { + // A MISSING phase directory is fine (no plans yet → fall through to the + // roadmap). A directory that exists but cannot be enumerated (EACCES/EIO) + // is a real read failure the gate must not silently pass (#2365 review). + if (isRealReadFailure(err)) { + return { + text: '', + readError: `could not read the phase directory: ${err instanceof Error ? err.message : String(err)}`, + }; } - } catch { - // ignore — fall through to roadmap } - if (chunks.join('').trim().length > 0) return chunks.join('\n\n'); + if (readError) return { text: chunks.join('\n\n'), readError }; + if (chunks.join('').trim().length > 0) return { text: chunks.join('\n\n'), readError: null }; // Fallback: ONLY this phase's ROADMAP section (not the whole file, which - // would pollute detection with sibling-phase prose). Best-effort; absence or - // an unresolvable section is non-fatal (detector returns not-detected). + // would pollute detection with sibling-phase prose). A MISSING roadmap/section + // is non-fatal; a roadmap that exists but cannot be read is a real failure. if (phaseNumber) { try { const section = getRoadmapPhaseWithFallback(projectDir, phaseNumber); - if (section) return section; - } catch { - // ignore + if (section) return { text: section, readError: null }; + } catch (err) { + if (isRealReadFailure(err)) { + return { + text: '', + readError: `could not read the roadmap fallback: ${err instanceof Error ? err.message : String(err)}`, + }; + } } } - return ''; + return { text: '', readError: null }; } function routeCheckCommand({ args, cwd, raw }: RouteCheckCommandOptions): void { @@ -1354,4 +1450,7 @@ export = { cmdCheckPredicate, buildPredicateDeps, parsePredicateFlags, + // Fail-closed phase-scope reader for the api-coverage gate — exported for + // in-process failure-injection tests (#2365 review). + readPhaseScope, }; diff --git a/src/markdown-sectionizer.cts b/src/markdown-sectionizer.cts index c279ebddd..b72021900 100644 --- a/src/markdown-sectionizer.cts +++ b/src/markdown-sectionizer.cts @@ -157,6 +157,115 @@ export function stripFencedCode(content: string): StripFencedResult { return { text: kept.join('\n'), unterminatedFence: openFence !== null }; } +// ─── stripInlineCode ────────────────────────────────────────────────────────── + +/** + * Remove CommonMark inline code spans (§6.1) from prose, line by line. + * + * A span opens with a run of N backticks and closes at the next run of EXACTLY + * N backticks on the same line (a longer or shorter run is span content, per + * CommonMark). The whole span — delimiters and content — is replaced by a + * single space so the surrounding words do not join. A run with no matching + * closer is literal text and is kept. Spans never cross line boundaries here: + * multi-line code in planning prose is fenced-block territory + * (`stripFencedCode`). + * + * Companion to `stripFencedCode` for term-matching callers (#2365): strip + * fenced blocks first, then inline spans, so a trigger term inside backticks + * is code, not prose evidence. + */ +export function stripInlineCode(content: string): string { + if (typeof content !== 'string' || content.length === 0) return ''; + return content.split('\n').map(stripInlineCodeLine).join('\n'); +} + +/** An inline code span located by `scanInlineCodeSpans`: [start, end) covers + * the WHOLE span including both backtick delimiters; `content` is the inner + * text between them. */ +export interface InlineCodeSpan { + start: number; + end: number; + content: string; +} + +/** + * Locate every inline code span in `content`, per line (offsets are into the + * full string; spans never cross a `\n`). Callers that need the span CONTENT + * (e.g. api-coverage's dependency-evidence scan, #2365) use this; callers that + * just want spans gone use `stripInlineCode`. + */ +export function scanInlineCodeSpans(content: string): InlineCodeSpan[] { + if (typeof content !== 'string' || content.length === 0) return []; + const out: InlineCodeSpan[] = []; + let lineStart = 0; + for (const line of content.split('\n')) { + for (const s of scanSpansInLine(line)) { + out.push({ start: lineStart + s.start, end: lineStart + s.end, content: s.content }); + } + lineStart += line.length + 1; + } + return out; +} + +function scanSpansInLine(line: string): InlineCodeSpan[] { + const spans: InlineCodeSpan[] = []; + if (line.indexOf('`') === -1) return spans; + // Collect the maximal backtick RUNS once, then match openers to closers using + // a per-length forward cursor. A naive "search the rest of the line for the + // closer" loop is O(n²) on a line of many unmatched increasing-length runs + // (#2365 review 9); precomputing runs makes the whole scan linear while + // preserving CommonMark semantics (closer = next run of EXACTLY the same len). + const runs: Array<[number, number]> = []; + for (let i = 0; i < line.length; ) { + if (line[i] === '`') { + let n = 1; + while (i + n < line.length && line[i + n] === '`') n++; + runs.push([i, n]); + i += n; + } else { + i++; + } + } + const runsByLen = new Map(); + for (let k = 0; k < runs.length; k++) { + const len = runs[k][1]; + const arr = runsByLen.get(len); + if (arr) arr.push(k); + else runsByLen.set(len, [k]); + } + const cursorByLen = new Map(); + let k = 0; + while (k < runs.length) { + const [openPos, n] = runs[k]; + const candidates = runsByLen.get(n)!; // n came from this map, always present + let ci = cursorByLen.get(n) ?? 0; + while (ci < candidates.length && candidates[ci] <= k) ci++; + if (ci < candidates.length) { + const closeK = candidates[ci]; + const closePos = runs[closeK][0]; + spans.push({ start: openPos, end: closePos + n, content: line.slice(openPos + n, closePos) }); + cursorByLen.set(n, ci + 1); + k = closeK + 1; // resume after the closer — runs inside the span are code + } else { + cursorByLen.set(n, ci); + k++; // unmatched run → literal text, next run is a fresh opener + } + } + return spans; +} + +function stripInlineCodeLine(line: string): string { + const spans = scanSpansInLine(line); + if (spans.length === 0) return line; + let out = ''; + let prev = 0; + for (const s of spans) { + out += line.slice(prev, s.start) + ' '; + prev = s.end; + } + return out + line.slice(prev); +} + // ─── extractFencedBlock ─────────────────────────────────────────────────────── /** A fenced code block located by `scanFencedBlocks`: line-index span + info string. */ diff --git a/src/roadmap.cts b/src/roadmap.cts index 87e48977a..add1d861c 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -210,9 +210,17 @@ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: s function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | null { if (/^999(?:\.|$)/.test(stripProjectCodePrefix(phaseNum))) return null; const roadmapPath = planningPaths(cwd).roadmap; - if (!fs.existsSync(roadmapPath)) return null; - - const rawContent = fs.readFileSync(roadmapPath, 'utf-8'); + // Read directly rather than gating on fs.existsSync: existsSync returns false + // on EACCES/EIO too, which would mask an UNREADABLE roadmap as "missing" and + // let a blocking gate certify empty scope (#2365 review). Honor the documented + // contract — null only when genuinely absent (ENOENT), otherwise throw. + let rawContent: string; + try { + rawContent = fs.readFileSync(roadmapPath, 'utf-8'); + } catch (err) { + if ((err as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') return null; + throw err; + } const milestoneContent = extractCurrentMilestone(rawContent, cwd); const fullContent = stripShippedMilestones(rawContent); diff --git a/tests/api-coverage-gate-e2e.test.cjs b/tests/api-coverage-gate-e2e.test.cjs index 96f07167d..d5e94caee 100644 --- a/tests/api-coverage-gate-e2e.test.cjs +++ b/tests/api-coverage-gate-e2e.test.cjs @@ -22,6 +22,10 @@ const path = require('node:path'); const { execFileSync } = require('node:child_process'); const { cleanup } = require('./helpers.cjs'); +// In-process seam for the fail-closed read-injection tests at the bottom of this +// file (#2365 review): readPhaseScope is the pure phase-scope reader behind the +// gate. Those tests monkeypatch fs rather than drive a subprocess. +const { readPhaseScope } = require('../gsd-core/bin/lib/check-command-router.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -239,6 +243,49 @@ describe('api-coverage.verify-pre — seal contract (#1562 acceptance #1,#2,#4,# assert.strictEqual(j.counts.surface, 1); }); + // ── #2365: detector false positives must not block, and a phase may declare + // "no external API integration" instead of fabricating a matrix row. + test('#2365 phase naming a first-party route path → does NOT block', () => { + fresh(); + writePlan( + phaseDir, + '01-PLAN.md', + '# Plan\nRun integration tests for src/app/api/profile/route.test.ts.' + ); + const r = runGate(tmpDir, phaseDir); + assert.ok(r.success, `gate should succeed. stderr: ${r.error}`); + const j = JSON.parse(r.output); + assert.strictEqual(j.block, false, 'a first-party route path is not an external API'); + assert.strictEqual(j.detected, false); + }); + + test('#2365 COVERAGE.md declaring no external API integration → passes the gate', () => { + fresh(); + writePlan(phaseDir, '01-PLAN.md', '# Plan\nRender the export page.'); + writeCoverage(phaseDir, 'No external API integration: UI-only phase, no third-party surface.\n'); + const r = runGate(tmpDir, phaseDir); + assert.ok(r.success, `gate should succeed. stderr: ${r.error}`); + const j = JSON.parse(r.output); + assert.strictEqual(j.block, false, 'a reasoned no-integration declaration satisfies the gate'); + assert.strictEqual(j.coverage_present, true); + assert.strictEqual(j.none_declared, true); + assert.strictEqual(j.detected, false, 'a non-API plan shows no overridden signals'); + }); + + test('#2365 declaration overriding live detection passes but SURFACES the contradiction', () => { + fresh(); + writePlan(phaseDir, '01-PLAN.md', '# Plan\nIntegrate the Stripe API for payments.'); + writeCoverage(phaseDir, 'No external API integration: detector over-fired; this phase is UI-only.\n'); + const r = runGate(tmpDir, phaseDir); + assert.ok(r.success, `gate should succeed. stderr: ${r.error}`); + const j = JSON.parse(r.output); + assert.strictEqual(j.block, false, 'the declaration is the human overrule — it must win'); + assert.strictEqual(j.none_declared, true); + assert.strictEqual(j.detected, true, 'the contradiction must be visible, not silent'); + assert.ok(Array.isArray(j.signals) && j.signals.length > 0); + assert.ok(/overrid/i.test(j.message), `message should surface the override: ${j.message}`); + }); + // ── Security (#1562 security review S1/S2): the phase arg is taken only as a // token resolved under .planning/phases/. Traversal / unresolvable args must // NOT read files outside the phase dir, and — since the phases tree exists — @@ -271,3 +318,69 @@ describe('api-coverage.verify-pre — seal contract (#1562 acceptance #1,#2,#4,# } }); }); + +// ─── Fail-closed phase-scope read failures (in-process, #2365 review) ────────── +// These exercise readPhaseScope directly and inject the read failure by +// monkeypatching fs (restored in finally) rather than chmod 0o000 — chmod does +// not fault under root and is the pattern this repo's IO-failure convention +// avoids. Deterministic and platform-independent, so no root/win32 skip needed. +describe('readPhaseScope — fail-closed on a real read failure (#2365 review)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + // Run `fn` with `fs[method]` throwing `code` for any path matching `pat`, + // delegating to the real implementation otherwise; always restored. + function withFsThrow(method, pat, code, fn) { + const orig = fs[method]; + fs[method] = (p, ...rest) => { + if (typeof p === 'string' && pat.test(p)) { + const err = new Error(`${code}: injected read failure`); + err.code = code; + throw err; + } + return orig(p, ...rest); + }; + try { return fn(); } finally { fs[method] = orig; } + } + + test('an EXISTING plan file that cannot be read → readError set (not silent-empty)', () => { + tmpDir = makeProject({ api_coverage_gate: true }); + const phaseDir = makePhaseDir(tmpDir, '01-pay'); + writePlan(phaseDir, '01-PLAN.md', '# Plan\nRefactor the UI.'); + writePlan(phaseDir, '02-PLAN.md', '# Plan\nIntegrate the Stripe API.'); + const res = withFsThrow('readFileSync', /02-PLAN\.md$/, 'EACCES', () => + readPhaseScope(tmpDir, phaseDir, '01')); + assert.ok(res.readError, 'a real plan read failure must set readError, not read as empty scope'); + assert.match(res.readError, /could not read/i); + }); + + test('a phase directory that cannot be enumerated → readError set', () => { + tmpDir = makeProject({ api_coverage_gate: true }); + const phaseDir = makePhaseDir(tmpDir, '01-pay'); + writePlan(phaseDir, '01-PLAN.md', '# Plan\nIntegrate the Stripe API.'); + const res = withFsThrow('readdirSync', new RegExp(phaseDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '$'), 'EACCES', () => + readPhaseScope(tmpDir, phaseDir, '01')); + assert.ok(res.readError, 'an unreadable phase directory must set readError, not read as empty'); + }); + + test('roadmap fallback that cannot be read → readError set', () => { + tmpDir = makeProject({ api_coverage_gate: true }); + const phaseDir = makePhaseDir(tmpDir, '01-pay'); // no plans → roadmap fallback + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n### Phase 01: Pay\n\nIntegrate the Stripe API.\n', + 'utf8' + ); + const res = withFsThrow('readFileSync', /ROADMAP\.md$/, 'EACCES', () => + readPhaseScope(tmpDir, phaseDir, '01')); + assert.ok(res.readError, 'an unreadable roadmap fallback must set readError, not read as absent'); + }); + + test('a MISSING phase dir / roadmap is legitimate absence (ENOENT) → readError null', () => { + tmpDir = makeProject({ api_coverage_gate: true }); + const missing = path.join(tmpDir, '.planning', 'phases', '99-does-not-exist'); + const res = readPhaseScope(tmpDir, missing, '99'); + assert.strictEqual(res.readError, null, 'ENOENT is absence, not a read failure — must not block'); + assert.strictEqual(res.text, ''); + }); +}); diff --git a/tests/api-coverage.test.cjs b/tests/api-coverage.test.cjs index 68dd49868..21793d117 100644 --- a/tests/api-coverage.test.cjs +++ b/tests/api-coverage.test.cjs @@ -167,6 +167,336 @@ describe('detectApiIntegration — pure detector (#1562)', () => { }); }); +// ────────────────────────────────────────────────────────────────────────────── +// #2365 — detector false positives (first-party paths, unrelated same-line +// clauses, descriptive "API" prose) + the no-integration declaration. +// ────────────────────────────────────────────────────────────────────────────── + +describe('#2365 detector false positives + no-integration declaration', () => { + let mod; + try { + mod = require(MODULE_PATH); + } catch (err) { + throw new Error(`Could not require ${MODULE_PATH}. Run "npm run build:lib". Underlying: ${err.message}`); + } + const { detectApiIntegration, parseCoverageMatrix, validateCoverageMatrix } = mod; + + // ── acceptance #1: first-party framework route paths are not integration prose + for (const [label, scope] of [ + ['Next.js route file in prose', 'Run integration tests for src/app/api/profile/route.test.ts'], + ['route handler path with verb', 'Wire the src/app/api/profile/route.ts handler into the settings page'], + ['inline-code span', 'Verify the `api` helper wiring end to end'], + ]) { + test(`NEGATIVE path/inline-code (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, false, `unexpected detection for [${label}]: ${scope}`); + }); + } + + // ── acceptance #2: verb + noun in unrelated clauses of one line + test('NEGATIVE unrelated clauses: verb and noun in different clauses do not compound', () => { + const r = detectApiIntegration( + 'Render the page and prove label endpoint, filename, and CSV/XLSX wiring.' + ); + assert.strictEqual(r.detected, false); + }); + + // ── acceptance #3: descriptive/local "API" prose (threat-model shape). + // NOTE: the detector is FAIL-CLOSED — the classes below stay clean because + // they are unambiguously NOT external integration (no integration verb + a + // named service, or a first-party-qualified surface). Prose that pairs an + // integration VERB with an API noun ("wire … the internal endpoint") is a + // fail-closed POSITIVE now (see the "#2365 review — fail-open fixes" group); + // a one-line COVERAGE.md declaration dismisses it if it is a false alarm. + for (const [label, scope] of [ + ['threat-model table cell', '| Tampering | Resolver-only API rejects arbitrary caller URLs. |'], + ['compound-modifier mid-sentence', 'The Resolver-only API rejects arbitrary caller URLs.'], + ['clause-initial capitalized prose', 'Internal API surface stays unchanged in this phase.'], + ['localhost URL', 'Run integration tests against https://localhost:3000/api/profile'], + ['bare external domain, no path', 'Integrate the design tokens from https://example.com into the theme'], + ['internal-qualified service (no verb)', 'The internal Payments API remains unchanged.'], + ['descriptor service + unrelated URL', 'Internal API surface stays unchanged; see https://example.com/style-guide.'], + ['Windows path', 'Wire tests for src\\app\\api\\profile\\route.ts.'], + ['loopback shorthand URL', 'Connect tests to http://127.1:3000/api/profile.'], + ['protocol-only surface', 'Document the REST API behavior for maintainers.'], + ['protocol-only surface (GraphQL)', 'Review the GraphQL API schema naming conventions.'], + ['cross-clause coordinate action', 'Wire the header, then update the endpoint docs'], + ]) { + test(`NEGATIVE descriptive API prose (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, false, `unexpected detection for [${label}]: ${scope}`); + }); + } + + // ── acceptance #4: true positives preserved (the fail-open guard — a fix that + // silences these is strictly worse than the false positives it removes). + for (const [label, scope] of [ + ['canonical compound', 'integrate the Stripe API'], + ['compound with trailing prose', 'Integrate the Stripe API for payment processing'], + ['surface rule, no verb', 'Add a Spotify API client'], + ['widest default-suite word gap', 'Consume the billing service over gRPC'], + ['clause-initial service + URL corroboration', 'Stripe API — docs at https://stripe.com/docs/api'], + ['webhook compound', 'Wire up the Slack webhook for deploy notifications'], + ['verb + API-naming URL', 'Connect the app to https://api.stripe.com/v1 for charges'], + ['slashed noun shorthand', 'Integrate the Stripe API/SDK for payments'], + ['long single-clause gap', "Connect our checkout to Stripe's hosted payment processing service through its v1 endpoints."], + ['non-http URI scheme', 'Connect the realtime client to wss://api.openai.com/v1/realtime.'], + ['versioned noun shorthand', 'Integrate Stripe API/v2 for legacy payments.'], + ['clause-initial service + object follower', 'Stripe API client for payments.'], + ['inline-code package corroboration', 'Stripe SDK client via `@stripe/stripe-js` for payment intents.'], + ['inline-code package as only noun', 'Integrate `stripe-sdk` for payment intents.'], + ['later surface after rejected first candidate', 'Internal API facade around Stripe SDK payment flows.'], + ['later surface after rejected modifier', 'Resolver-only API facade delegates to Stripe SDK for payments.'], + ]) { + test(`POSITIVE still fires (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, true, `true-positive regression [${label}]: ${scope}`); + }); + } + + // ── #2365 review — fail-open fixes. Codex's second-round review found the + // round-2 tightening had over-corrected into FAIL-OPEN false negatives: + // realistic external-API prose that a BLOCKING gate silently let through. + // Under the fail-closed decision these MUST detect. This is the guard the + // handoff flagged in bold — a fix that lets these slip is strictly worse + // than the false positives it removes. + for (const [label, scope] of [ + ['clause-initial service, plain follower (F1)', 'Stripe API for payment processing.'], + ['external host that names an API vocab word (F2)', 'Connect the client to api.stripe.com/v1 for charges.'], + ["vendor's first-party SDK (F3)", "Integrate Shopify's first-party SDK for checkout."], + ['long single integration clause (F4)', "Integrate Stripe's hosted payment processing service into checkout using the vendor-recommended asynchronous flow for recurring subscriptions and one-time card payments through its API."], + // Fail-closed reversal of the round-2 "internal" negatives: an integration + // verb bound to an API noun detects even when the noun is "internal"-qualified + // (Codex: "internal" can name the vendor's own API). Dismissed by declaration. + ['integration verb + internal noun', 'Wire the settings form to the internal endpoint.'], + ['coordinated integration verb + internal noun', 'Wire the form and document the internal API.'], + ['distant same-clause verb+noun', 'Wiring the settings drawer means the profile page the sidebar and the account menu all reach the same internal endpoint'], + // Qualification must NOT leak across a sentence/clause boundary. + ['qualifier does not leak across a sentence', 'The cache is private. Stripe API client for payments.'], + ['qualifier does not leak across a semicolon', 'Keep the cache private; Stripe API client for payments.'], + ]) { + test(`POSITIVE fail-open guard (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, true, `fail-open regression [${label}]: ${scope}`); + }); + } + + // ── #2365 review — false-positive fixes. The reviews found false positives + // from over-broad heuristics; these MUST stay clean. + for (const [label, scope] of [ + ['bare external domain, no path (F6)', 'Integrate the design tokens from https://example.com, document the endpoint terminology.'], + ['internal UI component, separate action (F7)', 'Wire the SettingsForm, then document the endpoint props.'], + ['protocol name as service (F8)', 'Document the REST API behavior for maintainers.'], + ['finite continuation after a period', 'Wire the settings form. Document endpoint props.'], + ['finite continuation after a semicolon', 'Wire the settings form; document endpoint props.'], + ['finite continuation after a comma', 'Wire the form, document endpoint props.'], + // Round-4 review: an external asset/link URL is NOT an API endpoint. + ['external stylesheet asset URL', 'Wire stylesheet from https://cdn.example.com/assets/theme.css into the page.'], + ['external URL with a query string', 'Wire the login link to https://example.com?next=/dashboard.'], + ['external docs/repo link, not an API', 'Wire the docs link to https://github.com/org/repo into the footer'], + // Round-4 review: an "-ing"-SPELLED noun ("billing") is not a participle. + ['-ing-spelled noun in an unrelated clause', 'Wire the new settings form component, billing endpoint terminology remains unchanged.'], + // Round-4 review: qualification survives markdown emphasis. + ['descriptor qualifies through markdown emphasis', 'The **internal** Payments API remains unchanged.'], + ]) { + test(`NEGATIVE fail-closed FP guard (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, false, `new false positive [${label}]: ${scope}`); + }); + } + + // ── #2365 — DOCUMENTED fail-open LIMITATIONS. Detection is same-clause only + // (no cross-clause binding) and an external host is evidence only when it + // NAMES an API vocabulary word. Catching the cases below robustly needs a + // vendor dictionary + coreference, which the issue rules out in principle; + // every lexical rule tried across four review rounds traded a false + // negative for a false positive. These are cheaply covered by the + // COVERAGE.md declaration and rare in real phase prose. The tests pin the + // behavior as INTENTIONAL — a future maintainer re-adding a cross-clause or + // every-URL heuristic would reintroduce the false positives above. + for (const [label, scope] of [ + ['service named only in a following participial clause', 'Integrate Stripe, exposing its endpoints for payment capture.'], + ['service named only in a following finite clause', 'Integrate Stripe; use its OAuth endpoints for checkout.'], + ['bare external host naming no vocab word', 'Connect the client to graph.microsoft.com:443/v1.0/me.'], + ]) { + test(`DOCUMENTED fail-open limitation stays clean (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, false, `limitation changed [${label}]: ${scope}`); + }); + } + + // ── #2365 review — DOCUMENTED fail-closed tradeoffs. A clause-initial + // capitalized common word before "API" ("Payment API", "Search API") is + // treated as a service name, and a long clause pairs a verb with a distant + // noun. Codex judged these acceptable because the COVERAGE.md declaration + // is a cheap override; these tests exist so the behavior is INTENTIONAL and + // a future maintainer does not "fix" it back into a fail-open cap. + for (const [label, scope] of [ + ['capitalized common word as service', 'Payment API remains unchanged in this refactor.'], + ['capitalized common word as service (Search)', 'Search API types are generated locally.'], + ['long clause pairs verb with distant noun', 'Wire the settings form to validation state so the designer can review field behavior and document every public API and endpoint symbol without changing runtime dependencies.'], + ]) { + test(`POSITIVE documented fail-closed tradeoff (${label}): "${scope}"`, () => { + const r = detectApiIntegration(scope); + assert.strictEqual(r.detected, true, `expected documented fail-closed detection [${label}]: ${scope}`); + }); + } + + // ── #2365 review finding 7: inline code spans are matched WITHIN a line by + // design (phase scope prose is line-oriented). A CommonMark code span that + // wraps a newline is NOT recognized, so its contents are treated as prose — + // a documented, narrow limitation (fail-closed: a stray detection is + // dismissed by the declaration). This test pins the current behavior. + test('multi-line inline code span is not treated as code (documented limitation)', () => { + const r = detectApiIntegration('Documentation example: `integrate\nStripe API` only.'); + assert.strictEqual(r.detected, true); + }); + + // ── acceptance #5: a legitimate, non-fabricated "no external API" declaration + test('declaration-only COVERAGE.md is VALID with zero rows (none_declared)', () => { + const md = '# API Coverage\n\nNo external API integration: UI-only phase, no third-party surface.\n'; + const v = validateCoverageMatrix(md); + assert.strictEqual(v.valid, true, `expected valid, errors: ${v.errors.join('; ')}`); + assert.strictEqual(v.none_declared, true); + assert.deepStrictEqual(v.counts, { surface: 0, integrate: 0, optout: 0 }); + }); + + test('declaration accepts the bold/em-dash form', () => { + const md = '**No external API integration** — resolver work is local-only.\n'; + const v = validateCoverageMatrix(md); + assert.strictEqual(v.valid, true); + assert.strictEqual(v.none_declared, true); + }); + + test('declaration WITHOUT a reason is not recognized (reasoned opt-out, like OPT-OUT rows)', () => { + const v = validateCoverageMatrix('No external API integration\n'); + assert.strictEqual(v.valid, false); + assert.notStrictEqual(v.none_declared, true); + }); + + test('declaration PLUS coverage rows is contradictory → invalid', () => { + const md = [ + 'No external API integration: nothing external here.', + '', + '| capability | decision | reason |', + '|---|---|---|', + '| search | INTEGRATE | |', + ].join('\n'); + const v = validateCoverageMatrix(md); + assert.strictEqual(v.valid, false); + assert.ok(v.errors.some((e) => /declar/i.test(e)), `errors: ${v.errors.join('; ')}`); + }); + + test('declaration inside a fenced code block is NOT recognized', () => { + const md = '```markdown\nNo external API integration: example only.\n```\n'; + const p = parseCoverageMatrix(md); + assert.notStrictEqual(p.declaration && p.declaration.none, true); + const v = validateCoverageMatrix(md); + assert.notStrictEqual(v.none_declared, true); + }); + + test('declaration inside an HTML comment is NOT recognized', () => { + const md = '\n'; + const v = validateCoverageMatrix(md); + assert.strictEqual(v.valid, false); + assert.notStrictEqual(v.none_declared, true); + }); + + test('blockquoted declaration is NOT recognized (quoted text is not a decision)', () => { + const v = validateCoverageMatrix('> No external API integration: copied from the old PLAN.md.\n'); + assert.strictEqual(v.valid, false); + assert.notStrictEqual(v.none_declared, true); + }); + + // ── A hostile line repeating one verb+noun pair thousands of times must + // collapse to a SINGLE signal — pairing is by distinct term, not a + // match×match cross product. Asserting the signal count is a deterministic + // proxy for that linearity (no wall-clock timing — Clock Seams rule). + test('hostile repeated-term line dedups to one signal', () => { + const s = 'integrate api '.repeat(10000); // 140 KB single line, 10k pairs + const r = detectApiIntegration(s); + assert.strictEqual(r.detected, true); + assert.strictEqual( + r.signals.length, + 1, + `repeated verb+noun pair must dedup to one signal, got ${r.signals.length}` + ); + }); +}); + +// ────────────────────────────────────────────────────────────────────────────── +// #2365 review — hardening-constant boundaries + parser fuzz property +// ────────────────────────────────────────────────────────────────────────────── + +describe('#2365 hardening-constant boundaries + parser property', () => { + const { detectApiIntegration, validateCoverageMatrix } = require(MODULE_PATH); + + // SERVICE_SURFACE_API_RE bounds the service token to `[A-Z][A-Za-z0-9_-]{1,40}` + // (2..41 chars) so a hostile hyphen run cannot drive O(n^2) backtracking. + test('surface service name at the 41-char limit still fires', () => { + const svc = 'S' + 'a'.repeat(40); // exactly 41 chars + assert.strictEqual(detectApiIntegration(`${svc} API`).detected, true); + }); + test('surface service name at 42 chars is past the length bound (surface path)', () => { + const svc = 'S' + 'a'.repeat(41); // 42 chars, no integration verb → surface-only + assert.strictEqual(detectApiIntegration(`${svc} API`).detected, false); + }); + + // QUALIFIER_LOOKBACK (24): a first-party descriptor only suppresses the surface + // within the bounded lookback window. Pin the EXACT constant: 8-char "internal" + // + 16 spaces places its start at offset-24 (the window edge) → still qualifies; + // + 17 spaces pushes its start one char outside → truncated → no longer + // qualifies. Both would pass for any lookback in ~9..37, so use the exact pair. + test('internal qualifier exactly at the 24-char window edge still suppresses the surface', () => { + const atEdge = 'internal' + ' '.repeat(16) + 'Payments API'; // start at offset-24 + assert.strictEqual(detectApiIntegration(atEdge).detected, false); + }); + test('internal qualifier one char past the 24-char window no longer qualifies (fires)', () => { + const pastEdge = 'internal' + ' '.repeat(17) + 'Payments API'; // start at offset-25 + assert.strictEqual(detectApiIntegration(pastEdge).detected, true); + }); + + // REASON_MAX_LEN (200): the no-integration declaration reason is length-bounded. + test('declaration reason at 200 chars is valid; 201 is rejected', () => { + const at = 'No external API integration: ' + 'x'.repeat(200) + '\n'; + const over = 'No external API integration: ' + 'x'.repeat(201) + '\n'; + assert.strictEqual(validateCoverageMatrix(at).valid, true); + const v = validateCoverageMatrix(over); + assert.strictEqual(v.valid, false); + assert.ok(v.errors.some((e) => /exceeds 200 chars/.test(e)), `errors: ${v.errors.join('; ')}`); + }); + + // Fuzz the tokenizer / clause splitter / masking (scanLineTokens, splitClauses, + // collectTermMatches) with adversarial tokens — slashes, backticks, URLs, + // clause punctuation. The detector must never throw, keep its typed shape, hold + // `detected ⇔ signals.length > 0`, and be deterministic on any input. + test('property: parser is total, shape-stable, and deterministic on arbitrary prose', () => { + const token = fc.oneof( + fc.constantFrom( + 'integrate', 'connect', 'wire', 'the', 'Stripe', 'API', 'SDK', 'endpoint', + 'api', 'internal', 'Resolver-only', '/', '//', '`', 'https://api.x.com/v1', + 'src/app/api/x.ts', 'graph.microsoft.com/v1' + ), + fc.stringMatching(/^[A-Za-z0-9/.:`_-]{0,12}$/) + ); + fc.assert( + fc.property( + fc.array(token, { maxLength: 40 }), + fc.constantFrom(' ', ', ', '. ', '; ', ' | ', '\n'), + (words, sep) => { + const line = words.join(sep); + const r = detectApiIntegration(line); + assert.ok(typeof r.detected === 'boolean' && Array.isArray(r.signals), 'typed shape'); + assert.strictEqual(r.detected, r.signals.length > 0, 'detected ⇔ signals present'); + assert.deepStrictEqual(detectApiIntegration(line), r, 'deterministic'); + return true; + } + ), + { numRuns: 300 } + ); + }); +}); + // ────────────────────────────────────────────────────────────────────────────── // Matrix parse / validate / render // ────────────────────────────────────────────────────────────────────────────── diff --git a/tests/fixtures/golden-install-parity/antigravity.json b/tests/fixtures/golden-install-parity/antigravity.json index c9478f6f6..de7603d07 100644 --- a/tests/fixtures/golden-install-parity/antigravity.json +++ b/tests/fixtures/golden-install-parity/antigravity.json @@ -53,7 +53,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "51b5d0ba5b1e98d9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "425dd69c629230e7", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "e176817364a7cbf4", "gsd-core/references/autonomous-smart-discuss.md": "efd80aca449032ad", "gsd-core/references/checkpoints.md": "130bb6ef705fc065", diff --git a/tests/fixtures/golden-install-parity/augment.json b/tests/fixtures/golden-install-parity/augment.json index 142fcd784..1a0e8f9fc 100644 --- a/tests/fixtures/golden-install-parity/augment.json +++ b/tests/fixtures/golden-install-parity/augment.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/claude-local.json b/tests/fixtures/golden-install-parity/claude-local.json index 75110a3c2..3e8a3ace8 100644 --- a/tests/fixtures/golden-install-parity/claude-local.json +++ b/tests/fixtures/golden-install-parity/claude-local.json @@ -123,7 +123,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "251040866a3a1818", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/claude.json b/tests/fixtures/golden-install-parity/claude.json index a86024fb9..9a2042451 100644 --- a/tests/fixtures/golden-install-parity/claude.json +++ b/tests/fixtures/golden-install-parity/claude.json @@ -52,7 +52,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/cline.json b/tests/fixtures/golden-install-parity/cline.json index f5a9cf9c2..08447c7d3 100644 --- a/tests/fixtures/golden-install-parity/cline.json +++ b/tests/fixtures/golden-install-parity/cline.json @@ -56,7 +56,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "59f782556e4e611f", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "f66fb7d5b435682d", diff --git a/tests/fixtures/golden-install-parity/codebuddy.json b/tests/fixtures/golden-install-parity/codebuddy.json index 7fc54c9ee..7f1743258 100644 --- a/tests/fixtures/golden-install-parity/codebuddy.json +++ b/tests/fixtures/golden-install-parity/codebuddy.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/codex.json b/tests/fixtures/golden-install-parity/codex.json index f783aeece..7463f337b 100644 --- a/tests/fixtures/golden-install-parity/codex.json +++ b/tests/fixtures/golden-install-parity/codex.json @@ -159,7 +159,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "51b5d0ba5b1e98d9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "425dd69c629230e7", - "gsd-core/references/api-coverage.md": "524382216a8e713f", + "gsd-core/references/api-coverage.md": "3e87a58d0cd6518a", "gsd-core/references/artifact-types.md": "3218cafb0c92dc32", "gsd-core/references/autonomous-smart-discuss.md": "4156025334411073", "gsd-core/references/checkpoints.md": "d52116f59e92ca43", diff --git a/tests/fixtures/golden-install-parity/copilot.json b/tests/fixtures/golden-install-parity/copilot.json index 18eb0aec9..aacd2d6c8 100644 --- a/tests/fixtures/golden-install-parity/copilot.json +++ b/tests/fixtures/golden-install-parity/copilot.json @@ -54,7 +54,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "51b5d0ba5b1e98d9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "425dd69c629230e7", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "f992de8b2b1a4420", "gsd-core/references/autonomous-smart-discuss.md": "efd80aca449032ad", "gsd-core/references/checkpoints.md": "d26b11ab5ece9e61", diff --git a/tests/fixtures/golden-install-parity/cursor.json b/tests/fixtures/golden-install-parity/cursor.json index fa075d617..cec30ce9d 100644 --- a/tests/fixtures/golden-install-parity/cursor.json +++ b/tests/fixtures/golden-install-parity/cursor.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "3d62d178004db5cc", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "273b371c5751f35a", "gsd-core/references/checkpoints.md": "a44e66095240c46e", diff --git a/tests/fixtures/golden-install-parity/hermes.json b/tests/fixtures/golden-install-parity/hermes.json index 3ab603323..758207686 100644 --- a/tests/fixtures/golden-install-parity/hermes.json +++ b/tests/fixtures/golden-install-parity/hermes.json @@ -53,7 +53,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "892f086e846cd8e4", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "6e4b08a89c485c71", diff --git a/tests/fixtures/golden-install-parity/kilo.json b/tests/fixtures/golden-install-parity/kilo.json index 86a761430..539237700 100644 --- a/tests/fixtures/golden-install-parity/kilo.json +++ b/tests/fixtures/golden-install-parity/kilo.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "51b5d0ba5b1e98d9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "425dd69c629230e7", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "3986d58011bf9006", "gsd-core/references/checkpoints.md": "d52116f59e92ca43", diff --git a/tests/fixtures/golden-install-parity/kimi.json b/tests/fixtures/golden-install-parity/kimi.json index a28874155..0c3de0750 100644 --- a/tests/fixtures/golden-install-parity/kimi.json +++ b/tests/fixtures/golden-install-parity/kimi.json @@ -117,7 +117,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/opencode.json b/tests/fixtures/golden-install-parity/opencode.json index cc8edad76..a5caab037 100644 --- a/tests/fixtures/golden-install-parity/opencode.json +++ b/tests/fixtures/golden-install-parity/opencode.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "51b5d0ba5b1e98d9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "425dd69c629230e7", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "218c55caf8aff6df", "gsd-core/references/autonomous-smart-discuss.md": "3986d58011bf9006", "gsd-core/references/checkpoints.md": "d52116f59e92ca43", diff --git a/tests/fixtures/golden-install-parity/pi.json b/tests/fixtures/golden-install-parity/pi.json index 364ff4da0..7a1ea1d36 100644 --- a/tests/fixtures/golden-install-parity/pi.json +++ b/tests/fixtures/golden-install-parity/pi.json @@ -20,7 +20,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/golden-install-parity/qwen.json b/tests/fixtures/golden-install-parity/qwen.json index 32a4628f0..356b9ba8f 100644 --- a/tests/fixtures/golden-install-parity/qwen.json +++ b/tests/fixtures/golden-install-parity/qwen.json @@ -53,7 +53,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "be09755fed7ad856", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "5e2d923303f16667", diff --git a/tests/fixtures/golden-install-parity/trae.json b/tests/fixtures/golden-install-parity/trae.json index 45448f719..d35818ce3 100644 --- a/tests/fixtures/golden-install-parity/trae.json +++ b/tests/fixtures/golden-install-parity/trae.json @@ -53,7 +53,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "18d1ff7c7fa0bab1", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "4d24ce110ed6c746", diff --git a/tests/fixtures/golden-install-parity/windsurf.json b/tests/fixtures/golden-install-parity/windsurf.json index b6a31316d..935aeda15 100644 --- a/tests/fixtures/golden-install-parity/windsurf.json +++ b/tests/fixtures/golden-install-parity/windsurf.json @@ -53,7 +53,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "849977da771f2b06", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "205a43c5fa7c221c", + "gsd-core/references/api-coverage.md": "53d290a68f83c7d0", "gsd-core/references/artifact-types.md": "8bd01fd75a2ba70e", "gsd-core/references/autonomous-smart-discuss.md": "273b371c5751f35a", "gsd-core/references/checkpoints.md": "900530adaf675e38", diff --git a/tests/fixtures/golden-install-parity/zcode.json b/tests/fixtures/golden-install-parity/zcode.json index 80ec55f9c..006235b20 100644 --- a/tests/fixtures/golden-install-parity/zcode.json +++ b/tests/fixtures/golden-install-parity/zcode.json @@ -124,7 +124,7 @@ "gsd-core/references/agent-skills-bootstrap.md": "5ab875054b1adda9", "gsd-core/references/ai-evals.md": "b5afa786b938671e", "gsd-core/references/ai-frameworks.md": "f827de93dde124eb", - "gsd-core/references/api-coverage.md": "66264d41dfd9154a", + "gsd-core/references/api-coverage.md": "913654a1cdb3baf1", "gsd-core/references/artifact-types.md": "a6d2e1f9453ffbf5", "gsd-core/references/autonomous-smart-discuss.md": "2fc710cde0ec7785", "gsd-core/references/checkpoints.md": "c2fe89c42ca88349", diff --git a/tests/fixtures/representative/api-coverage-detector/MANIFEST.json b/tests/fixtures/representative/api-coverage-detector/MANIFEST.json index 80dc98a30..9c35e8987 100644 --- a/tests/fixtures/representative/api-coverage-detector/MANIFEST.json +++ b/tests/fixtures/representative/api-coverage-detector/MANIFEST.json @@ -1,29 +1,27 @@ { "gate": "api-coverage.verify-pre", "sourceIssue": "#2365", + "note": "#2365 is fixed (this PR). The three fixtures below no longer carry a currentBuggyOutput — per the tripwire contract in representative-corpus.test.cjs, the fixer flips the assertion to expectedDetected once the bug is gone. The detector is now fail-closed and same-clause; all four fixtures return detected:false for the right reason.", "fixtures": [ { "file": "nextjs-route-path.txt", "expectedDetected": false, - "currentBuggyOutput": { "detected": true, "signal": { "verb": "integration", "noun": "api" } }, - "note": "First-party Next.js route path. The noun-boundary class [^a-zA-Z0-9] treats '/' as a word boundary, so 'api' inside the path matches as if it were prose. Highest blast radius: any Next.js project names a route file." + "note": "First-party Next.js route path. Fixed: path-shaped tokens are masked before matching, so 'api' inside a route path is no longer read as prose. (Was: the noun-boundary class treated '/' as a word boundary — the highest-blast-radius false positive, since any Next.js project names a route file.)" }, { "file": "unrelated-verb-noun.txt", "expectedDetected": false, - "currentBuggyOutput": { "detected": true, "signal": { "verb": "wiring", "noun": "endpoint" } }, - "note": "'wiring' and 'endpoint' co-occur on one line in unrelated clauses, reverse semantic order, no integration described. The verb/noun regexes are independently exec'd over the whole line with no proximity or grammatical relation." + "note": "'wiring' and 'endpoint' co-occur on one line in unrelated clauses. Fixed: the verb and noun must share one clause, so co-occurrence across clause boundaries no longer fires. (Was: the verb/noun regexes were exec'd independently over the whole line with no proximity or grammatical relation.)" }, { "file": "threat-model-prose.txt", "expectedDetected": false, - "currentBuggyOutput": { "detected": true, "signal": { "verb": "(surface)", "noun": "api" } }, - "note": "Threat-model table cell describing a LOCAL interface. SERVICE_SURFACE_API_RE matches any capitalized word before API/SDK/REST/GraphQL; the stopword denylist cannot enumerate every ordinary English word that precedes 'API' in a sentence." + "note": "Threat-model table cell describing a LOCAL interface. Fixed: the API surface rule rejects locality/protocol descriptors and compound modifiers ('Resolver-only API'), so ordinary English before 'API' no longer reads as a service name. (Was: any capitalized word before API/SDK/REST/GraphQL matched, and a stopword denylist could not enumerate every such word.)" }, { "file": "non-integration-assertion.txt", "expectedDetected": false, - "note": "This line explicitly ASSERTS non-integration ('no new command/dependency') and is still read as an integration signal in the real $gsd-verify-work occurrence this was drawn from — but in isolation it already returns detected:false today (the multi-signal real occurrence needed the OTHER lines' signals to trip the gate). No currentBuggyOutput: this fixture already passes for the right reason." + "note": "This line explicitly ASSERTS non-integration ('no new command/dependency') and is still read as an integration signal in the real $gsd-verify-work occurrence this was drawn from — but in isolation it already returned detected:false before #2365 (the multi-signal real occurrence needed the OTHER lines' signals to trip the gate). Included for corpus completeness." } ] } diff --git a/tests/markdown-sectionizer.test.cjs b/tests/markdown-sectionizer.test.cjs index 698eab5ae..f7c3e26f2 100644 --- a/tests/markdown-sectionizer.test.cjs +++ b/tests/markdown-sectionizer.test.cjs @@ -4,9 +4,10 @@ * Behavioral tests for markdown-sectionizer.cjs * * Module: gsd-core/bin/lib/markdown-sectionizer.cjs - * Exports: stripFencedCode, extractFencedBlock, tokenizeHeadings, collectSections, - * collectSection, iterateBullets, updateBullet, extractTaggedBlocks, - * stripTaggedBlocks, replaceSection, withSection, deleteSection + * Exports: stripFencedCode, stripInlineCode, extractFencedBlock, tokenizeHeadings, + * collectSections, collectSection, iterateBullets, updateBullet, + * extractTaggedBlocks, stripTaggedBlocks, replaceSection, withSection, + * deleteSection * * Covers the parser QA matrix from CONTRIBUTING.md §'Parser and project-file inputs': * - LF vs CRLF line endings @@ -42,8 +43,70 @@ const { replaceSection, withSection, deleteSection, + stripInlineCode, + scanInlineCodeSpans, } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs'); +// ─── stripInlineCode (#2365) ────────────────────────────────────────────────── + +describe('stripInlineCode', () => { + test('removes a single-backtick inline span, delimiters included', () => { + assert.strictEqual(stripInlineCode('Verify the `api` helper'), 'Verify the helper'); + }); + + test('removes a double-backtick span (CommonMark same-length closer)', () => { + assert.strictEqual(stripInlineCode('see ``a `nested` tick`` here'), 'see here'); + }); + + test('an unmatched backtick run is left as literal text', () => { + assert.strictEqual(stripInlineCode('a ` stray backtick'), 'a ` stray backtick'); + }); + + test('a longer closing run does not close a shorter opener (CommonMark §6.1)', () => { + // `x`` — no run of exactly 1 backtick follows, so nothing is stripped. + assert.strictEqual(stripInlineCode('a `x`` b'), 'a `x`` b'); + }); + + test('multiple spans on one line are all removed', () => { + assert.strictEqual(stripInlineCode('`a` and `b` remain not'), ' and remain not'); + }); + + test('spans do not cross line boundaries', () => { + assert.strictEqual(stripInlineCode('open `here\nstill` open'), 'open `here\nstill` open'); + }); + + test('non-string / empty input degrades to empty string without throwing', () => { + assert.strictEqual(stripInlineCode(undefined), ''); + assert.strictEqual(stripInlineCode(null), ''); + assert.strictEqual(stripInlineCode(''), ''); + }); + + test('text without backticks is returned unchanged', () => { + const s = 'plain prose, no code spans at all'; + assert.strictEqual(stripInlineCode(s), s); + }); +}); + +describe('scanInlineCodeSpans', () => { + test('returns spans covering the delimiters with inner content', () => { + const spans = scanInlineCodeSpans('use `api` and ``x``'); + assert.deepStrictEqual(spans, [ + { start: 4, end: 9, content: 'api' }, + { start: 14, end: 19, content: 'x' }, + ]); + }); + + test('offsets are into the full multi-line string; spans never cross lines', () => { + const spans = scanInlineCodeSpans('a `x`\nno `span\nhere'); + assert.deepStrictEqual(spans, [{ start: 2, end: 5, content: 'x' }]); + }); + + test('non-string / empty input yields no spans', () => { + assert.deepStrictEqual(scanInlineCodeSpans(undefined), []); + assert.deepStrictEqual(scanInlineCodeSpans(''), []); + }); +}); + // ─── stripFencedCode ────────────────────────────────────────────────────────── describe('stripFencedCode', () => {