diff --git a/.cargo/config.toml b/.cargo/config.toml index 9c9e20f089..5df8ea0c15 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -20,3 +20,7 @@ rustflags = ["-C", "target-feature=+aes,+sse2,+ssse3,+sse4.1,+sse4.2"] [target.x86_64-pc-windows-msvc] rustflags = ["-C", "target-feature=+aes,+sse2,+ssse3,+sse4.1,+sse4.2"] + +[env] +# test threads get the 16 MB stack of the server's runtime threads (THREAD_STACK_SIZE); debug async code outgrows 2 MB +RUST_MIN_STACK = "16777216" diff --git a/.github/workflows/ai-code-review.yml b/.github/workflows/ai-code-review.yml index 975fbc32ec..3114220bd4 100644 --- a/.github/workflows/ai-code-review.yml +++ b/.github/workflows/ai-code-review.yml @@ -68,7 +68,6 @@ jobs: - name: anthropic provider_id: anthropic-review model_id: claude-sonnet-5-5 - model_variant: high api_key_env: ANTHROPIC_REVIEW_API_KEY marker: "" steps: @@ -109,6 +108,11 @@ jobs: fi merge_base="$(gh api "repos/$REPO/compare/$base_sha...$head_sha" --jq '.merge_base_commit.sha' || true)" echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" + { + echo "author=$(jq -r '.user.login // ""' <<<"$pr_json")" + echo "changed_files=$(jq -r '.changed_files // ""' <<<"$pr_json")" + echo "commit_count=$(jq -r '.commits // ""' <<<"$pr_json")" + } >> "$GITHUB_OUTPUT" echo "REVIEW_RUN=true" >> "$GITHUB_ENV" # If minting fails (app not installed, bad key) the steps below fall back to github.token. @@ -149,6 +153,37 @@ jobs: .claude/skills/ui-architect .claude/skills/eslint-error-handling + # Bills nothing and posts nothing; a hand-started run, a failed listing or a failed check always reviews. + - name: Skip PRs with nothing to review + if: env.REVIEW_RUN == 'true' && github.event_name == 'pull_request_target' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + PR_AUTHOR: ${{ steps.pr.outputs.author }} + # The listings are checked against these, as both endpoints stop at a cap however far they are paged. + EXPECTED_FILES: ${{ steps.pr.outputs.changed_files }} + EXPECTED_COMMITS: ${{ steps.pr.outputs.commit_count }} + run: | + set -uo pipefail + list="$RUNNER_TEMP/pr-files.tsv" + commits="$RUNNER_TEMP/pr-commits.tsv" + # One line per file with its rename source, so code moved under docs/ is not taken for docs and the count can be checked. + if ! gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/files?per_page=100" --jq '.[] | [.filename, (.previous_filename // "")] | @tsv' > "$list"; then + echo "::warning::Could not list the PR's files; reviewing." + : > "$list" + fi + # One line per commit: author login, committer login, signature verified; a missing field lists as empty and reviews. + if ! gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/commits?per_page=100" --jq '.[] | [(.author.login // ""), (.committer.login // ""), ((.commit.verification.verified // "") | tostring)] | @tsv' > "$commits"; then + echo "::warning::Could not list the PR's commits; reviewing." + : > "$commits" + fi + reason="$(node engine/scripts/ai-review/skip-check.mjs "$list" "$commits")" || reason="" + if [ -n "$reason" ]; then + echo "::notice::PR #$PR_NUMBER needs no review ($reason); skipped." + echo "REVIEW_RUN=false" >> "$GITHUB_ENV" + fi + # Paired OSS+ENT changes are reviewed once, by the o2-enterprise workflow, which pulls this PR's diff in. - name: Defer to a paired o2-enterprise PR if: env.REVIEW_RUN == 'true' && !inputs.ignore_paired @@ -291,10 +326,12 @@ jobs: FORCE_FULL: ${{ inputs.force_full || 'false' }} REVIEW_PROVIDER_ID: ${{ matrix.provider.provider_id }} REVIEW_MODEL_ID: ${{ matrix.provider.model_id }} - REVIEW_MODEL_VARIANT: ${{ matrix.provider.model_variant }} + # The effort is set here only, so the variable is the single source; each value must be a variant in opencode.ci.jsonc. + REVIEW_MODEL_VARIANT: ${{ vars.AI_REVIEW_EFFORT || 'high' }} + REVIEW_COORDINATOR_VARIANT: ${{ vars.AI_REVIEW_COORDINATOR_EFFORT || vars.AI_REVIEW_EFFORT || 'high' }} REVIEW_API_KEY_ENV: ${{ matrix.provider.api_key_env }} REVIEW_MARKER: ${{ matrix.provider.marker }} - # opencode.ci.jsonc: every tool (shell, edit, webfetch) allowed with no prompts; "default" is the read-only opencode.jsonc. + # opencode.ci.jsonc: file tools plus a read-only shell allowlist, no web or sub-agent tools, no prompts; "default" is the read-only opencode.jsonc. REVIEW_OPENCODE_CONFIG: ci-full # Empty under the github.token fallback: any workflow a same-repo PR adds can also post as github-actions[bot]. REVIEW_BOT_LOGIN: ${{ steps.app_token.outputs.token && format('{0}[bot]', steps.app_token.outputs.app-slug) || '' }} @@ -313,6 +350,8 @@ jobs: # Optional ceiling on automatic reviews per PR; unset means no cap, so each new commit gets one review. The manual input bypasses it for one run. REVIEW_MAX_PASSES: ${{ vars.AI_REVIEW_MAX_PASSES }} REVIEW_IGNORE_PASS_LIMIT: ${{ inputs.ignore_pass_limit || 'false' }} + # A hand-started run reviews even a diff the last review already read. + REVIEW_MANUAL_RUN: ${{ github.event_name == 'workflow_dispatch' }} # Independent reviewer sessions per area (1-3), merged by the coordinator; one cold pass misses findings a second catches. REVIEW_COLD_PASSES: ${{ vars.AI_REVIEW_COLD_PASSES || '2' }} # An unknown value means off, so a typo can never start approving. diff --git a/opencode.ci.jsonc b/opencode.ci.jsonc index 339d07aef0..b3a3dc64fa 100644 --- a/opencode.ci.jsonc +++ b/opencode.ci.jsonc @@ -1,7 +1,5 @@ { - // Loaded from the trusted base-branch checkout via OPENCODE_CONFIG when REVIEW_OPENCODE_CONFIG=ci-full. - // Same provider, model and agents as opencode.jsonc, but every tool (shell, edit, webfetch, ...) is allowed - // and nothing asks for permission. Keep the two files in step apart from the agent permissions. + // ci-full, loaded from the trusted base checkout: keep in step with opencode.jsonc except agent permissions (file tools, shell allowlist, no prompts). "$schema": "https://opencode.ai/config.json", // Nothing here may reach into the PR tree: no `instructions`, no `skills`, no plugins. @@ -28,8 +26,14 @@ "models": { "claude-sonnet-5-5": { "name": "Claude Sonnet 5.5", - // Selected with REVIEW_MODEL_VARIANT=high; opencode sends adaptive thinking and "output_config": { "effort": "high" }. - "variants": [{ "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }], + // REVIEW_MODEL_VARIANT and REVIEW_COORDINATOR_VARIANT pick one; opencode sends adaptive thinking and "output_config": { "effort": "" }. + "variants": [ + { "id": "low", "settings": { "thinking": { "type": "adaptive" }, "effort": "low" } }, + { "id": "medium", "settings": { "thinking": { "type": "adaptive" }, "effort": "medium" } }, + { "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }, + { "id": "xhigh", "settings": { "thinking": { "type": "adaptive" }, "effort": "xhigh" } }, + { "id": "max", "settings": { "thinking": { "type": "adaptive" }, "effort": "max" } } + ], "limit": { "context": 1000000, "output": 128000 } } } @@ -46,7 +50,7 @@ "models": { "deepseek-flash": { "name": "DeepSeek-V4.1-Flash", - // Selected with REVIEW_MODEL_VARIANT=max (the workflow matrix); opencode sends "reasoning_effort": "max". + // Selected with REVIEW_MODEL_VARIANT=max; opencode sends "reasoning_effort": "max". "variants": [{ "id": "max", "settings": { "reasoningEffort": "max" } }], "limit": { "context": 1048576, "output": 393216 } } @@ -56,52 +60,907 @@ "model": "anthropic-review/claude-sonnet-5-5", - // One allow-all rule: it comes after opencode's built-in defaults (last match wins), so no tool ever asks. + // Last match wins after opencode's built-in defaults, so nothing asks; the shell allowlist guards cost, not secrets (see README). "agents": { "ai-review-security": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-correctness": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-code-quality": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-performance": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-documentation": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-release": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-frontend": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-qa": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-coordinator": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] } } } diff --git a/opencode.jsonc b/opencode.jsonc index 18fef5e1a1..daf03d4660 100644 --- a/opencode.jsonc +++ b/opencode.jsonc @@ -27,8 +27,14 @@ "models": { "claude-sonnet-5-5": { "name": "Claude Sonnet 5.5", - // Selected with REVIEW_MODEL_VARIANT=high; opencode sends adaptive thinking and "output_config": { "effort": "high" }. - "variants": [{ "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }], + // REVIEW_MODEL_VARIANT and REVIEW_COORDINATOR_VARIANT pick one; opencode sends adaptive thinking and "output_config": { "effort": "" }. + "variants": [ + { "id": "low", "settings": { "thinking": { "type": "adaptive" }, "effort": "low" } }, + { "id": "medium", "settings": { "thinking": { "type": "adaptive" }, "effort": "medium" } }, + { "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }, + { "id": "xhigh", "settings": { "thinking": { "type": "adaptive" }, "effort": "xhigh" } }, + { "id": "max", "settings": { "thinking": { "type": "adaptive" }, "effort": "max" } } + ], "limit": { "context": 1000000, "output": 128000 } } } @@ -45,7 +51,7 @@ "models": { "deepseek-flash": { "name": "DeepSeek-V4.1-Flash", - // Selected with REVIEW_MODEL_VARIANT=max (the workflow matrix); opencode sends "reasoning_effort": "max". + // Selected with REVIEW_MODEL_VARIANT=max; opencode sends "reasoning_effort": "max". "variants": [{ "id": "max", "settings": { "reasoningEffort": "max" } }], "limit": { "context": 1048576, "output": 393216 } } diff --git a/scripts/ai-review/README.md b/scripts/ai-review/README.md index 52c78f05ee..70766a60ab 100644 --- a/scripts/ai-review/README.md +++ b/scripts/ai-review/README.md @@ -1,7 +1,7 @@ # AI Code Review (`scripts/ai-review`) Automated PR review that posts a single consolidated comment on every pull request. Driven by -`.github/workflows/ai-code-review.yml` → `run-review.mjs`, using Claude Sonnet 5.5 (`anthropic-review/claude-sonnet-5-5`) at high effort with adaptive thinking via `opencode`. +`.github/workflows/ai-code-review.yml` → `run-review.mjs`, using Claude Sonnet 5.5 (`anthropic-review/claude-sonnet-5-5`) with adaptive thinking via `opencode`, at `high` effort unless the `AI_REVIEW_EFFORT` variable says otherwise (see Effort below). The posted comment is branded **OpenObserve Code Reviewer**. Branding is *cosmetic + identity only* — it does not change what the reviewer finds or decides. @@ -18,12 +18,12 @@ whose marker still names the provider, such as o2-enterprise's older one, keeps ## How it works (one paragraph) On each PR the workflow fetches the diff, filters noise, picks a risk tier (`trivial` / `lite` / -`full`), runs the tier's specialist agents (security, correctness, code-quality, performance, +`full` / `test-only` / `ci-only`), runs the tier's specialist agents (security, correctness, code-quality, performance, documentation) in parallel — plus any *conditional* agent whose domain the diff touches (frontend, qa, and release for migrations, `Cargo.toml`/`package.json`, `src/config/src/config.rs`, `.github/`, `deploy/` and Docker files) — and a coordinator consolidates their findings into one comment with a decision. Correctness owns the PR's behaviour end to end (callers, sibling -paths, state, the tests that prove the change); every tier runs it. Before the agents start, the +paths, state, the tests that prove the change); every tier but `ci-only` runs it. Before the agents start, the runner greps the PR tree for where each symbol the diff changes is used outside the diff, and for other definitions of the same name (`src/impact.mjs`), and gives that map to every reviewer. Security and correctness also get `openobserve-checklist.md`, the defect classes that recur in @@ -40,6 +40,50 @@ and folds findings at the same file and nearby line when one title's words conta reviewed when any of its passes finished, and the comment notes a lost pass. Each pass is billed, so the review costs about M times one pass; reviewer spans carry `review.pass`. +### Risk tiers + +`assessRiskTier` in `run-review.mjs` picks the tier. More than 50 files, or any path that looks +security-sensitive (`auth`, `token`, `secret`, `crypto`, `password`, `oauth`, `unsafe`, `acme`, +`cert`), is `full`. Otherwise a PR whose every file is test code (`tests/`, `test/`, `e2e/`, +`fixtures/`, `*.spec.*`, `*.test.*`, `*_test.rs`; `isTestOnlyPath` in `src/selection.mjs`) is +`test-only` and runs correctness and code-quality, whatever its size (qa joins when Playwright files change); a rename counts by both its source and +its destination, so code moved from `src/` into `tests/` is not test-only. A PR whose every file is CI +definition (`.github/workflows/**`, `.github/actions/**` or a top-level `.github/*.yml`; +`isCiOnlyPath`) is `ci-only` and runs security and release. Scripts, including this engine and the +two opencode configs, are code and take the size tiers below. Everything else is `trivial` (at most 10 +lines), `lite` (at most 100 lines) or `full`. Conditional agents are added on top as before, and +`force_full` still forces `full`. + +### Effort + +`AI_REVIEW_EFFORT` (`low`, `medium`, `high`, `xhigh` or `max`, default `high`) sets the reviewers' effort, and +`AI_REVIEW_COORDINATOR_EFFORT` the coordinator's (default: the reviewers' value). The workflow +passes them as `REVIEW_MODEL_VARIANT` and `REVIEW_COORDINATOR_VARIANT`, and the matrix carries no +variant of its own, so the variables are the only source. Each value is a variant of +`claude-sonnet-5-5` in both opencode configs, which opencode sends as adaptive thinking plus +`"output_config": { "effort": "" }`. A value that is not a declared variant stops the run +before it claims a pass or calls the model, and the comment's earlier verdict and `Auto-approved` +line give way to a 🔴 Low line saying the review did not run. Spans carry the effort in +`gen_ai.request.reasoning_effort`. + +### PRs that are never reviewed + +The workflow's `Skip PRs with nothing to review` step runs right after the engine checkout and +before anything is installed. It lists the PR's files (a rename lists its old path too) and the +author login, committer login and signature state of each commit, and runs `skip-check.mjs` (`nothingToReviewReason` in +`src/selection.mjs`). It skips nothing unless both listings match the PR's `changed_files` and +`commits` counts and stay under the endpoints' caps (3000 files, 250 commits; `listingComplete`), +since a listing cut short could hide a code file or a human commit. A PR is skipped, with nothing posted and no model called, when its author is +`dependabot[bot]` or `renovate[bot]` and every commit is authored by that same bot, committed by +it or by `web-flow` (how GitHub commits a bot's API-made commits) and carries a signature GitHub +verified, when every file is docs (`*.md` or `docs/**`, but not markdown under `scripts/ai-review/` or +`.github/`, nor any instruction loader the sanitizer strips, at any depth and in any case: `AGENTS.md`, `CLAUDE.md`, +`CONTEXT.md` or a file under `.claude/`, `.agents/` or `.opencode/`; `isLoaderPath` in `src/sanitize.mjs`), or when every file is one the +translation workflow commits (`web/src/locales/languages/*.json` other than `en-US.json`, which a +person writes, and `scripts/translations/.translation_state.json`). Every other bot or App author +is reviewed, and so is a dependency-bot PR someone else pushed to. A manual dispatch always +reviews, and so does a run where the file or commit listing, or `skip-check.mjs` itself, fails. + The coordinator also lists the findings it kept in a hidden JSON block, which the runner strips from the comment. Kept P0, P1 and P2 findings whose `file:line` is an added or context line of the diff are posted as one `COMMENT` review with inline threads (at most 10, never approve or request @@ -58,13 +102,34 @@ latest review is recorded, so a force-push back to an older reviewed head is rev finished review records what it covered in a hidden `o2-review-sha` tag (the head SHA, plus the paired head in a paired review, so a new commit on either side is new) and shows it in a visible `Reviewed commit` line with the review number. A run whose head matches the tag skips before any model call, which -covers a queued run, a re-label and a plain dispatch, including the paired dispatch from -openobserve. The tag is only written when it is true: the diff the reviewers read is rebuilt locally +covers a queued run, a re-label and the paired dispatch from openobserve (o2-enterprise does not +set `REVIEW_MANUAL_RUN`); a manual dispatch here, `force_full` or `ignore_pass_limit` reviews it again. The tag is only written when it is true: the diff the reviewers read is rebuilt locally from `MERGE_BASE_SHA` and `HEAD_SHA` (both sides of a paired review), because the API serves the branch as it is now. If that cannot be done, or no reviewer finished (each one failed or returned nothing parseable), the run writes no tag and names no commit, so that commit can be reviewed again. A run where no reviewer finished also reports `Not reviewed` and fails the check. +A stamped review that was complete (every reviewer pass finished without a failure, timeout or +wrap-up, and the coordinator merged them without falling back; `reviewComplete` in +`src/passes.mjs`) also records a fingerprint of the unfiltered diff, in a hidden +`o2-review-diff` tag (`diffFingerprint` in `src/pr-diff.mjs`: a SHA-256 of the diff with blob ids +and hunk line numbers removed but each hunk's function context kept, so the same lines changed in another function differ, while a rebase or a merge from main that leaves the PR's own changed +and context lines alone keeps it). Lockfiles, assets and generated files the reviewers never see are +in it, so adding one reviews again instead of carrying the old merge confidence over. Equal text proves +the same change only where it sits once in the base, so both the write and the skip also need every +hunk's old lines (context and removed lines) to occur exactly once in its base file (`hunksLocatable`); +an edit to one of two identical blocks, or a base file that cannot be read, reviews. When a new head's pinned diff has the same fingerprint, the run +skips with `unchanged_diff` before any model call: it uses no pass, keeps the review and its merge +confidence, and moves the `o2-review-sha` tag to the new head; the `Reviewed commit` line keeps the +commit the findings' line numbers refer to. Binary files keep their blob ids and trailing spaces count, so +both still change the fingerprint. The +check runs ahead of the review cap, so such a push is skipped, not refused, after the cap. A manual +dispatch (`REVIEW_MANUAL_RUN`), `force_full` or `ignore_pass_limit` always reviews, and so does a +diff that is not pinned to the head. In `approve` mode the skip still dismisses the earlier +approval, and the kept review's `Auto-approved` line becomes `Needs human review`, because a +skip does not rerun the approval gate's checks. A partial review records no fingerprint, so the next push +reviews again even when its diff is the same. + There is no cap on reviews per PR unless the variable `AI_REVIEW_MAX_PASSES` is a positive integer; unset, empty, `0` or an invalid value means no cap. Reviews are counted either way, in a hidden `o2-review-pass` tag written before the reviewers start, so a run stopped by removing the label or @@ -76,9 +141,9 @@ the list call fails) the run fails instead of reviewing, since neither the revie count can be checked. A push does not cancel a review in progress: the paid run finishes and only the newest push waits -in the concurrency queue. To review a commit again, dispatch the workflow with `ignore_pass_limit` -(it bypasses both the dedupe and the cap, and is forwarded to a paired o2-enterprise review) or with -`force_full` (it bypasses the dedupe only), or delete the review comment to reset the count. +in the concurrency queue. To review a commit again, dispatch the workflow (a plain dispatch or +`force_full` bypasses the dedupe only; `ignore_pass_limit` bypasses both the dedupe and the cap; the +two inputs are forwarded to a paired o2-enterprise review), or delete the review comment to reset the count. The comment marker is matched as a pattern, so a retired provider leg's comment is adopted and rewritten in place, including older comments whose marker was redacted to @@ -204,8 +269,8 @@ A PR must never be able to run code next to the review secrets or weaken its own | `src/opencode.mjs` env allowlist | `opencode serve` runs in `./pr` with only `PATH`, a fresh temp `HOME`/XDG dirs, `OPENCODE_CONFIG` (this repo's `opencode.jsonc` or `opencode.ci.jsonc` from `./engine`), the `OPENCODE_*` disable flags, a random server password and the one provider key. No `GH_*`, `GITHUB_*`, `ACTIONS_*`, `RUNNER_*` or `O2_*` variables. | | Config selection | `REVIEW_OPENCODE_CONFIG` = `default` (`opencode.jsonc`) or `ci-full` (`opencode.ci.jsonc`); any other value stops the run. The workflow sets `ci-full`, which keeps `approve` ineligible (see Auto-approve). | | `opencode.jsonc` permissions | Each `ai-review-*` agent: deny `*`, then allow `read`, `glob`, `grep`; `external_directory` denied. A `provider.use` policy allows only `anthropic-review`. LSP, formatters, snapshots and sharing are off. | -| `opencode.ci.jsonc` permissions | Same provider, model and agents, but each agent allows `*`: shell, edit, write and webfetch run with no prompt. The PR checkout is untrusted input to an agent that can run commands, and the provider key is in the opencode process environment, so a shell command can read it. | -| Startup self-check | Refuses to review if any agent is missing, any non-built-in plugin or MCP server loaded, or a granted tool's plugin is not active. Under `default` it also refuses any agent that resolves to allow a tool other than `read`/`glob`/`grep` or any outside directory; under `ci-full` it refuses an agent that does not allow every tool without asking. Permission prompts are rejected under `default` and approved once under `ci-full`. | +| `opencode.ci.jsonc` permissions | Same provider, model, variants and agents. Each agent allows `*` (read, glob, grep, edit, write run with no prompt), then denies `webfetch`, `websearch`, `subagent`, `question` (which would wait on an answer nobody in CI gives), `execute` (opencode 2.2.0's Code Mode: it runs JavaScript with `fetch`, so it would reach the network past the `webfetch` deny) and `skill` (reviewers read the skill copies with `read`). `shell` is kept to an allowlist: `git diff`, `git log`, `git show`, `git blame`, `git ls-files`, `git status` (each also after `git --no-pager`), `git rev-parse`, `git merge-base`, `git cat-file -p`/`-t`/`-s`, `rg`, `grep`, `cat`, `ls`, `head`, `tail`, `find`, `sed -n` and `wc`. Later deny rules refuse `find` with `-exec`, `-execdir`, `-ok`, `-okdir`, `-delete`, `-fprint`, `-fprint0`, `-fprintf` or `-fls` as a separate word; `tail -f`, `-F` and `--follow`; any absolute path, `../` path, `=/` argument or `environ` in `cat`, `head`, `tail`, `grep`, `rg`, `sed` and `wc` (so `/proc/*/environ` and `/dev/*` are not read through them); and any command containing `>` (an output redirect). `git grep` is not allowed (its `-O` runs a pager command). opencode checks every command in a pipeline, `&&` or `;` chain and inside `$(...)` or backticks, so the command runs only when every part is allowed, and a denied command fails outright, without a prompt. The allowlist stops builds, installs and long exploration. It is not a sandbox: `rg --pre` or a GNU `sed` `e` command can still run a program, a glob or a relative path inside the checkout is not inspected, and edit/write stay allowed in the disposable checkout. The PR checkout is untrusted input, and the provider key is in the opencode process environment, so a command that gets past these rules can read it. | +| Startup self-check | Refuses to review if any agent is missing, any non-built-in plugin or MCP server loaded, or a granted tool's plugin is not active. Under `default` it also refuses any agent that resolves to allow a tool other than `read`/`glob`/`grep` or any outside directory; under `ci-full` it refuses an agent that does not allow read, glob, grep, edit and write without asking, that leaves webfetch, websearch, subagent, question, execute or skill enabled, or whose shell is denied or not restricted to an allowlist. Permission prompts are rejected under `default` and approved once under `ci-full`. | | `src/comments.mjs` | The previous review is only ever a marker comment by the reviewer app (`REVIEW_BOT_LOGIN` = `[bot]`), read across every page; anyone else's comment carrying the marker is never fed to the reviewers or edited. When the app token cannot be minted the review posts as `github-actions[bot]`, which any same-repo PR workflow can also post as, so that run uses no previous review and posts a fresh comment. | | `src/scrub.mjs` | Before posting: redacts values of secret-named env vars and token-shaped strings (inside code too), breaks forged `"; @@ -6,6 +6,8 @@ export const REVIEW_PASS_TAG = ""; export const REVIEW_SHA_TAG = ""; // The verdict a later head's line replaced, kept in case the head returns to the reviewed commit. export const REVIEW_HELD_TAG = ""; +// Fingerprint of the diff the sha tag's review read, so a later head with the same changes (rebase, merge from main) needs no new review. +export const REVIEW_DIFF_TAG = ""; // Matched as a pattern so a retired leg's comment is still adopted; `[redacted]` is what redaction left in comments whose marker named the provider. export const ANY_REVIEW_MARKER = ""; // Tags a comment that stands in for a review (post-paired-pointer.sh, the "key not set" notice, the in-progress claim); it is replaced in place but is not a previous review. @@ -26,7 +28,7 @@ const AUTO_APPROVE_LINE_RE = /^(?:[^\w\n`]|<\/?[a-z]+>)*(?:auto[- ]?approved|nee const FORGED_LINE_RE = /mergeconfidence|autoapproved|needshumanreview|reviewedcommit/; // The fixed head of a finding line, glyph and location then category, which leaves no room for a status phrase ahead of the category. const FINDING_LINE_RE = /^\s*- (?:[^\w\s`*<]+\s+)?(?:`[^`\n]+`\s+)?\*\*\[[^\]\n]+\]\*\*/; -const TRAILING_TAGS_RE = /(?:\s*)+\s*$/i; +const TRAILING_TAGS_RE = /(?:\s*)+\s*$/i; const HTML_COMMENT_RE = /` : bare; +} + +// A forced run (dispatch, force_full, ignore_pass_limit) always reviews; otherwise a pinned diff equal to the reviewed one is skipped. +export function shouldSkipAsUnchangedDiff(body, fingerprint, { pinned = false, force = false } = {}) { + if (force || !pinned || !fingerprint) return false; + return reviewedDiffFingerprint(body) === fingerprint; +} + +// Costs no pass; the visible commit stays the reviewed one because finding line numbers refer to it, while the tag moves so the next push compares against this head. +export function bodyAtUnchangedDiff(body, key, maxPasses) { + return withReviewShaTag(bodyAtReviewedHead(body, maxPasses), key); +} + +export function shouldSkipAsReviewed(body, key, { ignorePassLimit = false, forceFull = false, manual = false } = {}) { + if (!key || ignorePassLimit || forceFull || manual) return false; return reviewedKey(body) === key; } @@ -103,8 +127,8 @@ export function passLimitReached(passes, maxPasses) { } // Dedupe comes first so a queued run on a reviewed head stays silent instead of rewriting the limit note. -export function reviewSkipReason({ body, key, passesDone, maxPasses, ignorePassLimit = false, forceFull = false }) { - if (shouldSkipAsReviewed(body, key, { ignorePassLimit, forceFull })) return "already_reviewed"; +export function reviewSkipReason({ body, key, passesDone, maxPasses, ignorePassLimit = false, forceFull = false, manual = false }) { + if (shouldSkipAsReviewed(body, key, { ignorePassLimit, forceFull, manual })) return "already_reviewed"; if (!ignorePassLimit && passLimitReached(passesDone, maxPasses)) return "pass_limit_reached"; return ""; } @@ -207,20 +231,25 @@ export function bodyAtPassLimit(body, maxPasses, head) { return bodyAtUnreviewedHead(withPassLimitNote(body, passLimitNote(maxPasses, reviewedKey(body), head)), head); } +// Even on the covered commit: the run already dismissed the approval, so the old verdict and gate line must not read as current. +export function bodyAtFailedRun(body, head) { + return head ? withConfidenceLine(withHeldVerdict(body), notRunConfidenceLine(shortKey(head))) : body; +} + // The sha tag says that exact commit was read and stops it being reviewed again, so a run that cannot prove that leaves it off. -export function finishReviewBody(body, { key, passes, maxPasses, reviewed, confidence, gateLine = "" }) { +export function finishReviewBody(body, { key, passes, maxPasses, reviewed, confidence, gateLine = "", diffFingerprint = "" }) { const stamped = reviewed && key; const lines = [confidence ? mergeConfidenceLine(confidence) : "", gateLine].filter(Boolean); let out = insertAfterCallout(body, lines.join("\n\n")); if (stamped) out += `\n\n${reviewedCommitLine(key, passes, maxPasses)}`; if (passLimitReached(passes, maxPasses)) out += `\n\n${passLimitNote(maxPasses)}`; - if (stamped) out = withReviewShaTag(out, key); + if (stamped) out = withReviewDiffTag(withReviewShaTag(out, key), diffFingerprint); return withPassTag(out, passes); } // What the reviewers see as : the review alone, none of the runner's bookkeeping. export function previousReviewText(body) { - return [REVIEW_SHA_TAG, REVIEW_PASS_TAG, REVIEW_HELD_TAG].reduce(withoutTag, stripRunnerLines(String(body ?? ""))); + return [REVIEW_SHA_TAG, REVIEW_PASS_TAG, REVIEW_HELD_TAG, REVIEW_DIFF_TAG].reduce(withoutTag, stripRunnerLines(String(body ?? ""))); } // Yields the text after each pass, last the first that changed nothing: a removal can splice a new match out of what surrounds it. @@ -303,7 +332,7 @@ function unwrapJsonBody(body) { export function sanitizeReviewBody(body, { marker, providerId, modelId, env }) { if (!body) return { text: "", secretHits: 0 }; const anyMarker = new RegExp(ANY_REVIEW_MARKER, "gi"); - const runnerTags = new RegExp(`${ANY_REVIEW_MARKER}|${REVIEW_PASS_TAG}|${REVIEW_SHA_TAG}|${REVIEW_HELD_TAG}`, "gi"); + const runnerTags = new RegExp(`${ANY_REVIEW_MARKER}|${REVIEW_PASS_TAG}|${REVIEW_SHA_TAG}|${REVIEW_HELD_TAG}|${REVIEW_DIFF_TAG}`, "gi"); let cleaned = unwrapJsonBody(body) .replace(/\\n/g, "\n") diff --git a/scripts/ai-review/src/confidence.mjs b/scripts/ai-review/src/confidence.mjs index 9345a11008..d9eed06825 100644 --- a/scripts/ai-review/src/confidence.mjs +++ b/scripts/ai-review/src/confidence.mjs @@ -25,8 +25,13 @@ export function reviewerFinished(r) { return !r.skipped && !r.error && r.parsed === true; } +// A reviewer interrupted at its soft deadline answered in haste, so its output counts as findings but not as a finished review. +export function reviewerCompleted(r) { + return reviewerFinished(r) && !r.wrappedUp; +} + export function unfinishedReviewers(ranResults) { - return ranResults.filter(r => !reviewerFinished(r)).map(r => r.agentName); + return ranResults.filter(r => !reviewerCompleted(r)).map(r => r.agentName); } function plural(count, noun) { @@ -78,6 +83,10 @@ export function staleConfidenceLine(shortHead) { return `**Merge confidence: ${CONFIDENCE_LABEL.low}** — \`${shortHead}\` has not been reviewed`; } +export function notRunConfidenceLine(shortHead) { + return `**Merge confidence: ${CONFIDENCE_LABEL.low}** — the review did not run because of a CI configuration error; \`${shortHead}\` has not been reviewed`; +} + export function reviewingConfidenceLine(shortHead) { return `**Merge confidence: 🟡 Reviewing** — \`${shortHead}\` is being reviewed`; } diff --git a/scripts/ai-review/src/jsonc.mjs b/scripts/ai-review/src/jsonc.mjs new file mode 100644 index 0000000000..3fd4bb70ad --- /dev/null +++ b/scripts/ai-review/src/jsonc.mjs @@ -0,0 +1,18 @@ +// Comments and trailing commas out, so the opencode configs parse as JSON. +export function stripJsonc(text) { + let out = ""; + for (let i = 0; i < text.length; i++) { + const c = text[i]; + if (c === '"') { + const start = i; + for (i++; i < text.length && text[i] !== '"'; i++) if (text[i] === "\\") i++; + out += text.slice(start, i + 1); + } else if (c === "/" && text[i + 1] === "/") { + while (i < text.length && text[i] !== "\n") i++; + out += "\n"; + } else { + out += c; + } + } + return out.replace(/,(\s*[}\]])/g, "$1"); +} diff --git a/scripts/ai-review/src/opencode.mjs b/scripts/ai-review/src/opencode.mjs index abe6174c79..6ecc0e7141 100644 --- a/scripts/ai-review/src/opencode.mjs +++ b/scripts/ai-review/src/opencode.mjs @@ -23,7 +23,11 @@ export const BUILTIN_TOOL_IDS = new Set([ const ALLOWED_TOOL_IDS = new Set(["read", "glob", "grep"]); const EDIT_TOOL_IDS = new Set(["edit", "write"]); // What ci-full must actually grant without asking; a config that fell back to the defaults fails here. -const CI_FULL_TOOL_IDS = ["read", "glob", "grep", "edit", "write", "shell", "webfetch"]; +const CI_FULL_TOOL_IDS = ["read", "glob", "grep", "edit", "write"]; +// question waits on an answer nobody in CI gives; execute runs JavaScript with fetch, so it would bypass the webfetch deny. +const CI_FULL_DENIED_TOOL_IDS = ["webfetch", "websearch", "subagent", "question", "execute", "skill"]; +// Offered, but only for the commands the config allowlists. +const CI_FULL_RESTRICTED_TOOL_IDS = ["shell"]; // 2.x dropped OPENCODE_PURE and the plugin, skill and LSP-download flags; sanitizePrTree covers what they did. const SANDBOX_FLAGS = { @@ -42,6 +46,7 @@ const INTERRUPTED_ERROR = { type: "interrupted", message: "opencode interrupted const DEFAULT_REQUEST_HEADERS_TIMEOUT_MS = 30_000; const STATUS_POLL_MS = 15_000; const STREAM_RETRY_MS = 1_000; +const INTERRUPT_SETTLE_MS = 20_000; const TRANSIENT_ERROR_RE = /ECONNRESET|ECONNREFUSED|ETIMEDOUT|EPIPE|socket hang up|overloaded|rate limit|too many requests|temporarily unavailable|bad gateway|service unavailable|gateway timeout/i; // opencode 2.x StructuredError types; the first set is worth a fresh session, the second never is, whatever the message says. const TRANSIENT_ERROR_TYPES = new Set(["provider.rate-limit", "provider.transport", "provider.timeout", "provider.internal"]); @@ -217,7 +222,7 @@ export class OpencodeClient { } // Sends the prompt (2.x admits it and returns at once) and waits on the event stream, so no request ever blocks for the length of an agent run (undici's 300 s headersTimeout killed every long review). - async prompt({ agent, model, variant, system, text, title, timeoutMs }) { + async prompt({ agent, model, variant, system, text, title, timeoutMs, wrapUp = null }) { await this.ensureEventStream(); // 2.x binds agent, model and variant to the session; the prompt itself carries only text. const session = (await this.json("POST", "/api/session", { @@ -233,17 +238,20 @@ export class OpencodeClient { if (system) { await this.json("PUT", `/api/experimental/session/${session.id}/instructions/entries/${SYSTEM_ENTRY_KEY}`, { value: system }); } + const softMs = wrapUpStartMs(timeoutMs, wrapUp); // Armed before the POST: the run can fail before the prompt call returns, and nothing would replay it. - const finished = waiter.wait(timeoutMs); + const finished = waiter.wait(softMs); await this.json("POST", `/api/session/${session.id}/prompt`, { text }); - const outcome = await finished; + let outcome = await finished; + let wrappedUp = false; + if (outcome.timedOut && softMs < timeoutMs) ({ outcome, wrappedUp } = await this.wrapUpSession(session.id, waiter, wrapUp.text, timeoutMs - softMs)); if (outcome.timedOut) { await this.request("POST", `/api/session/${session.id}/interrupt`).catch(() => {}); throw new OpencodeError(`opencode agent ${agent} timed out after ${timeoutMs}ms`, { name: "TimeoutError" }); } const messages = await this.json("GET", `/api/session/${session.id}/message?type=assistant&order=desc&limit=1`); totals = await this.json("GET", `/api/session/${session.id}`).catch(() => null); - return finalAssistantResult(messages?.data, totals?.data, outcome.error, waiter.rejectedPermissions); + return { ...finalAssistantResult(messages?.data, totals?.data, outcome.error, waiter.rejectedPermissions), wrappedUp }; } catch (err) { // A failed or timed-out session was still billed; its totals are gone once the session is deleted. totals ??= await this.json("GET", `/api/session/${session.id}`).catch(() => null); @@ -256,6 +264,23 @@ export class OpencodeClient { } } + // The interrupt keeps the finished steps in the session, so the follow-up prompt answers from everything the agent already read. + async wrapUpSession(sessionID, waiter, text, budgetMs) { + const deadline = Date.now() + budgetMs; + const stopped = waiter.wait(Math.min(INTERRUPT_SETTLE_MS, budgetMs)); + await this.request("POST", `/api/session/${sessionID}/interrupt`).catch(() => {}); + const stop = await stopped; + // An unconfirmed interrupt ends as the hard timeout; a run that ended on its own keeps its own outcome. + if (stop.timedOut || stop.error?.type !== "interrupted") return { outcome: stop, wrappedUp: false }; + // Nothing interrupts the follow-up before its wait ends, so a late interrupted event or idle record is the first run's. + waiter.ignoreInterrupted = true; + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return { outcome: { timedOut: true, error: null }, wrappedUp: true }; + const finished = waiter.wait(remainingMs); + await this.json("POST", `/api/session/${sessionID}/prompt`, { text }); + return { outcome: await finished, wrappedUp: true }; + } + close() { this.closed = true; this.stream?.close(); @@ -270,6 +295,7 @@ class SessionWaiter { this.sessionID = sessionID; this.rejectedPermissions = 0; this.settle = null; + this.ignoreInterrupted = false; } wait(timeoutMs) { @@ -291,6 +317,7 @@ class SessionWaiter { } onError(error) { + if (this.ignoreInterrupted && error?.type === "interrupted") return; this.settle?.({ timedOut: false, error: error || { type: "unknown", message: "opencode run failed" } }); } @@ -353,11 +380,13 @@ function permissionAction(toolId) { return EDIT_TOOL_IDS.has(toolId) ? "edit" : toolId; } +// Same as opencode 2.2.0: a trailing " *" also matches the bare command, so "git diff *" covers "git diff". export function wildcardMatch(input, pattern) { - const escaped = pattern + let escaped = pattern .replace(/[.+^${}()|[\]\\]/g, "\\$&") .replace(/\*/g, ".*") .replace(/\?/g, "."); + if (escaped.endsWith(" .*")) escaped = `${escaped.slice(0, -3)}( .*)?`; return new RegExp(`^${escaped}$`, "s").test(input); } @@ -375,7 +404,7 @@ export async function selfCheck(client, expectedAgents, { readOnly = true } = {} const broken = plugins.filter(p => p.state?.status !== "active"); if (broken.length > 0) throw new OpencodeError(`opencode plugins failed to load: ${broken.map(p => `${p.id}: ${p.state?.error || "unknown"}`).join("; ")}`); const active = new Set(plugins.map(p => p.id)); - const missing = (readOnly ? [...ALLOWED_TOOL_IDS] : CI_FULL_TOOL_IDS).filter(id => !active.has(`opencode.tool.${id}`)); + const missing = (readOnly ? [...ALLOWED_TOOL_IDS] : [...CI_FULL_TOOL_IDS, ...CI_FULL_RESTRICTED_TOOL_IDS]).filter(id => !active.has(`opencode.tool.${id}`)); if (missing.length > 0) throw new OpencodeError(`opencode tools did not load: ${missing.join(", ")}`); const mcp = (await client.json("GET", "/api/mcp"))?.data || []; if (mcp.length > 0) throw new OpencodeError(`opencode loaded MCP servers; refusing to review`); @@ -397,9 +426,7 @@ export async function selfCheck(client, expectedAgents, { readOnly = true } = {} const problem = externalDirectoryProblem(ruleset); if (problem) problems.push(`${name}: ${problem}`); } else { - for (const toolId of CI_FULL_TOOL_IDS) { - if (!toolAllowedWithoutAsking(toolId, ruleset)) problems.push(`${name}: ${toolId} not fully allowed`); - } + problems.push(...ciFullProblems(ruleset).map(problem => `${name}: ${problem}`)); } } if (problems.length > 0) throw new OpencodeError(`opencode permission self-check failed: ${problems.join("; ")}`); @@ -415,6 +442,28 @@ export function externalDirectoryProblem(ruleset) { return extra.length > 0 ? `external_directory allows ${extra.map(r => r.resource).join(", ")}` : ""; } +// ci-full grants the file tools outright, denies the tools that only cost time, and keeps the shell on an allowlist. +export function ciFullProblems(ruleset) { + const problems = CI_FULL_TOOL_IDS.filter(id => !toolAllowedWithoutAsking(id, ruleset)).map(id => `${id} not fully allowed`); + problems.push(...CI_FULL_DENIED_TOOL_IDS.filter(id => toolEnabled(id, ruleset)).map(id => `${id} allowed`)); + for (const id of CI_FULL_RESTRICTED_TOOL_IDS) { + if (!toolEnabled(id, ruleset)) problems.push(`${id} denied`); + else if (toolAllowedWithoutAsking(id, ruleset)) problems.push(`${id} allows every command`); + } + return problems; +} + +// The follow-up only gets a slot when the first run keeps at least as long as the follow-up; shorter sessions run to the hard timeout. +export function wrapUpStartMs(timeoutMs, wrapUp) { + return wrapUp?.ms > 0 && wrapUp.text && timeoutMs >= 2 * wrapUp.ms ? timeoutMs - wrapUp.ms : timeoutMs; +} + +// The variant ids a model declares in an opencode config; a variant opencode does not know must fail the run before any session starts. +export function modelVariantIds(config, providerId, modelId) { + const variants = config?.providers?.[providerId]?.models?.[modelId]?.variants; + return Array.isArray(variants) ? variants.map(v => v?.id).filter(Boolean) : []; +} + // A retry must leave room for its own full timeout plus whatever still has to run after it (the coordinator). export function shouldRetry({ err, attempt, maxRetries, remainingMs, timeoutMs, reserveMs = 0 }) { return attempt <= maxRetries && isTransientError(err) && remainingMs > timeoutMs + reserveMs; diff --git a/scripts/ai-review/src/passes.mjs b/scripts/ai-review/src/passes.mjs index a262e13886..c0048a7039 100644 --- a/scripts/ai-review/src/passes.mjs +++ b/scripts/ai-review/src/passes.mjs @@ -1,4 +1,4 @@ -import { reviewerFinished, shortName } from "./confidence.mjs"; +import { reviewerCompleted, reviewerFinished, shortName } from "./confidence.mjs"; export const DEFAULT_COLD_PASSES = 2; export const MAX_COLD_PASSES = 3; @@ -43,8 +43,9 @@ export async function runPooled(items, limit, worker) { function mergeArea(group) { const finished = group.filter(reviewerFinished); + const completed = finished.filter(reviewerCompleted); const ran = group.filter(r => !r.skipped); - const base = finished[0] ?? ran[0] ?? group[0]; + const base = completed[0] ?? finished[0] ?? ran[0] ?? group[0]; const covered = finished.length > 0; return { agentKey: base.agentKey, @@ -58,6 +59,8 @@ function mergeArea(group) { skipped: group.every(r => r.skipped), fullDiff: base.fullDiff, parsed: covered, + // Only an uninterrupted pass completes an area; wrapped-up output still counts as findings. + wrappedUp: covered && completed.length === 0, passes: group.length, passesFinished: finished.length, passResults: group, @@ -78,6 +81,12 @@ export function lostPasses(results) { return results.filter(r => !r.skipped && r.passesFinished > 0 && r.passesFinished < r.passes); } +// Only a review with every pass finished in time and a real coordinator merge may let the next push with the same diff skip. +export function reviewComplete(passResults, coordinatorResult) { + if (!coordinatorResult || coordinatorResult.fallback) return false; + return passResults.every(r => r.skipped || reviewerCompleted(r)); +} + // Written by code like the failed-reviewers line, so a reader learns an area got less scrutiny than the rest. export function lostPassesNote(results) { const lost = lostPasses(results); diff --git a/scripts/ai-review/src/pr-diff.mjs b/scripts/ai-review/src/pr-diff.mjs index 0563222bf3..0359400737 100644 --- a/scripts/ai-review/src/pr-diff.mjs +++ b/scripts/ai-review/src/pr-diff.mjs @@ -1,4 +1,5 @@ import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; const MAX_BUFFER = 200 * 1024 * 1024; const TOO_LARGE_RE = /exceeded the maximum number of lines|too_large|HTTP 406|HTTP 422/i; @@ -55,6 +56,39 @@ export function fetchPinnedDiff(params) { return pinDiff({ ...params, diff: fetchDiff(params) }); } +// Blob ids and hunk line numbers move with the base, so a rebase keeps the fingerprint; the function context after `@@` stays, as the same lines in another function are another change. +export function diffFingerprint(diff) { + const text = String(diff ?? ""); + // A binary file has no hunks, so its blob ids are the only record of its content. + const keepBlobIds = /^(Binary files .* differ|GIT binary patch)$/m.test(text); + const normalized = text + .split("\n") + .map(line => (keepBlobIds ? line : line.replace(/^index [0-9a-f]+\.\.[0-9a-f]+/, "index"))) + .map(line => line.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/, "@@")) + .join("\n") + .replace(/\n+$/, ""); + return createHash("sha256").update(normalized).digest("hex"); +} + +// Equal text names one change only where each hunk's old lines sit once in the base, so an edit to one of two identical blocks cannot pass for the other. +export function hunksLocatable(diff, readBase) { + for (const section of String(diff ?? "").split(/^diff --git /m).slice(1)) { + const firstHunk = section.search(/^@@ /m); + if (firstHunk === -1) continue; + const header = section.slice(0, firstHunk); + if (/^--- \/dev\/null$/m.test(header)) continue; + const path = header.match(/^--- a\/(.*?)\t?$/m)?.[1]; + const base = path ? readBase(path) : null; + if (typeof base !== "string") return false; + const baseLines = base.split("\n"); + for (const hunk of section.slice(firstHunk).split(/^@@ [^\n]*(?:\n|$)/m).slice(1)) { + const old = hunk.split("\n").filter(line => line.startsWith(" ") || line.startsWith("-")).map(line => line.slice(1)); + if (old.length === 0 ? base !== "" : occurrences(baseLines, old) !== 1) return false; + } + } + return true; +} + // `unfocused` keeps the whole diff for a gate candidate, whose approval must rest on reviewers that saw every file; requiresFocus agents still opt out. export function reviewerDiff(diff, { fileFocus, requiresFocus }, { unfocused = false, pathOf = path => path } = {}) { if (!fileFocus) return { diff, skipped: false, fullDiff: true }; @@ -107,3 +141,12 @@ function prefixGitHeader(line, p) { function prefixBinaryLine(line, p) { return line.replace(/(^Binary files |and )("?)([ab])\//g, `$1$2$3/${p}/`); } + +// Stops at two: the caller only needs to know whether a block is unique. +function occurrences(lines, block) { + let found = 0; + for (let i = 0; i + block.length <= lines.length && found < 2; i += 1) { + if (lines[i] === block[0] && block.every((line, j) => lines[i + j] === line)) found += 1; + } + return found; +} diff --git a/scripts/ai-review/src/sanitize.mjs b/scripts/ai-review/src/sanitize.mjs index fc9ad5666f..4357a57629 100644 --- a/scripts/ai-review/src/sanitize.mjs +++ b/scripts/ai-review/src/sanitize.mjs @@ -38,6 +38,12 @@ export function sanitizePrTree(prDir, contextSourceDir) { return { removed, contextFiles: copied }; } +// The same names the walk strips, so a PR touching only these still steers a reviewer and is never docs-only. +export function isLoaderPath(filePath) { + const parts = String(filePath ?? "").toLowerCase().split("/"); + return LOADER_FILES.has(parts.at(-1)) || parts.some(part => LOADER_DIRS.has(part)); +} + function walk(dir, root, removed) { for (const entry of readdirSync(dir, { withFileTypes: true })) { const path = join(dir, entry.name); diff --git a/scripts/ai-review/src/selection.mjs b/scripts/ai-review/src/selection.mjs index 1ddd11c8c5..9bc977444a 100644 --- a/scripts/ai-review/src/selection.mjs +++ b/scripts/ai-review/src/selection.mjs @@ -1,5 +1,12 @@ import { existsSync } from "node:fs"; import { basename, dirname, join } from "node:path"; +import { isLoaderPath } from "./sanitize.mjs"; + +// Dependency bots whose PRs only bump versions; any other bot or App can write code and is reviewed. +const DEPENDENCY_BOTS = new Set(["dependabot[bot]", "renovate[bot]"]); +// GitHub's files and commits endpoints stop at these counts however far they are paged. +const MAX_LISTED_FILES = 3000; +const MAX_LISTED_COMMITS = 250; // Diffs that can affect a release: migrations, dependency manifests, server config, CI and deployment files. export function isReleaseFile(filePath) { @@ -22,3 +29,57 @@ export function resolveRulesFiles(files, exists = existsSync) { } return out; } + +// Test code only: test trees, spec/test files and fixtures, in any language. +export function isTestOnlyPath(filePath) { + if (isCiOnlyPath(filePath)) return false; + if (/(^|\/)(tests?|__tests__|e2e|fixtures?|testdata)\//.test(filePath)) return true; + if (/\.(spec|test)\.[cm]?[jt]sx?$/.test(filePath)) return true; + return /(^|\/)[^/]+_test\.(rs|go|py)$/.test(filePath) || /(^|\/)test_[^/]+\.py$/.test(filePath); +} + +// Workflows, actions and .github config only; scripts, the review engine and its opencode configs are code and take the normal tiers. +export function isCiOnlyPath(filePath) { + return /^\.github\/((workflows|actions)\/|[^/]+\.ya?ml$)/.test(filePath); +} + +export function narrowTier(paths) { + if (paths.length === 0) return ""; + if (paths.every(isTestOnlyPath)) return "test-only"; + if (paths.every(isCiOnlyPath)) return "ci-only"; + return ""; +} + +// Prompt, agent-rule and workflow markdown steers the AI or CI, so it is reviewed like code. +export function isDocsPath(filePath) { + if (/^(scripts\/ai-review\/|\.github\/)/.test(filePath) || isLoaderPath(filePath)) return false; + return /\.md$/i.test(filePath) || /^docs\//.test(filePath); +} + +// What update-translations.yml commits; en-US.json is the English source a person writes, so it is still reviewed. +export function isTranslationPath(filePath) { + if (filePath === "scripts/translations/.translation_state.json") return true; + return /^web\/src\/locales\/languages\/[^/]+\.json$/.test(filePath) && !filePath.endsWith("/en-US.json"); +} + +// A listing is whole only when it matches the PR's own counts below each endpoint's cap; anything else may hide a file or commit. +export function listingComplete({ files, commits, expectedFiles, expectedCommits }) { + const count = value => (/^\d+$/.test(String(value ?? "").trim()) ? Number(value) : Number.NaN); + const wantFiles = count(expectedFiles); + const wantCommits = count(expectedCommits); + return files === wantFiles && commits === wantCommits && wantFiles < MAX_LISTED_FILES && wantCommits < MAX_LISTED_COMMITS; +} + +// GitHub commits a bot's API-made commits as web-flow, so the committer is either the bot or web-flow, and GitHub must have verified the signature. +export function botCommit(commit, bot) { + return commit?.author === bot && (commit.committer === bot || commit.committer === "web-flow") && commit.verified === true; +} + +// Why a PR needs no review at all, or "" when it does; a PR with no listed files is reviewed so a failed listing never skips one. +export function nothingToReviewReason({ author = "", commits = [], paths = [] }) { + if (DEPENDENCY_BOTS.has(author) && commits.length > 0 && commits.every(c => botCommit(c, author))) return "bot_author"; + if (paths.length === 0) return ""; + if (paths.every(isDocsPath)) return "docs_only"; + if (paths.every(isTranslationPath)) return "translation_only"; + return ""; +} diff --git a/scripts/ai-review/test/comments.test.mjs b/scripts/ai-review/test/comments.test.mjs index 3b5e3bd080..0da4a7e729 100644 --- a/scripts/ai-review/test/comments.test.mjs +++ b/scripts/ai-review/test/comments.test.mjs @@ -1,12 +1,14 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; import { ANY_REVIEW_MARKER, REVIEW_HEADING, bodyAtPassLimit, bodyAtReviewedHead, + bodyAtUnchangedDiff, bodyAtUnreviewedHead, claimedReviewBody, finishReviewBody, @@ -20,16 +22,18 @@ import { reviewShaTag, reviewSkipReason, reviewedCommitLine, + reviewedDiffFingerprint, reviewedKey, sanitizeReviewBody, selectOwnReviewComment, shouldSkipAsReviewed, + shouldSkipAsUnchangedDiff, withPassLimitNote, withPassTag, withReviewShaTag, withoutAutoApproveLine, } from "../src/comments.mjs"; -import { autoApproveLine } from "../src/auto-approve.mjs"; +import { autoApproveLine, withoutCarriedApproval } from "../src/auto-approve.mjs"; import { mergeConfidence } from "../src/confidence.mjs"; const MARKER = ANY_REVIEW_MARKER; @@ -637,3 +641,75 @@ test("a reviewed head drops a limit note once the cap no longer applies", () => } assert.equal(bodyAtReviewedHead(body, 2), body); }); + +const FP_A = "a".repeat(64); +const FP_B = "b".repeat(64); +const HEAD_1 = "1".repeat(40); +const HEAD_2 = "2".repeat(40); +const stampedReview = (fingerprint = FP_A, extra = {}) => finishReviewBody("## 🔎 OpenObserve Code Review\n\n> [!TIP]\n> ### ✅ Approved", { + key: HEAD_1, passes: 1, maxPasses: 0, reviewed: true, confidence: { level: "high", reasons: ["no blockers"] }, diffFingerprint: fingerprint, ...extra, +}); + +test("a stamped review carries the fingerprint of the diff it read; an unstamped one vouches for none", () => { + assert.equal(reviewedDiffFingerprint(stampedReview()), FP_A); + assert.equal(reviewedDiffFingerprint(stampedReview(FP_A, { reviewed: false })), ""); + assert.equal(reviewedDiffFingerprint(stampedReview("")), ""); + assert.equal(reviewedDiffFingerprint(`plain text `), "", "without a sha tag no head was reviewed"); +}); + +test("an unchanged diff is skipped unless the run is forced or the diff is not pinned to the head", () => { + const body = stampedReview(); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: true }), true); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_B, { pinned: true }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: false }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: true, force: true }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, "", { pinned: true }), false); +}); + +test("an unchanged-diff skip in approve mode keeps the review but not the auto-approval it can no longer back", () => { + const approved = autoApproveLine({ mode: "approve", approved: true, reasons: [] }); + const body = stampedReview(FP_A, { gateLine: approved }); + assert.ok(body.includes(approved)); + const next = withoutCarriedApproval(bodyAtUnchangedDiff(body, HEAD_2, 0)); + assert.equal(next.includes("Auto-approved"), false); + assert.match(next, /Needs human review/); + assert.equal(reviewedKey(next), HEAD_2); + assert.equal(reviewedDiffFingerprint(next), FP_A); + assert.equal(withoutCarriedApproval(stampedReview()), stampedReview(), "a body with no approval line is unchanged"); + const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); + assert.match(source, /AUTO_APPROVE_MODE === "approve" \? withoutCarriedApproval\(kept\) : kept/); +}); + +test("a partial review leaves no fingerprint, so the next push with the same diff is reviewed again", () => { + const partial = stampedReview(""); + assert.equal(shouldSkipAsUnchangedDiff(partial, FP_A, { pinned: true }), false); + const overOld = finishReviewBody(`${stampedReview()}`, { key: HEAD_2, passes: 2, maxPasses: 0, reviewed: true, diffFingerprint: "" }); + assert.equal(reviewedDiffFingerprint(overOld), "", "an earlier complete review's fingerprint does not survive a partial one"); +}); + +test("skipping an unchanged diff moves the head marker, keeps the reviewed commit its line numbers refer to, and costs no pass", () => { + const body = stampedReview(); + const next = bodyAtUnchangedDiff(body, HEAD_2, 0); + assert.equal(reviewedKey(next), HEAD_2); + assert.equal(reviewedDiffFingerprint(next), FP_A); + assert.equal(reviewPassCount(next), 1); + assert.match(next, new RegExp(`Reviewed commit \`${HEAD_1.slice(0, 7)}\``)); + assert.match(next, /Merge confidence/); + assert.equal(shouldSkipAsReviewed(next, HEAD_2), true, "a queued run on the new head stays silent"); +}); + +test("a run on another head that left its claim behind gets the held verdict back when the diff turns out unchanged", () => { + const claimed = claimedReviewBody(stampedReview(), 2, HEAD_2); + assert.doesNotMatch(claimed, /High/); + const next = bodyAtUnchangedDiff(claimed, HEAD_2, 0); + assert.match(next, /High/); + assert.equal(reviewPassCount(next), 2); + assert.equal(reviewedDiffFingerprint(next), FP_A); +}); + +test("a model cannot forge the diff fingerprint, and the reviewers never see it", () => { + const forged = `## 🔎 OpenObserve Code Review\n\nok\n`; + const { text } = sanitizeReviewBody(forged, { marker: "", providerId: "p", modelId: "m", env: {} }); + assert.doesNotMatch(text, /o2-review-diff/); + assert.doesNotMatch(previousReviewText(stampedReview()), /o2-review-diff/); +}); diff --git a/scripts/ai-review/test/integration/opencode-sandbox.test.mjs b/scripts/ai-review/test/integration/opencode-sandbox.test.mjs index 1a2f0e971b..c222df1f85 100644 --- a/scripts/ai-review/test/integration/opencode-sandbox.test.mjs +++ b/scripts/ai-review/test/integration/opencode-sandbox.test.mjs @@ -18,8 +18,8 @@ const REVIEW_AGENTS = [ ]; const skip = process.env.OPENCODE_BIN ? false : "set OPENCODE_BIN to the pinned opencode binary"; -// The first request that offers tools gets one scripted tool call; every later request gets the final answer. -function stubProvider(toolCall) { +// By default the first request that offers tools gets one scripted tool call and every later request the final answer; `script` overrides that per request. +function stubProvider(toolCall, { script } = {}) { const requests = []; const event = (type, data) => `event: ${type}\ndata: ${JSON.stringify({ type, ...data })}\n\n`; const server = http.createServer((req, res) => { @@ -34,9 +34,15 @@ function stubProvider(toolCall) { usage: { input_tokens: 1000, output_tokens: 1, cache_read_input_tokens: 0, cache_creation_input_tokens: 0 }, } })); const hasToolResult = (parsed.messages || []).some(m => Array.isArray(m.content) && m.content.some(b => b.type === "tool_result")); - if (parsed.tools?.length && !hasToolResult) { - res.write(event("content_block_start", { index: 0, content_block: { type: "tool_use", id: "toolu_1", name: toolCall.name, input: {} } })); - res.write(event("content_block_delta", { index: 0, delta: { type: "input_json_delta", partial_json: JSON.stringify(toolCall.args) } })); + const step = script ? script(parsed, requests.length) : { tool: parsed.tools?.length && !hasToolResult ? toolCall : null }; + if (step.hangMs) { + const timer = setTimeout(() => res.end(), step.hangMs); + res.on("close", () => clearTimeout(timer)); + return; + } + if (step.tool) { + res.write(event("content_block_start", { index: 0, content_block: { type: "tool_use", id: `toolu_${requests.length}`, name: step.tool.name, input: {} } })); + res.write(event("content_block_delta", { index: 0, delta: { type: "input_json_delta", partial_json: JSON.stringify(step.tool.args) } })); res.write(event("content_block_stop", { index: 0 })); res.write(event("message_delta", { delta: { stop_reason: "tool_use" }, usage: { output_tokens: 100 } })); } else { @@ -102,9 +108,10 @@ async function withServer(prDir, configPath, run, { readOnly = true } = {}) { } } -function stubConfig(file, stub, configDir) { +function stubConfig(file, stub, configDir, edit = () => {}) { const config = JSON.parse(stripJsonc(readFileSync(join(ENGINE_DIR, file), "utf-8"))); config.providers["anthropic-review"].settings = { baseURL: stub.url }; + edit(config); const configPath = join(configDir, "opencode.json"); writeFileSync(configPath, JSON.stringify(config)); return configPath; @@ -189,12 +196,17 @@ test("without sanitize, the env flags still keep project config, tools and agent assert.deepEqual(toolNames, ["glob", "grep", "read", "skill"]); }); -test("ci-full: every tool is offered and runs without a prompt, and the shell sees no GitHub token", { skip, timeout: 120_000 }, async t => { +const allowShell = (...commands) => config => { + config.agents["ai-review-correctness"].permissions.push(...commands.map(resource => ({ action: "shell", resource, effect: "allow" }))); +}; + +test("ci-full: file tools and the shell are offered without a prompt, and the shell sees no GitHub token", { skip, timeout: 120_000 }, async t => { const pr = plantedPr(); const stub = await stubProvider({ name: "shell", args: { command: "echo ran-$((6*7)); env", description: "probe" } }); const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); cleanup(t, stub, pr, configDir); - const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + // The allowlist refuses echo and env, so this probe of the shell's environment adds them for the test alone. + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, allowShell("echo *", "env *")); sanitizePrTree(pr.root, ENGINE_DIR); await withServer(pr.root, configPath, async client => { @@ -206,8 +218,128 @@ test("ci-full: every tool is offered and runs without a prompt, and the shell se }, { readOnly: false }); const toolNames = (stub.requests[0].body.tools || []).map(tool => tool.name); - for (const name of ["read", "glob", "grep", "edit", "write", "shell", "webfetch"]) assert.ok(toolNames.includes(name), name); + for (const name of ["read", "glob", "grep", "edit", "write", "shell"]) assert.ok(toolNames.includes(name), name); + for (const name of ["webfetch", "websearch", "subagent", "question", "execute"]) assert.equal(toolNames.includes(name), false, `${name} is offered`); const content = toolResultOf(stub.requests.at(-1)); assert.match(content, /ran-42/); assert.doesNotMatch(content, /ghs_must_not_leak/); }); + +test("the real self-check passes the shipped ci-full config and refuses it with execute or the whole shell re-allowed", { skip, timeout: 120_000 }, async t => { + for (const [extra, problem] of [[null, null], [{ action: "execute", resource: "*", effect: "allow" }, /ai-review-qa: execute allowed/], [{ action: "shell", resource: "*", effect: "allow" }, /ai-review-qa: shell allows every command/]]) { + const pr = plantedPr(); + const stub = await stubProvider(null); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, config => { if (extra) config.agents["ai-review-qa"].permissions.push(extra); }); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const check = selfCheck(client, REVIEW_AGENTS, { readOnly: false }); + if (problem) await assert.rejects(check, problem); + else assert.equal((await check).agents, REVIEW_AGENTS.length); + }, { readOnly: false }); + } +}); + +test("ci-full offers skill only because opencode appends a narrower rule, and refuses every skill", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + const stub = await stubProvider({ name: "skill", args: { id: "browser" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "skill" }); + assert.equal(result.rejectedPermissions, 0); + }, { readOnly: false }); + assert.match(toolResultOf(stub.requests.at(-1)), /Permission denied: skill/); +}); + +test("ci-full shell runs an allowlisted command and refuses a build outright, without asking", { skip, timeout: 120_000 }, async t => { + // A RegExp is the output an allowed command must print; null means opencode must refuse it. + const ONE = /\b1\b/; + const cases = [ + ["cat src/main.rs | wc -l", ONE], ["grep -c main src/main.rs", ONE], ["find src -name '*.rs' | wc -l", ONE], ["tail -n 1 src/main.rs | wc -l", ONE], + ["ls src && wc -l src/main.rs", ONE], ["find src -name '*-executor*' | wc -l", /\b0\b/], ["cargo build", null], ["ls && touch built.txt", null], + ["find . -name main.rs -exec touch built.txt ;", null], ["find src -fprint built.txt", null], ["git grep -nOsh main", null], + ["cat $(touch built.txt)", null], ["cat `touch built.txt`", null], ["cat src/main.rs > built.txt", null], ["cat /proc/self/environ", null], + ]; + for (const [command, output] of cases) { + const pr = plantedPr(); + const stub = await stubProvider({ name: "shell", args: { command, description: "probe" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "shell" }); + assert.equal(result.rejectedPermissions, 0, "a denied command must not reach a prompt that ci-full would approve"); + }, { readOnly: false }); + const content = toolResultOf(stub.requests.at(-1)); + assert.match(content, output ?? /Permission denied: shell/, command); + assert.equal(existsSync(join(pr.root, "built.txt")), false, command); + } +}); + +// Why neither config sets `steps`: if opencode ever stops ending the capped turn on an assistant message, this fails and the cap can come back. +test("opencode 2.2.0's step cap ends the capped turn on an assistant prefill, which the model rejects with thinking on", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + // Asks for another tool call on every turn that allows one, so only the cap can end the loop. + const stub = await stubProvider(null, { script: parsed => ({ tool: parsed.tool_choice?.type === "none" ? null : { name: "glob", args: { pattern: "*.rs" } } }) }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, config => { config.agents["ai-review-correctness"].steps = 3; }); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, client => client.prompt({ ...PROMPT, title: "steps" }), { readOnly: false }); + assert.equal(stub.requests.length, 3); + const capped = stub.requests.at(-1).body; + assert.deepEqual(capped.tool_choice, { type: "none" }); + assert.equal(capped.messages.at(-1).role, "assistant"); + assert.match(JSON.stringify(capped.messages.at(-1).content), /MAXIMUM STEPS REACHED/); + for (const request of stub.requests.slice(0, -1)) assert.equal(request.body.messages.at(-1).role, "user"); +}); + +test("each effort variant reaches the provider as its own effort level", { skip, timeout: 120_000 }, async t => { + for (const variant of ["low", "medium", "xhigh", "max"]) { + const pr = plantedPr(); + const stub = await stubProvider({ name: "glob", args: { pattern: "*.rs" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, client => client.prompt({ ...PROMPT, variant, title: variant }), { readOnly: false }); + for (const request of stub.requests) { + assert.deepEqual(request.body.output_config, { effort: variant }); + assert.equal(request.body.thinking?.type, "adaptive"); + for (const field of ["temperature", "top_p", "top_k", "tool_choice"]) assert.equal(request.body[field], undefined, field); + } + } +}); + +test("a session still running at its soft deadline is interrupted and answers the wrap-up prompt from the same context", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + const wrapText = "WRAP-UP-NOW-5d1e"; + // A tool step, then a model turn that never returns; only the wrap-up prompt gets an answer. + const stub = await stubProvider(null, { + script: (parsed, n) => { + if (parsed.messages?.some(m => JSON.stringify(m.content).includes(wrapText))) return { tool: null }; + return n === 1 ? { tool: { name: "glob", args: { pattern: "*.rs" } } } : { hangMs: 60_000 }; + }, + }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "wrap-up", timeoutMs: 20_000, wrapUp: { ms: 10_000, text: wrapText } }); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + }, { readOnly: false }); + const last = stub.requests.at(-1); + assert.match(last.raw, new RegExp(wrapText)); + assert.ok(last.body.messages.some(m => Array.isArray(m.content) && m.content.some(b => b.type === "tool_result")), "the wrap-up lost the steps the agent had already run"); + for (const request of stub.requests) { + for (const field of ["temperature", "top_p", "top_k", "tool_choice"]) assert.equal(request.body[field], undefined, field); + assert.equal(request.body.messages.at(-1).role, "user", "the wrap-up must not end on an assistant prefill"); + } +}); diff --git a/scripts/ai-review/test/jsonc.mjs b/scripts/ai-review/test/jsonc.mjs index 670c1bbac1..9adbc3e55a 100644 --- a/scripts/ai-review/test/jsonc.mjs +++ b/scripts/ai-review/test/jsonc.mjs @@ -1,18 +1 @@ -// Comments and trailing commas out, so the opencode configs parse as JSON in tests. -export function stripJsonc(text) { - let out = ""; - for (let i = 0; i < text.length; i++) { - const c = text[i]; - if (c === '"') { - const start = i; - for (i++; i < text.length && text[i] !== '"'; i++) if (text[i] === "\\") i++; - out += text.slice(start, i + 1); - } else if (c === "/" && text[i + 1] === "/") { - while (i < text.length && text[i] !== "\n") i++; - out += "\n"; - } else { - out += c; - } - } - return out.replace(/,(\s*[}\]])/g, "$1"); -} +export { stripJsonc } from "../src/jsonc.mjs"; diff --git a/scripts/ai-review/test/opencode.test.mjs b/scripts/ai-review/test/opencode.test.mjs index 6e48eb3d32..cd1bdafeb1 100644 --- a/scripts/ai-review/test/opencode.test.mjs +++ b/scripts/ai-review/test/opencode.test.mjs @@ -11,15 +11,19 @@ import { OPENCODE_CONFIGS, OpencodeClient, buildServerEnv, + ciFullProblems, createSseParser, externalDirectoryProblem, httpRequest, isTransientError, + modelVariantIds, resolveOpencodeConfig, selfCheck, shouldRetry, toolAllowedWithoutAsking, toolEnabled, + wildcardMatch, + wrapUpStartMs, } from "../src/opencode.mjs"; import { stripJsonc } from "./jsonc.mjs"; @@ -28,15 +32,16 @@ const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "../../.."); const AGENT_RUN_MS = 1500; const HEADERS_TIMEOUT_MS = 1000; -function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execution.succeeded", error = null, errorBeforeAck = null, eventStream = true } = {}) { +function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execution.succeeded", error = null, errorBeforeAck = null, eventStream = true, wrapUpAfterMs = null, interruptEventDelayMs = 50 } = {}) { const calls = []; + let prompts = 0; const streams = new Set(); let idle = null; const emit = (type, data) => { for (const res of streams) res.write(`data: ${JSON.stringify({ id: "evt", type, data })}\n\n`); }; const finishRun = () => { - idle = { type: "idle", outcome: finalEvent === "session.execution.succeeded" ? "succeeded" : "failed", ...(error ? { error } : {}) }; + idle = { id: `idle_${prompts}`, type: "idle", outcome: finalEvent === "session.execution.succeeded" ? "succeeded" : "failed", ...(error ? { error } : {}) }; if (eventStream) emit(finalEvent, { sessionID: "ses_1", ...(error ? { error } : {}) }); }; const server = http.createServer((req, res) => { @@ -73,7 +78,13 @@ function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execut setTimeout(() => send(200, { data: { id: "msg_u1" } }), 200); return; } - send(200, { data: { id: "msg_u1" } }); + prompts += 1; + send(200, { data: { id: `msg_u${prompts}` } }); + // The second prompt is the wrap-up: it answers, where the first ran on until interrupted. + if (wrapUpAfterMs !== null && prompts > 1) { + setTimeout(finishRun, wrapUpAfterMs); + return; + } setTimeout(() => emit("permission.asked", { id: "per_1", sessionID: "ses_1", action: "shell", resources: ["*"] }), 50); if (idleAfterMs !== null) setTimeout(finishRun, idleAfterMs); return; @@ -103,7 +114,11 @@ function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execut res.end(); return; } - if (req.method === "POST" && req.url === "/api/session/ses_1/interrupt") return send(200, { interrupted: true }); + if (req.method === "POST" && req.url === "/api/session/ses_1/interrupt") { + idle = { id: `idle_${prompts}`, type: "idle", outcome: "interrupted" }; + setTimeout(() => emit("session.execution.interrupted", { sessionID: "ses_1" }), interruptEventDelayMs); + return send(200, { interrupted: true }); + } if (req.method === "DELETE" && req.url === "/api/session/ses_1") { res.writeHead(204); res.end(); @@ -216,6 +231,61 @@ test("a timed-out agent is interrupted server-side and not reported as transient } }); +test("a reviewer still running near its timeout is interrupted and asked for its findings in the same session", async () => { + const fake = await fakeOpencode({ idleAfterMs: null, wrapUpAfterMs: 100 }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + const started = Date.now(); + const result = await client.prompt({ ...PROMPT, timeoutMs: 3000, wrapUp: { ms: 1000, text: "wrap up now" } }); + client.close(); + assert.ok(Date.now() - started < 3000, "waited for the hard timeout instead of the wrap-up answer"); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + assert.equal(result.info.tokens.input, 2000, "tokens are the whole session's, both prompts included"); + const prompts = fake.calls.filter(c => c.url === "/api/session/ses_1/prompt"); + assert.deepEqual(prompts.map(c => c.body.text), ["", "wrap up now"]); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/interrupt").length, 1); + } finally { + await fake.close(); + } +}); + +test("an interrupted event that arrives after the wrap-up prompt went out does not end the wrap-up", async () => { + // The poll confirms the interrupt first; the stream's interrupted event lands while the follow-up is running. + const fake = await fakeOpencode({ idleAfterMs: null, wrapUpAfterMs: 600, interruptEventDelayMs: 400 }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + const result = await client.prompt({ ...PROMPT, timeoutMs: 4000, wrapUp: { ms: 2000, text: "wrap up now" } }); + client.close(); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/prompt").length, 2); + } finally { + await fake.close(); + } +}); + +test("a wrap-up that does not answer in its slot still ends as a hard timeout", async () => { + const fake = await fakeOpencode({ idleAfterMs: null }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + await assert.rejects(client.prompt({ ...PROMPT, timeoutMs: 1600, wrapUp: { ms: 800, text: "wrap up now" } }), err => err.name === "TimeoutError"); + client.close(); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/prompt").length, 2); + } finally { + await fake.close(); + } +}); + +test("the wrap-up slot is taken only from a session at least twice its length", () => { + const wrapUp = { ms: 3 * 60_000, text: "wrap" }; + assert.equal(wrapUpStartMs(20 * 60_000, wrapUp), 17 * 60_000); + assert.equal(wrapUpStartMs(6 * 60_000, wrapUp), 3 * 60_000); + assert.equal(wrapUpStartMs(5 * 60_000, wrapUp), 5 * 60_000); + assert.equal(wrapUpStartMs(20 * 60_000, null), 20 * 60_000); + assert.equal(wrapUpStartMs(20 * 60_000, { ms: 60_000, text: "" }), 20 * 60_000); +}); + test("a provider rate limit surfaced as session.execution.failed is transient", async () => { const error = { type: "provider.rate-limit", message: "Too Many Requests", status: 429 }; const fake = await fakeOpencode({ idleAfterMs: 100, finalEvent: "session.execution.failed", error }); @@ -354,6 +424,14 @@ const READ_ONLY = [ rule("skill", "browser", "deny"), ]; const FULL = [...BASE, rule("*", "*", "allow"), rule("browser", "*", "deny")]; +const CI_FULL = [ + ...BASE, + rule("*", "*", "allow"), + ...["webfetch", "websearch", "subagent", "question", "execute", "skill"].map(id => rule(id, "*", "deny")), + rule("shell", "*", "deny"), + rule("shell", "git diff *", "allow"), + rule("browser", "*", "deny"), +]; const agent = (permissions, id = "ai-review-security") => ({ id, name: id, mode: "primary", permissions }); test("self-check passes only for read-only agents and built-in plugins", async () => { @@ -379,17 +457,21 @@ test("self-check passes only for read-only agents and built-in plugins", async ( await assert.rejects(selfCheck(fakeClient({ agents: [agent(FULL)] }), names), /shell allowed/, "the default config must refuse a full grant"); }); -test("ci-full self-check wants every agent registered with the full grant, and still refuses foreign plugins", async () => { +test("ci-full self-check wants file tools granted, web and sub-agent tools denied and the shell allowlisted", async () => { const names = ["ai-review-security", "ai-review-correctness"]; - const agents = [agent(FULL), agent(FULL, "ai-review-correctness")]; + const agents = [agent(CI_FULL), agent(CI_FULL, "ai-review-correctness")]; const result = await selfCheck(fakeClient({ agents }), names, { readOnly: false }); assert.equal(result.agents, 2); - await assert.rejects(selfCheck(fakeClient({ agents: [agent(FULL)] }), names, { readOnly: false }), /ai-review-correctness: not registered/); + await assert.rejects(selfCheck(fakeClient({ agents: [agent(CI_FULL)] }), names, { readOnly: false }), /ai-review-correctness: not registered/); await assert.rejects( - selfCheck(fakeClient({ agents: [agent(READ_ONLY), agent(FULL, "ai-review-correctness")] }), names, { readOnly: false }), + selfCheck(fakeClient({ agents: [agent(READ_ONLY), agent(CI_FULL, "ai-review-correctness")] }), names, { readOnly: false }), /ai-review-security: edit not fully allowed/, ); + assert.deepEqual(ciFullProblems(FULL), ["webfetch allowed", "websearch allowed", "subagent allowed", "question allowed", "execute allowed", "skill allowed", "shell allows every command"]); + assert.deepEqual(ciFullProblems([...CI_FULL, rule("execute", "*", "allow")]), ["execute allowed"]); + assert.deepEqual(ciFullProblems([...CI_FULL, rule("shell", "*", "deny")]), ["shell denied"]); + assert.deepEqual(ciFullProblems(CI_FULL), []); await assert.rejects( selfCheck(fakeClient({ agents, plugins: [...BUILTIN_PLUGINS, { id: "p", source: { type: "package", target: "evil" }, state: { status: "active" } }] }), names, { readOnly: false }), /non-built-in plugins/, @@ -427,10 +509,44 @@ test("both configs register the same agents, provider and model; only the defaul for (const toolId of BUILTIN_TOOL_IDS) assert.equal(toolEnabled(toolId, readOnly), ["read", "glob", "grep"].includes(toolId), `${name} ${toolId}`); assert.equal(externalDirectoryProblem(readOnly), "", name); const full = [...BASE, ...ci.agents[name].permissions]; - for (const toolId of BUILTIN_TOOL_IDS) assert.equal(toolAllowedWithoutAsking(toolId, full), true, `${name} ${toolId}`); + assert.deepEqual(ciFullProblems(full), [], name); + // opencode 2.2.0 ends the capped turn on an assistant message, a prefill the model rejects, so no agent sets a step cap. + for (const config of [base, ci]) for (const key of ["steps", "maxSteps"]) assert.equal(config.agents[name][key], undefined, `${name} ${key}`); } }); +// Evaluated as opencode 2.2.0 does: the last rule whose action and resource both match decides. +function shellDecision(rules, command) { + return [...BASE, ...rules].findLast(r => wildcardMatch("shell", r.action) && wildcardMatch(command, r.resource))?.effect; +} + +test("the ci-full shell runs the read-only allowlist and nothing that builds, installs or reaches the network", () => { + const ci = JSON.parse(stripJsonc(readFileSync(join(REPO_ROOT, OPENCODE_CONFIGS["ci-full"].file), "utf-8"))); + for (const [name, agentConfig] of Object.entries(ci.agents)) { + for (const command of ["git diff", "git diff origin/main -- src", "git log -5 --oneline", "git show HEAD:src/a.rs", "git blame -L 1,5 a.rs", "rg -n foo src", "cat a.rs", "ls", "ls -la web", "head -50 a.rs", "sed -n 1,40p a.rs", "wc -l a.rs", + "grep -rn foo src", "tail -n 20 a.rs", "find src -name '*.rs'", "find . -type f -newer a.rs", "find src -name '*-executor*'", "find . -name ok-delete.rs", + "git ls-files src", "git status", "git status --short", "git rev-parse HEAD", "git merge-base HEAD origin/main", "git cat-file -p HEAD:a.rs", "git cat-file -t HEAD", "git cat-file -s HEAD", + "git --no-pager diff --stat", "git --no-pager log -3", "git --no-pager show HEAD", "rg -n foo src/env_util.rs"]) { + assert.equal(shellDecision(agentConfig.permissions, command), "allow", `${name}: ${command}`); + } + for (const command of ["cargo build", "cargo test -p config", "npm ci", "npx vitest", "curl https://example.com", "git push", "git fetch origin", "git difftool", "sed -i s/a/b/ a.rs", "make", "python3 x.py", "bash -c ls", + "find . -exec rm {} ;", "find -execdir sh -c x ;", "find . -name x -ok rm {} ;", "find . -delete", "find . -fprint out.txt", "find . -fls out.txt", + "tail -f a.log", "tail -n 5 -F a.log", "tail --follow=name a.log", "git grep foo", "git grep -nOsh foo", "git grep --open-files=sh foo", "git --no-pager grep foo", + "git --no-pager push", "git --no-pager -c core.pager=sh log", "git cat-file --batch", "git cat-file blob HEAD", + "cat /proc/self/environ", "head -c 100 /proc/1/environ", "tail /dev/stdin", "grep -a KEY /proc/self/environ", "rg KEY /proc", "sed -n p /proc/self/environ", + "cat ../../proc/self/environ", "cat src/environ", "rg --file=/proc/self/environ x", "wc -c /p*/self/env*", "cat /etc/passwd", "cat a.rs > b.rs", "rg foo src>>out.txt", "cat a.rs 2>/dev/null"]) { + assert.equal(shellDecision(agentConfig.permissions, command), "deny", `${name}: ${command}`); + } + } +}); + +test("a trailing ' *' also matches the bare command, as in opencode 2.2.0", () => { + assert.equal(wildcardMatch("git diff", "git diff *"), true); + assert.equal(wildcardMatch("git diff --stat", "git diff *"), true); + assert.equal(wildcardMatch("git difftool", "git diff *"), false); + assert.equal(wildcardMatch("read", "*"), true); +}); + test("the review runs on the anthropic-review provider, the only one the policies allow, with the workflow's variant", () => { const base = JSON.parse(stripJsonc(readFileSync(join(REPO_ROOT, OPENCODE_CONFIGS.default.file), "utf-8"))); const allowed = base.policies.filter(p => p.action === "provider.use" && p.effect === "allow").map(p => p.resource); @@ -441,9 +557,14 @@ test("the review runs on the anthropic-review provider, the only one the policie assert.equal(provider.package, "@opencode/ai/providers/anthropic"); assert.deepEqual(provider.env, ["ANTHROPIC_REVIEW_API_KEY"]); const model = provider.models["claude-sonnet-5-5"]; - assert.deepEqual(model.variants, [{ id: "high", settings: { thinking: { type: "adaptive" }, effort: "high" } }]); + assert.deepEqual(model.variants, ["low", "medium", "high", "xhigh", "max"].map(id => ({ id, settings: { thinking: { type: "adaptive" }, effort: id } }))); + assert.deepEqual(modelVariantIds(base, "anthropic-review", "claude-sonnet-5-5"), ["low", "medium", "high", "xhigh", "max"]); + assert.deepEqual(modelVariantIds(base, "anthropic-review", "no-such-model"), []); const workflow = readFileSync(join(REPO_ROOT, ".github/workflows/ai-code-review.yml"), "utf-8"); - assert.match(workflow, /provider_id: anthropic-review\n\s+model_id: claude-sonnet-5-5\n\s+model_variant: high\n\s+api_key_env: ANTHROPIC_REVIEW_API_KEY\n/); + assert.match(workflow, /provider_id: anthropic-review\n\s+model_id: claude-sonnet-5-5\n\s+api_key_env: ANTHROPIC_REVIEW_API_KEY\n/); + assert.doesNotMatch(workflow, /model_variant/, "the effort variable is the only source of the variant"); + assert.match(workflow, /REVIEW_MODEL_VARIANT: \$\{\{ vars\.AI_REVIEW_EFFORT \|\| 'high' \}\}/); + assert.match(workflow, /REVIEW_COORDINATOR_VARIANT: \$\{\{ vars\.AI_REVIEW_COORDINATOR_EFFORT \|\| vars\.AI_REVIEW_EFFORT \|\| 'high' \}\}/); assert.match(workflow, /ANTHROPIC_REVIEW_API_KEY: \$\{\{ secrets\.USAGE_CREDIT_API_KEY \}\}/); }); diff --git a/scripts/ai-review/test/passes.test.mjs b/scripts/ai-review/test/passes.test.mjs index 72c022adfd..94680e6c96 100644 --- a/scripts/ai-review/test/passes.test.mjs +++ b/scripts/ai-review/test/passes.test.mjs @@ -1,5 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import { mergeConfidence, unfinishedReviewers } from "../src/confidence.mjs"; import { reviewerFacts } from "../src/auto-approve.mjs"; @@ -10,6 +11,7 @@ import { parseColdPasses, passLabel, passPlan, + reviewComplete, runPooled, } from "../src/passes.mjs"; import { buildFallbackReview, partialFindingsBlock, selectFallbackFindings } from "../src/report.mjs"; @@ -137,3 +139,22 @@ test("the fallback never merges two different blockers that share most of their ]); assert.equal(selectFallbackFindings(same).length, 1); }); + +test("only a review whose every pass finished in time and whose coordinator merged may skip an unchanged diff next time", () => { + const coordinator = { fallback: false }; + const skippedPass = { agentKey: "frontend", agentName: "frontend Reviewer", pass: 1, findings: [], error: null, skipped: true }; + assert.equal(reviewComplete([done("security", 1), done("security", 2), skippedPass], coordinator), true); + assert.equal(reviewComplete([done("security", 1), failed("security", 2)], coordinator), false, "a lost cold pass is a partial review"); + assert.equal(reviewComplete([done("security", 1), failed("security", 2, { timedOut: true })], coordinator), false); + assert.equal(reviewComplete([done("security", 1), { ...done("qa", 1), parsed: false }], coordinator), false, "unparseable output is unfinished"); + assert.equal(reviewComplete([done("security", 1), { ...done("qa", 1), wrappedUp: true }], coordinator), false, "a reviewer cut short at its soft deadline"); + assert.equal(reviewComplete([done("security", 1)], { fallback: true }), false, "the coordinator fell back"); + assert.equal(reviewComplete([done("security", 1)], null), false, "the coordinator never ran"); +}); + +test("run-review writes the diff fingerprint only for a complete review", () => { + const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); + assert.match(source, /const complete = reviewComplete\(passResults, coordinatorResult\);/); + assert.match(source, /diffFingerprint: complete && fullyRead && diffPinned && hunksLocatable\(diff, readBaseFile\) \? fingerprint : ""/); + assert.equal((source.match(/diffFingerprint:/g) || []).length, 1, "every finishReviewBody call goes through the completeness check"); +}); diff --git a/scripts/ai-review/test/pr-diff.test.mjs b/scripts/ai-review/test/pr-diff.test.mjs index 2d7551e9a1..dc75c557f8 100644 --- a/scripts/ai-review/test/pr-diff.test.mjs +++ b/scripts/ai-review/test/pr-diff.test.mjs @@ -6,7 +6,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseRawDiff, sizeReasons } from "../src/auto-approve.mjs"; -import { fetchDiff, fetchPinnedDiff, localDiff, pinDiff, prefixDiffPaths } from "../src/pr-diff.mjs"; +import { diffFingerprint, fetchDiff, fetchPinnedDiff, hunksLocatable, localDiff, pinDiff, prefixDiffPaths } from "../src/pr-diff.mjs"; const DIFF = [ "diff --git a/src/lib.rs b/src/lib.rs", @@ -199,3 +199,114 @@ test("a renamed symlink or submodule whose header carries no mode cannot pass as assert.match(edited, /^index \S+ 100644$/m); assert.deepEqual(parseRawDiff(edited).problems, []); }); + +test("a rebase that moves the PR's lines keeps its diff fingerprint, a changed line does not", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + try { + git("init", "-q", "-b", "main"); + const lines = Array.from({ length: 40 }, (_, i) => `line ${i}`); + writeFileSync(join(dir, "a.rs"), `${lines.join("\n")}\n`); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "pr"); + writeFileSync(join(dir, "a.rs"), `${lines.map((l, i) => (i === 30 ? "line 30 changed" : l)).join("\n")}\n`); + git("commit", "-qam", "pr change"); + const before = localDiff(dir, "main", "pr"); + git("checkout", "-q", "main"); + writeFileSync(join(dir, "a.rs"), `head 1\nhead 2\nhead 3\n${lines.join("\n")}\n`); + git("commit", "-qam", "main moves on"); + git("checkout", "-q", "pr"); + git("rebase", "-q", "main"); + const after = localDiff(dir, "main", "pr"); + assert.notEqual(before, after, "the rebase must change hunk numbers and blob ids, or this test proves nothing"); + assert.equal(diffFingerprint(after), diffFingerprint(before)); + writeFileSync(join(dir, "a.rs"), `head 1\nhead 2\nhead 3\n${lines.map((l, i) => (i === 30 ? "line 30 changed again" : l)).join("\n")}\n`); + git("commit", "-qam", "another change"); + assert.notEqual(diffFingerprint(localDiff(dir, "main", "pr")), diffFingerprint(before)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("the fingerprint ignores only blob ids and hunk numbers", () => { + const base = "diff --git a/a b/a\nindex 1111111..2222222 100644\n--- a/a\n+++ b/a\n@@ -1,2 +1,2 @@ fn x\n ctx\n-old\n+new\n"; + assert.equal(diffFingerprint(base.replace("1111111..2222222", "3333333..4444444").replace("-1,2 +1,2 @@ fn x", "-9,2 +12,2 @@ fn x")), diffFingerprint(base)); + assert.notEqual(diffFingerprint(base.replace("@@ fn x", "@@ fn y")), diffFingerprint(base), "the function a hunk sits in is part of what was reviewed"); + assert.notEqual(diffFingerprint(base.replace("+new", "+newer")), diffFingerprint(base)); + assert.notEqual(diffFingerprint(base.replace("diff --git a/a b/a", "diff --git a/b b/b")), diffFingerprint(base)); + assert.match(diffFingerprint(base), /^[0-9a-f]{64}$/); +}); + +test("the same lines changed inside a different function get a different fingerprint", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + const fn = (name, allowed = "user.active") => `function ${name}(user) {\n const id = user.id;\n const name = user.name;\n const role = user.role;\n const group = user.group;\n const allowed = ${allowed};\n const details = { id, name, role, group };\n const present = Boolean(details);\n const result = Boolean(allowed);\n return present && result;\n}\n`; + try { + git("init", "-q", "-b", "main"); + writeFileSync(join(dir, "rules.js"), `${fn("preview")}\n${fn("authorize")}`); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "safe"); + writeFileSync(join(dir, "rules.js"), `${fn("preview", "true")}\n${fn("authorize")}`); + git("commit", "-qam", "safe"); + git("checkout", "-qb", "unsafe", "main"); + writeFileSync(join(dir, "rules.js"), `${fn("preview")}\n${fn("authorize", "true")}`); + git("commit", "-qam", "unsafe"); + const safe = localDiff(dir, "main", "safe"); + const unsafe = localDiff(dir, "main", "unsafe"); + assert.match(safe, /^@@ .* @@ function preview\(user\) \{$/m); + assert.match(unsafe, /^@@ .* @@ function authorize\(user\) \{$/m); + assert.notEqual(diffFingerprint(unsafe), diffFingerprint(safe)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("an edit to one of two identical blocks in a function cannot be located, so it is never taken for the reviewed one", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + const block = (flag, allowed) => [` if (${flag}) {`, " const a = 1;", " const b = 2;", " const c = 3;", ` ok = ${allowed};`, " const d = 4;", " const e = 5;", " const f = 6;", " }"]; + const source = (first, second) => ["function check(user, internal) {", " let ok = false;", ...block("internal", first), ...block("!internal", second), " return ok;", "}", ""].join("\n"); + const readBase = ref => path => { try { return execFileSync("git", ["-C", dir, "cat-file", "blob", `${ref}:${path}`], { encoding: "utf-8" }); } catch { return null; } }; + try { + git("init", "-q", "-b", "main"); + writeFileSync(join(dir, "check.js"), source("user.admin", "user.admin")); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "internal"); + writeFileSync(join(dir, "check.js"), source("true", "user.admin")); + git("commit", "-qam", "internal"); + git("checkout", "-qb", "external", "main"); + writeFileSync(join(dir, "check.js"), source("user.admin", "true")); + git("commit", "-qam", "external"); + const internal = localDiff(dir, "main", "internal"); + const external = localDiff(dir, "main", "external"); + assert.notEqual(internal, external); + assert.equal(hunksLocatable(external, readBase("main")), false, "the edited block's old text sits twice in the base"); + assert.equal(hunksLocatable(internal, readBase("main")), false); + writeFileSync(join(dir, "other.js"), "a\nb\nc\nd\ne\nf\ng\n"); + git("add", "other.js"); + git("commit", "-qm", "unique"); + writeFileSync(join(dir, "other.js"), "a\nb\nc\nD\ne\nf\ng\n"); + git("commit", "-qam", "edit unique"); + const unique = localDiff(dir, "HEAD~1", "HEAD"); + assert.equal(hunksLocatable(unique, readBase("HEAD~1")), true); + assert.equal(hunksLocatable(unique, () => null), false, "an unreadable base file cannot locate the edit"); + assert.equal(hunksLocatable("diff --git a/n b/n\nnew file mode 100644\n--- /dev/null\n+++ b/n\n@@ -0,0 +1 @@\n+x\n", () => null), true, "a new file has nothing to locate"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a changed binary file or trailing space on the last line changes the fingerprint", () => { + const bin = id => `diff --git a/m.wasm b/m.wasm\nindex 1111111..${id} 100644\nBinary files a/m.wasm and b/m.wasm differ\n`; + assert.notEqual(diffFingerprint(bin("2222222")), diffFingerprint(bin("3333333"))); + const csv = tail => `diff --git a/a.csv b/a.csv\nindex 1111111..2222222 100644\n--- a/a.csv\n+++ b/a.csv\n@@ -1 +1 @@\n-x\n+secret${tail}\n`; + assert.notEqual(diffFingerprint(csv(" ")), diffFingerprint(csv(" "))); +}); + +test("a file mode change keeps a different fingerprint", () => { + const d = mode => `diff --git a/l b/l\nindex 1111111..2222222 ${mode}\n--- a/l\n+++ b/l\n@@ -1 +1 @@\n-old-target\n\\ No newline at end of file\n+new-target\n\\ No newline at end of file\n`; + assert.notEqual(diffFingerprint(d("100644")), diffFingerprint(d("120000"))); +}); diff --git a/scripts/ai-review/test/report.test.mjs b/scripts/ai-review/test/report.test.mjs index 6d796b5bc9..127861e545 100644 --- a/scripts/ai-review/test/report.test.mjs +++ b/scripts/ai-review/test/report.test.mjs @@ -37,7 +37,8 @@ import { } from "../src/report.mjs"; import { extractInlineBlock } from "../src/inline.mjs"; import { LOADER_DIRS, LOADER_FILES } from "../src/sanitize.mjs"; -import { reviewerDiff } from "../src/pr-diff.mjs"; +import { diffFingerprint, hunksLocatable, reviewerDiff } from "../src/pr-diff.mjs"; +import { narrowTier } from "../src/selection.mjs"; import * as approvalModule from "../src/auto-approve.mjs"; import * as commentsModule from "../src/comments.mjs"; import * as confidenceModule from "../src/confidence.mjs"; @@ -779,7 +780,8 @@ const mainSource = () => { const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); const fetcher = source.slice(source.indexOf("function fetchPrDiff("), source.indexOf("function pairedPrContext(")); const notes = source.slice(source.indexOf("function noteOnComment("), source.indexOf("function noteSkippedPush(")); - return `${fetcher}\n${notes}\n${source.slice(source.indexOf("async function main() {"), source.indexOf("// ─── Entry point"))}\nreturn main;`; + const unchanged = source.slice(source.indexOf("function unchangedDiffCheck("), source.indexOf("function prepareReviewTree(")); + return `${fetcher}\n${notes}\n${unchanged}\n${source.slice(source.indexOf("async function main() {"), source.indexOf("// ─── Entry point"))}\nreturn main;`; }; const cleanCoordinatorOutput = `${commentsModule.REVIEW_HEADING}\n\n> [!TIP]\n> ### ✅ Approved\n> Looks right.\n\n`; const finishedReviewer = key => ({ agentKey: key, agentName: `${key} Reviewer`, findings: [], parsed: true, fullDiff: true, error: null }); @@ -787,19 +789,19 @@ const silentReviewer = key => ({ ...finishedReviewer(key), parsed: false, rawTex const smallRawDiff = "diff --git a/docs/a.md b/docs/a.md\n--- a/docs/a.md\n+++ b/docs/a.md\n@@ -1 +1 @@\n-old\n+new\n"; // Every default makes the run an approvable one, so a test changes the single fact it is about. -async function runMain({ results = [finishedReviewer("security"), finishedReviewer("correctness")], byPass = null, pinned = {}, mode = "approve", existing = null, reviewedBody = "plain words", prNow = {}, paired = {}, contextFails = false, noiseOnly = false, diffFails = false, coordinatorOutput = cleanCoordinatorOutput, rawDiff = smallRawDiff, keptDiff = null } = {}) { +async function runMain({ results = [finishedReviewer("security"), finishedReviewer("correctness")], byPass = null, pinned = {}, mode = "approve", existing = null, reviewedBody = "plain words", prNow = {}, paired = {}, contextFails = false, noiseOnly = false, diffFails = false, manual = false, head = MAIN_HEAD, coordinatorOutput = cleanCoordinatorOutput, rawDiff = smallRawDiff, keptDiff = null, variantError = "", ignorePassLimit = false, forceFull = false, baseFile = "old\n" } = {}) { const run = { posted: "", approvals: [], logs: [], coordinatorCalls: 0, coordinatorArgs: null, reviewerCalls: [], artifact: null, updates: 0, creates: 0, claims: 0, sanitized: 0 }; - const pr = { ...eligibleInput().pr, title: "fix a bug", body: "plain words", head: { sha: MAIN_HEAD }, changed_files: 1, additions: 1, deletions: 1, ...prNow }; + const pr = { ...eligibleInput().pr, title: "fix a bug", body: "plain words", head: { sha: head }, changed_files: 1, additions: 1, deletions: 1, ...prNow }; const noop = () => {}; const scope = { ...commentsModule, ...confidenceModule, ...approvalModule, ...reportModule, ...passesModule, REVIEW_COLD_PASSES: 2, REVIEWER_CONCURRENCY: 16, - process: { env: { PR_NUMBER: "1", GITHUB_REPOSITORY: "test/repo", HEAD_SHA: MAIN_HEAD, MERGE_BASE_SHA: "c".repeat(40) } }, + process: { env: { PR_NUMBER: "1", GITHUB_REPOSITORY: "test/repo", HEAD_SHA: head, MERGE_BASE_SHA: "c".repeat(40), ...(forceFull ? { FORCE_FULL: "true" } : {}) } }, console: { log: (...args) => run.logs.push(args.join(" ")), warn: noop, error: noop }, TRACE: { startSpan: () => ({}), endSpan: noop, setSpanAttributes: noop }, activeRootSpan: null, isoNow: () => "now", prepareReviewTree: noop, findExistingReviewComment: () => ({ comment: existing, ok: true }), passesDoneIn: comment => (comment ? commentsModule.reviewPassCount(comment.body) : 0), - REVIEW_IGNORE_PASS_LIMIT: false, REVIEW_MAX_PASSES: 0, REVIEW_BOT_LOGIN: "review[bot]", REVIEW_PAIR_CLEAR: true, AUTO_APPROVE_MODE: mode, + REVIEW_IGNORE_PASS_LIMIT: ignorePassLimit, REVIEW_MANUAL_RUN: manual, MAX_DIFF_TOKENS: 150_000, diffFingerprint, hunksLocatable, readBaseFile: () => baseFile, variantProblem: () => variantError, MODEL_VARIANT: "high", COORDINATOR_VARIANT: "high", REVIEW_MAX_PASSES: 0, REVIEW_BOT_LOGIN: "review[bot]", REVIEW_PAIR_CLEAR: true, AUTO_APPROVE_MODE: mode, REVIEW_MARKER: "", PAIRED: { repo: "", prNumber: "", dir: "", mergeBase: "", head: "", ...paired }, PR_DIR: "/pr", resolve: (...parts) => parts.join("/"), githubReviewApi: () => ({ readPr: () => pr, listReviews: () => [], dismissReview: noop, approve: payload => run.approvals.push(payload) }), noteSkippedPush: noop, apiKey: () => "configured", API_KEY_VAR_NAME: "TEST_KEY", MODEL_SLUG: "test/model", @@ -922,6 +924,109 @@ test("a push the run stops on before claiming a pass loses the old verdict and k } }); +test("a new head whose diff matches the reviewed one is skipped without a pass, a model call or a claim", async () => { + const first = await runMain({ mode: "off" }); + assert.equal(commentsModule.reviewedDiffFingerprint(first.posted), diffFingerprint(smallRawDiff)); + const rebased = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD }); + assert.equal(rebased.error, undefined); + assert.ok(rebased.logs.some(line => line.includes("unchanged_diff"))); + assert.deepEqual(rebased.reviewerCalls, []); + assert.equal(rebased.coordinatorCalls, 0); + assert.equal(rebased.claims, 0); + assert.equal(rebased.updates, 1); + assert.equal(commentsModule.reviewedKey(rebased.posted), PAIRED_MAIN_HEAD); + assert.equal(commentsModule.reviewPassCount(rebased.posted), 1); + assert.match(rebased.posted, /Merge confidence: 🟢 High/); +}); + +test("a hand-started run, a changed diff or an unpinned diff is reviewed even after a matching review", async () => { + const first = await runMain({ mode: "off" }); + const existing = { id: 1, body: first.posted }; + const changed = `${smallRawDiff}+more\n`; + for (const run of [ + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, manual: true }), + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, rawDiff: changed }), + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, pinned: { "test/repo": false } }), + ]) { + assert.equal(run.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(run.claims, 1); + assert.ok(run.reviewerCalls.length > 0); + assert.equal(commentsModule.reviewPassCount(run.posted), 2); + } +}); + +test("a hand-started, force-full or pass-limit-ignoring run reviews the head the last review already covered", async () => { + const first = await runMain({ mode: "off" }); + const existing = { id: 1, body: first.posted }; + for (const bypass of [{ manual: true }, { forceFull: true }, { ignorePassLimit: true }]) { + const run = await runMain({ mode: "off", existing, ...bypass }); + assert.equal(run.error, undefined, JSON.stringify(bypass)); + assert.equal(run.logs.some(line => /already_reviewed|unchanged_diff/.test(line)), false, JSON.stringify(bypass)); + assert.ok(run.reviewerCalls.length > 0, JSON.stringify(bypass)); + assert.equal(run.claims, 1, JSON.stringify(bypass)); + } + const automatic = await runMain({ mode: "off", existing }); + assert.ok(automatic.logs.some(line => line.includes("already_reviewed"))); + assert.deepEqual(automatic.reviewerCalls, []); +}); + +test("reviewers cut short at their soft deadline are not finished, so the verdict is not High and the gate does not approve", async () => { + const wrapped = key => ({ ...finishedReviewer(key), wrappedUp: true }); + for (const results of [[wrapped("security"), wrapped("correctness")], [finishedReviewer("security"), wrapped("correctness")]]) { + const run = await runMain({ results }); + assert.equal(run.error, undefined); + assert.equal(run.approvals.length, 0); + assert.doesNotMatch(run.posted, /Merge confidence: 🟢 High/); + assert.doesNotMatch(run.posted, /all reviewers finished/); + assert.doesNotMatch(run.posted, /Auto-approved/); + assert.equal(commentsModule.reviewedDiffFingerprint(run.posted), ""); + } + const byPass = (key, pass) => ({ ...finishedReviewer(key), wrappedUp: pass === 2 }); + const oneCleanPass = await runMain({ byPass }); + assert.equal(oneCleanPass.approvals.length, 1, "an area with one uninterrupted pass is still reviewed"); +}); + +test("an invalid effort setting retracts the old verdict and approval line without claiming a pass", async () => { + const first = await runMain(); + assert.match(first.posted, /Merge confidence: 🟢 High/); + assert.match(first.posted, /Auto-approved/); + const variantError = 'variant "typo" is not declared for test/model in opencode.ci.jsonc (declared: high)'; + for (const stop of [{ head: PAIRED_MAIN_HEAD, rawDiff: `${smallRawDiff}+more\n` }, { manual: true }]) { + const run = await runMain({ existing: { id: 1, body: first.posted }, variantError, ...stop }); + assert.match(run.error, /variant "typo" is not declared/); + assert.equal(run.claims, 0); + assert.deepEqual(run.reviewerCalls, []); + assert.equal(run.updates, 1); + assert.doesNotMatch(run.posted, /High/); + assert.doesNotMatch(run.posted, /Auto-approved/); + assert.match(run.posted, /the review did not run/); + assert.doesNotMatch(run.posted, /typo|test\/model/, "the configuration detail stays in the log"); + assert.equal(commentsModule.reviewPassCount(run.posted), 1); + assert.equal(commentsModule.reviewedKey(run.posted), MAIN_HEAD); + } +}); + +test("an unchanged diff whose hunk text appears twice in the base is reviewed, as the edit cannot be located", async () => { + const first = await runMain({ mode: "off" }); + const twice = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, baseFile: "old\nx\nold\n" }); + assert.equal(twice.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(twice.claims, 1); + assert.equal(commentsModule.reviewedDiffFingerprint(twice.posted), "", "a review whose edit cannot be located records no fingerprint"); + const unreadable = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, baseFile: null }); + assert.equal(unreadable.claims, 1); + assert.ok((await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD })).logs.some(line => line.includes("unchanged_diff"))); +}); + +test("a push that only adds a production asset the noise filter hides is reviewed, not skipped with the old High", async () => { + const first = await runMain({ mode: "off" }); + assert.match(first.posted, /Merge confidence: 🟢 High/); + const svg = "diff --git a/web/src/assets/chart.svg b/web/src/assets/chart.svg\nnew file mode 100644\n--- /dev/null\n+++ b/web/src/assets/chart.svg\n@@ -0,0 +1 @@\n+\n"; + const next = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, rawDiff: `${smallRawDiff}${svg}`, keptDiff: smallRawDiff }); + assert.equal(next.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(next.claims, 1); + assert.doesNotMatch(next.posted, /Merge confidence: 🟢 High/); +}); + test("a dangling second block cannot read as a clean count, so the gate refuses", () => { const output = `${cleanCoordinatorOutput}\n - + + {{ t("oncall.unsnooze") }} + + + - + + {{ t("oncall.unsnooze") }} + + {{ t("oncall.snoozedUntil", { time: snoozedUntilLabel }) }} + - + + + {{ t("oncall.unsnooze") }} + + acknowledgeResponseMutation(orgId.value)); const snoozeWrite = useMutation(() => snoozeResponseMutation(orgId.value)); +const unsnoozeWrite = useMutation(() => unsnoozeResponseMutation(orgId.value)); const resolveWrite = useMutation(() => resolveResponseMutation(orgId.value)); const handoffWrite = useMutation(() => handoffResponseMutation(orgId.value)); const noteWrite = useMutation(() => addResponseNoteMutation(orgId.value)); @@ -1657,6 +1672,28 @@ async function snoozeRow(row: PageRow, minutes: number) { } } +function snoozedFirings(row: PageRow): OnCallResponse[] { + return row.escalating.filter((r) => isSnoozed(r)); +} + +/// Ends the snooze on every quiet firing the row stands for, so they all continue now. +async function unsnoozeRow(row: PageRow) { + busyId.value = row.rowKey; + try { + const results = await Promise.allSettled( + snoozedFirings(row).map((r) => unsnoozeWrite.mutateAsync(r.id)), + ); + if (results.some((r) => r.status === "rejected")) { + toast({ variant: "error", message: t("oncall.unsnoozeFailed") }); + } else { + toast({ variant: "success", message: t("oncall.unsnoozed") }); + } + await fetchResponses(); + } finally { + busyId.value = ""; + } +} + async function resolveRow(row: PageRow) { busyId.value = row.rowKey; try {