diff --git a/.changeset/calm-lemurs-sing.md b/.changeset/calm-lemurs-sing.md new file mode 100644 index 000000000..a38c2d1c7 --- /dev/null +++ b/.changeset/calm-lemurs-sing.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3416 +--- +**GSD now requires Node 24 or newer** — the `engines.node` floor moves from 22 to 24, and the Node 22 test lane is retired. Node 22 entered Maintenance LTS and this project tracks the Active LTS line; the change is what lets regex escaping delegate to the built-in `RegExp.escape` instead of a hand-rolled implementation. If you are on Node 22, upgrade before updating GSD. diff --git a/.github/ISSUE_TEMPLATE/chore.yml b/.github/ISSUE_TEMPLATE/chore.yml index 1bb45cfc4..ce3ecf642 100644 --- a/.github/ISSUE_TEMPLATE/chore.yml +++ b/.github/ISSUE_TEMPLATE/chore.yml @@ -89,7 +89,7 @@ body: placeholder: | - [ ] All test files use `node:assert/strict` - [ ] Zero `try/finally` cleanup blocks in test lifecycle code - - [ ] CI green on all matrix entries (Node 22/24, Ubuntu/macOS/Windows) + - [ ] CI green on all matrix entries (Node 24, Ubuntu/macOS/Windows) - [ ] No change to user-facing behavior validations: required: true diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml index 0b62bd48c..a487873ef 100644 --- a/.github/workflows/install-smoke.yml +++ b/.github/workflows/install-smoke.yml @@ -9,7 +9,7 @@ name: Install Smoke # installing from an unpacked local directory, so any stale tsc output lacking # execute bits will be caught by the unpacked job before release. # -# - PRs: path-filtered, minimal runner (ubuntu + Node LTS) for fast signal. +# - PRs: path-filtered, minimal runner (ubuntu + Node 24) for fast signal. # - Push to release branches / main: full matrix. # - workflow_call: invoked from release.yml as a pre-publish gate. @@ -65,16 +65,15 @@ jobs: strategy: fail-fast: false matrix: - # PRs run the minimal path (ubuntu + LTS). Pushes / release branches - # and workflow_call add macOS + Node 24 coverage. + # PRs run the minimal path (ubuntu + Node 24, the engines.node floor). + # Pushes / release branches and workflow_call add macOS coverage. + # (Node floor is 24 everywhere now, so the prior "ubuntu + Node 24, + # full_only" row is identical to this minimal row and was dropped — + # it would otherwise re-run the same os/node combo twice on push.) include: - - os: ubuntu-latest - node-version: 22 - full_only: false - shell: bash - os: ubuntu-latest node-version: 24 - full_only: true + full_only: false shell: bash - os: macos-latest node-version: 24 @@ -218,10 +217,10 @@ jobs: if: github.event_name == 'pull_request' run: node scripts/ci-rebase-check.cjs - - name: Set up Node.js 22 + - name: Set up Node.js 24 uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install root deps diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ccaa5a59e..dbbf3ac33 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -158,10 +158,13 @@ jobs: fail-fast: false matrix: include: - # Required default PR lanes: fast signal, minimum runtime, primary runtime, - # and one Windows shell/path lane. The non-primary OS/runtime lanes run - # scoped tests from scripts/ci-test-scope.cjs; Ubuntu/Node 24 runs the - # broader default suite. + # Required default PR lanes: fast signal (targeted scope), primary + # runtime (full scope, sharded), and one Windows shell/path lane. The + # Node floor is 24 (engines.node >=24.0.0), so the fast-signal lane + # runs the same Node version as the full-scope lanes; only the test + # selection (scope: targeted vs full) differs. The non-primary + # OS/runtime lanes run scoped tests from scripts/ci-test-scope.cjs; + # Ubuntu/Node 24 runs the broader default suite. # # #2952: the `scope: full` lane is SHARDED three ways. It was the only # unsharded lane in this file, and the whole unit suite under c8 on one @@ -180,7 +183,7 @@ jobs: # install-heavy suites. Its shards use the same `--shard i/n` # flag on scripts/run-tests.cjs, applied AFTER scope selection. - os: ubuntu-latest - node-version: 22 + node-version: 24 scope: targeted - os: ubuntu-latest node-version: 24 @@ -364,10 +367,10 @@ jobs: # CI stays green. base.sha is fixed for the life of the run. CI_REBASE_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: node scripts/ci-rebase-check.cjs - - name: Set up Node.js 22 + - name: Set up Node.js 24 uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Environment check run: npm run check:env @@ -401,8 +404,10 @@ jobs: # cliff. The cap stays at 20m as a generous backstop; a healthy shard now # finishes in roughly a third of the old single-lane wall-clock. # - # The matrix is the cross-product of 3 OS/node legs × 3 shards = 9 jobs, - # enumerated explicitly as `include:` rows. (A base `shard: [1,2,3]` + # The matrix is the cross-product of 2 OS/node legs × 3 shards = 6 jobs + # (windows-latest/24, macos-latest/24 — the Node floor is 24, so there is + # no separate node-22 leg to cross-product against), enumerated explicitly + # as `include:` rows. (A base `shard: [1,2,3]` # dimension would NOT cross-product against `include` legs — include rows # sharing no key with the base matrix are appended as standalone combos — # and a NESTED `leg.os` key is not resolvable by the H1 shell-policy linter @@ -433,29 +438,17 @@ jobs: matrix: include: - os: windows-latest - node-version: 22 + node-version: 24 shell: pwsh shard: 1 - os: windows-latest - node-version: 22 + node-version: 24 shell: pwsh shard: 2 - os: windows-latest - node-version: 22 + node-version: 24 shell: pwsh shard: 3 - - os: macos-latest - node-version: 22 - shell: 'zsh {0}' - shard: 1 - - os: macos-latest - node-version: 22 - shell: 'zsh {0}' - shard: 2 - - os: macos-latest - node-version: 22 - shell: 'zsh {0}' - shard: 3 - os: macos-latest node-version: 24 shell: 'zsh {0}' diff --git a/.gitignore b/.gitignore index 8770a8ed8..cb90eb946 100644 --- a/.gitignore +++ b/.gitignore @@ -196,6 +196,7 @@ build/ /gsd-core/bin/lib/planning-workspace.cjs /gsd-core/bin/lib/planning-scope.cjs /gsd-core/bin/lib/planning-snapshot.cjs +/gsd-core/bin/lib/pattern.cjs /gsd-core/bin/lib/health-diagnostic-types.cjs /gsd-core/bin/lib/health-diagnostic.cjs /gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs diff --git a/.nvmrc b/.nvmrc index 2bd5a0a98..a45fd52cc 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -22 +24 diff --git a/CONTEXT.md b/CONTEXT.md index fa98510f7..d5ecd0d49 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -103,6 +103,9 @@ Module owning legacy-key normalization, defaults merge, and explicit on-disk mig ### Planning Scope Module Leaf module owning the frozen `SCOPE` discriminator (`COMPLETE` / `TRUNCATED` / `UNSCOPED` / `UNREADABLE`) that every consolidated `.planning/` semantic derivation returns alongside its payload, per ADR-3180 Decision 2. It exists to make one distinction representable: `COMPLETE` with zero items is a REAL answer (a phase genuinely has no plans; a milestone genuinely has no phases yet), while the other three with zero items are NON-answers — the derivation could not see all of its input. Before it, those two cases were output-identical, which is the failure class epic #3180 removes: a truncated milestone window returned `phase_count: 0` with no error, indistinguishable from a freshly-declared milestone. It is a frozen enum rather than a message string because `CONTRIBUTING.md` bans raw-text matching on outputs and requires a typed IR, so callers branch on `result.scope === SCOPE.TRUNCATED`. Pure and import-free — the bottom of the dependency graph, so any consumer can depend on it without a cycle (mirrors the Phase Id Module's leaf position). Source of truth: `gsd-core/bin/lib/planning-scope.cjs` (generated from `src/planning-scope.cts`). The contract is PROVISIONAL: #3183 is its first real implementation, and ADR-3180 requires the ADR be amended before Phase 2 rather than the contract worked around, if it does not fit. +### Pattern Module +Leaf module owning the construction of a regex from a **runtime value**, per ADR-3212 §1 (epic #3212 Phase 1, #3412). Exposes `escapeRegex(value) → string` and `literalPattern(value, flags?) → RegExp`. `escapeRegex` **delegates to the built-in `RegExp.escape`** (TC39 Stage 4, ES2026, Node 24+) — it is deliberately NOT an implementation, which is the whole point: the ~39 hand-rolled copies it replaces existed because every author re-derived the metacharacter set, and TC39 standardized the primitive precisely because userland versions "miss edge cases." `escapeRegex` is the primary export (the large majority of call sites build a regex *source string* and interpolate it into a larger pattern); `literalPattern` is the minority convenience for the `new RegExp(escapeRegex(v))` shape. Pure and import-free — a leaf, so any consumer can depend on it without a cycle (mirrors the Planning Scope and Phase Id modules' position). **Behavioral note, measured not assumed:** `RegExp.escape` is a *superset* escaper and produces different pattern SOURCE TEXT than the hand-rolled class did — it hex-escapes the leading character of nearly every string (`"abc"` → `"\x61bc"`), plus `-`, space, `/`, and control chars. It is **match-equivalent** (verified by a seeded `fast-check` property test against the deleted implementation as oracle, plus a fixed corpus), so no consumer's matching behavior changes; but anything asserting on pattern text rather than match results does. It also **fixes a latent bug as a side effect**: a hyphen-bearing value interpolated into a character class previously formed a real range (`[a-z]` built from an escaped `"a-z"` matched `"m"`), and no longer does. Enforced by `eslint-rules/no-adhoc-regex-escape.cjs` (fires on the `.replace(, '\\$&')` shape anywhere outside this module, matching on shape rather than exact bytes, and on `new RegExp` built from an unescaped runtime value; exempts reviewed pattern-fragment constants such as `PHASE_NUMBER_TOKEN_SOURCE` by structural provenance — a module-scope const with a static initializer — not by name alone) plus `scripts/lint-no-adhoc-regex-escape.cjs`, a whole-tree companion covering directories ESLint's globs miss. Requires `engines.node >= 24.0.0`; a seam test asserts `typeof RegExp.escape === 'function'` so the floor and the capability cannot silently diverge. Source of truth: `gsd-core/bin/lib/pattern.cjs` (generated from `src/pattern.cts`). Design: `.gsd/phase/chore-3412-pattern-seam/40-design.md`. + ### Planning Snapshot Module Module owning the parsed projection of `.planning/` that a diagnostic rule may read, per ADR-3180 §8.1 (Decision 8, Phase 10, #3308). `buildPlanningSnapshot(cwd) → PlanningSnapshot` is composed EXCLUSIVELY from the already-consolidated §7 owners — `getMilestoneInfo` (Roadmap Parser Module), `listMilestonePhaseDirs` (Phase Locator Module), `isPhaseComplete` (Verification Module), `scanPhasePlans` (Plan Scan Module), `stateFieldValue`/`stateCurrentPositionSlice` (STATE.md Document Module), `planningPaths` (Planning Workspace Module) — and introduces no new semantic derivation of its own. `PlanningSnapshot` exposes `milestone`/`phaseDirs`/`phases`/`currentPhaseLabel`, each a `{value, scope}` pair per the Planning Scope Module's frozen `SCOPE` enum; `phases` additionally carries a `PhaseSnapshot[]` (`dir`, `complete`, `verificationStatus`, `planCount`, `summaryCount`, `scope`). The one new piece of logic this module adds is `worstScope(...scopes) → Scope`, a pure severity-ordered combinator (`UNREADABLE` > `UNSCOPED` > `TRUNCATED` > `COMPLETE`) that folds several independently-scoped owner answers about the same phase directory into one composite signal — NOT a re-derivation of any owner (each owner's own algorithm is untouched; only their already-computed `scope` verdicts are combined), but new coordination logic no single owner has the visibility to express. Every exposed field carries PARSED values only, never raw document text — this is structural, not advisory: a diagnostic rule given only the parsed value cannot re-derive a field's location the way `#3162`'s three inert `Current Phase` literal-search predicates did. Read failures on STATE.md (exists-but-unreadable, distinct from absent) are reported via the Unusable Input Diagnostic Module's `warnUnusableInput(UNUSABLE_REASON.STATE_UNREADABLE)`. Guarded by `scripts/lint-planning-snapshot-bypass-drift.cjs` (ratcheted per Decision 4(e), scoped to `DIAGNOSTIC_RULE_FUNCTIONS` — currently `cmdValidateHealth` in `src/verify.cts` only, acknowledging its existing raw `.planning/` reads as debt owned by Phase 11, #3309, which migrates it onto this snapshot). Source of truth: `gsd-core/bin/lib/planning-snapshot.cjs` (generated from `src/planning-snapshot.cts`). Design: `.gsd/phase/refactor-3308-planning-snapshot-parsed-projection/40-design.md`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f852debbb..1e62d1287 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -195,7 +195,7 @@ Contributor requirements (summary): - **One concern per PR** — bug fixes, enhancements, and features must be separate PRs - **No drive-by formatting** — don't reformat code unrelated to your change - **Don't bundle test-fixture updates into `docs:` or unrelated commits** — when a production change makes an existing test assertion stale, the test correction MUST land as its own `test:` (or `fix:`) commit, not bundled into a `docs:` commit that also updates the explanation. The release-sdk hotfix cherry-pick filter routes by commit-subject prefix (`fix:`, `chore:`, `test:`); a test-fixture correction packed under a `docs:` prefix is invisible to the picker and ships a half-state to the hotfix branch — production code changed, test assertion stale. v1.42.3 hit this exact mode (#3621). The fix is upstream: keep the test-fixture commit separate. -- **CI must pass** — all configured matrix jobs must be green. Node 22 remains the compatibility floor; Node 24 is the primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available. +- **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available. - **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue ## CHANGELOG Entries — Drop a Fragment @@ -871,17 +871,16 @@ For everything else, if a test reaches for `.includes()` / `.startsWith()` / `as ### Node.js Version Compatibility -**Node 22 is the minimum supported version.** Node 24 is the primary CI target. Node 26 is the forward-compatibility target: do not add tests or production code that depend on deprecated behavior likely to fail there. +**Node 24 is the minimum supported version.** Node 24 is also the primary CI target. Node 26 is the forward-compatibility target: do not add tests or production code that depend on deprecated behavior likely to fail there. | Version | Status | |---------|--------| -| **Node 22** | Minimum required — Active LTS until October 2026, Maintenance LTS until April 2027 | -| **Node 24** | Primary CI target — current Active LTS, all tests must pass | +| **Node 24** | Minimum required and primary CI target — Active LTS, all tests must pass | | Node 26 | Forward-compatible target — avoid deprecated APIs and exact runtime-error prose | Do not use: - Deprecated APIs -- APIs not available in Node 22 +- APIs not available in Node 24 Safe to use: - `node:test` — stable since Node 18, fully featured in 24 diff --git a/bin/install.js b/bin/install.js index a392c169b..7a9bfbb29 100755 --- a/bin/install.js +++ b/bin/install.js @@ -59,6 +59,7 @@ const { composeWorkflow } = require('../gsd-core/bin/lib/workflow-fragments.cjs' // MCP catalog, src/mcp-catalog.cts) — see the comment at its call site below. const { shouldCompose } = require('../gsd-core/bin/lib/mcp-catalog.cjs'); const runtimeArtifactConversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); +const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); // #2544: the CommonJS marker's single source of truth. classifyMarker() backs // BOTH ensureCommonJsMarker() (install) and removeCommonJsMarker() (uninstall), // so the write side can no longer clobber a package.json the remove side would @@ -1970,10 +1971,6 @@ function buildKiloAgentPermissionBlock(claudeTools) { return lines; } -function escapeRegExp(value) { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function replaceRelativePathReference(content, fromPath, toPath) { const escapedPath = escapeRegExp(fromPath); return content.replace( diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index c70d7f328..ff55391dc 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -438,6 +438,7 @@ "onboard-projection.cjs", "package-identity.cjs", "package-legitimacy.cjs", + "pattern.cjs", "phase-command-router.cjs", "phase-estimation.cjs", "phase-id.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 10081190e..981cb8aa9 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -540,6 +540,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `model-resolver.cjs` | Model/effort resolution policy — resolves model, tier, granularity, effort, and fast-mode for an agent from config + model profiles/catalog (extracted from `core.cjs`, ADR-857) | | `package-identity.cjs` | Generated single source for GSD's published-package coordinates (npm name, bin name, repo slug, changelog URL, manual-install command), derived from package.json; read by the update worker, `check-latest-version`, and installer (#498) | | `package-legitimacy.cjs` | Registry-API package legitimacy verdicts (OK/SUS/SLOP) from npm/PyPI/crates, slopcheck optional | +| `pattern.cjs` | The pattern-construction seam — `escapeRegex` (delegates to the built-in `RegExp.escape`) and `literalPattern`; sole owner of building a `RegExp` from a runtime value (ADR-3212 §1, epic #3212 Phase 1, #3412) | | `phase-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phase` | | `phase-estimation.cjs` | Pure phase-effort estimation — `estimate`/`actuals` schema parse+render, smart-zone budget classification, and estimate-vs-actual calibration (median ratio, clamped, sample-gated). Confidence is derived from calibration sample count, never self-rated (ADR-2629) | | `phase-id.cjs` | Pure phase-id parsing/matching helpers — normalize, token match, milestone/phase-dir id parsing, phase-markdown regex builders (extracted from `core.cjs`, ADR-857) | diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index d11fa580f..0eb209629 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -307,33 +307,35 @@ either way — it is never the same as clean. The `Tests` workflow runs every PR through a scoped gate generated by `scripts/ci-test-scope.cjs`. -| Lane | Node 22 | Node 24 | -|---|---|---| -| `ubuntu-latest` | scoped tests | unit + integration + security | -| `windows-latest` | — | scoped Windows/path/shell tests | -| `macos-latest` | full parity when required | full parity when required | +All lanes run on **Node 24** — the `engines.node` floor (`>=24.0.0`) and the +only supported runtime. + +| Lane | Scope | +|---|---| +| `ubuntu-latest` (scoped) | scoped tests — fast PR signal | +| `ubuntu-latest` (full, sharded) | unit + integration + security | +| `windows-latest` | scoped Windows/path/shell tests | +| `macos-latest` | full parity when required | -- **Node 22** is the `engines.node` floor (`>=22.0.0`) — must stay green. -- **Node 24** is the default development lane. - **Scoped tests** are selected from the changed paths, plus a small CLI/package smoke set. They are for confidence on the affected surface, not for counting tests. The default PR gate runs the broad `unit` (under the c8 coverage gate), `integration`, and `security` suites once on Ubuntu / Node 24, scoped tests on -Ubuntu / Node 22, and scoped tests on Windows / Node 24. "Scoped" means the -diff-selected list from the rule table — not the full suite and not a fixed -smoke set (the fixed smoke list is only the empty-selection fallback). The -Windows lane's list is the Windows-sensitive subset of the selection, plus -**every changed test file, unconditionally** (the #494 invariant, narrowed): a -modified test is exercised on the divergent OS before merge at per-file cost, -without paying for the three full parity lanes. +a second Ubuntu / Node 24 lane, and scoped tests on Windows / Node 24. +"Scoped" means the diff-selected list from the rule table — not the full suite +and not a fixed smoke set (the fixed smoke list is only the empty-selection +fallback). The Windows lane's list is the Windows-sensitive subset of the +selection, plus **every changed test file, unconditionally** (the #494 +invariant, narrowed): a modified test is exercised on the divergent OS before +merge at per-file cost, without paying for the three full parity lanes. PRs touching workflow, package, test-runner, install, release, or Windows-sensitive surfaces also run the full parity matrix on macOS and the older Windows runtime, plus `install` and `slow` on the primary Ubuntu lane. Everything (including the full parity matrix) runs on every push to `next`, -which covers the residual macOS / Windows-Node-22 cross-product for scoped PRs. +which covers the residual macOS / Windows cross-product for scoped PRs. Coverage runs inside the Ubuntu / Node 24 full lane (not a separate job — that duplicated the entire unit run) and stays single-lane because multiplying @@ -391,8 +393,8 @@ event stream from a `gsd-test` run: ```bash node scripts/gen-test-timings.cjs \ - ~/.local/state/gsd-test/runs//test-events-linux-node22.jsonl \ - ~/.local/state/gsd-test/runs//test-events-linux-node24.jsonl + ~/.local/state/gsd-test/runs//test-events-linux-node24.jsonl \ + ~/.local/state/gsd-test/runs//test-events-macos-node24.jsonl ``` Pass every lane you have. A file's recorded time is the **max** across the diff --git a/docs/branch-protection.md b/docs/branch-protection.md index 24201721f..11a047db9 100644 --- a/docs/branch-protection.md +++ b/docs/branch-protection.md @@ -74,7 +74,7 @@ branch-protection context is the single aggregate `Required tests` job. Doc-only PRs run the lightweight lint and aggregate jobs only. Code-touching PRs run the default required matrix: -- Ubuntu / Node 22 scoped tests +- Ubuntu / Node 24 scoped tests - Ubuntu / Node 24 unit, integration, and security suites - Windows / Node 24 scoped Windows/path/shell tests - Ubuntu / Node 24 coverage @@ -83,8 +83,7 @@ PRs that touch workflow, package, test-runner, install, release, or Windows sensitive surfaces also run install/slow on the primary Ubuntu lane and the full parity matrix: -- Windows / Node 22 -- macOS / Node 22 +- Windows / Node 24 - macOS / Node 24 ### Canonical code-paths list diff --git a/docs/contributing/bootstrap.md b/docs/contributing/bootstrap.md index b28252cb7..1c2cb29c6 100644 --- a/docs/contributing/bootstrap.md +++ b/docs/contributing/bootstrap.md @@ -83,7 +83,7 @@ This runs `scripts/check-env.cjs` and reports pass/fail for each check: | Check | What it verifies | |---|---| -| `node-version` | Active Node satisfies `engines.node` (`>=22.0.0`) | +| `node-version` | Active Node satisfies `engines.node` (`>=24.0.0`) | | `npm-version` | Active npm satisfies `engines.npm` (`>=10.0.0`) | | `lockfile-present` | `package-lock.json` exists at root | | `lockfile-sync` | `npm ci --dry-run` exits 0 (lockfile matches installed state) | @@ -104,7 +104,7 @@ npm run check:env -- --json ## Troubleshooting -### `node-version` FAIL — Node X does NOT satisfy `>=22.0.0` +### `node-version` FAIL — Node X does NOT satisfy `>=24.0.0` **Cause:** The system Node is too old, or the version manager hasn't activated the correct version. diff --git a/eslint-rules/no-adhoc-regex-escape.cjs b/eslint-rules/no-adhoc-regex-escape.cjs new file mode 100644 index 000000000..a6953d09b --- /dev/null +++ b/eslint-rules/no-adhoc-regex-escape.cjs @@ -0,0 +1,442 @@ +'use strict'; + +/** + * no-adhoc-regex-escape + * + * ADR-3212 (lexical-seam-consolidation) §7 / #3412 Phase 1: + * `src/pattern.cts` is the single seam for regex-metacharacter escaping + * (`escapeRegex` / `literalPattern`, delegating to the built-in + * `RegExp.escape`). This rule flags the two ways a 13th hand-rolled copy of + * that concern can be reintroduced: + * + * 1. ADHOC-REPLACE — an inline `.replace(, '\$&')` call anywhere outside + * `src/pattern.cts`. Matches on the character-class + * MEMBER SET (the 14 regex metacharacters + * `. * + ? ^ $ { } ( ) | [ ] \`), not on exact byte + * order or escaping style — a reordered or + * differently-escaped class with the same member set + * is the same shape and still fires (design doc + * row 25). + * + * 2. UNSAFE-NEW-REGEXP — `new RegExp()` where `` + * is a genuinely dynamic, unescaped runtime value + * (a function parameter, or a `let`/`var` binding, or + * a `const` whose initializer is not a static + * string/template) with no seam routing. + * + * Negative space (design doc "Not-corruption"; test-matrix rows 26-28) — + * deliberately NOT flagged: + * + * - `src/pattern.cts` itself — the seam owns this shape. + * - A regex literal that merely CONTAINS the escape-class characters as + * DATA (e.g. `/^[.*+?]$/`) with no `.replace(..., '\$&')` shape present. + * - `new RegExp()` where `` is a REVIEWED PATTERN + * FRAGMENT. Provenance is accepted under EITHER of two conditions + * (strongest first): + * (a) STRUCTURAL — the identifier resolves (via scope walk) to a + * `const` declaration whose initializer is a string Literal or a + * TemplateLiteral with zero `${...}` expressions. This cannot be + * evaded by renaming the identifier (#3410's guard-evasion class: + * a rule keyed only on an identifier's NAME is defeated by a + * rename; a rule keyed on the const/static-initializer SHAPE is + * not). + * (b) NAMING CONVENTION (fallback) — the identifier ends in `_SOURCE` + * AND resolves (via scope walk) to one of: an ES `import` binding, + * or a `require(...)`-derived module-scope `const` (either + * `const { X_SOURCE } = require('./mod.cjs')` or + * `const X_SOURCE = require('./mod.cjs').X_SOURCE`). Kept for the + * cross-module case where the initializer itself is not in scope + * to inspect (condition (a) cannot see into another file), e.g. an + * imported `PHASE_NUMBER_TOKEN_SOURCE`. (a) is the real check; (b) + * only covers what (a) structurally cannot see — and is bound to + * the identifier's ACTUAL BINDING KIND, not just its spelling: a + * function parameter, a `let`/`var`, a reassigned binding, or an + * unresolvable identifier is NEVER exempted by naming alone, no + * matter what it's called. A naming-only check (no scope/binding + * verification) is exactly #3410's guard-evasion class — a rule + * keyed on spelling is defeated by picking a matching name for a + * genuinely dynamic value (e.g. a function parameter named + * `userInput_SOURCE`). If scope analysis cannot resolve the + * identifier's binding at all, this fails CLOSED (flags it) — + * an unresolvable binding is not evidence of safety. + * A `new RegExp()` argument that already routes through + * the seam (`escapeRegex(...)` / `literalPattern(...)`, by name or as a + * `pattern.escapeRegex(...)` member call) is also treated as safe — that + * IS the fix, not a violation. + * + * Per-finding exemption: add // allow-adhoc-regex-escape: as a + * trailing comment on the same source line, OR as a standalone comment on + * the line immediately preceding the flagged node (mirrors + * no-adhoc-markdown-parsing's `// allow-adhoc-markdown:` convention). + */ + +// The 14 regex metacharacters the census copies' char class escapes. +const ESCAPE_ALL_METACHARS_SET = new Set(['.', '*', '+', '?', '^', '$', '{', '}', '(', ')', '|', '[', ']', '\\']); + +// The self-reference replacement string every copy uses: '\$&' (3 chars: +// backslash, dollar, ampersand). +const SELF_REFERENCE_REPLACEMENT = '\\$&'; + +const SOURCE_SUFFIX_RE = /_SOURCE$/; + +/** + * Parse the first top-level (unescaped) `[...]` character class out of a + * regex source string and return its member-character set, or `null` if no + * class is present / it is unterminated. Single-pass, escape-aware — mirrors + * the style of no-adhoc-markdown-parsing's hasQualifyingNegatedPipeClass + * (walk once, honor `\` escapes, never backtrack into the class body). + */ +function parseCharClassMembers(src) { + let i = 0; + while (i < src.length) { + const ch = src[i]; + if (ch === '\\') { + i += 2; + continue; + } + if (ch === '[') break; + i += 1; + } + if (i >= src.length) return null; + + let j = i + 1; + let negated = false; + if (src[j] === '^') { + negated = true; + j += 1; + } + + const members = new Set(); + while (j < src.length) { + const c = src[j]; + if (c === '\\') { + const next = src[j + 1]; + if (next === undefined) return null; // unterminated escape + members.add(next); + j += 2; + continue; + } + if (c === ']') { + return { members, negated }; + } + members.add(c); + j += 1; + } + return null; // unterminated class +} + +/** + * Does `src` (a regex literal's `.pattern`) contain, as its first char + * class, EXACTLY the escape-all-metachars member set — regardless of + * ordering or which members are backslash-escaped (row 25: shape, not + * bytes)? + */ +function isEscapeAllMetacharsClassSource(src) { + const parsed = parseCharClassMembers(src || ''); + if (!parsed || parsed.negated) return false; + if (parsed.members.size !== ESCAPE_ALL_METACHARS_SET.size) return false; + for (const m of ESCAPE_ALL_METACHARS_SET) { + if (!parsed.members.has(m)) return false; + } + return true; +} + +/** Is `node` the regex literal `.replace()` first-argument shape? */ +function isEscapeAllMetacharsRegexLiteral(node) { + return node.type === 'Literal' && !!node.regex && isEscapeAllMetacharsClassSource(node.regex.pattern); +} + +/** Is `node` the string literal `'\$&'` self-reference replacement? */ +function isSelfReferenceReplacement(node) { + return node.type === 'Literal' && typeof node.value === 'string' && node.value === SELF_REFERENCE_REPLACEMENT; +} + +/** + * Walk up the scope chain from `scope` looking for `identifierName`'s + * declaration. Returns `{ def, kind }` where `kind` is 'param' for a + * function-parameter binding, 'import' for an ES `import` binding, or the + * VariableDeclaration `kind` ('const'/'let'/'var') for a variable binding — + * or `null` if unresolved in any enclosing scope reachable from here (fails + * closed: callers must treat `null` as "not provably safe"). + */ +function resolveBinding(identifierName, scope) { + let s = scope; + while (s) { + const variable = s.variables.find((v) => v.name === identifierName); + if (variable) { + const def = variable.defs && variable.defs[0]; + if (!def) return null; + if (def.type === 'Parameter') return { def, kind: 'param' }; + if (def.type === 'ImportBinding') return { def, kind: 'import' }; + if ( + def.node + && def.node.type === 'VariableDeclarator' + && def.parent + && def.parent.type === 'VariableDeclaration' + ) { + return { def, kind: def.parent.kind }; + } + return null; + } + s = s.upper; + } + return null; +} + +/** Is `node` a `require(...)` call expression? */ +function isRequireCallExpression(node) { + return !!node && node.type === 'CallExpression' + && node.callee && node.callee.type === 'Identifier' && node.callee.name === 'require'; +} + +/** + * Provenance condition (b), require branch — does `identifierName` resolve + * to a module-scope `const` whose initializer is EITHER `require(...)` + * itself (the `const { X_SOURCE } = require('./mod.cjs')` destructure shape) + * OR a non-computed member access on a `require(...)` call (the + * `const X_SOURCE = require('./mod.cjs').X_SOURCE` shape)? + */ +function resolvesToRequireDerivedConstBinding(identifierName, scope) { + const binding = resolveBinding(identifierName, scope); + if (!binding || binding.kind !== 'const') return false; + const init = binding.def.node.init; + if (!init) return false; + if (isRequireCallExpression(init)) return true; + if (init.type === 'MemberExpression' && !init.computed && isRequireCallExpression(init.object)) return true; + return false; +} + +/** Provenance condition (b), import branch — does `identifierName` resolve to an ES `import` binding? */ +function resolvesToImportBinding(identifierName, scope) { + const binding = resolveBinding(identifierName, scope); + return !!binding && binding.kind === 'import'; +} + +/** + * Provenance condition (a) — STRUCTURAL: does `identifierName` resolve to a + * `const` whose initializer is a static string Literal or a TemplateLiteral + * with no `${...}` expressions? + */ +function resolvesToStaticConstFragment(identifierName, scope) { + const binding = resolveBinding(identifierName, scope); + if (!binding || binding.kind !== 'const') return false; + const init = binding.def.node.init; + if (!init) return false; + if (init.type === 'Literal' && typeof init.value === 'string') return true; + if (init.type === 'TemplateLiteral' && (!init.expressions || init.expressions.length === 0)) return true; + return false; +} + +/** + * UNSAFE-NEW-REGEXP is deliberately narrow. An earlier version of this rule + * flagged ANY `new RegExp()` whose identifier resolved to a + * function parameter, a `let`/`var`, or a `const` with a non-static + * initializer — e.g. `const src = someHelper(x); new RegExp(src);`. A first + * whole-repo run proved that heuristic unusable: it fired ~25 times across + * `tests/**` on code with no relation to the escape-helper concern — + * `for (const level of [...]) { ...new RegExp(level)... }` (a fixed literal + * array), `patterns.filter(p => new RegExp(p).test(...))` (`p` is itself an + * already-regex-shaped source string, not literal text needing escaping), + * `const src = phaseId.phaseMarkdownRegexSource(x); new RegExp(src)` (a + * value produced by a function that ALREADY routes through escapeRegex + * internally, just not through a literal `new RegExp(escapeRegex(...))` + * one-liner). A syntax-only rule cannot distinguish "raw literal text that + * needed escaping and didn't get it" from "an already-valid regex-source + * string produced elsewhere" for an arbitrary non-literal expression — so it + * does not try to for the general case. + * + * What IS a reliable, false-positive-free signal is the row-29 shape + * itself: a function whose ENTIRE body is `return new RegExp()` (or an arrow function's implicit-return equivalent) — i.e. a + * hand-rolled re-implementation of `literalPattern()` that skips the + * escaping step. That shape cannot arise from "value already computed + * upstream and merely re-used" (there is no upstream — the parameter IS the + * function's only input), so it is exactly ADR §7's "new RegExp() built + * from a non-literal without routing through the seam" with no other + * plausible reading. + */ +function isSoleReturnOfOwnParameter(newExprNode, argIdentifier) { + let fn = null; + + const parent = newExprNode.parent; + if (!parent) return false; + + if (parent.type === 'ReturnStatement' && parent.argument === newExprNode) { + const block = parent.parent; + if ( + block + && block.type === 'BlockStatement' + && block.body.length === 1 + && block.parent + && (block.parent.type === 'FunctionDeclaration' + || block.parent.type === 'FunctionExpression' + || block.parent.type === 'ArrowFunctionExpression') + && block.parent.body === block + ) { + fn = block.parent; + } + } + else if (parent.type === 'ArrowFunctionExpression' && parent.body === newExprNode) { + fn = parent; + } + + if (!fn) return false; + return (fn.params || []).some((p) => p.type === 'Identifier' && p.name === argIdentifier.name); +} + +function isReviewedPatternFragmentIdentifier(identifierName, scope) { + if (resolvesToStaticConstFragment(identifierName, scope)) return true; // (a) structural + if (SOURCE_SUFFIX_RE.test(identifierName)) { + // (b) naming fallback — only trusted when the identifier's ACTUAL + // BINDING is an import or a require()-derived module-scope const. + // A function parameter, a let/var, a reassigned binding, or an + // unresolvable identifier is never exempted by spelling alone (#3410 + // guard-evasion class) — fails closed via resolveBinding returning null. + if (resolvesToImportBinding(identifierName, scope)) return true; + if (resolvesToRequireDerivedConstBinding(identifierName, scope)) return true; + return false; + } + return false; +} + +/** Does `node` route through the seam (`escapeRegex(...)` / `literalPattern(...)`)? */ +function isSeamRoutedCall(node) { + if (node.type !== 'CallExpression') return false; + const callee = node.callee; + if (!callee) return false; + if (callee.type === 'Identifier') { + return callee.name === 'escapeRegex' || callee.name === 'literalPattern'; + } + if (callee.type === 'MemberExpression' && !callee.computed && callee.property) { + return callee.property.name === 'escapeRegex' || callee.property.name === 'literalPattern'; + } + return false; +} + +/** @type {import('eslint').Rule.RuleModule} */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow a hand-rolled regex-metacharacter-escape helper or an unrouted new RegExp() from a runtime value outside src/pattern.cts — import escapeRegex()/literalPattern() from the seam instead.', + category: 'Best Practices', + }, + schema: [], + messages: { + adhocRegexEscape: + 'Ad-hoc regex-metacharacter-escape detected (.replace() against the escape-all-metachars character class, with the \'\\$&\' self-reference replacement). Import escapeRegex() from ./pattern (src/pattern.cts) instead. Suppress with: // allow-adhoc-regex-escape: ', + unsafeNewRegExp: + 'new RegExp() built from a runtime value with no escaping and no seam routing. Route it through escapeRegex()/literalPattern() from ./pattern (src/pattern.cts) first. Suppress with: // allow-adhoc-regex-escape: ', + }, + }, + + create(context) { + const filename = context.getFilename ? context.getFilename() : context.filename; + const normalizedFilename = filename.replace(/\\/g, '/'); + // The seam owns this shape — it is not a violation of itself. + if (/(?:^|\/)src\/pattern\.cts$/.test(normalizedFilename)) { + return {}; + } + + const sourceCode = context.getSourceCode ? context.getSourceCode() : context.sourceCode; + + function isAllowed(node) { + const nodeStartLine = node.loc.start.line; + const allComments = sourceCode.getAllComments(); + return allComments.some((c) => { + if (!/allow-adhoc-regex-escape:\s*\S/.test(c.value)) return false; + return c.loc.start.line === nodeStartLine || c.loc.start.line === nodeStartLine - 1; + }); + } + + return { + // ── ADHOC-REPLACE: .replace(, '\$&') ────────────── + CallExpression(node) { + const callee = node.callee; + if (callee && callee.type === 'MemberExpression' && !callee.computed + && callee.property && callee.property.name === 'replace') { + const args = node.arguments || []; + const patternArg = args[0]; + const replacementArg = args[1]; + if ( + patternArg + && replacementArg + && isEscapeAllMetacharsRegexLiteral(patternArg) + && isSelfReferenceReplacement(replacementArg) + ) { + if (!isAllowed(node)) { + context.report({ node, messageId: 'adhocRegexEscape' }); + } + } + return; + } + + // ── UNSAFE-NEW-REGEXP is checked on NewExpression below; nothing + // else to do for other CallExpressions here. + }, + + // ── UNSAFE-NEW-REGEXP: new RegExp() ────── + NewExpression(node) { + if (!node.callee || node.callee.type !== 'Identifier' || node.callee.name !== 'RegExp') return; + const arg = node.arguments && node.arguments[0]; + if (!arg) return; + + // Static text (string literal / no-interpolation template literal) — + // fully known at lint time, not a runtime value. Never flagged. + if (arg.type === 'Literal' && typeof arg.value === 'string') return; + if (arg.type === 'TemplateLiteral' && (!arg.expressions || arg.expressions.length === 0)) return; + + // Already routed through the seam — that IS the fix. + if (isSeamRoutedCall(arg)) return; + + if (arg.type === 'Identifier') { + const scope = context.getScope ? context.getScope() : sourceCode.getScope(node); + if (isReviewedPatternFragmentIdentifier(arg.name, scope)) { + return; + } + + // The `_SOURCE` suffix exists ONLY to claim provenance exemption + // (b). An identifier that carries the suffix but did NOT resolve + // to a legitimate import/require-derived binding above (or is + // unresolvable at all) IS the #3410-class evasion this check + // exists to close — flag it unconditionally, independent of the + // narrower sole-return-of-parameter shape below. This cannot + // reproduce the ~25 unrelated false positives an earlier, broader + // "any non-literal identifier" heuristic produced (see + // isSoleReturnOfOwnParameter's doc comment): those identifiers + // never carried the `_SOURCE` naming convention in the first + // place, so this branch never reaches them. + if (SOURCE_SUFFIX_RE.test(arg.name)) { + if (!isAllowed(node)) { + context.report({ node, messageId: 'unsafeNewRegExp' }); + } + return; + } + + // Narrow, false-positive-free shape only — see + // isSoleReturnOfOwnParameter's doc comment for why the broader + // "any non-literal identifier" heuristic was rejected. + if (isSoleReturnOfOwnParameter(node, arg)) { + if (!isAllowed(node)) { + context.report({ node, messageId: 'unsafeNewRegExp' }); + } + } + } + }, + }; + }, +}; + +// Exposed for reuse by scripts/lint-no-adhoc-regex-escape.cjs (the whole-tree +// parity backstop for directories/extensions this rule's eslint.config.mjs +// wiring does not reach, e.g. tests/fixtures/**/*.cts). ESLint itself only +// reads `.create`/`.meta` off a required rule module, so these extra +// properties are inert to the linter and exist purely as a single source of +// truth for the shape-detection logic (avoids a second, drifting copy of the +// character-class parser — the exact class of bug this rule exists to end). +rule.isEscapeAllMetacharsClassSource = isEscapeAllMetacharsClassSource; +rule.SELF_REFERENCE_REPLACEMENT = SELF_REFERENCE_REPLACEMENT; + +module.exports = rule; diff --git a/eslint.config.mjs b/eslint.config.mjs index 2ecb346d3..11bb01aa8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -15,6 +15,7 @@ import noElapsedAssertion from './eslint-rules/no-elapsed-assertion.cjs'; import noRawRmsyncInTests from './eslint-rules/no-raw-rmsync-in-tests.cjs'; import noTautologicalAssert from './eslint-rules/no-tautological-assert.cjs'; import noAdhocMarkdownParsing from './eslint-rules/no-adhoc-markdown-parsing.cjs'; +import noAdhocRegexEscape from './eslint-rules/no-adhoc-regex-escape.cjs'; import noPathLiteralInAssert from './eslint-rules/no-path-literal-in-assert.cjs'; import noPosixModeBitAssert from './eslint-rules/no-posix-mode-bit-assert.cjs'; import noUnguardedNonportableExec from './eslint-rules/no-unguarded-nonportable-exec.cjs'; @@ -36,6 +37,7 @@ const localPlugin = { 'no-raw-rmsync-in-tests': noRawRmsyncInTests, 'no-tautological-assert': noTautologicalAssert, 'no-adhoc-markdown-parsing': noAdhocMarkdownParsing, + 'no-adhoc-regex-escape': noAdhocRegexEscape, 'no-path-literal-in-assert': noPathLiteralInAssert, 'no-posix-mode-bit-assert': noPosixModeBitAssert, 'no-unguarded-nonportable-exec': noUnguardedNonportableExec, @@ -135,6 +137,7 @@ export default tseslint.config( 'gsd-core/bin/lib/configuration.cjs', 'gsd-core/bin/lib/state-document.cjs', 'gsd-core/bin/lib/planning-snapshot.cjs', + 'gsd-core/bin/lib/pattern.cjs', 'gsd-core/bin/lib/health-diagnostic-types.cjs', 'gsd-core/bin/lib/health-diagnostic.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/root-existence.cjs', @@ -306,6 +309,11 @@ export default tseslint.config( // ADR-1372 T7: enforce use of the markdown-sectionizer seam; grandfather // pre-migration sites with // allow-adhoc-markdown: 'local/no-adhoc-markdown-parsing': 'error', + // ADR-3212 Phase 1 (#3412): enforce the pattern-construction seam + // (src/pattern.cts's escapeRegex/literalPattern) — flags a re-inlined + // escape-all-metachars .replace() helper or an unrouted new RegExp() + // from a runtime value. + 'local/no-adhoc-regex-escape': 'error', // ADR-1703 Phase 5: flag path-returning calls interpolated into content // (markdown @-references, workflow files, generated docs) without POSIX // normalization. Promoted to 'error' after precision review (path.basename @@ -347,6 +355,10 @@ export default tseslint.config( rules: { 'local/normalize-path-in-content': 'error', 'local/require-fs-op-fallback': 'error', + // ADR-3212 Phase 1 (#3412): pattern-construction seam prohibition — + // scripts/build-hooks.js is a .js file, so it falls outside the + // scripts/**/*.cjs glob below and needs it registered here too. + 'local/no-adhoc-regex-escape': 'error', }, }, @@ -398,6 +410,9 @@ export default tseslint.config( // Promoted to error (#3313) — a fresh non-cached `npx eslint .` run found // zero live violations of this rule in this glob at promotion time. 'local/no-source-grep': 'error', + // ADR-3212 Phase 1 (#3412): pattern-construction seam prohibition — + // see the src/**/*.cts block above for detail. + 'local/no-adhoc-regex-escape': 'error', }, }, @@ -414,6 +429,8 @@ export default tseslint.config( 'no-empty': ['warn', { allowEmptyCatch: true }], 'no-useless-escape': 'warn', 'n/no-path-concat': 'error', + // ADR-3212 Phase 1 (#3412): pattern-construction seam prohibition. + 'local/no-adhoc-regex-escape': 'error', // n/no-process-exit is deliberately OFF for hooks ONLY. // // A hook is a standalone process whose ENTIRE contract is its exit code: the @@ -493,6 +510,12 @@ export default tseslint.config( // Ban a consolidation-epic folded suite appearing twice in one host file (#3271). // A second copy runs the same tests twice on every lane and drifts silently. 'local/no-duplicate-fold-marker': 'error', + // ADR-3212 Phase 1 (#3412): pattern-construction seam prohibition — + // see the src/**/*.cts block above for detail. The historical oracle + // inlined in tests/pattern.test.cjs is exempted per-finding with + // // allow-adhoc-regex-escape: comments (design doc Notes: "not a 13th + // production copy"). + 'local/no-adhoc-regex-escape': 'error', // Ban raw setTimeout sync + elapsed/duration-style assertions via no-restricted-syntax 'no-restricted-syntax': [ 'error', diff --git a/package-lock.json b/package-lock.json index 47b165dcc..e1400b351 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,6 +26,7 @@ "eslint": "^9.39.4", "eslint-plugin-n": "^17.24.0", "eslint-plugin-no-only-tests": "^3.4.0", + "espree": "^10.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", "js-yaml": "^4.3.1", @@ -33,7 +34,7 @@ "typescript-eslint": "^8.60.0" }, "engines": { - "node": ">=22.0.0", + "node": ">=24.0.0", "npm": ">=10.0.0" }, "optionalDependencies": { diff --git a/package.json b/package.json index 2b921edf2..42fe58be2 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "!scripts/run-tests.cjs", "!scripts/affected-tests-lib.cjs", "!scripts/run-affected-tests.cjs", + "!scripts/lint-no-adhoc-regex-escape.cjs", "pi", "vscode" ], @@ -54,7 +55,7 @@ "access": "public" }, "engines": { - "node": ">=22.0.0", + "node": ">=24.0.0", "npm": ">=10.0.0" }, "dependencies": { @@ -69,6 +70,7 @@ "eslint": "^9.39.4", "eslint-plugin-n": "^17.24.0", "eslint-plugin-no-only-tests": "^3.4.0", + "espree": "^10.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", "js-yaml": "^4.3.1", @@ -114,7 +116,7 @@ "lint:table-schema-drift": "node scripts/lint-table-schema-drift.cjs", "lint:frontmatter-scalar-broad-grep": "node scripts/lint-frontmatter-scalar-broad-grep.cjs", "lint:removed-but-needed": "node scripts/lint-removed-but-needed.cjs", - "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-planning-artifact-writer-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs", + "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-planning-artifact-writer-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs && node scripts/lint-no-adhoc-regex-escape.cjs", "lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs", "lint:regression-names": "node scripts/lint-regression-test-names.cjs", "lint:descriptions": "node scripts/lint-descriptions.cjs", diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index 48823c128..da1e130ac 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -474,8 +474,8 @@ function classify(files) { // the scoped windows lane instead of triggering the three full parity // lanes. (full_matrix fired on 15/15 sampled PRs because test-driven // PRs always touch tests/, costing ~25 runner-minutes each.) Changed - // tests already run on ubuntu-22 and ubuntu-24 via targeted_tests; the - // residual macOS / windows-node-22 cross-product is covered by the full + // tests already run on the two ubuntu-24 lanes via targeted_tests; the + // residual macOS / windows cross-product is covered by the full // matrix on every push to next. windows.add(file); } diff --git a/scripts/gen-adr-index.cjs b/scripts/gen-adr-index.cjs index 7947f8399..3429c36f0 100644 --- a/scripts/gen-adr-index.cjs +++ b/scripts/gen-adr-index.cjs @@ -30,6 +30,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); const ROOT = path.resolve(__dirname, '..'); const ADR_DIR = path.join(ROOT, 'docs', 'adr'); @@ -95,19 +96,12 @@ const REASON = Object.freeze({ * added to `STATUSES` now covers the bracket for free, and the corpus's * parity test iterates the real exported array rather than a copy. */ -/** - * Escape a string for literal inclusion inside a dynamic RegExp alternation. - * Defence-in-depth, not a live-bug fix: `STATUSES` is a static array literal - * today, so nothing in it can currently carry a regex metacharacter. But - * nothing enforces that it STAYS static — if a future change ever derives it - * from external input (a config file, a corpus scan), an unescaped `join('|')` - * would let a status token break out of the alternation it is meant to be one - * branch of. - */ -function escapeRegExp(s) { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - +// Escaped for defence-in-depth, not a live-bug fix: `STATUSES` is a static +// array literal today, so nothing in it can currently carry a regex +// metacharacter. But nothing enforces that it STAYS static — if a future +// change ever derives it from external input (a config file, a corpus scan), +// an unescaped `join('|')` would let a status token break out of the +// alternation it is meant to be one branch of. const STATUS_BRACKET_RE = new RegExp(String.raw`\s*\[(${STATUSES.map(escapeRegExp).join('|')})\]\s*$`, 'i'); /** diff --git a/scripts/gen-loop-host-contract.cjs b/scripts/gen-loop-host-contract.cjs index 9d7bed19c..012d1ad72 100644 --- a/scripts/gen-loop-host-contract.cjs +++ b/scripts/gen-loop-host-contract.cjs @@ -20,6 +20,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); const ROOT = path.resolve(__dirname, '..'); const WORKFLOWS_DIR = path.join(ROOT, 'gsd-core', 'workflows'); @@ -191,13 +192,6 @@ function parseLoopHostBlock(content, fileName) { * @param {string} fileName For error messages * @returns {string[]} Array of error strings; empty = OK */ -/** - * Escape a string for literal use in a RegExp. - */ -function escapeRegExp(s) { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function crossCheckRoles(content, agentRoles, fileName) { const errors = []; for (const role of agentRoles) { diff --git a/scripts/lint-completion-predicate-drift.cjs b/scripts/lint-completion-predicate-drift.cjs index e37512427..b9fac4bf7 100644 --- a/scripts/lint-completion-predicate-drift.cjs +++ b/scripts/lint-completion-predicate-drift.cjs @@ -159,6 +159,7 @@ const path = require('node:path'); const driftScan = require('./lib/drift-scan.cjs'); const { MAX_REGEX_LITERAL_LEN, sanitizeForReport, scanTree } = driftScan; +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); // ─── SHARED TOKENIZER + FUNCTION ATTRIBUTION (mirrors lint-state-field-drift.cjs) ── // @@ -696,7 +697,7 @@ function findScanPhasePlansCompletedReadDrift(text, relPath, exemptFunctions) { for (const [fnKey, varNames] of varsByFn) { if (fnKey !== FN_KEY_MODULE && exemptFunctions && exemptFunctions.has(fnKey)) continue; for (const varName of varNames) { - const escaped = varName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escaped = escapeRegex(varName); const readRe = new RegExp(`\\b${escaped}\\.completed\\b`); for (let i = 0; i < lines.length; i++) { const fnHere = innermostAt[i] || FN_KEY_MODULE; diff --git a/scripts/lint-no-adhoc-regex-escape.cjs b/scripts/lint-no-adhoc-regex-escape.cjs new file mode 100644 index 000000000..76dfc8803 --- /dev/null +++ b/scripts/lint-no-adhoc-regex-escape.cjs @@ -0,0 +1,236 @@ +#!/usr/bin/env node +'use strict'; + +/** + * lint-no-adhoc-regex-escape.cjs — the whole-tree parity backstop for + * `local/no-adhoc-regex-escape` (ADR-3212 §7 / #3412 Phase 1; test-matrix + * row 30: "a 13th copy of the escape body anywhere in the tree" must fail + * CI, not review). + * + * ## Why this exists alongside the ESLint rule + * + * `eslint.config.mjs` covers `src/**\/*.cts`, `scripts/**\/*.cjs`, + * `tests/**\/*.cjs`, `hooks/**\/*.js`+`*.cjs`, and a short explicit list of + * top-level `.js` files — but NOT every extension in every directory. A + * whole-tree file-inventory check found `tests/fixtures/brand-typing/*.cts` + * (6 files): `.cts` under `tests/` matches neither the `src/**\/*.cts` glob + * (wrong directory) nor the `tests/**\/*.cjs` glob (wrong extension), so the + * AST rule never sees them. A hand-rolled escape helper planted in a + * directory/extension combination outside every `eslint.config.mjs` glob + * would pass `npx eslint .` cleanly. This script closes that gap with a + * plain-text, whole-repo-tree scan (mirrors scripts/lint-removed-but-needed.cjs's + * SCAN_ROOTS + walk() + ExitError structure) instead of another eslint.config.mjs + * glob entry, so growing the covered surface never depends on remembering to + * add a new files: [...] block for the next unanticipated extension. + * + * ## What this checks + * + * Walks the ENTIRE repository tree (skipping node_modules/.git/dist/coverage/ + * .worktrees/.claude, mirroring eslint.config.mjs's global ignores), and for + * every source-like file (.cjs/.js/.mjs/.cts/.ts), text-scans for a + * `.replace(, )` call whose regex + * literal's character class matches the escape-all-metachars member set + * (reusing `eslint-rules/no-adhoc-regex-escape.cjs`'s + * `isEscapeAllMetacharsClassSource` — single source of truth for the shape, + * per CLAUDE.md's Generative Fix Divergence guidance) and whose replacement + * string is `'\$&'`. `src/pattern.cts` (the seam) and any line carrying a + * trailing/preceding `// allow-adhoc-regex-escape: ` comment are + * exempt, mirroring the ESLint rule's own suppression convention. + * + * A plain regex/text scan (not a full parser) is a deliberate, disclosed + * trade-off: it only needs to close the residual gap outside the AST rule's + * glob coverage, not replace it — the AST rule remains the primary, + * precision enforcement for everything it does cover. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { isEscapeAllMetacharsClassSource, SELF_REFERENCE_REPLACEMENT } = require('../eslint-rules/no-adhoc-regex-escape.cjs'); + +const ROOT = path.join(__dirname, '..'); + +const SKIP_DIRS = new Set(['node_modules', '.git', 'dist', 'coverage', '.worktrees', '.claude']); +const SOURCE_EXT = new Set(['.cjs', '.js', '.mjs', '.cts', '.ts']); + +// The seam owns this shape — not a violation of itself. +const SEAM_FILE_RE = /(?:^|\/)src\/pattern\.cts$/; + +// NOTE: bin/install.js was considered for a blanket "generated bundle, +// downstream mirror" exemption (the same reasoning that excuses +// gsd-core/bin/lib/**/*.cjs below) but REJECTED after inspection — +// bin/install.js:1974 carries a genuine, hand-written `function +// escapeRegExp(value) { return value.replace(...); }` of its own (real +// executable code, not an embedded string copy of another file's content). +// It is therefore a real 13th-plus copy and deliberately left un-exempted; +// see this script's own findings for it. +const GENERATED_BUNDLE_FILES = new Set(); + +// The RuleTester fixture corpus for the ESLint rule itself. Row 24/25's +// `code:` fixtures deliberately contain the flagged shape as STRING DATA +// (inside String.raw / template literals) to prove the rule fires — this is +// not runnable production code, and the AST-based ESLint rule correctly does +// not fire on it either (it never becomes a real `.replace()` CallExpression +// in this file's own AST). This text-only scanner cannot make that +// distinction, so the file is exempted explicitly. Per the dispatch brief, +// this test file is a fixed contract and is not edited to route around this. +const RULE_FIXTURE_FILES = new Set(['tests/eslint-no-adhoc-regex-escape.test.cjs']); + +// Two suppression conventions are honored, matching what actually appears in +// this repo: the rule's own per-finding `// allow-adhoc-regex-escape: ` +// marker (mirrors no-adhoc-markdown-parsing's convention), and a native +// `// eslint-disable-next-line local/no-adhoc-regex-escape -- ` +// directive (what tests/pattern.test.cjs's historical-oracle exemptions use — +// real ESLint already honors this natively; this text-only scanner has to +// recognize it explicitly since it does not run through ESLint's engine). +const ALLOW_COMMENT_RE = /allow-adhoc-regex-escape:\s*\S/; +const ESLINT_DISABLE_RE = /eslint-disable(?:-next-line|-line)?\b[^\n]*\bno-adhoc-regex-escape\b/; + +function isSuppressedByComment(line) { + return ALLOW_COMMENT_RE.test(line) || ESLINT_DISABLE_RE.test(line); +} + +/** + * Is `rel` (repo-relative, POSIX-slash) a tsc-generated `.cjs` mirror of a + * `src/**\/*.cts` source (ADR-457)? Dynamic (checks the filesystem for the + * sibling `.cts`) rather than a hardcoded list, so it tracks whichever `.cts` + * modules exist without needing its own upkeep as the seam migration lands. + * @param {string} rel + * @param {string} root + */ +function isGeneratedLibMirror(rel, root) { + if (!rel.startsWith('gsd-core/bin/lib/') || !rel.endsWith('.cjs')) return false; + const candidateSrc = `src/${rel.slice('gsd-core/bin/lib/'.length, -'.cjs'.length)}.cts`; + return fs.existsSync(path.join(root, candidateSrc)); +} + +// A permissive, single-pass extraction of `.replace(, +// )` call text. Deliberately simple (no nested-nesting / +// multi-line-argument support) — this is a coverage backstop for the AST +// rule, not a replacement for it; every real census copy is a single-line +// `.replace(/[...]/g, '\$&')` call. +// +// ReDoS fix (#3412, CodeQL js/redos): the original outer alternation let a +// `[...]` run be consumed EITHER by the character-class branch OR one +// character at a time by the trailing catch-all branch, so on a failing +// match the engine explored both parses of every bracket pair — exponential +// (measured: n=26 -> 204ms, n=28 -> 791ms, n=30 -> 3475ms, ~2^n). Two +// independent fixes, both required, neither alone sufficient long-term: +// (a) the trailing branch is now `[^/\\\n[\]]` — it excludes `[` and `]`, +// so a bracket can only ever be consumed by the character-class +// branch. This removes the ambiguity and makes matching linear. +// Consequence: a regex literal containing a BARE unescaped `]` +// outside a character class is no longer matched by this backstop — +// acceptable, since this is a coverage backstop for the AST rule, not +// a replacement for it, and no census shape has that form. +// (b) every `*` is now a bounded quantifier (ADR-3212's locked +// bounded-quantifiers decision) — a second line of defense that caps +// worst-case work even if the grammar above is later loosened. +const REPLACE_CALL_RE = /\.replace\(\s{0,20}\/((?:\\.|\[(?:\\.|[^\]\\]){0,200}\]|[^/\\\n[\]]){1,400})\/([a-z]{0,10})\s{0,20},\s{0,20}(['"`])((?:\\.|(?!\3)[^\\]){0,400})\3\s{0,20}\)/g; + +function unescapeSimpleStringLiteral(raw) { + // Handles the two-char escapes this specific replacement string ever uses + // (\\ and \$); good enough for the '\$&' shape this script looks for. + return raw.replace(/\\(.)/g, '$1'); +} + +function walk(dir) { + const out = []; + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } + catch { + return out; + } + for (const entry of entries) { + if (SKIP_DIRS.has(entry.name)) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...walk(full)); + else if (entry.isFile() && SOURCE_EXT.has(path.extname(entry.name))) out.push(full); + } + return out; +} + +/** + * Pure: does `content` (the text of a single file) contain the + * escape-all-metachars `.replace(, '\$&')` shape on a line that is + * NOT exempted by an `allow-adhoc-regex-escape:` comment? + * @param {string} content + * @returns {{ line: number }[]} + */ +function findViolations(content) { + const violations = []; + const lines = content.split('\n'); + let match; + REPLACE_CALL_RE.lastIndex = 0; + while ((match = REPLACE_CALL_RE.exec(content)) !== null) { + const [, classSource, , , replacementRaw] = match; + if (!isEscapeAllMetacharsClassSource(`[${classSource}]`)) continue; + if (unescapeSimpleStringLiteral(replacementRaw) !== SELF_REFERENCE_REPLACEMENT) continue; + + const upToMatch = content.slice(0, match.index); + const line = upToMatch.split('\n').length; + const sameLine = lines[line - 1] || ''; + const prevLine = lines[line - 2] || ''; + if (isSuppressedByComment(sameLine) || isSuppressedByComment(prevLine)) continue; + + violations.push({ line }); + } + return violations; +} + +/** + * Pure: scan the whole tree rooted at `root` and return every violation. + * @param {string} root + * @returns {{ file: string, line: number }[]} + */ +function scan(root) { + const violations = []; + for (const abs of walk(root)) { + const rel = path.relative(root, abs).replace(/\\/g, '/'); + if (SEAM_FILE_RE.test(rel)) continue; + if (GENERATED_BUNDLE_FILES.has(rel)) continue; + if (RULE_FIXTURE_FILES.has(rel)) continue; + if (isGeneratedLibMirror(rel, root)) continue; + let content; + try { + content = fs.readFileSync(abs, 'utf8'); + } + catch { + continue; // unreadable (broken symlink, binary) — skip + } + for (const v of findViolations(content)) { + violations.push({ file: rel, line: v.line }); + } + } + return violations; +} + +function main() { + const violations = scan(ROOT); + if (violations.length > 0) { + const detail = violations.map((v) => ` ${v.file}:${v.line}`).join('\n'); + throw new ExitError( + 1, + 'lint-no-adhoc-regex-escape: a hand-rolled regex-metacharacter-escape\n' + + '.replace(, \'\\$&\') copy was found outside\n' + + 'src/pattern.cts (ADR-3212 §7 / #3412, test-matrix row 30 — parity\n' + + 'assertion). Import escapeRegex() from src/pattern.cts instead, or\n' + + 'suppress a genuine non-production exception with a trailing\n' + + '// allow-adhoc-regex-escape: comment:\n' + + detail, + ); + } + console.log('ok lint-no-adhoc-regex-escape: no hand-rolled escape-metachars copy found outside src/pattern.cts'); +} + +module.exports = { + findViolations, + scan, + walk, + SOURCE_EXT, + SKIP_DIRS, +}; + +if (require.main === module) runMain(main); diff --git a/scripts/lint-removed-but-needed.cjs b/scripts/lint-removed-but-needed.cjs index 1bd165426..4c1c20e07 100644 --- a/scripts/lint-removed-but-needed.cjs +++ b/scripts/lint-removed-but-needed.cjs @@ -33,6 +33,7 @@ const fs = require('node:fs'); const path = require('node:path'); const cp = require('node:child_process'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const ROOT = path.join(__dirname, '..'); const SCAN_ROOTS = ['.github/workflows', 'gsd-core', 'docs']; @@ -42,10 +43,6 @@ const EXTRA_FILES = ['package.json']; // never carry a meaningful basename reference, and is often large. const SKIP_EXT = new Set(['.png', '.jpg', '.jpeg', '.gif', '.ico', '.woff', '.woff2', '.ttf', '.zip']); -function escapeRegex(s) { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - /** * Pure: does `content` contain a literal reference to `basename`, delimited * by non-identifier/non-path characters on both sides (so "foo.json" doesn't diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 10fc76205..476ad6bf0 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -949,9 +949,12 @@ function main() { // makes a chunk's `node --test` child hang ~150s on Windows AFTER its last test // prints; two such stalls push the windows full lane past its 20m cap and the // job is CANCELLED with no failed step — a false-negative gate (#1051, recurrence - // of #869). --test-force-exit (Node >=22; engines requires >=22.0.0) exits the - // runner once all tests finish regardless of lingering handles. The leaking - // tests are also fixed at the source; this is the defensive backstop. + // of #869). --test-force-exit (available since Node 22; engines.node now + // requires >=24.0.0, so it is always available here — the nodeMajor check + // below is kept as a floor-independent CLI-flag-availability guard, not a + // statement of this repo's supported version) exits the runner once all + // tests finish regardless of lingering handles. The leaking tests are also + // fixed at the source; this is the defensive backstop. // RUN_TESTS_NO_FORCE_EXIT=1 disables it (used by the harness regression test to // observe the pre-fix hang). const nodeMajor = Number(process.versions.node.split('.')[0]); diff --git a/scripts/sync-runtime-launcher.cjs b/scripts/sync-runtime-launcher.cjs index dbc38a2b4..ebf5047b4 100644 --- a/scripts/sync-runtime-launcher.cjs +++ b/scripts/sync-runtime-launcher.cjs @@ -19,6 +19,8 @@ const fs = require('node:fs'); const path = require('node:path'); +const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); + const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); @@ -373,10 +375,6 @@ function transformFile(content, preamble) { return outputLines.join('\n'); } -function escapeRegExp(str) { - return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - /** * A file "delegates to the shared resolver" when it pulls the canonical gsd_run * preamble in from gsd-core/references/gsd-run-resolver.md via an @-include diff --git a/src/api-coverage.cts b/src/api-coverage.cts index 4afb4bd74..961ae82fb 100644 --- a/src/api-coverage.cts +++ b/src/api-coverage.cts @@ -57,6 +57,7 @@ */ import { stripFencedCode, scanInlineCodeSpans, extractFencedBlock } from './markdown-sectionizer.cjs'; +import { escapeRegex } from './pattern.cjs'; // ─── Integration-signal vocabulary ──────────────────────────────────────────── @@ -170,10 +171,6 @@ function resolveTerms(terms?: Partial): ApiCoverageTermSet { return { verbs: merge('verbs'), nouns: merge('nouns') }; } -function escapeRegex(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function makeSnippet(line: string, anchor: string): string { const cleaned = line.replace(/\s+/g, ' ').trim(); if (cleaned.length <= 120) return cleaned; diff --git a/src/assumption-delta.cts b/src/assumption-delta.cts index 8cae79677..ba1a93965 100644 --- a/src/assumption-delta.cts +++ b/src/assumption-delta.cts @@ -34,6 +34,7 @@ */ import { stripFencedCode } from './markdown-sectionizer.cjs'; +import { escapeRegex } from './pattern.cjs'; export type AssumptionDeltaKind = 'pluralization' | 'optional' | 'chosen'; @@ -210,10 +211,6 @@ export function detectAssumptionDelta( return { detected: signals.length > 0, signals, terms: effective }; } -function escapeRegex(s: string): string { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - // ── 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] diff --git a/src/gap-checker.cts b/src/gap-checker.cts index 0cedde9e2..0db0771f1 100644 --- a/src/gap-checker.cts +++ b/src/gap-checker.cts @@ -21,9 +21,7 @@ import path from 'node:path'; // eslint-disable-next-line @typescript-eslint/no-require-imports import io = require('./io.cjs'); const { output, error } = io; -// eslint-disable-next-line @typescript-eslint/no-require-imports -import phaseId = require('./phase-id.cjs'); -const { escapeRegex } = phaseId; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { planningPaths, planningDir, findContextMdIn } = planningWorkspace; diff --git a/src/init.cts b/src/init.cts index 2aebea796..d64f62183 100644 --- a/src/init.cts +++ b/src/init.cts @@ -11,6 +11,7 @@ import path from 'node:path'; import os from 'node:os'; import { execGit, platformWriteSync, platformReadSync, toNativePath, posixNormalize } from './shell-command-projection.cjs'; import { realClock } from './clock.cjs'; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- io.cjs is an export= CommonJS module import io = require('./io.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module @@ -88,7 +89,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { escapeRegex, normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId } = phaseId; +const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { diff --git a/src/markdown-sectionizer.cts b/src/markdown-sectionizer.cts index 3bf4e8b04..edf5ca427 100644 --- a/src/markdown-sectionizer.cts +++ b/src/markdown-sectionizer.cts @@ -10,6 +10,8 @@ * ADR-457 build-at-publish: compiled by tsc to gsd-core/bin/lib/markdown-sectionizer.cjs. */ +import { escapeRegex } from './pattern.cjs'; + // ─── Types ──────────────────────────────────────────────────────────────────── /** Result of stripping fenced code blocks from markdown content. */ @@ -940,7 +942,7 @@ export function extractTaggedBlocks(content: string, tagName: string, allowAttri * open>` marks the ACTIVE milestone and must be preserved, not stripped (#557). */ function taggedBlockPattern(tagName: string, flags: string, allowAttributes: boolean): RegExp { - const esc = tagName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const esc = escapeRegex(tagName); const open = allowAttributes ? `<${esc}(?:\\s[^>]{0,1000})?>` : `<${esc}>`; const boundary = allowAttributes ? `<${esc}[\\s>]` : `<${esc}>`; return new RegExp(`${open}((?:(?!${boundary})[\\s\\S])*?)`, flags); diff --git a/src/milestone.cts b/src/milestone.cts index 56aa7cf31..bd02444ba 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -26,7 +26,8 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, matchPhaseDirs, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, matchPhaseDirs, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { diff --git a/src/pattern.cts b/src/pattern.cts new file mode 100644 index 000000000..de1067fd2 --- /dev/null +++ b/src/pattern.cts @@ -0,0 +1,50 @@ +/** + * pattern.cts — the pattern-construction seam (ADR-3212 §1, epic #3212 Phase 1, + * #3412). + * + * Source in src/pattern.cts, compiled to gsd-core/bin/lib/pattern.cjs + * (gitignored), per the repo's ADR-457 build-at-publish convention. + * + * Sole owner of building a `RegExp` from a runtime value. `escapeRegex` + * delegates to the built-in `RegExp.escape` (ES2026 / Node 24+) rather than + * hand-rolling yet another copy of the metacharacter-escape helper this + * module replaces — see ADR §1 ("No module outside the seam escapes a value + * for regex use"). + * + * Counts, corrected during implementation (design doc "Ground truth" #1; + * .gsd/phase/chore-3412-pattern-seam/40-design.md): the ADR's census counted + * ~12 *named* helper functions. The shape-based lint guard + * (eslint-rules/no-adhoc-regex-escape.cjs), which matches the escape-class + * SHAPE wherever it appears rather than named-function bodies, found 27 + * additional inline copies that census-by-name could not see — **~39 escape + * sites total**. Call sites follow the same pattern: 17 were surveyed + * directly, but `src/phase-id.cts`'s `escapeRegex` turned out to have 8 + * external production importers the pre-implementation survey missed, plus + * the 27 guard-found inline sites also call into the seam — **~44 call + * sites total**. The lesson worth keeping: a named-function census + * structurally cannot see an inline `.replace(...)` copy or an + * externally-imported symbol; only a shape-based guard (or a direct + * importer graph query) does. + * + * `escapeRegex` is the PRIMARY export: the large majority of call sites + * build a *source string* (alternation via `.map(escapeRegex).join('|')`, or + * template/concat interpolation into a larger pattern) rather than a + * standalone literal match. `literalPattern` is the minority convenience + * wrapper for the remaining "match this value literally" shape — not the + * dominant one (design doc "Ground truth" #2). + * + * Behavior-preserving for MATCH RESULTS, not for pattern TEXT: `RegExp.escape` + * hex-escapes the leading character of nearly every non-empty input (and `-`, + * space, and control characters throughout), so the escaped source string + * differs from the twelve deleted copies' output for almost every value. + * Match behavior against that source is unaffected — verified by the + * migration-equivalence property sweep in tests/pattern.test.cjs (rows 15-17). + */ + +export function escapeRegex(value: string): string { + return RegExp.escape(value); +} + +export function literalPattern(value: string, flags?: string): RegExp { + return new RegExp(escapeRegex(value), flags); +} diff --git a/src/phase-id.cts b/src/phase-id.cts index 28ecc4b17..08f6a9aea 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -7,14 +7,14 @@ * boundary moved. The core.cjs re-export spine was retired in epic #1267; * callers import phase-id helpers from phase-id.cjs directly. * - * Dependencies: none (pure string/regex, no Node built-ins required). + * Dependencies: + * - ./pattern.cjs (escapeRegex — #3212 Phase 1 seam; this module is no + * longer the owner of pattern-escaping, only a consumer) */ -// ─── Phase-id helpers ───────────────────────────────────────────────────────── +import { escapeRegex } from './pattern.cjs'; -function escapeRegex(value: unknown): string { - return String(value).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} +// ─── Phase-id helpers ───────────────────────────────────────────────────────── // project_code values start with an uppercase letter (e.g. PROJ, APP_CODE); // leading underscores are not valid project codes per .planning/config.json. @@ -437,7 +437,11 @@ function phaseMarkdownRegexSource(phaseNum: unknown): string { // Plain numeric phase: 1, 01, 12A, 12.1 const match = stripped.match(/^0*(\d+)([A-Z])?((?:\.\d+)*)$/i); - if (!match) return escapeRegex(phaseNum); + // #3212: escapeRegex now requires a string (the seam owns coercion policy, + // not this module) — String(...) here preserves this function's own + // pre-existing `unknown` acceptance for callers that pass a non-string + // phaseNum through to this fallback branch. + if (!match) return escapeRegex(String(phaseNum)); const integer = match[1].replace(/^0+/, '') || '0'; const letter = match[2] ? escapeRegex(match[2]) : ''; @@ -924,7 +928,6 @@ function roadmapPhaseLookupSources(phaseNum: unknown): string[] { } export = { - escapeRegex, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, diff --git a/src/phase.cts b/src/phase.cts index 05b33855a..d0d66cd8f 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -38,7 +38,6 @@ const { // eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-id.cjs is an export= CommonJS module import phaseIdMod = require('./phase-id.cjs'); const { - escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, comparePhaseNum, @@ -48,6 +47,7 @@ const { OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, } = phaseIdMod; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-locator.cjs is an export= CommonJS module import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal, getArchivedPhaseDirs, listMilestonePhaseDirs } = phaseLocatorMod; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index a76521314..bddd251d4 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -10,7 +10,8 @@ * * Dependencies (leaf modules only — no loadConfig): * - node:fs / node:path (stdlib) - * - ./phase-id.cjs (escapeRegex, phaseMarkdownRegexSource) + * - ./phase-id.cjs (phaseMarkdownRegexSource) + * - ./pattern.cjs (escapeRegex — #3212 Phase 1 seam) * - ./planning-workspace.cjs (planningDir) * - ./shell-command-projection.cjs (platformReadSync) * - ./markdown-sectionizer.cjs (tokenizeHeadings, stripTaggedBlocks, withSection, collectSection) @@ -19,10 +20,10 @@ import fs from 'node:fs'; import path from 'node:path'; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdModule = require('./phase-id.cjs'); const { - escapeRegex, phaseMarkdownRegexSource, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, diff --git a/src/roadmap.cts b/src/roadmap.cts index 1050522e9..65d1291f6 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -9,12 +9,13 @@ import fs from 'node:fs'; import path from 'node:path'; import { realClock } from './clock.cjs'; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, matchPhaseDirs, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, phaseMarkdownRegexSource, matchPhaseDirs, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal, listMilestonePhaseDirs } = phaseLocatorMod; @@ -124,17 +125,26 @@ function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { // ─── searchPhaseInContent ───────────────────────────────────────────────────── +/** + * Build the phase-heading regex used by `searchPhaseInContent` for a given + * pre-escaped phase source. Extracted (#3412) so tests can assert against the + * exact production pattern instead of hand-duplicating it. + * #1729: OPTIONAL_PHASE_TAG_SOURCE after the number tolerates a pre-colon ( ) tag. + */ +function buildPhaseHeadingRegex(escapedPhase: string): RegExp { + return new RegExp( + `^(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+${escapedPhase}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, + 'i' + ); +} + /** * Search for a phase header (and its section) within the given content string. * Returns a result object if found (either a full match or a malformed_roadmap * checklist-only match), or null if the phase is not present at all. */ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: string): PhaseSearchResult | null { - // #1729: OPTIONAL_PHASE_TAG_SOURCE after the number tolerates a pre-colon ( ) tag. - const headingPattern = new RegExp( - `^(?:\\[[^\\]]{1,200}\\]\\s*)?Phase\\s+${escapedPhase}${OPTIONAL_PHASE_TAG_SOURCE}:\\s*(.+)$`, - 'i' - ); + const headingPattern = buildPhaseHeadingRegex(escapedPhase); const headings = tokenizeHeadings(content); const headingIndex = headings.findIndex((heading) => headingPattern.test(heading.text)); const headerMatch = headingIndex === -1 ? null : headings[headingIndex].text.match(headingPattern); @@ -1084,4 +1094,5 @@ export = { cmdRoadmapAnalyze, cmdRoadmapUpdatePlanProgress, cmdRoadmapAnnotateDependencies, + buildPhaseHeadingRegex, }; diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index 442537a42..8ee05dc62 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -26,6 +26,7 @@ const { getDirName } = runtimeNamePolicy; import capabilityRegistry = require('./capability-registry.cjs'); import hostIntegration = require('./host-integration.cjs'); import { posixNormalize } from './shell-command-projection.cjs'; +import { escapeRegex as escapeRegExp } from './pattern.cjs'; // #2870: install-scope.cts is a leaf-tier sibling (imports only // runtime-homes.cjs + node builtins, never this module) — no cycle. See the // isGlobal sites below for why the boolean projection is centralized here too. @@ -292,10 +293,6 @@ function buildKiloAgentPermissionBlock(claudeTools) { return lines; } -function escapeRegExp(value) { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function replaceRelativePathReference(content, fromPath, toPath) { const escapedPath = escapeRegExp(fromPath); return content.replace( diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index cf0f8ec6b..5b7558b57 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -17,6 +17,7 @@ import fs from 'node:fs'; // can intercept calls from this seam — destructured imports capture references // at load time and become un-mockable. import childProcess from 'node:child_process'; +import { escapeRegex } from './pattern.cjs'; /** * Convert a filesystem path to POSIX form (forward slashes) by translating the @@ -297,7 +298,7 @@ export function isManagedHookCommand(commandText: unknown, opts: { surface?: str } for (const basename of managedBasenames) { - const escapedBasename = basename.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escapedBasename = escapeRegex(basename); const pattern = new RegExp(`(^|[\\\\/\\s"'` + '`' + `])${escapedBasename}(?=$|[\\s"'` + '`' + `])`); if (pattern.test(normalizedCommand)) return true; } diff --git a/src/state-document.cts b/src/state-document.cts index 192c89c97..a47aa9f8b 100644 --- a/src/state-document.cts +++ b/src/state-document.cts @@ -11,16 +11,12 @@ import { splitTableRow } from './markdown-table.cjs'; import { clampPercentFromFraction } from './phase-lifecycle.cjs'; import { collectSection } from './markdown-sectionizer.cjs'; import type { HeadingToken } from './markdown-sectionizer.cjs'; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module import planningScopeMod = require('./planning-scope.cjs'); const { SCOPE } = planningScopeMod; type Scope = planningScopeMod.Scope; -// Internal helpers -function escapeRegex(str: string): string { - return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function toFiniteNumber(value: unknown): number | null { const number = Number(value); return Number.isFinite(number) ? number : null; diff --git a/src/state-transition.cts b/src/state-transition.cts index 859c64182..8f370e414 100644 --- a/src/state-transition.cts +++ b/src/state-transition.cts @@ -21,11 +21,9 @@ import { KNOWN_TEMPLATE_DEFAULTS } from './state-document.cjs'; import { tokenizeHeadings } from './markdown-sectionizer.cjs'; import type { HeadingToken } from './markdown-sectionizer.cjs'; import { deriveProgressFromRoadmap, clampPercent } from './phase-lifecycle.cjs'; -// eslint-disable-next-line @typescript-eslint/no-require-imports -import phaseIdMod = require('./phase-id.cjs'); +import { escapeRegex } from './pattern.cjs'; const { extractFrontmatter, reconstructFrontmatter, stripFrontmatter } = frontmatter; -const { escapeRegex } = phaseIdMod; // Stop predicate for section-body slicing: a level-2+ heading ends the section. const STOP_H2_PLUS = (lv: number): boolean => lv >= 2; diff --git a/src/state.cts b/src/state.cts index fafffbb2d..7f52f562b 100644 --- a/src/state.cts +++ b/src/state.cts @@ -8,6 +8,7 @@ import fs from 'node:fs'; import path from 'node:path'; +import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import ioMod = require('./io.cjs'); const { output, error } = ioMod; @@ -16,7 +17,7 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir, isSentinelPhaseId } = phaseIdMod; +const { parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { getMilestoneInfo, extractCurrentMilestone, isMilestoneBoundedInRoadmap, hasMilestoneSectioning } = roadmapParserMod; diff --git a/tests/adr-index-gate.test.cjs b/tests/adr-index-gate.test.cjs index d35e09664..077a68304 100644 --- a/tests/adr-index-gate.test.cjs +++ b/tests/adr-index-gate.test.cjs @@ -16,6 +16,7 @@ const path = require('node:path'); const { spawnSync } = require('node:child_process'); const { createTempDir, cleanup } = require('./helpers.cjs'); +const { copyScriptWithDeps } = require('./helpers/copy-script-fixture.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const SCRIPT_REL = path.join('scripts', 'gen-adr-index.cjs'); @@ -25,22 +26,18 @@ const END = ''; /** * Build a throwaway repo whose docs/adr/ contains exactly `files`, and whose - * scripts/ holds a copy of the generator + its cli-exit dependency. A unique - * mkdtemp per call keeps parallel tests from colliding, and the dir is removed - * via `t.after()` so a failing assertion cannot leak it. + * scripts/ holds a copy of the generator together with its transitive + * relative-require graph. A unique mkdtemp per call keeps parallel tests from + * colliding, and the dir is removed via `t.after()` so a failing assertion + * cannot leak it. */ function makeRepo(t, files) { // helpers.cleanup (not raw fs.rmSync) carries the Windows-EBUSY retry budget. const root = createTempDir('gsd-adr-index-'); t.after(() => cleanup(root)); fs.mkdirSync(path.join(root, 'docs', 'adr'), { recursive: true }); - fs.mkdirSync(path.join(root, 'scripts', 'lib'), { recursive: true }); - fs.copyFileSync(path.join(REPO_ROOT, SCRIPT_REL), path.join(root, SCRIPT_REL)); - fs.copyFileSync( - path.join(REPO_ROOT, 'scripts', 'lib', 'cli-exit.cjs'), - path.join(root, 'scripts', 'lib', 'cli-exit.cjs'), - ); + copyScriptWithDeps(REPO_ROOT, root, SCRIPT_REL); for (const [name, body] of Object.entries(files)) { fs.writeFileSync(path.join(root, 'docs', 'adr', name), body); diff --git a/tests/api-coverage-gate-e2e.test.cjs b/tests/api-coverage-gate-e2e.test.cjs index 6dee682f0..4cb629afb 100644 --- a/tests/api-coverage-gate-e2e.test.cjs +++ b/tests/api-coverage-gate-e2e.test.cjs @@ -26,6 +26,7 @@ const { runNode, OUTCOME } = require('./helpers/process-seam.cjs'); // file (#2365 review): readPhaseScope is the pure phase-scope reader behind the // gate. Those tests monkeypatch fs rather than drive a subprocess. const { readPhaseScope } = require('../gsd-core/bin/lib/check-command-router.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); @@ -342,7 +343,7 @@ describe('readPhaseScope — fail-closed on a real read failure (#2365 review)', tmpDir = makeProject({ api_coverage_gate: true }); const phaseDir = makePhaseDir(tmpDir, '01-pay'); writePlan(phaseDir, '01-PLAN.md', '# Plan\nIntegrate the Stripe API.'); - const res = withFsThrow('readdirSync', new RegExp(phaseDir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '$'), 'EACCES', () => + const res = withFsThrow('readdirSync', new RegExp(escapeRegex(phaseDir) + '$'), 'EACCES', () => readPhaseScope(tmpDir, phaseDir, '01')); assert.ok(res.readError, 'an unreadable phase directory must set readError, not read as empty'); }); diff --git a/tests/capability-lifecycle.test.cjs b/tests/capability-lifecycle.test.cjs index 28ce73138..1045effe2 100644 --- a/tests/capability-lifecycle.test.cjs +++ b/tests/capability-lifecycle.test.cjs @@ -20,6 +20,7 @@ const path = require('node:path'); const { cleanup } = require('./helpers.cjs'); const lifecycle = require('../gsd-core/bin/lib/capability-lifecycle.cjs'); const ledgerMod = require('../gsd-core/bin/lib/capability-ledger.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const { CAP_MARKER } = lifecycle; // --------------------------------------------------------------------------- @@ -3218,7 +3219,7 @@ test('IC-05/WIN-2: a consent-store write failure leaves the install status:insta assert.ok(fs.existsSync(path.join(dir, '.gsd', 'capabilities', 'rofs-cap', 'capability.json')), 'the bundle is committed on disk'); assert.match(buf, /capability consent:/i, 'a consent diagnostic was written to stderr'); assert.match(buf, /could not write the consent record/i, 'the warning explains the write failure'); - assert.match(buf, new RegExp(home.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), 'the warning names the consent store path'); + assert.match(buf, new RegExp(escapeRegex(home)), 'the warning names the consent store path'); }); // --------------------------------------------------------------------------- diff --git a/tests/check-env.test.cjs b/tests/check-env.test.cjs index 0d585fc37..7582cc75e 100644 --- a/tests/check-env.test.cjs +++ b/tests/check-env.test.cjs @@ -222,7 +222,8 @@ describe('check-env.cjs', () => { } assert.equal(typeof parsed.pass, 'boolean', 'Live repo JSON must have boolean pass'); assert.ok(Array.isArray(parsed.checks), 'Live repo JSON must have checks array'); - // Node version check must be present and pass (Node >=22 is installed) + // Node version check must be present and pass (Node >=24 is installed, + // per package.json engines.node) const nodeCheck = parsed.checks.find((c) => c.name === 'node-version'); assert.ok(nodeCheck, 'node-version check must be present in live repo output'); assert.equal(nodeCheck.status, 'pass', `node-version should pass on live repo, got: ${nodeCheck.status} — ${nodeCheck.message}`); diff --git a/tests/code-review-agent-skills.test.cjs b/tests/code-review-agent-skills.test.cjs index a6e211c17..ebff77ae2 100644 --- a/tests/code-review-agent-skills.test.cjs +++ b/tests/code-review-agent-skills.test.cjs @@ -16,6 +16,8 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { escapeRegex: escapeRe } = require('../gsd-core/bin/lib/pattern.cjs'); + const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); // Workflow file -> EVERY subagent type it spawns and must inject skills for. @@ -29,10 +31,6 @@ const REVIEW_FAMILY = [ { file: 'eval-review.md', agentTypes: ['gsd-eval-auditor'] }, ]; -function escapeRe(s) { - return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - const cases = REVIEW_FAMILY.flatMap(({ file, agentTypes }) => agentTypes.map((agentType) => ({ file, agentType })), ); diff --git a/tests/code-review.test.cjs b/tests/code-review.test.cjs index dcdf77758..91215cd01 100644 --- a/tests/code-review.test.cjs +++ b/tests/code-review.test.cjs @@ -26,6 +26,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); // --- Test Environment Setup --- @@ -747,7 +748,7 @@ describe('bug-2839: /gsd-code-review-fix cleanup is transactional', () => { // (`rm -f .../.review-fix-recovery-pending.json`) or a shell-variable form // referring to the previously-declared `sentinel` variable // (`rm -f "$sentinel"` / `rm -f "${sentinel}"`). - const escapedName = SENTINEL_NAME.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escapedName = escapeRegex(SENTINEL_NAME); const sentinelRemovalRe = new RegExp( `(rm\\s+(?:-f\\s+)?[^\\n]*(?:${escapedName}|\\$\\{?sentinel\\}?)|unlink[^\\n]*(?:${escapedName}|\\$\\{?sentinel\\}?))` ); diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index 702367a33..235ba83e4 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -23,6 +23,7 @@ const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); const fc = require('fast-check'); const { CLAUDE_AGENT_ALIASES } = require('../gsd-core/bin/lib/model-resolver.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); // #3241 — the intended new home for CLAUDE_AGENT_ALIASES + isAnthropicFlavoredModel // (see .gsd/phase/feat-3241-codex-omit-model-by-default/40-design.md "The seam // decision"). Neither export exists on model-catalog.cjs yet; requiring the @@ -1721,7 +1722,7 @@ describe('mergeCodexConfig', () => { assert.ok(!content.includes('Updated description'), 'no per-agent description in config.toml'); assert.ok(!content.includes('[agents.gsd-planner]'), 'no agent role table (canonical source is the standalone TOML)'); // Verify no duplicate markers - const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const markerCount = (content.match(new RegExp(escapeRegex(GSD_CODEX_MARKER), 'g')) || []).length; assert.strictEqual(markerCount, 1, 'exactly one marker'); }); @@ -1775,7 +1776,7 @@ describe('mergeCodexConfig', () => { // GSD section (stripLeakedGsdCodexSections) and the fresh block never // re-adds one (#2406) — zero role tables should remain anywhere. const gsdStructCount = (content.match(/^\[agents\.gsd-executor\]\s*$/gm) || []).length; - const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const markerCount = (content.match(new RegExp(escapeRegex(GSD_CODEX_MARKER), 'g')) || []).length; assert.ok(content.includes('[model]'), 'preserves user content'); assert.ok(content.includes('[agents.custom-agent]'), 'preserves non-GSD agent section'); @@ -1808,7 +1809,7 @@ describe('mergeCodexConfig', () => { assert.ok(content.includes('other_feature = true'), 'preserves user feature keys'); assert.ok(!content.includes('[agents.gsd-executor]'), 'no agent role table (canonical source is the standalone TOML)'); // Verify no duplicate markers - const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const markerCount = (content.match(new RegExp(escapeRegex(GSD_CODEX_MARKER), 'g')) || []).length; assert.strictEqual(markerCount, 1, 'exactly one marker'); }); @@ -2128,7 +2129,7 @@ describe('mergeCodexConfig trailing-content preservation (#2940)', () => { assert.ok(content.includes('[model]'), 'user [model] section preserved after re-merge'); assert.ok(content.includes('name = "gpt-5.4"'), 'user model value preserved verbatim'); assert.ok(content.includes(GSD_CODEX_MARKER), 'GSD marker still present'); - const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const markerCount = (content.match(new RegExp(escapeRegex(GSD_CODEX_MARKER), 'g')) || []).length; assert.strictEqual(markerCount, 1, 'exactly one marker (no duplication)'); assert.ok(content.includes('max_depth ='), 'GSD-managed [agents] block regenerated'); }); @@ -2237,7 +2238,7 @@ describe('mergeCodexConfig trailing-content preservation (#2940)', () => { const content = fs.readFileSync(configPath, 'utf8'); assert.ok(content.includes('[profiles.work]'), 'content before marker preserved'); assert.ok(content.includes('[mcp_servers.github]'), 'content after marker preserved'); - const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length; + const markerCount = (content.match(new RegExp(escapeRegex(GSD_CODEX_MARKER), 'g')) || []).length; assert.strictEqual(markerCount, 1, 'exactly one marker'); }); @@ -4712,7 +4713,7 @@ describe('#2760 fix 4 — Write-failure rollback (atomic write + snapshot restor const preInstallBytes = fs.readFileSync(path.join(codexHome, 'config.toml')); const configPath = path.join(codexHome, 'config.toml'); - const tempPattern = new RegExp('^' + configPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\.tmp-'); + const tempPattern = new RegExp('^' + escapeRegex(configPath) + '\\.tmp-'); // Stub: allow writes to atomic temp files (which renameSync overwrites // the target, never truncating it directly) but throw on any direct @@ -4778,7 +4779,7 @@ describe('#2760 fix 4 — Write-failure rollback (atomic write + snapshot restor const preInstallBytes = fs.readFileSync(path.join(codexHome, 'config.toml')); const configPath = path.join(codexHome, 'config.toml'); - const tempPattern = new RegExp('^' + configPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\.tmp-'); + const tempPattern = new RegExp('^' + escapeRegex(configPath) + '\\.tmp-'); // Stub: fault writes targeting the atomic temp file (the pre-rename branch // of atomicWriteFileSync). Other writes (agent .toml files in CODEX_HOME) diff --git a/tests/config-schema.property.test.cjs b/tests/config-schema.property.test.cjs index 403b33e96..3b7767853 100644 --- a/tests/config-schema.property.test.cjs +++ b/tests/config-schema.property.test.cjs @@ -31,6 +31,7 @@ const { VALID_CONFIG_KEYS, RUNTIME_STATE_KEYS, } = require('../gsd-core/bin/lib/config-schema.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); describe('config-schema: isValidConfigKey properties', () => { // (a) Never throws on any input @@ -487,7 +488,7 @@ const SECTION_HEADERS = ['Planning', 'Execution', 'Docs & Output', 'Features', ' function hasPathLike(block, field) { const parts = field.split('.'); if (parts.length === 1) return block.includes(parts[0]); - const escaped = parts.map((p) => p.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); + const escaped = parts.map((p) => escapeRegex(p)); const pattern = new RegExp(escaped.join('[\\s\\S]{0,600}'), 'i'); return pattern.test(block); } diff --git a/tests/cursor-hook-workspace-roots.test.cjs b/tests/cursor-hook-workspace-roots.test.cjs index 089421772..6d45e5eb3 100644 --- a/tests/cursor-hook-workspace-roots.test.cjs +++ b/tests/cursor-hook-workspace-roots.test.cjs @@ -32,6 +32,7 @@ const path = require('node:path'); const { execFileSync } = require('node:child_process'); const { createTempDir, cleanup } = require('./helpers.cjs'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const HOOKS = path.join(__dirname, '..', 'hooks'); const SESSION_START = path.join(HOOKS, 'gsd-cursor-session-start.js'); @@ -85,7 +86,7 @@ describe('#2587: cursor hooks resolve the workspace from workspace_roots, not cw }); assert.match( out.additional_context || '', - new RegExp(MSG_PRESENT_FRAGMENT.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), + new RegExp(escapeRegex(MSG_PRESENT_FRAGMENT)), 'must report STATE.md present when workspace_roots points at the project', ); } finally { diff --git a/tests/docs-parity-live-registry.test.cjs b/tests/docs-parity-live-registry.test.cjs index 85ed92d32..d8a8d331a 100644 --- a/tests/docs-parity-live-registry.test.cjs +++ b/tests/docs-parity-live-registry.test.cjs @@ -231,9 +231,12 @@ function extractCommandTokens(content) { /** * Walk a directory and return all .md files recursively. - * Uses hand-rolled DFS for Node 20 compat (Node 22+ recursive readdirSync is - * not available in all CI matrix entries). Surfaces permission-denied errors - * as structured warnings (PRED.k302) rather than silently skipping. + * Uses hand-rolled DFS rather than `fs.readdirSync(dir, { recursive: true })` + * — the recursive option is available on every CI matrix entry now that the + * Node floor is >=24.0.0 (it landed in Node 20.1), but this walker predates + * that bump and was not rewritten as part of it; unchanged, not stale. + * Surfaces permission-denied errors as structured warnings (PRED.k302) rather + * than silently skipping. */ function listMdFiles(dir) { if (!fs.existsSync(dir)) return []; diff --git a/tests/emitted-attribution.test.cjs b/tests/emitted-attribution.test.cjs index f352d1ae9..14121d3ef 100644 --- a/tests/emitted-attribution.test.cjs +++ b/tests/emitted-attribution.test.cjs @@ -41,6 +41,7 @@ const fc = require('fast-check'); const { cleanup, createTempDir } = require('./helpers.cjs'); const { BUILD_SCRIPT, buildParityManifest, buildInstallTree, PKG_VERSION } = require('./helpers/install-shared.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const { resolveChangedPaths, resolveBase, @@ -891,7 +892,7 @@ test('listFragmentFiles: exactly MAX_ACK_FRAGMENTS entries passes, one over fail assert.throws( () => listFragmentFiles(dir), (err) => { - assert.match(err.message, new RegExp(dir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))); + assert.match(err.message, new RegExp(escapeRegex(dir))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS + 1))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS))); return true; @@ -1233,7 +1234,7 @@ test('listAckFragmentFiles: exactly MAX_ACK_FRAGMENTS entries passes, one over f assert.throws( () => listAckFragmentFiles(dir), (err) => { - assert.match(err.message, new RegExp(dir.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))); + assert.match(err.message, new RegExp(escapeRegex(dir))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS + 1))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS))); return true; @@ -1262,7 +1263,7 @@ test('listAckFragmentFilesAtRef: exactly MAX_ACK_FRAGMENTS entries passes, one o assert.throws( () => listAckFragmentFilesAtRef(SHA_A, { run: overCap }), (err) => { - assert.match(err.message, new RegExp(`${ACK_DIR_REPO_PATH}/ at "${SHA_A}"`.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))); + assert.match(err.message, new RegExp(escapeRegex(`${ACK_DIR_REPO_PATH}/ at "${SHA_A}"`))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS + 1))); assert.match(err.message, new RegExp(String(MAX_ACK_FRAGMENTS))); return true; diff --git a/tests/eslint-no-adhoc-regex-escape.test.cjs b/tests/eslint-no-adhoc-regex-escape.test.cjs new file mode 100644 index 000000000..ebf038d8a --- /dev/null +++ b/tests/eslint-no-adhoc-regex-escape.test.cjs @@ -0,0 +1,306 @@ +'use strict'; + +/** + * RuleTester unit tests for `local/no-adhoc-regex-escape` + * (#3212 Phase 1, #3412). + * + * Design: .gsd/phase/chore-3412-pattern-seam/40-design.md (Negative space) + * Test matrix: .gsd/phase/chore-3412-pattern-seam/50-test-matrix.md — section 5, rows 24-29 + * ADR: docs/adr/3212-lexical-seam-consolidation.md §7 + * + * TDD RED: `eslint-rules/no-adhoc-regex-escape.cjs` does not exist yet — + * this file's require() throws MODULE_NOT_FOUND until the implementing phase + * adds it. That is the intended starting state. + * + * RuleTester setup (languageOptions, sourceType, filename conventions) mirrors + * tests/eslint-rules.test.cjs's `no-adhoc-markdown-parsing` suite, which is + * this repo's other src/*.cts-scoped structural-shape rule. + * + * Contract this test file locks for the not-yet-written rule (per ADR §7 and + * the design doc's Negative space section): + * - FIRES on an inline `.replace(, '\$&')` + * shape outside src/pattern.cts, matching the SHAPE (character-class + * membership set) not exact byte order (row 25). + * - Does NOT fire inside src/pattern.cts itself — the seam is the owner + * (row 26). + * - Does NOT fire on `new RegExp()` when the identifier is a + * `*_SOURCE`-suffixed constant — the design's named provenance marker for + * "deliberate, reviewed pattern fragments" (row 27). + * - Does NOT fire on a regex literal that merely contains the escape class + * as DATA, with no `.replace(..., '\$&')` shape present (row 28). + * - FIRES on `new RegExp()` when the identifier is a plain + * runtime value with no `_SOURCE` provenance marker and no routing + * through escapeRegex/literalPattern — ADR §7's "new RegExp() built from + * a non-literal without routing through the seam" (row 29). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const { RuleTester } = require('eslint'); + +const noAdhocRegexEscape = require('../eslint-rules/no-adhoc-regex-escape.cjs'); +const { findViolations } = require('../scripts/lint-no-adhoc-regex-escape.cjs'); + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'commonjs', + }, +}); + +// Separate instance for the ES-module `import` binding case (row 27b) — the +// default instance above uses sourceType 'commonjs', which cannot parse +// `import` statements. +const moduleRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, +}); + +describe('no-adhoc-regex-escape rule', () => { + test('rule module exports a create function', () => { + assert.strictEqual(typeof noAdhocRegexEscape.create, 'function'); + }); + + // ── row 24: invalid — a new inline escape-all-metachars .replace() ──────── + + test('row 24 invalid: a new inline .replace(/[.*+?^${}()|[\\]\\\\]/g, \'\\\\$&\') outside the seam', () => { + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [], + invalid: [ + { + // NOT String.raw: the code-under-test contains a literal `${` (part + // of the escape-all-metachars character class), which String.raw + // would still interpret as a template-literal substitution marker. + // A normal (non-raw) template literal is used instead, with `\$` + // and doubled backslashes to produce the exact literal source text. + code: ` + function esc(s) { + return s.replace(/[.*+?^\${}()|[\\]\\\\]/g, '\\\\$&'); + } + `, + filename: 'src/some-module.cts', + errors: [{ messageId: 'adhocRegexEscape' }], + }, + ], + }); + }); + + // ── row 25: invalid — reordered/differently-escaped char class, same shape ─ + + test('row 25 invalid: same escape shape with a REORDERED character class still fires (matches shape, not exact bytes)', () => { + // Same 14-member set { . * + ? ^ $ { } ( ) | [ ] \ } as row 24, but written + // in a different order and with a different (still valid) escaping style. + // If the rule only string-matched the exact literal bytes of the census + // copies, this would slip through — it must not. + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [], + invalid: [ + { + code: String.raw` + function escapeForRegex(value) { + return value.replace(/[\^$.|?*+()[\]{}\\]/g, '\\$&'); + } + `, + filename: 'src/another-module.cts', + errors: [{ messageId: 'adhocRegexEscape' }], + }, + ], + }); + }); + + // ── row 26: valid — the seam file itself is exempt ───────────────────────── + + test('row 26 valid: src/pattern.cts itself is exempt (the seam owns this shape)', () => { + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [ + { + // Not String.raw — see the row-24 comment above (`${` needs escaping + // in a non-raw template literal). + code: ` + function escapeRegex(value) { + return value.replace(/[.*+?^\${}()|[\\]\\\\]/g, '\\\\$&'); + } + `, + filename: 'src/pattern.cts', + }, + ], + invalid: [], + }); + }); + + // ── row 27: valid (negative space) — new RegExp from an exported *_SOURCE ── + + test('row 27 valid: new RegExp built from an exported *_SOURCE constant is NOT flagged (critical negative-space case)', () => { + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [ + { + // A deliberate, reviewed pattern fragment (design doc Negative space: + // PHASE_NUMBER_TOKEN_SOURCE, MILESTONE_HEADING_LINE_SOURCE, etc.) — + // its metacharacters are load-bearing and must not be treated as an + // unescaped runtime value. + code: String.raw` + const PHASE_NUMBER_TOKEN_SOURCE = '\\d+[A-Z]?'; + const re = new RegExp(PHASE_NUMBER_TOKEN_SOURCE); + `, + filename: 'src/phase-id.cts', + }, + { + code: String.raw` + const MILESTONE_HEADING_LINE_SOURCE = '^##\\s+Milestone\\s'; + const re = new RegExp(MILESTONE_HEADING_LINE_SOURCE, 'm'); + `, + filename: 'src/roadmap-parser.cts', + }, + ], + invalid: [], + }); + }); + + // ── row 28: valid (negative space) — regex literal containing the class as data ─ + + test('row 28 valid: a regex literal that merely CONTAINS [.*+?] as data is NOT flagged', () => { + // A legitimate validator matching one of the metacharacters as literal + // DATA — no .replace(..., '\$&') shape present, so this must not fire. + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [ + { + code: String.raw`const isMetachar = /^[.*+?]$/.test(char);`, + filename: 'src/some-validator.cts', + }, + ], + invalid: [], + }); + }); + + // ── #3412 Standards-review Finding 1: _SOURCE naming-only evasion closed ── + // A rule that trusts the `_SOURCE` suffix on spelling alone (no + // scope/binding check) is exactly #3410's guard-evasion class: naming a + // genuinely dynamic value with a matching suffix defeats it. These + // cases lock that the fallback is now bound to the identifier's ACTUAL + // BINDING KIND (import / require()-derived const), not its name. + + test('Finding 1 invalid: a function-parameter identifier named *_SOURCE is NOT exempted by naming alone', () => { + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [], + invalid: [ + { + // The evasion: naming a plain, unescaped function parameter with + // the `_SOURCE` suffix used to sail past condition (b) because + // that check was pure identifier-name regex matching with no + // scope/binding verification. It must fire now. + code: String.raw` + function f(userInput_SOURCE) { + return new RegExp(userInput_SOURCE); + } + `, + filename: 'src/some-module.cts', + errors: [{ messageId: 'unsafeNewRegExp' }], + }, + ], + }); + }); + + test('Finding 1 invalid: a reassigned let identifier named *_SOURCE is NOT exempted by naming alone', () => { + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [], + invalid: [ + { + // A `let` binding holding genuinely dynamic content (not a + // static const, not an import, not a require()-derived const) — + // the suffix alone must not exempt it. + code: String.raw` + let mutable_SOURCE = getUserInput(); + const re = new RegExp(mutable_SOURCE); + `, + filename: 'src/some-module.cts', + errors: [{ messageId: 'unsafeNewRegExp' }], + }, + ], + }); + }); + + test('Finding 1 valid: a require()-derived *_SOURCE const (destructured) still passes', () => { + // The legitimate cross-module case condition (b) exists to cover: + // `const { X_SOURCE } = require('./mod.cjs')`. Proves the tightened + // check did not regress this repo's dominant CJS import shape. + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [ + { + code: String.raw` + const { PHASE_NUMBER_TOKEN_SOURCE } = require('./phase-id.cjs'); + const re = new RegExp(PHASE_NUMBER_TOKEN_SOURCE); + `, + filename: 'src/roadmap-parser.cts', + }, + { + // The other require()-derived shape condition (b) covers: + // `const X_SOURCE = require('./mod.cjs').X_SOURCE`. + code: String.raw` + const PHASE_NUMBER_TOKEN_SOURCE = require('./phase-id.cjs').PHASE_NUMBER_TOKEN_SOURCE; + const re = new RegExp(PHASE_NUMBER_TOKEN_SOURCE); + `, + filename: 'src/roadmap-parser.cts', + }, + ], + invalid: [], + }); + }); + + test('Finding 1 valid: an ES `import`-derived *_SOURCE binding still passes', () => { + moduleRuleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [ + { + code: String.raw` + import { PHASE_NUMBER_TOKEN_SOURCE } from './phase-id.cts'; + const re = new RegExp(PHASE_NUMBER_TOKEN_SOURCE); + `, + filename: 'src/roadmap-parser.cts', + }, + ], + invalid: [], + }); + }); + + // ── row 29: invalid — new RegExp from a genuinely unescaped runtime value ── + + test('row 29 invalid: new RegExp from a genuinely unescaped runtime value fires', () => { + // A plain, non-`_SOURCE`-suffixed parameter interpolated straight into + // new RegExp() with no seam routing (no escapeRegex/literalPattern call + // anywhere on it) — ADR §7: "flags new RegExp() built from a non-literal + // without routing through the seam." This is the minimal-pair inverse of + // row 27: same `new RegExp()` shape, but the identifier + // carries no `_SOURCE` provenance marker. + ruleTester.run('no-adhoc-regex-escape', noAdhocRegexEscape, { + valid: [], + invalid: [ + { + code: String.raw` + function buildLiteralMatcher(userSuppliedValue) { + return new RegExp(userSuppliedValue); + } + `, + filename: 'src/some-module.cts', + errors: [{ messageId: 'unsafeNewRegExp' }], + }, + ], + }); + }); + + // ── ReDoS regression — scripts/lint-no-adhoc-regex-escape.cjs's own regex ─ + + test('an adversarial [] run after .replace(/ terminates instead of backtracking exponentially (#3412)', () => { + // Pre-fix, REPLACE_CALL_RE's outer alternation let a `[...]` run be + // consumed either by the character-class branch or one char at a time + // by the catch-all branch, so a failing match explored both parses of + // every bracket pair: measured n=30 -> 3475ms (~2^n growth per +2). This + // adversarial input never closes the `.replace(/` call (no trailing + // `, '...')`), forcing the engine all the way through backtracking on a + // pre-fix regex. n=2000 is comfortably past where the pre-fix regex + // would already be unusable; a linear-time regex resolves it instantly. + const adversarial = '.replace(/' + '[]'.repeat(2000) + 'X'; + const result = findViolations(adversarial); + assert.deepEqual(result, []); + }); +}); diff --git a/tests/helpers/copy-script-fixture.cjs b/tests/helpers/copy-script-fixture.cjs new file mode 100644 index 000000000..5b0cdb8df --- /dev/null +++ b/tests/helpers/copy-script-fixture.cjs @@ -0,0 +1,265 @@ +'use strict'; + +/** + * Copy a repo script into a throwaway fixture tree ALONG WITH its transitive + * relative-require dependencies, preserving repo-relative layout. + * + * Several suites drive a `scripts/*.cjs` end-to-end by copying it into an + * mkdtemp fixture and spawning it there — necessary because those scripts + * resolve their scan root from `path.join(__dirname, '..')`, so running the + * REAL script would scan the real repo instead of the fixture (see + * tests/removed-but-needed-lint.test.cjs's copyScriptInto for the original + * statement of that constraint). + * + * Each such harness used to hand-list the script's dependencies + * (`fs.copyFileSync(... 'scripts/lib/cli-exit.cjs' ...)`). That made every new + * require in a covered script a silent, duplicated edit across N harnesses, + * and the failure mode was a MODULE_NOT_FOUND that only ever appeared in CI. + * #3412 collected the bill: adding one require of the new pattern seam to + * gen-adr-index.cjs broke 82 tests across two suites that had each hand-copied + * a now-incomplete dependency list. + * + * Walking the require graph instead means a script's dependencies are derived, + * never re-declared, so this class cannot recur. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const espree = require('espree'); + +/** Relative specifiers — the only kind that resolves inside the fixture tree. */ +const RELATIVE_SPEC_RE = /^\.{1,2}[\\/]/; + +/** + * Parse attempts tried in order by `extractRequires`. `globalReturn: true` is + * required on the `script` attempt because Node wraps every CommonJS module + * body in an implicit function, which makes a top-level `return` legal there + * even though it is not legal in a bare ECMAScript Program — `espree` without + * the flag rejects it with "'return' outside of function". A real shipped + * script relies on exactly this (`scripts/check-coverage-gate.cjs` has a + * top-level `return`), so dropping the flag silently breaks the #2858 + * packaging guard that scans it. Do not remove this without re-verifying + * every shipped `scripts/**`, `bin/**`, and `gsd-core/bin/**` .cjs/.js file + * still parses. + */ +const PARSE_ATTEMPTS = [ + { ecmaVersion: 'latest', sourceType: 'script', ecmaFeatures: { globalReturn: true } }, + { ecmaVersion: 'latest', sourceType: 'module' }, +]; + +/** + * Extract every static `require('...')` string-literal call from a CJS or ESM + * source, via a real AST parse rather than pattern-matching. Commented-out, + * string-embedded, and template-literal occurrences are correctly ignored; + * `foo.require('x')` (a method call, not a bare identifier callee) is + * correctly ignored; dynamic `require(variable)` remains out of scope — it + * cannot be resolved statically, and in a script that ships it would itself + * be a red flag (see tests/packaging-shipped-scripts-require-only-shipped. + * test.cjs, which consumes this same extractor so the two guards cannot + * disagree about what "a require" is). + * + * Currently a spurious hit (a require-shaped call this function reports that + * the source does not actually reach) is not merely harmless: an unresolvable + * specifier makes `copyScriptWithDeps` throw. With a real parser this is + * largely moot — there is no pattern-matching left to produce a false + * positive from comment/string/regex content. + * + * @param {string} source + * @returns {string[]} specifiers, in source order, duplicates included + */ +function extractRequires(source) { + let ast = null; + for (const options of PARSE_ATTEMPTS) { + try { + ast = espree.parse(source, options); + break; + } catch { + /* try next */ + } + } + if (ast === null) { + throw new Error('extractRequires: source did not parse as script or module'); + } + const found = []; + (function walk(node) { + if (node === null || typeof node !== 'object') return; + if (Array.isArray(node)) { + node.forEach(walk); + return; + } + if ( + node.type === 'CallExpression' && + node.callee && + node.callee.type === 'Identifier' && + node.callee.name === 'require' && + node.arguments.length === 1 && + node.arguments[0].type === 'Literal' && + typeof node.arguments[0].value === 'string' + ) { + found.push(node.arguments[0].value); + } + for (const key of Object.keys(node)) walk(node[key]); + })(ast); + return found; +} + +/** + * Resolve a relative require specifier to a real file, applying Node's CJS + * extension/index candidates. + * + * @param {string} fromAbsFile absolute path of the requiring file + * @param {string} spec the relative specifier + * @returns {string|null} absolute path of the resolved file, or null + */ +function resolveRelativeRequire(fromAbsFile, spec) { + const base = path.resolve(path.dirname(fromAbsFile), spec); + const candidates = [ + base, + `${base}.cjs`, + `${base}.js`, + `${base}.json`, + path.join(base, 'index.cjs'), + path.join(base, 'index.js'), + ]; + for (const candidate of candidates) { + if (fs.existsSync(candidate) && fs.statSync(candidate).isFile()) return candidate; + } + return null; +} + +/** + * True when `rel` (a `path.relative(base, target)` result) climbs outside + * `base` — i.e. `target` is not contained within `base`. Guards against the + * `'..foo'.startsWith('..')` false positive (a real sibling directory named + * `..foo` is NOT an escape) by requiring either an exact `..` or a + * `..`-prefixed relative path, using the platform separator since + * `path.relative` returns platform-native separators. + * + * @param {string} rel + * @returns {boolean} + */ +function escapesContainment(rel) { + return rel === '..' || rel.startsWith(`..${path.sep}`) || path.isAbsolute(rel); +} + +/** + * Copy `scriptRel` (repo-relative, e.g. `scripts/gen-adr-index.cjs`) and every + * file reachable from it through static relative requires into `fixtureRoot`, + * at the same repo-relative paths. Bare specifiers (`node:fs`, npm packages) + * are left alone — they resolve from the real installation. + * + * Both the entry script and every discovered dependency are validated against + * the repo boundary before being copied (symlinks resolved via + * `fs.realpathSync` first, since the containment check is otherwise lexical + * while `copyFileSync`/`readFileSync` follow links — a repo-committed symlink + * pointing outside the repo must not smuggle its target's contents in). The + * ORIGINAL repo-relative path (not the realpath-derived one) is preserved for + * the destination location, so the copied layout is unchanged; the + * realpath-derived repo-relative path is used as the dedupe key so a + * directory-symlink cycle cannot mint a new key per level. + * + * Throws when a relative require does not resolve on disk, or when the entry + * or a dependency resolves outside the repo. Unresolved-require failures are + * nearly always an unbuilt artifact (`gsd-core/bin/lib/*.cjs` requires + * `npm run build:lib`), and failing here names the cause instead of letting + * the spawned subprocess die with a bare MODULE_NOT_FOUND. + * + * @param {string} repoRoot absolute path to the repo root + * @param {string} fixtureRoot absolute path to the temp fixture root + * @param {string} scriptRel repo-relative path of the script to copy + * @returns {string} absolute path of the copied script inside `fixtureRoot` + */ +function copyScriptWithDeps(repoRoot, fixtureRoot, scriptRel) { + const toPosix = (p) => p.split(path.sep).join('/'); + const entry = toPosix(scriptRel); + const copied = new Set(); + const unresolved = []; + + let realRepoRoot; + try { + realRepoRoot = fs.realpathSync(repoRoot); + } catch (err) { + throw new Error(`copyScriptWithDeps: could not resolve repoRoot ${repoRoot}: ${err.message}`); + } + + /** + * @param {string} abs absolute path (not yet realpath-resolved) + * @param {string} label human-readable label for error messages + * @returns {{ dedupeKey: string } | null} null when unresolvable or escaping + */ + function checkContainment(abs, label) { + let real; + try { + real = fs.realpathSync(abs); + } catch (err) { + unresolved.push(`${label} could not be resolved on disk: ${err.message}`); + return null; + } + const dedupeKey = toPosix(path.relative(realRepoRoot, real)); + if (escapesContainment(dedupeKey)) { + unresolved.push(`${label} resolves outside the repo (${real}) — refusing to copy outside the fixture`); + return null; + } + return { dedupeKey }; + } + + /** @param {string} rel repo-relative posix path (ORIGINAL, pre-realpath) */ + function copyOne(rel) { + const abs = path.join(repoRoot, rel); + if (!fs.existsSync(abs)) { + if (!copied.has(rel)) { + copied.add(rel); + unresolved.push(`${rel} (does not exist in the repo)`); + } + return; + } + + const containment = checkContainment(abs, rel); + if (!containment) return; + if (copied.has(containment.dedupeKey)) return; + copied.add(containment.dedupeKey); + + const dest = path.join(fixtureRoot, rel); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.copyFileSync(abs, dest); + + for (const spec of extractRequires(fs.readFileSync(abs, 'utf-8'))) { + if (!RELATIVE_SPEC_RE.test(spec)) continue; + const depAbs = resolveRelativeRequire(abs, spec); + if (!depAbs) { + unresolved.push(`require('${spec}') from ${rel}`); + continue; + } + const depRel = toPosix(path.relative(repoRoot, depAbs)); + copyOne(depRel); + } + } + + // F2: the entry path bypasses the sandbox guard the same way a dependency + // could — validate it against the same containment rule before copying. + const entryAbs = path.join(repoRoot, entry); + if (!fs.existsSync(entryAbs)) { + throw new Error(`copyScriptWithDeps: entry script does not exist in the repo: ${entry}`); + } + const entryContainment = checkContainment(entryAbs, `entry script ${entry}`); + if (!entryContainment) { + throw new Error( + `copyScriptWithDeps: entry script ${entry} resolves outside the repo — refusing to copy outside the fixture`, + ); + } + + copyOne(entry); + + if (unresolved.length > 0) { + throw new Error( + `copyScriptWithDeps(${entry}) could not resolve ${unresolved.length} ` + + `relative require(s); the fixture would fail with MODULE_NOT_FOUND:\n` + + unresolved.map((u) => ` - ${u}`).join('\n') + + `\nIf these are compiled artifacts under gsd-core/bin/lib/, run \`npm run build:lib\`.`, + ); + } + + return path.join(fixtureRoot, scriptRel); +} + +module.exports = { extractRequires, resolveRelativeRequire, copyScriptWithDeps }; diff --git a/tests/helpers/install-shared.cjs b/tests/helpers/install-shared.cjs index 75cfbc211..299e7f548 100644 --- a/tests/helpers/install-shared.cjs +++ b/tests/helpers/install-shared.cjs @@ -24,6 +24,7 @@ const { runNode } = require('./process-seam.cjs'); const { resolveRuntimeArtifactLayout, } = require('../../gsd-core/bin/lib/runtime-artifact-layout.cjs'); +const { escapeRegex: escapeRegExp } = require('../../gsd-core/bin/lib/pattern.cjs'); const INSTALL_SCRIPT = path.join(__dirname, '..', '..', 'bin', 'install.js'); const MANIFEST_NAME = 'gsd-file-manifest.json'; @@ -227,9 +228,6 @@ function stripAnsi(str) { // A version string can itself contain regex metacharacters (`.`, and — via // prerelease/build metadata — `-`/`+`), so it must be escaped before being spliced // into a RegExp source, or e.g. the `.` in "1.9.0" would match ANY character. -function escapeRegExp(str) { - return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} // Loosely semver-shaped: leading `MAJOR.MINOR.PATCH`, optional `-prerelease` and/or // `+build` metadata (e.g. `1.9.0`, `1.9.0-rc.1`, `1.9.0+abc`). Deliberately loose diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index 96ba6a6e7..21e81efb2 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -42,6 +42,7 @@ const { } = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs'); const { applySurface } = require('../gsd-core/bin/lib/surface.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const { loadSkillsManifest, @@ -433,7 +434,7 @@ describe('installOpencodeFamilySkills — emits skills//SKILL.md (#784)', ); // Regression guard for the prefix-overlap double-rewrite (e.g. kilo-alt-alt). assert.ok( - !new RegExp(`${defaultBase.replace(/[\\.*+?^${}()|[\]]/g, '\\$&')}-[^/\\s]*-`).test(body), + !new RegExp(`${escapeRegex(defaultBase)}-[^/\\s]*-`).test(body), `${skillName}: must not contain a doubled config-dir suffix`, ); } diff --git a/tests/no-unbounded-spawn-allowlist.test.cjs b/tests/no-unbounded-spawn-allowlist.test.cjs index 78f14aae1..61023d872 100644 --- a/tests/no-unbounded-spawn-allowlist.test.cjs +++ b/tests/no-unbounded-spawn-allowlist.test.cjs @@ -49,7 +49,7 @@ const REPO_ROOT = path.join(__dirname, '..'); * Recursively list every `.cjs` file under `tests/`, including subdirectories * (`tests/helpers/`, `tests/qa/`, `tests/observability/`, `tests/fixtures/`, * `tests/dispatch/`, etc.). `fs.readdirSync(dir, { recursive: true, - * withFileTypes: true })` is available on the repo's Node floor (>=22.0.0 per + * withFileTypes: true })` is available on the repo's Node floor (>=24.0.0 per * package.json `engines`; the option landed in Node 20.1). Each returned * `Dirent` carries `parentPath` — its containing directory, which for a * nested entry is the subdirectory, not `TESTS_DIR` — so the joined path is diff --git a/tests/packaging-shipped-scripts-require-only-shipped.test.cjs b/tests/packaging-shipped-scripts-require-only-shipped.test.cjs index 774a3d4df..fa329a6ad 100644 --- a/tests/packaging-shipped-scripts-require-only-shipped.test.cjs +++ b/tests/packaging-shipped-scripts-require-only-shipped.test.cjs @@ -21,6 +21,8 @@ const { execFileSync } = require('node:child_process'); const REPO_ROOT = path.join(__dirname, '..'); +const { extractRequires } = require('./helpers/copy-script-fixture.cjs'); + /** * Resolve the tarball file list via `npm pack --dry-run --json`. * This is the ACTUAL set of files that ship — not a hardcoded list — so adding @@ -47,27 +49,6 @@ function resolveTarballFiles() { return new Set(parsed[0].files.map((f) => f.path.replace(/\\/g, '/'))); } -/** - * Extract all require('...') string-literal calls from a .cjs source. - * Only static string-literal requires are checked — dynamic require(variable) - * is out of scope (and would itself be a red flag in a shipped script). - */ -function extractRequires(source) { - const requires = []; - // Strip block comments (/* ... */) and inline line comments (// ...) before - // matching, so a require() appearing in a doc comment or fenced code block - // inside a /* */ does not produce a false positive. - const stripped = source - .replace(/\/\*[\s\S]*?\*\//g, '') - .replace(/\/\/.*$/gm, ''); - const requireRe = /\brequire\s*\(\s*['"]([^'"]+)['"]\s*\)/g; - let m; - while ((m = requireRe.exec(stripped)) !== null) { - requires.push(m[1]); - } - return requires; -} - /** * Classify a require specifier from a given file path: * - 'builtin' — node: prefix or a Node builtin (fs, path, etc.) @@ -198,6 +179,15 @@ describe('#2858 — shipped scripts require only shipped paths', () => { ); }); + test('lint-no-adhoc-regex-escape.cjs does NOT ship (repo-only CI tooling)', () => { + // This script requires ../eslint-rules/ which does not ship. It is + // repo-only CI tooling. It must be excluded from the npm tarball (#3412). + assert.ok( + !shippedFiles.has('scripts/lint-no-adhoc-regex-escape.cjs'), + 'scripts/lint-no-adhoc-regex-escape.cjs must NOT ship — it requires ../eslint-rules/ which does not ship (#3412)', + ); + }); + test('a script requiring a sibling in the same shipped dir resolves as shipped (positive case)', () => { // Sanity: scripts/lib/cli-exit.cjs ships and is required by shipped scripts. // This confirms the guard's positive path works — a valid intra-shipped require diff --git a/tests/pattern.test.cjs b/tests/pattern.test.cjs new file mode 100644 index 000000000..ccdbd6852 --- /dev/null +++ b/tests/pattern.test.cjs @@ -0,0 +1,204 @@ +'use strict'; + +/** + * Tests for `src/pattern.cts` — the pattern-construction seam (#3212 Phase 1, #3412). + * + * Design: .gsd/phase/chore-3412-pattern-seam/40-design.md + * Test matrix: .gsd/phase/chore-3412-pattern-seam/50-test-matrix.md + * ADR: docs/adr/3212-lexical-seam-consolidation.md §1, §2, §7 + * + * TDD RED: `src/pattern.cts` does not exist yet — this file's + * `require('../gsd-core/bin/lib/pattern.cjs')` throws MODULE_NOT_FOUND until + * the implementing phase adds it. That is the intended starting state (mirrors + * tests/planning-snapshot.test.cjs's RED convention). + * + * Covers test-matrix sections 1 (escapeRegex, rows 1-10), 2 (literalPattern, + * rows 11-14), and 3 (migration equivalence, rows 15-17). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +// Seeded fast-check convention: require the shared setup helper (NOT +// 'fast-check' directly) so numRuns/seed are configured globally before any +// fc.assert() call — mirrors tests/frontmatter.property.test.cjs and every +// other *.property.test.cjs file. seed: 42, overridable via GSD_FC_SEED. +const fc = require('./helpers/fast-check-setup.cjs'); + +const { escapeRegex, literalPattern } = require('../gsd-core/bin/lib/pattern.cjs'); + +// ─── Section 1: escapeRegex — rows 1-10 ─────────────────────────────────── + +describe('escapeRegex', () => { + test('row 1: "" returns ""', () => { + assert.strictEqual(escapeRegex(''), ''); + }); + + test('row 2: ordinary chars only ("_x") are unchanged', () => { + assert.strictEqual(escapeRegex('_x'), '_x'); + }); + + test('row 3: each metacharacter is escaped and the result matches its own literal form', () => { + const metachars = ['.', '*', '+', '?', '^', '$', '{', '}', '(', ')', '|', '[', ']', '\\']; + for (const ch of metachars) { + const escaped = escapeRegex(ch); + const re = new RegExp(escaped); + assert.ok(re.test(ch), `escapeRegex(${JSON.stringify(ch)}) -> ${JSON.stringify(escaped)} must match its own literal char`); + } + }); + + test('row 4: leading ASCII letter locks the MEASURED hex-escape ("abc" -> "\\x61bc", Node v26.5.1)', () => { + assert.strictEqual(escapeRegex('abc'), '\\x61bc'); + }); + + test('row 5: leading digit locks the MEASURED hex-escape ("5abc" -> "\\x35abc", Node v26.5.1)', () => { + assert.strictEqual(escapeRegex('5abc'), '\\x35abc'); + }); + + test('row 6: hyphen anywhere locks the MEASURED hex-escape ("a-b" -> "\\x61\\x2db", Node v26.5.1)', () => { + assert.strictEqual(escapeRegex('a-b'), '\\x61\\x2db'); + }); + + test('row 7: whitespace / newline / control chars are escaped and the result still constructs', () => { + const values = [' ', '\n', '\t', '\r', 'a\nb', 'a\tb\rc']; + for (const v of values) { + const escaped = escapeRegex(v); + assert.doesNotThrow(() => new RegExp(escaped)); + assert.ok(new RegExp(escaped).test(v)); + } + }); + + // #3212 Spec-axis review correction: this throw behavior matches MOST of the + // deleted copies' `.replace` throw, but NOT ALL of them — `src/phase-id.cts`'s + // original `escapeRegex(value: unknown)` did `String(value).replace(...)` and + // never threw (`escapeRegex(42) === '42'`, `escapeRegex(null) === 'null'`, per + // the deleted `tests/phase-id.test.cjs` assertions). That is a deliberate, + // disclosed behavior change for phase-id's former callers: the seam's locked + // signature (`escapeRegex(value: string): string`, ADR §1) does not coerce, so + // a non-string reaching it now throws instead of silently stringifying. + // Mitigated by an explicit `String(...)` wrap at the one former phase-id call + // site that genuinely needed it (phaseMarkdownRegexSource's fallback branch, + // src/phase-id.cts) — the seam's throw itself is intentional and stays. + test('row 8: non-string input throws TypeError (matches 11 of the 12 deleted copies\' .replace throw — phase-id.cts\'s copy coerced instead, see comment above)', () => { + for (const bad of [null, undefined, 42, {}, [], true]) { + assert.throws(() => escapeRegex(bad), TypeError, `escapeRegex(${JSON.stringify(bad)}) must throw TypeError`); + } + }); + + test('row 9: a 10,000-char value does not throw and completes without catastrophic time', () => { + const big = 'a-b.c*d'.repeat(1429); // ~10,001 chars + let escaped; + assert.doesNotThrow(() => { + escaped = escapeRegex(big); + }); + assert.doesNotThrow(() => new RegExp(escaped)); + }); + + test('row 10: RegExp.escape availability — the ADR §2 floor-vs-capability assertion', () => { + assert.strictEqual(typeof RegExp.escape, 'function'); + }); +}); + +// ─── Section 2: literalPattern — rows 11-14 ─────────────────────────────── + +describe('literalPattern', () => { + test('row 11: value + no flags produces a RegExp matching the value literally', () => { + const re = literalPattern('abc'); + assert.ok(re instanceof RegExp); + assert.ok(re.test('abc')); + }); + + test('row 12: value + valid flags ("gi") applies the flags', () => { + const re = literalPattern('abc', 'gi'); + assert.strictEqual(re.flags, 'gi'); + assert.ok(re.test('ABC')); + }); + + test('row 13: invalid flags ("qq") throws SyntaxError from RegExp', () => { + assert.throws(() => literalPattern('abc', 'qq'), SyntaxError); + }); + + test('row 14: metacharacter-heavy value matches literally, not by its metacharacter interpretation', () => { + const re = literalPattern('a.b*c'); + assert.ok(re.test('a.b*c'), 'must match the literal string'); + // If '.' and '*' were left live, this would ALSO match (any-char + zero-or-more) + assert.ok(!re.test('aXbYYYc'), 'must NOT match the metacharacter interpretation'); + }); +}); + +// ─── Section 3: migration equivalence — rows 15-17 (load-bearing) ───────── + +describe('migration equivalence (row-9 sweep)', () => { + test('row 15: property — hand oracle and escapeRegex are match-equivalent for all (s, probe) pairs (seeded)', () => { + // `hand` is the historical oracle: the deleted pre-migration implementation, + // byte-identical across all twelve copies before this seam existed (design + // doc "Ground truth" #1; ADR §7 Class 1 census). It is inlined here ONLY as + // a test oracle for this equivalence sweep — this is NOT a 13th production + // copy of the escape helper (design doc Notes: "not a 13th production copy"). + // eslint-disable-next-line local/no-adhoc-regex-escape -- historical oracle for the row-15 equivalence sweep, not a production copy (#3412) + const hand = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + + fc.assert( + fc.property( + fc.string({ maxLength: 200 }), + fc.string({ maxLength: 50 }), + (s, probe) => { + const handResult = new RegExp(hand(s)).test(probe); + const seamResult = new RegExp(escapeRegex(s)).test(probe); + assert.strictEqual( + seamResult, + handResult, + `match divergence for s=${JSON.stringify(s)} probe=${JSON.stringify(probe)}` + ); + } + ) + ); + }); + + test('row 16: the real corpus — phase tokens, milestone versions, STATE field names, runtime ids are match-equivalent', () => { + // eslint-disable-next-line local/no-adhoc-regex-escape -- historical oracle for the row-16 equivalence sweep, not a production copy (#3412) + const hand = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const corpus = [ + '1A', + 'PROJ-05', + '999.1', + 'v1.2', + '**Current Phase:**', + 'claude-code', + 'gemini-cli', + ]; + const probes = ['1A', 'PROJ-05', '999.1', 'v1.2', '**Current Phase:**', 'claude-code', 'gemini-cli', 'no-match-here']; + for (const s of corpus) { + for (const probe of probes) { + const handResult = new RegExp(hand(s)).test(probe); + const seamResult = new RegExp(escapeRegex(s)).test(probe); + assert.strictEqual( + seamResult, + handResult, + `corpus divergence for s=${JSON.stringify(s)} probe=${JSON.stringify(probe)}` + ); + } + } + }); + + test('row 17: latent-bug fix — a hyphen interpolated into a character class no longer forms a range', () => { + // Pre-migration (hand-rolled, verified): new RegExp('[' + hand('a-z') + ']').test('m') === true + // — the unescaped '-' formed an unintended a-through-z RANGE that happened + // to match 'm'. This is design row 10's confirmed latent bug. + // eslint-disable-next-line local/no-adhoc-regex-escape -- historical oracle for the row-17 latent-bug check, not a production copy (#3412) + const hand = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + assert.strictEqual( + new RegExp('[' + hand('a-z') + ']').test('m'), + true, + 'sanity check: pre-migration hand-rolled behavior was true (verified)' + ); + + // Post-migration (the seam): the fix. 'a', '-', 'z' are all escaped as + // discrete literal characters, so '[...]' no longer forms a range and 'm' + // (interior to the would-be range) must NOT match. + assert.strictEqual( + new RegExp('[' + escapeRegex('a-z') + ']').test('m'), + false, + 'deliberate fix, not an accident: the seam must not let a value form a character-class range' + ); + }); +}); diff --git a/tests/phase-id-drift-guard.test.cjs b/tests/phase-id-drift-guard.test.cjs index 960483dac..149cbf39b 100644 --- a/tests/phase-id-drift-guard.test.cjs +++ b/tests/phase-id-drift-guard.test.cjs @@ -32,8 +32,15 @@ const phaseId = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'phase-id.cjs' // The locked canonical surface (ADR-2121 Decision 1/2; PHASE_NUMBER_TOKEN_SOURCE // added in Phase 4). Every name is exported by phase-id.cjs; the identity guard // forbids any other module from re-exporting a divergent copy of one. +// #3212 Phase 1: `escapeRegex` moved off this locked surface entirely — it is +// no longer owned (or re-exported) by phase-id.cjs, it is owned by the +// pattern-construction seam (src/pattern.cts / gsd-core/bin/lib/pattern.cjs). +// Dropped from CANONICAL rather than kept: phase-id.cjs no longer exports the +// name at all, so `name in phaseId` below would fail if it stayed listed, and +// the identity-guard test's job (no consumer re-exports a DIVERGENT copy) is +// now the pattern seam's own single-owner property, not phase-id's. const CANONICAL = [ - 'escapeRegex', 'OPTIONAL_PROJECT_CODE_PREFIX_SOURCE', 'OPTIONAL_PHASE_TAG_SOURCE', + 'OPTIONAL_PROJECT_CODE_PREFIX_SOURCE', 'OPTIONAL_PHASE_TAG_SOURCE', 'PHASE_NUMBER_TOKEN_SOURCE', 'stripProjectCodePrefix', 'normalizePhaseName', 'getMilestoneFromPhaseId', 'getPhaseDirFromPhaseId', 'phaseMarkdownRegexSource', 'phaseMarkdownRegexSourceExact', 'comparePhaseNum', 'extractPhaseToken', diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs index 07d53025a..273cc7843 100644 --- a/tests/phase-id.test.cjs +++ b/tests/phase-id.test.cjs @@ -2,7 +2,6 @@ * Tests for src/phase-id.cts (compiled to gsd-core/bin/lib/phase-id.cjs). * * Verifies behavioural contracts of the extracted pure phase-id helpers: - * - escapeRegex * - normalizePhaseName * - comparePhaseNum * - extractPhaseToken @@ -22,53 +21,27 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); +const { buildPhaseHeadingRegex: headingRegex } = require('../gsd-core/bin/lib/roadmap.cjs'); const fc = require('fast-check'); -// ─── escapeRegex ───────────────────────────────────────────────────────────── +// escapeRegex moved off phase-id.cjs entirely in #3212 Phase 1 (#3412): it is +// now owned by the pattern-construction seam (src/pattern.cts, tests in +// tests/pattern.test.cjs) and phase-id.cjs no longer exports it — see +// tests/phase-id-drift-guard.test.cjs's CANONICAL list, which drops it for +// the same reason. -describe('escapeRegex', () => { - test('escapes all regex special characters', () => { - assert.strictEqual(phaseId.escapeRegex('.'), '\\.'); - assert.strictEqual(phaseId.escapeRegex('*'), '\\*'); - assert.strictEqual(phaseId.escapeRegex('+'), '\\+'); - assert.strictEqual(phaseId.escapeRegex('?'), '\\?'); - assert.strictEqual(phaseId.escapeRegex('^'), '\\^'); - assert.strictEqual(phaseId.escapeRegex('$'), '\\$'); - assert.strictEqual(phaseId.escapeRegex('{'), '\\{'); - assert.strictEqual(phaseId.escapeRegex('}'), '\\}'); - assert.strictEqual(phaseId.escapeRegex('('), '\\('); - assert.strictEqual(phaseId.escapeRegex(')'), '\\)'); - assert.strictEqual(phaseId.escapeRegex('|'), '\\|'); - assert.strictEqual(phaseId.escapeRegex('['), '\\['); - assert.strictEqual(phaseId.escapeRegex(']'), '\\]'); - assert.strictEqual(phaseId.escapeRegex('\\'), '\\\\'); - }); - - test('leaves alphanumeric and hyphen characters unescaped', () => { - assert.strictEqual(phaseId.escapeRegex('abc'), 'abc'); - assert.strictEqual(phaseId.escapeRegex('01-02'), '01-02'); - assert.strictEqual(phaseId.escapeRegex('v1.0'), 'v1\\.0'); - }); - - test('coerces non-string values via String()', () => { - assert.strictEqual(phaseId.escapeRegex(42), '42'); - assert.strictEqual(phaseId.escapeRegex(null), 'null'); - assert.strictEqual(phaseId.escapeRegex(undefined), 'undefined'); - }); - - test('adversarial: path-traversal-like inputs are treated as literals', () => { - const result = phaseId.escapeRegex('../../../etc/passwd'); - // The dots get escaped; slashes and alphanumeric pass through unchanged - assert.strictEqual(result, '\\.\\./\\.\\./\\.\\./etc/passwd'); - // The result forms a valid regex (no throws) - assert.doesNotThrow(() => new RegExp(result)); - }); - - test('unicode passthrough', () => { - assert.strictEqual(phaseId.escapeRegex('Phase Name'), 'Phase Name'); - assert.strictEqual(phaseId.escapeRegex('中文'), '中文'); - }); -}); +// ─── #3412 shared test helper ───────────────────────────────────────────────── +// +// phase-id.cts now delegates regex escaping to RegExp.escape via src/pattern.cts. +// RegExp.escape is MATCH-equivalent to the retired hand-rolled escaper but not +// TEXT-equivalent (it hex-escapes the first character and all hyphens), so any +// test that pinned the literal source text (e.g. `'0*29'`, `'PROJ-42'`) is +// brittle to that internal encoding, not to actual behavior. `headingRegex` +// (imported above as `buildPhaseHeadingRegex` from gsd-core/bin/lib/roadmap.cjs) +// is the SAME production function src/roadmap.cts's searchPhaseInContent uses to +// build its heading regex — not a hand-duplicated copy — so tests assert what +// matches and what doesn't — the real contract — rather than the escaper's +// spelling, with no parity gap against the production pattern. // ─── normalizePhaseName ─────────────────────────────────────────────────────── @@ -387,20 +360,50 @@ describe('phaseMarkdownRegexSource', () => { const re = new RegExp(src); assert.ok(!re.test('3X1'), 'unescaped dot would match any char — must be escaped'); }); + + test('regex metacharacters in a phase id are neutralized, not interpreted (#3412)', () => { + // The property RegExp.escape exists for: a literal metacharacter in the + // phase id (the dot in "1.2") must not behave as a regex wildcard once the + // source is compiled into the same heading regex production uses. + const src = phaseId.phaseMarkdownRegexSource('1.2'); + const re = headingRegex(src); + assert.ok(re.test('Phase 1.2: Title')); + assert.ok(!re.test('Phase 1X2: Title')); + }); }); // ─── phaseMarkdownRegexSourceExact ──────────────────────────────────────────── describe('phaseMarkdownRegexSourceExact', () => { test('returns escaped form for project-code-prefixed IDs', () => { - const result = phaseId.phaseMarkdownRegexSourceExact('PROJ-42'); - // hyphen is not a regex special char so it passes through unescaped - assert.strictEqual(result, 'PROJ-42'); - // The result is a valid regex source - assert.doesNotThrow(() => new RegExp(result)); - assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('MANIFOLD-117'), 'MANIFOLD-117'); - assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('APP1-117'), 'APP1-117'); - assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('APP_1-117'), 'APP_1-117'); + // Source text is RegExp.escape's business (#3412) — the actual contract + // is a non-null, compilable source that matches its own prefixed heading + // and rejects the bare-numeric heading. + for (const [id, bareHeadingNum, foreignPrefix] of [ + ['PROJ-42', '42', 'OTHER'], + ['AB-29', '29', 'CK'], + ['MANIFOLD-117', '117', 'OTHER'], + ['APP1-117', '117', 'OTHER'], + ['APP_1-117', '117', 'OTHER'], + ]) { + const result = phaseId.phaseMarkdownRegexSourceExact(id); + assert.ok(result !== null, id); + assert.doesNotThrow(() => new RegExp(result)); + const re = headingRegex(result); + assert.ok(re.test(`Phase ${id}: Title`), `${id} must match its own heading`); + assert.ok(!re.test(`Phase ${bareHeadingNum}: Title`), `${id} must not match the bare-numeric heading`); + // #3599 regression: the exact source must be tied to the FULL prefixed + // id, not just its trailing number — a different prefix with the same + // number must not match (an impl returning `[A-Z]+-42` would pass every + // assertion above but fail this one). + assert.ok( + !re.test(`Phase ${foreignPrefix}-${bareHeadingNum}: Title`), + `${id} must not match a foreign-prefixed heading with the same number`, + ); + // Case-insensitivity (the 'i' flag) — canonicalizes the same way a + // literal would, despite the hex escape (#3412). + assert.ok(re.test(`phase ${id.toLowerCase()}: title`)); + } }); test('returns null for non-prefixed IDs', () => { @@ -657,24 +660,53 @@ describe('isForeignPrefixedPhaseQuery', () => { // ─── roadmapPhaseLookupSources (#2121, owned here after the move) ───────────── describe('roadmapPhaseLookupSources', () => { - const PREFIX_TOLERANT = `${phaseId.OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}0*29`; - test('a bare numeric query yields the numeric then prefix-tolerant sources', () => { const sources = phaseId.roadmapPhaseLookupSources('29'); - assert.deepEqual(sources, ['0*29', PREFIX_TOLERANT]); + assert.equal(sources.length, 2); + // Source text is RegExp.escape's business (#3412) — pin matching + // behavior instead: source[0] matches the bare and zero-padded heading + // but not a project-code-prefixed one; source[1] additionally tolerates + // the prefix. + const re0 = headingRegex(sources[0]); + const re1 = headingRegex(sources[1]); + assert.ok(re0.test('Phase 29: Title')); + assert.ok(re0.test('Phase 029: Title')); + assert.ok(!re0.test('Phase CK-29: Title')); + assert.ok(re1.test('Phase CK-29: Title')); + assert.ok(re1.test('Phase 29: Title')); }); test('the bare numeric source precedes the prefix-tolerant fallback', () => { const sources = phaseId.roadmapPhaseLookupSources('29'); - assert.ok(sources.indexOf('0*29') < sources.indexOf(PREFIX_TOLERANT)); + // Behavioral ordering (#3412): the source that rejects a project-code + // prefix must appear before the source that accepts one. + const bareIdx = sources.findIndex((s) => !headingRegex(s).test('Phase CK-29: Title')); + const prefixTolerantIdx = sources.findIndex((s) => headingRegex(s).test('Phase CK-29: Title')); + assert.notEqual(bareIdx, -1); + assert.notEqual(prefixTolerantIdx, -1); + assert.ok(bareIdx < prefixTolerantIdx); }); test('a project-code-prefixed query adds the exact source first (3 sources)', () => { const sources = phaseId.roadmapPhaseLookupSources('AB-29'); assert.equal(sources.length, 3); - assert.equal(sources[0], 'AB-29'); - assert.ok(sources.includes('0*29')); - assert.ok(sources.includes(PREFIX_TOLERANT)); + // source[0] is the EXACT source (#3599): matches its own prefixed + // heading and must NOT match the bare numeric heading — that ordering + // is the whole point of #3599. Source text itself is RegExp.escape's + // business (#3412). + const reExact = headingRegex(sources[0]); + assert.ok(reExact.test('Phase AB-29: Title')); + assert.ok(!reExact.test('Phase 29: Title')); + // #3599 regression: the exact source is tied to the FULL prefix, not just + // the trailing number — a foreign prefix with the same number must not match. + assert.ok(!reExact.test('Phase CK-29: Title')); + // Case-insensitivity (the 'i' flag) — canonicalizes the same way a + // literal would, despite the hex escape (#3412). + assert.ok(reExact.test('phase ab-29: title')); + // The remaining two sources behave as the numeric/prefix-tolerant pair. + const rest = sources.slice(1).map(headingRegex); + assert.ok(rest.some((re) => re.test('Phase 29: Title') && !re.test('Phase CK-29: Title'))); + assert.ok(rest.some((re) => re.test('Phase CK-29: Title'))); }); test('zero-padding is tolerated: 029 resolves the same sources as 29', () => { diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 01f4d62cd..c92c05d41 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -8558,6 +8558,7 @@ const { cleanup, runGsdTools } = require('./helpers.cjs'); // phase-command-router.cjs delegates to SDK when available; we must test the // CJS implementation directly since that is where the bug lives. const phaseModule = require('../gsd-core/bin/lib/phase.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const { cmdPhaseComplete } = phaseModule; function writePassedVerificationFile(phaseDir, phase = '01') { @@ -8698,7 +8699,7 @@ function roadmapCompletionSnapshot(roadmapContent) { } function extractField(stateContent, fieldName) { - const escaped = fieldName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escaped = escapeRegex(fieldName); const boldMatch = stateContent.match(new RegExp(`\\*\\*${escaped}:\\*\\*[ \\t]*(.+)`, 'i')); if (boldMatch) return boldMatch[1].trim(); const plainMatch = stateContent.match(new RegExp(`^${escaped}:[ \\t]*(.+)`, 'im')); diff --git a/tests/phase6-capstone-conformance.test.cjs b/tests/phase6-capstone-conformance.test.cjs index 32d3cd471..08af76cf4 100644 --- a/tests/phase6-capstone-conformance.test.cjs +++ b/tests/phase6-capstone-conformance.test.cjs @@ -21,15 +21,12 @@ const CORE_SUBSTRATE_TERMS = [ const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); const { isCentralConfigKey } = require('../gsd-core/bin/lib/config-schema.cjs'); +const { escapeRegex: escapeRegExp } = require('../gsd-core/bin/lib/pattern.cjs'); function readRepoFile(relativePath) { return fs.readFileSync(path.join(ROOT, relativePath), 'utf8'); } -function escapeRegExp(value) { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - function activeWhenKeys() { const keys = new Set(); for (const cap of Object.values(registry.capabilities)) { diff --git a/tests/product-name-purity.test.cjs b/tests/product-name-purity.test.cjs index d834d1541..4ed6c5963 100644 --- a/tests/product-name-purity.test.cjs +++ b/tests/product-name-purity.test.cjs @@ -15,6 +15,7 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const ROOT = path.join(__dirname, '..'); @@ -48,7 +49,7 @@ function findProductParentheticals(content) { for (const product of PRODUCTS) { // Match "ProductName (something)" but not "ProductName (v1.2.3)" (version refs are ok) const pattern = new RegExp( - product.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + + escapeRegex(product) + '\\s*\\([^)]*(?!v?\\d+\\.\\d)[^)]*\\)', 'g' ); diff --git a/tests/removed-but-needed-lint.test.cjs b/tests/removed-but-needed-lint.test.cjs index 04e74ae8d..932df7323 100644 --- a/tests/removed-but-needed-lint.test.cjs +++ b/tests/removed-but-needed-lint.test.cjs @@ -23,6 +23,7 @@ const { referencesBasename, referencesNpmLockfileDependency, findSurvivingRefere const { cleanup } = require('./helpers.cjs'); const { runNode } = require('./helpers/process-seam.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { copyScriptWithDeps } = require('./helpers/copy-script-fixture.cjs'); describe('removed-but-needed lint: referencesBasename (pure)', () => { test('a plain filename reference in prose is found', () => { @@ -158,13 +159,7 @@ function buildTempRepo(tmpDir, baseFiles, prFiles) { * @returns {string} path to the copied script inside tmpDir/scripts/ */ function copyScriptInto(tmpDir) { - const scriptsDir = path.join(tmpDir, 'scripts'); - const libDir = path.join(scriptsDir, 'lib'); - fs.mkdirSync(libDir, { recursive: true }); - const scriptCopy = path.join(scriptsDir, 'lint-removed-but-needed.cjs'); - fs.copyFileSync(LINT_SCRIPT, scriptCopy); - fs.copyFileSync(path.join(ROOT, 'scripts', 'lib', 'cli-exit.cjs'), path.join(libDir, 'cli-exit.cjs')); - return scriptCopy; + return copyScriptWithDeps(ROOT, tmpDir, path.relative(ROOT, LINT_SCRIPT)); } describe('removed-but-needed lint: main() end-to-end wiring', () => { diff --git a/tests/reversibility-tagging.test.cjs b/tests/reversibility-tagging.test.cjs index f4e6ddb4b..1245248d0 100644 --- a/tests/reversibility-tagging.test.cjs +++ b/tests/reversibility-tagging.test.cjs @@ -13,6 +13,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const ROOT = path.resolve(__dirname, '..'); const PLANNER = path.join(ROOT, 'agents', 'gsd-planner.md'); @@ -47,7 +48,7 @@ function namesRating(text, rating) { // js/incomplete-sanitization (CodeQL, high). `-` needs no escaping outside a // character class, so the previous `-`-only replace was both incomplete and // unnecessary. - const escaped = rating.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escaped = escapeRegex(rating); return new RegExp(`\\b${escaped}\\b`).test(text); } diff --git a/tests/runtime-launcher-parity.test.cjs b/tests/runtime-launcher-parity.test.cjs index 45ff4bc89..cbbb5cc50 100644 --- a/tests/runtime-launcher-parity.test.cjs +++ b/tests/runtime-launcher-parity.test.cjs @@ -32,6 +32,7 @@ const os = require('node:os'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); @@ -87,7 +88,7 @@ function extractShellBlocks(content) { blockLang = (fenceOpen[2] || '').toLowerCase(); blockLines = []; // Closing pattern: same indent prefix + ``` - closingPattern = new RegExp('^' + blockIndent.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '```\\s*$'); + closingPattern = new RegExp('^' + escapeRegex(blockIndent) + '```\\s*$'); continue; } } else { @@ -1027,6 +1028,7 @@ const os = require('node:os'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); const { throwIfFailed } = require('./helpers/git-fixture.cjs'); const { cleanup } = require('./helpers.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); @@ -1104,7 +1106,7 @@ function extractShellBlocks(content) { blockIndent = fenceOpen[1]; blockLang = (fenceOpen[2] || '').toLowerCase(); blockLines = []; - closingPattern = new RegExp('^' + blockIndent.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '```\\s*$'); + closingPattern = new RegExp('^' + escapeRegex(blockIndent) + '```\\s*$'); continue; } } else { diff --git a/tests/workstream-name-policy.test.cjs b/tests/workstream-name-policy.test.cjs index a0273c5f3..46429fe26 100644 --- a/tests/workstream-name-policy.test.cjs +++ b/tests/workstream-name-policy.test.cjs @@ -8,6 +8,7 @@ const { isValidActiveWorkstreamName, INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE, } = require('../gsd-core/bin/lib/workstream-name-policy.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); describe('workstream-name-policy', () => { test('normalizeWorkstreamNameInput trims and nulls empty input', () => { @@ -39,7 +40,7 @@ describe('workstream-name-policy', () => { assert.equal(assertValidActiveWorkstreamName(' alpha '), 'alpha'); assert.throws( () => assertValidActiveWorkstreamName('alpha/beta'), - new RegExp(INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + new RegExp(escapeRegex(INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE)) ); }); diff --git a/tests/worktree-safety.test.cjs b/tests/worktree-safety.test.cjs index c3076bf66..e6ba62e0d 100644 --- a/tests/worktree-safety.test.cjs +++ b/tests/worktree-safety.test.cjs @@ -23,6 +23,7 @@ const fc = require('fast-check'); const { createTempDir, cleanup } = require('./helpers.cjs'); const { createFixture } = require('./fixtures/index.cjs'); const { makeFaultyGit } = require('./helpers/faulty-deps.cjs'); +const { escapeRegex } = require('../gsd-core/bin/lib/pattern.cjs'); // 30000ms: this file's single named bound for every migrated subprocess call // below (git plumbing on small mkdtemp fixtures, gsd-tools.cjs/hook CLI runs, @@ -974,7 +975,7 @@ describe('planWorktreeRecordAgent', () => { }); assert.equal(plan.reason, 'missing_field'); for (const flag of ['--agent-id', '--path', '--branch', '--base']) { - assert.match(plan.hint, new RegExp(flag.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))); + assert.match(plan.hint, new RegExp(escapeRegex(flag))); } }); diff --git a/tsconfig.build.json b/tsconfig.build.json index 51f927db2..75a0590cb 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -6,7 +6,7 @@ "module": "nodenext", "moduleResolution": "nodenext", "target": "ES2022", - "lib": ["ES2022"], + "lib": ["ES2022", "ES2025.RegExp"], "types": ["node"], "strict": true, "declaration": false,