From 13fcbe45a69834d9ef781f857cc17a3bcd646a1f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:19:17 +0000 Subject: [PATCH 01/10] chore: bump version to 1.9.1 for hotfix --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- capabilities/ai-integration/capability.json | 2 +- capabilities/antigravity/capability.json | 2 +- capabilities/assumption-delta/capability.json | 2 +- capabilities/audit/capability.json | 2 +- capabilities/augment/capability.json | 2 +- capabilities/broken-windows/capability.json | 2 +- .../claude-orchestration/capability.json | 2 +- capabilities/claude/capability.json | 2 +- capabilities/cline/capability.json | 2 +- capabilities/code-review/capability.json | 2 +- capabilities/codebuddy/capability.json | 2 +- capabilities/coderabbit/capability.json | 2 +- capabilities/codex/capability.json | 2 +- capabilities/copilot/capability.json | 2 +- capabilities/cursor/capability.json | 2 +- capabilities/drift/capability.json | 2 +- capabilities/external-job/capability.json | 2 +- capabilities/gap-analysis/capability.json | 2 +- capabilities/gemini/capability.json | 2 +- capabilities/graphify/capability.json | 2 +- capabilities/hermes/capability.json | 2 +- capabilities/intel/capability.json | 2 +- capabilities/kilo/capability.json | 2 +- capabilities/kimi-code/capability.json | 2 +- capabilities/kimi/capability.json | 2 +- capabilities/llama-cpp/capability.json | 2 +- capabilities/lm-studio/capability.json | 2 +- capabilities/mempalace/capability.json | 2 +- capabilities/nyquist/capability.json | 2 +- capabilities/ollama/capability.json | 2 +- capabilities/opencode/capability.json | 2 +- capabilities/pattern-mapper/capability.json | 2 +- capabilities/pi/capability.json | 2 +- capabilities/profile-pipeline/capability.json | 2 +- capabilities/qwen/capability.json | 2 +- capabilities/research/capability.json | 2 +- capabilities/schema-gate/capability.json | 2 +- capabilities/security/capability.json | 2 +- capabilities/tdd/capability.json | 2 +- capabilities/trae/capability.json | 2 +- capabilities/ui/capability.json | 2 +- capabilities/vscode/capability.json | 2 +- capabilities/windsurf/capability.json | 2 +- capabilities/zcode/capability.json | 2 +- gsd-core/bin/lib/capability-registry.cjs | 126 +++++++++--------- package-lock.json | 4 +- package.json | 2 +- vscode/package.json | 2 +- 50 files changed, 113 insertions(+), 113 deletions(-) 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/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/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/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/vscode/package.json b/vscode/package.json index 883e493fd..a250daec7 100644 --- a/vscode/package.json +++ b/vscode/package.json @@ -2,7 +2,7 @@ "name": "gsd-core-vscode", "displayName": "GSD Core", "description": "GSD orchestration engine embedded in VS Code (ADR-1239 IDE profile).", - "version": "1.9.0", + "version": "1.9.1", "publisher": "opengsd", "engines": { "vscode": "^1.105.0" From 14cfbdaad0f5f6a1912e4521ca4ee12c77f3cce3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 30 Jul 2026 23:14:17 -0400 Subject: [PATCH 02/10] fix(#2667): run-with-timeout mediates .cmd/.bat spawns on Windows (CVE-2024-27980); fallow pre-pass names failure kind (#2897) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2667): mediate .cmd/.bat/.exe spawns on Windows; split fallow pre-pass failure diagnostic run-with-timeout spawned .cmd/.bat/.exe commands without shell:true on Windows, tripping Node's CVE-2024-27980 EINVAL (April 2024 security hardening). The fallow structural pre-pass then no-op'd silently — a hard execution failure read the same as 'optional dependency absent'. (A) gsd-core/bin/gsd-tools.cjs runWithTimeout: gate shell:true on (win32 && command ends in .cmd/.bat/.exe). Narrow by design — never fires for the 7 `bash -c` callers (command is `bash`, no such suffix), so the recorded no-shell-for-argv-array security contract (DEFECT.UNBOUNDED-SUBPROCESS) is preserved; cmdArgs stays an array. POSIX untouched. (B) code-review.md fallow pre-pass: name the failure KIND (timeout / spawn failure / crash / not-found) so a Windows .cmd spawn failure is not mistaken for an absent binary. Regression test in tests/run-with-timeout.test.cjs gated to win32 (.cmd/.bat/.exe shims run with exit 0 + non-empty stdout; pre-fix EINVAL → exit 125/empty). POSIX negative-space test guards the unchanged bash -c callers. * chore(#2667): changeset fragment * chore(#2667): backfill changeset PR 2897 + correct body (cmd.exe array, not shell:true) * fix(#2667): exclude .exe from the win32 spawn-mediation gate; ack code-review.md growth CI caught two failures on the first push: 1. windows-24: 'exits 124 when the wall-clock budget is exceeded' regressed. The gate matched .exe, so the HANG command (node.exe -e 'setTimeout(...)') was wrapped in 'cmd.exe /c node.exe ...' — the wrapped child escaped the timeout cap's process-group reap (exit 124 never fired; hit the 30s harness backstop) AND cmd.exe risked mis-parsing the -e script arg. .exe is INTENTIONALLY excluded now: real PE executables spawn fine directly; only .cmd/.bat are the CVE-2024-27980 EINVAL cases. The .exe test becomes a negative-space test (node.exe spawned directly, exit 0). 2. ubuntu-22: emitted-attribution — code-review.md grew 1177 bytes from the #2667 fallow pre-pass failure-KIND case statement; acknowledge it. --------- Co-authored-by: Test (cherry picked from commit 79ed181ec0621a655f76730029627a0f5284ddab) --- .changeset/gallant-koalas-forage.md | 5 +++ gsd-core/bin/gsd-tools.cjs | 29 ++++++++++++++- gsd-core/workflows/code-review.md | 19 ++++++++-- tests/emitted-drift-ack.json | 2 +- tests/run-with-timeout.test.cjs | 55 +++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 .changeset/gallant-koalas-forage.md diff --git a/.changeset/gallant-koalas-forage.md b/.changeset/gallant-koalas-forage.md new file mode 100644 index 000000000..58591693f --- /dev/null +++ b/.changeset/gallant-koalas-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2897 +--- +**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) diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index cd6b66fb1..baa31cedb 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)); 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/tests/emitted-drift-ack.json b/tests/emitted-drift-ack.json index fa2f84cde..ede87875f 100644 --- a/tests/emitted-drift-ack.json +++ b/tests/emitted-drift-ack.json @@ -14,7 +14,7 @@ "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." + "reason": "#2667: the fallow structural pre-pass failure WARNING now names the failure KIND (timeout / spawn failure / crash / not-found) via a case statement, so a Windows .cmd spawn failure (CVE-2024-27980) is not mistaken for an absent binary. Growth is the case statement + the explanatory prose." } } } diff --git a/tests/run-with-timeout.test.cjs b/tests/run-with-timeout.test.cjs index 0cc820493..64bb151f5 100644 --- a/tests/run-with-timeout.test.cjs +++ b/tests/run-with-timeout.test.cjs @@ -199,6 +199,61 @@ describe('#2351 run-with-timeout — kill semantics (POSIX process groups)', () }); }); +describe('#2667 run-with-timeout — Windows .cmd/.bat/.exe spawn mediation (CVE-2024-27980)', () => { + // Node's CVE-2024-27980 hardening throws EINVAL when child_process.spawn is + // given a .cmd/.bat without a shell. run-with-timeout now mediates .cmd/.bat + // on win32 via an explicit `cmd.exe /d /s /c ...args` argv ARRAY (not + // shell:true — that space-joins unescaped args per DEP0190), while leaving + // every `bash`/argv-array caller unchanged (the recorded no-shell-for-argv- + // array contract). .exe is INTENTIONALLY excluded — real PEs spawn fine + // directly and mediating them breaks the timeout reap + risks arg mis-parse. + const isWin = process.platform === 'win32'; + + test('win32 RED: a .cmd shim runs (exit 0, non-empty stdout) — pre-fix this threw EINVAL → exit 125 / empty stdout', { skip: !isWin ? 'win32-only' : false }, () => { + const dir = createTempDir('rwt-2667-cmd'); + try { + // A .cmd shim that echoes JSON to stdout (mimics fallow.cmd audit --format json). + const shim = path.join(dir, 'fake.cmd'); + fs.writeFileSync(shim, '@echo {"verdict":"clean"}\r\n', 'utf8'); + const r = runVerb(['10', '--', shim]); + assert.equal(r.status, 0, `expected the .cmd shim to run (exit 0); got ${r.status}. stderr: ${r.stderr}`); + assert.ok((r.stdout || '').includes('clean'), `expected non-empty JSON stdout from the .cmd shim; got: ${r.stdout}`); + } finally { + cleanup(dir); + } + }); + + test('win32: a .bat shim is also mediated (exit 0, non-empty stdout)', { skip: !isWin ? 'win32-only' : false }, () => { + const dir = createTempDir('rwt-2667-bat'); + try { + const shim = path.join(dir, 'fake.bat'); + fs.writeFileSync(shim, '@echo {"verdict":"clean"}\r\n', 'utf8'); + const r = runVerb(['10', '--', shim]); + assert.equal(r.status, 0, `expected the .bat shim to run (exit 0); got ${r.status}. stderr: ${r.stderr}`); + assert.ok((r.stdout || '').length > 0, 'expected non-empty stdout from the .bat shim'); + } finally { + cleanup(dir); + } + }); + + test('win32 negative-space: a .exe (node.exe) is spawned DIRECTLY, not mediated — no cmd.exe wrap', { skip: !isWin ? 'win32-only' : false }, () => { + // .exe is intentionally excluded from the gate: real PE executables spawn + // fine directly, and wrapping them in cmd.exe /c breaks the timeout cap's + // process-group reap AND risks cmd.exe mis-parsing args (e.g. -e "code()"). + // node.exe -e "process.exit(0)" must exit 0 directly. + const r = runVerb(['10', '--', process.execPath, '-e', 'process.exit(0)']); + assert.equal(r.status, 0, `expected node.exe to run directly (exit 0); got ${r.status}. stderr: ${r.stderr}`); + }); + + test('POSIX negative-space: a bash -c caller is unchanged (no shell:true added) — argv stays array-only', { skip: isWin ? 'posix-only' : false }, () => { + // The fix's gate (win32 && .cmd/.bat) skips `bash` on POSIX: behavior + // must be identical to before. `bash -c 'echo ok'` exits 0 with stdout "ok". + const r = runVerb(['10', '--', 'bash', '-c', 'echo ok']); + assert.equal(r.status, 0, `expected bash caller to still work (exit 0); got ${r.status}`); + assert.equal((r.stdout || '').trim(), 'ok', 'expected stdout "ok" from the unchanged bash caller'); + }); +}); + describe('#2351 run-with-timeout — coreutils independence (the regression)', () => { // The whole point: no dependency on GNU `timeout`/`gtimeout`. Prove it by // scrubbing PATH so neither could be found, and driving the child by absolute From a38e4d080df33c0720ea5da61595f59f618e5bff Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 00:04:04 -0400 Subject: [PATCH 03/10] fix(#2788): recover Gaps Found rows; mark-complete no longer false-succeeds on a rejected row (#2902) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2788): recover Gaps Found rows; mark-complete no longer false-succeeds on a rejected row Two coupled defects in the requirement traceability state machine: Defect 1 (terminal state): requirements revert-phase (the gaps_found response) left a row at 'Gaps Found' with no inverse — neither mark-complete's /^pending$/i guard nor the phase-complete reconcile's /^(?:pending|in progress)$/i accepted it, so a single failed verification stranded every requirement permanently and blocked the milestone. Widen both guards to accept 'gaps found' so a genuinely-satisfied stranded row reaches Complete again. Defect 2 (false success): mark-complete ORed checkboxHit || tableHit for 'updated', so on a Gaps Found row it flipped the checkbox but could not move the row, yet reported updated:true. When a traceability table has a row for an ID, gate 'updated' on the row moving (tableHit) — a checkbox-only flip on a table-bearing file no longer lies. The #2140 table_unmatched path (no row for the ID) is preserved. * chore(#2788): backfill changeset PR 2902 --------- Co-authored-by: Test (cherry picked from commit 9f567a16273e9011a6af73e5219406486060760d) --- .changeset/noble-cranes-march.md | 5 ++ src/milestone.cts | 36 ++++++++-- src/phase.cts | 4 +- tests/milestone.test.cjs | 111 +++++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 .changeset/noble-cranes-march.md diff --git a/.changeset/noble-cranes-march.md b/.changeset/noble-cranes-march.md new file mode 100644 index 000000000..23564f1e5 --- /dev/null +++ b/.changeset/noble-cranes-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2902 +--- +**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) 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/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'); + }); }); // ───────────────────────────────────────────────────────────────────────────── From dc736805325b93f97f30883e19ab41bda00b4cc0 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 00:47:55 -0400 Subject: [PATCH 04/10] fix(#2834): write defaults.json before agent TOML generation on clean Codex install (#2900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2834): write defaults.json before agent TOML generation on clean Codex install Extracted writeNonClaudeDefaults(runtime) and called it BEFORE installCodexConfig so the runtime-aware model resolver has resolve_model_ids=omit + runtime=codex in ~/.gsd/defaults.json before agent TOMLs are generated. Pre-fix, a clean first Codex install generated TOMLs with no model fields (the resolver didn't know the runtime); a second run fixed it. The original inline defaults-write block (which ran AFTER agent generation) is replaced by the earlier function call (idempotent). * chore(#2834): changeset fragment * fix+test(#2834): acknowledge codex TOML drift (emitted-attribution) + fix test comment window The emitted-attribution gate flags 19 codex agent TOMLs that now carry model-routing fields (the fix's correct effect) but can't link them to a .md or src/ change (the fix is in bin/install.js ordering). Acknowledge the drift in emitted-drift-ack.json. Fix the test's comment-detection window (300 chars to capture the #2834 rationale). * fix(#2834): ack remaining 15 codex TOML drift paths * chore(#2834): backfill changeset PR number (2900) * chore(#2834): ack code-review.md growth from concurrent merge (rebase pickup) * fix(#2834): remove stale code-review.md ack (emitted-attribution failure) CI failed: 'differential attribution over the real tree' — the code-review.md ack added in 6e0b4b3b8 ('ack code-review.md growth from concurrent merge') is STALE: this PR's diff does not touch code-review.md (only bin/install.js + tests + changeset), so the ack explains growth that isn't here. The base already absorbed the concurrent code-review.md growth; the ack is inert here and the gate flags it as stale. Remove it. --------- Co-authored-by: Test (cherry picked from commit 00c859fae5c9be13f13719c5fd4aea86fe71591d) --- .changeset/graceful-lynx-sing.md | 5 + bin/install.js | 102 ++++++++-------- tests/emitted-drift-ack.json | 113 ++++++++++++++++-- ...2834-codex-install-model-ordering.test.cjs | 43 +++++++ 4 files changed, 204 insertions(+), 59 deletions(-) create mode 100644 .changeset/graceful-lynx-sing.md create mode 100644 tests/issue-2834-codex-install-model-ordering.test.cjs diff --git a/.changeset/graceful-lynx-sing.md b/.changeset/graceful-lynx-sing.md new file mode 100644 index 000000000..ea340f3db --- /dev/null +++ b/.changeset/graceful-lynx-sing.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2900 +--- +**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) 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/tests/emitted-drift-ack.json b/tests/emitted-drift-ack.json index ede87875f..5ca42ea3a 100644 --- a/tests/emitted-drift-ack.json +++ b/tests/emitted-drift-ack.json @@ -2,19 +2,118 @@ "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." + "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 \u00a713a 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)." + "reason": "#2772: re-synced to the parent canonical block (had drifted \u2014 lost the 'Other' empty-text branch) + fixed the auto_advance\u2192confirm_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." + "reason": "#2800 \u2014 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." + "reason": "#2800 \u2014 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": "#2667: the fallow structural pre-pass failure WARNING now names the failure KIND (timeout / spawn failure / crash / not-found) via a case statement, so a Windows .cmd spawn failure (CVE-2024-27980) is not mistaken for an absent binary. Growth is the case statement + the explanatory prose." + "agents/gsd-advisor-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-ai-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-assumptions-analyzer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-code-fixer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-code-reviewer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-codebase-mapper.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-debug-session-manager.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-debugger.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-doc-classifier.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-doc-synthesizer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-doc-verifier.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-doc-writer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-domain-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-eval-auditor.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-eval-planner.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-executor.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-framework-selector.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-integration-checker.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-intel-updater.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-mempalace-curator.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-nyquist-auditor.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-pattern-mapper.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-phase-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-plan-checker.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-planner.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-project-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-research-synthesizer.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-roadmapper.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-security-auditor.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-ui-auditor.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-ui-checker.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-ui-researcher.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-user-profiler.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." + }, + "agents/gsd-verifier.toml": { + "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." } } -} +} \ No newline at end of file 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'); +}); From 3aabb0f441949a40c8d5b8340bb48b8652a5dae0 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 01:57:50 -0400 Subject: [PATCH 05/10] fix(#2825): gsd-code-fixer honors workflow.use_worktrees; never rm -rf a reparse point (#2905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2825): gsd-code-fixer honors workflow.use_worktrees; never rm -rf a reparse point gsd-code-fixer was the only writer that hand-rolled a git worktree inside the agent prompt and the only one that never read workflow.use_worktrees. With the setting explicitly false, --fix still created worktrees; the fresh worktree had no node_modules, so runs improvised a teardown whose `rm -rf` followed a Windows junction into the REAL node_modules (silent data loss, 3x observed). Defect 1: gate setup_worktree + its cleanup tail on workflow.use_worktrees (same gsd_run query config-get read the four sibling workflows use). When false: edit/commit in the main checkout (wt='.', no temp branch, no sentinel, no cleanup). Defect 2: forbid rm -rf on a possible reparse point in the spec — never fall through to a destructive remove; on failure, stop and surface the error. Defect 3: REVIEW-FIX records where verification ran (main checkout vs worktree). The transactional worktree path (#2839/#2990/#2686) is unchanged when worktrees are enabled. Docs-parity guards in tests/code-review.test.cjs bind the spec to the fix. * chore(#2825): backfill changeset PR 2905 * fix(#2825): drop stale agents/gsd-code-fixer.toml ack + spent entries (emitted-attribution) gsd-test failed: agents/gsd-code-fixer.toml is a STALE ack here — that TOML was changed by #2834 (now in next), not this PR. The 4 base-carried acks (autonomous/discuss-phase-assumptions/next/plan-phase) are spent/inert. --------- Co-authored-by: Test (cherry picked from commit 5d0fd4dc539c3ffc625fa25cf062a51c7af9d30d) --- .changeset/quick-birds-run.md | 5 ++ agents/gsd-code-fixer.md | 131 ++++++++++++++++++++++------- tests/code-review.test.cjs | 33 ++++++++ tests/emitted-drift-ack.json | 150 ++++++++-------------------------- 4 files changed, 175 insertions(+), 144 deletions(-) create mode 100644 .changeset/quick-birds-run.md diff --git a/.changeset/quick-birds-run.md b/.changeset/quick-birds-run.md new file mode 100644 index 000000000..c1d3260be --- /dev/null +++ b/.changeset/quick-birds-run.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2905 +--- +**`/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. 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/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 5ca42ea3a..1284ce27d 100644 --- a/tests/emitted-drift-ack.json +++ b/tests/emitted-drift-ack.json @@ -1,119 +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 \u00a713a 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 \u2014 lost the 'Other' empty-text branch) + fixed the auto_advance\u2192confirm_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 \u2014 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 \u2014 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." - }, - "agents/gsd-advisor-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-ai-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-assumptions-analyzer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-code-fixer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-code-reviewer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-codebase-mapper.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-debug-session-manager.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-debugger.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-doc-classifier.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-doc-synthesizer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-doc-verifier.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-doc-writer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-domain-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-eval-auditor.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-eval-planner.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-executor.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-framework-selector.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-integration-checker.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-intel-updater.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-mempalace-curator.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-nyquist-auditor.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-pattern-mapper.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-phase-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-plan-checker.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-planner.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-project-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-research-synthesizer.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-roadmapper.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-security-auditor.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-ui-auditor.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-ui-checker.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-ui-researcher.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-user-profiler.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - }, - "agents/gsd-verifier.toml": { - "reason": "#2834: Codex agent TOML now carries model-routing fields on first install." - } + "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." } -} \ No newline at end of file +} From e1b275766d856d677b195a178158b46d55ea4079 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 02:30:50 -0400 Subject: [PATCH 06/10] fix(#2843): findProjectRoot stops at git-repo boundaries, not just MAX_DEPTH (#2909) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2843): findProjectRoot stops at git-repo boundaries, not just MAX_DEPTH findProjectRoot's heuristic (3) used isInsideGitRepo(parent), which only checked 'does SOME .git exist between start and the ancestor' — it never verified the .git was co-located with / bounded the trusted .planning/. A nested child repo (own .git, no .planning) under an ancestor GSD project satisfied the check, so resolution silently crossed into the ancestor project (wrong identity, exit 0). Add nearestGitRoot(from, upTo) (fs-walk, no spawn) and use it in heuristics (3) and (4): if the caller is inside its own nested repo whose root is strictly below the candidate ancestor, do not return that ancestor. The plain-descendant (#1414), co-located .git+.planning, and sub_repos/multiRepo cases are unchanged. Regression test: a nested child .git under an ancestor .planning no longer resolves to the ancestor; the co-located single-repo case still resolves. * chore(#2843): backfill changeset PR 2909 --------- Co-authored-by: Test (cherry picked from commit 39dbe5e0f5c9e55f788535150a934f946dc29f58) --- .changeset/lucky-eagles-hop.md | 5 ++++ src/project-root.cts | 45 ++++++++++++++++++++++++++++++++++ tests/project-root.test.cjs | 40 ++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 .changeset/lucky-eagles-hop.md diff --git a/.changeset/lucky-eagles-hop.md b/.changeset/lucky-eagles-hop.md new file mode 100644 index 000000000..6dcf1c51b --- /dev/null +++ b/.changeset/lucky-eagles-hop.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2909 +--- +**`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. 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/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'); + }); }); From 7112c6ca47753ac34568bd8ff0fdf9cad06bfb6a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 03:16:15 -0400 Subject: [PATCH 07/10] fix(#2844): verify-summary ignores future/prose path mentions; resolves project root (#2910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2844): verify-summary binds file-claim extraction to a creation-claim context verify-summary's Pattern 1 matched any backticked path-like token with no context check, so a prose mention of a future deliverable (`shared/types.ts` in a 'next phase will add…' sentence) was checked for existence and its absence failed the verdict on a healthy phase. #2685 added shape filtering but no context check. - src/verify.cts: both extraction patterns now require a claim label on the line (Created/Modified/Added/Updated/Edited/key-files). A bare prose mention no longer matches; genuine labeled claims still do. - gsd-core/bin/gsd-tools.cjs: remove 'verify-summary' from SKIP_ROOT_RESOLUTION so relative claim paths resolve against the project root, not the raw cwd (subdirectory invocation no longer manufactures missing files). Regression tests: prose mention not treated as a claim; prose-only SUMMARY passes; absent claimed file still fails. * chore(#2844): backfill changeset PR 2910 --------- Co-authored-by: Test (cherry picked from commit 42f4f184c0e2a20579b777d1cbe4af5605dc1b60) --- .changeset/graceful-tigers-chatter.md | 5 +++ gsd-core/bin/gsd-tools.cjs | 6 ++- src/verify.cts | 21 +++++++-- tests/verify.test.cjs | 64 +++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 .changeset/graceful-tigers-chatter.md diff --git a/.changeset/graceful-tigers-chatter.md b/.changeset/graceful-tigers-chatter.md new file mode 100644 index 000000000..2bc47bc5c --- /dev/null +++ b/.changeset/graceful-tigers-chatter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2910 +--- +**`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. diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index baa31cedb..e17a54f75 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -3336,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/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/verify.test.cjs b/tests/verify.test.cjs index c8efb929a..daf3bd66b 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -920,6 +920,70 @@ describe('verify summary command', () => { `Expected checked <= 1, got ${output.checks.files_created.checked}` ); }); + + // #2844: a prose MENTION of a path (not a creation claim) must not be treated + // as a file claim. Pre-fix Pattern 1 matched any backticked path-like token, so + // `shared/types.ts` in a "next phase will add…" sentence was checked for + // existence and its absence failed the verdict on a healthy phase. + test('#2844 a prose path mention is not treated as a missing file claim', () => { + // Real created file exists. + fs.mkdirSync(path.join(tmpDir, 'src'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'src', 'real.ts'), 'export const x = 1;\n'); + const summaryPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-SUMMARY.md'); + fs.writeFileSync(summaryPath, [ + '# Summary', + '', + 'This phase investigated the schema surface.', + '', // PROSE mention — NOT a creation claim; shared/types.ts does NOT exist. + 'Next phase will add `shared/types.ts` for the shared schema.', + '', + 'Created: `src/real.ts`', + ].join('\n')); + + const result = runGsdTools('verify-summary .planning/phases/01-test/01-01-SUMMARY.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.passed, true, + `prose mention must not fail the verdict; errors: ${JSON.stringify(output.errors)}`); + assert.ok(!JSON.stringify(output.checks.files_created.missing).includes('shared/types.ts'), + 'shared/types.ts (a prose mention, absent) must NOT be reported missing'); + }); + + test('#2844 a SUMMARY with only future/prose path mentions passes', () => { + // The mentioned paths are FUTURE deliverables (not produced this phase) and + // are absent — they must not be probed. isFutureMention excludes the lines. + const summaryPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-SUMMARY.md'); + fs.writeFileSync(summaryPath, [ + '# Summary', + '', + 'Investigation only. No artifacts created this phase.', + '`docs/schema.md` is planned for a later phase.', + 'Next phase will add `shared/types.ts` for the shared schema.', + ].join('\n')); + + const result = runGsdTools('verify-summary .planning/phases/01-test/01-01-SUMMARY.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.passed, true, + `future/prose mentions must not fail the verdict; errors: ${JSON.stringify(output.errors)}`); + }); + + test('#2844 negative-space: a real Created claim for an ABSENT file still fails', () => { + // src/missing.ts is claimed but does NOT exist — must still be caught. + const summaryPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-SUMMARY.md'); + fs.writeFileSync(summaryPath, [ + '# Summary', + '', + 'Created: `src/missing.ts`', + ].join('\n')); + + const result = runGsdTools('verify-summary .planning/phases/01-test/01-01-SUMMARY.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.passed, false, 'an absent claimed file must fail the verdict'); + assert.ok(JSON.stringify(output.checks.files_created.missing).includes('src/missing.ts'), + 'src/missing.ts must be reported missing'); + }); }); // ───────────────────────────────────────────────────────────────────────────── From f72f70ad3985d7ffd28fefd86a8f9ca9b6f0d5ac Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 07:25:01 -0400 Subject: [PATCH 08/10] docs(#2782): how-to for declaring a reviewer lane in a capability (#2906) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(#2782): how-to for declaring a reviewer lane in a capability ADR-2782 shipped the Reviewer Lane capability surface in 1.9.0, but the only documentation was reference (capability-manifest.md) and rationale (ADR-2782). A capability author had no task-oriented path from "I have a review CLI" to "/gsd:review invokes it". Adds docs/how-to/ship-a-reviewer-lane.md: role selection, the spawn and openai-http worked examples, federated config ownership, the review-lane query surface as the verification step, what the install disclosure and egress-host re-verification mean for the author, and the data-only boundary with the two named CLIs that do not fit today. Also corrects the manifest reference's `invoke` row, which understated three enums against the shipped validator: promptChannel omitted `argv`, effortChannel omitted `env`, and the openai-http sub-shape plus the required-with-file-arg `outputArg` were undocumented entirely. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2782): correct four claims found by the two orthogonal reviews Security review (1 major, 2 minor): - "a reserved slug" implied the gsd-/anthropic- namespace rule, which guards the capability id, not reviewer.slug. The slug guard is isReservedName (__proto__/constructor/prototype), a prototype-pollution barrier. Both rules are now stated and kept apart. - Added ADR-2782 D5's own caveat verbatim: disclosure and host pinning make the channel visible, pinned and revocable, not safe, and consent-at-install is a weaker gate for a standing egress channel than for a hook. - Named integrity/SHA pinning and engines.gsd as the controls that make the disclosure tamper-evident and the version range enforceable. Correctness review (1 major, 1 minor): - Claimed a name collision is "a hard failure at install". It is not. installCapability never runs validateCrossCapability; the check runs in loadRegistry, and a colliding overlay is dropped from acceptedMap with a warning while the install reports success. Documented as the quiet failure mode it is, with the symptom to look for. - The feature-only field list omitted hooks and activationKey, both of which FEATURE_FIELDS_FORBIDDEN_ON_REVIEWER rejects. Both worked examples re-validated against validateCapability() -> []. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2782): restore Kimi Code to the cross-AI reviewer list set-up-cross-ai-review.md named eleven reviewers; twelve lanes ship. The kimi-code lane (added by #2718, declared as manifest data by #2798) was never added here — the same roster-drift class as #2781, which #2800's parity gate covers for COMMANDS.md and FEATURES.md but not for how-to prose. Also points readers at the declared-lane model rather than a static list: the roster is now generated, a capability can ship its own lane, and `gsd-tools review-lane sections` answers "what do I actually have". Verified against the twelve declared bodies in capabilities/*/capability.json. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2782): use the invocation form the runtime descriptors actually declare The new guide used /gsd:review. Nothing in this repo produces that form. - capability-validator VALID_COMMAND_STYLES is {slash-hyphen, shell-var}; there is no colon/namespaced style in the vocabulary at all. - 18 of 19 runtime capabilities declare commandStyle "slash-hyphen", claude included; codex is "shell-var". Every artifactLayout prefix is "gsd-". - A plain-file command install never namespaces, so .claude/commands/ gsd-review.md is typed /gsd-review. - The Claude Code plugin surface would namespace on plugin.json "name", which is "gsd-core" -- so the plugin form would be /gsd-core:review. The commands/gsd/ subdirectory is cosmetic and contributes nothing to the invoked name. So /gsd:review is neither the installed form nor the plugin form. Uses /gsd-review, matching set-up-cross-ai-review.md and the 18 descriptors. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2782): state that lane listing is not wired yet The guide could describe authoring, validating, and installing a lane but implied a publishing path that does not exist. Neither discoverability catalog can accept one: a Community Capability Registry entry requires a non-empty loopExtensionPoints plus hookKinds, and a role:"reviewer" capability is forbidden from declaring steps/contributions/gates, so both fields are unsatisfiable rather than merely unset. The EoS Registry is ADR-1239 host integrations, which a lane is not. The registry schema predates the reviewer role by 17 days (#2182 Jul 11, ADR-2782 Jul 28) and registry-schema.cjs has zero occurrences of "reviewer". Tracked for a 1.9.x point release by #2904. Says so explicitly, and tells authors NOT to file a loop extension point they do not use to get past validation -- a schema satisfiable only by lying is one that will be lied to. Co-Authored-By: Claude Opus 5 (1M context) * chore(#2782): backfill PR number and fix the changeset invocation form pr: 0 -> 2906. Also corrects /gsd:review -> /gsd-review in the fragment body. The fragment renders into CHANGELOG.md, which is a reader-facing docs surface and is never passed through the install-time converter -- so the colon form would ship the #2903 drift into a permanent release artifact. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2907): propagate the invocation-form fix and restore lane order Two findings from an isolated review of the post-review delta. - docs/README.md and develop-a-capability.md still said /gsd:review in the cross-links added for the new guide. The form was corrected in the guide itself but not in the two entries pointing at it, leaving three docs making the same claim in two different forms. - set-up-cross-ai-review.md inserted Kimi Code between Antigravity and Ollama. REVIEWER_LANES is ordered by write_reviews order and kimi-code is 12th, appended after llama_cpp; the prose list mirrored declaration order before this change and now does again. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Test Co-authored-by: Claude Opus 5 (1M context) (cherry picked from commit 557d46984e6d00874949b6866fa14f428436f205) --- .changeset/humble-wolves-run.md | 5 + docs/README.md | 1 + docs/how-to/develop-a-capability.md | 1 + docs/how-to/set-up-cross-ai-review.md | 5 +- docs/how-to/ship-a-reviewer-lane.md | 245 ++++++++++++++++++++++++++ docs/reference/capability-manifest.md | 4 +- 6 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 .changeset/humble-wolves-run.md create mode 100644 docs/how-to/ship-a-reviewer-lane.md diff --git a/.changeset/humble-wolves-run.md b/.changeset/humble-wolves-run.md new file mode 100644 index 000000000..7ecbcf54c --- /dev/null +++ b/.changeset/humble-wolves-run.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 2906 +--- +**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) 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. | From 538cb0fc1da499069005ba5045e211b62a500d27 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 08:11:57 -0400 Subject: [PATCH 09/10] enh(#2904): add a `reviewer` entry type so third-party reviewer lanes are discoverable (#2912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#2904): add a `reviewer` entry type so third-party reviewer lanes are discoverable ADR-2782 made a reviewer lane installable by a third party, but neither discoverability catalog could hold one. The Community Capability Registry requires a non-empty `loopExtensionPoints` and forbids a lane from declaring any hook kind, so a `role: "reviewer"` entry is unsatisfiable by construction; the EoS Registry is for ADR-1239 host integrations, which a lane is not. Adds a third catalog — `docs/registries/reviewers.json` → `docs/registries/reviewer-registry.md` — whose `interactions` describes the lane: slug, flags, transport, evidenceClass, reviewsSection, requiresBinaries, configKeys, runtimeCompat. The lane vocabulary is a hand-written mirror of `capability-validator.cjs` (the same pattern as `AXES` mirroring `HOST_INTEGRATION_AXES`), with parity enforced by tests/registry-reviewer-parity.test.cjs. `slug` deliberately uses the runtime `LANE_SLUG_RE` grammar rather than the registry's kebab-only `id` rule, so real lanes (`lm_studio`, `4o-mini`) are not rejected. Two binary type branches became three-way Map dispatch. Both now fail loudly on an unrecognized type instead of silently treating it as a capability — `renderMarkdown` in particular writes a committed catalog file, so a silent wrong-title render was the worst failure mode available. Also fixed while here: `gen-registry.cjs` parsed source JSON with no error handling, so a malformed or non-array `capabilities.json` surfaced as a raw SyntaxError/TypeError instead of an actionable CLI error. Closes #2904 * fix(#2904): bound and sanitize untrusted registry `interactions` strings Review findings from the pre-PR passes. Security (isolated pass): `interactions` string fields reached the generated, committed Markdown catalog with no control-character check and no length bound. A `reviewsSection` carrying ESC and a `requiresBinaries` element carrying NUL plus 5000 characters validated clean and landed verbatim in the rendered page — `mdInline` escapes Markdown metacharacters and collapses CRLF, but nothing else. The identical gap already existed on the capability type's `configKeys`/`requires`/`runtimeCompat`/`produces`/`consumes`, so it is fixed there too rather than inherited into a third type. `hasDisallowedControlChar` is lifted to module scope so exactly one implementation exists, and a shared `validateStringArrayField` enforces control-character rejection, a 200-character element cap and a 50-element array cap for both types. Correctness (standards pass): `renderMarkdown`'s per-entry summary builder was still an if/else-if chain whose final `else` was the capability branch — the one per-type dispatch point this change had not converted, and the same silent fallthrough it removes elsewhere. It now lives in `RENDER_META` alongside the title, so a fourth type cannot silently inherit capability's rendering. All three types' rendered output is byte-identical to before the refactor. Also corrects a test comment that still claimed the reviewer suites were failing-first against an unmodified module. * chore(#2904): backfill changeset PR number (#2912) (cherry picked from commit 90771ddf026d6e0dbf9bf897b7c54b60b6a97da4) --- .changeset/merry-seals-climb.md | 5 + .../PULL_REQUEST_TEMPLATE/registry-entry.md | 6 +- CONTEXT.md | 5 +- docs/registries/README.md | 83 +- docs/registries/reviewer-registry.md | 9 + docs/registries/reviewers.json | 1 + scripts/gen-registry.cjs | 54 +- scripts/registry-schema.cjs | 419 +++++++--- scripts/validate-registry.cjs | 16 +- tests/gen-registry.test.cjs | 190 +++++ tests/registry-reviewer-parity.test.cjs | 153 ++++ tests/registry-schema.test.cjs | 769 ++++++++++++++++++ tests/validate-registry.test.cjs | 188 +++++ 13 files changed, 1768 insertions(+), 130 deletions(-) create mode 100644 .changeset/merry-seals-climb.md create mode 100644 docs/registries/reviewer-registry.md create mode 100644 docs/registries/reviewers.json create mode 100644 tests/registry-reviewer-parity.test.cjs diff --git a/.changeset/merry-seals-climb.md b/.changeset/merry-seals-climb.md new file mode 100644 index 000000000..3c686b902 --- /dev/null +++ b/.changeset/merry-seals-climb.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 2912 +--- +**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) 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/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/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/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/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/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```