* 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>
This commit is contained in:
committed by
GitHub
parent
d0bacc2517
commit
d04e287fa9
@@ -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 "<Service> 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 "<Service> 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: <reason>` 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 "<Word> 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 `<Service> 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 `<Service> 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 `<Service> 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<string>): 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 "<Service> 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 `<Service> 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 `<Service> 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<string>();
|
||||
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 <Service> 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 <Service> 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: <reason>` (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 = /<!--[\s\S]*?-->/g;
|
||||
|
||||
const VALID_DECISIONS = new Set<CoverageDecision>(['INTEGRATE', 'OPT-OUT']);
|
||||
|
||||
/**
|
||||
@@ -305,10 +616,20 @@ const VALID_DECISIONS = new Set<CoverageDecision>(['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 } };
|
||||
|
||||
Reference in New Issue
Block a user