fix(review): reviewer posts to GitHub and records its verdict autonomously (#259)

* fix(review): make the reviewer post to GitHub and record its verdict autonomously

The claude-code reviewer never completed a review on its own. Three defects
in the reviewer launch + flow:

- It launched with no permission mode, so a headless pane stalled on the
  first tool-permission prompt and never ran gh/ao. Launch with
  bypassPermissions (read-only is enforced by the prompt, not a sandbox).
- The reviewer pane got no pinned PATH, so `ao review submit` resolved to a
  foreign `ao` on the inherited PATH and failed. Pin PATH to the daemon's
  own dir the same way worker sessions do — export HookPATH and reuse it in
  the launcher.
- The prompt did not enforce ordering. Make it post the review on the PR
  via gh first, then run `ao review submit`.

Fixes #258

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

* fix(review): fall back to a comment review when self-approval is rejected

GitHub does not let an author approve their own PR, so a reviewer running
under the same account can't post an `approve`. Tell the reviewer to post
the approval as a regular comment review (COMMENT event stating it is an
approval) when the provider rejects the self-approval, instead of failing.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

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

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 {

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 <approved|changes_requested> --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
 }

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.
+// prepending its directory would not change what `ao` resolves to. Exported so
-func hookPATH(executable func() (string, error), getenv func(string) string, projectEnv map[string]string) (string, error) {
+// 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)

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)
 			}
 		})
 	}