feat(review): configurable AO code review backend (V1) (#192)

backend/internal/adapters/reviewer/claudecode/claudecode.go

@@ -0,0 +1,68 @@
+// Package claudecode is the claude-code reviewer adapter. claude-code is a
+// prompt-driven agent, so this reviewer builds a review prompt and reuses the
+// worker claude-code adapter's launch-command construction (binary resolution,
+// flags). The reviewer contract itself stays prompt-agnostic, so a one-shot CLI
+// reviewer (e.g. greptile) can implement it without a prompt.
+package claudecode
+
+import (
+	"context"
+	"fmt"
+
+	workeragent "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/claudecode"
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
+)
+
+// Reviewer is the claude-code code-review adapter.
+type Reviewer struct {
+	agent ports.Agent
+}
+
+// New builds the claude-code 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.ReviewerClaudeCode
+}
+
+var _ ports.Reviewer = (*Reviewer)(nil)
+
+// ReviewCommand builds a claude-code invocation that reviews the worker's
+// checkout for the PR, with the review prompt baked in.
+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:        reviewPrompt(inv),
+	})
+	if err != nil {
+		return ports.ReviewCommandSpec{}, err
+	}
+	return ports.ReviewCommandSpec{Argv: argv}, nil
+}
+
+// ReviewMessage is the text injected into an already-running reviewer pane to
+// review a new commit. It carries the same explicit instructions as the spawn
+// prompt.
+func (r *Reviewer) ReviewMessage(_ context.Context, inv ports.ReviewInvocation) (string, error) {
+	return reviewPrompt(inv), nil
+}
+
+func reviewPrompt(inv ports.ReviewInvocation) string {
+	return fmt.Sprintf(`You are an AO code reviewer. The current working directory is a checkout containing the changes for pull request %s (head commit %s). Review only this PR's changes — do not start unrelated work.
+
+Steps:
+1. Inspect what the PR changed by diffing the checkout against the PR's base branch.
+2. Review for correctness bugs, missing error handling, security issues, test coverage, and clear deviations from the surrounding code's conventions. Prefer a few high-confidence findings over nitpicks.
+3. Post your review on the pull request using the available review tooling (request changes if it needs work, approve if it is ready), with inline comments for specific findings.
+4. Record the outcome with AO so the worker is nudged: write your full review to review.md, then run exactly:
+
+     ao review submit --session %s --run %s --verdict <approved|changes_requested> --body review.md
+
+Constraints: do not push commits, edit files, or modify the branch — review only. If you cannot post the review, still run the `+"`ao review submit`"+` command above so the result is recorded.`,
+		inv.PRURL, inv.TargetSHA, inv.WorkerSessionID, inv.RunID)
+}

backend/internal/adapters/reviewer/registry.go

@@ -0,0 +1,58 @@
+// 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.
+package reviewer
+
+import (
+	"fmt"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer/claudecode"
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
+)
+
+// Adapter is a registered reviewer: a ports.Reviewer that names its harness.
+type Adapter interface {
+	ports.Reviewer
+	Harness() domain.ReviewerHarness
+}
+
+// Constructors returns every reviewer adapter the daemon ships. Add a reviewer
+// here (and to domain.AllReviewerHarnesses) to register it.
+func Constructors() []Adapter {
+	return []Adapter{
+		claudecode.New(),
+	}
+}
+
+// Resolver maps a reviewer harness onto its adapter.
+type Resolver struct {
+	reviewers map[domain.ReviewerHarness]ports.Reviewer
+}
+
+var _ ports.ReviewerResolver = (*Resolver)(nil)
+
+// NewResolver builds a Resolver from the shipped reviewer adapters. It fails if
+// two adapters claim the same harness, or if a registered harness is not in the
+// domain reviewer vocabulary (the two must stay in sync).
+func NewResolver() (*Resolver, error) {
+	m := make(map[domain.ReviewerHarness]ports.Reviewer)
+	for _, a := range Constructors() {
+		h := a.Harness()
+		if !h.IsKnown() {
+			return nil, fmt.Errorf("reviewer adapter %q is not in domain.AllReviewerHarnesses", h)
+		}
+		if _, dup := m[h]; dup {
+			return nil, fmt.Errorf("reviewer harness %q is registered twice", h)
+		}
+		m[h] = a
+	}
+	return &Resolver{reviewers: m}, nil
+}
+
+// Reviewer returns the adapter for a harness, ok=false when none is registered.
+func (r *Resolver) Reviewer(harness domain.ReviewerHarness) (ports.Reviewer, bool) {
+	rv, ok := r.reviewers[harness]
+	return rv, ok
+}

backend/internal/adapters/reviewer/registry_test.go

@@ -0,0 +1,44 @@
+package reviewer
+
+import (
+	"testing"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+)
+
+// TestRegistryMatchesDomainVocabulary enforces that the shipped reviewer
+// adapters and domain.AllReviewerHarnesses stay in sync: every registered
+// adapter is a known reviewer harness, and every known harness has an adapter.
+func TestRegistryMatchesDomainVocabulary(t *testing.T) {
+	registered := map[domain.ReviewerHarness]bool{}
+	for _, a := range Constructors() {
+		h := a.Harness()
+		if !h.IsKnown() {
+			t.Errorf("adapter harness %q is not in domain.AllReviewerHarnesses", h)
+		}
+		if registered[h] {
+			t.Errorf("reviewer harness %q registered twice", h)
+		}
+		registered[h] = true
+	}
+	for _, h := range domain.AllReviewerHarnesses {
+		if !registered[h] {
+			t.Errorf("reviewer harness %q has no registered adapter", h)
+		}
+	}
+}
+
+func TestNewResolverResolvesShippedReviewers(t *testing.T) {
+	resolver, err := NewResolver()
+	if err != nil {
+		t.Fatalf("NewResolver: %v", err)
+	}
+	for _, h := range domain.AllReviewerHarnesses {
+		if _, ok := resolver.Reviewer(h); !ok {
+			t.Errorf("resolver missing reviewer %q", h)
+		}
+	}
+	if _, ok := resolver.Reviewer("nope"); ok {
+		t.Error("resolver returned an adapter for an unknown harness")
+	}
+}

backend/internal/cli/review.go

@@ -0,0 +1,104 @@
+package cli
+
+import (
+	"errors"
+	"fmt"
+	"net/url"
+	"os"
+	"strings"
+	"time"
+
+	"github.com/spf13/cobra"
+)
+
+// reviewRun mirrors the daemon's domain.ReviewRun for the CLI client.
+type reviewRun struct {
+	ID        string    `json:"id"`
+	SessionID string    `json:"sessionId"`
+	Harness   string    `json:"harness"`
+	PRURL     string    `json:"prUrl"`
+	TargetSHA string    `json:"targetSha"`
+	Status    string    `json:"status"`
+	Verdict   string    `json:"verdict"`
+	Body      string    `json:"body"`
+	CreatedAt time.Time `json:"createdAt"`
+}
+
+// reviewRunResponse mirrors controllers.ReviewRunResponse.
+type reviewRunResponse struct {
+	Review           reviewRun `json:"review"`
+	ReviewerHandleID string    `json:"reviewerHandleId"`
+}
+
+// submitReviewRequest mirrors controllers.SubmitReviewInput.
+type submitReviewRequest struct {
+	RunID   string `json:"runId"`
+	Verdict string `json:"verdict"`
+	Body    string `json:"body"`
+}
+
+type reviewSubmitOptions struct {
+	session string
+	runID   string
+	verdict string
+	body    string
+}
+
+func newReviewCommand(ctx *commandContext) *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "review",
+		Short: "Manage AO code reviews of a worker's PR",
+	}
+	cmd.AddCommand(newReviewSubmitCommand(ctx))
+	return cmd
+}
+
+func newReviewSubmitCommand(ctx *commandContext) *cobra.Command {
+	var opts reviewSubmitOptions
+	cmd := &cobra.Command{
+		Use:   "submit [worker-session-id]",
+		Short: "Record a reviewer's result for a worker's PR",
+		Args:  cobra.MaximumNArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return ctx.submitReview(cmd, args, opts)
+		},
+	}
+	cmd.Flags().StringVar(&opts.session, "session", "", "Worker session id (or pass it as the positional argument)")
+	cmd.Flags().StringVar(&opts.runID, "run", "", "Review run id (required)")
+	cmd.Flags().StringVar(&opts.verdict, "verdict", "", "Review verdict: approved or changes_requested (required)")
+	cmd.Flags().StringVar(&opts.body, "body", "", "Path to a Markdown file with the review body")
+	return cmd
+}
+
+func (c *commandContext) submitReview(cmd *cobra.Command, args []string, opts reviewSubmitOptions) error {
+	session := strings.TrimSpace(opts.session)
+	if len(args) == 1 {
+		session = strings.TrimSpace(args[0])
+	}
+	if session == "" {
+		return usageError{errors.New("usage: worker session id is required (positional or --session)")}
+	}
+	runID := strings.TrimSpace(opts.runID)
+	if runID == "" {
+		return usageError{errors.New("usage: --run is required")}
+	}
+	verdict := strings.TrimSpace(opts.verdict)
+	if verdict == "" {
+		return usageError{errors.New("usage: --verdict is required (approved or changes_requested)")}
+	}
+	var body string
+	if path := strings.TrimSpace(opts.body); path != "" {
+		raw, err := os.ReadFile(path)
+		if err != nil {
+			return usageError{fmt.Errorf("read body file: %w", err)}
+		}
+		body = string(raw)
+	}
+	path := "sessions/" + url.PathEscape(session) + "/reviews/submit"
+	var res reviewRunResponse
+	if err := c.postJSON(cmd.Context(), path, submitReviewRequest{RunID: runID, Verdict: verdict, Body: body}, &res); err != nil {
+		return err
+	}
+	_, err := fmt.Fprintf(cmd.OutOrStdout(), "recorded %s review for %s\n", res.Review.Verdict, session)
+	return err
+}

backend/internal/cli/review_test.go

@@ -0,0 +1,100 @@
+package cli
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// reviewCapture records the method/path/body of the request the CLI made.
+type reviewCapture struct {
+	method string
+	path   string
+	body   string
+}
+
+func reviewServer(t *testing.T, status int, respBody string) (*httptest.Server, *reviewCapture) {
+	t.Helper()
+	capture := &reviewCapture{}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		capture.method = r.Method
+		capture.path = r.URL.Path
+		capture.body = string(body)
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(status)
+		_, _ = io.WriteString(w, respBody)
+	}))
+	t.Cleanup(srv.Close)
+	return srv, capture
+}
+
+func aliveDeps() Deps { return Deps{ProcessAlive: func(int) bool { return true }} }
+
+func TestReviewSubmitReadsBodyFile(t *testing.T) {
+	cfg := setConfigEnv(t)
+	srv, capture := reviewServer(t, http.StatusOK, `{"review":{"verdict":"changes_requested"}}`)
+	writeRunFileFor(t, cfg, srv)
+
+	bodyFile := filepath.Join(t.TempDir(), "review.md")
+	if err := os.WriteFile(bodyFile, []byte("please fix"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	_, errOut, err := executeCLI(t, aliveDeps(),
+		"review", "submit", "mer-1", "--run", "run-1", "--verdict", "changes_requested", "--body", bodyFile)
+	if err != nil {
+		t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut)
+	}
+	if capture.method != http.MethodPost || capture.path != "/api/v1/sessions/mer-1/reviews/submit" {
+		t.Fatalf("request = %s %s", capture.method, capture.path)
+	}
+	var req submitReviewRequest
+	if err := json.Unmarshal([]byte(capture.body), &req); err != nil {
+		t.Fatalf("decode body: %v", err)
+	}
+	if req.RunID != "run-1" || req.Verdict != "changes_requested" || req.Body != "please fix" {
+		t.Fatalf("request = %+v", req)
+	}
+}
+
+func TestReviewSubmitUsesSessionFlag(t *testing.T) {
+	cfg := setConfigEnv(t)
+	srv, capture := reviewServer(t, http.StatusOK, `{"review":{"verdict":"approved"}}`)
+	writeRunFileFor(t, cfg, srv)
+
+	if _, errOut, err := executeCLI(t, aliveDeps(), "review", "submit", "--session", "mer-7", "--run", "run-7", "--verdict", "approved"); err != nil {
+		t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut)
+	}
+	if capture.path != "/api/v1/sessions/mer-7/reviews/submit" {
+		t.Fatalf("path = %q, want mer-7", capture.path)
+	}
+}
+
+func TestReviewSubmitMissingVerdictIsUsageError(t *testing.T) {
+	setConfigEnv(t)
+	_, _, err := executeCLI(t, aliveDeps(), "review", "submit", "mer-1", "--run", "run-1")
+	if got := ExitCode(err); got != 2 {
+		t.Fatalf("exit code = %d, want 2 (usage); err=%v", got, err)
+	}
+}
+
+func TestReviewSubmitMissingWorkerIsUsageError(t *testing.T) {
+	setConfigEnv(t)
+	_, _, err := executeCLI(t, aliveDeps(), "review", "submit", "--run", "run-1", "--verdict", "approved")
+	if got := ExitCode(err); got != 2 {
+		t.Fatalf("exit code = %d, want 2 (usage); err=%v", got, err)
+	}
+}
+
+func TestReviewSubmitMissingRunIsUsageError(t *testing.T) {
+	setConfigEnv(t)
+	_, _, err := executeCLI(t, aliveDeps(), "review", "submit", "mer-1", "--verdict", "approved")
+	if got := ExitCode(err); got != 2 {
+		t.Fatalf("exit code = %d, want 2 (usage); err=%v", got, err)
+	}
+}

backend/internal/cli/root.go

@@ -171,6 +171,7 @@ func NewRootCommand(deps Deps) *cobra.Command {
 	root.AddCommand(newProjectCommand(ctx))
 	root.AddCommand(newSessionCommand(ctx))
 	root.AddCommand(newOrchestratorCommand(ctx))
+	root.AddCommand(newReviewCommand(ctx))
 	root.AddCommand(newCompletionCommand())
 	root.AddCommand(newVersionCommand())