mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
chore(autoreview): align org review defaults (#104343)
* docs(autoreview): sync canonical review policy * fix(autoreview): sync isolated review runtime * test(autoreview): sync hardening coverage * fix(autoreview): sync final isolation hardening * test(autoreview): sync final isolation coverage * fix(autoreview): sync final input hardening * test(autoreview): sync final hardening coverage * fix(autoreview): sync reviewer trust hardening * test(autoreview): sync fallback coverage * test(autoreview): sync boundary coverage * docs(autoreview): align runtime contracts * fix(autoreview): reject unbundled gitlinks * test(autoreview): cover final trust guards * feat(autoreview): accept max Codex reasoning
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
# Autoreview Skill
|
||||
|
||||
- Canonical source: `openclaw/agent-skills`, under `skills/autoreview`.
|
||||
- Before editing any copy, fast-forward a checkout of `openclaw/agent-skills` from `origin/main`.
|
||||
- Make and validate shared changes in canonical `skills/autoreview` first, then sync the complete directory into downstream repos.
|
||||
- Never create repo-local behavior variants; downstream differences belong in repo-level validation, not the skill.
|
||||
+1
@@ -0,0 +1 @@
|
||||
AGENTS.md
|
||||
@@ -1,19 +1,19 @@
|
||||
---
|
||||
name: autoreview
|
||||
description: "Pre-commit/ship code review: Codex default; optional Claude, Pi, Droid, Copilot, Cursor, or OpenCode."
|
||||
description: "Pre-commit/ship code review: Codex default; optional Claude or Pi."
|
||||
---
|
||||
|
||||
# Auto Review
|
||||
|
||||
Run the bundled structured review helper as a closeout check. This is code review, not Guardian `auto_review` approval routing.
|
||||
|
||||
Codex review is the default when no engine is set. It uses `gpt-5.6-sol` by default, usually delivers the best review results, and should remain the normal final closeout engine. Claude review is optional and uses `claude-fable-5` by default.
|
||||
Codex review is the default when no engine is set. It uses `gpt-5.6-sol` with `high` reasoning by default, then retries once with `gpt-5.6-terra` only when the account cannot access Sol. Claude review is optional and uses `claude-fable-5` by default.
|
||||
|
||||
For user-visible behavior, pair autoreview with `behavior-validator`. Autoreview is source-aware and judges the change bundle; behavior validation is source-blind and judges the running product or tool against a behavior contract. A clean autoreview is not proof that a UI, CLI, API, or generated artifact works from the user's perspective.
|
||||
|
||||
Use when:
|
||||
|
||||
- user asks for Codex review / Claude review / Pi review / Droid review / Cursor review / OpenCode review / autoreview / second-model review
|
||||
- user asks for Codex review / Claude review / Pi review / autoreview / second-model review
|
||||
- after non-trivial code edits, before final/commit/ship
|
||||
- reviewing a local branch or PR branch after fixes
|
||||
|
||||
@@ -29,12 +29,14 @@ Use when:
|
||||
- Keep going until structured review returns no accepted/actionable findings only while the work remains inside the original task scope.
|
||||
- If a review-triggered fix changes code, rerun focused tests and rerun the structured review helper.
|
||||
- For security-audit suppression changes, verify accepted findings remain auditable: suppressed findings stay in structured output, active output keeps an unsuppressible suppression notice, and aggregate findings cannot hide unrelated active risk.
|
||||
- Never switch or override the requested review engine/model. If the review hits model capacity, retry the same command a few times with the same engine/model.
|
||||
- Never switch or override the requested review engine/model except for the documented Codex Sol-to-Terra account-access fallback. Capacity, rate-limit, and unrelated failures keep the same engine/model.
|
||||
- Be patient with large bundles. Structured review can take up to 30 minutes while the model call is active, especially with Codex tools or web search.
|
||||
- Treat heartbeat lines like `review still running: ... elapsed=... pid=...` as healthy progress, not a hang. Let the helper continue while heartbeats are advancing. Pass `--stream-engine-output` when live engine text is useful; Codex, Claude, and Cursor filter tool/file chatter, other engines pass raw output through.
|
||||
- Treat heartbeat lines like `review still running: ... elapsed=... pid=...` as healthy progress, not a hang. Let the helper continue while heartbeats are advancing. Pass `--stream-engine-output` when live engine text is useful; Codex and Claude filter tool/file chatter, other runnable engines pass raw output through.
|
||||
- Do not kill a review just because it has been quiet for 2-5 minutes, or because it is still running under the 30-minute window. Inspect the process only after missing multiple expected heartbeats, after 30 minutes, or after an obviously failed subprocess; prefer letting the same helper command finish.
|
||||
- Tools are useful in review mode. The helper allows read-only inspection tools and web search by default so reviewers can check dependency contracts, upstream docs, and current behavior.
|
||||
- Tools are useful in review mode. Codex receives the validated bundle in an empty workspace so ignored files and linked-worktree metadata remain unreadable; web search stays available for dependency contracts and upstream docs.
|
||||
- Security perspective is always included, but it should not cripple legitimate functionality. Report security findings only when the change creates a concrete, actionable risk or removes an important safety check.
|
||||
- Reviewer subprocesses preserve engine authentication and non-credentialed proxy variables needed by headless or restricted-network environments while stripping process-injection, Git override, and credentialed proxy values.
|
||||
- Review bundles fail closed before engine invocation when tracked or untracked paths look sensitive, patch text looks secret-like, or a Git diff exceeds the bundle limit. Redact/split the change; never accept a truncated patch as complete review proof.
|
||||
- For regression provenance, keep roles separate: blamed code author, blamed PR author, PR merger/committer, current PR author, and PR/date. If no blamed PR is traceable, use the blamed commit as the provenance: commit SHA, date, and author username. Do not guess a merger or frame missing PR metadata as a separate finding.
|
||||
- If the blamed PR was merged by `clawsweeper[bot]` or another automation, identify the human trigger when practical. Check timeline/comments first; if rate-limited, use gitcrawl/cache or public PR HTML. Look for maintainer commands such as `@clawsweeper automerge`, `/landpr`, or labels/status comments that armed automerge. Report `automerge triggered by @login`; if not found, say trigger unknown.
|
||||
- Do not invoke built-in `codex review`, nested reviewers, or reviewer panels from inside the review. The helper builds one bundle, calls one selected engine, validates one structured result, and stops.
|
||||
@@ -109,6 +111,27 @@ export AUTOREVIEW_HARNESS="$AGENTS_HOME/skills/autoreview/scripts/test-review-ha
|
||||
|
||||
When using Claude Code, set `AGENTS_HOME="$HOME/.claude"` for global skills. Project-local skills live under `.claude/skills/` in the current repo.
|
||||
|
||||
On native Windows, choose the matching pair:
|
||||
|
||||
```powershell
|
||||
# Project-local skill in the current repo:
|
||||
$AUTOREVIEW = ".agents\skills\autoreview\scripts\autoreview"
|
||||
$AUTOREVIEW_HARNESS = ".agents\skills\autoreview\scripts\test-review-harness.ps1"
|
||||
```
|
||||
|
||||
```powershell
|
||||
# Source checkout of openclaw/agent-skills:
|
||||
$AUTOREVIEW = "skills\autoreview\scripts\autoreview"
|
||||
$AUTOREVIEW_HARNESS = "skills\autoreview\scripts\test-review-harness.ps1"
|
||||
```
|
||||
|
||||
```powershell
|
||||
# Global skill:
|
||||
$AgentsHome = if ($env:AGENTS_HOME) { $env:AGENTS_HOME } else { Join-Path $HOME ".agents" }
|
||||
$AUTOREVIEW = Join-Path $AgentsHome "skills\autoreview\scripts\autoreview"
|
||||
$AUTOREVIEW_HARNESS = Join-Path $AgentsHome "skills\autoreview\scripts\test-review-harness.ps1"
|
||||
```
|
||||
|
||||
## Pick Target
|
||||
|
||||
Dirty local work:
|
||||
@@ -173,7 +196,7 @@ Tradeoff: tests may force code changes that stale the review. If tests or review
|
||||
Run multiple reviewers against one frozen bundle:
|
||||
|
||||
```bash
|
||||
"$AUTOREVIEW" --reviewers codex,claude,pi,opencode
|
||||
"$AUTOREVIEW" --reviewers codex,claude,pi
|
||||
```
|
||||
|
||||
`--panel` is shorthand for Codex plus Claude unless `--engine` changes the first reviewer:
|
||||
@@ -198,14 +221,10 @@ For models with slashes or extra colons, prefer keyed form:
|
||||
|
||||
```bash
|
||||
"$AUTOREVIEW" --engine pi --model anthropic/claude-sonnet-4 --thinking high
|
||||
"$AUTOREVIEW" --engine opencode --model opencode/north-mini-code-free --thinking high
|
||||
"$AUTOREVIEW" --engine cursor --model auto --cursor-allow-workspace-instructions
|
||||
"$AUTOREVIEW" --reviewers codex,pi --model codex=gpt-5.6-sol --model pi=anthropic/claude-sonnet-4
|
||||
"$AUTOREVIEW" --reviewers codex,opencode --model codex=gpt-5.6-sol --model opencode=opencode/north-mini-code-free
|
||||
"$AUTOREVIEW" --reviewers codex,cursor --model codex=gpt-5.6-sol --model cursor=auto --cursor-allow-workspace-instructions
|
||||
```
|
||||
|
||||
`--reviewers all` covers Codex, Claude, Copilot, Pi, and OpenCode. Cursor requires both explicit selection (`--engine cursor` or named in `--reviewers`) and `--cursor-allow-workspace-instructions` because the current Cursor CLI does not document a per-run flag that ignores project-local instructions/config. Droid selection currently fails closed because its CLI cannot disable both project instructions and all tools.
|
||||
`--reviewers all` covers Codex, Claude, and Pi. Droid, Copilot, Cursor, and OpenCode selections fail closed because their current CLI contracts cannot confine project instructions, filesystem reads, or network fetches to the review boundary.
|
||||
|
||||
## Models and thinking
|
||||
|
||||
@@ -213,25 +232,27 @@ The helper accepts `--model` globally or per engine (`engine=model`) and `--thin
|
||||
|
||||
Recommended model defaults:
|
||||
|
||||
| Engine | Default model | Source note |
|
||||
| ------------------- | ---------------- | ----------------------------------------------------- |
|
||||
| **codex** (default) | `gpt-5.6-sol` | OpenAI's current high-capability GPT-5.6 tier |
|
||||
| **claude** | `claude-fable-5` | Anthropic's most capable widely released Claude model |
|
||||
| Engine | Default model | Source note |
|
||||
| ------------------- | -------------------------------------------------- | ----------------------------------------------------- |
|
||||
| **codex** (default) | `gpt-5.6-sol` -> `gpt-5.6-terra` on access failure | OpenClaw org review default |
|
||||
| **claude** | `claude-fable-5` | Anthropic's most capable widely released Claude model |
|
||||
|
||||
CLI flags and environment variables override these defaults. Droid, Copilot, Pi, Cursor, and OpenCode do not get built-in model defaults here because their provider catalogs are external to the Codex/Claude closeout path and may vary by installation.
|
||||
CLI flags and environment variables override these defaults. Pi does not get a built-in model default because its provider catalog may vary by installation. Droid, Copilot, Cursor, and OpenCode are currently refused.
|
||||
|
||||
| Engine | Model flag | Example model IDs | Thinking flag | Accepted levels |
|
||||
| ------------------- | -------------------------- | ---------------------------------------------------------------------------- | ----------------------------- | ------------------------------------------------------ |
|
||||
| **codex** (default) | `codex --model X exec ...` | `gpt-5.6-sol`, `gpt-5.6-luna` | `-c model_reasoning_effort=Y` | `none`, `minimal`, `low`, `medium`, `high`, `xhigh` |
|
||||
| **claude** | `claude --model X` | `claude-fable-5`, `claude-opus-4-8`, `claude-sonnet-4-6`, `claude-haiku-4-5` | `--effort Y` | `low`, `medium`, `high`, `xhigh`, `max` |
|
||||
| **droid** | currently refused | Factory model IDs | `-r, --reasoning-effort Y` | `off`, `none`, `low`, `medium`, `high`, `xhigh`, `max` |
|
||||
| **copilot** | `copilot --model X` | `gpt-5.2`, Copilot model aliases | not supported | n/a |
|
||||
| **pi** | `pi --model X` | `anthropic/claude-sonnet-4`, `openai/gpt-4o` | `--thinking Y` | `off`, `minimal`, `low`, `medium`, `high`, `xhigh` |
|
||||
| **cursor** | `cursor-agent --model X` | `auto`, Cursor model aliases | not supported | n/a |
|
||||
| **opencode** | `opencode run -m X` | `opencode/north-mini-code-free`, OpenCode provider/model IDs | `--variant Y` | `minimal`, `low`, `medium`, `high`, `max` |
|
||||
| Engine | Model flag | Example model IDs | Thinking flag | Accepted levels |
|
||||
| ------------------- | -------------------------- | ---------------------------------------------------------------------------- | ----------------------------- | ---------------------------------------------------------- |
|
||||
| **codex** (default) | `codex --model X exec ...` | `gpt-5.6-sol`, then `gpt-5.6-terra` on Sol access failure | `-c model_reasoning_effort=Y` | `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max` |
|
||||
| **claude** | `claude --model X` | `claude-fable-5`, `claude-opus-4-8`, `claude-sonnet-4-6`, `claude-haiku-4-5` | `--effort Y` | `low`, `medium`, `high`, `xhigh`, `max` |
|
||||
| **droid** | currently refused | Factory model IDs | `-r, --reasoning-effort Y` | `off`, `none`, `low`, `medium`, `high`, `xhigh`, `max` |
|
||||
| **copilot** | currently refused | Copilot model aliases | not supported | n/a |
|
||||
| **pi** | `pi --model X` | `anthropic/claude-sonnet-4`, `openai/gpt-4o` | `--thinking Y` | `off`, `minimal`, `low`, `medium`, `high`, `xhigh` |
|
||||
| **cursor** | currently refused | Cursor model aliases | not supported | n/a |
|
||||
| **opencode** | currently refused | OpenCode provider/model IDs | not supported | n/a |
|
||||
|
||||
Claude also supports `--fallback-model a,b` for availability-based fallback chains ([model-config](https://code.claude.com/docs/en/model-config)). Current Claude docs note that auth, billing, rate-limit, request-size, and transport errors do not trigger fallback, and the changelog documents interactive-session support in `v2.1.166`.
|
||||
|
||||
[OpenAI's model guidance](https://developers.openai.com/api/docs/guides/latest-model) identifies Sol as the GPT-5.6 frontier-capability route and documents `max` support. Autoreview keeps `high` as its default; use `max` only for the hardest quality-first reviews after comparing its latency and cost with `xhigh` on representative changes.
|
||||
|
||||
Examples matching current `main` behavior:
|
||||
|
||||
```bash
|
||||
@@ -241,24 +262,16 @@ Examples matching current `main` behavior:
|
||||
# Codex fast mode (priority service tier); needs a model whose catalog lists the tier, silently standard otherwise
|
||||
"$AUTOREVIEW" --engine codex --codex-speed fast
|
||||
|
||||
# Arbitrary Codex config overrides (isolation flags still win; --codex-speed wins over a service_tier here)
|
||||
# Safe Codex model/response tuning overrides (--codex-speed wins over a service_tier here)
|
||||
"$AUTOREVIEW" --engine codex --codex-config 'service_tier="fast"'
|
||||
|
||||
# Claude Code aliases or full model names, with optional availability fallback
|
||||
"$AUTOREVIEW" --engine claude --model claude-fable-5 --thinking max
|
||||
"$AUTOREVIEW" --engine claude --model claude-fable-5 --fallback-model claude-opus-4-8,claude-sonnet-4-6
|
||||
|
||||
# GitHub Copilot (model only; no thinking knob)
|
||||
"$AUTOREVIEW" --engine copilot --model gpt-5.2
|
||||
|
||||
# Pi with explicit model and thinking level
|
||||
"$AUTOREVIEW" --engine pi --model anthropic/claude-sonnet-4 --thinking high --pi-bin pi
|
||||
|
||||
# Cursor print-mode review (`cursor-agent` remains a compatibility alias)
|
||||
"$AUTOREVIEW" --engine cursor --model auto --cursor-bin cursor-agent --cursor-allow-workspace-instructions
|
||||
|
||||
# OpenCode with explicit provider/model and variant
|
||||
"$AUTOREVIEW" --engine opencode --model opencode/north-mini-code-free --thinking high
|
||||
```
|
||||
|
||||
`--cursor-agent-bin` and `CURSOR_AGENT_BIN` remain compatibility aliases for
|
||||
@@ -272,34 +285,36 @@ Store persistent personal defaults in your shell startup file or launcher
|
||||
environment. For repository-local defaults, use an existing local environment
|
||||
loader such as an untracked `.envrc`; the helper does not write a config file.
|
||||
|
||||
| Variable | Purpose |
|
||||
| ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `AUTOREVIEW_MODEL` | Override the built-in default `--model` for all engines |
|
||||
| `AUTOREVIEW_THINKING` | Default `--thinking` for all engines |
|
||||
| `AUTOREVIEW_FALLBACK_MODEL` | Default Claude `--fallback-model` chain |
|
||||
| `AUTOREVIEW_<ENGINE>_MODEL` | Per-engine model override, for example `AUTOREVIEW_CODEX_MODEL=gpt-5.6-sol` |
|
||||
| `AUTOREVIEW_<ENGINE>_THINKING` | Per-engine thinking override |
|
||||
| `AUTOREVIEW_CODEX_CONFIG` | Default Codex `-c key=value` overrides, semicolon-separated, e.g. `service_tier="fast"`; isolation flags still win |
|
||||
| `AUTOREVIEW_CODEX_SPEED` | Default Codex service tier: `fast` (priority), `flex`, or `default`; silently standard when the model does not list the tier |
|
||||
| `AUTOREVIEW_CLAUDE_FALLBACK_MODEL` | Claude-only fallback chain |
|
||||
| `AUTOREVIEW_CURSOR_ALLOW_WORKSPACE_INSTRUCTIONS` | Required `1`/true opt-in for Cursor reviews of trusted repositories |
|
||||
| Variable | Purpose |
|
||||
| ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `AUTOREVIEW_MODEL` | Override the built-in default `--model` for all engines |
|
||||
| `AUTOREVIEW_THINKING` | Default `--thinking` for all engines |
|
||||
| `AUTOREVIEW_FALLBACK_MODEL` | Default Claude `--fallback-model` chain |
|
||||
| `AUTOREVIEW_<ENGINE>_MODEL` | Per-engine model override, for example `AUTOREVIEW_CODEX_MODEL=gpt-5.6-sol` |
|
||||
| `AUTOREVIEW_<ENGINE>_THINKING` | Per-engine thinking override |
|
||||
| `AUTOREVIEW_CODEX_CONFIG` | Safe Codex model/response tuning overrides, semicolon-separated, e.g. `service_tier="fast"`; capability-bearing keys fail closed |
|
||||
| `AUTOREVIEW_CODEX_SPEED` | Codex service tier override: `fast` (priority), `flex`, or `default`; silently standard when the model does not list the tier |
|
||||
| `AUTOREVIEW_CLAUDE_FALLBACK_MODEL` | Claude-only fallback chain |
|
||||
|
||||
Codex maps thinking to `model_reasoning_effort`. Claude maps thinking to `--effort`. Droid maps thinking to `-r, --reasoning-effort`. Pi maps thinking to `--thinking`. OpenCode maps thinking to `--variant`. Copilot and Cursor reject `--thinking`. Only Claude accepts `--fallback-model`; global CLI/env fallback requires at least one Claude reviewer, and engine-specific fallback overrides require that reviewer to be selected. Non-Claude fallback overrides, including `AUTOREVIEW_<NONCLAUDE>_FALLBACK_MODEL`, fail closed instead of being silently ignored.
|
||||
Codex maps thinking to `model_reasoning_effort`. Claude maps thinking to `--effort`. Pi maps thinking to `--thinking`. Only Claude accepts `--fallback-model`; global CLI/env fallback requires at least one Claude reviewer, and engine-specific fallback overrides require that reviewer to be selected. Non-Claude fallback overrides, including `AUTOREVIEW_<NONCLAUDE>_FALLBACK_MODEL`, fail closed instead of being silently ignored.
|
||||
|
||||
## Review engine isolation
|
||||
|
||||
When autoreview runs inside the repository under review, external reviewer CLIs must not load project-local trust or configuration that the branch controls.
|
||||
|
||||
| Engine | Isolation flags | Reference |
|
||||
| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| **codex** | Auth-only config overrides, `-c project_doc_max_bytes=0`, repo `trust_level="untrusted"`, `exec --ignore-user-config --ignore-rules`, plus read-only sandbox | Codex CLI `exec --help` |
|
||||
| **claude** | `--safe-mode --setting-sources user --strict-mcp-config --disallowedTools mcp__*` plus explicit `--allowedTools` (`--safe-mode` requires Claude Code `v2.1.169+`) | Claude Code [CLI reference](https://code.claude.com/docs/en/cli-reference) |
|
||||
| **droid** | Fails closed: current CLI cannot disable both project instructions and all tools | Droid CLI `exec --help` and `--list-tools` |
|
||||
| **pi** | `--no-approve --no-session --no-context-files --no-extensions --no-skills --no-prompt-templates --no-themes --no-tools` | Pi CLI `--help`; requires Pi `v0.79.0+` |
|
||||
| **opencode** | `opencode run --dir <repo> --pure --format json`, prompt over stdin, neutral subprocess cwd, injected deny-by-default permissions, project config disabled | OpenCode CLI `--help` |
|
||||
| **cursor** | `cursor-agent --print --mode ask --sandbox enabled --output-format json\|stream-json`, prompt over stdin, temporary read-only permission config, help-probed flags, and mandatory explicit trusted-workspace opt-in | Cursor CLI [headless mode](https://cursor.com/docs/cli/headless), [output format](https://cursor.com/docs/cli/reference/output-format), [permissions](https://cursor.com/docs/cli/reference/permissions), [configuration](https://cursor.com/docs/cli/reference/configuration) |
|
||||
| Engine | Isolation flags | Reference |
|
||||
| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------- |
|
||||
| **codex** | Auth-only config overrides, isolated workspace, `exec --ignore-user-config --ignore-rules --skip-git-repo-check`, plus read-only sandbox | Codex CLI `exec --help` |
|
||||
| **claude** | `--safe-mode --setting-sources user --strict-mcp-config --disallowedTools mcp__*`; auto-memory and filesystem/shell tools disabled; empty external workspace; WebSearch by default (`v2.1.169+`) | Claude Code [CLI reference](https://code.claude.com/docs/en/cli-reference) |
|
||||
| **droid** | Fails closed: current CLI cannot disable both project instructions and all tools | Droid CLI `exec --help` and `--list-tools` |
|
||||
| **copilot** | Fails closed: repository read tools also expose ignored files outside the reviewed bundle | GitHub Copilot CLI command reference |
|
||||
| **pi** | `--no-approve --no-session --no-context-files --no-extensions --no-skills --no-prompt-templates --no-themes --no-tools` | Pi CLI `--help`; requires Pi `v0.79.0+` |
|
||||
| **opencode** | Fails closed: project/global config isolation and private-network fetch denial are not both proven | OpenCode CLI contract |
|
||||
| **cursor** | Fails closed: documented read permissions can target absolute host paths and no proven repository-only filesystem sandbox is exposed | Cursor CLI [permissions](https://cursor.com/docs/cli/reference/permissions) |
|
||||
|
||||
Codex `--ignore-user-config` skips config loading for the exec run. Autoreview reconstructs only the documented `cli_auth_credentials_store`, `forced_login_method`, and `forced_chatgpt_workspace_id` settings from `CODEX_HOME/config.toml`, keeping authentication and workspace restrictions usable without forwarding unrelated user configuration. The explicit repo trust override and zero project-doc budget keep reviewed-repo `AGENTS.md` and `.codex/` trust surfaces out of the review prompt. `--ignore-rules` skips user/project execpolicy rules. Claude `--safe-mode` disables project hooks, skills, plugins, MCP servers, and CLAUDE.md while preserving normal authentication, model selection, built-in tools, and permissions; managed settings policy can still apply. `--setting-sources user` avoids project/local settings from the reviewed checkout, and current Claude Code docs note the project-skill blocking behavior was fixed in `v2.1.69`. `--strict-mcp-config` and `--disallowedTools mcp__*` keep MCP unavailable to the review run. `--bare` is not used here because Claude's headless docs say it skips OAuth and keychain reads. Droid fails closed because its CLI cannot disable reviewed-repository `AGENTS.md` loading and all tools in the same run. Pi `--no-approve` ignores project-local files for one run; the helper requires Pi `v0.79.0+` plus help output that advertises every required isolation flag because older legacy binaries can ignore unknown flags. The current package is `@earendil-works/pi-coding-agent`; deprecated `@mariozechner/pi-coding-agent` `0.73.x` is intentionally rejected. Pi version/help probes and the review command run from neutral temporary directories, not the reviewed repo. Pi `--no-context-files` removes `AGENTS.md`/`CLAUDE.md`, the resource-disable flags keep `.pi` extensions, skills, prompts, and themes out of the run, `--no-session` avoids writing review sessions, and `--no-tools` prevents built-in read tools from escaping the repository through absolute paths. OpenCode starts from a neutral temporary directory, points at the reviewed repo with `--dir`, disables project config through `OPENCODE_DISABLE_PROJECT_CONFIG=1`, and injects `OPENCODE_CONFIG_CONTENT`; permissions default to deny, allow read/grep/glob, preserve OpenCode's `.env` ask rules, and gate `websearch`/`webfetch` with `--no-web-search`. The injected config also clears command/instruction/plugin arrays and disables write/edit/bash/task/skill/todowrite tools without changing user auth storage. Cursor's documented headless path is print mode with JSON output and workspace-relative project-resource discovery. Because the CLI exposes no per-run flag that disables every current and future project instruction surface, autoreview requires `--cursor-allow-workspace-instructions` (or its environment equivalent) for every Cursor run. Project-local Cursor/Claude hook settings, project MCP config, and global Cursor MCP config remain hard refusals because hooks execute host commands and MCP tools cannot be constrained to read-only review access. Cursor capability probes run from neutral temporary directories with the sanitized engine environment. Review runs set documented `CURSOR_CONFIG_DIR` to an ephemeral configuration that allows workspace reads while denying shell commands and relative or absolute writes. The helper sends review prompts to OpenCode and Cursor over stdin rather than argv and extracts final structured JSON from terminal result/text events. OpenCode and Cursor reject `--no-tools`; Cursor also rejects `--no-web-search` because the CLI does not expose a documented per-run web-search disable flag.
|
||||
Codex `--ignore-user-config` skips config loading for the exec run. Autoreview reconstructs only the documented `cli_auth_credentials_store`, `forced_login_method`, and `forced_chatgpt_workspace_id` settings from `CODEX_HOME/config.toml`, keeping authentication usable without forwarding unrelated user configuration. Codex runs in an empty temporary workspace: the validated bundle is its sole repository input, ignored files and linked-worktree metadata remain unreadable, and the zero project-doc budget keeps workspace instructions out of the prompt. `--ignore-rules` skips user/project execpolicy rules. Claude `--safe-mode` disables project hooks, skills, plugins, MCP servers, and CLAUDE.md; autoreview supplies WebSearch by default, permits only explicitly domain-constrained WebFetch rules, and exposes no filesystem or shell tools. Pi runs from a neutral temporary directory with project resources disabled and `--no-tools`. Droid, Copilot, Cursor, and OpenCode fail closed because their current CLI contracts cannot isolate untrusted review input from host, project, or private-network trust surfaces.
|
||||
|
||||
Codex uses a named permission profile that grants read access only to an empty temporary workspace. This is narrower than repository-root access, which would expose ignored credentials, and narrower than the legacy `read-only` sandbox, which permits reads across the host filesystem.
|
||||
|
||||
## Context Efficiency
|
||||
|
||||
@@ -322,13 +337,13 @@ The smoke harness has thin shell wrappers over a shared Python implementation:
|
||||
On native Windows, invoke the extensionless Python helper through Python:
|
||||
|
||||
```powershell
|
||||
python skills\autoreview\scripts\autoreview --help
|
||||
python $AUTOREVIEW --help
|
||||
```
|
||||
|
||||
and the smoke harness:
|
||||
|
||||
```powershell
|
||||
skills\autoreview\scripts\test-review-harness.ps1 -Fixture benign -Engine codex
|
||||
& $AUTOREVIEW_HARNESS -Fixture benign -Engine codex
|
||||
```
|
||||
|
||||
The helper:
|
||||
@@ -338,21 +353,19 @@ The helper:
|
||||
- otherwise uses current PR base if `gh pr view` works
|
||||
- otherwise uses `origin/main` for non-main branches
|
||||
- does not fetch automatically during branch review; the selected base ref must already resolve locally
|
||||
- recognizes `--engine droid` only to fail closed with an isolation error; runnable engines are `codex`, `claude`, `copilot`, `pi`, `opencode`, and `cursor`; default is `AUTOREVIEW_ENGINE` or `codex`
|
||||
- resolves bare `git`, `gh`, reviewer, and PowerShell shell commands from absolute `PATH` entries only, never from the reviewed checkout; explicit relative `--*-bin` paths are resolved from the reviewed repository root
|
||||
- recognizes `--engine droid`, `copilot`, `cursor`, and `opencode` only to fail closed with isolation errors; runnable engines are `codex`, `claude`, and `pi`; default is `AUTOREVIEW_ENGINE` or `codex`
|
||||
- resolves bare `git`, `gh`, reviewer, and PowerShell shell commands from absolute `PATH` entries only, never from the reviewed checkout; explicit `--*-bin` paths are interpreted from the reviewed repository root when relative and accepted only when both the supplied path and resolved target stay outside the reviewed repository
|
||||
- use `--mode commit --commit <ref>` for already-committed work, especially clean `main` after landing
|
||||
- should be left in `--mode auto` or forced to `--mode branch` for PR/branch work; do not force `--mode local` after committing
|
||||
- writes only to stdout unless `--output`, `--json-output`, or live streamed engine stderr is set
|
||||
- supports `--dry-run`, `--parallel-tests`, `--parallel-tests-shell`, `--prompt`, repo-relative `--prompt-file`, repo-relative `--dataset`, `--no-tools`, `--no-web-search`, repeatable Codex-only `--codex-config key=value`, Codex-only `--codex-speed fast|flex|default`, and commit refs
|
||||
- supports `--stream-engine-output` or `AUTOREVIEW_STREAM_ENGINE_OUTPUT=1` for live engine text while preserving structured validation; Codex, Claude, and Cursor hide tool/file event details, emit compact activity summaries, and report usage at turn completion
|
||||
- supports `--dry-run`, `--parallel-tests`, `--parallel-tests-shell`, `--prompt`, repo-relative `--prompt-file`, repo-relative `--dataset`, `--no-tools`, `--no-web-search`, repeatable Codex-only safe model/response tuning with `--codex-config key=value`, Codex-only `--codex-speed fast|flex|default`, and commit refs
|
||||
- supports `--stream-engine-output` or `AUTOREVIEW_STREAM_ENGINE_OUTPUT=1` for live engine text while preserving structured validation; Codex and Claude hide tool/file event details, emit compact activity summaries, and report usage at turn completion
|
||||
- supports opt-in review panels with `--panel` / `--reviewers`, plus per-engine `--model`, `--thinking`, and Claude `--fallback-model`
|
||||
- uses built-in model defaults `codex=gpt-5.6-sol` and `claude=claude-fable-5`; honors `AUTOREVIEW_MODEL`, `AUTOREVIEW_THINKING`, `AUTOREVIEW_FALLBACK_MODEL`, and per-engine `AUTOREVIEW_<ENGINE>_MODEL` / `AUTOREVIEW_<ENGINE>_THINKING` environment overrides when CLI flags are omitted
|
||||
- allows read-only tools and web search by default where the selected CLI supports them; forbids nested review in the prompt; Codex is run through `codex exec` with auth-only user settings, read-only sandbox, reviewed-repo instruction/config/rule isolation flags, and structured output
|
||||
- runs Claude with `--safe-mode` (`v2.1.169+`), `--setting-sources user`, MCP disabled, explicit allowed tools, and `--fallback-model` when set, so reviewed-repo hooks/skills/MCP do not affect the review run while normal auth still works; managed settings policy can still apply
|
||||
- refuses Droid reviews until the CLI exposes a complete project-instruction and tool-isolation contract
|
||||
- uses built-in defaults `codex=gpt-5.6-sol` with `high` reasoning and an access-only `gpt-5.6-terra` retry, plus `claude=claude-fable-5`; honors `AUTOREVIEW_MODEL`, `AUTOREVIEW_THINKING`, `AUTOREVIEW_FALLBACK_MODEL`, and per-engine `AUTOREVIEW_<ENGINE>_MODEL` / `AUTOREVIEW_<ENGINE>_THINKING` environment overrides when CLI flags are omitted
|
||||
- gives Codex the bundle in an empty workspace with web search available; Claude receives the bundle plus WebSearch by default and optional domain-constrained WebFetch, and Pi receives the bundle with no tools
|
||||
- runs Claude with `--safe-mode` (`v2.1.169+`), `--setting-sources user`, MCP and auto-memory disabled, no filesystem/shell tools, an empty external workspace, and `--fallback-model` when set
|
||||
- refuses Droid, Copilot, Cursor, and OpenCode reviews until their CLIs expose the required project, filesystem, and network isolation
|
||||
- runs Pi `v0.79.0+` from neutral temporary directories with `--no-approve`, `--no-session`, disabled Pi context/resource loading, and `--no-tools` because its built-in read tools are not repository-confined
|
||||
- runs OpenCode with `opencode run --dir <repo> --pure --format json` from a neutral temporary directory, forwards `--model` and `--variant`, injects deny-by-default permissions, disables project config loading, and passes the review prompt over stdin
|
||||
- runs Cursor only with mandatory trusted-workspace opt-in, uses `cursor-agent --print --mode ask --sandbox enabled --output-format json`, forwards `--model`, passes the review prompt over stdin, and always refuses project-local hooks/MCP
|
||||
- prints `review still running: <engine> elapsed=<seconds>s pid=<pid>` to stderr at long-running intervals while waiting for the selected review engine, unless streamed output or compact Codex activity has been visible recently
|
||||
- prints `autoreview clean: no accepted/actionable findings reported` when the selected review command exits 0
|
||||
- exits nonzero when accepted/actionable findings are present
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -125,22 +125,11 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
os.environ[key] = value
|
||||
cls.home_dir.cleanup()
|
||||
|
||||
def test_harness_opts_both_cursor_aliases_into_trusted_fixture(self) -> None:
|
||||
def test_harness_rejects_disabled_cursor_engine(self) -> None:
|
||||
harness_path = SCRIPT_PATH.with_name("test-review-harness.py")
|
||||
namespace = runpy.run_path(str(harness_path))
|
||||
commands: list[list[str]] = []
|
||||
run_reviews = namespace["run_reviews"]
|
||||
with mock.patch.dict(
|
||||
run_reviews.__globals__,
|
||||
{
|
||||
"run": lambda command, _cwd: commands.append(command),
|
||||
"validate_prompt_policy": lambda _repo, _autoreview: None,
|
||||
},
|
||||
), tempfile.TemporaryDirectory(prefix="autoreview-harness-test.") as tmpdir:
|
||||
run_reviews(Path(tmpdir), SCRIPT_PATH.parent, "benign", ["cursor", "cursor-agent"])
|
||||
self.assertEqual(len(commands), 2)
|
||||
for command in commands:
|
||||
self.assertIn("--cursor-allow-workspace-instructions", command)
|
||||
with self.assertRaises(SystemExit):
|
||||
namespace["parse_args"](["--engine", "cursor"])
|
||||
|
||||
def test_cursor_agent_bin_cli_alias(self) -> None:
|
||||
with mock.patch.object(
|
||||
@@ -175,8 +164,226 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
)
|
||||
|
||||
def test_codex_config_status_exposes_keys_only(self) -> None:
|
||||
args = argparse.Namespace(codex_config=['model_provider="private-value"'])
|
||||
self.assertEqual(AUTOREVIEW.codex_config_keys(args), ["model_provider"])
|
||||
args = argparse.Namespace(codex_config=['model_verbosity="low"'])
|
||||
self.assertEqual(AUTOREVIEW.codex_config_keys(args), ["model_verbosity"])
|
||||
|
||||
def test_codex_retries_terra_after_sol_access_failure(self) -> None:
|
||||
args = argparse.Namespace(
|
||||
codex_bin="codex",
|
||||
codex_config=None,
|
||||
codex_speed=None,
|
||||
fallback_model="gpt-5.6-terra",
|
||||
model="gpt-5.6-sol",
|
||||
stream_engine_output=False,
|
||||
thinking="high",
|
||||
tools=True,
|
||||
web_search=False,
|
||||
)
|
||||
models: list[str] = []
|
||||
|
||||
def fake_run(command: list[str], *_args: object, **_kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
model = command[command.index("--model") + 1]
|
||||
models.append(model)
|
||||
if model == "gpt-5.6-sol":
|
||||
return subprocess.CompletedProcess(
|
||||
command,
|
||||
1,
|
||||
"",
|
||||
"The model `gpt-5.6-sol` does not exist or you do not have access to it.",
|
||||
)
|
||||
output_path = Path(command[command.index("--output-last-message") + 1])
|
||||
output_path.write_text(json.dumps(FINAL_REPORT))
|
||||
return subprocess.CompletedProcess(command, 0, "", "")
|
||||
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-codex-fallback.") as tmpdir, mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"resolve_command",
|
||||
return_value="/usr/bin/codex",
|
||||
), mock.patch.object(AUTOREVIEW, "codex_auth_config_flags", return_value=[]), mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"run_with_heartbeat",
|
||||
side_effect=fake_run,
|
||||
):
|
||||
output = AUTOREVIEW.run_codex(args, Path(tmpdir), "review")
|
||||
|
||||
self.assertEqual(json.loads(output), FINAL_REPORT)
|
||||
self.assertEqual(models, ["gpt-5.6-sol", "gpt-5.6-terra"])
|
||||
|
||||
def test_codex_runs_outside_repo_with_bundle_only_workspace(self) -> None:
|
||||
args = argparse.Namespace(
|
||||
codex_bin="codex",
|
||||
codex_config=None,
|
||||
codex_speed=None,
|
||||
fallback_model=None,
|
||||
model="gpt-5.6-sol",
|
||||
stream_engine_output=False,
|
||||
thinking="high",
|
||||
tools=True,
|
||||
web_search=False,
|
||||
)
|
||||
observed: dict[str, object] = {}
|
||||
|
||||
def fake_run(
|
||||
command: list[str],
|
||||
cwd: Path,
|
||||
*_args: object,
|
||||
**_kwargs: object,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
observed["cwd"] = cwd
|
||||
observed["command_cwd"] = Path(command[command.index("-C") + 1])
|
||||
observed["workspace_entries"] = list(cwd.iterdir())
|
||||
output_path = Path(command[command.index("--output-last-message") + 1])
|
||||
output_path.write_text(json.dumps(FINAL_REPORT))
|
||||
return subprocess.CompletedProcess(command, 0, "", "")
|
||||
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-codex-workspace-test.") as tmpdir:
|
||||
repo = Path(tmpdir)
|
||||
(repo / ".env").write_text("OPENAI_API_KEY=ignored-secret\n")
|
||||
with mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"resolve_command",
|
||||
return_value="/usr/bin/codex",
|
||||
), mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"codex_auth_config_flags",
|
||||
return_value=[],
|
||||
), mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"run_with_heartbeat",
|
||||
side_effect=fake_run,
|
||||
):
|
||||
output = AUTOREVIEW.run_codex(args, repo, "review")
|
||||
|
||||
self.assertEqual(json.loads(output), FINAL_REPORT)
|
||||
observed_cwd = observed["cwd"]
|
||||
command_cwd = observed["command_cwd"]
|
||||
self.assertIsInstance(observed_cwd, Path)
|
||||
self.assertIsInstance(command_cwd, Path)
|
||||
assert isinstance(observed_cwd, Path)
|
||||
assert isinstance(command_cwd, Path)
|
||||
self.assertNotEqual(observed_cwd.resolve(), repo.resolve())
|
||||
self.assertEqual(observed_cwd, command_cwd)
|
||||
self.assertEqual(observed["workspace_entries"], [])
|
||||
|
||||
def test_codex_does_not_fallback_after_unrelated_failure(self) -> None:
|
||||
args = argparse.Namespace(
|
||||
codex_bin="codex",
|
||||
codex_config=None,
|
||||
codex_speed=None,
|
||||
fallback_model="gpt-5.6-terra",
|
||||
model="gpt-5.6-sol",
|
||||
stream_engine_output=False,
|
||||
thinking="high",
|
||||
tools=True,
|
||||
web_search=False,
|
||||
)
|
||||
models: list[str] = []
|
||||
|
||||
def fake_run(command: list[str], *_args: object, **_kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
models.append(command[command.index("--model") + 1])
|
||||
return subprocess.CompletedProcess(command, 1, "", "network timeout")
|
||||
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-codex-fallback.") as tmpdir, mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"resolve_command",
|
||||
return_value="/usr/bin/codex",
|
||||
), mock.patch.object(AUTOREVIEW, "codex_auth_config_flags", return_value=[]), mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"run_with_heartbeat",
|
||||
side_effect=fake_run,
|
||||
):
|
||||
with self.assertRaisesRegex(SystemExit, "network timeout"):
|
||||
AUTOREVIEW.run_codex(args, Path(tmpdir), "review")
|
||||
|
||||
self.assertEqual(models, ["gpt-5.6-sol"])
|
||||
|
||||
def test_codex_does_not_fallback_after_model_capacity_failure(self) -> None:
|
||||
args = argparse.Namespace(
|
||||
codex_bin="codex",
|
||||
codex_config=None,
|
||||
codex_speed=None,
|
||||
fallback_model="gpt-5.6-terra",
|
||||
model="gpt-5.6-sol",
|
||||
stream_engine_output=False,
|
||||
thinking="high",
|
||||
tools=True,
|
||||
web_search=False,
|
||||
)
|
||||
models: list[str] = []
|
||||
|
||||
def fake_run(command: list[str], *_args: object, **_kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
models.append(command[command.index("--model") + 1])
|
||||
return subprocess.CompletedProcess(
|
||||
command,
|
||||
1,
|
||||
"",
|
||||
"model_not_available: gpt-5.6-sol is temporarily unavailable due to capacity",
|
||||
)
|
||||
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-codex-fallback.") as tmpdir, mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"resolve_command",
|
||||
return_value="/usr/bin/codex",
|
||||
), mock.patch.object(AUTOREVIEW, "codex_auth_config_flags", return_value=[]), mock.patch.object(
|
||||
AUTOREVIEW,
|
||||
"run_with_heartbeat",
|
||||
side_effect=fake_run,
|
||||
):
|
||||
with self.assertRaisesRegex(SystemExit, "temporarily unavailable"):
|
||||
AUTOREVIEW.run_codex(args, Path(tmpdir), "review")
|
||||
|
||||
self.assertEqual(models, ["gpt-5.6-sol"])
|
||||
|
||||
def test_codex_access_fallback_ignores_structured_output_text(self) -> None:
|
||||
result = subprocess.CompletedProcess(
|
||||
["codex"],
|
||||
1,
|
||||
'{"type":"agent_message","text":"gpt-5.6-sol does not exist or you do not have access"}',
|
||||
'{"type":"agent_message","message":"gpt-5.6-sol does not exist or you do not have access"}',
|
||||
)
|
||||
|
||||
self.assertFalse(
|
||||
AUTOREVIEW.codex_model_access_failure(result, "gpt-5.6-sol")
|
||||
)
|
||||
|
||||
def test_codex_access_fallback_accepts_terminal_error_event(self) -> None:
|
||||
result = subprocess.CompletedProcess(
|
||||
["codex"],
|
||||
1,
|
||||
'{"type":"error","message":"gpt-5.6-sol does not exist or you do not have access"}',
|
||||
"",
|
||||
)
|
||||
|
||||
self.assertTrue(
|
||||
AUTOREVIEW.codex_model_access_failure(result, "gpt-5.6-sol")
|
||||
)
|
||||
|
||||
def test_codex_access_fallback_accepts_account_model_list_error(self) -> None:
|
||||
result = subprocess.CompletedProcess(
|
||||
["codex"],
|
||||
1,
|
||||
"",
|
||||
(
|
||||
"The model gpt-5.6-sol does not appear in the list of models "
|
||||
"available to your account"
|
||||
),
|
||||
)
|
||||
|
||||
self.assertTrue(
|
||||
AUTOREVIEW.codex_model_access_failure(result, "gpt-5.6-sol")
|
||||
)
|
||||
|
||||
def test_codex_access_fallback_ignores_plain_stdout(self) -> None:
|
||||
message = "gpt-5.6-sol does not exist or you do not have access"
|
||||
stdout_result = subprocess.CompletedProcess(["codex"], 1, message, "")
|
||||
stderr_result = subprocess.CompletedProcess(["codex"], 1, "", message)
|
||||
|
||||
self.assertFalse(
|
||||
AUTOREVIEW.codex_model_access_failure(stdout_result, "gpt-5.6-sol")
|
||||
)
|
||||
self.assertTrue(
|
||||
AUTOREVIEW.codex_model_access_failure(stderr_result, "gpt-5.6-sol")
|
||||
)
|
||||
|
||||
def test_extract_json_accepts_dict_result_payload(self) -> None:
|
||||
payload = {
|
||||
@@ -188,32 +395,14 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
}
|
||||
self.assertEqual(AUTOREVIEW.extract_json(json.dumps(payload)), FINAL_REPORT)
|
||||
|
||||
def test_extract_json_accepts_result_string_with_preamble(self) -> None:
|
||||
def test_extract_json_rejects_result_string_with_preamble(self) -> None:
|
||||
payload = {
|
||||
"type": "result",
|
||||
"subtype": "success",
|
||||
"result": "Inspecting the diff first.\n" + json.dumps(FINAL_REPORT),
|
||||
}
|
||||
self.assertEqual(AUTOREVIEW.extract_json(json.dumps(payload)), FINAL_REPORT)
|
||||
|
||||
def test_extract_findings_json_from_text_prefers_last_findings_object(self) -> None:
|
||||
later_report = {
|
||||
"findings": [
|
||||
{
|
||||
"title": "Later finding",
|
||||
"body": "later",
|
||||
"priority": "P2",
|
||||
"confidence": 0.8,
|
||||
"category": "bug",
|
||||
"code_location": {"file_path": "later.js", "line": 2},
|
||||
}
|
||||
],
|
||||
"overall_correctness": "patch is incorrect",
|
||||
"overall_explanation": "later",
|
||||
"overall_confidence": 0.8,
|
||||
}
|
||||
text = f"{json.dumps(FINAL_REPORT)} separator {json.dumps(later_report)}"
|
||||
self.assertEqual(AUTOREVIEW.extract_findings_json_from_text(text), later_report)
|
||||
with self.assertRaisesRegex(SystemExit, "result was not structured JSON"):
|
||||
AUTOREVIEW.extract_json(json.dumps(payload))
|
||||
|
||||
def test_retry_filter_only_matches_parse_failures(self) -> None:
|
||||
self.assertTrue(AUTOREVIEW.is_structured_output_failure("review engine returned non-JSON output: nope"))
|
||||
@@ -235,7 +424,7 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
)
|
||||
with self.assertRaises(SystemExit) as exc_info:
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
self.assertIn("requires --cursor-allow-workspace-instructions", str(exc_info.exception))
|
||||
self.assertIn("cursor engine is unavailable", str(exc_info.exception))
|
||||
|
||||
def test_cursor_local_mcp_requires_explicit_approval(self) -> None:
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-cursor-test.") as tmpdir:
|
||||
@@ -253,7 +442,7 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
)
|
||||
with self.assertRaises(SystemExit) as exc_info:
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
self.assertIn("cursor engine refused project-local MCP config", str(exc_info.exception))
|
||||
self.assertIn("cursor engine is unavailable", str(exc_info.exception))
|
||||
|
||||
def test_cursor_local_hooks_are_always_refused(self) -> None:
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-cursor-test.") as tmpdir:
|
||||
@@ -271,7 +460,7 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
)
|
||||
with self.assertRaises(SystemExit) as exc_info:
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
self.assertIn("cursor engine refused project-local hooks", str(exc_info.exception))
|
||||
self.assertIn("cursor engine is unavailable", str(exc_info.exception))
|
||||
|
||||
def test_cursor_local_permissions_are_always_refused(self) -> None:
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-cursor-test.") as tmpdir:
|
||||
@@ -289,15 +478,14 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
)
|
||||
with self.assertRaises(SystemExit) as exc_info:
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
self.assertIn("cursor engine refused project-local permission config", str(exc_info.exception))
|
||||
self.assertIn("cursor engine is unavailable", str(exc_info.exception))
|
||||
|
||||
def test_cursor_command_uses_current_print_contract(self) -> None:
|
||||
def test_cursor_is_disabled_without_repo_only_read_sandbox(self) -> None:
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-cursor-test.") as tmpdir:
|
||||
root = Path(tmpdir)
|
||||
repo = root / "repo"
|
||||
repo.mkdir()
|
||||
cursor_bin = root / "cursor-agent"
|
||||
record_path = root / "record.json"
|
||||
AUTOREVIEW.write_executable(cursor_bin, AUTOREVIEW.fake_cursor_script())
|
||||
args = argparse.Namespace(
|
||||
thinking=None,
|
||||
@@ -308,29 +496,11 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
model=None,
|
||||
stream_engine_output=False,
|
||||
)
|
||||
old_record = os.environ.get("AUTOREVIEW_FAKE_RECORD")
|
||||
try:
|
||||
os.environ["AUTOREVIEW_FAKE_RECORD"] = str(record_path)
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
finally:
|
||||
if old_record is None:
|
||||
os.environ.pop("AUTOREVIEW_FAKE_RECORD", None)
|
||||
else:
|
||||
os.environ["AUTOREVIEW_FAKE_RECORD"] = old_record
|
||||
record = json.loads(record_path.read_text())
|
||||
self.assertEqual(Path(record["cwd"]).resolve(), repo.resolve())
|
||||
self.assertEqual(record["stdin"], "prompt")
|
||||
self.assertIn("--print", record["argv"])
|
||||
self.assertIn("--output-format", record["argv"])
|
||||
self.assertIn("json", record["argv"])
|
||||
self.assertIn("--mode", record["argv"])
|
||||
self.assertIn("ask", record["argv"])
|
||||
self.assertIn("--sandbox", record["argv"])
|
||||
self.assertIn("enabled", record["argv"])
|
||||
for unsupported in ("--workspace", "--trust"):
|
||||
self.assertNotIn(unsupported, record["argv"])
|
||||
with mock.patch.object(AUTOREVIEW, "cursor_global_hook_paths", return_value=[]):
|
||||
with self.assertRaisesRegex(SystemExit, "Cursor read permissions"):
|
||||
AUTOREVIEW.run_cursor(args, repo, "prompt")
|
||||
|
||||
def test_cursor_engine_runs_end_to_end_with_sanitized_environment(self) -> None:
|
||||
def test_cursor_engine_fails_closed_end_to_end(self) -> None:
|
||||
with tempfile.TemporaryDirectory(prefix="autoreview-cursor-e2e.") as tmpdir:
|
||||
root = Path(tmpdir)
|
||||
repo = root / "repo"
|
||||
@@ -356,6 +526,8 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
"NODE_OPTIONS": "--require=hostile.js",
|
||||
"PYTHONPATH": str(root / "hostile-python"),
|
||||
"PATH": f"{repo}{os.pathsep}{env.get('PATH', '')}",
|
||||
"HOME": str(root),
|
||||
"USERPROFILE": str(root),
|
||||
}
|
||||
)
|
||||
result = subprocess.run(
|
||||
@@ -377,33 +549,9 @@ class AutoreviewCompatibilityTests(unittest.TestCase):
|
||||
check=False,
|
||||
)
|
||||
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn("autoreview clean: no accepted/actionable findings reported", result.stdout)
|
||||
record = json.loads(record_path.read_text())
|
||||
self.assertEqual(Path(record["cwd"]).resolve(), repo.resolve())
|
||||
self.assertIn("diff --git a/example.txt b/example.txt", record["stdin"])
|
||||
self.assertIn("-before", record["stdin"])
|
||||
self.assertIn("+after", record["stdin"])
|
||||
self.assertEqual(record["environment"]["GIT_CONFIG_GLOBAL"], None)
|
||||
self.assertEqual(record["environment"]["NODE_OPTIONS"], None)
|
||||
self.assertEqual(record["environment"]["PYTHONPATH"], None)
|
||||
self.assertNotIn(str(repo), record["environment"]["PATH"].split(os.pathsep))
|
||||
cursor_config_dir = Path(record["environment"]["CURSOR_CONFIG_DIR"])
|
||||
self.assertFalse(cursor_config_dir.exists())
|
||||
cursor_config = json.loads(record["cursor_config"])
|
||||
self.assertEqual(cursor_config["permissions"]["allow"], ["Read(**)"])
|
||||
self.assertEqual(
|
||||
cursor_config["permissions"]["deny"],
|
||||
["Shell(*)", "Write(**)", "Write(/**)"],
|
||||
)
|
||||
|
||||
invocations = [json.loads(line) for line in (root / "cursor-invocations.jsonl").read_text().splitlines()]
|
||||
help_invocation = next(invocation for invocation in invocations if "--help" in invocation["argv"])
|
||||
self.assertNotEqual(Path(help_invocation["cwd"]).resolve(), repo.resolve())
|
||||
self.assertEqual(help_invocation["environment"]["GIT_CONFIG_GLOBAL"], None)
|
||||
self.assertEqual(help_invocation["environment"]["NODE_OPTIONS"], None)
|
||||
self.assertEqual(help_invocation["environment"]["PYTHONPATH"], None)
|
||||
self.assertNotIn(str(repo), help_invocation["environment"]["PATH"].split(os.pathsep))
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertIn("Cursor read permissions", result.stderr)
|
||||
self.assertFalse(record_path.exists())
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -3,7 +3,7 @@ param(
|
||||
[ValidateSet('malicious', 'benign')]
|
||||
[string] $Fixture,
|
||||
|
||||
[ValidateSet('codex', 'claude', 'droid', 'copilot', 'pi', 'opencode', 'cursor', 'cursor-agent')]
|
||||
[ValidateSet('codex', 'claude', 'pi')]
|
||||
[string[]] $Engine,
|
||||
|
||||
[Alias('h')]
|
||||
|
||||
@@ -13,7 +13,7 @@ from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
ENGINES = ("codex", "claude", "droid", "copilot", "pi", "opencode", "cursor", "cursor-agent")
|
||||
ENGINES = ("codex", "claude", "pi")
|
||||
DEFAULT_ENGINES = ("codex", "claude")
|
||||
|
||||
MALICIOUS_INITIAL = """export function uploadPath(name) {
|
||||
@@ -175,10 +175,6 @@ def run_reviews(repo: Path, script_dir: Path, fixture: str, engines: list[str])
|
||||
"--prompt",
|
||||
MALICIOUS_PROMPT if fixture == "malicious" else BENIGN_PROMPT,
|
||||
]
|
||||
if engine in {"cursor", "cursor-agent"}:
|
||||
# The harness owns this temporary fixture, so it can make the
|
||||
# trusted-workspace assertion required by Cursor reviews.
|
||||
command.append("--cursor-allow-workspace-instructions")
|
||||
if fixture == "malicious":
|
||||
command.extend(["--require-finding", "command", "--expect-findings"])
|
||||
run(command, repo)
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user