* fix(#2365): stop the api-coverage detector false-positiving non-API phases detectApiIntegration fired on any integration verb co-occurring anywhere on a line with any API noun, treated / as a word boundary (so a first-party Next.js src/app/api/... route path matched the noun "api"), and read any capitalized word before API/SDK/REST/GraphQL as a service name behind a fixed stopword denylist (so threat-model prose like "Resolver-only API" fired). Because the verify:pre seal gate is BLOCKING, a phase touching no external API could not reach UAT without fabricating a coverage matrix. The compound rule now requires the verb and noun to share one clause (sentence punctuation and table-cell walls end a clause) within a bounded word gap. Non-prose spans are excluded before matching: fenced code (already), inline code spans (new stripInlineCode in the markdown-sectionizer seam), and path-shaped tokens. The <Service> API surface rule requires proper-noun position — a clause-initial capitalized word is ordinary English and needs dependency evidence (URL / package reference) on the same line — and rejects compound modifiers ("Resolver-only", lowercase after the hyphen). A phase that integrates no external API now has a first-class, reasoned way to say so: a COVERAGE.md containing "No external API integration: <reason>" satisfies the gate (declaration + rows is contradictory and blocks). The true-positive path is pinned by regression tests: every default-vocabulary positive still fires, including the widest word-gap pairing and the surface-rule-only shape. Fixes #2365 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#2365): tighten api-coverage detector per Codex review (round 2) Applies the Codex review findings on the initial #2365 fix: - S-1: a COVERAGE.md "no external API integration" declaration is the human override for a fallible detector, so it must PASS even when detection still fires — but the contradiction is now SURFACED in the gate output (overridden signal count + terms) instead of passing silently. - S-2: verb/noun pairing is now a term-group nearest-pair merge walk over precomputed word ordinals (computeWordStarts / minWordGap), not a match×match cross product — a hostile line repeating one pair thousands of times stays linear instead of going quadratic. - FN-4: package-shaped inline-code spans (`stripe-sdk`, `@stripe/stripe-js`) are kept as noun/dependency evidence rather than being fully masked, so a genuine dependency reference inside code ticks still corroborates. - C-1: the <Service> API surface rule now scans every candidate in every clause; a rejected first candidate no longer shadows a later genuine service. - Cross-clause binding: a verb may bind a noun in the immediately following clause only when its own clause names a service object, within a tight gap — admits "Integrate Stripe, exposing its endpoints …" without re-admitting the unrelated-clauses false-positive class. - Internal-descriptor negative evidence ("internal Payments API", "the internal endpoint") never pairs; URL/scheme matching generalized beyond http(s). All 5 acceptance criteria still hold: the three reported false positives are clean and "integrate the Stripe API" still fires. Built .cjs committed alongside the .cts. tsc + eslint (incl. no-adhoc-markdown-parsing) + lint:regression-names clean; affected suites 256/256 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#2365): retune api-coverage detector fail-closed per Codex review (round 3) Codex's second-round review found the round-2 tightening had over-corrected into FAIL-OPEN false negatives — realistic external-API prose that the BLOCKING seal gate silently let through (the catastrophic class, since a missed API surface is worse than a dismissable false positive). Retuned the detector to be explicitly fail-closed: lean toward detecting, and let the one-line COVERAGE.md "no external API integration" declaration dismiss the residual false positives. Fail-open false negatives fixed (all now detect): - F1 clause-initial `<Service> API` with a plain follower ("Stripe API for payment processing") — dropped the follower-allowlist / corroboration gate on clause-initial surfaces; a service that is not a stopword, descriptor, or compound modifier is a real name from any clause position. - F2 scheme-less external host ("api.stripe.com/v1") — a dotted host with an alphabetic final label now contributes its API nouns; a first-party route path (no dotted host) still does not. - F3 vendor's first-party SDK ("Integrate Shopify's first-party SDK") — the compound path no longer filters nouns on "internal"/"first-party" (Codex: the qualifier can describe the vendor's own API, not the consuming project's). - F4 long single integration clause — removed the word-gap cap entirely: it could not separate a 21-word genuine clause from an 18-word internal one, so the clause boundary is now the whole relationship test. - F5 lowercase cross-clause service — cross-clause binding no longer requires a capitalized "service object". New false positives fixed (all now clean): - F6 a URL token that swallowed a trailing clause comma, merging two clauses — trailing clause punctuation is kept literal so the split survives. - F7 a capitalized internal component authorizing cross-clause binding — the new gate requires a dependent elaboration, not a new coordinate clause opened by a conjunction ("…, then document…"). - F8 a protocol name read as a service ("REST API", "GraphQL API") — protocol and locality descriptors are rejected in the `<Service>` position. - Finding 9: the inline-code-span scanner was O(n^2) on pathological backtick runs; rewritten to linear via a per-length run cursor (2 MB: 4.15 s -> ~6 ms), semantics preserved (148 sectionizer tests unchanged). Net simplification: the fail-closed model removed the round-2 minWordGap / groupByTerm / follower / corroboration machinery (350 insertions vs 445 deletions across the touched files). Under fail-closed, three round-2 negative tests now correctly detect (integration verb + "internal"-qualified noun, and the distant-same-clause case); none were trek-e acceptance FPs. Verified: 1491/1491 unit tests pass; tsc + eslint (incl. no-adhoc-markdown- parsing) + lint:regression-names clean; all 8 review findings reproduced as regression tests, both directions. Built .cjs committed alongside the .cts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#2365): resolve round-3 Codex review findings (fail-closed, round 4) Codex's round-3 adversarial review found the fail-closed retune had introduced new holes in both directions. Resolved: Fail-open false negatives (now detect): - External host addressing a PATH ("graph.microsoft.com/v1.0/me") is itself an integration surface and contributes an endpoint noun even when the host names no vocabulary word. A bare domain link with no path ("https://example.com") stays a non-signal, so "Integrate … from example.com, document …" is still clean. - Locality qualification ("internal", "private") no longer leaks across a sentence or clause boundary: only plain spaces may separate the descriptor from the service, so "The cache is private. Stripe API …" now detects. - Cross-clause binding: the fragile head-word cap (which could not tell a genuine "Connect … to Stripe payments, exposing its endpoints" from an unrelated "Integrate … from URL, document …" — both 4 words after the verb) is replaced by a participial-continuation rule: a verb binds a noun in the next clause only when that clause begins with an "-ing" elaboration. This fixes the 4-word-head false negative AND the false positive below at once. False positives (now clean): - Cross-clause no longer binds a finite continuation regardless of separator: "Wire the settings form. Document endpoint props." / "…; document …" / "…, document …" are separate actions, not elaborations. Perf (quadratic → linear): - The trailing-punctuation peel is a backward char scan instead of an unanchored `[…]+$` regex (16k chars: 156 ms → ~1 ms). - SERVICE_SURFACE_API_RE bounds the service-name length {1,40} so a hostile "A-A-…-x" run cannot drive O(n^2) backtracking (16k: 385 ms → ~3 ms). Consumer fail-open (blocking gate): - readPhaseScope now distinguishes "no plans" from a plan that EXISTS but is unreadable. On a read error the gate BLOCKS ("could not read the phase scope …") instead of silently certifying no-integration from partial scope — an unreadable plan could be the one describing the integration. Documented fail-closed tradeoffs, now pinned with tests so they are not "fixed" back into a fail-open: a clause-initial capitalized common word before "API" ("Payment API", "Search API") reads as a service name; a long clause pairs a verb with a distant noun; and a CommonMark inline code span that wraps a newline is matched within-line only. Codex judged these acceptable because the COVERAGE.md declaration is a cheap override. One documented limitation remains out of scope: "Integrate Stripe, and authenticate requests with its API" (a coordinate finite clause whose noun refers back by pronoun) needs coreference resolution, beyond a lexical detector. Verified: 379/379 affected + command-router tests pass (+14 new regression tests covering every round-3 finding, both directions); tsc + eslint (no-adhoc-markdown-parsing) + lint:regression-names clean. Built .cjs committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#2365): simplify to robust core — remove whack-a-mole heuristics (round 5) Round-4 review confirmed the detector's two most complex features generate findings in both directions no matter how they are tuned, because they need a vendor dictionary + coreference the issue rules out in principle. Per the operator's "ship the robust core" decision, both are removed and their gaps are documented rather than chased further: - Cross-clause binding DELETED (allowsCrossClause / participle rule). It caused a fail-open on finite continuations ("Integrate Stripe; use its OAuth endpoints" — missed) and a false positive on "-ing"-SPELLED nouns ("…, billing endpoint terminology…" — wrongly fired). Detection is now same-clause only. - URL-path-as-evidence REVERTED. Treating every path-bearing URL as an endpoint fired on ordinary asset/link URLs ("…/theme.css", "…?next=/x", a docs/repo link) and recreated routine UI-phase false positives. An external URL is evidence only when it NAMES an API vocabulary word ("api.stripe.com/v1"). Two fail-open cases are now DOCUMENTED limitations, pinned by tests so a future maintainer does not re-add the heuristics that caused the false positives above: a service named only in a clause separate from its API noun, and a bare external host that names no vocabulary word. Both are cheaply covered by the COVERAGE.md declaration and rare in real phase prose ("integrate the X API"). Also fixed from the round-4 review: - Qualification now survives markdown emphasis ("The **internal** Payments API" stays clean) while still not crossing a sentence/clause boundary. - readPhaseScope fail-closes on a REAL read failure (EACCES/EIO) enumerating the phase directory or reading the roadmap fallback — not only per-plan-file failures; a missing directory/section remains a legitimate no-op. The declaration-override path surfaces scope_read_error so an incomplete-scope override stays visible. - SERVICE_SURFACE_API_RE length-bound comment no longer overclaims. Net: the detector is same-clause verb+noun + `<Service> API` surface, with path/code/inline masking and a fail-closed posture. All five acceptance criteria hold. 1573/1573 unit tests pass; tsc + eslint (no-adhoc-markdown-parsing) + lint:regression-names clean. Built .cjs committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#2365): close roadmap-fallback fail-open + stale JSDoc (round-5 review) The round-5 sanity review confirmed the detector simplification is sound (all acceptance positives fire, all required negatives clean) and flagged one real blocker plus a nit: - Blocker: readPhaseScope's roadmap fallback could still silently pass an UNREADABLE roadmap. getRoadmapPhaseWithFallback gated on fs.existsSync(), which returns false on EACCES/EIO too — so an unreadable ROADMAP.md read as "absent", no exception reached isRealReadFailure, and the blocking gate certified empty scope. Fixed at the source: read the roadmap directly and honor the function's OWN documented contract — null only on ENOENT (genuinely absent), otherwise throw. Both existing callers already wrap it in try/catch expecting that throw, and readPhaseScope now fail-closes (blocks) via its roadmap catch. Verified by a new e2e test (unreadable roadmap fallback → block). - Nit: the detectApiIntegration JSDoc still described the removed cross-clause participial binding and "every external hostname counts" — corrected to the actual same-clause-only behavior and the names-a-vocab-word URL rule. Verified: full unit suite green; tsc + eslint + lint:regression-names clean. Built .cjs committed (roadmap.cjs is gitignored/rebuilt, per repo convention). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#2365): backfill changeset PR number (#2397) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#2365): sync generated capability-registry + recapture install goldens CI surfaced two generated-artifact staleness issues (all failing test shards + lint-tests traced to these, not to a logic defect): - gsd-core/bin/lib/capability-registry.cjs was stale: the initial fix edited the ai-integration `api-coverage-plan-pre.md` fragment (added the "No external API integration" declaration section) but did not regenerate the registry, which embeds an inline copy of that fragment. Regenerated via `gen-capability-registry.cjs --write` — the diff is exactly the fragment text sync. Fixes `lint:generated-sync` and the "committed registry is in sync" + "registry integration" tests. - The 18 golden-install-parity fixtures were stale by exactly one hash line each — `gsd-core/references/api-coverage.md`, which this PR edits and which is a hashed installed artifact. Recaptured with `UPDATE_GOLDEN=1`; the diff is that single hash per runtime and nothing else. Fixes the `golden parity — *` tests. No source or behavior change — generated artifacts only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#2365): flip representative-corpus manifest to assert the fixed behavior The #2371 representative corpus (merged into next after this branch was cut) is a known-bug tripwire: it asserts each fixture's currentBuggyOutput so the test fails loudly the moment #2365 is fixed, at which point — per its own contract in representative-corpus.test.cjs — the fixer removes currentBuggyOutput so the assertion checks expectedDetected instead. This is that moment. Removed currentBuggyOutput from the three detector fixtures (nextjs-route-path, unrelated-verb-noun, threat-model-prose); the corpus now asserts detected:false, which the fail-closed same-clause detector satisfies. Notes updated to describe the fix rather than the bug. The #2366 matrix corpus is left untouched — that tripwire belongs to its own PR (#2374). Corpus test: 7/7 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#2365): skip chmod-000 fail-closed e2e tests on Windows The three fail-closed gate tests induce an unreadable plan / directory / roadmap with chmod 000, but Windows does not enforce POSIX mode bits — readFileSync still succeeds, so the gate never reaches the read-error path and the assertion fails on the windows-latest CI leg. The fail-closed LOGIC is platform- independent (readError → block) and is fully exercised on the macOS/Linux legs; only the method of inducing EACCES is POSIX-specific. Guard the three tests to skip on win32 as well as root, mirroring golden-install-parity's win32 skip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#2365): address trek-e review — glossary, clock-seam, IO injection, bounds Review response to PR #2397 (trek-e, CHANGES_REQUESTED). Fix logic unchanged; this closes the test/process-hygiene findings. Major: - CONTEXT.md "Markdown Sectionizer" glossary now lists the two exports this fix relies on, `stripInlineCode` and `scanInlineCodeSpans` (glossary is a PR gate). - Replaced the banned wall-clock assertion in the "hostile repeated-term line" test (Clock Seams rule — no elapsed-time asserts) with a deterministic signal-count assertion, which also directly verifies the term-dedup that keeps pairing linear (one signal for a 10k-pair line, not thousands). - Rewrote the three fail-closed read-failure tests: instead of chmod 0o000 (a no-op under root / on Windows, the pattern the repo's IO-failure convention avoids) they now exercise the newly-exported `readPhaseScope` in-process and inject the failure by monkeypatching fs.readFileSync/readdirSync to throw, restoring in finally. Deterministic and platform-independent (no skip needed), and they add the ENOENT-is-absence case that the chmod tests couldn't express. Minor: - Added limit / limit+1 boundary tests for SERVICE_SURFACE_API_RE's {1,40} service-name bound, QUALIFIER_LOOKBACK's 24-char window, and REASON_MAX_LEN (200) on the declaration reason. - Added a fast-check property that fuzzes the tokenizer / clause splitter / masking (scanLineTokens, splitClauses, collectTermMatches) with adversarial tokens (slashes, backticks, URLs, clause punctuation) and asserts the detector is total (never throws), shape-stable, holds detected <=> signals, and is deterministic. readPhaseScope is exported for the in-process tests. Verified: 125 detector + 19 gate tests pass; tsc + eslint + generated-sync (glossary/registry) + lint-regression-test-names + lint-test-file-count clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
135 lines
6.2 KiB
Markdown
135 lines
6.2 KiB
Markdown
# API Coverage Gate (Full Coverage by Default — Opt Out, Never Opt In)
|
|
|
|
> Reference for the `api-coverage` gate on the `ai-integration` capability (#1562).
|
|
> Config key: `workflow.api_coverage_gate` (default `true`). Gate point: `verify:pre`.
|
|
|
|
## The problem this closes
|
|
|
|
"We integrated the API" too often silently means "we integrated whatever the
|
|
first use case exercised." Every un-built capability is then an invisible hole,
|
|
discovered later by a user who reasonably expected it to work. The phase sealed
|
|
green because its tasks completed — nobody *decided* the gaps were acceptable,
|
|
because nobody *enumerated* them.
|
|
|
|
This gate makes the API surface **visible and decided** before the phase can
|
|
seal. Full coverage is the default starting position; the coverage matrix is the
|
|
*subtraction record*. Every gap is an explicit, reasoned opt-out rather than a
|
|
surprise.
|
|
|
|
## When it fires
|
|
|
|
The gate runs at `verify:pre` (before `/gsd:verify-work` begins UAT). A phase is
|
|
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 and an external-API noun **in the same clause**, or an explicit
|
|
`<Service> API|SDK|REST|GraphQL` surface naming a real service) and no matrix
|
|
yet exists.
|
|
|
|
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
|
|
`<Service> 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
|
|
|
|
1. **Plan time (`plan:pre`).** A contribution to the planner prompts it to run
|
|
the deterministic detector over the phase scope and, when an integration is
|
|
detected, produce `COVERAGE.md`. See
|
|
`capabilities/ai-integration/fragments/api-coverage-plan-pre.md`.
|
|
2. **Seal time (`verify:pre`).** The blocking `api-coverage.verify-pre` gate
|
|
runs `check api-coverage.verify-pre <phase-dir>` and blocks unless a valid
|
|
matrix exists (or no integration is detected).
|
|
|
|
## The coverage matrix format
|
|
|
|
Canonical form — a markdown table (human-editable, diff-friendly):
|
|
|
|
```markdown
|
|
# API Coverage — <service>
|
|
|
|
> Full coverage by default. Opt-outs are explicit, reasoned decisions.
|
|
|
|
| capability | decision | reason |
|
|
|---|---|---|
|
|
| search | INTEGRATE | |
|
|
| playlists | INTEGRATE | |
|
|
| skip | OPT-OUT | not needed yet — tracked for follow-up phase |
|
|
```
|
|
|
|
- **`INTEGRATE` is the default.** Every capability starts as INTEGRATE.
|
|
- **Every `OPT-OUT` MUST carry a one-line reason** (`not needed`, `not needed
|
|
yet`, `explicitly out of scope`, …). An opt-out without a reason is an
|
|
un-decided hole — the exact failure mode this gate exists to close.
|
|
- A fenced ` ```coverage ` JSON block (`[{"capability":…,"decision":…,"reason":…}]`)
|
|
is also accepted for machine-generated matrices.
|
|
|
|
Rules enforced at seal time: the matrix must be non-empty; every capability name
|
|
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
|
|
Spotify for media playback) starts from the **same full-coverage baseline** as
|
|
the first. Do not carry over the first integration's opt-outs silently —
|
|
re-decide each capability for the new surface, so a first-class/fallback
|
|
asymmetry cannot accumulate into a later user-facing bug.
|
|
|
|
## The matrix persists
|
|
|
|
`COVERAGE.md` is a phase artifact. A future phase that extends the same
|
|
integration starts from the recorded surface and decisions rather than from
|
|
zero — the matrix is the durable subtraction record.
|
|
|
|
## Tuning
|
|
|
|
- **Disable entirely:** set `workflow.api_coverage_gate: false` in
|
|
`.planning/config.json` (the gate unregisters from `verify:pre`).
|
|
- **Widen the trigger vocabulary:** the detector accepts `--verbs` / `--nouns`
|
|
overrides (see `capabilities/ai-integration/fragments/api-coverage-plan-pre.md`).
|
|
The default vocabulary is additive-only.
|
|
|
|
## Detector CLI
|
|
|
|
```bash
|
|
echo "$PHASE_SCOPE" | node gsd-core/bin/lib/api-coverage.cjs --json
|
|
# exit 0 = integration detected, 1 = none, 2 = startup error
|
|
```
|
|
|
|
The detector is a pure function (`detectApiIntegration` → `{ detected, signals,
|
|
terms }`) shared by the plan-time prompt and the seal-time gate, so the
|
|
low-false-positive guarantee is testable rather than a judgment call.
|