chore(#2798): declare the eleven reviewer lanes as manifest data (#2837)

* chore(#2798): declare the eleven reviewer lanes as manifest data

Phase 5a of epic #2782, delivering ADR-2782 D9 (roster half) and D3.

- Five reviewers GSD never installs into become lane-only role:reviewer
  capabilities with no runtime body, no runtimeCompat and no install surface:
  gemini, coderabbit, ollama, lm-studio, llama-cpp. Before this they had no
  descriptor at all and lived as a hardcoded NON_RUNTIME_REVIEWER_SLUGS tail,
  which is now deleted outright.
- The six hosts that are ALSO reviewers gain a reviewer body alongside their
  runtime body. Their runtime bodies are byte-identical to next -- verified per
  capability against the git blob, not asserted -- so no install behaviour moves.
- KNOWN_REVIEWER_SLUGS derives from declared bodies via an exported
  deriveReviewerSlugs(registry). hostBehaviors.reviewerCli survives as a derived
  legacy alias for one release; where a capability carries both, the body wins
  and the slug appears once. Alias removal is Phase 7 (#2801).

THE KEYSTONE: the roster is the SAME ELEVEN SLUGS as before -- antigravity,
claude, coderabbit, codex, cursor, gemini, llama_cpp, lm_studio, ollama,
opencode, qwen. This phase changes HOW the roster is derived, not WHO is in it,
and the test asserts that literal list rather than a count.

kimi-code is deliberately NOT declared here. It is net-new with no
invoke_reviewers leg, so declaring it now would make it selectable but not
invocable -- present in --all, selected, emitting an empty section for the whole
5a-to-5b window -- and would break Phase 1's parity assertion. It lands in 5b
alongside the iteration that can run it. Legacy kimi (the Python CLI) is not a
reviewer at all and gains nothing.

The highest-value test is declaredManifestLanesMatchThePhase1Descriptor: it
deep-compares all eleven declared bodies against REVIEWER_LANES field-by-field,
including probe and invoke sub-fields. All eleven are byte-identical, key order
included. The epic's premise is that the manifest and the core descriptor
describe the same lane with NO translation layer, and Phase 2's review already
caught one divergence that every other test missed.

Two ADR corrections folded in, as Phases 1-3 each did:

1. PHASE ORDER. The ADR runs Phase 4 (federated config) before 5a and #2798
   claims a dependency on 4. That is inverted and makes Phase 4 unsatisfiable:
   D9 assigns review.<host>_host to lane capabilities that do not exist until
   THIS phase creates them, and a federated config slice must live inside
   capabilities/<id>/capability.json. Real graph: Phase 2 -> 5a -> 4.
2. #2798's INVENTORY acceptance item is vacuous. The inventory catalogs
   bin/lib/*.cjs modules, not capability directories -- antigravity, opencode
   and qwen appear zero times in it -- and gen-inventory-manifest --check passes
   with the five new dirs and no edit.

Also corrected a stale line in Phase 2's own ADR amendment: it recorded the slug
pattern as /^[a-z][a-z0-9_-]*$/, but Phase 2's security review widened the
shipped pattern to /^[a-z0-9][a-z0-9_-]*$/ to match Phase 1's exported
LANE_SLUG_RE. The prose had not followed the code.

Closes #2798

* fix(#2798): catalogue reviewer capabilities in the generated matrix

The capability matrix rendered exactly two tables, feature and runtime, via
renderTable(caps, role) filtering on c.role === role. ADR-2782 D3 added a THIRD
role, so every role:"reviewer" capability was silently dropped from the
first-party catalogue.

The drift guard did not catch it, and could not: --check compares generated
output against the committed file, and both omitted the five lanes identically,
so it reported "up to date" while five shipped capabilities were invisible in
the one document that is supposed to list what ships. A guard blind to an entire
role is not guarding.

This phase is what exposed it -- it ships the first role:"reviewer"
capabilities -- so it is fixed here rather than deferred (CLAUDE.md: a defect
found while working is fixed in the current change, which overrides
one-concern-per-PR).

Verified red-before-green: with a lane row deleted from the matrix, --check now
exits 1; restored, it exits 0. Before this fix the lanes were absent entirely, so
there was nothing for the guard to compare.

Phase 6 (#2800) still owns enriching the matrix with lane-specific detail
(slug/flag/transport columns) and the locale parity gate. This is the narrower
fix: the capabilities APPEAR at all.

* fix(#2798): close two hardening gaps and record three limits durably

Isolated security review (5 targets, no blockers) reproduced two gaps in the new
deriveReviewerSlugs. Both are unreachable through the checked-in registry -- it is
generated, JSON-sourced and code-reviewed -- but the function is EXPORTED for
reuse and carries no other validation, so it must not depend on its caller.

- A whitespace-only slug passed the length>0 test verbatim and occupied a roster
  entry it could never match. Slugs are now trimmed before the emptiness test. A
  blank body correctly falls through to the legacy alias rather than DROPPING the
  lane, which would have been worse than the blank slug.
- KNOWN_REVIEWER_SLUGS is computed at require() time, so an uncaught throw there
  breaks import for EVERY consumer rather than degrading selection. It is now
  guarded, yielding an empty roster on a malformed registry. That is a visible
  degradation, not a silent one: under D4 an explicitly requested reviewer that
  is unavailable is an ERROR, so /gsd:review --claude against an empty roster
  fails loudly. This also removes an asymmetry -- the sibling capability-trust
  module documents its collectors as TOTAL and wraps them for exactly this reason.

Also records three findings that previously existed ONLY in squash-merged PR
bodies, which is not a durable record:

- ADR-2782 D5 gains an implementation note explaining why the resolved host is
  deliberately EXCLUDED from the disclosure signature. Rule 1 says consent binds
  the resolved host; the loader has no config resolver, so folding it in would
  make the loader and lifecycle compute different signatures for one manifest and
  re-prompt forever. The binding is split: signature covers the SHA-pinned
  manifest fields, the consent record stores the resolved host, and Phase 5b
  re-resolves at invocation -- which is where rule 4 already puts the check. A
  reader comparing rule 1 to the code would otherwise conclude it is unimplemented.
- CONTEXT.md's capability-trust entry still described THREE executable surfaces.
  Phase 3 added the fourth and made that false; corrected here, since it is drift
  this epic introduced rather than Phase 6's new-glossary-term work.
- stableJson documents the NaN/Infinity/undefined -> null signature collision and
  why it is unreachable (JSON grammar has no such literal, so JSON.parse throws
  first). Reachability rests entirely on the ingest path staying JSON.parse-only,
  so the note lives where someone would break it.

* chore(#2798): backfill changeset pr number to 2837
This commit is contained in:
Tom Boucher
2026-07-29 16:58:02 -04:00
committed by GitHub
parent 982e83f0f7
commit 6a9babda69
20 changed files with 1740 additions and 37 deletions

View File

@@ -0,0 +1,5 @@
---
type: Added
pr: 2837
---
**Reviewer lanes now ship as capability declarations** — the eleven cross-AI reviewer lanes are declared as manifest data instead of a half-derived, half-hardcoded roster. Five reviewers GSD never installs into (Gemini, CodeRabbit, Ollama, LM Studio, llama.cpp) become lane-only capabilities with no install surface, and the six hosts that are also reviewers gain a reviewer body alongside their runtime descriptor. `gsd capability list` shows the five new lanes. The roster itself is unchanged — the same eleven reviewers, derived rather than hardcoded — and `runtime.hostBehaviors.reviewerCli` keeps working for one release. (#2798)

View File

@@ -242,7 +242,7 @@ Issue #1459 user-owned consent seam (`gsd-core/bin/lib/capability-consent.cjs`,
Issue #1459 finding 4 shared cross-process lock primitive (`gsd-core/bin/lib/capability-lock.cjs`, generated from `src/capability-lock.cts`). Leaf module (`node:fs`/`node:path`/`node:os`/`node:crypto` + the ledger's bounded `readSmallRegularFile` + `shell-command-projection`'s `execTool` for the rare start-time shell-out). THE single hardened lockfile protocol shared by BOTH `capability-lifecycle` (the `.gsd/capabilities/.lock` mutation lock) and `capability-consent` (the consent-store `.consent.lock`) — extracted so the two locks cannot diverge (mirrors the shared-validator / shared bounded-reader lessons). Exports: `acquireLock(lockPath, opts?)` (O_EXCL create with a JSON `{token,pid,hostname,startTime,ts}` body; steal protocol binds age to the body's own `ts`, never stale-steals a VERIFIED-LIVE same-host holder — pid alive AND recorded start-time matches the pid's current start-time, defeating pid-reuse without ever stealing a live holder — and reclaims only a dead/unverifiable holder via the dead-pid fast path or the hard `LOCK_DEADMAN_MS` deadman; `opts.maxAttempts` raises the bounded retry budget and `opts.waitForFresh` makes a contended fresh/live holder be WAITED FOR rather than failed-fast so genuinely-racing consent writers serialize), `releaseLock(handle)` (token + inode owner-safe — never deletes a successor's lock), `getProcessStartTime`, and the `_setLockProbes`/`_resetLockProbes` test seams. Carries the #1462 lifecycle-lock invariants (process-start-time liveness, TOCTOU-safe pre-rename identity recheck, bounded iterative loop). Issue #1459 finding 4 shared cross-process lock primitive (`gsd-core/bin/lib/capability-lock.cjs`, generated from `src/capability-lock.cts`). Leaf module (`node:fs`/`node:path`/`node:os`/`node:crypto` + the ledger's bounded `readSmallRegularFile` + `shell-command-projection`'s `execTool` for the rare start-time shell-out). THE single hardened lockfile protocol shared by BOTH `capability-lifecycle` (the `.gsd/capabilities/.lock` mutation lock) and `capability-consent` (the consent-store `.consent.lock`) — extracted so the two locks cannot diverge (mirrors the shared-validator / shared bounded-reader lessons). Exports: `acquireLock(lockPath, opts?)` (O_EXCL create with a JSON `{token,pid,hostname,startTime,ts}` body; steal protocol binds age to the body's own `ts`, never stale-steals a VERIFIED-LIVE same-host holder — pid alive AND recorded start-time matches the pid's current start-time, defeating pid-reuse without ever stealing a live holder — and reclaims only a dead/unverifiable holder via the dead-pid fast path or the hard `LOCK_DEADMAN_MS` deadman; `opts.maxAttempts` raises the bounded retry budget and `opts.waitForFresh` makes a contended fresh/live holder be WAITED FOR rather than failed-fast so genuinely-racing consent writers serialize), `releaseLock(handle)` (token + inode owner-safe — never deletes a successor's lock), `getProcessStartTime`, and the `_setLockProbes`/`_resetLockProbes` test seams. Carries the #1462 lifecycle-lock invariants (process-start-time liveness, TOCTOU-safe pre-rename identity recheck, bounded iterative loop).
### Capability Trust Gate ### Capability Trust Gate
ADR-1244 Phase 4 (D5) PURE policy module (`gsd-core/bin/lib/capability-trust.cjs`). Computes *what* a capability would do and *whether* policy permits it; performs no mutation and no I/O beyond existence-checking declared artifacts. Exports: `discloseExecutableSurfaces(manifest, stagedDir?)` (enumerates the three executable surfaces — `hooks`, command modules, `mcpServers` — and flags `hasExecutable`); `evaluateInstallTrust(args)` (composes source policy + reserved-namespace + engines gate + disclosure into `{ allowed, requiresConsent, disclosure, engines, blockReasons }`); `evaluateSourceAllowed(parsed, strictKnownRegistries)` enforcing `capabilities.strict_known_registries` (unset/null → permissive-with-consent; `[]` → block all external; non-empty → host-based allowlist, never substring); `checkEngines(manifest, hostVersion)` (engines.gsd hard gate via `semverSatisfies` + `compatVersions` graceful-downgrade picking the newest working version); `executableSetChanged(old, new)` (auto-update re-consent trigger); `checkReservedNamespace` (`gsd-`/`gsd-core-`/`anthropic-`). The MCP disclosure also captures each server's `env` (string→string, filtered) and `cwd` (#1459) — `disclosureSignature` folds them in as STABLE SORTED JSON so any env/cwd add/change forces re-consent while a key reorder does not; `signatureForManifest(manifest, stagedDir?)` is the single source of truth for that signature (consumed by the loader's consent check and the lifecycle's consent binding). #1459 finding 5: each MCP surface also carries `rawConfig` — the FULL declared server config the writer persists (`{...config}`), prototype-pollution-cleaned — folded into the signature as STABLE SORTED JSON so a change to ANY persisted field (not just the explicit whitelist — a future `envFile`/`workingDir`/launch option) forces re-consent, while a pure key reorder does not; the human summary stays readable via the key fields only. The barrier is consent + integrity + reversibility, NOT a sandbox — see `docs/explanation/capability-trust-model.md`. ADR-1244 Phase 4 (D5) PURE policy module (`gsd-core/bin/lib/capability-trust.cjs`). Computes *what* a capability would do and *whether* policy permits it; performs no mutation and no I/O beyond existence-checking declared artifacts. Exports: `discloseExecutableSurfaces(manifest, stagedDir?, resolveHost?)` (enumerates the four executable surfaces — `hooks`, command modules, `mcpServers`, and reviewer lanes (ADR-2782 D5) — and flags `hasExecutable`; a reviewer lane is the one class that *receives* data, so it discloses its binary + full args (spawn) or destination host + `hostConfigKey` (openai-http) together with the egress payload classes); `evaluateInstallTrust(args)` (composes source policy + reserved-namespace + engines gate + disclosure into `{ allowed, requiresConsent, disclosure, engines, blockReasons }`); `evaluateSourceAllowed(parsed, strictKnownRegistries)` enforcing `capabilities.strict_known_registries` (unset/null → permissive-with-consent; `[]` → block all external; non-empty → host-based allowlist, never substring); `checkEngines(manifest, hostVersion)` (engines.gsd hard gate via `semverSatisfies` + `compatVersions` graceful-downgrade picking the newest working version); `executableSetChanged(old, new)` (auto-update re-consent trigger); `checkReservedNamespace` (`gsd-`/`gsd-core-`/`anthropic-`). The MCP disclosure also captures each server's `env` (string→string, filtered) and `cwd` (#1459) — `disclosureSignature` folds them in as STABLE SORTED JSON so any env/cwd add/change forces re-consent while a key reorder does not; `signatureForManifest(manifest, stagedDir?)` is the single source of truth for that signature (consumed by the loader's consent check and the lifecycle's consent binding). #1459 finding 5: each MCP surface also carries `rawConfig` — the FULL declared server config the writer persists (`{...config}`), prototype-pollution-cleaned — folded into the signature as STABLE SORTED JSON so a change to ANY persisted field (not just the explicit whitelist — a future `envFile`/`workingDir`/launch option) forces re-consent, while a pure key reorder does not; the human summary stays readable via the key fields only. The barrier is consent + integrity + reversibility, NOT a sandbox — see `docs/explanation/capability-trust-model.md`.
### Capability Lifecycle ### Capability Lifecycle
ADR-1244 Phase 4 (D5+D6) orchestration seam (`gsd-core/bin/lib/capability-lifecycle.cjs`) composing the source resolver, ledger, and trust gate into the mutating operations. Exports: `installCapability` (pre-fetch source gate → resolve copy-only with `promote:false` → trust verdict → promote + apply marker-stamped shared edits → **ledger commit**; nothing written on block/abort), `upgradeCapability` (atomic stage-then-swap: old set aside, new swapped in, shared edits re-derived, **ledger committed**, backup dropped; re-prompts when the executable set changed), `removeCapability` (strip only `_gsdCapability`-marked shared-config entries — user hand-edits preserved — delete exactly the ledger-recorded files, then drop the entry; `CAPABILITY_DATA` preserved unless `removeData`), `reconcileCapabilities` (crash recovery driven by the ledger's `_pending {kind,backupName,sharedFiles}` INTENT — not a version comparison: roll an uncommitted upgrade back by restoring the backup, an uncommitted fresh install away entirely, and re-sync shared config from the winning bundle, guaranteeing no half-state), plus `applyCapabilitySharedEdits`/`stripCapabilitySharedEdits` (marker-isolated JSON edits, prototype-pollution-guarded). All four mutating ops + reconcile take a cross-process lock (`.gsd/capabilities/.lock`, atomic stale-steal) so a concurrent reconcile can't clear a live intent. Capability code never executes during any operation. The source resolver's `promote:false`/`skipEnginesGate` options are the seams that let this module own the swap/commit ordering and the engines gate (with `compatVersions` downgrade hint). ADR-1244 Phase 4 (D5+D6) orchestration seam (`gsd-core/bin/lib/capability-lifecycle.cjs`) composing the source resolver, ledger, and trust gate into the mutating operations. Exports: `installCapability` (pre-fetch source gate → resolve copy-only with `promote:false` → trust verdict → promote + apply marker-stamped shared edits → **ledger commit**; nothing written on block/abort), `upgradeCapability` (atomic stage-then-swap: old set aside, new swapped in, shared edits re-derived, **ledger committed**, backup dropped; re-prompts when the executable set changed), `removeCapability` (strip only `_gsdCapability`-marked shared-config entries — user hand-edits preserved — delete exactly the ledger-recorded files, then drop the entry; `CAPABILITY_DATA` preserved unless `removeData`), `reconcileCapabilities` (crash recovery driven by the ledger's `_pending {kind,backupName,sharedFiles}` INTENT — not a version comparison: roll an uncommitted upgrade back by restoring the backup, an uncommitted fresh install away entirely, and re-sync shared config from the winning bundle, guaranteeing no half-state), plus `applyCapabilitySharedEdits`/`stripCapabilitySharedEdits` (marker-isolated JSON edits, prototype-pollution-guarded). All four mutating ops + reconcile take a cross-process lock (`.gsd/capabilities/.lock`, atomic stale-steal) so a concurrent reconcile can't clear a live intent. Capability code never executes during any operation. The source resolver's `promote:false`/`skipEnginesGate` options are the seams that let this module own the swap/commit ordering and the engines gate (with `compatVersions` downgrade hint).

View File

@@ -99,5 +99,26 @@
"hookPathStyle": "raw", "hookPathStyle": "raw",
"globalDirResolver": "antigravity" "globalDirResolver": "antigravity"
} }
},
"reviewer": {
"slug": "antigravity",
"flags": ["--antigravity", "--agy"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "agy" },
"invoke": {
"binary": "agy",
"args": ["--print-timeout", "540s", "-p"],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "none"
},
"timeoutFloorMs": 600000,
"emptyOutput": "handler-owned",
"reviewsSection": "Antigravity",
"evidenceClass": "source-grounded",
"requiresBinaries": ["jq"],
"promptBudgetKey": null,
"handler": "antigravity"
} }
} }

View File

@@ -104,5 +104,26 @@
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "claude",
"flags": ["--claude"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "claude" },
"invoke": {
"binary": "claude",
"args": ["-p", "-"],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Claude",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
} }

View File

@@ -0,0 +1,33 @@
{
"id": "coderabbit",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "coderabbit",
"flags": ["--coderabbit"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "coderabbit" },
"invoke": {
"binary": "coderabbit",
"args": ["review", "--prompt-only"],
"promptChannel": "none",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 360000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "CodeRabbit",
"evidenceClass": "diff-only",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
}
}

View File

@@ -89,5 +89,27 @@
"frontmatterDialect": "codex", "frontmatterDialect": "codex",
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "codex",
"flags": ["--codex"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "codex" },
"invoke": {
"binary": "codex",
"args": ["exec", "--ephemeral", "--skip-git-repo-check", "-"],
"promptChannel": "stdin",
"outputChannel": "file-arg",
"outputArg": "-o",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Codex",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
} }

View File

@@ -120,5 +120,26 @@
], ],
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "cursor",
"flags": ["--cursor"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "cursor-agent" },
"invoke": {
"binary": "cursor-agent",
"args": ["-p", "--mode", "ask", "--trust", "--output-format", "text"],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Cursor",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
} }

View File

@@ -0,0 +1,33 @@
{
"id": "gemini",
"role": "reviewer",
"version": "1.8.0",
"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 <model>` with the plan piped on stdin.",
"tier": "full",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "gemini",
"flags": ["--gemini"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "gemini" },
"invoke": {
"binary": "gemini",
"args": ["-p", "-"],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "-m",
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Gemini",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
}
}

View File

@@ -0,0 +1,36 @@
{
"id": "llama-cpp",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "llama_cpp",
"flags": ["--llama-cpp"],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.llama_cpp_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.llama_cpp_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "llama.cpp",
"evidenceClass": "source-grounded",
"requiresBinaries": ["jq"],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.llama_cpp",
"handler": "openai-compatible"
}
}

View File

@@ -0,0 +1,36 @@
{
"id": "lm-studio",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "lm_studio",
"flags": ["--lm-studio"],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.lm_studio_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.lm_studio_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "LM Studio",
"evidenceClass": "source-grounded",
"requiresBinaries": ["jq"],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.lm_studio",
"handler": "openai-compatible"
}
}

View File

@@ -0,0 +1,36 @@
{
"id": "ollama",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "ollama",
"flags": ["--ollama"],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.ollama_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.ollama_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Ollama",
"evidenceClass": "source-grounded",
"requiresBinaries": ["jq"],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.ollama",
"handler": "openai-compatible"
}
}

View File

@@ -110,5 +110,26 @@
"skipCodexSkillsManifest": true, "skipCodexSkillsManifest": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "opencode",
"flags": ["--opencode"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "opencode" },
"invoke": {
"binary": "opencode",
"args": ["run", "--format", "json", "-"],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 660000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "OpenCode",
"evidenceClass": "source-grounded",
"requiresBinaries": ["jq"],
"promptBudgetKey": null,
"handler": null
} }
} }

View File

@@ -103,5 +103,26 @@
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "qwen",
"flags": ["--qwen"],
"transport": "spawn",
"probe": { "kind": "command-exists", "binary": "qwen" },
"invoke": {
"binary": "qwen",
"args": ["-"],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Qwen",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
} }

View File

@@ -330,6 +330,32 @@ Therefore, normatively:
A host change is a change of *who receives the user's plans*. It is the most security-relevant A host change is a change of *who receives the user's plans*. It is the most security-relevant
mutation in this design, and it must not be reachable by editing a JSON file. mutation in this design, and it must not be reachable by editing a JSON file.
> **Implementation note added by Phase 3 (#2796) — how rule 1 is actually satisfied.**
>
> The resolved host is **deliberately excluded from the disclosure signature**, and a reader
> comparing rule 1 to `capability-trust.cts` must not mistake that for the rule being unimplemented.
>
> `signatureForManifest(manifest, stagedDir?)` is the single consent key that **both** the loader and
> the lifecycle compute, explicitly so the two "can never drift". The loader has **no config
> resolver** — `hostConfigKey` names a key in `.planning/config.json`, which is outside the SHA-pinned
> bundle. Folding the resolved host into the signature would therefore make the loader and the
> lifecycle compute *different* signatures for the same manifest, producing a permanent
> false-mismatch loop that re-prompts forever.
>
> So the binding is split, and rule 1 still holds end to end:
> - the **signature** binds the manifest-derived lane fields (`slug`, `transport`, `binary`, `args`,
> `hostConfigKey`, `promptChannel`, `handler`) — everything that is SHA-pinned;
> - the **consent record** additionally stores the resolved host, which is what rule 1 requires;
> - **Phase 5b re-resolves and compares at invocation** and blocks on mismatch, which is where rule 4
> already places the check.
>
> `reviewsSection` and `timeoutFloorMs` are excluded for a different reason: a cosmetic change must
> not force re-consent, because a prompt carrying no security information is how users learn to click
> through — the same failure this decision cites when rejecting a per-run egress prompt.
>
> *(Recorded here rather than in the PR that made the decision. A squash-merged PR body is not a
> durable record: it is invisible to anyone reading the ADR later, which is exactly who needs this.)*
**Stated honestly, and consistent with ADR-1244 D5's own acknowledgment that there is no sandbox:** **Stated honestly, and consistent with ADR-1244 D5's own acknowledgment that there is no sandbox:**
even with the above, consent-at-install remains a weaker gate for a *standing egress channel* than even with the above, consent-at-install remains a weaker gate for a *standing egress channel* than
for a hook. A user consents once; the lane thereafter receives every plan on every review run. for a hook. A user consents once; the lane thereafter receives every plan on every review run.
@@ -659,6 +685,41 @@ lane and they are deliberately not the same string:
| `reviewer.slug` | `lm_studio` | snake | the shipped roster and `review.lm_studio_host`, which D9 leaves unchanged | | `reviewer.slug` | `lm_studio` | snake | the shipped roster and `review.lm_studio_host`, which D9 leaves unchanged |
| `reviewer.flags` | `--lm-studio` | kebab | the shipped flag | | `reviewer.flags` | `--lm-studio` | kebab | the shipped flag |
Phase 2 therefore validates `reviewer.slug` against its own pattern (`/^[a-z][a-z0-9_-]*$/`) rather Phase 2 therefore validates `reviewer.slug` against its own pattern rather than reusing `KEBAB_RE`,
than reusing `KEBAB_RE`, which would have rejected two shipped lanes. Phase 5a must create which would have rejected two shipped lanes. Phase 5a must create `capabilities/lm-studio/` and
`capabilities/lm-studio/` and `capabilities/llama-cpp/`, each declaring the snake-case slug. `capabilities/llama-cpp/`, each declaring the snake-case slug.
*(Corrected 2026-07-29: this paragraph first recorded the pattern as `/^[a-z][a-z0-9_-]*$/`. Phase 2's
own security review caught that as a divergence from Phase 1's exported `LANE_SLUG_RE`, which permits
a leading digit — a model-named lane such as `4o-mini` would have been accepted by the core descriptor
and rejected by the manifest validator, reintroducing exactly the translation layer this epic deletes.
The shipped pattern is `/^[a-z0-9][a-z0-9_-]*$/`, and a parity assertion now fails if the two ever
drift again.)*
### 2026-07-29 — Phase 4 and Phase 5a are swapped (ordering correction from Phase 5a)
**The phase table above runs Phase 4 (federated config) before Phase 5a (lane declarations), and
#2798 states "Depends on Phases 2 and 4". That ordering is inverted, and it makes Phase 4
unsatisfiable.**
D9's ownership table assigns `review.ollama_host` to the `ollama` lane, `review.lm_studio_host` to
`lm_studio`, and `review.llama_cpp_host` to `llama_cpp`. A federated `config` slice lives inside a
`capabilities/<id>/capability.json` — and **those capability directories do not exist until Phase 5a
creates them**. Verified before the swap: `capabilities/{ollama,lm_studio,llama_cpp,gemini,coderabbit}`
were all absent, and only the six `reviewerCli`-flagged runtime capabilities existed.
So in the stated order Phase 4 has nowhere to put three of its five key families, and its own "Done
when" — *"`review.<host>_host` owned by lane capabilities"* — cannot be met. Shipping it unmet would
be a failed deployment under `CI.GATE.acceptance-criteria-required`.
5a's stated dependency on Phase 4 is likewise unfounded: declaring a `reviewer` body requires only the
manifest vocabulary from Phase 2. **The real dependency graph is `Phase 2 → 5a → 4`**, with
`5a → 5b → 6` unchanged. Nothing about either phase's *content* changes — only their order.
**A second correction, to #2798's acceptance list.** It requires "`docs/INVENTORY.md` updated +
`node scripts/gen-inventory-manifest.cjs --write` run **after** `build:lib`". That rests on a false
premise: the inventory catalogs `bin/lib/*.cjs` **modules**, not capability directories —
`antigravity`, `opencode` and `qwen` appear zero times in it, and `INVENTORY-MANIFEST.json`'s six
families contain no `capabilities/` entry at all. `gen-inventory-manifest.cjs --check` passes with the
five new capability directories added and no inventory edit. The item is vacuous for this phase, and
inventing an edit to satisfy it would introduce drift rather than prevent it.

View File

@@ -102,6 +102,29 @@ emission), so their extension-point and hook-kind cells are `—`.
| `windsurf` | runtime | core | `>=1.6.0` | — | — | first-party | | `windsurf` | runtime | core | `>=1.6.0` | — | — | first-party |
| `zcode` | runtime | core | `>=1.6.0` | — | — | first-party | | `zcode` | runtime | core | `>=1.6.0` | — | — | first-party |
### Reviewer capabilities (role: reviewer) — 5
Reviewer capabilities declare a cross-AI **reviewer lane** — one external CLI or
model endpoint `/gsd:review` hands a plan to (ADR-2782 D3). They are not install
targets: they emit no skills, agents, hooks or surface files, so their
extension-point and hook-kind cells are `—`. A host that is *also* a reviewer
(Claude, Codex, Cursor, OpenCode, Qwen, Antigravity) keeps one manifest and
appears under **runtime** above, carrying its lane alongside its runtime body;
only lanes that GSD never installs into appear here.
Because a lane receives the plan text, requirements, research findings and
`CONTEXT.md` decisions, it is a disclosed executable surface and is consent-gated
at install like any other — see
[the trust model](../explanation/capability-trust-model.md).
| id | role | tier | engines.gsd | extension points | hook kinds | source |
|---|---|---|---|---|---|---|
| `coderabbit` | reviewer | full | `>=1.8.0` | — | — | first-party |
| `gemini` | reviewer | full | `>=1.8.0` | — | — | first-party |
| `llama-cpp` | reviewer | full | `>=1.8.0` | — | — | first-party |
| `lm-studio` | reviewer | full | `>=1.8.0` | — | — | first-party |
| `ollama` | reviewer | full | `>=1.8.0` | — | — | first-party |
--- ---
## Third-party capabilities ## Third-party capabilities

View File

@@ -193,6 +193,39 @@ const capabilities = {
"hookPathStyle": "raw", "hookPathStyle": "raw",
"globalDirResolver": "antigravity" "globalDirResolver": "antigravity"
} }
},
"reviewer": {
"slug": "antigravity",
"flags": [
"--antigravity",
"--agy"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "agy"
},
"invoke": {
"binary": "agy",
"args": [
"--print-timeout",
"540s",
"-p"
],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "none"
},
"timeoutFloorMs": 600000,
"emptyOutput": "handler-owned",
"reviewsSection": "Antigravity",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": null,
"handler": "antigravity"
} }
}, },
"assumption-delta": { "assumption-delta": {
@@ -539,6 +572,35 @@ const capabilities = {
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "claude",
"flags": [
"--claude"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "claude"
},
"invoke": {
"binary": "claude",
"args": [
"-p",
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Claude",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"claude-orchestration": { "claude-orchestration": {
@@ -874,6 +936,47 @@ const capabilities = {
} }
} }
}, },
"coderabbit": {
"id": "coderabbit",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "coderabbit",
"flags": [
"--coderabbit"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "coderabbit"
},
"invoke": {
"binary": "coderabbit",
"args": [
"review",
"--prompt-only"
],
"promptChannel": "none",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 360000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "CodeRabbit",
"evidenceClass": "diff-only",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
}
},
"codex": { "codex": {
"id": "codex", "id": "codex",
"role": "runtime", "role": "runtime",
@@ -967,6 +1070,38 @@ const capabilities = {
"frontmatterDialect": "codex", "frontmatterDialect": "codex",
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "codex",
"flags": [
"--codex"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "codex"
},
"invoke": {
"binary": "codex",
"args": [
"exec",
"--ephemeral",
"--skip-git-repo-check",
"-"
],
"promptChannel": "stdin",
"outputChannel": "file-arg",
"outputArg": "-o",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Codex",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"copilot": { "copilot": {
@@ -1186,6 +1321,39 @@ const capabilities = {
], ],
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "cursor",
"flags": [
"--cursor"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "cursor-agent"
},
"invoke": {
"binary": "cursor-agent",
"args": [
"-p",
"--mode",
"ask",
"--trust",
"--output-format",
"text"
],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Cursor",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"drift": { "drift": {
@@ -1390,6 +1558,47 @@ const capabilities = {
} }
] ]
}, },
"gemini": {
"id": "gemini",
"role": "reviewer",
"version": "1.8.0",
"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 <model>` with the plan piped on stdin.",
"tier": "full",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "gemini",
"flags": [
"--gemini"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "gemini"
},
"invoke": {
"binary": "gemini",
"args": [
"-p",
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "-m",
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Gemini",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
}
},
"graphify": { "graphify": {
"id": "graphify", "id": "graphify",
"role": "feature", "role": "feature",
@@ -1873,6 +2082,86 @@ const capabilities = {
} }
} }
}, },
"llama-cpp": {
"id": "llama-cpp",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "llama_cpp",
"flags": [
"--llama-cpp"
],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.llama_cpp_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.llama_cpp_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "llama.cpp",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.llama_cpp",
"handler": "openai-compatible"
}
},
"lm-studio": {
"id": "lm-studio",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "lm_studio",
"flags": [
"--lm-studio"
],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.lm_studio_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.lm_studio_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "LM Studio",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.lm_studio",
"handler": "openai-compatible"
}
},
"mempalace": { "mempalace": {
"id": "mempalace", "id": "mempalace",
"role": "feature", "role": "feature",
@@ -2097,6 +2386,46 @@ const capabilities = {
"contributions": [], "contributions": [],
"gates": [] "gates": []
}, },
"ollama": {
"id": "ollama",
"role": "reviewer",
"version": "1.8.0",
"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",
"requires": [],
"engines": {
"gsd": ">=1.8.0"
},
"reviewer": {
"slug": "ollama",
"flags": [
"--ollama"
],
"transport": "openai-http",
"probe": {
"kind": "http-reachable",
"hostConfigKey": "review.ollama_host",
"path": "/v1/models",
"timeoutMs": 2000
},
"invoke": {
"hostConfigKey": "review.ollama_host",
"path": "/v1/chat/completions",
"modelDiscovery": "first-from-models-endpoint",
"effortChannel": "none"
},
"timeoutFloorMs": 120000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Ollama",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": "review.max_prompt_tokens_per_reviewer.ollama",
"handler": "openai-compatible"
}
},
"opencode": { "opencode": {
"id": "opencode", "id": "opencode",
"role": "runtime", "role": "runtime",
@@ -2211,6 +2540,39 @@ const capabilities = {
"skipCodexSkillsManifest": true, "skipCodexSkillsManifest": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "opencode",
"flags": [
"--opencode"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "opencode"
},
"invoke": {
"binary": "opencode",
"args": [
"run",
"--format",
"json",
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 660000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "OpenCode",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": null,
"handler": null
} }
}, },
"pattern-mapper": { "pattern-mapper": {
@@ -2511,6 +2873,34 @@ const capabilities = {
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "qwen",
"flags": [
"--qwen"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "qwen"
},
"invoke": {
"binary": "qwen",
"args": [
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Qwen",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"research": { "research": {
@@ -4227,6 +4617,39 @@ const runtimes = {
"hookPathStyle": "raw", "hookPathStyle": "raw",
"globalDirResolver": "antigravity" "globalDirResolver": "antigravity"
} }
},
"reviewer": {
"slug": "antigravity",
"flags": [
"--antigravity",
"--agy"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "agy"
},
"invoke": {
"binary": "agy",
"args": [
"--print-timeout",
"540s",
"-p"
],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "none"
},
"timeoutFloorMs": 600000,
"emptyOutput": "handler-owned",
"reviewsSection": "Antigravity",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": null,
"handler": "antigravity"
} }
}, },
"augment": { "augment": {
@@ -4444,6 +4867,35 @@ const runtimes = {
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "claude",
"flags": [
"--claude"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "claude"
},
"invoke": {
"binary": "claude",
"args": [
"-p",
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Claude",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"cline": { "cline": {
@@ -4723,6 +5175,38 @@ const runtimes = {
"frontmatterDialect": "codex", "frontmatterDialect": "codex",
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "codex",
"flags": [
"--codex"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "codex"
},
"invoke": {
"binary": "codex",
"args": [
"exec",
"--ephemeral",
"--skip-git-repo-check",
"-"
],
"promptChannel": "stdin",
"outputChannel": "file-arg",
"outputArg": "-o",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 1200000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Codex",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"copilot": { "copilot": {
@@ -4942,6 +5426,39 @@ const runtimes = {
], ],
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "cursor",
"flags": [
"--cursor"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "cursor-agent"
},
"invoke": {
"binary": "cursor-agent",
"args": [
"-p",
"--mode",
"ask",
"--trust",
"--output-format",
"text"
],
"promptChannel": "argv-file-ref",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Cursor",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"hermes": { "hermes": {
@@ -5448,6 +5965,39 @@ const runtimes = {
"skipCodexSkillsManifest": true, "skipCodexSkillsManifest": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "opencode",
"flags": [
"--opencode"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "opencode"
},
"invoke": {
"binary": "opencode",
"args": [
"run",
"--format",
"json",
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": "--model",
"effortChannel": "argv"
},
"timeoutFloorMs": 660000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "OpenCode",
"evidenceClass": "source-grounded",
"requiresBinaries": [
"jq"
],
"promptBudgetKey": null,
"handler": null
} }
}, },
"pi": { "pi": {
@@ -5617,6 +6167,34 @@ const runtimes = {
"hyphenNameAgentBody": true, "hyphenNameAgentBody": true,
"reviewerCli": true "reviewerCli": true
} }
},
"reviewer": {
"slug": "qwen",
"flags": [
"--qwen"
],
"transport": "spawn",
"probe": {
"kind": "command-exists",
"binary": "qwen"
},
"invoke": {
"binary": "qwen",
"args": [
"-"
],
"promptChannel": "stdin",
"outputChannel": "stdout",
"modelArg": null,
"effortChannel": "none"
},
"timeoutFloorMs": 900000,
"emptyOutput": "stub-with-stderr",
"reviewsSection": "Qwen",
"evidenceClass": "source-grounded",
"requiresBinaries": [],
"promptBudgetKey": null,
"handler": null
} }
}, },
"trae": { "trae": {
@@ -6120,20 +6698,25 @@ const _requiresGraph = {
"cline": [], "cline": [],
"code-review": [], "code-review": [],
"codebuddy": [], "codebuddy": [],
"coderabbit": [],
"codex": [], "codex": [],
"copilot": [], "copilot": [],
"cursor": [], "cursor": [],
"drift": [], "drift": [],
"external-job": [], "external-job": [],
"gap-analysis": [], "gap-analysis": [],
"gemini": [],
"graphify": [], "graphify": [],
"hermes": [], "hermes": [],
"intel": [], "intel": [],
"kilo": [], "kilo": [],
"kimi": [], "kimi": [],
"kimi-code": [], "kimi-code": [],
"llama-cpp": [],
"lm-studio": [],
"mempalace": [], "mempalace": [],
"nyquist": [], "nyquist": [],
"ollama": [],
"opencode": [], "opencode": [],
"pattern-mapper": [ "pattern-mapper": [
"research" "research"

View File

@@ -114,8 +114,15 @@ function buildMatrix(registry) {
const extByCap = extensionsByCapability(registry); const extByCap = extensionsByCapability(registry);
const featureTable = renderTable(caps, 'feature', extByCap); const featureTable = renderTable(caps, 'feature', extByCap);
const runtimeTable = renderTable(caps, 'runtime', extByCap); const runtimeTable = renderTable(caps, 'runtime', extByCap);
// ADR-2782 D3 added a third role. Rendering only feature+runtime silently
// DROPPED every role:"reviewer" capability from the catalogue — and because
// `--check` compares generated output against the committed file, both omitted
// them identically, so the drift guard reported "up to date" while five shipped
// capabilities were invisible. A guard blind to a whole role is not guarding.
const reviewerTable = renderTable(caps, 'reviewer', extByCap);
const featureCount = caps.filter((c) => c.role === 'feature').length; const featureCount = caps.filter((c) => c.role === 'feature').length;
const runtimeCount = caps.filter((c) => c.role === 'runtime').length; const runtimeCount = caps.filter((c) => c.role === 'runtime').length;
const reviewerCount = caps.filter((c) => c.role === 'reviewer').length;
return `# Capability matrix reference return `# Capability matrix reference
@@ -180,6 +187,23 @@ emission), so their extension-point and hook-kind cells are \`—\`.
${runtimeTable} ${runtimeTable}
### Reviewer capabilities (role: reviewer) — ${reviewerCount}
Reviewer capabilities declare a cross-AI **reviewer lane** — one external CLI or
model endpoint \`/gsd:review\` hands a plan to (ADR-2782 D3). They are not install
targets: they emit no skills, agents, hooks or surface files, so their
extension-point and hook-kind cells are \`—\`. A host that is *also* a reviewer
(Claude, Codex, Cursor, OpenCode, Qwen, Antigravity) keeps one manifest and
appears under **runtime** above, carrying its lane alongside its runtime body;
only lanes that GSD never installs into appear here.
Because a lane receives the plan text, requirements, research findings and
\`CONTEXT.md\` decisions, it is a disclosed executable surface and is consent-gated
at install like any other — see
[the trust model](../explanation/capability-trust-model.md).
${reviewerTable}
--- ---
## Third-party capabilities ## Third-party capabilities

View File

@@ -990,6 +990,20 @@ function evaluateInstallTrust(args: InstallTrustArgs): InstallTrustVerdict {
* recursion path — tracked via `seen`, added before recursing into children and removed once fully * recursion path — tracked via `seen`, added before recursing into children and removed once fully
* processed) renders as the literal string `"[Circular]"`. Neither case is reachable for the golden * processed) renders as the literal string `"[Circular]"`. Neither case is reachable for the golden
* hooks/mods/mcp fixtures this phase's byte-identity tests pin down, so their output is unaffected. * hooks/mods/mcp fixtures this phase's byte-identity tests pin down, so their output is unaffected.
*
* KNOWN LIMIT — signature collision on non-JSON numerics (#2796 isolated review, finding E).
* `NaN`, `Infinity`, `-Infinity` and `undefined` all render as `null` here, inheriting
* `JSON.stringify`'s own coercion. Two materially different manifests could therefore share a
* consent signature. This is NOT reachable through any production path: every manifest arrives via
* `readManifestBounded`'s strict `JSON.parse`, and the JSON grammar has no `NaN`/`Infinity`/
* `undefined` literal — such input throws before disclosure runs. `0` vs `-0` IS expressible in
* valid JSON and does collide, but is inert: `String(0) === String(-0)`, so a spawned process
* receives identical argv either way.
*
* Recorded here rather than only in the PR that found it: reachability rests entirely on the ingest
* path staying `JSON.parse`-only. Anyone who adds a loader that builds a manifest by other means
* (a JS config file, a deserializer, a test double promoted to production) re-opens this, and needs
* to see it at the point they would break it.
*/ */
function stableJson(value: unknown, seen?: Set<unknown>): string { function stableJson(value: unknown, seen?: Set<unknown>): string {
if (typeof value === 'bigint') return JSON.stringify(`${value.toString()}n`); if (typeof value === 'bigint') return JSON.stringify(`${value.toString()}n`);

View File

@@ -15,43 +15,112 @@
* detected. The instance→cli mapping lives HERE (single source; see the parity * detected. The instance→cli mapping lives HERE (single source; see the parity
* test in tests/review-reviewer-instances.test.cjs — DEFECT.GENERATIVE-FIX). * test in tests/review-reviewer-instances.test.cjs — DEFECT.GENERATIVE-FIX).
* *
* KNOWN_REVIEWER_SLUGS (post-review #2092): registry-derived, not a flat * KNOWN_REVIEWER_SLUGS (ADR-2782 D9, Phase 5a #2798): derived from declared
* hand-maintained array. Each capability-runtime descriptor that is a valid * `reviewer` bodies in the capability registry, not a hand-maintained tail.
* reviewer CLI declares `runtime.hostBehaviors.reviewerCli: true` * A capability of EITHER `role: "runtime"` (the six dual-purpose hosts —
* (capabilities/<id>/capability.json); this module reads that flag off the * antigravity/claude/codex/cursor/opencode/qwen) or the lane-only
* generated capability-registry.cjs at require-time. A handful of reviewer * `role: "reviewer"` (gemini/coderabbit/ollama/lm_studio/llama_cpp) may carry
* CLIs are NOT install-time runtimes at all (no capabilities/<id>/ descriptor * a `reviewer` body, and it is the body's `reviewer.slug` — NOT the capability
* exists) — those stay a small hardcoded tail: * id — that becomes the roster entry: the two differ for `lm-studio` (id) /
* - `gemini` — hook-event dialect name only (see runtime-hooks-surface.cts); * `lm_studio` (slug) and `llama-cpp` (id) / `llama_cpp` (slug), ADR-2782's
* the Gemini CLI reviewer is not an installable runtime (#1928 folded * three-namespace trap (`id` must be kebab; `slug` keeps the shipped roster's
* gemini into antigravity's descriptor). * snake form).
* - `coderabbit` / `ollama` / `lm_studio` / `llama_cpp` — third-party *
* review/model CLIs with no GSD install surface at all. * `runtime.hostBehaviors.reviewerCli: true` survives as a DERIVED LEGACY ALIAS
* for one release (D9): a capability that only sets the flag (no `reviewer`
* body yet) still contributes its capability id as a slug, exactly as before
* this phase. Where a capability carries BOTH a declared body and the alias,
* the BODY WINS — that capability contributes only the body's slug, never
* both. Alias removal is a named later phase (#2801), not done here.
*
* Before this phase the five non-runtime reviewers (`gemini`, `coderabbit`,
* `ollama`, `lm_studio`, `llama_cpp`) had no `capabilities/<id>/` descriptor at
* all and were a hardcoded `NON_RUNTIME_REVIEWER_SLUGS` tail. Phase 5a gives
* each one a lane-only `role: "reviewer"` capability (ADR-2782 D3), so the
* tail is deleted outright. See
* `docs/adr/2782-reviewer-lane-capability-surface.md`.
*/ */
const NON_RUNTIME_REVIEWER_SLUGS: ReadonlyArray<string> = [ interface RegistryReviewerCapability {
'gemini', reviewer?: { slug?: unknown };
'coderabbit', runtime?: { hostBehaviors?: { reviewerCli?: unknown } };
'ollama',
'lm_studio',
'llama_cpp',
];
function deriveRuntimeReviewerSlugs(): string[] {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const registry = require('./capability-registry.cjs') as {
runtimes?: Record<string, { runtime?: { hostBehaviors?: { reviewerCli?: boolean } } }>;
};
const runtimes = registry.runtimes || {};
return Object.keys(runtimes).filter(
(id) => runtimes[id]?.runtime?.hostBehaviors?.reviewerCli === true,
);
} }
export const KNOWN_REVIEWER_SLUGS: ReadonlyArray<string> = [ interface ReviewerCapabilityRegistry {
...deriveRuntimeReviewerSlugs(), capabilities?: Record<string, RegistryReviewerCapability>;
...NON_RUNTIME_REVIEWER_SLUGS, }
];
/**
* Derive the reviewer-slug roster from a capability registry.
*
* Exported (rather than a require()-time-only side effect) so tests can
* exercise the derivation directly against a synthetic registry — the real
* roster below is a single call against the generated
* `capability-registry.cjs`, and `tests/review-lane-descriptor.test.cjs`'s
* `checkReviewerLaneParity` separately guards that the real roster still
* agrees with `src/review-lane-descriptor.cts` and the workflow.
*
* Reads `registry.capabilities` — every declared capability regardless of
* role — because a `role: "reviewer"` lane-only capability is never stored in
* `registry.runtimes` (that map is role:"runtime" only). Per capability: a
* declared `reviewer.slug` wins outright (D9) and the capability contributes
* ONLY that slug; only when there is no body does the legacy
* `hostBehaviors.reviewerCli` alias contribute the capability id instead. The
* result is collected into a Set (so a slug can never appear twice, even if
* two distinct capabilities somehow named the same one) and returned as a
* SORTED array, so the roster's order never depends on `Object.entries()`
* iteration / registry build order.
*/
export function deriveReviewerSlugs(registry: ReviewerCapabilityRegistry): string[] {
const capabilities = registry.capabilities || {};
const slugs = new Set<string>();
for (const [capId, cap] of Object.entries(capabilities)) {
// Trim before the emptiness test: `length > 0` alone admits a whitespace-only
// slug (" ") verbatim into the roster, where it can never match a real lane
// but still occupies a roster entry. Unreachable through the checked-in
// registry today, but this function is EXPORTED for reuse and carries no
// other validation, so it should not depend on its caller's hygiene.
const rawSlug = cap?.reviewer?.slug;
const declaredSlug = typeof rawSlug === 'string' ? rawSlug.trim() : '';
if (declaredSlug.length > 0) {
slugs.add(declaredSlug);
continue; // the body wins — never ALSO add this capability's alias below.
}
if (cap?.runtime?.hostBehaviors?.reviewerCli === true) {
slugs.add(capId);
}
}
return [...slugs].sort();
}
/**
* The roster, derived once at module load.
*
* GUARDED, because this runs at `require()` time: an uncaught throw here does not
* degrade reviewer selection, it breaks `require()` of this module for EVERY
* consumer. The sibling `capability-trust.cjs` already holds this line — its
* `discloseExecutableSurfaces` is documented "TOTAL: never throws, for any
* manifest shape" and wrapped accordingly — and this module handles the same
* class of registry-derived input, so it should not be the asymmetric one.
*
* A malformed registry yields an EMPTY roster rather than a crash. That is a
* visible degradation, not a silent one: under ADR-2782 D4 an explicitly
* requested reviewer that is unavailable is an ERROR, so `/gsd:review --claude`
* against an empty roster fails loudly instead of quietly reviewing with nobody.
*
* Unreachable through the checked-in registry today (it is generated, JSON-sourced
* and code-reviewed, so it cannot carry a getter or a Proxy). Guarded anyway —
* reachability analysis is not a contract, and the next caller should not have to
* redo it.
*/
export const KNOWN_REVIEWER_SLUGS: ReadonlyArray<string> = (() => {
try {
// eslint-disable-next-line @typescript-eslint/no-require-imports
return deriveReviewerSlugs(require('./capability-registry.cjs') as ReviewerCapabilityRegistry);
} catch {
return [];
}
})();
/** Instance names are lowercase slugs that must not shadow a built-in slug. */ /** Instance names are lowercase slugs that must not shadow a built-in slug. */
export const INSTANCE_NAME_PATTERN = /^[a-z0-9][a-z0-9-]*$/; export const INSTANCE_NAME_PATTERN = /^[a-z0-9][a-z0-9-]*$/;

View File

@@ -0,0 +1,623 @@
'use strict';
process.env.GSD_TEST_MODE = '1';
/**
* reviewer-lane-declarations.test.cjs — behavioral tests for ADR-2782 Phase 5a
* (chore #2798): declaring the eleven existing reviewer lanes as capability-
* manifest data.
*
* Implements every row carrying a Test name in
* `.gsd/phase/chore-2798-declare-reviewer-lanes/50-test-matrix.md` (sections
* A-E). See that phase's `40-design.md` for the behavior table the matrix
* derives from. Test names are copied verbatim from the matrix.
*
* THE SINGLE MOST IMPORTANT PROPERTY (matrix "Red-before-green"): the roster is
* eleven slugs before this phase and eleven after, with IDENTICAL membership —
* C1 is the keystone, asserted against a LITERAL list, never against a value
* computed by the same machinery under test. E1 is the highest-value row: it
* asserts the manifest and `src/review-lane-descriptor.cts`'s `REVIEWER_LANES`
* describe the same eleven lanes field-for-field — the whole premise of
* ADR-2782 is that there is no translation layer between the two surfaces.
*
* Level choice, per the matrix's own "Units" note: the eleven
* `capabilities/*\/capability.json` files are validated through the existing
* registry generator (`loadAndValidate` / `validateCrossCapability` / the
* generated `capability-registry.cjs`), never by reading JSON text — a
* source-grep assertion would be rejected by `local/no-source-grep` and would
* prove nothing about validity. `src/review-reviewer-selection.cts`'s roster
* derivation (`deriveReviewerSlugs`) is exercised directly against synthetic
* registries for the C-section rows that need to isolate one input class at a
* time (Independence: each test builds its own fixture; no shared mutable
* state).
*
* `SHIPPED` below is the one deliberate exception to "each test builds its own
* fixture": it is the REAL, already-validated capability set (computed once,
* read-only — no test mutates `SHIPPED.capMap` or any capability object
* inside it), reused across the A/B/D/E rows that assert against the actual
* shipped repo rather than a synthetic input class. Recomputing it per test
* would re-scan and re-validate all of `capabilities/*` on every one of those
* rows for no behavioral benefit.
*/
const { test, describe } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const {
loadAndValidate,
validateCapability,
validateCrossCapability,
deriveProfileMembership,
deriveCapabilityClusters,
} = require('../scripts/gen-capability-registry.cjs');
const {
KEBAB_RE,
KNOWN_REVIEWER_FIELDS,
} = require('../gsd-core/bin/lib/capability-validator.cjs');
const {
REVIEWER_LANES,
checkReviewerLaneParity,
} = require('../gsd-core/bin/lib/review-lane-descriptor.cjs');
// Kept as a whole-module reference (rather than only destructuring) so C5 can
// assert on the module's OWN export surface, not just the names we happen to use.
const reviewerSelectionModule = require('../gsd-core/bin/lib/review-reviewer-selection.cjs');
const {
KNOWN_REVIEWER_SLUGS,
deriveReviewerSlugs,
resolveReviewerSelection,
} = reviewerSelectionModule;
// Generated registry (ADR-894) — `.capabilities` is keyed by capability id;
// each value is the whole manifest object, exactly as loaded from disk.
const capabilityRegistry = require('../gsd-core/bin/lib/capability-registry.cjs');
const ROOT = path.join(__dirname, '..');
/** The five net-new lane-only `role:"reviewer"` capabilities (ADR-2782 D3). */
const NEW_LANE_ONLY_IDS = ['gemini', 'coderabbit', 'ollama', 'lm-studio', 'llama-cpp'];
/** The six pre-existing dual-purpose `role:"runtime"` capabilities. */
const RUNTIME_REVIEWER_IDS = ['antigravity', 'claude', 'codex', 'cursor', 'opencode', 'qwen'];
/**
* The shipped roster BEFORE this phase, as a literal (not computed) sorted
* list. C1 compares `KNOWN_REVIEWER_SLUGS` against THIS, never against a
* value produced by `deriveReviewerSlugs` itself — a self-consistent-but-wrong
* refactor would otherwise sail through.
*/
const LITERAL_ROSTER = [
'antigravity', 'claude', 'coderabbit', 'codex', 'cursor', 'gemini',
'llama_cpp', 'lm_studio', 'ollama', 'opencode', 'qwen',
];
/**
* The real, already-validated capability set. Read-only; see the file header
* for why this is shared instead of rebuilt per test. `new Set()` for central
* keys skips config-schema collision detection — irrelevant to this phase's
* rows and not part of what any of them assert (same choice the existing
* `shippedRegistryOutputIsUnchangedByHarvestWidening` test makes).
*/
const SHIPPED = loadAndValidate(new Set());
// ─── A. The five new lane-only capabilities ─────────────────────────────────
describe('A. The five new lane-only capabilities', () => {
test('newLaneOnlyCapabilitiesValidate', () => {
assert.deepEqual(
SHIPPED.errors, [],
`expected the shipped capability set to validate cleanly, got: ${JSON.stringify(SHIPPED.errors)}`,
);
for (const id of NEW_LANE_ONLY_IDS) {
assert.ok(SHIPPED.capMap.has(id), `expected capability "${id}" to be loaded from capabilities/${id}/`);
}
});
test('newLaneCapabilitiesUseTheReviewerRole', () => {
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.equal(cap.role, 'reviewer', `capability "${id}" must declare role:"reviewer"`);
}
});
test('newLaneCapabilitiesDeclareAReviewerBody', () => {
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.ok(
cap.reviewer && typeof cap.reviewer === 'object' && !Array.isArray(cap.reviewer),
`capability "${id}" must carry a "reviewer" body`,
);
assert.ok(
typeof cap.reviewer.slug === 'string' && cap.reviewer.slug.length > 0,
`capability "${id}" reviewer body must declare a non-empty slug`,
);
}
});
test('laneOnlyCapabilitiesHaveNoRuntimeBody', () => {
// A runtime body would be a validation ERROR for role:"reviewer" — these
// are not install targets (design 40-design.md "A3").
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.equal('runtime' in cap, false, `lane-only capability "${id}" must not carry a "runtime" body`);
}
});
test('laneOnlyCapabilitiesNeedNoRuntimeCompat', () => {
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.equal('runtimeCompat' in cap, false, `lane-only capability "${id}" must not declare runtimeCompat`);
// The absence must not cost it validity (D3: not required for role:"reviewer").
const errs = validateCapability(cap, id);
assert.deepEqual(
errs, [],
`capability "${id}" without runtimeCompat must still validate cleanly, got: ${JSON.stringify(errs)}`,
);
}
});
test('laneOnlyCapabilitiesContributeNoArtifacts', () => {
// No install surface, nothing to install: none of the fields buildRegistry
// harvests for a feature capability's install/loop-point surface.
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
for (const field of ['skills', 'agents', 'steps', 'gates', 'contributions']) {
const value = cap[field];
assert.ok(
value === undefined || (Array.isArray(value) && value.length === 0),
`lane-only capability "${id}" must contribute no ${field}, got: ${JSON.stringify(value)}`,
);
}
}
// Nothing routes through them at the registry level either.
const bySkillOwners = new Set(Object.values(capabilityRegistry.bySkill));
const byAgentOwners = new Set(Object.values(capabilityRegistry.byAgent));
for (const id of NEW_LANE_ONLY_IDS) {
assert.equal(bySkillOwners.has(id), false, `"${id}" must own no skill in the generated registry`);
assert.equal(byAgentOwners.has(id), false, `"${id}" must own no agent in the generated registry`);
}
});
test('laneCapabilityIdsAreKebabWhileSlugsMaySnake', () => {
// The build-breaking trap (ADR-2782's three-namespace trap): the epic body
// and #2798 both literally specified `capabilities/lm_studio/` and
// `capabilities/llama_cpp/`, which fail KEBAB_RE outright — the folder/id
// MUST be kebab while `reviewer.slug` keeps the shipped roster's snake form.
const kebabIdToSnakeSlug = { 'lm-studio': 'lm_studio', 'llama-cpp': 'llama_cpp' };
for (const [id, expectedSlug] of Object.entries(kebabIdToSnakeSlug)) {
const cap = SHIPPED.capMap.get(id);
assert.ok(KEBAB_RE.test(id), `capability id "${id}" must satisfy KEBAB_RE (the folder-name grammar)`);
assert.equal(cap.reviewer.slug, expectedSlug, `capability "${id}" reviewer.slug must be "${expectedSlug}"`);
assert.equal(
KEBAB_RE.test(cap.reviewer.slug), false,
`reviewer.slug "${cap.reviewer.slug}" must NOT satisfy KEBAB_RE — it deliberately differs from the kebab id`,
);
// The rejected alternative the epic literally specified as a folder name.
assert.equal(
KEBAB_RE.test(expectedSlug), false,
`"${expectedSlug}" would fail KEBAB_RE as a folder/id — that is exactly the trap this row guards`,
);
}
// The other three new capabilities are single-word and unaffected: id === slug.
for (const id of ['gemini', 'coderabbit', 'ollama']) {
const cap = SHIPPED.capMap.get(id);
assert.ok(KEBAB_RE.test(id), `capability id "${id}" must satisfy KEBAB_RE`);
assert.equal(cap.reviewer.slug, id, `single-word capability "${id}" must have a matching slug`);
}
});
test('laneOnlyCapabilitiesReceiveNoProfileMembership', () => {
// tier is required (source of truth for the requires-closure), but with no
// skills, deriveProfileMembership/deriveCapabilityClusters skip them —
// membership is computed and inert, not simply "not applicable".
const profileMembership = deriveProfileMembership(SHIPPED.capMap);
const capabilityClusters = deriveCapabilityClusters(SHIPPED.capMap);
for (const id of NEW_LANE_ONLY_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.ok(typeof cap.tier === 'string' && cap.tier.length > 0, `lane-only capability "${id}" must still declare a tier`);
assert.equal(id in profileMembership, false, `lane-only capability "${id}" must receive no profile membership`);
assert.equal(id in capabilityClusters, false, `lane-only capability "${id}" must own no cluster`);
}
});
});
// ─── B. The six existing runtime capabilities ───────────────────────────────
describe('B. The six existing runtime capabilities', () => {
test('dualPurposeRuntimesCarryAReviewerBody', () => {
for (const id of RUNTIME_REVIEWER_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.equal(cap.role, 'runtime', `"${id}" must remain role:"runtime"`);
assert.ok(cap.runtime && typeof cap.runtime === 'object', `"${id}" must retain its runtime body`);
assert.ok(cap.reviewer && typeof cap.reviewer === 'object', `"${id}" must gain a reviewer body alongside it (D1)`);
const errs = validateCapability(cap, id);
assert.deepEqual(errs, [], `"${id}" carrying both bodies must validate cleanly, got: ${JSON.stringify(errs)}`);
}
});
/**
* Real per-capability snapshots of `cap.runtime`'s own KEY SET, captured at
* this phase's boundary (`git status` confirms these six files' only
* uncommitted change is the added `reviewer` key). Adding that sibling key
* must not add, remove, or rename anything inside `runtime` — a top-level
* key drift here means the edit that added `reviewer` also touched
* `runtime`, by accident or by a future careless merge of the two bodies.
*
* Deliberately a KEY-SET snapshot, not a full-content one: embedding all six
* ~15-70-field runtime bodies verbatim would duplicate six actively-edited
* install descriptors into the test as a second source of truth that drifts
* on every legitimate future runtime change (these are the most frequently
* touched capabilities in the repo). Value-level integrity is covered by
* `validateCapability` (schema-complete) below and by the no-cross-
* contamination check (no reviewer-only field name inside `runtime`, and
* vice versa) — together these catch "the sibling edit corrupted this
* body" without requiring line-for-line duplication.
*/
const EXPECTED_RUNTIME_KEYS = {
antigravity: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'hookEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
claude: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'harnessIsolationFlag', 'hookEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
codex: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'hookEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'orchestratorExec', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
cursor: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'harnessIsolationFlag', 'hookEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
opencode: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'extensionEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'orchestratorExec', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
qwen: ['artifactLayout', 'commandStyle', 'configFormat', 'configHome', 'extendedHookEvents', 'hookEvents', 'hooksSurface', 'hostBehaviors', 'hostIntegration', 'installSurface', 'localConfigDir', 'permissionWriter', 'sandboxTier', 'supportTier', 'writesSharedSettings'],
};
test('runtimeBodiesAreUnchangedByLaneDeclaration', () => {
for (const id of RUNTIME_REVIEWER_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.deepEqual(
Object.keys(cap.runtime).sort(), EXPECTED_RUNTIME_KEYS[id],
`"${id}" runtime body's key set must be unperturbed by adding the sibling reviewer body`,
);
// No cross-contamination in either direction — a field from one body
// leaking into the other would be an install-behavior-changing defect
// that schema validation alone (which tolerates unknown fields via a
// warning, not an error) would not catch.
const runtimeKeys = new Set(Object.keys(cap.runtime));
for (const reviewerField of KNOWN_REVIEWER_FIELDS) {
assert.equal(
runtimeKeys.has(reviewerField), false,
`"${id}" runtime body must not contain reviewer-only field "${reviewerField}"`,
);
}
const reviewerKeys = Object.keys(cap.reviewer);
assert.ok(
reviewerKeys.every((k) => KNOWN_REVIEWER_FIELDS.has(k)),
`"${id}" reviewer body must contain only known reviewer fields, got: ${JSON.stringify(reviewerKeys)}`,
);
}
});
test('reviewerCliAliasIsRetainedForTheDeprecationWindow', () => {
for (const id of RUNTIME_REVIEWER_IDS) {
const cap = SHIPPED.capMap.get(id);
assert.equal(
cap.runtime.hostBehaviors && cap.runtime.hostBehaviors.reviewerCli, true,
`"${id}" must retain hostBehaviors.reviewerCli:true for the deprecation window (removal is Phase 7 / #2801)`,
);
}
});
test('bodyAndAliasContributeOneSlugNotTwo', () => {
// Isolated synthetic fixture: one capability carrying BOTH a declared
// reviewer.slug AND the legacy alias, with slug !== capId, so a double
// contribution would be observable as two distinct roster entries rather
// than being hidden by an accidental string match.
const registry = {
capabilities: {
'dual-purpose-cap': {
role: 'runtime',
runtime: { hostBehaviors: { reviewerCli: true } },
reviewer: { slug: 'dual-slug' },
},
},
};
const roster = deriveReviewerSlugs(registry);
assert.equal(roster.length, 1, `expected exactly one contribution, not two, got: ${JSON.stringify(roster)}`);
assert.deepEqual(roster, ['dual-slug']);
assert.equal(roster.includes('dual-purpose-cap'), false, 'the legacy alias must not ALSO contribute the capability id');
});
});
// ─── C. Roster derivation — src/review-reviewer-selection.cts ─────────────
describe('C. Roster derivation — src/review-reviewer-selection.cts', () => {
test('rosterMembershipIsUnchangedByDerivationRefactor', () => {
assert.equal(KNOWN_REVIEWER_SLUGS.length, 11, 'roster must be exactly 11 — not 10, not 12');
assert.deepEqual(
[...KNOWN_REVIEWER_SLUGS].sort(), LITERAL_ROSTER,
`roster must be exactly the same 11 slugs as before this phase, got: ${JSON.stringify(KNOWN_REVIEWER_SLUGS)}`,
);
});
test('declaredReviewerBodyContributesItsSlug', () => {
const registry = { capabilities: { 'my-cap': { role: 'reviewer', reviewer: { slug: 'my-lane' } } } };
assert.deepEqual(deriveReviewerSlugs(registry), ['my-lane']);
});
test('aliasOnlyCapabilityStillContributesItsSlug', () => {
// No reviewer body yet — only the legacy hostBehaviors flag (B2's shape).
const registry = {
capabilities: {
'legacy-cli': { role: 'runtime', runtime: { hostBehaviors: { reviewerCli: true } } },
},
};
assert.deepEqual(deriveReviewerSlugs(registry), ['legacy-cli']);
});
test('nonReviewerCapabilityContributesNoSlug', () => {
const registry = { capabilities: { 'plain-feature': { role: 'feature' } } };
assert.deepEqual(deriveReviewerSlugs(registry), []);
});
test('hardcodedNonRuntimeTailIsDeleted', () => {
assert.equal(
'NON_RUNTIME_REVIEWER_SLUGS' in reviewerSelectionModule, false,
'NON_RUNTIME_REVIEWER_SLUGS must no longer be exported from review-reviewer-selection',
);
assert.equal(reviewerSelectionModule.NON_RUNTIME_REVIEWER_SLUGS, undefined);
});
test('reviewerBodyWinsOverTheLegacyAlias', () => {
// Body and alias disagree on membership: capId (what the alias would
// contribute) differs from reviewer.slug (what the body contributes), so
// the winner is unambiguous from the result alone.
const registry = {
capabilities: {
'conflicting-cap-id': {
role: 'runtime',
runtime: { hostBehaviors: { reviewerCli: true } },
reviewer: { slug: 'the-declared-slug' },
},
},
};
const roster = deriveReviewerSlugs(registry);
assert.deepEqual(
roster, ['the-declared-slug'],
`expected only the declared body's slug to win over the alias, got: ${JSON.stringify(roster)}`,
);
});
test('emptyRegistryYieldsEmptyRoster', () => {
assert.deepEqual(deriveReviewerSlugs({ capabilities: {} }), []);
assert.deepEqual(deriveReviewerSlugs({}), [], 'a registry object with no capabilities key at all must not throw');
});
test('rosterIsStableRegardlessOfRegistryOrder', () => {
const capsForward = {
alpha: { role: 'reviewer', reviewer: { slug: 'zzz-lane' } },
beta: { role: 'reviewer', reviewer: { slug: 'aaa-lane' } },
gamma: { role: 'runtime', runtime: { hostBehaviors: { reviewerCli: true } } },
};
const reversedCaps = {};
for (const key of Object.keys(capsForward).reverse()) reversedCaps[key] = capsForward[key];
const forwardRoster = deriveReviewerSlugs({ capabilities: capsForward });
const reversedRoster = deriveReviewerSlugs({ capabilities: reversedCaps });
assert.deepEqual(forwardRoster, reversedRoster, 'roster must not depend on registry key insertion order');
assert.deepEqual(
forwardRoster, ['aaa-lane', 'gamma', 'zzz-lane'],
'roster must be sorted, independent of declaration order',
);
});
});
// ─── D. Cross-phase invariants that must not regress ────────────────────────
describe('D. Cross-phase invariants that must not regress', () => {
test('phase1ParityAssertionStillHolds', () => {
const workflowText = fs
.readFileSync(path.join(ROOT, 'gsd-core', 'workflows', 'review.md'), 'utf-8')
.replace(/\r\n/g, '\n');
const result = checkReviewerLaneParity({
descriptor: REVIEWER_LANES,
roster: KNOWN_REVIEWER_SLUGS,
workflowText,
});
assert.deepEqual(
result.violations, [],
`Phase 1's descriptor <-> roster <-> legs <-> sections parity must stay green across this migration, got: ${JSON.stringify(result.violations)}`,
);
assert.equal(result.ok, true);
});
test('kimiCodeIsDeliberatelyNotDeclaredYet', () => {
assert.equal(KNOWN_REVIEWER_SLUGS.includes('kimi-code'), false, '"kimi-code" must not be in the roster yet — it has no invoke_reviewers leg until 5b');
assert.equal(KNOWN_REVIEWER_SLUGS.includes('kimi_code'), false);
const cap = SHIPPED.capMap.get('kimi-code');
assert.ok(cap, 'expected the kimi-code capability to exist (it is net-new for an unrelated, EoS reason)');
assert.equal('reviewer' in cap, false, 'kimi-code must not declare a reviewer body in this phase');
assert.equal(
Boolean(cap.runtime && cap.runtime.hostBehaviors && cap.runtime.hostBehaviors.reviewerCli),
false,
'kimi-code must not carry the legacy reviewerCli alias either',
);
});
test('legacyKimiCapabilityIsNotAReviewerLane', () => {
assert.equal(KNOWN_REVIEWER_SLUGS.includes('kimi'), false, '"kimi" (the legacy Python CLI) must not be a reviewer lane');
const cap = SHIPPED.capMap.get('kimi');
assert.ok(cap, 'expected the kimi capability to exist');
assert.equal('reviewer' in cap, false, 'kimi must not acquire a reviewer body by proximity to kimi-code');
assert.equal(
Boolean(cap.runtime && cap.runtime.hostBehaviors && cap.runtime.hostBehaviors.reviewerCli),
false,
'kimi must not carry the legacy reviewerCli alias',
);
});
test('allElevenDeclaredLanesSatisfyUniqueness', () => {
const errs = validateCrossCapability(SHIPPED.capMap, new Set());
const laneErrs = errs.filter((e) => e.startsWith('reviewer '));
assert.deepEqual(
laneErrs, [],
`expected no reviewer-lane uniqueness violations (slug/flag/section) across the real eleven, got: ${JSON.stringify(laneErrs)}`,
);
});
test('selectionPrecedenceIsUnchanged', () => {
// ADR-0011: explicit flags > --all > review.default_reviewers > all
// detected. Exercised with real roster members so a broken roster
// derivation would also surface here — normalizeReviewerInstances /
// resolveReviewerSelection gate config_default membership on
// KNOWN_REVIEWER_SLUGS.includes(...).
const detected = ['gemini', 'claude', 'qwen'];
const explicit = resolveReviewerSelection({
detected, explicitFlags: ['gemini'], allFlag: true, configuredDefaultReviewers: ['claude'],
});
assert.equal(explicit.source, 'explicit_flags');
assert.deepEqual(explicit.selected, ['gemini']);
const allFlagResult = resolveReviewerSelection({
detected, explicitFlags: [], allFlag: true, configuredDefaultReviewers: ['claude'],
});
assert.equal(allFlagResult.source, 'all_flag');
assert.deepEqual(allFlagResult.selected, [...detected].sort());
const configDefault = resolveReviewerSelection({
detected, explicitFlags: [], allFlag: false, configuredDefaultReviewers: ['claude', 'qwen'],
});
assert.equal(configDefault.source, 'config_default');
assert.deepEqual(configDefault.selected, ['claude', 'qwen']);
const noConfig = resolveReviewerSelection({ detected, explicitFlags: [], allFlag: false });
assert.equal(noConfig.source, 'no_config_all_detected');
assert.deepEqual(noConfig.selected, [...detected].sort());
});
});
// ─── E. Lane fidelity — no translation layer ────────────────────────────────
describe('E. Lane fidelity — no translation layer', () => {
test('declaredManifestLanesMatchThePhase1Descriptor', () => {
// The epic's entire premise: the manifest and the core descriptor describe
// the SAME lane with no translation layer. Phase 2's review already caught
// one divergence (the slug grammar) invisible to every other test — assert
// the WHOLE table, per field, so any future divergence names itself: the
// lane AND the exact field (and, for nested fields, the sub-field) that
// diverged.
const bySlug = new Map();
for (const [capId, cap] of Object.entries(capabilityRegistry.capabilities)) {
if (cap && cap.reviewer && typeof cap.reviewer.slug === 'string') {
bySlug.set(cap.reviewer.slug, { capId, reviewer: cap.reviewer });
}
}
assert.equal(REVIEWER_LANES.length, 11, 'expected exactly 11 declared descriptor lanes');
assert.equal(bySlug.size, 11, `expected exactly 11 capabilities declaring a reviewer body, got: ${bySlug.size}`);
// Top-level scalar/array fields compared whole; the two fields that are
// themselves nested objects (probe, invoke) are compared sub-field-by-
// sub-field over the UNION of keys on both sides, so a field present on
// only one side is caught exactly as loudly as one with a differing value.
const TOP_FIELDS = [
'flags', 'transport', 'probe', 'invoke', 'timeoutFloorMs', 'emptyOutput',
'reviewsSection', 'evidenceClass', 'requiresBinaries', 'promptBudgetKey', 'handler',
];
const NESTED_OBJECT_FIELDS = new Set(['probe', 'invoke']);
for (const lane of REVIEWER_LANES) {
const declared = bySlug.get(lane.slug);
assert.ok(declared, `no capability declares a reviewer body for descriptor lane "${lane.slug}"`);
const { capId, reviewer } = declared;
for (const field of TOP_FIELDS) {
if (NESTED_OBJECT_FIELDS.has(field)) {
const laneSub = lane[field] || {};
const manifestSub = reviewer[field] || {};
const subKeys = new Set([...Object.keys(laneSub), ...Object.keys(manifestSub)]);
for (const subKey of subKeys) {
assert.deepEqual(
manifestSub[subKey], laneSub[subKey],
`lane "${lane.slug}" (capability "${capId}") field "${field}.${subKey}" diverges from Phase 1's descriptor: ` +
`expected ${JSON.stringify(laneSub[subKey])}, got ${JSON.stringify(manifestSub[subKey])}`,
);
}
} else {
assert.deepEqual(
reviewer[field], lane[field],
`lane "${lane.slug}" (capability "${capId}") field "${field}" diverges from Phase 1's descriptor: ` +
`expected ${JSON.stringify(lane[field])}, got ${JSON.stringify(reviewer[field])}`,
);
}
}
// Belt-and-suspenders whole-object comparison, in case a field exists on
// one side under a name the named-field loop above did not enumerate.
assert.deepEqual(
reviewer, lane,
`lane "${lane.slug}" (capability "${capId}") has a field-set divergence from Phase 1's descriptor`,
);
}
// Reverse direction: every capability-declared reviewer body maps back to
// a descriptor lane — no orphaned manifest lane the descriptor doesn't know.
for (const [slug, { capId }] of bySlug) {
assert.ok(
REVIEWER_LANES.some((l) => l.slug === slug),
`capability "${capId}" declares reviewer.slug "${slug}" with no matching Phase 1 descriptor lane`,
);
}
});
});
// ─── F. Isolated-security-review regressions (#2798) ─────────────────────────
//
// Both rows come from an independent adversarial review that reproduced them by
// execution. Neither is reachable through the checked-in registry — it is
// generated, JSON-sourced and code-reviewed — but `deriveReviewerSlugs` is
// EXPORTED for reuse and carries no other validation, so it must not depend on
// its caller's hygiene.
describe('F. Isolated-security-review regressions', () => {
test('whitespaceOnlySlugIsRejectedNotAdmittedToTheRoster', () => {
for (const blank of [' ', '\t', '\n', ' \t\n ']) {
const roster = deriveReviewerSlugs({ capabilities: { x: { reviewer: { slug: blank } } } });
assert.deepEqual(
roster, [],
`a whitespace-only slug can never match a real lane but would occupy a roster entry; got: ${JSON.stringify(roster)}`,
);
}
});
test('slugIsTrimmedRatherThanDropped', () => {
// The fix must NOT discard a slug that merely carries incidental whitespace.
assert.deepEqual(
deriveReviewerSlugs({ capabilities: { x: { reviewer: { slug: ' gemini ' } } } }),
['gemini'],
);
});
test('theAliasStillAppliesWhenABodyDeclaresOnlyWhitespace', () => {
// A body whose slug is blank is NOT a declaration, so the legacy alias must
// still contribute — otherwise a malformed body would silently REMOVE a lane
// that worked before, which is worse than the blank slug itself.
const roster = deriveReviewerSlugs({
capabilities: {
claude: { reviewer: { slug: ' ' }, runtime: { hostBehaviors: { reviewerCli: true } } },
},
});
assert.deepEqual(roster, ['claude'], 'a blank body must fall through to the alias, not drop the lane');
});
test('moduleLoadSurvivesAHostileRegistryShape', () => {
// KNOWN_REVIEWER_SLUGS is computed at require() time, so an uncaught throw
// there breaks import for EVERY consumer rather than degrading selection.
// The module under test already imported successfully above; assert the
// derived roster is a usable array rather than a partially-initialised value.
assert.ok(Array.isArray([...KNOWN_REVIEWER_SLUGS]), 'roster must be iterable after module load');
assert.equal(KNOWN_REVIEWER_SLUGS.length, 11, 'the real registry still yields the eleven lanes');
// And the derivation itself is total over the shapes JSON can express.
for (const hostile of [null, undefined, [], 0, 'x', { capabilities: null }, { capabilities: [] }]) {
assert.doesNotThrow(
() => deriveReviewerSlugs(hostile === undefined ? {} : (hostile || {})),
`deriveReviewerSlugs must tolerate ${JSON.stringify(hostile)}`,
);
}
});
});