Conversation
gh/glab issue create prints the new issue's URL, not JSON — so appending
-q .number / --jq errors and can abort the create inside a $(…) capture. Add
the URL-parsing idiom (URL=$(gh issue create …); NUM=${URL##*/}) next to the
existing 'capture the issue numbers for the summary' instruction, plus a nudge
toward --body-file for multi-line bodies, so consumers (/do:plan-task,
/do:next, /do:replan) don't reach for an invalid -q flag.
docs(plan-issue-mode): show how to capture a created issue number (avoid invalid -q on gh issue create)
fix: fail closed on reviewer parse errors
--review-iterations is global and only ever reached the copilot and @<login> loops; the local agents and ollama had a hardcoded MAX_ITERATIONS=3 with no way to move it. That made a slow local model expensive to include in a review list, and made it impossible to budget reviewers differently in one run. A per-entry ~max=<n> suffix now caps that one reviewer's review -> fix -> re-review cycles, so --review-with claude~max=2,ollama~max=1,codex~max=3 gives each its own budget. It chains with ~opt in either order, is not part of the dedup identity (first occurrence's cap wins on collapse), and rides through /do:config saved defaults. This needed a second change to be usable: the local-agent and ollama loops report `guardrail` on cap exhaustion, which is inconclusive and blocks the merge gate -- so ~max=1 would have blocked nearly every merge. All four loops now report `capped` (clean-equivalent) when the cap was user-configured, and keep `guardrail` for a built-in ceiling. The split is deliberate: a ceiling you chose is a budget, one you didn't is an unfinished review. Runs that never pass ~max are unaffected. Ollama's coverage-gap rule still outranks both -- a partially-reviewed diff reports `incomplete`, never `capped`. ~max is ignored (with a warning) in --review-mode parallel, where each reviewer runs a single review-only pass.
The ~max=<n> docs landed in #129 but didn't say the suffix only has meaning in series mode. In parallel mode each reviewer runs a single review-only pass and the orchestrator applies the union once, so multi-reviewer-loop.md warns and ignores the cap -- a reader budgeting reviewers with ~max in parallel mode would otherwise expect it to apply.
…ewable The new parse-error accounting added a `REVIEW_ERRORS + PARSE_ERRORS == REVIEWABLE` -> cli-error check without the `REVIEWABLE > 0` guard its sibling total-failure check carries. A diff of nothing but pure renames/mode changes skips every file as empty-diff, leaving REVIEWABLE=0 with zero errors, so 0 == 0 fired and the pass reported the hard-error cli-error -- which short-circuits the multi-reviewer wrapper to `dirty` and blocks the merge on a diff that simply had nothing to review. Also clamp the Coverage numerator at 0: REVIEW_ERRORS/PARSE_ERRORS and TRUNCATED can count the same file twice (truncation is decided before the invocation), which could render a negative count.
Seven fixes from the claude review pass on the release diff, all in the
~max / fail-closed-parsing change:
- The strict verdict contract validated $LOG_FILE, but step 2 captured
the reviewer with `> "$LOG_FILE" 2>&1` -- so any CLI startup banner,
deprecation notice, or agy/grok progress line made a clean review parse
as malformed. Split stderr into $ERR_FILE, as the Ollama loop already
does, and point the cli-error diagnostics at it.
- An unparseable verdict reported cli-error, which is a hard error: it
fires the wrapper's short-circuit (skipping every remaining reviewer on
a branch that is actually fine) and is the one class ~opt explicitly
does not excuse -- while ~opt's own docs name "no-verdict" as an
inconclusive status it does excuse. Added `no-verdict` as an
inconclusive status and threaded it through the wrapper's status set,
stop-mode table, and inconclusive enumerations.
- Ollama's PARSE_ERRORS double-counted invocation-error files: the
invocation writes a `===== FILE:` section for every file including
failed ones, so an errored file's marker-only section was re-counted as
a parse error. Sum could then exceed REVIEWABLE, leaving both the
total-failure and partial branches unmatched. Count each file once and
compare with >=.
- /do:rpr promised to forward {ENTRY_MAX} to the local-agent and Ollama
loops but its dispatch bullets never listed it (and step 8 still said
those loops have a fixed cap), so `--review-with claude~max=1` silently
ran the built-in 3 rounds.
- pr.md, better.md, and better-swift.md still glossed `capped` as
GitHub-side-only, so a codex/ollama pass that returned capped would be
treated as not-clean and leave a merge-eligible PR open -- contradicting
each command's own ~max bullet.
- The dedup tie-break was stated two incompatible ways ("first
occurrence's cap wins" vs "first occurrence that carried a ~max"); for
`ollama,ollama~max=1` those give cap 1 vs the built-in 3.
- Changelog was missing the fail-closed parsing change entirely and cited
the pre-change coverage-gap formula.
Also added contract tests for no-verdict, the stderr split, the
REVIEWABLE > 0 guard, and the ~max threading.
… rules Both statuses were reachable from an inner loop but stranded in the wrapper's aggregate evaluation: - `no-verdict` (added in the previous commit) was missing from the optional-inconclusive exclusion and from the aggregate `inconclusive` rule, so a `~opt` local reviewer returning it qualified for neither `clean` nor `partial` -- exactly the blocking the ~opt contract promises to prevent. - `partial` accepted only `clean` passes. A `~max=1` reviewer that lands fixes returns `capped`, which is precisely what trips `--review-stop-on-findings` -- and since `clean` excludes stop-short-circuited runs, that combination matched NO rule at all and blocked a merge that both statuses document as eligible. Also updated the per-command copies of the same rules (release.md's inconclusive enumeration, better/better-swift's partial gloss) and added a contract test pinning both.
…etry loop
The previous commit wired {MAX_ITERATIONS} into rpr's dispatch, which made
rpr's own retry loop the leak: each re-dispatch handed the inner loop a
fresh n, so `--review-with ollama~max=1` could run an unbounded number of
review-and-fix cycles. rpr now tracks rounds spent per entry across the
outer loop and stops re-dispatching an explicitly capped entry once the
budget is spent, forwarding the remaining budget rather than n. Entries on
a built-in cap or ~max=0 keep the historical worthiness-driven behavior.
…t loop rpr accepts ~max on every slug but its Copilot path is a hand-rolled request/poll loop, not a dispatch into lib/copilot-review-loop.md, so nothing enforced the budget there -- and rpr accepts no --review-iterations, making ~max the only cap available. copilot~max=1 was therefore accepted and silently ignored. The step-8 cap accounting now covers every reviewer type explicitly, and an uncapped copilot entry is documented as worthiness-governed (unchanged from before this release).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release v3.25.0
Released: 2026-07-26
Highlights
~max=<n>suffix. Suffix any--review-withslot with~max=<n>—--review-with=claude~max=2,ollama~max=1,codex~max=3— to give each reviewer its own budget of review → fix → re-review cycles in a single call. This is the per-entry form of--review-iterations, and unlike that flag it reaches every reviewer type: the local agents (codex/claude/agy/grok) andollamahad hardcoded 3-iteration caps with no way to change them, which made a slow local model expensive to include.ollama[qwen3.6:35b]~max=1now buys exactly one review-and-fix pass.Added
~max=<n>iteration-cap marker parsed by the multi-reviewer loop and every command that accepts--review-with(/do:pr,/do:release,/do:review,/do:better,/do:better-swift,/do:depfree,/do:rpr, and/do:nextvia pass-through to/do:pr).<n>is a non-negative integer;0means "loop until clean", bounded by each loop's 10-iteration safety guardrail. The suffix is stripped into a per-entry{ENTRY_MAX}before slug/[model]/@loginparsing, chains with~optin either order (ollama~opt~max=1≡ollama~max=1~opt), and is not part of the dedup identity (ollama~max=2andollamacollapse; the first occurrence's cap wins). Like~optit is free of shell metacharacters, so a--review-withvalue stays inert wherever it lands in a command string.{MAX_ITERATIONS}/{MAX_EXPLICIT}inputs on the local-agent and Ollama review loops, replacing their hardcodedMAX_ITERATIONS=3. The multi-reviewer loop resolves each entry's effective cap once — per-entry~max=<n>→--review-iterations(GitHub-side reviewers only, matching that flag's existing scope) → the loop's built-in default — so no inner loop re-implements the precedence.cappedstatus on the local-agent and Ollama loops, matching the copilot and@<login>loops. It means the reviewer spent an explicitly configured budget after applying every fix it surfaced, and is clean-equivalent for the merge gate./do:config --review-with=claude~max=2,ollama~opt~max=1validates the cap and stores both suffixes, re-appended in canonical order (~optfirst, then~max=<n>) so a stored value is byte-stable regardless of how it was typed. The markers ride through.slashdo.json/.slashdo-config.jsonuntouched.Iterationscolumn ({rounds run}/{cap}, rendering∞for~max=0) in the Multi-Reviewer Summary, so acappedrow shows it stopped on the budget you set rather than running out of findings.Changed
cappedvsguardrailis now a distinction about who chose the ceiling. Reaching a user-configured cap (a~max=<n>, or--review-iterationson a GitHub-side pass) yieldscapped— clean-equivalent, merge-eligible. Reaching a built-in ceiling (the local-agent / Ollama default of 3, or the 10-iteration guardrail in unlimited mode) still yieldsguardrail, which stays inconclusive and blocks the merge. Existing runs that never pass~maxare unaffected: their built-in caps behave exactly as before.capped. A partially-reviewed diff (REVIEW_ERRORS + TRUNCATED > 0) still reportsincomplete, nevercapped, so a~maxcan't turn an unreviewed diff into a merge-eligible pass.~maxis ignored in--review-mode parallel(each reviewer runs exactly one review-only pass there, so there are no per-reviewer cycles to cap). The loop warns rather than aborting, matching how it already handles--reviewer-appliesand the stop-mode flags in parallel mode.Unknown --review-with valueabort message now notes each slug may be suffixed~optand/or~max=<n>.Full Changelog
Full Diff: v3.24.1...v3.25.0