diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 33c4fdfe4..df32012b8 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -9,7 +9,7 @@ { "name": "gsd-core", "description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.", - "version": "1.9.0", + "version": "1.9.1", "source": "./", "author": { "name": "open-gsd", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 0ddecad24..dd0925945 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "gsd-core", "displayName": "GSD Core", - "version": "1.9.0", + "version": "1.9.1", "description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.", "author": { "name": "open-gsd", diff --git a/.github/PULL_REQUEST_TEMPLATE/registry-entry.md b/.github/PULL_REQUEST_TEMPLATE/registry-entry.md index 6a6b4afb0..e9baa3601 100644 --- a/.github/PULL_REQUEST_TEMPLATE/registry-entry.md +++ b/.github/PULL_REQUEST_TEMPLATE/registry-entry.md @@ -15,6 +15,7 @@ Full schema and process: [docs/registries/README.md](../../docs/registries/READM - [ ] Capability Registry entry — adds/updates one object in `docs/registries/capabilities.json` - [ ] EoS Registry entry — adds/updates one object in `docs/registries/eos.json` +- [ ] Reviewer Lane Registry entry — adds/updates one object in `docs/registries/reviewers.json` ## The entry @@ -44,6 +45,7 @@ Full schema and process: [docs/registries/README.md](../../docs/registries/READM - [ ] `id`, `name`, `type`, `repo`, `description`, `author`, `license`, `enginesGsd`, `install`, `uninstall`, `interactions`, `discussion` are all present and non-empty - [ ] **(Capability entries only)** `interactions.loopExtensionPoints` is a non-empty subset of the 12 Loop Extension Points, `interactions.hookKinds` ⊆ `{step, contribution, gate}`, and `interactions.configKeys` / `requires` / `runtimeCompat` / `produces` / `consumes` are present (empty arrays are fine where nothing applies) - [ ] **(EoS entries only)** `protocolVersion` is an integer ≥ 1, `interactions.interfacePoints` is a non-empty subset of the six interface points, `interactions.profile` is one of `programmatic-cli` / `declarative-cli` / `ide`, and `interactions.axes` has exactly the eight required axis keys plus, optionally, `effortSurface` (`argv` / `none`) +- [ ] **(Reviewer entries only)** `interactions.slug` matches the lane slug grammar `^[a-z0-9][a-z0-9_-]*$`, `interactions.flags` is a non-empty array matching `^--[a-z0-9][a-z0-9-]*$`, `interactions.transport` is `spawn` or `openai-http`, `interactions.evidenceClass` is `source-grounded` or `diff-only`, `interactions.reviewsSection` is a non-empty string (max 200 characters), and `interactions.requiresBinaries` / `configKeys` / `runtimeCompat` are present (empty arrays are fine where nothing applies) ## Ownership & non-endorsement @@ -53,12 +55,12 @@ Full schema and process: [docs/registries/README.md](../../docs/registries/READM ## One entry, one PR -- [ ] This PR adds or updates exactly **one** entry, in exactly one of `capabilities.json` / `eos.json` +- [ ] This PR adds or updates exactly **one** entry, in exactly one of `capabilities.json` / `eos.json` / `reviewers.json` - [ ] I have not bundled any other registry entry, code change, or unrelated docs change into this PR ## Generated file in sync -- [ ] I ran `npm run gen:registry` after editing the JSON source, and this PR includes the regenerated `docs/registries/capability-registry.md` or `docs/registries/eos-registry.md` +- [ ] I ran `npm run gen:registry` after editing the JSON source, and this PR includes the regenerated `docs/registries/capability-registry.md`, `docs/registries/eos-registry.md`, or `docs/registries/reviewer-registry.md` - [ ] I did **not** hand-edit the generated `.md` file directly — all edits were made to the JSON source ## Documentation diff --git a/CHANGELOG.md b/CHANGELOG.md index 03aac07ca..774c45096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +## [1.9.1] - 2026-07-31 + +### Added + +- **Reviewer lanes are now documented as an authorable capability surface** — a new how-to walks capability authors through declaring a `reviewer` body so `/gsd-review` discovers, invokes, and renders their external review CLI or model endpoint, and the manifest reference's `invoke` row now lists the full accepted vocabulary for both transports. (#2782) (#2906) +- **Third-party reviewer lanes can now be listed in a discoverability catalog.** ADR-2782 made a reviewer lane installable by a third party, but the two existing registries could not hold one — the Community Capability Registry requires a non-empty `loopExtensionPoints`, which a lane registers on none of, and the EoS Registry is for host integrations. A new Reviewer Lane Registry (`docs/registries/reviewers.json` → `docs/registries/reviewer-registry.md`) gives lanes a home, with an entry schema describing the lane itself: slug, flags, transport, evidence class, and REVIEWS.md section. (#2904) (#2912) + +### Fixed + +- **Fallow structural pre-pass no longer silently no-ops on Windows** — `run-with-timeout` now mediates `.cmd`/`.bat`/`.exe` spawns via an explicit `cmd.exe /c` argv array (Node's CVE-2024-27980 hardening requires a shell for these on Windows), and the fallow pre-pass names the failure kind so a Windows spawn failure is not mistaken for an absent binary. The existing `bash -c` callers and POSIX behavior are unchanged. (#2667) (#2897) +- **A clean Codex install now applies balanced model settings to agent TOMLs on the first run** — `~/.gsd/defaults.json` (`resolve_model_ids` + `runtime`) is now written before agent TOML generation, so the runtime-aware model resolver knows the target runtime during the first pass. Previously a second install was required. (#2834) (#2900) +- **`verify-summary` no longer reports a valid SUMMARY as failed because of a path mentioned in prose** — file-claim extraction is now bound to a creation/modification claim (a `Created:`/`Modified:`/`key-files` line), so a prose mention of a future deliverable is not checked for existence; and `verify-summary` now resolves the project root, so invoking it from a subdirectory no longer manufactures missing files. (#2910) +- **`findProjectRoot` no longer silently resolves to a parent project across a git-repo boundary** — when invoked from a nested git repository that has no `.planning/` of its own, resolution stays within the caller's repo (or falls back to the start directory) instead of crossing into an ancestor GSD project. The existing plain-descendant and co-located `.git`+`.planning` cases are unchanged. (#2909) +- **A requirement row stranded at `Gaps Found` can now be completed again, and `requirements mark-complete` no longer reports false success on a row it could not move** — the completion guards now accept `Gaps Found` (so `revert-phase`'s stranded rows are recoverable instead of permanently blocking the milestone), and when a traceability table has a row for an ID, `mark-complete` counts it as updated only if the row actually moved (not merely because the checkbox flipped). (#2788) (#2902) +- **`/gsd-code-review --fix` now honors `workflow.use_worktrees`** — when the setting is `false`, the fixer edits and commits in the main checkout instead of creating a git worktree (matching the other writer workflows), and the spec forbids `rm -rf` on a possible Windows reparse point so an improvised worktree teardown can no longer delete the real `node_modules`. The REVIEW-FIX report also records where verification ran. (#2905) + ## [1.9.0] - 2026-07-31 ### Added diff --git a/CONTEXT.md b/CONTEXT.md index f8cb30dd0..8d762f79d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -230,7 +230,10 @@ Runtime seam (`gsd-core/bin/lib/capability-loader.cjs`, ADR-1244 D2) that compos Human-facing discoverability catalog (`docs/registries/capability-registry.md`, generated from `docs/registries/capabilities.json`; issue #2182) listing third-party Feature Capabilities registered by a docs PR so a solo developer can find one before installing it. Distinct from **Capability Registry** (the generated runtime manifest compiled from first-party `capability.json` declarations, ADR-894) and **Capability Registry Overlay** (the runtime seam that merges an installed third-party manifest into that generated registry at load time, ADR-1244 D2): this registry is a static document rendered by `scripts/gen-registry.cjs`, not a runtime data structure or loader. Each entry enumerates the capability's Loop Extension Points and hook kinds so a reader can judge blast radius before running `gsd capability install`, and declares its `engines.gsd` range. Inclusion is an explicit non-endorsement — a maintainer merged a link, nothing more — per `docs/registries/README.md`. ### EoS Registry -Human-facing discoverability catalog (`docs/registries/eos-registry.md`, generated from `docs/registries/eos.json`; issue #2182) listing third-party Embeddable Orchestration System (EoS) host integrations — projects that embed GSD as an orchestration engine behind the ADR-1239 six-interface-point Host-Integration Interface. Entries are registered by the same docs-PR process, schema conventions, and non-endorsement stance as the **Community Capability Registry**, but enumerate the six interface points, the eight negotiated axes plus an optional ninth (`effortSurface`), and `protocolVersion` in place of Loop Extension Points and hook kinds. It has no generated-manifest or Capability Registry Overlay counterpart: an ADR-1239 host integration runs inside the third-party host, not inside GSD's own capability loader, so there is nothing for a runtime registry to merge. See `docs/registries/README.md` for the full entry schema. +Human-facing discoverability catalog (`docs/registries/eos-registry.md`, generated from `docs/registries/eos.json`; issue #2182) listing third-party Embeddable Orchestration System (EoS) host integrations — projects that embed GSD as an orchestration engine behind the ADR-1239 six-interface-point Host-Integration Interface. Entries are registered by the same docs-PR process, schema conventions, and non-endorsement stance as the **Community Capability Registry**, but enumerate the six interface points, the eight negotiated axes plus an optional ninth (`effortSurface`), and `protocolVersion` in place of Loop Extension Points and hook kinds. It has no generated-manifest or Capability Registry Overlay counterpart: an ADR-1239 host integration runs inside the third-party host, not inside GSD's own capability loader, so there is nothing for a runtime registry to merge. See `docs/registries/README.md` for the full entry schema. One of three third-party discoverability catalogs alongside the **Community Capability Registry** and the **Reviewer Lane Registry**. + +### Reviewer Lane Registry +Human-facing discoverability catalog (`docs/registries/reviewer-registry.md`, generated from `docs/registries/reviewers.json`; issue #2904) listing third-party `role: "reviewer"` capabilities (ADR-2782) — reviewer lanes that add an external review lane to `/gsd-review`, installed with `gsd capability install `. A lane registers on zero Loop Extension Points and owns no artifacts, which is why it cannot be listed on the **Community Capability Registry**: the Capability entry schema's `loopExtensionPoints`/`hookKinds` are unsatisfiable by construction for a lane. Entries are registered by the same docs-PR process, schema conventions, and non-endorsement stance as the other two registries, but enumerate `slug`, `flags`, `transport`, `evidenceClass`, and `reviewsSection` in place of Loop Extension Points and hook kinds. See `docs/registries/README.md` for the full entry schema and disambiguation against the **Community Capability Registry** and **EoS Registry**. ### Capability Validator Shared conformance validator (`gsd-core/bin/lib/capability-validator.cjs`, ADR-1244 D2) extracted from `scripts/gen-capability-registry.cjs` so the build-time generator and the runtime overlay loader share one validator implementation. Exports the same `validateCapability(manifest)` surface consumed by both the generator (build-time) and `capability-loader.cjs` (runtime). Generative-parity is CI-guarded: a drift between the generator's validation logic and the extracted module is a hard failure. Callers that previously inlined validation against the generator's internal helpers are migrated to import this module directly. Source of truth: `gsd-core/bin/lib/capability-validator.cjs`. diff --git a/agents/gsd-code-fixer.md b/agents/gsd-code-fixer.md index f4394d2a2..4126a6157 100644 --- a/agents/gsd-code-fixer.md +++ b/agents/gsd-code-fixer.md @@ -216,9 +216,36 @@ If a finding references multiple files (in Fix section or Issue section): This agent runs as a background process that makes commits. Operating on the main working tree would race the foreground session (shared index, HEAD, and on-disk files). Instead, every instance runs in its own isolated worktree. +**#2825: honor `workflow.use_worktrees`.** This is the ONLY writer that hand-rolls a git worktree +inside the agent prompt; every other writer path (`/gsd:execute-phase`, `/gsd:execute-plan`, +`/gsd:quick`, `/gsd:diagnose-issues`) reads `workflow.use_worktrees` and skips isolation when it is +`false`. Read the same flag here and, when it is `false`, edit and commit in the main checkout +directly (set `wt="."`, no `reviewfix_branch`, no recovery sentinel, no `git worktree add`, and skip +the cleanup tail — there is no worktree to remove). When the flag is not `false`, the transactional +worktree path below runs unchanged. A user who explicitly opted out of worktrees must never have a +worktree created; the hand-rolled worktree also cannot run the project's gates safely (no +`node_modules`), so the opt-out is also the safe path. + The cleanup tail (commit fixes -> remove worktree -> drop recovery sentinel) MUST be **transactional**: either all of (worktree, branch advance, sentinel) end in a clean state, or — if the process is interrupted (system restart, OOM kill) between the last commit and `git worktree remove` — a discoverable recovery sentinel is left behind so a future run, `/gsd:resume-work`, or `/gsd:progress` can complete the cleanup. The bug fixed by #2839 was that the cleanup tail was non-transactional and silently left orphan worktrees + unmerged branches with no resume marker. ```bash +# #2825: honor workflow.use_worktrees — the documented opt-out. When false, +# edit/commit in the main checkout (wt=".", no temp branch, no sentinel, no +# cleanup tail). Read the flag the same way the four sibling writer workflows +# do. NOTE: this read parses .planning/config.json directly via `node` rather +# than the gsd-tools CLI, because setup_worktree runs BEFORE the canonical +# launcher preamble is sourced — invoking the CLI here would be undefined at +# runtime and violates the runtime-launcher-parity preamble-ordering rule. +# Once the preamble is sourced (later steps), the CLI is available. +USE_WORKTREES=$(node -e ' + try { + const fs = require("fs"); + const p = (process.env.GSD_PROJECT_DIR || process.cwd()) + "/.planning/config.json"; + const cfg = JSON.parse(fs.readFileSync(p, "utf8")); + process.stdout.write(String((cfg.workflow && cfg.workflow.use_worktrees) ?? true)); + } catch { process.stdout.write("true"); } +') + # Derive worktree path from padded_phase (parsed from config in next step, # but the shell snippet below is illustrative — adapt once config is parsed). # In practice: parse padded_phase from config first, then run: @@ -264,34 +291,47 @@ if [ -f "$sentinel" ]; then rm -f "$sentinel" fi -wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX") +# #2825: when the user opted out of worktrees, edit/commit in the main +# checkout directly — no temp branch, no sentinel, no cleanup tail. This is +# the safe path: the hand-rolled worktree has no node_modules, so it cannot +# run the project's gates, and an improvised teardown can destroy the real +# node_modules on Windows (a junction followed by rm -rf). wt="." means every +# downstream read/edit/commit lands in the main working tree, and the cleanup +# tail below is a no-op (nothing to fast-forward, no worktree to remove). +if [ "$USE_WORKTREES" = "false" ]; then + wt="." + reviewfix_branch="$branch" + echo "workflow.use_worktrees=false — editing/committing in the main checkout (no worktree)." +else + wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX") -# Create a temp branch from the current branch tip so the worktree -# attaches to that NEW branch rather than the user's currently-checked-out -# branch (#2990: git refuses to check out the same branch in two -# worktrees by default; the original `git worktree add "$wt" "$branch"` -# failed before the agent could do any work). The temp branch shares -# history with $branch up to the moment of creation, so commits made -# inside the worktree fast-forward $branch on cleanup. -reviewfix_branch="gsd-reviewfix/${padded_phase}-$$" -git worktree add -b "$reviewfix_branch" "$wt" "$branch" + # Create a temp branch from the current branch tip so the worktree + # attaches to that NEW branch rather than the user's currently-checked-out + # branch (#2990: git refuses to check out the same branch in two + # worktrees by default; the original `git worktree add "$wt" "$branch"` + # failed before the agent could do any work). The temp branch shares + # history with $branch up to the moment of creation, so commits made + # inside the worktree fast-forward $branch on cleanup. + reviewfix_branch="gsd-reviewfix/${padded_phase}-$$" + git worktree add -b "$reviewfix_branch" "$wt" "$branch" -# Write the recovery sentinel ONLY AFTER `git worktree add` succeeds. -# Writing it before would leave a sentinel pointing at a worktree that does -# not exist if `git worktree add` itself failed. -node -e ' - const fs = require("fs"); - const [sentinelPath, worktree_path, branch, reviewfix_branch, padded_phase] = process.argv.slice(1); - fs.writeFileSync(sentinelPath, JSON.stringify({ - worktree_path, - branch, - reviewfix_branch, - padded_phase, - started_at: new Date().toISOString() - }, null, 2)); -' "$sentinel" "$wt" "$branch" "$reviewfix_branch" "$padded_phase" + # Write the recovery sentinel ONLY AFTER `git worktree add` succeeds. + # Writing it before would leave a sentinel pointing at a worktree that does + # not exist if `git worktree add` itself failed. + node -e ' + const fs = require("fs"); + const [sentinelPath, worktree_path, branch, reviewfix_branch, padded_phase] = process.argv.slice(1); + fs.writeFileSync(sentinelPath, JSON.stringify({ + worktree_path, + branch, + reviewfix_branch, + padded_phase, + started_at: new Date().toISOString() + }, null, 2)); + ' "$sentinel" "$wt" "$branch" "$reviewfix_branch" "$padded_phase" -cd "$wt" + cd "$wt" +fi ``` Concrete steps: @@ -305,9 +345,18 @@ Concrete steps: **If `git worktree add` fails**, surface the error and exit — do not force-remove the path, as another concurrent run may be holding it. Do not write the sentinel (the worktree does not exist). Do not delete `$reviewfix_branch` either; if `-b` failed, no temp branch was created. -**Cleanup tail (transactional, ALWAYS — even on failure):** After writing REVIEW-FIX.md and before returning to the orchestrator, run the cleanup in this exact order: +**Cleanup tail (transactional, ALWAYS — even on failure — when a worktree was created):** After writing REVIEW-FIX.md and before returning to the orchestrator, run the cleanup in this exact order. (When `workflow.use_worktrees` is `false`, no worktree was created — the cleanup is a no-op and the bash below early-exits.) ```bash +# #2825: when worktrees were disabled, there is nothing to clean up — the +# agent edited/committed on $branch directly in the main checkout (wt=".", +# reviewfix_branch==$branch, no sentinel, no temp worktree). Skip the whole +# tail; the four steps below are all no-ops or harmful (e.g. `git worktree +# remove "."` ) in that mode. +if [ "$USE_WORKTREES" = "false" ]; then + exit 0 +fi + # Step 1 (#2990): fast-forward $branch to capture the commits the agent # made on $reviewfix_branch. Run from the main repo (not $wt) — the user's # checkout owns $branch. --ff-only ensures we never silently drop or @@ -354,7 +403,7 @@ fi rm -f "$sentinel" ``` -This cleanup is unconditional — register it mentally as a finally-block obligation. If the agent exits early (config error, no findings, etc.), still run the cleanup tail in order (fast-forward → worktree remove → temp branch delete → sentinel rm) before exit. The sentinel must NEVER be removed before `git worktree remove` succeeds. The temp branch must NEVER be deleted while the fast-forward is in a diverged state. +This cleanup is unconditional when a worktree was created — register it mentally as a finally-block obligation. If the agent exits early (config error, no findings, etc.), still run the cleanup tail in order (fast-forward → worktree remove → temp branch delete → sentinel rm) before exit. (When `workflow.use_worktrees` is `false`, no worktree exists and the bash above early-exits before these steps.) The sentinel must NEVER be removed before `git worktree remove` succeeds. The temp branch must NEVER be deleted while the fast-forward is in a diverged state. @@ -587,9 +636,33 @@ _Iteration: {N}_ -**ALWAYS run inside the isolated worktree** — set up via `branch=$(git branch --show-current)` + `wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX")` + `git worktree add -b "$reviewfix_branch" "$wt" "$branch"` at the very start (see `setup_worktree` step). Using `mktemp` ensures concurrent runs do not collide. Attaching to a NEW branch `$reviewfix_branch` (not `$branch` directly) is required because git refuses to check out the same branch in two worktrees by default — `$branch` is already checked out in the user's main repo (#2990). Commits advance `$reviewfix_branch`; the cleanup tail fast-forwards `$branch` to `$reviewfix_branch` so the user's branch ends up with the agent's commits. Every file read, edit, and commit must happen inside `$wt`. Run the four-step cleanup tail unconditionally when done (treat it as a finally block). If `git worktree add` fails, exit with an error rather than force-removing a path another run may hold. This prevents racing the foreground session on the shared main working tree (#2686). +**ALWAYS run inside the isolated worktree** — set up via `branch=$(git branch --show-current)` + `wt=$(mktemp -d "/tmp/sv-${padded_phase}-reviewfix-XXXXXX")` + `git worktree add -b "$reviewfix_branch" "$wt" "$branch"` at the very start (see `setup_worktree` step). Using `mktemp` ensures concurrent runs do not collide. Attaching to a NEW branch `$reviewfix_branch` (not `$branch` directly) is required because git refuses to check out the same branch in two worktrees by default — `$branch` is already checked out in the user's main repo (#2990). Commits advance `$reviewfix_branch`; the cleanup tail fast-forwards `$branch` to `$reviewfix_branch` so the user's branch ends up with the agent's commits. Every file read, edit, and commit must happen inside `$wt`. Run the four-step cleanup tail when done (treat it as a finally block) — but only when a worktree was actually created; when `workflow.use_worktrees` is `false` the cleanup early-exits (no worktree to remove). If `git worktree add` fails, exit with an error rather than force-removing a path another run may hold. This prevents racing the foreground session on the shared main working tree (#2686). -**ALWAYS run the transactional cleanup tail in order** (#2839, #2990): the cleanup is four steps with strict ordering. (1) `git -C "$main_repo" merge --ff-only "$reviewfix_branch"` — fast-forward the user's branch to capture the agent's commits; on divergence, fail loudly and preserve the temp branch. (2) `git worktree remove "$wt" --force`. (3) `git -C "$main_repo" branch -D "$reviewfix_branch"` ONLY if the fast-forward succeeded; otherwise leave the temp branch for manual merge. (4) `rm -f "$sentinel"` (the recovery sentinel at `${phase_dir}/.review-fix-recovery-pending.json`). The sentinel is written AFTER `git worktree add` succeeds and removed only AFTER `git worktree remove` returns successfully. The temp branch is deleted only when the fast-forward succeeded. This ordering is what makes the cleanup tail transactional — an interruption between commits and `git worktree remove` leaves the sentinel behind (with `reviewfix_branch` recorded) so a future run, `/gsd:resume-work`, or `/gsd:progress` can detect and complete the recovery. Reversing the order recreates the orphan-worktree bug. +**#2825 — honor `workflow.use_worktrees`.** Before creating a worktree, read the +`workflow.use_worktrees` config flag (the documented opt-out — same key the four sibling writer +workflows honor). `setup_worktree` reads it via `node` directly from `.planning/config.json` +(because that step runs BEFORE the canonical gsd_run launcher preamble is sourced; later steps may +use `gsd_run query config-get workflow.use_worktrees`). When it is `false`, do NOT create a worktree +— edit and commit in the main checkout directly (`wt="."`, no temp branch, no sentinel, no cleanup +tail). A user who opted out of worktrees must +never have one created. See the `setup_worktree` step for the gated bash. + +**NEVER `rm -rf` a possible reparse point** (#2825). On Windows, `node_modules` inside the worktree +may be a junction/reparse point whose target is the REAL `node_modules` in the main checkout — and +`rm -rf` follows the link and deletes the target's contents (silent, misdiagnosable data loss). Do +NOT improvise a `node_modules` teardown. The worktree has no `node_modules` by design; if you need +the project's gates, run them in the main checkout after the fast-forward, OR leave the worktree's +dependency handling to `git worktree remove` (which does not recurse into a separately-managed +link). Never use `rm -rf` (or `2>/dev/null || rm -rf || true`) as a fallback for removing a path +that might be a reparse point — on failure, STOP and surface the error rather than falling through +to a destructive remove. + +**Record where verification ran** (#2825). The REVIEW-FIX.md verification section must state whether +the gates ran in the main checkout or the isolated worktree, so a reader can tell whether the numbers +are reproducible from the tree they are looking at (a worktree-env run is not reproducible from the +main checkout after teardown). + +**ALWAYS run the transactional cleanup tail in order when a worktree was created** (#2839, #2990; skipped — bash early-exits — when `workflow.use_worktrees` is `false`): the cleanup is four steps with strict ordering. (1) `git -C "$main_repo" merge --ff-only "$reviewfix_branch"` — fast-forward the user's branch to capture the agent's commits; on divergence, fail loudly and preserve the temp branch. (2) `git worktree remove "$wt" --force`. (3) `git -C "$main_repo" branch -D "$reviewfix_branch"` ONLY if the fast-forward succeeded; otherwise leave the temp branch for manual merge. (4) `rm -f "$sentinel"` (the recovery sentinel at `${phase_dir}/.review-fix-recovery-pending.json`). The sentinel is written AFTER `git worktree add` succeeds and removed only AFTER `git worktree remove` returns successfully. The temp branch is deleted only when the fast-forward succeeded. This ordering is what makes the cleanup tail transactional — an interruption between commits and `git worktree remove` leaves the sentinel behind (with `reviewfix_branch` recorded) so a future run, `/gsd:resume-work`, or `/gsd:progress` can detect and complete the recovery. Reversing the order recreates the orphan-worktree bug. **ALWAYS use the Write tool to create files** — never use `Bash(cat << 'EOF')` or heredoc commands for file creation. diff --git a/bin/install.js b/bin/install.js index d942836cb..bc2ad0085 100755 --- a/bin/install.js +++ b/bin/install.js @@ -6983,6 +6983,47 @@ function writeCopilotHookConfig(targetDir) { * Generate config.toml and per-agent .toml files for Codex. * Reads agent .md files from source, extracts metadata, writes .toml configs. */ + +/** + * #2834: Write ~/.gsd/defaults.json for non-Claude runtimes — sets + * resolve_model_ids="omit" (so resolveModelInternal() returns '' instead of + * Claude aliases the runtime can't resolve) and runtime= (so + * resolveRuntime() resolves correctly out of the box). MUST be called BEFORE + * installCodexConfig (or any other step that reads defaults.json at generation + * time), so a clean first install produces correctly-model-routed agent TOMLs. + * No-op for Claude runtimes (Claude is the resolveRuntime fallback + has native + * model aliases). Preserves an explicit `true` opt-in and existing values. + */ +function writeNonClaudeDefaults(runtime) { + if (_hostBehaviors(runtime).nativeModelAliases || process.env.GSD_TEST_MODE) return; + const gsdDir = path.join(os.homedir(), '.gsd'); + const defaultsPath = path.join(gsdDir, 'defaults.json'); + try { + fs.mkdirSync(gsdDir, { recursive: true }); + let defaults = {}; + try { defaults = JSON.parse(fs.readFileSync(defaultsPath, 'utf8')); } catch { /* new file */ } + if (defaults === null || typeof defaults !== 'object' || Array.isArray(defaults)) { + defaults = {}; + } + // Three-valued domain: false/absent → aliases; true → full IDs; "omit" → ''. + const existing = defaults.resolve_model_ids; + const shouldDefaultToOmit = existing !== true && existing !== 'omit'; + if (shouldDefaultToOmit) { + defaults.resolve_model_ids = 'omit'; + fs.writeFileSync(defaultsPath, JSON.stringify(defaults, null, 2) + '\n'); + console.log(` ${green}✓${reset} Set resolve_model_ids: "omit" in ~/.gsd/defaults.json`); + } + // #2395: persist runtime for non-Claude runtimes. + if (defaults.runtime === undefined || defaults.runtime === null || defaults.runtime === '') { + defaults.runtime = runtime; + fs.writeFileSync(defaultsPath, JSON.stringify(defaults, null, 2) + '\n'); + console.log(` ${green}✓${reset} Set runtime: "${runtime}" in ~/.gsd/defaults.json`); + } + } catch (e) { + console.log(` ${yellow}⚠${reset} Could not write ~/.gsd/defaults.json: ${e.message}`); + } +} + function installCodexConfig(targetDir, agentsSrc, sandboxTier = 'codex-agent-sandbox') { // ADR-1239 Phase B write-confinement: every Codex config write stays under targetDir. const configPath = assertDestWithinConfigHome(targetDir, 'config.toml'); @@ -11586,6 +11627,10 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) { let agentCount = 0; if (!isMinimalMode(_effectiveInstallMode)) { + // #2834: write ~/.gsd/defaults.json (resolve_model_ids + runtime) BEFORE generating + // agent TOMLs — installCodexConfig reads defaults.json at generation time, so on a + // clean first install the runtime-aware model resolver must already know the runtime. + writeNonClaudeDefaults(runtime); try { // Generate Codex config.toml and per-agent .toml files. agentCount = installCodexConfig(targetDir, agentsSrc, plan.sandboxTier); @@ -12361,58 +12406,11 @@ function finishInstall(settingsPath, settings, statuslineCommand, shouldInstallS configureAntigravityMcpConfig(isGlobal, configDir); } - // For non-Claude runtimes, DEFAULT resolve_model_ids to "omit" in ~/.gsd/defaults.json - // when it is absent or falsy, so resolveModelInternal() returns '' instead of Claude - // aliases (opus/sonnet/haiku) the runtime can't resolve. An explicit `true` opt-in - // (resolveModelInternal returns full materialized model IDs) MUST be preserved — - // rewriting it to "omit" would make generated agent manifests inherit the active - // chat model instead of pinning the resolved model. See #1156 (default-to-omit - // intent) and #1569 (preserve explicit true). Guard matches the #130-class pattern - // on configureOpencodePermissions above. - if (!_hostBehaviors(runtime).nativeModelAliases && !process.env.GSD_TEST_MODE) { - const gsdDir = path.join(os.homedir(), '.gsd'); - const defaultsPath = path.join(gsdDir, 'defaults.json'); - try { - fs.mkdirSync(gsdDir, { recursive: true }); - let defaults = {}; - try { defaults = JSON.parse(fs.readFileSync(defaultsPath, 'utf8')); } catch { /* new file */ } - // Recover a malformed (valid-JSON-but-non-object) defaults.json to a fresh object so - // the write below succeeds and the file is no longer broken. Without this, `null` / - // `[]` / a number / a string bypass the parse catch and either throw a TypeError on - // property access (swallowed by the outer try/catch, leaving the file broken) or get - // a property set that won't round-trip through JSON.stringify. (#1657) - if (defaults === null || typeof defaults !== 'object' || Array.isArray(defaults)) { - defaults = {}; - } - // Three-valued domain: false/absent → aliases; true → full IDs; "omit" → ''. - // Honor ONLY an explicit canonical `true` opt-in (full model IDs) and an existing - // "omit"; default everything else — absent, falsy, OR any non-canonical value — to - // "omit", the safe non-Claude default. Allowlist-based so malformed values - // (0, "", "yes", {}, …) don't leak Claude aliases the runtime can't resolve (#1569). - const existing = defaults.resolve_model_ids; - const shouldDefaultToOmit = existing !== true && existing !== 'omit'; - if (shouldDefaultToOmit) { - defaults.resolve_model_ids = 'omit'; - fs.writeFileSync(defaultsPath, JSON.stringify(defaults, null, 2) + '\n'); - console.log(` ${green}✓${reset} Set resolve_model_ids: "omit" in ~/.gsd/defaults.json`); - } - - // #2395: also persist `runtime: ` for non-Claude runtimes, so - // resolveRuntime() (precedence: GSD_RUNTIME env > config.runtime > 'claude') - // resolves to the install's actual runtime identity out of the box — without - // this, agent_runtime and every runtime-branded slash hint falls through to - // the hard-coded 'claude' default. Mirrors the resolve_model_ids write above: - // honor an explicit pre-existing value (any string), only default-populating - // when absent. Claude is the resolveRuntime() fallback, so it needs no write. - if (defaults.runtime === undefined || defaults.runtime === null || defaults.runtime === '') { - defaults.runtime = runtime; - fs.writeFileSync(defaultsPath, JSON.stringify(defaults, null, 2) + '\n'); - console.log(` ${green}✓${reset} Set runtime: "${runtime}" in ~/.gsd/defaults.json`); - } - } catch (e) { - console.log(` ${yellow}⚠${reset} Could not write ~/.gsd/defaults.json: ${e.message}`); - } - } + // #2834: defaults.json (resolve_model_ids + runtime) is now written BEFORE + // installCodexConfig via writeNonClaudeDefaults(runtime) — extracted into a + // function so it can run at the right point in the flow (before agent TOML + // generation reads it). This call is idempotent (preserves existing values). + writeNonClaudeDefaults(runtime); // program + command are now single-source lookups (ADR-1239 Phase B / #1679): // program is the runtime display label; command is the per-host /gsd-new-project diff --git a/capabilities/ai-integration/capability.json b/capabilities/ai-integration/capability.json index 34b18e974..ff863c202 100644 --- a/capabilities/ai-integration/capability.json +++ b/capabilities/ai-integration/capability.json @@ -1,7 +1,7 @@ { "id": "ai-integration", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "AI design contract", "description": "AI-SPEC design contract workflow for phases that build AI systems; owns the AI integration command, agents, and workflow.ai_integration_phase activation key.", "tier": "full", diff --git a/capabilities/antigravity/capability.json b/capabilities/antigravity/capability.json index 03fd51986..457da115e 100644 --- a/capabilities/antigravity/capability.json +++ b/capabilities/antigravity/capability.json @@ -1,7 +1,7 @@ { "id": "antigravity", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Antigravity", "description": "Google Antigravity IDE — nested under ~/.gemini/antigravity; probed across 1.x and 2.x layouts; Gemini hook event dialect; flat skill layout; tier-1 support.", "tier": "core", diff --git a/capabilities/assumption-delta/capability.json b/capabilities/assumption-delta/capability.json index d76de0bdf..a7cfdbae9 100644 --- a/capabilities/assumption-delta/capability.json +++ b/capabilities/assumption-delta/capability.json @@ -1,7 +1,7 @@ { "id": "assumption-delta", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Assumption-delta architecture checkpoint", "description": "Rarely-firing advisory checkpoint that triggers when a phase makes something plural, optional, or chosen that used to be singular, required, or derived. Surfaces one identity-model question (promote the new general representation to primary, or add it alongside?) so a silent primary-key drift does not accumulate into a later user-facing bug. Non-blocking; fires only on a detected signal.", "tier": "full", diff --git a/capabilities/audit/capability.json b/capabilities/audit/capability.json index 1c2255a46..9c6c9310f 100644 --- a/capabilities/audit/capability.json +++ b/capabilities/audit/capability.json @@ -1,7 +1,7 @@ { "id": "audit", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Audit", "description": "Open-artifact audit and UAT-gap audit for milestone close gates; exposes `gsd-tools audit-uat` (cross-phase UAT outstanding items) and `gsd-tools audit-open` (structured open-artifact scan across debug, tasks, threads, todos, seeds, UAT, verification, context-questions).", "tier": "full", diff --git a/capabilities/augment/capability.json b/capabilities/augment/capability.json index e7163b40d..a38a7472e 100644 --- a/capabilities/augment/capability.json +++ b/capabilities/augment/capability.json @@ -1,7 +1,7 @@ { "id": "augment", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Augment Code", "description": "Augment Code CLI — commands + nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", diff --git a/capabilities/broken-windows/capability.json b/capabilities/broken-windows/capability.json index d58cf390f..a26cb976f 100644 --- a/capabilities/broken-windows/capability.json +++ b/capabilities/broken-windows/capability.json @@ -1,7 +1,7 @@ { "id": "broken-windows", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Broken-windows ledger", "description": "Cross-phase defect register accumulating stubs, TODOs, skipped tests, unrun verifies, and unmet truths into .planning/WINDOWS.md. Blocks /gsd-ship while any window is open unless explicitly waived with a recorded reason. Operationalizes GSD's no-defer discipline as a tracked, enforced artifact (issue #1950).", "tier": "full", diff --git a/capabilities/claude-orchestration/capability.json b/capabilities/claude-orchestration/capability.json index 148717016..e9293ae52 100644 --- a/capabilities/claude-orchestration/capability.json +++ b/capabilities/claude-orchestration/capability.json @@ -1,7 +1,7 @@ { "id": "claude-orchestration", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Claude orchestration (Workflow backend)", "description": "Default-off, BETA, claude-only capability that adopts Claude Code's Workflow tool (the engine behind /effort ultracode) as an optional parallel-execution backend for the GSD loop. When the runtime exposes the Workflow tool and claude_orchestration.execution_backend resolves to 'workflow', execute-phase emits a generated Workflow script (waves -> parallel() barriers, plans -> agent({ agentType: 'gsd-executor', isolation: 'worktree' }), files_modified overlap -> separate sequential stages, resumeFromRunId wired to the phase run id, shared token budget) that composes the SAME gsd-executor agent and worktree isolation the inline path uses, restoring the wave parallelism the #853 backgrounded-agent nesting limitation forces inline on Claude Code. (The plan-checker and verifier remain inline until separately wired — this capability delivers the parallel-execution backend, not those gates.) Also folds the ultraplan plan-offload under one runtime gate (plan:* surface). On any runtime lacking the Workflow tool, or when the capability is disabled, behaviour is byte-identical to today (inline/manual dispatch). Detection + emission live in gsd-core/bin/lib/claude-orchestration.cjs (pure, fail-closed). Mirrors the existing gsd-ultraplan-phase BETA-isolation posture.", "tier": "full", diff --git a/capabilities/claude/capability.json b/capabilities/claude/capability.json index f6d6e5664..6e7ed60a5 100644 --- a/capabilities/claude/capability.json +++ b/capabilities/claude/capability.json @@ -1,7 +1,7 @@ { "id": "claude", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Claude Code", "description": "Anthropic Claude Code — primary development runtime; tier-1 support with full hook surface and skills-based global install.", "tier": "core", diff --git a/capabilities/cline/capability.json b/capabilities/cline/capability.json index 9ff77a496..7a53034f8 100644 --- a/capabilities/cline/capability.json +++ b/capabilities/cline/capability.json @@ -1,7 +1,7 @@ { "id": "cline", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cline", "description": "Cline (VS Code extension) — global-only nested-skill layout; cline-rules hook surface (.clinerules); no hook events emitted; tier-2 support.", "tier": "core", diff --git a/capabilities/code-review/capability.json b/capabilities/code-review/capability.json index e56af5f5d..9f282cd65 100644 --- a/capabilities/code-review/capability.json +++ b/capabilities/code-review/capability.json @@ -1,7 +1,7 @@ { "id": "code-review", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Code review", "description": "Source-file code review and review-fix workflow support for completed execution work.", "tier": "full", diff --git a/capabilities/codebuddy/capability.json b/capabilities/codebuddy/capability.json index b7a556029..ceaaa20dd 100644 --- a/capabilities/codebuddy/capability.json +++ b/capabilities/codebuddy/capability.json @@ -1,7 +1,7 @@ { "id": "codebuddy", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "CodeBuddy", "description": "CodeBuddy (Tencent) — converted commands + skills artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", diff --git a/capabilities/coderabbit/capability.json b/capabilities/coderabbit/capability.json index bc746ad3c..99a55531c 100644 --- a/capabilities/coderabbit/capability.json +++ b/capabilities/coderabbit/capability.json @@ -1,7 +1,7 @@ { "id": "coderabbit", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "CodeRabbit", "description": "CodeRabbit CLI — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). Reviews the working-tree diff (`coderabbit review --prompt-only`), not the source tree, and accepts neither a prompt nor a model flag; findings are down-weighted in consensus (evidenceClass: diff-only).", "tier": "full", diff --git a/capabilities/codex/capability.json b/capabilities/codex/capability.json index b29166d8a..3e5452250 100644 --- a/capabilities/codex/capability.json +++ b/capabilities/codex/capability.json @@ -1,7 +1,7 @@ { "id": "codex", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenAI Codex CLI", "description": "OpenAI Codex CLI — shell-var command style; per-agent sandbox tiers; config.toml + hooks.json hook surface; tier-1 support.", "tier": "core", diff --git a/capabilities/copilot/capability.json b/capabilities/copilot/capability.json index 32e83ef46..195a42949 100644 --- a/capabilities/copilot/capability.json +++ b/capabilities/copilot/capability.json @@ -1,7 +1,7 @@ { "id": "copilot", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "GitHub Copilot", "description": "GitHub Copilot (VS Code) — markdown config format; copilot-inline hook surface; no hook events emitted; flat skill nesting (unconfirmed recursive loader); tier-2 support.", "tier": "core", diff --git a/capabilities/cursor/capability.json b/capabilities/cursor/capability.json index 24b5cd246..8f3d0476f 100644 --- a/capabilities/cursor/capability.json +++ b/capabilities/cursor/capability.json @@ -1,7 +1,7 @@ { "id": "cursor", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cursor", "description": "Cursor IDE — skills + converted commands artifact layout; hooks.json surface; Claude hook event dialect; recursive skill loader (flat nesting); tier-2 support.", "tier": "core", diff --git a/capabilities/drift/capability.json b/capabilities/drift/capability.json index 95c85e74d..71dc1f599 100644 --- a/capabilities/drift/capability.json +++ b/capabilities/drift/capability.json @@ -1,7 +1,7 @@ { "id": "drift", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Drift detection gates", "description": "Drift detection gates for the planning loop. At execute:wave:post: a blocking schema drift gate (detects schema files changed without a database push) and a non-blocking codebase drift gate (detects structural additions not reflected in STRUCTURE.md). At plan:pre: a non-blocking, warn-only codebase drift gate (gated on workflow.plan_drift_precheck) that flags a stale codebase map before planning, so plans are authored against a fresh STRUCTURE.md instead of discovering drift mid-execution.", "tier": "full", diff --git a/capabilities/external-job/capability.json b/capabilities/external-job/capability.json index d668a77ad..f579d9f0d 100644 --- a/capabilities/external-job/capability.json +++ b/capabilities/external-job/capability.json @@ -1,7 +1,7 @@ { "id": "external-job", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Async external-job scheduler adapter", "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies execute:wave:pre, but execute-phase.md only dispatches execute:wave:post today (wave:pre is declared in the loop host contract but not rendered); wiring wave:pre dispatch is a core-loop change #1164 explicitly puts out of scope, so this capability registers at wave:post and the executor honors the runtime_budget classification guidance before running any tagged task. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", "tier": "full", diff --git a/capabilities/gap-analysis/capability.json b/capabilities/gap-analysis/capability.json index 55e32c655..0d80148a3 100644 --- a/capabilities/gap-analysis/capability.json +++ b/capabilities/gap-analysis/capability.json @@ -1,7 +1,7 @@ { "id": "gap-analysis", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Post-planning gap analysis", "description": "Proactive, non-blocking post-planning coverage report. After all PLAN.md files are generated, cross-references every REQ-ID and D-ID from REQUIREMENTS.md and CONTEXT.md against plan bodies. Emits a Source | Item | Status table. Does not block phase advancement.", "tier": "standard", diff --git a/capabilities/gemini/capability.json b/capabilities/gemini/capability.json index 4bafe8596..071d81ae6 100644 --- a/capabilities/gemini/capability.json +++ b/capabilities/gemini/capability.json @@ -1,7 +1,7 @@ { "id": "gemini", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "Gemini CLI", "description": "Google Gemini CLI — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). Spawned as `gemini -p - -m ` with the plan piped on stdin.", "tier": "full", diff --git a/capabilities/graphify/capability.json b/capabilities/graphify/capability.json index 049249e8f..7393eee28 100644 --- a/capabilities/graphify/capability.json +++ b/capabilities/graphify/capability.json @@ -1,7 +1,7 @@ { "id": "graphify", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Knowledge graph", "description": "Build, query, and inspect the project knowledge graph in `.planning/graphs/`; exposes graphify CLI subcommands (build, query, status, diff) and the /gsd-graphify skill.", "tier": "full", diff --git a/capabilities/hermes/capability.json b/capabilities/hermes/capability.json index a21404c81..a045f71df 100644 --- a/capabilities/hermes/capability.json +++ b/capabilities/hermes/capability.json @@ -1,7 +1,7 @@ { "id": "hermes", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Hermes Agent", "description": "Hermes Agent (NousResearch) — skills nest under skills/gsd/ category bucket; nested skill layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", diff --git a/capabilities/intel/capability.json b/capabilities/intel/capability.json index a3808b7a9..68ce14c8e 100644 --- a/capabilities/intel/capability.json +++ b/capabilities/intel/capability.json @@ -1,7 +1,7 @@ { "id": "intel", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Codebase intelligence", "description": "Code-intelligence store for codebase querying, diff, snapshot, and API-surface extraction; exposes `gsd-tools intel` subcommands (query, status, update, diff, snapshot, patch-meta, validate, extract-exports, api-surface) and backs `/gsd-map-codebase` and `gsd-intel-updater`.", "tier": "full", diff --git a/capabilities/kilo/capability.json b/capabilities/kilo/capability.json index 839b99948..728520b56 100644 --- a/capabilities/kilo/capability.json +++ b/capabilities/kilo/capability.json @@ -1,7 +1,7 @@ { "id": "kilo", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kilo Code", "description": "Kilo Code — XDG-based config dir; global skills at ~/.kilo/skills (separate from XDG config); flat command/ + skills artifact layout; no lifecycle hook registration; tier-2 support.", "tier": "core", diff --git a/capabilities/kimi-code/capability.json b/capabilities/kimi-code/capability.json index 221bbb63b..85b99d05b 100644 --- a/capabilities/kimi-code/capability.json +++ b/capabilities/kimi-code/capability.json @@ -1,7 +1,7 @@ { "id": "kimi-code", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi Code CLI", "description": "Kimi Code CLI (Moonshot AI, Node) — Agent Skills auto-discovered at ~/.kimi-code/skills; global AGENTS.md at ~/.kimi-code/AGENTS.md; native config.toml + [[hooks]] bus; three built-in subagents (coder/explore/plan), NO custom named subagents; background dispatch; tier-2 support. Distinct from Python kimi-cli (the 'kimi' capability) per ADR-1239 EoS — Kimi Code cannot dispatch named subagents so the kimi-agents YAML layout does NOT apply; persona injection rides the existing ${AGENT_SKILLS_*} workflow fallback. Install-layout, agent-install-check, and install-time decision (kimi vs kimi-code) land in follow-up PRs; this descriptor is the EoS foundation.", "tier": "core", diff --git a/capabilities/kimi/capability.json b/capabilities/kimi/capability.json index 4bef4d843..b97dab77e 100644 --- a/capabilities/kimi/capability.json +++ b/capabilities/kimi/capability.json @@ -1,7 +1,7 @@ { "id": "kimi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi CLI", "description": "Kimi CLI (Moonshot AI) — generic agents root at ~/.config/agents; skills + kimi-agents artifact layout; native config.toml [[hooks]] bus at ~/.kimi/config.toml; background dispatch; tier-2 support.", "tier": "core", diff --git a/capabilities/llama-cpp/capability.json b/capabilities/llama-cpp/capability.json index 68ed465fe..eb7a315e5 100644 --- a/capabilities/llama-cpp/capability.json +++ b/capabilities/llama-cpp/capability.json @@ -1,7 +1,7 @@ { "id": "llama-cpp", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "llama.cpp", "description": "llama.cpp server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.llama_cpp_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq. Capability id/folder are kebab (`llama-cpp`, required by KEBAB_RE); `reviewer.slug` stays snake (`llama_cpp`) to match the shipped roster and the `review.llama_cpp_host` config key (ADR-2782's three-namespace trap).", "tier": "full", diff --git a/capabilities/lm-studio/capability.json b/capabilities/lm-studio/capability.json index 1f3b945b1..ccc8f74fa 100644 --- a/capabilities/lm-studio/capability.json +++ b/capabilities/lm-studio/capability.json @@ -1,7 +1,7 @@ { "id": "lm-studio", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "LM Studio", "description": "LM Studio local model server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.lm_studio_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq. Capability id/folder are kebab (`lm-studio`, required by KEBAB_RE); `reviewer.slug` stays snake (`lm_studio`) to match the shipped roster and the `review.lm_studio_host` config key (ADR-2782's three-namespace trap).", "tier": "full", diff --git a/capabilities/mempalace/capability.json b/capabilities/mempalace/capability.json index c257ecdd1..1e89267ca 100644 --- a/capabilities/mempalace/capability.json +++ b/capabilities/mempalace/capability.json @@ -1,7 +1,7 @@ { "id": "mempalace", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "MemPalace memory", "description": "Cross-session, cross-project memory: deliberate recall before discuss/plan and verbatim capture + temporal-KG sync at phase boundaries, via the MemPalace MCP server and CLI.", "tier": "full", diff --git a/capabilities/nyquist/capability.json b/capabilities/nyquist/capability.json index 0bc7e3940..8a5dd0f7d 100644 --- a/capabilities/nyquist/capability.json +++ b/capabilities/nyquist/capability.json @@ -1,7 +1,7 @@ { "id": "nyquist", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Nyquist validation", "description": "Validation coverage audit that maps executed work back to tests and manual-only evidence.", "tier": "full", diff --git a/capabilities/ollama/capability.json b/capabilities/ollama/capability.json index edec85a47..82585b547 100644 --- a/capabilities/ollama/capability.json +++ b/capabilities/ollama/capability.json @@ -1,7 +1,7 @@ { "id": "ollama", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "Ollama", "description": "Ollama local model server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.ollama_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq.", "tier": "full", diff --git a/capabilities/opencode/capability.json b/capabilities/opencode/capability.json index be15a3ff1..2ababe381 100644 --- a/capabilities/opencode/capability.json +++ b/capabilities/opencode/capability.json @@ -1,7 +1,7 @@ { "id": "opencode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenCode", "description": "OpenCode — XDG-based config dir; flat commands/ + skills artifact layout; settings-json config format; no lifecycle hook registration; tier-2 support.", "tier": "core", diff --git a/capabilities/pattern-mapper/capability.json b/capabilities/pattern-mapper/capability.json index 2c7d83cd0..10dab9c15 100644 --- a/capabilities/pattern-mapper/capability.json +++ b/capabilities/pattern-mapper/capability.json @@ -1,7 +1,7 @@ { "id": "pattern-mapper", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Pattern mapping", "description": "Optional codebase-pattern mapping before planning; owns the pattern mapper agent and workflow.pattern_mapper activation key.", "tier": "full", diff --git a/capabilities/pi/capability.json b/capabilities/pi/capability.json index cd89f7588..47ce0c82f 100644 --- a/capabilities/pi/capability.json +++ b/capabilities/pi/capability.json @@ -1,7 +1,7 @@ { "id": "pi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "pi", "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", diff --git a/capabilities/profile-pipeline/capability.json b/capabilities/profile-pipeline/capability.json index d6d11245a..0763f4212 100644 --- a/capabilities/profile-pipeline/capability.json +++ b/capabilities/profile-pipeline/capability.json @@ -1,7 +1,7 @@ { "id": "profile-pipeline", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Developer profiling pipeline", "description": "Developer behavioral profiling from Claude Code session history; scans session JSONL files, extracts and samples user messages, and generates profile artifacts (USER-PROFILE.md, dev-preferences.md, CLAUDE.md sections). Exposes eight `gsd-tools` commands: scan-sessions, extract-messages, profile-sample (pipeline phase) and write-profile, profile-questionnaire, generate-dev-preferences, generate-claude-profile, generate-claude-md (output phase). Backs the /gsd-profile-user skill and gsd-user-profiler agent.", "tier": "full", diff --git a/capabilities/qwen/capability.json b/capabilities/qwen/capability.json index 9c426649a..875aaad28 100644 --- a/capabilities/qwen/capability.json +++ b/capabilities/qwen/capability.json @@ -1,7 +1,7 @@ { "id": "qwen", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Qwen Code", "description": "Qwen Code (Alibaba) — nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", diff --git a/capabilities/research/capability.json b/capabilities/research/capability.json index a9a6273be..2b39bc5f7 100644 --- a/capabilities/research/capability.json +++ b/capabilities/research/capability.json @@ -1,7 +1,7 @@ { "id": "research", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Phase research", "description": "Optional phase research before planning; owns the phase researcher agent and workflow.research activation key.", "tier": "standard", diff --git a/capabilities/schema-gate/capability.json b/capabilities/schema-gate/capability.json index 9c1098f4f..5164ad9cc 100644 --- a/capabilities/schema-gate/capability.json +++ b/capabilities/schema-gate/capability.json @@ -1,7 +1,7 @@ { "id": "schema-gate", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Schema push detection gate", "description": "Detects ORM schema-relevant files in the phase scope during planning and injects a mandatory [BLOCKING] schema push task into the plan. Prevents false-positive verification where build/types pass because TypeScript types come from config, not the live database.", "tier": "full", diff --git a/capabilities/security/capability.json b/capabilities/security/capability.json index 3d8fdb990..4e35605b2 100644 --- a/capabilities/security/capability.json +++ b/capabilities/security/capability.json @@ -1,7 +1,7 @@ { "id": "security", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Security enforcement", "description": "Threat mitigation verification and ship-time security blocking for phases with security enforcement enabled.", "tier": "full", diff --git a/capabilities/tdd/capability.json b/capabilities/tdd/capability.json index 667bed8a7..f645f1c46 100644 --- a/capabilities/tdd/capability.json +++ b/capabilities/tdd/capability.json @@ -1,7 +1,7 @@ { "id": "tdd", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Test-driven development", "description": "Injects TDD heuristics into the planner and enforces RED/GREEN gate compliance on type:tdd plans after execution. Owns workflow.tdd_mode; the --tdd CLI flag is the ephemeral override.", "tier": "full", diff --git a/capabilities/trae/capability.json b/capabilities/trae/capability.json index 9f55a9662..af4dfea03 100644 --- a/capabilities/trae/capability.json +++ b/capabilities/trae/capability.json @@ -1,7 +1,7 @@ { "id": "trae", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Trae IDE", "description": "Trae IDE — nested-skill artifact layout; no hook surface (profile-marker-only config); tier-2 support.", "tier": "core", diff --git a/capabilities/ui/capability.json b/capabilities/ui/capability.json index de8b03495..c9d525873 100644 --- a/capabilities/ui/capability.json +++ b/capabilities/ui/capability.json @@ -1,7 +1,7 @@ { "id": "ui", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "UI design contracts", "description": "UI-SPEC design contract + retrospective UI audit for frontend phases.", "tier": "full", diff --git a/capabilities/vscode/capability.json b/capabilities/vscode/capability.json index 984546656..5edf8248f 100644 --- a/capabilities/vscode/capability.json +++ b/capabilities/vscode/capability.json @@ -1,7 +1,7 @@ { "id": "vscode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "VS Code", "description": "VS Code — Marketplace/VSIX extension; no file-projected config directory; IDE-profile reference host (active vscode.lm model, engine-owned hook bus, sandboxed globalState/workspaceState stateIO).", "tier": "core", diff --git a/capabilities/windsurf/capability.json b/capabilities/windsurf/capability.json index cc56004d2..123a451f6 100644 --- a/capabilities/windsurf/capability.json +++ b/capabilities/windsurf/capability.json @@ -1,7 +1,7 @@ { "id": "windsurf", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Windsurf", "description": "Windsurf (Codeium) — workspace workflow artifact layout for slash commands; Cascade native hooks.json blocking hook bus (pre_write_code, pre_run_command); tier-2 support.", "tier": "core", diff --git a/capabilities/zcode/capability.json b/capabilities/zcode/capability.json index e934bcf3e..f117f588b 100644 --- a/capabilities/zcode/capability.json +++ b/capabilities/zcode/capability.json @@ -1,7 +1,7 @@ { "id": "zcode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "ZCode", "description": "ZCode (Z.ai) — desktop Agentic Development Environment for GLM-5.2; Claude-shaped nested skills at ~/.zcode/skills//SKILL.md, slash commands, named subagents, native MCP; declarative plugin surface; profile-marker install; tier-2 community support.", "tier": "core", diff --git a/docs/README.md b/docs/README.md index 4d1d1f5ba..08a22c305 100644 --- a/docs/README.md +++ b/docs/README.md @@ -36,6 +36,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [Spike and sketch](how-to/spike-and-sketch.md) — use `/gsd-spike` and `/gsd-sketch` for exploratory work before committing to a plan - [Design a UI phase](how-to/design-a-ui-phase.md) — use the UI phase loop for frontend and visual work - [Develop a Capability for GSD 1.5+](how-to/develop-a-capability.md) — add feature Capabilities, hook fragments, and registry entries +- [Ship a reviewer lane in your capability](how-to/ship-a-reviewer-lane.md) — declare a `reviewer` body so `/gsd-review` discovers, invokes, and renders your external review CLI or model endpoint - [Add or update a host's integration](how-to/add-or-update-a-host-integration.md) — set a host's documentation-sourced `runtime.hostIntegration` axes (ADR-1239 Phase A), with the `undocumented` sentinel rule - [Turn a capability off (and keep it off)](how-to/turn-a-capability-off.md) — disable a capability via the surface, or gate individual hooks off without removing the capability - [Drive GSD from a tracker issue](how-to/drive-gsd-from-a-tracker-issue.md) — start a phase from a GitHub, Linear, or Jira issue diff --git a/docs/how-to/develop-a-capability.md b/docs/how-to/develop-a-capability.md index 75de679c3..38cae6abe 100644 --- a/docs/how-to/develop-a-capability.md +++ b/docs/how-to/develop-a-capability.md @@ -245,6 +245,7 @@ From GSD 1.6.0, capabilities are versioned (the `version` field is required in ` - **How-to** — [Import a capability from a URL](../how-to/import-a-capability-from-a-url.md): install a third-party capability from a git URL, tarball, or npm package. - **How-to** — [Version and update a capability](../how-to/version-a-capability.md): manage `version`, `engines.gsd`, and `compatVersions`; use `gsd capability update`. - **How-to** — [Remove a capability](../how-to/remove-a-capability.md): uninstall cleanly with `gsd capability remove`, including the `--purge-data` option. +- **How-to** — [Ship a reviewer lane in your capability](../how-to/ship-a-reviewer-lane.md): declare a `reviewer` body (GSD 1.9.0+) so `/gsd-review` discovers and invokes your external review CLI or model endpoint. - **Reference** — [Capability manifest](../reference/capability-manifest.md): all fields and validation rules for `capability.json`. - **Reference** — [Capability matrix](../reference/capability-matrix.md): which first-party capabilities exist, their extension points, and their compatibility matrix. - **Explanation** — [Capability trust model](../explanation/capability-trust-model.md): how declarative and executable capabilities are treated differently at install time. diff --git a/docs/how-to/set-up-cross-ai-review.md b/docs/how-to/set-up-cross-ai-review.md index 583404614..b7f88266e 100644 --- a/docs/how-to/set-up-cross-ai-review.md +++ b/docs/how-to/set-up-cross-ai-review.md @@ -8,7 +8,9 @@ ## Decide which reviewers to use -GSD Core can route review requests to any combination of: Gemini CLI, Claude (separate session), Codex CLI, CodeRabbit, OpenCode, Qwen Code, Cursor, Antigravity CLI, Ollama, LM Studio, and llama.cpp. +GSD Core can route review requests to any combination of: Gemini CLI, Claude (separate session), Codex CLI, CodeRabbit, OpenCode, Qwen Code, Cursor, Antigravity CLI, Ollama, LM Studio, llama.cpp, and Kimi Code. + +That list is not fixed. Each of those is a declared reviewer lane, and a capability can ship its own — see [Ship a reviewer lane in your capability](ship-a-reviewer-lane.md). To see exactly which lanes your installation has, run `gsd-tools review-lane sections`. Each reviewer runs the same structured prompt against your `PLAN.md` files independently. Because different models have different blind spots, multi-reviewer consensus catches more issues than any single reviewer. @@ -156,6 +158,7 @@ This runs `plan-phase → review → replan → re-review` up to three cycles (d ## Related - [Verify and ship](verify-and-ship.md) +- [Ship a reviewer lane in your capability](ship-a-reviewer-lane.md) — add a reviewer GSD does not ship with, by declaring it in a capability manifest - [Configuration](../CONFIGURATION.md) - [Commands](../COMMANDS.md) - [docs index](../README.md) diff --git a/docs/how-to/ship-a-reviewer-lane.md b/docs/how-to/ship-a-reviewer-lane.md new file mode 100644 index 000000000..a874449db --- /dev/null +++ b/docs/how-to/ship-a-reviewer-lane.md @@ -0,0 +1,245 @@ +# How to ship a reviewer lane in your capability + +**Goal:** Declare a *reviewer lane* in a capability manifest so `/gsd-review` discovers your external review CLI or model endpoint, offers a flag for it, invokes it, and renders its output into `REVIEWS.md` — without patching GSD core. + +**Prerequisites:** You already have a capability (`capability.json`), or you are creating one. The reviewer tool is installed and works from your shell. GSD 1.9.0 or later. + +Before 1.9.0 a reviewer was a core patch: a hardcoded roster entry, a hand-authored bash leg in `review.md`, a hardcoded output heading, and central config keys. From 1.9.0 a lane is manifest data, so shipping a reviewer is shipping a capability. See [ADR-2782](../adr/2782-reviewer-lane-capability-surface.md) for the decision record. + +--- + +## Decide which shape your lane takes + +A `reviewer` body is admissible on two roles. Pick by whether GSD installs *into* your tool. + +| Your situation | Use | Why | +|---|---|---| +| Your capability is already a runtime GSD installs into (it has a `runtime` body) and that same CLI can also review | Keep `role: "runtime"`, add a `reviewer` body | One manifest stays one manifest — this is how `codex`, `cursor`, and `antigravity` ship | +| Your tool only reviews — GSD never installs commands, agents, or skills into it | `role: "reviewer"` | The honest description: a lane with no install surface, like `gemini`, `coderabbit`, and `ollama` | +| Your capability adds planning steps, gates, or contributions | `role: "feature"` — and a separate lane capability | A feature manifest may not carry a `reviewer` body; the validator rejects it | + +A `role: "reviewer"` capability **must** carry a `reviewer` body, **must not** carry a `runtime` body, and **must not** carry any feature-only field — `skills`, `agents`, `steps`, `contributions`, `gates`, `hooks`, or `activationKey`. A lane owns no artifacts and wires no loop extension point. + +--- + +## Declare a spawned-CLI lane + +Most lanes are `transport: "spawn"` — GSD runs a binary and reads its output. Add a `reviewer` block to your manifest: + +```json +{ + "id": "acme-review", + "role": "reviewer", + "version": "1.0.0", + "title": "Acme Review CLI", + "description": "Acme CLI — cross-AI /gsd-review reviewer lane only; not a GSD install target.", + "tier": "full", + "requires": [], + "engines": { "gsd": ">=1.9.0" }, + + "reviewer": { + "slug": "acme", + "flags": ["--acme"], + "transport": "spawn", + "probe": { "kind": "command-exists", "binary": "acme" }, + "invoke": { + "binary": "acme", + "args": ["review", "{{model}}", "-p", "-"], + "promptChannel": "stdin", + "outputChannel": "stdout", + "modelArg": "--model", + "effortChannel": "none" + }, + "timeoutFloorMs": 900000, + "emptyOutput": "stub-with-stderr", + "reviewsSection": "Acme", + "evidenceClass": "source-grounded", + "requiresBinaries": [], + "promptBudgetKey": null, + "modelConfigKey": "review.models.acme", + "handler": null + }, + + "config": { + "review.models.acme": { + "type": "string", + "default": "", + "description": "Model passed to the Acme reviewer lane." + } + } +} +``` + +Four fields decide whether the lane works at all, so get these right first: + +- **`invoke.args`** carries the `{{model}}`, `{{prompt}}`, `{{effort}}`, and `{{output}}` placeholders. GSD substitutes them; anything else is passed through literally. +- **`promptChannel`** says how the plan text reaches the tool — `stdin` when it reads a pipe, `argv` or `argv-file-ref` when it takes the prompt path as an argument, `none` when the tool reads the working tree itself (as CodeRabbit does). +- **`outputChannel`** is `stdout`, or `file-arg` when the tool writes to a path you name (then also declare `invoke.outputArg`, as `codex` does with `-o`). +- **`timeoutFloorMs`** is the measured wall-clock floor for *your* tool. Lane divergence here is expected and correct — the descriptor exists to declare divergence in one place, not to impose one number on every lane. + +`reviewsSection` is the heading your findings render under in `REVIEWS.md`. It must be unique across every installed lane; two lanes sharing a heading would silently merge their output into apparent consensus that never happened. + +For the full field table — every type, enum member, and default — see [Capability manifest § Reviewer body](../reference/capability-manifest.md#reviewer-body-role-reviewer-or-on-any-role). + +--- + +## Declare an OpenAI-compatible HTTP lane instead + +If your reviewer is a served model endpoint rather than a CLI, use `transport: "openai-http"`. The `invoke` block takes a different shape — a destination, not a binary: + +```json +"reviewer": { + "slug": "acme_local", + "flags": ["--acme-local"], + "transport": "openai-http", + "probe": { + "kind": "http-reachable", + "hostConfigKey": "review.acme_host", + "path": "/v1/models", + "timeoutMs": 2000 + }, + "invoke": { + "hostConfigKey": "review.acme_host", + "defaultHost": "http://localhost:8080", + "path": "/v1/chat/completions", + "modelDiscovery": "first-from-models-endpoint", + "fallbackModel": "acme-7b", + "effortChannel": "none" + }, + "timeoutFloorMs": 120000, + "emptyOutput": "stub-with-stderr", + "reviewsSection": "Acme Local", + "evidenceClass": "source-grounded", + "requiresBinaries": [], + "promptBudgetKey": "review.max_prompt_tokens_per_reviewer.acme_local", + "modelConfigKey": "review.models.acme_local", + "handler": "openai-compatible" +} +``` + +`handler: "openai-compatible"` is what gives you model discovery against `/v1/models`, the request and response shape, and the served-model mismatch warning. Declare the matching `review.acme_host` key in your `config` block alongside the model key. + +**Every probe is bounded.** An unbounded `--help | grep` probe is a named defect in this repo. `http-reachable` requires `timeoutMs`; so does `command-capability`, the probe kind you use when a bare binary name is ambiguous. + +--- + +## Own your lane's config keys + +Declare the lane's keys in your own manifest `config` block, never in the central schema. A key present in both is a build failure, not a warning — federated ownership is exclusive. + +Name `modelConfigKey` and `promptBudgetKey` to match keys you actually declare. A lane pointing at a key nobody owns resolves to nothing, which reads to the user as "my model override is being ignored." + +Users then set them the ordinary way, in `.planning/config.json`: + +```json +{ "review": { "models": { "acme": "acme-large" } } } +``` + +--- + +## Build and install + +If your capability lives in the GSD repo, regenerate the committed registry and check for drift: + +```bash +npm run gen:capability-registry +npm run lint:generated-sync +``` + +If you are shipping out-of-tree, package and install it like any other capability: + +```bash +gsd capability install +``` + +Uniqueness is checked across the merged first-party ∪ overlay set — a duplicate `slug`, a duplicate entry in `flags`, or a duplicate `reviewsSection` collides. **Expect that collision to be quiet.** `gsd capability install` does not run the cross-capability check; it runs at *load* time, and a colliding overlay is dropped from the active set with a warning rather than failing the install. First-party always wins. + +That failure mode is worth internalizing before you debug it: the install command reports success, and your lane simply never appears. If a lane you just installed is missing from `gsd-tools review-lane sections`, suspect a name collision before you suspect the probe. Malformed values inside the body — a `slug` outside `^[a-z0-9][a-z0-9_-]*$`, an enum member that does not exist, an `outputArg` without `outputChannel: "file-arg"` — are ordinary validation errors and are reported directly. + +Two naming rules are easy to conflate, so keep them apart. Your `slug` may not be `__proto__`, `constructor`, or `prototype` — a prototype-pollution guard, not a namespace policy; any other grammatical slug is yours, including one starting `gsd-`. Your capability **`id`**, separately, may not begin with `gsd-`, `gsd-core-`, or `anthropic-`; those prefixes are reserved so nothing can impersonate a first-party capability. + +An *unknown* field inside your `reviewer` body behaves differently: it is a non-fatal warning on stderr, never a build failure. A manifest built against a newer GSD degrades visibly instead of crashing. + +### Listing your lane is not wired yet + +You can build, install, and run a third-party lane today. You cannot yet **list** it in a discoverability catalog, and it is better to know that before you write the entry than after. + +Neither existing registry accepts a lane. A [Community Capability Registry](../registries/capability-registry.md) entry requires a non-empty `loopExtensionPoints` and a `hookKinds` value, and a lane registers on zero loop extension points by design — the two fields are unsatisfiable rather than merely unset. The [EoS Registry](../registries/eos-registry.md) is for host integrations that embed the orchestration engine through the ADR-1239 interface, which a reviewer lane does not do. + +Do not work around this by filing a loop extension point your lane does not use. A third `reviewer` entry type is tracked by [#2904](https://github.com/open-gsd/gsd-core/issues/2904) for a 1.9.x point release; until it lands, distribute your lane by URL and it will install and run normally. + +--- + +## Verify the lane resolves + +Check that GSD sees your lane before you run a real review: + +```bash +gsd-tools review-lane sections +gsd-tools review-lane flags +``` + +`sections` lists every `slug` with the heading it renders under; `flags` lists every selector flag. Your lane appears in both, or it is not installed. + +Then dry-check the invocation plan for your lane alone: + +```bash +gsd-tools review-lane plan --selected acme +``` + +A resolvable lane returns `"ok": true` with its `section`, `transport`, and prompt path. Once that is green, run it for real against a planned phase: + +```bash +/gsd-review --phase 3 --acme +``` + +If the lane is absent from `--all`, the probe is the usual culprit: `command-exists` fails silently when the binary is not on `PATH` in the environment GSD runs in. + +--- + +## Know what your users are consenting to + +A reviewer lane is a **fourth executable-surface disclosure class**, alongside hooks, command modules, and MCP servers — and it is the only one that *receives* data. Your lane is piped plan text, requirements, research findings, and `CONTEXT.md` decisions. Install-time disclosure says so plainly: + +```text + reviewer lane (1): an external reviewer receives plan/review data on every run + - acme -> acme review --model acme-large -p - + sends: plan text, requirements, research findings, CONTEXT.md decisions +``` + +Be clear-eyed about what that buys, because your users are trusting your judgment as much as the mechanism. ADR-2782 D5 says it plainly: disclosure and host pinning make the channel *"visible, pinned, and revocable — it does not make it safe"*, and consent-at-install is a **weaker gate for a standing egress channel than for a hook**. A user consents once; your lane thereafter receives every plan on every review run. A per-run prompt was considered and rejected as consent fatigue. Design your lane as if that single consent is the only one you will ever get, because it is. + +Three consequences you should design for: + +- **Your `args` are signature-bound, not just your binary.** Changing `binary`, `args`, `hostConfigKey`, `promptChannel`, or `handler` in a new version re-triggers consent on update. Changing `reviewsSection` or `timeoutFloorMs` does not — a cosmetic prompt is how users learn to click through. +- **An `openai-http` lane binds the *resolved host*, not just the config key.** GSD re-resolves `hostConfigKey` before every invocation and blocks the lane if the destination changed, rather than silently sending plans somewhere new. Users see the lane refuse and must re-consent. Point `defaultHost` at the address you actually mean. +- **What the user saw is only tamper-evident because the bundle is pinned.** The disclosure is trustworthy because the downloaded capability is integrity-checked and its hash recorded at install; a later change to your `args` or `hostConfigKey` shows up as a changed signature rather than sliding in quietly. Keep `engines.gsd` honest for the same reason — it is a hard gate, so a lane declaring a range it does not actually work on is blocked at install and skipped at load rather than failing confusingly at review time. + +For the reasoning behind consent-plus-integrity rather than a sandbox, see [The capability trust model](../explanation/capability-trust-model.md). + +--- + +## Conditionals: when the vocabulary does not fit your tool + +Third-party lanes are **data-only**. `handler` is a closed enum of first-party names (`antigravity`, `openai-compatible`, `opencode`, or `null`) — you may reference an existing member, but you cannot ship your own handler module. + +| Your tool | What to do | +|---|---| +| Runs one command, reads a prompt, writes a review | Declare it — the vocabulary covers this, which is eight of the twelve shipped lanes | +| Is an OpenAI-compatible endpoint | `transport: "openai-http"` with `handler: "openai-compatible"` | +| Needs a stateful setup turn before it can review (Plandex-style `new` then `review`) | Not expressible today — the descriptor describes one invocation. File an issue naming the primitive | +| Edits files or commits by default (Aider-style) | Not expressible today — there is no way to declare a read-only invocation posture, and the prompt asking politely is not a guarantee. File an issue naming the primitive | +| Needs genuinely imperative behavior for an upstream bug | File an issue. Named handlers are added first-party after review, the same path that widened the vocabulary to add `openai-http` | + +Filing the issue is the supported route, not a workaround. The `openai-http` transport exists because three real lanes did not fit and the vocabulary widened on that evidence. + +--- + +## Related + +- [Capability manifest](../reference/capability-manifest.md) — the full `reviewer` body field table and validation rules +- [Set up cross-AI review](set-up-cross-ai-review.md) — the user-facing side: choosing, configuring, and running reviewers +- [Develop a Capability for GSD 1.5+](develop-a-capability.md) — manifests, registry generation, and federated config +- [Publish a capability](publish-a-capability.md) — versioning, `engines.gsd`, and distribution +- [The capability trust model](../explanation/capability-trust-model.md) — disclosure, consent, and integrity +- [ADR-2782](../adr/2782-reviewer-lane-capability-surface.md) — why the lane became a capability surface, and what it deliberately excludes diff --git a/docs/reference/capability-manifest.md b/docs/reference/capability-manifest.md index ad6b78419..cbab1e46f 100644 --- a/docs/reference/capability-manifest.md +++ b/docs/reference/capability-manifest.md @@ -184,6 +184,8 @@ For a minimal `role: "runtime"` example, see [ADR-1016 §Decision 8](../adr/1016 [ADR-2782](../adr/2782-reviewer-lane-capability-surface.md) introduces the *reviewer lane*: one external CLI or model endpoint that `/gsd:review` hands a plan to for independent review. +To declare one, follow [Ship a reviewer lane in your capability](../how-to/ship-a-reviewer-lane.md). This section is the field reference behind that guide. + The `reviewer` body is **optional and absent-safe at every layer**. A capability with no `reviewer` body is simply not a lane — that is never a validation error. This is a normative forward/backward-compatibility invariant, not a nicety: a plugin, a runtime, or a future GSD version may omit `reviewer` entirely with no consequence. The shape is **hybrid**: @@ -201,7 +203,7 @@ All 12 shipped lane declarations carry all 13 fields below. | `flags` | string[] | User-facing CLI flags that select this lane. A lane may declare more than one — `antigravity` declares `--antigravity` and `--agy`. 12 lanes declare 13 flags in total. | | `transport` | closed enum | `spawn` \| `openai-http`. | | `probe` | object | Availability check. `probe.kind` is a closed enum: `command-exists` \| `command-capability` \| `http-reachable`. `command-capability` additionally takes `binary`, `needle`, and a **required** `timeoutMs` — it exists because a bare binary name can be ambiguous (`kimi` is claimed by both the Kimi Code CLI and the legacy Python `kimi-cli`), and the timeout bound is mandatory because an unbounded `--help \| grep` probe is this repo's named Unbounded Subprocesses defect. | -| `invoke` | object | `binary`, `args[]`, `promptChannel` (`stdin` \| `argv-file-ref` \| `none`), `outputChannel` (`stdout` \| `file-arg`), `modelArg` (string or `null`), `effortChannel` (`argv` \| `none`). `args` supports the `{{model}}` and `{{prompt}}` placeholders. | +| `invoke` | object | Shape is selected by `transport`. For `spawn`: `binary`, `args[]`, `promptChannel` (`stdin` \| `argv` \| `argv-file-ref` \| `none`), `outputChannel` (`stdout` \| `file-arg`), `outputArg` (required when `outputChannel` is `file-arg`), `modelArg` (string or `null`), `effortChannel` (`none` \| `argv` \| `env`). For `openai-http`: `hostConfigKey`, `defaultHost`, `path`, `modelDiscovery` (`none` \| `first-from-models-endpoint`), `fallbackModel`, `effortChannel`. `args` supports the `{{model}}`, `{{prompt}}`, `{{effort}}`, and `{{output}}` placeholders. | | `timeoutFloorMs` | number | Measured per-lane floor. Lane divergence here is real and correct — the descriptor's job is to declare divergence in one place, not to promise uniformity. | | `emptyOutput` | closed enum | `stub-with-stderr` \| `handler-owned`. | | `reviewsSection` | string | The `REVIEWS.md` heading this lane renders under. Must be unique across the merged roster. | diff --git a/docs/registries/README.md b/docs/registries/README.md index 8c3dc2133..bdf650244 100644 --- a/docs/registries/README.md +++ b/docs/registries/README.md @@ -1,6 +1,6 @@ -# GSD Registries: Community Capability Registry & EoS Registry +# GSD Registries: Community Capability Registry, EoS Registry & Reviewer Lane Registry -Specification, entry schema, and submission process for GSD's two third-party discoverability catalogs — the **GSD Community Capability Registry** and the **GSD EoS Registry**. +Specification, entry schema, and submission process for GSD's three third-party discoverability catalogs — the **GSD Community Capability Registry**, the **GSD EoS Registry**, and the **GSD Reviewer Lane Registry**. --- @@ -8,7 +8,7 @@ Specification, entry schema, and submission process for GSD's two third-party di > Inclusion in this registry means only that a maintainer merged a PR that linked to the author's repository. It is not an endorsement. GSD has not reviewed, tested, audited, or verified the correctness, quality, safety, or security of any listed solution, nor its claimed GSD interactions. Use at your own risk; evaluate the linked source yourself. Entries are removed only for illegal content, malware, spam, or a link that is dead/completely non-functional — never curated for quality. -This stance applies identically to every entry in both registries. It is reproduced verbatim at the top of each generated catalog (`capability-registry.md`, `eos-registry.md`). +This stance applies identically to every entry in all three registries. It is reproduced verbatim at the top of each generated catalog (`capability-registry.md`, `eos-registry.md`, `reviewer-registry.md`). ## Narrow removal policy @@ -25,18 +25,19 @@ A registry entry is **never** removed for quality, staleness of a working projec ## What gets listed -Two independent catalogs, sharing one schema shape, one non-endorsement stance, and one submission process: +Three independent catalogs, sharing one schema shape, one non-endorsement stance, and one submission process: - **Community Capability Registry** (`docs/registries/capability-registry.md`, generated from `docs/registries/capabilities.json`) — third-party **Feature Capabilities**: plug-ins that attach at GSD's Loop Extension Points (ADR-857, ADR-894, ADR-1244) and are installed with `gsd capability install `. - **EoS Registry** (`docs/registries/eos-registry.md`, generated from `docs/registries/eos.json`) — third-party **Embeddable Orchestration System (EoS)** host integrations: projects that embed GSD as an orchestration engine inside a host through the ADR-1239 Host-Integration Interface. +- **Reviewer Lane Registry** (`docs/registries/reviewer-registry.md`, generated from `docs/registries/reviewers.json`) — third-party **reviewer lanes**: `role: "reviewer"` capabilities (ADR-2782) that add an external review lane to `/gsd-review`, installed with `gsd capability install `. A lane registers on zero Loop Extension Points and owns no artifacts, which is why it cannot be listed as a Feature Capability. -Both registries are non-endorsing discoverability catalogs (issue #2182). Neither is the runtime **Capability Registry** (the generated manifest consumed at load time, ADR-894 §5) or the **Capability Registry Overlay** (the runtime loader that merges an installed third-party manifest into that generated registry, ADR-1244 D2) — see `CONTEXT.md` → "Community Capability Registry" and "EoS Registry" for the full disambiguation. +All three registries are non-endorsing discoverability catalogs (issue #2182, plus #2904 for the Reviewer Lane Registry). None is the runtime **Capability Registry** (the generated manifest consumed at load time, ADR-894 §5) or the **Capability Registry Overlay** (the runtime loader that merges an installed third-party manifest into that generated registry, ADR-1244 D2) — see `CONTEXT.md` → "Community Capability Registry", "EoS Registry", and "Reviewer Lane Registry" for the full disambiguation. --- ## Entry schema -Every entry is one JSON object in `docs/registries/capabilities.json` or `docs/registries/eos.json`, validated by `scripts/registry-schema.cjs`. Field names below are exact and case-sensitive; unknown top-level keys are rejected. +Every entry is one JSON object in `docs/registries/capabilities.json`, `docs/registries/eos.json`, or `docs/registries/reviewers.json`, validated by `scripts/registry-schema.cjs`. Field names below are exact and case-sensitive; unknown top-level keys are rejected. ### Capability entries (`capabilities.json`, `type: "capability"`) @@ -166,6 +167,66 @@ Example: } ``` +### Reviewer entries (`reviewers.json`, `type: "reviewer"`) + +| Field | Required | Meaning | +|---|---|---| +| `id` | yes | Unique slug across the registry (`^[a-z0-9]+(-[a-z0-9]+)*$`). | +| `name` | yes | Human-readable name. | +| `type` | yes | Must equal `"reviewer"`. | +| `repo` | yes | `owner/repo` on github.com — the author's own repository. | +| `description` | yes | One-paragraph plain-language description of the reviewer lane and what it reviews. | +| `author` | yes | Author name (and, optionally, contact). | +| `license` | yes | SPDX identifier (or `UNLICENSED` / `Proprietary`). | +| `enginesGsd` | yes | Declared `engines.gsd` semver range (ADR-1244 D1), e.g. `>=1.8.0`. | +| `install` | yes | Exact, copy-pasteable install command — the ADR-1244 URL-import flow, e.g. `gsd capability install https://github.com/OWNER/REPO.git#v1.0.0`. | +| `uninstall` | yes | Exact, copy-pasteable removal command, e.g. `gsd capability remove `. | +| `interactions` | yes | Object — see below. | +| `discussion` | yes | URL of this entry's GitHub Discussion (`https://github.com///discussions/`). | + +`interactions` (Reviewer): + +| Field | Required | Meaning | +|---|---|---| +| `slug` | yes | Lane identity, matching the manifest's `reviewer.slug`. Must match the runtime lane grammar `^[a-z0-9][a-z0-9_-]*$` — underscores and a leading digit are permitted (`lm_studio`, `4o-mini`), unlike the kebab-only `id` field. | +| `flags` | yes, non-empty | The CLI flags that select the lane, e.g. `["--gemini"]`. Each must match `^--[a-z0-9][a-z0-9-]*$` — flags are kebab even when the slug is snake (`lm_studio` → `--lm-studio`). | +| `transport` | yes | `spawn` or `openai-http`. | +| `evidenceClass` | yes | `source-grounded` or `diff-only`. | +| `reviewsSection` | yes | The `REVIEWS.md` heading the lane renders under (max 200 characters). | +| `requiresBinaries` | yes | External binaries the lane needs (may be empty). | +| `configKeys` | yes | Federated config keys it owns (may be empty). | +| `runtimeCompat` | yes | Array of compatible runtimes; `["all"]` is allowed. | + +Example: + +```json +{ + "id": "acme-review-lane", + "name": "Acme Review Lane", + "type": "reviewer", + "repo": "some-org/gsd-lane-acme", + "description": "Adds an Acme-hosted model as an external reviewer lane for /gsd-review, evaluating diffs against Acme's static-analysis findings.", + "author": "Some Org ", + "license": "MIT", + "enginesGsd": ">=1.8.0", + "install": "gsd capability install https://github.com/some-org/gsd-lane-acme.git#v1.0.0", + "uninstall": "gsd capability remove acme-review-lane", + "interactions": { + "slug": "acme", + "flags": ["--acme"], + "transport": "openai-http", + "evidenceClass": "diff-only", + "reviewsSection": "## Acme Review", + "requiresBinaries": [], + "configKeys": ["acme.api_key"], + "runtimeCompat": ["all"] + }, + "discussion": "https://github.com/open-gsd/gsd-core/discussions/1236" +} +``` + +A `role: "runtime"` capability that also carries a `reviewer` body (a host that is also a reviewer keeps one manifest, ADR-2782 D1) lists under whichever catalog matches its primary install shape — the Reviewer Lane Registry is for lanes that are not install targets in their own right. + --- ## Submission process @@ -173,14 +234,14 @@ Example: Registration is a **documentation PR**, per [CONTRIBUTING.md → Documentation Updates](../../CONTRIBUTING.md#documentation-updates--update-the-relevant-docs): 1. **Fork** the repository. -2. **Edit** `docs/registries/capabilities.json` (Capability Registry) or `docs/registries/eos.json` (EoS Registry) and append exactly one entry matching the [schema](#entry-schema) above. -3. **Run `npm run gen:registry`** to regenerate the corresponding `docs/registries/capability-registry.md` or `docs/registries/eos-registry.md`. Commit both the JSON source and the regenerated markdown. +2. **Edit** `docs/registries/capabilities.json` (Capability Registry), `docs/registries/eos.json` (EoS Registry), or `docs/registries/reviewers.json` (Reviewer Lane Registry) and append exactly one entry matching the [schema](#entry-schema) above. +3. **Run `npm run gen:registry`** to regenerate the corresponding `docs/registries/capability-registry.md`, `docs/registries/eos-registry.md`, or `docs/registries/reviewer-registry.md`. Commit both the JSON source and the regenerated markdown. 4. **Open a PR** from a `docs/-` branch (see CONTRIBUTING.md branch-naming conventions) using the [registry-entry PR template](../../.github/PULL_REQUEST_TEMPLATE/registry-entry.md). 5. A maintainer reviews and merges. The only gate is whether the entry is a real, linkable solution with all required fields present — not a quality judgment (see [Non-endorsement stance](#non-endorsement-stance)). **One entry = one PR.** Do not bundle multiple registry additions, updates, or removals into a single PR. -**The generated `.md` files are GENERATED — never hand-edit them.** `docs/registries/capability-registry.md` and `docs/registries/eos-registry.md` are produced by `scripts/gen-registry.cjs` from `capabilities.json` / `eos.json`. A PR that edits the generated markdown without a matching JSON source change will fail the `gen:registry --check` drift gate. Always edit the JSON and regenerate. +**The generated `.md` files are GENERATED — never hand-edit them.** `docs/registries/capability-registry.md`, `docs/registries/eos-registry.md`, and `docs/registries/reviewer-registry.md` are produced by `scripts/gen-registry.cjs` from `capabilities.json` / `eos.json` / `reviewers.json`. A PR that edits the generated markdown without a matching JSON source change will fail the `gen:registry --check` drift gate. Always edit the JSON and regenerate. --- @@ -202,11 +263,11 @@ There is no re-registration on new releases: register once, and your GitHub Rele ## Ranking + comments -Ranking and community feedback live in **GitHub Discussions**, not in the registry markdown. Each merged entry — from either registry — gets exactly one Discussion in the dedicated `EoS Registry` Discussions category: +Ranking and community feedback live in **GitHub Discussions**, not in the registry markdown. Each merged entry — from any of the three registries — gets exactly one Discussion in the dedicated `EoS Registry` Discussions category: - **Upvotes** on the Discussion post and on individual comments, with GitHub's built-in **Top** sort surfacing the most-upvoted community feedback first. - **Threaded comments** for experience reports, questions, and follow-up from other users. -**Operational setup (one-time, per repo):** a repo admin creates the `EoS Registry` category under this repository's Discussions settings, using the **open-ended discussion** format. From then on, every merged entry gets its own Discussion thread created in that category, and the thread's URL is recorded in the entry's `discussion` field (see [Entry schema](#entry-schema) above) so the generated catalog links directly to it. Despite its name, the category carries threads for **both** registries — `discussion` is required on Capability entries exactly as it is on EoS entries. +**Operational setup (one-time, per repo):** a repo admin creates the `EoS Registry` category under this repository's Discussions settings, using the **open-ended discussion** format. From then on, every merged entry gets its own Discussion thread created in that category, and the thread's URL is recorded in the entry's `discussion` field (see [Entry schema](#entry-schema) above) so the generated catalog links directly to it. Despite its name, the category carries threads for **all three** catalogs — `discussion` is required on Capability and Reviewer entries exactly as it is on EoS entries. **The open-ended format is required, and the choice is not cosmetic.** Because `discussion` is a required field, the thread must exist *before* the entry's PR is opened — and the person opening it is the entry's author, an outside contributor holding neither `maintain` nor `admin` permission on this repository. GitHub's **Announcement** format restricts starting new discussions to those two permission levels, so choosing it blocks every external submission at the first step, while still looking correctly configured to the admin who set it up. **Question/Answer** adds answer-marking, which pins one reply above the rest of a thread — a directory entry has no answer, and the pinning cuts across the upvote **Top** ordering described above. Open-ended is the format this process requires. diff --git a/docs/registries/reviewer-registry.md b/docs/registries/reviewer-registry.md new file mode 100644 index 000000000..b4ded1ce3 --- /dev/null +++ b/docs/registries/reviewer-registry.md @@ -0,0 +1,9 @@ + + +# GSD Reviewer Lane Registry + +> **Not an endorsement.** Inclusion means only that a maintainer merged a PR linking the author's repository — GSD has not reviewed, tested, or verified any listing. See the [registry README](./README.md). + +_To add your reviewer lane, see the [registry README](./README.md)._ + +_No entries yet — be the first: see [README](./README.md)._ diff --git a/docs/registries/reviewers.json b/docs/registries/reviewers.json new file mode 100644 index 000000000..fe51488c7 --- /dev/null +++ b/docs/registries/reviewers.json @@ -0,0 +1 @@ +[] diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index cd6b66fb1..e17a54f75 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -3033,6 +3033,29 @@ function runWithTimeout(argv) { const detached = !isWin && secs > 0; const spawnFailureCode = (err) => (err && err.code === 'ENOENT' ? 127 : err && err.code === 'EACCES' ? 126 : 125); + // #2667: on Windows, a `.cmd`/`.bat`/`.exe` command cannot be spawned directly + // — Node's CVE-2024-27980 hardening (April 2024, all active lines incl. 22.x) + // throws EINVAL when child_process.spawn is given a `.cmd`/`.bat` without a + // shell, so e.g. `run-with-timeout 120 -- node_modules/.bin/fallow.cmd` silently + // produced empty stdout + exit 125 and the fallow pre-pass no-op'd. + // + // We do NOT use `shell: true` for this: with `shell:true`, Node space-joins the + // unescaped cmdArgs into a cmd.exe command string (DEP0190) — that would re-open + // a shell-injection surface and violate the recorded no-shell-for-argv-array + // contract (DEFECT.UNBOUNDED-SUBPROCESS, CONTEXT.md:772). Instead we spawn + // `cmd.exe /c ` with an explicit argv ARRAY, which is what Node's + // own exec does internally and keeps every arg a discrete, un-interpolated + // token. The gate is NARROW: it fires ONLY for the Windows shim extensions, + // never for the `bash -c` callers (command is `bash`, no such suffix), so the 7 + // bash callers keep their array-only argv on every platform. POSIX untouched. + // NOTE: .exe is INTENTIONALLY excluded — real PE executables (node.exe, etc.) + // spawn fine directly and mediating them through cmd.exe /c breaks the timeout + // cap's process-group kill (the wrapped child escapes reap → exit 124 never + // fires) and risks cmd.exe mis-parsing an arg like `-e "setTimeout(()=>{})"`. + // Only .cmd/.bat are the CVE-2024-27980 EINVAL cases that require mediation. + const winShim = isWin && /\.(cmd|bat)$/i.test(path.basename(cmd)); + const spawnCmd = winShim ? (process.env.ComSpec || 'cmd.exe') : cmd; + const spawnArgs = winShim ? ['/d', '/s', '/c', cmd, ...cmdArgs] : cmdArgs; // Node's setTimeout delay is a 32-bit signed ms int; a larger value silently // clamps to 1ms → a spurious immediate timeout. Cap the budget (~24.8 days). const timerMs = Math.min(Math.round(secs * 1000), 2 ** 31 - 1); @@ -3043,7 +3066,11 @@ function runWithTimeout(argv) { return new Promise((resolve) => { let child; try { - child = spawn(cmd, cmdArgs, { stdio: 'inherit', detached }); + // #2667: on win32 `.cmd`/`.bat`/`.exe`, spawn cmd.exe with an explicit argv + // array (spawnCmd/spawnArgs) rather than the shim directly — preserves the + // array-only, no-shell-string argv contract. `detached` is always false on + // win32, so it never co-occurs with the cmd.exe mediation. + child = spawn(spawnCmd, spawnArgs, { stdio: 'inherit', detached }); } catch (err) { process.stderr.write(`run-with-timeout: ${cmd}: ${err && err.message ? err.message : 'failed to start'}\n`); resolve(spawnFailureCode(err)); @@ -3309,7 +3336,11 @@ async function main() { // move the other at the same time (keep them consistent). const SKIP_ROOT_RESOLUTION = new Set([ 'generate-slug', 'current-timestamp', 'verify-path-exists', - 'verify-summary', 'template', 'frontmatter', 'detect-custom-files', + // #2844: verify-summary was previously skipped, leaving relative file-claim + // paths resolved against the raw process.cwd() — invoking from a subdirectory + // manufactured "missing files" on an otherwise-correct SUMMARY. It now goes + // through findProjectRoot so claims resolve against the project root. + 'template', 'frontmatter', 'detect-custom-files', // #1854: restore-custom-files operates on a runtime config dir passed // explicitly via --config-dir; it never reads .planning/. 'restore-custom-files', diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index e3061e1d9..9528b5f1d 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -10,7 +10,7 @@ const capabilities = { "ai-integration": { "id": "ai-integration", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "AI design contract", "description": "AI-SPEC design contract workflow for phases that build AI systems; owns the AI integration command, agents, and workflow.ai_integration_phase activation key.", "tier": "full", @@ -95,7 +95,7 @@ const capabilities = { "antigravity": { "id": "antigravity", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Antigravity", "description": "Google Antigravity IDE — nested under ~/.gemini/antigravity; probed across 1.x and 2.x layouts; Gemini hook event dialect; flat skill layout; tier-1 support.", "tier": "core", @@ -239,7 +239,7 @@ const capabilities = { "assumption-delta": { "id": "assumption-delta", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Assumption-delta architecture checkpoint", "description": "Rarely-firing advisory checkpoint that triggers when a phase makes something plural, optional, or chosen that used to be singular, required, or derived. Surfaces one identity-model question (promote the new general representation to primary, or add it alongside?) so a silent primary-key drift does not accumulate into a later user-facing bug. Non-blocking; fires only on a detected signal.", "tier": "full", @@ -285,7 +285,7 @@ const capabilities = { "audit": { "id": "audit", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Audit", "description": "Open-artifact audit and UAT-gap audit for milestone close gates; exposes `gsd-tools audit-uat` (cross-phase UAT outstanding items) and `gsd-tools audit-open` (structured open-artifact scan across debug, tasks, threads, todos, seeds, UAT, verification, context-questions).", "tier": "full", @@ -322,7 +322,7 @@ const capabilities = { "augment": { "id": "augment", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Augment Code", "description": "Augment Code CLI — commands + nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -431,7 +431,7 @@ const capabilities = { "broken-windows": { "id": "broken-windows", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Broken-windows ledger", "description": "Cross-phase defect register accumulating stubs, TODOs, skipped tests, unrun verifies, and unmet truths into .planning/WINDOWS.md. Blocks /gsd-ship while any window is open unless explicitly waived with a recorded reason. Operationalizes GSD's no-defer discipline as a tracked, enforced artifact (issue #1950).", "tier": "full", @@ -477,7 +477,7 @@ const capabilities = { "claude": { "id": "claude", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Claude Code", "description": "Anthropic Claude Code — primary development runtime; tier-1 support with full hook surface and skills-based global install.", "tier": "core", @@ -624,7 +624,7 @@ const capabilities = { "claude-orchestration": { "id": "claude-orchestration", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Claude orchestration (Workflow backend)", "description": "Default-off, BETA, claude-only capability that adopts Claude Code's Workflow tool (the engine behind /effort ultracode) as an optional parallel-execution backend for the GSD loop. When the runtime exposes the Workflow tool and claude_orchestration.execution_backend resolves to 'workflow', execute-phase emits a generated Workflow script (waves -> parallel() barriers, plans -> agent({ agentType: 'gsd-executor', isolation: 'worktree' }), files_modified overlap -> separate sequential stages, resumeFromRunId wired to the phase run id, shared token budget) that composes the SAME gsd-executor agent and worktree isolation the inline path uses, restoring the wave parallelism the #853 backgrounded-agent nesting limitation forces inline on Claude Code. (The plan-checker and verifier remain inline until separately wired — this capability delivers the parallel-execution backend, not those gates.) Also folds the ultraplan plan-offload under one runtime gate (plan:* surface). On any runtime lacking the Workflow tool, or when the capability is disabled, behaviour is byte-identical to today (inline/manual dispatch). Detection + emission live in gsd-core/bin/lib/claude-orchestration.cjs (pure, fail-closed). Mirrors the existing gsd-ultraplan-phase BETA-isolation posture.", "tier": "full", @@ -712,7 +712,7 @@ const capabilities = { "cline": { "id": "cline", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cline", "description": "Cline (VS Code extension) — global-only nested-skill layout; cline-rules hook surface (.clinerules); no hook events emitted; tier-2 support.", "tier": "core", @@ -783,7 +783,7 @@ const capabilities = { "code-review": { "id": "code-review", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Code review", "description": "Source-file code review and review-fix workflow support for completed execution work.", "tier": "full", @@ -844,7 +844,7 @@ const capabilities = { "codebuddy": { "id": "codebuddy", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "CodeBuddy", "description": "CodeBuddy (Tencent) — converted commands + skills artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -957,7 +957,7 @@ const capabilities = { "coderabbit": { "id": "coderabbit", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "CodeRabbit", "description": "CodeRabbit CLI — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). Reviews the working-tree diff (`coderabbit review --prompt-only`), not the source tree, and accepts neither a prompt nor a model flag; findings are down-weighted in consensus (evidenceClass: diff-only).", "tier": "full", @@ -999,7 +999,7 @@ const capabilities = { "codex": { "id": "codex", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenAI Codex CLI", "description": "OpenAI Codex CLI — shell-var command style; per-agent sandbox tiers; config.toml + hooks.json hook surface; tier-1 support.", "tier": "core", @@ -1137,7 +1137,7 @@ const capabilities = { "copilot": { "id": "copilot", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "GitHub Copilot", "description": "GitHub Copilot (VS Code) — markdown config format; copilot-inline hook surface; no hook events emitted; flat skill nesting (unconfirmed recursive loader); tier-2 support.", "tier": "core", @@ -1232,7 +1232,7 @@ const capabilities = { "cursor": { "id": "cursor", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cursor", "description": "Cursor IDE — skills + converted commands artifact layout; hooks.json surface; Claude hook event dialect; recursive skill loader (flat nesting); tier-2 support.", "tier": "core", @@ -1391,7 +1391,7 @@ const capabilities = { "drift": { "id": "drift", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Drift detection gates", "description": "Drift detection gates for the planning loop. At execute:wave:post: a blocking schema drift gate (detects schema files changed without a database push) and a non-blocking codebase drift gate (detects structural additions not reflected in STRUCTURE.md). At plan:pre: a non-blocking, warn-only codebase drift gate (gated on workflow.plan_drift_precheck) that flags a stale codebase map before planning, so plans are authored against a fresh STRUCTURE.md instead of discovering drift mid-execution.", "tier": "full", @@ -1469,7 +1469,7 @@ const capabilities = { "external-job": { "id": "external-job", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Async external-job scheduler adapter", "description": "Default-off producer of the async external-job manifest (#1164). At execute:wave:post an executor can externalize long-running compute (SLURM first, scheduler-pluggable), commit a .planning/async-jobs/.json manifest, defer SUMMARY.md, and return external_job_waiting. The core loop (#1165) consumes the manifest; this capability is the only thing that writes it. NOTE on contribution point: #1164 specifies execute:wave:pre, but execute-phase.md only dispatches execute:wave:post today (wave:pre is declared in the loop host contract but not rendered); wiring wave:pre dispatch is a core-loop change #1164 explicitly puts out of scope, so this capability registers at wave:post and the executor honors the runtime_budget classification guidance before running any tagged task. The adapter (scripts/slurm-adapter.cjs) reads external_job.submit_timeout_ms / poll_timeout_ms / artifact_dir through the canonical capability-config seam (env override > config > registry default).", "tier": "full", @@ -1552,7 +1552,7 @@ const capabilities = { "gap-analysis": { "id": "gap-analysis", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Post-planning gap analysis", "description": "Proactive, non-blocking post-planning coverage report. After all PLAN.md files are generated, cross-references every REQ-ID and D-ID from REQUIREMENTS.md and CONTEXT.md against plan bodies. Emits a Source | Item | Status table. Does not block phase advancement.", "tier": "standard", @@ -1593,7 +1593,7 @@ const capabilities = { "gemini": { "id": "gemini", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "Gemini CLI", "description": "Google Gemini CLI — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). Spawned as `gemini -p - -m ` with the plan piped on stdin.", "tier": "full", @@ -1643,7 +1643,7 @@ const capabilities = { "graphify": { "id": "graphify", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Knowledge graph", "description": "Build, query, and inspect the project knowledge graph in `.planning/graphs/`; exposes graphify CLI subcommands (build, query, status, diff) and the /gsd-graphify skill.", "tier": "full", @@ -1684,7 +1684,7 @@ const capabilities = { "hermes": { "id": "hermes", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Hermes Agent", "description": "Hermes Agent (NousResearch) — skills nest under skills/gsd/ category bucket; nested skill layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -1775,7 +1775,7 @@ const capabilities = { "intel": { "id": "intel", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Codebase intelligence", "description": "Code-intelligence store for codebase querying, diff, snapshot, and API-surface extraction; exposes `gsd-tools intel` subcommands (query, status, update, diff, snapshot, patch-meta, validate, extract-exports, api-surface) and backs `/gsd-map-codebase` and `gsd-intel-updater`.", "tier": "full", @@ -1827,7 +1827,7 @@ const capabilities = { "kilo": { "id": "kilo", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kilo Code", "description": "Kilo Code — XDG-based config dir; global skills at ~/.kilo/skills (separate from XDG config); flat command/ + skills artifact layout; no lifecycle hook registration; tier-2 support.", "tier": "core", @@ -1936,7 +1936,7 @@ const capabilities = { "kimi": { "id": "kimi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi CLI", "description": "Kimi CLI (Moonshot AI) — generic agents root at ~/.config/agents; skills + kimi-agents artifact layout; native config.toml [[hooks]] bus at ~/.kimi/config.toml; background dispatch; tier-2 support.", "tier": "core", @@ -2034,7 +2034,7 @@ const capabilities = { "kimi-code": { "id": "kimi-code", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi Code CLI", "description": "Kimi Code CLI (Moonshot AI, Node) — Agent Skills auto-discovered at ~/.kimi-code/skills; global AGENTS.md at ~/.kimi-code/AGENTS.md; native config.toml + [[hooks]] bus; three built-in subagents (coder/explore/plan), NO custom named subagents; background dispatch; tier-2 support. Distinct from Python kimi-cli (the 'kimi' capability) per ADR-1239 EoS — Kimi Code cannot dispatch named subagents so the kimi-agents YAML layout does NOT apply; persona injection rides the existing ${AGENT_SKILLS_*} workflow fallback. Install-layout, agent-install-check, and install-time decision (kimi vs kimi-code) land in follow-up PRs; this descriptor is the EoS foundation.", "tier": "core", @@ -2166,7 +2166,7 @@ const capabilities = { "llama-cpp": { "id": "llama-cpp", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "llama.cpp", "description": "llama.cpp server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.llama_cpp_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq. Capability id/folder are kebab (`llama-cpp`, required by KEBAB_RE); `reviewer.slug` stays snake (`llama_cpp`) to match the shipped roster and the `review.llama_cpp_host` config key (ADR-2782's three-namespace trap).", "tier": "full", @@ -2224,7 +2224,7 @@ const capabilities = { "lm-studio": { "id": "lm-studio", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "LM Studio", "description": "LM Studio local model server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.lm_studio_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq. Capability id/folder are kebab (`lm-studio`, required by KEBAB_RE); `reviewer.slug` stays snake (`lm_studio`) to match the shipped roster and the `review.lm_studio_host` config key (ADR-2782's three-namespace trap).", "tier": "full", @@ -2282,7 +2282,7 @@ const capabilities = { "mempalace": { "id": "mempalace", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "MemPalace memory", "description": "Cross-session, cross-project memory: deliberate recall before discuss/plan and verbatim capture + temporal-KG sync at phase boundaries, via the MemPalace MCP server and CLI.", "tier": "full", @@ -2456,7 +2456,7 @@ const capabilities = { "nyquist": { "id": "nyquist", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Nyquist validation", "description": "Validation coverage audit that maps executed work back to tests and manual-only evidence.", "tier": "full", @@ -2506,7 +2506,7 @@ const capabilities = { "ollama": { "id": "ollama", "role": "reviewer", - "version": "1.9.0", + "version": "1.9.1", "title": "Ollama", "description": "Ollama local model server — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts). OpenAI-compatible HTTP transport against a user-configured `review.ollama_host` (POST /v1/chat/completions); model discovered via GET /v1/models piped through jq.", "tier": "full", @@ -2564,7 +2564,7 @@ const capabilities = { "opencode": { "id": "opencode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenCode", "description": "OpenCode — XDG-based config dir; flat commands/ + skills artifact layout; settings-json config format; no lifecycle hook registration; tier-2 support.", "tier": "core", @@ -2721,7 +2721,7 @@ const capabilities = { "pattern-mapper": { "id": "pattern-mapper", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Pattern mapping", "description": "Optional codebase-pattern mapping before planning; owns the pattern mapper agent and workflow.pattern_mapper activation key.", "tier": "full", @@ -2775,7 +2775,7 @@ const capabilities = { "pi": { "id": "pi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "pi", "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", @@ -2837,7 +2837,7 @@ const capabilities = { "profile-pipeline": { "id": "profile-pipeline", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Developer profiling pipeline", "description": "Developer behavioral profiling from Claude Code session history; scans session JSONL files, extracts and samples user messages, and generates profile artifacts (USER-PROFILE.md, dev-preferences.md, CLAUDE.md sections). Exposes eight `gsd-tools` commands: scan-sessions, extract-messages, profile-sample (pipeline phase) and write-profile, profile-questionnaire, generate-dev-preferences, generate-claude-profile, generate-claude-md (output phase). Backs the /gsd-profile-user skill and gsd-user-profiler agent.", "tier": "full", @@ -2914,7 +2914,7 @@ const capabilities = { "qwen": { "id": "qwen", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Qwen Code", "description": "Qwen Code (Alibaba) — nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -3050,7 +3050,7 @@ const capabilities = { "research": { "id": "research", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Phase research", "description": "Optional phase research before planning; owns the phase researcher agent and workflow.research activation key.", "tier": "standard", @@ -3102,7 +3102,7 @@ const capabilities = { "schema-gate": { "id": "schema-gate", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Schema push detection gate", "description": "Detects ORM schema-relevant files in the phase scope during planning and injects a mandatory [BLOCKING] schema push task into the plan. Prevents false-positive verification where build/types pass because TypeScript types come from config, not the live database.", "tier": "full", @@ -3148,7 +3148,7 @@ const capabilities = { "security": { "id": "security", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Security enforcement", "description": "Threat mitigation verification and ship-time security blocking for phases with security enforcement enabled.", "tier": "full", @@ -3247,7 +3247,7 @@ const capabilities = { "tdd": { "id": "tdd", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "Test-driven development", "description": "Injects TDD heuristics into the planner and enforces RED/GREEN gate compliance on type:tdd plans after execution. Owns workflow.tdd_mode; the --tdd CLI flag is the ephemeral override.", "tier": "full", @@ -3300,7 +3300,7 @@ const capabilities = { "trae": { "id": "trae", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Trae IDE", "description": "Trae IDE — nested-skill artifact layout; no hook surface (profile-marker-only config); tier-2 support.", "tier": "core", @@ -3392,7 +3392,7 @@ const capabilities = { "ui": { "id": "ui", "role": "feature", - "version": "1.9.0", + "version": "1.9.1", "title": "UI design contracts", "description": "UI-SPEC design contract + retrospective UI audit for frontend phases.", "tier": "full", @@ -3487,7 +3487,7 @@ const capabilities = { "vscode": { "id": "vscode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "VS Code", "description": "VS Code — Marketplace/VSIX extension; no file-projected config directory; IDE-profile reference host (active vscode.lm model, engine-owned hook bus, sandboxed globalState/workspaceState stateIO).", "tier": "core", @@ -3540,7 +3540,7 @@ const capabilities = { "windsurf": { "id": "windsurf", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Windsurf", "description": "Windsurf (Codeium) — workspace workflow artifact layout for slash commands; Cascade native hooks.json blocking hook bus (pre_write_code, pre_run_command); tier-2 support.", "tier": "core", @@ -3627,7 +3627,7 @@ const capabilities = { "zcode": { "id": "zcode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "ZCode", "description": "ZCode (Z.ai) — desktop Agentic Development Environment for GLM-5.2; Claude-shaped nested skills at ~/.zcode/skills//SKILL.md, slash commands, named subagents, native MCP; declarative plugin surface; profile-marker install; tier-2 community support.", "tier": "core", @@ -4768,7 +4768,7 @@ const runtimes = { "antigravity": { "id": "antigravity", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Antigravity", "description": "Google Antigravity IDE — nested under ~/.gemini/antigravity; probed across 1.x and 2.x layouts; Gemini hook event dialect; flat skill layout; tier-1 support.", "tier": "core", @@ -4912,7 +4912,7 @@ const runtimes = { "augment": { "id": "augment", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Augment Code", "description": "Augment Code CLI — commands + nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -5021,7 +5021,7 @@ const runtimes = { "claude": { "id": "claude", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Claude Code", "description": "Anthropic Claude Code — primary development runtime; tier-1 support with full hook surface and skills-based global install.", "tier": "core", @@ -5168,7 +5168,7 @@ const runtimes = { "cline": { "id": "cline", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cline", "description": "Cline (VS Code extension) — global-only nested-skill layout; cline-rules hook surface (.clinerules); no hook events emitted; tier-2 support.", "tier": "core", @@ -5239,7 +5239,7 @@ const runtimes = { "codebuddy": { "id": "codebuddy", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "CodeBuddy", "description": "CodeBuddy (Tencent) — converted commands + skills artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -5352,7 +5352,7 @@ const runtimes = { "codex": { "id": "codex", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenAI Codex CLI", "description": "OpenAI Codex CLI — shell-var command style; per-agent sandbox tiers; config.toml + hooks.json hook surface; tier-1 support.", "tier": "core", @@ -5490,7 +5490,7 @@ const runtimes = { "copilot": { "id": "copilot", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "GitHub Copilot", "description": "GitHub Copilot (VS Code) — markdown config format; copilot-inline hook surface; no hook events emitted; flat skill nesting (unconfirmed recursive loader); tier-2 support.", "tier": "core", @@ -5585,7 +5585,7 @@ const runtimes = { "cursor": { "id": "cursor", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Cursor", "description": "Cursor IDE — skills + converted commands artifact layout; hooks.json surface; Claude hook event dialect; recursive skill loader (flat nesting); tier-2 support.", "tier": "core", @@ -5744,7 +5744,7 @@ const runtimes = { "hermes": { "id": "hermes", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Hermes Agent", "description": "Hermes Agent (NousResearch) — skills nest under skills/gsd/ category bucket; nested skill layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -5835,7 +5835,7 @@ const runtimes = { "kilo": { "id": "kilo", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kilo Code", "description": "Kilo Code — XDG-based config dir; global skills at ~/.kilo/skills (separate from XDG config); flat command/ + skills artifact layout; no lifecycle hook registration; tier-2 support.", "tier": "core", @@ -5944,7 +5944,7 @@ const runtimes = { "kimi": { "id": "kimi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi CLI", "description": "Kimi CLI (Moonshot AI) — generic agents root at ~/.config/agents; skills + kimi-agents artifact layout; native config.toml [[hooks]] bus at ~/.kimi/config.toml; background dispatch; tier-2 support.", "tier": "core", @@ -6042,7 +6042,7 @@ const runtimes = { "kimi-code": { "id": "kimi-code", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Kimi Code CLI", "description": "Kimi Code CLI (Moonshot AI, Node) — Agent Skills auto-discovered at ~/.kimi-code/skills; global AGENTS.md at ~/.kimi-code/AGENTS.md; native config.toml + [[hooks]] bus; three built-in subagents (coder/explore/plan), NO custom named subagents; background dispatch; tier-2 support. Distinct from Python kimi-cli (the 'kimi' capability) per ADR-1239 EoS — Kimi Code cannot dispatch named subagents so the kimi-agents YAML layout does NOT apply; persona injection rides the existing ${AGENT_SKILLS_*} workflow fallback. Install-layout, agent-install-check, and install-time decision (kimi vs kimi-code) land in follow-up PRs; this descriptor is the EoS foundation.", "tier": "core", @@ -6174,7 +6174,7 @@ const runtimes = { "opencode": { "id": "opencode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "OpenCode", "description": "OpenCode — XDG-based config dir; flat commands/ + skills artifact layout; settings-json config format; no lifecycle hook registration; tier-2 support.", "tier": "core", @@ -6331,7 +6331,7 @@ const runtimes = { "pi": { "id": "pi", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "pi", "description": "pi (pi.dev) — bun-runtime programmatic-CLI; TS ExtensionAPI (registerCommand/registerTool/registerProvider/pi.on); single native-extension file at ~/.pi/agent/extensions/gsd.js (.js, not .cjs — pi's extension auto-discovery accepts only .ts/.js, #2470); no shared-settings hook surface; tier-2 support.", "tier": "core", @@ -6393,7 +6393,7 @@ const runtimes = { "qwen": { "id": "qwen", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Qwen Code", "description": "Qwen Code (Alibaba) — nested-skill artifact layout; settings-json hook surface; Claude hook event dialect; tier-2 support.", "tier": "core", @@ -6529,7 +6529,7 @@ const runtimes = { "trae": { "id": "trae", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Trae IDE", "description": "Trae IDE — nested-skill artifact layout; no hook surface (profile-marker-only config); tier-2 support.", "tier": "core", @@ -6621,7 +6621,7 @@ const runtimes = { "vscode": { "id": "vscode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "VS Code", "description": "VS Code — Marketplace/VSIX extension; no file-projected config directory; IDE-profile reference host (active vscode.lm model, engine-owned hook bus, sandboxed globalState/workspaceState stateIO).", "tier": "core", @@ -6674,7 +6674,7 @@ const runtimes = { "windsurf": { "id": "windsurf", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "Windsurf", "description": "Windsurf (Codeium) — workspace workflow artifact layout for slash commands; Cascade native hooks.json blocking hook bus (pre_write_code, pre_run_command); tier-2 support.", "tier": "core", @@ -6761,7 +6761,7 @@ const runtimes = { "zcode": { "id": "zcode", "role": "runtime", - "version": "1.9.0", + "version": "1.9.1", "title": "ZCode", "description": "ZCode (Z.ai) — desktop Agentic Development Environment for GLM-5.2; Claude-shaped nested skills at ~/.zcode/skills//SKILL.md, slash commands, named subagents, native MCP; declarative plugin surface; profile-marker install; tier-2 community support.", "tier": "core", diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md index 27f72edbb..71515ec07 100644 --- a/gsd-core/workflows/code-review.md +++ b/gsd-core/workflows/code-review.md @@ -464,7 +464,22 @@ FALLOW_OK=$(FALLOW_TMP=\"${FALLOW_JSON_PATH}.tmp\" node -e \" if [ \"$FALLOW_OK\" != \"1\" ]; then FALLOW_STDERR_SUMMARY=$(head -5 \"$FALLOW_STDERR_TMP\") rm -f \"${FALLOW_JSON_PATH}.tmp\" \"$FALLOW_STDERR_TMP\" - echo \"WARNING: fallow structural pre-pass failed (exit ${FALLOW_EXIT}): ${FALLOW_STDERR_SUMMARY}\" + # #2667: distinguish a hard EXECUTION failure (the binary was found at step 1 + # but would not run) from the binary-missing path (step 2). Exit 124 = timeout, + # 2 = usage error, 125 = spawn failure (e.g. Windows EINVAL on a .cmd shim — + # CVE-2024-27980, now mediated by run-with-timeout), 126/127 = not executable / + # not found. A non-zero exit here with a resolved binary means fallow is + # installed but did not produce a report — surface that loudly so a Windows + # user does not mistake it for "fallow absent". + case \"$FALLOW_EXIT\" in + 124) FALLOW_FAIL_KIND=\"timed out\" ;; + 2) FALLOW_FAIL_KIND=\"usage error\" ;; + 125) FALLOW_FAIL_KIND=\"spawn failure (the binary was found but did not start — e.g. a Windows .cmd shim; run-with-timeout mediates this)\" ;; + 126) FALLOW_FAIL_KIND=\"not executable\" ;; + 127) FALLOW_FAIL_KIND=\"not found\" ;; + *) FALLOW_FAIL_KIND=\"crashed\" ;; + esac + echo \"WARNING: fallow structural pre-pass failed (${FALLOW_FAIL_KIND}, exit ${FALLOW_EXIT}): ${FALLOW_STDERR_SUMMARY}\" FALLOW_JSON_PATH=\"\" else mv \"${FALLOW_JSON_PATH}.tmp\" \"$FALLOW_JSON_PATH\" @@ -472,7 +487,7 @@ else fi ``` -On any failure of the structural pre-pass (binary missing, timeout, empty output, or unparseable JSON), the workflow continues with no `` injection; the reviewer agent receives a normal review request. +On any failure of the structural pre-pass (binary missing at step 2, or an execution failure here — timeout, spawn failure, crash, empty output, or unparseable JSON), the workflow continues with no `` injection; the reviewer agent receives a normal review request. The WARNING above names the failure KIND so a hard execution failure (e.g. a Windows `.cmd` spawn failure) is not mistaken for an absent optional dependency. 4) Optional MCP bridge path (runtime-dependent): - If `FALLOW_MCP=true`, set reviewer input mode to MCP-backed structural findings. diff --git a/package-lock.json b/package-lock.json index cf8e00ac1..ebfc087ae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@opengsd/gsd-core", - "version": "1.9.0", + "version": "1.9.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@opengsd/gsd-core", - "version": "1.9.0", + "version": "1.9.1", "license": "MIT", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.84", diff --git a/package.json b/package.json index ff4fe9a87..27bee7a4a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@opengsd/gsd-core", - "version": "1.9.0", + "version": "1.9.1", "description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.", "main": ".opencode/plugins/gsd-core.js", "bin": { diff --git a/scripts/gen-registry.cjs b/scripts/gen-registry.cjs index 385c7f17b..93c10202e 100644 --- a/scripts/gen-registry.cjs +++ b/scripts/gen-registry.cjs @@ -2,10 +2,14 @@ 'use strict'; /** - * scripts/gen-registry.cjs — generates docs/registries/capability-registry.md - * (and, once PR2 ships docs/registries/eos.json, docs/registries/eos-registry.md) - * from the corresponding source JSON, via registry-schema.cjs#renderMarkdown. - * Issue #2182. + * scripts/gen-registry.cjs — generates docs/registries/capability-registry.md, + * docs/registries/eos-registry.md, and docs/registries/reviewer-registry.md + * from their corresponding source JSON, via registry-schema.cjs#renderMarkdown. + * Issue #2182 (capability/eos); issue #2904 (reviewer). + * + * eos.json and reviewers.json are both OPTIONAL sources (`SOURCES[].optional`) + * — an absent one is skipped silently. capabilities.json is the primary + * source and is never optional. * * NOT to be confused with `scripts/gen-capability-registry.cjs`: that script * generates the RUNTIME capability manifest consumed by the host at runtime @@ -34,7 +38,8 @@ const { renderMarkdown } = require('./registry-schema.cjs'); const SOURCES = [ { type: 'capability', jsonFile: 'capabilities.json', mdFile: 'capability-registry.md' }, - { type: 'eos', jsonFile: 'eos.json', mdFile: 'eos-registry.md' }, + { type: 'eos', jsonFile: 'eos.json', mdFile: 'eos-registry.md', optional: true }, + { type: 'reviewer', jsonFile: 'reviewers.json', mdFile: 'reviewer-registry.md', optional: true }, ]; /** @@ -57,14 +62,16 @@ function getRegistriesDir() { * Render the markdown for a single registry type from its committed source * JSON. * - * Only `eos.json` is optional (pre-PR2, before that source JSON ships) — - * an absent `eos.json` returns null and callers treat that as "nothing to - * do". `capabilities.json` is the primary registry source: a missing - * `capabilities.json` is ALWAYS an error (never a silent "up to date" - * pass), mirroring the type distinction in `scripts/validate-registry.cjs` - * (`type === 'eos' && !exists → continue`). + * Optionality is a per-source data flag (`SOURCES[].optional`), not a + * hardcoded type literal: `eos.json` (pre-PR2) and `reviewers.json` (issue + * #2904) are both optional — an absent source JSON returns null and callers + * treat that as "nothing to do". `capabilities.json` is still the primary + * registry source and is never optional: a missing `capabilities.json` is + * ALWAYS an error (never a silent "up to date" pass), mirroring the same + * `optional` flag in `scripts/validate-registry.cjs` + * (`optional && !exists → continue`). * - * @param {'capability'|'eos'} type + * @param {'capability'|'eos'|'reviewer'} type * @returns {string|null} */ function renderFor(type) { @@ -73,14 +80,31 @@ function renderFor(type) { const jsonPath = path.join(getRegistriesDir(), source.jsonFile); if (!fs.existsSync(jsonPath)) { - if (type === 'eos') return null; + if (source.optional) return null; throw new ExitError( 1, `${source.jsonFile} does not exist at ${jsonPath}. Run:\n node scripts/gen-registry.cjs --write\n(after adding docs/registries/${source.jsonFile})`, ); } - const entries = JSON.parse(fs.readFileSync(jsonPath, 'utf8')); + // A malformed source JSON must surface as an actionable CLI error, not an + // unhandled SyntaxError with a raw Node stack trace — mirrors + // scripts/validate-registry.cjs#validateFile's try/catch around JSON.parse. + let entries; + try { + entries = JSON.parse(fs.readFileSync(jsonPath, 'utf8')); + } catch (err) { + throw new ExitError(1, `${source.jsonFile} is not valid JSON at ${jsonPath}: ${err.message}`); + } + + // Mirrors validate-registry.cjs's explicit non-array rejection: a source + // JSON that parses to a non-array (object, string, etc.) would otherwise + // throw an opaque TypeError from `[...entries].sort()` in renderMarkdown, + // or silently mis-render for an iterable-but-wrong-shape value like a string. + if (!Array.isArray(entries)) { + throw new ExitError(1, `${source.jsonFile} must be a JSON array of entries`); + } + return renderMarkdown(entries, { type, sourceFile: source.jsonFile }); } @@ -91,7 +115,7 @@ function main() { for (const { type, mdFile } of SOURCES) { const rendered = renderFor(type); - if (rendered === null) continue; // source JSON absent (eos.json before PR2) + if (rendered === null) continue; // source JSON absent and optional (eos.json before PR2 / reviewers.json) const mdPath = path.join(registriesDir, mdFile); diff --git a/scripts/registry-schema.cjs b/scripts/registry-schema.cjs index 89448eba7..b25cb611d 100644 --- a/scripts/registry-schema.cjs +++ b/scripts/registry-schema.cjs @@ -2,11 +2,12 @@ /** * scripts/registry-schema.cjs — pure schema/vocab constants + validation + - * markdown-generation logic for the two third-party discoverability catalogs - * (issue #2182): + * markdown-generation logic for the three third-party discoverability catalogs + * (issue #2182, plus #2904): * * - `docs/registries/capabilities.json` → "GSD Community Capability Registry" * - `docs/registries/eos.json` → "GSD EoS Registry" (PR2) + * - `docs/registries/reviewers.json` → "GSD Reviewer Lane Registry" (issue #2904) * * The vocabulary constants below are ADDITIVE CONTRACTS that track the * runtime/ADR closed vocabularies they describe — they are a documentation- @@ -41,10 +42,14 @@ * (every entry published before the amendment stays valid) or declare * it as `argv` | `none`, mirroring `HOST_INTEGRATION_AXES.effortSurface` * in `src/host-integration.cts`. - * - `CAPABILITY_REQUIRED` / `EOS_REQUIRED` mirror the required top-level - * fields for each entry type, including `enginesGsd` (ADR-1244 D1 - * "Versioned capability manifest" — the `engines.gsd` semver-range gate, - * modelled on VS Code's `engines.vscode`). + * - `CAPABILITY_REQUIRED` / `EOS_REQUIRED` / `REVIEWER_REQUIRED` mirror the + * required top-level fields for each entry type, including `enginesGsd` + * (ADR-1244 D1 "Versioned capability manifest" — the `engines.gsd` + * semver-range gate, modelled on VS Code's `engines.vscode`). + * - `REVIEWER_LANE_TRANSPORTS` / `REVIEWER_EVIDENCE_CLASSES` / + * `REVIEWER_SLUG_RE` / `REVIEWER_FLAG_RE` / `REVIEWER_SECTION_MAX` mirror + * the ADR-2782 reviewer-lane vocabulary (`capability-validator.cjs`) for + * the `reviewer` entry type's `interactions` sub-object (issue #2904). * * This module is pure — no `fs`/`process`/child-process access — so tests * can `require()` it directly and assert on structured return values. @@ -107,37 +112,118 @@ const OPTIONAL_AXES = Object.freeze({ effortSurface: Object.freeze(['argv', 'none']), }); -// ─── Required top-level fields ─────────────────────────────────────────────── -const CAPABILITY_REQUIRED = Object.freeze([ - 'id', - 'name', - 'type', - 'repo', - 'description', - 'author', - 'license', - 'enginesGsd', - 'install', - 'uninstall', - 'interactions', - 'discussion', -]); +// ─── ADR-2782 reviewer-lane vocabulary (issue #2904) ───────────────────────── +// A THIRD catalog: third-party reviewer lanes (`role: "reviewer"`, ADR-2782 +// D3). A lane registers on ZERO Loop Extension Points and is forbidden from +// declaring `steps`/`contributions`/`gates`/`skills`/`agents`/`hooks` +// (`FEATURE_FIELDS_FORBIDDEN_ON_REVIEWER`, capability-validator.cjs), so the +// Capability entry's two required `interactions` fields are unsatisfiable by +// construction for a lane — hence its own entry type rather than a relaxation +// of the Capability schema. +// +// These constants are ADDITIVE CONTRACTS mirroring the canonical runtime +// vocabulary in `gsd-core/bin/lib/capability-validator.cjs`, exactly the way +// `AXES` mirrors `HOST_INTEGRATION_AXES`. They are hand-written mirrors, NOT +// imports: this module is documented pure (no `fs`/`process`), and requiring a +// `gsd-core/bin/lib` runtime module from a docs-pipeline script would invert +// that. Parity is enforced instead by `tests/registry-reviewer-parity.test.cjs`. +// +// `REVIEWER_SLUG_RE` deliberately does NOT reuse the registry's kebab-case `id` +// grammar. `LANE_SLUG_RE` permits underscores AND a leading digit — +// `lm_studio`, `llama_cpp`, `4o-mini` are real shipped lane slugs — and +// capability-validator.cjs:807-810 requires the two grammars stay +// byte-identical. A kebab-only rule here would reject well-formed entries and +// leave authors with a schema satisfiable only by lying. +const REVIEWER_LANE_TRANSPORTS = Object.freeze(['spawn', 'openai-http']); +const REVIEWER_EVIDENCE_CLASSES = Object.freeze(['source-grounded', 'diff-only']); +const REVIEWER_SLUG_RE = /^[a-z0-9][a-z0-9_-]*$/; +// Flags are kebab even when the slug is snake: `lm_studio` → `--lm-studio`. +const REVIEWER_FLAG_RE = /^--[a-z0-9][a-z0-9-]*$/; +// Cap for the one free-text reviewer interactions field, mirroring the 300-cap +// on the equivalently free-form `axes.dispatch`. A REVIEWS.md heading is short. +const REVIEWER_SECTION_MAX = 200; -const EOS_REQUIRED = Object.freeze([ - 'id', - 'name', - 'type', - 'repo', - 'description', - 'author', - 'license', - 'enginesGsd', - 'install', - 'uninstall', - 'interactions', - 'discussion', - 'protocolVersion', +// ─── Required top-level fields ─────────────────────────────────────────────── +// The twelve fields every entry type requires. Each type's set is DERIVED from +// this one so a future shared field cannot be added to one type's list and +// silently forgotten in another (DEFECT.GENERATIVE-FIX). The three sets are +// distinct frozen arrays, not aliases, so a type may still diverge deliberately +// — as `eos` already does with `protocolVersion`. +const BASE_REQUIRED = Object.freeze([ + 'id', 'name', 'type', 'repo', 'description', 'author', 'license', + 'enginesGsd', 'install', 'uninstall', 'interactions', 'discussion', ]); +const CAPABILITY_REQUIRED = Object.freeze([...BASE_REQUIRED]); +const EOS_REQUIRED = Object.freeze([...BASE_REQUIRED, 'protocolVersion']); +// A lane is installed with `gsd capability install`, owns a repo, a license and +// an `engines.gsd` range exactly as a Feature Capability does — so it requires +// the same twelve top-level fields. Only `interactions` differs. +const REVIEWER_REQUIRED = Object.freeze([...BASE_REQUIRED]); + +// Control-character rejection (defense in depth): `allowTabNewline` widens the +// reject-set exception for the two shell-snippet fields (install/uninstall), +// which legitimately contain tabs/newlines; every other free text field +// disallows ALL C0 control characters plus DEL (incl. \n/\t). Checked via char +// codes (not a literal control-char regex range) — same approach as +// capability-validator.cjs's hooks[].matcher check, which avoids tripping +// ESLint's no-control-regex rule. Module-scope so both the top-level field +// checks inside `validateEntries` and the `interactions` sub-object +// validators (module-level functions, outside that closure) share the ONE +// implementation rather than each keeping their own copy. +function hasDisallowedControlChar(v, allowTabNewline) { + for (let c = 0; c < v.length; c += 1) { + const code = v.charCodeAt(c); + if (allowTabNewline && (code === 0x09 || code === 0x0a)) continue; + if (code < 0x20 || code === 0x7f) return true; + } + return false; +} + +// Caps for `interactions` array-of-strings fields (configKeys, requires, +// runtimeCompat, produces, consumes, requiresBinaries, ...). These bound +// UNTRUSTED third-party strings that are rendered verbatim (after mdInline +// escaping) into a committed Markdown catalog — an unbounded count or length +// lets a malicious registry PR blow up the generated doc. +const INTERACTION_STRING_MAX = 200; +const INTERACTION_ARRAY_MAX = 50; + +/** + * Validate an interactions field that is an array of free-form untrusted + * strings: shape, element count, per-element length, and control characters. + * `allowEmpty` distinguishes "may be empty" fields from non-empty-required + * ones — non-empty-required fields' blank-array message is expected to be + * handled by the caller (this helper does not special-case emptiness itself + * beyond letting an empty array with `allowEmpty: true` through). + * + * @param {object} interactions + * @param {string} field + * @param {(field: string, reason: string) => void} addError + * @param {{allowEmpty?: boolean}} [opts] + * @returns {void} + */ +function validateStringArrayField(interactions, field, addError, { allowEmpty = true } = {}) { + const v = interactions[field]; + const qualifiedField = `interactions.${field}`; + + if (!Array.isArray(v) || !v.every((x) => typeof x === 'string')) { + addError(qualifiedField, 'must be an array of strings'); + return; + } + + if (!allowEmpty && v.length === 0) return; + + if (v.length > INTERACTION_ARRAY_MAX) { + addError(qualifiedField, `exceeds max entries ${INTERACTION_ARRAY_MAX}`); + } + + for (const x of v) { + if (x.length > INTERACTION_STRING_MAX) { + addError(qualifiedField, `exceeds max length ${INTERACTION_STRING_MAX}`); + } else if (hasDisallowedControlChar(x, false)) { + addError(qualifiedField, 'must not contain control characters'); + } + } +} // Escape Markdown inline metacharacters in UNTRUSTED free text so a registry // entry cannot inject links/tables/code-spans into the generated catalog. @@ -221,10 +307,7 @@ function validateCapabilityInteractions(interactions, addError) { for (const field of ['configKeys', 'requires', 'runtimeCompat', 'produces', 'consumes']) { if (interactions[field] === undefined) continue; - const v = interactions[field]; - if (!Array.isArray(v) || !v.every((x) => typeof x === 'string')) { - addError(`interactions.${field}`, 'must be an array of strings'); - } + validateStringArrayField(interactions, field, addError); } } @@ -312,12 +395,93 @@ function validateEosInteractions(interactions, addError) { } } +/** + * Validate the `interactions` sub-object for a reviewer entry (ADR-2782 D3 + * lane vocabulary — issue #2904). + * + * @param {object} interactions + * @param {(field: string, reason: string) => void} addError + * @returns {void} + */ +function validateReviewerInteractions(interactions, addError) { + const allowedKeys = new Set([ + 'slug', + 'flags', + 'transport', + 'evidenceClass', + 'reviewsSection', + 'requiresBinaries', + 'configKeys', + 'runtimeCompat', + ]); + for (const key of Object.keys(interactions)) { + if (!allowedKeys.has(key)) addError(`interactions.${key}`, 'unknown field'); + } + + for (const field of allowedKeys) { + if (interactions[field] === undefined) addError(`interactions.${field}`, 'missing required field'); + } + + if (interactions.slug !== undefined) { + const v = interactions.slug; + if (typeof v !== 'string' || !REVIEWER_SLUG_RE.test(v)) { + addError('interactions.slug', 'must match the reviewer lane slug grammar'); + } + } + + if (interactions.flags !== undefined) { + const v = interactions.flags; + if (!Array.isArray(v) || v.length === 0 || !v.every((x) => typeof x === 'string' && REVIEWER_FLAG_RE.test(x))) { + addError('interactions.flags', 'must be a non-empty array of lane CLI flags'); + } + } + + if (interactions.transport !== undefined) { + const v = interactions.transport; + if (typeof v !== 'string' || !REVIEWER_LANE_TRANSPORTS.includes(v)) { + addError('interactions.transport', 'must be one of the allowed lane transports'); + } + } + + if (interactions.evidenceClass !== undefined) { + const v = interactions.evidenceClass; + if (typeof v !== 'string' || !REVIEWER_EVIDENCE_CLASSES.includes(v)) { + addError('interactions.evidenceClass', 'must be one of the allowed evidence classes'); + } + } + + if (interactions.reviewsSection !== undefined) { + const v = interactions.reviewsSection; + if (typeof v !== 'string' || v.trim() === '') { + addError('interactions.reviewsSection', 'must be a non-empty string'); + } else if (v.length > REVIEWER_SECTION_MAX) { + addError('interactions.reviewsSection', `exceeds max length ${REVIEWER_SECTION_MAX}`); + } else if (hasDisallowedControlChar(v, false)) { + addError('interactions.reviewsSection', 'must not contain control characters'); + } + } + + for (const field of ['requiresBinaries', 'configKeys', 'runtimeCompat']) { + if (interactions[field] === undefined) continue; + validateStringArrayField(interactions, field, addError); + } +} + +// Per-type rules. A Map (not a plain object) so the lookup below is not a +// bracket-read on a caller-supplied key — that shape reads as a +// prototype-pollution sink to CodeQL, and a Map.get does not. +const TYPE_RULES = new Map([ + ['capability', { required: CAPABILITY_REQUIRED, validateInteractions: validateCapabilityInteractions }], + ['eos', { required: EOS_REQUIRED, validateInteractions: validateEosInteractions }], + ['reviewer', { required: REVIEWER_REQUIRED, validateInteractions: validateReviewerInteractions }], +]); + /** * Validate an array of registry entries against the closed schema for - * `opts.type` ('capability' | 'eos'). + * `opts.type` ('capability' | 'eos' | 'reviewer'). * * @param {object[]} entries - * @param {{type: 'capability'|'eos'}} opts + * @param {{type: 'capability'|'eos'|'reviewer'}} opts * @returns {{ok: boolean, errors: Array<{index: number, id?: string, field: string, reason: string}>}} */ function validateEntries(entries, opts) { @@ -325,13 +489,22 @@ function validateEntries(entries, opts) { return { ok: false, errors: [{ index: -1, field: '(root)', reason: 'entries must be an array' }] }; } + // An unrecognized type is a hard error, not a silent fallthrough. Before the + // third type existed this was a binary ternary whose ELSE branch was + // `capability`, so a typo'd type validated against the wrong schema and + // reported plausible-looking per-entry errors. + const rules = TYPE_RULES.get(opts.type); + if (!rules) { + return { ok: false, errors: [{ index: -1, field: '(root)', reason: `unknown registry type "${opts.type}"` }] }; + } + // Entry-count cap: a pathologically large array (e.g. from an automated or // malicious PR) is rejected wholesale rather than validated entry-by-entry. if (entries.length > 2000) { return { ok: false, errors: [{ index: -1, field: '(root)', reason: 'too many entries (max 2000)' }] }; } - const required = opts.type === 'eos' ? EOS_REQUIRED : CAPABILITY_REQUIRED; + const required = rules.required; const requiredSet = new Set(required); const seenIds = new Set(); const errors = []; @@ -363,21 +536,9 @@ function validateEntries(entries, opts) { } } - // Control-character rejection (defense in depth): `allowTabNewline` widens - // the reject-set exception for the two shell-snippet fields (install/ - // uninstall), which legitimately contain tabs/newlines; every other free - // text field disallows ALL C0 control characters plus DEL (incl. \n/\t). - // Checked via char codes (not a literal control-char regex range) — same - // approach as capability-validator.cjs's hooks[].matcher check, which - // avoids tripping ESLint's no-control-regex rule. - const hasDisallowedControlChar = (v, allowTabNewline) => { - for (let c = 0; c < v.length; c += 1) { - const code = v.charCodeAt(c); - if (allowTabNewline && (code === 0x09 || code === 0x0a)) continue; - if (code < 0x20 || code === 0x7f) return true; - } - return false; - }; + // Control-character rejection (defense in depth) — delegates to the + // module-scope `hasDisallowedControlChar` (shared with the `interactions` + // sub-object validators below) so there is exactly one implementation. const checkNoControlChars = (field, allowTabNewline) => { if (missing.has(field)) return; const v = entry[field]; @@ -460,10 +621,8 @@ function validateEntries(entries, opts) { const interactions = entry.interactions; if (typeof interactions !== 'object' || interactions === null || Array.isArray(interactions)) { addError('interactions', 'interactions must be an object'); - } else if (opts.type === 'eos') { - validateEosInteractions(interactions, addError); } else { - validateCapabilityInteractions(interactions, addError); + rules.validateInteractions(interactions, addError); } } @@ -477,12 +636,92 @@ function validateEntries(entries, opts) { return { ok: errors.length === 0, errors }; } +// Per-type page presentation AND per-type interaction summary both live in +// this ONE table (Map, for the same CodeQL reason as TYPE_RULES): title/ +// addNoun drive the page header, buildSummary drives the per-entry "Every +// interaction with GSD" line. Folding both into a single lookup means a +// future fourth registry type MUST supply its own buildSummary or the +// `RENDER_META.get` miss below throws — it cannot silently inherit +// capability's (or any other type's) rendering the way the old if/else-if/ +// else chain's final `else` branch used to. +const RENDER_META = new Map([ + [ + 'capability', + { + title: 'GSD Community Capability Registry', + addNoun: 'capability', + buildSummary(entry, interactions) { + let summary = + `Loop Extension Points: ${(interactions.loopExtensionPoints || []).join(', ')}; ` + + `hook kinds: ${(interactions.hookKinds || []).join(', ')}`; + for (const field of ['configKeys', 'requires', 'runtimeCompat', 'produces', 'consumes']) { + const v = interactions[field]; + if (Array.isArray(v) && v.length > 0) summary += `; ${field}: ${v.join(', ')}`; + } + // configKeys/requires/runtimeCompat/produces/consumes are untrusted + // free-form strings (schema only requires "array of strings") — same + // single-pass mdInline rationale as the eos branch above. + return summary; + }, + }, + ], + [ + 'eos', + { + title: 'GSD EoS Registry', + addNoun: 'integration', + buildSummary(entry, interactions) { + // Required AXES keys always render, in their fixed order; an OPTIONAL_AXES + // key (e.g. `effortSurface`) renders ONLY when the entry actually carries + // it — an entry that omits it must render byte-identical to before + // OPTIONAL_AXES existed (no `effortSurface=undefined` noise). + const presentOptionalKeys = Object.keys(OPTIONAL_AXES).filter( + (key) => interactions.axes && Object.hasOwn(interactions.axes, key), + ); + const axesSummary = [...Object.keys(AXES), ...presentOptionalKeys] + .map((key) => `${key}=${interactions.axes ? interactions.axes[key] : undefined}`) + .join(', '); + return ( + `Interface points: ${(interactions.interfacePoints || []).join(', ')}; ` + + `profile: ${interactions.profile}; protocol v${entry.protocolVersion}; axes: ${axesSummary}` + ); + }, + }, + ], + [ + 'reviewer', + { + title: 'GSD Reviewer Lane Registry', + addNoun: 'reviewer lane', + buildSummary(entry, interactions) { + let summary = + `Lane: ${interactions.slug}; ` + + `flags: ${(interactions.flags || []).join(', ')}; ` + + `transport: ${interactions.transport}; ` + + `evidence: ${interactions.evidenceClass}; ` + + `REVIEWS.md section: ${interactions.reviewsSection}`; + for (const field of ['requiresBinaries', 'configKeys', 'runtimeCompat']) { + const v = interactions[field]; + if (Array.isArray(v) && v.length > 0) summary += `; ${field}: ${v.join(', ')}`; + } + // slug/flags/transport are vocab-constrained; reviewsSection and the + // three arrays are untrusted free text — same single-pass mdInline + // rationale as the eos/capability branches above: none of the literal + // separator text contains Markdown metacharacters, so one pass over the + // assembled summary neutralizes every embedded value. + return summary; + }, + }, + ], +]); + /** * Render the deterministic Markdown document for a registry. * * @param {object[]} entries - * @param {{type: 'capability'|'eos', sourceFile?: string}} opts + * @param {{type: 'capability'|'eos'|'reviewer', sourceFile?: string}} opts * @returns {string} + * @throws {Error} when opts.type is not a known registry type */ function renderMarkdown(entries, opts) { const sorted = [...entries].sort((a, b) => { @@ -491,19 +730,27 @@ function renderMarkdown(entries, opts) { return 0; }); const isEos = opts.type === 'eos'; + // An unrecognized type must fail loudly rather than silently render a + // "GSD Community Capability Registry" page — mirroring the validateEntries + // unknown-type guard above. This function writes a COMMITTED catalog file, + // so a silent wrong-title render is the worst failure mode available. + // Message shape mirrors gen-registry.cjs#renderFor's existing + // `gen-registry: unknown registry type "..."` throw. + const meta = RENDER_META.get(opts.type); + if (!meta) throw new Error(`registry-schema: unknown registry type "${opts.type}"`); const lines = []; lines.push( ``, ); lines.push(''); - lines.push(isEos ? '# GSD EoS Registry' : '# GSD Community Capability Registry'); + lines.push(`# ${meta.title}`); lines.push(''); lines.push( "> **Not an endorsement.** Inclusion means only that a maintainer merged a PR linking the author's repository — GSD has not reviewed, tested, or verified any listing. See the [registry README](./README.md).", ); lines.push(''); - lines.push(`_To add your ${isEos ? 'integration' : 'capability'}, see the [registry README](./README.md)._`); + lines.push(`_To add your ${meta.addNoun}, see the [registry README](./README.md)._`); lines.push(''); if (sorted.length === 0) { @@ -536,38 +783,12 @@ function renderMarkdown(entries, opts) { lines.push(`- **What it is:** ${mdInline(entry.description)}`); lines.push(`- **Author:** ${mdInline(entry.author)}`); - if (isEos) { - // Required AXES keys always render, in their fixed order; an OPTIONAL_AXES - // key (e.g. `effortSurface`) renders ONLY when the entry actually carries - // it — an entry that omits it must render byte-identical to before - // OPTIONAL_AXES existed (no `effortSurface=undefined` noise). - const presentOptionalKeys = Object.keys(OPTIONAL_AXES).filter( - (key) => interactions.axes && Object.hasOwn(interactions.axes, key), - ); - const axesSummary = [...Object.keys(AXES), ...presentOptionalKeys] - .map((key) => `${key}=${interactions.axes ? interactions.axes[key] : undefined}`) - .join(', '); - const summary = - `Interface points: ${(interactions.interfacePoints || []).join(', ')}; ` + - `profile: ${interactions.profile}; protocol v${entry.protocolVersion}; axes: ${axesSummary}`; - // Single mdInline pass over the fully-assembled summary: none of the - // literal separator text above contains Markdown metacharacters, so - // this equally neutralizes every embedded free-text/vocab value - // (notably interactions.axes.dispatch, a free-form untrusted string). - lines.push(`- **Every interaction with GSD:** ${mdInline(summary)}`); - } else { - let summary = - `Loop Extension Points: ${(interactions.loopExtensionPoints || []).join(', ')}; ` + - `hook kinds: ${(interactions.hookKinds || []).join(', ')}`; - for (const field of ['configKeys', 'requires', 'runtimeCompat', 'produces', 'consumes']) { - const v = interactions[field]; - if (Array.isArray(v) && v.length > 0) summary += `; ${field}: ${v.join(', ')}`; - } - // configKeys/requires/runtimeCompat/produces/consumes are untrusted - // free-form strings (schema only requires "array of strings") — same - // single-pass mdInline rationale as the eos branch above. - lines.push(`- **Every interaction with GSD:** ${mdInline(summary)}`); - } + // Single mdInline pass over the fully-assembled per-type summary: none of + // the literal separator text in any RENDER_META buildSummary implementation + // contains Markdown metacharacters, so one pass over the assembled string + // equally neutralizes every embedded free-text/vocab value (notably eos's + // interactions.axes.dispatch, a free-form untrusted string). + lines.push(`- **Every interaction with GSD:** ${mdInline(meta.buildSummary(entry, interactions))}`); // Code-span content (install/uninstall) is NOT mdInline-escaped — it is a // verbatim shell snippet, not inline prose. Instead each block picks a @@ -608,6 +829,14 @@ module.exports = { AXES_FREE_STRING, CAPABILITY_REQUIRED, EOS_REQUIRED, + REVIEWER_REQUIRED, + REVIEWER_LANE_TRANSPORTS, + REVIEWER_EVIDENCE_CLASSES, + REVIEWER_SLUG_RE, + REVIEWER_FLAG_RE, + REVIEWER_SECTION_MAX, + INTERACTION_STRING_MAX, + INTERACTION_ARRAY_MAX, isValidGsdRange, validateEntries, renderMarkdown, diff --git a/scripts/validate-registry.cjs b/scripts/validate-registry.cjs index 1e9671d1e..de3eac51c 100644 --- a/scripts/validate-registry.cjs +++ b/scripts/validate-registry.cjs @@ -3,11 +3,13 @@ /** * scripts/validate-registry.cjs — CLI validator for the third-party - * discoverability catalogs (issue #2182): + * discoverability catalogs (issue #2182, plus #2904): * * - docs/registries/capabilities.json ("GSD Community Capability Registry") * - docs/registries/eos.json ("GSD EoS Registry", PR2 — optional * until that JSON file ships) + * - docs/registries/reviewers.json ("GSD Reviewer Lane Registry", + * issue #2904 — optional until that JSON file ships) * * Validates each source's JSON array against the closed schema in * scripts/registry-schema.cjs (validateEntries). Human-readable errors go to @@ -33,14 +35,15 @@ const { validateEntries } = require('./registry-schema.cjs'); // a subprocess against isolated temp-fixture directories via `cwd`. const SOURCES = [ { file: 'capabilities.json', type: 'capability' }, - { file: 'eos.json', type: 'eos' }, + { file: 'eos.json', type: 'eos', optional: true }, + { file: 'reviewers.json', type: 'reviewer', optional: true }, ]; /** * Load + validate a single registry JSON file. * * @param {string} jsonPath absolute path to the registry JSON file - * @param {'capability'|'eos'} type + * @param {'capability'|'eos'|'reviewer'} type * @returns {{ok: boolean, errors: Array<{index: number, id?: string, field: string, reason: string}>}} */ function validateFile(jsonPath, type) { @@ -81,10 +84,11 @@ function main() { const results = []; let anyFailed = false; - for (const { file, type } of SOURCES) { + for (const { file, type, optional } of SOURCES) { const jsonPath = path.join(registriesDir, file); - // eos.json is optional until PR2 ships it — skip silently when absent. - if (type === 'eos' && !fs.existsSync(jsonPath)) continue; + // eos.json (pre-PR2) and reviewers.json (issue #2904) are optional until + // their source JSON ships — skip silently when absent. + if (optional && !fs.existsSync(jsonPath)) continue; const verdict = validateFile(jsonPath, type); results.push({ file, type, ok: verdict.ok, errors: verdict.errors }); diff --git a/src/milestone.cts b/src/milestone.cts index ffdcb4ac7..6ce333999 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -156,10 +156,15 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool // Surface 1 — the checkbox: - [ ] **REQ-ID** → - [x] **REQ-ID** // Use replace() + compare to avoid the test()+replace() global regex // lastIndex bug where test() advances state and replace() misses matches. + // (#2788 defect 2: the flip is CONDITIONAL — when a traceability row EXISTS + // for this ID but its Status write is rejected, the checkbox must NOT flip, + // so the two surfaces cannot silently diverge. The row-write outcome below + // gates whether the flip is kept.) const checkboxPattern = new RegExp(`(-\\s*\\[)[ ](\\]\\s*\\*\\*${reqEscaped}\\*\\*)`, 'gi'); + const beforeCheckbox = reqContent; const afterCheckbox = reqContent.replace(checkboxPattern, '$1x$2'); - const checkboxHit = afterCheckbox !== reqContent; - if (checkboxHit) reqContent = afterCheckbox; + const checkboxFlipped = afterCheckbox !== beforeCheckbox; + if (checkboxFlipped) reqContent = afterCheckbox; // Surface 2 — the traceability row: | | Phase N | Pending | → ... Complete | // via the markdown-table seam (ADR-2143 §7) — supersedes the prior ordinal @@ -178,7 +183,11 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool // updateTableCell call both probes the current value and writes. let tableHit = false; const tableUpdate = updateTraceabilityCell(reqContent, rowMatch, 'Status', (current) => { - if (/^pending$/i.test(current.trim())) { + // #2788: accept `Gaps Found` as a forward input too — `revert-phase` (the + // documented gaps_found response) leaves a row stranded at Gaps Found with + // no inverse; a genuinely-satisfied requirement must be able to reach + // Complete again via mark-complete, or the milestone is blocked forever. + if (/^(pending|gaps found)$/i.test(current.trim())) { tableHit = true; return ' Complete '; } @@ -188,6 +197,18 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool reqContent = tableUpdate.value; } + // #2788 defect 2: if a row EXISTS for this ID but its Status write was + // rejected (e.g. the row reads `Blocked`, which mark-complete does not + // accept), roll the checkbox back so the checkbox and the row cannot + // silently diverge. The checkbox and the row are two representations of the + // same fact; flipping one while the other rejects the write is the lie. + let checkboxHit = checkboxFlipped; + const rowExistsProbe = tableUpdate; // ok === a row matched (probes existence) + if (checkboxFlipped && rowExistsProbe.ok && !tableHit) { + reqContent = beforeCheckbox; + checkboxHit = false; + } + // ADR-2143 §6 per-ID write-set entries: this ID's checkbox surface is // always tracked; the traceability surface is tracked only when the file // has a traceability table at all (same `hasTable` gate the existing @@ -215,7 +236,14 @@ function cmdRequirementsMarkComplete(cwd: string, reqIdsRaw: string[], raw: bool const doneCheckbox = new RegExp(`-\\s*\\[x\\]\\s*\\*\\*${reqEscaped}\\*\\*`, 'i').test(reqContent); const doneTable = Boolean(hasRow && /^complete$/i.test(currentStatusCell.trim())); - if (checkboxHit || tableHit) { + // #2788 defect 2: when a traceability table exists AND this ID has a row in + // it, `updated`/`marked_complete` must reflect the ROW moving, not a + // checkbox-only flip. Otherwise (`table_unmatched` — no row for this ID, or + // no table at all) the checkbox flip is a legitimate partial reconcile / the + // sole completion surface, so the #2140 OR semantics are preserved. + const rowExists = hasTable && hasRow; + const idUpdated = rowExists ? tableHit : (checkboxHit || tableHit); + if (idUpdated) { updated.push(reqId); } else if (doneTable || (doneCheckbox && !hasTable)) { // Fully reconciled: the table row is Complete, OR the checkbox is done and diff --git a/src/phase.cts b/src/phase.cts index 9d80b29b7..9e87f0880 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -2078,7 +2078,9 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // Complete" gate is folded into the newValue callback so one // updateTableCell call both probes and writes. const reqUpdate = updateTraceabilityCell(reqContent, reqRowMatch, 'Status', (current) => - /^(?:pending|in progress)$/i.test(current.trim()) ? ' Complete ' : current); + // #2788: accept `Gaps Found` too so a phase stranded by revert-phase (the + // gaps_found response) can complete without hand-editing the table. + /^(?:pending|in progress|gaps found)$/i.test(current.trim()) ? ' Complete ' : current); if (reqUpdate.ok) { reqContent = reqUpdate.value; } else if (!isPlaceholderReqId(reqId)) { diff --git a/src/project-root.cts b/src/project-root.cts index 821e93b9a..dd2555fab 100644 --- a/src/project-root.cts +++ b/src/project-root.cts @@ -57,6 +57,30 @@ export function findProjectRoot(startDir: string): string { return false; } + // #2843: nearest ancestor (including `from` itself) that contains a `.git`, + // bounded by `upTo` (exclusive). Returns the git-repo root, or null if none + // exists before `upTo` / the filesystem root. Used to detect a NESTED child + // repo whose root is strictly below a candidate ancestor `.planning/` — in + // that case the caller's repo boundary sits between start and the ancestor, + // so trusting the ancestor's `.planning/` would silently cross into a + // different project. (No `git` subprocess — fs walk only, matching + // isInsideGitRepo's deliberate no-spawn contract.) + function nearestGitRoot(from: string, upTo: string): string | null { + let d = from; + while (d !== fsRoot) { + if (d === upTo) break; + try { + if (fs.existsSync(d + path.sep + '.git')) return d; + } catch { + // ignore + } + const next = path.dirname(d); + if (next === d) break; + d = next; + } + return null; + } + let dir = resolvedStart; let depth = 0; @@ -107,6 +131,18 @@ export function findProjectRoot(startDir: string): string { // claims our startDir — explicit sub_repos config takes precedence over the // implicit .git signal. (#1422) if (isInsideGitRepo(parent)) { + // #2843: do NOT cross a git-repo boundary. If the caller is inside its + // OWN nested repo whose root is strictly below `parent`, trusting + // `parent`'s .planning/ would silently resolve to a DIFFERENT project. + // isInsideGitRepo only proved SOME .git exists between start and parent; + // verify that .git is parent's own (or absent between), not a nested + // child repo. If nearestGitRoot finds a .git strictly below parent, + // the boundary is crossed — fall through (do not return parent). + if (nearestGitRoot(resolvedStart, parent) !== null) { + dir = parent; + depth += 1; + continue; + } // Lookahead: walk ancestors above `parent` to find a sub_repos claim. let ancestor = path.dirname(parent); let ancestorDepth = 0; @@ -162,6 +198,15 @@ export function findProjectRoot(startDir: string): string { try { const candidatePlanning = parent2 + path.sep + '.planning'; if (fs.existsSync(candidatePlanning) && fs.statSync(candidatePlanning).isDirectory()) { + // #2843: do not cross a git-repo boundary. If the caller is inside its + // own nested repo (a .git strictly below parent2), parent2's .planning/ + // belongs to a DIFFERENT project — keep walking is wrong; stop and fall + // through to the startDir fallback instead of silently resolving to the + // ancestor project. (Reached only when no .git exists anywhere in the + // chain per the triage, but guard defensively.) + if (nearestGitRoot(resolvedStart, parent2) !== null) { + break; + } return parent2; } } catch { diff --git a/src/verify.cts b/src/verify.cts index 75e676783..f64be88be 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -176,6 +176,16 @@ function verifySummaryCore( // is still not read. Recovering it needs a real frontmatter parse, which is // deliberately left as a follow-up rather than smuggled in here. const mentionedFiles = new Set(); + // #2844: Pattern 1 matches any backticked path-like token. A SUMMARY body is + // predominantly about what the phase DID, so a backticked path in prose ("Built + // `src/kept.ts`", a `- \`src/x.ts\`` list item) is a legitimate claim (#2685 + // pins this). The false-positive class #2844 fixes is a path mentioned as a + // FUTURE/CONDITIONAL deliverable — "next phase will add `shared/types.ts`", + // "planned", "would", "to be created" — which is NOT a claim about this phase. + // Exclude those lines rather than requiring an explicit claim verb (which would + // drop the legitimate "Built …" / list-item forms #2685 protects). + const isFutureMention = (line: string): boolean => + /\b(?:will(?:\s+(?:add|create|build|land))?(?:[^.])?|(?:next|later|future)\s+phase|planned?|would\s+(?:be|add|create|build)|to\s+be\s+(?:added|created|built)|eventually|not\s+yet)\b/i.test(line); const patterns = [ /`([^`]+\.[a-zA-Z]+)`/g, /(?:Created|Modified|Added|Updated|Edited):\s*`?([^\s`[\]]+\.[a-zA-Z]+)`?/gi, @@ -185,9 +195,14 @@ function verifySummaryCore( let m: RegExpExecArray | null; while ((m = pattern.exec(content)) !== null) { const filePath = m[1]; - if (filePath && isProbableProjectFile(filePath)) { - mentionedFiles.add(filePath); - } + if (!filePath || !isProbableProjectFile(filePath)) continue; + // #2844: skip a backticked path on a future/conditional line — it names a + // deliverable this phase did NOT produce, so probing it is a false positive. + const lineStart = content.lastIndexOf('\n', m.index) + 1; + const lineEnd = content.indexOf('\n', m.index); + const line = content.slice(lineStart, lineEnd === -1 ? undefined : lineEnd); + if (isFutureMention(line)) continue; + mentionedFiles.add(filePath); } } diff --git a/tests/code-review.test.cjs b/tests/code-review.test.cjs index cd4f554b8..0e2940949 100644 --- a/tests/code-review.test.cjs +++ b/tests/code-review.test.cjs @@ -260,6 +260,39 @@ describe('CR-AGENT: code review agent frontmatter', () => { assert.ok(content.includes('files_reviewed_list'), 'gsd-code-reviewer REVIEW.md frontmatter spec must include files_reviewed_list for --auto scope persistence'); }); + + // #2825: gsd-code-fixer is the only writer that hand-rolls a git worktree; it + // must honor workflow.use_worktrees (the documented opt-out) like its four + // sibling writer workflows, and never rm -rf a possible Windows reparse point. + test('#2825 gsd-code-fixer.md reads workflow.use_worktrees and gates git worktree add on it', () => { + const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8'); + assert.ok( + content.includes('workflow.use_worktrees'), + 'gsd-code-fixer setup_worktree must read the workflow.use_worktrees config flag (#2825)', + ); + // The git worktree add must be CONDITIONAL on the flag, not unconditional. + // Locate the worktree-add line and confirm a USE_WORKTREES gate precedes it. + assert.ok( + /USE_WORKTREES=.false./.test(content) || content.includes('if [ "$USE_WORKTREES" = "false" ]'), + 'gsd-code-fixer must gate worktree creation on USE_WORKTREES=false (skip when opted out) (#2825)', + ); + }); + + test('#2825 gsd-code-fixer.md forbids rm -rf on a possible reparse point (Windows junction safety)', () => { + const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8'); + assert.ok( + /rm -rf.*reparse point|reparse point.*rm -rf|NEVER .rm -rf.|never use .rm -rf/i.test(content), + 'gsd-code-fixer must forbid rm -rf on a possible reparse point/junction (#2825) — on Windows that is the delete-the-target path', + ); + }); + + test('#2825 gsd-code-fixer.md records where verification ran (main checkout vs worktree)', () => { + const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-code-fixer.md'), 'utf-8'); + assert.ok( + /verification[\s\S]*(main checkout|worktree)|(main checkout|worktree)[\s\S]*verification/i.test(content), + 'gsd-code-fixer REVIEW-FIX.md must record where verification ran (main checkout vs worktree) so a reader knows if the numbers are reproducible (#2825)', + ); + }); }); // --- CR-CMD: code review command structure --- diff --git a/tests/emitted-drift-ack.json b/tests/emitted-drift-ack.json index fa2f84cde..1284ce27d 100644 --- a/tests/emitted-drift-ack.json +++ b/tests/emitted-drift-ack.json @@ -1,20 +1,39 @@ { "version": 1, "paths": { - "plan-phase.md": { - "reason": "#2770: decision-coverage gate recomputes CONTEXT_PATH in-block + guards the empty-glob case (handler now fails closed on empty arg). Net growth kept under the ADR-857 size cap by condensing adjacent §13a prose/JSON; the residual +89 bytes are the irreducible glob+guard logic." - }, - "discuss-phase-assumptions.md": { - "reason": "#2772: re-synced to the parent canonical block (had drifted — lost the 'Other' empty-text branch) + fixed the auto_advance→confirm_creation circularity (end the workflow). discuss-phase.md is net -11 (condensed); auto.md shrank -4650 (removed a dead MAX_PASSES resolver shim)." - }, - "autonomous.md": { - "reason": "#2800 — the hand-enumerated reviewer-flag list is replaced by a loop over `gsd_run review-lane flags`, and the whole CONVERGENCE_ARGS construction is relocated below the runtime-launcher preamble because it now calls gsd_run and each bash fence is its own shell. Growth is the explanatory comments carrying that constraint plus the loop body, against nine deleted flag literals. Deliberate: the byte delta is the cost of the flags no longer being hand-maintained in three places that had drifted apart." - }, - "next.md": { - "reason": "#2800 — same derived-flag-loop change as autonomous.md. next.md needed no relocation (its launcher preamble already precedes the loop), so the growth here is only the loop plus the comment recording that --all and --text stay literal because they are convergence controls, not reviewer lanes." - }, - "code-review.md": { - "reason": "#2666: the Tier-2 SUMMARY extractor predicate is relaxed to accept root-level and extensionless build files (Dockerfile/Makefile/etc.), and the Tier-3 git-diff fallback is converted from an eq-zero gate into an intersect-and-warn that cross-checks the SUMMARY scope against `git diff --name-only` with exact whole-line matching (grep -Fxq) and warns about + adds any changed files the extractor missed. Growth is the relaxed predicate + the portable cross-check branch + their explanatory comments." - } + "agents/gsd-advisor-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-ai-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-assumptions-analyzer.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-code-reviewer.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-codebase-mapper.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-debug-session-manager.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-debugger.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-doc-classifier.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-doc-synthesizer.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-doc-verifier.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-doc-writer.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-domain-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-eval-auditor.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-eval-planner.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-executor.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-framework-selector.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-integration-checker.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-intel-updater.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-mempalace-curator.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-nyquist-auditor.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-pattern-mapper.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-phase-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-plan-checker.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-planner.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-project-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-research-synthesizer.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-roadmapper.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-security-auditor.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-ui-auditor.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-ui-checker.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-ui-researcher.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-user-profiler.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "agents/gsd-verifier.toml": "#2834: Codex agent TOML now carries model-routing fields on first install.", + "gsd-code-fixer.md": "#2825: setup_worktree now reads workflow.use_worktrees and gates git worktree add on it (skipping worktree creation when the user opted out), the cleanup tail is gated to a no-op in that mode, and the spec adds three safety guardrails — honor the opt-out, never rm -rf a possible Windows reparse point/junction (the delete-the-target path that wiped real node_modules), and record where verification ran. Growth is the gated bash branch + the three guardrail paragraphs." } } diff --git a/tests/gen-registry.test.cjs b/tests/gen-registry.test.cjs index cb3ee7d71..7202feb82 100644 --- a/tests/gen-registry.test.cjs +++ b/tests/gen-registry.test.cjs @@ -41,6 +41,63 @@ function validCapabilityEntry() { }; } +function validEosEntry() { + return { + id: 'my-host-plugin', + name: 'My Host Plugin', + type: 'eos', + repo: 'octocat/my-host-plugin', + description: 'Embeds GSD as an orchestration engine in My Host.', + author: 'Octocat', + license: 'MIT', + enginesGsd: '>=1.6.0 <3.0.0', + install: 'See the My Host plugin marketplace listing.', + uninstall: 'Uninstall via the My Host plugin manager.', + protocolVersion: 1, + interactions: { + interfacePoints: ['command', 'state'], + profile: 'programmatic-cli', + axes: { + embeddingMode: 'imperative', + commandSurface: 'slash-file', + dispatch: 'Supports nested background dispatch up to depth 3.', + modelMode: 'active', + hookBus: 'host', + stateIO: 'filesystem', + transport: 'mcp', + runtime: 'node', + }, + }, + discussion: 'https://github.com/octocat/my-host-plugin/discussions/2', + }; +} + +function validReviewerEntry() { + return { + id: 'my-reviewer', + name: 'My Reviewer', + type: 'reviewer', + repo: 'octocat/my-reviewer', + description: 'Reviews GSD PRs for a specific concern.', + author: 'Octocat', + license: 'MIT', + enginesGsd: '>=1.6.0 <3.0.0', + install: 'gsd capability install https://github.com/octocat/my-reviewer.git#v1.0.0', + uninstall: 'gsd capability remove my-reviewer', + interactions: { + slug: 'my-reviewer', + flags: ['--my-reviewer'], + transport: 'spawn', + evidenceClass: 'source-grounded', + reviewsSection: 'My Reviewer', + requiresBinaries: [], + configKeys: [], + runtimeCompat: ['all'], + }, + discussion: 'https://github.com/octocat/my-reviewer/discussions/1', + }; +} + function withFixture(entries, fn) { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-gen-registry-')); try { @@ -53,6 +110,22 @@ function withFixture(entries, fn) { } } +// Same as withFixture, but also writes docs/registries/reviewers.json — used +// by the reviewer-catalog cases below (#2904), which need capabilities.json +// AND reviewers.json present simultaneously. +function withReviewerFixture(capabilityEntries, reviewerEntries, fn) { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-gen-registry-reviewer-')); + try { + const registriesDir = path.join(tmp, 'docs', 'registries'); + fs.mkdirSync(registriesDir, { recursive: true }); + fs.writeFileSync(path.join(registriesDir, 'capabilities.json'), JSON.stringify(capabilityEntries, null, 2) + '\n'); + fs.writeFileSync(path.join(registriesDir, 'reviewers.json'), JSON.stringify(reviewerEntries, null, 2) + '\n'); + fn(tmp, registriesDir); + } finally { + cleanup(tmp); + } +} + function runGen(cwd, args = []) { return spawnSync(process.execPath, [SCRIPT_PATH, ...args], { cwd, encoding: 'utf8' }); } @@ -142,3 +215,120 @@ describe('gen-registry: renderMarkdown (direct, via registry-schema)', () => { assert.ok(rendered.includes(entry.discussion)); }); }); + +// ─── gen-registry CLI (subprocess): reviewer catalog (#2904) ─────────────── +// +// scripts/gen-registry.cjs's SOURCES array does not yet include the reviewer +// { type:'reviewer', jsonFile:'reviewers.json', mdFile:'reviewer-registry.md', +// optional:true } entry — every case below is FAILING-FIRST against the +// unmodified script. See +// .gsd/phase/feat-2904-enh-registries-add-a-reviewer-entry-type/50-test-matrix.md. + +describe('gen-registry CLI (subprocess): reviewer catalog', () => { + test('--write emits all three catalogs when all sources exist', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-gen-registry-all3-')); + try { + const registriesDir = path.join(tmp, 'docs', 'registries'); + fs.mkdirSync(registriesDir, { recursive: true }); + fs.writeFileSync(path.join(registriesDir, 'capabilities.json'), JSON.stringify([validCapabilityEntry()], null, 2) + '\n'); + fs.writeFileSync(path.join(registriesDir, 'eos.json'), JSON.stringify([validEosEntry()], null, 2) + '\n'); + fs.writeFileSync(path.join(registriesDir, 'reviewers.json'), JSON.stringify([validReviewerEntry()], null, 2) + '\n'); + + const write = runGen(tmp, ['--write']); + assert.equal(write.status, 0, `stderr: ${write.stderr}`); + assert.ok(fs.existsSync(path.join(registriesDir, 'capability-registry.md'))); + assert.ok(fs.existsSync(path.join(registriesDir, 'eos-registry.md'))); + assert.ok( + fs.existsSync(path.join(registriesDir, 'reviewer-registry.md')), + 'expected reviewer-registry.md to be written', + ); + } finally { + cleanup(tmp); + } + }); + + test('an absent reviewers.json is skipped, not an error', () => { + withFixture([validCapabilityEntry()], (tmp, registriesDir) => { + const write = runGen(tmp, ['--write']); + assert.equal(write.status, 0, `stderr: ${write.stderr}`); + assert.ok(fs.existsSync(path.join(registriesDir, 'capability-registry.md'))); + assert.ok( + !fs.existsSync(path.join(registriesDir, 'reviewer-registry.md')), + 'expected no reviewer-registry.md when reviewers.json is absent', + ); + }); + }); + + test('--check fails when the reviewer catalog is missing', () => { + withReviewerFixture([validCapabilityEntry()], [validReviewerEntry()], (tmp, registriesDir) => { + // Write only the capability md by hand (simulating a repo that has + // reviewers.json committed but never ran --write for it). + fs.writeFileSync( + path.join(registriesDir, 'capability-registry.md'), + renderMarkdown([validCapabilityEntry()], { type: 'capability', sourceFile: 'capabilities.json' }), + ); + const check = runGen(tmp, ['--check']); + assert.notEqual(check.status, 0, `expected non-zero exit, got 0. stdout: ${check.stdout}`); + assert.match(check.stderr, /reviewer-registry\.md does not exist/); + }); + }); + + test('--check fails on reviewer catalog drift', () => { + withReviewerFixture([validCapabilityEntry()], [validReviewerEntry()], (tmp, registriesDir) => { + const write = runGen(tmp, ['--write']); + assert.equal(write.status, 0, `stderr: ${write.stderr}`); + + const mdPath = path.join(registriesDir, 'reviewer-registry.md'); + fs.appendFileSync(mdPath, '\nhand-edited drift line\n'); + + const check = runGen(tmp, ['--check']); + assert.notEqual(check.status, 0); + assert.match(check.stderr, /reviewer-registry\.md is stale/); + }); + }); + + test('--check passes on a fresh reviewer catalog', () => { + withReviewerFixture([validCapabilityEntry()], [validReviewerEntry()], (tmp) => { + const write = runGen(tmp, ['--write']); + assert.equal(write.status, 0, `stderr: ${write.stderr}`); + + const check = runGen(tmp, ['--check']); + assert.equal(check.status, 0, `stderr: ${check.stderr}`); + }); + }); + + test('CRLF in the committed reviewer catalog is not drift', () => { + withReviewerFixture([validCapabilityEntry()], [validReviewerEntry()], (tmp, registriesDir) => { + const write = runGen(tmp, ['--write']); + assert.equal(write.status, 0, `stderr: ${write.stderr}`); + + const mdPath = path.join(registriesDir, 'reviewer-registry.md'); + const original = fs.readFileSync(mdPath, 'utf8'); + // Normalize via \r?\n so the conversion is idempotent, ensuring the fixture is exactly + // the CRLF variant even on a checkout that already delivered CRLF line endings. + fs.writeFileSync(mdPath, original.replace(/\r?\n/g, '\r\n')); + + const check = runGen(tmp, ['--check']); + assert.equal(check.status, 0, `expected CRLF-only diff to not be drift. stderr: ${check.stderr}`); + }); + }); + + test('malformed reviewers.json fails cleanly', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-gen-registry-badjson-')); + try { + const registriesDir = path.join(tmp, 'docs', 'registries'); + fs.mkdirSync(registriesDir, { recursive: true }); + fs.writeFileSync(path.join(registriesDir, 'capabilities.json'), JSON.stringify([validCapabilityEntry()], null, 2) + '\n'); + fs.writeFileSync(path.join(registriesDir, 'reviewers.json'), '{ this is not valid JSON'); + + const result = runGen(tmp, ['--write']); + assert.notEqual(result.status, 0, `expected non-zero exit, got 0. stdout: ${result.stdout}`); + assert.ok( + !/at Object\./.test(result.stderr) && !/\.js:\d+:\d+/.test(result.stderr), + `expected no raw Node stack trace leaked to stderr, got: ${result.stderr}`, + ); + } finally { + cleanup(tmp); + } + }); +}); diff --git a/tests/issue-2834-codex-install-model-ordering.test.cjs b/tests/issue-2834-codex-install-model-ordering.test.cjs new file mode 100644 index 000000000..6be649462 --- /dev/null +++ b/tests/issue-2834-codex-install-model-ordering.test.cjs @@ -0,0 +1,43 @@ +// allow-test-rule: structural-implementation-guard (#2834) +'use strict'; + +// Regression guard for #2834: on a clean Codex install, agent TOMLs contained no +// model-routing fields because defaults.json (resolve_model_ids + runtime) was written +// AFTER installCodexConfig generated the TOMLs. The fix extracts writeNonClaudeDefaults +// and calls it BEFORE installCodexConfig. This test asserts the ordering invariant in +// the install source so a future edit can't silently re-introduce the gap. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const INSTALL_JS = path.join(__dirname, '..', 'bin', 'install.js'); + +test('writeNonClaudeDefaults is called before installCodexConfig in the Codex install flow (#2834)', () => { + const src = fs.readFileSync(INSTALL_JS, 'utf8'); + + // Find the call to writeNonClaudeDefaults that precedes installCodexConfig. + const writeIdx = src.indexOf('writeNonClaudeDefaults(runtime);'); + assert.ok(writeIdx !== -1, 'writeNonClaudeDefaults(runtime) must be called in the install flow'); + + // Find the FIRST installCodexConfig call AFTER the writeNonClaudeDefaults call. + const codexGenIdx = src.indexOf('installCodexConfig(targetDir', writeIdx); + assert.ok(codexGenIdx !== -1 && codexGenIdx > writeIdx, + 'installCodexConfig must be called AFTER writeNonClaudeDefaults so defaults.json ' + + '(resolve_model_ids + runtime) exists before agent TOML generation reads it (#2834)'); + + // The #2834 comment must be present at the call site. + const callSite = src.slice(writeIdx - 300, writeIdx + 100); + assert.ok(/#2834/.test(callSite), 'the writeNonClaudeDefaults call must carry the #2834 rationale comment'); +}); + +test('writeNonClaudeDefaults function exists and is a no-op for Claude (#2834)', () => { + const src = fs.readFileSync(INSTALL_JS, 'utf8'); + const fnIdx = src.indexOf('function writeNonClaudeDefaults('); + assert.ok(fnIdx !== -1, 'writeNonClaudeDefaults must be defined as a function'); + const fnBody = src.slice(fnIdx, fnIdx + 1200); + assert.ok(/nativeModelAliases/.test(fnBody), 'writeNonClaudeDefaults must early-return for Claude (nativeModelAliases check)'); + assert.ok(/resolve_model_ids/.test(fnBody), 'writeNonClaudeDefaults must write resolve_model_ids'); + assert.ok(/defaults\.runtime/.test(fnBody), 'writeNonClaudeDefaults must write runtime'); +}); diff --git a/tests/milestone.test.cjs b/tests/milestone.test.cjs index f2bff639f..342714ae7 100644 --- a/tests/milestone.test.cjs +++ b/tests/milestone.test.cjs @@ -971,6 +971,117 @@ describe('requirements mark-complete command', () => { assert.strictEqual(output.updated, false); assert.strictEqual(output.reason, 'REQUIREMENTS.md not found'); }); + + // #2788: a requirement row stranded at `Gaps Found` (by revert-phase, the + // gaps_found response) must be recoverable — mark-complete moves it to Complete. + // Pre-fix the `/^pending$/i` guard rejected `Gaps Found`, stranding the row + // permanently (no inverse transition existed) AND mark-complete reported + // `updated: true` while the row stayed `Gaps Found` (defect 2, the lie). + const GAPS_FOUND_REQUIREMENTS = `# Requirements + +## Coverage +- [ ] **REQ-01**: feature one +- [ ] **REQ-02**: feature two + +## Traceability + +| Requirement | Phase | Status | +|-------------|-------|--------| +| REQ-01 | Phase 1 | Gaps Found | +| REQ-02 | Phase 1 | Complete | +`; + + test('#2788 defect 1: a Gaps Found row moves to Complete via mark-complete (no longer terminal)', () => { + writeRequirements(tmpDir, GAPS_FOUND_REQUIREMENTS); + const result = runGsdTools('requirements mark-complete REQ-01', tmpDir); + assert.ok(result.success); + const out = JSON.parse(result.output); + // The row EXISTS and moved to Complete, so updated/marked_complete are truthful. + assert.ok(out.updated, 'the stranded Gaps Found row must be recoverable'); + assert.ok(out.marked_complete.includes('REQ-01')); + const content = readRequirements(tmpDir); + assert.ok(/REQ-01 \| Phase 1 \| Complete/.test(content), + 'the row must read Complete after mark-complete; got:\n' + content); + assert.ok(content.includes('- [x] **REQ-01**'), 'the checkbox must be checked'); + }); + + test('#2788 defect 2: when a row EXISTS but the write is rejected, updated is FALSE (no false success)', () => { + // A row at `Blocked` (a status mark-complete does not accept) EXISTS for REQ-01. + // The checkbox flips, but the row does not move — `updated` must be false so the + // operator is not told it worked while the audit row still reads Blocked. + const blockedRequirements = `# Requirements + +## Coverage +- [ ] **REQ-01**: feature one + +## Traceability + +| Requirement | Phase | Status | +|-------------|-------|--------| +| REQ-01 | Phase 1 | Blocked | +`; + writeRequirements(tmpDir, blockedRequirements); + const result = runGsdTools('requirements mark-complete REQ-01', tmpDir); + assert.ok(result.success); + const out = JSON.parse(result.output); + assert.strictEqual(out.updated, false, + 'a checkbox flip on a table-bearing file whose row EXISTS but did not move must NOT report updated:true'); + assert.ok(!out.marked_complete.includes('REQ-01'), + 'REQ-01 must not be in marked_complete when the row write was rejected'); + const content = readRequirements(tmpDir); + assert.ok(/REQ-01 \| Phase 1 \| Blocked/.test(content), + 'the row must still read Blocked (write rejected); got:\n' + content); + // #2788 defect 2: the checkbox must NOT flip when the row write is rejected — + // the checkbox and the row are two representations of the same fact, so they + // must not silently diverge. The checkbox stays unchecked on disk. + assert.ok(content.includes('- [ ] **REQ-01**'), + 'the checkbox must stay unchecked when the row write was rejected; got:\n' + content); + // write_set carries the truth: NEITHER surface applied (checkbox rolled back, + // traceability rejected). + const checkboxEntry = out.write_set.find( + (e) => e.requirement === 'REQ-01' && e.surface === 'checkbox'); + assert.ok(checkboxEntry && checkboxEntry.applied === false, + 'write_set must record the checkbox surface as not applied (rolled back)'); + const traceabilityEntry = out.write_set.find( + (e) => e.requirement === 'REQ-01' && e.surface === 'traceability'); + assert.ok(traceabilityEntry && traceabilityEntry.applied === false, + 'write_set must record the traceability surface as not applied'); + }); + + test('#2788 end-to-end: revert-phase (Complete → Gaps Found) then mark-complete (Gaps Found → Complete) round-trips', () => { + const completeRequirements = `# Requirements + +## Coverage +- [x] **REQ-01**: feature one + +## Traceability + +| Requirement | Phase | Status | +|-------------|-------|--------| +| REQ-01 | Phase 1 | Complete | +`; + writeRequirements(tmpDir, completeRequirements); + // revert-phase strands the row at Gaps Found (the gaps_found response). + const reverted = JSON.parse(runGsdTools('requirements revert-phase REQ-01', tmpDir).output); + assert.ok(reverted.reverted.includes('REQ-01')); + let content = readRequirements(tmpDir); + assert.ok(/REQ-01 \| Phase 1 \| Gaps Found/.test(content), 'revert should strand at Gaps Found'); + // Now the requirement is genuinely satisfied again — mark-complete must recover it. + const marked = JSON.parse(runGsdTools('requirements mark-complete REQ-01', tmpDir).output); + assert.ok(marked.updated, 'the stranded row must be recoverable via mark-complete'); + content = readRequirements(tmpDir); + assert.ok(/REQ-01 \| Phase 1 \| Complete/.test(content), + 'after mark-complete the row must read Complete again'); + }); + + test('#2788 negative-space: the normal Pending → Complete path is unchanged', () => { + writeRequirements(tmpDir, STANDARD_REQUIREMENTS); + const out = JSON.parse(runGsdTools('requirements mark-complete TEST-01', tmpDir).output); + assert.ok(out.updated); + assert.ok(out.marked_complete.includes('TEST-01')); + const content = readRequirements(tmpDir); + assert.ok(/TEST-01 \| Phase 1 \| Complete/.test(content), 'Pending row still moves to Complete'); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/project-root.test.cjs b/tests/project-root.test.cjs index 0942a4fab..b226bceeb 100644 --- a/tests/project-root.test.cjs +++ b/tests/project-root.test.cjs @@ -275,4 +275,44 @@ describe('findProjectRoot nearest-.planning resolution (#1414)', () => { assert.strictEqual(result, nested, 'Should return startDir unchanged when no ancestor has .planning/ within the depth bound'); }); + + // #2843: findProjectRoot must NOT cross a git-repo boundary. A nested child + // repo (own .git, no .planning of its own) under an ancestor GSD project must + // NOT resolve to the ancestor's root — that would silently return a different + // project's identity with exit 0. Pre-fix heuristic (3)'s isInsideGitRepo only + // checked "does SOME .git exist between start and the ancestor" — the child's + // own .git satisfied it, crossing the boundary. + test('#2843 does not resolve to an ancestor project across a nested child .git boundary', () => { + // tmpDir/ (plain — not a git repo, not HOME) + // parent-gsd/.planning/ ← ancestor GSD project + // parent-gsd/child-app/.git ← nested child repo, NO .planning of its own + // parent-gsd/child-app/src/ ← startDir + const parentGsd = mkDeep(tmpDir, 'parent-gsd'); + fs.mkdirSync(path.join(parentGsd, '.planning'), { recursive: true }); + const childApp = mkDeep(parentGsd, 'child-app'); + fs.mkdirSync(path.join(childApp, '.git'), { recursive: true }); + const startDir = mkDeep(childApp, 'src'); + + const result = findProjectRoot(startDir); + assert.notStrictEqual(result, parentGsd, + 'must NOT cross child-app\'s .git boundary to resolve to the ancestor parent-gsd project'); + // The child repo has no .planning of its own, so resolution falls back to + // the startDir (or a path within child-app) — never the ancestor. + assert.ok( + result === startDir || result === childApp, + `expected to stay within the child repo (startDir or childApp fallback), got: ${result}`, + ); + }); + + test('#2843 negative-space: a co-located .git + .planning (normal single-repo) still resolves to the project root', () => { + // The normal case: .git and .planning at the SAME level. The caller's .git + // IS the project's .git, so the boundary check passes (no nested child repo). + fs.mkdirSync(path.join(tmpDir, '.git'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); + const nested = mkDeep(tmpDir, 'src', 'lib'); + + const result = findProjectRoot(nested); + assert.strictEqual(result, tmpDir, + 'a co-located .git + .planning (single-repo project) must still resolve to the project root'); + }); }); diff --git a/tests/registry-reviewer-parity.test.cjs b/tests/registry-reviewer-parity.test.cjs new file mode 100644 index 000000000..ab30573c6 --- /dev/null +++ b/tests/registry-reviewer-parity.test.cjs @@ -0,0 +1,153 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * tests/registry-reviewer-parity.test.cjs — regression coverage for issue + * #2904 ("reviewer" registry entry type). + * + * `scripts/registry-schema.cjs`'s reviewer vocabulary + * (`REVIEWER_LANE_TRANSPORTS`, `REVIEWER_EVIDENCE_CLASSES`, + * `REVIEWER_SLUG_RE`, `REVIEWER_FLAG_RE`) and + * `gsd-core/bin/lib/capability-validator.cjs`'s runtime reviewer-lane + * vocabulary (`VALID_LANE_TRANSPORTS`, `VALID_EVIDENCE_CLASSES`, + * `LANE_SLUG_RE`, `LANE_FLAG_RE`) are two independent, hand-written mirrors + * of the same underlying grammar — the registry is a third-party + * DISCOVERABILITY catalog (documentation-scoped), the capability-validator + * is the RUNTIME manifest validator that actually gates what a shipped + * `capabilities//capability.json`'s `reviewer` body may declare. Nothing + * imports one from the other (capability-validator.cjs's own header comment, + * `gsd-core/bin/lib/capability-validator.cjs:798-810`, explains why: the + * canonical descriptor `LANE_SLUG_RE` mirrors lives in + * `src/review-lane-descriptor.cts`, which compiles to gitignored build + * output that this committed plain `.cjs` cannot depend on before + * `npm run build:lib` has ever run) — so the two vocabularies can silently + * drift apart with no error to read: a registry entry that faithfully + * mirrors a real shipped lane (e.g. `lm_studio`, `llama_cpp`, `4o-mini` — + * all real slugs that a naive kebab-only grammar would reject) would look + * "strict but simply wrong" if the registry's copy of the slug grammar ever + * diverged from `LANE_SLUG_RE`. + * + * `capability-validator.cjs:807-810` states the byte-identical requirement + * explicitly: "A LEADING DIGIT IS PERMITTED. ... Keep the two grammars + * byte-identical." This file is that parity guard for the reviewer registry + * entry type, sibling in structure/intent to + * `tests/registry-axes-parity.test.cjs` (which pins `AXES`/`OPTIONAL_AXES` + * against `HOST_INTEGRATION_AXES`). + * + * Row 75 additionally reads every real, shipped `capabilities//capability.json` + * and asserts each one's `reviewer.slug` (where present) validates against + * `REVIEWER_SLUG_RE` — a reality check that the registry grammar isn't just + * parity-pinned against `capability-validator.cjs` in the abstract, but + * actually accepts every lane slug the repository ships today. This reads + * JSON DATA files (not source), so it does not trip `local/no-source-grep` + * and is not a source-grep-in-disguise — no `.cjs`/`.js`/`.ts` source file is + * ever `readFileSync`'d and string-matched in this file. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { + REVIEWER_LANE_TRANSPORTS, + REVIEWER_EVIDENCE_CLASSES, + REVIEWER_SLUG_RE, + REVIEWER_FLAG_RE, +} = require(path.join(__dirname, '..', 'scripts', 'registry-schema.cjs')); + +const { + VALID_LANE_TRANSPORTS, + VALID_EVIDENCE_CLASSES, + LANE_SLUG_RE, + LANE_FLAG_RE, +} = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'capability-validator.cjs')); + +// ─── Parity: transport / evidence-class vocabularies ─────────────────────── + +describe('registry-reviewer-parity: vocab set-equality vs capability-validator.cjs', () => { + test('registry transport vocab matches capability-validator', () => { + const registrySet = new Set(REVIEWER_LANE_TRANSPORTS); + assert.ok(registrySet.size > 0, 'expected REVIEWER_LANE_TRANSPORTS to be non-empty'); + assert.ok(VALID_LANE_TRANSPORTS.size > 0, 'expected VALID_LANE_TRANSPORTS to be non-empty'); + assert.deepEqual( + [...registrySet].sort(), + [...VALID_LANE_TRANSPORTS].sort(), + 'REVIEWER_LANE_TRANSPORTS must be set-equal to capability-validator.cjs VALID_LANE_TRANSPORTS', + ); + }); + + test('registry evidence-class vocab matches capability-validator', () => { + const registrySet = new Set(REVIEWER_EVIDENCE_CLASSES); + assert.ok(registrySet.size > 0, 'expected REVIEWER_EVIDENCE_CLASSES to be non-empty'); + assert.ok(VALID_EVIDENCE_CLASSES.size > 0, 'expected VALID_EVIDENCE_CLASSES to be non-empty'); + assert.deepEqual( + [...registrySet].sort(), + [...VALID_EVIDENCE_CLASSES].sort(), + 'REVIEWER_EVIDENCE_CLASSES must be set-equal to capability-validator.cjs VALID_EVIDENCE_CLASSES', + ); + }); +}); + +// ─── Parity: slug / flag grammar — byte-identical regexes ────────────────── + +describe('registry-reviewer-parity: grammar regexes are byte-identical to capability-validator.cjs', () => { + test('registry slug grammar is byte-identical to LANE_SLUG_RE', () => { + assert.equal( + REVIEWER_SLUG_RE.source, + LANE_SLUG_RE.source, + 'REVIEWER_SLUG_RE.source must equal LANE_SLUG_RE.source — "keep the two grammars byte-identical" (capability-validator.cjs:807-810)', + ); + assert.equal( + REVIEWER_SLUG_RE.flags, + LANE_SLUG_RE.flags, + 'REVIEWER_SLUG_RE.flags must equal LANE_SLUG_RE.flags', + ); + }); + + test('registry flag grammar is byte-identical to LANE_FLAG_RE', () => { + assert.equal( + REVIEWER_FLAG_RE.source, + LANE_FLAG_RE.source, + 'REVIEWER_FLAG_RE.source must equal LANE_FLAG_RE.source', + ); + assert.equal( + REVIEWER_FLAG_RE.flags, + LANE_FLAG_RE.flags, + 'REVIEWER_FLAG_RE.flags must equal LANE_FLAG_RE.flags', + ); + }); +}); + +// ─── Reality check: every shipped first-party lane slug validates ───────── + +describe('registry-reviewer-parity: every first-party lane slug is accepted by the registry schema', () => { + test('every first-party lane slug is accepted by the registry schema', () => { + const capabilitiesDir = path.join(__dirname, '..', 'capabilities'); + const capabilityDirs = fs.readdirSync(capabilitiesDir, { withFileTypes: true }).filter((d) => d.isDirectory()); + + const collectedSlugs = []; + for (const dirent of capabilityDirs) { + const capabilityJsonPath = path.join(capabilitiesDir, dirent.name, 'capability.json'); + if (!fs.existsSync(capabilityJsonPath)) continue; + const data = JSON.parse(fs.readFileSync(capabilityJsonPath, 'utf8')); + if (data && typeof data === 'object' && data.reviewer && typeof data.reviewer.slug === 'string') { + collectedSlugs.push({ id: dirent.name, slug: data.reviewer.slug }); + } + } + + // Sanity: the collected set must be non-empty, or the loop below would + // pass vacuously — a glob that silently matched nothing must fail loudly. + assert.ok( + collectedSlugs.length > 0, + 'expected at least one capabilities/*/capability.json with a reviewer.slug — found none', + ); + + for (const { id, slug } of collectedSlugs) { + assert.ok( + REVIEWER_SLUG_RE.test(slug), + `expected capabilities/${id}/capability.json reviewer.slug "${slug}" to match REVIEWER_SLUG_RE (${REVIEWER_SLUG_RE})`, + ); + } + }); +}); diff --git a/tests/registry-schema.test.cjs b/tests/registry-schema.test.cjs index 5ff3bb677..7e8bd3956 100644 --- a/tests/registry-schema.test.cjs +++ b/tests/registry-schema.test.cjs @@ -15,6 +15,12 @@ const { AXES_FREE_STRING, CAPABILITY_REQUIRED, EOS_REQUIRED, + REVIEWER_REQUIRED, + REVIEWER_LANE_TRANSPORTS, + REVIEWER_EVIDENCE_CLASSES, + REVIEWER_SECTION_MAX, + INTERACTION_STRING_MAX, + INTERACTION_ARRAY_MAX, isValidGsdRange, validateEntries, renderMarkdown, @@ -78,6 +84,32 @@ function validEosEntry() { }; } +function validReviewerEntry() { + return { + id: 'my-reviewer', + name: 'My Reviewer', + type: 'reviewer', + repo: 'octocat/my-reviewer', + description: 'Reviews GSD PRs for a specific concern.', + author: 'Octocat', + license: 'MIT', + enginesGsd: '>=1.6.0 <3.0.0', + install: 'gsd capability install https://github.com/octocat/my-reviewer.git#v1.0.0', + uninstall: 'gsd capability remove my-reviewer', + interactions: { + slug: 'my-reviewer', + flags: ['--my-reviewer'], + transport: 'spawn', + evidenceClass: 'source-grounded', + reviewsSection: 'My Reviewer', + requiresBinaries: [], + configKeys: [], + runtimeCompat: ['all'], + }, + discussion: 'https://github.com/octocat/my-reviewer/discussions/1', + }; +} + // ─── Vocabulary constants ─────────────────────────────────────────────────── describe('registry-schema: closed vocabulary constants', () => { @@ -349,6 +381,18 @@ describe('renderMarkdown', () => { const rendered = renderMarkdown([], { type: 'capability', sourceFile: 'capabilities.json' }); assert.match(rendered, /No entries yet/); }); + + test('an unknown registry type throws rather than silently rendering the capability page', () => { + assert.throws( + () => renderMarkdown([], { type: 'bogus-type', sourceFile: 'x.json' }), + { message: /bogus-type/ }, + ); + }); + + test('a recognized type still renders the capability page (guard is not unconditional)', () => { + const rendered = renderMarkdown([], { type: 'capability', sourceFile: 'capabilities.json' }); + assert.equal(rendered.split('\n')[2], '# GSD Community Capability Registry'); + }); }); // ─── isValidGsdRange ──────────────────────────────────────────────────────── @@ -627,3 +671,728 @@ describe('validateEntries: tightened discussion/license regexes', () => { assert.ok(!verdict.errors.some((e) => e.field === 'license')); }); }); + +// ─── reviewer entry type (#2904) ──────────────────────────────────────────── +// +// The describe blocks below cover the `reviewer` entry type: the +// REVIEWER_REQUIRED / REVIEWER_LANE_TRANSPORTS / REVIEWER_EVIDENCE_CLASSES / +// REVIEWER_SECTION_MAX vocabulary constants, `interactions` validation, and +// renderMarkdown's `type: 'reviewer'` output. They were authored FAILING-FIRST +// against the unmodified module, ahead of the implementation. See +// .gsd/phase/feat-2904-enh-registries-add-a-reviewer-entry-type/50-test-matrix.md. + +describe('registry-schema: reviewer vocabulary constants', () => { + test('REVIEWER_REQUIRED lists the 12 required reviewer entry fields', () => { + assert.deepEqual(REVIEWER_REQUIRED, [ + 'id', 'name', 'type', 'repo', 'description', 'author', 'license', + 'enginesGsd', 'install', 'uninstall', 'interactions', 'discussion', + ]); + }); + + test('reviewer vocab constants are non-empty frozen arrays', () => { + assert.deepEqual(REVIEWER_LANE_TRANSPORTS, ['spawn', 'openai-http']); + assert.deepEqual(REVIEWER_EVIDENCE_CLASSES, ['source-grounded', 'diff-only']); + assert.ok(Object.isFrozen(REVIEWER_LANE_TRANSPORTS), 'expected REVIEWER_LANE_TRANSPORTS to be frozen'); + assert.ok(Object.isFrozen(REVIEWER_EVIDENCE_CLASSES), 'expected REVIEWER_EVIDENCE_CLASSES to be frozen'); + assert.equal(REVIEWER_SECTION_MAX, 200); + }); +}); + +describe('validateEntries: reviewer — happy path', () => { + test('a fully-valid reviewer entry passes', () => { + const verdict = validateEntries([validReviewerEntry()], { type: 'reviewer' }); + assert.equal(verdict.ok, true); + assert.deepEqual(verdict.errors, []); + }); + + test('an empty reviewer array passes', () => { + const verdict = validateEntries([], { type: 'reviewer' }); + assert.equal(verdict.ok, true); + assert.deepEqual(verdict.errors, []); + }); +}); + +describe('validateEntries: reviewer — type dispatch', () => { + test('an unknown opts.type is a root error, not a silent capability validation', () => { + const verdict = validateEntries([validReviewerEntry()], { type: 'typo' }); + assert.equal(verdict.ok, false); + assert.deepEqual(verdict.errors, [{ index: -1, field: '(root)', reason: 'unknown registry type "typo"' }]); + }); + + test('a capability-typed entry in the reviewer catalog fails', () => { + const entry = validReviewerEntry(); + entry.type = 'capability'; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find((e) => e.field === 'type'); + assert.ok(err, `expected a type error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'type must be "reviewer"'); + }); + + test('an eos-only top-level field is rejected on a reviewer entry', () => { + const entry = validReviewerEntry(); + entry.protocolVersion = 1; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find((e) => e.field === 'protocolVersion'); + assert.ok(err, `expected a protocolVersion error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'unknown field'); + }); + + test('a non-array under a valid type still reports the array error', () => { + const verdict = validateEntries(null, { type: 'reviewer' }); + assert.equal(verdict.ok, false); + assert.deepEqual(verdict.errors, [{ index: -1, field: '(root)', reason: 'entries must be an array' }]); + }); +}); + +describe('validateEntries: reviewer — interactions required-key sweep', () => { + const REVIEWER_INTERACTIONS_KEYS = [ + 'slug', 'flags', 'transport', 'evidenceClass', 'reviewsSection', + 'requiresBinaries', 'configKeys', 'runtimeCompat', + ]; + + for (const key of REVIEWER_INTERACTIONS_KEYS) { + test(`interactions.${key} is individually required`, () => { + const entry = validReviewerEntry(); + delete entry.interactions[key]; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find((e) => e.field === `interactions.${key}`); + assert.ok(err, `expected interactions.${key} error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'missing required field'); + }); + } + + test('multiple missing interactions keys each report once', () => { + const entry = validReviewerEntry(); + delete entry.interactions.slug; + delete entry.interactions.flags; + delete entry.interactions.transport; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const missingErrors = verdict.errors.filter((e) => e.reason === 'missing required field'); + assert.equal(missingErrors.length, 3, `expected exactly 3 missing-field errors, got: ${JSON.stringify(verdict.errors)}`); + assert.deepEqual( + missingErrors.map((e) => e.field).sort(), + ['interactions.flags', 'interactions.slug', 'interactions.transport'], + ); + }); + + test('an absent interactions object reports once, not nine times', () => { + const entry = validReviewerEntry(); + delete entry.interactions; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + assert.equal(verdict.errors.filter((e) => e.field === 'interactions').length, 1); + assert.ok(verdict.errors.some((e) => e.field === 'interactions' && e.reason === 'missing required field')); + assert.equal( + verdict.errors.filter((e) => e.field.startsWith('interactions.')).length, + 0, + `expected no interactions.* sub-errors when interactions itself is absent, got: ${JSON.stringify(verdict.errors)}`, + ); + }); + + test('a non-object interactions is rejected before key checks', () => { + for (const bad of [null, [], 'x']) { + const entry = validReviewerEntry(); + entry.interactions = bad; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(bad)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions'); + assert.ok(err, `expected interactions error for ${JSON.stringify(bad)}`); + assert.equal(err.reason, 'interactions must be an object'); + } + }); + + test('capability-only interactions keys are rejected on a reviewer', () => { + const entry = validReviewerEntry(); + entry.interactions.loopExtensionPoints = ['execute:pre']; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find((e) => e.field === 'interactions.loopExtensionPoints'); + assert.ok(err, `expected an interactions.loopExtensionPoints error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'unknown field'); + }); + + test('manifest-body keys outside the 8 registry fields are rejected', () => { + for (const key of ['probe', 'invoke']) { + const entry = validReviewerEntry(); + entry.interactions[key] = {}; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected interactions.${key} to be rejected`); + const err = verdict.errors.find((e) => e.field === `interactions.${key}`); + assert.ok(err, `expected interactions.${key} error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'unknown field'); + } + }); + + test('an unknown key does not suppress the missing-key sweep', () => { + const entry = validReviewerEntry(); + entry.interactions.bogus = 'x'; + delete entry.interactions.slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const bogusErr = verdict.errors.find((e) => e.field === 'interactions.bogus'); + assert.ok(bogusErr); + assert.equal(bogusErr.reason, 'unknown field'); + const slugErr = verdict.errors.find((e) => e.field === 'interactions.slug'); + assert.ok(slugErr); + assert.equal(slugErr.reason, 'missing required field'); + }); +}); + +describe('validateEntries: reviewer — slug grammar', () => { + test('a kebab slug is valid', () => { + for (const slug of ['gemini', 'a']) { + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected "${slug}" valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('an underscored lane slug is valid', () => { + for (const slug of ['lm_studio', 'llama_cpp']) { + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected "${slug}" valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('a leading-digit lane slug is valid', () => { + const entry = validReviewerEntry(); + entry.interactions.slug = '4o-mini'; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected "4o-mini" valid, got: ${JSON.stringify(verdict.errors)}`); + }); + + test('a slug may not start with a separator', () => { + for (const slug of ['-lead', '_lead']) { + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected "${slug}" invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.slug'); + assert.ok(err, `expected interactions.slug error for "${slug}"`); + assert.equal(err.reason, 'must match the reviewer lane slug grammar'); + } + }); + + test('a slug outside the lane grammar is rejected', () => { + for (const slug of ['Upper', 'has space', 'dot.ted', '']) { + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected "${slug}" invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.slug'); + assert.ok(err, `expected interactions.slug error for "${slug}"`); + assert.equal(err.reason, 'must match the reviewer lane slug grammar'); + } + }); + + test('a non-string slug is rejected without throwing', () => { + for (const slug of [123, null]) { + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + let verdict; + assert.doesNotThrow(() => { + verdict = validateEntries([entry], { type: 'reviewer' }); + }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(slug)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.slug'); + assert.ok(err); + assert.equal(err.reason, 'must match the reviewer lane slug grammar'); + } + }); + + test('fast-check property: any lane-grammar slug is accepted', () => { + fc.assert( + fc.property( + fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789'.split('')), + fc.array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789_-'.split('')), { maxLength: 20 }), + (first, rest) => { + const slug = first + rest.join(''); + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected "${slug}" valid, got: ${JSON.stringify(verdict.errors)}`); + }, + ), + ); + }); + + test('fast-check property: an out-of-grammar character always rejects', () => { + fc.assert( + fc.property( + fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789'.split('')), + fc.array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789_-'.split('')), { maxLength: 10 }), + fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ .!@#$%^&*'.split('')), + fc.nat(10), + (first, rest, badChar, insertAt) => { + const base = first + rest.join(''); + const pos = Math.min(insertAt, base.length); + const slug = base.slice(0, pos) + badChar + base.slice(pos); + const entry = validReviewerEntry(); + entry.interactions.slug = slug; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected "${slug}" invalid`); + }, + ), + ); + }); +}); + +describe('validateEntries: reviewer — flags grammar', () => { + test('a single well-formed flag is valid', () => { + for (const flags of [['--gemini'], ['--a', '--b', '--c']]) { + const entry = validReviewerEntry(); + entry.interactions.flags = flags; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected ${JSON.stringify(flags)} valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('an empty flags array is rejected', () => { + const entry = validReviewerEntry(); + entry.interactions.flags = []; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find((e) => e.field === 'interactions.flags'); + assert.ok(err); + assert.equal(err.reason, 'must be a non-empty array of lane CLI flags'); + }); + + test('duplicate flags are accepted — the registry is a directory, not the runtime validator', () => { + const entry = validReviewerEntry(); + entry.interactions.flags = ['--gemini', '--gemini']; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected duplicate flags valid, got: ${JSON.stringify(verdict.errors)}`); + }); + + test('a flag must carry the double-dash prefix', () => { + for (const flags of [['gemini'], ['-g']]) { + const entry = validReviewerEntry(); + entry.interactions.flags = flags; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(flags)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.flags'); + assert.ok(err); + assert.equal(err.reason, 'must be a non-empty array of lane CLI flags'); + } + }); + + test('a flag outside the kebab flag grammar is rejected', () => { + for (const flags of [['--Gemini'], ['--lm_studio']]) { + const entry = validReviewerEntry(); + entry.interactions.flags = flags; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(flags)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.flags'); + assert.ok(err); + assert.equal(err.reason, 'must be a non-empty array of lane CLI flags'); + } + }); + + test('a non-array / non-string-element flags is rejected', () => { + for (const flags of [[1], '--gemini']) { + const entry = validReviewerEntry(); + entry.interactions.flags = flags; + let verdict; + assert.doesNotThrow(() => { + verdict = validateEntries([entry], { type: 'reviewer' }); + }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(flags)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.flags'); + assert.ok(err); + assert.equal(err.reason, 'must be a non-empty array of lane CLI flags'); + } + }); +}); + +describe('validateEntries: reviewer — transport / evidenceClass', () => { + test('each allowed transport is accepted', () => { + for (const transport of REVIEWER_LANE_TRANSPORTS) { + const entry = validReviewerEntry(); + entry.interactions.transport = transport; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected transport "${transport}" valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('an unknown transport is rejected', () => { + for (const transport of ['SPAWN', 'http', '', 1, null, ['spawn']]) { + const entry = validReviewerEntry(); + entry.interactions.transport = transport; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected transport ${JSON.stringify(transport)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.transport'); + assert.ok(err); + assert.equal(err.reason, 'must be one of the allowed lane transports'); + } + }); + + test('each allowed evidence class is accepted', () => { + for (const evidenceClass of REVIEWER_EVIDENCE_CLASSES) { + const entry = validReviewerEntry(); + entry.interactions.evidenceClass = evidenceClass; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected evidenceClass "${evidenceClass}" valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('an unknown evidence class is rejected', () => { + for (const evidenceClass of ['diff', 'Source-Grounded', 1, null, ['diff-only']]) { + const entry = validReviewerEntry(); + entry.interactions.evidenceClass = evidenceClass; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected evidenceClass ${JSON.stringify(evidenceClass)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.evidenceClass'); + assert.ok(err); + assert.equal(err.reason, 'must be one of the allowed evidence classes'); + } + }); +}); + +describe('validateEntries: reviewer — reviewsSection', () => { + test('a plain reviewsSection is valid', () => { + const entry = validReviewerEntry(); + entry.interactions.reviewsSection = 'Gemini'; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected valid, got: ${JSON.stringify(verdict.errors)}`); + }); + + test('reviewsSection at and just below the cap is valid', () => { + for (const len of [REVIEWER_SECTION_MAX - 1, REVIEWER_SECTION_MAX]) { + const entry = validReviewerEntry(); + entry.interactions.reviewsSection = 'x'.repeat(len); + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected len ${len} valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('reviewsSection above the cap is rejected', () => { + const entry = validReviewerEntry(); + entry.interactions.reviewsSection = 'x'.repeat(REVIEWER_SECTION_MAX + 1); + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false); + const err = verdict.errors.find( + (e) => e.field === 'interactions.reviewsSection' && new RegExp(`exceeds max length ${REVIEWER_SECTION_MAX}`).test(e.reason), + ); + assert.ok(err, `expected an exceeds-max-length error, got: ${JSON.stringify(verdict.errors)}`); + }); + + test('a blank reviewsSection is rejected', () => { + for (const reviewsSection of ['', ' ', 123, null]) { + const entry = validReviewerEntry(); + entry.interactions.reviewsSection = reviewsSection; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected ${JSON.stringify(reviewsSection)} invalid`); + const err = verdict.errors.find((e) => e.field === 'interactions.reviewsSection'); + assert.ok(err); + assert.equal(err.reason, 'must be a non-empty string'); + } + }); +}); + +describe('validateEntries: reviewer — may-be-empty arrays', () => { + test('the may-be-empty arrays accept []', () => { + const entry = validReviewerEntry(); + entry.interactions.requiresBinaries = []; + entry.interactions.configKeys = []; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected valid, got: ${JSON.stringify(verdict.errors)}`); + }); + + test('runtimeCompat accepts the all wildcard', () => { + for (const runtimeCompat of [['all'], []]) { + const entry = validReviewerEntry(); + entry.interactions.runtimeCompat = runtimeCompat; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, true, `expected ${JSON.stringify(runtimeCompat)} valid, got: ${JSON.stringify(verdict.errors)}`); + } + }); + + test('a non-string-array is rejected for each array field', () => { + for (const field of ['requiresBinaries', 'configKeys', 'runtimeCompat']) { + for (const bad of [[1], 'a', {}]) { + const entry = validReviewerEntry(); + entry.interactions[field] = bad; + const verdict = validateEntries([entry], { type: 'reviewer' }); + assert.equal(verdict.ok, false, `expected interactions.${field} = ${JSON.stringify(bad)} invalid`); + const err = verdict.errors.find((e) => e.field === `interactions.${field}`); + assert.ok(err, `expected interactions.${field} error, got: ${JSON.stringify(verdict.errors)}`); + assert.equal(err.reason, 'must be an array of strings'); + } + } + }); +}); + +// ─── renderMarkdown: reviewer registry ───────────────────────────────────── + +describe('renderMarkdown: reviewer registry', () => { + test('an empty reviewer catalog renders the reviewer heading, not the capability one', () => { + const rendered = renderMarkdown([], { type: 'reviewer', sourceFile: 'reviewers.json' }); + assert.match(rendered, /# GSD Reviewer Lane Registry/); + assert.ok(rendered.includes('_To add your reviewer lane, see the [registry README](./README.md)._')); + assert.match(rendered, /No entries yet/); + }); + + test('a reviewer entry renders its lane summary', () => { + const entry = validReviewerEntry(); + const rendered = renderMarkdown([entry], { type: 'reviewer', sourceFile: 'reviewers.json' }); + const { slug, flags, transport, evidenceClass, reviewsSection } = entry.interactions; + const expected = + `Lane: ${slug}; flags: ${flags.join(', ')}; transport: ${transport}; evidence: ${evidenceClass}; ` + + `REVIEWS.md section: ${reviewsSection}`; + const line = rendered.split('\n').find((l) => l.startsWith('- **Every interaction with GSD:** ')); + assert.ok(line, `expected the summary bullet line, got: ${rendered}`); + assert.ok(line.includes(expected), `expected summary to include "${expected}", got: ${line}`); + }); + + test('empty optional arrays are omitted from the rendered summary', () => { + const entry = validReviewerEntry(); + entry.interactions.requiresBinaries = []; + entry.interactions.configKeys = []; + const renderedEmpty = renderMarkdown([entry], { type: 'reviewer', sourceFile: 'reviewers.json' }); + assert.ok(!renderedEmpty.includes('requiresBinaries:')); + assert.ok(!renderedEmpty.includes('configKeys:')); + + entry.interactions.requiresBinaries = ['ffmpeg']; + entry.interactions.configKeys = ['review.foo']; + const renderedPopulated = renderMarkdown([entry], { type: 'reviewer', sourceFile: 'reviewers.json' }); + assert.ok(renderedPopulated.includes('requiresBinaries: ffmpeg')); + assert.ok(renderedPopulated.includes('configKeys: review.foo')); + }); + + test('reviewer rendering is deterministic regardless of input order', () => { + const a = validReviewerEntry(); + const b = { ...validReviewerEntry(), id: 'zzz-reviewer', name: 'ZZZ Reviewer' }; + const first = renderMarkdown([a, b], { type: 'reviewer', sourceFile: 'reviewers.json' }); + const second = renderMarkdown([b, a], { type: 'reviewer', sourceFile: 'reviewers.json' }); + assert.equal(first, second); + }); + + test('untrusted reviewer free text cannot break out of the table', () => { + const entry = validReviewerEntry(); + entry.description = 'Good stuff | ![x](https://evil/track.png) | text'; + entry.interactions.reviewsSection = 'Gemini | ```evil```