Skip to content

Review command

Muhammet Şafak edited this page Jul 26, 2026 · 10 revisions

Home / Commands / Review

Review (default command)

When invoked without a subcommand, commitbrief runs a code review against the active scope.

Synopsis

commitbrief [scope-flag] [global-flags...]

Scope flags (mutually exclusive)

Flag Short Default What it reviews
--staged -s yes Staged changes (git diff --cached).
--unstaged -u no Working-tree changes (git diff).

If neither is set, --staged is the default. The two flags are mutually exclusive — passing both is a cobra-level error.

For historic ranges (HEAD~3 HEAD, main...feature, <merge-sha>) use the diff subcommand instead. The v0.x --commit/--branch/--pull-request scope flags were collapsed into diff in v0.9.0.

Path filters

Flag Short Repeatable Notes
--file <path> -f yes Review only these files. Combines with the scope flag.
--dir <path> -d yes Review only files under these directories.
--exclude-file <path> yes Skip these files. Same matching rules as --file, applied after it.
--exclude-dir <path> yes Skip files under these directories.
commitbrief --staged -f src/main.go -f src/util.go
commitbrief --unstaged -d database/seeder -d app/Models
commitbrief --staged -d internal --exclude-dir internal/cli

Path filters apply after the diff is parsed and after the ignore layers, and the denylist runs last so an exclusion wins; see Filtering for the full pipeline.

Commit filters

--author, --committer, --start-date, --end-date and --text review a set of commits instead of the index or working tree. They are mutually exclusive with --staged / --unstaged — those scopes have no commits yet — and passing both is an error.

commitbrief --author alice                          # Alice's commits on HEAD's history
commitbrief --author alice --author bob             # either person
commitbrief --start-date 2026-03-01 --end-date 2026-03-31
commitbrief --text payment                          # message OR branch name
commitbrief --author alice --dir internal --max-commits 20

Different kinds are AND'd, multiple values of one kind are OR'd. The reviewed diff is the concatenation of the matching commits' patches, so a file changed in several of them appears several times. Use dry-run to see the counts before paying for a review. Full semantics: Filtering.

Pipeline (what happens, in order)

  1. Resolve context — load merged config, pick active provider, resolve language.
  2. Load rules — read COMMITBRIEF.md (or fall back to the embedded default) and OUTPUT.md (template).
  3. Fetch diff — staged/unstaged from the git layer (go-git or git CLI per ADR-0002), or — when a commit filter is set — the concatenated patches of the matching commits (ADR-0035). The filter is validated first, so a malformed date or a scope conflict fails before anything else runs.
  4. Filter — built-in ignore patterns → .commitbriefignore--file/--dir allowlist → --exclude-file/--exclude-dir denylist.
  5. Pre-send guards:
    • .commitbrief/ guard (ADR-0007): if the diff touches files under .commitbrief/, prompt for confirmation in a TTY; abort in a non-TTY unless --yes is set.
    • Secret scanner: if guard.secret_scan is true and the diff contains credential-shaped patterns, prompt or abort. Bypass with --allow-secrets. --yes does NOT bypass the scanner (since v0.9.1).
  6. Build prompt — wraps rules content in <project_rules>...</project_rules> XML; if review.architecture is on and an architecture.json is present, inserts an <architecture_constraints> block (see Architecture-aware review); appends the severity rubric, response-format contract, a language directive, and the diff.
  7. Cache lookup — SHA-256 cache key over diff + system_prompt + provider + model + lang + schema_version. Hit replays the cached response; miss continues.
  8. Cost preflight — estimates token cost; if it exceeds the configured cost.warn_threshold_usd, prompts (TTY) or aborts (non-TTY). Bypass with --no-cost-check. --yes does NOT bypass the preflight (since v0.9.1).
  9. Provider call — sends the prompt; receives structured findings JSON (API providers) or pre-formatted plain text (CLI-tool-backed providers).
  10. Flaky pre-pass merge (ADR-0022) — for API/mock providers, a deterministic static scan of the changed test files' added lines (see Flaky-test detection) is merged into the findings, skipping any line the model already flagged. Off via --no-flaky / review.flaky: false; inert for CLI-tool providers.
  11. Signal control (ADR-0027) — baseline then inline suppression are applied as true removals (see Signal control): findings recorded in the user-private .commitbrief/baseline.json, and findings on a line carrying a commitbrief-ignore marker, are dropped before fail-on / render. Counts are reported, not silent.
  12. Render — cards (default TTY), JSON, or markdown.
  13. Copy (optional) — pushes findings to clipboard via OSC 52 plus native tools.
  14. Fail-on — if --fail-on=<severity> is set, exits non-zero when any finding meets or exceeds that severity.

Flaky-test detection

A deterministic, provider-free pre-pass (ADR-0022) scans the added lines of changed test files for high-precision flakiness anti-patterns and merges any matches into the findings — so they render, count toward --fail-on, and --copy like a model finding, but without a model call or any JSON-schema change.

Detected today:

  • Hard-coded sleeps / fixed waits (severity medium): time.Sleep, Thread.sleep, Task.Delay, asyncio.sleep, *.waitForTimeout, numeric cy.wait(<n>), usleep, sleep(<n>).
  • Unseeded randomness (severity low): Math.random, Python random.*, Go math/rand globals (rand.Intn, …).
  • Brittle selectors (severity low, JS/TS only): absolute/positional XPath (//section[2]/button[1], xpath=/…, By.xpath('//…'), .locator('//…')), CSS :nth-child / :nth-of-type, Cypress .eq(<n>), Playwright .nth(<n>). Stable data-testid / role / attribute selectors are not flagged.
  • Over-mocking (severity low): a single test function setting up more than five mock/stub statements (jest.mock/spyOn/mock*Value, when(…).thenReturn, sinon.stub, patch(…)/MagicMock, gomock/.EXPECT(), Mockery shouldReceive). The one finding anchors at the threshold-crossing setup; fixtures with two or three doubles are not flagged.
  • Time-dependent assertions (severity low): a wall-clock read (time.Now(), Date.now(), new Date(), datetime.now/utcnow, System.currentTimeMillis()) used directly in an assertion instead of an injected clock. A clock used only for setup is not flagged.

Test files are detected by convention (*_test.go, *.test.* / *.spec.*, test_*.py / *_test.py, *Test(s).java / .cs, *_spec.rb, *Test.php, and __tests__ / tests / spec / e2e / cypress directories). Finding text is localized (en/tr).

On by default for API/mock providers. Disable per-run with --no-flaky, or persistently with review.flaky: false (config set review.flaky false). CLI-tool-backed plain-text providers are unaffected for now.

Sandbox-rerun confirmation (opt-in)

The rules above infer flakiness from anti-patterns. Sandbox-rerun raises confidence by actually re-running a flagged test in isolation N times and classifying it by the observed pass/fail mix:

  • mixed pass + fail → confirmed flaky (the finding is kept; its suggestion notes the empirical confirmation);
  • all fail → a real failure — the test is genuinely red, so the note says so plainly; it is not quarantined as a flake;
  • all passtransient / resolved — the flake did not reproduce, so the finding is demoted to info and won't trip a commit-stage --fail-on.

Enable it with --sandbox-rerun[=N] (a bare --sandbox-rerun uses N=5; --sandbox-rerun=3 sets N) or persistently with review.sandbox_rerun: <N> (config set review.sandbox_rerun 5; 0 = off, the default). Precedence: --sandbox-rerun > review.sandbox_rerun > off.

Confirmation requires a double opt-in. A positive --sandbox-rerun/ review.sandbox_rerun alone is not enough — you also need a non-empty review.sandbox_command, the runner CommitBrief actually executes. Either one without the other stays a no-op:

review:
  sandbox_rerun: 5
  sandbox_command: ["go", "test", "-count=1", "-run", "^{{.Test}}$", "./..."]

sandbox_command is a list of argv elements, not a shell string. Each element is rendered as a Go text/template over {{.File}} (repo-relative, slash-normalized), {{.Line}} (int), and {{.Test}} (the enclosing test function name), then the rendered argv is handed straight to exec.CommandContextno shell is invoked, so there is no quoting/injection surface. config set review.sandbox_command is rejected — it's hand-edit only, the same treatment guard.secret_patterns gets, since a config key that can arm code execution deserves extra friction.

CommitBrief ships no built-in test runner for any language — your own command is the runner, and CommitBrief only renders the template and observes the exit code. Each rerun attempt is bounded by its own 2-minute timeout, so one hung test costs one attempt (recorded as unobserved, never mistaken for a fail), not the whole review. A stderr notice names the configured command template — the un-rendered argv, with placeholders like {{.Test}} still literal — once per review, before any finding's target is even known; this is the review path's first code-execution stage, so it is never silent about it.

The command runs against the working tree, not the staged snapshot a review may be scoped to: whatever is on disk is what actually gets executed, so test-name resolution reads the same tree the command sees. A staged-only review whose worktree has diverged from the index gets a rerun verdict about the worktree's version of the test, not the staged one under review — the stderr notice above is the mitigation, making it visible every time.

Important

Sandbox-rerun is not cached. The flaky pre-pass — including any bound sandbox-rerun confirmation — runs before the cache lookup, so it merges into both a cache hit and a fresh provider call. A repeated review against the same diff still re-executes sandbox_command, even though the review body itself comes from cache and the footer reports Saved. Total cost scales with how many findings get flagged — worst case findings × N × 2 minutes — with no cap and no progress indicator while it runs.

commitbrief mcp and commitbrief guard never run the bound command, unconditionally, regardless of flag or config — see MCP server and guard.

Important

Test-name resolution is Go-only. {{.Test}} is resolved by parsing *_test.go files with Go's own go/parser; every other language returns no name. When a finding's test name can't be resolved, that finding skips the rerun (with a stderr warning) and keeps its bare, unconfirmed static finding — it never falls back to running the whole suite. Python, JS, PHP, and Java tests keep full static flaky detection (all 5 rules above still apply); they just never get sandbox-rerun confirmation. This was a deliberate scope cut, not an oversight: a hand-rolled multi-language scanner went through three rounds of confident-but-wrong test names before being replaced by the exact, parser-based Go-only resolver.

Resolution additionally skips any file the Go toolchain would not compile on the machine running the review: an unsatisfied //go:build constraint, a GOOS-suffixed name like _windows_test.go on Linux, or anything under testdata/. Note this includes build tags your own sandbox_command supplies — putting -tags=integration in the command does not make an //go:build integration file resolvable, so those tests keep the static finding and skip confirmation. The trade is deliberate: the alternative is emitting a name go test cannot select, which exits 0 and reports a real flake as "did not reproduce" without ever running it.

See ADR-0033 (the maintainer's private decision-record SSOT) for the full execution-boundary rationale.

Architecture-aware review

CommitBrief can make a review architecture-aware by reading the architecture.json produced by its sibling tool archlint — a deterministic linter for import-boundary rules. When the file is present, CommitBrief renders a compact summary of its declared layers (sets of path prefixes) and their allowed / forbidden import edges into the prompt as a distinct <architecture_constraints> block. The reviewer can then flag a diff that crosses a declared boundary — for example, an import that adds a domain → db dependency the architecture forbids.

This is a strict one-way read of archlint's public config: CommitBrief never lints the import graph itself and never enforces anything — archlint owns enforcement (run archlint check in CI for the deterministic gate). CommitBrief only informs the LLM so it can reason about the change.

architecture.json

The minimal shape CommitBrief reads (other archlint keys such as module and aliases are ignored):

{
  "layers": {
    "domain": ["internal/domain"],
    "db":     ["internal/db"],
    "http":   ["internal/http"]
  },
  "rules": {
    "domain": [],
    "db":     ["domain"],
    "http":   ["domain", "db"]
  }
}

layers maps a layer name to its repo-relative path prefixes; rules maps a layer to the layers it is allowed to import ([] = may import no other layer; a same-layer import is always allowed). CommitBrief derives the forbidden edges by complementing the allow-list against the declared layers, so the block tells the model both what each layer may and must not import.

Behavior

  • On by default (review.architecture: true), but inert unless an architecture.json actually exists.
  • The discovery path is the repo root by default; override it with review.architecture_file (relative to the repo root, or absolute) — e.g. config set review.architecture_file config/architecture.json.
  • A missing or malformed file is a transparent no-op: the review proceeds exactly as if the feature were off (it never breaks a review). A path you set explicitly via review.architecture_file that doesn't exist is an error, so a typo surfaces instead of silently disabling the feature.
  • Disable per-run with --no-architecture, or persistently with review.architecture: false (config set review.architecture false).
  • The architecture block folds into the system prompt, so editing architecture.json invalidates stale cached reviews automatically; a repo without the file keeps a byte-identical cache key (no mass invalidation).
  • Applies to the review (default) and dry-run paths. dry-run includes the block in its token/cost estimate so the estimate matches the real run.

The block summary is bounded in size (layers and edges are capped) to keep the added token cost small.

Signal control

Three layers cut repeat noise. --min-severity is display-only (it never touches --json or --fail-on). The baseline and inline suppression are true removals: a removed finding no longer counts toward --fail-on, no longer appears in --json findings[], and is hidden from the render. Neither is silent — optional additive meta.baselined / meta.suppressed fields appear in --json (the schema stays v1) and a one-line N baselined · M suppressed footer prints to stderr.

Baseline (.commitbrief/baseline.json, user-private)

On a brownfield repo, accept the current findings once:

commitbrief --staged --update-baseline   # writes fingerprints; does NOT filter this run
commitbrief --staged                      # later runs: only NEW findings remain

The baseline file is per-developer and gitignored — it is never committed, so CI and a reviewer's gate apply no baseline and see every finding (it can't hide a real bug from a senior). The stored fingerprint is sha256(File + Severity + normalize(Title)), which is resilient to line drift (a finding that moves up or down the file stays baselined) and ignores the LLM-volatile description/snippet. Re-accept any time with --update-baseline; ignore the baseline for one run with --no-baseline; disable persistently with review.baseline: false (config set review.baseline false). A missing baseline file is a transparent no-op.

Limit: the same title twice in one file (same severity) collides into one fingerprint. The escape hatch is --update-baseline.

Inline suppression (commitbrief-ignore)

Silence one finding with a visible, written reason — on the offending line or the line directly above it:

result := db.Query(userInput) // commitbrief-ignore[high]: parameterized below, false positive
  • commitbrief-ignore: <reason> silences any finding on the line.
  • commitbrief-ignore[<severity>]: <reason> silences only that severity.
  • The comment prefix is irrelevant — //, #, --, /* */ all work.

Markers are read from the added diff lines, so the suppression is part of the change under review and a reviewer sees it in the diff. No config gate (always active) — because it lives in committed, reviewer-visible source, it carries none of the baseline's hide-vector risk.

Inherited global flags

Every flag on Global flags applies. Most relevant:

  • Output: --json, --markdown, --output <file>, --copy, --compact, --verbose/-v, --quiet/-q, --color.
  • Provider override: --provider <name>, --model <name>, --cli <claude|gemini> (shorthand for --provider <name>-cli; mutually exclusive with --provider AND with --json / --markdown since CLI providers emit pre-formatted text).
  • Cache: --no-cache to bypass.
  • Guards: --allow-secrets, --no-cost-check, --no-flaky, --sandbox-rerun[=N], --no-architecture, --yes.
  • CI gating: --fail-on=<critical|high|medium|low|info|any|none>.
  • Display filter: --min-severity=<critical|high|medium|low|info> hides lower-severity findings from the rendered output (not --json/--fail-on).
  • Signal control (ADR-0027): --update-baseline / --no-baseline (and the commitbrief-ignore source marker) — see Signal control. True removals (affect --fail-on and --json), unlike --min-severity.
  • Locale: --lang en|tr.

Suggest a commit message

--suggest-commit adds a second step after the review: a free-form provider call that prints one Conventional Commit message for the staged diff to stdout.

commitbrief --staged --suggest-commit
  • Read-only. It suggests; you commit. CommitBrief never writes to git or the working tree (NG4).
  • Staged scope only. Works with --staged or the default run; rejected with --unstaged, the diff subcommand, and with --json / --markdown / --output (the suggestion is plain stdout, not part of the structured/file output).
  • Any provider. API providers return the message via the free-form path (no JSON contract for this call); CLI-tool-backed providers emit it directly.
  • The message follows Conventional Commits (type(scope): subject + an optional body). The review's COMMITBRIEF.md rules are not applied to authoring — they govern critique only.
  • The suggestion is a fresh call each run (not cached); the review itself still uses the cache. See [ADR-0015] in the project docs.

Exit codes

Code Meaning
0 Review completed; no --fail-on threshold reached.
1 An error occurred (git failure, provider error, parse failure, guard abort, etc.) OR --fail-on threshold was reached.

See Exit codes for the precise --fail-on semantics.

Examples

# Standard review of staged changes, default rendering.
commitbrief

# Working-tree review, plain markdown to a file.
commitbrief --unstaged --markdown --output review.md

# Pipe JSON into jq for tooling.
commitbrief --json | jq '.findings[] | select(.severity=="critical")'

# CI gate: exit non-zero on any high-or-worse finding.
commitbrief --staged --fail-on=high

# Restrict to a single subdirectory.
commitbrief --staged --dir cmd/server

# Everything one teammate shipped this month, tests excluded.
commitbrief --author alice --start-date 2026-07-01 --exclude-file '*_test.go'

# Override the configured provider for this one invocation.
commitbrief --provider gemini --model gemini-2.5-flash

# Use the locally-installed Claude Code CLI as the backend.
commitbrief --cli claude --staged

See also

  • diff — review arbitrary historic ranges.
  • Global flags — every flag this command inherits.
  • Filtering — the full ignore + path-filter pipeline.
  • Exit codes — precise --fail-on semantics.

Clone this wiki locally