From 7372d99a2659b8f3754421a6f0bd671b5114fb5b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 30 Jul 2026 19:14:13 -0400 Subject: [PATCH] enhance(#2800): derive reviewer flag lists and gate reviewer lane docs across locales (#2882) * chore(#2800): derive reviewer flag lists and gate reviewer lane docs across locales The reviewer lane roster was hand-enumerated across five documentation surfaces and three workflow files that had drifted apart: --kimi-code was missing from all four translated COMMANDS.md mirrors, --coderabbit from every workflow forwarding list, and --antigravity from FEATURES.md. Adds checkReviewerDocsParity, a second pure gate deliberately separate from checkReviewerLaneParity so a stale doc cannot make the runtime checker look red. Workflows now derive their flag lists from a new review-lane flags query instead of hand-enumerating them, which also retires the unanchored grep that matched --agy inside --antigravity. Documents the previously absent reviewer body and hostBehaviors field in the capability manifest reference. Closes #2800 Closes #2781 Closes #2272 * fix(#2800): key the docs parity table arm on first-cell position Review found the flag arm was file-scoped, so the forwarding row that lists every flag in its third cell satisfied it on its own. Deleting a lane's own reviewer-table row -- the #2781 regression this gate exists to prevent -- therefore passed undetected. Arm 4 keys on the FIRST table cell, which separates a lane row from the forwarding row structurally and in every locale. Regression test included. * fix(#2800): shape-filter the flags subcommand output All three consumers read review-lane flags through an unquoted command substitution so the output word-splits into loop items. Phase 2 admits third-party overlay lanes, so an overlay flag containing whitespace would inject a second loop item and one containing a glob would expand against the cwd. Emit only well-formed flags so neither reaches the shell. * fix(#2800): remove the regex length ceiling and count only prose mentions Review found two real defects in the docs parity gate. The never-throws contract was false: building a RegExp from a declared flag or section title throws SyntaxError past ~100k chars, and Phase 2 admits overlay lanes whose declared strings are untrusted in length. Every one of these matches is literal, so String.includes replaces the regex outright, which also deletes escapeLiteral and the llama.cpp escaping it existed for. Arm 1 was context-blind: a flag mentioned only inside a fenced example or a commented-out row counted as documented. Both are stripped before matching. Also advertises all 13 lane flags in the argument-hint and corrects a stale eleven-lane count in the slug grammar note. * test(#2800): repoint the convergence suite off deleted workflow text The derived flag loop deleted the literal per-flag grep lines four tests matched on. Two of those failed loudly. The behavioral and property tests failed SILENTLY instead: their end marker no longer resolved, so the parse block extracted empty and both passed vacuously, and the property test's gsd_run stub had a no-op default that hid it. All now share one extractor and execute the real deployed block through a gsd_run shim backed by the actual binary. The whitelist assertions become an anti-parity check: re-adding a hand-written flag list must fail. Also repairs two vacuous cases in the docs parity suite. The unreadable-doc test called its own mock rather than the reader, and the integration test bounded nothing, so a doc losing its marker would have been silently skipped and still passed green. * fix(#2800): run the derived flag loop after the launcher preamble The remote matrix caught a real runtime bug, not a test artifact. In autonomous.md and plan-review-convergence.md the launcher preamble that defines gsd_run lives in a separate, LATER bash fence than the derived loop. Each fence is its own shell, so gsd_run was undefined where the loop ran: the command substitution yielded nothing and zero reviewer flags would have been forwarded. Worse than the drift this epic fixes, and silent. The whole CONVERGENCE_ARGS construction moves as one unit, because the --max-cycles append sits between the loop and the preamble and would otherwise have run against an uninitialized variable and then been dropped by the relocated initializer. Also documents all 13 lane flags in help/modes/full.md, which the repo gates bidirectionally against each command's argument-hint. * test(#2800): repoint the two converge suites off deleted flag literals Both asserted workflow.includes('--codex') against the hand-enumerated list the derived loop removed. They now assert the derivation itself, keep --all and --text (convergence controls, still literal), and add an anti-parity guard so re-adding a hardcoded list fails. The lost pass-through proof is replaced with a real one: every flag the tests used to hardcode is asserted present in the actual roster emitted by the binary, which is the property the old assertion was protecting. * test(#2800): acknowledge the workflow byte growth from the derived flag loop * chore(#2800): backfill changeset pr number to 2882 * fix(#2800): strip HTML comments to a fixed point in the parity gate CodeQL js/incomplete-multi-character-sanitization (high) on PR #2882: the single-pass strip can leave a live --> leaves a dangling --> rather than a live + -...-->), the ipt> shape, genuinely regresses on the single-pass strip and is what the test now uses. --------- Co-authored-by: Test --- .changeset/gallant-pandas-greet.md | 5 + CONTEXT.md | 2 +- commands/gsd/plan-review-convergence.md | 6 +- docs/COMMANDS.md | 6 +- docs/FEATURES.md | 2 +- docs/INVENTORY.md | 3 + docs/ja-JP/COMMANDS.md | 3 +- docs/ja-JP/FEATURES.md | 4 +- docs/ko-KR/COMMANDS.md | 3 +- docs/ko-KR/FEATURES.md | 4 +- docs/pt-BR/COMMANDS.md | 3 +- docs/reference/capability-manifest.md | 103 ++- docs/reference/capability-matrix.md | 4 +- docs/zh-CN/COMMANDS.md | 3 +- docs/zh-CN/FEATURES.md | 4 +- gsd-core/bin/gsd-tools.cjs | 26 +- gsd-core/workflows/autonomous.md | 30 +- gsd-core/workflows/help/modes/full.md | 2 +- gsd-core/workflows/next.md | 4 +- gsd-core/workflows/plan-review-convergence.md | 27 +- scripts/gen-capability-matrix.cjs | 4 +- skills/gsd-plan-review-convergence/SKILL.md | 6 +- src/review-lane-descriptor.cts | 363 +++++++- tests/adr-15-progress-converge.test.cjs | 45 +- tests/autonomous-converge.test.cjs | 45 +- tests/emitted-drift-ack.json | 6 + tests/plan-review-convergence.test.cjs | 134 +-- tests/reviewer-docs-parity.test.cjs | 798 ++++++++++++++++++ 28 files changed, 1533 insertions(+), 112 deletions(-) create mode 100644 .changeset/gallant-pandas-greet.md create mode 100644 tests/reviewer-docs-parity.test.cjs 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 ')); + }); +});