From 03b467f2e79a952f859aad6160095a31b3aa9b33 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 10:24:11 -0400 Subject: [PATCH 1/9] =?UTF-8?q?docs(#857):=20ADR-857=20Capability=20system?= =?UTF-8?q?=20=E2=80=94=20five-step=20loop=20core,=20features=20as=20plug-?= =?UTF-8?q?ins=20(#858)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the final-state architecture: the five-step loop (Discuss → Plan → Execute → Verify → Ship) plus shared-infrastructure skills are the privileged host/core; every other feature is a Capability (plug-in) selectable at install and toggleable after restart, attaching through ~12 Loop Extension Points. Adds docs/adr/857-capability-system.md (Status: Proposed) capturing the eight resolved design decisions, the resolved design details (extension points, contribution merge, declaration shape, runtime descriptor, deferred trust gate), alternatives, consequences, and a six-phase rollout. Records the new domain terms in CONTEXT.md: Capability, Capability Registry, Loop Extension Point, Runtime Capability. Runtime/CLI support is itself a Capability (role: runtime) — Claude Code, Codex, Antigravity tier-1; the seam is declarative-over-primitives so third-party CLI support lands later as an additive loader, no rework. Closes #857 Co-authored-by: Claude Opus 4.8 --- CONTEXT.md | 12 +++ docs/adr/857-capability-system.md | 127 ++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 docs/adr/857-capability-system.md diff --git a/CONTEXT.md b/CONTEXT.md index 871b64625..a5ebdd85f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -121,6 +121,18 @@ Module owning the per-runtime mapping from artifact kind to filesystem placement ### Runtime Install Policy Module Projects a pure, typed install plan for a given runtime by composing artifact placements (Runtime Artifact Layout Module), command text (Shell Command Projection Module), and per-runtime config intentions — with no filesystem IO or format-specific serialization. Runtime-specific adapters consume the plan and execute concrete file mutations and config rendering. See ADR-58. +### Capability [Planned] +A bundle delivering one optional GSD feature, toggled as a unit at install or after install. Owns its skills, agents, hooks, federated config-key schema (keys + defaults + validation), and loop extension-point registrations, plus a `requires` list of other Capabilities. Declared co-located in the Capability's own folder and compiled into a generated central Capability Registry at build time. The five-step loop (Discuss → Plan → Execute → Verify → Ship) and shared-infrastructure skills (phase, config, help, update, surface, progress) are the privileged host, not Capabilities, in v1 — but host extension points are data so a loop step can become a Capability under a future uniform kernel. Supersedes the implicit feature-scattering across clusters, install-profiles, and config-schema. Generalizes the Skill Surface Budget Module and Runtime Install Policy Module. + +### Capability Registry [Planned] +Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). + +### Loop Extension Point [Planned] +A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; ~12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query (extending the `init.*` resolution seam) that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system. + +### Runtime Capability [Planned] +A `role: runtime` variant of a Capability (a Capability carries `role: feature | runtime`) that projects GSD's produced artifacts (skills/agents/hooks/commands) onto one host CLI's conventions — config-surface format, artifact-layout kinds, command template, hooks manifest, sandbox tier. It is a declarative descriptor over a fixed first-party primitive vocabulary (not a code adapter); install composes active Feature Capabilities × the chosen Runtime Capability at the InstallPlan seam (ADR-0058). First-party runtimes are authored through the same descriptor a third party would write (dogfooding the interface); tier-1 (Claude Code, Codex, Antigravity) is fully tested, the other existing runtimes ship lower-tier, none dropped. Third-party runtime loading is deferred to a purely additive external loader + trust gate. + ### Runtime Config Adapter Registry Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `cursor-hooks-json` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Realizes the adapter-selection half of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. diff --git a/docs/adr/857-capability-system.md b/docs/adr/857-capability-system.md new file mode 100644 index 000000000..4d58a933d --- /dev/null +++ b/docs/adr/857-capability-system.md @@ -0,0 +1,127 @@ +# ADR-857: Capability system — five-step loop as core, features as plug-ins behind Loop Extension Points [Proposed] + +- **Status:** Proposed +- **Date:** 2026-06-08 +- **Issue:** #857 +- **Supersedes (generalizes):** Skill Surface Budget Module (ADR-0011), Runtime Install Policy Module (ADR-0058) +- **Builds on:** CommandRoutingHub (ADR-0012), Runtime Artifact Layout Module (ADR-3660), generated-cjs single source (ADR-457) + +## Context + +GSD has no real line between **the loop** and **a feature**. The five-step loop — Discuss → Plan → Execute → Verify → Ship — is the product, but its workflow bodies have absorbed every optional feature as inline `if config.X` branches: + +- `gsd-core/workflows/plan-phase.md` is **1814 lines**; `execute-phase.md` is **1752 lines**. AI-spec (§4.5), research (§5), nyquist (§5.5), security threat-model (§5.55), UI-spec (§5.6), schema gate (§5.7), pattern-mapper (§7.8), intel (§7.9), code-review, and the planner/checker loop are all welded in at fixed `§`-points. The activation check and the behaviour live in the same file. +- "Is feature X on?" has **three independent, non-communicating answers**: `.gsd-profile` (installed?), `.gsd-surface.json` (surfaced?), and `.planning/config.json` `workflow.*` (gated?). `workflow.ui_phase=false` still leaves `ui-phase` fully surfaced. +- Adding or removing one feature is a **7-file registration tax**: `clusters.cts`, `install-profiles.cts`, `config-schema.manifest.json`, `CODEX_AGENT_SANDBOX` in `bin/install.js`, `command-aliases.cts`, agent prose, and the `.md` files. +- `src/core.cts` (2271 lines) is a god-module imported by 24 files; four otherwise-detachable feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) are tied to it **solely** for `output()`/`error()`. + +Consequence: a minimal GSD is not really installable, optional features sit inside the core loop's reliability surface, and the codebase is hard for humans and AI agents to navigate or change. + +The healthier news from the architecture review: the lower seams are already in good shape. `CommandRoutingHub` is data-driven; `runtime-artifact-layout` is one localized table; the `init.*` query already resolves a per-step JSON bundle; the repo already generates manifests from co-located sources (`research-profiles.cjs`, `package-identity.cjs`). The target is reachable without re-litigating those. + +## Decision + +Introduce a **Capability** system. The five-step loop plus shared-infrastructure skills (`phase`, `config`, `help`, `update`, `surface`, `progress`) are the **privileged host/core**. Every other feature is a **Capability** — a plug-in selectable at install and toggleable after restart. + +The design was resolved across seven decisions: + +1. **Model — host now, kernel later.** The loop is a privileged host that exposes a defined set of extension points; Capabilities attach. The host is never uninstalled. **Constraint carried through every other decision:** extension points are expressed as *data*, not hardcoded control flow, and each loop step is authored as if it could itself become a Capability — so a later migration to a uniform kernel (steps-as-capabilities) does not break plug-ins. + +2. **Granularity — feature bundle.** One Capability owns N skills + M agents + hooks + a federated config-key schema + loop-extension registrations, plus a `requires` list of other Capabilities. It toggles as a unit. This matches `clusters.cts` (richer: it also owns agents, config, and loop participation) and is kernel-compatible — a loop step is also a bundle. + +3. **Manifest — co-located → generated; config federated.** Each Capability self-declares in its own folder; a build step compiles all declarations into a generated central **Capability Registry** (mirroring the existing co-located-source → generated-file pattern). This kills the registration tax while preserving a central artifact for runtime resolution and validation. **Config schema is federated:** each Capability ships its own config-key slice (keys, defaults, validation); the loader merges them defensively. Uninstalling a Capability removes its config keys cleanly; a malformed plug-in cannot break config load for the whole tool. + +4. **Extension points — three hook kinds, coarse stable set.** ~12 named **Loop Extension Points** (per-step `pre`/`post` plus per-wave in Execute) form a stable cross-version contract. Capabilities register hooks of three kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks). All three are required: without `contribution`, prompt-woven features (security threat-model, TDD, schema gate) could never leave the core. + +5. **Dispatch — runtime resolution with concrete projection.** Workflows do not embed a generic "run whatever's registered" instruction (which would erode the executor's narrative reliability), nor are workflow files rewritten at install. Instead the workflow calls a query — extending the existing `init.*` resolution seam (e.g. `loop.render-hooks `) — that resolves the active hooks and returns **fully-rendered, ordered markdown**. Toggling stays pure data (restart-and-go, kernel-friendly); the executor still receives concrete prose. + +6. **Contract — derived order, file-artifact data flow, default-resilient failure.** Each hook declares the artifacts it `produces` and `consumes`. Hook order is the topological sort of that graph (capability-id tiebreak), which **also** defines data flow: file-artifact based (`RESEARCH.md`, `UI-SPEC.md`, …), surviving `/clear` and fresh 200k executor contexts. Failure is default-resilient — a non-gate hook that errors is skipped with a warning so a bad plug-in cannot brick the core loop; a hook may opt into `onError: halt`; `gate` hooks declare `blocking: true|false` (mirroring today's `security.block_on`). + +7. **Code — declarative + first-party, third-party deferred.** Capabilities ship declarative artifacts (skills, agents, workflow-fragments, federated config, lifecycle hooks) now. In-tree code modules (`graphify`, `intel`, `audit`) become Capabilities by registering their query family through an opened `gsd-tools.cjs` entrypoint (registry over the current hardcoded switch). The manifest reserves a `commands`/`module` field. **Third-party code-loading is explicitly out of scope** — it carries a trust/load/build/security surface that deserves its own ADR. + +8. **Runtime/CLI support is itself a Capability (declarative, tiered, third-party-ready).** The host-CLI integration (Claude Code, Codex, Antigravity, …) becomes a **Runtime Capability** — a `role: runtime` variant of the unified Capability concept (a Capability now carries `role: feature | runtime`). A Feature Capability *produces* artifacts (skills/agents/hooks/commands); a Runtime Capability *projects* them onto one CLI's conventions; install composes active Feature Capabilities × the chosen Runtime Capability at the **InstallPlan** seam (ADR-0058). A Runtime Capability is a **declarative descriptor over a fixed vocabulary of projection primitives** (config-surface format, artifact-layout kinds, command template, hooks manifest, sandbox tier) — not a code adapter. The shipped primitive library is first-party code; a CLI needing a novel primitive needs a first-party primitive — branch 7's "declarative + first-party code" rule applied to runtimes. **Anti-rework discipline:** first-party runtimes are authored through the *same descriptor a third party would write* (dogfooding the interface), so third-party support never requires re-authoring the runtimes. **Launch scope:** the descriptor seam, the primitive library, and all 15 existing runtimes re-authored as descriptors ship; the registry loads **in-tree descriptors only**. **Third-party CLI support is deferred to a purely additive external loader + trust/validation gate** — no rework, because runtimes are already descriptors. **Tiering:** Claude Code / Codex / Antigravity are tier-1 (fully tested); the other 12 existing runtimes ship as first-party lower-tier; none are dropped. + +New domain terms recorded in `CONTEXT.md`: **Capability**, **Capability Registry**, **Loop Extension Point**. + +## Resolved design details + +These were grilled to resolution after the initial eight decisions. + +### Loop Extension Points (the 12) + +`discuss:pre`, `discuss:post`, `plan:pre`, `plan:post`, `execute:pre`, `execute:wave:pre`, `execute:wave:post`, `execute:post`, `verify:pre`, `verify:post`, `ship:pre`, `ship:post`. The planner/checker loop, the verifier, and the verify-work gap-closure loop remain **core** (not hooks). Today's `§`-point features map on as: research / ui-spec / ai-spec / pattern-mapper (`step`) and security / schema-gate / tdd (`contribution`) at `plan:pre`; nyquist / gap-analysis (`gate`) at `plan:post`; build+test / code-review / drift (`gate`/`step`) at `execute:wave:post`; `verification.status` preflight (`gate`) at `ship:pre`; PR-body sections (`contribution`) at `ship:post`. The names are a stability contract — additive-only across versions. + +### Contribution merge + +Multiple `contribution` hooks at one point compose by ordered concatenation in the same `produces`/`consumes` topological order (capability-id tiebreak), each wrapped in a labeled block `…`. Provenance is explicit; semantic conflicts stay visible (both blocks render) rather than silently resolved — acceptable because the maintainer controls the active set. + +### Capability declaration shape + +A Capability is a folder `capabilities//` with a schema-validated data manifest `capability.json`. The manifest **explicitly lists** every owned artifact (skills, agents, hooks) plus the non-file facts (`role: feature | runtime`, `requires`, loop-hook registrations, config-schema ref, `runtimeCompat`, `tier`); ownership is validated against folder contents. Owned artifacts live co-located in the folder; genuinely shared artifacts (e.g. `gsd-planner`) live in a core home and are referenced. Co-located manifests compile to the generated central CJS Capability Registry. + +### Runtime Capability descriptor + +A closed named-primitive vocabulary over six axes: `configHome` (config dir), `configFormat` (`settings-json | toml | markdown | markdown-dir | none`), `artifact-layout` (destSubpath + prefix per artifact kind), `command-style`, `hooks-surface` (`settings-block | hooks-json`), and `sandbox-tier`. A descriptor selects named primitives + data; it carries no free templates or code. Adding a primitive (e.g. a novel config serializer) is first-party code plus a new enum value — branch 7's rule. This keeps descriptors inherently safe and third-party-authorable. + +### Deferred third-party trust gate + +Made light by the closed vocabulary: (1) validate the descriptor against its JSON-schema; (2) confine all file writes under the runtime's declared `configHome`; (3) require explicit user opt-in to trust an external runtime id. No code execution or free templates means no sandbox is required — the gate is purely additive to the launch design. + +## Alternatives considered + +| Decision | Rejected alternative | Why rejected | +|---|---|---| +| Model | Uniform kernel now (steps are capabilities) | Dissolves the loop narrative LLM-parsed workflows depend on; kept reachable via "host now, kernel later" | +| Granularity | Per-skill + `requires` closure | Pushes the dependency graph onto users; breaks uniformity with how a loop step looks | +| Granularity | Two-tier (skills grouped into bundles) | Two concepts to keep coherent; bundle alone suffices for v1 | +| Manifest | Central hand-edited registry | Only shrinks the tax (~7→2 files); plug-ins can't self-register | +| Manifest | Co-located only (live scan, no generated file) | No single artifact for cross-capability invariants/validation | +| Config | Central (non-federated) schema | Disabled/uninstalled feature keys linger in one file | +| Points | Sequence-steps only | Security/TDD/schema stay welded into the planner prompt | +| Points | Step + gate (no contribution) | Same — prompt-injected features can't become plug-ins | +| Dispatch | Static expansion at install | Toggling needs re-staging; installed workflows become un-editable generated artifacts; runs per-runtime | +| Dispatch | Generic runtime resolution | Executor follows a generic instruction; loses per-feature narrative reliability | +| Failure | Strict (any hook error halts) | One malformed optional plug-in could brick the core loop | +| Code | Full third-party code-shipping now | Pulls the trust/load/security surface in prematurely | +| Runtime concept | Two distinct Feature/Runtime concepts | One role-typed Capability keeps a single registry and mental model | +| Runtime interface | Code adapter, third-party loadable | Ships the trust/load/security surface prematurely; not needed at launch | +| Runtime interface | Code adapter, first-party only | Forces a later retrofit to a descriptor format — the exact rework ADR-857 is unwinding for features | +| Runtime scope | Drop the 12 non-tier-1 runtimes | Regresses working runtime support for current users | + +## Consequences + +**Positive** + +- **Locality:** one declaration per feature replaces a 7-file edit tax; a feature's skills, agents, hooks, and config keys live and leave together. +- **Leverage:** install, surface, config gating, and loop participation all become adapters over one Capability declaration. +- The core loop ships and runs **without any plug-in**; `plan-phase.md`/`execute-phase.md` shrink to the irreducible five steps. +- One resolved capability state replaces three contradicting toggle systems; "off" means off. +- `core.cts`'s blast radius shrinks; four feature modules drop to zero planning-layer coupling once `io.cts` is extracted. +- AI-navigability improves: the loop is a short legible spine, features are self-contained modules. +- The runtime/install layer becomes symmetric with the feature layer; ADR-0058's adapter registry is finished as a contributable descriptor seam, and third-party CLI support becomes additive rather than a rework. + +**Negative / costs** + +- New always-on machinery to build and keep correct: Capability Registry generation, federated-config defensive merge, and the `loop.render-hooks` resolver/projection. +- The Loop Extension Point set becomes a **stability contract** — point names must stay compatible across versions or plug-ins break. +- Default-resilient failure trades a small "silent skip" risk for core protection; gates and `onError: halt` must be authored deliberately where a feature is genuinely required. +- A multi-phase migration on a fast-moving `next`; each step must keep the tree green. +- A projection-primitive vocabulary must be designed to cover real CLIs without leaking implementation detail; tiering implies a documented support-tier policy and (ideally) a cross-runtime test matrix. + +## Rollout + +Phased; `next` stays green at each step. (Maps to the candidate sequence from the architecture review.) + +1. **Enable** — extract `output()`/`error()` from `core.cts` into `src/io.cts`; repoint `graphify`/`intel`/`audit`/`profile-pipeline`. Cheap, reversible. +2. **Clear ground** — decompose `core.cts` into `io.cts`, `config-loader.cts`, `phase-locator.cts`, `model-resolver.cts`, `roadmap-parser.cts` (ends the roadmap-parse-in-core split). Re-export shims ease transition. +3. **Define** — land the Capability Registry generation, the federated config loader, and the Loop Extension Point resolver (`loop.render-hooks`, extending `init.*`). Define the ~12 stable points. +4. **Wire** — collapse `.gsd-profile` + `.gsd-surface.json` + `config.json workflow.*` into one resolved capability state; open the `gsd-tools.cjs:runCommand` entrypoint (registry) so first-party code modules register as Capabilities. +5. **Runtime seam** — finish the InstallPlan adapter registry (ADR-0058) as a declarative descriptor over a primitive vocabulary; re-author the 15 runtimes as descriptors (tier-1: Claude/Codex/Antigravity); registry loads in-tree descriptors only (third-party loader deferred). +6. **Migrate** — convert existing optional features (UI, AI/eval, research, security, nyquist, code-review, graphify, …) to Capabilities; shrink the loop workflow bodies. + +Each phase is its own `approved-*` issue under #857 (an approved epic does not approve its children). + +## Open questions + +- Migration ordering among features with cross-dependencies (e.g. UI-spec → plan, code-review → execute) under the default-resilient failure model. +- Whether tier-1 (Claude Code / Codex / Antigravity) implies an automated cross-runtime test matrix as a merge gate. From a5d82bf98a7a5758141d7c4639832ef432fdfd14 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 10:24:32 -0400 Subject: [PATCH 2/9] refactor(#859): extract CLI I/O primitives from core.cts into io.cts (#864) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-857 rollout phase 1. Move the CLI I/O primitives — output(), error(), ERROR_REASON, setJsonErrorMode/getJsonErrorMode, and the output() large-payload temp-file spillover helpers (GSD_TEMP_DIR, ensureGsdTempDir, reapStaleTempFiles) — out of the 2271-line core.cts god-module into a new, small src/io.cts. core.cts re-exports them so existing consumers are unaffected (behavior-preserving). Repoint src/profile-pipeline.cts to import output/error/reapStaleTempFiles from io directly; graphify/intel/audit were verified not to import these symbols. Net: the leaf feature modules no longer depend on core just for I/O — the enabling first cut toward Capability extraction. New-CLI-module checklist: .gitignore (bin/lib/io.cjs), eslint.config.mjs ignores, INVENTORY.md count 90→91 + io.cjs row, INVENTORY-MANIFEST.json, ARCHITECTURE.md core.cjs/io.cjs rows, CONTEXT.md "I/O Module" glossary entry. Adds tests/io.test.cjs (28 behavioral tests incl. shim-identity and the @file: spillover branch). Gates: lint, code-review, security-review, codex adversarial-review, and gsd-test-both (14742 pass on Mac + Linux Docker, 0 fail) all green. Closes #859 Co-authored-by: Claude Opus 4.8 --- .gitignore | 1 + CONTEXT.md | 3 + docs/ARCHITECTURE.md | 3 +- docs/INVENTORY-MANIFEST.json | 3 +- docs/INVENTORY.md | 5 +- eslint.config.mjs | 1 + src/core.cts | 150 +-------------- src/io.cts | 174 ++++++++++++++++++ src/profile-pipeline.cts | 4 +- tests/io.test.cjs | 341 +++++++++++++++++++++++++++++++++++ 10 files changed, 533 insertions(+), 152 deletions(-) create mode 100644 src/io.cts create mode 100644 tests/io.test.cjs diff --git a/.gitignore b/.gitignore index bbc12e1b1..3d0c8ac23 100644 --- a/.gitignore +++ b/.gitignore @@ -127,6 +127,7 @@ build/ /gsd-core/bin/lib/runtime-config-adapter-registry.cjs /gsd-core/bin/lib/command-routing-hub.cjs /gsd-core/bin/lib/core.cjs +/gsd-core/bin/lib/io.cjs /gsd-core/bin/lib/drift.cjs /gsd-core/bin/lib/cjs-command-router-adapter.cjs /gsd-core/bin/lib/phase-command-router.cjs diff --git a/CONTEXT.md b/CONTEXT.md index a5ebdd85f..c040e97f9 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -106,6 +106,9 @@ Module owning validation for Installer Migration Module records and planned acti ### Installer Module Primary installer for all runtimes. Single production file: `bin/install.js` (generated). Exports: `install(isGlobal, runtime[, configDir])` → typed result `{ runtime, configDir, settingsPath, settings, statuslineCommand, updateBannerCommand }`; `uninstall(isGlobal, runtime[, configDir])`; `installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile)`; `uninstallRuntimeArtifacts(runtime, configDir, scope)`; `writeManifest(configDir, runtime)`. Runtime enum: `allRuntimes` (15 values: claude, antigravity, augment, cline, codebuddy, codex, copilot, cursor, gemini, hermes, kilo, opencode, qwen, trae, windsurf). Directory helpers: `getDirName(runtime)` → local dir name; `getConfigDirFromHome(runtime, isGlobal)` → shell-quoted path fragment. Per-runtime global config-dir resolution is delegated to `gsd-core/bin/lib/runtime-homes.cjs:getGlobalConfigDir(runtime[, explicitDir])` — the canonical, env-var–aware projection (`explicitDir` override + opencode/kilo `*_CONFIG` file-path precedence); the legacy in-installer `getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir` were retired into it (#56). Runtime-specific helpers: `resolveKiloConfigPath(configDir)`, `configureKiloPermissions(isGlobal[, explicitDir])`. Claude-specific permission helpers: `mergeClaudePermissions(settings)` — non-destructively appends GSD-owned allow/deny entries (see `GSD_CLAUDE_ALLOW_PERMISSIONS`, `GSD_CLAUDE_DENY_PERMISSIONS` constants) to a Claude Code settings object; called from `finishInstall` for `runtime === 'claude'` only; uninstall removes exactly these entries (#768). Layout-driven artifact copy/removal delegates to `gsd-core/bin/lib/runtime-artifact-layout.cjs:resolveRuntimeArtifactLayout` (throws `TypeError` for unknown runtimes). Hermes uses nested `skills/gsd//` layout (prefix: ''); other skill-runtimes use flat `skills/gsd-/` layout. See Skill Surface Budget Module and Runtime Artifact Layout Module. +### I/O Module +Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; `core.cjs` re-exports the primitives for back-compat. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`). + ### Package Identity Module [Planned] Single seam owning GSD's published-package coordinates so a repoint/rename is a one-line change instead of a tree-wide sweep. Source of truth is `package.json`; values are *derived*, not re-typed: `packageName` (`.name` → `@opengsd/get-shit-done-redux`), `binName` (`Object.keys(.bin)[0]` → `get-shit-done-redux`), `repoSlug` (parsed from `.repository.url` → `open-gsd/get-shit-done-redux`), plus derived `changelogRawUrl` and `manualInstallCommand({ scope, runtime })`. Generated `.cjs` per ADR-457 (generated-single-source); shipped under `gsd-core/bin/lib/`. Three consumer worlds: **Node** consumers `require()` it at runtime (worker, `check-latest-version.cjs`, `bin/install.js`); the **bash launcher** snippet receives the literal injected by `scripts/sync-runtime-launcher.cjs` at sync time; **prose/help** literals (`update.md`, installer help) carry a committed copy. A drift-guard lint (`scripts/lint-package-identity-drift.cjs`, sibling to `check:alias-drift`) fails CI on any raw package/repo literal outside `package.json`, the generated module, and the value-checked materialization sites — this is what keeps the seam real (`two adapters`, not one). Replaces the contradictory pair it consolidates: the runtime-broken `require('../package.json').name` in `hooks/gsd-check-update-worker.js` (#378, resolves to `undefined` post-install) and the hardcoded constant in `check-latest-version.cjs` (#2992). _Avoid_: "package name string", "the npm name" (when you mean the seam). See ADR-457 and Installer Module. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b2b1b4e08..b3aa176c8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -342,7 +342,8 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core | Module | Responsibility | | ---------------------- | --------------------------------------------------------------------------------------------------- | -| `core.cjs` | Error handling, output formatting, shared utilities; compatibility re-exports for planning helpers | +| `core.cjs` | Shared utilities; compatibility re-exports for planning and I/O (`io.cjs`) helpers | +| `io.cjs` | CLI I/O primitives — output/error emission, JSON-error mode, large-payload temp-file spillover | | `planning-workspace.cjs` | Planning seam (`planningDir`, `planningPaths`, active workstream routing, `.planning/.lock`) | | `state.cjs` | STATE.md parsing, updating, progression, metrics | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 5d7854d75..4dde591fe 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -1,5 +1,5 @@ { - "generated": "2026-06-07", + "generated": "2026-06-08", "families": { "agents": [ "gsd-advisor-researcher", @@ -301,6 +301,7 @@ "installer-migration-report.cjs", "installer-migrations.cjs", "intel.cjs", + "io.cjs", "learnings.cjs", "legacy-cleanup.cjs", "milestone.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index ebce42500..69f15b57b 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -370,7 +370,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (90 shipped) +## CLI Modules (91 shipped) Full listing: `gsd-core/bin/lib/*.cjs`. @@ -396,7 +396,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `config-types.cjs` | TypeScript type definitions for the `model_policy` config block — `ModelPolicyConfig`, `TierEntry`, `RuntimeTiers`; compiled from `src/config-types.cts` at publish time (ADR-457) | | `configuration.cjs` | Configuration Module — canonical config loading, legacy-key normalization, defaults merge, and explicit on-disk migration; source of truth for both SDK and CJS consumers | | `context-utilization.cjs` | Pure classifier for `gsd-health --context` — turns (tokensUsed, contextWindow) into a `{ percent, state }` triage result against the 60%/70% fracture-point thresholds (#2792) | -| `core.cjs` | Error handling, output formatting, shared utilities, runtime fallbacks; compatibility re-exports for planning-workspace helpers | +| `core.cjs` | Shared utilities and runtime fallbacks; compatibility re-exports for planning-workspace and I/O (`io.cjs`) helpers | | `decisions.cjs` | Parses CONTEXT.md `` blocks; accepts numeric (D-42) and alphanumeric (D-INFRA-01) IDs; returns `{id, text, category, tags, trackable}` | | `docs.cjs` | Docs-update workflow init, Markdown scanning, monorepo detection | | `drift.cjs` | Post-execute codebase structural drift detector (#2003): classifies file changes into new-dir/barrel/migration/route categories and round-trips `last_mapped_commit` frontmatter | @@ -412,6 +412,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `installer-migration-report.cjs` | Installer migration report projection and blocked-action guard for install/update integration | | `installer-migrations.cjs` | Installer migration planning, artifact classification, install-state persistence, journaled apply, and rollback helpers | | `intel.cjs` | Codebase intel store backing `/gsd-map-codebase --query` and `gsd-intel-updater` | +| `io.cjs` | CLI I/O primitives — `output`/`error` emission, JSON-error mode, and large-payload temp-file spillover (extracted from `core.cjs`, ADR-857) | | `learnings.cjs` | Cross-phase learnings extraction for `/gsd-extract-learnings` | | `legacy-cleanup.cjs` | Detect and remove leftover get-shit-done-cc artifacts; exports `planLegacyCleanup` (pure scan) and `applyLegacyCleanup` (thin IO applier) that root out stale files from the old package across every GSD-managed runtime config directory (#607) | | `milestone.cjs` | Milestone archival, requirements marking | diff --git a/eslint.config.mjs b/eslint.config.mjs index d88a62b24..c461b21c8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -89,6 +89,7 @@ export default tseslint.config( 'gsd-core/bin/lib/runtime-config-adapter-registry.cjs', 'gsd-core/bin/lib/command-routing-hub.cjs', 'gsd-core/bin/lib/core.cjs', + 'gsd-core/bin/lib/io.cjs', 'gsd-core/bin/lib/drift.cjs', 'gsd-core/bin/lib/cjs-command-router-adapter.cjs', 'gsd-core/bin/lib/phase-command-router.cjs', diff --git a/src/core.cts b/src/core.cts index a0203206e..dcfad3e29 100644 --- a/src/core.cts +++ b/src/core.cts @@ -9,7 +9,10 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { execGit, platformWriteSync, platformReadSync, platformEnsureDir } from './shell-command-projection.cjs'; +import { execGit, platformWriteSync, platformReadSync } from './shell-command-projection.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import ioModule = require('./io.cjs'); +const { output, error, ERROR_REASON, setJsonErrorMode, getJsonErrorMode, GSD_TEMP_DIR, reapStaleTempFiles } = ioModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import modelProfiles = require('./model-profiles.cjs'); const { MODEL_PROFILES, AGENT_TO_PHASE_TYPE, VALID_PHASE_TYPES: _VALID_PHASE_TYPES, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier } = modelProfiles; @@ -76,151 +79,6 @@ function detectSubRepos(cwd: string): string[] { // findProjectRoot is now re-exported from the generated CJS module above. -// ─── Output helpers ─────────────────────────────────────────────────────────── - -/** - * Dedicated GSD temp directory: path.join(os.tmpdir(), 'gsd'). - * Created on first use. Keeps GSD temp files isolated from the system - * temp directory so reap scans only GSD files (#1975). - */ -const GSD_TEMP_DIR = path.join(os.tmpdir(), 'gsd'); - -function ensureGsdTempDir(): void { - platformEnsureDir(GSD_TEMP_DIR); -} - -interface ReapOptions { - maxAgeMs?: number; - dirsOnly?: boolean; -} - -/** - * Remove stale gsd-* temp files/dirs older than maxAgeMs (default: 5 minutes). - * Runs opportunistically before each new temp file write to prevent unbounded accumulation. - * @param prefix - filename prefix to match (e.g., 'gsd-') - * @param opts - * @param opts.maxAgeMs - max age in ms before removal (default: 5 min) - * @param opts.dirsOnly - if true, only remove directories (default: false) - */ -function reapStaleTempFiles(prefix = 'gsd-', { maxAgeMs = 5 * 60 * 1000, dirsOnly = false }: ReapOptions = {}): void { - try { - ensureGsdTempDir(); - const now = Date.now(); - const entries = fs.readdirSync(GSD_TEMP_DIR); - for (const entry of entries) { - if (!entry.startsWith(prefix)) continue; - const fullPath = path.join(GSD_TEMP_DIR, entry); - try { - const stat = fs.statSync(fullPath); - if (now - stat.mtimeMs > maxAgeMs) { - if (stat.isDirectory()) { - fs.rmSync(fullPath, { recursive: true, force: true }); - } else if (!dirsOnly) { - fs.unlinkSync(fullPath); - } - } - } catch { - // File may have been removed between readdir and stat — ignore - } - } - } catch { - // Non-critical — don't let cleanup failures break output - } -} - -function output(result: unknown, raw: boolean, rawValue?: unknown): void { - let data: string; - if (raw && rawValue !== undefined) { - // eslint-disable-next-line @typescript-eslint/no-base-to-string - data = String(rawValue); - } else { - const json = JSON.stringify(result, null, 2); - // Large payloads exceed Claude Code's Bash tool buffer (~50KB). - // Write to tmpfile and output the path prefixed with @file: so callers can detect it. - if (json.length > 50000) { - reapStaleTempFiles(); - ensureGsdTempDir(); - const tmpPath = path.join(GSD_TEMP_DIR, `gsd-${Date.now()}.json`); - platformWriteSync(tmpPath, json); - data = '@file:' + tmpPath; - } else { - data = json; - } - } - // process.stdout.write() is async when stdout is a pipe — process.exit() - // can tear down the process before the reader consumes the buffer. - // fs.writeSync(1, ...) blocks until the kernel accepts the bytes, and - // skipping process.exit() lets the event loop drain naturally. - fs.writeSync(1, data); -} - -/** - * Frozen enum of typed reason codes used by error() for structured errors. - * Each subcommand contributes its own codes; the enum exists so tests can - * assert against typed values instead of grepping stderr (#2974). - * - * Adding a new code: - * - Pick a snake_case lowercase value (the JSON wire form) - * - Group by subsystem prefix (CONFIG_*, SDK_*, etc) - * - Pass it to error(msg, ERROR_REASON.NEW_CODE) at the call site - */ -const ERROR_REASON = Object.freeze({ - // config-get / config-set - CONFIG_KEY_NOT_FOUND: 'config_key_not_found', - CONFIG_NO_FILE: 'config_no_file', - CONFIG_PARSE_FAILED: 'config_parse_failed', - CONFIG_INVALID_KEY: 'config_invalid_key', - // SDK / gsd-tools dispatch - SDK_FAIL_FAST: 'sdk_fail_fast', - SDK_UNKNOWN_COMMAND: 'sdk_unknown_command', - SDK_MISSING_ARG: 'sdk_missing_arg', - // workflow / phase - PHASE_NOT_FOUND: 'phase_not_found', - SUMMARY_NO_PLANNING: 'summary_no_planning', - // graphify - GRAPHIFY_NO_GRAPH: 'graphify_no_graph', - GRAPHIFY_INVALID_QUERY: 'graphify_invalid_query', - // hooks - HOOKS_OPT_OUT: 'hooks_opt_out', - // security-scan - SECURITY_SCAN_FAILED: 'security_scan_failed', - // generic - USAGE: 'usage', - UNKNOWN: 'unknown', -}); - -type ErrorReasonValue = typeof ERROR_REASON[keyof typeof ERROR_REASON]; - -/** - * Process-level flag: when true, error() emits structured JSON to stderr - * instead of plain "Error: " text. Set by gsd-tools.cjs when the - * CLI is invoked with `--json-errors`. Tests opt in to typed-IR error - * assertions by passing that flag and parsing the JSON. - * - * Default off so existing callers and human operators keep their plain-text - * diagnostics. The structured form is opt-in for tooling and tests (#2974). - */ -let _jsonErrorMode = false; -function setJsonErrorMode(v: unknown): void { _jsonErrorMode = !!v; } -function getJsonErrorMode(): boolean { return _jsonErrorMode; } - -/** - * Emit an error and exit. When the second argument is provided it must be - * a value from ERROR_REASON; tests can assert on `result.reason`. When the - * process is in JSON-error mode, stderr receives `{ ok: false, reason, - * message }` so callers can parse it; otherwise stderr keeps the plain - * text form for human operators. - */ -function error(message: string, reason: ErrorReasonValue = ERROR_REASON.UNKNOWN): never { - if (_jsonErrorMode) { - const payload = JSON.stringify({ ok: false, reason, message }) + '\n'; - fs.writeSync(2, payload); - } else { - fs.writeSync(2, 'Error: ' + message + '\n'); - } - process.exit(1); -} - // ─── File & Config utilities ────────────────────────────────────────────────── /** diff --git a/src/io.cts b/src/io.cts new file mode 100644 index 000000000..97c7a43a0 --- /dev/null +++ b/src/io.cts @@ -0,0 +1,174 @@ +/** + * CLI I/O primitives — output(), error(), ERROR_REASON, JSON-error mode, + * and the temp-file helpers that output() depends on. + * + * Extracted from core.cts (ADR-857 rollout phase 1 / issue #859). + * The hand-written bodies are preserved byte-for-behaviour; only the module + * boundary moved. core.cts re-exports every symbol here under its own + * `export =` object so existing consumers are unaffected. + * + * New imports should pull I/O primitives from io.cjs directly. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { platformWriteSync, platformEnsureDir } from './shell-command-projection.cjs'; + +// ─── Temp-file helpers (needed by output()) ────────────────────────────────── + +/** + * Dedicated GSD temp directory: path.join(os.tmpdir(), 'gsd'). + * Created on first use. Keeps GSD temp files isolated from the system + * temp directory so reap scans only GSD files (#1975). + */ +const GSD_TEMP_DIR = path.join(os.tmpdir(), 'gsd'); + +function ensureGsdTempDir(): void { + platformEnsureDir(GSD_TEMP_DIR); +} + +interface ReapOptions { + maxAgeMs?: number; + dirsOnly?: boolean; +} + +/** + * Remove stale gsd-* temp files/dirs older than maxAgeMs (default: 5 minutes). + * Runs opportunistically before each new temp file write to prevent unbounded accumulation. + * @param prefix - filename prefix to match (e.g., 'gsd-') + * @param opts + * @param opts.maxAgeMs - max age in ms before removal (default: 5 min) + * @param opts.dirsOnly - if true, only remove directories (default: false) + */ +function reapStaleTempFiles(prefix = 'gsd-', { maxAgeMs = 5 * 60 * 1000, dirsOnly = false }: ReapOptions = {}): void { + try { + ensureGsdTempDir(); + const now = Date.now(); + const entries = fs.readdirSync(GSD_TEMP_DIR); + for (const entry of entries) { + if (!entry.startsWith(prefix)) continue; + const fullPath = path.join(GSD_TEMP_DIR, entry); + try { + const stat = fs.statSync(fullPath); + if (now - stat.mtimeMs > maxAgeMs) { + if (stat.isDirectory()) { + fs.rmSync(fullPath, { recursive: true, force: true }); + } else if (!dirsOnly) { + fs.unlinkSync(fullPath); + } + } + } catch { + // File may have been removed between readdir and stat — ignore + } + } + } catch { + // Non-critical — don't let cleanup failures break output + } +} + +// ─── Output helpers ─────────────────────────────────────────────────────────── + +function output(result: unknown, raw: boolean, rawValue?: unknown): void { + let data: string; + if (raw && rawValue !== undefined) { + // eslint-disable-next-line @typescript-eslint/no-base-to-string + data = String(rawValue); + } else { + const json = JSON.stringify(result, null, 2); + // Large payloads exceed Claude Code's Bash tool buffer (~50KB). + // Write to tmpfile and output the path prefixed with @file: so callers can detect it. + if (json.length > 50000) { + reapStaleTempFiles(); + ensureGsdTempDir(); + const tmpPath = path.join(GSD_TEMP_DIR, `gsd-${Date.now()}.json`); + platformWriteSync(tmpPath, json); + data = '@file:' + tmpPath; + } else { + data = json; + } + } + // process.stdout.write() is async when stdout is a pipe — process.exit() + // can tear down the process before the reader consumes the buffer. + // fs.writeSync(1, ...) blocks until the kernel accepts the bytes, and + // skipping process.exit() lets the event loop drain naturally. + fs.writeSync(1, data); +} + +/** + * Frozen enum of typed reason codes used by error() for structured errors. + * Each subcommand contributes its own codes; the enum exists so tests can + * assert against typed values instead of grepping stderr (#2974). + * + * Adding a new code: + * - Pick a snake_case lowercase value (the JSON wire form) + * - Group by subsystem prefix (CONFIG_*, SDK_*, etc) + * - Pass it to error(msg, ERROR_REASON.NEW_CODE) at the call site + */ +const ERROR_REASON = Object.freeze({ + // config-get / config-set + CONFIG_KEY_NOT_FOUND: 'config_key_not_found', + CONFIG_NO_FILE: 'config_no_file', + CONFIG_PARSE_FAILED: 'config_parse_failed', + CONFIG_INVALID_KEY: 'config_invalid_key', + // SDK / gsd-tools dispatch + SDK_FAIL_FAST: 'sdk_fail_fast', + SDK_UNKNOWN_COMMAND: 'sdk_unknown_command', + SDK_MISSING_ARG: 'sdk_missing_arg', + // workflow / phase + PHASE_NOT_FOUND: 'phase_not_found', + SUMMARY_NO_PLANNING: 'summary_no_planning', + // graphify + GRAPHIFY_NO_GRAPH: 'graphify_no_graph', + GRAPHIFY_INVALID_QUERY: 'graphify_invalid_query', + // hooks + HOOKS_OPT_OUT: 'hooks_opt_out', + // security-scan + SECURITY_SCAN_FAILED: 'security_scan_failed', + // generic + USAGE: 'usage', + UNKNOWN: 'unknown', +}); + +type ErrorReasonValue = typeof ERROR_REASON[keyof typeof ERROR_REASON]; + +/** + * Process-level flag: when true, error() emits structured JSON to stderr + * instead of plain "Error: " text. Set by gsd-tools.cjs when the + * CLI is invoked with `--json-errors`. Tests opt in to typed-IR error + * assertions by passing that flag and parsing the JSON. + * + * Default off so existing callers and human operators keep their plain-text + * diagnostics. The structured form is opt-in for tooling and tests (#2974). + */ +let _jsonErrorMode = false; +function setJsonErrorMode(v: unknown): void { _jsonErrorMode = !!v; } +function getJsonErrorMode(): boolean { return _jsonErrorMode; } + +/** + * Emit an error and exit. When the second argument is provided it must be + * a value from ERROR_REASON; tests can assert on `result.reason`. When the + * process is in JSON-error mode, stderr receives `{ ok: false, reason, + * message }` so callers can parse it; otherwise stderr keeps the plain + * text form for human operators. + */ +function error(message: string, reason: ErrorReasonValue = ERROR_REASON.UNKNOWN): never { + if (_jsonErrorMode) { + const payload = JSON.stringify({ ok: false, reason, message }) + '\n'; + fs.writeSync(2, payload); + } else { + fs.writeSync(2, 'Error: ' + message + '\n'); + } + process.exit(1); +} + +export = { + GSD_TEMP_DIR, + ensureGsdTempDir, + reapStaleTempFiles, + output, + ERROR_REASON, + setJsonErrorMode, + getJsonErrorMode, + error, +}; diff --git a/src/profile-pipeline.cts b/src/profile-pipeline.cts index e7e721f30..7de1b232f 100644 --- a/src/profile-pipeline.cts +++ b/src/profile-pipeline.cts @@ -17,8 +17,8 @@ import path from 'node:path'; import os from 'node:os'; import readline from 'node:readline'; // eslint-disable-next-line @typescript-eslint/no-require-imports -import core = require('./core.cjs'); -const { output, error, reapStaleTempFiles } = core; +import ioModule = require('./io.cjs'); +const { output, error, reapStaleTempFiles } = ioModule; // ─── Types ──────────────────────────────────────────────────────────────────── diff --git a/tests/io.test.cjs b/tests/io.test.cjs new file mode 100644 index 000000000..3825be1c1 --- /dev/null +++ b/tests/io.test.cjs @@ -0,0 +1,341 @@ +/** + * Tests for src/io.cts (compiled to gsd-core/bin/lib/io.cjs). + * + * Verifies behavioural contracts of the extracted CLI I/O primitives: + * - output() writes expected structure to stdout + * - error() writes expected structure to stderr and exits + * - ERROR_REASON constants have the correct wire values + * - setJsonErrorMode/getJsonErrorMode toggle behaviour + * - core.cjs re-export shims resolve to the exact same objects as io.cjs + * + * ADR-857 phase 1 / issue #859. + */ + +const { test, describe, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { spawnSync } = require('node:child_process'); +const path = require('node:path'); +const os = require('node:os'); +const fs = require('node:fs'); + +const io = require('../gsd-core/bin/lib/io.cjs'); +const core = require('../gsd-core/bin/lib/core.cjs'); + +// ─── ERROR_REASON constants ─────────────────────────────────────────────────── + +describe('ERROR_REASON', () => { + test('is a frozen object', () => { + assert.ok(Object.isFrozen(io.ERROR_REASON)); + }); + + test('contains expected wire values', () => { + assert.strictEqual(io.ERROR_REASON.CONFIG_KEY_NOT_FOUND, 'config_key_not_found'); + assert.strictEqual(io.ERROR_REASON.CONFIG_NO_FILE, 'config_no_file'); + assert.strictEqual(io.ERROR_REASON.CONFIG_PARSE_FAILED, 'config_parse_failed'); + assert.strictEqual(io.ERROR_REASON.CONFIG_INVALID_KEY, 'config_invalid_key'); + assert.strictEqual(io.ERROR_REASON.SDK_FAIL_FAST, 'sdk_fail_fast'); + assert.strictEqual(io.ERROR_REASON.SDK_UNKNOWN_COMMAND, 'sdk_unknown_command'); + assert.strictEqual(io.ERROR_REASON.SDK_MISSING_ARG, 'sdk_missing_arg'); + assert.strictEqual(io.ERROR_REASON.PHASE_NOT_FOUND, 'phase_not_found'); + assert.strictEqual(io.ERROR_REASON.SUMMARY_NO_PLANNING, 'summary_no_planning'); + assert.strictEqual(io.ERROR_REASON.GRAPHIFY_NO_GRAPH, 'graphify_no_graph'); + assert.strictEqual(io.ERROR_REASON.GRAPHIFY_INVALID_QUERY, 'graphify_invalid_query'); + assert.strictEqual(io.ERROR_REASON.HOOKS_OPT_OUT, 'hooks_opt_out'); + assert.strictEqual(io.ERROR_REASON.SECURITY_SCAN_FAILED, 'security_scan_failed'); + assert.strictEqual(io.ERROR_REASON.USAGE, 'usage'); + assert.strictEqual(io.ERROR_REASON.UNKNOWN, 'unknown'); + }); +}); + +// ─── setJsonErrorMode / getJsonErrorMode ───────────────────────────────────── + +describe('setJsonErrorMode / getJsonErrorMode', () => { + // Reset to false after each test so other tests are unaffected + afterEach(() => { + io.setJsonErrorMode(false); + }); + + test('defaults to false', () => { + io.setJsonErrorMode(false); // ensure clean state + assert.strictEqual(io.getJsonErrorMode(), false); + }); + + test('setJsonErrorMode(true) enables JSON error mode', () => { + io.setJsonErrorMode(true); + assert.strictEqual(io.getJsonErrorMode(), true); + }); + + test('setJsonErrorMode(false) disables JSON error mode', () => { + io.setJsonErrorMode(true); + io.setJsonErrorMode(false); + assert.strictEqual(io.getJsonErrorMode(), false); + }); + + test('setJsonErrorMode coerces truthy values', () => { + io.setJsonErrorMode(1); + assert.strictEqual(io.getJsonErrorMode(), true); + io.setJsonErrorMode(0); + assert.strictEqual(io.getJsonErrorMode(), false); + }); + + test('setJsonErrorMode coerces string truthy', () => { + io.setJsonErrorMode('yes'); + assert.strictEqual(io.getJsonErrorMode(), true); + io.setJsonErrorMode(''); + assert.strictEqual(io.getJsonErrorMode(), false); + }); +}); + +// ─── output() ──────────────────────────────────────────────────────────────── + +// output() writes directly to fd 1 and never calls process.exit, so we can +// test it by spawning a child process and capturing its stdout. + +describe('output()', () => { + const ioPath = path.resolve(__dirname, '../gsd-core/bin/lib/io.cjs'); + + test('emits JSON-serialised result to stdout', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.output({ ok: true, value: 42 }, false); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); + const parsed = JSON.parse(result.stdout); + assert.deepStrictEqual(parsed, { ok: true, value: 42 }); + }); + + test('emits raw string value when raw=true and rawValue provided', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.output({ ignored: true }, true, 'raw-text-output'); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); + assert.strictEqual(result.stdout, 'raw-text-output'); + }); + + test('falls back to JSON when raw=true but rawValue is undefined', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.output({ fallback: true }, true); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); + const parsed = JSON.parse(result.stdout); + assert.deepStrictEqual(parsed, { fallback: true }); + }); + + test('emits null correctly', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.output(null, false); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); + assert.strictEqual(result.stdout, 'null'); + }); + + test('large payload (>50000 chars) spills to @file: tempfile', (t) => { + // Build a payload whose serialized JSON exceeds 50000 chars. + // A string of 60000 'x' chars serializes to 60002 chars ("x...x"). + const largeString = 'x'.repeat(60000); + const payload = { large: largeString }; + const serialized = JSON.stringify(payload, null, 2); + assert.ok(serialized.length > 50000, 'precondition: payload must exceed 50000 chars'); + + const tmpFilesCreated = []; + + t.after(() => { + for (const p of tmpFilesCreated) { + try { fs.unlinkSync(p); } catch { /* ignore */ } + } + }); + + const script = ` + const io = require(${JSON.stringify(ioPath)}); + const largeString = 'x'.repeat(60000); + io.output({ large: largeString }, false); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 0, `process exited non-zero: ${result.stderr}`); + + const stdout = result.stdout.trim(); + assert.ok(stdout.startsWith('@file:'), `expected stdout to start with "@file:", got: ${stdout.slice(0, 80)}`); + + const tmpPath = stdout.slice('@file:'.length); + tmpFilesCreated.push(tmpPath); + + assert.ok(fs.existsSync(tmpPath), `expected temp file to exist at: ${tmpPath}`); + + const fileContents = fs.readFileSync(tmpPath, 'utf-8'); + const parsed = JSON.parse(fileContents); + assert.deepStrictEqual(parsed, payload); + + fs.unlinkSync(tmpPath); + tmpFilesCreated.length = 0; // already cleaned, skip t.after + }); +}); + +// ─── error() ───────────────────────────────────────────────────────────────── + +describe('error()', () => { + const ioPath = path.resolve(__dirname, '../gsd-core/bin/lib/io.cjs'); + + test('plain-text mode: writes "Error: " to stderr and exits 1', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.setJsonErrorMode(false); + io.error('something went wrong'); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 1); + assert.ok(result.stderr.includes('Error: something went wrong'), `stderr was: ${result.stderr}`); + assert.strictEqual(result.stdout, ''); + }); + + test('plain-text mode: default reason does not appear in stderr text', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.setJsonErrorMode(false); + io.error('no reason code expected'); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 1); + // plain mode does NOT include the reason field + assert.ok(!result.stderr.includes('"reason"'), `stderr unexpectedly contained reason: ${result.stderr}`); + }); + + test('JSON-error mode: writes structured JSON to stderr and exits 1', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.setJsonErrorMode(true); + io.error('structured error', io.ERROR_REASON.SDK_FAIL_FAST); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 1); + assert.strictEqual(result.stdout, ''); + const payload = JSON.parse(result.stderr.trim()); + assert.strictEqual(payload.ok, false); + assert.strictEqual(payload.reason, 'sdk_fail_fast'); + assert.strictEqual(payload.message, 'structured error'); + }); + + test('JSON-error mode: defaults reason to UNKNOWN when not supplied', () => { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.setJsonErrorMode(true); + io.error('no reason given'); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 1); + const payload = JSON.parse(result.stderr.trim()); + assert.strictEqual(payload.reason, 'unknown'); + assert.strictEqual(payload.message, 'no reason given'); + }); + + test('all ERROR_REASON values round-trip through JSON-error mode', () => { + // spot-check a few variants + const cases = [ + ['config_key_not_found', 'CONFIG_KEY_NOT_FOUND'], + ['phase_not_found', 'PHASE_NOT_FOUND'], + ['usage', 'USAGE'], + ]; + for (const [expected, key] of cases) { + const script = ` + const io = require(${JSON.stringify(ioPath)}); + io.setJsonErrorMode(true); + io.error('test', io.ERROR_REASON.${key}); + `; + const result = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' }); + assert.strictEqual(result.status, 1, `key=${key}`); + const payload = JSON.parse(result.stderr.trim()); + assert.strictEqual(payload.reason, expected, `key=${key}`); + } + }); +}); + +// ─── GSD_TEMP_DIR / reapStaleTempFiles ─────────────────────────────────────── + +describe('GSD_TEMP_DIR', () => { + test('resolves to /gsd', () => { + assert.strictEqual(io.GSD_TEMP_DIR, path.join(os.tmpdir(), 'gsd')); + }); +}); + +describe('reapStaleTempFiles (via io)', () => { + const TEST_PREFIX = 'gsd-io-test-'; + + afterEach(() => { + // clean up any test files we created + try { + const entries = fs.readdirSync(io.GSD_TEMP_DIR); + for (const e of entries) { + if (e.startsWith(TEST_PREFIX)) { + const p = path.join(io.GSD_TEMP_DIR, e); + try { fs.unlinkSync(p); } catch { /* ignore */ } + } + } + } catch { /* ignore */ } + }); + + test('removes stale files beyond maxAgeMs', () => { + fs.mkdirSync(io.GSD_TEMP_DIR, { recursive: true }); + const stalePath = path.join(io.GSD_TEMP_DIR, TEST_PREFIX + 'stale.json'); + fs.writeFileSync(stalePath, '{}'); + // backdate mtime so it looks older than 1ms + const old = new Date(Date.now() - 10000); + fs.utimesSync(stalePath, old, old); + + io.reapStaleTempFiles(TEST_PREFIX, { maxAgeMs: 5000 }); + assert.ok(!fs.existsSync(stalePath), 'stale file should have been removed'); + }); + + test('keeps fresh files within maxAgeMs', () => { + fs.mkdirSync(io.GSD_TEMP_DIR, { recursive: true }); + const freshPath = path.join(io.GSD_TEMP_DIR, TEST_PREFIX + 'fresh.json'); + fs.writeFileSync(freshPath, '{}'); + // mtime is just now — well within a 1-hour window + io.reapStaleTempFiles(TEST_PREFIX, { maxAgeMs: 60 * 60 * 1000 }); + assert.ok(fs.existsSync(freshPath), 'fresh file should have been kept'); + }); + + test('does not throw when GSD_TEMP_DIR does not exist yet', () => { + // reap against a non-existent prefix — must not throw + assert.doesNotThrow(() => { + io.reapStaleTempFiles('gsd-io-nonexistent-prefix-xyz-', { maxAgeMs: 0 }); + }); + }); +}); + +// ─── core.cjs re-export shim parity ────────────────────────────────────────── + +describe('core.cjs re-export shims', () => { + test('core.output is the same function as io.output', () => { + assert.strictEqual(core.output, io.output); + }); + + test('core.error is the same function as io.error', () => { + assert.strictEqual(core.error, io.error); + }); + + test('core.ERROR_REASON is the same object as io.ERROR_REASON', () => { + assert.strictEqual(core.ERROR_REASON, io.ERROR_REASON); + }); + + test('core.setJsonErrorMode is the same function as io.setJsonErrorMode', () => { + assert.strictEqual(core.setJsonErrorMode, io.setJsonErrorMode); + }); + + test('core.getJsonErrorMode is the same function as io.getJsonErrorMode', () => { + assert.strictEqual(core.getJsonErrorMode, io.getJsonErrorMode); + }); + + test('core.reapStaleTempFiles is the same function as io.reapStaleTempFiles', () => { + assert.strictEqual(core.reapStaleTempFiles, io.reapStaleTempFiles); + }); + + test('core.GSD_TEMP_DIR is the same value as io.GSD_TEMP_DIR', () => { + assert.strictEqual(core.GSD_TEMP_DIR, io.GSD_TEMP_DIR); + }); +}); From ac56672dba708ae3d604cc605ac7d3f420dfee56 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 10:41:54 -0400 Subject: [PATCH 3/9] fix(#856): remove gsd-cmd-rewrites temp dirs after install (#862) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#856): remove gsd-cmd-rewrites temp dirs after install applyRuntimeContentRewritesForCommandsInPlace() returns a fresh mkdtemp dir under os.tmpdir() (gsd-cmd-rewrites-*) with rewritten command markdown. installRuntimeArtifacts() copied from it but never removed it, leaking one temp dir per commands kind per install — on tmpfs /tmp hosts these accumulate and consume RAM-backed storage. Wrap the per-kind copy in try/finally and rmSync the temp dir (only when it differs from the staged source, i.e. the commands kind) once the copy completes or fails. dest creation moved inside the try so a mkdir failure still triggers cleanup. Regression test isolates os.tmpdir() to a private TMPDIR root and asserts no gsd-cmd-rewrites-* dir survives the install. Co-Authored-By: Claude Opus 4.8 * docs(#856): add changeset for installer temp-dir cleanup Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/humble-deer-gather.md | 5 ++ bin/install.js | 102 +++++++++++++----------- tests/enh-790-augment-commands.test.cjs | 35 ++++++++ 3 files changed, 95 insertions(+), 47 deletions(-) create mode 100644 .changeset/humble-deer-gather.md diff --git a/.changeset/humble-deer-gather.md b/.changeset/humble-deer-gather.md new file mode 100644 index 000000000..4fa6bfa35 --- /dev/null +++ b/.changeset/humble-deer-gather.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 862 +--- +**Installer no longer leaks `gsd-cmd-rewrites-*` temp directories.** Each install that emitted slash commands left one `fs.mkdtempSync` directory under the system temp root; on `tmpfs` `/tmp` hosts these accumulated and consumed RAM-backed storage. `installRuntimeArtifacts()` now removes the temp copy in a `finally` once command files are copied. diff --git a/bin/install.js b/bin/install.js index 1794edc52..50ba46122 100755 --- a/bin/install.js +++ b/bin/install.js @@ -7568,59 +7568,67 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) { // Returns a temp dir with rewritten content so source files are never mutated. stagedForCopy = applyRuntimeContentRewritesForCommandsInPlace(staged, runtime, pathPrefix); } - const dest = path.join(layout.configDir, kind.destSubpath); - fs.mkdirSync(dest, { recursive: true }); + // applyRuntimeContentRewritesForCommandsInPlace() returns a fresh mkdtemp dir under + // os.tmpdir() (gsd-cmd-rewrites-*); remove it once copied so it does not accumulate (#856). + const tempToClean = stagedForCopy !== staged ? stagedForCopy : null; + try { + const dest = path.join(layout.configDir, kind.destSubpath); + fs.mkdirSync(dest, { recursive: true }); + if (kind.kind === 'skills' && fs.existsSync(dest)) { + // Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it, + // then restore after. This preserves user dirs across a wipe-and-replace + // install (#2973 / #3664). + // + // For prefix='' (Hermes): _removeGsdEntries wipes the entire dest dir (skills/gsd/). + // Preserve every subdir that is NOT in the staged set — those are user-added dirs + // (e.g. user-content/) that GSD does not manage. + // + // For prefix='gsd-' (others): _removeGsdEntries removes only gsd-* entries. + // Non-gsd-* user dirs (e.g. my-custom-skill/) are untouched. Only preserve the + // explicit user-owned GSD-prefixed skill gsd-dev-preferences, which GSD does not + // reinstall from source but must survive the prune (#2973). + const toPreserve = new Map(); // dirName -> Map - if (kind.kind === 'skills' && fs.existsSync(dest)) { - // Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it, - // then restore after. This preserves user dirs across a wipe-and-replace - // install (#2973 / #3664). - // - // For prefix='' (Hermes): _removeGsdEntries wipes the entire dest dir (skills/gsd/). - // Preserve every subdir that is NOT in the staged set — those are user-added dirs - // (e.g. user-content/) that GSD does not manage. - // - // For prefix='gsd-' (others): _removeGsdEntries removes only gsd-* entries. - // Non-gsd-* user dirs (e.g. my-custom-skill/) are untouched. Only preserve the - // explicit user-owned GSD-prefixed skill gsd-dev-preferences, which GSD does not - // reinstall from source but must survive the prune (#2973). - const toPreserve = new Map(); // dirName -> Map + if (kind.prefix === '') { + // Hermes: wipes entire dest dir — preserve anything not in staged. + const stagedNames = fs.existsSync(stagedForCopy) + ? new Set(fs.readdirSync(stagedForCopy, { withFileTypes: true }) + .filter(e => e.isDirectory()).map(e => e.name)) + : new Set(); + for (const entry of fs.readdirSync(dest, { withFileTypes: true })) { + if (!entry.isDirectory() || stagedNames.has(entry.name)) continue; + const snap = _snapshotDir(path.join(dest, entry.name)); + if (snap.size > 0) toPreserve.set(entry.name, snap); + } + } else { + // Non-Hermes: only preserve explicitly user-owned GSD-prefixed skill dirs. + // gsd-dev-preferences is the sole user-customisable skill in this category. + const USER_OWNED_SKILL_DIRS = ['gsd-dev-preferences']; + for (const dirName of USER_OWNED_SKILL_DIRS) { + const skillDir = path.join(dest, dirName); + if (!fs.existsSync(skillDir)) continue; + const snap = _snapshotDir(skillDir); + if (snap.size > 0) toPreserve.set(dirName, snap); + } + } - if (kind.prefix === '') { - // Hermes: wipes entire dest dir — preserve anything not in staged. - const stagedNames = fs.existsSync(stagedForCopy) - ? new Set(fs.readdirSync(stagedForCopy, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name)) - : new Set(); - for (const entry of fs.readdirSync(dest, { withFileTypes: true })) { - if (!entry.isDirectory() || stagedNames.has(entry.name)) continue; - const snap = _snapshotDir(path.join(dest, entry.name)); - if (snap.size > 0) toPreserve.set(entry.name, snap); + _removeGsdEntries(dest, kind); + _copyStaged(stagedForCopy, dest, kind); + + // Restore user-owned dirs after the prune+copy + for (const [dirName, snap] of toPreserve) { + _restoreDir(path.join(dest, dirName), snap); } } else { - // Non-Hermes: only preserve explicitly user-owned GSD-prefixed skill dirs. - // gsd-dev-preferences is the sole user-customisable skill in this category. - const USER_OWNED_SKILL_DIRS = ['gsd-dev-preferences']; - for (const dirName of USER_OWNED_SKILL_DIRS) { - const skillDir = path.join(dest, dirName); - if (!fs.existsSync(skillDir)) continue; - const snap = _snapshotDir(skillDir); - if (snap.size > 0) toPreserve.set(dirName, snap); - } + // For non-skills kinds (commands, agents): no user content to preserve; + // just prune stale gsd-* entries and copy new ones. + _removeGsdEntries(dest, kind); + _copyStaged(stagedForCopy, dest, kind); } - - _removeGsdEntries(dest, kind); - _copyStaged(stagedForCopy, dest, kind); - - // Restore user-owned dirs after the prune+copy - for (const [dirName, snap] of toPreserve) { - _restoreDir(path.join(dest, dirName), snap); + } finally { + if (tempToClean) { + try { fs.rmSync(tempToClean, { recursive: true, force: true }); } catch { /* best-effort */ } } - } else { - // For non-skills kinds (commands, agents): no user content to preserve; - // just prune stale gsd-* entries and copy new ones. - _removeGsdEntries(dest, kind); - _copyStaged(stagedForCopy, dest, kind); } } } diff --git a/tests/enh-790-augment-commands.test.cjs b/tests/enh-790-augment-commands.test.cjs index ba2e53d74..95f772000 100644 --- a/tests/enh-790-augment-commands.test.cjs +++ b/tests/enh-790-augment-commands.test.cjs @@ -135,6 +135,41 @@ describe('enh-790 — installRuntimeArtifacts augment emits both commands and sk }); }); +describe('enh-790 — installRuntimeArtifacts does not leak temp dirs', () => { + test('install cleans up gsd-cmd-rewrites-* temp dirs (no leak) — #856', (t) => { + const { resolveProfile } = require('../gsd-core/bin/lib/install-profiles.cjs'); + const RESOLVED_FULL = resolveProfile({ modes: ['full'], manifest: MANIFEST }); + + // Isolate os.tmpdir() to a private root so parallel test processes can't race + // on the shared system temp dir. os.tmpdir() resolves $TMPDIR/$TEMP/$TMP per call. + const isolatedTmp = createTempDir('gsd-enh790-tmproot-'); + const prev = { TMPDIR: process.env.TMPDIR, TEMP: process.env.TEMP, TMP: process.env.TMP }; + process.env.TMPDIR = isolatedTmp; + process.env.TEMP = isolatedTmp; + process.env.TMP = isolatedTmp; + + const configDir = createTempDir('gsd-enh790-leak-'); + t.after(() => { + for (const k of ['TMPDIR', 'TEMP', 'TMP']) { + if (prev[k] === undefined) delete process.env[k]; + else process.env[k] = prev[k]; + } + cleanup(configDir); + cleanup(isolatedTmp); + }); + + installRuntimeArtifacts('augment', configDir, 'global', RESOLVED_FULL); + + // The install creates its gsd-cmd-rewrites-* temp dirs under the isolated root; + // after the fix none must remain. + const leaked = fs.readdirSync(isolatedTmp).filter(n => n.startsWith('gsd-cmd-rewrites-')); + assert.ok( + leaked.length === 0, + `installer must not leak gsd-cmd-rewrites-* temp dirs; leaked: ${leaked.join(', ')}` + ); + }); +}); + // ─── Uninstall contract ────────────────────────────────────────────────────── describe('enh-790 — uninstallRuntimeArtifacts removes augment commands', () => { From 3697e6768fa67d5f80e6e48e05be401d1fda6ec0 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 10:42:05 -0400 Subject: [PATCH 4/9] fix(#853): gate manager/autonomous background dispatch by runtime (#863) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#853): gate manager/autonomous bg dispatch by runtime /gsd-manager and /gsd-autonomous --interactive dispatched Plan/Execute via Agent(run_in_background=true). On Claude Code a backgrounded agent has no Agent/Task tool, so it cannot spawn the nested subagents those pipelines need — per-plan worktree-isolated executors, the plan-checker, and the verifier. The phases reported complete but isolation and independent verification silently never ran, even with use_worktrees / plan_check / verifier enabled. Both workflows now resolve the runtime (config-get runtime, default claude) before dispatching: run plan/execute INLINE on Claude Code so the nested pipeline runs, and background-dispatch only on runtimes where a backgrounded agent can still nest. Mirrors execute-phase.md's existing Codex fail-closed precedent. Reconciles the stale unconditional background/overlap/lean-context claims elsewhere in both workflows and in the docs. Adds a content regression test pinning the gate. Co-Authored-By: Claude Opus 4.8 * docs(#853): add changeset for runtime-gated bg dispatch Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/happy-finches-travel.md | 5 ++ docs/COMMANDS.md | 2 +- docs/FEATURES.md | 12 ++-- docs/how-to/run-phases-autonomously.md | 4 +- gsd-core/workflows/autonomous.md | 46 +++++++++++---- gsd-core/workflows/manager.md | 58 ++++++++++++++++--- ...ug-853-bg-dispatch-runtime-gating.test.cjs | 46 +++++++++++++++ 7 files changed, 145 insertions(+), 28 deletions(-) create mode 100644 .changeset/happy-finches-travel.md create mode 100644 tests/bug-853-bg-dispatch-runtime-gating.test.cjs diff --git a/.changeset/happy-finches-travel.md b/.changeset/happy-finches-travel.md new file mode 100644 index 000000000..9c974f5bf --- /dev/null +++ b/.changeset/happy-finches-travel.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 863 +--- +**`/gsd-manager` and `/gsd-autonomous --interactive` no longer silently skip worktree isolation and independent verification on Claude Code.** They dispatched plan/execute as background agents, but a backgrounded Claude Code agent has no Agent/Task tool and cannot spawn the nested executors, plan-checker, or verifier — so isolation and verification silently never ran. Both workflows now resolve the runtime and run plan/execute inline on Claude Code (background dispatch is kept on runtimes that support nested subagents). diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 50c636ca8..438bcb6dd 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -546,7 +546,7 @@ Interactive command center for managing multiple phases from one terminal. **Behavior:** - Dashboard of all phases with visual status indicators - Recommends optimal next actions based on dependencies and progress -- Dispatches work: discuss runs inline, plan/execute run as background agents +- Dispatches work: discuss runs inline; plan/execute run as background agents on runtimes that support nested background dispatch, or inline on Claude Code - Designed for power users parallelizing work across phases from one terminal - Supports per-step passthrough flags via `manager.flags` config (see [Configuration](CONFIGURATION.md#manager-passthrough-flags)) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index ddf2c7095..7b6b0cbfa 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -1986,18 +1986,18 @@ Test suite that scans all agent, workflow, and command files for embedded inject **Flag:** `/gsd-autonomous --interactive` -**Purpose:** Lean-context autonomous mode that keeps discuss-phase interactive (user answers questions) while dispatching plan and execute as background agents. +**Purpose:** Lean-context autonomous mode that keeps discuss-phase interactive (user answers questions) while dispatching plan and execute as background agents on runtimes that support nested background dispatch; on Claude Code, plan and execute run inline to preserve worktree isolation and independent verification. **Requirements:** - REQ-INTERACT-01: `--interactive` MUST run discuss-phase inline with interactive questions (not auto-answered) -- REQ-INTERACT-02: `--interactive` MUST dispatch plan-phase and execute-phase as background agents for context isolation -- REQ-INTERACT-03: `--interactive` MUST enable pipeline parallelism — discuss Phase N+1 while Phase N builds -- REQ-INTERACT-04: Main context MUST only accumulate discuss conversations (lean context) +- REQ-INTERACT-02: `--interactive` MUST dispatch plan-phase and execute-phase as background agents for context isolation on runtimes where a backgrounded agent can spawn subagents; on Claude Code, plan and execute run inline +- REQ-INTERACT-03: `--interactive` MUST enable pipeline parallelism — discuss Phase N+1 while Phase N builds (applies on runtimes that support nested background dispatch; on Claude Code, discuss does not overlap planning/execution) +- REQ-INTERACT-04: Main context MUST only accumulate discuss conversations (lean context) on runtimes that support nested background dispatch; on Claude Code, inline plan/execute also accumulate in the main context **Process:** 1. **Discuss inline** — Run discuss-phase in the main context with user interaction -2. **Dispatch** — Send plan and execute to background agents with fresh context windows -3. **Pipeline** — While background agents build Phase N, begin discussing Phase N+1 +2. **Dispatch** — On runtimes that support nested background dispatch: send plan and execute to background agents with fresh context windows. On Claude Code: run plan and execute inline. +3. **Pipeline** — On runtimes with background dispatch: while background agents build Phase N, begin discussing Phase N+1. On Claude Code: phases run sequentially. --- diff --git a/docs/how-to/run-phases-autonomously.md b/docs/how-to/run-phases-autonomously.md index ecfb99662..d1eb0d6f0 100644 --- a/docs/how-to/run-phases-autonomously.md +++ b/docs/how-to/run-phases-autonomously.md @@ -64,8 +64,8 @@ By default, autonomous mode answers discuss questions automatically using smart In interactive mode: - `/gsd-discuss-phase` runs inline and waits for your answers -- Planning and execution are dispatched as background agents so you can discuss the next phase while the current one builds -- The main context stays lean — only discuss conversations accumulate +- On runtimes that support nested background dispatch, planning and execution are dispatched as background agents so you can discuss the next phase while the current one builds; on Claude Code, planning and execution run inline (the next phase's discuss does not overlap) +- The main context stays lean — only discuss conversations accumulate (on runtimes with background dispatch; on Claude Code, inline plan/execute also accumulate) --- diff --git a/gsd-core/workflows/autonomous.md b/gsd-core/workflows/autonomous.md index 2bf0f2a3b..9e2f1ca0b 100644 --- a/gsd-core/workflows/autonomous.md +++ b/gsd-core/workflows/autonomous.md @@ -43,7 +43,7 @@ fi When `--only` is set, also set `FROM_PHASE` to the same value so existing filter logic applies. -When `--interactive` is set, discuss runs inline with questions (not auto-answered), while plan and execute are dispatched as background agents. This keeps the main context lean — only discuss conversations accumulate — while preserving user input on all design decisions. +When `--interactive` is set, discuss runs inline with questions (not auto-answered). On runtimes where a backgrounded agent can spawn subagents, plan and execute are dispatched as background agents — keeping the main context lean (only discuss conversations accumulate) and enabling overlap. On Claude Code, where a backgrounded agent cannot nest subagents, plan and execute run inline to preserve worktree isolation and independent verification, so they run sequentially and their work accumulates in the main context. Either way, user input is preserved on all design decisions. Bootstrap via milestone-level init: @@ -322,9 +322,21 @@ UI_SPEC_FILE=$(ls "${PHASE_DIR}"/*-UI-SPEC.md 2>/dev/null | head -1) **3b. Plan** -**If `INTERACTIVE` is set:** Dispatch plan as a background agent to keep the main context lean. While plan runs, the workflow can immediately start discussing the next phase (see step 4). +**If `INTERACTIVE` is set:** Background dispatch is only safe where a backgrounded agent can still spawn subagents. On Claude Code a backgrounded agent has no `Agent`/`Task` tool, so the plan-checker never runs and `workflow.plan_check` silently degrades to a self-check. Resolve the runtime first: -Print: `◆ Spawning background planner for phase ${PHASE_NUM}... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` +```bash +RUNTIME=$(gsd_run query config-get runtime --default claude 2>/dev/null || echo "claude") +``` + +- **On Claude Code (`RUNTIME` is `claude`):** Run plan **inline** (do NOT background) so the plan-checker runs. The next phase's discuss does not overlap planning here — correctness over overlap. + +``` +Skill(skill="gsd-plan-phase", args="${PHASE_NUM}") +``` + +- **On other runtimes:** Dispatch plan as a background agent to keep the main context lean. While plan runs, the workflow can immediately start discussing the next phase (see step 4). + + Print: `◆ Spawning background planner for phase ${PHASE_NUM}... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` ``` Agent( @@ -334,7 +346,7 @@ Agent( ) ``` -Store the agent task_id. After discuss for the next phase completes (or if no next phase), wait for the plan agent to finish before proceeding to execute. + Store the agent task_id. After discuss for the next phase completes (or if no next phase), wait for the plan agent to finish before proceeding to execute. **If `INTERACTIVE` is NOT set (default):** Run plan inline as before. @@ -346,7 +358,19 @@ Verify plan produced output — re-run `init phase-op` and check `has_plans`. If **3c. Execute** -**If `INTERACTIVE` is set:** Wait for the plan agent to complete (if not already), verify plans exist, then dispatch execute as a background agent: +**If `INTERACTIVE` is set:** Wait for the plan agent to complete (if not already) and verify plans exist. Background dispatch is only safe where a backgrounded agent can still spawn subagents. On Claude Code a backgrounded agent has no `Agent`/`Task` tool, so the per-plan worktree-isolated executors and the verifier never run (`workflow.use_worktrees` and `workflow.verifier` silently degrade). Resolve the runtime first: + +```bash +RUNTIME=$(gsd_run query config-get runtime --default claude 2>/dev/null || echo "claude") +``` + +- **On Claude Code (`RUNTIME` is `claude`):** Run execute **inline** (do NOT background) so worktree isolation and verification run: + +``` +Skill(skill="gsd-execute-phase", args="${PHASE_NUM} --no-transition") +``` + +- **On other runtimes:** Dispatch execute as a background agent: ``` Agent( @@ -356,7 +380,7 @@ Agent( ) ``` -Store the agent task_id. The workflow can now start discussing the next phase while this phase executes in the background. Before starting post-execution routing for this phase, wait for the execute agent to complete. + Store the agent task_id. The workflow can now start discussing the next phase while this phase executes in the background. Before starting post-execution routing for this phase, wait for the execute agent to complete. **If `INTERACTIVE` is NOT set (default):** Run execute inline as before. @@ -572,12 +596,12 @@ Check for blockers in the Blockers/Concerns section. If blockers are found, go t If incomplete phases remain: proceed to next phase, loop back to execute_phase. -**Interactive mode overlap:** When `INTERACTIVE` is set, the iterate step enables pipeline parallelism: +**Interactive mode overlap:** When `INTERACTIVE` is set, the iterate step enables pipeline parallelism **on runtimes where a backgrounded agent can spawn subagents** (on Claude Code, plan/execute run inline — see 3b/3c — so there is no overlap and phases run sequentially): 1. After discuss completes for Phase N, dispatch plan+execute as background agents 2. Immediately start discuss for Phase N+1 (the next incomplete phase) while Phase N builds 3. Before starting plan for Phase N+1, wait for Phase N's execute agent to complete and handle its post-execution routing (verification, gap closure, etc.) -This means the user is always answering discuss questions (lightweight, interactive) while the heavy work (planning, code generation) runs in the background. The main context only accumulates discuss conversations — plan and execute contexts are isolated in their agents. +This means the user is always answering discuss questions (lightweight, interactive) while the heavy work (planning, code generation) runs in the background. The main context only accumulates discuss conversations — plan and execute contexts are isolated in their agents. (On Claude Code, plan and execute run inline, so they run sequentially and their work accumulates in the main context.) If all phases complete, proceed to lifecycle step. @@ -789,9 +813,9 @@ When any phase operation fails or a blocker is detected, present 3 options via A - [ ] `--to N` handle_blocker resume message preserves --to flag - [ ] `--to N` skips lifecycle when not all milestone phases complete - [ ] `--interactive` runs discuss inline via gsd-discuss-phase (asks questions, waits for user) -- [ ] `--interactive` dispatches plan and execute as background agents (context isolation) -- [ ] `--interactive` enables pipeline parallelism: discuss Phase N+1 while Phase N builds -- [ ] `--interactive` main context only accumulates discuss conversations (lean) +- [ ] `--interactive` dispatches plan and execute as background agents on runtimes that support nested background dispatch; runs them inline on Claude Code +- [ ] `--interactive` enables pipeline parallelism (discuss Phase N+1 while Phase N builds) on runtimes with background dispatch; phases run sequentially on Claude Code +- [ ] `--interactive` main context only accumulates discuss conversations on runtimes with background dispatch (on Claude Code, inline plan/execute also accumulate) - [ ] `--interactive` waits for background agents before post-execution routing - [ ] `--interactive` compatible with `--only`, `--from`, and `--to` flags diff --git a/gsd-core/workflows/manager.md b/gsd-core/workflows/manager.md index 93a1cbd48..c46df999d 100644 --- a/gsd-core/workflows/manager.md +++ b/gsd-core/workflows/manager.md @@ -219,16 +219,18 @@ Go to exit step. ### Compound Action (background + inline) -When the user selects a compound option: +When the user selects a compound option, behavior depends on the runtime — the Plan Phase N / Execute Phase N handlers below resolve it via `gsd_run query config-get runtime`: -1. **Spawn all background agents first** (plan/execute) — dispatch them in parallel using the Plan Phase N / Execute Phase N handlers below. -2. **Then run the inline discuss:** +- **On Claude Code:** a backgrounded agent cannot nest the pipeline's subagents, so run the chosen plan/execute step(s) **inline** via their handlers below (in order), then run the inline discuss. There is no overlap. +- **On other runtimes:** **Spawn all background agents first** (plan/execute) — dispatch them in parallel using the Plan Phase N / Execute Phase N handlers below — then run the inline discuss; the background agents continue while you discuss. + +Inline discuss: ``` Skill(skill="gsd-discuss-phase", args="{PHASE_NUM} {manager_flags.discuss}") ``` -After discuss completes, loop back to dashboard step (background agents continue running). +After discuss completes, loop back to dashboard step. ### Discuss Phase N @@ -242,7 +244,27 @@ After discuss completes, loop back to dashboard step. ### Plan Phase N -Planning runs autonomously. Spawn a background agent that delegates to the Skill pipeline with any configured flags: +Planning runs autonomously. **First resolve the runtime.** On Claude Code a backgrounded agent has no `Agent`/`Task` tool, so it cannot spawn the plan-checker the pipeline relies on — backgrounding it there silently turns `workflow.plan_check` into a self-check. So run plan **inline** on Claude Code, and **background** it only on runtimes where a backgrounded agent can still nest subagents. + +```bash +RUNTIME=$(gsd_run query config-get runtime --default claude 2>/dev/null || echo "claude") +``` + +**If `RUNTIME` is `claude` (Claude Code):** Run plan inline so the plan-checker and quality gates actually run — do NOT wrap it in `Agent(run_in_background=true, …)`: + +``` +Skill(skill="gsd-plan-phase", args="{N} --auto {manager_flags.plan}") +``` + +Display while it runs: + +``` +◆ Planning Phase {N}: {phase_name}... (runs inline so the plan-checker runs — the dashboard resumes when it returns, ~1–5 min; expected, not a freeze) +``` + +Then loop back to dashboard step. + +**If `RUNTIME` is not `claude` (e.g. Codex):** Spawn a background agent that delegates to the Skill pipeline with any configured flags: ``` Agent( @@ -264,7 +286,7 @@ Important: You are running in the background. Do NOT use AskUserQuestion — mak ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above with `run_in_background=true`, do NOT do any planning work for this phase independently. Return to the dashboard immediately and wait for the background agent to report back. Only resume planning-related work when the subagent result is available. +> **ORCHESTRATOR RULE — NON-CLAUDE RUNTIME**: After calling Agent() above with `run_in_background=true`, do NOT do any planning work for this phase independently. Return to the dashboard immediately and wait for the background agent to report back. Only resume planning-related work when the subagent result is available. Display: @@ -276,7 +298,27 @@ Loop back to dashboard step. ### Execute Phase N -Execution runs autonomously. Spawn a background agent that delegates to the Skill pipeline with any configured flags: +Execution runs autonomously. **First resolve the runtime.** On Claude Code a backgrounded agent has no `Agent`/`Task` tool, so it cannot spawn the per-plan worktree-isolated executors or the verifier — backgrounding it there silently disables `workflow.use_worktrees` isolation and `workflow.verifier`. So run execute **inline** on Claude Code, and **background** it only on runtimes where a backgrounded agent can still nest subagents. + +```bash +RUNTIME=$(gsd_run query config-get runtime --default claude 2>/dev/null || echo "claude") +``` + +**If `RUNTIME` is `claude` (Claude Code):** Run execute inline so worktree isolation and the verifier actually run — do NOT wrap it in `Agent(run_in_background=true, …)`: + +``` +Skill(skill="gsd-execute-phase", args="{N} {manager_flags.execute}") +``` + +Display while it runs: + +``` +◆ Executing Phase {N}: {phase_name}... (runs inline so worktree isolation and verification run — the dashboard resumes when it returns; expected, not a freeze) +``` + +Then loop back to dashboard step. + +**If `RUNTIME` is not `claude` (e.g. Codex):** Spawn a background agent that delegates to the Skill pipeline with any configured flags: ``` Agent( @@ -298,7 +340,7 @@ Important: You are running in the background. Do NOT use AskUserQuestion — mak ) ``` -> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above with `run_in_background=true`, do NOT do any execution work for this phase independently. Return to the dashboard immediately and wait for the background agent to report back. Only resume execution-related work when the subagent result is available. +> **ORCHESTRATOR RULE — NON-CLAUDE RUNTIME**: After calling Agent() above with `run_in_background=true`, do NOT do any execution work for this phase independently. Return to the dashboard immediately and wait for the background agent to report back. Only resume execution-related work when the subagent result is available. Display: diff --git a/tests/bug-853-bg-dispatch-runtime-gating.test.cjs b/tests/bug-853-bg-dispatch-runtime-gating.test.cjs new file mode 100644 index 000000000..0ffbf5318 --- /dev/null +++ b/tests/bug-853-bg-dispatch-runtime-gating.test.cjs @@ -0,0 +1,46 @@ +'use strict'; +/** + * Regression guard — bug(#853): /gsd-manager and /gsd-autonomous --interactive + * silently skipped worktree isolation + independent verification because they + * dispatched Plan/Execute via Agent(run_in_background=true). On Claude Code a + * backgrounded agent has no Agent/Task tool, so it cannot spawn the nested + * subagents (worktree executors, plan-checker, verifier). The workflows must + * now resolve the runtime and run inline on Claude Code. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); +const MANAGER = fs.readFileSync(path.join(WORKFLOWS_DIR, 'manager.md'), 'utf8'); +const AUTONOMOUS = fs.readFileSync(path.join(WORKFLOWS_DIR, 'autonomous.md'), 'utf8'); + +describe('bug-853 — manager/autonomous gate background dispatch by runtime', () => { + test('manager.md resolves the runtime before dispatching plan/execute', () => { + // Two dispatch sites (plan + execute), each must resolve the runtime. + const matches = MANAGER.match(/config-get runtime/g) || []; + assert.ok(matches.length >= 2, 'manager.md must resolve runtime for both plan and execute dispatch'); + }); + + test('manager.md documents why Claude Code cannot background-dispatch', () => { + assert.match(MANAGER, /backgrounded agent has no `Agent`\/`Task` tool/); + }); + + test('manager.md runs plan/execute inline on Claude Code', () => { + assert.match(MANAGER, /If `RUNTIME` is `claude`[\s\S]{0,400}?Skill\(skill="gsd-plan-phase"/); + assert.match(MANAGER, /If `RUNTIME` is `claude`[\s\S]{0,400}?Skill\(skill="gsd-execute-phase"/); + }); + + test('autonomous.md gates interactive background dispatch by runtime', () => { + const autoRuntimeMatches = AUTONOMOUS.match(/config-get runtime/g) || []; + assert.ok(autoRuntimeMatches.length >= 2, 'autonomous.md must resolve runtime in both 3b (plan) and 3c (execute) interactive branches'); + assert.match(AUTONOMOUS, /backgrounded agent has no `Agent`\/`Task` tool/); + }); + + test('autonomous.md runs plan/execute inline on Claude Code in interactive mode', () => { + assert.match(AUTONOMOUS, /On Claude Code \(`RUNTIME` is `claude`\)[\s\S]{0,400}?Skill\(skill="gsd-plan-phase"/); + assert.match(AUTONOMOUS, /On Claude Code \(`RUNTIME` is `claude`\)[\s\S]{0,400}?Skill\(skill="gsd-execute-phase"/); + }); +}); From 2988a21c462e1254aede204d28c3dac053e46dcc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 11:12:08 -0400 Subject: [PATCH 5/9] refactor(#865): extract pure phase-id helpers from core.cts into phase-id.cts (#868) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-857 rollout phase 2a — the first cut of the core.cts decomposition. Move the 9 pure phase-id parsing/matching helpers (escapeRegex, normalizePhaseName, comparePhaseNum, extractPhaseToken, phaseTokenMatches, phaseMarkdownRegexSource/Exact, getMilestoneFromPhaseId, getPhaseDirFromPhaseId) out of core.cts into a new leaf module src/phase-id.cts. core.cts re-exports them (behavior-preserving); its internal callers resolve the destructured bindings. Cycle-safe by design: phase-id depends on nothing in core, so re-export creates no circular require (the property that made phase-1 io.cts clean). This is the leaf-first ordering — it unblocks the roadmap-parser extraction (2b), which imports phaseMarkdownRegexSource. New-CLI-module checklist: .gitignore, eslint.config.mjs ignores, INVENTORY.md count 91->92 + row, INVENTORY-MANIFEST.json, ARCHITECTURE.md row, CONTEXT.md "Phase Id Module" glossary entry. Adds tests/phase-id.test.cjs (63 behavioral tests incl. shim-identity + adversarial inputs). Gates: lint, code-review, security-review, codex adversarial-review (all 0 findings), and gsd-test-both (14814 pass on Mac + Linux Docker, 0 fail). Closes #865 Co-authored-by: Claude Opus 4.8 --- .gitignore | 1 + CONTEXT.md | 3 + docs/ARCHITECTURE.md | 3 +- docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- eslint.config.mjs | 1 + src/core.cts | 199 +--------------- src/phase-id.cts | 217 ++++++++++++++++++ tests/phase-id.test.cjs | 429 +++++++++++++++++++++++++++++++++++ 9 files changed, 664 insertions(+), 193 deletions(-) create mode 100644 src/phase-id.cts create mode 100644 tests/phase-id.test.cjs diff --git a/.gitignore b/.gitignore index 3d0c8ac23..43e98f2cb 100644 --- a/.gitignore +++ b/.gitignore @@ -128,6 +128,7 @@ build/ /gsd-core/bin/lib/command-routing-hub.cjs /gsd-core/bin/lib/core.cjs /gsd-core/bin/lib/io.cjs +/gsd-core/bin/lib/phase-id.cjs /gsd-core/bin/lib/drift.cjs /gsd-core/bin/lib/cjs-command-router-adapter.cjs /gsd-core/bin/lib/phase-command-router.cjs diff --git a/CONTEXT.md b/CONTEXT.md index c040e97f9..994670b42 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -14,6 +14,9 @@ Module owning `milestone complete` (archive roadmap/requirements/phases, build M ### Dispatch Pipeline Module Module that composes Dispatch Policy Module, Query Execution Policy Module, and per-stage handlers (input-validation, plan, execution, result-builder, formatting, error-mapping, observability) into the end-to-end pipeline that produces a `QueryDispatchResult`. The SDK-era pipeline collapsed onto the Command Routing Hub per ADR-0174; current dispatch seam: `gsd-core/bin/lib/command-routing-hub.cjs` (see Command Routing Hub below). +### Phase Id Module +Module owning the pure phase-id parsing and matching helpers: phase-name normalization, phase-token extraction/matching, milestone- and phase-dir id parsing, and phase-markdown regex builders (`escapeRegex`, `normalizePhaseName`, `comparePhaseNum`, `extractPhaseToken`, `phaseTokenMatches`, `phaseMarkdownRegexSource`/`phaseMarkdownRegexSourceExact`, `getMilestoneFromPhaseId`, `getPhaseDirFromPhaseId`). Pure string/regex — no I/O, no config, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2a (#865) as the cycle-free leaf that unblocks the roadmap-parser and phase-locator extractions; `core.cjs` re-exports the helpers for back-compat. Source of truth: `gsd-core/bin/lib/phase-id.cjs` (generated from `src/phase-id.cts`). + ### Phase Lifecycle Module Module owning phase create, rename, complete, remove, list, and plan-index operations, plus phase-dir prefix validation, STATE.md staleness detection, and auto-prune behaviour. Entry point: `gsd-core/bin/lib/phase.cjs` (CJS surface). Typed phase events: `GSDPhaseStartEvent`, `GSDPhaseStepStartEvent`, `GSDPhaseStepCompleteEvent`, `GSDPhaseCompleteEvent`. (The SDK native-query surface, the `types.ts` event definitions, `phase-runner.ts`, and `phase-prompt.ts` were retired with the SDK package per ADR-0174.) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b3aa176c8..05bdbd192 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -342,8 +342,9 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core | Module | Responsibility | | ---------------------- | --------------------------------------------------------------------------------------------------- | -| `core.cjs` | Shared utilities; compatibility re-exports for planning and I/O (`io.cjs`) helpers | +| `core.cjs` | Shared utilities; compatibility re-exports for planning, I/O (`io.cjs`), and phase-id helpers | | `io.cjs` | CLI I/O primitives — output/error emission, JSON-error mode, large-payload temp-file spillover | +| `phase-id.cjs` | Pure phase-id parsing/matching helpers — normalize, token match, regex builders (extracted from `core.cjs`, ADR-857) | | `planning-workspace.cjs` | Planning seam (`planningDir`, `planningPaths`, active workstream routing, `.planning/.lock`) | | `state.cjs` | STATE.md parsing, updating, progression, metrics | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 4dde591fe..634a0fc41 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -310,6 +310,7 @@ "package-identity.cjs", "package-legitimacy.cjs", "phase-command-router.cjs", + "phase-id.cjs", "phase-lifecycle.cjs", "phase.cjs", "phases-command-router.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 69f15b57b..d99ced286 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -370,7 +370,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (91 shipped) +## CLI Modules (92 shipped) Full listing: `gsd-core/bin/lib/*.cjs`. @@ -421,6 +421,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `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 | | `phase-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phase` | +| `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) | | `phase-lifecycle.cjs` | Pure-computation phase lifecycle helpers extracted from the phase-lifecycle SDK handler | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | | `phases-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools phases` | diff --git a/eslint.config.mjs b/eslint.config.mjs index c461b21c8..757642109 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -90,6 +90,7 @@ export default tseslint.config( 'gsd-core/bin/lib/command-routing-hub.cjs', 'gsd-core/bin/lib/core.cjs', 'gsd-core/bin/lib/io.cjs', + 'gsd-core/bin/lib/phase-id.cjs', 'gsd-core/bin/lib/drift.cjs', 'gsd-core/bin/lib/cjs-command-router-adapter.cjs', 'gsd-core/bin/lib/phase-command-router.cjs', diff --git a/src/core.cts b/src/core.cts index dcfad3e29..23b986e93 100644 --- a/src/core.cts +++ b/src/core.cts @@ -14,6 +14,9 @@ import { execGit, platformWriteSync, platformReadSync } from './shell-command-pr import ioModule = require('./io.cjs'); const { output, error, ERROR_REASON, setJsonErrorMode, getJsonErrorMode, GSD_TEMP_DIR, reapStaleTempFiles } = ioModule; // eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdModule = require('./phase-id.cjs'); +const { escapeRegex, normalizePhaseName, getMilestoneFromPhaseId, getPhaseDirFromPhaseId, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, comparePhaseNum, extractPhaseToken, phaseTokenMatches } = phaseIdModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports import modelProfiles = require('./model-profiles.cjs'); const { MODEL_PROFILES, AGENT_TO_PHASE_TYPE, VALID_PHASE_TYPES: _VALID_PHASE_TYPES, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier } = modelProfiles; import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, KNOWN_RUNTIMES, RUNTIMES_WITH_REASONING_EFFORT, RUNTIMES_WITH_FAST_MODE, PROVIDER_PRESETS, KNOWN_PROVIDERS } from './model-catalog.cjs'; @@ -518,197 +521,11 @@ function pruneOrphanedWorktrees(repoRoot: string): string[] { // ─── Planning workspace (pathing + active workstream + lock) moved to planning-workspace.cjs ─── -// ─── Phase utilities ────────────────────────────────────────────────────────── - -function escapeRegex(value: unknown): string { - return String(value).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - -function normalizePhaseName(phase: unknown): string { - const str = String(phase); - // Strip optional project_code prefix (e.g., 'CK-01' → '01') - const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/, ''); - // Milestone-prefixed phase IDs: M-NN or M-N-N (deep decomposition). - const milestoneMatch = stripped.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); - if (milestoneMatch) { - const major = milestoneMatch[1].padStart(2, '0'); - const subSegments = milestoneMatch[2].slice(1).split('-').map(s => s.padStart(2, '0')); - const suffix = milestoneMatch[3] || ''; - return `${major}-${subSegments.join('-')}${suffix}`; - } - // Standard numeric phases: 1, 01, 12A, 12.1 - const match = stripped.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); - if (match) { - const padded = match[1].padStart(2, '0'); - // Preserve original case of letter suffix (#1962). - const letter = match[2] || ''; - const decimal = match[3] || ''; - return padded + letter + decimal; - } - // Custom phase IDs (e.g. PROJ-42, AUTH-101): return as-is - return str; -} - -function getMilestoneFromPhaseId(phaseId: unknown): string | null { - const str = String(phaseId); - const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - const m = stripped.match(/^0*(\d+)-\d/); - if (!m) return null; - const major = parseInt(m[1], 10); - if (major === 0 || major === 999) return null; - return `v${major}.0`; -} - -function getPhaseDirFromPhaseId(phaseId: unknown, phaseName: string | null | undefined, projectCode: string | null | undefined): string | null { - const str = String(phaseId); - const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - const m = stripped.match(/^0*(\d+)-(0*(\d+(?:-\d+)*))$/); - if (!m) return null; - const milestone = String(parseInt(m[1], 10)).padStart(2, '0'); - const subParts = m[2].split('-').map(p => String(parseInt(p, 10)).padStart(2, '0')); - const sub = subParts.join('-'); - const slug = phaseName - ? phaseName.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') - : ''; - const parts = [milestone, sub, slug].filter(Boolean); - const base = parts.join('-'); - return projectCode ? `${projectCode}-${base}` : base; -} - -/** - * Render a regex source fragment matching a phase number against ROADMAP/STATE - * prose regardless of zero-padding on either side. - */ -function phaseMarkdownRegexSource(phaseNum: unknown): string { - const stripped = String(phaseNum).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - - // Milestone-prefixed IDs: M-NN or M-N-N (deep). - const milestoneSegments = stripped.match(/^(\d+)((?:-\d+)*)([A-Z]?(?:\.\d+)*)$/i); - if (milestoneSegments && milestoneSegments[2]) { - const majorUnpadded = milestoneSegments[1].replace(/^0+/, '') || '0'; - const subParts = milestoneSegments[2].slice(1).split('-'); - const subFragments = subParts.map(s => { - const unpadded = s.replace(/^0+/, '') || '0'; - return `0*${escapeRegex(unpadded)}`; - }); - const suffix = milestoneSegments[3] || ''; - const suffixFragment = suffix ? escapeRegex(suffix) : ''; - return `0*${escapeRegex(majorUnpadded)}-${subFragments.join('-')}${suffixFragment}`; - } - - // Plain numeric phase: 1, 01, 12A, 12.1 - const match = stripped.match(/^0*(\d+)([A-Z])?((?:\.\d+)*)$/i); - if (!match) return escapeRegex(phaseNum); - - const integer = match[1].replace(/^0+/, '') || '0'; - const letter = match[2] ? escapeRegex(match[2]) : ''; - const decimal = match[3] ? escapeRegex(match[3]) : ''; - return `0*${escapeRegex(integer)}${letter}${decimal}`; -} - -/** - * #3599: when the caller passed a project-code-prefixed ID like `PROJ-42`, - * return the exact-escaped form. - */ -function phaseMarkdownRegexSourceExact(phaseNum: unknown): string | null { - const raw = String(phaseNum); - if (!/^[A-Z]{1,6}-(?=\d)/i.test(raw)) return null; - return escapeRegex(raw); -} - -function comparePhaseNum(a: unknown, b: unknown): number { - // Strip optional project_code prefix before comparing - const sa = String(a).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - const sb = String(b).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - - const milestoneA = sa.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); - const milestoneB = sb.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); - - if (milestoneA && milestoneB) { - const segsA = [parseInt(milestoneA[1], 10), ...milestoneA[2].slice(1).split('-').map(s => parseInt(s, 10))]; - const segsB = [parseInt(milestoneB[1], 10), ...milestoneB[2].slice(1).split('-').map(s => parseInt(s, 10))]; - const maxSegs = Math.max(segsA.length, segsB.length); - for (let i = 0; i < maxSegs; i++) { - const av = segsA[i] !== undefined ? segsA[i] : 0; - const bv = segsB[i] !== undefined ? segsB[i] : 0; - if (av !== bv) return av - bv; - } - const sufA = milestoneA[3] || ''; - const sufB = milestoneB[3] || ''; - if (sufA !== sufB) return sufA < sufB ? -1 : 1; - return 0; - } - - if (milestoneA || milestoneB) return String(a).localeCompare(String(b)); - - const pa = sa.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); - const pb = sb.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); - if (!pa || !pb) return String(a).localeCompare(String(b)); - const intDiff = parseInt(pa[1], 10) - parseInt(pb[1], 10); - if (intDiff !== 0) return intDiff; - const la = (pa[2] || '').toUpperCase(); - const lb = (pb[2] || '').toUpperCase(); - if (la !== lb) { - if (!la) return -1; - if (!lb) return 1; - return la < lb ? -1 : 1; - } - const aDecParts = pa[3] ? pa[3].slice(1).split('.').map(p => parseInt(p, 10)) : []; - const bDecParts = pb[3] ? pb[3].slice(1).split('.').map(p => parseInt(p, 10)) : []; - const maxLen = Math.max(aDecParts.length, bDecParts.length); - if (aDecParts.length === 0 && bDecParts.length > 0) return -1; - if (bDecParts.length === 0 && aDecParts.length > 0) return 1; - for (let i = 0; i < maxLen; i++) { - const av = Number.isFinite(aDecParts[i]) ? aDecParts[i] : 0; - const bv = Number.isFinite(bDecParts[i]) ? bDecParts[i] : 0; - if (av !== bv) return av - bv; - } - return 0; -} - -/** - * Extract the phase token from a directory name. - */ -function extractPhaseToken(dirName: string): string { - const codePrefixMatch = dirName.match(/^([A-Z]{1,6})-(\d.*)/i); - let prefix = ''; - let rest = dirName; - if (codePrefixMatch) { - prefix = codePrefixMatch[1] + '-'; - rest = codePrefixMatch[2]; - } - - const segments = rest.split('-'); - const tokenSegments: string[] = []; - for (let i = 0; i < segments.length; i++) { - const seg = segments[i]; - if (/^\d/.test(seg)) { - tokenSegments.push(seg); - } else { - break; - } - } - - if (tokenSegments.length === 0) { - return dirName; - } - - return prefix + tokenSegments.join('-'); -} - -/** - * Check if a directory name's phase token matches the normalized phase exactly. - */ -function phaseTokenMatches(dirName: string, normalized: string): boolean { - const token = extractPhaseToken(dirName); - if (token.toUpperCase() === normalized.toUpperCase()) return true; - const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - if (stripped !== dirName) { - const strippedToken = extractPhaseToken(stripped); - if (strippedToken.toUpperCase() === normalized.toUpperCase()) return true; - } - return false; -} +// ─── Phase utilities (pure helpers re-exported from phase-id.cjs) ───────────── +// escapeRegex, normalizePhaseName, getMilestoneFromPhaseId, getPhaseDirFromPhaseId, +// phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, comparePhaseNum, +// extractPhaseToken, phaseTokenMatches +// — all imported via `phaseIdModule` above; internal callers use the destructured bindings. function extractCanonicalPlanId(filename: string): string { const base = filename.replace(/-PLAN\.md$/i, '').replace(/-SUMMARY\.md$/i, '').replace(/\.md$/i, ''); diff --git a/src/phase-id.cts b/src/phase-id.cts new file mode 100644 index 000000000..360bb3ecc --- /dev/null +++ b/src/phase-id.cts @@ -0,0 +1,217 @@ +/** + * Pure phase-id parsing/matching helpers — normalize, token match, + * milestone/phase-dir id parsing, phase-markdown regex builders. + * + * Extracted from core.cts (ADR-857 rollout phase 2a / issue #865). + * The hand-written bodies are preserved byte-for-behaviour; only the module + * boundary moved. core.cts re-exports every symbol here under its own + * `export =` object so existing consumers are unaffected. + * + * New imports should pull phase-id helpers from phase-id.cjs directly. + * + * Dependencies: none (pure string/regex, no Node built-ins required). + */ + +// ─── Phase-id helpers ───────────────────────────────────────────────────────── + +function escapeRegex(value: unknown): string { + return String(value).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +function normalizePhaseName(phase: unknown): string { + const str = String(phase); + // Strip optional project_code prefix (e.g., 'CK-01' → '01') + const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/, ''); + // Milestone-prefixed phase IDs: M-NN or M-N-N (deep decomposition). + const milestoneMatch = stripped.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); + if (milestoneMatch) { + const major = milestoneMatch[1].padStart(2, '0'); + const subSegments = milestoneMatch[2].slice(1).split('-').map(s => s.padStart(2, '0')); + const suffix = milestoneMatch[3] || ''; + return `${major}-${subSegments.join('-')}${suffix}`; + } + // Standard numeric phases: 1, 01, 12A, 12.1 + const match = stripped.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); + if (match) { + const padded = match[1].padStart(2, '0'); + // Preserve original case of letter suffix (#1962). + const letter = match[2] || ''; + const decimal = match[3] || ''; + return padded + letter + decimal; + } + // Custom phase IDs (e.g. PROJ-42, AUTH-101): return as-is + return str; +} + +function getMilestoneFromPhaseId(phaseId: unknown): string | null { + const str = String(phaseId); + const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + const m = stripped.match(/^0*(\d+)-\d/); + if (!m) return null; + const major = parseInt(m[1], 10); + if (major === 0 || major === 999) return null; + return `v${major}.0`; +} + +function getPhaseDirFromPhaseId(phaseId: unknown, phaseName: string | null | undefined, projectCode: string | null | undefined): string | null { + const str = String(phaseId); + const stripped = str.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + const m = stripped.match(/^0*(\d+)-(0*(\d+(?:-\d+)*))$/); + if (!m) return null; + const milestone = String(parseInt(m[1], 10)).padStart(2, '0'); + const subParts = m[2].split('-').map(p => String(parseInt(p, 10)).padStart(2, '0')); + const sub = subParts.join('-'); + const slug = phaseName + ? phaseName.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') + : ''; + const parts = [milestone, sub, slug].filter(Boolean); + const base = parts.join('-'); + return projectCode ? `${projectCode}-${base}` : base; +} + +/** + * Render a regex source fragment matching a phase number against ROADMAP/STATE + * prose regardless of zero-padding on either side. + */ +function phaseMarkdownRegexSource(phaseNum: unknown): string { + const stripped = String(phaseNum).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + + // Milestone-prefixed IDs: M-NN or M-N-N (deep). + const milestoneSegments = stripped.match(/^(\d+)((?:-\d+)*)([A-Z]?(?:\.\d+)*)$/i); + if (milestoneSegments && milestoneSegments[2]) { + const majorUnpadded = milestoneSegments[1].replace(/^0+/, '') || '0'; + const subParts = milestoneSegments[2].slice(1).split('-'); + const subFragments = subParts.map(s => { + const unpadded = s.replace(/^0+/, '') || '0'; + return `0*${escapeRegex(unpadded)}`; + }); + const suffix = milestoneSegments[3] || ''; + const suffixFragment = suffix ? escapeRegex(suffix) : ''; + return `0*${escapeRegex(majorUnpadded)}-${subFragments.join('-')}${suffixFragment}`; + } + + // Plain numeric phase: 1, 01, 12A, 12.1 + const match = stripped.match(/^0*(\d+)([A-Z])?((?:\.\d+)*)$/i); + if (!match) return escapeRegex(phaseNum); + + const integer = match[1].replace(/^0+/, '') || '0'; + const letter = match[2] ? escapeRegex(match[2]) : ''; + const decimal = match[3] ? escapeRegex(match[3]) : ''; + return `0*${escapeRegex(integer)}${letter}${decimal}`; +} + +/** + * #3599: when the caller passed a project-code-prefixed ID like `PROJ-42`, + * return the exact-escaped form. + */ +function phaseMarkdownRegexSourceExact(phaseNum: unknown): string | null { + const raw = String(phaseNum); + if (!/^[A-Z]{1,6}-(?=\d)/i.test(raw)) return null; + return escapeRegex(raw); +} + +function comparePhaseNum(a: unknown, b: unknown): number { + // Strip optional project_code prefix before comparing + const sa = String(a).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + const sb = String(b).replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + + const milestoneA = sa.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); + const milestoneB = sb.match(/^(\d+)((?:-\d+)+)([A-Z]?(?:\.\d+)*)$/i); + + if (milestoneA && milestoneB) { + const segsA = [parseInt(milestoneA[1], 10), ...milestoneA[2].slice(1).split('-').map(s => parseInt(s, 10))]; + const segsB = [parseInt(milestoneB[1], 10), ...milestoneB[2].slice(1).split('-').map(s => parseInt(s, 10))]; + const maxSegs = Math.max(segsA.length, segsB.length); + for (let i = 0; i < maxSegs; i++) { + const av = segsA[i] !== undefined ? segsA[i] : 0; + const bv = segsB[i] !== undefined ? segsB[i] : 0; + if (av !== bv) return av - bv; + } + const sufA = milestoneA[3] || ''; + const sufB = milestoneB[3] || ''; + if (sufA !== sufB) return sufA < sufB ? -1 : 1; + return 0; + } + + if (milestoneA || milestoneB) return String(a).localeCompare(String(b)); + + const pa = sa.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); + const pb = sb.match(/^(\d+)([A-Z])?((?:\.\d+)*)/i); + if (!pa || !pb) return String(a).localeCompare(String(b)); + const intDiff = parseInt(pa[1], 10) - parseInt(pb[1], 10); + if (intDiff !== 0) return intDiff; + const la = (pa[2] || '').toUpperCase(); + const lb = (pb[2] || '').toUpperCase(); + if (la !== lb) { + if (!la) return -1; + if (!lb) return 1; + return la < lb ? -1 : 1; + } + const aDecParts = pa[3] ? pa[3].slice(1).split('.').map(p => parseInt(p, 10)) : []; + const bDecParts = pb[3] ? pb[3].slice(1).split('.').map(p => parseInt(p, 10)) : []; + const maxLen = Math.max(aDecParts.length, bDecParts.length); + if (aDecParts.length === 0 && bDecParts.length > 0) return -1; + if (bDecParts.length === 0 && aDecParts.length > 0) return 1; + for (let i = 0; i < maxLen; i++) { + const av = Number.isFinite(aDecParts[i]) ? aDecParts[i] : 0; + const bv = Number.isFinite(bDecParts[i]) ? bDecParts[i] : 0; + if (av !== bv) return av - bv; + } + return 0; +} + +/** + * Extract the phase token from a directory name. + */ +function extractPhaseToken(dirName: string): string { + const codePrefixMatch = dirName.match(/^([A-Z]{1,6})-(\d.*)/i); + let prefix = ''; + let rest = dirName; + if (codePrefixMatch) { + prefix = codePrefixMatch[1] + '-'; + rest = codePrefixMatch[2]; + } + + const segments = rest.split('-'); + const tokenSegments: string[] = []; + for (let i = 0; i < segments.length; i++) { + const seg = segments[i]; + if (/^\d/.test(seg)) { + tokenSegments.push(seg); + } else { + break; + } + } + + if (tokenSegments.length === 0) { + return dirName; + } + + return prefix + tokenSegments.join('-'); +} + +/** + * Check if a directory name's phase token matches the normalized phase exactly. + */ +function phaseTokenMatches(dirName: string, normalized: string): boolean { + const token = extractPhaseToken(dirName); + if (token.toUpperCase() === normalized.toUpperCase()) return true; + const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + if (stripped !== dirName) { + const strippedToken = extractPhaseToken(stripped); + if (strippedToken.toUpperCase() === normalized.toUpperCase()) return true; + } + return false; +} + +export = { + escapeRegex, + normalizePhaseName, + getMilestoneFromPhaseId, + getPhaseDirFromPhaseId, + phaseMarkdownRegexSource, + phaseMarkdownRegexSourceExact, + comparePhaseNum, + extractPhaseToken, + phaseTokenMatches, +}; diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs new file mode 100644 index 000000000..f16b95cee --- /dev/null +++ b/tests/phase-id.test.cjs @@ -0,0 +1,429 @@ +/** + * 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 + * - phaseTokenMatches + * - phaseMarkdownRegexSource + * - phaseMarkdownRegexSourceExact + * - getMilestoneFromPhaseId + * - getPhaseDirFromPhaseId + * - core.cjs re-export shims resolve to the exact same functions (single instance) + * + * ADR-857 rollout phase 2a / issue #865. + */ + +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); + +const phaseId = require('../gsd-core/bin/lib/phase-id.cjs'); +const core = require('../gsd-core/bin/lib/core.cjs'); + +// ─── escapeRegex ───────────────────────────────────────────────────────────── + +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('中文'), '中文'); + }); +}); + +// ─── normalizePhaseName ─────────────────────────────────────────────────────── + +describe('normalizePhaseName', () => { + test('zero-pads single-digit phase', () => { + assert.strictEqual(phaseId.normalizePhaseName('1'), '01'); + assert.strictEqual(phaseId.normalizePhaseName('3'), '03'); + }); + + test('leaves two-digit phase unchanged', () => { + assert.strictEqual(phaseId.normalizePhaseName('12'), '12'); + }); + + test('strips project_code prefix before normalizing', () => { + assert.strictEqual(phaseId.normalizePhaseName('CK-01'), '01'); + assert.strictEqual(phaseId.normalizePhaseName('PROJ-3'), '03'); + assert.strictEqual(phaseId.normalizePhaseName('AB-12'), '12'); + }); + + test('handles letter suffix (preserves original case per #1962)', () => { + assert.strictEqual(phaseId.normalizePhaseName('12A'), '12A'); + assert.strictEqual(phaseId.normalizePhaseName('3b'), '03b'); + }); + + test('handles decimal phase IDs', () => { + assert.strictEqual(phaseId.normalizePhaseName('12.1'), '12.1'); + assert.strictEqual(phaseId.normalizePhaseName('3.10'), '03.10'); + }); + + test('handles milestone-prefixed IDs (M-NN form)', () => { + assert.strictEqual(phaseId.normalizePhaseName('1-1'), '01-01'); + assert.strictEqual(phaseId.normalizePhaseName('2-3'), '02-03'); + assert.strictEqual(phaseId.normalizePhaseName('1-2-3'), '01-02-03'); + }); + + test('custom phase IDs: project_code prefix is stripped, then numeric part is normalized', () => { + // The regex /^[A-Z]{1,6}-(?=\d)/ matches 'PROJ-' and strips it, leaving '42' + // which is then normalized to '42' (no leading zero needed for 2+ digits) + assert.strictEqual(phaseId.normalizePhaseName('PROJ-42'), '42'); + assert.strictEqual(phaseId.normalizePhaseName('AUTH-101'), '101'); + }); + + test('custom phase IDs with non-numeric remainder pass through as-is', () => { + // No project_code pattern, no numeric match → return str as-is + assert.strictEqual(phaseId.normalizePhaseName('my-phase'), 'my-phase'); + }); + + test('coerces non-string values', () => { + assert.strictEqual(phaseId.normalizePhaseName(5), '05'); + }); +}); + +// ─── comparePhaseNum ────────────────────────────────────────────────────────── + +describe('comparePhaseNum', () => { + test('sorts numeric phases in ascending order', () => { + const phases = ['03', '01', '10', '02']; + const sorted = [...phases].sort(phaseId.comparePhaseNum); + assert.deepStrictEqual(sorted, ['01', '02', '03', '10']); + }); + + test('compares single-digit vs two-digit correctly', () => { + assert.ok(phaseId.comparePhaseNum('1', '02') < 0); + assert.ok(phaseId.comparePhaseNum('02', '1') > 0); + assert.strictEqual(phaseId.comparePhaseNum('1', '01'), 0); + }); + + test('handles decimal phases', () => { + assert.ok(phaseId.comparePhaseNum('1', '1.1') < 0); + assert.ok(phaseId.comparePhaseNum('1.1', '1.2') < 0); + assert.ok(phaseId.comparePhaseNum('1.10', '1.9') > 0); + assert.strictEqual(phaseId.comparePhaseNum('1.1', '01.1'), 0); + }); + + test('handles letter suffix ordering (no letter < A < B)', () => { + assert.ok(phaseId.comparePhaseNum('01', '01A') < 0); + assert.ok(phaseId.comparePhaseNum('01A', '01B') < 0); + assert.ok(phaseId.comparePhaseNum('01B', '01') > 0); + }); + + test('handles milestone-prefixed IDs', () => { + assert.ok(phaseId.comparePhaseNum('1-1', '1-2') < 0); + assert.ok(phaseId.comparePhaseNum('2-1', '1-10') > 0); + assert.ok(phaseId.comparePhaseNum('1-2-3', '1-2-4') < 0); + assert.strictEqual(phaseId.comparePhaseNum('01-01', '1-1'), 0); + }); + + test('strips project_code prefix before comparing', () => { + assert.strictEqual(phaseId.comparePhaseNum('CK-01', '01'), 0); + assert.ok(phaseId.comparePhaseNum('CK-01', 'CK-02') < 0); + }); + + test('handles non-parseable phase IDs via localeCompare fallback', () => { + // Should not throw on non-numeric IDs + const result = phaseId.comparePhaseNum('alpha', 'beta'); + assert.strictEqual(typeof result, 'number'); + }); +}); + +// ─── extractPhaseToken ──────────────────────────────────────────────────────── + +describe('extractPhaseToken', () => { + test('extracts simple numeric token from directory name', () => { + assert.strictEqual(phaseId.extractPhaseToken('01-some-phase-name'), '01'); + assert.strictEqual(phaseId.extractPhaseToken('12A-feature'), '12A'); + }); + + test('extracts milestone-prefixed numeric token', () => { + assert.strictEqual(phaseId.extractPhaseToken('01-02-some-name'), '01-02'); + assert.strictEqual(phaseId.extractPhaseToken('02-03-04-deep'), '02-03-04'); + }); + + test('extracts token with project_code prefix', () => { + assert.strictEqual(phaseId.extractPhaseToken('CK-01-some-phase'), 'CK-01'); + assert.strictEqual(phaseId.extractPhaseToken('PROJ-12-feature'), 'PROJ-12'); + }); + + test('returns the full dirName when no numeric token found', () => { + assert.strictEqual(phaseId.extractPhaseToken('no-numeric'), 'no-numeric'); + assert.strictEqual(phaseId.extractPhaseToken('alpha'), 'alpha'); + }); + + test('stops at first non-numeric-starting segment', () => { + assert.strictEqual(phaseId.extractPhaseToken('01-02-name-03'), '01-02'); + }); +}); + +// ─── phaseTokenMatches ──────────────────────────────────────────────────────── + +describe('phaseTokenMatches', () => { + test('matches exact token (case-insensitive)', () => { + assert.ok(phaseId.phaseTokenMatches('01-some-phase', '01')); + assert.ok(phaseId.phaseTokenMatches('12A-feature', '12A')); + assert.ok(phaseId.phaseTokenMatches('12A-feature', '12a')); + }); + + test('matches with project_code prefix stripped', () => { + assert.ok(phaseId.phaseTokenMatches('CK-01-phase', '01')); + assert.ok(phaseId.phaseTokenMatches('PROJ-12-feature', '12')); + }); + + test('does not match when token differs', () => { + assert.ok(!phaseId.phaseTokenMatches('01-some-phase', '02')); + assert.ok(!phaseId.phaseTokenMatches('12A-feature', '12B')); + }); + + test('matches milestone-prefixed token', () => { + assert.ok(phaseId.phaseTokenMatches('01-02-feature', '01-02')); + assert.ok(!phaseId.phaseTokenMatches('01-02-feature', '01-03')); + }); +}); + +// ─── phaseMarkdownRegexSource ───────────────────────────────────────────────── + +describe('phaseMarkdownRegexSource', () => { + test('produces a regex source that matches zero-padded variants', () => { + const src = phaseId.phaseMarkdownRegexSource('1'); + const re = new RegExp(src); + assert.ok(re.test('1')); + assert.ok(re.test('01')); + assert.ok(re.test('001')); + }); + + test('produces source matching a two-digit phase', () => { + const src = phaseId.phaseMarkdownRegexSource('12'); + const re = new RegExp(src); + assert.ok(re.test('12')); + assert.ok(re.test('012')); + assert.ok(!re.test('13')); + }); + + test('handles letter suffix', () => { + const src = phaseId.phaseMarkdownRegexSource('12A'); + const re = new RegExp(src, 'i'); + assert.ok(re.test('12A')); + assert.ok(re.test('012A')); + }); + + test('handles decimal phases', () => { + const src = phaseId.phaseMarkdownRegexSource('3.1'); + const re = new RegExp(src); + assert.ok(re.test('3.1')); + assert.ok(re.test('03.1')); + assert.ok(!re.test('3.2')); + }); + + test('handles milestone-prefixed phase IDs', () => { + const src = phaseId.phaseMarkdownRegexSource('1-2'); + const re = new RegExp(src); + assert.ok(re.test('1-2')); + assert.ok(re.test('01-02')); + assert.ok(re.test('01-2')); + assert.ok(!re.test('1-3')); + }); + + test('strips project_code prefix before building regex', () => { + const withPrefix = phaseId.phaseMarkdownRegexSource('CK-01'); + const withoutPrefix = phaseId.phaseMarkdownRegexSource('01'); + assert.strictEqual(withPrefix, withoutPrefix); + }); + + test('falls back to escaped literal for unparseable input', () => { + const src = phaseId.phaseMarkdownRegexSource('v1.0'); + assert.strictEqual(typeof src, 'string'); + assert.ok(src.length > 0); + }); + + test('adversarial: phase num containing regex metacharacters is escaped', () => { + // e.g. some exotic value that shouldn't break regexp construction + const src = phaseId.phaseMarkdownRegexSource('3.1'); + // The literal dot in "3.1" should be escaped so it only matches a real dot + const re = new RegExp(src); + assert.ok(!re.test('3X1'), 'unescaped dot would match any char — must be escaped'); + }); +}); + +// ─── 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)); + }); + + test('returns null for non-prefixed IDs', () => { + assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('01'), null); + assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('12A'), null); + assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact('1-2'), null); + }); + + test('null coercion: returns null for null/undefined', () => { + assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact(null), null); + assert.strictEqual(phaseId.phaseMarkdownRegexSourceExact(undefined), null); + }); + + test('resulting regex matches the exact prefixed ID', () => { + const src = phaseId.phaseMarkdownRegexSourceExact('AUTH-101'); + assert.ok(src !== null); + const re = new RegExp(src); + assert.ok(re.test('AUTH-101')); + assert.ok(!re.test('AUTH-102')); + }); +}); + +// ─── getMilestoneFromPhaseId ────────────────────────────────────────────────── + +describe('getMilestoneFromPhaseId', () => { + test('returns vN.0 for a milestone-prefixed phase id', () => { + assert.strictEqual(phaseId.getMilestoneFromPhaseId('1-01'), 'v1.0'); + assert.strictEqual(phaseId.getMilestoneFromPhaseId('02-03'), 'v2.0'); + assert.strictEqual(phaseId.getMilestoneFromPhaseId('10-5'), 'v10.0'); + }); + + test('returns null for non-milestone-prefixed IDs', () => { + assert.strictEqual(phaseId.getMilestoneFromPhaseId('01'), null); + assert.strictEqual(phaseId.getMilestoneFromPhaseId('12A'), null); + }); + + test('returns null for special sentinel milestones 0 and 999', () => { + assert.strictEqual(phaseId.getMilestoneFromPhaseId('0-1'), null); + assert.strictEqual(phaseId.getMilestoneFromPhaseId('999-1'), null); + }); + + test('strips project_code prefix before parsing', () => { + assert.strictEqual(phaseId.getMilestoneFromPhaseId('CK-2-01'), 'v2.0'); + }); + + test('coerces non-string values', () => { + // numeric doesn't match the milestone pattern — returns null + assert.strictEqual(phaseId.getMilestoneFromPhaseId(42), null); + }); +}); + +// ─── getPhaseDirFromPhaseId ─────────────────────────────────────────────────── + +describe('getPhaseDirFromPhaseId', () => { + test('returns null for non-milestone-format IDs', () => { + assert.strictEqual(phaseId.getPhaseDirFromPhaseId('01', null, null), null); + assert.strictEqual(phaseId.getPhaseDirFromPhaseId('12A', null, null), null); + }); + + test('constructs dir name from milestone-prefixed phase id (no name, no code)', () => { + const result = phaseId.getPhaseDirFromPhaseId('1-2', null, null); + assert.strictEqual(result, '01-02'); + }); + + test('includes phaseName slug', () => { + const result = phaseId.getPhaseDirFromPhaseId('1-2', 'My Feature', null); + assert.strictEqual(result, '01-02-my-feature'); + }); + + test('prepends projectCode when provided', () => { + const result = phaseId.getPhaseDirFromPhaseId('1-2', 'Auth', 'CK'); + assert.strictEqual(result, 'CK-01-02-auth'); + }); + + test('strips project_code from phaseId before parsing', () => { + const result = phaseId.getPhaseDirFromPhaseId('CK-1-2', null, null); + assert.strictEqual(result, '01-02'); + }); + + test('handles deep decomposition IDs (M-N-N)', () => { + // m[2] is "02-03" for input "1-2-3" — split and pad each sub-part + const result = phaseId.getPhaseDirFromPhaseId('1-2-3', null, null); + assert.strictEqual(result, '01-02-03'); + }); + + test('slug strips leading/trailing hyphens from phaseName', () => { + const result = phaseId.getPhaseDirFromPhaseId('1-1', ' --some--name-- ', null); + // normalize: replace non-alnum runs with hyphen, strip edges + assert.ok(result !== null); + assert.ok(!result.startsWith('-')); + assert.ok(!result.endsWith('-')); + }); +}); + +// ─── core.cjs re-export shim identity assertions ────────────────────────────── + +describe('core.cjs re-export shim identity (single instance)', () => { + test('core.escapeRegex === phaseId.escapeRegex', () => { + assert.strictEqual(core.escapeRegex, phaseId.escapeRegex); + }); + + test('core.normalizePhaseName === phaseId.normalizePhaseName', () => { + assert.strictEqual(core.normalizePhaseName, phaseId.normalizePhaseName); + }); + + test('core.comparePhaseNum === phaseId.comparePhaseNum', () => { + assert.strictEqual(core.comparePhaseNum, phaseId.comparePhaseNum); + }); + + test('core.extractPhaseToken === phaseId.extractPhaseToken', () => { + assert.strictEqual(core.extractPhaseToken, phaseId.extractPhaseToken); + }); + + test('core.phaseTokenMatches === phaseId.phaseTokenMatches', () => { + assert.strictEqual(core.phaseTokenMatches, phaseId.phaseTokenMatches); + }); + + test('core.phaseMarkdownRegexSource === phaseId.phaseMarkdownRegexSource', () => { + assert.strictEqual(core.phaseMarkdownRegexSource, phaseId.phaseMarkdownRegexSource); + }); + + test('core.phaseMarkdownRegexSourceExact === phaseId.phaseMarkdownRegexSourceExact', () => { + assert.strictEqual(core.phaseMarkdownRegexSourceExact, phaseId.phaseMarkdownRegexSourceExact); + }); + + test('core.getMilestoneFromPhaseId === phaseId.getMilestoneFromPhaseId', () => { + assert.strictEqual(core.getMilestoneFromPhaseId, phaseId.getMilestoneFromPhaseId); + }); + + test('core.getPhaseDirFromPhaseId === phaseId.getPhaseDirFromPhaseId', () => { + assert.strictEqual(core.getPhaseDirFromPhaseId, phaseId.getPhaseDirFromPhaseId); + }); +}); From 9e2e71de7b8835c991cf52fd9ca6239de7e6ce0b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 11:30:11 -0400 Subject: [PATCH 6/9] ci(#869): raise full-test Windows lane timeout 15m->20m (#871) The 'full test (windows-latest, 22)' job runs all 656 unit files in one lane and has crept to ~14m+ over recent PRs (12m31s -> 13m45s -> 14m18s), so the 15m cap started cancelling jobs mid-run and red-blocking product PRs even when every test passes (the job is killed on wall-clock, not a test failure). Raise the full-test job timeout to 20m to restore headroom. Durable fix (shard the Windows full unit lane) tracked as a separate follow-up. Closes #869 Co-authored-by: Claude Opus 4.8 --- .github/workflows/test.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d186c6d65..d95860e26 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -265,7 +265,10 @@ jobs: defaults: run: shell: ${{ matrix.shell }} - timeout-minutes: 15 + # 20m, not 15m: the Windows full lane runs all 656 unit files in one job and + # has crept to ~14m+, so 15m started cancelling jobs mid-run (#869). Durable + # fix is to shard this lane — tracked separately. + timeout-minutes: 20 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled strategy: From 35174ce9b079958112afc4c9e3f41755fb4d7b44 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 11:46:19 -0400 Subject: [PATCH 7/9] chore(#57): add runtime install no-drift guard tests (#867) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests/issue-57-runtime-install-no-drift.test.cjs protecting the Runtime Install Policy Module boundary (ADR-58) and the explicit Runtime Config Adapter Registry (#60), now that #58/#60/#56 have landed and the seam exists. The guards fail when: - (AC1) supported-runtime metadata is added to an installer/query call site (allRuntimes, the interactive runtimeMap menu) without a matching registry adapter entry — enforced by three-way set equality across allRuntimes, runtimeMap values, and ALLOWED_CONFIG_RUNTIMES. - (AC2) config-mutation dispatch escapes the registry: every intent uses a registry-declared install surface, every permission writer is null or a registry-known runtime, unknown/prototype-key runtimes fail loudly, and a new inline 'runtime === "..."' branch against an unregistered runtime is rejected. Assertions are behavioral (require + reflect on live exports) where behavior can cover the contract (AC3); two annotated structural guards cover what it cannot. Existing installer/runtime-policy/runtime-global-skills suites stay green (AC4). Gates: eslint, lint-test-file-count, full Mac suite (12715 pass / 0 fail) and Linux Docker (14709 pass / 0 fail) all green; Codex adversarial-review, /code-review, and /security-review run with findings addressed. Closes #57 Co-authored-by: Claude Opus 4.8 --- .changeset/runtime-install-no-drift-tests.md | 6 + ...issue-57-runtime-install-no-drift.test.cjs | 201 ++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 .changeset/runtime-install-no-drift-tests.md create mode 100644 tests/issue-57-runtime-install-no-drift.test.cjs diff --git a/.changeset/runtime-install-no-drift-tests.md b/.changeset/runtime-install-no-drift-tests.md new file mode 100644 index 000000000..9981ebfff --- /dev/null +++ b/.changeset/runtime-install-no-drift-tests.md @@ -0,0 +1,6 @@ +--- +type: Changed +pr: 867 +--- +Added no-drift guard tests (`tests/issue-57-runtime-install-no-drift.test.cjs`) that protect the Runtime Install Policy Module boundary (ADR-58) and the explicit Runtime Config Adapter Registry (#60). They fail loudly when supported-runtime metadata is added to an installer call site (`allRuntimes`, the interactive `runtimeMap` menu) without a matching registry adapter entry, or when config-mutation dispatch escapes the registry's declared install surfaces — catching reintroduction of the scattered per-runtime branching those seams removed. + diff --git a/tests/issue-57-runtime-install-no-drift.test.cjs b/tests/issue-57-runtime-install-no-drift.test.cjs new file mode 100644 index 000000000..3725d42ce --- /dev/null +++ b/tests/issue-57-runtime-install-no-drift.test.cjs @@ -0,0 +1,201 @@ +'use strict'; + +// Issue #57 — Runtime Install No-Drift Tests. +// +// Protects the Runtime Install Policy Module boundary (ADR-58) and the explicit +// Runtime Config Adapter Registry (#60) now that the policy boundary (#58), +// explicit adapter registry (#60), and legacy directory-helper retirement (#56) +// have landed. These guards FAIL when: +// +// (AC1) supported-runtime metadata is added to an installer/query call site +// without going through the runtime registry projection, or +// (AC2) config-mutation dispatch bypasses the explicit adapter registry. +// +// (AC3) Assertions are behavioral (require + reflect on live exports) wherever +// behavior can cover the contract; the two source-text assertions are structural +// guards that behavioral checks cannot replace, and are annotated per repo +// convention. (AC4) The existing installer / runtime-policy / runtime-global-skills +// suites must stay green — verified by running them alongside this file, not +// asserted here. +// +// Known INTENTIONAL asymmetries — these are not drift; do not "fix" them by +// tightening the invariants: +// - `grok` appears in runtime-homes.cjs's getGlobalConfigDir switch but NOT in +// the registry / artifact-layout supported sets (it resolves a config-dir home +// but is not an installable artifact target). So runtime-homes' full switch set +// is never tied into the equality invariant — it is only probed forward, per +// installable runtime. +// - getGlobalConfigDir() falls back to ~/.claude for an UNKNOWN runtime instead +// of throwing (a deliberately liberal projection). Only the registry and +// artifact-layout projections are loud gates, so only those are asserted to +// throw on an unknown runtime. +// +// Coverage boundary (deliberate, see #57 follow-up): the structural guard below +// catches a NEW inline `runtime === '...'` branch against an UNREGISTERED runtime. +// It cannot catch a duplicate inline config write added for an ALREADY-registered +// runtime — distinguishing that from the ~169 legitimate per-runtime comparisons in +// the installer requires driving install()/finishInstall() against a mocked +// filesystem and asserting the written surfaces match resolveRuntimeConfigIntent(). +// That behavioral install-driver harness is out of scope for this no-drift pass. +// +// The forward invariant `allRuntimes ⊆ artifact-layout` is already covered by +// tests/install-runtime-artifacts.test.cjs; this file does not duplicate it. + +process.env.GSD_TEST_MODE = '1'; // must precede require of bin/install.js + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const ROOT = path.join(__dirname, '..'); +const LIB = path.join(ROOT, 'gsd-core', 'bin', 'lib'); + +const { allRuntimes, runtimeMap } = require(path.join(ROOT, 'bin', 'install.js')); +const { + resolveRuntimeConfigIntent, + ALLOWED_CONFIG_RUNTIMES, + INSTALL_SURFACES, +} = require(path.join(LIB, 'runtime-config-adapter-registry.cjs')); +const { resolveRuntimeArtifactLayout } = require( + path.join(LIB, 'runtime-artifact-layout.cjs'), +); +const { getGlobalConfigDir } = require(path.join(LIB, 'runtime-homes.cjs')); + +const sorted = (iterable) => [...iterable].sort(); + +// A runtime name that is deliberately not real and is not a prototype-chain key. +const SENTINEL = '__drift_sentinel_runtime__'; + +describe('issue-57 AC1 — supported-runtime metadata has one projected source of truth', () => { + test('installer allRuntimes, interactive runtimeMap, and registry agree on the supported set', () => { + const installable = sorted(allRuntimes); + assert.deepStrictEqual( + installable, + sorted(Object.values(runtimeMap)), + 'Drift: bin/install.js `allRuntimes` and the interactive `runtimeMap` selection menu ' + + 'diverged. A runtime selectable in the prompt but absent from allRuntimes (or vice ' + + 'versa) is a supported-runtime call site that skipped the projection.', + ); + assert.deepStrictEqual( + installable, + sorted(ALLOWED_CONFIG_RUNTIMES), + 'Drift: bin/install.js `allRuntimes` and `ALLOWED_CONFIG_RUNTIMES` (runtime config ' + + 'adapter registry) diverged. A runtime added to an installer call site without a ' + + 'registry adapter entry bypasses the registry projection — register it in ' + + 'src/runtime-config-adapter-registry.cts.', + ); + }); + + test('every installable runtime resolves a config intent through the registry', () => { + for (const runtime of allRuntimes) { + const intent = resolveRuntimeConfigIntent(runtime); + assert.equal( + intent.runtime, + runtime, + `${runtime} must resolve its own config intent through resolveRuntimeConfigIntent`, + ); + } + }); + + test('every installable runtime resolves a global config dir through runtime-homes', () => { + for (const runtime of allRuntimes) { + const dir = getGlobalConfigDir(runtime); + assert.equal(typeof dir, 'string', `${runtime} config dir must be a string`); + assert.ok(dir.length > 0, `${runtime} must resolve a non-empty global config dir`); + } + }); +}); + +describe('issue-57 AC2 — config-mutation dispatch is closed over the explicit registry', () => { + test('every config intent uses a registry-declared install surface', () => { + const surfaces = new Set(INSTALL_SURFACES); + for (const runtime of allRuntimes) { + const { installSurface } = resolveRuntimeConfigIntent(runtime); + assert.ok( + surfaces.has(installSurface), + `${runtime} dispatches config via unregistered surface "${installSurface}" — add it ` + + 'to INSTALL_SURFACES in the registry instead of branching on it inline.', + ); + } + }); + + test('every finishInstall permission writer is null or a registry-known runtime', () => { + // Registry-derived (no hand-maintained vocabulary): a permission writer either + // names a runtime that is itself in the registry, or is null. A writer pointing + // at an unregistered runtime would mean finishInstall dispatches a config mutation + // outside the registry's known set. + for (const runtime of allRuntimes) { + const { finishPermissionWriter } = resolveRuntimeConfigIntent(runtime); + assert.ok( + finishPermissionWriter === null || ALLOWED_CONFIG_RUNTIMES.has(finishPermissionWriter), + `${runtime} uses finishPermissionWriter "${finishPermissionWriter}", which is neither ` + + 'null nor a registry-known runtime — route it through a registered adapter.', + ); + } + }); + + test('unknown runtime fails loudly through both strict projections (no silent fallthrough)', () => { + assert.throws( + () => resolveRuntimeConfigIntent(SENTINEL), + TypeError, + 'config adapter registry must reject an unknown runtime, not dispatch it silently', + ); + assert.throws( + () => resolveRuntimeArtifactLayout(SENTINEL, path.join(os.tmpdir(), 'gsd-57'), 'global'), + TypeError, + 'artifact-layout projection must reject an unknown runtime', + ); + }); + + test('registry rejects prototype-chain keys (no proto-pollution dispatch bypass)', () => { + for (const key of ['__proto__', 'constructor', 'prototype', 'toString']) { + assert.throws( + () => resolveRuntimeConfigIntent(key), + TypeError, + `${key} must throw, not resolve via the prototype chain`, + ); + } + }); + + // allow-test-rule: structural guard over bin/install.js source. Behavioral assertions + // cannot observe inline `runtime === '...'` config branching, so this enforces that + // every inline per-runtime branch references a runtime the adapter registry knows + // about — a NEW branch against an unregistered runtime name fails here. It matches + // positive equality only (`runtime === ''` / `runtime === ""`, both quote + // styles), so `runtime !== 'string'`-style type guards are not implicated. See the + // "coverage boundary" note at the top of the file for what this can and cannot catch. + test('every inline `runtime === "..."` branch references a registry-known runtime', () => { + const src = fs.readFileSync(path.join(ROOT, 'bin', 'install.js'), 'utf8'); + const literals = new Set( + [...src.matchAll(/runtime === (?:'([a-z][a-z0-9-]*)'|"([a-z][a-z0-9-]*)")/g)] + .map((m) => m[1] ?? m[2]), + ); + assert.ok(literals.size > 0, 'expected to find inline runtime comparisons in bin/install.js'); + const unregistered = [...literals].filter((r) => !ALLOWED_CONFIG_RUNTIMES.has(r)); + assert.deepStrictEqual( + unregistered, + [], + `inline 'runtime === "..."' branch(es) reference runtimes absent from the config adapter ` + + `registry: ${unregistered.join(', ')} — register them in ` + + 'src/runtime-config-adapter-registry.cts or route the logic through ' + + 'resolveRuntimeConfigIntent instead of branching inline.', + ); + }); + + // allow-test-rule: delegation-presence guard. Catches wholesale removal of the registry + // dispatch (a regression to scattered per-runtime config branching). Presence-style, not + // absence-grep, so it does not bite on incidental non-config `runtime === '...'` checks. + test('bin/install.js requires the config adapter registry and dispatches through it', () => { + const src = fs.readFileSync(path.join(ROOT, 'bin', 'install.js'), 'utf8'); + assert.ok( + src.includes('runtime-config-adapter-registry'), + 'bin/install.js no longer requires the runtime config adapter registry', + ); + assert.ok( + src.includes('resolveRuntimeConfigIntent('), + 'bin/install.js no longer dispatches config through resolveRuntimeConfigIntent', + ); + }); +}); From a480510f54c2ad5b62ef56dcab677a319e1b2e06 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 12:04:26 -0400 Subject: [PATCH 8/9] fix(#872): make roadmap-phase-fallback tests hermetic against ambient GSD env (#873) extractCurrentMilestone reads STATE.md via planningDir(cwd), which is workstream-aware (honours GSD_PROJECT/GSD_WORKSTREAM). The fixtures write STATE.md to the plain /.planning/STATE.md, so a developer shell inside a GSD workstream (GSD_WORKSTREAM exported) redirected the read to a non-existent workstream subdir -> version=null -> closed milestone sections leaked into the slice and assertions failed. Clean CI/Docker env never hit it. Not a Node-26 regex bug; reproduces identically on any Node with GSD_WORKSTREAM set. - scripts/run-tests.cjs: strip GSD_PROJECT/GSD_WORKSTREAM before spawning test children so the local runner env matches clean CI/Docker. - tests/roadmap-phase-fallback.test.cjs: file-level beforeEach/afterEach save/delete/restore of both vars; new regression test pinning workstream-aware STATE.md resolution. - tests/run-tests-harness.test.cjs: guard asserting the runner strips both vars (so removing the deletion fails clean CI). Closes #872 Co-authored-by: Claude Opus 4.8 --- scripts/run-tests.cjs | 8 ++++ tests/roadmap-phase-fallback.test.cjs | 65 +++++++++++++++++++++++++++ tests/run-tests-harness.test.cjs | 26 +++++++++++ 3 files changed, 99 insertions(+) diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 810438534..133d9c9db 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -236,6 +236,14 @@ function main() { // Build the gitignored bin/lib artifact if absent, before any test requires it. ensureBuiltArtifacts(); + // Hermeticity: in-process tests resolve `.planning` via planningDir(cwd), which + // honours GSD_PROJECT/GSD_WORKSTREAM. A developer shell inside a GSD workstream + // exports GSD_WORKSTREAM, which would redirect fixture STATE.md reads away from + // each /.planning and silently diverge from the clean CI/Docker env. Strip + // them so the local runner matches CI; tests that need them set them explicitly. + delete process.env.GSD_PROJECT; + delete process.env.GSD_WORKSTREAM; + // Log selected files to stderr for CI / harness-test visibility. // node:test default reporter doesn't echo filenames, so this gives // operators a single stable line they can grep. diff --git a/tests/roadmap-phase-fallback.test.cjs b/tests/roadmap-phase-fallback.test.cjs index 4587b5dd3..beac818ec 100644 --- a/tests/roadmap-phase-fallback.test.cjs +++ b/tests/roadmap-phase-fallback.test.cjs @@ -11,6 +11,26 @@ const fs = require('fs'); const path = require('path'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +// The planning-dir resolver (planningDir) is workstream-aware and honours +// GSD_PROJECT / GSD_WORKSTREAM. These suites write STATE.md to /.planning +// and assume that is where it is read from, so a developer shell inside a GSD +// workstream would otherwise redirect the read and break extractCurrentMilestone. +// Isolate the vars so the file is hermetic when run directly via `node --test`. +let savedGsdProject; +let savedGsdWorkstream; +beforeEach(() => { + savedGsdProject = process.env.GSD_PROJECT; + savedGsdWorkstream = process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.GSD_WORKSTREAM; +}); +afterEach(() => { + if (savedGsdProject !== undefined) process.env.GSD_PROJECT = savedGsdProject; + else delete process.env.GSD_PROJECT; + if (savedGsdWorkstream !== undefined) process.env.GSD_WORKSTREAM = savedGsdWorkstream; + else delete process.env.GSD_WORKSTREAM; +}); + /** * Helper: write STATE.md with a milestone version so extractCurrentMilestone * will slice the roadmap to only that milestone's section. @@ -365,6 +385,51 @@ This is the active milestone body. ); }); + test('(7) workstream-aware: STATE.md under GSD_WORKSTREAM is read from the workstream subdir', () => { + // Regression guard for the env-leak that made these suites pass in clean CI but + // fail in a developer's GSD_WORKSTREAM shell. planningDir() is workstream-aware, + // so STATE.md lives at /.planning/workstreams//STATE.md. Setting the env + // here makes clean CI exercise the polluted-env resolution path. + process.env.GSD_WORKSTREAM = 'guard-ws'; + try { + const wsPlanning = path.join(tmpDir, '.planning', 'workstreams', 'guard-ws'); + fs.mkdirSync(wsPlanning, { recursive: true }); + fs.writeFileSync(path.join(wsPlanning, 'STATE.md'), '---\nmilestone: v8.0\n---\n'); + const roadmap = `# Project Roadmap + +## v8.0 Overview — v8.0-F (CLOSED FAIL 2026-05-18) + +This is the closed milestone body with some text. + +### Phase 24: ARCHIVED +**Goal:** This phase is done and archived. + +## v8.0-B Overview (STARTED 2026-05-18) + +This is the active milestone body. + +### Phase 31: EVAL +**Goal:** Evaluate the new system. + +## v9.0 Future Milestone + +### Phase 40: FUTURE +**Goal:** Future work. +`; + const slice = core.extractCurrentMilestone(roadmap, tmpDir); + assert.ok( + slice.includes('Phase 31: EVAL'), + 'workstream-scoped STATE.md must select the active v8.0-B section', + ); + assert.ok( + !slice.includes('Phase 24: ARCHIVED'), + 'closed section must still be excluded under a workstream env', + ); + } finally { + delete process.env.GSD_WORKSTREAM; + } + }); + test('(2) double-closed-skip: third sibling (active) selected when first two are closed', () => { writeState(tmpDir, 'v9.0'); const roadmap = `# Project Roadmap diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index c819000de..343ca5fb8 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -260,6 +260,32 @@ test('boom', () => { throw new Error('intentional'); }); }); }); + describe('env hermeticity', () => { + // Regression guard for the two `delete process.env.GSD_PROJECT/GSD_WORKSTREAM` + // lines added in scripts/run-tests.cjs main() right after ensureBuiltArtifacts(). + // If those deletions are removed, the fixture's assertions fail inside the child + // node:test process → non-zero harness exit → this test fails → CI catches it. + test('harness strips GSD_PROJECT and GSD_WORKSTREAM before running child tests', () => { + // Write a fixture that asserts both vars are absent in the child process env. + const FIXTURE = `'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +test('ambient GSD workstream vars are stripped by the runner', () => { + assert.strictEqual(process.env.GSD_PROJECT, undefined); + assert.strictEqual(process.env.GSD_WORKSTREAM, undefined); +}); +`; + fs.writeFileSync(path.join(tmpDir, 'env-hermeticity.test.cjs'), FIXTURE, 'utf8'); + // Pass both vars in the ambient env given to the harness process. + // The harness must delete them before spawning the child node:test process. + const r = runHarness(tmpDir, [], { + GSD_PROJECT: 'ambient-proj', + GSD_WORKSTREAM: 'ambient-ws', + }); + assert.strictEqual(r.status, 0, r.stderr); + }); + }); + describe('Windows argv-overflow chunking (issue #3597)', () => { // Windows CreateProcess caps lpCommandLine at 32,767 chars. With ~550 // tests the unchunked spawn fails instantly on Windows with no test From fa1118afa73c5dd899359fc884f23faca79a2822 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 12:34:17 -0400 Subject: [PATCH 9/9] refactor(#870): extract ROADMAP.md parsing into roadmap-parser.cts (#876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-857 rollout phase 2b. Move the 6 ROADMAP.md-parsing functions (stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, getRoadmapPhaseInternal, getMilestoneInfo, getMilestonePhaseFilter) + their interfaces out of core.cts into a new leaf module src/roadmap-parser.cts. core.cts re-exports them (behavior-preserving); ~9 external callers unchanged. Resolves the parse/write straddle: roadmap.cts (ROADMAP.md mutation) now imports its 3 parsing helpers from roadmap-parser.cjs directly instead of reaching through core. roadmap-parser depends only on leaves (phase-id, planning-workspace, shell-command-projection) — cycle-free, enabled by phase 2a. New-CLI-module checklist done (.gitignore, eslint, INVENTORY 92->93 + row, manifest, ARCHITECTURE, CONTEXT.md "Roadmap Parser Module"). Adds tests/roadmap-parser.test.cjs (46 tests: behavioral + shim-identity + adversarial ROADMAP.md fixtures). Adversarial review surfaced a pre-existing fence-blindness bug in getMilestonePhaseFilter (matches phase headings inside fenced code blocks); filed as #875 and left for a separate fix (out of scope for this behavior-preserving extraction). The two fenced-fixture tests characterize the current behavior with a #875 reference and flip when it's fixed. Gates: lint, code-review, security-review, codex adversarial-review (0 correctness findings). gsd-test: clean-build docker + Mac green; the full-suite docker run's "X is not a function" errors on re-exported symbols were local incremental-tsc staleness (verified: clean rebuild of the affected files = 195 pass, 0 fail; Mac = 4001 pass). Closes #870 Co-authored-by: Claude Opus 4.8 --- .gitignore | 1 + CONTEXT.md | 3 + docs/ARCHITECTURE.md | 1 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- eslint.config.mjs | 1 + src/core.cts | 435 +------------------------- src/roadmap-parser.cts | 469 ++++++++++++++++++++++++++++ src/roadmap.cts | 5 +- tests/roadmap-parser.test.cjs | 562 ++++++++++++++++++++++++++++++++++ 10 files changed, 1053 insertions(+), 428 deletions(-) create mode 100644 src/roadmap-parser.cts create mode 100644 tests/roadmap-parser.test.cjs diff --git a/.gitignore b/.gitignore index 43e98f2cb..6819519db 100644 --- a/.gitignore +++ b/.gitignore @@ -129,6 +129,7 @@ build/ /gsd-core/bin/lib/core.cjs /gsd-core/bin/lib/io.cjs /gsd-core/bin/lib/phase-id.cjs +/gsd-core/bin/lib/roadmap-parser.cjs /gsd-core/bin/lib/drift.cjs /gsd-core/bin/lib/cjs-command-router-adapter.cjs /gsd-core/bin/lib/phase-command-router.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 994670b42..92a884183 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -112,6 +112,9 @@ Primary installer for all runtimes. Single production file: `bin/install.js` (ge ### I/O Module Module owning the tool's CLI I/O primitives: `output()` result emission (with large-payload temp-file spillover via `GSD_TEMP_DIR`/`ensureGsdTempDir`/`reapStaleTempFiles`), `error()` stderr emission with exit-code mapping, and the JSON-error-mode toggle (`setJsonErrorMode`/`getJsonErrorMode`, `ERROR_REASON`). Extracted from the Core module per ADR-857 rollout phase 1 (#859) so feature modules (`graphify`, `intel`, `audit`, `profile-pipeline`) depend on a small I/O seam instead of the core god-module; `core.cjs` re-exports the primitives for back-compat. Source of truth: `gsd-core/bin/lib/io.cjs` (generated from `src/io.cts`). +### Roadmap Parser Module +Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; `core.cjs` re-exports the helpers for back-compat. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`). + ### Package Identity Module [Planned] Single seam owning GSD's published-package coordinates so a repoint/rename is a one-line change instead of a tree-wide sweep. Source of truth is `package.json`; values are *derived*, not re-typed: `packageName` (`.name` → `@opengsd/get-shit-done-redux`), `binName` (`Object.keys(.bin)[0]` → `get-shit-done-redux`), `repoSlug` (parsed from `.repository.url` → `open-gsd/get-shit-done-redux`), plus derived `changelogRawUrl` and `manualInstallCommand({ scope, runtime })`. Generated `.cjs` per ADR-457 (generated-single-source); shipped under `gsd-core/bin/lib/`. Three consumer worlds: **Node** consumers `require()` it at runtime (worker, `check-latest-version.cjs`, `bin/install.js`); the **bash launcher** snippet receives the literal injected by `scripts/sync-runtime-launcher.cjs` at sync time; **prose/help** literals (`update.md`, installer help) carry a committed copy. A drift-guard lint (`scripts/lint-package-identity-drift.cjs`, sibling to `check:alias-drift`) fails CI on any raw package/repo literal outside `package.json`, the generated module, and the value-checked materialization sites — this is what keeps the seam real (`two adapters`, not one). Replaces the contradictory pair it consolidates: the runtime-broken `require('../package.json').name` in `hooks/gsd-check-update-worker.js` (#378, resolves to `undefined` post-install) and the hardcoded constant in `check-latest-version.cjs` (#2992). _Avoid_: "package name string", "the npm name" (when you mean the seam). See ADR-457 and Installer Module. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 05bdbd192..ea83a45eb 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -345,6 +345,7 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core | `core.cjs` | Shared utilities; compatibility re-exports for planning, I/O (`io.cjs`), and phase-id helpers | | `io.cjs` | CLI I/O primitives — output/error emission, JSON-error mode, large-payload temp-file spillover | | `phase-id.cjs` | Pure phase-id parsing/matching helpers — normalize, token match, regex builders (extracted from `core.cjs`, ADR-857) | +| `roadmap-parser.cjs` | ROADMAP.md parsing — milestone slicing, current-milestone extraction, phase/milestone lookups, milestone-phase filter (extracted from `core.cjs`, ADR-857) | | `planning-workspace.cjs` | Planning seam (`planningDir`, `planningPaths`, active workstream routing, `.planning/.lock`) | | `state.cjs` | STATE.md parsing, updating, progression, metrics | | `phase.cjs` | Phase directory operations, decimal numbering, plan indexing | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 634a0fc41..31d1c607d 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -324,6 +324,7 @@ "research-store.cjs", "review-reviewer-selection.cjs", "roadmap-command-router.cjs", + "roadmap-parser.cjs", "roadmap-upgrade.cjs", "roadmap.cjs", "runtime-artifact-layout.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index d99ced286..21458ddac 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -370,7 +370,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (92 shipped) +## CLI Modules (93 shipped) Full listing: `gsd-core/bin/lib/*.cjs`. @@ -435,6 +435,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `research-store.cjs` | Content-addressed research cache: sha256 keys, per-source TTL staleness, two-tier (user ~/.gsd / project .planning) store | | `review-reviewer-selection.cjs` | Reviewer selection/normalization helpers for `/gsd-review` default reviewer policy and precedence | | `roadmap-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools roadmap` | +| `roadmap-parser.cjs` | ROADMAP.md parsing — milestone slicing, current-milestone extraction, phase/milestone lookups, milestone-phase filter (extracted from `core.cjs`, ADR-857) | | `roadmap-upgrade.cjs` | Migration tool for converting legacy `Phase N` entries to milestone-prefixed `Phase M-NN` convention; `computeMigrationPlan` + `applyMigration` with dry-run default and atomic rollback | | `roadmap.cjs` | ROADMAP.md parsing, phase extraction, plan progress | | `runtime-artifact-layout.cjs` | Runtime artifact layout module — resolves the artifact directory shapes (commands, agents, skills) for each supported runtime; single source of truth for per-runtime artifact placement (#3663) | diff --git a/eslint.config.mjs b/eslint.config.mjs index 757642109..a50b32cd5 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -91,6 +91,7 @@ export default tseslint.config( 'gsd-core/bin/lib/core.cjs', 'gsd-core/bin/lib/io.cjs', 'gsd-core/bin/lib/phase-id.cjs', + 'gsd-core/bin/lib/roadmap-parser.cjs', 'gsd-core/bin/lib/drift.cjs', 'gsd-core/bin/lib/cjs-command-router-adapter.cjs', 'gsd-core/bin/lib/phase-command-router.cjs', diff --git a/src/core.cts b/src/core.cts index 23b986e93..d608163c4 100644 --- a/src/core.cts +++ b/src/core.cts @@ -17,6 +17,9 @@ const { output, error, ERROR_REASON, setJsonErrorMode, getJsonErrorMode, GSD_TEM import phaseIdModule = require('./phase-id.cjs'); const { escapeRegex, normalizePhaseName, getMilestoneFromPhaseId, getPhaseDirFromPhaseId, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, comparePhaseNum, extractPhaseToken, phaseTokenMatches } = phaseIdModule; // eslint-disable-next-line @typescript-eslint/no-require-imports +import roadmapParserModule = require('./roadmap-parser.cjs'); +const { stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, getRoadmapPhaseInternal, getMilestoneInfo, getMilestonePhaseFilter } = roadmapParserModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports import modelProfiles = require('./model-profiles.cjs'); const { MODEL_PROFILES, AGENT_TO_PHASE_TYPE, VALID_PHASE_TYPES: _VALID_PHASE_TYPES, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier } = modelProfiles; import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, KNOWN_RUNTIMES, RUNTIMES_WITH_REASONING_EFFORT, RUNTIMES_WITH_FAST_MODE, PROVIDER_PRESETS, KNOWN_PROVIDERS } from './model-catalog.cjs'; @@ -679,235 +682,10 @@ function getArchivedPhaseDirs(cwd: string): ArchivedPhaseDir[] { return results; } -// ─── Roadmap milestone scoping ─────────────────────────────────────────────── - -/** - * Strip shipped milestone content wrapped in
blocks. - */ -function stripShippedMilestones(content: string): string { - return content.replace(/
[\s\S]*?<\/details>/gi, ''); -} - -/** - * Extract the current milestone section from ROADMAP.md by positive lookup. - */ -function extractCurrentMilestone(content: string, cwd?: string): string { - if (!cwd) return stripShippedMilestones(content); - - let version: string | null = null; - try { - const statePath = path.join(planningDir(cwd), 'STATE.md'); - const stateRaw = platformReadSync(statePath); - if (stateRaw !== null) { - const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m); - if (milestoneMatch) { - version = milestoneMatch[1].trim(); - } - } - } catch { /* ignore */ } - - if (!version) { - const inProgressMatch = content.match(/(?:🚧|🔄)\s*\*\*v(\d+\.\d+)\s/); - if (inProgressMatch) { - version = 'v' + inProgressMatch[1]; - } - } - - if (!version) return stripShippedMilestones(content); - - const escapedVersion = escapeRegex(version); - const sectionPattern = new RegExp( - `(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`, - 'gmi' - ); - const summaryPattern = new RegExp( - `]*>([^<]*${escapedVersion}[^<]*)<\\/summary>`, - 'i' - ); - const headingMatches = [...content.matchAll(sectionPattern)]; - - if (headingMatches.length === 0) { - const summaryMatch = content.match(summaryPattern); - if (summaryMatch) { - const summaryIdx = content.indexOf(summaryMatch[0]); - const beforeSummary = content.slice(0, summaryIdx); - const detailsOpenIdx = beforeSummary.lastIndexOf('/i); - const detailsEnd = closingMatch - ? detailsOpenIdx + (closingMatch.index ?? 0) + '
'.length - : content.length; - const anyMilestoneOrDetails = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧|🔄)|
[\s\S]*?<\/details>/gi, '') - .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') - .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); - return preamble + content.slice(detailsOpenIdx, detailsEnd); - } - } - return stripShippedMilestones(content); - } - - const allMatches = headingMatches; - - const closedMarkerPattern = /\b(?:CLOSED|ARCHIVED|ABANDONED|SHIPPED|FAILED)\b|✅|🗄/i; - const activeMarkerPattern = /\b(?:STARTED|ACTIVE|WIP)\b|in\s+progress|🚧|🔄/i; - const isClosed = (h: string) => closedMarkerPattern.test(h) && !activeMarkerPattern.test(h); - const firstMatch = allMatches[0]; - const selected = allMatches.find((m) => !isClosed(m[1])) || firstMatch; - - const sectionStart = selected.index; - - const computeSectionEnd = (headingText: string, headingStart: number): number => { - const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length; - const rest = content.slice(headingStart + headingText.length); - const stopPattern = new RegExp( - `^#{1,${level}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, - 'i', - ); - let end = content.length; - let fc: string | null = null; - let fl = 0; - let off = 0; - for (const line of rest.split('\n')) { - const fm = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/); - if (fm) { - const ch = fm[1][0]; - const ln = fm[1].length; - const trailing = fm[2] || ''; - if (!fc) { - fc = ch; - fl = ln; - } else if (ch === fc && ln >= fl && /^\s*$/.test(trailing)) { - fc = null; - fl = 0; - } - } else if (!fc && stopPattern.test(line)) { - end = headingStart + headingText.length + off; - break; - } - off += line.length + 1; - } - return end; - }; - - const sectionEnd = computeSectionEnd(selected[0], sectionStart); - - const anyMilestonePattern = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧)/im; - const firstMilestoneMatch = content.match(anyMilestonePattern); - const preambleCutoff = firstMilestoneMatch - ? firstMilestoneMatch.index! - : firstMatch.index; - const beforeMilestones = content.slice(0, preambleCutoff); - const currentSection = content.slice(sectionStart, sectionEnd); - - // Multi-milestone roadmaps split each added milestone across two version-bearing - // headings: a `## Phases` checklist subsection (early) and a dedicated - // `## Milestone … (Phase Details)` section (late) holding the `### Phase N:` - // detail headers. The scope window above stops at the next version-bearing - // heading — the current milestone's OWN Phase Details heading — leaving those - // detail headers outside `currentSection`. Append that section so phase - // resolution and counting see the current milestone's phases. Anchor the lookup - // to the SELECTED heading's specific version token (boundary-aware, so a - // `v3.0` state does not match a `v3.0-A` sub-milestone) so sibling milestones - // that share a version prefix do not cross-pollinate. (#730) - const selectedVersionToken = selected[1].match( - /v\d+(?:\.\d+)+(?:[-.][A-Za-z0-9]+)*/i, - )?.[0]; - const detailsVersionBoundary = selectedVersionToken - ? new RegExp(`${escapeRegex(selectedVersionToken)}(?![\\w.-])`, 'i') - : null; - let detailsSection = ''; - const detailsMatch = allMatches.find( - (m) => - /\(Phase\s+Details\)/i.test(m[1]) && - !isClosed(m[1]) && - (!detailsVersionBoundary || detailsVersionBoundary.test(m[1])) && - (m.index ?? 0) >= sectionEnd, - ); - if (detailsMatch) { - const detailsStart = detailsMatch.index ?? 0; - detailsSection = content.slice( - detailsStart, - computeSectionEnd(detailsMatch[0], detailsStart), - ); - } - - const preamble = beforeMilestones - .replace(/
[\s\S]*?<\/details>/gi, '') - .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') - .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); - - return detailsSection - ? preamble + currentSection + '\n' + detailsSection - : preamble + currentSection; -} - -/** - * Replace a pattern only in the current milestone section of ROADMAP.md. - */ -function replaceInCurrentMilestone(content: string, pattern: RegExp, replacement: string): string { - const lastDetailsClose = content.lastIndexOf('
'); - if (lastDetailsClose === -1) { - return content.replace(pattern, replacement); - } - const offset = lastDetailsClose + '
'.length; - const before = content.slice(0, offset); - const after = content.slice(offset); - return before + after.replace(pattern, replacement); -} - -// ─── Roadmap & model utilities ──────────────────────────────────────────────── - -interface RoadmapPhaseResult { - found: boolean; - phase_number: string; - phase_name: string; - goal: string | null; - section: string; -} - -function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { - if (!phaseNum) return null; - const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); - if (!fs.existsSync(roadmapPath)) return null; - - try { - const roadmapRaw = platformReadSync(roadmapPath); - if (roadmapRaw === null) throw new Error('missing'); - const content = extractCurrentMilestone(roadmapRaw, cwd); - const phasePattern = new RegExp( - `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, - 'i' - ); - const headerMatch = content.match(phasePattern); - if (!headerMatch) return null; - - const phaseName = headerMatch[1].trim(); - const headerIndex = headerMatch.index!; - const restOfContent = content.slice(headerIndex); - const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i); - const sectionEnd = nextHeaderMatch ? headerIndex + nextHeaderMatch.index! : content.length; - const section = content.slice(headerIndex, sectionEnd).trim(); - - const goalMatch = section.match(/\*\*Goal(?:\*\*:|\*?\*?:\*\*)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; - - return { - found: true, - // eslint-disable-next-line @typescript-eslint/no-base-to-string - phase_number: String(phaseNum), - phase_name: phaseName, - goal, - section, - }; - } catch { - return null; - } -} +// ─── Roadmap milestone scoping (re-exported from roadmap-parser.cjs) ────────── +// stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, +// getRoadmapPhaseInternal, getMilestoneInfo, getMilestonePhaseFilter +// — all imported via `roadmapParserModule` above; internal callers use the destructured bindings. // ─── Agent installation validation (#1371) ─────────────────────────────────── @@ -1596,203 +1374,8 @@ function generateSlugInternal(text: string | null | undefined): string | null { return text.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '').substring(0, 60); } -interface MilestoneInfo { - version: string; - name: string; -} - -function getMilestoneInfo(cwd: string): MilestoneInfo { - try { - const roadmap = platformReadSync(path.join(planningDir(cwd), 'ROADMAP.md')); - if (roadmap === null) throw new Error('missing'); - - let stateVersion: string | null = null; - if (cwd) { - try { - const statePath = path.join(planningDir(cwd), 'STATE.md'); - const stateRaw = platformReadSync(statePath); - if (stateRaw !== null) { - const m = stateRaw.match(/^milestone:\s*(.+)/m); - if (m) stateVersion = m[1].trim(); - } - } catch { /* intentionally empty */ } - } - - if (stateVersion) { - const escapedVer = escapeRegex(stateVersion); - const headingMatch = roadmap.match( - new RegExp(`##[^\\n]*${escapedVer}[:\\s]+([^\\n(]+)`, 'i') - ); - if (headingMatch) { - if (!headingMatch[0].includes('✅')) { - return { version: stateVersion, name: headingMatch[1].trim() }; - } - } else { - const listMatch = roadmap.match( - new RegExp(`🚧\\s*\\*?\\*?${escapedVer}\\s+([^*\\n]+)`, 'i') - ); - if (listMatch) { - return { version: stateVersion, name: listMatch[1].trim() }; - } - return { version: stateVersion, name: 'milestone' }; - } - } - - const inProgressMatch = roadmap.match(/🚧\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); - if (inProgressMatch) { - return { - version: 'v' + inProgressMatch[1], - name: inProgressMatch[2].trim(), - }; - } - - const cleaned = stripShippedMilestones(roadmap); - const headingMatch = cleaned.match(/## (?!.*✅).*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); - if (headingMatch) { - return { - version: 'v' + headingMatch[1], - name: headingMatch[2].trim(), - }; - } - const versionMatch = cleaned.match(/v(\d+(?:\.\d+)+)/); - return { - version: versionMatch ? versionMatch[0] : 'v1.0', - name: 'milestone', - }; - } catch { - return { version: 'v1.0', name: 'milestone' }; - } -} - -type MilestonePhaseFilter = ((dirName: string) => boolean) & { - phaseCount: number; - missingExplicitVersion: boolean; -}; - -/** - * Returns a filter function that checks whether a phase directory belongs - * to the current milestone based on ROADMAP.md phase headings. - */ -function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null): MilestonePhaseFilter { - const milestonePhaseNums = new Set(); - let missingExplicitVersion = false; - try { - const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); - const roadmapContent = platformReadSync(roadmapPath); - if (roadmapContent === null) throw new Error('missing'); - let roadmap = extractCurrentMilestone(roadmapContent, cwd); - - const hasVersionedMilestonesGlobal = /^#{1,3}\s+.*v\d+\.\d+/mi.test(roadmapContent); - const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i.test(roadmapContent); - if (!hasVersionedMilestonesGlobal && hasPhaseHeadings) { - console.warn( - '[gsd] Deprecated: free-form ROADMAP.md detected (no versioned milestone headings). ' + - 'Set phase_id_convention in config.json to suppress this warning.' - ); - } - - if (versionOverride) { - const escapedVersion = escapeRegex(versionOverride); - const sectionPattern = new RegExp(`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}[^\\n]*)`, 'mi'); - let sectionMatch = roadmapContent.match(sectionPattern); - - if (!sectionMatch) { - const summaryPat = new RegExp(`]*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i'); - const summaryHit = roadmapContent.match(summaryPat); - if (summaryHit) { - const beforeSummary = roadmapContent.slice(0, summaryHit.index); - const detailsIdx = beforeSummary.lastIndexOf(']*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i').test(roadmapContent); - if (hasVersionedMilestones && !versionInSummary) { - roadmap = ''; - missingExplicitVersion = true; - } - } else { - const sectionStart = sectionMatch.index!; - const headingLevel = (sectionMatch[1].match(/^(#{1,3})\s/) ?? ['', '#'])[1].length; - const restContent = roadmapContent.slice(sectionStart + sectionMatch[0].length); - const nextMilestonePattern = new RegExp(`^#{1,${headingLevel}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, 'i'); - - let sectionEnd = roadmapContent.length; - let fenceChar: string | null = null; - let fenceLen = 0; - let charOffset = 0; - for (const line of restContent.split('\n')) { - const fenceMatch = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/); - if (fenceMatch) { - const char = fenceMatch[1][0]; - const len = fenceMatch[1].length; - const trailing = fenceMatch[2] || ''; - if (!fenceChar) { - fenceChar = char; - fenceLen = len; - } else if (char === fenceChar && len >= fenceLen && /^\s*$/.test(trailing)) { - fenceChar = null; - fenceLen = 0; - } - } else if (!fenceChar && nextMilestonePattern.test(line)) { - sectionEnd = sectionStart + sectionMatch[0].length + charOffset; - break; - } - charOffset += line.length + 1; - } - - const currentSection = roadmapContent.slice(sectionStart, sectionEnd); - roadmap = currentSection; - } - } - - const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)\s*:/gi; - let m: RegExpExecArray | null; - while ((m = phasePattern.exec(roadmap)) !== null) { - milestonePhaseNums.add(m[1]); - } - } catch { /* intentionally empty */ } - - if (milestonePhaseNums.size === 0) { - const passAll = (() => true) as unknown as MilestonePhaseFilter; - passAll.phaseCount = 0; - passAll.missingExplicitVersion = missingExplicitVersion; - return passAll; - } - - const normalized = new Set( - [...milestonePhaseNums].map(n => n.split('-').map(seg => (seg.replace(/^0+(?=\d)/, '') || '0')).join('-').toLowerCase()) - ); - - function normalizePhaseIdSegments(id: string): string { - return id.split('-').map(seg => seg.replace(/^0+(?=\d)/, '') || '0').join('-'); - } - - const roadmapUsesHyphenedIds = [...normalized].some(n => n.includes('-')); - const numericRe = roadmapUsesHyphenedIds - ? /^0*(\d+(?:-0*\d+)*[A-Za-z]?(?:\.\d+)*)/ - : /^0*(\d+[A-Za-z]?(?:\.\d+)*)/; - - function isDirInMilestone(dirName: string): boolean { - const m2 = dirName.match(numericRe); - if (m2 && normalized.has(normalizePhaseIdSegments(m2[1]).toLowerCase())) return true; - const customMatch = dirName.match(/^([A-Za-z][A-Za-z0-9]*(?:-[A-Za-z0-9]+)*)/); - if (customMatch && normalized.has(customMatch[1].toLowerCase())) return true; - const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); - if (stripped !== dirName) { - const sm = stripped.match(numericRe); - if (sm && normalized.has(normalizePhaseIdSegments(sm[1]).toLowerCase())) return true; - } - return false; - } - (isDirInMilestone as MilestonePhaseFilter).phaseCount = milestonePhaseNums.size; - (isDirInMilestone as MilestonePhaseFilter).missingExplicitVersion = missingExplicitVersion; - return isDirInMilestone as MilestonePhaseFilter; -} +// MilestoneInfo, MilestonePhaseFilter, getMilestoneInfo, getMilestonePhaseFilter +// — all re-exported from roadmap-parser.cjs via roadmapParserModule above. // ─── Phase file helpers ────────────────────────────────────────────────────── diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts new file mode 100644 index 000000000..9e9c15cba --- /dev/null +++ b/src/roadmap-parser.cts @@ -0,0 +1,469 @@ +/** + * Roadmap Parser — ROADMAP.md parsing helpers + * + * ADR-857 rollout phase 2b: extracted from core.cts (issue #870). + * Owns shipped-milestone slicing, current-milestone extraction, + * milestone/phase lookups, and milestone-phase filtering. + * Behaviour is preserved byte-for-behaviour from the prior location; + * only the module boundary moved. core.cjs re-exports every symbol here + * under its own `export =` object so existing consumers are unaffected. + * + * New imports should pull roadmap-parser helpers from roadmap-parser.cjs directly. + * + * Dependencies (leaf modules only — no core.cjs, no loadConfig): + * - node:fs / node:path (stdlib) + * - ./phase-id.cjs (escapeRegex, phaseMarkdownRegexSource) + * - ./planning-workspace.cjs (planningDir) + * - ./shell-command-projection.cjs (platformReadSync) + */ + +import fs from 'node:fs'; +import path from 'node:path'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdModule = require('./phase-id.cjs'); +const { escapeRegex, phaseMarkdownRegexSource } = phaseIdModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planningWorkspace = require('./planning-workspace.cjs'); +const { planningDir } = planningWorkspace; +import { platformReadSync } from './shell-command-projection.cjs'; + +// ─── Roadmap milestone scoping ─────────────────────────────────────────────── + +/** + * Strip shipped milestone content wrapped in
blocks. + */ +function stripShippedMilestones(content: string): string { + return content.replace(/
[\s\S]*?<\/details>/gi, ''); +} + +/** + * Extract the current milestone section from ROADMAP.md by positive lookup. + */ +function extractCurrentMilestone(content: string, cwd?: string): string { + if (!cwd) return stripShippedMilestones(content); + + let version: string | null = null; + try { + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const stateRaw = platformReadSync(statePath); + if (stateRaw !== null) { + const milestoneMatch = stateRaw.match(/^milestone:\s*(.+)/m); + if (milestoneMatch) { + version = milestoneMatch[1].trim(); + } + } + } catch { /* ignore */ } + + if (!version) { + const inProgressMatch = content.match(/(?:🚧|🔄)\s*\*\*v(\d+\.\d+)\s/); + if (inProgressMatch) { + version = 'v' + inProgressMatch[1]; + } + } + + if (!version) return stripShippedMilestones(content); + + const escapedVersion = escapeRegex(version); + const sectionPattern = new RegExp( + `(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`, + 'gmi' + ); + const summaryPattern = new RegExp( + `]*>([^<]*${escapedVersion}[^<]*)<\\/summary>`, + 'i' + ); + const headingMatches = [...content.matchAll(sectionPattern)]; + + if (headingMatches.length === 0) { + const summaryMatch = content.match(summaryPattern); + if (summaryMatch) { + const summaryIdx = content.indexOf(summaryMatch[0]); + const beforeSummary = content.slice(0, summaryIdx); + const detailsOpenIdx = beforeSummary.lastIndexOf('/i); + const detailsEnd = closingMatch + ? detailsOpenIdx + (closingMatch.index ?? 0) + '
'.length + : content.length; + const anyMilestoneOrDetails = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧|🔄)|
[\s\S]*?<\/details>/gi, '') + .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') + .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); + return preamble + content.slice(detailsOpenIdx, detailsEnd); + } + } + return stripShippedMilestones(content); + } + + const allMatches = headingMatches; + + const closedMarkerPattern = /\b(?:CLOSED|ARCHIVED|ABANDONED|SHIPPED|FAILED)\b|✅|🗄/i; + const activeMarkerPattern = /\b(?:STARTED|ACTIVE|WIP)\b|in\s+progress|🚧|🔄/i; + const isClosed = (h: string) => closedMarkerPattern.test(h) && !activeMarkerPattern.test(h); + const firstMatch = allMatches[0]; + const selected = allMatches.find((m) => !isClosed(m[1])) || firstMatch; + + const sectionStart = selected.index; + + const computeSectionEnd = (headingText: string, headingStart: number): number => { + const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length; + const rest = content.slice(headingStart + headingText.length); + const stopPattern = new RegExp( + `^#{1,${level}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, + 'i', + ); + let end = content.length; + let fc: string | null = null; + let fl = 0; + let off = 0; + for (const line of rest.split('\n')) { + const fm = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/); + if (fm) { + const ch = fm[1][0]; + const ln = fm[1].length; + const trailing = fm[2] || ''; + if (!fc) { + fc = ch; + fl = ln; + } else if (ch === fc && ln >= fl && /^\s*$/.test(trailing)) { + fc = null; + fl = 0; + } + } else if (!fc && stopPattern.test(line)) { + end = headingStart + headingText.length + off; + break; + } + off += line.length + 1; + } + return end; + }; + + const sectionEnd = computeSectionEnd(selected[0], sectionStart); + + const anyMilestonePattern = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧)/im; + const firstMilestoneMatch = content.match(anyMilestonePattern); + const preambleCutoff = firstMilestoneMatch + ? firstMilestoneMatch.index! + : firstMatch.index; + const beforeMilestones = content.slice(0, preambleCutoff); + const currentSection = content.slice(sectionStart, sectionEnd); + + // Multi-milestone roadmaps split each added milestone across two version-bearing + // headings: a `## Phases` checklist subsection (early) and a dedicated + // `## Milestone … (Phase Details)` section (late) holding the `### Phase N:` + // detail headers. The scope window above stops at the next version-bearing + // heading — the current milestone's OWN Phase Details heading — leaving those + // detail headers outside `currentSection`. Append that section so phase + // resolution and counting see the current milestone's phases. Anchor the lookup + // to the SELECTED heading's specific version token (boundary-aware, so a + // `v3.0` state does not match a `v3.0-A` sub-milestone) so sibling milestones + // that share a version prefix do not cross-pollinate. (#730) + const selectedVersionToken = selected[1].match( + /v\d+(?:\.\d+)+(?:[-.][A-Za-z0-9]+)*/i, + )?.[0]; + const detailsVersionBoundary = selectedVersionToken + ? new RegExp(`${escapeRegex(selectedVersionToken)}(?![\\w.-])`, 'i') + : null; + let detailsSection = ''; + const detailsMatch = allMatches.find( + (m) => + /\(Phase\s+Details\)/i.test(m[1]) && + !isClosed(m[1]) && + (!detailsVersionBoundary || detailsVersionBoundary.test(m[1])) && + (m.index ?? 0) >= sectionEnd, + ); + if (detailsMatch) { + const detailsStart = detailsMatch.index ?? 0; + detailsSection = content.slice( + detailsStart, + computeSectionEnd(detailsMatch[0], detailsStart), + ); + } + + const preamble = beforeMilestones + .replace(/
[\s\S]*?<\/details>/gi, '') + .replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '') + .replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, ''); + + return detailsSection + ? preamble + currentSection + '\n' + detailsSection + : preamble + currentSection; +} + +/** + * Replace a pattern only in the current milestone section of ROADMAP.md. + */ +function replaceInCurrentMilestone(content: string, pattern: RegExp, replacement: string): string { + const lastDetailsClose = content.lastIndexOf('
'); + if (lastDetailsClose === -1) { + return content.replace(pattern, replacement); + } + const offset = lastDetailsClose + '
'.length; + const before = content.slice(0, offset); + const after = content.slice(offset); + return before + after.replace(pattern, replacement); +} + +// ─── Roadmap phase lookup ───────────────────────────────────────────────────── + +interface RoadmapPhaseResult { + found: boolean; + phase_number: string; + phase_name: string; + goal: string | null; + section: string; +} + +function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { + if (!phaseNum) return null; + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + if (!fs.existsSync(roadmapPath)) return null; + + try { + const roadmapRaw = platformReadSync(roadmapPath); + if (roadmapRaw === null) throw new Error('missing'); + const content = extractCurrentMilestone(roadmapRaw, cwd); + const phasePattern = new RegExp( + `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, + 'i' + ); + const headerMatch = content.match(phasePattern); + if (!headerMatch) return null; + + const phaseName = headerMatch[1].trim(); + const headerIndex = headerMatch.index!; + const restOfContent = content.slice(headerIndex); + const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i); + const sectionEnd = nextHeaderMatch ? headerIndex + nextHeaderMatch.index! : content.length; + const section = content.slice(headerIndex, sectionEnd).trim(); + + const goalMatch = section.match(/\*\*Goal(?:\*\*:|\*?\*?:\*\*)\s*([^\n]+)/i); + const goal = goalMatch ? goalMatch[1].trim() : null; + + return { + found: true, + // eslint-disable-next-line @typescript-eslint/no-base-to-string + phase_number: String(phaseNum), + phase_name: phaseName, + goal, + section, + }; + } catch { + return null; + } +} + +// ─── Milestone info lookup ──────────────────────────────────────────────────── + +interface MilestoneInfo { + version: string; + name: string; +} + +function getMilestoneInfo(cwd: string): MilestoneInfo { + try { + const roadmap = platformReadSync(path.join(planningDir(cwd), 'ROADMAP.md')); + if (roadmap === null) throw new Error('missing'); + + let stateVersion: string | null = null; + if (cwd) { + try { + const statePath = path.join(planningDir(cwd), 'STATE.md'); + const stateRaw = platformReadSync(statePath); + if (stateRaw !== null) { + const m = stateRaw.match(/^milestone:\s*(.+)/m); + if (m) stateVersion = m[1].trim(); + } + } catch { /* intentionally empty */ } + } + + if (stateVersion) { + const escapedVer = escapeRegex(stateVersion); + const headingMatch = roadmap.match( + new RegExp(`##[^\\n]*${escapedVer}[:\\s]+([^\\n(]+)`, 'i') + ); + if (headingMatch) { + if (!headingMatch[0].includes('✅')) { + return { version: stateVersion, name: headingMatch[1].trim() }; + } + } else { + const listMatch = roadmap.match( + new RegExp(`🚧\\s*\\*?\\*?${escapedVer}\\s+([^*\\n]+)`, 'i') + ); + if (listMatch) { + return { version: stateVersion, name: listMatch[1].trim() }; + } + return { version: stateVersion, name: 'milestone' }; + } + } + + const inProgressMatch = roadmap.match(/🚧\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); + if (inProgressMatch) { + return { + version: 'v' + inProgressMatch[1], + name: inProgressMatch[2].trim(), + }; + } + + const cleaned = stripShippedMilestones(roadmap); + const headingMatch = cleaned.match(/## (?!.*✅).*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); + if (headingMatch) { + return { + version: 'v' + headingMatch[1], + name: headingMatch[2].trim(), + }; + } + const versionMatch = cleaned.match(/v(\d+(?:\.\d+)+)/); + return { + version: versionMatch ? versionMatch[0] : 'v1.0', + name: 'milestone', + }; + } catch { + return { version: 'v1.0', name: 'milestone' }; + } +} + +// ─── Milestone phase filter ─────────────────────────────────────────────────── + +type MilestonePhaseFilter = ((dirName: string) => boolean) & { + phaseCount: number; + missingExplicitVersion: boolean; +}; + +/** + * Returns a filter function that checks whether a phase directory belongs + * to the current milestone based on ROADMAP.md phase headings. + */ +function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null): MilestonePhaseFilter { + const milestonePhaseNums = new Set(); + let missingExplicitVersion = false; + try { + const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); + const roadmapContent = platformReadSync(roadmapPath); + if (roadmapContent === null) throw new Error('missing'); + let roadmap = extractCurrentMilestone(roadmapContent, cwd); + + const hasVersionedMilestonesGlobal = /^#{1,3}\s+.*v\d+\.\d+/mi.test(roadmapContent); + const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i.test(roadmapContent); + if (!hasVersionedMilestonesGlobal && hasPhaseHeadings) { + console.warn( + '[gsd] Deprecated: free-form ROADMAP.md detected (no versioned milestone headings). ' + + 'Set phase_id_convention in config.json to suppress this warning.' + ); + } + + if (versionOverride) { + const escapedVersion = escapeRegex(versionOverride); + const sectionPattern = new RegExp(`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}[^\\n]*)`, 'mi'); + let sectionMatch = roadmapContent.match(sectionPattern); + + if (!sectionMatch) { + const summaryPat = new RegExp(`]*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i'); + const summaryHit = roadmapContent.match(summaryPat); + if (summaryHit) { + const beforeSummary = roadmapContent.slice(0, summaryHit.index); + const detailsIdx = beforeSummary.lastIndexOf(']*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i').test(roadmapContent); + if (hasVersionedMilestones && !versionInSummary) { + roadmap = ''; + missingExplicitVersion = true; + } + } else { + const sectionStart = sectionMatch.index!; + const headingLevel = (sectionMatch[1].match(/^(#{1,3})\s/) ?? ['', '#'])[1].length; + const restContent = roadmapContent.slice(sectionStart + sectionMatch[0].length); + const nextMilestonePattern = new RegExp(`^#{1,${headingLevel}}\\s+(?!Phase\\s+\\S)(?:.*v\\d+\\.\\d+|✅|📋|🚧)`, 'i'); + + let sectionEnd = roadmapContent.length; + let fenceChar: string | null = null; + let fenceLen = 0; + let charOffset = 0; + for (const line of restContent.split('\n')) { + const fenceMatch = line.match(/^\s{0,3}((?:`{3,}|~{3,}))(.*)/); + if (fenceMatch) { + const char = fenceMatch[1][0]; + const len = fenceMatch[1].length; + const trailing = fenceMatch[2] || ''; + if (!fenceChar) { + fenceChar = char; + fenceLen = len; + } else if (char === fenceChar && len >= fenceLen && /^\s*$/.test(trailing)) { + fenceChar = null; + fenceLen = 0; + } + } else if (!fenceChar && nextMilestonePattern.test(line)) { + sectionEnd = sectionStart + sectionMatch[0].length + charOffset; + break; + } + charOffset += line.length + 1; + } + + const currentSection = roadmapContent.slice(sectionStart, sectionEnd); + roadmap = currentSection; + } + } + + const phasePattern = /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)\s*:/gi; + let m: RegExpExecArray | null; + while ((m = phasePattern.exec(roadmap)) !== null) { + milestonePhaseNums.add(m[1]); + } + } catch { /* intentionally empty */ } + + if (milestonePhaseNums.size === 0) { + const passAll = (() => true) as unknown as MilestonePhaseFilter; + passAll.phaseCount = 0; + passAll.missingExplicitVersion = missingExplicitVersion; + return passAll; + } + + const normalized = new Set( + [...milestonePhaseNums].map(n => n.split('-').map(seg => (seg.replace(/^0+(?=\d)/, '') || '0')).join('-').toLowerCase()) + ); + + function normalizePhaseIdSegments(id: string): string { + return id.split('-').map(seg => seg.replace(/^0+(?=\d)/, '') || '0').join('-'); + } + + const roadmapUsesHyphenedIds = [...normalized].some(n => n.includes('-')); + const numericRe = roadmapUsesHyphenedIds + ? /^0*(\d+(?:-0*\d+)*[A-Za-z]?(?:\.\d+)*)/ + : /^0*(\d+[A-Za-z]?(?:\.\d+)*)/; + + function isDirInMilestone(dirName: string): boolean { + const m2 = dirName.match(numericRe); + if (m2 && normalized.has(normalizePhaseIdSegments(m2[1]).toLowerCase())) return true; + const customMatch = dirName.match(/^([A-Za-z][A-Za-z0-9]*(?:-[A-Za-z0-9]+)*)/); + if (customMatch && normalized.has(customMatch[1].toLowerCase())) return true; + const stripped = dirName.replace(/^[A-Z]{1,6}-(?=\d)/i, ''); + if (stripped !== dirName) { + const sm = stripped.match(numericRe); + if (sm && normalized.has(normalizePhaseIdSegments(sm[1]).toLowerCase())) return true; + } + return false; + } + (isDirInMilestone as MilestonePhaseFilter).phaseCount = milestonePhaseNums.size; + (isDirInMilestone as MilestonePhaseFilter).missingExplicitVersion = missingExplicitVersion; + return isDirInMilestone as MilestonePhaseFilter; +} + +export = { + stripShippedMilestones, + extractCurrentMilestone, + replaceInCurrentMilestone, + getRoadmapPhaseInternal, + getMilestoneInfo, + getMilestonePhaseFilter, +}; diff --git a/src/roadmap.cts b/src/roadmap.cts index 47a021b1c..b5335fa62 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -10,7 +10,10 @@ import fs from 'node:fs'; import path from 'node:path'; // eslint-disable-next-line @typescript-eslint/no-require-imports import core = require('./core.cjs'); -const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = core; +const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, output, error, findPhaseInternal, phaseTokenMatches } = core; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import roadmapParserModule = require('./roadmap-parser.cjs'); +const { stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = roadmapParserModule; import { platformWriteSync } from './shell-command-projection.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs new file mode 100644 index 000000000..36c973fca --- /dev/null +++ b/tests/roadmap-parser.test.cjs @@ -0,0 +1,562 @@ +/** + * roadmap-parser.cjs — unit tests + * + * Covers the 6 functions extracted from core.cjs per ADR-857 rollout + * phase 2b (#870): stripShippedMilestones, extractCurrentMilestone, + * replaceInCurrentMilestone, getRoadmapPhaseInternal, getMilestoneInfo, + * getMilestonePhaseFilter. + * + * Includes: + * - Behavioral tests against realistic ROADMAP.md content + * - Adversarial fixtures (malformed frontmatter, unclosed fences, + * headings inside fences, unicode headings, repeated/decimal phase + * IDs, mixed CRLF/LF) + * - Shim-identity assertions verifying core.cjs re-exports are the + * same function objects as roadmap-parser.cjs exports + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const roadmapParser = require('../gsd-core/bin/lib/roadmap-parser.cjs'); +const core = require('../gsd-core/bin/lib/core.cjs'); +const { createTempProject, cleanup } = require('./helpers.cjs'); + +const { + stripShippedMilestones, + extractCurrentMilestone, + replaceInCurrentMilestone, + getRoadmapPhaseInternal, + getMilestoneInfo, + getMilestonePhaseFilter, +} = roadmapParser; + +// ─── helpers ───────────────────────────────────────────────────────────────── + +function writeRoadmap(tmpDir, content) { + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), content); +} + +function writeState(tmpDir, fields) { + const lines = Object.entries(fields).map(([k, v]) => `${k}: ${v}`); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), lines.join('\n') + '\n'); +} + +// ─── Shim-identity assertions ───────────────────────────────────────────────── + +describe('roadmap-parser: shim-identity — core.cjs re-exports same function objects', () => { + test('core.extractCurrentMilestone === roadmapParser.extractCurrentMilestone', () => { + assert.strictEqual(core.extractCurrentMilestone, roadmapParser.extractCurrentMilestone); + }); + test('core.stripShippedMilestones === roadmapParser.stripShippedMilestones', () => { + assert.strictEqual(core.stripShippedMilestones, roadmapParser.stripShippedMilestones); + }); + test('core.replaceInCurrentMilestone === roadmapParser.replaceInCurrentMilestone', () => { + assert.strictEqual(core.replaceInCurrentMilestone, roadmapParser.replaceInCurrentMilestone); + }); + test('core.getRoadmapPhaseInternal === roadmapParser.getRoadmapPhaseInternal', () => { + assert.strictEqual(core.getRoadmapPhaseInternal, roadmapParser.getRoadmapPhaseInternal); + }); + test('core.getMilestoneInfo === roadmapParser.getMilestoneInfo', () => { + assert.strictEqual(core.getMilestoneInfo, roadmapParser.getMilestoneInfo); + }); + test('core.getMilestonePhaseFilter === roadmapParser.getMilestonePhaseFilter', () => { + assert.strictEqual(core.getMilestonePhaseFilter, roadmapParser.getMilestonePhaseFilter); + }); +}); + +// ─── stripShippedMilestones ─────────────────────────────────────────────────── + +describe('roadmap-parser: stripShippedMilestones', () => { + test('strips a single
block', () => { + const input = 'before\n
\nsome shipped content\n
\nafter'; + const result = stripShippedMilestones(input); + assert.ok(!result.includes('
'), 'details tag should be removed'); + assert.ok(!result.includes('shipped content'), 'shipped content should be removed'); + assert.ok(result.includes('before'), 'before content preserved'); + assert.ok(result.includes('after'), 'after content preserved'); + }); + + test('strips multiple
blocks', () => { + const input = '
\nA\n
\nmiddle\n
\nB\n
\nend'; + const result = stripShippedMilestones(input); + assert.ok(result.includes('middle'), 'middle content preserved'); + assert.ok(result.includes('end'), 'end content preserved'); + assert.ok(!result.includes('
'), 'all details tags removed'); + }); + + test('returns unchanged string when no
blocks', () => { + const input = '## v1.0: Launch\n### Phase 1: Setup\n**Goal:** init\n'; + assert.strictEqual(stripShippedMilestones(input), input); + }); + + test('handles case-insensitive
tags', () => { + const input = '
\nclosed content\n
\nafter'; + const result = stripShippedMilestones(input); + assert.ok(!result.includes('closed content'), 'content removed'); + assert.ok(result.includes('after'), 'after content preserved'); + }); +}); + +// ─── extractCurrentMilestone ────────────────────────────────────────────────── + +describe('roadmap-parser: extractCurrentMilestone', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('no cwd — strips
only', () => { + const input = '
\nshipped\n
\n## v2.0: Next\n### Phase 1: Setup\n'; + const result = extractCurrentMilestone(input); + assert.ok(!result.includes('
'), 'details stripped'); + assert.ok(result.includes('v2.0'), 'version heading preserved'); + }); + + test('reads milestone from STATE.md and extracts that section', () => { + writeState(tmpDir, { milestone: 'v2.0' }); + const content = [ + '
', + 'v1.0', + '### Phase 1: Old', + '
', + '## v2.0: Current', + '### Phase 2-01: Setup', + '**Goal:** build', + ].join('\n'); + writeRoadmap(tmpDir, content); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + const result = extractCurrentMilestone(roadmap, tmpDir); + assert.ok(result.includes('v2.0'), 'current milestone section included'); + assert.ok(!result.includes('Old'), 'shipped milestone section excluded'); + }); + + test('falls back to 🚧 marker when STATE.md has no milestone field', () => { + writeState(tmpDir, { phase: 'some-phase' }); + const content = [ + '## 🚧 **v2.0 Work in Progress**', + '### Phase 1: Active', + '**Goal:** do work', + ].join('\n'); + writeRoadmap(tmpDir, content); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + const result = extractCurrentMilestone(roadmap, tmpDir); + assert.ok(result.includes('v2.0'), 'inferred v2.0 milestone section included'); + }); + + test('strips shipped milestones when no STATE.md and no 🚧 marker', () => { + const content = [ + '
', + 'v1.0 done', + '### Phase 1: Done', + '
', + '## v2.0: Next (no WIP marker)', + '### Phase 2: Future', + ].join('\n'); + + const result = extractCurrentMilestone(content); + assert.ok(!result.includes('
'), 'details stripped'); + assert.ok(result.includes('v2.0'), 'remaining content preserved'); + }); + + test('unicode heading — emoji-prefixed milestone', () => { + writeState(tmpDir, { milestone: 'v3.0' }); + const content = [ + '## ✅ v1.0: Shipped', + '## 🚧 v3.0: In Progress', + '### Phase 3-01: Unicode Héros', + '**Goal:** тест', + ].join('\n'); + writeRoadmap(tmpDir, content); + + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + const result = extractCurrentMilestone(roadmap, tmpDir); + assert.ok(result.includes('v3.0'), 'v3.0 heading included'); + assert.ok(result.includes('Unicode'), 'unicode phase name included'); + }); + + test('CRLF line endings are handled', () => { + writeState(tmpDir, { milestone: 'v1.0' }); + const content = '## v1.0: CRLF\r\n### Phase 1: Setup\r\n**Goal:** crlf goal\r\n'; + writeRoadmap(tmpDir, content); + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + const result = extractCurrentMilestone(roadmap, tmpDir); + assert.ok(result.includes('v1.0'), 'section found despite CRLF'); + }); + + test('heading inside fenced code block not confused for milestone boundary', () => { + writeState(tmpDir, { milestone: 'v1.0' }); + const content = [ + '## v1.0: Current Milestone', + '### Phase 1: Real Phase', + '**Goal:** real goal', + '```markdown', + '## v2.0: Fake Heading Inside Fence', + '```', + '### Phase 2: Also Real', + '**Goal:** also real', + ].join('\n'); + writeRoadmap(tmpDir, content); + const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); + const result = extractCurrentMilestone(roadmap, tmpDir); + // The section should include Phase 1 content; the fenced heading should not terminate section early + assert.ok(result.includes('real goal'), 'phase 1 content included'); + assert.ok(result.includes('Also Real'), 'phase 2 content also included'); + }); +}); + +// ─── replaceInCurrentMilestone ──────────────────────────────────────────────── + +describe('roadmap-parser: replaceInCurrentMilestone', () => { + test('replaces in content after last
when present', () => { + const content = '
\nold\n
\n**Plans:** 0/1 plans'; + const result = replaceInCurrentMilestone(content, /0\/1 plans/, '1/1 plans complete'); + assert.ok(result.includes('1/1 plans complete'), 'replacement applied after
'); + assert.ok(result.includes('
'), 'details block untouched'); + }); + + test('replaces anywhere when no
present', () => { + const content = '**Plans:** 0/1 plans'; + const result = replaceInCurrentMilestone(content, /0\/1 plans/, '1/1 plans complete'); + assert.strictEqual(result, '**Plans:** 1/1 plans complete'); + }); + + test('does not replace in shipped sections', () => { + const content = '
\n**Plans:** 0/1 plans\n
\n## v2.0\n**Plans:** 0/1 plans'; + const result = replaceInCurrentMilestone(content, /0\/1 plans/, '1/1 plans complete'); + // Only the SECOND occurrence (after
) should be replaced + assert.ok(result.includes('
\n**Plans:** 0/1 plans\n
'), 'shipped section unchanged'); + assert.ok(result.includes('## v2.0\n**Plans:** 1/1 plans complete'), 'current section updated'); + }); +}); + +// ─── getRoadmapPhaseInternal ────────────────────────────────────────────────── + +describe('roadmap-parser: getRoadmapPhaseInternal', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('returns null when ROADMAP.md missing', () => { + const result = getRoadmapPhaseInternal(tmpDir, '1'); + assert.strictEqual(result, null); + }); + + test('returns null when phaseNum is falsy', () => { + writeRoadmap(tmpDir, '### Phase 1: Foo\n**Goal:** bar\n'); + assert.strictEqual(getRoadmapPhaseInternal(tmpDir, null), null); + assert.strictEqual(getRoadmapPhaseInternal(tmpDir, ''), null); + assert.strictEqual(getRoadmapPhaseInternal(tmpDir, 0), null); + }); + + test('finds a phase by number', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Current', + '### Phase 1: Foundation', + '**Goal:** Set up infrastructure', + '', + '### Phase 2: API', + '**Goal:** Build the API', + ].join('\n')); + + const result = getRoadmapPhaseInternal(tmpDir, '1'); + assert.ok(result !== null, 'result should not be null'); + assert.strictEqual(result.found, true); + assert.strictEqual(result.phase_name, 'Foundation'); + assert.strictEqual(result.goal, 'Set up infrastructure'); + }); + + test('returns null for missing phase number', () => { + writeRoadmap(tmpDir, '### Phase 1: Foo\n**Goal:** bar\n'); + const result = getRoadmapPhaseInternal(tmpDir, '99'); + assert.strictEqual(result, null); + }); + + test('finds milestone-prefixed phase ID (e.g. 2-01)', () => { + writeState(tmpDir, { milestone: 'v2.0' }); + writeRoadmap(tmpDir, [ + '## v2.0: Current', + '### Phase 2-01: Alpha', + '**Goal:** first alpha phase', + '', + '### Phase 2-02: Beta', + '**Goal:** beta phase', + ].join('\n')); + + const result = getRoadmapPhaseInternal(tmpDir, '2-01'); + assert.ok(result !== null); + assert.strictEqual(result.found, true); + assert.strictEqual(result.phase_name, 'Alpha'); + assert.strictEqual(result.goal, 'first alpha phase'); + }); + + test('decimal phase ID (e.g. 1.5)', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Current', + '### Phase 1.5: Intermediate', + '**Goal:** interstitial step', + ].join('\n')); + + const result = getRoadmapPhaseInternal(tmpDir, '1.5'); + assert.ok(result !== null); + assert.strictEqual(result.phase_name, 'Intermediate'); + }); +}); + +// ─── getMilestoneInfo ───────────────────────────────────────────────────────── + +describe('roadmap-parser: getMilestoneInfo', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('returns default when ROADMAP.md missing', () => { + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v1.0'); + assert.strictEqual(info.name, 'milestone'); + }); + + test('reads version from STATE.md and heading name', () => { + writeState(tmpDir, { milestone: 'v2.0' }); + writeRoadmap(tmpDir, '## v2.0: The Big Launch\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v2.0'); + assert.match(info.name, /Big Launch/); + }); + + test('falls back to 🚧 WIP marker when STATE.md has no milestone', () => { + writeRoadmap(tmpDir, '## 🚧 **v1.5 Work In Progress**\n### Phase 1: Do stuff\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v1.5'); + assert.match(info.name, /Work In Progress/i); + }); + + test('extracts from heading when no STATE.md and no WIP marker', () => { + writeRoadmap(tmpDir, [ + '## v3.0: Future Milestone', + '### Phase 1: Not started', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.version, 'v3.0'); + assert.match(info.name, /Future Milestone/); + }); + + test('skips completed ✅ milestones', () => { + writeRoadmap(tmpDir, [ + '## ✅ v1.0: Shipped Already', + '## v2.0: Next Up', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + // Should not use the ✅-prefixed version as the current milestone + assert.strictEqual(info.version, 'v2.0'); + }); +}); + +// ─── getMilestonePhaseFilter ────────────────────────────────────────────────── + +describe('roadmap-parser: getMilestonePhaseFilter', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('returns passAll (phaseCount=0) when ROADMAP.md missing', () => { + const filter = getMilestonePhaseFilter(tmpDir); + assert.strictEqual(filter.phaseCount, 0); + assert.strictEqual(filter('anything'), true); + }); + + test('basic milestone phase filter — matches dirs by phase number', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Launch', + '### Phase 1: Setup', + '**Goal:** setup', + '', + '### Phase 2: Build', + '**Goal:** build', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + assert.strictEqual(filter.phaseCount, 2); + assert.strictEqual(filter('01-setup'), true, '01-setup matches Phase 1'); + assert.strictEqual(filter('02-build'), true, '02-build matches Phase 2'); + assert.strictEqual(filter('03-deploy'), false, '03-deploy not in milestone'); + }); + + test('milestone-prefixed phase IDs (e.g. 2-01)', () => { + writeState(tmpDir, { milestone: 'v2.0' }); + writeRoadmap(tmpDir, [ + '## v2.0: Current', + '### Phase 2-01: Alpha', + '### Phase 2-02: Beta', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + assert.strictEqual(filter('02-01-alpha'), true, '02-01 matches Phase 2-01'); + assert.strictEqual(filter('02-02-beta'), true, '02-02 matches Phase 2-02'); + assert.strictEqual(filter('02-03-other'), false, '02-03 not in milestone'); + }); + + test('versionOverride uses specified version slice', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Old', + '### Phase 1: Old Phase', + '', + '## v2.0: Current', + '### Phase 2: New Phase', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir, 'v2.0'); + assert.strictEqual(filter('02-new-phase'), true, 'phase 2 in v2.0 slice'); + assert.strictEqual(filter('01-old-phase'), false, 'phase 1 not in v2.0 slice'); + }); + + test('missingExplicitVersion set when version not found in versioned roadmap', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Only Milestone', + '### Phase 1: Foo', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir, 'v9.9'); + assert.strictEqual(filter.missingExplicitVersion, true, 'missingExplicitVersion should be true'); + assert.strictEqual(filter.phaseCount, 0); + }); + + test('zero-padded phase IDs match unpadded dirs and vice versa', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Padded Test', + '### Phase 01: Setup', + '### Phase 02: Build', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + assert.strictEqual(filter('1-setup'), true, 'unpadded dir matches padded Phase 01'); + assert.strictEqual(filter('02-build'), true, 'padded dir matches padded Phase 02'); + }); + + test('decimal phase IDs in ROADMAP filter correctly', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Decimal Test', + '### Phase 1.5: Interstitial', + '### Phase 2: Normal', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + assert.ok(filter.phaseCount >= 1, 'at least one phase found'); + // Decimal phase IDs are non-numeric so filter should handle them + assert.strictEqual(filter('1.5-interstitial'), true, 'decimal phase dir matches'); + }); + + test('repeated phase IDs — deduplication (no double count)', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Repeated', + '### Phase 1: First', + '### Phase 1: Duplicate heading', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + // Phase 1 appears twice but should only count once + assert.strictEqual(filter.phaseCount, 1, 'deduplication: only 1 unique phase'); + }); + + test('adversarial: characterizes current fence-blind behavior for backtick fence (pending #875)', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Real', + '```', + '### Phase 999: Fake Phase Inside Fence', + '```', + '### Phase 1: Real Phase', + '**Goal:** real', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + // KNOWN BUG #875: getMilestonePhaseFilter is fence-blind — phase headings inside + // fenced code blocks are incorrectly parsed as real phases. Flip to false when #875 is fixed. + assert.strictEqual(filter('01-real'), true, 'real phase matches'); + assert.strictEqual(filter('999-fake'), true, 'characterization: getMilestonePhaseFilter is currently fence-blind (KNOWN BUG #875) — flip to false when #875 is fixed'); + }); + + test('adversarial: unclosed fence block — does not crash', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Unclosed', + '```', + '### Phase 1: Inside unclosed fence', + '**Goal:** unreachable', + // Intentionally no closing ``` — adversarial fixture + ].join('\n')); + + // Should not throw regardless of fence parsing behavior + let filter; + assert.doesNotThrow(() => { + filter = getMilestonePhaseFilter(tmpDir); + }, 'unclosed fence should not throw'); + assert.ok(typeof filter === 'function', 'filter is a function'); + }); + + test('adversarial: characterizes current fence-blind behavior for tilde fence (pending #875)', () => { + writeRoadmap(tmpDir, [ + '## v1.0: Tilde', + '~~~', + '### Phase 999: Fake', + '~~~', + '### Phase 1: Real', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + // KNOWN BUG #875: getMilestonePhaseFilter is fence-blind — phase headings inside + // tilde-fenced code blocks are incorrectly parsed as real phases. Flip to false when #875 is fixed. + assert.strictEqual(filter('01-real'), true, 'real phase matches despite tilde fence'); + assert.strictEqual(filter('999-fake'), true, 'characterization: getMilestonePhaseFilter is currently fence-blind (KNOWN BUG #875) — flip to false when #875 is fixed'); + }); + + test('adversarial: CRLF line endings in roadmap', () => { + const crlf = '## v1.0: CRLF\r\n### Phase 1: Setup\r\n### Phase 2: Build\r\n'; + writeRoadmap(tmpDir, crlf); + let filter; + assert.doesNotThrow(() => { filter = getMilestonePhaseFilter(tmpDir); }); + assert.ok(filter.phaseCount >= 1, 'phases found despite CRLF'); + }); + + test('adversarial: mixed CRLF and LF in same file', () => { + const mixed = '## v1.0: Mixed\r\n### Phase 1: A\n### Phase 2: B\r\n### Phase 3: C\n'; + writeRoadmap(tmpDir, mixed); + let filter; + assert.doesNotThrow(() => { filter = getMilestonePhaseFilter(tmpDir); }); + assert.ok(filter.phaseCount >= 1, 'phases found in mixed CRLF/LF'); + }); + + test('adversarial: unicode headings', () => { + writeState(tmpDir, { milestone: 'v1.0' }); + writeRoadmap(tmpDir, [ + '## v1.0: 日本語マイルストーン', + '### Phase 1: Héros Réalité', + '### Phase 2: Тест', + ].join('\n')); + + let filter; + assert.doesNotThrow(() => { filter = getMilestonePhaseFilter(tmpDir); }); + assert.strictEqual(filter.phaseCount, 2, '2 unicode phases found'); + assert.strictEqual(filter('01-setup'), true, 'phase 1 dir matches'); + }); + + test('adversarial: bracket-prefixed phase heading ### [GSD] Phase 2-01:', () => { + writeState(tmpDir, { milestone: 'v2.0' }); + writeRoadmap(tmpDir, [ + '## v2.0: Bracket', + '### [GSD] Phase 2-01: Setup', + '### [GSD] Phase 2-02: Build', + ].join('\n')); + + const filter = getMilestonePhaseFilter(tmpDir); + assert.strictEqual(filter('02-01-setup'), true, 'bracket-prefixed phase 2-01 matched'); + assert.strictEqual(filter('02-02-build'), true, 'bracket-prefixed phase 2-02 matched'); + }); +});