diff --git a/PRD.md b/PRD.md index 94d163e..8258c84 100644 --- a/PRD.md +++ b/PRD.md @@ -63,7 +63,7 @@ Aligned with [GOALS.md](./GOALS.md)'s Core Goals: |---|---|---|---| | FR-11 | The system MUST persist user-set defaults (e.g. `review-with`, `merge`, `autoUpdate`) to a per-environment config file and make them available to commands without the flag being re-specified each time. | Must | Setting `review-with` once via `/do:config` causes `/do:pr` to use it on a later run with no flag passed. | | FR-12 | Saved config MUST survive a filtered (single-command) uninstall — only a full uninstall or explicit reset clears it. | Must | Uninstalling just `push` leaves `.slashdo-config.json` intact. | -| FR-13 | Config values MUST round-trip complex reviewer syntax verbatim (bracket groups, `~opt`, `~max=`, `@login[bot]`). | Should | A saved `review-with` value with brackets and `~max=3` reads back identical to what was written. | +| FR-13 | Config values MUST round-trip complex reviewer syntax verbatim (bracket groups, `~opt`, `~max=`, `~effort=`, `@login[bot]`). | Should | A saved `review-with` value with brackets, `~max=3`, and `~effort=max` reads back identical to what was written. | ### Self-Update diff --git a/README.md b/README.md index f9ee19d..e94919e 100644 --- a/README.md +++ b/README.md @@ -56,8 +56,35 @@ That's it. slashdo detects your installed AI coding environments and installs co ## Workflows +> **Note on Command Syntax:** In the examples below, Claude Code syntax (`/do:`) is shown. Syntax varies slightly by AI assistant: +> - **Claude Code**: `/do:*` (e.g. `/do:plan-task`, `/do:next`, `/do:pr`) +> - **Codex**: `$do-*` (e.g. `$do-plan-task`, `$do-next`, `$do-pr`) +> - **Antigravity CLI (`agy`/`gemini`), OpenCode, Grok Build**: `/do-*` (e.g. `/do-plan-task`, `/do-next`, `/do-pr`) + Real end-to-end examples of how the commands compose. Every flag shown here is optional — the bare command always works. +### Typical Developer Loop: Plan → Implement → Custom Review PR + +A complete end-to-end workflow from idea to reviewed, merged PR: + +1. **Plan & file a decision-complete task:** + ``` + /do:plan-task add a --json flag to the export command + ``` + *Investigates the codebase, drafts a comprehensive issue with acceptance criteria, and files it in your tracker.* + +2. **Claim & implement the task in isolation:** + ``` + /do:next --issues #123 + ``` + *Claims issue `#123`, implements the solution in an isolated git worktree, verifies tests, and opens a PR.* + +3. **Ship with custom multi-agent code reviews:** + ``` + /do:pr --review-with=ollama[qwen2.5-coder:32b]~opt,codex[gpt-5.6-luna]~effort=max~opt --merge + ``` + *Runs a local fast Ollama pass (`~opt` non-blocking) followed by a maximum-effort Codex pass (`~effort=max~opt`), automatically applying fixes and merging once CI passes.* + ### Ship the work in your working tree You've been coding with your assistant and want it committed, pushed, and PR'd: @@ -239,11 +266,13 @@ Reviewers run **in the order listed**, and whatever you list is exactly what run **Optional reviewers** (`~opt` suffix): the reviewer runs and its findings get fixed, but an *inconclusive* result (timeout / skipped / no verdict) is excluded from the merge gate, so it never blocks `--merge`. A hard error from it (broken build / failed tests) still blocks. Use it for a second-opinion reviewer that doesn't reliably return a verdict, such as a local Ollama model. -**Per-reviewer iteration caps** (`~max=` suffix): caps how many **review → fix → re-review cycles** that one reviewer runs. It is the per-entry form of `--review-iterations`, and unlike that flag it reaches every reviewer type — including `codex`/`agy`/`claude`/`grok` and `ollama`, whose caps are otherwise fixed at 3 — so a single run can budget each reviewer differently: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` is a non-negative integer; `0` means "loop until clean", bounded by a 10-iteration safety guardrail. A reviewer that stops because it spent a cap *you* set reports `capped`, which counts as clean for the merge gate — as opposed to `guardrail`, which is what a *built-in* cap reports when it cuts off a reviewer that was still finding real problems, and which blocks the merge. +**Per-reviewer iteration caps** (`~max=` suffix): caps how many **review → fix → re-review cycles** that one reviewer runs. It is the per-entry form of `--review-iterations`, and unlike that flag it reaches every reviewer type — including `codex`/`agy`/`claude`/`grok` and `ollama`, whose caps are otherwise fixed at 3 — so a single run can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` is a non-negative integer; `0` means "loop until clean", bounded by a 10-iteration safety guardrail. A reviewer that stops because it spent a cap *you* set reports `capped`, which counts as clean for the merge gate — as opposed to `guardrail`, which is what a *built-in* cap reports when it cuts off a reviewer that was still finding real problems, and which blocks the merge. + +**Per-reviewer reasoning effort** (`~effort=` suffix): specifies the reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`) for that reviewer: `--review-with codex[gpt-5.6-luna]~effort=max~opt`, `--review-with claude~effort=high~max=2`. `~max` applies in `series` mode (the default). In `--review-mode parallel` each reviewer runs a single review-only pass and the orchestrator applies the union once, so there are no per-reviewer cycles to cap — `~max` is ignored there with a warning. -Both suffixes chain in either order and are shell-safe: `ollama[qwen2.5-coder:32b]~opt~max=1`. Neither affects reviewer identity, so `ollama~max=2` and `ollama` still dedupe to one pass. Both also ride through `/do:config` saved defaults. +All three suffixes chain in any order and are shell-safe: `codex[gpt-5.6-luna]~effort=max~opt~max=1`. None affects reviewer identity, so `ollama~effort=high` and `ollama` still dedupe to one pass. All ride through `/do:config` saved defaults. ### Loop flags @@ -385,13 +414,15 @@ Defaults are stored per host CLI (the one you run `/do:config` in) under a `defa ## Supported Environments -``` - Claude Code ~/.claude/commands/do/ YAML frontmatter + subdirectories - OpenCode ~/.config/opencode/commands/ YAML frontmatter + flat naming - Antigravity CLI ~/.gemini/antigravity-cli/skills/ Agent Skills (SKILL.md) — aliases: gemini, agy - Codex ~/.codex/skills/ SKILL.md per-command directories - Grok Build ~/.grok/skills/ SKILL.md per-command directories -``` +Each environment formats commands appropriately for its host assistant: + +| Assistant / Environment | Invocation Syntax | Installed Path | Format | +|:---|:---|:---|:---| +| **Claude Code** | `/do:` (e.g. `/do:plan-task`, `/do:pr`) | `~/.claude/commands/do/` | YAML frontmatter + subdirectories | +| **Codex** | `$do-` (e.g. `$do-plan-task`, `$do-pr`) | `~/.codex/skills/` | SKILL.md per-command directories | +| **Antigravity CLI** (`agy`/`gemini`) | `/do-` (e.g. `/do-plan-task`, `/do-pr`) | `~/.gemini/antigravity-cli/skills/` | Agent Skills (SKILL.md) | +| **OpenCode** | `/do-` (e.g. `/do-plan-task`, `/do-pr`) | `~/.config/opencode/commands/` | YAML frontmatter + flat naming | +| **Grok Build** | `/do-` (e.g. `/do-plan-task`, `/do-pr`) | `~/.grok/skills/` | SKILL.md per-command directories | slashdo auto-detects which environments you have installed. Or specify manually: diff --git a/commands/do/better-swift.md b/commands/do/better-swift.md index be46581..46e4ce4 100644 --- a/commands/do/better-swift.md +++ b/commands/do/better-swift.md @@ -15,7 +15,7 @@ Parse `$ARGUMENTS` for: - **`--interactive`**: pause at each decision point for user approval - **`--scan-only`**: run Phase 0 + 1 + 2 only (audit and plan), skip remediation — no worktree, no code changes, no PRs. **When `ISSUE_MODE` is also true, this is the "audit and file the work, don't touch my code" combination**: every surviving finding is filed as a labelled tracker issue before the run exits, not just the deferred subset (see the Phase 2 gate). `--scan-only` is the flag that stops the pipeline; `--issues` only chooses where findings are recorded - **`--no-merge`**: run through PR creation (Phase 5), skip the review loop and merge -- **`--review-with `**: which reviewer(s) run the Phase 6 review loop on each PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 6 skipped, PRs left open without merging — see Phase 6). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs — the per-entry form of `--review-iterations`, and the only way to move the local-agent / `ollama` caps (otherwise fixed at 3), so one run can budget each reviewer separately (`claude~max=2,ollama~max=1,codex~max=3`). `` is a non-negative integer (`0` = loop until clean, bounded by each loop's 10-iteration guardrail); strip it into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token in either order before slug parsing, and neither is part of the dedup identity (the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the merge gate. See `lib/multi-reviewer-loop.md`. Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. +- **`--review-with `**: which reviewer(s) run the Phase 6 review loop on each PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 6 skipped, PRs left open without merging — see Phase 6). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, or `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) to specify its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). Strip suffixes off the right of each token in any order before slug parsing. Deduplication preserves first-occurrence order and excludes `~` suffixes (survivor takes `~opt` if any had it, and cap/effort level from the first that carried them). Reject a malformed suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. - **`--review-stop-on-findings`** / **`--review-stop-on-clean`** (mutually exclusive): forwarded to the multi-reviewer loop for each PR; control when a per-PR reviewer list stops early. Set `REVIEW_STOP_MODE` (`all` default, `on-findings`, or `on-clean`). If both are present, abort with `--review-stop-on-findings and --review-stop-on-clean cannot be combined`. - **`--review-mode `**: forwarded to each PR's multi-reviewer loop. `series` (default) runs the reviewers one-at-a-time so each sees the prior's committed fixes; `parallel` runs their reviews concurrently against one baseline and applies the deduped union once (`--reviewer-applies` and the stop-modes are ignored in parallel). Set `REVIEW_MODE`; if omitted, leave it **unset for now** (saved-defaults fills it from `review-mode`; built-in default `series`). Abort with `--review-mode must be one of series, parallel (got: {value}).` on any other value. - **`--reviewer-applies`**: forwarded to each PR's review loop — the reviewing CLI applies fixes directly instead of the orchestrator (no effect on copilot or `@` passes, which are read-only cloud-side reviews). Record `REVIEWER_APPLIES=true`/`false`. diff --git a/commands/do/better.md b/commands/do/better.md index 70ef70f..b9c427f 100644 --- a/commands/do/better.md +++ b/commands/do/better.md @@ -16,7 +16,7 @@ Parse `$ARGUMENTS` for: - **`--scan-only`**: run Phase 0 + 1 + 2 only (audit and plan), skip remediation — no worktree, no code changes, no PRs. **When `ISSUE_MODE` is also true, this is the "audit and file the work, don't touch my code" combination**: every surviving finding is filed as a labelled tracker issue before the run exits, not just the deferred subset (see the Phase 2 gate). `--scan-only` is the flag that stops the pipeline; `--issues` only chooses where findings are recorded - **`--simplify-only`** (alias: **`--refactor-only`**): narrow the run to structural quality — refactoring, architecture, DRY, simplification, and cognitive load. Set `SIMPLIFY_ONLY=true`. The pipeline is unchanged (worktree remediation → per-category PRs → CI → review loop → merge); only the audit roster, the category set, and the remediation contract narrow. See [Simplify-Only Mode](#simplify-only-mode---simplify-only) for the exact deltas. `/do:simplify` is the shorthand for `/do:better --simplify-only` - **`--no-merge`**: run through PR creation (Phase 5), skip the review loop and merge -- **`--review-with `**: which reviewer(s) run the Phase 6 review loop on each PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 6 skipped, PRs left open without merging — see Phase 6). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs — the per-entry form of `--review-iterations`, and the only way to move the local-agent / `ollama` caps (otherwise fixed at 3), so one run can budget each reviewer separately (`claude~max=2,ollama~max=1,codex~max=3`). `` is a non-negative integer (`0` = loop until clean, bounded by each loop's 10-iteration guardrail); strip it into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token in either order before slug parsing, and neither is part of the dedup identity (the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the merge gate. See `lib/multi-reviewer-loop.md`. Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. +- **`--review-with `**: which reviewer(s) run the Phase 6 review loop on each PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 6 skipped, PRs left open without merging — see Phase 6). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, or `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) to specify its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). Strip suffixes off the right of each token in any order before slug parsing. Deduplication preserves first-occurrence order and excludes `~` suffixes (survivor takes `~opt` if any had it, and cap/effort level from the first that carried them). Reject a malformed suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. - **`--review-stop-on-findings`** / **`--review-stop-on-clean`** (mutually exclusive): forwarded to the multi-reviewer loop for each PR; control when a per-PR reviewer list stops early. Set `REVIEW_STOP_MODE` (`all` default, `on-findings`, or `on-clean`). If both are present, abort with `--review-stop-on-findings and --review-stop-on-clean cannot be combined`. - **`--review-mode `**: forwarded to each PR's multi-reviewer loop. `series` (default) runs the reviewers one-at-a-time so each sees the prior's committed fixes; `parallel` runs their reviews concurrently against one baseline and applies the deduped union once (`--reviewer-applies` and the stop-modes are ignored in parallel). Set `REVIEW_MODE`; if omitted, leave it **unset for now** (saved-defaults fills it from `review-mode`; built-in default `series`). Abort with `--review-mode must be one of series, parallel (got: {value}).` on any other value. - **`--reviewer-applies`**: forwarded to each PR's review loop — the reviewing CLI applies fixes directly instead of the orchestrator (no effect on copilot or `@` passes, which are read-only cloud-side reviews). Record `REVIEWER_APPLIES=true`/`false`. diff --git a/commands/do/config.md b/commands/do/config.md index 355fc7f..c4f1a00 100644 --- a/commands/do/config.md +++ b/commands/do/config.md @@ -42,7 +42,7 @@ At read time, **per-project overrides global, key by key** (see `lib/review-conf - **Set** — one or more recognized setting flags are present: validate and merge them into `defaults`. `--unset` and setting flags may be combined in one call; apply unsets first, then sets. 3. **Recognized setting flags** (parse and validate each that is present — reuse the exact same rules the review commands use): - - `--review-with ` → key `review-with`. Split on `,`, trim, then for each entry strip **every** trailing `~` suffix — `~opt` into a per-entry optional flag and `~max=` into a per-entry iteration cap — **before** validating the remaining slug (so an optional or capped reviewer saves correctly; the two suffixes may appear in either order). Validate each `~max` value as a non-negative integer, at most one per entry, rejecting anything else with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and may appear at most once; the only suffixes are ~opt and ~max=.` Then normalize `gemini`/`antigravity` → `agy`, validate each remaining slug ∈ {`copilot`, `codex`, `agy`, `claude`, `grok`, `ollama`} with an optional `[]` bracket on the model-taking reviewers `codex`/`claude`/`agy`/`grok`/`ollama` (the bracket value is free-form — model names churn and may contain spaces/parens, e.g. `agy[Gemini 3.5 Flash (High)]` — so validate its shape, not against an allowlist; `copilot[…]` and `@login[…]` are invalid) — or an arbitrary GitHub login `@` (a user or App/bot whose review is requested on the PR; login must match `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$`) — dedupe preserving first-occurrence order (each model-taking agent's `[]` bracket suffix and the `@` login, lowercased, are part of the identity; neither `~` suffix is — `ollama~opt`, `ollama~max=2` and `ollama` all collapse; optional-wins for `~opt`, first-occurrence-wins for `~max`). Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` Store the **normalized, deduped** string with each entry's `~` suffixes **re-appended in canonical order — `~opt` first, then `~max=`** (e.g. `claude~max=2,ollama~opt~max=1,codex`), so both markers ride through to the review commands that consume the saved default. Canonicalizing the order here means a stored value is byte-stable no matter which order the user typed the suffixes in. The literal `none` (case-insensitive) is also accepted and stored verbatim as `none` — it is an explicit "no external reviewer" tombstone. This is most useful with `--project`: a project-scoped `review-with=none` masks an inherited **global** reviewer default for that one repo (per-project overrides global at read time), which `--unset review-with` cannot do (unsetting the project key just falls back to the global value). The review commands resolve a saved `none` to `REVIEW_AGENTS=[]`. Use `--unset review-with` when you instead want to *remove* the key and fall back to the lower-precedence default. + - `--review-with ` → key `review-with`. Split on `,`, trim, then for each entry strip **every** trailing `~` suffix — `~opt` into a per-entry optional flag, `~max=` into a per-entry iteration cap, and `~effort=` into a per-entry reasoning effort — **before** validating the remaining slug (so an optional, capped, or effort-pinned reviewer saves correctly; the suffixes may appear in any order). Validate each `~max` value as a non-negative integer and each `~effort` value as one of `low`, `medium`, `high`, `xhigh`, `max`, at most one of each per entry, rejecting anything else with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Then normalize `gemini`/`antigravity` → `agy`, validate each remaining slug ∈ {`copilot`, `codex`, `agy`, `claude`, `grok`, `ollama`} with an optional `[]` bracket on the model-taking reviewers `codex`/`claude`/`agy`/`grok`/`ollama` (the bracket value is free-form — model names churn and may contain spaces/parens, e.g. `agy[Gemini 3.5 Flash (High)]` — so validate its shape, not against an allowlist; `copilot[…]` and `@login[…]` are invalid) — or an arbitrary GitHub login `@` (a user or App/bot whose review is requested on the PR; login must match `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$`) — dedupe preserving first-occurrence order (each model-taking agent's `[]` bracket suffix and the `@` login, lowercased, are part of the identity; no `~` suffix is — `ollama~opt`, `ollama~max=2`, `ollama~effort=high` and `ollama` all collapse; optional-wins for `~opt`, first-occurrence-wins for `~max` and `~effort`). Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` Store the **normalized, deduped** string with each entry's `~` suffixes **re-appended in canonical order — `~opt` first, then `~max=`, then `~effort=`** (e.g. `claude~opt~max=2~effort=high,ollama[qwen2.5-coder:32b]~opt~max=1,codex`), so all markers ride through to the review commands that consume the saved default. Canonicalizing the order here means a stored value is byte-stable no matter which order the user typed the suffixes in. The literal `none` (case-insensitive) is also accepted and stored verbatim as `none` — it is an explicit "no external reviewer" tombstone. This is most useful with `--project`: a project-scoped `review-with=none` masks an inherited **global** reviewer default for that one repo (per-project overrides global at read time), which `--unset review-with` cannot do (unsetting the project key just falls back to the global value). The review commands resolve a saved `none` to `REVIEW_AGENTS=[]`. Use `--unset review-with` when you instead want to *remove* the key and fall back to the lower-precedence default. - `--review-models ` → key `review-models`, a JSON **object** keyed by agent slug (e.g. `{"codex":"o3","claude":"claude-opus-4-8"}`). Split the value on `,` into entries; split each entry on the **first** `=` into `slug=model`. Normalize `gemini`/`antigravity` → `agy`. Validate each slug ∈ {`codex`, `claude`, `agy`, `grok`, `ollama`} — the model-taking reviewers; reject `copilot`, an `@`, or any other slug with `--review-models only accepts codex, claude, agy, grok, ollama (got: {slug}) — copilot and @ reviewers take no model.` The model is a free-form string (trim surrounding whitespace; it may contain spaces/parens, e.g. `Gemini 3.5 Flash (High)`) — validate its shape, not against an allowlist. **Merge into the existing `review-models` object key-by-key** (so setting one agent's model leaves the others intact — do not clobber the whole map). An entry with an **empty** model — `codex=` — **removes** that agent's key from the map (the per-agent analog of `--unset`). Reject a malformed entry (no `=`, or an empty slug) with `--review-models entries must be = (got: {entry}).` Store the resulting object. `review-models` never selects which reviewers run (that is `--review-with`) — it only pins the model for a reviewer already listed there, and an explicit `[]` bracket (typed on the run or carried by a saved `review-with` default) overrides it. - `--review-iterations ` → key `review-iterations`. Must be a non-negative integer; else abort with `--review-iterations must be a non-negative integer (got: {value}).` Store as a number. - `--reviewer-applies` → key `reviewer-applies`, value `true`. Its explicit opposite `--no-reviewer-applies` → key `reviewer-applies`, value `false` — store this (rather than `--unset`) when a **project** default needs to override an inherited global `reviewer-applies=true` back off. (`--unset reviewer-applies` removes the key entirely and falls back to the lower-precedence value.) `--reviewer-applies` and `--no-reviewer-applies` are mutually exclusive. diff --git a/commands/do/depfree.md b/commands/do/depfree.md index 9fa727f..da8e9f5 100644 --- a/commands/do/depfree.md +++ b/commands/do/depfree.md @@ -18,7 +18,7 @@ Parse `$ARGUMENTS` for: - **`--scan-only`**: run Phase 0 + 1 + 2 only (audit and plan), skip remediation — no worktree, no code changes, no PRs. **When `ISSUE_MODE` is also true, this is the "audit and file the work, don't touch my code" combination**: every surviving finding is filed as a labelled tracker issue before the run exits, not just the deferred subset (see the Phase 2 gate). `--scan-only` is the flag that stops the pipeline; `--issues` only chooses where findings are recorded - **`--no-merge`**: run through PR creation, skip the review loop and merge - **`--heavy`**: aggressive mode — only keep foundational frameworks and language runtimes; replace everything else that is feasibly replaceable (see Heavy Mode below) -- **`--review-with `**: which reviewer(s) run the Phase 5c review loop on the PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 5c skipped, PR left open without merging — see Phase 5c/5d). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs — the per-entry form of `--review-iterations`, and the only way to move the local-agent / `ollama` caps (otherwise fixed at 3), so one run can budget each reviewer separately (`claude~max=2,ollama~max=1,codex~max=3`). `` is a non-negative integer (`0` = loop until clean, bounded by each loop's 10-iteration guardrail); strip it into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token in either order before slug parsing, and neither is part of the dedup identity (the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the merge gate. See `lib/multi-reviewer-loop.md`. Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. +- **`--review-with `**: which reviewer(s) run the Phase 5c review loop on the PR. Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself) (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Record as `REVIEW_AGENTS`. **There is no built-in default** — if omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (Phase 5c skipped, PR left open without merging — see Phase 5c/5d). `copilot` is never added implicitly. Any slot may end in `~opt` (e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it (timeout/skipped/incomplete/no-verdict) never blocks the PR merge (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, and it is **not** part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, or `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) to specify its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). Strip suffixes off the right of each token in any order before slug parsing. Deduplication preserves first-occurrence order and excludes `~` suffixes (survivor takes `~opt` if any had it, and cap/effort level from the first that carried them). Reject a malformed suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. - **`--review-stop-on-findings`** / **`--review-stop-on-clean`** (mutually exclusive): forwarded to the multi-reviewer loop; control when the reviewer list stops early. Set `REVIEW_STOP_MODE` (`all` default, `on-findings`, or `on-clean`). If both are present, abort with `--review-stop-on-findings and --review-stop-on-clean cannot be combined`. - **`--review-mode `**: forwarded to the multi-reviewer loop. `series` (default) runs the reviewers one-at-a-time so each sees the prior's committed fixes; `parallel` runs their reviews concurrently against one baseline and applies the deduped union once (`--reviewer-applies` and the stop-modes are ignored in parallel). Set `REVIEW_MODE`; if omitted, leave it **unset for now** (saved-defaults fills it from `review-mode`; built-in default `series`). Abort with `--review-mode must be one of series, parallel (got: {value}).` on any other value. - **`--reviewer-applies`**: forwarded to the review loop — the reviewing CLI applies fixes directly instead of the orchestrator (no effect on copilot or `@` passes, which are read-only cloud-side reviews). Record `REVIEWER_APPLIES=true`/`false`. diff --git a/commands/do/pr.md b/commands/do/pr.md index 4299c12..1360ee1 100644 --- a/commands/do/pr.md +++ b/commands/do/pr.md @@ -11,12 +11,13 @@ Parse `$ARGUMENTS` for `--review-with `: - `codex`, `claude`, `agy`, and `grok` likewise accept a `[]` bracket to pin the reviewing model — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `agy[Gemini 3.5 Flash (High)]`, `grok[grok-code-fast-1]`. Strip the bracket into a per-entry `REVIEW_MODEL` (empty → the reviewer's built-in default: the codex/claude/grok CLI default, or agy's pinned default) and keep the base slug. The bracket value is free-form (may contain spaces/parens — validate shape, not an allowlist); `copilot` and `@` take **no** model bracket. When a token omits the bracket, a saved `review-models` default (see `/do:config`) supplies the model — the explicit bracket always wins. Full mechanics in `lib/multi-reviewer-loop.md`. - `@` requests a review from an **arbitrary GitHub reviewer** — any user or App/bot login (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`). slashdo requests their review on the PR and waits for it, fixing what it surfaces (same request → poll → fix → resolve flow as `copilot`). Strip the leading `@` into a per-entry `REVIEWER_LOGIN`; the login must match `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$` (reject otherwise with the unknown-value abort below). GitHub only (skipped on GitLab). slashdo never posts an approval itself — it only requests, waits, and fixes. - **Optional (non-blocking) suffix `~opt`:** any slot may end in `~opt` — e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`, `@some-bot~opt`, `copilot~opt` — to mark that reviewer **optional**: it is still requested, still runs, and its findings are still fixed, but an *inconclusive* result from it (timeout / skipped / incomplete / no-verdict) is **excluded from the merge gate** and never blocks `--merge`. A hard-error from it (broken build / failed tests / rejected) still blocks — optionality never lets a broken tree merge. Strip the `~opt` suffix into a per-entry `{OPTIONAL}` flag **before** the slug/`[model]`/`@login` parsing; it is **not** part of the dedup identity (`ollama~opt` and `ollama` are the same reviewer, optional-wins on collapse). `~opt` is shell-metacharacter-free by design. Full mechanics in `lib/multi-reviewer-loop.md`. -- **Per-reviewer iteration cap suffix `~max=`:** any slot may also end in `~max=` — e.g. `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3` — capping how many **review → fix → re-review cycles** that one reviewer runs. This is the per-entry form of `--review-iterations` and, unlike that flag, it applies to every reviewer type including the local agents and `ollama` (whose caps are otherwise fixed at 3), so one call can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` must be a non-negative integer; `0` means "loop until clean", bounded by each inner loop's 10-iteration safety guardrail. Strip `~max=` into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token, in either order, **before** the slug/`[model]`/`@login` parsing, and neither is part of the dedup identity (on collapse, the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). Reject a malformed or repeated suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and may appear at most once; the only suffixes are ~opt and ~max=.` A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the merge gate. Full mechanics in `lib/multi-reviewer-loop.md`. +- **Per-reviewer iteration cap suffix `~max=`:** any slot may also end in `~max=` — e.g. `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3` — capping how many **review → fix → re-review cycles** that one reviewer runs. This is the per-entry form of `--review-iterations` and, unlike that flag, it applies to every reviewer type including the local agents and `ollama` (whose caps are otherwise fixed at 3), so one call can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` must be a non-negative integer; `0` means "loop until clean", bounded by each inner loop's 10-iteration safety guardrail. Strip `~max=` into a per-entry `{ENTRY_MAX}` alongside `~opt` and `~effort=` — all suffixes come off the right of the token, in any order, **before** the slug/`[model]`/`@login` parsing. Full mechanics in `lib/multi-reviewer-loop.md`. +- **Per-reviewer reasoning effort suffix `~effort=`:** any slot may also end in `~effort=` — e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2` — specifying the reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`) for that reviewer. Strip `~effort=` into a per-entry `{ENTRY_EFFORT}` alongside `~opt` and `~max=`. Reject a malformed or repeated suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Full mechanics in `lib/multi-reviewer-loop.md`. - **Reserved value `none`:** the token `none` (case-insensitive) is not a reviewer slug. `--review-with none` means *no external reviewer this run* — set `REVIEW_AGENTS=[]`, skip the slug validation below, and skip applying any saved `review-with` default. This is the explicit escape hatch over a default saved via `/do:config`. - The value may be a single agent or a **comma-separated, ordered list** (e.g. `--review-with codex,agy,copilot`). Split on `,`, trim whitespace around each slug. Normalize `gemini`/`antigravity` → `agy`. - Record the resulting list as `REVIEW_AGENTS`. **There is no built-in default reviewer.** If `--review-with` is omitted, leave `REVIEW_AGENTS` **unset for now** — the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** does the built-in default apply (`REVIEW_AGENTS=[]` — no external review pass; the Local Code Review gate below still runs unconditionally). Whatever ends up in the list is exactly what runs, in order: `--review-with codex` runs codex only; copilot is never added implicitly. -- Dedupe preserving first-occurrence order (compare on the normalized slug — for a model-taking agent (`codex`/`claude`/`agy`/`grok`/`ollama`) the `[]` bracket suffix is part of the identity, so `codex[a]` and `codex[b]` are distinct while two bare `ollama`s collapse; for `@` the login is the identity, compared lowercased, so `@Octocat`/`@octocat` collapse while distinct logins stay separate; neither `~` suffix is part of the identity, so `ollama~opt` and `ollama~max=2` both collapse with `ollama` — the survivor is optional if any collapsed occurrence had `~opt`, and takes its cap from the first occurrence that carried a `~max`); if duplicates were dropped, print: `Note: deduped --review-with list to {final list}.` -- If any value is not in the accepted set, abort with a usage error: `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` +- Dedupe preserving first-occurrence order (compare on the normalized slug — for a model-taking agent (`codex`/`claude`/`agy`/`grok`/`ollama`) the `[]` bracket suffix is part of the identity, so `codex[a]` and `codex[b]` are distinct while two bare `ollama`s collapse; for `@` the login is the identity, compared lowercased, so `@Octocat`/`@octocat` collapse while distinct logins stay separate; no `~` suffix is part of the identity, so `ollama~opt`, `ollama~max=2`, `ollama~effort=high` all collapse with `ollama` — the survivor is optional if any collapsed occurrence had `~opt`, and takes its cap and effort level from the first occurrence that carried them); if duplicates were dropped, print: `Note: deduped --review-with list to {final list}.` +- If any value is not in the accepted set, abort with a usage error: `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` Parse `$ARGUMENTS` for the stop-mode flags (mutually exclusive): - `--review-stop-on-findings` — stop the multi-reviewer loop after the first reviewer that fixed at least one finding (subsequent reviewers in the list are skipped). diff --git a/commands/do/release.md b/commands/do/release.md index 4923900..7ea615e 100644 --- a/commands/do/release.md +++ b/commands/do/release.md @@ -15,12 +15,13 @@ Parse `$ARGUMENTS` for `--review-with `: - `codex`, `claude`, `agy`, and `grok` likewise accept a `[]` bracket to pin the reviewing model — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `agy[Gemini 3.5 Flash (High)]`, `grok[grok-code-fast-1]`. Strip the bracket into a per-entry `REVIEW_MODEL` (empty → the reviewer's built-in default) and keep the base slug. The bracket value is free-form (validate shape, not an allowlist); `copilot` and `@` take no model bracket. A saved `review-models` default (see `/do:config`) supplies the model when a token omits the bracket — the bracket wins. See `lib/multi-reviewer-loop.md`. - `@` requests a review from an **arbitrary GitHub reviewer** — any user or App/bot login (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`). slashdo requests their review on the PR and waits for it, fixing what it surfaces (same flow as `copilot`). Strip the leading `@` into a per-entry `REVIEWER_LOGIN`; the login must match `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$`. GitHub only; never posts an approval itself. - **Optional (non-blocking) suffix `~opt`:** any slot may end in `~opt` — e.g. `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`, `@some-bot~opt`, `copilot~opt` — to mark that reviewer **optional**: it is still requested, still runs, and its findings are still fixed, but an *inconclusive* result from it (timeout / skipped / incomplete / no-verdict) is **excluded from the merge gate** and never blocks the release merge. A hard-error from it (broken build / failed tests / rejected) still blocks — optionality never lets a broken tree merge. Strip the `~opt` suffix into a per-entry `{OPTIONAL}` flag **before** the slug/`[model]`/`@login` parsing; it is **not** part of the dedup identity (`ollama~opt` and `ollama` are the same reviewer, optional-wins on collapse). `~opt` is shell-metacharacter-free by design. Full mechanics in `lib/multi-reviewer-loop.md`. -- **Per-reviewer iteration cap suffix `~max=`:** any slot may also end in `~max=` — e.g. `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3` — capping how many **review → fix → re-review cycles** that one reviewer runs. This is the per-entry form of `--review-iterations` and, unlike that flag, it applies to every reviewer type including the local agents and `ollama` (whose caps are otherwise fixed at 3), so one call can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` must be a non-negative integer; `0` means "loop until clean", bounded by each inner loop's 10-iteration safety guardrail. Strip `~max=` into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token, in either order, **before** the slug/`[model]`/`@login` parsing, and neither is part of the dedup identity (on collapse, the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). Reject a malformed or repeated suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and may appear at most once; the only suffixes are ~opt and ~max=.` A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the release merge gate — unlike `guardrail` (a built-in cap cutting off a still-productive loop), which still blocks. Full mechanics in `lib/multi-reviewer-loop.md`. +- **Per-reviewer iteration cap suffix `~max=`:** any slot may also end in `~max=` — e.g. `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3` — capping how many **review → fix → re-review cycles** that one reviewer runs. This is the per-entry form of `--review-iterations` and, unlike that flag, it applies to every reviewer type including the local agents and `ollama` (whose caps are otherwise fixed at 3), so one call can budget each reviewer separately: `--review-with claude~max=2,ollama~max=1,codex~max=3`. `` must be a non-negative integer; `0` means "loop until clean", bounded by each inner loop's 10-iteration safety guardrail. Strip `~max=` into a per-entry `{ENTRY_MAX}` alongside `~opt` and `~effort=` — all suffixes come off the right of the token, in any order, **before** the slug/`[model]`/`@login` parsing. Full mechanics in `lib/multi-reviewer-loop.md`. +- **Per-reviewer reasoning effort suffix `~effort=`:** any slot may also end in `~effort=` — e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2` — specifying the reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`) for that reviewer. Strip `~effort=` into a per-entry `{ENTRY_EFFORT}` alongside `~opt` and `~max=`. Reject a malformed or repeated suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Full mechanics in `lib/multi-reviewer-loop.md`. - **Reserved value `none`:** the token `none` (case-insensitive) is not a reviewer slug. `--review-with none` means *no external reviewer this run* — set `REVIEW_AGENTS=[]`, skip the slug validation below, and skip applying any saved `review-with` default. This is the explicit escape hatch over a default saved via `/do:config`. - The value may be a single agent or a **comma-separated, ordered list** (e.g. `--review-with codex,agy,copilot`). Split on `,`, trim whitespace around each slug. Normalize `gemini`/`antigravity` → `agy`. - Record the resulting list as `REVIEW_AGENTS`. **There is no built-in default reviewer.** If `--review-with` is omitted, leave `REVIEW_AGENTS` **unset for now** — the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** does the built-in default apply (`REVIEW_AGENTS=[]` — no external review pass; the Local Code Review gate below still runs unconditionally). Whatever ends up in the list is exactly what runs, in order: `--review-with codex` runs codex only; copilot is never added implicitly. -- Dedupe preserving first-occurrence order (compare on the normalized slug — for a model-taking agent (`codex`/`claude`/`agy`/`grok`/`ollama`) the `[]` bracket suffix is part of the identity, so `codex[a]` and `codex[b]` are distinct while two bare `ollama`s collapse; for `@` the login is the identity, compared lowercased; neither `~` suffix is part of the identity, so `ollama~opt` and `ollama~max=2` both collapse with `ollama` — the survivor is optional if any collapsed occurrence had `~opt`, and takes its cap from the first occurrence that carried a `~max`); if duplicates were dropped, print: `Note: deduped --review-with list to {final list}.` -- If any value is not in the accepted set, abort with a usage error: `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` +- Dedupe preserving first-occurrence order (compare on the normalized slug — for a model-taking agent (`codex`/`claude`/`agy`/`grok`/`ollama`) the `[]` bracket suffix is part of the identity, so `codex[a]` and `codex[b]` are distinct while two bare `ollama`s collapse; for `@` the login is the identity, compared lowercased; no `~` suffix is part of the identity, so `ollama~opt`, `ollama~max=2`, `ollama~effort=high` all collapse with `ollama` — the survivor is optional if any collapsed occurrence had `~opt`, and takes its cap and effort level from the first occurrence that carried them); if duplicates were dropped, print: `Note: deduped --review-with list to {final list}.` +- If any value is not in the accepted set, abort with a usage error: `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` Parse `$ARGUMENTS` for the stop-mode flags (mutually exclusive): - `--review-stop-on-findings` — stop the multi-reviewer loop after the first reviewer that fixed at least one finding. diff --git a/commands/do/review.md b/commands/do/review.md index 934f1bb..5657976 100644 --- a/commands/do/review.md +++ b/commands/do/review.md @@ -8,7 +8,7 @@ argument-hint: "[--strict|--nuclear] [--draft] [--review-with [,.. Parse `$ARGUMENTS` for: - **`--strict`** (alias: **`--nuclear`**): enable the Structural Ambition agent (6th agent) and promote structural findings to blocker tier. Use for branches you want to land cleanly — flags file-size growth past 1000 lines, ad-hoc conditionals bolted onto unrelated flows, thin wrappers, boundary leaks, and missed code-judo simplifications. - **`--draft`** (PR mode only): write the review payload to `/tmp/do-review-pr-{PR_NUM}-payload.json` and print the `gh api` command to publish it manually, instead of posting the review immediately. Ignored when `PR_MODE=false`. -- **`--review-with `** (optional): after the host CLI's self-review completes (the multi-agent flow defined below), delegate **additional** review passes to the named external CLIs in order. Accepted slugs per slot: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself). Split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order (for a model-taking agent — `codex`/`claude`/`agy`/`grok`/`ollama` — the `[]` bracket suffix is part of the dedup identity). Any slot may end in `~opt` (e.g. `ollama~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it never contributes a merge-blocking `inconclusive` aggregate (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, not part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs — the per-entry form of `--review-iterations`, and the only way to move the local-agent / `ollama` caps (otherwise fixed at 3), so one run can budget each reviewer separately (`claude~max=2,ollama~max=1,codex~max=3`). `` is a non-negative integer (`0` = loop until clean, bounded by each loop's 10-iteration guardrail); strip it into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token in either order before slug parsing, and neither is part of the dedup identity (the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). A reviewer that stops because it reached a `~max` you set returns `capped`, which is clean-equivalent for the merge gate. See `lib/multi-reviewer-loop.md`. Abort with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` on any unknown slug. The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no delegated reviewers (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. If omitted, leave `REVIEW_AGENTS` **unset for now** — the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (no delegated passes — behavior matches the historical `/do:review` self-review only). **The host CLI is not implied in this list** — whichever CLI is hosting the review command (claude, codex, or agy) runs the self-review first regardless. The list names *additional* reviewers; an explicit `claude` entry while running under claude means "start a fresh claude headless session for a second-pass perspective," which is allowed. +- **`--review-with `** (optional): after the host CLI's self-review completes (the multi-agent flow defined below), delegate **additional** review passes to the named external CLIs in order. Accepted slugs per slot: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you name it, never selected implicitly), or an arbitrary GitHub login `@` — any GitHub user or App/bot (e.g. `@octocat`, `@org-review-bot`, `@some-app[bot]`); slashdo requests its review on the PR and waits for it (GitHub only, never posts an approval itself). Split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order (for a model-taking agent — `codex`/`claude`/`agy`/`grok`/`ollama` — the `[]` bracket suffix is part of the dedup identity). Any slot may end in `~opt` (e.g. `ollama~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it never contributes a merge-blocking `inconclusive` aggregate (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, not part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, or `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) to specify its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). Strip suffixes off the right of each token in any order before slug parsing. Deduplication preserves first-occurrence order and excludes `~` suffixes (survivor takes `~opt` if any had it, and cap/effort level from the first that carried them). Reject a malformed suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Abort with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` on any unknown slug. The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no delegated reviewers (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. If omitted, leave `REVIEW_AGENTS` **unset for now** — the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (no delegated passes — behavior matches the historical `/do:review` self-review only). **The host CLI is not implied in this list** — whichever CLI is hosting the review command (claude, codex, or agy) runs the self-review first regardless. The list names *additional* reviewers; an explicit `claude` entry while running under claude means "start a fresh claude headless session for a second-pass perspective," which is allowed. - **`--review-stop-on-findings` / `--review-stop-on-clean`** (mutually exclusive, optional): stop-mode for the delegated passes. Default `REVIEW_STOP_MODE=all` (run every listed agent). `on-findings` stops after the first delegated reviewer that surfaces a non-empty change set; `on-clean` stops after the first delegated reviewer that reports zero findings. Abort with `--review-stop-on-findings and --review-stop-on-clean cannot be combined` if both appear. - **`--review-mode `** (optional): how the delegated passes are dispatched. `series` (default) runs the listed reviewers one-at-a-time so each sees the prior reviewer's committed fixes; `parallel` runs their reviews concurrently against one frozen baseline and then applies the deduped union of findings once (faster, but no reviewer sees another's fixes — `--reviewer-applies` and the stop-modes are ignored in this mode). Record as `REVIEW_MODE`; if omitted, leave it **unset for now** (the saved-defaults step fills it from the `review-mode` default; built-in default `series`). Abort with `--review-mode must be one of series, parallel (got: {value}).` on any other value. - **`--reviewer-applies`** (optional, boolean): forwarded to each delegated local-agent pass to route fixes through the reviewing CLI instead of the orchestrator. See `lib/local-agent-review-loop.md` "Editing mode" for the trade-offs. No effect on the copilot path, the `@` path, the ollama path (Ollama is non-agentic — always review-only), or the host's self-review. diff --git a/commands/do/rpr.md b/commands/do/rpr.md index 7a4fe57..1ea8ceb 100644 --- a/commands/do/rpr.md +++ b/commands/do/rpr.md @@ -14,7 +14,7 @@ Address the latest code review feedback on the current branch's pull request usi ## Parse Arguments Parse `$ARGUMENTS` for `--review-with `: -- Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you ask for it, never selected implicitly), or an arbitrary GitHub login `@` (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Any slot may end in `~opt` (e.g. `ollama~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it never contributes a merge-blocking `inconclusive` aggregate (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, not part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, so one run can budget each reviewer separately (`claude~max=2,ollama~max=1`). `` is a non-negative integer (`0` = loop until clean, bounded by each loop's 10-iteration guardrail); strip it into a per-entry `{ENTRY_MAX}` alongside `~opt` — both suffixes come off the right of the token in either order before slug parsing, and neither is part of the dedup identity (the cap comes from the first occurrence that carried a `~max`, so a bare earlier occurrence does not erase a later cap). rpr forwards `{ENTRY_MAX}` as `{MAX_ITERATIONS}` (with `{MAX_EXPLICIT}=true`) to the **local-agent** and **Ollama** loops it dispatches — the same loops `/do:pr` uses, so the cap behaves identically there and a reviewer that spends it returns `capped`. It has **no effect on rpr's `copilot` entry**, which runs rpr's own bespoke request/monitor flow rather than the shared Copilot loop; rpr doesn't accept `--review-iterations` either, for the same reason. A `~max` on a `copilot` entry is accepted and ignored silently. See `lib/multi-reviewer-loop.md`. Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. +- Accepted slugs: `codex`, `agy` (aliases `gemini` / `antigravity` — all run the Antigravity CLI's `agy` binary), `claude`, `grok`, `ollama` (bare `ollama` auto-selects the most capable installed coding model; `ollama[]` pins a specific installed model, e.g. `ollama[qwen2.5-coder:32b]` — strip the bracket into a per-entry `OLLAMA_MODEL`; `codex`/`claude`/`agy`/`grok` likewise accept a `[]` bracket — e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `grok[grok-code-fast-1]` — stripped into a per-entry `REVIEW_MODEL`, empty → the reviewer's built-in default; `copilot` and `@` take no model bracket), `copilot` (**legacy** — GitHub's cloud Copilot review; still supported when you ask for it, never selected implicitly), or an arbitrary GitHub login `@` (comma-separated, ordered list; split on `,`, trim whitespace, normalize `gemini`/`antigravity` → `agy`, dedupe preserving first-occurrence order, with each model-taking agent's (`codex`/`claude`/`agy`/`grok`/`ollama`) `[]` bracket suffix part of the dedup identity). Any slot may end in `~opt` (e.g. `ollama~opt`) to mark that reviewer **optional/non-blocking** — still requested and its findings still fixed, but an inconclusive result from it never contributes a merge-blocking `inconclusive` aggregate (a hard-error from it still does); strip `~opt` into a per-entry `{OPTIONAL}` flag before slug parsing, not part of the dedup identity (`ollama~opt` == `ollama`, optional-wins on collapse). A slot may also end in `~max=` (e.g. `claude~max=2`, `ollama~max=1`) to cap how many review → fix → re-review cycles **that one reviewer** runs, or `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) to specify its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). Strip suffixes off the right of each token in any order before slug parsing. Deduplication preserves first-occurrence order and excludes `~` suffixes (survivor takes `~opt` if any had it, and cap/effort level from the first that carried them). Reject a malformed suffix with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` rpr forwards `{ENTRY_MAX}` as `{MAX_ITERATIONS}` and `{ENTRY_EFFORT}` as `{REVIEW_EFFORT}` / `{OLLAMA_EFFORT}` to the **local-agent** and **Ollama** loops it dispatches — the same loops `/do:pr` uses. It has **no effect on rpr's `copilot` entry**, which runs rpr's own bespoke request/monitor flow rather than the shared Copilot loop. See `lib/multi-reviewer-loop.md`. Abort on an unknown slug with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` The reserved token `none` (case-insensitive) is **not** validated as a slug — `--review-with none` means no reviewer (set `REVIEW_AGENTS=[]`) and overrides any saved `review-with` default. - **`@` entries are accepted by the parser but never requested**: rpr's only GitHub-side request path is its bespoke Copilot request/monitor flow, not the multi-reviewer loop, so there is no dispatch path for an arbitrary GitHub reviewer here yet (tracked as a follow-up). Drop any `@` entry from `REVIEW_AGENTS` after parsing/dedup and print a one-line notice: `Note: @ is not yet supported by /do:rpr — dropped from --review-with.` This applies whether the entry was typed directly or inherited from a saved `review-with` default — a global default saved for `/do:pr`/`/do:release`/etc. that happens to include `@` must not make every `/do:rpr` invocation abort. - Record as `REVIEW_AGENTS`. **There is no built-in default reviewer** — `copilot` is never added implicitly. If `--review-with` is omitted, leave `REVIEW_AGENTS` **unset for now**; the saved-defaults step below fills it from `/do:config` if a default exists, and **only if it is still unset after that** is `REVIEW_AGENTS=[]` (rpr requests no new review and instead fetches and resolves whatever unresolved threads the PR already carries — its reviewer-agnostic core job). - **`@`-only values resolve to no reviewer**: if dropping `@` entries per the bullet above leaves `REVIEW_AGENTS` empty, the emptiness stands — set `REVIEW_AGENTS=[]` and run the no-reviewer path (resolve existing threads only). Do **not** fall through to the saved-defaults step when the user explicitly typed `--review-with` this run; an explicit flag already wins over a saved default, per the precedence rule below. diff --git a/lib/local-agent-review-loop.md b/lib/local-agent-review-loop.md index 4331962..216c79b 100644 --- a/lib/local-agent-review-loop.md +++ b/lib/local-agent-review-loop.md @@ -25,6 +25,7 @@ When to use this: 5. Record `{REVIEWER_APPLIES}` — boolean, defaults to `false`. Set to `true` when the orchestrating command was invoked with `--reviewer-applies`. This flag selects which side of the loop holds the editor: when `false` (default), the orchestrator applies fixes from the CLI's findings log; when `true`, the headless CLI applies fixes directly in the working tree and the orchestrator only verifies. 6. Record `{REVIEW_MODEL}` — the model to run this reviewer on, resolved by the caller (the multi-reviewer loop: explicit `[]` bracket → saved `review-models[slug]` default → empty). **May be empty**, which means "use the reviewer's built-in default" — for `codex`/`claude`/`grok` that is the CLI's own default model (no `--model` flag passed); for `agy` it is the pinned `AGY_REVIEW_MODEL` default resolved below. When set, it is passed through to the reviewer's invocation (`codex --model`, `claude --model` / the in-process `Agent` tool's `model`, `agy --model`, or `grok --model`) so a run/config can pin which model reviews. The value is free-form (model names churn and may contain spaces/parens, e.g. `Gemini 3.5 Flash (High)`) — do not validate it against an allowlist; pass it verbatim. 7. Record `{MAX_ITERATIONS}` — how many review → fix → re-review cycles this reviewer may run, resolved by the caller (the multi-reviewer loop: a per-entry `~max=` suffix on the `--review-with` token → this loop's built-in default of `3`). **Defaults to `3`** when the caller passes nothing, which is the historical behavior. `0` means **unlimited** — loop until the reviewer is clean or the convergence gate converges, bounded by the 10-iteration safety guardrail in Step 6. Also record `{MAX_EXPLICIT}` — boolean, `true` only when the cap came from a `~max=` the user typed (or saved), `false` when it is this loop's built-in `3`. Step 6 uses it to decide whether exhausting the cap is `capped` (a budget the user chose — clean-equivalent for the merge gate) or `guardrail` (a built-in ceiling nobody vouched for — inconclusive). Note the `--review-iterations` flag never reaches this loop; `~max` is the only way to move this cap. +8. Record `{REVIEW_EFFORT}` — optional reasoning effort string for this reviewer (`low`, `medium`, `high`, `xhigh`, `max`), resolved by the caller (the multi-reviewer loop: explicit `~effort=` suffix on the `--review-with` token → empty). **Defaults to empty** when unset. When set, it is appended as advisory reasoning effort to the prompt preamble and passed as `--effort` where supported. ### Editing mode @@ -54,6 +55,7 @@ REVIEW_TITLE=$(git log -1 --format=%s HEAD) # subject of HEAD commit; falls ba # Shared review task. The "do NOT dispatch/spawn sub-agents" clause is load-bearing: # it is what keeps the review a single synchronous agent the print-mode CLI can wait on. REVIEW_TASK="Review the code changes on the current branch against the base branch '$BASE_BRANCH'. Do the review YOURSELF in this single session — do NOT dispatch, spawn, or delegate to sub-agents or background tasks (a fanned-out review never re-syncs into print/headless output and the run will time out with no findings). Run \`git diff $BASE_BRANCH...HEAD --stat\` then \`git diff $BASE_BRANCH...HEAD\`, read each changed file in full for context, and review for correctness bugs, security issues, broken producer/consumer contracts, resource leaks, and missing test coverage. The project's linter, type-checker, and test suite already run separately — do NOT spend effort on syntax, lint, formatting, import order, or build errors; they are covered. Report only logic issues found by reasoning about behavior, each tied to a concrete wrong outcome — not style preferences, renames, or 'extract a helper' suggestions." +[ -n "$REVIEW_EFFORT" ] && REVIEW_TASK="$REVIEW_TASK Target reasoning effort level: $REVIEW_EFFORT." if [ "$REVIEWER_APPLIES" = "true" ]; then LOCAL_PROMPT="$REVIEW_TASK @@ -77,6 +79,7 @@ fi # Codex-only prompt for REVIEWER_APPLIES=true (codex exec invocation — # codex doesn't have slashdo installed, so describe the task directly). CODEX_APPLY_PROMPT="Review the diff from $BASE_BRANCH to HEAD in this repo for logic issues (correctness, security, test coverage, contract drift). The linter, type-checker, and test suite already run separately — do NOT spend effort on syntax, lint, formatting, or build errors, and do NOT raise style/rename/extract-a-helper suggestions; report only behavior bugs you can tie to a concrete wrong outcome. For each finding, apply the fix in the working tree, then run \`$BUILD_CMD\` (skip if empty) and \`$TEST_CMD\` to verify, and commit each fix with message 'address review (codex): '. Do not introduce changes beyond the scope of fixing the findings. Do not skip tests or weaken assertions." +[ -n "$REVIEW_EFFORT" ] && CODEX_APPLY_PROMPT="$CODEX_APPLY_PROMPT Target reasoning effort level: $REVIEW_EFFORT." # Resolve the timeout wrapper used by the step-2 invocation # (`${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} {INVOCATION}`). @@ -106,6 +109,8 @@ elif command -v gtimeout >/dev/null 2>&1; then TIMEOUT_CMD=(gtimeout 1800); fi # (built-in default), so its flag is never empty. MODEL_FLAG=() [ -n "$REVIEW_MODEL" ] && MODEL_FLAG=(--model "$REVIEW_MODEL") +EFFORT_FLAG=() +[ -n "$REVIEW_EFFORT" ] && EFFORT_FLAG=(--effort "$REVIEW_EFFORT") # agy only: pin the review model. A per-run/config model wins via {REVIEW_MODEL} # (the `agy[]` bracket or a saved `review-models` default), then the @@ -137,17 +142,17 @@ Pick the invocation based on `{REVIEW_AGENT}` and `{REVIEWER_APPLIES}`: | `claude` | Dispatch an in-process sub-agent via the `Agent` tool with `subagent_type: "general-purpose"` and `$LOCAL_PROMPT` (see Step 2) — **not** `claude -p`, so it stays on plan billing | Same sub-agent dispatch; the sub-agent applies and commits fixes directly in the shared working tree | -| `claude` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --dangerously-skip-permissions` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --dangerously-skip-permissions` | +| `claude` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --dangerously-skip-permissions` | `claude -p "$LOCAL_PROMPT" ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --dangerously-skip-permissions` | -| `codex` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --sandbox read-only review --base "$BASE_BRANCH" --title "$REVIEW_TITLE"` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} --sandbox danger-full-access -a never exec "$CODEX_APPLY_PROMPT"` | -| `agy` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m -p "$LOCAL_PROMPT"` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" --print-timeout 30m -p "$LOCAL_PROMPT"` | -| `grok` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} -p "$LOCAL_PROMPT"` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} -p "$LOCAL_PROMPT"` | +| `codex` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --sandbox read-only review --base "$BASE_BRANCH" --title "$REVIEW_TITLE"` | `codex ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --sandbox danger-full-access -a never exec "$CODEX_APPLY_PROMPT"` | +| `agy` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --print-timeout 30m -p "$LOCAL_PROMPT"` | `agy --dangerously-skip-permissions --model "$AGY_REVIEW_MODEL" ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} --print-timeout 30m -p "$LOCAL_PROMPT"` | +| `grok` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} -p "$LOCAL_PROMPT"` | `grok --permission-mode bypassPermissions ${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"} ${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"} -p "$LOCAL_PROMPT"` | For `claude`, `agy`, and `grok`, the same `$LOCAL_PROMPT` drives both modes — it already encodes the mode (review-only vs reviewer-applies) directly, branching on `$REVIEWER_APPLIES` above. For `codex`, the invocation itself swaps because `codex review` (review-only) and `codex exec` (apply-fixes) are different subcommands with incompatible flag sets. `--print-timeout 30m` raises agy's print-mode wait above its 5-minute default so a real review of a multi-file diff isn't cut off mid-stream; on stock macOS (no `timeout`/`gtimeout`, so `TIMEOUT_CMD` is empty) it is also the only *shell-level* bound on the invocation. **But these bounds only take effect when the invocation runs in the background (Step 2).** Run as a blocking foreground Bash call, the run is killed first by the host tool's ~10-minute foreground cap — earlier than either `timeout 1800` or `--print-timeout 30m` — which is the timeout consumers were hitting. `--print-timeout 30m` does NOT cut off an actively-streaming agent — it bounds the wait for the *next* response chunk — which is why it's safe to set generously, and why it never masked the old skill hang (that hang was the orchestrator sitting idle waiting on background sub-agents, not a slow stream). `--model "$AGY_REVIEW_MODEL"` pins the reviewing model (resolved in pre-flight): agy's *default* may be a heavy "Thinking" tier that spends many minutes in hidden reasoning plus multi-round tool calls. How much output is visible meanwhile is **model-dependent** — lighter models narrate their actions incrementally, heavy thinking tiers can emit nothing until the final answer — so on a slow model a routine review shows little or no output for 20-30 minutes and is easily mistaken for a hang. A quiet log during Step 2's poll is therefore NOT evidence the reviewer is stuck; only a `$DONE_FILE` with a non-zero code, or a 30-minute overrun, is. Pinning a fast-but-capable model keeps reviews prompt; bump `AGY_REVIEW_MODEL` to a heavier tier when you want more depth and accept the longer wait (the background launch + 30-minute bound cover it). > **Pass the prompt as a positional argument — never via stdin.** `claude -p`, `agy -p` (`--print`), and `grok -p` (`--single`) all take the prompt as the argument directly after the flag: `agy --dangerously-skip-permissions -p "$LOCAL_PROMPT"`, `grok --permission-mode bypassPermissions -p "$LOCAL_PROMPT"`. They do **not** read the prompt from stdin. Do NOT write `echo "$LOCAL_PROMPT" | agy --dangerously-skip-permissions -p`, `agy -p < prompt.txt`, or `printf … | agy -p` — agy ignores piped stdin and exits with `agy --print takes the prompt as an argument, not stdin`, forcing a wasted second invocation. The `> "$LOG_FILE" 2> "$ERR_FILE"` redirect in Step 2 captures the reviewer's *output*; it is unrelated to how the prompt goes in. Keep `"$LOCAL_PROMPT"` as the quoted argument to `-p` exactly as shown in the invocation table. -**Pinning the reviewer's model (`${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` / `--model`).** When `{REVIEW_MODEL}` is set (from an `[]` bracket or a saved `review-models` default — resolved by the caller), the reviewer runs on that model; when empty, `MODEL_FLAG` is an empty array so `codex`/`claude`/`grok` fall back to the CLI's own default. For **codex**, `-m`/`--model` is a **top-level** Codex option (like `--sandbox` and `-a`), so it MUST precede the `review`/`exec` subcommand — that is why `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` sits before `--sandbox` in both codex invocations; passing it after the subcommand would exit 2 with an unexpected-argument error, exactly as `-a` does. (The two paths pass *different* sandbox policies — `read-only` for review-only, `danger-full-access` for reviewer-applies — see below.) For **claude**, `--model` is a session flag valid alongside `-p`. For **grok**, `-m`/`--model` is a session flag valid alongside `-p`, so `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` sits inline in the invocation (empty array → grok's own default). For **agy**, the model is always pinned via `--model "$AGY_REVIEW_MODEL"` (resolved above with `{REVIEW_MODEL}` taking precedence over the `AGY_REVIEW_MODEL` env and the built-in default) — agy's own default may be a slow "Thinking" tier, so it is never left unpinned. Because the model string may contain spaces/parens, `MODEL_FLAG` is a shell array (see the pre-flight block) — never a bare string. +**Pinning the reviewer's model (`${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` / `--model`) and reasoning effort (`${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` / `--effort`).** When `{REVIEW_MODEL}` / `{REVIEW_EFFORT}` is set (from an `[]` bracket, a `~effort=` suffix, or a saved default — resolved by the caller), the reviewer runs on that model and effort level; when empty, `MODEL_FLAG` / `EFFORT_FLAG` are empty arrays so `codex`/`claude`/`agy`/`grok` fall back to their default or session values. For **codex**, `-m`/`--model` and `--effort` are **top-level** Codex options (like `--sandbox` and `-a`), so they MUST precede the `review`/`exec` subcommand — that is why `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` and `${EFFORT_FLAG[@]+"${EFFORT_FLAG[@]}"}` sit before `--sandbox` in both codex invocations; passing them after the subcommand would exit 2 with an unexpected-argument error, exactly as `-a` does. (The two paths pass *different* sandbox policies — `read-only` for review-only, `danger-full-access` for reviewer-applies — see below.) For **claude**, `--model` is a session flag valid alongside `-p`. For **grok**, `-m`/`--model` is a session flag valid alongside `-p`, so `${MODEL_FLAG[@]+"${MODEL_FLAG[@]}"}` sits inline in the invocation (empty array → grok's own default). For **agy**, the model is always pinned via `--model "$AGY_REVIEW_MODEL"` (resolved above with `{REVIEW_MODEL}` taking precedence over the `AGY_REVIEW_MODEL` env and the built-in default) — agy's own default may be a slow "Thinking" tier, so it is never left unpinned. Because the model string may contain spaces/parens, `MODEL_FLAG` is a shell array (see the pre-flight block) — never a bare string. Notes on each invocation: - **claude / agy / grok** run the self-contained `$LOCAL_PROMPT` (a single-agent inline review), **not** slashdo's `/do-review` skill — the skill's sub-agent fan-out never re-syncs into a print-mode/headless response, so it would hang and emit zero findings (see the `$LOCAL_PROMPT` rationale above). Under Claude Code the `claude` reviewer is an in-process sub-agent (via the `Agent` tool) that runs `$LOCAL_PROMPT` directly, rather than a `claude -p` subprocess — and because the prompt is a single-agent inline review, it does not recursively spawn the skill's own sub-agents. In `REVIEWER_APPLIES=true` mode, `$LOCAL_PROMPT` tells the CLI to apply each fix, verify with build+tests, commit as `address review (): ` (`` = the reviewing CLI's slug, `claude`, `agy`, or `grok`), and NOT push (the orchestrating agent verifies and pushes). The parenthesized agent name records which reviewer surfaced the finding, useful when scanning the log of a release that ran multiple reviewers. In `REVIEWER_APPLIES=false` mode, `$LOCAL_PROMPT` tells the CLI to emit `FINDING :` blocks (or `NO FINDINGS`) to stdout for the orchestrator to parse — the orchestrator then commits the fixes using the same `address review (): ` form to preserve attribution. diff --git a/lib/multi-reviewer-loop.md b/lib/multi-reviewer-loop.md index 7cbc2b8..bc9c4a8 100644 --- a/lib/multi-reviewer-loop.md +++ b/lib/multi-reviewer-loop.md @@ -11,13 +11,14 @@ The calling command must populate these before reaching this loop: - `{REVIEW_AGENTS}` — ordered list of reviewer slugs, e.g. `[codex, agy, copilot]`. May contain a single entry. The `gemini` and `antigravity` slugs normalize to `agy`. Deduped left-to-right by the parser (after normalization); the first occurrence wins. A **model-taking** entry (`codex`, `claude`, `agy`, `grok`, or `ollama`) may carry a bracketed model selector — `[]` (e.g. `codex[o3]`, `claude[claude-opus-4-8]`, `agy[Gemini 3.5 Flash (High)]`, `grok[grok-code-fast-1]`, `ollama[qwen2.5-coder:32b]`) — which the parser strips off into a per-entry model string and forwards to that agent's loop: the Ollama loop reads it as `{OLLAMA_MODEL}` (empty → auto-select), the local-agent loop (`codex`/`claude`/`agy`/`grok`) reads it as `{REVIEW_MODEL}` (empty → the reviewer's built-in default). The GitHub-side reviewers `copilot` and `@` take **no** model bracket. An `@` entry targets an arbitrary GitHub reviewer (user or App/bot) — the parser strips the leading `@` into a per-entry `{REVIEWER_LOGIN}` (e.g. `@octocat` → `octocat`) and forwards it to the GitHub-reviewer loop. The `[]` selector (for a model-taking agent) and the login (for `@`) are part of the dedup identity, so `codex[a]` and `codex[b]` — and `@octocat` and `@org-review-bot` — are distinct entries, while two bare `ollama`s (or two `@octocat`s, compared lowercased) collapse to one. The full token grammar, suffixes included, is: ``` - entry := ( [ "[" "]" ] | "@" ) ( "~opt" | "~max=" )* + entry := ( [ "[" "]" ] | "@" ) ( "~opt" | "~max=" | "~effort=" )* ``` - Both suffixes are optional, order-independent, and repeat-free (at most one of each); see the two bullets below for their semantics. + All three suffixes are optional, order-independent, and repeat-free (at most one of each); see the three bullets below for their semantics. - `{REVIEW_MODELS}` — optional per-agent **default-model map** the calling command resolved from saved config (the `review-models` key, project-over-global merged per agent — see `~/.claude/lib/review-config-defaults.md`). Keys are agent slugs (`codex`/`claude`/`agy`/`grok`/`ollama`); values are model strings. It supplies an entry's model **only when that entry's `--review-with` token carried no `[]` bracket** — the bracket always wins. Absent/empty when the command resolved no saved default. Precedence per entry, top wins: explicit `[]` bracket → `{REVIEW_MODELS}[slug]` → the reviewer's built-in default. - **Optional (non-blocking) marker `~opt`.** Any entry may carry a trailing `~opt` suffix — `ollama~opt`, `ollama[qwen2.5-coder:32b]~opt`, `@some-bot~opt`, `copilot~opt` — marking that reviewer **optional**: it is still requested, still runs, and its findings are still fixed exactly like a required reviewer, but an *inconclusive* result from it (timeout / skipped / incomplete / no-verdict) is **excluded from the merge gate** — it never flips `{OVERALL_STATUS}` to `inconclusive` and so never blocks `--merge`. This is for reviewers you want a second opinion from but that don't reliably return a verdict (e.g. a local Ollama model that sometimes emits no findings). The parser strips `~opt` into a per-entry boolean `{OPTIONAL}` (default `false`) **before** any slug/`[model]`/`@login` parsing; the `~opt` suffix is **not** part of the dedup identity, so `ollama~opt` and `ollama` are the same reviewer. Two things are **not** exempted (see the aggregate rules): a hard-error (`cli-error`/`broken-build`/`test-failed`/`rejected`) — a broken tree still blocks regardless of optionality — and `push-failed`, the wrapper's own status for a pass whose fix commits never reached the remote, since an optional reviewer's fixes stranded locally still mean the merged tree is not the reviewed tree. `~opt` is deliberately shell-metacharacter-free so a `--review-with` value stays inert wherever it lands in a command string. - - **Per-reviewer iteration cap `~max=`.** Any entry may carry a trailing `~max=` suffix — `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3`, `copilot~max=0` — capping how many **review → fix → re-review cycles** that one reviewer runs before it stops. This is the per-entry form of `--review-iterations`, and unlike that flag it applies to **every** reviewer type, including the local agents (`codex`/`claude`/`agy`/`grok`) and `ollama` whose caps were otherwise fixed at 3. It is what makes a mixed run like `--review-with claude~max=2,ollama~max=1,codex~max=3` express a different budget per reviewer in one call. `` must be a **non-negative integer**: any `n ≥ 1` runs at most `n` cycles (still exiting early when a round is clean or the convergence gate converges), and `0` means "loop until that reviewer is clean", bounded by each inner loop's own 10-iteration safety guardrail. A reviewer that stops because it reached an **explicitly configured** `~max=` (n ≥ 1) with work still outstanding returns `capped`, which is **clean-equivalent for the merge gate** — you asked for exactly `n` rounds and got them, so the cap is not a failure. That is the difference between `~max` and the built-in caps: exhausting a *built-in* cap still returns `guardrail` (inconclusive, blocks the merge), because nobody chose that ceiling. The suffix chains freely with `~opt` in either order (`ollama~opt~max=1` ≡ `ollama~max=1~opt`); at most one `~max=` per entry. Like `~opt` it is **not** part of the dedup identity (`ollama~max=2` and `ollama` are the same reviewer) and is shell-metacharacter-free by design. + - **Per-reviewer iteration cap `~max=`.** Any entry may carry a trailing `~max=` suffix — `claude~max=2`, `ollama[qwen2.5-coder:32b]~max=1`, `@some-bot~max=3`, `copilot~max=0` — capping how many **review → fix → re-review cycles** that one reviewer runs before it stops. This is the per-entry form of `--review-iterations`, and unlike that flag it applies to **every** reviewer type, including the local agents (`codex`/`claude`/`agy`/`grok`) and `ollama` whose caps were otherwise fixed at 3. It is what makes a mixed run like `--review-with claude~max=2,ollama~max=1,codex~max=3` express a different budget per reviewer in one call. `` must be a **non-negative integer**: any `n ≥ 1` runs at most `n` cycles (still exiting early when a round is clean or the convergence gate converges), and `0` means "loop until that reviewer is clean", bounded by each inner loop's own 10-iteration safety guardrail. A reviewer that stops because it reached an **explicitly configured** `~max=` (n ≥ 1) with work still outstanding returns `capped`, which is **clean-equivalent for the merge gate** — you asked for exactly `n` rounds and got them, so the cap is not a failure. That is the difference between `~max` and the built-in caps: exhausting a *built-in* cap still returns `guardrail` (inconclusive, blocks the merge), because nobody chose that ceiling. The suffix chains freely with `~opt` and `~effort=` in any order (`ollama~opt~max=1~effort=high` ≡ `ollama~effort=high~opt~max=1`); at most one `~max=` per entry. Like `~opt` it is **not** part of the dedup identity (`ollama~max=2` and `ollama` are the same reviewer) and is shell-metacharacter-free by design. + - **Per-reviewer reasoning effort `~effort=`.** Any entry may carry a trailing `~effort=` suffix — `codex[gpt-5.6-luna]~effort=max`, `claude~effort=high`, `ollama[qwen2.5-coder:32b]~opt~effort=low` — specifying the reasoning effort level for that reviewer. `` must be one of `low`, `medium`, `high`, `xhigh`, `max`. The parser strips `~effort=` into a per-entry `{ENTRY_EFFORT}` (default: unset/empty) **before** slug/`[model]`/`@login` parsing. Like `~opt` and `~max`, it is **not** part of the dedup identity; when duplicates collapse, the effort level comes from the first occurrence that carried a `~effort` (`first-occurrence-wins`). - `{REVIEW_STOP_MODE}` — one of: - `all` (default) — run every listed reviewer in order, regardless of what each reports - `on-findings` — stop after the first reviewer that produced a verdict status (`clean`, `capped`, or copilot `too-large`) AND added at least one commit since `PASS_START_SHA` (i.e. the orchestrator actually landed a fix). Reviewer-reported "comments" without resulting commits do NOT trigger the stop — the signal is the commit-graph delta, not the count of suggestions. The assumption is that once a verdict pass has landed fixes, subsequent reviewers would mostly duplicate the same surface and the user wants speed @@ -32,8 +33,8 @@ The calling command must populate these before reaching this loop: ### Pre-flight 1. **Validate the list is non-empty.** If empty, abort with `--review-with requires at least one agent`. -2. **Strip the `~` suffixes, then validate each slug.** First, for each entry, strip **every** trailing `~`-suffix off the token — `~opt` into a per-entry boolean `{OPTIONAL}` (default `false`), and `~max=` into a per-entry `{ENTRY_MAX}` (default: unset, meaning "no per-entry cap was requested"). Strip from the **right**, repeatedly, so the two may appear in either order and the remaining token is suffix-free before any slug/`[model]`/`@login` parsing below. Validate each `~max` value as a **non-negative integer**; reject anything else — including a repeated `~max=` on one entry, or a `~`-suffix that is neither `opt` nor `max=` — with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and may appear at most once; the only suffixes are ~opt and ~max=.` Note the `[]` bracket is stripped *after* the suffixes but its value is free-form and may itself contain a `~`, so match the suffixes only outside the outermost brackets. Then validate the remaining slug is one of `copilot`, `codex`, `agy` (aliases `gemini` / `antigravity`), `claude`, `grok`, `ollama`, each of `codex`/`claude`/`agy`/`grok`/`ollama` optionally carrying a `[]` bracket, or an arbitrary GitHub login `@`. Abort on the first unknown value with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt and/or ~max=).` Normalize `gemini`/`antigravity` → `agy` before proceeding. For a **model-taking** entry that carries a `[]` bracket (`codex`/`claude`/`agy`/`grok`/`ollama` — the bracket value is the substring between the outermost `[` and `]`, free-form so it may contain spaces or parens, e.g. `agy[Gemini 3.5 Flash (High)]`; `copilot[…]` and `@login[…]` are invalid — reject with the unknown-value abort), record its model string for that entry — `{OLLAMA_MODEL}` for `ollama` (empty for a bare `ollama` → auto-select), `{REVIEW_MODEL}` for `codex`/`claude`/`agy`/`grok` — but **keep the bracket suffix attached to the entry as its identity** — do not collapse `codex[a]` and `codex[b]` to a bare `codex` here, or the dedupe in step 3 would drop one of the requested model passes. For an `@` entry, strip the leading `@` into `{REVIEWER_LOGIN}` and validate it against `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$` (GitHub user charset plus the optional `[bot]` App suffix); reject a malformed login with the same unknown-value abort. Keep the full login as the entry's identity. -3. **Dedupe** preserving first-occurrence order. Compare on the normalized identity: the slug, plus the `[]` selector for a model-taking entry (`codex`/`claude`/`agy`/`grok`/`ollama`) and the `{REVIEWER_LOGIN}` (lowercased — GitHub logins are case-insensitive) for `@` entries (so `gemini` and `agy` are duplicates, two bare `ollama`s are duplicates and `@Octocat`/`@octocat` are duplicates, but `codex[a]`/`codex[b]`, `ollama[a]`/`ollama[b]` and `@octocat`/`@org-review-bot` are distinct). Neither `~` suffix is part of the identity — `ollama~opt`, `ollama~max=2` and `ollama` are all duplicates. When duplicates collapse, the surviving entry is **optional if *any* of the collapsed occurrences was marked `~opt`** (optional-wins: a reviewer you asked to be non-blocking in even one place shouldn't gate the merge), and takes its `{ENTRY_MAX}` from the **first** occurrence that carried one (the base first-occurrence-wins rule; `~opt` is the deliberate exception because non-blocking is a safety property, while a cap is just a budget). Print a warning if duplicates were dropped: `Note: deduped --review-with list to {final list}.` +2. **Strip the `~` suffixes, then validate each slug.** First, for each entry, strip **every** trailing `~`-suffix off the token — `~opt` into a per-entry boolean `{OPTIONAL}` (default `false`), `~max=` into a per-entry `{ENTRY_MAX}` (default: unset, meaning "no per-entry cap was requested"), and `~effort=` into a per-entry `{ENTRY_EFFORT}` (default: unset/empty). Strip from the **right**, repeatedly, so the suffixes may appear in any order and the remaining token is suffix-free before any slug/`[model]`/`@login` parsing below. Validate each `~max` value as a **non-negative integer** and each `~effort` value as one of `low`, `medium`, `high`, `xhigh`, `max`; reject anything else — including a repeated `~max=` or `~effort=` on one entry, or an unknown `~`-suffix — with `Invalid --review-with suffix on {entry}: ~max must be a non-negative integer and ~effort must be one of low, medium, high, xhigh, max, each appearing at most once; the only suffixes are ~opt, ~max=, and ~effort=.` Note the `[]` bracket is stripped *after* the suffixes but its value is free-form and may itself contain a `~`, so match the suffixes only outside the outermost brackets. Then validate the remaining slug is one of `copilot`, `codex`, `agy` (aliases `gemini` / `antigravity`), `claude`, `grok`, `ollama`, each of `codex`/`claude`/`agy`/`grok`/`ollama` optionally carrying a `[]` bracket, or an arbitrary GitHub login `@`. Abort on the first unknown value with `Unknown --review-with value: {value}. Use one of: codex, agy, claude, grok, ollama, copilot, @ (each optionally suffixed ~opt, ~max=, and/or ~effort=).` Normalize `gemini`/`antigravity` → `agy` before proceeding. For a **model-taking** entry that carries a `[]` bracket (`codex`/`claude`/`agy`/`grok`/`ollama` — the bracket value is the substring between the outermost `[` and `]`, free-form so it may contain spaces or parens, e.g. `agy[Gemini 3.5 Flash (High)]`; `copilot[…]` and `@login[…]` are invalid — reject with the unknown-value abort), record its model string for that entry — `{OLLAMA_MODEL}` for `ollama` (empty for a bare `ollama` → auto-select), `{REVIEW_MODEL}` for `codex`/`claude`/`agy`/`grok` — but **keep the bracket suffix attached to the entry as its identity** — do not collapse `codex[a]` and `codex[b]` to a bare `codex` here, or the dedupe in step 3 would drop one of the requested model passes. For an `@` entry, strip the leading `@` into `{REVIEWER_LOGIN}` and validate it against `^[A-Za-z0-9][A-Za-z0-9-]*(\[bot\])?$` (GitHub user charset plus the optional `[bot]` App suffix); reject a malformed login with the same unknown-value abort. Keep the full login as the entry's identity. +3. **Dedupe** preserving first-occurrence order. Compare on the normalized identity: the slug, plus the `[]` selector for a model-taking entry (`codex`/`claude`/`agy`/`grok`/`ollama`) and the `{REVIEWER_LOGIN}` (lowercased — GitHub logins are case-insensitive) for `@` entries (so `gemini` and `agy` are duplicates, two bare `ollama`s are duplicates and `@Octocat`/`@octocat` are duplicates, but `codex[a]`/`codex[b]`, `ollama[a]`/`ollama[b]` and `@octocat`/`@org-review-bot` are distinct). No `~` suffix is part of the identity — `ollama~opt`, `ollama~max=2`, `ollama~effort=high` and `ollama` are all duplicates. When duplicates collapse, the surviving entry is **optional if *any* of the collapsed occurrences was marked `~opt`** (optional-wins: a reviewer you asked to be non-blocking in even one place shouldn't gate the merge), takes its `{ENTRY_MAX}` from the **first** occurrence that carried one, and takes its `{ENTRY_EFFORT}` from the **first** occurrence that carried one. Print a warning if duplicates were dropped: `Note: deduped --review-with list to {final list}.` - **Resolve each entry's effective model** after dedupe, for every model-taking entry (`codex`/`claude`/`agy`/`grok`/`ollama`): if the entry carried a `[]` bracket, keep that value; else if `{REVIEW_MODELS}` has an entry for this agent's normalized slug, use it (the caller already merged project-over-global); else leave it empty. Empty means the reviewer's built-in default — `codex`/`claude`/`grok` fall back to the CLI's own default model, `agy` to its pinned default (`Gemini 3.5 Flash (High)`, overridable via `AGY_REVIEW_MODEL`), `ollama` to auto-select. Store the result as the entry's `{REVIEW_MODEL}` (or `{OLLAMA_MODEL}` for `ollama`) for the dispatch step to forward. The bracket-wins-over-config precedence lives here so no inner loop re-implements it. - **Resolve each entry's effective iteration cap** after dedupe, for **every** entry (all reviewer types). Precedence, top wins: 1. the entry's `{ENTRY_MAX}` (from its `~max=` suffix), when it carried one; @@ -59,11 +60,11 @@ This is the default path. Iterate `REVIEW_AGENTS` in order, running each reviewe 1. **Print a banner**: `--- Review pass {n}/{N}: {REVIEW_AGENT} ---` 2. **Capture baseline**: `PASS_START_SHA=$(git rev-parse HEAD)` so the wrapper can tell whether this reviewer changed anything (independent of the inner loop's own tracking). 3. **Dispatch** to the matching single-reviewer loop. The loop file lives under the host CLI's lib directory (`~/.claude/lib/` for Claude, `~/.config/opencode/lib/` for OpenCode — slashdo's installer rewrites command-spec `!cat` references for each env, so use the same lib basename in whichever env this wrapper executes. For Antigravity and Codex there is no separate lib path: slashdo inlines the loop bodies directly into the installed skill, so the dispatch targets below are already present in-context rather than at a file path): - Forward this entry's resolved `{MAX_ITERATIONS}` and `{MAX_EXPLICIT}` (from the cap-resolution step) to **every** target below — that pair is what carries a `~max=` into the reviewer: + Forward this entry's resolved `{MAX_ITERATIONS}` and `{MAX_EXPLICIT}` (from the cap-resolution step) to **every** target below — that pair is what carries a `~max=` into the reviewer; forward `{ENTRY_EFFORT}` as `{REVIEW_EFFORT}` / `{OLLAMA_EFFORT}` (empty if unset): - `copilot` → `{LIB_DIR}/copilot-review-loop.md` (forward `{GH_HOST}`, and this entry's `{MAX_ITERATIONS}` as its `{REVIEW_ITERATIONS}` iteration cap) - `@` → `{LIB_DIR}/github-reviewer-loop.md` (forward `{GH_HOST}`, this entry's `{REVIEWER_LOGIN}`, and this entry's `{MAX_ITERATIONS}` as its `{REVIEW_ITERATIONS}` iteration cap) - - `codex` | `agy` | `claude` | `grok` → `{LIB_DIR}/local-agent-review-loop.md` (forward this entry's resolved `{REVIEW_MODEL}` — empty means the reviewer's built-in default — and its `{MAX_ITERATIONS}`, which is the loop's built-in `3` unless a `~max=` moved it; the `--review-iterations` flag itself never reaches this loop) - - `ollama` → `{LIB_DIR}/ollama-review-loop.md` (forward this entry's `{OLLAMA_MODEL}` — empty means auto-select — and its `{MAX_ITERATIONS}`, built-in `3` unless a `~max=` moved it; `{REVIEWER_APPLIES}` does not apply — the Ollama loop is always review-only, and the `--review-iterations` flag never reaches it) + - `codex` | `agy` | `claude` | `grok` → `{LIB_DIR}/local-agent-review-loop.md` (forward this entry's resolved `{REVIEW_MODEL}` — empty means the reviewer's built-in default —, `{REVIEW_EFFORT}` — empty means CLI/session default —, and its `{MAX_ITERATIONS}`, which is the loop's built-in `3` unless a `~max=` moved it; the `--review-iterations` flag itself never reaches this loop) + - `ollama` → `{LIB_DIR}/ollama-review-loop.md` (forward this entry's `{OLLAMA_MODEL}` — empty means auto-select —, `{OLLAMA_EFFORT}` — empty means unset —, and its `{MAX_ITERATIONS}`, built-in `3` unless a `~max=` moved it; `{REVIEWER_APPLIES}` does not apply — the Ollama loop is always review-only, and the `--review-iterations` flag never reaches it) The inner loop already handles its own iterations, fix-and-push cycles, and verification. It returns a `{STATUS}` value: - Copilot loop: `clean | capped | timeout | error | guardrail | too-large` diff --git a/lib/ollama-review-loop.md b/lib/ollama-review-loop.md index f187be6..f1f6e58 100644 --- a/lib/ollama-review-loop.md +++ b/lib/ollama-review-loop.md @@ -21,7 +21,7 @@ When to use this: 2. Confirm the Ollama server is reachable: `ollama list` must succeed (it errors if the daemon isn't running). If it fails, in **default mode** set `STATUS=skipped` and print `ollama server not reachable — start it with \`ollama serve\` (recording as skipped)`; in **interactive mode** offer to start it. 3. **Resolve `{OLLAMA_MODEL}`** (see "Model resolution" below). If resolution yields no usable model, set `STATUS=skipped` and return. 4. Force review-only: set `REVIEWER_APPLIES=false` regardless of what the caller passed. If the caller passed `--reviewer-applies`, print: `--reviewer-applies has no effect on the ollama pass; Ollama is non-agentic, so the orchestrator always applies the fixes.` -5. Record `{REPO_DIR}` (`git rev-parse --show-toplevel`), `{BRANCH_NAME}` (`git branch --show-current`), `{BASE_BRANCH}`, `{BUILD_CMD}`, and `{TEST_CMD}`. Also record `{MAX_ITERATIONS}` — how many review → fix → re-review cycles this reviewer may run, resolved by the caller (the multi-reviewer loop: a per-entry `~max=` suffix on the `--review-with` token → this loop's built-in default of `3`). **Defaults to `3`**; `0` means **unlimited**, bounded by the 10-iteration safety guardrail in the Loop's step 6. Local models are the most common reason to want a small cap — `ollama~max=1` buys one review-and-fix pass without paying for re-review rounds on slow hardware. Record `{MAX_EXPLICIT}` alongside it — `true` only when the cap came from a `~max=` the user typed or saved — which step 6 uses to distinguish `capped` (a budget the user chose, clean-equivalent for the merge gate) from `guardrail` (a built-in ceiling, inconclusive). The `--review-iterations` flag never reaches this loop; `~max` is the only way to move this cap. +5. Record `{REPO_DIR}` (`git rev-parse --show-toplevel`), `{BRANCH_NAME}` (`git branch --show-current`), `{BASE_BRANCH}`, `{BUILD_CMD}`, and `{TEST_CMD}`. Also record `{MAX_ITERATIONS}` — how many review → fix → re-review cycles this reviewer may run, resolved by the caller (the multi-reviewer loop: a per-entry `~max=` suffix on the `--review-with` token → this loop's built-in default of `3`). **Defaults to `3`**; `0` means **unlimited**, bounded by the 10-iteration safety guardrail in the Loop's step 6. Local models are the most common reason to want a small cap — `ollama~max=1` buys one review-and-fix pass without paying for re-review rounds on slow hardware. Record `{MAX_EXPLICIT}` alongside it — `true` only when the cap came from a `~max=` the user typed or saved — which step 6 uses to distinguish `capped` (a budget the user chose, clean-equivalent for the merge gate) from `guardrail` (a built-in ceiling, inconclusive). The `--review-iterations` flag never reaches this loop; `~max` is the only way to move this cap. Also record `{OLLAMA_EFFORT}` — optional reasoning effort string for this reviewer (`low`, `medium`, `high`, `xhigh`, `max`), resolved by the caller (multi-reviewer loop: explicit `~effort=` suffix → empty). Defaults to empty when unset. 6. **Resolve the timeout wrapper.** Stock macOS ships **neither** `timeout(1)` (GNU coreutils) nor `gtimeout` (the Homebrew-prefixed coreutils build), so an empty `TIMEOUT_CMD` is the *common* case on a Mac, not an edge case. An empty array = no wrapper (rely on Ollama's own limits), which is a **supported configuration**: the review runs unbounded and must never be recorded as a reviewer failure. An ARRAY, not a string, for the same zsh reason as `OLLAMA_FLAGS` below: zsh does not word-split an unquoted expansion, so a two-word string (`timeout 600`) would be executed as one bogus command name and fail every invocation precisely on machines that HAVE coreutils installed. Settled logic — run it verbatim, do NOT narrate the probe or the fallback: ```bash TIMEOUT_CMD=() @@ -119,6 +119,7 @@ If the diff has no logic issues worth raising, return {\"findings\": []}. --- DIFF --- $FILE_DIFF" + [ -n "$OLLAMA_EFFORT" ] && PROMPT="$PROMPT Target reasoning effort level: $OLLAMA_EFFORT." RESP=$(printf '%s' "$PROMPT" | ${TIMEOUT_CMD[@]+"${TIMEOUT_CMD[@]}"} ollama run ${OLLAMA_FLAGS[@]+"${OLLAMA_FLAGS[@]}"} "$OLLAMA_MODEL" 2>> "$ERR_FILE") RC=$? printf '\n===== FILE: %s =====\n%s\n' "$F" "$RESP" >> "$LOG_FILE" diff --git a/lib/review-config-defaults.md b/lib/review-config-defaults.md index 8f247e2..bb2cc20 100644 --- a/lib/review-config-defaults.md +++ b/lib/review-config-defaults.md @@ -21,7 +21,7 @@ Procedure (run once, during argument parsing): Parse it as JSON; `PROJECT_DEFAULTS = .defaults` (missing/invalid → `{}`). 3. Merge: `EFFECTIVE = { ...GLOBAL_DEFAULTS, ...PROJECT_DEFAULTS }` — a per-project value overrides the global one key-by-key. 4. For each shared flag **this command supports** that was **not** present in `$ARGUMENTS`, take its value from `EFFECTIVE` using these keys, then feed that value through this command's normal parsing and validation exactly as if the user had typed it (so `[...]` model brackets on `codex`/`claude`/`agy`/`grok`/`ollama`, slug validation, dedupe, integer checks, and mutual-exclusion rules all still apply — a malformed saved default is rejected with the same error a typed one would get). **"Not present in `$ARGUMENTS`" is decided purely by the flag's absence from the user's command line — NOT by whether a variable already holds a value.** The earlier per-flag parse bullets may have eagerly named a built-in default (e.g. "set `REVIEW_STOP_MODE=all`", `REVIEWER_APPLIES=false`, `REVIEW_ITERATIONS=1`); treat any such value as *provisional, not yet resolved* at this point. A saved default still applies to every key the user did not type — not just `review-with` — and the built-in default is only the final fallback in step 5. The keys: - - `review-with` → the `--review-with` list (string). Any entry may carry a trailing `~opt` (e.g. `claude,ollama~opt,codex`) marking that reviewer **optional/non-blocking**, and/or a trailing `~max=` (e.g. `claude~max=2,ollama~opt~max=1,codex~max=3`) capping that reviewer's review → fix → re-review cycles. Both markers ride through the saved value verbatim and are parsed by the same rules as a typed flag (see `lib/multi-reviewer-loop.md`), so a saved default can pin a non-blocking reviewer and a per-reviewer iteration budget without separate keys. Note `~max` is per-entry and therefore **more specific than the `review-iterations` key** — a saved `review-with` carrying `~max` overrides a saved `review-iterations` for the entry that carries it, exactly as a typed `~max` overrides a typed `--review-iterations`. **Tombstone:** if the effective `review-with` value is the literal `none` (case-insensitive) — which a user saves, typically with `--project`, to opt one repo out of an inherited global reviewer — set `REVIEW_AGENTS=[]` and do **not** fall back to any lower-precedence source or built-in default. A saved `none` is an explicit opt-out, exactly like passing `--review-with none` on the command line; it is not a reviewer slug and is not validated as one. + - `review-with` → the `--review-with` list (string). Any entry may carry a trailing `~opt` (e.g. `claude,ollama~opt,codex`) marking that reviewer **optional/non-blocking**, a trailing `~max=` (e.g. `claude~max=2,ollama~opt~max=1,codex~max=3`) capping that reviewer's review → fix → re-review cycles, and/or a trailing `~effort=` (e.g. `codex[gpt-5.6-luna]~effort=max~opt`, `claude~effort=high~max=2`) setting its reasoning effort level (`low`, `medium`, `high`, `xhigh`, `max`). All three markers ride through the saved value verbatim and are parsed by the same rules as a typed flag (see `lib/multi-reviewer-loop.md`), so a saved default can pin non-blocking reviewers, per-reviewer iteration budgets, and reasoning effort levels without separate keys. Note `~max` is per-entry and therefore **more specific than the `review-iterations` key** — a saved `review-with` carrying `~max` overrides a saved `review-iterations` for the entry that carries it, exactly as a typed `~max` overrides a typed `--review-iterations`. **Tombstone:** if the effective `review-with` value is the literal `none` (case-insensitive) — which a user saves, typically with `--project`, to opt one repo out of an inherited global reviewer — set `REVIEW_AGENTS=[]` and do **not** fall back to any lower-precedence source or built-in default. A saved `none` is an explicit opt-out, exactly like passing `--review-with none` on the command line; it is not a reviewer slug and is not validated as one. - `review-models` → the per-agent **default-model map** (a JSON object keyed by agent slug, e.g. `{"codex":"o3","claude":"claude-opus-4-8","agy":"Gemini 3.5 Flash (High)","ollama":"qwen2.5-coder:32b"}`). Unlike the other keys this is **not** a `--review-with`-style flag — it is a supplementary map that supplies a reviewer's model when its `--review-with` entry carried **no** `[]` bracket. Resolve it as `EFFECTIVE_REVIEW_MODELS`, a **per-agent (deep) merge** of the global and project maps — a project entry for one agent overrides the global entry for *that agent*, while agents present only in the global map are inherited (do **not** replace the whole object as the flat `{ ...GLOBAL, ...PROJECT }` merge in step 3 would; merge key-by-key at the agent level). Valid agent keys are `codex`/`claude`/`agy`/`grok`/`ollama` (the model-taking reviewers); `copilot` and `@` take no model. Every command that runs the multi-reviewer loop passes `EFFECTIVE_REVIEW_MODELS` to it as `{REVIEW_MODELS}`; the loop applies the per-entry precedence **explicit `[]` bracket → project `review-models[slug]` → global `review-models[slug]` → the reviewer's built-in default** (the deep merge above already collapses project-over-global, so the loop only sees the merged map). A saved `review-models` never selects *which* reviewers run — that is `review-with`'s job — it only pins the model for a reviewer already in the list. - `review-iterations` → `--review-iterations` (integer) - `reviewer-applies` → `--reviewer-applies` (boolean; `true` means the flag is set) diff --git a/test/config.test.js b/test/config.test.js index 8af8d50..0a03bab 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -185,4 +185,21 @@ describe('writeConfig', () => { assert.equal(readConfig(file).defaults['review-iterations'], 1); fs.rmSync(dir, { recursive: true }); }); + + it('round-trips a review-with with per-reviewer reasoning effort (~effort=) unchanged', () => { + const { dir, file } = tmpFile(); + const cfg = { + defaults: { + 'review-with': + 'codex[gpt-5.6-luna]~effort=max~opt,claude~effort=high~max=2,ollama[qwen2.5-coder:32b]~opt~max=1~effort=low', + }, + }; + writeConfig(file, cfg); + assert.deepEqual(readConfig(file), cfg); + assert.equal( + readConfig(file).defaults['review-with'], + 'codex[gpt-5.6-luna]~effort=max~opt,claude~effort=high~max=2,ollama[qwen2.5-coder:32b]~opt~max=1~effort=low', + ); + fs.rmSync(dir, { recursive: true }); + }); }); diff --git a/test/review-loop-contract.test.js b/test/review-loop-contract.test.js index 7b79dd7..f6e85b3 100644 --- a/test/review-loop-contract.test.js +++ b/test/review-loop-contract.test.js @@ -93,6 +93,22 @@ describe('review-loop parse contracts', () => { } }); + it('threads the per-reviewer ~effort level through to the loops that honor it', () => { + const wrapper = readLib('multi-reviewer-loop.md'); + assert.match(wrapper, /~effort=/); + assert.match(wrapper, /ENTRY_EFFORT/); + assert.match(wrapper, /REVIEW_EFFORT/); + + const localAgent = readLib('local-agent-review-loop.md'); + assert.match(localAgent, /REVIEW_EFFORT/); + assert.match(localAgent, /EFFORT_FLAG/); + assert.match(localAgent, /\$\{EFFORT_FLAG\[@\]\+"\$\{EFFORT_FLAG\[@\]\}"\}/); + + const ollama = readLib('ollama-review-loop.md'); + assert.match(ollama, /OLLAMA_EFFORT/); + assert.match(ollama, /PROMPT="\$PROMPT Target reasoning effort level: \$OLLAMA_EFFORT\."/); + }); + it('lets ~opt excuse no-verdict and lets capped satisfy partial', () => { // Two ways a new status gets stranded: added to a loop's status set but not to // the aggregate rules that consume it. A ~opt no-verdict must reach the