diff --git a/.changeset/gallant-pandas-greet.md b/.changeset/gallant-pandas-greet.md new file mode 100644 index 000000000..c9cb58ae7 --- /dev/null +++ b/.changeset/gallant-pandas-greet.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2882 +--- +**Reviewer lane flags and section titles are now gated across every documentation surface** — `/gsd:review` reviewer flags were hand-enumerated in five docs and three workflow files that had silently drifted apart: `--kimi-code` was missing from all four translated `COMMANDS.md` mirrors, `--coderabbit` from every workflow forwarding list, and `--antigravity` from `FEATURES.md` entirely. The lane roster is now the single declared source: workflows derive their flag lists from a new `review-lane flags` query, and a parity gate fails the build when any documented flag or reviewer section title diverges from it. The capability manifest reference also gains the previously undocumented `reviewer` body and `hostBehaviors` field. (#2800) diff --git a/CONTEXT.md b/CONTEXT.md index 3b3c88293..986008b20 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -110,7 +110,7 @@ Module owning project-root resolution from any starting directory. Walks the anc Module owning projection from project/workstream context to concrete `.planning` paths. Policy precedence is `explicit workstream > env workstream > env project > root`. Invalid workspace context is a validation error at this seam rather than a silent fallback. ### Reviewer Lane Descriptor Module -Module owning the single declared contract for a **reviewer lane** — one external CLI or model endpoint that `/gsd:review` hands a plan to for independent review (ADR-2782 Phase 1, #2794; subsumes #2690). Before it, a lane was declared across three unrelated surfaces — the roster in `src/review-reviewer-selection.cts`, ~640 lines of hand-authored per-CLI bash in the `invoke_reviewers` step of `gsd-core/workflows/review.md`, and the hardcoded section headings in `write_reviews` — so cross-cutting fixes landed per-leg (#2494 and #2605 were the same empty-output defect filed twice; #2475/#2295/#2272 are the same shape). Interface: `REVIEWER_LANES` (frozen table of the 11 shipped lanes, each declaring `slug`, `flags`, `probe`, `invoke`, `timeoutFloorMs`, `emptyOutput`, `reviewsSection`, `evidenceClass`, `requiresBinaries`, `promptBudgetKey`, `handler`), `PARITY_VIOLATION` (frozen reason enum — adding a reason is three coordinated changes: enum + emitting site + the test locking `Object.keys(...).sort()`), and `checkReviewerLaneParity({descriptor, roster, workflowText}) → {ok, violations[]}`. **The module DECLARES; it does not execute** — `invoke_reviewers` still runs hand-authored legs until Phase 5b (#2799) makes it iterate; what Phase 1 guarantees is that a leg cannot be added, removed, or renamed without the table and the `REVIEWS.md` section moving with it. Field names, **nesting**, and enum members track ADR-2782 D1/D2/D6/D7 so Phase 2 (#2795) harvests the shape into the capability manifest with no translation layer — including `transport` at the **lane level** (a sibling of `probe`/`invoke`, as D1's manifest example places it) rather than nested inside `invoke`, which would read more naturally as a TS discriminated union and is exactly the convenience Phase 2 would have to translate away. **Four** vocabulary widenings were forced by surveying the eleven shipped legs, all additive widenings of closed enums, each forced by a lane that exists today — and **ADR-2782 was amended in the same PR** (its Amendments section, 2026-07-29) rather than left diverging, so Phase 2 implements the validator against the amended vocabulary: `promptChannel: 'none'` (CodeRabbit is fed no prompt — it reviews the working-tree diff), `outputChannel: 'file-arg'` (Codex captures the review through its own `-o/--output-last-message` and discards stdout, #1698), `outputArg` (its companion — knowing the review lands in a file is useless without the argument naming the file), and `flags: string[]` where D1 shows a singular `flag` (Antigravity is selected by both `--antigravity` and `--agy`, which one field cannot express; this also widens D8's uniqueness invariant, enforced here over the flattened flag set). `LANE_SLUG_RE` (`^[a-z0-9][a-z0-9_-]*$`) pins the slug grammar because `LEG_MARKER_RE` can only capture `[a-z0-9_-]`: a slug outside that class is **unmatchable** — its marker can be present and correct and the scan still never sees it — so a violating slug is reported `INVALID_SLUG` rather than reported missing forever. A loud named violation beats a silent miss. The descriptor deliberately does **not** promise uniformity — lane divergence is real and frequently correct (three lanes are HTTP endpoints with no binary; timeout floors genuinely differ; Antigravity needs a three-layer fallback for an upstream stdout bug) — so behavior data cannot express is delegated to a named `handler` (closed first-party enum: `null` | `antigravity` | `openai-compatible`), never to conditionals inside the table. `checkReviewerLaneParity` is the `DEFECT.GENERATIVE-FIX` assertion the roster has never had, and it is **bidirectional**: a forward-only check misses the failure it exists to catch (#2718 added a lane leg, #2781 was the drift that followed), so an undeclared leg fails too. Legs are identified by an explicit `` marker rather than inferred from prose shape, because five non-lane bold labels in `invoke_reviewers` share the bold-then-fence shape a heuristic would key on. Section matching is anchored at h2 with an exact ` Review` suffix and no parenthetical, so ADR-1517 reviewer-instance headings (`## OpenCode Review (opencode-deepseek)`) are exempt — ADR-2782 D8: instances are not lanes. Pure and total: no filesystem access, CRLF-insensitive, and **never throws on any input** — every field is validated before use (`MALFORMED_LANE` / `INVALID_SLUG`) rather than trusted, because Phase 2 feeds this same function manifest-derived data from third-party overlays, and a parity gate that crashes on bad input is indistinguishable from one that was never run. Empty input degrades to violations so a read failure is never mistaken for a clean bill of health. Totality, determinism (no leaked regex `lastIndex`), and the invalid-slug contract are `fast-check` property-tested with a pinned seed. Source of truth: `src/review-lane-descriptor.cts`. Test anchor: `tests/review-lane-descriptor.test.cjs`. See `docs/adr/2782-reviewer-lane-capability-surface.md`. +Module owning the single declared contract for a **reviewer lane** — one external CLI or model endpoint that `/gsd:review` hands a plan to for independent review (ADR-2782 Phase 1, #2794; subsumes #2690). Before it, a lane was declared across three unrelated surfaces — the roster in `src/review-reviewer-selection.cts`, ~640 lines of hand-authored per-CLI bash in the `invoke_reviewers` step of `gsd-core/workflows/review.md`, and the hardcoded section headings in `write_reviews` — so cross-cutting fixes landed per-leg (#2494 and #2605 were the same empty-output defect filed twice; #2475/#2295/#2272 are the same shape). Interface: `REVIEWER_LANES` (frozen table of the 11 shipped lanes, each declaring `slug`, `flags`, `probe`, `invoke`, `timeoutFloorMs`, `emptyOutput`, `reviewsSection`, `evidenceClass`, `requiresBinaries`, `promptBudgetKey`, `handler`), `PARITY_VIOLATION` (frozen reason enum — adding a reason is three coordinated changes: enum + emitting site + the test locking `Object.keys(...).sort()`), and `checkReviewerLaneParity({descriptor, roster, workflowText}) → {ok, violations[]}`. **The module DECLARES; it does not execute** — `invoke_reviewers` still runs hand-authored legs until Phase 5b (#2799) makes it iterate; what Phase 1 guarantees is that a leg cannot be added, removed, or renamed without the table and the `REVIEWS.md` section moving with it. Field names, **nesting**, and enum members track ADR-2782 D1/D2/D6/D7 so Phase 2 (#2795) harvests the shape into the capability manifest with no translation layer — including `transport` at the **lane level** (a sibling of `probe`/`invoke`, as D1's manifest example places it) rather than nested inside `invoke`, which would read more naturally as a TS discriminated union and is exactly the convenience Phase 2 would have to translate away. **Four** vocabulary widenings were forced by surveying the eleven shipped legs, all additive widenings of closed enums, each forced by a lane that exists today — and **ADR-2782 was amended in the same PR** (its Amendments section, 2026-07-29) rather than left diverging, so Phase 2 implements the validator against the amended vocabulary: `promptChannel: 'none'` (CodeRabbit is fed no prompt — it reviews the working-tree diff), `outputChannel: 'file-arg'` (Codex captures the review through its own `-o/--output-last-message` and discards stdout, #1698), `outputArg` (its companion — knowing the review lands in a file is useless without the argument naming the file), and `flags: string[]` where D1 shows a singular `flag` (Antigravity is selected by both `--antigravity` and `--agy`, which one field cannot express; this also widens D8's uniqueness invariant, enforced here over the flattened flag set). `LANE_SLUG_RE` (`^[a-z0-9][a-z0-9_-]*$`) pins the slug grammar because `LEG_MARKER_RE` can only capture `[a-z0-9_-]`: a slug outside that class is **unmatchable** — its marker can be present and correct and the scan still never sees it — so a violating slug is reported `INVALID_SLUG` rather than reported missing forever. A loud named violation beats a silent miss. The descriptor deliberately does **not** promise uniformity — lane divergence is real and frequently correct (three lanes are HTTP endpoints with no binary; timeout floors genuinely differ; Antigravity needs a three-layer fallback for an upstream stdout bug) — so behavior data cannot express is delegated to a named `handler` (closed first-party enum: `null` | `antigravity` | `openai-compatible`), never to conditionals inside the table. `checkReviewerLaneParity` is the `DEFECT.GENERATIVE-FIX` assertion the roster has never had, and it is **bidirectional**: a forward-only check misses the failure it exists to catch (#2718 added a lane leg, #2781 was the drift that followed), so an undeclared leg fails too. Legs are identified by an explicit `` marker rather than inferred from prose shape, because five non-lane bold labels in `invoke_reviewers` share the bold-then-fence shape a heuristic would key on. Section matching is anchored at h2 with an exact ` Review` suffix and no parenthetical, so ADR-1517 reviewer-instance headings (`## OpenCode Review (opencode-deepseek)`) are exempt — ADR-2782 D8: instances are not lanes. Pure and total: no filesystem access, CRLF-insensitive, and **never throws on any input** — every field is validated before use (`MALFORMED_LANE` / `INVALID_SLUG`) rather than trusted, because Phase 2 feeds this same function manifest-derived data from third-party overlays, and a parity gate that crashes on bad input is indistinguishable from one that was never run. Empty input degrades to violations so a read failure is never mistaken for a clean bill of health. Totality, determinism (no leaked regex `lastIndex`), and the invalid-slug contract are `fast-check` property-tested with a pinned seed. Phase 6 (#2800) adds a SECOND, deliberately separate pure gate in the same module — `checkReviewerDocsParity({descriptor, docs}) → {ok, violations, skipped}` with its own frozen `DOCS_PARITY_VIOLATION` enum — answering *what is documented* rather than *what runs*, so a stale doc can never make the runtime checker look red and the seven dependents of `checkReviewerLaneParity` never move. It is the `DEFECT.GENERATIVE-FIX` parity assertion this roster had always required and never had (only the Cursor lane ever carried one). Three independent arms: declared flags appear delimited (backticked or bracketed, so a bare flag in a fenced example cannot satisfy the gate) in `docs/COMMANDS.md`, `docs/FEATURES.md` and their four locale mirrors; a `Command:` signature line is held to the full roster and rejects undeclared bracketed lane flags; and the Purpose paragraph beneath it must name every declared `reviewsSection` — the arm that catches the class where a `Command:` line is updated and the line below it is not. Section titles are matched LITERALLY (`llama.cpp` would otherwise let `llamaXcpp` pass). A mirror carrying no `/gsd:review` surface is reported in `skipped`, never failed, so a partial translation is not misread as drift. Its totality is property-tested too, which is how the non-callable-`toString` coercion crash was found before it shipped. Source of truth: `src/review-lane-descriptor.cts`. Test anchors: `tests/review-lane-descriptor.test.cjs`, `tests/reviewer-docs-parity.test.cjs`. See `docs/adr/2782-reviewer-lane-capability-surface.md`. ### Reviewer Lane Invocation Module Module owning the projection from a **declared reviewer lane** plus resolved configuration to a concrete **invocation plan** — the value a lane is actually run from (ADR-2782 Phase 5b, #2799). Pure: no filesystem, network, subprocess or clock; configuration arrives through a `configGet` seam. Interface: `resolveLanePlan({lane, configGet, runDir, repoRoot, effortArgs}) → {ok, plan} | {ok:false, reason, detail}`, `LANE_UNAVAILABLE` (frozen reason enum — a lane that will not run reports WHY, because the ambiguity between "failed" and "ran cleanly with nothing to report" is the defect class this epic closes), plus `isEmptyReview`, `normalizeHost` and `fileRefPrompt`. TOTAL — a malformed lane yields an unavailable result, never a throw, because third-party overlay manifests reach this seam. `invoke.args` is an **argv template** over a closed four-member placeholder vocabulary (`{{model}}`, `{{effort}}`, `{{output}}`, `{{prompt}}`), not a prefix: the injected pieces do not all go in the same place — `codex` injects the model after its `exec` subcommand and the output file later still, while five lanes end with a bare `-` that must stay last. Each lane's plan was derived FROM its former bash leg, and a golden table asserts all twelve; that table is the strangler-fig substitute for a parallel run. diff --git a/commands/gsd/plan-review-convergence.md b/commands/gsd/plan-review-convergence.md index 4070620e1..736e60362 100644 --- a/commands/gsd/plan-review-convergence.md +++ b/commands/gsd/plan-review-convergence.md @@ -1,7 +1,7 @@ --- name: gsd:plan-review-convergence description: "Cross-AI plan convergence - replan until review concerns are resolved." -argument-hint: " [--codex] [--gemini] [--claude] [--opencode] [--ollama] [--lm-studio] [--llama-cpp] [--agy] [--text] [--ws ] [--all] [--max-cycles N]" +argument-hint: " [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--antigravity] [--agy] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--text] [--ws ] [--all] [--max-cycles N]" allowed-tools: - Read - Write @@ -44,10 +44,14 @@ Phase number: extracted from $ARGUMENTS (required) - `--gemini` — Use Gemini CLI as reviewer - `--agy` / `--antigravity` — Use Antigravity CLI as reviewer (successor to the discontinued Gemini CLI) - `--claude` — Use Claude CLI as reviewer (separate session) +- `--coderabbit` — Use CodeRabbit as reviewer (reviews the working-tree diff, not the source tree) - `--opencode` — Use OpenCode as reviewer +- `--qwen` — Use Qwen Code CLI as reviewer (Alibaba Qwen models) +- `--cursor` — Use Cursor agent as reviewer - `--ollama` — Use local Ollama server as reviewer (OpenAI-compatible, default host `http://localhost:11434`; configure model via `review.models.ollama`) - `--lm-studio` — Use local LM Studio server as reviewer (OpenAI-compatible, default host `http://localhost:1234`; configure model via `review.models.lm_studio`) - `--llama-cpp` — Use local llama.cpp server as reviewer (OpenAI-compatible, default host `http://localhost:8080`; configure model via `review.models.llama_cpp`) +- `--kimi-code` — Use Kimi Code CLI as reviewer (Moonshot AI) - `--all` — Use all available CLIs and running local model servers - `--max-cycles N` — Maximum replan→review cycles (default: 3) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index db3d2d9de..26d7ec734 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -263,7 +263,7 @@ Cross-AI plan convergence loop — replan with review feedback until no HIGH con | Argument / Flag | Required | Description | |-----------------|----------|-------------| | `N` | **Yes** | Phase number to plan and review | -| `--codex` / `--gemini` / `--claude` / `--opencode` | No | Single-reviewer selection | +| Reviewer flags | No | Pass through every reviewer lane flag: `--gemini`, `--claude`, `--codex`, `--coderabbit`, `--opencode`, `--qwen`, `--cursor`, `--agy` / `--antigravity`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--kimi-code` | | `--all` | No | Run every configured reviewer in parallel | | `--max-cycles N` | No | Override cycle cap (default 3) | @@ -628,7 +628,7 @@ Show status, next steps, and automatically advance to the next logical workflow | `--next --auto` | Like `--next`, but chains steps automatically until milestone completion or a blocking decision | | `--next --converge` | When the next action is planning, route it through `/gsd-plan-review-convergence`; requires `workflow.plan_review_convergence=true` | | `--cross-ai` | Alias for `--converge` | -| Reviewer flags | With `--converge`, pass through `--codex`, `--gemini`, `--claude`, `--opencode`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--all`, and `--max-cycles N` | +| Reviewer flags | With `--converge`, pass through every reviewer lane flag: `--gemini`, `--claude`, `--codex`, `--coderabbit`, `--opencode`, `--qwen`, `--cursor`, `--agy` / `--antigravity`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--kimi-code`, `--all`, and `--max-cycles N` | | `--do "task description"` | Analyze freeform intent and dispatch to the most appropriate GSD command | | `--forensic` | Append a 6-check integrity audit after the standard report (STATE consistency, orphaned handoffs, deferred scope drift, memory-flagged pending work, blocking todos, uncommitted code) | @@ -858,7 +858,7 @@ Run all remaining phases autonomously. | `--interactive` | Lean context with user input | | `--converge` | Route each planning step through `/gsd-plan-review-convergence`; requires `workflow.plan_review_convergence=true` | | `--cross-ai` | Alias for `--converge` | -| Reviewer flags | With `--converge`, pass through `--codex`, `--gemini`, `--claude`, `--opencode`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--all`, and `--max-cycles N` | +| Reviewer flags | With `--converge`, pass through every reviewer lane flag: `--gemini`, `--claude`, `--codex`, `--coderabbit`, `--opencode`, `--qwen`, `--cursor`, `--agy` / `--antigravity`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--kimi-code`, `--all`, and `--max-cycles N` | | `--text` | Replace `AskUserQuestion` prompts with plain numbered lists | ```bash diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 4f038fe23..d9c069e70 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -1242,7 +1242,7 @@ When verification returns `human_needed`, items are persisted as a trackable HUM ### 42. Cross-AI Peer Review -**Command:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--ollama] [--lm-studio] [--llama-cpp] [--all]` +**Command:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--antigravity] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--all]` **Purpose:** Invoke external AI CLIs (Gemini, Claude, Codex, CodeRabbit, OpenCode, Qwen Code, Cursor, Antigravity, Kimi Code) and local OpenAI-compatible servers (Ollama, LM Studio, llama.cpp) to independently review phase plans. Produces structured REVIEWS.md with per-reviewer feedback. diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index e379a1227..9cc6d5f92 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -507,6 +507,9 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `research-store.cjs` | Content-addressed research cache: sha256 keys, per-source TTL staleness, two-tier (user ~/.gsd / project .planning) store | | `probe-core.cjs` | Generic spec-phase probe resolution model (compiled from `src/probe-core.cts`, gitignored; ADR-550 Decision 7) — the status×verification re-cut (`status: resolved/dismissed/unresolved` × per-probe `verification`), `validateResolution`/`validateRequirement`, `analyzeCoverage(items, resolutions?, validators)` merge/rollup/orphan-reject, the `byVerification` rollup, and the `runProbeCli` I/O scaffold; the shared seam consumed by `edge-probe` (and the prohibition probe #644); exports `VALID_STATUS`, `validateResolution`, `validateRequirement`, `analyzeCoverage`, `runProbeCli` (#550) | | `prohibition-enforcement.cjs` | Deterministic test-tier prohibition PRODUCER/gate (compiled from `src/prohibition-enforcement.cts`, gitignored; #1259, ADR-550 D5d "heavy half") — locates the wired mechanical check (`node-test` or `lint-rule`), confirms it is fail-first, runs it via an injectable runner, builds typed `enforcementEvidence`, and emits the `dispositionForProhibition` verdict; a passing wired check disposes green, a missing/failing/non-fail-first check hard-gates (flagged, non-green) in both interactive and autonomous modes; exports `runProhibitionEnforcement`, `routeProhibitionEnforcement`; CLI surface `gsd_run check prohibition-enforcement ` | +| `review-lane-descriptor.cjs` | Declared reviewer-lane contract (compiled from `src/review-lane-descriptor.cts`, gitignored; ADR-2782) — the frozen `REVIEWER_LANES` roster, the lane slug grammar, and two pure parity gates: `checkReviewerLaneParity` (descriptor ↔ roster ↔ registry, plus anti-parity against re-added bespoke workflow legs) and `checkReviewerDocsParity` (declared flags and section titles ↔ `docs/COMMANDS.md`, `docs/FEATURES.md` and their locale mirrors; #2800, closes #2781/#2272); exports `REVIEWER_LANES`, `PARITY_VIOLATION`, `DOCS_PARITY_VIOLATION`, `LANE_SLUG_RE` | +| `review-lane-invocation.cjs` | Pure projection from a declared reviewer lane plus resolved config to a concrete invocation plan (compiled from `src/review-lane-invocation.cts`, gitignored; ADR-2782 Phase 5b) — no filesystem, network or clock; config arrives through a `configGet` seam; exports `resolveLanePlan`, `LANE_UNAVAILABLE` | +| `review-lane-runner.cjs` | Execution of a reviewer-lane invocation plan (compiled from `src/review-lane-runner.cts`, gitignored; ADR-2782 Phase 5b) — probe, spawn or HTTP call, empty-output policy, egress-host check, and dispatch of the three first-party `handler` modules; exports `runLane`, `probeLane`, `checkEgressHost`, `writeReviewOrStub` | | `review-reviewer-selection.cjs` | Reviewer selection/normalization helpers for `/gsd-review` default reviewer policy and precedence | | `roadmap-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools roadmap` | | `roadmap-parser.cjs` | ROADMAP.md parsing — milestone slicing, current-milestone extraction, phase/milestone lookups, milestone-phase filter (extracted from `core.cjs`, ADR-857) | diff --git a/docs/ja-JP/COMMANDS.md b/docs/ja-JP/COMMANDS.md index 02b448508..8998a142f 100644 --- a/docs/ja-JP/COMMANDS.md +++ b/docs/ja-JP/COMMANDS.md @@ -218,7 +218,7 @@ WebSearch から取得したパッケージは `[ASSUMED]`（`[VERIFIED]` では | 引数 / フラグ | 必須 | 説明 | |-----------------|----------|-------------| | `N` | **Yes** | 計画およびレビューするフェーズ番号 | -| `--codex` / `--gemini` / `--claude` / `--opencode` | No | 単一レビュアーの選択 | +| レビュアーフラグ | No | すべてのレビュアーレーンフラグをそのまま渡す: `--gemini`、`--claude`、`--codex`、`--coderabbit`、`--opencode`、`--qwen`、`--cursor`、`--agy` / `--antigravity`、`--ollama`、`--lm-studio`、`--llama-cpp`、`--kimi-code` | | `--all` | No | 設定済みのすべてのレビュアーを並列で実行 | | `--max-cycles N` | No | サイクル上限を上書き（デフォルト3） | @@ -1244,6 +1244,7 @@ AI システムの構築を含むフェーズの AI-SPEC.md デザインコン | `--qwen` | Qwen Code レビューを含める（Alibaba Qwen モデル） | | `--cursor` | Cursor エージェントレビューを含める | | `--agy` / `--antigravity` | Antigravity CLI レビューを含める（Google 認証情報で無料） | +| `--kimi-code` | Kimi Code CLI レビューを含める（Moonshot AI） | | `--ollama` | Ollama サーバーレビューを含める | | `--lm-studio` | LM Studio サーバーレビューを含める | | `--llama-cpp` | llama.cpp サーバーレビューを含める | diff --git a/docs/ja-JP/FEATURES.md b/docs/ja-JP/FEATURES.md index 499f8c7af..82edbcabb 100644 --- a/docs/ja-JP/FEATURES.md +++ b/docs/ja-JP/FEATURES.md @@ -1163,9 +1163,9 @@ fix(03-01): correct auth token expiry ### 42. クロス AI ピアレビュー -**コマンド:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--all]` +**コマンド:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--antigravity] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--all]` -**目的:** 外部の AI CLI（Gemini、Claude、Codex、CodeRabbit、OpenCode、Qwen Code、Cursor、Antigravity）を呼び出して、フェーズプランを独立してレビューします。レビュアーごとのフィードバックを含む構造化された REVIEWS.md を生成します。 +**目的:** 外部の AI CLI（Gemini、Claude、Codex、CodeRabbit、OpenCode、Qwen Code、Cursor、Antigravity、Kimi Code）とローカルの OpenAI 互換サーバー（Ollama、LM Studio、llama.cpp）を呼び出して、フェーズプランを独立してレビューします。レビュアーごとのフィードバックを含む構造化された REVIEWS.md を生成します。 **要件:** - REQ-REVIEW-01: システムはシステム上で利用可能な AI CLI を検出しなければならない diff --git a/docs/ko-KR/COMMANDS.md b/docs/ko-KR/COMMANDS.md index 313b0b1fb..57d0bb5d0 100644 --- a/docs/ko-KR/COMMANDS.md +++ b/docs/ko-KR/COMMANDS.md @@ -218,7 +218,7 @@ WebSearch에서 가져온 패키지는 `[ASSUMED]`(`[VERIFIED]`가 아님)로 | 인수 / 플래그 | 필수 | 설명 | |-----------------|----------|-------------| | `N` | **예** | 계획 및 리뷰할 단계 번호 | -| `--codex` / `--gemini` / `--claude` / `--opencode` | 아니요 | 단일 리뷰어 선택 | +| 리뷰어 플래그 | 아니요 | 모든 리뷰어 레인 플래그를 그대로 전달: `--gemini`, `--claude`, `--codex`, `--coderabbit`, `--opencode`, `--qwen`, `--cursor`, `--agy` / `--antigravity`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--kimi-code` | | `--all` | 아니요 | 구성된 모든 리뷰어를 병렬로 실행 | | `--max-cycles N` | 아니요 | 사이클 상한 재정의 (기본값 3) | @@ -1250,6 +1250,7 @@ AI 시스템 구축을 포함하는 단계에 대한 AI-SPEC.md 디자인 계약 | `--qwen` | Qwen Code 검토 포함 (Alibaba Qwen 모델) | | `--cursor` | Cursor 에이전트 검토 포함 | | `--agy` / `--antigravity` | Antigravity CLI 검토 포함 (Google 자격증명으로 무료) | +| `--kimi-code` | Kimi Code CLI 검토 포함 (Moonshot AI) | | `--ollama` | Ollama 서버 검토 포함 | | `--lm-studio` | LM Studio 서버 검토 포함 | | `--llama-cpp` | llama.cpp 서버 검토 포함 | diff --git a/docs/ko-KR/FEATURES.md b/docs/ko-KR/FEATURES.md index a6663acd3..1865f9931 100644 --- a/docs/ko-KR/FEATURES.md +++ b/docs/ko-KR/FEATURES.md @@ -1067,9 +1067,9 @@ fix(03-01): correct auth token expiry ### 42. Cross-AI Peer Review -**명령어:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--all]` +**명령어:** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--antigravity] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--all]` -**목적:** 외부 AI CLI(Gemini, Claude, Codex, CodeRabbit, OpenCode, Qwen Code, Cursor, Antigravity)를 호출하여 페이즈 계획을 독립적으로 검토합니다. 검토자별 피드백이 담긴 구조화된 REVIEWS.md를 생성합니다. +**목적:** 외부 AI CLI(Gemini, Claude, Codex, CodeRabbit, OpenCode, Qwen Code, Cursor, Antigravity, Kimi Code)와 로컬 OpenAI 호환 서버(Ollama, LM Studio, llama.cpp)를 호출하여 페이즈 계획을 독립적으로 검토합니다. 검토자별 피드백이 담긴 구조화된 REVIEWS.md를 생성합니다. **요구사항.** - REQ-REVIEW-01: 시스템에서 사용 가능한 AI CLI를 감지해야 합니다. diff --git a/docs/pt-BR/COMMANDS.md b/docs/pt-BR/COMMANDS.md index c670dbbf2..d452dda8b 100644 --- a/docs/pt-BR/COMMANDS.md +++ b/docs/pt-BR/COMMANDS.md @@ -218,7 +218,7 @@ Loop de convergência de planos cross-AI — replaneja com feedback de revisão | Argumento / Flag | Obrigatório | Descrição | |------------------|-------------|-----------| | `N` | **Sim** | Número da fase a planejar e revisar | -| `--codex` / `--gemini` / `--claude` / `--opencode` | Não | Seleção de revisor único | +| Flags de revisor | Não | Repassa todas as flags de lane de revisor: `--gemini`, `--claude`, `--codex`, `--coderabbit`, `--opencode`, `--qwen`, `--cursor`, `--agy` / `--antigravity`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--kimi-code` | | `--all` | Não | Executa todos os revisores configurados em paralelo | | `--max-cycles N` | Não | Substitui o limite de ciclos (padrão 3) | @@ -1247,6 +1247,7 @@ Revisão por pares cross-AI de planos de fase a partir de CLIs de IA externas. | `--qwen` | Inclui revisão pelo Qwen Code (modelos Alibaba Qwen) | | `--cursor` | Inclui revisão pelo agente Cursor | | `--agy` / `--antigravity` | Inclui revisão pelo Antigravity CLI (gratuito com credenciais Google) | +| `--kimi-code` | Inclui revisão pelo Kimi Code CLI (Moonshot AI) | | `--ollama` | Inclui revisão pelo servidor Ollama | | `--lm-studio` | Inclui revisão pelo servidor LM Studio | | `--llama-cpp` | Inclui revisão pelo servidor llama.cpp | diff --git a/docs/reference/capability-manifest.md b/docs/reference/capability-manifest.md index f64d1ac2a..ad6b78419 100644 --- a/docs/reference/capability-manifest.md +++ b/docs/reference/capability-manifest.md @@ -4,18 +4,18 @@ > **See also:** [How to develop a capability](../how-to/develop-a-capability.md) · [Capability Command Reference](gsd-capability-command.md) Each capability is a folder `capabilities//` (or an overlay root `~/.gsd/capabilities//` / `.gsd/capabilities//`) containing one `capability.json` declaration. -The file is schema-validated JSON with a common **envelope** plus a **role-typed body** (`role: "feature"` or `role: "runtime"`). +The file is schema-validated JSON with a common **envelope** plus a **role-typed body** (`role: "feature"`, `role: "runtime"`, or `role: "reviewer"`). --- ## Envelope fields -These fields are present for both `role: "feature"` and `role: "runtime"` capabilities. +These fields are present for `role: "feature"`, `role: "runtime"`, and `role: "reviewer"` capabilities. | Field | Type | Required | Description | |---|---|---|---| | `id` | string (kebab-case) | Yes | Unique identifier; **must equal the folder name**. The prefix `gsd-`, `gsd-core-`, and `anthropic-` are reserved for first-party use. | -| `role` | `"feature"` \| `"runtime"` | Yes | Discriminator that selects the body schema. | +| `role` | `"feature"` \| `"runtime"` \| `"reviewer"` | Yes | Discriminator that selects the body schema. `"reviewer"` is for lane-only capabilities that ship a `reviewer` body and nothing else — see [Reviewer body](#reviewer-body-role-reviewer-or-on-any-role) below. | | `version` | semver string | Yes (1.6.0+) | Semantic version of this capability. The registry rejects a manifest without one. | | `title` | string | Yes | Short human-readable label. Must be a non-empty string. | | `description` | string | Yes | Longer summary sentence. Must be a non-empty string. | @@ -155,10 +155,107 @@ Runtime capabilities describe how GSD projects its artefacts onto one host CLI. | Permission writer | `runtime.permissionWriter` | `null` \| `"opencode"` \| `"kilo"` \| `"antigravity"`. The finish-time permissions-sidecar writer. | | Extended hook events | `runtime.extendedHookEvents` | string[] over a closed vocabulary: `SubagentStop`, `Stop`, `PreCompact`, `FileChanged`, `BeforeAgent`, `AfterAgent`, `BeforeModel`, `SubagentStart`. | +### `hostBehaviors` + +`runtime.hostBehaviors` is an **open, unvalidated bag** of per-host behavior switches consumed directly by installer and runtime-adaptation code. Unlike every axis in the table above, it is **not covered by any schema**: the key `hostBehaviors` appears zero times in `scripts/gen-capability-registry.cjs` and zero times in `scripts/registry-schema.cjs`. An unknown key inside `hostBehaviors` is neither rejected nor warned about — it is simply ignored by any code path that does not look for it by name. + +58 distinct keys are declared across the shipped runtime manifests; most are set by exactly one capability. This table is not exhaustive — it lists the keys with the widest reuse so a reader can pattern-match new ones against the same shape: + +| Key | Capabilities declaring it | +|---|---| +| `reapplyCommand` | 9 | +| `skipSharedHooksInstall` | 8 | +| `reviewerCli` | 6 | +| `frontmatterDialect` | 5 | +| `hyphenNameAgentBody` | 3 | +| `legacyCommandsGsdInstallMigration` | 3 | +| `skipUpdateBannerCommand` | 3 | +| `verificationStyle` | 3 | + +**`reviewerCli` is deprecated.** It is a boolean that historically marked a runtime capability as also being a reviewer lane. It is now a **derived legacy alias**, retained for one release so an out-of-tree runtime descriptor that still sets it keeps working. A declared `reviewer` body (see below) takes precedence over the alias, and a capability declaring both contributes **one** slug, not two. `reviewerCli` is superseded by the `reviewer` body; its removal is tracked by issue #2801. It is currently set by 6 capabilities: `antigravity`, `claude`, `codex`, `cursor`, `opencode`, `qwen`. + +See [ADR-1016](../adr/1016-runtime-capability-descriptor.md) (the runtime body is a closed 8-axis plus 4 install-surface vocabulary; `hostBehaviors` is the deliberate open seam beside it) and [ADR-2782](../adr/2782-reviewer-lane-capability-surface.md) (introduces the `reviewer` body and the `reviewerCli` alias's deprecation). + For a minimal `role: "runtime"` example, see [ADR-1016 §Decision 8](../adr/1016-runtime-capability-descriptor.md). --- +## Reviewer body (`role: "reviewer"`, or on any role) + +[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. + +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**: + +- A `reviewer` body is admissible on `role: "runtime"`, so an existing runtime capability — `codex`, `antigravity` — keeps **one** manifest that is both an installable runtime and a reviewer lane. +- A third role, `role: "reviewer"`, exists for lane-only CLIs that GSD never installs into. There are currently 5: `coderabbit`, `gemini`, `llama-cpp`, `lm-studio`, `ollama`. + +Current role counts across `capabilities/`: `feature` 20, `runtime` 19, `reviewer` 5. + +All 12 shipped lane declarations carry all 13 fields below. + +| Field | Type | Notes | +|---|---|---| +| `slug` | string | Lane identity; grammar `^[a-z0-9][a-z0-9_-]*$`. May use `_` (`lm_studio`, `llama_cpp`) even where the capability *folder id* is kebab-case (`lm-studio`). | +| `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. | +| `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. | +| `evidenceClass` | closed enum | `source-grounded` \| `diff-only` (diff-only findings are down-weighted in consensus). | +| `requiresBinaries` | string[] | Extra binaries the lane needs beyond `invoke.binary`. | +| `promptBudgetKey` | string or `null` | Federated config key bounding prompt size. | +| `modelConfigKey` | string or `null` | Federated config key naming the model, e.g. `review.models.kimi-code`. | +| `handler` | closed enum or `null` | `antigravity` \| `openai-compatible` \| `opencode` \| `null`. | + +**`handler` is a closed enum of first-party handler names, not an open escape hatch.** [ADR-1016](../adr/1016-runtime-capability-descriptor.md) explicitly rejected "arbitrary code in the descriptor"; hard shapes are absorbed by adding a named primitive that is reviewed first-party. The consequence, stated plainly: **a third-party reviewer lane is strictly data-only.** A plugin can ship a lane, but not a quirky lane that needs imperative code — a lane requiring behavior beyond the closed `handler` set is not expressible and must be proposed and merged first-party. + +Uniqueness is enforced across the merged first-party ∪ overlay set: duplicate `slug`, duplicate `flags` entry, and duplicate `reviewsSection` are each build-time violations. Two lanes sharing a `reviewsSection` heading would silently merge their output in `REVIEWS.md`, producing apparent consensus that does not exist. + +An unknown field inside a `reviewer` body is a **non-fatal warning on stderr, never a build failure** ([ADR-2782](../adr/2782-reviewer-lane-capability-surface.md) D4), so a manifest built against a newer GSD degrades visibly rather than crashing. + +### Example — lane-only `role: "reviewer"` capability + +```json +{ + "id": "coderabbit", + "role": "reviewer", + "version": "1.8.0", + "title": "CodeRabbit", + "description": "CodeRabbit CLI — cross-AI /gsd:review reviewer lane only; not a GSD install target (no runtime body, no artifacts).", + "tier": "full", + "requires": [], + "engines": { "gsd": ">=1.8.0" }, + "reviewer": { + "slug": "coderabbit", + "flags": ["--coderabbit"], + "transport": "spawn", + "probe": { "kind": "command-exists", "binary": "coderabbit" }, + "invoke": { + "binary": "coderabbit", + "args": ["review", "--prompt-only"], + "promptChannel": "none", + "outputChannel": "stdout", + "modelArg": null, + "effortChannel": "none" + }, + "timeoutFloorMs": 360000, + "emptyOutput": "stub-with-stderr", + "reviewsSection": "CodeRabbit", + "evidenceClass": "diff-only", + "requiresBinaries": [], + "promptBudgetKey": null, + "modelConfigKey": null, + "handler": null + } +} +``` + +--- + ## Conformance invariants The following invariants are enforced at **build time** by `scripts/gen-capability-registry.cjs` and at **install time** by the runtime-callable `validateCapability()` / `validateCrossCapability()` over the merged first-party ∪ overlay set. diff --git a/docs/reference/capability-matrix.md b/docs/reference/capability-matrix.md index 34a35850d..228f87efe 100644 --- a/docs/reference/capability-matrix.md +++ b/docs/reference/capability-matrix.md @@ -19,7 +19,7 @@ See also: [ADR-1244](../adr/1244-capability-ecosystem.md) — | Column | Description | |---|---| | **id** | Canonical capability identifier; unique across first- and third-party capabilities. Reserved prefixes: `gsd-`, `gsd-core-`, `anthropic-`. | -| **role** | `feature` — extends what the loop does; `runtime` — adapts GSD to a specific AI runtime/IDE. | +| **role** | `feature` — extends what the loop does; `runtime` — adapts GSD to a specific AI runtime/IDE; `reviewer` — declares a cross-AI reviewer lane (ADR-2782). A capability may be both a runtime and a reviewer. | | **tier** | `core` — always active; `standard` — active when the runtime supports it; `full` — opt-in or runtime-specific. | | **engines.gsd** | Semver RANGE expressing host-version compatibility. A hard gate at install and at load. `—` means the capability declares no range. | | **extension points** | The loop points this capability registers hooks into (from the registry's `byLoopPoint` index). `—` means it registers none (typical for runtime capabilities, whose job is surface emission). | @@ -144,7 +144,7 @@ fields described below, with `source` = `third-party`. | Column | Value | |---|---| | **id** | As declared in `capability.json`. Must not use reserved prefixes (`gsd-`, `gsd-core-`, `anthropic-`). | -| **role** | `feature` or `runtime`, as declared. | +| **role** | `feature`, `runtime`, or `reviewer`, as declared. | | **tier** | `core`, `standard`, or `full`, as declared. | | **engines.gsd** | Range from `capability.json`; verified at install and at each load. | | **extension points** | The loop points the capability registers into, validated against the known 12 identifiers. | diff --git a/docs/zh-CN/COMMANDS.md b/docs/zh-CN/COMMANDS.md index 0f2333820..9699f4f93 100644 --- a/docs/zh-CN/COMMANDS.md +++ b/docs/zh-CN/COMMANDS.md @@ -218,7 +218,7 @@ v1.40 中，六个命名空间路由器作为第一阶段入口点随附发布 | 参数 / 标志 | 必填 | 描述 | |-----------------|----------|-------------| | `N` | **是** | 要规划和审查的阶段编号 | -| `--codex` / `--gemini` / `--claude` / `--opencode` | 否 | 单一审查者选择 | +| 审查者标志 | 否 | 原样传递所有审查者通道标志：`--gemini`、`--claude`、`--codex`、`--coderabbit`、`--opencode`、`--qwen`、`--cursor`、`--agy` / `--antigravity`、`--ollama`、`--lm-studio`、`--llama-cpp`、`--kimi-code` | | `--all` | 否 | 并行运行所有已配置的审查者 | | `--max-cycles N` | 否 | 覆盖循环上限（默认 3） | @@ -1244,6 +1244,7 @@ node gsd-tools.cjs intel api-surface # 渲染 api-map.json → API- | `--qwen` | 包含 Qwen Code 审查（阿里巴巴 Qwen 模型） | | `--cursor` | 包含 Cursor 代理审查 | | `--agy` / `--antigravity` | 包含 Antigravity CLI 审查（使用 Google 凭证免费） | +| `--kimi-code` | 包含 Kimi Code CLI 审查（Moonshot AI） | | `--ollama` | 包含 Ollama 服务器审查 | | `--lm-studio` | 包含 LM Studio 服务器审查 | | `--llama-cpp` | 包含 llama.cpp 服务器审查 | diff --git a/docs/zh-CN/FEATURES.md b/docs/zh-CN/FEATURES.md index 96c641ed3..663d26f39 100644 --- a/docs/zh-CN/FEATURES.md +++ b/docs/zh-CN/FEATURES.md @@ -1173,9 +1173,9 @@ GSD update available: 1.39.0 → 1.40.0. Run /gsd-update. ### 42. 跨 AI 同行评审 -**命令：** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--ollama] [--lm-studio] [--llama-cpp] [--all]` +**命令：** `/gsd-review --phase N [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy] [--antigravity] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--all]` -**目的：** 调用外部 AI CLI（Gemini、Claude、Codex、CodeRabbit、OpenCode、Qwen Code、Cursor、Antigravity）独立审查阶段计划。生成包含每位审查者反馈的结构化 REVIEWS.md。 +**目的：** 调用外部 AI CLI（Gemini、Claude、Codex、CodeRabbit、OpenCode、Qwen Code、Cursor、Antigravity、Kimi Code）和本地 OpenAI 兼容服务器（Ollama、LM Studio、llama.cpp）独立审查阶段计划。生成包含每位审查者反馈的结构化 REVIEWS.md。 **需求：** - REQ-REVIEW-01：系统必须检测系统上可用的 AI CLI diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index e87436c2d..cd6b66fb1 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1188,6 +1188,30 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load return; } + // Phase 6 (#2800, closes #2272). The reviewer-flag lists in + // plan-review-convergence.md, autonomous.md and next.md were hand-enumerated in three places + // and had drifted apart: `--coderabbit` was missing from all three and had been silently + // falling back to `--codex`. One declared source, three consumers. + // + // Emits FLAGS, not slugs: `antigravity` declares two (`--antigravity`, `--agy`), so the flag + // count (13) is deliberately not the lane count (12). Same output contract as `sections` — + // one token per line, descriptor order, and no trailing newline on an empty result. + if (sub === 'flags') { + const rows = chosen + .map((s) => laneBySlug.get(s)) + .filter(Boolean) + .flatMap((l) => (Array.isArray(l.flags) ? l.flags : [])) + // Shape-filtered, not merely non-empty. All three consumers read this through an + // UNQUOTED `$(gsd_run review-lane flags)` so the newline-separated output word-splits + // into loop items — which is the intent. Phase 2 (#2795) admits third-party overlay + // lanes into this same descriptor, so an overlay declaring `--foo bar` would inject a + // second loop item, and one declaring a glob character would expand against the cwd. + // Emitting only well-formed flags keeps that from reaching the shell at all. + .filter((f) => typeof f === 'string' && /^--[a-z0-9][a-z0-9-]*$/.test(f)); + process.stdout.write(rows.join('\n') + (rows.length ? '\n' : '')); + return; + } + // Effort argv is resolved per lane by the host's own execution policy, exactly as the legs did // via `resolve-execution … --pick effort_argv_string`. A lane whose slug is not a known host // simply gets none. @@ -1267,7 +1291,7 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load } if (sub !== 'invoke') { - error("Usage: review-lane [--selected a,b] [--run-dir D] [--repo-root R]"); + error("Usage: review-lane [--selected a,b] [--run-dir D] [--repo-root R]"); return; } diff --git a/gsd-core/workflows/autonomous.md b/gsd-core/workflows/autonomous.md index 7a4f91c93..1adb4b232 100644 --- a/gsd-core/workflows/autonomous.md +++ b/gsd-core/workflows/autonomous.md @@ -44,19 +44,6 @@ PLAN_STRATEGY="local" if echo "$ARGUMENTS" | grep -qE '(^|[[:space:]])\-\-(converge|cross-ai)([[:space:]]|$)'; then PLAN_STRATEGY="converge" fi - -CONVERGENCE_ARGS="" -for REVIEW_FLAG in --codex --gemini --claude --opencode --ollama --lm-studio --llama-cpp --all --text; do - if echo "$ARGUMENTS" | grep -qE "(^|[[:space:]])${REVIEW_FLAG}([[:space:]]|$)"; then - CONVERGENCE_ARGS="${CONVERGENCE_ARGS} ${REVIEW_FLAG}" - fi -done - -MAX_CYCLES_ARG="" -if echo "$ARGUMENTS" | grep -qE '\-\-max-cycles\s+[0-9]+'; then - MAX_CYCLES_ARG=$(echo "$ARGUMENTS" | grep -oE '\-\-max-cycles\s+[0-9]+' | awk '{print $2}') - CONVERGENCE_ARGS="${CONVERGENCE_ARGS} --max-cycles ${MAX_CYCLES_ARG}" -fi ``` When `--only` is set, also set `FROM_PHASE` to the same value so existing filter logic applies. @@ -76,6 +63,23 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi If `PLAN_STRATEGY` is `converge`, fail fast unless the existing convergence feature gate is enabled: ```bash +# Lane flags derived from the declared roster (#2800/#2272); --all and --text are convergence +# controls, not reviewer lanes, so they stay literal. +# This block must stay AFTER the launcher preamble (above) because it calls `gsd_run` — +# do not move it back above the preamble in a future edit. +CONVERGENCE_ARGS="" +for REVIEW_FLAG in $(gsd_run review-lane flags) --all --text; do + if echo "$ARGUMENTS" | grep -qE "(^|[[:space:]])${REVIEW_FLAG}([[:space:]]|$)"; then + CONVERGENCE_ARGS="${CONVERGENCE_ARGS} ${REVIEW_FLAG}" + fi +done + +MAX_CYCLES_ARG="" +if echo "$ARGUMENTS" | grep -qE '\-\-max-cycles\s+[0-9]+'; then + MAX_CYCLES_ARG=$(echo "$ARGUMENTS" | grep -oE '\-\-max-cycles\s+[0-9]+' | awk '{print $2}') + CONVERGENCE_ARGS="${CONVERGENCE_ARGS} --max-cycles ${MAX_CYCLES_ARG}" +fi + if [ "$PLAN_STRATEGY" = "converge" ]; then CONVERGENCE_ENABLED=$(gsd_run query config-get workflow.plan_review_convergence 2>/dev/null || echo "false") if [ "$CONVERGENCE_ENABLED" != "true" ]; then diff --git a/gsd-core/workflows/help/modes/full.md b/gsd-core/workflows/help/modes/full.md index 242f67268..8759cc3b8 100644 --- a/gsd-core/workflows/help/modes/full.md +++ b/gsd-core/workflows/help/modes/full.md @@ -624,7 +624,7 @@ The commands above cover the most common day-to-day flows. Every command listed - **`/gsd:mvp-phase `** — Plan a phase as a vertical MVP slice (user story + SPIDR splitting) before handing off to plan-phase. Same end-state as `/gsd:plan-phase --mvp`, with a guided MVP-shaping intro. - **`/gsd:ultraplan-phase [phase]`** — [BETA] Offload plan phase to Claude Code's ultraplan cloud; review in browser and import back. -- **`/gsd:plan-review-convergence [--codex] [--gemini] [--claude] [--opencode] [--ollama] [--lm-studio] [--llama-cpp] [--all] [--text] [--ws ] [--max-cycles N]`** — Cross-AI plan convergence loop — replan with review feedback until no HIGH concerns remain. Supports both cloud reviewers (Codex/Gemini/Claude/OpenCode) and local model runtimes (Ollama, LM Studio, llama.cpp). +- **`/gsd:plan-review-convergence [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--agy/--antigravity] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--all] [--text] [--ws ] [--max-cycles N]`** — Cross-AI plan convergence loop — replan with review feedback until no HIGH concerns remain. Supports both cloud reviewers (Gemini/Claude/Codex/CodeRabbit/OpenCode/Qwen/Cursor/Antigravity/Kimi Code) and local model runtimes (Ollama, LM Studio, llama.cpp). - **`/gsd:autonomous [--from N] [--to N] [--only N] [--interactive] [--converge]`** — Run all remaining phases autonomously: discuss → plan → execute per phase. `--converge` routes planning through plan-review convergence; `--cross-ai` is an alias. ### Quality, Review & Verification diff --git a/gsd-core/workflows/next.md b/gsd-core/workflows/next.md index 63b36bc5e..0e4807fe3 100644 --- a/gsd-core/workflows/next.md +++ b/gsd-core/workflows/next.md @@ -264,7 +264,9 @@ if echo "$ARGUMENTS" | grep -qE '(^|[[:space:]])\-\-(converge|cross-ai)([[:space fi CONVERGENCE_ARGS="" -for REVIEW_FLAG in --codex --gemini --claude --opencode --ollama --lm-studio --llama-cpp --all --text; do +# Lane flags derived from the declared roster (#2800/#2272); --all and --text are convergence +# controls, not reviewer lanes, so they stay literal. +for REVIEW_FLAG in $(gsd_run review-lane flags) --all --text; do if echo "$ARGUMENTS" | grep -qE "(^|[[:space:]])${REVIEW_FLAG}([[:space:]]|$)"; then CONVERGENCE_ARGS="${CONVERGENCE_ARGS} ${REVIEW_FLAG}" fi diff --git a/gsd-core/workflows/plan-review-convergence.md b/gsd-core/workflows/plan-review-convergence.md index 6ddcd83bc..2e38ff8e8 100644 --- a/gsd-core/workflows/plan-review-convergence.md +++ b/gsd-core/workflows/plan-review-convergence.md @@ -18,22 +18,11 @@ Read all files referenced by the invoking prompt's execution_context before star ## 1. Parse and Normalize Arguments -Extract from $ARGUMENTS: phase number, reviewer flags (`--codex`, `--gemini`, `--agy`/`--antigravity`, `--claude`, `--opencode`, `--ollama`, `--lm-studio`, `--llama-cpp`, `--all`), `--max-cycles N`, `--text`, `--ws`. +Extract from $ARGUMENTS: phase number, reviewer flags (the declared reviewer lane flags, plus `--all`), `--max-cycles N`, `--text`, `--ws`. ```bash PHASE=$(echo "$ARGUMENTS" | grep -oE '[0-9]+\.?[0-9]*' | head -1) -REVIEWER_FLAGS="" -echo "$ARGUMENTS" | grep -q '\-\-codex' && REVIEWER_FLAGS="$REVIEWER_FLAGS --codex" -echo "$ARGUMENTS" | grep -q '\-\-gemini' && REVIEWER_FLAGS="$REVIEWER_FLAGS --gemini" -echo "$ARGUMENTS" | grep -q '\-\-agy' && REVIEWER_FLAGS="$REVIEWER_FLAGS --agy" -echo "$ARGUMENTS" | grep -q '\-\-antigravity' && REVIEWER_FLAGS="$REVIEWER_FLAGS --antigravity" -echo "$ARGUMENTS" | grep -q '\-\-claude' && REVIEWER_FLAGS="$REVIEWER_FLAGS --claude" -echo "$ARGUMENTS" | grep -q '\-\-opencode' && REVIEWER_FLAGS="$REVIEWER_FLAGS --opencode" -echo "$ARGUMENTS" | grep -q '\-\-ollama' && REVIEWER_FLAGS="$REVIEWER_FLAGS --ollama" -echo "$ARGUMENTS" | grep -q '\-\-lm-studio' && REVIEWER_FLAGS="$REVIEWER_FLAGS --lm-studio" -echo "$ARGUMENTS" | grep -q '\-\-llama-cpp' && REVIEWER_FLAGS="$REVIEWER_FLAGS --llama-cpp" -echo "$ARGUMENTS" | grep -q '\-\-all' && REVIEWER_FLAGS="$REVIEWER_FLAGS --all" # #2315: do NOT default REVIEWER_FLAGS to --codex here. The default is resolved # against review.default_reviewers in step 1.5 (after the config gate) so a bare # invocation respects the configured reviewer lineup per ADR-0011 / ADR-0015. @@ -66,6 +55,20 @@ Then re-run: /gsd:plan-review-convergence {PHASE} ``` ```bash +# Reviewer flags are DERIVED from the declared lane roster (#2800/#2272), never hand-listed. +# Three surfaces used to enumerate them independently and had drifted: --coderabbit was missing +# from all three, --qwen/--cursor/--kimi-code from this one, and the old unanchored +# `grep -q '\-\-agy'` matched INSIDE --antigravity, appending both for one user flag. +# `--all` is a selection control, not a lane, so it stays literal. +# This block must stay AFTER the launcher preamble (below) because it calls `gsd_run` — +# do not move it back above the preamble in a future edit. +REVIEWER_FLAGS="" +for REVIEW_FLAG in $(gsd_run review-lane flags) --all; do + if echo "$ARGUMENTS" | grep -qE "(^|[[:space:]])${REVIEW_FLAG}([[:space:]]|$)"; then + REVIEWER_FLAGS="$REVIEWER_FLAGS $REVIEW_FLAG" + fi +done + # #2315: Resolve reviewer selection when no explicit flag was given. # The pre-fix bug unconditionally set REVIEWER_FLAGS="--codex" in step 1, BEFORE # the config gate — silently overriding any configured review.default_reviewers diff --git a/scripts/gen-capability-matrix.cjs b/scripts/gen-capability-matrix.cjs index 509c5b282..ab86db3fc 100644 --- a/scripts/gen-capability-matrix.cjs +++ b/scripts/gen-capability-matrix.cjs @@ -145,7 +145,7 @@ See also: [ADR-1244](../adr/1244-capability-ecosystem.md) — | Column | Description | |---|---| | **id** | Canonical capability identifier; unique across first- and third-party capabilities. Reserved prefixes: \`gsd-\`, \`gsd-core-\`, \`anthropic-\`. | -| **role** | \`feature\` — extends what the loop does; \`runtime\` — adapts GSD to a specific AI runtime/IDE. | +| **role** | \`feature\` — extends what the loop does; \`runtime\` — adapts GSD to a specific AI runtime/IDE; \`reviewer\` — declares a cross-AI reviewer lane (ADR-2782). A capability may be both a runtime and a reviewer. | | **tier** | \`core\` — always active; \`standard\` — active when the runtime supports it; \`full\` — opt-in or runtime-specific. | | **engines.gsd** | Semver RANGE expressing host-version compatibility. A hard gate at install and at load. \`—\` means the capability declares no range. | | **extension points** | The loop points this capability registers hooks into (from the registry's \`byLoopPoint\` index). \`—\` means it registers none (typical for runtime capabilities, whose job is surface emission). | @@ -223,7 +223,7 @@ fields described below, with \`source\` = \`third-party\`. | Column | Value | |---|---| | **id** | As declared in \`capability.json\`. Must not use reserved prefixes (\`gsd-\`, \`gsd-core-\`, \`anthropic-\`). | -| **role** | \`feature\` or \`runtime\`, as declared. | +| **role** | \`feature\`, \`runtime\`, or \`reviewer\`, as declared. | | **tier** | \`core\`, \`standard\`, or \`full\`, as declared. | | **engines.gsd** | Range from \`capability.json\`; verified at install and at each load. | | **extension points** | The loop points the capability registers into, validated against the known 12 identifiers. | diff --git a/skills/gsd-plan-review-convergence/SKILL.md b/skills/gsd-plan-review-convergence/SKILL.md index 58b5c878b..0026ceda2 100644 --- a/skills/gsd-plan-review-convergence/SKILL.md +++ b/skills/gsd-plan-review-convergence/SKILL.md @@ -1,7 +1,7 @@ --- name: gsd-plan-review-convergence description: "Cross-AI plan convergence - replan until review concerns are resolved." -argument-hint: " [--codex] [--gemini] [--claude] [--opencode] [--ollama] [--lm-studio] [--llama-cpp] [--agy] [--text] [--ws ] [--all] [--max-cycles N]" +argument-hint: " [--gemini] [--claude] [--codex] [--coderabbit] [--opencode] [--qwen] [--cursor] [--antigravity] [--agy] [--ollama] [--lm-studio] [--llama-cpp] [--kimi-code] [--text] [--ws ] [--all] [--max-cycles N]" allowed-tools: - Read - Write @@ -44,10 +44,14 @@ Phase number: extracted from $ARGUMENTS (required) - `--gemini` — Use Gemini CLI as reviewer - `--agy` / `--antigravity` — Use Antigravity CLI as reviewer (successor to the discontinued Gemini CLI) - `--claude` — Use Claude CLI as reviewer (separate session) +- `--coderabbit` — Use CodeRabbit as reviewer (reviews the working-tree diff, not the source tree) - `--opencode` — Use OpenCode as reviewer +- `--qwen` — Use Qwen Code CLI as reviewer (Alibaba Qwen models) +- `--cursor` — Use Cursor agent as reviewer - `--ollama` — Use local Ollama server as reviewer (OpenAI-compatible, default host `http://localhost:11434`; configure model via `review.models.ollama`) - `--lm-studio` — Use local LM Studio server as reviewer (OpenAI-compatible, default host `http://localhost:1234`; configure model via `review.models.lm_studio`) - `--llama-cpp` — Use local llama.cpp server as reviewer (OpenAI-compatible, default host `http://localhost:8080`; configure model via `review.models.llama_cpp`) +- `--kimi-code` — Use Kimi Code CLI as reviewer (Moonshot AI) - `--all` — Use all available CLIs and running local model servers - `--max-cycles N` — Maximum replan→review cycles (default: 3) diff --git a/src/review-lane-descriptor.cts b/src/review-lane-descriptor.cts index e5d504f8d..383331d30 100644 --- a/src/review-lane-descriptor.cts +++ b/src/review-lane-descriptor.cts @@ -627,7 +627,7 @@ const LEG_MARKER_RE = //g; * `LEG_MARKER_RE` can only capture `[a-z0-9_-]`. A lane whose slug falls outside * that class is therefore UNMATCHABLE in the workflow: its marker can be present * and correct and the scan will still never see it, so the lane reports - * `LEG_MARKER_MISSING` forever with no indication why. All eleven shipped slugs + * `LEG_MARKER_MISSING` forever with no indication why. All twelve shipped slugs * sit inside the class (`lm_studio`, `llama_cpp` use the underscore), so this * never bites today — but Phase 2 (#2795) admits third-party overlay lanes, and * a slug like `acme.reviewer` would silently vanish from a review. @@ -786,3 +786,364 @@ export function checkReviewerLaneParity(input: ParityInput): ParityResult { return { ok: violations.length === 0, violations }; } + +/* ------------------------------------------------------------------ * + * Documentation parity (ADR-2782 Phase 6, #2800 — closes #2781/#2272) + * ------------------------------------------------------------------ */ + +/** + * Frozen reason enum for the DOCUMENTATION arm. + * + * Deliberately separate from `PARITY_VIOLATION` rather than an extension of it. The two functions + * answer different questions — `checkReviewerLaneParity` asks *what runs* (descriptor ↔ roster ↔ + * registry), this asks *what is documented*. Fusing them would change the signature of a function + * with seven dependents and would let a stale doc make the runtime checker look red. + * + * Same three-coordinated-changes rule as its sibling: this enum, the emitting site, and the test + * locking `Object.keys(...).sort()`. + */ +export const DOCS_PARITY_VIOLATION = Object.freeze({ + MALFORMED_LANE: 'malformed_lane', + DOC_FLAG_MISSING: 'doc_flag_missing', + DOC_FLAG_UNDECLARED: 'doc_flag_undeclared', + DOC_TITLE_MISSING: 'doc_title_missing', + SIGNATURE_FLAG_MISSING: 'signature_flag_missing', + DOC_UNREADABLE: 'doc_unreadable', + TABLE_ROW_MISSING: 'table_row_missing', +} as const); + +export type DocsParityViolationReason = + (typeof DOCS_PARITY_VIOLATION)[keyof typeof DOCS_PARITY_VIOLATION]; + +export interface DocsParityViolation { + reason: DocsParityViolationReason; + /** The document the violation was found in. */ + doc: string; + /** The flag or section title at fault. */ + subject: string; +} + +export interface DocsParityResult { + ok: boolean; + violations: DocsParityViolation[]; + /** + * Documents that carry no `/gsd-review` surface at all and were therefore skipped. + * + * Reported rather than silently dropped: `docs/pt-BR/FEATURES.md` is a 77-line stub, and a + * caller that cannot tell "this mirror does not document the command" from "this mirror agrees" + * has the same blindness this whole gate exists to remove. + */ + skipped: string[]; +} + +export interface DocsParityInput { + descriptor: ReadonlyArray; + /** Map of document label (repo-relative path) to its full text. */ + docs: Record; +} + +/** + * Non-lane tokens that legitimately share the `/gsd-review` signature line. + * + * `--all` is a selection control, not a reviewer lane. Without this allow-list the undeclared-flag + * arm would fire on correct documentation — the gate failing on a doc that is right is the fastest + * way to get a gate deleted. + */ +const NON_LANE_SIGNATURE_FLAGS: ReadonlySet = new Set(['--all']); + +/** + * Is `flag` documented in `text` under one of the two structural shapes docs actually use? + * + * Backticked is the `COMMANDS.md` table-cell shape; bracketed (`[--gemini]`) is the + * `FEATURES.md` signature shape. Requiring one of those two delimiters — rather than a bare + * substring — is what keeps prose and fenced examples from satisfying the gate, and it bounds the + * token for free: a backticked `--claude` demands its closing backtick, so a backticked + * `--claude-foo` cannot satisfy it. + * + * Literal `includes`, deliberately NOT a RegExp. `new RegExp` throws `SyntaxError` on a pattern + * beyond ~100k chars, and Phase 2 (#2795) admits third-party overlay lanes whose declared strings + * are untrusted in length — a parity gate that throws on its own input is indistinguishable from + * one that never ran. Literal matching also removes the need to escape metacharacters, which is + * what `llama.cpp` needed. + */ +function flagIsDocumented(text: string, flag: string): boolean { + return text.includes('`' + flag + '`') || text.includes('[' + flag + ']'); +} + +/** + * Strip fenced code blocks and HTML comments before parity matching. + * + * Both are places a flag can be *mentioned* without being *documented*: a fenced block showing + * markdown syntax, or a commented-out row left behind by an edit. Counting either is a false pass + * — the gate would report a lane as documented when a reader never sees it. + * + * Lines are replaced with empty strings rather than removed so that every downstream line index + * (the signature line, the Purpose paragraph beneath it) still refers to the same physical line. + */ +function stripNonProse(text: string): string { + const lines = text.split('\n'); + const out: string[] = []; + let inFence = false; + let inComment = false; + for (const line of lines) { + const trimmed = line.trim(); + if (!inComment && /^(`{3,}|~{3,})/.test(trimmed)) { + inFence = !inFence; + out.push(''); + continue; + } + if (inFence) { + out.push(''); + continue; + } + let working = line; + if (!inComment && working.includes('')) { + inComment = true; + working = working.slice(0, working.indexOf('')) { + inComment = false; + working = working.slice(working.indexOf('-->') + 3); + out.push(working); + } else { + out.push(''); + } + continue; + } + // Single-line comments: strip to a FIXED POINT, then honor any unterminated opener. + // + // One pass is not enough. `-->` strips the inner span and leaves a live `/g, ''); + } while (working !== previous); + // Whatever `/g, + // '')` exactly ONCE per line. A single `.replace(/g, '')` pass only removes matches found in the + // ORIGINAL string; it never rescans the text IT JUST PRODUCED. So when removing one self- + // contained `` span joins the fragments on either side of it into a brand-new, + // complete `` span, that new span survives the pass verbatim — including whatever + // reviewer-flag row is wrapped inside it — and the row is (wrongly) counted as documented. This + // is the classic "ipt>" class of defect (CodeQL js/incomplete-multi-character- + // sanitization) applied to HTML comments instead of script tags. The fix iterates the same + // regex to a fixed point, then treats anything still starting with a bare `` + `-| `--kimi-code` | d |-->`: one pass strips the self-contained + // `` in the middle, which joins the leftover `` closer. A + // single-pass strip leaves that whole new span — row included — untouched in the output, so + // the row reads as documented. Regressed exactly here: verified against the pre-fix + // single-pass implementation, only `TABLE_ROW_MISSING` fired and `DOC_FLAG_MISSING` did not. + const joinLine = '-| `--kimi-code` | d |-->'; + const doc = ['### `/gsd-review`', '', ...NON_KIMI_ROWS, '', joinLine, ''].join('\n'); + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: doc } }); + const kimi = r.violations.filter((v) => v.subject === '--kimi-code'); + assert.ok( + kimi.some((v) => v.reason === DOCS_PARITY_VIOLATION.DOC_FLAG_MISSING) + || kimi.some((v) => v.reason === DOCS_PARITY_VIOLATION.TABLE_ROW_MISSING), + '--kimi-code must still be reported missing once the join-trick comment is fully stripped', + ); + }); + + test('anUnterminatedCommentOpenerSwallowsTheRestOfTheDocument', () => { + const doc = [ + '### `/gsd-review`', + '', + ...NON_KIMI_ROWS, + '', + '', + '| `--kimi-code` | d |', + '', + ].join('\n'); + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: doc } }); + const kimi = r.violations.filter((v) => v.subject === '--kimi-code'); + assert.deepStrictEqual(kimi, [], 'content after a real comment close must not be treated as commented out'); + }); + + test('multipleIndependentCommentsOnOneLineAreAllStripped', () => { + const line = 'A B '; + const doc = ['### `/gsd-review`', '', ...NON_KIMI_ROWS, '', line, ''].join('\n'); + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: doc } }); + const kimi = r.violations.filter((v) => v.subject === '--kimi-code'); + assert.ok(kimi.length > 0, 'both independent comment spans on one line must be stripped'); + }); +}); + +describe('reviewer docs parity — hostile and malformed input', () => { + test('an empty docs map is not a clean bill of health', () => { + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: {} }); + assert.strictEqual(r.ok, false); + assert.strictEqual(r.violations.length, ALL_FLAGS.length); + assert.ok(r.violations.every((v) => v.reason === DOCS_PARITY_VIOLATION.DOC_FLAG_MISSING)); + }); + + test('an absent docs map degrades to violations', () => { + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES }); + assert.strictEqual(r.ok, false); + assert.ok(Array.isArray(r.violations)); + }); + + test('non-string doc values are reported, not silently skipped', () => { + for (const bad of [null, 0, [], {}, true]) { + assert.doesNotThrow(() => { + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { bad } }); + assert.deepStrictEqual( + r.violations, + [{ reason: DOCS_PARITY_VIOLATION.DOC_UNREADABLE, doc: 'bad', subject: typeof bad }], + ); + assert.ok(!r.skipped.includes('bad'), `expected ${JSON.stringify(bad)} NOT to be silently skipped`); + }); + } + }); + + test('an empty string doc is skipped, unlike a non-string doc', () => { + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { empty: '' } }); + assert.deepStrictEqual(r.skipped, ['empty']); + assert.ok(!r.violations.some((v) => v.reason === DOCS_PARITY_VIOLATION.DOC_UNREADABLE)); + + const nonString = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { bad: null } }); + assert.deepStrictEqual(nonString.skipped, []); + assert.deepStrictEqual( + nonString.violations, + [{ reason: DOCS_PARITY_VIOLATION.DOC_UNREADABLE, doc: 'bad', subject: 'object' }], + ); + }); + + test('a DOC_UNREADABLE violation never masks a real one', () => { + const missing = ALL_FLAGS.filter((f) => f !== '--kimi-code'); + const r = checkReviewerDocsParity({ + descriptor: REVIEWER_LANES, + docs: { bad: null, good: commandsDoc(missing) }, + }); + assert.deepStrictEqual( + r.violations, + [ + { reason: DOCS_PARITY_VIOLATION.DOC_UNREADABLE, doc: 'bad', subject: 'object' }, + { reason: DOCS_PARITY_VIOLATION.DOC_FLAG_MISSING, doc: 'good', subject: '--kimi-code' }, + ], + ); + }); + + test('a non-array descriptor is reported, not thrown', () => { + assert.doesNotThrow(() => { + checkReviewerDocsParity({ descriptor: 'nope', docs: { d: commandsDoc(ALL_FLAGS) } }); + }); + }); + + test('a malformed lane is named', () => { + const r = checkReviewerDocsParity({ + descriptor: [null, 'not-an-object', ...REVIEWER_LANES], + docs: { d: commandsDoc(ALL_FLAGS) }, + }); + const malformed = r.violations.filter((v) => v.reason === DOCS_PARITY_VIOLATION.MALFORMED_LANE); + assert.strictEqual(malformed.length, 2); + }); + + test('a prototype-key slug is inert', () => { + const protoLane = { ...REVIEWER_LANES[0], slug: '__proto__', flags: ['--proto-lane'] }; + assert.doesNotThrow(() => { + checkReviewerDocsParity({ + descriptor: [...REVIEWER_LANES, protoLane], + docs: { d: commandsDoc([...ALL_FLAGS, '--proto-lane']) }, + }); + }); + assert.strictEqual(({}).polluted, undefined); + }); +}); + +describe('reviewer docs parity — cross-platform (CRLF)', () => { + test('parity is CRLF-insensitive', () => { + const doc = commandsDoc(ALL_FLAGS); + const crlf = doc.split('\n').join('\r\n'); + const lf = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: doc } }); + const cr = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: crlf } }); + assert.deepStrictEqual(cr, lf); + }); + + test('a divergence is still caught under CRLF', () => { + const missing = ALL_FLAGS.filter((f) => f !== '--kimi-code'); + const crlf = commandsDoc(missing).split('\n').join('\r\n'); + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { d: crlf } }); + assert.strictEqual(r.violations.length, 1); + assert.strictEqual(r.violations[0].subject, '--kimi-code'); + }); +}); + +describe('reviewer docs parity — independence and properties', () => { + const FC = { seed: 20260730, numRuns: 200 }; + + test('repeated evaluation is stable', () => { + const input = { descriptor: REVIEWER_LANES, docs: { d: commandsDoc(ALL_FLAGS) } }; + const a = checkReviewerDocsParity(input); + const b = checkReviewerDocsParity(input); + assert.deepStrictEqual(a, b); + }); + + test('verdict is independent of doc key insertion order', () => { + const dirty = commandsDoc(ALL_FLAGS.filter((f) => f !== '--gemini')); + const clean = commandsDoc(ALL_FLAGS); + const forward = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { a: dirty, b: clean } }); + const reversed = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: { b: clean, a: dirty } }); + const sortViolations = (r) => + r.violations.map((v) => `${v.reason}:${v.doc}:${v.subject}`).sort(); + assert.deepStrictEqual(sortViolations(forward), sortViolations(reversed)); + }); + + test('ok always agrees with violations and the function never throws', () => { + fc.assert( + fc.property( + // Widened from the default (~10 chars) so the generator can occasionally reach + // pathologically long keys/values, not just short ones — see the dedicated + // pathological-length case below for the deterministic ~100k-char regression guard. + fc.dictionary(fc.string({ maxLength: 5000 }), fc.anything()), + fc.anything(), + (docs, descriptor) => { + let r; + try { + r = checkReviewerDocsParity({ descriptor, docs }); + } catch { + return false; + } + return ( + typeof r.ok === 'boolean' && + Array.isArray(r.violations) && + Array.isArray(r.skipped) && + r.ok === (r.violations.length === 0) + ); + }, + ), + FC, + ); + }); + + // #2800 review finding: the "never throws" property above uses generators that top out + // around 5,000 chars, so it could never structurally reach the ~100k-char threshold that, + // until fixed, made the implementation throw `SyntaxError` from `new RegExp` (the fix + // switched to literal `String.includes`). This case pins that specific regression with a + // deterministic, explicit pathologically-long descriptor rather than relying on the property + // generator to stumble into it. + test('neverThrowsOnPathologicallyLongDeclaredStrings', () => { + const pathologicalLane = { + slug: 'pathological', + flags: ['--' + 'a'.repeat(200000)], + reviewsSection: 'T'.repeat(200000), + }; + const doc = '/gsd-review some prose mentioning the command.'; + let result; + assert.doesNotThrow(() => { + result = checkReviewerDocsParity({ descriptor: [pathologicalLane], docs: { d: doc } }); + }); + assert.strictEqual(typeof result.ok, 'boolean'); + assert.ok(Array.isArray(result.violations)); + assert.ok(Array.isArray(result.skipped)); + }); + + test('the reason enum is locked', () => { + assert.deepStrictEqual(Object.keys(DOCS_PARITY_VIOLATION).sort(), [ + 'DOC_FLAG_MISSING', + 'DOC_FLAG_UNDECLARED', + 'DOC_TITLE_MISSING', + 'DOC_UNREADABLE', + 'MALFORMED_LANE', + 'SIGNATURE_FLAG_MISSING', + 'TABLE_ROW_MISSING', + ]); + assert.ok(Object.isFrozen(DOCS_PARITY_VIOLATION)); + }); +}); + +describe('reviewer docs parity — the shipped repo', () => { + const DOC_PATHS = [ + 'docs/COMMANDS.md', + 'docs/ja-JP/COMMANDS.md', + 'docs/ko-KR/COMMANDS.md', + 'docs/pt-BR/COMMANDS.md', + 'docs/zh-CN/COMMANDS.md', + 'docs/FEATURES.md', + 'docs/ja-JP/FEATURES.md', + 'docs/ko-KR/FEATURES.md', + 'docs/pt-BR/FEATURES.md', + 'docs/zh-CN/FEATURES.md', + ]; + + function loadShippedDocs() { + const docs = {}; + for (const rel of DOC_PATHS) { + docs[rel] = fs.readFileSync(path.join(ROOT, rel), 'utf-8'); + } + return docs; + } + + test('the shipped descriptor is non-empty', () => { + // Guards the vacuous-truth failure mode: an empty roster trivially satisfies every check below. + assert.ok(REVIEWER_LANES.length >= 12, 'expected at least the 12 shipped lanes'); + }); + + test('the shipped docs satisfy reviewer lane parity', () => { + const r = checkReviewerDocsParity({ descriptor: REVIEWER_LANES, docs: loadShippedDocs() }); + assert.deepStrictEqual( + r.violations, + [], + `shipped docs must satisfy reviewer docs parity; got: ${JSON.stringify(r.violations)}`, + ); + assert.strictEqual(r.ok, true); + // Bounded, not merely `.includes(...)`: an unbounded skipped set is exactly how a doc + // silently drops out of coverage — if a real shipped doc lost its `/gsd-review` marker + // (accidentally or via a bad edit), `checkReviewerDocsParity` would add it to `skipped` + // instead of gating it, and an `.includes()`-only assertion would still pass green with + // that doc no longer checked at all. Asserting the exact set closes that gap. + assert.deepStrictEqual( + r.skipped.slice().sort(), + ['docs/pt-BR/FEATURES.md'].sort(), + 'the skipped set must be EXACTLY this one known stub (a 77-line doc with no /gsd-review ' + + 'section) — any other entry means a real doc silently dropped out of coverage', + ); + }); + + test('an unreadable doc fails loudly', (t) => { + // Exercises the REAL doc-loading path (`loadShippedDocs`, the same helper the integration + // test above drives) rather than invoking the monkeypatched mock directly — calling the mock + // proves nothing about the actual reader and would pass regardless of behavior. + const original = fs.readFileSync; + t.after(() => { + fs.readFileSync = original; + }); + const targetPath = path.join(ROOT, 'docs', 'COMMANDS.md'); + fs.readFileSync = (p, ...rest) => { + if (p === targetPath) throw new Error('injected read failure'); + return original(p, ...rest); + }; + assert.throws(() => { + loadShippedDocs(); + }, /injected read failure/); + }); +}); + +describe('review-lane flags — emitted shape', () => { + const cp = require('node:child_process'); + const TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); + const runFlags = (args = []) => + cp.spawnSync(process.execPath, [TOOLS, 'review-lane', 'flags', ...args], { encoding: 'utf8' }); + + test('emitsEveryDeclaredFlagInDescriptorOrder', () => { + const r = runFlags(); + assert.strictEqual(r.status, 0); + const lines = r.stdout.split('\n').filter(Boolean); + assert.deepStrictEqual(lines, REVIEWER_LANES.flatMap((l) => l.flags)); + }); + + test('everyEmittedTokenIsAWellFormedFlag', () => { + const r = runFlags(); + const lines = r.stdout.split('\n').filter(Boolean); + assert.ok(lines.length > 0, 'expected at least one emitted flag'); + for (const line of lines) { + assert.match(line, /^--[a-z0-9][a-z0-9-]*$/); + } + }); + + test('emitsNoTokenContainingWhitespaceOrGlobCharacters', () => { + const r = runFlags(); + const lines = r.stdout.split('\n').filter(Boolean); + const hostileChars = [' ', '\t', '*', '?', '[', ']', '$', '`', ';', '&', '|', '(', ')']; + for (const line of lines) { + for (const ch of hostileChars) { + assert.ok(!line.includes(ch), `expected ${JSON.stringify(line)} not to contain ${JSON.stringify(ch)}`); + } + } + }); + + test('honorsSelected', () => { + const r = runFlags(['--selected', 'antigravity']); + assert.strictEqual(r.status, 0); + const lines = r.stdout.split('\n').filter(Boolean); + assert.deepStrictEqual(lines, ['--antigravity', '--agy']); + }); + + test('dropsUnknownSlugsAndExitsZero', () => { + const r = runFlags(['--selected', 'nosuchlane']); + assert.strictEqual(r.status, 0); + assert.deepStrictEqual(r.stdout.split('\n').filter(Boolean), []); + }); + + test('emitsNoTrailingNewlineWhenEmpty', () => { + const r = runFlags(['--selected', 'nosuchlane']); + assert.strictEqual(r.stdout, ''); + }); + + test('defaultsToEveryLaneWhenSelectedIsEmpty', () => { + const withEmpty = runFlags(['--selected', '']); + const withNone = runFlags(); + assert.strictEqual(withEmpty.status, 0); + assert.strictEqual(withEmpty.stdout, withNone.stdout); + }); + + test('doesNotInterpolateHostileSelectors', () => { + const hostile = ';echo pwned;`id`;$(id)'; + const r = runFlags(['--selected', hostile]); + assert.strictEqual(r.status, 0); + assert.strictEqual(r.stdout, ''); + assert.ok(!r.stdout.includes('pwned')); + assert.ok(!r.stdout.includes('uid=')); + }); + + test('anUnknownSubcommandErrorsWithoutAStackTrace', () => { + const r = cp.spawnSync(process.execPath, [TOOLS, 'review-lane', 'bogus'], { encoding: 'utf8' }); + const combined = `${r.stdout || ''}${r.stderr || ''}`; + assert.match(combined, /flags/); + assert.ok(!combined.includes('at Object.')); + assert.ok(!combined.includes(' at ')); + }); +});