diff --git a/backend/internal/adapters/reviewer/claudecode/claudecode.go b/backend/internal/adapters/reviewer/claudecode/claudecode.go index 5573553..8f595e5 100644 --- a/backend/internal/adapters/reviewer/claudecode/claudecode.go +++ b/backend/internal/adapters/reviewer/claudecode/claudecode.go @@ -39,6 +39,10 @@ func (r *Reviewer) ReviewCommand(ctx context.Context, inv ports.ReviewInvocation WorkspacePath: inv.WorkspacePath, Prompt: inv.Prompt, SystemPrompt: inv.SystemPrompt, + // The reviewer runs headless with no human to approve tool prompts; it + // is read-only by prompt and must run gh/ao on its own, so bypass the + // permission gate rather than stall on the first prompt. + Permissions: ports.PermissionModeBypassPermissions, }) if err != nil { return ports.ReviewCommandSpec{}, err diff --git a/backend/internal/review/launcher.go b/backend/internal/review/launcher.go index 28e312f..fef9d0b 100644 --- a/backend/internal/review/launcher.go +++ b/backend/internal/review/launcher.go @@ -3,9 +3,11 @@ package review import ( "context" "fmt" + "os" "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/ports" + sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager" ) // Launcher spawns, re-notifies, and probes a reviewer over a worker's worktree. @@ -97,7 +99,7 @@ func (l *agentLauncher) Spawn(ctx context.Context, spec LaunchSpec) (string, err SessionID: domain.SessionID(handleID), WorkspacePath: spec.WorkspacePath, Argv: cmd.Argv, - Env: cmd.Env, + Env: pinnedEnv(cmd.Env), }) if err != nil { return "", fmt.Errorf("reviewer runtime: %w", err) @@ -105,6 +107,24 @@ func (l *agentLauncher) Spawn(ctx context.Context, spec LaunchSpec) (string, err return handle.ID, nil } +// pinnedEnv returns the reviewer command's env with PATH pinned to the daemon's +// own directory, so the bare `ao` the reviewer runs (e.g. `ao review submit`) +// resolves to this daemon's CLI rather than a foreign `ao` first on the +// inherited PATH. Mirrors the worker-session pin in the session manager. +// Best-effort: an unpinnable daemon (not named "ao") keeps the inherited PATH. +func pinnedEnv(base map[string]string) map[string]string { + path, err := sessionmanager.HookPATH(os.Executable, os.Getenv, base) + if err != nil { + return base + } + env := make(map[string]string, len(base)+1) + for k, v := range base { + env[k] = v + } + env["PATH"] = path + return env +} + func (l *agentLauncher) Notify(ctx context.Context, handleID string, spec LaunchSpec) error { reviewer, ok := l.reviewers.Reviewer(spec.Harness) if !ok { diff --git a/backend/internal/review/prompt.go b/backend/internal/review/prompt.go index a054fbf..1a5ee24 100644 --- a/backend/internal/review/prompt.go +++ b/backend/internal/review/prompt.go @@ -20,11 +20,15 @@ Post your review on the pull request using the available review tooling (request prompt = fmt.Sprintf(`Review pull request %s (head commit %s). -When done, write your full review to review.md and record the result with AO by running exactly: +Do these steps in order: +1. Post your review on the pull request with `+"`gh`"+`, with inline comments for specific findings: + - If changes are needed, request changes. + - If it is ready, approve it. GitHub does not let you approve a PR you opened — if the approval is rejected because you are the PR author, post the same review as a regular comment instead (a COMMENT-event review whose body states it is an approval). +2. Write your full review to review.md and record the result with AO by running exactly: ao review submit --session %s --run %s --verdict --body review.md -If you cannot post the review on the provider, still run the command above so the result is recorded.`, +Only if step 1 genuinely fails on the provider, still run step 2 so the result is recorded.`, spec.PRURL, spec.TargetSHA, spec.WorkerID, spec.RunID) return prompt, systemPrompt } diff --git a/backend/internal/session_manager/manager.go b/backend/internal/session_manager/manager.go index af1aa59..624b104 100644 --- a/backend/internal/session_manager/manager.go +++ b/backend/internal/session_manager/manager.go @@ -720,7 +720,7 @@ func spawnEnv(id domain.SessionID, project domain.ProjectID, issue domain.IssueI // logged so the degradation isn't silent. func (m *Manager) runtimeEnv(id domain.SessionID, project domain.ProjectID, issue domain.IssueID, projectEnv map[string]string) map[string]string { env := spawnEnv(id, project, issue, m.dataDir, projectEnv) - path, err := hookPATH(m.executable, os.Getenv, projectEnv) + path, err := HookPATH(m.executable, os.Getenv, projectEnv) if err != nil { m.logger.Warn("session PATH not pinned to the daemon binary; `ao hooks` callbacks may resolve to a different ao and activity tracking will stall", "session", id, "error", err) @@ -730,13 +730,14 @@ func (m *Manager) runtimeEnv(id domain.SessionID, project domain.ProjectID, issu return env } -// hookPATH builds the PATH value pinned into a spawned session: the daemon +// HookPATH builds the PATH value pinned into a spawned session: the daemon // executable's directory prepended to the base PATH (the project's PATH // override when set, else the daemon's inherited PATH — matching what the // runtime would have exported anyway). An error means the pin cannot be // applied: the executable is unresolvable, or is not named "ao", in which case -// prepending its directory would not change what `ao` resolves to. -func hookPATH(executable func() (string, error), getenv func(string) string, projectEnv map[string]string) (string, error) { +// prepending its directory would not change what `ao` resolves to. Exported so +// the reviewer launcher can pin its pane's PATH the same way. +func HookPATH(executable func() (string, error), getenv func(string) string, projectEnv map[string]string) (string, error) { exe, err := executable() if err != nil { return "", fmt.Errorf("resolve daemon executable: %w", err) diff --git a/backend/internal/session_manager/provision_test.go b/backend/internal/session_manager/provision_test.go index ba04606..d82d6e3 100644 --- a/backend/internal/session_manager/provision_test.go +++ b/backend/internal/session_manager/provision_test.go @@ -82,18 +82,18 @@ func TestHookPATH(t *testing.T) { } return "" } - got, err := hookPATH(tc.executable, getenv, tc.projectEnv) + got, err := HookPATH(tc.executable, getenv, tc.projectEnv) if tc.wantErr { if err == nil { - t.Fatalf("hookPATH = %q, want error", got) + t.Fatalf("HookPATH = %q, want error", got) } return } if err != nil { - t.Fatalf("hookPATH: %v", err) + t.Fatalf("HookPATH: %v", err) } if got != tc.want { - t.Fatalf("hookPATH = %q, want %q", got, tc.want) + t.Fatalf("HookPATH = %q, want %q", got, tc.want) } }) }