diff --git a/backend/internal/adapters/reviewer/claudecode/claudecode.go b/backend/internal/adapters/reviewer/claudecode/claudecode.go index e9c567905..159d67a67 100644 --- a/backend/internal/adapters/reviewer/claudecode/claudecode.go +++ b/backend/internal/adapters/reviewer/claudecode/claudecode.go @@ -37,12 +37,14 @@ var _ ports.Reviewer = (*Reviewer)(nil) // system entirely and ignores allow/deny rules — it launches in the default // 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 -// needs (git diff/log/show to inspect the PR, gh to post the review, and -// `ao review submit` to record the verdict) without stalling. +// needs (git diff/log/show to inspect the PR, printf to pipe review JSON into +// 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{ "Read", "Grep", "Glob", + "Bash(printf:*)", "Bash(gh:*)", "Bash(git diff:*)", "Bash(git log:*)", diff --git a/backend/internal/adapters/reviewer/claudecode/claudecode_test.go b/backend/internal/adapters/reviewer/claudecode/claudecode_test.go index 53b1bb5f4..50a394837 100644 --- a/backend/internal/adapters/reviewer/claudecode/claudecode_test.go +++ b/backend/internal/adapters/reviewer/claudecode/claudecode_test.go @@ -2,6 +2,7 @@ package claudecode import ( "context" + "strings" "testing" "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 { for _, v := range values { if v == needle { diff --git a/backend/internal/adapters/reviewer/codex/codex.go b/backend/internal/adapters/reviewer/codex/codex.go new file mode 100644 index 000000000..a166b78c7 --- /dev/null +++ b/backend/internal/adapters/reviewer/codex/codex.go @@ -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...) +} diff --git a/backend/internal/adapters/reviewer/codex/codex_test.go b/backend/internal/adapters/reviewer/codex/codex_test.go new file mode 100644 index 000000000..9912c6c24 --- /dev/null +++ b/backend/internal/adapters/reviewer/codex/codex_test.go @@ -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) + } +} diff --git a/backend/internal/adapters/reviewer/opencode/opencode.go b/backend/internal/adapters/reviewer/opencode/opencode.go new file mode 100644 index 000000000..2ffd3b5a3 --- /dev/null +++ b/backend/internal/adapters/reviewer/opencode/opencode.go @@ -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 +} diff --git a/backend/internal/adapters/reviewer/opencode/opencode_test.go b/backend/internal/adapters/reviewer/opencode/opencode_test.go new file mode 100644 index 000000000..9086ed58a --- /dev/null +++ b/backend/internal/adapters/reviewer/opencode/opencode_test.go @@ -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) +} diff --git a/backend/internal/adapters/reviewer/registry.go b/backend/internal/adapters/reviewer/registry.go index b5fbcb7d0..dccc77e36 100644 --- a/backend/internal/adapters/reviewer/registry.go +++ b/backend/internal/adapters/reviewer/registry.go @@ -1,13 +1,14 @@ // 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: -// adding a reviewer (claude-code today, greptile tomorrow) is one edit here and -// does not widen the worker AgentHarness vocabulary. +// adding a reviewer here does not widen the worker AgentHarness vocabulary. package reviewer import ( "fmt" "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/ports" ) @@ -23,6 +24,8 @@ type Adapter interface { func Constructors() []Adapter { return []Adapter{ claudecode.New(), + codex.New(), + opencode.New(), } } diff --git a/backend/internal/domain/projectconfig_test.go b/backend/internal/domain/projectconfig_test.go index 4ad643d1c..2542c5653 100644 --- a/backend/internal/domain/projectconfig_test.go +++ b/backend/internal/domain/projectconfig_test.go @@ -24,8 +24,10 @@ func TestProjectConfigValidate(t *testing.T) { {"symlink embedded parent", ProjectConfig{Symlinks: []string{"a/../../b"}}, true}, {"symlink bare ..", ProjectConfig{Symlinks: []string{".."}}, true}, {"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}, - {"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}, } for _, tt := range tests { @@ -80,10 +82,17 @@ func TestResolveReviewerHarness(t *testing.T) { 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 { 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. if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessAider); got != FallbackReviewerHarness { diff --git a/backend/internal/domain/reviewerharness.go b/backend/internal/domain/reviewerharness.go index 760be13ec..ed1c69027 100644 --- a/backend/internal/domain/reviewerharness.go +++ b/backend/internal/domain/reviewerharness.go @@ -4,19 +4,23 @@ package domain // 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 // 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 // Supported reviewer harnesses. Add a reviewer-only tool here (and register its // adapter) without widening the worker AgentHarness set. const ( ReviewerClaudeCode ReviewerHarness = "claude-code" + ReviewerCodex ReviewerHarness = "codex" + ReviewerOpenCode ReviewerHarness = "opencode" ) // AllReviewerHarnesses is the canonical set used to validate a configured // reviewer harness. var AllReviewerHarnesses = []ReviewerHarness{ ReviewerClaudeCode, + ReviewerCodex, + ReviewerOpenCode, } // IsKnown reports whether h is one of the supported reviewer harnesses. diff --git a/backend/internal/review/prompt.go b/backend/internal/review/prompt.go index 42dfca8c0..a7af5b76a 100644 --- a/backend/internal/review/prompt.go +++ b/backend/internal/review/prompt.go @@ -30,23 +30,15 @@ Complete every review task in the queue autonomously. Do not ask the user whethe 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: - gh api --method POST repos/{owner}/{repo}/pulls/{number}/reviews --input - --jq '.id' <<'JSON' - { "event": "COMMENT", "body": "", - "comments": [ { "path": "", "line": , "body": "" } ] } - JSON + printf '%%s' '{ "event": "COMMENT", "body": "", "comments": [ { "path": "", "line": , "body": "" } ] }' | gh api --method POST repos/{owner}/{repo}/pulls/{number}/reviews --input - --jq '.id' - 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. - 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: - ao review submit --session %s --reviews - <<'JSON' - { - "reviews": [ - { "runId": "", "verdict": "", "githubReviewId": "", "body": "" } - ] - } - JSON + printf '%%s' '{ "reviews": [ { "runId": "", "verdict": "", "githubReviewId": "", "body": "" } ] }' | ao review submit --session %s --reviews - 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) diff --git a/backend/internal/review/prompt_test.go b/backend/internal/review/prompt_test.go index c2428782e..ba3a472a4 100644 --- a/backend/internal/review/prompt_test.go +++ b/backend/internal/review/prompt_test.go @@ -27,6 +27,8 @@ func TestReviewTextsIncludesMultiPRQueue(t *testing.T) { "* 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)", "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 -", `"reviews": [`, } { diff --git a/frontend/src/renderer/components/CreateProjectAgentSheet.tsx b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx index 76f6054ed..a2c52fe73 100644 --- a/frontend/src/renderer/components/CreateProjectAgentSheet.tsx +++ b/frontend/src/renderer/components/CreateProjectAgentSheet.tsx @@ -142,4 +142,4 @@ export const RequiredAgentField = memo(function RequiredAgentField({ ); -}); \ No newline at end of file +}); diff --git a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx index 26776054d..41bb524ac 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.test.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.test.tsx @@ -170,6 +170,35 @@ describe("ProjectSettingsForm", () => { 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 () => { getMock.mockResolvedValue({ data: { @@ -210,4 +239,4 @@ describe("ProjectSettingsForm", () => { }); expect(await screen.findByText("Saved.")).toBeInTheDocument(); }); -}); \ No newline at end of file +}); diff --git a/frontend/src/renderer/components/ProjectSettingsForm.tsx b/frontend/src/renderer/components/ProjectSettingsForm.tsx index ea51b7fd4..c6be21a19 100644 --- a/frontend/src/renderer/components/ProjectSettingsForm.tsx +++ b/frontend/src/renderer/components/ProjectSettingsForm.tsx @@ -21,7 +21,7 @@ const PERMISSION_MODE_OPTIONS = [ { value: "bypass-permissions", label: "Bypass permissions" }, ] 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; @@ -300,4 +300,4 @@ function CenteredNote({ children }: { children: React.ReactNode }) { // rather than an empty {} the daemon would persist. function blankToUndefined(obj: T): T | undefined { return Object.values(obj).some((v) => v !== undefined) ? obj : undefined; -} \ No newline at end of file +} diff --git a/frontend/src/renderer/lib/agent-options.ts b/frontend/src/renderer/lib/agent-options.ts index 7cdc56eee..39ade7eae 100644 --- a/frontend/src/renderer/lib/agent-options.ts +++ b/frontend/src/renderer/lib/agent-options.ts @@ -26,4 +26,4 @@ export const AGENT_OPTIONS = [ // 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. -export const DEFAULT_PROJECT_AGENT: (typeof AGENT_OPTIONS)[number] = "claude-code"; \ No newline at end of file +export const DEFAULT_PROJECT_AGENT: (typeof AGENT_OPTIONS)[number] = "claude-code";