From c5f2b94b277de23cbb74b8e2c579424545174b55 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 27 Aug 2026 11:37:31 -0400 Subject: [PATCH] enhance(#3907): gates report no-input instead of a verdict they never reached (#3932) * feat(#3907): gates report no-input instead of asserting a verdict they never reached The three stdin-reading gates bound 2 to a stdin read error only, with no arm for stdin closed at zero bytes - so empty input flowed into the detector, found nothing, and exited 1, which each module's own comment defines as a negative verdict. An unset PHASE_SECTION made the UI gate assert the phase has no UI. Empty and whitespace-only input now exit NO_INPUT, and a read error exits UNAVAILABLE rather than a locally-invented 2, both resolved through the registry and delivered by terminateNow. The exit code was only half of it: under --json the same input emitted {detected:false}, byte-identical to the fabricated payload #3909 exists to fix, and the blocking coverage gate reads that payload. Empty input now emits the in-tree {skipped:true,reason} form with no detected key at all. teams-status is excluded: it never reads stdin and has no invented 2, so the four-module framing in the issue and ADR is wrong. The dead root bin/lib/ui-safety-gate.cjs is deleted - no installer reference, no workflow invocation, and the live fallback chains are for other modules. Its removal restores the unit tests to the module that actually ships; they had been asserting the stale copy's two-field shape, which is why it drifted unnoticed. * fix(#3907): drive gate tests through the process seam, and make removed-but-needed basename-precise CONTRIBUTING requires every subprocess go through tests/helpers/process-seam.cjs; two of the three gate suites hand-rolled spawnSync while the third, added in the same change, used runNode correctly for the identical injection case. Converted the blocks this change added, leaving pre-existing ones alone. Deleting one of two files sharing a basename made lint-removed-but-needed report 14 references that were all to the surviving canonical module - the false-positive class its own docstring names. It now matches on the deleted file's full path when a surviving file shares its basename, which is more precise rather than weaker: a genuine full-path reference still fails, and behaviour is unchanged when no basename collides. It immediately caught a docstring on this branch that spelled the deleted path. * test(#3907): update the one existing assertion that pinned the old empty-stdin verdict A pre-existing test asserted exit 1 on empty stdin - the defect this phase removes - and was missed because the change added new blocks without auditing existing ones pinning the old contract. Audited the rest: the other three status-1 assertions in that file all feed real input and are the genuine-negative controls that must keep returning 1, so exactly one was stale. The retired 2 is gone from the describe's contract comment too. * chore(#3907): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/zesty-yaks-hop.md | 5 + bin/lib/ui-safety-gate.cjs | 109 ----------- .../fragments/api-coverage-plan-pre.md | 16 +- docs/INVENTORY.md | 6 +- gsd-core/bin/lib/capability-registry.cjs | 4 +- gsd-core/references/api-coverage.md | 5 +- ...ocs-guard-registration.exempt-baseline.cjs | 4 +- scripts/lint-removed-but-needed.cjs | 154 ++++++++++++++-- src/api-coverage.cts | 40 +++- src/assumption-delta.cts | 42 +++-- src/ui-safety-gate.cts | 45 ++++- tests/api-coverage.test.cjs | 112 ++++++++++++ tests/assumption-delta.test.cjs | 126 ++++++++++++- tests/check-ui-safety-gate.test.cjs | 172 ++++++++++++++++-- tests/helpers/inject-stdin-error.cjs | 41 +++++ tests/removed-but-needed-lint.test.cjs | 134 +++++++++++++- 16 files changed, 838 insertions(+), 177 deletions(-) create mode 100644 .changeset/zesty-yaks-hop.md delete mode 100644 bin/lib/ui-safety-gate.cjs create mode 100644 tests/helpers/inject-stdin-error.cjs diff --git a/.changeset/zesty-yaks-hop.md b/.changeset/zesty-yaks-hop.md new file mode 100644 index 000000000..40dda8736 --- /dev/null +++ b/.changeset/zesty-yaks-hop.md @@ -0,0 +1,5 @@ +--- +type: Removed +pr: 3932 +--- +Removed the hand-written root bin/lib/ui-safety-gate.cjs — the GSD installer and every shipped workflow only ever resolved gsd-core/bin/lib/ui-safety-gate.cjs, so the root copy was unused dead code. diff --git a/bin/lib/ui-safety-gate.cjs b/bin/lib/ui-safety-gate.cjs deleted file mode 100644 index 112ec1b71..000000000 --- a/bin/lib/ui-safety-gate.cjs +++ /dev/null @@ -1,109 +0,0 @@ -'use strict'; - -/** - * UI Safety Gate — shell-free implementation (#3706, #3718) - * - * Replaces the bash shell-based one-liner that silently degraded on Windows - * PowerShell / cmd.exe because the locale env-var prefix was not recognised. - * This module runs inside Node.js — no shell dependency, works identically - * on bash, Git-Bash, PowerShell, and cmd.exe. - * - * Word-boundary anchoring: - * (^|[^a-zA-Z0-9])(TOKEN)([^a-zA-Z0-9]|$) - * Equivalent to POSIX ERE [^[:alnum:]] — matches tokens only when they are not - * interior substrings of alphanumeric compound words (e.g. "microfrontend" is NOT - * matched; "micro-frontend" and "micro frontend" ARE matched). - * - * Public API: - * checkUiPresence(text: string): { hasUI: boolean, tokens: string[] } - * - * CLI usage — reads phase-section text from STDIN to avoid ARG_MAX limits: - * echo "$PHASE_SECTION" | node bin/lib/ui-safety-gate.cjs - * echo $? → 0 if UI tokens found, 1 if not, 2 on usage error - * - * Exit codes mirror grep: 0 = match found, 1 = no match, 2 = usage error. - */ - -const UI_TOKENS = [ - 'UI', - 'interface', - 'frontend', - 'component', - 'layout', - 'page', - 'screen', - 'view', - 'form', - 'dashboard', - 'widget', -]; - -/** - * Built once at module load — no per-call compilation overhead. - * ASCII word boundaries — matches the original ASCII-grep intent of #3706. - * Note: JS [a-zA-Z0-9] is ASCII-only and NOT equivalent to POSIX [[:alnum:]], - * which is locale-sensitive and includes accented characters. - */ -const UI_GATE_PATTERN = new RegExp( - '(^|[^a-zA-Z0-9])(' + UI_TOKENS.join('|') + ')([^a-zA-Z0-9]|$)', - 'i' -); - -// Global-flagged variant for extracting ALL matches per line (matchAll). -const UI_GATE_PATTERN_GLOBAL = new RegExp(UI_GATE_PATTERN.source, 'gi'); - -/** - * Check a roadmap phase section string for frontend UI indicators. - * - * @param {string} text - The roadmap phase section content (may be multi-line, CRLF or LF). - * @returns {{ hasUI: boolean, tokens: string[] }} - * hasUI — true if any UI token was matched as a standalone word. - * tokens — matched token strings (lowercased), deduplicated. - */ -function checkUiPresence(text) { - if (typeof text !== 'string') { - return { hasUI: false, tokens: [] }; - } - - // Normalise CRLF so the pattern sees consistent line boundaries. - const normalised = text.replace(/\r\n/g, '\n'); - - const found = new Set(); - for (const line of normalised.split('\n')) { - // Reset lastIndex before each line so the global pattern restarts from 0. - UI_GATE_PATTERN_GLOBAL.lastIndex = 0; - for (const m of line.matchAll(UI_GATE_PATTERN_GLOBAL)) { - found.add(m[2].toLowerCase()); - } - } - - return { hasUI: found.size > 0, tokens: [...found] }; -} - -module.exports = { checkUiPresence, UI_TOKENS }; - -// ── CLI entry point ───────────────────────────────────────────────────────── -// Reads phase-section text from STDIN (not argv) to avoid OS ARG_MAX limits. -// Invoked by workflow .md bash blocks as: echo "$PHASE_SECTION" | node bin/lib/ui-safety-gate.cjs -// Exit 0 = UI found, 1 = no UI, 2 = startup error. - -if (require.main === module) { - // Collect stdin chunks asynchronously. - const chunks = []; - process.stdin.setEncoding('utf-8'); - - process.stdin.on('data', (chunk) => chunks.push(chunk)); - - process.stdin.on('end', () => { - const input = chunks.join(''); - const result = checkUiPresence(input); - // eslint-disable-next-line n/no-process-exit -- CLI entry point (require.main===module): async stdin handler, nothing else terminates the process (#3059) - process.exit(result.hasUI ? 0 : 1); - }); - - process.stdin.on('error', (err) => { - process.stderr.write(`ERROR: ui-safety-gate.cjs stdin read failed: ${err.message}\n`); - // eslint-disable-next-line n/no-process-exit -- CLI entry point (require.main===module): async stdin handler, nothing else terminates the process (#3059) - process.exit(2); - }); -} diff --git a/capabilities/ai-integration/fragments/api-coverage-plan-pre.md b/capabilities/ai-integration/fragments/api-coverage-plan-pre.md index d0a88af7b..2cfd6fb8c 100644 --- a/capabilities/ai-integration/fragments/api-coverage-plan-pre.md +++ b/capabilities/ai-integration/fragments/api-coverage-plan-pre.md @@ -25,8 +25,20 @@ SCOPE="$(cat "${PHASE_DIR}"/*-PLAN.md 2>/dev/null) $(gsd_run query roadmap.get-p API_COVERAGE_JSON=$(printf '%s' "$SCOPE" | node gsd-core/bin/lib/api-coverage.cjs --json 2>/dev/null || echo '{"detected":false,"signals":[]}') ``` -Read `API_COVERAGE_JSON.detected`. Act on it only — do **not** pattern-match the -prose yourself. +The detector's exit code and `--json` payload now distinguish a real negative +from an unexamined input (ADR-3889 Phase 3, #3907): empty/whitespace-only +`$SCOPE` or a stdin read failure emit `{"skipped":true,"reason":"no_input"| +"stdin_error"}` — no `detected` key at all. **Check for `skipped` before +reading `detected`**: a `skipped` payload is not a confirmed "no API +integration" verdict, it means the detector never examined real input. Do not +treat it as `detected:false`. Read `API_COVERAGE_JSON.detected` only when +`skipped` is absent — act on it only, do **not** pattern-match the prose +yourself. + +**If `skipped` is `true`:** the detector could not establish a scope (empty +`$SCOPE`) or failed to run (stdin read error). Skip the checkpoint for this +run rather than asserting a verdict about input that was never examined; do +not raise it with the user. **If `detected` is `false`:** this phase does not integrate an external API. Skip the checkpoint entirely and continue planning. Do not raise it with the user. diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index dc9b112fd..2d1c4768e 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -461,9 +461,9 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `agent-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools agent` | | `agent-install-check.cjs` | Agent-installation probe — owns `getAgentsDir` and `checkAgentsInstalled`, the single resolution the health-diagnostic agent-install rule consumes (ADR-857, #1268) | | `health-diagnostic-rules/agent-install.cjs` | Health-diagnostic rule: agent-installation-completeness check (W010) — the single `checkAgentsInstalled` call site's four mutually exclusive conditions, ported behavior-preserving from `cmdValidateHealth` (ADR-3180 §8.2/§8.3/§8.5, Phase 11, #3309) | -| `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`) | +| `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/NO_INPUT=empty-or-whitespace-only stdin/UNAVAILABLE=stdin read failed — registry codes, ADR-3889 Phase 3, #3907); 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 | -| `assumption-delta.cjs` | Detects identity-model assumption transitions in phase text for discuss-phase assumptions mode (#1561) | +| `assumption-delta.cjs` | Detects identity-model assumption transitions in phase text for discuss-phase assumptions mode (#1561); STDIN CLI (`echo "$PHASE_SECTION" \| node .../assumption-delta.cjs [--json]`, exit 0=detected/1=none/NO_INPUT=empty-or-whitespace-only stdin/UNAVAILABLE=stdin read failed — registry codes, ADR-3889 Phase 3, #3907) | | `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 | | `capability-activation.cjs` | Capability activation resolver shared by config validation and capability-state consumers — resolves registry-owned config keys from raw runtime config without re-centralizing migrated settings | @@ -655,7 +655,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `uat.cjs` | UAT file parsing, verification debt tracking, audit-uat support | | `uat-predicate.cjs` | UAT-passed predicate — markdown-aware evaluation of HUMAN-UAT results; returns pass only when all required checks pass; ignores false-positive contexts (frontmatter, fenced code, blockquotes, HTML comments) | | `ui-consideration-probe.cjs` | Spec-completeness UI-consideration probe (compiled from `src/ui-consideration-probe.cts`, gitignored) — the third adapter of the `probe-core` resolution model (ADR-550 Decision 7): element-kind classification, applicable-category relevance filter, consideration proposal, `proposeElements`/`autoResolve` (propose-then-confirm + the `--auto` never-dismiss floor), and the `{explicit, backstop}` validators; delegates merge/rollup/CLI to `probe-core`; exports `classifyElement`, `applicableCategories`, `proposeConsiderations`, `proposeElements`, `autoResolve`, `analyzeCoverage`, `UI_TAXONOMY` (#1867) | -| `ui-safety-gate.cjs` | Shell-free word-boundary UI token detector (#3706, #3718); reads phase-section text from stdin, exits 0 (UI found) or 1 (no UI); also deployed to `gsd-core/bin/lib/` so the GSD installer ships it to `$RUNTIME_DIR` (#448) | +| `ui-safety-gate.cjs` | Shell-free word-boundary UI token detector (#3706, #3718); reads phase-section text from stdin, exits 0 (UI found), 1 (no UI — real input, examined), NO_INPUT (empty/whitespace-only stdin) or UNAVAILABLE (stdin read failed) — the latter two are registry codes (ADR-3889 Phase 3, #3907); also deployed to `gsd-core/bin/lib/` so the GSD installer ships it to `$RUNTIME_DIR` (#448) | | `ui-frontend-evidence.cjs` | Static frontend-evidence detector (compiled from `src/ui-frontend-evidence.cts`, gitignored) — plan-time structural corroboration for `computeUiPlanGate` (#3312): a `package.json` UI-framework dependency or a component-framework file (`*.tsx/*.jsx/*.vue/*.svelte`) in the tree, so a UI-token match on a hyphenated proper noun (e.g. repo `dashboard-financeiro`) cannot block planning in a repo with no frontend; mirrors the post-wave `computeUiSafetyGate` git-diff corroboration | | `unusable-input.cjs` | Deduplicates stderr diagnostics for corrupt or unreadable configuration (#1879) | | `update-context.cjs` | Pure install-context resolver for `/gsd-update` — runtime/scope/config-dir/version detection (LOCAL/GLOBAL/UNKNOWN) ported from update.md bash; backs `gsd-tools update-context` (#498) | diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index 98efa468d..69b9debbe 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**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" + "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\nThe detector's exit code and `--json` payload now distinguish a real negative\nfrom an unexamined input (ADR-3889 Phase 3, #3907): empty/whitespace-only\n`$SCOPE` or a stdin read failure emit `{\"skipped\":true,\"reason\":\"no_input\"|\n\"stdin_error\"}` — no `detected` key at all. **Check for `skipped` before\nreading `detected`**: a `skipped` payload is not a confirmed \"no API\nintegration\" verdict, it means the detector never examined real input. Do not\ntreat it as `detected:false`. Read `API_COVERAGE_JSON.detected` only when\n`skipped` is absent — act on it only, do **not** pattern-match the prose\nyourself.\n\n**If `skipped` is `true`:** the detector could not establish a scope (empty\n`$SCOPE`) or failed to run (stdin read error). Skip the checkpoint for this\nrun rather than asserting a verdict about input that was never examined; do\nnot raise it with the user.\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" @@ -4256,7 +4256,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**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" + "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\nThe detector's exit code and `--json` payload now distinguish a real negative\nfrom an unexamined input (ADR-3889 Phase 3, #3907): empty/whitespace-only\n`$SCOPE` or a stdin read failure emit `{\"skipped\":true,\"reason\":\"no_input\"|\n\"stdin_error\"}` — no `detected` key at all. **Check for `skipped` before\nreading `detected`**: a `skipped` payload is not a confirmed \"no API\nintegration\" verdict, it means the detector never examined real input. Do not\ntreat it as `detected:false`. Read `API_COVERAGE_JSON.detected` only when\n`skipped` is absent — act on it only, do **not** pattern-match the prose\nyourself.\n\n**If `skipped` is `true`:** the detector could not establish a scope (empty\n`$SCOPE`) or failed to run (stdin read error). Skip the checkpoint for this\nrun rather than asserting a verdict about input that was never examined; do\nnot raise it with the user.\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 233a26444..7d30ba287 100644 --- a/gsd-core/references/api-coverage.md +++ b/gsd-core/references/api-coverage.md @@ -126,7 +126,10 @@ zero — the matrix is the durable subtraction record. ```bash echo "$PHASE_SCOPE" | node gsd-core/bin/lib/api-coverage.cjs --json -# exit 0 = integration detected, 1 = none, 2 = startup error +# exit 0 = integration detected, 1 = none (real input, examined, no signal) +# NO_INPUT = stdin was empty or whitespace-only (registry code — ADR-3889 Phase 3, #3907) +# UNAVAILABLE = stdin read failed (registry code) +# NO_INPUT/UNAVAILABLE emit {"skipped":true,"reason":"no_input"|"stdin_error"} — no `detected` key ``` The detector is a pure function (`detectApiIntegration` → `{ detected, signals, diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index 5415769b2..d48cad352 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -168,7 +168,9 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { 'phase.test.cjs': ['docs/adr/3524-...md', 'docs/adr/3524-cjs-sdk-hard-seam.md'], 'pr-branch-planning-filter.test.cjs': ['docs/readme.md'], 'precommit-alias-drift-hook.test.cjs': ['docs/adr/0174-...md', 'docs/adr/0174-retire-gsd-sdk-package-boundary.md'], - 'removed-but-needed-lint.test.cjs': ['docs/getting-started.md', 'docs/gsd-new-workspace.md', 'docs/setup.md'], + 'removed-but-needed-lint.test.cjs': [ + 'docs/README.md', 'docs/getting-started.md', 'docs/gsd-new-workspace.md', 'docs/setup.md', 'docs/some-doc.md', + ], 'repo-invariants.test.cjs': ['docs/FEATURES.md', 'docs/workflows/README'], 'require-issue-link-policy.test.cjs': ['docs/-prefixed', 'docs/CONFIGURATION.md', 'docs/a.md', 'docs/b.md', 'docs/guide.md'], 'reviewer-manifest-body.test.cjs': ['docs/how-to/ship-a-reviewer-lane.md'], diff --git a/scripts/lint-removed-but-needed.cjs b/scripts/lint-removed-but-needed.cjs index 8c5cdfd04..7994a178f 100644 --- a/scripts/lint-removed-but-needed.cjs +++ b/scripts/lint-removed-but-needed.cjs @@ -46,6 +46,38 @@ * A common basename (`index.js`, `config.json`) can coincidentally match an * unrelated file, and this only catches LITERAL string references — not a * variable holding the filename or a glob that happened to match it. + * + * ## Basename-collision refinement (#3907) + * + * #3907 deleted `bin/lib/ui-safety-gate.cjs` while the SEPARATE, still-live + * `gsd-core/bin/lib/ui-safety-gate.cjs` survives — every reference to the + * survivor (including it referencing itself) was misattributed to the + * deletion, 14 false violations on a correct tree. This epic is about + * de-duplicating modules, so "delete one of two files sharing a basename" + * recurs by construction. + * + * The fix is precision, not suppression: for each deleted file, if its + * basename is ALSO the basename of a file that still exists in the + * post-diff tree — `git ls-files` (tracked files) UNIONED with the + * already filesystem-walked corpus/testsCorpus file lists, because + * `git ls-files` alone misses gitignored BUILD ARTIFACTS such as + * `gsd-core/bin/lib/*.cjs` (compiled from `.cts`, never committed) — + * matching switches from the bare basename to the deleted file's full + * repo-relative path. Because the deletion is already committed on this + * branch, none of these sources can list the deleted file itself, so any + * hit is necessarily a DIFFERENT file. A genuine + * full-path reference to the deleted file is still caught (strictly more + * precise, not weaker); a bare-basename reference becomes correctly + * unattributable to the deleted file specifically and is no longer + * blamed on it. When no surviving file shares the basename, nothing + * changes — the original basename rule still applies exactly as before. + * + * Residual known limit: a bare-basename reference to a deleted file whose + * basename survives elsewhere is now invisible to this guard (it cannot + * tell whether the bare mention meant the deleted file or its + * basename-twin). Same trade the docstring already makes for prose + * mentions above — a false violation reds a correct tree; a missed + * ambiguous mention only loses one detection channel. */ const fs = require('node:fs'); @@ -78,6 +110,40 @@ function referencesBasename(content, basename) { return re.test(content); } +/** + * Pure: does `content` contain a literal reference to the FULL + * repo-relative path `relPath` (used instead of {@link referencesBasename} + * when the deleted file's basename collides with a surviving file — #3907)? + * + * Same delimited-match idea as `referencesBasename`, but the left boundary + * additionally excludes `/` — a `/` immediately to the left would mean the + * matched text is really a SUFFIX of a longer path (e.g. content contains + * `gsd-core/bin/lib/x.cjs` and `relPath` is `bin/lib/x.cjs`: without this, + * the survivor's own path would be mistaken for a reference to the + * deleted file it merely shares a basename with). + * @param {string} content + * @param {string} relPath - repo-relative path, forward-slash separated + * @returns {boolean} + */ +function referencesPath(content, relPath) { + const re = new RegExp(`(^|[^\\w./-])${escapeRegex(relPath)}($|[^\\w.-])`); + return re.test(content); +} + +/** + * Pure: given the post-diff tree's file list (repo-relative paths), build + * the set of basenames that still exist. Used to decide, per deleted file, + * whether basename matching would be ambiguous (#3907). + * @param {string[]} survivingFiles - repo-relative paths of files present + * in the post-diff tree (must NOT include the deleted files themselves) + * @returns {Set} + */ +function buildSurvivingBasenames(survivingFiles) { + const set = new Set(); + for (const f of survivingFiles) set.add(path.basename(f)); + return set; +} + /** * Pure: given the deleted file's basename, does content contain a * lockfile-dependent idiom (`npm ci`, `cache: 'npm'` / `cache: "npm"`)? @@ -110,15 +176,26 @@ function walk(dir) { * of the post-diff tree, find every surviving reference. * @param {string[]} deletedFiles - repo-relative deleted paths * @param {{ file: string, content: string }[]} corpus + * @param {Set} [survivingBasenames] - basenames still present in the + * post-diff tree (#3907); when a deleted file's basename is in this set, + * matching switches from basename to the deleted file's full path * @returns {{ deletedFile: string, referencedIn: string, reason: string }[]} */ -function findSurvivingReferences(deletedFiles, corpus) { +function findSurvivingReferences(deletedFiles, corpus, survivingBasenames = new Set()) { const violations = []; for (const deletedFile of deletedFiles) { const basename = path.basename(deletedFile); + const collides = survivingBasenames.has(basename); for (const { file, content } of corpus) { - if (referencesBasename(content, basename)) { - violations.push({ deletedFile, referencedIn: file, reason: `basename '${basename}' still referenced` }); + const matched = collides ? referencesPath(content, deletedFile) : referencesBasename(content, basename); + if (matched) { + violations.push({ + deletedFile, + referencedIn: file, + reason: collides + ? `full path '${deletedFile}' still referenced (basename '${basename}' also belongs to a surviving file, so matched by path — #3907)` + : `basename '${basename}' still referenced`, + }); } } if (basename === 'package-lock.json') { @@ -154,9 +231,12 @@ function findSurvivingReferences(deletedFiles, corpus) { * * @param {string} line * @param {string} basename + * @param {string} [matchTarget] - text to look for as a quoted object key + * (#3907: the deleted file's full path when its basename collides with a + * surviving file; defaults to `basename`, the original behaviour) * @returns {'asserts-absence'|'pins-existence'|null} */ -function classifyTestReference(line, basename) { +function classifyTestReference(line, basename, matchTarget = basename) { const negatedCheck = /!\s*[A-Za-z_$][\w.$]*\.(includes|indexOf|search|startsWith|endsWith|match)\s*\(/.test(line) || /!\s*(fs\.)?(existsSync|statSync)\s*\(/.test(line) || @@ -170,7 +250,7 @@ function classifyTestReference(line, basename) { // Template literal needs \\s so the RegExp constructor receives \s — a // bare \s in a template literal collapses to 's' and silently matches a // literal 's' instead of whitespace. - if (new RegExp(`['"\`]${escapeRegex(basename)}['"\`]\\s*:`).test(line)) return 'pins-existence'; + if (new RegExp(`['"\`]${escapeRegex(matchTarget)}['"\`]\\s*:`).test(line)) return 'pins-existence'; return null; } @@ -183,24 +263,38 @@ function classifyTestReference(line, basename) { * * @param {string[]} deletedFiles - repo-relative deleted paths * @param {{ file: string, content: string }[]} testsCorpus + * @param {Set} [survivingBasenames] - basenames still present in the + * post-diff tree (#3907); when a deleted file's basename is in this set, + * matching switches from basename to the deleted file's full path, same + * refinement as {@link findSurvivingReferences} so the two scanners + * cannot disagree about what "a reference" means * @returns {{ deletedFile: string, referencedIn: string, reason: string }[]} */ -function findSurvivingTestReferences(deletedFiles, testsCorpus) { +function findSurvivingTestReferences(deletedFiles, testsCorpus, survivingBasenames = new Set()) { const violations = []; for (const deletedFile of deletedFiles) { const basename = path.basename(deletedFile); - const refRe = new RegExp(`(^|[^\\w.-])${escapeRegex(basename)}($|[^\\w.-])`); + const collides = survivingBasenames.has(basename); + const matchTarget = collides ? deletedFile : basename; + // The left-boundary class matches referencesPath's when collides + // (excludes '/' so a survivor's own longer path isn't mistaken for a + // reference to the deleted file it merely shares a basename with). + const refRe = collides + ? new RegExp(`(^|[^\\w./-])${escapeRegex(matchTarget)}($|[^\\w.-])`) + : new RegExp(`(^|[^\\w.-])${escapeRegex(matchTarget)}($|[^\\w.-])`); for (const { file, content } of testsCorpus) { for (const line of content.split(/\r?\n/)) { if (!refRe.test(line)) continue; - const kind = classifyTestReference(line, basename); + const kind = classifyTestReference(line, basename, matchTarget); if (kind === 'pins-existence') { violations.push({ deletedFile, referencedIn: file, // control-char-stripped + capped: this text is echoed into lint // output that AI agents read as trusted instructions - reason: `pins-existence: test depends on deleted basename '${basename}' — ${sanitizeEcho(line.trim())}`, + reason: collides + ? `pins-existence: test depends on deleted path '${matchTarget}' — ${sanitizeEcho(line.trim())}` + : `pins-existence: test depends on deleted basename '${basename}' — ${sanitizeEcho(line.trim())}`, }); } // 'asserts-absence' is correct post-deletion state; null is a @@ -231,6 +325,28 @@ function getDeletedFiles(root, baseRef) { .map((line) => line.slice(2)); } +/** + * All tracked files at the current tree state (the post-diff tree, since + * this scan runs against a branch where the deletion is already committed — + * `git ls-files` therefore CANNOT list a deleted file, so any hit for a + * deleted file's basename is necessarily a different, surviving file). + * Used to build `survivingBasenames` for the #3907 collision refinement. + * @param {string} root + * @returns {string[]} repo-relative paths, forward-slash separated + */ +function getSurvivingFiles(root) { + const out = cp.execFileSync('git', ['ls-files'], { + cwd: root, + encoding: 'utf8', + timeout: 15000, + }); + return out + .trim() + .split('\n') + .filter(Boolean) + .map((f) => f.replace(/\\/g, '/')); +} + function buildCorpus(root) { const corpus = []; for (const rel of SCAN_ROOTS) { @@ -257,7 +373,6 @@ function scan(root, baseRef) { const deletedFiles = getDeletedFiles(root, baseRef); if (deletedFiles.length === 0) return []; const corpus = buildCorpus(root); - const violations = findSurvivingReferences(deletedFiles, corpus); // tests/ arm (#3565): same deletions, discriminated references. A bare // mention that pins nothing is skipped; an absence assertion is the // correct post-deletion state; only a pin on a deleted file fails. @@ -272,7 +387,21 @@ function scan(root, baseRef) { // unreadable (broken symlink, binary that slipped past SKIP_EXT) — skip } } - violations.push(...findSurvivingTestReferences(deletedFiles, testsCorpus)); + // #3907: per-deletion basename-collision detection — if a surviving file + // shares the basename, matching switches from basename to full path (see + // the header's "Basename-collision refinement" section) for BOTH arms. + // `git ls-files` alone misses gitignored BUILD ARTIFACTS (e.g. + // gsd-core/bin/lib/*.cjs, compiled from .cts and never committed) — + // exactly the #3907 collision partner — so it is unioned with the + // already filesystem-walked corpus/testsCorpus file lists, which do see + // them. + const survivingBasenames = buildSurvivingBasenames([ + ...getSurvivingFiles(root), + ...corpus.map((c) => c.file), + ...testsCorpus.map((t) => t.file), + ]); + const violations = findSurvivingReferences(deletedFiles, corpus, survivingBasenames); + violations.push(...findSurvivingTestReferences(deletedFiles, testsCorpus, survivingBasenames)); return violations; } @@ -305,11 +434,14 @@ function main() { module.exports = { referencesBasename, + referencesPath, + buildSurvivingBasenames, referencesNpmLockfileDependency, findSurvivingReferences, classifyTestReference, findSurvivingTestReferences, getDeletedFiles, + getSurvivingFiles, buildCorpus, scan, SCAN_ROOTS, diff --git a/src/api-coverage.cts b/src/api-coverage.cts index 961ae82fb..59a279591 100644 --- a/src/api-coverage.cts +++ b/src/api-coverage.cts @@ -51,9 +51,13 @@ * renderCoverageMatrix(rows) -> string * DEFAULT_API_COVERAGE_TERMS * - * CLI: + * CLI (ADR-3889 Phase 3, #3907): * echo "$SCOPE" | node gsd-core/bin/lib/api-coverage.cjs [--json] - * exit 0 = integration detected, 1 = none, 2 = startup error + * exit 0 = integration detected, 1 = none (real input, examined, no signal), + * NO_INPUT (registry, src/cli-exit.cts) = stdin empty/whitespace-only, + * UNAVAILABLE (registry) = stdin read failed + * --json additionally prints the typed IR on stdout (unchanged for 0/1); + * NO_INPUT/UNAVAILABLE print {skipped:true, reason:"no_input"|"stdin_error"} */ import { stripFencedCode, scanInlineCodeSpans, extractFencedBlock } from './markdown-sectionizer.cjs'; @@ -939,13 +943,27 @@ export function renderCoverageMatrix(rows: readonly CoverageRow[]): string { // ── CLI entry point ────────────────────────────────────────────────────────── // Reads phase-scope text from STDIN (not argv) to avoid OS ARG_MAX limits. // Invoked by workflow bash as: echo "$SCOPE" | node .../api-coverage.cjs [--json] -// Exit 0 = integration detected, 1 = none, 2 = startup error. Mirrors -// assumption-delta.cjs / ui-safety-gate.cjs. +// +// Exit codes (ADR-3889 Phase 3, #3907): 0 = integration detected, 1 = none +// (real input, examined, no signal), NO_INPUT (registry — src/cli-exit.cts) = +// stdin empty/whitespace-only, UNAVAILABLE (registry) = stdin read failed. +// The prior single "2 = startup error" arm let empty input flow into the +// detector and exit 1 — an unexamined input reported as an authoritative +// negative. Under --json, NO_INPUT/UNAVAILABLE emit `{skipped:true, +// reason:"no_input"|"stdin_error"}` — no `detected` key, so a stale +// `detected:false` can never sit beside `skipped:true`; the detected/no-signal +// `--json` payloads are byte-identical to before. Terminates via terminateNow +// (write-then-terminate, total by construction), not raw process.exit — these +// exits fire from inside a stdin event handler. Mirrors assumption-delta.cjs / +// ui-safety-gate.cjs. if (require.main === module) { const argv = process.argv.slice(2); const wantJson = argv.includes('--json'); + // eslint-disable-next-line @typescript-eslint/no-require-imports + const cliExit = require('./cli-exit.cjs') as { terminateNow: (outcome: string, payload: unknown) => never }; + let termsOverride: Partial | undefined; const verbsIdx = argv.indexOf('--verbs'); const verbsVal = verbsIdx !== -1 ? argv[verbsIdx + 1] : undefined; @@ -970,14 +988,18 @@ if (require.main === module) { process.stdin.on('data', (chunk: string) => chunks.push(chunk)); process.stdin.on('end', () => { const input = chunks.join(''); - const result = detectApiIntegration(input, termsOverride); - if (wantJson) { - process.stdout.write(JSON.stringify(result) + '\n'); + // Whitespace-only counts as empty (ADR-3889 §1's NO_INPUT: "zero units + // were in scope, and that emptiness is known to be genuine"). Real input + // — including a lone NUL byte, which .trim() does not strip — always + // falls through to the detector. + if (input.trim().length === 0) { + cliExit.terminateNow('NO_INPUT', wantJson ? { skipped: true, reason: 'no_input' } : undefined); } - process.exit(result.detected ? 0 : 1); + const result = detectApiIntegration(input, termsOverride); + cliExit.terminateNow(result.detected ? 'PASS' : 'FAIL', wantJson ? result : undefined); }); process.stdin.on('error', (err: Error) => { process.stderr.write(`ERROR: api-coverage.cjs stdin read failed: ${err.message}\n`); - process.exit(2); + cliExit.terminateNow('UNAVAILABLE', wantJson ? { skipped: true, reason: 'stdin_error' } : undefined); }); } diff --git a/src/assumption-delta.cts b/src/assumption-delta.cts index ba1a93965..e09163729 100644 --- a/src/assumption-delta.cts +++ b/src/assumption-delta.cts @@ -20,17 +20,19 @@ * tunable (config + the `terms` parameter) so teams can widen it. * - FENCED CODE BLOCKS ARE STRIPPED first (via the markdown-sectionizer seam) * so a trigger term that appears only inside a code snippet does not fire. - * - Mirrors ui-safety-gate.cts: a pure function + a STDIN-reading CLI whose - * exit codes mirror grep (0 = signal found, 1 = none, 2 = usage error). + * - Mirrors ui-safety-gate.cts: a pure function + a STDIN-reading CLI. * * Public API: * detectAssumptionDelta(text, terms?) -> { detected, signals, terms } * DEFAULT_ASSUMPTION_DELTA_TERMS * - * CLI: + * CLI (ADR-3889 Phase 3, #3907): * echo "$PHASE_SECTION" | node gsd-core/bin/lib/assumption-delta.cjs [--json] - * exit 0 = signal detected, 1 = none, 2 = startup error - * --json additionally prints the typed IR on stdout + * exit 0 = signal detected, 1 = none (real input, examined, no signal), + * NO_INPUT (registry, src/cli-exit.cts) = stdin empty/whitespace-only, + * UNAVAILABLE (registry) = stdin read failed + * --json additionally prints the typed IR on stdout (unchanged for 0/1); + * NO_INPUT/UNAVAILABLE print {skipped:true, reason:"no_input"|"stdin_error"} */ import { stripFencedCode } from './markdown-sectionizer.cjs'; @@ -214,11 +216,25 @@ export function detectAssumptionDelta( // ── CLI entry point ────────────────────────────────────────────────────────── // Reads phase-section text from STDIN (not argv) to avoid OS ARG_MAX limits. // Invoked by workflow bash as: echo "$PHASE_SECTION" | node .../assumption-delta.cjs [--json] -// Exit 0 = signal detected, 1 = none, 2 = startup error. Mirrors ui-safety-gate. +// +// Exit codes (ADR-3889 Phase 3, #3907): 0 = signal detected, 1 = none (real +// input, examined, no signal), NO_INPUT (registry — src/cli-exit.cts) = stdin +// empty/whitespace-only, UNAVAILABLE (registry) = stdin read failed. The prior +// single "2 = startup error" arm let empty input flow into the detector and +// exit 1 — an unexamined input reported as an authoritative negative. Under +// --json, NO_INPUT/UNAVAILABLE emit `{skipped:true, reason:"no_input"| +// "stdin_error"}` — no `detected` key, so a stale `detected:false` can never +// sit beside `skipped:true`; the detected/no-signal `--json` payloads are +// byte-identical to before. Terminates via terminateNow (write-then-terminate, +// total by construction), not raw process.exit — these exits fire from inside +// a stdin event handler. Mirrors ui-safety-gate.cjs / api-coverage.cjs. if (require.main === module) { const argv = process.argv.slice(2); const wantJson = argv.includes('--json'); + + // eslint-disable-next-line @typescript-eslint/no-require-imports + const cliExit = require('./cli-exit.cjs') as { terminateNow: (outcome: string, payload: unknown) => never }; // --terms : config-tunable vocabulary override. Replaces the // pluralization cues (the primary trigger); optional/chosen keep defaults. // An EMPTY value ("") or a flag-shaped value restores the curated defaults @@ -241,15 +257,19 @@ if (require.main === module) { process.stdin.on('end', () => { const input = chunks.join(''); - const result = detectAssumptionDelta(input, termsOverride); - if (wantJson) { - process.stdout.write(JSON.stringify(result) + '\n'); + // Whitespace-only counts as empty (ADR-3889 §1's NO_INPUT: "zero units + // were in scope, and that emptiness is known to be genuine"). Real input + // — including a lone NUL byte, which .trim() does not strip — always + // falls through to the detector. + if (input.trim().length === 0) { + cliExit.terminateNow('NO_INPUT', wantJson ? { skipped: true, reason: 'no_input' } : undefined); } - process.exit(result.detected ? 0 : 1); + const result = detectAssumptionDelta(input, termsOverride); + cliExit.terminateNow(result.detected ? 'PASS' : 'FAIL', wantJson ? result : undefined); }); process.stdin.on('error', (err: Error) => { process.stderr.write(`ERROR: assumption-delta.cjs stdin read failed: ${err.message}\n`); - process.exit(2); + cliExit.terminateNow('UNAVAILABLE', wantJson ? { skipped: true, reason: 'stdin_error' } : undefined); }); } diff --git a/src/ui-safety-gate.cts b/src/ui-safety-gate.cts index cf4959741..e19cfafcc 100644 --- a/src/ui-safety-gate.cts +++ b/src/ui-safety-gate.cts @@ -1,6 +1,6 @@ /** * UI Safety Gate — shell-free implementation (ADR-457 build-at-publish: the - * hand-written bin/lib/ui-safety-gate.cjs collapsed to a TypeScript source of + * hand-written root copy of this file collapsed to a TypeScript source of * truth). Behaviour is preserved byte-for-behaviour from the prior hand-written * .cjs; only types are added. * @@ -21,13 +21,17 @@ * * CLI usage — reads phase-section text from STDIN to avoid ARG_MAX limits: * echo "$PHASE_SECTION" | node gsd-core/bin/lib/ui-safety-gate.cjs - * echo $? → 0 if UI tokens found, 1 if not, 2 on usage error + * echo $? → 0 if UI tokens found, 1 if not (real input, none found), + * NO_INPUT (registry, src/cli-exit.cts) if stdin was empty or + * whitespace-only, UNAVAILABLE (registry) if stdin read failed + * (ADR-3889 Phase 3, #3907) * - * Exit codes mirror grep: 0 = match found, 1 = no match, 2 = usage error. * * Canonical location: gsd-core/bin/lib/ui-safety-gate.cjs (#448) * This path is deployed by the GSD installer to $RUNTIME_DIR/gsd-core/bin/lib/. - * bin/lib/ui-safety-gate.cjs (root) is retained for source-repo and npm usage. + * The former root-level copy of this file (outside gsd-core/) was removed in + * #3907 as dead code: no installer reference, no workflow invocation, and no + * fallback chain in shipped content ever pointed at it. Do not re-create it. */ export interface UiPresenceResult { @@ -134,9 +138,29 @@ export function checkUiPresence(text: string): UiPresenceResult { // ── CLI entry point ───────────────────────────────────────────────────────── // Reads phase-section text from STDIN (not argv) to avoid OS ARG_MAX limits. // Invoked by workflow .md bash blocks as: echo "$PHASE_SECTION" | node .../ui-safety-gate.cjs -// Exit 0 = UI found, 1 = no UI, 2 = startup error. +// +// Exit codes (ADR-3889 Phase 3, #3907): 0 = UI found, 1 = no UI (real input, +// examined, nothing found), NO_INPUT (registry — see src/cli-exit.cts) = stdin +// closed with zero bytes (or whitespace-only), UNAVAILABLE (registry) = stdin +// read failed. The prior single "2 = startup error" arm conflated "I was +// handed nothing" with "the phase says it has no UI" — those are different +// answers; only the former is new here. `hint === 'no'` on REAL input still +// exits 1 — that is the author's own declaration, not an empty-input +// artifact. +// +// Terminates via terminateNow (src/cli-exit.cts), not raw process.exit: these +// exits fire from inside a stdin event handler, which is exactly what +// terminateNow (write-then-terminate) exists for, and it is total (cannot +// throw or return). This module never emits --json, so every payload here is +// `undefined` — terminateNow's JSON.stringify(undefined) is not a valid +// Buffer source and is swallowed by its own emission-failure guard, so +// nothing is written to stdout, preserving this CLI's historical contract of +// silence. if (require.main === module) { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const cliExit = require('./cli-exit.cjs') as { terminateNow: (outcome: string, payload: unknown) => never }; + // Collect stdin chunks asynchronously. const chunks: string[] = []; process.stdin.setEncoding('utf-8'); @@ -145,12 +169,19 @@ if (require.main === module) { process.stdin.on('end', () => { const input = chunks.join(''); + // Whitespace-only counts as empty (ADR-3889 §1's NO_INPUT: "zero units + // were in scope, and that emptiness is known to be genuine"). Real input + // — including a lone NUL byte, which .trim() does not strip — always + // falls through to the detector. + if (input.trim().length === 0) { + cliExit.terminateNow('NO_INPUT', undefined); + } const result = checkUiPresence(input); - process.exit(result.hasUI ? 0 : 1); + cliExit.terminateNow(result.hasUI ? 'PASS' : 'FAIL', undefined); }); process.stdin.on('error', (err: Error) => { process.stderr.write(`ERROR: ui-safety-gate.cjs stdin read failed: ${err.message}\n`); - process.exit(2); + cliExit.terminateNow('UNAVAILABLE', undefined); }); } diff --git a/tests/api-coverage.test.cjs b/tests/api-coverage.test.cjs index 905ba5789..bbc9058ba 100644 --- a/tests/api-coverage.test.cjs +++ b/tests/api-coverage.test.cjs @@ -1003,3 +1003,115 @@ describe('api-coverage CLI — STDIN + exit codes', () => { assert.strictEqual(r.exitCode, 1); }); }); + +// ────────────────────────────────────────────────────────────────────────────── +// ADR-3889 Phase 3 (#3907): NO_INPUT / UNAVAILABLE — empty/whitespace-only +// stdin and a stdin read error must not be reported as the authoritative +// "no integration" verdict (exit 1). Spawns the REAL module (not the pure +// detector) and asserts on the child's exit status + parsed stdout, per +// RULESET.TESTS. +// ────────────────────────────────────────────────────────────────────────────── +describe('api-coverage CLI — NO_INPUT / UNAVAILABLE (ADR-3889 Phase 3, #3907)', () => { + const CLI = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'api-coverage.cjs'); + const INJECT_STDIN_ERROR = path.join(__dirname, 'helpers', 'inject-stdin-error.cjs'); + const { exitCodeFor } = require('../gsd-core/bin/lib/exit-code-registry.cjs'); + const { runNode } = require('./helpers/process-seam.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + + function runCliJson(stdin, extraArgs = [], extraEnv = {}) { + const r = runNode([CLI, '--json', ...extraArgs], { + input: stdin, + timeoutMs: PROBE_TIMEOUT_MS, + env: { ...process.env, ...extraEnv }, + }); + return { exitCode: r.exitCode, stdout: r.stdout, stderr: r.stderr }; + } + + // ── The controls (load-bearing): without these, "always return NO_INPUT" + // would satisfy the empty/whitespace-only assertions below. ────────────── + test('control: detected input still exits 0 with unchanged --json payload', () => { + const r = runCliJson('Integrate the Stripe API for payments'); + assert.strictEqual(r.exitCode, 0); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, true); + assert.ok(!('skipped' in body), 'a detected result must not carry a skipped key'); + }); + + test('control: genuine-negative (real input, no signal) still exits 1, not NO_INPUT', () => { + const r = runCliJson('Refactor the login helper'); + assert.strictEqual(r.exitCode, 1); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, false); + assert.deepStrictEqual(body.signals, []); + assert.ok(!('skipped' in body), 'a genuine no-signal result must not carry a skipped key'); + }); + + test('empty stdin exits NO_INPUT (registry integer, never hardcoded)', () => { + const r = runCliJson(''); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + }); + + test('whitespace-only stdin (spaces / newlines / tabs / CRLF) exits NO_INPUT', () => { + for (const ws of [' ', '\n\n\n', '\t\t\t', '\r\n\r\n', ' \n\t\r\n ']) { + const r = runCliJson(ws); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT'), `ws=${JSON.stringify(ws)}`); + } + }); + + test('"x" and " x " are REAL input — must NOT be NO_INPUT', () => { + const bare = runCliJson('x'); + assert.notStrictEqual(bare.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(bare.exitCode, 1); + + const padded = runCliJson(' x '); + assert.notStrictEqual(padded.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(padded.exitCode, 1); + }); + + test('a NUL byte is real input (not stripped by whitespace trimming)', () => { + const r = runCliJson('\0'); + assert.notStrictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(r.exitCode, 1); + }); + + test('--json empty stdin: {"skipped":true,"reason":"no_input"} with NO detected key', () => { + const r = runCliJson(''); + const body = JSON.parse(r.stdout); + assert.deepStrictEqual(body, { skipped: true, reason: 'no_input' }); + assert.ok(!('detected' in body), 'skipped payload must not carry a detected key'); + }); + + test('--verbs foo with empty stdin is still NO_INPUT (flag does not bypass the input check)', () => { + const r = runCliJson('', ['--verbs', 'foo']); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + }); + + test('empty --verbs/--nouns value still restores curated defaults (unrelated to stdin gating)', () => { + const r = runCliJson('Integrate the Stripe API for payments', ['--verbs', '', '--nouns', '']); + assert.strictEqual(r.exitCode, 0, 'empty flag values must fall back to defaults → still detects'); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, true); + }); + + test('a stdin read error exits UNAVAILABLE (injected via monkeypatched process.stdin, not chmod)', () => { + const r = runNode(['-r', INJECT_STDIN_ERROR, CLI, '--json'], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, exitCodeFor('UNAVAILABLE')); + const body = JSON.parse(r.stdout); + assert.deepStrictEqual(body, { skipped: true, reason: 'stdin_error' }); + assert.ok(!('detected' in body)); + assert.notStrictEqual(body.reason, 'no_input', 'stdin_error must be distinguishable from no_input'); + }); + + test('NO_INPUT / UNAVAILABLE codes are identical under GSD_EXIT_CONTRACT=v1 and v2', () => { + for (const version of ['v1', 'v2']) { + const emptyResult = runCliJson('', [], { GSD_EXIT_CONTRACT: version }); + assert.strictEqual(emptyResult.exitCode, exitCodeFor('NO_INPUT'), `NO_INPUT under ${version}`); + + const errResult = runNode(['-r', INJECT_STDIN_ERROR, CLI, '--json'], { + timeoutMs: PROBE_TIMEOUT_MS, + env: { ...process.env, GSD_EXIT_CONTRACT: version }, + }); + assert.strictEqual(errResult.exitCode, exitCodeFor('UNAVAILABLE'), `UNAVAILABLE under ${version}`); + } + }); +}); diff --git a/tests/assumption-delta.test.cjs b/tests/assumption-delta.test.cjs index ccbce009a..1a5e420a5 100644 --- a/tests/assumption-delta.test.cjs +++ b/tests/assumption-delta.test.cjs @@ -6,13 +6,15 @@ * rendered prose — per RULESET.TESTS (no raw text matching on outputs). * * The detector mirrors ui-safety-gate.cts: a pure function plus a STDIN-reading - * CLI (exit 0 = signal detected, 1 = none, 2 = usage error). + * CLI (exit 0 = signal detected, 1 = none; NO_INPUT/UNAVAILABLE registry codes + * for empty/whitespace-only stdin and a stdin read error, per ADR-3889 Phase 3). */ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const { spawnSync } = require('node:child_process'); const path = require('node:path'); +const { exitCodeFor } = require('../gsd-core/bin/lib/exit-code-registry.cjs'); const MODULE_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'assumption-delta.cjs'); @@ -174,7 +176,10 @@ describe('detectAssumptionDelta — pure detector (#1561)', () => { }); describe('assumption-delta CLI — STDIN exit codes (mirrors ui-safety-gate)', () => { - // Exit code contract: 0 = signal detected, 1 = none, 2 = usage/startup error. + // Exit code contract: 0 = signal detected, 1 = genuine negative (real input, + // no signal). Empty/whitespace-only stdin and a stdin read error are NOT + // "1" — nothing was examined, so they resolve NO_INPUT / UNAVAILABLE via + // the exit-code registry (see the ADR-3889 Phase 3 describe block below). function runCli(stdin) { const res = spawnSync(process.execPath, [MODULE_PATH], { input: stdin, @@ -194,9 +199,13 @@ describe('assumption-delta CLI — STDIN exit codes (mirrors ui-safety-gate)', ( assert.strictEqual(r.status, 1); }); - test('exit 1 on empty stdin (no signal)', () => { + // Empty stdin is NOT a "no signal" verdict — nothing was examined, so it + // must be distinct from the genuine-negative case directly above (real + // input, no cue found). Per ADR-3889 Phase 3 (#3907), it resolves the + // registry's NO_INPUT code, never a hardcoded exit status. + test('exit NO_INPUT on empty stdin (nothing examined, not a genuine negative)', () => { const r = runCli(''); - assert.strictEqual(r.status, 1); + assert.strictEqual(r.status, exitCodeFor('NO_INPUT')); }); test('--json emits typed IR with detected field on stdout (exit 0)', () => { @@ -249,6 +258,115 @@ describe('assumption-delta CLI — STDIN exit codes (mirrors ui-safety-gate)', ( }); }); +// ────────────────────────────────────────────────────────────────────────────── +// ADR-3889 Phase 3 (#3907): NO_INPUT / UNAVAILABLE — empty/whitespace-only +// stdin and a stdin read error must not be reported as the authoritative +// "no signal" verdict (exit 1). Spawns the REAL module and asserts on the +// child's exit status + parsed stdout, per RULESET.TESTS. +// ────────────────────────────────────────────────────────────────────────────── +describe('assumption-delta CLI — NO_INPUT / UNAVAILABLE (ADR-3889 Phase 3, #3907)', () => { + const INJECT_STDIN_ERROR = path.join(__dirname, 'helpers', 'inject-stdin-error.cjs'); + const { runNode } = require('./helpers/process-seam.cjs'); + const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + + function runCliJson(stdin, extraArgs = [], extraEnv = {}) { + const r = runNode([MODULE_PATH, '--json', ...extraArgs], { + input: stdin, + timeoutMs: PROBE_TIMEOUT_MS, + env: { ...process.env, ...extraEnv }, + }); + return { exitCode: r.exitCode, stdout: r.stdout, stderr: r.stderr }; + } + + // ── The controls (load-bearing): without these, "always return NO_INPUT" + // would satisfy the empty/whitespace-only assertions below. ────────────── + test('control: detected input still exits 0 with unchanged --json payload', () => { + const r = runCliJson('This phase adds a second platform alongside the existing one.'); + assert.strictEqual(r.exitCode, 0); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, true); + assert.ok(!('skipped' in body)); + }); + + test('control: genuine-negative (real input, no signal) still exits 1, not NO_INPUT', () => { + const r = runCliJson('Refactor the login function to be smaller.'); + assert.strictEqual(r.exitCode, 1); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, false); + assert.deepStrictEqual(body.signals, []); + assert.ok(!('skipped' in body)); + }); + + test('empty stdin exits NO_INPUT (registry integer, never hardcoded)', () => { + const r = runCliJson(''); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + }); + + test('whitespace-only stdin (spaces / newlines / tabs / CRLF) exits NO_INPUT', () => { + for (const ws of [' ', '\n\n\n', '\t\t\t', '\r\n\r\n', ' \n\t\r\n ']) { + const r = runCliJson(ws); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT'), `ws=${JSON.stringify(ws)}`); + } + }); + + test('"x" and " x " are REAL input — must NOT be NO_INPUT', () => { + const bare = runCliJson('x'); + assert.notStrictEqual(bare.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(bare.exitCode, 1); + + const padded = runCliJson(' x '); + assert.notStrictEqual(padded.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(padded.exitCode, 1); + }); + + test('a NUL byte is real input (not stripped by whitespace trimming)', () => { + const r = runCliJson('\0'); + assert.notStrictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + assert.strictEqual(r.exitCode, 1); + }); + + test('--json empty stdin: {"skipped":true,"reason":"no_input"} with NO detected key', () => { + const r = runCliJson(''); + const body = JSON.parse(r.stdout); + assert.deepStrictEqual(body, { skipped: true, reason: 'no_input' }); + assert.ok(!('detected' in body)); + }); + + test('--terms foo with empty stdin is still NO_INPUT (flag does not bypass the input check)', () => { + const r = runCliJson('', ['--terms', 'foo']); + assert.strictEqual(r.exitCode, exitCodeFor('NO_INPUT')); + }); + + test('empty --terms value still restores curated defaults (unrelated to stdin gating)', () => { + const r = runCliJson('adds a second platform', ['--terms', '']); + assert.strictEqual(r.exitCode, 0, 'empty --terms must fall back to defaults → still detects'); + const body = JSON.parse(r.stdout); + assert.strictEqual(body.detected, true); + }); + + test('a stdin read error exits UNAVAILABLE (injected via monkeypatched process.stdin, not chmod)', () => { + const r = runNode(['-r', INJECT_STDIN_ERROR, MODULE_PATH, '--json'], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual(r.exitCode, exitCodeFor('UNAVAILABLE')); + const body = JSON.parse(r.stdout); + assert.deepStrictEqual(body, { skipped: true, reason: 'stdin_error' }); + assert.ok(!('detected' in body)); + assert.notStrictEqual(body.reason, 'no_input', 'stdin_error must be distinguishable from no_input'); + }); + + test('NO_INPUT / UNAVAILABLE codes are identical under GSD_EXIT_CONTRACT=v1 and v2', () => { + for (const version of ['v1', 'v2']) { + const emptyResult = runCliJson('', [], { GSD_EXIT_CONTRACT: version }); + assert.strictEqual(emptyResult.exitCode, exitCodeFor('NO_INPUT'), `NO_INPUT under ${version}`); + + const errResult = runNode(['-r', INJECT_STDIN_ERROR, MODULE_PATH, '--json'], { + timeoutMs: PROBE_TIMEOUT_MS, + env: { ...process.env, GSD_EXIT_CONTRACT: version }, + }); + assert.strictEqual(errResult.exitCode, exitCodeFor('UNAVAILABLE'), `UNAVAILABLE under ${version}`); + } + }); +}); + // ─── Hardening (Codex Step-4 review fixes) ──────────────────────────────────── describe('assumption-delta hardening (Codex review)', () => { const { detectAssumptionDelta } = require(MODULE_PATH); diff --git a/tests/check-ui-safety-gate.test.cjs b/tests/check-ui-safety-gate.test.cjs index 604ad6a52..7517ee474 100644 --- a/tests/check-ui-safety-gate.test.cjs +++ b/tests/check-ui-safety-gate.test.cjs @@ -25,6 +25,7 @@ const os = require('node:os'); const path = require('node:path'); const { cleanup } = require('./helpers.cjs'); +const { copyScriptWithDeps } = require('./helpers/copy-script-fixture.cjs'); const { computeUiSafetyGate } = require('../gsd-core/bin/lib/check-command-router.cjs'); // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -276,9 +277,9 @@ describe('computeUiSafetyGate — ui.safety-gate check logic (#1168)', () => { * Root cause (#3718): shell-based invocation (with locale env-var prefix) silently * degrades on Windows PowerShell — the prefix is not recognised by pwsh. * - * Fix (#3718, Approach A): gate logic moved to `bin/lib/ui-safety-gate.cjs` (Node.js). + * Fix (#3718, Approach A): gate logic moved to `gsd-core/bin/lib/ui-safety-gate.cjs` (Node.js). * Reads phase text from STDIN (not argv) to avoid OS ARG_MAX limits. - * Invoked as: printf '%s' "$PHASE_SECTION" | node "${GSD_REPO_ROOT}/bin/lib/ui-safety-gate.cjs" + * Invoked as: printf '%s' "$PHASE_SECTION" | node "${GSD_REPO_ROOT}/gsd-core/bin/lib/ui-safety-gate.cjs" * Path anchored to repo root via `git rev-parse --show-toplevel`. * * Test strategy: @@ -300,7 +301,7 @@ const { runNode, runHook } = require('./helpers/process-seam.cjs'); const { toLegacyResult } = require('./helpers/git-fixture.cjs'); const { PROBE_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); -const HELPER_PATH = path.join(__dirname, '..', 'bin', 'lib', 'ui-safety-gate.cjs'); +const HELPER_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'ui-safety-gate.cjs'); const PLAN_PHASE_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'plan-phase.md'); const AUTONOMOUS_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'autonomous.md'); const AUTONOMOUS_UI_DESIGN_CONTRACT_REF_PATH = path.join( @@ -418,9 +419,15 @@ describe('Cross-shell portability — spawnSync with shell:false, stdin (#3718)' assert.strictEqual(result.status, 1, `Expected exit 1 (no UI), got ${result.status}. stderr: ${result.stderr}`); }); - test('spawn with shell:false + stdin: empty input exits 1', () => { + test('spawn with shell:false + stdin: empty input exits NO_INPUT (#3907)', () => { + // See the `gsd-core/bin/lib/ui-safety-gate.cjs CLI — NO_INPUT / UNAVAILABLE` + // describe block below for the full NO_INPUT/UNAVAILABLE coverage matrix. + const { exitCodeFor } = require('../gsd-core/bin/lib/exit-code-registry.cjs'); const result = spawnGate(''); - assert.strictEqual(result.status, 1, `Expected exit 1 (no UI for empty input), got ${result.status}. stderr: ${result.stderr}`); + assert.strictEqual( + result.status, exitCodeFor('NO_INPUT'), + `Expected NO_INPUT (${exitCodeFor('NO_INPUT')}) for empty input, got ${result.status}. stderr: ${result.stderr}`, + ); }); test('spawn with shell:false + stdin: CRLF line endings handled correctly', () => { @@ -449,6 +456,120 @@ describe('Cross-shell portability — spawnSync with shell:false, stdin (#3718)' }); }); +// ── ADR-3889 Phase 3 (#3907): NO_INPUT / UNAVAILABLE exit codes ─────────────── +// The prior single "2 = startup error" arm was bound to stdin.on('error') only +// — there was no arm for stdin closed with ZERO BYTES, so empty input flowed +// into the detector and exited 1 ("no UI"), asserting a verdict about input +// that was never examined. These tests spawn the REAL module and assert on +// the child's exit status, per RULESET.TESTS. +// +// NOTE (scope note): this block targets `gsd-core/bin/lib/ui-safety-gate.cjs` +// — the tsc-compiled output of `src/ui-safety-gate.cts`, the module #3907 +// fixed. That is also the only shipped copy: the hand-written root +// `bin/lib/ui-safety-gate.cjs` was removed as dead code (#3907) — no +// installer reference, no workflow invocation, no fallback chain — so there +// is no second copy to keep in sync. +describe('gsd-core/bin/lib/ui-safety-gate.cjs CLI — NO_INPUT / UNAVAILABLE (ADR-3889 Phase 3, #3907)', () => { + const { exitCodeFor } = require('../gsd-core/bin/lib/exit-code-registry.cjs'); + const GSD_CORE_HELPER_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'ui-safety-gate.cjs'); + const INJECT_STDIN_ERROR = path.join(__dirname, 'helpers', 'inject-stdin-error.cjs'); + + function spawnCompiledGate(input, extraEnv = {}) { + return runNode([GSD_CORE_HELPER_PATH], { + input, + env: { ...process.env, ...extraEnv }, + timeoutMs: PROBE_TIMEOUT_MS, + }); + } + + // ── The controls (load-bearing): without these, "always return NO_INPUT" + // would satisfy the empty/whitespace-only assertions below. ────────────── + test('control: a detected input still exits 0', () => { + const result = spawnCompiledGate('Build the analytics dashboard.'); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + }); + + test('control: a genuine-negative (real input, no signal) still exits 1, not NO_INPUT', () => { + const result = spawnCompiledGate('Requirements analysis for the backend service.'); + assert.strictEqual(result.exitCode, 1, `stderr: ${result.stderr}`); + }); + + test('whitespace-only stdin (spaces / newlines / tabs / CR) exits NO_INPUT', () => { + for (const ws of [' ', '\n\n\n', '\t\t\t', '\r\r\r', ' \n\t\r\n ']) { + const result = spawnCompiledGate(ws); + assert.strictEqual( + result.exitCode, exitCodeFor('NO_INPUT'), + `whitespace-only input ${JSON.stringify(ws)} must exit NO_INPUT; got ${result.exitCode}. stderr: ${result.stderr}`, + ); + } + }); + + test('"x" and " x " are REAL input — must NOT be NO_INPUT', () => { + const bare = spawnCompiledGate('x'); + assert.notStrictEqual(bare.exitCode, exitCodeFor('NO_INPUT'), `"x" must not be NO_INPUT; stderr: ${bare.stderr}`); + assert.strictEqual(bare.exitCode, 1, `"x" carries no UI token, so it is the genuine negative (1); stderr: ${bare.stderr}`); + + const padded = spawnCompiledGate(' x '); + assert.notStrictEqual(padded.exitCode, exitCodeFor('NO_INPUT'), `" x " must not be NO_INPUT; stderr: ${padded.stderr}`); + assert.strictEqual(padded.exitCode, 1, `" x " carries no UI token, so it is the genuine negative (1); stderr: ${padded.stderr}`); + }); + + test('a NUL byte is real input (not stripped by whitespace trimming) — falls through to the detector', () => { + const result = spawnCompiledGate('\0'); + assert.notStrictEqual(result.exitCode, exitCodeFor('NO_INPUT'), `NUL byte must not be treated as empty; stderr: ${result.stderr}`); + assert.strictEqual(result.exitCode, 1, `NUL byte alone carries no UI token; stderr: ${result.stderr}`); + }); + + test('#3907 negative space: an explicit `**UI hint**: no` on REAL input still exits 1, not NO_INPUT', () => { + // "the phase says it has no UI" and "I was handed nothing" are different + // answers (ui-safety-gate.cts:124-126 returns hasUI:false deliberately here). + const result = spawnCompiledGate('**UI hint**: no\n\nBuild the dashboard UI.\n'); + assert.strictEqual(result.exitCode, 1, `hint:no on real input must exit 1, not NO_INPUT; stderr: ${result.stderr}`); + assert.notStrictEqual(result.exitCode, exitCodeFor('NO_INPUT')); + }); + + test('a stdin read error exits UNAVAILABLE (injected via monkeypatched process.stdin, not chmod)', () => { + const result = runNode(['-r', INJECT_STDIN_ERROR, GSD_CORE_HELPER_PATH], { timeoutMs: PROBE_TIMEOUT_MS }); + assert.strictEqual( + result.exitCode, exitCodeFor('UNAVAILABLE'), + `stdin read error must exit UNAVAILABLE (${exitCodeFor('UNAVAILABLE')}); got ${result.exitCode}. stderr: ${result.stderr}`, + ); + }); + + test('NO_INPUT / UNAVAILABLE codes are identical under GSD_EXIT_CONTRACT=v1 and v2', () => { + for (const version of ['v1', 'v2']) { + const emptyResult = spawnCompiledGate('', { GSD_EXIT_CONTRACT: version }); + assert.strictEqual( + emptyResult.exitCode, exitCodeFor('NO_INPUT'), + `NO_INPUT must be ${exitCodeFor('NO_INPUT')} under ${version}; got ${emptyResult.exitCode}`, + ); + + const errResult = runNode(['-r', INJECT_STDIN_ERROR, GSD_CORE_HELPER_PATH], { + env: { ...process.env, GSD_EXIT_CONTRACT: version }, + timeoutMs: PROBE_TIMEOUT_MS, + }); + assert.strictEqual( + errResult.exitCode, exitCodeFor('UNAVAILABLE'), + `UNAVAILABLE must be ${exitCodeFor('UNAVAILABLE')} under ${version}; got ${errResult.exitCode}`, + ); + } + }); +}); + +// ── The root copy stays dead (#3907) ────────────────────────────────────────── +// The hand-written root `bin/lib/ui-safety-gate.cjs` was removed as dead code: +// no installer reference (bin/install.js only ever requires +// gsd-core/bin/lib/*), no workflow or capability content invokes it, and it +// had no fallback chain in shipped content. This guards against it silently +// coming back (e.g. a re-added generator, a stray hand copy). +describe('bin/lib/ui-safety-gate.cjs (root copy) — removed as dead code (#3907)', () => { + test('root bin/lib/ui-safety-gate.cjs does not exist', () => { + const rootCopy = path.join(__dirname, '..', 'bin', 'lib', 'ui-safety-gate.cjs'); + assert.strictEqual(fs.existsSync(rootCopy), false, + 'bin/lib/ui-safety-gate.cjs must not exist — it was removed as dead code in #3907'); + }); +}); + // ── Behavioral test matrix (via module import) ──────────────────────────────── /** @@ -572,7 +693,7 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep // the CWD is a consuming project that has no bin/lib of its own. const GATE_SNIPPET = [ '_GSD_RT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"', - 'UI_GATE_JS=$(for _c in "$_GSD_RT/gsd-core/bin/lib/ui-safety-gate.cjs" "$_GSD_RT/bin/lib/ui-safety-gate.cjs" "$_GSD_RT/.claude/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/gsd-core/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/bin/lib/ui-safety-gate.cjs"; do [ -f "$_c" ] && { echo "$_c"; break; }; done)', + 'UI_GATE_JS=$(for _c in "$_GSD_RT/gsd-core/bin/lib/ui-safety-gate.cjs" "$_GSD_RT/.claude/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/gsd-core/bin/lib/ui-safety-gate.cjs" "$HOME/.claude/bin/lib/ui-safety-gate.cjs"; do [ -f "$_c" ] && { echo "$_c"; break; }; done)', 'if [ -n "$UI_GATE_JS" ]; then printf \'%s\' "$PHASE_SECTION" | node "$UI_GATE_JS" >/dev/null 2>&1; HAS_UI=$?; else HAS_UI=0; fi', 'echo "$HAS_UI"', ].join('\n'); @@ -622,12 +743,11 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep const fakeRuntime = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-installed-rt-')); const consumingProject = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-consuming-')); try { - const installedLibDir = path.join(fakeRuntime, 'gsd-core', 'bin', 'lib'); - fs.mkdirSync(installedLibDir, { recursive: true }); - fs.copyFileSync( - path.join(REPO_ROOT, 'gsd-core', 'bin', 'lib', 'ui-safety-gate.cjs'), - path.join(installedLibDir, 'ui-safety-gate.cjs') - ); + // ui-safety-gate.cjs requires ./cli-exit.cjs (-> ./exit-code-registry.cjs), + // so a hand-copy of just the one file would MODULE_NOT_FOUND. Walk the + // require graph instead (tests/helpers/copy-script-fixture.cjs) — see + // its docstring for the #3412-class bug this avoids. + copyScriptWithDeps(REPO_ROOT, fakeRuntime, path.join('gsd-core', 'bin', 'lib', 'ui-safety-gate.cjs')); // Same bash FAN-OUT class as runGateFrom above (git rev-parse + a // candidate-path probe loop + node) — see HOOK_FANOUT_TIMEOUT_MS in @@ -678,10 +798,30 @@ describe('checkUiPresence() return value API', () => { assert.strictEqual(uiCount, 1, 'Duplicate tokens must be deduplicated'); }); - test('non-string input returns { hasUI: false, tokens: [] }', () => { - assert.deepStrictEqual(checkUiPresence(null), { hasUI: false, tokens: [] }); - assert.deepStrictEqual(checkUiPresence(undefined), { hasUI: false, tokens: [] }); - assert.deepStrictEqual(checkUiPresence(42), { hasUI: false, tokens: [] }); + test('non-string input returns { hasUI: false, tokens: [], matchedToken: null, matchedLine: null }', () => { + const expected = { hasUI: false, tokens: [], matchedToken: null, matchedLine: null }; + assert.deepStrictEqual(checkUiPresence(null), expected); + assert.deepStrictEqual(checkUiPresence(undefined), expected); + assert.deepStrictEqual(checkUiPresence(42), expected); + }); + + test('UI-present input sets matchedToken/matchedLine to the first match', () => { + const result = checkUiPresence('Build the dashboard and UI form'); + assert.strictEqual(result.matchedToken, 'dashboard'); + assert.strictEqual(result.matchedLine, 'Build the dashboard and UI form'); + }); + + test('no-UI input sets matchedToken/matchedLine to null', () => { + const result = checkUiPresence('Requirements: backend REST API only'); + assert.strictEqual(result.matchedToken, null); + assert.strictEqual(result.matchedLine, null); + }); + + test('"**UI hint**: no" short-circuits to hasUI:false with matchedToken/matchedLine null', () => { + const result = checkUiPresence('**UI hint**: no\nBuild the dashboard'); + assert.strictEqual(result.hasUI, false); + assert.strictEqual(result.matchedToken, null); + assert.strictEqual(result.matchedLine, null); }); test('multiple distinct UI tokens on same line are ALL captured', () => { diff --git a/tests/helpers/inject-stdin-error.cjs b/tests/helpers/inject-stdin-error.cjs new file mode 100644 index 000000000..7a3f8ec3a --- /dev/null +++ b/tests/helpers/inject-stdin-error.cjs @@ -0,0 +1,41 @@ +'use strict'; + +/** + * Deterministic stdin-read-failure injector for exit-code tests (ADR-3889 + * Phase 3, #3907). + * + * Cross-platform IO-failure injection must be done via method monkeypatching, + * never mode-bit tricks (`fs.chmodSync(path, 0o000)` is bypassed by root in + * Docker/CI and would assert zero coverage there). This module replaces + * `process.stdin` with a fake EventEmitter BEFORE the target CLI module loads + * — load it with `node -r tests/helpers/inject-stdin-error.cjs ` + * — so the target's own `process.stdin.on('data'|'end'|'error', ...)` + * subscriptions attach to the fake, and the fake emits ONLY `'error'` + * (never `'data'`/`'end'`), deterministically exercising the module's + * stdin-read-failure arm without any real pipe/fd involved. + * + * `-r` (require preload) runs before Node loads the main module, so the + * spawned CLI still sees `require.main === module` for ITSELF — the + * preload script's own module identity never satisfies that check — which is + * what lets the target's `if (require.main === module)` CLI entry point run + * normally, reading from (the now-fake) `process.stdin`. + */ + +const { EventEmitter } = require('node:events'); + +const fakeStdin = new EventEmitter(); +fakeStdin.setEncoding = () => {}; +fakeStdin.resume = () => {}; +fakeStdin.pause = () => {}; + +Object.defineProperty(process, 'stdin', { + value: fakeStdin, + configurable: true, +}); + +// Fire on the next tick — after the target module's require.main===module +// block has synchronously registered its 'data'/'end'/'error' listeners — +// so the 'error' event is never emitted to zero listeners. +setImmediate(() => { + fakeStdin.emit('error', new Error('simulated stdin read failure (injected by inject-stdin-error.cjs)')); +}); diff --git a/tests/removed-but-needed-lint.test.cjs b/tests/removed-but-needed-lint.test.cjs index 0fe89757a..c8d563161 100644 --- a/tests/removed-but-needed-lint.test.cjs +++ b/tests/removed-but-needed-lint.test.cjs @@ -1,4 +1,4 @@ -// docs-guard-exempt: docs/getting-started.md and docs/setup.md are synthetic { file, content } corpus fixtures fed to a lint checker, not real repo docs. +// docs-guard-exempt: docs/getting-started.md, docs/setup.md, docs/README.md, and docs/some-doc.md are synthetic { file, content } corpus fixtures fed to a lint checker, not real repo docs. 'use strict'; process.env.GSD_TEST_MODE = '1'; @@ -22,6 +22,8 @@ const ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(ROOT, 'scripts', 'lint-removed-but-needed.cjs'); const { referencesBasename, + referencesPath, + buildSurvivingBasenames, referencesNpmLockfileDependency, findSurvivingReferences, classifyTestReference, @@ -65,6 +67,84 @@ describe('removed-but-needed lint: referencesNpmLockfileDependency (pure)', () = }); }); +// ─── #3907: basename-collision refinement — match by full path, not bare +// basename, when the deleted file's basename also belongs to a surviving +// file. The original defect shape: #3907 deleted `bin/lib/ui-safety-gate.cjs` +// while the SEPARATE, still-live `gsd-core/bin/lib/ui-safety-gate.cjs` +// survived — every reference to the survivor (including the survivor +// referencing itself) was misattributed to the deletion. + +describe('removed-but-needed lint: referencesPath (pure, #3907)', () => { + test('an exact full-path reference is found', () => { + assert.equal(referencesPath('see bin/lib/ui-safety-gate.cjs for the old copy', 'bin/lib/ui-safety-gate.cjs'), true); + }); + + test('a longer path that has the target as a SUFFIX is NOT a false match', () => { + assert.equal( + referencesPath('canonical: gsd-core/bin/lib/ui-safety-gate.cjs', 'bin/lib/ui-safety-gate.cjs'), + false, + ); + }); + + test('no reference at all is not found', () => { + assert.equal(referencesPath('nothing to see here', 'bin/lib/ui-safety-gate.cjs'), false); + }); +}); + +describe('removed-but-needed lint: buildSurvivingBasenames (pure, #3907)', () => { + test('collects basenames from repo-relative paths', () => { + const set = buildSurvivingBasenames(['gsd-core/bin/lib/ui-safety-gate.cjs', 'docs/README.md']); + assert.equal(set.has('ui-safety-gate.cjs'), true); + assert.equal(set.has('README.md'), true); + assert.equal(set.has('nope.md'), false); + }); +}); + +describe('removed-but-needed lint: findSurvivingReferences basename-collision refinement (#3907)', () => { + test('PROBE 1 — unique basename, surviving reference STILL FAILS (refinement did not neuter the guard)', () => { + const violations = findSurvivingReferences( + ['bin/lib/unique-only.cjs'], + [{ file: 'gsd-core/some-consumer.cjs', content: "require('./unique-only.cjs')" }], + buildSurvivingBasenames(['gsd-core/some-consumer.cjs']), + ); + assert.equal(violations.length, 1); + assert.match(violations[0].reason, /basename 'unique-only\.cjs' still referenced/); + }); + + test('PROBE 2 — colliding basename, FULL-PATH reference to the deleted file STILL FAILS', () => { + const survivingBasenames = buildSurvivingBasenames(['gsd-core/bin/lib/ui-safety-gate.cjs']); + const violations = findSurvivingReferences( + ['bin/lib/ui-safety-gate.cjs'], + [{ file: 'docs/some-doc.md', content: 'see bin/lib/ui-safety-gate.cjs for the old copy' }], + survivingBasenames, + ); + assert.equal(violations.length, 1); + assert.match(violations[0].reason, /full path 'bin\/lib\/ui-safety-gate\.cjs' still referenced/); + }); + + test('PROBE 3 — colliding basename, bare-basename reference only PASSES (the actual #3907 shape)', () => { + const survivingBasenames = buildSurvivingBasenames(['gsd-core/bin/lib/ui-safety-gate.cjs']); + const violations = findSurvivingReferences( + ['bin/lib/ui-safety-gate.cjs'], + [ + { file: 'gsd-core/bin/lib/check-command-router.cjs', content: "require('./ui-safety-gate.cjs')" }, + { file: 'gsd-core/bin/lib/ui-safety-gate.cjs', content: '// canonical gsd-core/bin/lib/ui-safety-gate.cjs' }, + ], + survivingBasenames, + ); + assert.deepEqual(violations, []); + }); + + test('no collision (default empty set): behaviour is unchanged from the original basename rule', () => { + const violations = findSurvivingReferences( + ['bin/lib/ui-safety-gate.cjs'], + [{ file: 'gsd-core/bin/lib/check-command-router.cjs', content: "require('./ui-safety-gate.cjs')" }], + ); + assert.equal(violations.length, 1); + assert.match(violations[0].reason, /basename 'ui-safety-gate\.cjs' still referenced/); + }); +}); + describe('removed-but-needed lint: findSurvivingReferences (pure)', () => { test('the real #3316 defect shape IS flagged: package-lock.json deleted, workflow still runs npm ci', () => { const violations = findSurvivingReferences( @@ -231,6 +311,58 @@ describe('removed-but-needed lint: main() end-to-end wiring', () => { }); }); +describe('removed-but-needed lint: main() end-to-end, basename-collision refinement (#3907)', () => { + test('exit 0 reproducing the real #3907 shape: deleted root copy, surviving gsd-core twin referencing itself', (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-removed-but-needed-e2e-collision-clean-')); + t.after(() => cleanup(tmpDir)); + buildTempRepo( + tmpDir, + [ + { file: 'bin/lib/ui-safety-gate.cjs', content: '// root copy\n' }, + { + file: 'gsd-core/bin/lib/ui-safety-gate.cjs', + content: '// canonical gsd-core/bin/lib/ui-safety-gate.cjs\nmodule.exports = {};\n', + }, + { + file: 'gsd-core/bin/lib/check-command-router.cjs', + content: "require('./ui-safety-gate.cjs');\n", + }, + ], + [{ file: 'bin/lib/ui-safety-gate.cjs', content: null }], + ); + const scriptCopy = copyScriptInto(tmpDir); + const result = runNode( + [scriptCopy], + { cwd: tmpDir, env: { ...process.env, GSD_REMOVED_BUT_NEEDED_BASE: 'main' } }, + ); + assert.equal(result.exitCode, 0, `expected exit 0, got ${result.exitCode}: ${result.stderr}`); + }); + + test('exit 1 when, despite the basename collision, something still references the deleted file by its FULL PATH', (t) => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-removed-but-needed-e2e-collision-full-path-')); + t.after(() => cleanup(tmpDir)); + buildTempRepo( + tmpDir, + [ + { file: 'bin/lib/ui-safety-gate.cjs', content: '// root copy\n' }, + { + file: 'gsd-core/bin/lib/ui-safety-gate.cjs', + content: '// canonical gsd-core/bin/lib/ui-safety-gate.cjs\nmodule.exports = {};\n', + }, + { file: 'docs/setup.md', content: 'run: node bin/lib/ui-safety-gate.cjs to check\n' }, + ], + [{ file: 'bin/lib/ui-safety-gate.cjs', content: null }], + ); + const scriptCopy = copyScriptInto(tmpDir); + const result = runNode( + [scriptCopy], + { cwd: tmpDir, env: { ...process.env, GSD_REMOVED_BUT_NEEDED_BASE: 'main' } }, + ); + assert.equal(result.exitCode, 1, `expected exit 1, got ${result.exitCode}: ${result.stderr}`); + assert.match(result.stderr, /full path 'bin\/lib\/ui-safety-gate\.cjs' still referenced/); + }); +}); + // ─── #3565: tests/ arm — pins-existence vs asserts-absence ────────────────── describe('removed-but-needed lint: classifyTestReference (pure, #3565)', () => {