Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions backend/internal/adapters/reviewer/claudecode/claudecode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 21 additions & 1 deletion backend/internal/review/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -97,14 +99,32 @@ 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)
}
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 {
Expand Down
8 changes: 6 additions & 2 deletions backend/internal/review/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 5 additions & 4 deletions backend/internal/session_manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions backend/internal/session_manager/provision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down
Loading