Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions crates/goose-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,104 @@ enum Command {
bin_name: String,
},

/// Local code review.
///
/// Discovers `**/.agents/checks/*.md` subagent reviewers and
/// `**/.agents/REVIEW.md` scoped prompt overrides, builds a review
/// request from the working tree (or an explicit diff range), and
/// runs the review through goose.
#[command(about = "Review the current diff using goose")]
Review {
/// Diff range to review (e.g. "main...HEAD"). Defaults to the working
/// tree vs HEAD.
#[arg(value_name = "RANGE")]
range: Option<String>,

/// Path to a Markdown file with a custom base review prompt. Replaces
/// the embedded default prompt.
#[arg(long = "prompt", value_name = "FILE")]
prompt: Option<PathBuf>,

/// Default model used for the main review agent and for any check
/// that does not declare its own `model:` in frontmatter.
#[arg(long = "model", value_name = "MODEL")]
model: Option<String>,

/// Provider for the main review agent.
#[arg(long = "provider", value_name = "PROVIDER")]
provider: Option<String>,

/// Force every discovered check to use this model, regardless of
/// the check's own `model:` field.
#[arg(long = "override-model", value_name = "MODEL")]
override_model: Option<String>,

/// Default `turn-limit` applied to checks that do not declare their
/// own.
#[arg(long = "turn-limit", value_name = "N")]
turn_limit: Option<usize>,

/// Print the assembled review prompt and discovered checks instead of
/// running the review.
#[arg(long = "dry-run")]
dry_run: bool,

/// Suppress non-result output from the underlying agent.
#[arg(long, short = 'q')]
quiet: bool,

/// Disable the Rust-driven parallel orchestrator and fall back to
/// the single-prompt path that asks the main agent to delegate
/// each check via `delegate(... async: true ...)`. The default
/// orchestrator dispatches one `goose run` subprocess per check
/// (capped at 4 concurrent), bounding wall-clock to the slowest
/// single check rather than waiting on the model to issue
/// dispatches.
#[arg(long = "no-orchestrate")]
no_orchestrate: bool,

/// Additional free-form instructions to prepend to the review
/// (e.g. PR intent, commit-message context, "this is a refactor,
/// flag any behavior change"). Mirrors `amp review --instructions`
/// for drop-in compatibility with existing reviewer wrappers.
#[arg(long = "instructions", short = 'i', value_name = "TEXT")]
instructions: Option<String>,

/// Restrict the review to a specific set of files. Other files in
/// the diff are still passed to the agent for context but are
/// excluded from the assembled diff sent to checks. Mirrors
/// `amp review --files`.
#[arg(long = "files", short = 'f', value_name = "FILE", num_args = 1..)]
files: Vec<String>,

/// Only run checks whose `name` matches one of these. Other
/// discovered checks are skipped. Mirrors `amp review --check-filter`.
#[arg(long = "check-filter", short = 'c', value_name = "NAME", num_args = 1..)]
check_filter: Vec<String>,

/// Alternate directory to search for `.agents/checks/*.md` instead
/// of the repo root. Mirrors `amp review --check-scope`.
#[arg(long = "check-scope", short = 's', value_name = "DIR")]
check_scope: Option<PathBuf>,

/// Skip the main correctness pass and only run check subagents.
/// Mirrors `amp review --checks-only`.
#[arg(long = "checks-only")]
checks_only: bool,

/// Print only the diff summary; skip the full review.
/// Mirrors `amp review --summary-only`.
#[arg(long = "summary-only")]
summary_only: bool,

/// Minimum severity to display. Findings below this rank are
/// dropped from the output. Default is `medium`, matching
/// Amp's CLI which hides `low` from review output. Pass
/// `--severity low` to surface every finding.
#[arg(long = "severity", value_name = "LEVEL", default_value = "medium")]
severity: String,
},

#[command(
name = "validate-extensions",
about = "Validate a bundled-extensions.json file",
Expand Down Expand Up @@ -1157,6 +1255,7 @@ fn get_command_name(command: &Option<Command>) -> &'static str {
#[cfg(feature = "local-inference")]
Some(Command::LocalModels { .. }) => "local-models",
Some(Command::Completion { .. }) => "completion",
Some(Command::Review { .. }) => "review",
Some(Command::ValidateExtensions { .. }) => "validate-extensions",
None => "default_session",
}
Expand Down Expand Up @@ -1990,6 +2089,45 @@ pub async fn cli() -> anyhow::Result<()> {
Some(Command::Term { command }) => handle_term_subcommand(command).await,
#[cfg(feature = "local-inference")]
Some(Command::LocalModels { command }) => handle_local_models_command(command).await,
Some(Command::Review {
range,
prompt,
model,
provider,
override_model,
turn_limit,
dry_run,
quiet,
no_orchestrate,
instructions,
files,
check_filter,
check_scope,
checks_only,
summary_only,
severity,
}) => {
use crate::commands::review::{handle_review, ReviewOptions};
handle_review(ReviewOptions {
range,
prompt_file: prompt,
default_model: model,
provider,
override_model,
default_turn_limit: turn_limit,
dry_run,
quiet,
no_orchestrate,
instructions,
files,
check_filter,
check_scope,
checks_only,
summary_only,
severity,
})
.await
}
Some(Command::ValidateExtensions { file }) => {
use goose::agents::validate_extensions::validate_bundled_extensions;
match validate_bundled_extensions(&file) {
Expand Down
1 change: 1 addition & 0 deletions crates/goose-cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub mod info;
pub mod plugin;
pub mod project;
pub mod recipe;
pub mod review;
pub mod schedule;
pub mod session;
pub mod term;
Expand Down
108 changes: 108 additions & 0 deletions crates/goose-cli/src/commands/review/default_review_prompt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
You are reviewing a code change for **correctness bugs**, security issues,
performance problems, and style violations. Be precise and concrete; cite the
exact line(s) and explain the failure mode.

## Output

For every issue you find, emit a single JSON object on its own line with the
fields:

- `severity` — one of `low`, `medium`, `high`, `critical`
- `path` — repo-relative file path
- `line_start` — first line the comment applies to (1-indexed)
- `line_end` — last line the comment applies to
- `summary` — one-paragraph explanation of the issue and the fix
- `check` — the `name` of the check that produced the finding, or
`main` for findings produced by the main review pass

If there are no issues, emit a single line containing `[]`.

## Correctness pass (run this for every diff)

Before delegating to subagent checks, do a careful correctness pass on the
diff yourself. Walk every changed function and look hard for:

- **Silent error paths.** Missing-key, missing-row, `None`/`null`, and
exception cases that produce a default value instead of surfacing the
error. Flag every place where a missing record is silently coerced to
`0`, `""`, `[]`, etc.
- **Off-by-one and boundary errors.** Loop bounds, slice indices,
ranges, and inclusive vs. exclusive comparisons.
- **Unhandled error returns.** Functions that return `Result`/`Error`/`err`
whose return value is dropped or ignored.
- **Concurrency hazards.** Shared mutable state without a lock, missing
`await`, blocking I/O on async paths, deadlock-prone lock ordering.
- **Resource lifecycle.** File handles, sockets, threads, or subprocess
handles that are not closed/joined on every path (including error
paths).
- **Input validation.** Untrusted input flowing into SQL, shell, file
paths, deserialization, or template rendering without sanitization.
- **State that leaks across requests.** Module-level mutables, default
arguments, and singleton caches that retain user data across calls.
- **Logic that contradicts the comment, docstring, or function name.**
These signal that one of them is wrong; flag the inconsistency.

Emit findings from this pass with `"check": "main"`.

## Code-quality pass

Alongside the correctness pass, walk every changed hunk and call out:

- **Bugs and hackiness.** Suspicious workarounds, copy-pasted blocks
that drifted, anything that looks like a fix-as-you-go.
- **Unnecessary code.** Dead branches, unreachable paths, redundant
null checks, work that could be deleted without changing behavior.
- **Too much shared mutable state.** Module-level singletons, globals,
parameters mutated across helpers, structures whose ownership is
unclear.
- **Abstraction fit, in both directions.** Flag *unnecessary
indirection* (factories, wrappers, traits, adapters that have one
caller and add no leverage) and *missing abstractions* (the same
five-line block repeated across the diff, or hard-coded values that
belong behind a name). For each finding, cite concrete locations
and recommend exactly one action — only when it improves the
current code, not because it is a "best practice".

## Guidelines

- Only comment on the diff. Do not flag pre-existing code unless the diff
meaningfully changes its behavior.
- Prefer high-signal findings over coverage. A small number of correct,
actionable comments is better than many low-confidence ones.
- Treat style nits as `low` severity; reserve `high`/`critical` for real
bugs, regressions, or security issues.

## Checks

If the request below lists subagent **checks**, **dispatch them all in
parallel** before doing anything else. For each check:

```
delegate(
instructions = <check body>,
async = true, # IMPORTANT: parallelize
model = <check model>,
max_turns = <check turn_limit>,
)
```

Do NOT pass the check's `tools` value to `extensions`. The `extensions`
parameter filters by **extension name** (e.g. `developer`, `summon`),
not tool name (e.g. `Read`, `Grep`), so passing a tool list there
silently disables every extension and the subagent ends up with no
tools at all. Treat the per-check `tools` column in the request as
informational guidance for the subagent's prompt, not as an
extensions filter.

This returns a `taskId` immediately. After dispatching every check, call
`load(taskId)` once per check to wait for the results. **Do not** issue
the next `delegate` call after the previous one has completed — that is
sequential and slow; we want every check executing concurrently.

Run your own correctness pass while the subagents are in flight, so the
wall-clock time is bounded by the slowest single check rather than by
their sum.

Each subagent must include the originating check's `name` in the `check`
field of every finding so attribution is preserved end-to-end.
Aggregate all findings (yours and theirs) into the same JSON output.
Loading