feat(review): support additional reviewer harnesses (#2306)

* feat(review): support more reviewer harnesses

* fix(review): preserve reviewer daemon context

* fix(review): allow printf-piped review commands in reviewer allowlists

The review prompt now pipes JSON into `gh api` and `ao review submit`
via `printf '%s' ... | cmd` (heredocs break in interactive PTY panes).
Widen the claude-code allowlist with `Bash(printf:*)` and the opencode
permission policy with `printf * | gh api *` / `printf * | ao review
submit *` so those piped commands pass each tool's allowlist. Cover
both with tests that model each tool's matching semantics.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: format with prettier [skip ci]

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Anirudh Sharma 2026-07-02 19:25:18 +05:30 committed by GitHub
parent c267420632
commit e74fb65a85
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 501 additions and 23 deletions

View File

@ -37,12 +37,14 @@ var _ ports.Reviewer = (*Reviewer)(nil)
// system entirely and ignores allow/deny rules — it launches in the default // system entirely and ignores allow/deny rules — it launches in the default
// mode where these rules are honored: allow rules auto-approve without // mode where these rules are honored: allow rules auto-approve without
// prompting, so the reviewer can read the checkout and run the few commands it // prompting, so the reviewer can read the checkout and run the few commands it
// needs (git diff/log/show to inspect the PR, gh to post the review, and // needs (git diff/log/show to inspect the PR, printf to pipe review JSON into
// `ao review submit` to record the verdict) without stalling. // the downstream commands without writing a worktree file, gh to post the
// review, and `ao review submit` to record the verdict) without stalling.
var reviewerAllowedTools = []string{ var reviewerAllowedTools = []string{
"Read", "Read",
"Grep", "Grep",
"Glob", "Glob",
"Bash(printf:*)",
"Bash(gh:*)", "Bash(gh:*)",
"Bash(git diff:*)", "Bash(git diff:*)",
"Bash(git log:*)", "Bash(git log:*)",

View File

@ -2,6 +2,7 @@ package claudecode
import ( import (
"context" "context"
"strings"
"testing" "testing"
"github.com/aoagents/agent-orchestrator/backend/internal/ports" "github.com/aoagents/agent-orchestrator/backend/internal/ports"
@ -61,6 +62,75 @@ func TestReviewCommandLaunchesReadOnlyOffBypass(t *testing.T) {
} }
} }
func TestAllowlistCoversPromptRequiredPipedCommands(t *testing.T) {
agent := &captureAgent{}
r := &Reviewer{agent: agent}
if _, err := r.ReviewCommand(context.Background(), ports.ReviewInvocation{
ReviewerID: "review-w1",
WorkspacePath: "/ws/w1",
Prompt: "review it",
SystemPrompt: "you are a reviewer",
}); err != nil {
t.Fatalf("ReviewCommand: %v", err)
}
if !contains(agent.got.AllowedTools, "Bash(printf:*)") {
t.Fatalf("allowlist missing printf for piped review commands: %#v", agent.got.AllowedTools)
}
for _, cmd := range []string{
"printf '%s' '{ \"event\": \"COMMENT\", \"body\": \"x\" }' | gh api --method POST repos/o/r/pulls/1/reviews --input - --jq '.id'",
"printf '%s' '{ \"reviews\": [] }' | ao review submit --session sess-1 --reviews -",
} {
if !compoundCommandCovered(agent.got.AllowedTools, cmd) {
t.Fatalf("allowlist does not cover prompt-required command %q with tools %#v", cmd, agent.got.AllowedTools)
}
}
disallowed := "printf x | rm -rf /"
if compoundCommandCovered(agent.got.AllowedTools, disallowed) {
t.Fatalf("allowlist unexpectedly covers disallowed command %q with tools %#v", disallowed, agent.got.AllowedTools)
}
}
func compoundCommandCovered(allowedTools []string, cmd string) bool {
for _, segment := range splitPipedCommand(cmd) {
if !bashSegmentCovered(allowedTools, segment) {
return false
}
}
return true
}
func splitPipedCommand(cmd string) []string {
rawSegments := strings.Split(cmd, "|")
segments := make([]string, 0, len(rawSegments))
for _, segment := range rawSegments {
if trimmed := strings.TrimSpace(segment); trimmed != "" {
segments = append(segments, trimmed)
}
}
return segments
}
func bashSegmentCovered(allowedTools []string, segment string) bool {
for _, tool := range allowedTools {
cmd, ok := strings.CutPrefix(tool, "Bash(")
if !ok {
continue
}
cmd, ok = strings.CutSuffix(cmd, ":*)")
if !ok {
continue
}
if strings.HasPrefix(segment, cmd) {
return true
}
}
return false
}
func contains(values []string, needle string) bool { func contains(values []string, needle string) bool {
for _, v := range values { for _, v := range values {
if v == needle { if v == needle {

View File

@ -0,0 +1,78 @@
// Package codex adapts the codex worker agent for code-review sessions.
package codex
import (
"context"
"encoding/json"
"fmt"
"os"
workeragent "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/codex"
"github.com/aoagents/agent-orchestrator/backend/internal/domain"
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)
// Reviewer is the codex code-review adapter.
type Reviewer struct {
agent ports.Agent
}
// New builds the codex reviewer adapter.
func New() *Reviewer {
return &Reviewer{agent: workeragent.New()}
}
// Harness identifies this reviewer in the reviewer registry.
func (r *Reviewer) Harness() domain.ReviewerHarness {
return domain.ReviewerCodex
}
var _ ports.Reviewer = (*Reviewer)(nil)
// ReviewCommand launches the reviewer with an enforced read-only filesystem
// sandbox. Auto approval lets the headless session request the narrowly needed
// network access for posting the review and reporting its result.
func (r *Reviewer) ReviewCommand(ctx context.Context, inv ports.ReviewInvocation) (ports.ReviewCommandSpec, error) {
argv, err := r.agent.GetLaunchCommand(ctx, ports.LaunchConfig{
SessionID: inv.ReviewerID,
WorkspacePath: inv.WorkspacePath,
Prompt: inv.Prompt,
SystemPrompt: inv.SystemPrompt,
Permissions: ports.PermissionModeAuto,
})
if err != nil {
return ports.ReviewCommandSpec{}, err
}
extra := []string{"--sandbox", "read-only"}
// Shell commands inherit only Codex's core environment by default. Preserve
// the AO location overrides the reviewer needs to submit to this daemon.
for _, name := range []string{"AO_PORT", "AO_DATA_DIR", "AO_RUN_FILE"} {
value := os.Getenv(name)
if value == "" {
continue
}
encoded, err := json.Marshal(value)
if err != nil {
return ports.ReviewCommandSpec{}, fmt.Errorf("encode %s: %w", name, err)
}
extra = append(extra, "-c", "shell_environment_policy.set."+name+"="+string(encoded))
}
return ports.ReviewCommandSpec{Argv: insertBeforePrompt(argv, extra...)}, nil
}
// ReviewMessage returns the centrally-authored task for an existing pane.
func (r *Reviewer) ReviewMessage(_ context.Context, inv ports.ReviewInvocation) (string, error) {
return inv.Prompt, nil
}
func insertBeforePrompt(argv []string, extra ...string) []string {
for i, arg := range argv {
if arg == "--" {
out := make([]string, 0, len(argv)+len(extra))
out = append(out, argv[:i]...)
out = append(out, extra...)
return append(out, argv[i:]...)
}
}
return append(argv, extra...)
}

View File

@ -0,0 +1,77 @@
package codex
import (
"context"
"slices"
"testing"
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)
type captureAgent struct {
got ports.LaunchConfig
}
func (a *captureAgent) GetConfigSpec(context.Context) (ports.ConfigSpec, error) {
return ports.ConfigSpec{}, nil
}
func (a *captureAgent) GetLaunchCommand(_ context.Context, cfg ports.LaunchConfig) ([]string, error) {
a.got = cfg
return []string{"agent", "--", cfg.Prompt}, nil
}
func (a *captureAgent) GetPromptDeliveryStrategy(context.Context, ports.LaunchConfig) (ports.PromptDeliveryStrategy, error) {
return ports.PromptDeliveryInCommand, nil
}
func (a *captureAgent) GetAgentHooks(context.Context, ports.WorkspaceHookConfig) error { return nil }
func (a *captureAgent) GetRestoreCommand(context.Context, ports.RestoreConfig) ([]string, bool, error) {
return nil, false, nil
}
func (a *captureAgent) SessionInfo(context.Context, ports.SessionRef) (ports.SessionInfo, bool, error) {
return ports.SessionInfo{}, false, nil
}
func TestReviewCommandUsesReadOnlySandbox(t *testing.T) {
t.Setenv("AO_PORT", "3103")
t.Setenv("AO_DATA_DIR", "/tmp/ao data")
t.Setenv("AO_RUN_FILE", "/tmp/ao data/running.json")
agent := &captureAgent{}
r := &Reviewer{agent: agent}
got, err := r.ReviewCommand(context.Background(), ports.ReviewInvocation{
ReviewerID: "review-w1",
WorkspacePath: "/ws/w1",
Prompt: "review it",
SystemPrompt: "review only",
})
if err != nil {
t.Fatalf("ReviewCommand: %v", err)
}
want := []string{
"agent",
"--sandbox", "read-only",
"-c", `shell_environment_policy.set.AO_PORT="3103"`,
"-c", `shell_environment_policy.set.AO_DATA_DIR="/tmp/ao data"`,
"-c", `shell_environment_policy.set.AO_RUN_FILE="/tmp/ao data/running.json"`,
"--", "review it",
}
if !slices.Equal(got.Argv, want) {
t.Fatalf("argv = %#v, want %#v", got.Argv, want)
}
if agent.got.Permissions != ports.PermissionModeAuto {
t.Fatalf("permissions = %q, want auto", agent.got.Permissions)
}
if agent.got.SystemPrompt != "review only" {
t.Fatalf("system prompt = %q", agent.got.SystemPrompt)
}
}
func TestReviewMessageReturnsTaskPrompt(t *testing.T) {
got, err := (&Reviewer{}).ReviewMessage(context.Background(), ports.ReviewInvocation{Prompt: "next review"})
if err != nil {
t.Fatalf("ReviewMessage: %v", err)
}
if got != "next review" {
t.Fatalf("message = %q", got)
}
}

View File

@ -0,0 +1,56 @@
// Package opencode adapts the opencode worker agent for code-review sessions.
package opencode
import (
"context"
"strings"
workeragent "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/opencode"
"github.com/aoagents/agent-orchestrator/backend/internal/domain"
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)
const reviewerConfig = `{"permission":{"*":"deny","read":"allow","glob":"allow","grep":"allow","bash":{"*":"deny","gh api *":"allow","git diff*":"allow","git log*":"allow","git show*":"allow","git status*":"allow","ao review submit *":"allow","printf * | gh api *":"allow","printf * | ao review submit *":"allow"}}}`
// Reviewer is the opencode code-review adapter.
type Reviewer struct {
agent ports.Agent
}
// New builds the opencode reviewer adapter.
func New() *Reviewer {
return &Reviewer{agent: workeragent.New()}
}
// Harness identifies this reviewer in the reviewer registry.
func (r *Reviewer) Harness() domain.ReviewerHarness {
return domain.ReviewerOpenCode
}
var _ ports.Reviewer = (*Reviewer)(nil)
// ReviewCommand launches the reviewer with an inline permission policy that
// permits inspection and the two reporting commands while denying edits and
// every other tool. The system role is folded into the initial prompt because
// the worker CLI has no separate system-prompt flag.
func (r *Reviewer) ReviewCommand(ctx context.Context, inv ports.ReviewInvocation) (ports.ReviewCommandSpec, error) {
prompt := strings.TrimSpace(inv.SystemPrompt + "\n\n" + inv.Prompt)
argv, err := r.agent.GetLaunchCommand(ctx, ports.LaunchConfig{
SessionID: inv.ReviewerID,
WorkspacePath: inv.WorkspacePath,
Prompt: prompt,
Permissions: ports.PermissionModeAuto,
})
if err != nil {
return ports.ReviewCommandSpec{}, err
}
return ports.ReviewCommandSpec{
Argv: argv,
Env: map[string]string{"OPENCODE_CONFIG_CONTENT": reviewerConfig},
}, nil
}
// ReviewMessage returns the centrally-authored task for an existing pane.
func (r *Reviewer) ReviewMessage(_ context.Context, inv ports.ReviewInvocation) (string, error) {
return inv.Prompt, nil
}

View File

@ -0,0 +1,156 @@
package opencode
import (
"context"
"encoding/json"
"regexp"
"strings"
"testing"
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
)
type captureAgent struct {
got ports.LaunchConfig
}
func (a *captureAgent) GetConfigSpec(context.Context) (ports.ConfigSpec, error) {
return ports.ConfigSpec{}, nil
}
func (a *captureAgent) GetLaunchCommand(_ context.Context, cfg ports.LaunchConfig) ([]string, error) {
a.got = cfg
return []string{"agent", "--prompt", cfg.Prompt}, nil
}
func (a *captureAgent) GetPromptDeliveryStrategy(context.Context, ports.LaunchConfig) (ports.PromptDeliveryStrategy, error) {
return ports.PromptDeliveryInCommand, nil
}
func (a *captureAgent) GetAgentHooks(context.Context, ports.WorkspaceHookConfig) error { return nil }
func (a *captureAgent) GetRestoreCommand(context.Context, ports.RestoreConfig) ([]string, bool, error) {
return nil, false, nil
}
func (a *captureAgent) SessionInfo(context.Context, ports.SessionRef) (ports.SessionInfo, bool, error) {
return ports.SessionInfo{}, false, nil
}
func TestReviewCommandUsesReadOnlyPermissionPolicy(t *testing.T) {
agent := &captureAgent{}
r := &Reviewer{agent: agent}
got, err := r.ReviewCommand(context.Background(), ports.ReviewInvocation{
ReviewerID: "review-w1",
WorkspacePath: "/ws/w1",
Prompt: "review it",
SystemPrompt: "review only",
})
if err != nil {
t.Fatalf("ReviewCommand: %v", err)
}
if agent.got.Prompt != "review only\n\nreview it" {
t.Fatalf("prompt = %q", agent.got.Prompt)
}
if agent.got.Permissions != ports.PermissionModeAuto {
t.Fatalf("permissions = %q, want auto", agent.got.Permissions)
}
config := map[string]any{}
if err := json.Unmarshal([]byte(got.Env["OPENCODE_CONFIG_CONTENT"]), &config); err != nil {
t.Fatalf("inline config is invalid JSON: %v", err)
}
permission := config["permission"].(map[string]any)
if permission["*"] != "deny" || permission["read"] != "allow" {
t.Fatalf("permission policy = %#v", permission)
}
bash := permission["bash"].(map[string]any)
if bash["*"] != "deny" || bash["gh api *"] != "allow" || bash["ao review submit *"] != "allow" {
t.Fatalf("bash policy = %#v", bash)
}
}
func TestBashAllowlistCoversPromptRequiredCommands(t *testing.T) {
bash := reviewerConfigBashPolicy(t)
tests := []struct {
name string
command string
allowed bool
}{
{
name: "github review creation",
command: `printf '%s' '{ "event": "COMMENT", "body": "x" }' | gh api --method POST repos/o/r/pulls/1/reviews --input - --jq '.id'`,
allowed: true,
},
{
name: "local review submit",
command: `printf '%s' '{ "reviews": [] }' | ao review submit --session sess-1 --reviews -`,
allowed: true,
},
{
name: "arbitrary shell command",
command: `rm -rf /`,
allowed: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := bashAllowsCommand(t, bash, tt.command); got != tt.allowed {
t.Fatalf("bashAllowsCommand(%q) = %v, want %v", tt.command, got, tt.allowed)
}
})
}
}
func TestReviewMessageReturnsTaskPrompt(t *testing.T) {
got, err := (&Reviewer{}).ReviewMessage(context.Background(), ports.ReviewInvocation{Prompt: "next review"})
if err != nil {
t.Fatalf("ReviewMessage: %v", err)
}
if got != "next review" {
t.Fatalf("message = %q", got)
}
}
func reviewerConfigBashPolicy(t *testing.T) map[string]string {
t.Helper()
var config struct {
Permission struct {
Bash map[string]string `json:"bash"`
} `json:"permission"`
}
if err := json.Unmarshal([]byte(reviewerConfig), &config); err != nil {
t.Fatalf("reviewerConfig is invalid JSON: %v", err)
}
if len(config.Permission.Bash) == 0 {
t.Fatal("reviewerConfig permission.bash is empty")
}
return config.Permission.Bash
}
func bashAllowsCommand(t *testing.T, bash map[string]string, command string) bool {
t.Helper()
for pattern, action := range bash {
if action == "deny" {
continue
}
if simplePicomatchGlobMatches(t, pattern, command) {
return true
}
}
return false
}
func simplePicomatchGlobMatches(t *testing.T, pattern, command string) bool {
t.Helper()
parts := strings.Split(pattern, "*")
for i, part := range parts {
parts[i] = regexp.QuoteMeta(part)
}
re, err := regexp.Compile("^" + strings.Join(parts, ".*") + "$")
if err != nil {
t.Fatalf("compile pattern %q: %v", pattern, err)
}
return re.MatchString(command)
}

View File

@ -1,13 +1,14 @@
// Package reviewer is the single source of truth for the code-review adapters // Package reviewer is the single source of truth for the code-review adapters
// the daemon ships. It mirrors the worker agent registry but is a separate set: // the daemon ships. It mirrors the worker agent registry but is a separate set:
// adding a reviewer (claude-code today, greptile tomorrow) is one edit here and // adding a reviewer here does not widen the worker AgentHarness vocabulary.
// does not widen the worker AgentHarness vocabulary.
package reviewer package reviewer
import ( import (
"fmt" "fmt"
"github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer/claudecode" "github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer/claudecode"
"github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer/codex"
"github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer/opencode"
"github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/domain"
"github.com/aoagents/agent-orchestrator/backend/internal/ports" "github.com/aoagents/agent-orchestrator/backend/internal/ports"
) )
@ -23,6 +24,8 @@ type Adapter interface {
func Constructors() []Adapter { func Constructors() []Adapter {
return []Adapter{ return []Adapter{
claudecode.New(), claudecode.New(),
codex.New(),
opencode.New(),
} }
} }

View File

@ -24,8 +24,10 @@ func TestProjectConfigValidate(t *testing.T) {
{"symlink embedded parent", ProjectConfig{Symlinks: []string{"a/../../b"}}, true}, {"symlink embedded parent", ProjectConfig{Symlinks: []string{"a/../../b"}}, true},
{"symlink bare ..", ProjectConfig{Symlinks: []string{".."}}, true}, {"symlink bare ..", ProjectConfig{Symlinks: []string{".."}}, true},
{"good reviewers", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerClaudeCode}}}, false}, {"good reviewers", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerClaudeCode}}}, false},
{"good codex reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerCodex}}}, false},
{"good opencode reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerOpenCode}}}, false},
{"unknown reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: "nope"}}}, true}, {"unknown reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: "nope"}}}, true},
{"worker harness is not auto a reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerHarness(HarnessCodex)}}}, true}, {"worker-only harness is not auto a reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerHarness(HarnessAider)}}}, true},
{"empty reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ""}}}, true}, {"empty reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ""}}}, true},
} }
for _, tt := range tests { for _, tt := range tests {
@ -80,10 +82,17 @@ func TestResolveReviewerHarness(t *testing.T) {
t.Fatalf("configured reviewer = %q, want claude-code", got) t.Fatalf("configured reviewer = %q, want claude-code", got)
} }
// No reviewer configured: always use claude-code. // No reviewer configured: always use claude-code, regardless of the worker
// harness (see #2241).
if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessClaudeCode); got != ReviewerClaudeCode { if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessClaudeCode); got != ReviewerClaudeCode {
t.Fatalf("default = %q, want reviewer claude-code", got) t.Fatalf("default = %q, want reviewer claude-code", got)
} }
if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessCodex); got != ReviewerClaudeCode {
t.Fatalf("default = %q, want reviewer claude-code", got)
}
if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessOpenCode); got != ReviewerClaudeCode {
t.Fatalf("default = %q, want reviewer claude-code", got)
}
// A worker harness that is not claude-code also falls back to claude-code. // A worker harness that is not claude-code also falls back to claude-code.
if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessAider); got != FallbackReviewerHarness { if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessAider); got != FallbackReviewerHarness {

View File

@ -4,19 +4,23 @@ package domain
// from AgentHarness on purpose: a reviewer-only tool (e.g. the Greptile CLI) // from AgentHarness on purpose: a reviewer-only tool (e.g. the Greptile CLI)
// must not become a valid worker, and a worker harness does not automatically // must not become a valid worker, and a worker harness does not automatically
// become a valid reviewer. The two sets are maintained independently and only // become a valid reviewer. The two sets are maintained independently and only
// happen to share ids where the same tool serves both roles (claude-code). // happen to share ids where the same tool serves both roles.
type ReviewerHarness string type ReviewerHarness string
// Supported reviewer harnesses. Add a reviewer-only tool here (and register its // Supported reviewer harnesses. Add a reviewer-only tool here (and register its
// adapter) without widening the worker AgentHarness set. // adapter) without widening the worker AgentHarness set.
const ( const (
ReviewerClaudeCode ReviewerHarness = "claude-code" ReviewerClaudeCode ReviewerHarness = "claude-code"
ReviewerCodex ReviewerHarness = "codex"
ReviewerOpenCode ReviewerHarness = "opencode"
) )
// AllReviewerHarnesses is the canonical set used to validate a configured // AllReviewerHarnesses is the canonical set used to validate a configured
// reviewer harness. // reviewer harness.
var AllReviewerHarnesses = []ReviewerHarness{ var AllReviewerHarnesses = []ReviewerHarness{
ReviewerClaudeCode, ReviewerClaudeCode,
ReviewerCodex,
ReviewerOpenCode,
} }
// IsKnown reports whether h is one of the supported reviewer harnesses. // IsKnown reports whether h is one of the supported reviewer harnesses.

View File

@ -30,23 +30,15 @@ Complete every review task in the queue autonomously. Do not ask the user whethe
Do these steps in order: Do these steps in order:
1. For each PR below, post a separate review on that pull request and capture its id in one call. Post with `+"`gh api`"+` rather than `+"`gh pr review`"+`: it is the only way to attach inline comments, and its response carries the created review's id, so AO can tell the worker exactly which review to address. Send the review as a JSON body so the inline comments form a proper array of objects: 1. For each PR below, post a separate review on that pull request and capture its id in one call. Post with `+"`gh api`"+` rather than `+"`gh pr review`"+`: it is the only way to attach inline comments, and its response carries the created review's id, so AO can tell the worker exactly which review to address. Send the review as a JSON body so the inline comments form a proper array of objects:
gh api --method POST repos/{owner}/{repo}/pulls/{number}/reviews --input - --jq '.id' <<'JSON' printf '%%s' '{ "event": "COMMENT", "body": "<summary>", "comments": [ { "path": "<file>", "line": <n>, "body": "<finding>" } ] }' | gh api --method POST repos/{owner}/{repo}/pulls/{number}/reviews --input - --jq '.id'
{ "event": "COMMENT", "body": "<summary>",
"comments": [ { "path": "<file>", "line": <n>, "body": "<finding>" } ] }
JSON
- Substitute the PR's owner/repo/number. Add one object to "comments" per inline finding; omit the field for a review with no inline comments. - Substitute the PR's owner/repo/number. Add one object to "comments" per inline finding; omit the field for a review with no inline comments.
- Keep the JSON on one line and shell-escape any single quotes in review text before passing it to printf; do not use a heredoc because reviewer panes run through an interactive PTY.
- Always use "event": "COMMENT": reviews are posted from the PR author's own account, and GitHub rejects both APPROVE and REQUEST_CHANGES on your own PR. State in the body whether you are requesting changes or approving; the machine-readable verdict goes to AO in step 2. - Always use "event": "COMMENT": reviews are posted from the PR author's own account, and GitHub rejects both APPROVE and REQUEST_CHANGES on your own PR. State in the body whether you are requesting changes or approving; the machine-readable verdict goes to AO in step 2.
- The printed number is the review id. If the call fails on the provider, leave the id empty. - The printed number is the review id. If the call fails on the provider, leave the id empty.
2. After every PR has its own GitHub review from step 1, record AO's bookkeeping for those already-posted reviews using one command. Pass JSON on stdin so nothing is ever written into the worktree (a file there could be committed onto the worker's branch). Include one object per PR/run from the queue: 2. After every PR has its own GitHub review from step 1, record AO's bookkeeping for those already-posted reviews using one command. Pass JSON on stdin so nothing is ever written into the worktree (a file there could be committed onto the worker's branch). Include one object per PR/run from the queue:
ao review submit --session %s --reviews - <<'JSON' printf '%%s' '{ "reviews": [ { "runId": "<run-id>", "verdict": "<approved|changes_requested>", "githubReviewId": "<id-from-step-1-or-empty>", "body": "<your full review markdown>" } ] }' | ao review submit --session %s --reviews -
{
"reviews": [
{ "runId": "<run-id>", "verdict": "<approved|changes_requested>", "githubReviewId": "<id-from-step-1-or-empty>", "body": "<your full review markdown>" }
]
}
JSON
Only if step 1 genuinely fails on the provider for a PR, still include that run in step 2 with an empty githubReviewId so the result is recorded.`, Only if step 1 genuinely fails on the provider for a PR, still include that run in step 2 with an empty githubReviewId so the result is recorded.`,
spec.WorkerID, queueText, spec.WorkerID) spec.WorkerID, queueText, spec.WorkerID)

View File

@ -27,6 +27,8 @@ func TestReviewTextsIncludesMultiPRQueue(t *testing.T) {
"* 1. https://github.com/o/r/pull/1 (head commit sha1, run run-1)", "* 1. https://github.com/o/r/pull/1 (head commit sha1, run run-1)",
"* 2. https://github.com/o/r/pull/2 (head commit sha2, run run-2)", "* 2. https://github.com/o/r/pull/2 (head commit sha2, run run-2)",
"After every PR has its own GitHub review from step 1", "After every PR has its own GitHub review from step 1",
"printf '%s'",
"do not use a heredoc",
"ao review submit --session mer-1 --reviews -", "ao review submit --session mer-1 --reviews -",
`"reviews": [`, `"reviews": [`,
} { } {

View File

@ -142,4 +142,4 @@ export const RequiredAgentField = memo(function RequiredAgentField({
</Select> </Select>
</div> </div>
); );
}); });

View File

@ -170,6 +170,35 @@ describe("ProjectSettingsForm", () => {
expect(screen.queryByText("Saved.")).not.toBeInTheDocument(); expect(screen.queryByText("Saved.")).not.toBeInTheDocument();
}); });
it("offers every supported reviewer harness", async () => {
getMock.mockResolvedValue({
data: {
status: "ok",
project: {
id: "proj-1",
name: "Project One",
kind: "single_repo",
path: "/repo/project-one",
repo: "",
defaultBranch: "main",
config: {
worker: { agent: "codex" },
orchestrator: { agent: "claude-code" },
},
},
},
error: undefined,
});
renderSettings();
const reviewerAgent = await screen.findByRole("combobox", { name: "Default reviewer agent" });
await userEvent.click(reviewerAgent);
for (const option of ["claude-code (default)", "claude-code", "codex", "opencode"]) {
expect(await screen.findByRole("option", { name: option })).toBeInTheDocument();
}
});
it("defaults worker and orchestrator to claude-code for projects missing role config", async () => { it("defaults worker and orchestrator to claude-code for projects missing role config", async () => {
getMock.mockResolvedValue({ getMock.mockResolvedValue({
data: { data: {
@ -210,4 +239,4 @@ describe("ProjectSettingsForm", () => {
}); });
expect(await screen.findByText("Saved.")).toBeInTheDocument(); expect(await screen.findByText("Saved.")).toBeInTheDocument();
}); });
}); });

View File

@ -21,7 +21,7 @@ const PERMISSION_MODE_OPTIONS = [
{ value: "bypass-permissions", label: "Bypass permissions" }, { value: "bypass-permissions", label: "Bypass permissions" },
] as const; ] as const;
const REVIEWER_OPTIONS = ["claude-code"] as const; const REVIEWER_OPTIONS = ["claude-code", "codex", "opencode"] as const;
const projectQueryKey = (id: string) => ["project", id] as const; const projectQueryKey = (id: string) => ["project", id] as const;
@ -300,4 +300,4 @@ function CenteredNote({ children }: { children: React.ReactNode }) {
// rather than an empty {} the daemon would persist. // rather than an empty {} the daemon would persist.
function blankToUndefined<T extends object>(obj: T): T | undefined { function blankToUndefined<T extends object>(obj: T): T | undefined {
return Object.values(obj).some((v) => v !== undefined) ? obj : undefined; return Object.values(obj).some((v) => v !== undefined) ? obj : undefined;
} }

View File

@ -26,4 +26,4 @@ export const AGENT_OPTIONS = [
// The agent new projects use by default, and the fallback for worker/orchestrator // The agent new projects use by default, and the fallback for worker/orchestrator
// role fields that have no explicit configuration. Users can change it per project. // role fields that have no explicit configuration. Users can change it per project.
export const DEFAULT_PROJECT_AGENT: (typeof AGENT_OPTIONS)[number] = "claude-code"; export const DEFAULT_PROJECT_AGENT: (typeof AGENT_OPTIONS)[number] = "claude-code";