diff --git a/backend/internal/adapters/reviewer/claudecode/claudecode.go b/backend/internal/adapters/reviewer/claudecode/claudecode.go new file mode 100644 index 00000000..91e890d8 --- /dev/null +++ b/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 --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) +} diff --git a/backend/internal/adapters/reviewer/registry.go b/backend/internal/adapters/reviewer/registry.go new file mode 100644 index 00000000..b5fbcb7d --- /dev/null +++ b/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 +} diff --git a/backend/internal/adapters/reviewer/registry_test.go b/backend/internal/adapters/reviewer/registry_test.go new file mode 100644 index 00000000..fba7020f --- /dev/null +++ b/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") + } +} diff --git a/backend/internal/cli/review.go b/backend/internal/cli/review.go new file mode 100644 index 00000000..7bb2f7f5 --- /dev/null +++ b/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 +} diff --git a/backend/internal/cli/review_test.go b/backend/internal/cli/review_test.go new file mode 100644 index 00000000..29cdee64 --- /dev/null +++ b/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) + } +} diff --git a/backend/internal/cli/root.go b/backend/internal/cli/root.go index 4dec629c..f536459c 100644 --- a/backend/internal/cli/root.go +++ b/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()) diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 260ccd55..a0b9fda3 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -16,7 +16,6 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/httpd" "github.com/aoagents/agent-orchestrator/backend/internal/runfile" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" - reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review" "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" "github.com/aoagents/agent-orchestrator/backend/internal/terminal" ) @@ -93,7 +92,7 @@ func Run() error { // zellij runtime, a gitworktree workspace, the per-session agent resolver // (AO_AGENT default, validated here), and the agent messenger, then mount it // on the API. - sessionSvc, err := startSession(cfg, runtimeAdapter, store, lcStack.LCM, messenger, log) + sessionSvc, reviewSvc, err := startSession(cfg, runtimeAdapter, store, lcStack.LCM, messenger, log) if err != nil { stop() lcStack.Stop() @@ -106,7 +105,7 @@ func Run() error { srv, err := httpd.NewWithDeps(cfg, log, termMgr, httpd.APIDeps{ Projects: projectsvc.New(store), Sessions: sessionSvc, - Reviews: reviewsvc.NewInMemory(), + Reviews: reviewSvc, CDC: store, Events: cdcPipe.Broadcaster, Activity: lcStack.LCM, diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index b4d40905..1b5f9a5e 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -9,12 +9,16 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/adapters" "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/activitydispatch" agentregistry "github.com/aoagents/agent-orchestrator/backend/internal/adapters/agent/registry" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/reviewer" + "github.com/aoagents/agent-orchestrator/backend/internal/adapters/runtime/zellij" "github.com/aoagents/agent-orchestrator/backend/internal/adapters/workspace/gitworktree" "github.com/aoagents/agent-orchestrator/backend/internal/config" "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/lifecycle" "github.com/aoagents/agent-orchestrator/backend/internal/observe/reaper" "github.com/aoagents/agent-orchestrator/backend/internal/ports" + reviewcore "github.com/aoagents/agent-orchestrator/backend/internal/review" + reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review" sessionsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/session" sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager" "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" @@ -54,10 +58,10 @@ func (l *lifecycleStack) Stop() { // over the real zellij runtime, a per-session gitworktree workspace, the shared // store + LCM, the per-session agent resolver (AO_AGENT default), and the // agent messenger. The returned service is mounted at httpd APIDeps.Sessions. -func startSession(cfg config.Config, runtime ports.Runtime, store *sqlite.Store, lcm *lifecycle.Manager, messenger ports.AgentMessenger, log *slog.Logger) (*sessionsvc.Service, error) { +func startSession(cfg config.Config, runtime *zellij.Runtime, store *sqlite.Store, lcm *lifecycle.Manager, messenger ports.AgentMessenger, log *slog.Logger) (*sessionsvc.Service, reviewsvc.Manager, error) { agents, err := buildAgentResolver(cfg.Agent, log) if err != nil { - return nil, err + return nil, nil, err } ws, err := gitworktree.New(gitworktree.Options{ // Per-session worktrees live under the data dir, so a single AO_DATA_DIR @@ -69,7 +73,7 @@ func startSession(cfg config.Config, runtime ports.Runtime, store *sqlite.Store, RepoResolver: projectRepoResolver{store: store}, }) if err != nil { - return nil, fmt.Errorf("session workspace: %w", err) + return nil, nil, fmt.Errorf("session workspace: %w", err) } mgr := sessionmanager.New(sessionmanager.Deps{ Runtime: runtime, @@ -85,7 +89,7 @@ func startSession(cfg config.Config, runtime ports.Runtime, store *sqlite.Store, if err != nil { logSCMProviderDisabled(log, err) } - return sessionsvc.NewWithDeps(sessionsvc.Deps{ + sessionSvc := sessionsvc.NewWithDeps(sessionsvc.Deps{ Manager: mgr, Store: store, PRClaimer: store, @@ -93,7 +97,24 @@ func startSession(cfg config.Config, runtime ports.Runtime, store *sqlite.Store, // no_signal only makes sense for harnesses whose adapters install // activity hooks; the deriver registry is the source of truth for that. SignalCapable: activitydispatch.SupportsHarness, - }), nil + }) + // Triggering a review spawns a reviewer over the worker's worktree, resolved + // from the reviewer registry (distinct from the worker agent set). The + // reviewer posts its review to the PR itself, so the service needs no SCM + // writer. + reviewers, err := reviewer.NewResolver() + if err != nil { + return nil, nil, fmt.Errorf("reviewer resolver: %w", err) + } + reviewEngine := reviewcore.New(reviewcore.Deps{ + Store: store, + Sessions: store, + PRs: store, + Projects: store, + Launcher: reviewcore.NewLauncher(reviewers, runtime), + }) + reviewSvc := reviewsvc.New(reviewEngine) + return sessionSvc, reviewSvc, nil } // runtimeMessageSender is the narrow part of the concrete runtime needed by diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index 0e4815d0..19bc965b 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -149,13 +149,16 @@ func TestWiring_StartSessionBuildsSessionService(t *testing.T) { runtime := zellij.New(zellij.Options{}) messenger := newSessionMessenger(store, runtime, log) - svc, err := startSession(cfg, runtime, store, lcm, messenger, log) + svc, reviewSvc, err := startSession(cfg, runtime, store, lcm, messenger, log) if err != nil { t.Fatalf("startSession: %v", err) } if svc == nil { t.Fatal("startSession returned nil session service") } + if reviewSvc == nil { + t.Fatal("startSession returned nil review service") + } } type captureRuntimeSender struct { diff --git a/backend/internal/domain/projectconfig.go b/backend/internal/domain/projectconfig.go index 1e6cf32f..9724155e 100644 --- a/backend/internal/domain/projectconfig.go +++ b/backend/internal/domain/projectconfig.go @@ -36,6 +36,35 @@ type ProjectConfig struct { // Worker and Orchestrator are role-specific harness/agent-config overrides. Worker RoleOverride `json:"worker,omitempty"` Orchestrator RoleOverride `json:"orchestrator,omitempty"` + + // Reviewers names the agent(s) that review a worker's PR when a review is + // triggered. It is configured independently of the Worker override; an empty + // list falls back to the worker's own harness (see ResolveReviewerHarness). + Reviewers []ReviewerConfig `json:"reviewers,omitempty"` +} + +// ReviewerConfig names one reviewer agent by harness. The harness is drawn from +// the reviewer vocabulary (ReviewerHarness), which is distinct from the worker +// AgentHarness set. +type ReviewerConfig struct { + Harness ReviewerHarness `json:"harness"` +} + +// FallbackReviewerHarness is the reviewer used when a project configures none +// and the worker's harness is not itself a supported reviewer. +const FallbackReviewerHarness = ReviewerClaudeCode + +// ResolveReviewerHarness picks the reviewer harness for a worker. A configured +// reviewer wins; otherwise it reuses the worker's own harness when that harness +// is also a supported reviewer, falling back to claude-code. +func (c ProjectConfig) ResolveReviewerHarness(workerHarness AgentHarness) ReviewerHarness { + if len(c.Reviewers) > 0 { + return c.Reviewers[0].Harness + } + if h := ReviewerHarness(workerHarness); h.IsKnown() { + return h + } + return FallbackReviewerHarness } // RoleOverride overrides the harness and/or agent config for a session role. @@ -91,6 +120,11 @@ func (c ProjectConfig) Validate() error { return fmt.Errorf("symlink %q: %w", s, err) } } + for i, rv := range c.Reviewers { + if !rv.Harness.IsKnown() { + return fmt.Errorf("reviewers[%d].harness: unknown harness %q", i, rv.Harness) + } + } return nil } diff --git a/backend/internal/domain/projectconfig_test.go b/backend/internal/domain/projectconfig_test.go index 76155101..b7a969f9 100644 --- a/backend/internal/domain/projectconfig_test.go +++ b/backend/internal/domain/projectconfig_test.go @@ -19,6 +19,10 @@ func TestProjectConfigValidate(t *testing.T) { {"symlink parent escape", ProjectConfig{Symlinks: []string{"../escape"}}, true}, {"symlink embedded parent", ProjectConfig{Symlinks: []string{"a/../../b"}}, true}, {"symlink bare ..", ProjectConfig{Symlinks: []string{".."}}, true}, + {"good reviewers", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerClaudeCode}}}, false}, + {"unknown reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: "nope"}}}, true}, + {"worker harness is not auto a reviewer", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerHarness(HarnessCodex)}}}, true}, + {"empty reviewer harness", ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ""}}}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -65,6 +69,25 @@ func TestProjectConfigWithDefaults(t *testing.T) { } } +func TestResolveReviewerHarness(t *testing.T) { + // A configured reviewer always wins, regardless of the worker harness. + cfg := ProjectConfig{Reviewers: []ReviewerConfig{{Harness: ReviewerClaudeCode}}} + if got := cfg.ResolveReviewerHarness(HarnessAider); got != ReviewerClaudeCode { + t.Fatalf("configured reviewer = %q, want claude-code", got) + } + + // No reviewer configured: reuse the worker harness when it is also a + // supported reviewer (claude-code is). + if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessClaudeCode); got != ReviewerClaudeCode { + t.Fatalf("default = %q, want reviewer claude-code", got) + } + + // A worker harness that is not a supported reviewer falls back to claude-code. + if got := (ProjectConfig{}).ResolveReviewerHarness(HarnessAider); got != FallbackReviewerHarness { + t.Fatalf("fallback = %q, want %q", got, FallbackReviewerHarness) + } +} + func TestProjectConfigIsZero(t *testing.T) { if !(ProjectConfig{}).IsZero() { t.Fatal("empty config should be zero") diff --git a/backend/internal/domain/review.go b/backend/internal/domain/review.go new file mode 100644 index 00000000..a71556da --- /dev/null +++ b/backend/internal/domain/review.go @@ -0,0 +1,63 @@ +package domain + +import "time" + +// Review is the per-worker code-review record: one row per worker session +// (SessionID is unique). A repeat trigger reuses this row; the per-pass facts +// live on ReviewRun. +type Review struct { + ID string `json:"id"` + SessionID SessionID `json:"sessionId"` + ProjectID ProjectID `json:"projectId"` + Harness ReviewerHarness `json:"harness"` + PRURL string `json:"prUrl"` + // ReviewerHandleID is the runtime handle of the live reviewer pane, reused + // across passes and exposed so the UI can attach its terminal. + ReviewerHandleID string `json:"reviewerHandleId"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` +} + +// ReviewRun is one review pass against a worker's PR. +type ReviewRun struct { + ID string `json:"id"` + ReviewID string `json:"reviewId"` + SessionID SessionID `json:"sessionId"` + Harness ReviewerHarness `json:"harness"` + PRURL string `json:"prUrl"` + // TargetSHA is the PR head commit this pass reviewed. + TargetSHA string `json:"targetSha"` + Status ReviewRunStatus `json:"status"` + Verdict ReviewVerdict `json:"verdict"` + // Body is the review text the reviewer submitted. It is recorded for AO's + // own tracking; the reviewer also posts the review to the PR itself. + Body string `json:"body"` + CreatedAt time.Time `json:"createdAt"` +} + +// ReviewRunStatus is the lifecycle state of a single review pass. +type ReviewRunStatus string + +// Review run statuses. +const ( + ReviewRunRunning ReviewRunStatus = "running" + ReviewRunComplete ReviewRunStatus = "complete" + ReviewRunFailed ReviewRunStatus = "failed" +) + +// ReviewVerdict is the outcome a reviewer reports. The empty verdict marks a +// run that has not produced an outcome yet. +type ReviewVerdict string + +// Review verdicts. +const ( + VerdictNone ReviewVerdict = "" + VerdictApproved ReviewVerdict = "approved" + VerdictChangesRequested ReviewVerdict = "changes_requested" +) + +// Valid reports whether v is a verdict a reviewer may submit (the empty verdict +// is a stored default, not a submittable one). +func (v ReviewVerdict) Valid() bool { + return v == VerdictApproved || v == VerdictChangesRequested +} diff --git a/backend/internal/domain/reviewerharness.go b/backend/internal/domain/reviewerharness.go new file mode 100644 index 00000000..760be13e --- /dev/null +++ b/backend/internal/domain/reviewerharness.go @@ -0,0 +1,30 @@ +package domain + +// ReviewerHarness identifies a code-review agent. It is a separate vocabulary +// 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). +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" +) + +// AllReviewerHarnesses is the canonical set used to validate a configured +// reviewer harness. +var AllReviewerHarnesses = []ReviewerHarness{ + ReviewerClaudeCode, +} + +// IsKnown reports whether h is one of the supported reviewer harnesses. +func (h ReviewerHarness) IsKnown() bool { + for _, k := range AllReviewerHarnesses { + if h == k { + return true + } + } + return false +} diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index c91c02e3..82b13334 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -412,95 +412,6 @@ paths: summary: Resolve review threads on a pull request tags: - prs - /api/v1/reviews: - get: - operationId: listReviews - responses: - "200": - content: - application/json: - schema: - $ref: '#/components/schemas/ListReviewsResponse' - description: OK - "501": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Not Implemented - summary: List code-review runs - tags: - - reviews - /api/v1/reviews/{id}/send: - post: - operationId: sendReview - parameters: - - description: Review run id. - in: path - name: id - required: true - schema: - description: Review run id. - type: string - responses: - "200": - content: - application/json: - schema: - $ref: '#/components/schemas/ReviewResponse' - description: OK - "404": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Not Found - "501": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Not Implemented - summary: Send a review run's findings to its PR - tags: - - reviews - /api/v1/reviews/execute: - post: - operationId: executeReview - requestBody: - content: - application/json: - schema: - $ref: '#/components/schemas/ExecuteReviewInput' - required: true - responses: - "201": - content: - application/json: - schema: - $ref: '#/components/schemas/ReviewResponse' - description: Created - "400": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Bad Request - "422": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Unprocessable Entity - "501": - content: - application/json: - schema: - $ref: '#/components/schemas/APIError' - description: Not Implemented - summary: Start a code-review run for a session's PR - tags: - - reviews /api/v1/sessions: get: operationId: listSessions @@ -909,6 +820,135 @@ paths: summary: Restore a terminated session tags: - sessions + /api/v1/sessions/{sessionId}/reviews: + get: + operationId: listReviews + parameters: + - description: Session identifier, e.g. project-1. + in: path + name: sessionId + required: true + schema: + description: Session identifier, e.g. project-1. + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/ListReviewsResponse' + description: OK + "422": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Unprocessable Entity + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: List a worker's code-review runs + tags: + - reviews + /api/v1/sessions/{sessionId}/reviews/submit: + post: + operationId: submitReview + parameters: + - description: Session identifier, e.g. project-1. + in: path + name: sessionId + required: true + schema: + description: Session identifier, e.g. project-1. + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/SubmitReviewInput' + required: true + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/ReviewRunResponse' + description: OK + "400": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Bad Request + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Found + "422": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Unprocessable Entity + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Record a reviewer's result for a worker's PR + tags: + - reviews + /api/v1/sessions/{sessionId}/reviews/trigger: + post: + operationId: triggerReview + parameters: + - description: Session identifier, e.g. project-1. + in: path + name: sessionId + required: true + schema: + description: Session identifier, e.g. project-1. + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/ReviewRunResponse' + description: OK + "201": + content: + application/json: + schema: + $ref: '#/components/schemas/ReviewRunResponse' + description: Created + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Found + "422": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Unprocessable Entity + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Trigger a code review of a worker's PR + tags: + - reviews /api/v1/sessions/{sessionId}/rollback: post: operationId: rollbackSession @@ -1166,13 +1206,12 @@ components: - state - lastActivityAt type: object - ExecuteReviewInput: + DomainReviewerConfig: properties: - sessionId: - description: Session whose PR to review. + harness: type: string required: - - sessionId + - harness type: object KillSessionResponse: properties: @@ -1197,11 +1236,14 @@ components: type: object ListReviewsResponse: properties: + reviewerHandleId: + type: string reviews: items: $ref: '#/components/schemas/ReviewRun' type: array required: + - reviewerHandleId - reviews type: object ListSessionPRsResponse: @@ -1296,6 +1338,10 @@ components: items: type: string type: array + reviewers: + items: + $ref: '#/components/schemas/DomainReviewerConfig' + type: array sessionPrefix: type: string symlinks: @@ -1405,53 +1451,50 @@ components: - sessionId - session type: object - ReviewFinding: + ReviewRun: properties: body: type: string - id: - type: string - line: - type: integer - path: - type: string - severity: - type: string - required: - - id - - path - - line - - severity - - body - type: object - ReviewResponse: - properties: - review: - $ref: '#/components/schemas/ReviewRun' - required: - - review - type: object - ReviewRun: - properties: createdAt: format: date-time type: string - findings: - items: - $ref: '#/components/schemas/ReviewFinding' - type: array + harness: + type: string id: type: string + prUrl: + type: string + reviewId: + type: string sessionId: type: string status: type: string + targetSha: + type: string + verdict: + type: string required: - id + - reviewId - sessionId + - harness + - prUrl + - targetSha - status + - verdict + - body - createdAt - - findings + type: object + ReviewRunResponse: + properties: + review: + $ref: '#/components/schemas/ReviewRun' + reviewerHandleId: + type: string + required: + - review + - reviewerHandleId type: object RoleOverride: properties: @@ -1669,6 +1712,22 @@ components: required: - projectId type: object + SubmitReviewInput: + properties: + body: + description: Review body recorded by AO. Required for changes_requested. + type: string + runId: + description: Review run id being completed. + type: string + verdict: + description: 'Review verdict: approved or changes_requested.' + type: string + required: + - runId + - verdict + - body + type: object WorkspaceRepo: properties: name: diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 502cc252..eb9b47a0 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -161,11 +161,10 @@ var schemaNames = map[string]string{ "ControllersResolveCommentsResponse": "ResolveCommentsResponse", // httpd/controllers — review wire envelopes "ControllersListReviewsResponse": "ListReviewsResponse", - "ControllersExecuteReviewInput": "ExecuteReviewInput", - "ControllersReviewResponse": "ReviewResponse", - // service/review entities - "ReviewRun": "ReviewRun", - "ReviewFinding": "ReviewFinding", + "ControllersReviewRunResponse": "ReviewRunResponse", + "ControllersSubmitReviewInput": "SubmitReviewInput", + // domain review entities + "DomainReviewRun": "ReviewRun", // service/project entities + DTOs "ProjectProject": "Project", "ProjectSummary": "ProjectSummary", @@ -255,35 +254,42 @@ func operations() []operation { return ops } -// reviewOperations declares the /reviews operations. Must stay 1:1 with the -// routes ReviewsController.Register mounts (enforced by the parity test). +// reviewOperations declares the session-scoped /reviews operations. Must stay +// 1:1 with the routes ReviewsController.Register mounts (enforced by the parity +// test). func reviewOperations() []operation { return []operation{ { - method: http.MethodGet, path: "/api/v1/reviews", id: "listReviews", tag: "reviews", - summary: "List code-review runs", + method: http.MethodGet, path: "/api/v1/sessions/{sessionId}/reviews", id: "listReviews", tag: "reviews", + summary: "List a worker's code-review runs", + pathParams: []any{controllers.SessionIDParam{}}, resps: []respUnit{ {http.StatusOK, controllers.ListReviewsResponse{}}, + {http.StatusUnprocessableEntity, envelope.APIError{}}, {http.StatusNotImplemented, envelope.APIError{}}, }, }, { - method: http.MethodPost, path: "/api/v1/reviews/execute", id: "executeReview", tag: "reviews", - summary: "Start a code-review run for a session's PR", - reqBody: controllers.ExecuteReviewInput{}, + method: http.MethodPost, path: "/api/v1/sessions/{sessionId}/reviews/trigger", id: "triggerReview", tag: "reviews", + summary: "Trigger a code review of a worker's PR", + pathParams: []any{controllers.SessionIDParam{}}, resps: []respUnit{ - {http.StatusCreated, controllers.ReviewResponse{}}, - {http.StatusBadRequest, envelope.APIError{}}, + {http.StatusOK, controllers.ReviewRunResponse{}}, + {http.StatusCreated, controllers.ReviewRunResponse{}}, {http.StatusUnprocessableEntity, envelope.APIError{}}, + {http.StatusNotFound, envelope.APIError{}}, {http.StatusNotImplemented, envelope.APIError{}}, }, }, { - method: http.MethodPost, path: "/api/v1/reviews/{id}/send", id: "sendReview", tag: "reviews", - summary: "Send a review run's findings to its PR", - pathParams: []any{controllers.ReviewIDParam{}}, + method: http.MethodPost, path: "/api/v1/sessions/{sessionId}/reviews/submit", id: "submitReview", tag: "reviews", + summary: "Record a reviewer's result for a worker's PR", + pathParams: []any{controllers.SessionIDParam{}}, + reqBody: controllers.SubmitReviewInput{}, resps: []respUnit{ - {http.StatusOK, controllers.ReviewResponse{}}, + {http.StatusOK, controllers.ReviewRunResponse{}}, + {http.StatusBadRequest, envelope.APIError{}}, + {http.StatusUnprocessableEntity, envelope.APIError{}}, {http.StatusNotFound, envelope.APIError{}}, {http.StatusNotImplemented, envelope.APIError{}}, }, diff --git a/backend/internal/httpd/controllers/reviews.go b/backend/internal/httpd/controllers/reviews.go index 5d7df6bd..b9fc3c68 100644 --- a/backend/internal/httpd/controllers/reviews.go +++ b/backend/internal/httpd/controllers/reviews.go @@ -7,88 +7,98 @@ import ( "github.com/go-chi/chi/v5" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review" ) -// ReviewIDParam is the {id} path parameter on the /reviews/{id} routes. -type ReviewIDParam struct { - ID string `path:"id" description:"Review run id."` -} - -// ListReviewsResponse is the body of GET /api/v1/reviews. +// ListReviewsResponse is the body of GET /api/v1/sessions/{sessionId}/reviews. +// reviewerHandleId is the live reviewer pane's runtime handle, for the UI to +// attach its terminal over /mux (empty when no reviewer has run). type ListReviewsResponse struct { - Reviews []reviewsvc.Run `json:"reviews"` + ReviewerHandleID string `json:"reviewerHandleId"` + Reviews []domain.ReviewRun `json:"reviews"` } -// ExecuteReviewInput is the body of POST /api/v1/reviews/execute. -type ExecuteReviewInput struct { - SessionID string `json:"sessionId" description:"Session whose PR to review."` +// ReviewRunResponse is the body of trigger (200/201) and submit (200). It +// carries the run plus the reviewer pane handle so the UI can attach a terminal. +type ReviewRunResponse struct { + Review domain.ReviewRun `json:"review"` + ReviewerHandleID string `json:"reviewerHandleId"` } -// ReviewResponse is the { review } body of execute (201) and send (200). -type ReviewResponse struct { - Review reviewsvc.Run `json:"review"` +// SubmitReviewInput is the body of POST /api/v1/sessions/{sessionId}/reviews/submit. +type SubmitReviewInput struct { + RunID string `json:"runId" description:"Review run id being completed."` + Verdict string `json:"verdict" description:"Review verdict: approved or changes_requested."` + Body string `json:"body" description:"Review body recorded by AO. Required for changes_requested."` } -// ReviewsController owns the /reviews routes. A nil Svc returns 501. +// ReviewsController owns the session-scoped /reviews routes. A nil Svc returns 501. type ReviewsController struct { Svc reviewsvc.Manager } // Register mounts the review routes on the supplied router. func (c *ReviewsController) Register(r chi.Router) { - r.Get("/reviews", c.list) - r.Post("/reviews/execute", c.execute) - r.Post("/reviews/{id}/send", c.send) + r.Get("/sessions/{sessionId}/reviews", c.list) + r.Post("/sessions/{sessionId}/reviews/trigger", c.trigger) + r.Post("/sessions/{sessionId}/reviews/submit", c.submit) } func (c *ReviewsController) list(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { - apispec.NotImplemented(w, r, "GET", "/api/v1/reviews") + apispec.NotImplemented(w, r, "GET", "/api/v1/sessions/{sessionId}/reviews") return } - runs, err := c.Svc.List(r.Context()) + res, err := c.Svc.List(r.Context(), sessionID(r)) if err != nil { writeReviewError(w, r, err) return } + runs := res.Runs if runs == nil { - runs = []reviewsvc.Run{} + runs = []domain.ReviewRun{} } - envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{Reviews: runs}) + envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{ReviewerHandleID: res.ReviewerHandleID, Reviews: runs}) } -func (c *ReviewsController) execute(w http.ResponseWriter, r *http.Request) { +func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { - apispec.NotImplemented(w, r, "POST", "/api/v1/reviews/execute") - return - } - var in ExecuteReviewInput - if err := json.NewDecoder(r.Body).Decode(&in); err != nil { - envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_BODY", "Invalid request body", nil) + apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/reviews/trigger") return } - run, err := c.Svc.Execute(r.Context(), in.SessionID) + res, err := c.Svc.Trigger(r.Context(), sessionID(r)) if err != nil { writeReviewError(w, r, err) return } - envelope.WriteJSON(w, http.StatusCreated, ReviewResponse{Review: run}) + // 201 when a new pass was started; 200 when an existing run for the same + // commit was reused. + status := http.StatusOK + if res.Created { + status = http.StatusCreated + } + envelope.WriteJSON(w, status, ReviewRunResponse{Review: res.Run, ReviewerHandleID: res.ReviewerHandleID}) } -func (c *ReviewsController) send(w http.ResponseWriter, r *http.Request) { +func (c *ReviewsController) submit(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { - apispec.NotImplemented(w, r, "POST", "/api/v1/reviews/{id}/send") + apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/reviews/submit") + return + } + var in SubmitReviewInput + if err := json.NewDecoder(r.Body).Decode(&in); err != nil { + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_BODY", "Invalid request body", nil) return } - run, err := c.Svc.Send(r.Context(), chi.URLParam(r, "id")) + run, err := c.Svc.Submit(r.Context(), sessionID(r), in.RunID, domain.ReviewVerdict(in.Verdict), in.Body) if err != nil { writeReviewError(w, r, err) return } - envelope.WriteJSON(w, http.StatusOK, ReviewResponse{Review: run}) + envelope.WriteJSON(w, http.StatusOK, ReviewRunResponse{Review: run}) } func writeReviewError(w http.ResponseWriter, r *http.Request, err error) { @@ -96,7 +106,7 @@ func writeReviewError(w http.ResponseWriter, r *http.Request, err error) { case errors.Is(err, reviewsvc.ErrInvalid): envelope.WriteAPIError(w, r, http.StatusUnprocessableEntity, "unprocessable", "REVIEW_INVALID", err.Error(), nil) case errors.Is(err, reviewsvc.ErrNotFound): - envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "REVIEW_NOT_FOUND", "Unknown review run", nil) + envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "REVIEW_NOT_FOUND", err.Error(), nil) default: envelope.WriteAPIError(w, r, http.StatusInternalServerError, "internal", "REVIEW_OPERATION_FAILED", "Review operation failed", nil) } diff --git a/backend/internal/ports/reviewer.go b/backend/internal/ports/reviewer.go new file mode 100644 index 00000000..938fb070 --- /dev/null +++ b/backend/internal/ports/reviewer.go @@ -0,0 +1,55 @@ +package ports + +import ( + "context" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// Reviewer is the contract a code-review adapter satisfies. It is deliberately +// separate from Agent: a reviewer is invoked once over a checkout to review a +// PR, and need not be a prompt-fed interactive agent. A prompt-driven reviewer +// (claude-code) builds its own prompt internally; a one-shot CLI (greptile) +// returns its own argv with no prompt at all. +type Reviewer interface { + // ReviewCommand builds the command (and any extra env) AO should run to + // spawn a fresh reviewer over the worker's checkout for a PR. + ReviewCommand(ctx context.Context, inv ReviewInvocation) (ReviewCommandSpec, error) + // ReviewMessage builds the text AO injects into an already-running reviewer + // pane to ask it to review a new commit. It must be self-contained (carry + // the ids the reviewer needs to submit) since AO passes no environment. + ReviewMessage(ctx context.Context, inv ReviewInvocation) (string, error) +} + +// ReviewInvocation describes one review pass for a reviewer to act on. All ids +// the reviewer needs are passed explicitly here (and embedded in the prompt / +// message), never through environment variables. +type ReviewInvocation struct { + // ReviewerID is a stable id for the reviewer's runtime instance (pane, + // native session id), derived from the worker session. + ReviewerID string + // RunID is the review_run this pass completes; the reviewer passes it to + // `ao review submit`. + RunID string + // WorkerSessionID is the worker whose PR is under review. + WorkerSessionID domain.SessionID + // PRURL is the pull request to review. + PRURL string + // TargetSHA is the PR head commit under review. + TargetSHA string + // WorkspacePath is the worker's checkout the reviewer reads. + WorkspacePath string +} + +// ReviewCommandSpec is how to launch a reviewer: the argv and any extra env the +// adapter needs. AO supplies the workspace and review-tracking env around it. +type ReviewCommandSpec struct { + Argv []string + Env map[string]string +} + +// ReviewerResolver maps a reviewer harness onto its adapter. ok=false means no +// adapter is registered for that harness. +type ReviewerResolver interface { + Reviewer(harness domain.ReviewerHarness) (Reviewer, bool) +} diff --git a/backend/internal/review/launcher.go b/backend/internal/review/launcher.go new file mode 100644 index 00000000..f376ac81 --- /dev/null +++ b/backend/internal/review/launcher.go @@ -0,0 +1,115 @@ +package review + +import ( + "context" + "fmt" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +// Launcher spawns, re-notifies, and probes a reviewer over a worker's worktree. +// It is the side of the engine that talks to the reviewer registry and runtime; +// the engine owns the orchestration and persistence. +type Launcher interface { + // Spawn launches a fresh reviewer and returns the runtime handle id of the + // live pane (stable per worker, reused across passes). + Spawn(ctx context.Context, spec LaunchSpec) (handleID string, err error) + // Notify asks an already-running reviewer pane to review a new commit. + Notify(ctx context.Context, handleID string, spec LaunchSpec) error + // Alive reports whether a reviewer pane is still running. + Alive(ctx context.Context, handleID string) (bool, error) +} + +// LaunchSpec is the engine's request to (re)launch a reviewer for one pass. +type LaunchSpec struct { + RunID string + WorkerID domain.SessionID + Harness domain.ReviewerHarness + WorkspacePath string + PRURL string + TargetSHA string +} + +// reviewerRuntime is the runtime surface the launcher needs: create a pane, +// inject a message into a running pane, and probe liveness. The zellij runtime +// satisfies it. +type reviewerRuntime interface { + Create(ctx context.Context, cfg ports.RuntimeConfig) (ports.RuntimeHandle, error) + IsAlive(ctx context.Context, handle ports.RuntimeHandle) (bool, error) + SendMessage(ctx context.Context, handle ports.RuntimeHandle, message string) error +} + +// agentLauncher resolves a reviewer adapter from the registry and drives the +// runtime. The reviewer reuses the worker's worktree (a fresh session worktree +// would branch off the default branch and so would not contain the PR changes). +type agentLauncher struct { + reviewers ports.ReviewerResolver + runtime reviewerRuntime +} + +// NewLauncher builds the production reviewer launcher. +func NewLauncher(reviewers ports.ReviewerResolver, runtime reviewerRuntime) Launcher { + return &agentLauncher{reviewers: reviewers, runtime: runtime} +} + +// reviewerHandleID is the stable runtime handle for a worker's reviewer pane, so +// one live reviewer is reused across passes. +func reviewerHandleID(workerID domain.SessionID) string { + return "review-" + string(workerID) +} + +func (l *agentLauncher) invocation(spec LaunchSpec) ports.ReviewInvocation { + return ports.ReviewInvocation{ + ReviewerID: reviewerHandleID(spec.WorkerID), + RunID: spec.RunID, + WorkerSessionID: spec.WorkerID, + PRURL: spec.PRURL, + TargetSHA: spec.TargetSHA, + WorkspacePath: spec.WorkspacePath, + } +} + +func (l *agentLauncher) Spawn(ctx context.Context, spec LaunchSpec) (string, error) { + reviewer, ok := l.reviewers.Reviewer(spec.Harness) + if !ok { + return "", fmt.Errorf("no reviewer adapter for harness %q", spec.Harness) + } + handleID := reviewerHandleID(spec.WorkerID) + cmd, err := reviewer.ReviewCommand(ctx, l.invocation(spec)) + if err != nil { + return "", fmt.Errorf("reviewer command: %w", err) + } + handle, err := l.runtime.Create(ctx, ports.RuntimeConfig{ + SessionID: domain.SessionID(handleID), + WorkspacePath: spec.WorkspacePath, + Argv: cmd.Argv, + Env: cmd.Env, + }) + if err != nil { + return "", fmt.Errorf("reviewer runtime: %w", err) + } + return handle.ID, nil +} + +func (l *agentLauncher) Notify(ctx context.Context, handleID string, spec LaunchSpec) error { + reviewer, ok := l.reviewers.Reviewer(spec.Harness) + if !ok { + return fmt.Errorf("no reviewer adapter for harness %q", spec.Harness) + } + msg, err := reviewer.ReviewMessage(ctx, l.invocation(spec)) + if err != nil { + return fmt.Errorf("reviewer message: %w", err) + } + if err := l.runtime.SendMessage(ctx, ports.RuntimeHandle{ID: handleID}, msg); err != nil { + return fmt.Errorf("notify reviewer: %w", err) + } + return nil +} + +func (l *agentLauncher) Alive(ctx context.Context, handleID string) (bool, error) { + if handleID == "" { + return false, nil + } + return l.runtime.IsAlive(ctx, ports.RuntimeHandle{ID: handleID}) +} diff --git a/backend/internal/review/launcher_test.go b/backend/internal/review/launcher_test.go new file mode 100644 index 00000000..71098371 --- /dev/null +++ b/backend/internal/review/launcher_test.go @@ -0,0 +1,113 @@ +package review + +import ( + "context" + "strings" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +type fakeReviewer struct { + gotInv ports.ReviewInvocation +} + +func (f *fakeReviewer) ReviewCommand(_ context.Context, inv ports.ReviewInvocation) (ports.ReviewCommandSpec, error) { + f.gotInv = inv + return ports.ReviewCommandSpec{Argv: []string{"greptile", "review"}}, nil +} +func (f *fakeReviewer) ReviewMessage(_ context.Context, inv ports.ReviewInvocation) (string, error) { + f.gotInv = inv + return "review run " + inv.RunID, nil +} + +type fakeReviewerResolver struct { + reviewer ports.Reviewer + ok bool +} + +func (f fakeReviewerResolver) Reviewer(domain.ReviewerHarness) (ports.Reviewer, bool) { + return f.reviewer, f.ok +} + +type fakeRuntime struct { + createCfg ports.RuntimeConfig + sentMsg string + sentTo string + alive bool +} + +func (f *fakeRuntime) Create(_ context.Context, cfg ports.RuntimeConfig) (ports.RuntimeHandle, error) { + f.createCfg = cfg + return ports.RuntimeHandle{ID: string(cfg.SessionID)}, nil +} +func (f *fakeRuntime) IsAlive(_ context.Context, _ ports.RuntimeHandle) (bool, error) { + return f.alive, nil +} +func (f *fakeRuntime) SendMessage(_ context.Context, handle ports.RuntimeHandle, msg string) error { + f.sentTo = handle.ID + f.sentMsg = msg + return nil +} + +func launchSpec() LaunchSpec { + return LaunchSpec{ + RunID: "run-1", WorkerID: "mer-1", Harness: domain.ReviewerClaudeCode, + WorkspacePath: "/ws/mer-1", PRURL: "https://github.com/o/r/pull/1", TargetSHA: "sha1", + } +} + +func TestLauncherSpawnReturnsStableHandle(t *testing.T) { + reviewer := &fakeReviewer{} + rt := &fakeRuntime{} + l := NewLauncher(fakeReviewerResolver{reviewer: reviewer, ok: true}, rt) + + handle, err := l.Spawn(context.Background(), launchSpec()) + if err != nil { + t.Fatalf("Spawn: %v", err) + } + if handle != "review-mer-1" { + t.Fatalf("handle = %q, want review-mer-1", handle) + } + if rt.createCfg.WorkspacePath != "/ws/mer-1" || len(rt.createCfg.Argv) == 0 || rt.createCfg.Argv[0] != "greptile" { + t.Fatalf("create cfg = %+v", rt.createCfg) + } + // No environment is used to carry review identity. + if len(rt.createCfg.Env) != 0 { + t.Fatalf("expected no env, got %v", rt.createCfg.Env) + } + if reviewer.gotInv.RunID != "run-1" || reviewer.gotInv.TargetSHA != "sha1" || reviewer.gotInv.ReviewerID != "review-mer-1" { + t.Fatalf("invocation = %+v", reviewer.gotInv) + } +} + +func TestLauncherNotifySendsMessageToHandle(t *testing.T) { + reviewer := &fakeReviewer{} + rt := &fakeRuntime{} + l := NewLauncher(fakeReviewerResolver{reviewer: reviewer, ok: true}, rt) + + if err := l.Notify(context.Background(), "review-mer-1", launchSpec()); err != nil { + t.Fatalf("Notify: %v", err) + } + if rt.sentTo != "review-mer-1" || !strings.Contains(rt.sentMsg, "run-1") { + t.Fatalf("sent to %q msg %q", rt.sentTo, rt.sentMsg) + } +} + +func TestLauncherAlive(t *testing.T) { + l := NewLauncher(fakeReviewerResolver{ok: true}, &fakeRuntime{alive: true}) + if ok, _ := l.Alive(context.Background(), "review-mer-1"); !ok { + t.Fatal("want alive true") + } + if ok, _ := l.Alive(context.Background(), ""); ok { + t.Fatal("empty handle should not be alive") + } +} + +func TestLauncherSpawnNoAdapter(t *testing.T) { + l := NewLauncher(fakeReviewerResolver{ok: false}, &fakeRuntime{}) + if _, err := l.Spawn(context.Background(), launchSpec()); err == nil || !strings.Contains(err.Error(), "no reviewer adapter") { + t.Fatalf("err = %v, want no-adapter", err) + } +} diff --git a/backend/internal/review/review.go b/backend/internal/review/review.go new file mode 100644 index 00000000..9664b0a0 --- /dev/null +++ b/backend/internal/review/review.go @@ -0,0 +1,330 @@ +// Package review holds the core code-review logic: triggering a reviewer over a +// worker's worktree, recording review runs, and accepting submitted results. +// +// It is independent of any transport. The daemon's HTTP service +// (internal/service/review) is a thin boundary over this engine today, and the +// same engine can back an in-process CLI trigger later without going through the +// API. Transport-specific concerns (DTOs, error→status mapping) stay in the +// service/controller layers; the orchestration and run-id generation live here. +package review + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/google/uuid" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// ErrInvalid and ErrNotFound let the transport layer map failures to 422/404. +var ( + ErrInvalid = errors.New("review: invalid input") + ErrNotFound = errors.New("review: not found") +) + +// Store is the persistence surface the engine needs. *sqlite.Store satisfies it +// in production; tests use a fake. +type Store interface { + UpsertReview(ctx context.Context, r domain.Review) error + GetReviewBySession(ctx context.Context, id domain.SessionID) (domain.Review, bool, error) + InsertReviewRun(ctx context.Context, r domain.ReviewRun) error + UpdateReviewRunResult(ctx context.Context, id string, status domain.ReviewRunStatus, verdict domain.ReviewVerdict, body string) (bool, error) + GetReviewRun(ctx context.Context, id string) (domain.ReviewRun, bool, error) + GetReviewRunBySessionAndSHA(ctx context.Context, id domain.SessionID, targetSHA string) (domain.ReviewRun, bool, error) + ListReviewRunsBySession(ctx context.Context, id domain.SessionID) ([]domain.ReviewRun, error) +} + +// Sessions resolves the worker session under review. +type Sessions interface { + GetSession(ctx context.Context, id domain.SessionID) (domain.SessionRecord, bool, error) +} + +// PRs resolves the PR a worker owns. +type PRs interface { + ListPRsBySession(ctx context.Context, id domain.SessionID) ([]domain.PullRequest, error) +} + +// Projects resolves the per-project reviewer config. +type Projects interface { + GetProject(ctx context.Context, id string) (domain.ProjectRecord, bool, error) +} + +// Deps wires the engine. +type Deps struct { + Store Store + Sessions Sessions + PRs PRs + Projects Projects + Launcher Launcher + + // Clock and NewID are injectable for deterministic tests. + Clock func() time.Time + NewID func() string +} + +// Engine is the core code-review engine. +type Engine struct { + store Store + sessions Sessions + prs PRs + projects Projects + launcher Launcher + clock func() time.Time + newID func() string +} + +// New wires an Engine from its dependencies, defaulting the clock and id source. +func New(d Deps) *Engine { + clock := d.Clock + if clock == nil { + clock = func() time.Time { return time.Now().UTC() } + } + newID := d.NewID + if newID == nil { + newID = uuid.NewString + } + return &Engine{ + store: d.Store, + sessions: d.Sessions, + prs: d.PRs, + projects: d.Projects, + launcher: d.Launcher, + clock: clock, + newID: newID, + } +} + +// TriggerResult is the outcome of a trigger: the (new or existing) run, the live +// reviewer pane's handle so the UI can attach its terminal, and whether a new +// pass was started (false when an existing run for the same commit was reused). +type TriggerResult struct { + Run domain.ReviewRun + ReviewerHandleID string + Created bool +} + +// SessionReviews is a worker's review state: the live reviewer handle plus its +// recorded passes, newest first. +type SessionReviews struct { + ReviewerHandleID string + Runs []domain.ReviewRun +} + +// Trigger starts (or reuses) a review of a worker's PR at its current head: +// - if a run already exists for this commit, it is returned unchanged; +// - otherwise, if a live reviewer pane exists, it is messaged to review the +// new commit; if not, a fresh reviewer is spawned; +// - only after the reviewer is launched is the run recorded. +func (e *Engine) Trigger(ctx context.Context, workerID domain.SessionID) (TriggerResult, error) { + if workerID == "" { + return TriggerResult{}, fmt.Errorf("%w: worker session id is required", ErrInvalid) + } + worker, ok, err := e.sessions.GetSession(ctx, workerID) + if err != nil { + return TriggerResult{}, err + } + if !ok { + return TriggerResult{}, fmt.Errorf("%w: worker session %q", ErrNotFound, workerID) + } + if worker.IsTerminated { + return TriggerResult{}, fmt.Errorf("%w: worker session %q is terminated", ErrInvalid, workerID) + } + if worker.Metadata.WorkspacePath == "" { + return TriggerResult{}, fmt.Errorf("%w: worker session %q has no workspace to review", ErrInvalid, workerID) + } + + pr, err := e.workerPR(ctx, workerID) + if err != nil { + return TriggerResult{}, err + } + targetSHA := pr.HeadSHA + + review, hasReview, err := e.store.GetReviewBySession(ctx, workerID) + if err != nil { + return TriggerResult{}, err + } + + // Idempotency: a pass already exists for this commit — return it as-is. + if existing, ok, err := e.store.GetReviewRunBySessionAndSHA(ctx, workerID, targetSHA); err != nil { + return TriggerResult{}, err + } else if ok { + return TriggerResult{Run: existing, ReviewerHandleID: review.ReviewerHandleID, Created: false}, nil + } + + harness, err := e.reviewerHarness(ctx, worker) + if err != nil { + return TriggerResult{}, err + } + + now := e.clock() + runID := e.newID() + spec := LaunchSpec{ + RunID: runID, + WorkerID: workerID, + Harness: harness, + WorkspacePath: worker.Metadata.WorkspacePath, + PRURL: pr.URL, + TargetSHA: targetSHA, + } + + // Reuse a live reviewer pane if there is one; otherwise spawn a fresh one. + handleID := "" + if hasReview && review.ReviewerHandleID != "" { + alive, err := e.launcher.Alive(ctx, review.ReviewerHandleID) + if err != nil { + return TriggerResult{}, err + } + if alive { + if err := e.launcher.Notify(ctx, review.ReviewerHandleID, spec); err != nil { + return TriggerResult{}, fmt.Errorf("notify reviewer: %w", err) + } + handleID = review.ReviewerHandleID + } + } + if handleID == "" { + h, err := e.launcher.Spawn(ctx, spec) + if err != nil { + return TriggerResult{}, fmt.Errorf("launch reviewer: %w", err) + } + handleID = h + } + + // The reviewer is running; now record the pass. + review, err = e.upsertReview(ctx, worker, harness, pr.URL, handleID, now) + if err != nil { + return TriggerResult{}, err + } + run := domain.ReviewRun{ + ID: runID, + ReviewID: review.ID, + SessionID: workerID, + Harness: harness, + PRURL: pr.URL, + TargetSHA: targetSHA, + Status: domain.ReviewRunRunning, + Verdict: domain.VerdictNone, + CreatedAt: now, + } + if err := e.store.InsertReviewRun(ctx, run); err != nil { + return TriggerResult{}, err + } + return TriggerResult{Run: run, ReviewerHandleID: handleID, Created: true}, nil +} + +// Submit records the reviewer's result for a specific worker review pass: it +// marks the run complete and stores the verdict and body. AO does not post the +// review — the reviewer agent posts it to the PR itself. +func (e *Engine) Submit(ctx context.Context, workerID domain.SessionID, runID string, verdict domain.ReviewVerdict, body string) (domain.ReviewRun, error) { + if workerID == "" { + return domain.ReviewRun{}, fmt.Errorf("%w: worker session id is required", ErrInvalid) + } + if runID == "" { + return domain.ReviewRun{}, fmt.Errorf("%w: review run id is required", ErrInvalid) + } + if !verdict.Valid() { + return domain.ReviewRun{}, fmt.Errorf("%w: verdict must be %q or %q", ErrInvalid, domain.VerdictApproved, domain.VerdictChangesRequested) + } + if verdict == domain.VerdictChangesRequested && body == "" { + return domain.ReviewRun{}, fmt.Errorf("%w: a changes_requested review requires a body", ErrInvalid) + } + + run, ok, err := e.store.GetReviewRun(ctx, runID) + if err != nil { + return domain.ReviewRun{}, err + } + if !ok { + return domain.ReviewRun{}, fmt.Errorf("%w: review run %q", ErrNotFound, runID) + } + if run.SessionID != workerID { + return domain.ReviewRun{}, fmt.Errorf("%w: review run %q does not belong to worker %q", ErrInvalid, runID, workerID) + } + if run.Status != domain.ReviewRunRunning { + return domain.ReviewRun{}, fmt.Errorf("%w: review run %q is not running", ErrInvalid, runID) + } + + updated, err := e.store.UpdateReviewRunResult(ctx, run.ID, domain.ReviewRunComplete, verdict, body) + if err != nil { + return domain.ReviewRun{}, err + } + if !updated { + return domain.ReviewRun{}, fmt.Errorf("%w: review run %q is not running", ErrInvalid, runID) + } + run.Status = domain.ReviewRunComplete + run.Verdict = verdict + run.Body = body + return run, nil +} + +// List returns a worker's review state: the live reviewer handle and its passes. +func (e *Engine) List(ctx context.Context, workerID domain.SessionID) (SessionReviews, error) { + if workerID == "" { + return SessionReviews{}, fmt.Errorf("%w: worker session id is required", ErrInvalid) + } + runs, err := e.store.ListReviewRunsBySession(ctx, workerID) + if err != nil { + return SessionReviews{}, err + } + var handle string + if review, ok, err := e.store.GetReviewBySession(ctx, workerID); err != nil { + return SessionReviews{}, err + } else if ok { + handle = review.ReviewerHandleID + } + return SessionReviews{ReviewerHandleID: handle, Runs: runs}, nil +} + +func (e *Engine) workerPR(ctx context.Context, workerID domain.SessionID) (domain.PullRequest, error) { + prs, err := e.prs.ListPRsBySession(ctx, workerID) + if err != nil { + return domain.PullRequest{}, err + } + if len(prs) == 0 { + return domain.PullRequest{}, fmt.Errorf("%w: worker %q has no PR to review", ErrInvalid, workerID) + } + return prs[0], nil +} + +// reviewerHarness resolves which harness reviews the worker's PR: a configured +// reviewer wins, otherwise the worker's own harness is reused (falling back to +// claude-code), per domain.ResolveReviewerHarness. +func (e *Engine) reviewerHarness(ctx context.Context, worker domain.SessionRecord) (domain.ReviewerHarness, error) { + var cfg domain.ProjectConfig + if e.projects != nil { + if proj, ok, err := e.projects.GetProject(ctx, string(worker.ProjectID)); err != nil { + return "", err + } else if ok { + cfg = proj.Config + } + } + return cfg.ResolveReviewerHarness(worker.Harness), nil +} + +func (e *Engine) upsertReview(ctx context.Context, worker domain.SessionRecord, harness domain.ReviewerHarness, prURL, handleID string, now time.Time) (domain.Review, error) { + existing, ok, err := e.store.GetReviewBySession(ctx, worker.ID) + if err != nil { + return domain.Review{}, err + } + review := domain.Review{ + ID: e.newID(), + SessionID: worker.ID, + ProjectID: worker.ProjectID, + Harness: harness, + PRURL: prURL, + ReviewerHandleID: handleID, + CreatedAt: now, + UpdatedAt: now, + } + if ok { + // Reuse the existing row's identity and creation time; UpsertReview + // refreshes harness/pr_url/reviewer_handle_id/updated_at. + review.ID = existing.ID + review.CreatedAt = existing.CreatedAt + } + if err := e.store.UpsertReview(ctx, review); err != nil { + return domain.Review{}, err + } + return review, nil +} diff --git a/backend/internal/review/review_test.go b/backend/internal/review/review_test.go new file mode 100644 index 00000000..333bcdba --- /dev/null +++ b/backend/internal/review/review_test.go @@ -0,0 +1,315 @@ +package review + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// --- fakes --- + +type fakeStore struct { + review *domain.Review + runs []domain.ReviewRun +} + +func (f *fakeStore) UpsertReview(_ context.Context, r domain.Review) error { + cp := r + f.review = &cp + return nil +} +func (f *fakeStore) GetReviewBySession(_ context.Context, _ domain.SessionID) (domain.Review, bool, error) { + if f.review == nil { + return domain.Review{}, false, nil + } + return *f.review, true, nil +} +func (f *fakeStore) InsertReviewRun(_ context.Context, r domain.ReviewRun) error { + f.runs = append(f.runs, r) + return nil +} +func (f *fakeStore) UpdateReviewRunResult(_ context.Context, id string, status domain.ReviewRunStatus, verdict domain.ReviewVerdict, body string) (bool, error) { + for i := range f.runs { + if f.runs[i].ID == id { + if f.runs[i].Status != domain.ReviewRunRunning { + return false, nil + } + f.runs[i].Status = status + f.runs[i].Verdict = verdict + f.runs[i].Body = body + return true, nil + } + } + return false, nil +} +func (f *fakeStore) GetReviewRun(_ context.Context, id string) (domain.ReviewRun, bool, error) { + for _, r := range f.runs { + if r.ID == id { + return r, true, nil + } + } + return domain.ReviewRun{}, false, nil +} +func (f *fakeStore) GetReviewRunBySessionAndSHA(_ context.Context, _ domain.SessionID, sha string) (domain.ReviewRun, bool, error) { + for i := len(f.runs) - 1; i >= 0; i-- { + if f.runs[i].TargetSHA == sha { + return f.runs[i], true, nil + } + } + return domain.ReviewRun{}, false, nil +} +func (f *fakeStore) ListReviewRunsBySession(_ context.Context, _ domain.SessionID) ([]domain.ReviewRun, error) { + return f.runs, nil +} + +type fakeSessions struct { + rec domain.SessionRecord + ok bool +} + +func (f fakeSessions) GetSession(_ context.Context, _ domain.SessionID) (domain.SessionRecord, bool, error) { + return f.rec, f.ok, nil +} + +type fakePRs struct{ prs []domain.PullRequest } + +func (f fakePRs) ListPRsBySession(_ context.Context, _ domain.SessionID) ([]domain.PullRequest, error) { + return f.prs, nil +} + +type fakeProjects struct{ cfg domain.ProjectConfig } + +func (f fakeProjects) GetProject(_ context.Context, id string) (domain.ProjectRecord, bool, error) { + return domain.ProjectRecord{ID: id, Config: f.cfg}, true, nil +} + +type fakeLauncher struct { + handle string + alive bool + spawnErr error + notifyErr error + spawned bool + notified bool + gotSpec LaunchSpec + gotHandle string +} + +func (f *fakeLauncher) Spawn(_ context.Context, spec LaunchSpec) (string, error) { + f.spawned = true + f.gotSpec = spec + if f.spawnErr != nil { + return "", f.spawnErr + } + return f.handle, nil +} +func (f *fakeLauncher) Notify(_ context.Context, handleID string, spec LaunchSpec) error { + f.notified = true + f.gotHandle = handleID + f.gotSpec = spec + return f.notifyErr +} +func (f *fakeLauncher) Alive(_ context.Context, _ string) (bool, error) { return f.alive, nil } + +func liveWorker() domain.SessionRecord { + return domain.SessionRecord{ + ID: "mer-1", + ProjectID: "mer", + Harness: domain.HarnessClaudeCode, + Metadata: domain.SessionMetadata{WorkspacePath: "/ws/mer-1"}, + } +} + +func newEngineForTest(store Store, sessions Sessions, prs PRs, projects Projects, launcher Launcher) *Engine { + ids := 0 + return New(Deps{ + Store: store, Sessions: sessions, PRs: prs, Projects: projects, Launcher: launcher, + Clock: func() time.Time { return time.Unix(0, 0).UTC() }, + NewID: func() string { ids++; return "id-" + string(rune('0'+ids)) }, + }) +} + +func prAt(sha string) fakePRs { + return fakePRs{prs: []domain.PullRequest{{URL: "https://github.com/o/r/pull/1", HeadSHA: sha}}} +} + +// --- tests --- + +func TestTriggerSpawnsNewReviewerAndRecordsRunAfterLaunch(t *testing.T) { + store := &fakeStore{} + launcher := &fakeLauncher{handle: "review-mer-1"} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, launcher) + + res, err := eng.Trigger(context.Background(), "mer-1") + if err != nil { + t.Fatalf("Trigger: %v", err) + } + if !res.Created || res.ReviewerHandleID != "review-mer-1" { + t.Fatalf("result = %+v", res) + } + if !launcher.spawned || launcher.notified { + t.Fatalf("expected spawn (no live reviewer): %+v", launcher) + } + if res.Run.TargetSHA != "sha1" || res.Run.Status != domain.ReviewRunRunning || res.Run.Harness != domain.ReviewerClaudeCode { + t.Fatalf("run = %+v", res.Run) + } + if launcher.gotSpec.RunID != res.Run.ID { + t.Fatalf("launch spec run id %q != run id %q", launcher.gotSpec.RunID, res.Run.ID) + } + if len(store.runs) != 1 || store.review == nil || store.review.ReviewerHandleID != "review-mer-1" { + t.Fatalf("persisted review=%+v runs=%+v", store.review, store.runs) + } +} + +func TestTriggerIsIdempotentForSameCommit(t *testing.T) { + store := &fakeStore{ + review: &domain.Review{ID: "rev-1", SessionID: "mer-1", ReviewerHandleID: "review-mer-1"}, + runs: []domain.ReviewRun{{ID: "run-1", SessionID: "mer-1", TargetSHA: "sha1", Status: domain.ReviewRunRunning}}, + } + launcher := &fakeLauncher{} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, launcher) + + res, err := eng.Trigger(context.Background(), "mer-1") + if err != nil { + t.Fatalf("Trigger: %v", err) + } + if res.Created || res.Run.ID != "run-1" || res.ReviewerHandleID != "review-mer-1" { + t.Fatalf("expected reuse of existing run: %+v", res) + } + if launcher.spawned || launcher.notified { + t.Fatalf("should not launch for an already-reviewed commit: %+v", launcher) + } + if len(store.runs) != 1 { + t.Fatalf("should not insert another run: %+v", store.runs) + } +} + +func TestTriggerNotifiesLiveReviewerOnNewCommit(t *testing.T) { + store := &fakeStore{ + review: &domain.Review{ID: "rev-1", SessionID: "mer-1", ReviewerHandleID: "review-mer-1"}, + runs: []domain.ReviewRun{{ID: "run-0", SessionID: "mer-1", TargetSHA: "sha0", Status: domain.ReviewRunComplete}}, + } + launcher := &fakeLauncher{alive: true} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, launcher) + + res, err := eng.Trigger(context.Background(), "mer-1") + if err != nil { + t.Fatalf("Trigger: %v", err) + } + if !launcher.notified || launcher.spawned { + t.Fatalf("expected notify on live reviewer: %+v", launcher) + } + if launcher.gotHandle != "review-mer-1" { + t.Fatalf("notify handle = %q", launcher.gotHandle) + } + if !res.Created || res.Run.TargetSHA != "sha1" || len(store.runs) != 2 { + t.Fatalf("expected a new run for sha1: res=%+v runs=%+v", res, store.runs) + } +} + +func TestTriggerSpawnsWhenReviewerDead(t *testing.T) { + store := &fakeStore{ + review: &domain.Review{ID: "rev-1", SessionID: "mer-1", ReviewerHandleID: "review-mer-1"}, + runs: []domain.ReviewRun{{ID: "run-0", SessionID: "mer-1", TargetSHA: "sha0", Status: domain.ReviewRunComplete}}, + } + launcher := &fakeLauncher{alive: false, handle: "review-mer-1"} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, launcher) + + if _, err := eng.Trigger(context.Background(), "mer-1"); err != nil { + t.Fatalf("Trigger: %v", err) + } + if !launcher.spawned || launcher.notified { + t.Fatalf("expected spawn when reviewer dead: %+v", launcher) + } +} + +func TestTriggerLaunchFailureRecordsNothing(t *testing.T) { + store := &fakeStore{} + launcher := &fakeLauncher{spawnErr: errors.New("boom")} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, launcher) + + if _, err := eng.Trigger(context.Background(), "mer-1"); err == nil { + t.Fatal("want launch error") + } + if len(store.runs) != 0 || store.review != nil { + t.Fatalf("nothing should be persisted on launch failure: review=%+v runs=%+v", store.review, store.runs) + } +} + +func TestTriggerUsesConfiguredReviewerHarness(t *testing.T) { + store := &fakeStore{} + projects := fakeProjects{cfg: domain.ProjectConfig{Reviewers: []domain.ReviewerConfig{{Harness: domain.ReviewerHarness("greptile")}}}} + launcher := &fakeLauncher{handle: "review-mer-1"} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), projects, launcher) + + res, err := eng.Trigger(context.Background(), "mer-1") + if err != nil { + t.Fatalf("Trigger: %v", err) + } + if res.Run.Harness != domain.ReviewerHarness("greptile") || launcher.gotSpec.Harness != domain.ReviewerHarness("greptile") { + t.Fatalf("harness not used: run=%+v spec=%+v", res.Run, launcher.gotSpec) + } +} + +func TestTriggerRejectsBadWorkerState(t *testing.T) { + t.Run("unknown worker", func(t *testing.T) { + eng := newEngineForTest(&fakeStore{}, fakeSessions{ok: false}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) + if _, err := eng.Trigger(context.Background(), "mer-1"); !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } + }) + t.Run("no pr", func(t *testing.T) { + eng := newEngineForTest(&fakeStore{}, fakeSessions{rec: liveWorker(), ok: true}, fakePRs{}, fakeProjects{}, &fakeLauncher{}) + if _, err := eng.Trigger(context.Background(), "mer-1"); !errors.Is(err, ErrInvalid) { + t.Fatalf("err = %v, want ErrInvalid", err) + } + }) +} + +func TestSubmitRecordsVerdictAndBody(t *testing.T) { + store := &fakeStore{runs: []domain.ReviewRun{{ID: "run-1", SessionID: "mer-1", Status: domain.ReviewRunRunning}}} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) + + run, err := eng.Submit(context.Background(), "mer-1", "run-1", domain.VerdictChangesRequested, "please fix") + if err != nil { + t.Fatalf("Submit: %v", err) + } + if run.Status != domain.ReviewRunComplete || run.Verdict != domain.VerdictChangesRequested || run.Body != "please fix" { + t.Fatalf("run = %+v", run) + } +} + +func TestSubmitValidationAndOwnership(t *testing.T) { + store := &fakeStore{runs: []domain.ReviewRun{{ID: "run-1", SessionID: "other", Status: domain.ReviewRunRunning}}} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) + + if _, err := eng.Submit(context.Background(), "mer-1", "", domain.VerdictApproved, ""); !errors.Is(err, ErrInvalid) { + t.Fatalf("missing run id err = %v", err) + } + if _, err := eng.Submit(context.Background(), "mer-1", "run-1", "garbage", "b"); !errors.Is(err, ErrInvalid) { + t.Fatalf("bad verdict err = %v", err) + } + if _, err := eng.Submit(context.Background(), "mer-1", "missing", domain.VerdictApproved, ""); !errors.Is(err, ErrNotFound) { + t.Fatalf("unknown run err = %v", err) + } + if _, err := eng.Submit(context.Background(), "mer-1", "run-1", domain.VerdictApproved, ""); !errors.Is(err, ErrInvalid) { + t.Fatalf("ownership err = %v", err) + } +} + +func TestListReturnsHandleAndRuns(t *testing.T) { + store := &fakeStore{ + review: &domain.Review{ID: "rev-1", SessionID: "mer-1", ReviewerHandleID: "review-mer-1"}, + runs: []domain.ReviewRun{{ID: "run-1", SessionID: "mer-1", TargetSHA: "sha1"}}, + } + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) + got, err := eng.List(context.Background(), "mer-1") + if err != nil { + t.Fatalf("List: %v", err) + } + if got.ReviewerHandleID != "review-mer-1" || len(got.Runs) != 1 { + t.Fatalf("list = %+v", got) + } +} diff --git a/backend/internal/service/review/review.go b/backend/internal/service/review/review.go index 2e23c6b8..1bfc8e3f 100644 --- a/backend/internal/service/review/review.go +++ b/backend/internal/service/review/review.go @@ -1,108 +1,53 @@ -// Package review is the daemon's code-review surface: review runs against a -// session's PR and the findings they produce. -// -// This is an in-memory implementation. Execution is not yet wired to a real -// review agent — Execute records a pending run so the HTTP surface is live and -// the dashboard renders against real endpoints; agent-backed findings and -// persistence are a follow-up. Mirrors agent-orchestrator's reviews feature -// (packages/web/src/app/api/reviews) on reverbcode's daemon. +// Package review is the daemon's HTTP-facing code-review service boundary. The +// core orchestration lives in internal/review; this layer is the thin contract +// the API controller depends on and delegates to the engine, so the same engine +// can also back a future in-process CLI trigger. package review import ( "context" - "errors" - "fmt" - "sync" - "time" -) -// ErrInvalid and ErrNotFound let the HTTP layer map service failures to 422/404. -var ( - ErrInvalid = errors.New("review: invalid input") - ErrNotFound = errors.New("review: not found") + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + reviewcore "github.com/aoagents/agent-orchestrator/backend/internal/review" ) -// Severity ranks a finding by how much it should block the human; one of -// "info" | "warning" | "error". Kept as a plain string on the wire. -const ( - SeverityInfo = "info" - SeverityWarning = "warning" - SeverityError = "error" +// ErrInvalid and ErrNotFound re-export the engine sentinels so the HTTP +// controller maps service failures to 422/404 without importing the core. +var ( + ErrInvalid = reviewcore.ErrInvalid + ErrNotFound = reviewcore.ErrNotFound ) -// Finding is one review comment produced for a run. -type Finding struct { - ID string `json:"id"` - Path string `json:"path"` - Line int `json:"line"` - Severity string `json:"severity"` - Body string `json:"body"` -} - -// Run is one code-review execution against a session's PR. -type Run struct { - ID string `json:"id"` - SessionID string `json:"sessionId"` - // Status is one of: pending | complete | sent. - Status string `json:"status"` - CreatedAt time.Time `json:"createdAt"` - Findings []Finding `json:"findings"` -} - // Manager is the reviews surface the HTTP controller depends on. type Manager interface { - List(ctx context.Context) ([]Run, error) - Execute(ctx context.Context, sessionID string) (Run, error) - Send(ctx context.Context, id string) (Run, error) + Trigger(ctx context.Context, workerID domain.SessionID) (reviewcore.TriggerResult, error) + Submit(ctx context.Context, workerID domain.SessionID, runID string, verdict domain.ReviewVerdict, body string) (domain.ReviewRun, error) + List(ctx context.Context, workerID domain.SessionID) (reviewcore.SessionReviews, error) } -type memStore struct { - mu sync.Mutex - runs map[string]*Run - seq int +// Service is the API-facing review service. It delegates to the core engine. +type Service struct { + engine *reviewcore.Engine } -// NewInMemory returns an in-memory Manager. Runs do not survive a daemon -// restart. -func NewInMemory() Manager { - return &memStore{runs: map[string]*Run{}} +var _ Manager = (*Service)(nil) + +// New wraps a core review engine as the API-facing service. +func New(engine *reviewcore.Engine) *Service { + return &Service{engine: engine} } -func (s *memStore) List(_ context.Context) ([]Run, error) { - s.mu.Lock() - defer s.mu.Unlock() - out := make([]Run, 0, len(s.runs)) - for _, run := range s.runs { - out = append(out, *run) - } - return out, nil +// Trigger starts (or reuses) a review pass for a worker's PR. +func (s *Service) Trigger(ctx context.Context, workerID domain.SessionID) (reviewcore.TriggerResult, error) { + return s.engine.Trigger(ctx, workerID) } -func (s *memStore) Execute(_ context.Context, sessionID string) (Run, error) { - if sessionID == "" { - return Run{}, fmt.Errorf("%w: sessionId is required", ErrInvalid) - } - s.mu.Lock() - defer s.mu.Unlock() - s.seq++ - run := &Run{ - ID: fmt.Sprintf("rev-%d", s.seq), - SessionID: sessionID, - Status: "pending", - CreatedAt: time.Now().UTC(), - Findings: []Finding{}, - } - s.runs[run.ID] = run - return *run, nil +// Submit records a reviewer's result for a specific worker review pass. +func (s *Service) Submit(ctx context.Context, workerID domain.SessionID, runID string, verdict domain.ReviewVerdict, body string) (domain.ReviewRun, error) { + return s.engine.Submit(ctx, workerID, runID, verdict, body) } -func (s *memStore) Send(_ context.Context, id string) (Run, error) { - s.mu.Lock() - defer s.mu.Unlock() - run, ok := s.runs[id] - if !ok { - return Run{}, fmt.Errorf("%w: review %q", ErrNotFound, id) - } - run.Status = "sent" - return *run, nil +// List returns a worker's review state. +func (s *Service) List(ctx context.Context, workerID domain.SessionID) (reviewcore.SessionReviews, error) { + return s.engine.List(ctx, workerID) } diff --git a/backend/internal/storage/sqlite/gen/models.go b/backend/internal/storage/sqlite/gen/models.go index f8d614bf..57b4f285 100644 --- a/backend/internal/storage/sqlite/gen/models.go +++ b/backend/internal/storage/sqlite/gen/models.go @@ -111,6 +111,30 @@ type Project struct { Kind string } +type Review struct { + ID string + SessionID domain.SessionID + ProjectID domain.ProjectID + Harness domain.ReviewerHarness + PRURL string + ReviewerHandleID string + CreatedAt time.Time + UpdatedAt time.Time +} + +type ReviewRun struct { + ID string + ReviewID string + SessionID domain.SessionID + Harness domain.ReviewerHarness + PRURL string + TargetSha string + Status domain.ReviewRunStatus + Verdict domain.ReviewVerdict + Body string + CreatedAt time.Time +} + type Session struct { ID domain.SessionID ProjectID domain.ProjectID diff --git a/backend/internal/storage/sqlite/gen/review.sql.go b/backend/internal/storage/sqlite/gen/review.sql.go new file mode 100644 index 00000000..c075e2b5 --- /dev/null +++ b/backend/internal/storage/sqlite/gen/review.sql.go @@ -0,0 +1,217 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: review.sql + +package gen + +import ( + "context" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +const getReviewBySession = `-- name: GetReviewBySession :one +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review WHERE session_id = ? +` + +func (q *Queries) GetReviewBySession(ctx context.Context, sessionID domain.SessionID) (Review, error) { + row := q.db.QueryRowContext(ctx, getReviewBySession, sessionID) + var i Review + err := row.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.Harness, + &i.PRURL, + &i.ReviewerHandleID, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + +const getReviewRun = `-- name: GetReviewRun :one +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE id = ? +` + +func (q *Queries) GetReviewRun(ctx context.Context, id string) (ReviewRun, error) { + row := q.db.QueryRowContext(ctx, getReviewRun, id) + var i ReviewRun + err := row.Scan( + &i.ID, + &i.ReviewID, + &i.SessionID, + &i.Harness, + &i.PRURL, + &i.TargetSha, + &i.Status, + &i.Verdict, + &i.Body, + &i.CreatedAt, + ) + return i, err +} + +const getReviewRunBySessionAndSHA = `-- name: GetReviewRunBySessionAndSHA :one +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? AND target_sha = ? ORDER BY created_at DESC LIMIT 1 +` + +type GetReviewRunBySessionAndSHAParams struct { + SessionID domain.SessionID + TargetSha string +} + +func (q *Queries) GetReviewRunBySessionAndSHA(ctx context.Context, arg GetReviewRunBySessionAndSHAParams) (ReviewRun, error) { + row := q.db.QueryRowContext(ctx, getReviewRunBySessionAndSHA, arg.SessionID, arg.TargetSha) + var i ReviewRun + err := row.Scan( + &i.ID, + &i.ReviewID, + &i.SessionID, + &i.Harness, + &i.PRURL, + &i.TargetSha, + &i.Status, + &i.Verdict, + &i.Body, + &i.CreatedAt, + ) + return i, err +} + +const insertReviewRun = `-- name: InsertReviewRun :exec +INSERT INTO review_run (id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at) +VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +` + +type InsertReviewRunParams struct { + ID string + ReviewID string + SessionID domain.SessionID + Harness domain.ReviewerHarness + PRURL string + TargetSha string + Status domain.ReviewRunStatus + Verdict domain.ReviewVerdict + Body string + CreatedAt time.Time +} + +func (q *Queries) InsertReviewRun(ctx context.Context, arg InsertReviewRunParams) error { + _, err := q.db.ExecContext(ctx, insertReviewRun, + arg.ID, + arg.ReviewID, + arg.SessionID, + arg.Harness, + arg.PRURL, + arg.TargetSha, + arg.Status, + arg.Verdict, + arg.Body, + arg.CreatedAt, + ) + return err +} + +const listReviewRunsBySession = `-- name: ListReviewRunsBySession :many +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? ORDER BY created_at DESC +` + +func (q *Queries) ListReviewRunsBySession(ctx context.Context, sessionID domain.SessionID) ([]ReviewRun, error) { + rows, err := q.db.QueryContext(ctx, listReviewRunsBySession, sessionID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []ReviewRun{} + for rows.Next() { + var i ReviewRun + if err := rows.Scan( + &i.ID, + &i.ReviewID, + &i.SessionID, + &i.Harness, + &i.PRURL, + &i.TargetSha, + &i.Status, + &i.Verdict, + &i.Body, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const updateReviewRunResult = `-- name: UpdateReviewRunResult :execrows +UPDATE review_run SET status = ?, verdict = ?, body = ? WHERE id = ? AND status = 'running' +` + +type UpdateReviewRunResultParams struct { + Status domain.ReviewRunStatus + Verdict domain.ReviewVerdict + Body string + ID string +} + +func (q *Queries) UpdateReviewRunResult(ctx context.Context, arg UpdateReviewRunResultParams) (int64, error) { + result, err := q.db.ExecContext(ctx, updateReviewRunResult, + arg.Status, + arg.Verdict, + arg.Body, + arg.ID, + ) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + +const upsertReview = `-- name: UpsertReview :exec +INSERT INTO review (id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at) +VALUES (?, ?, ?, ?, ?, ?, ?, ?) +ON CONFLICT (session_id) DO UPDATE SET + harness = excluded.harness, + pr_url = excluded.pr_url, + reviewer_handle_id = excluded.reviewer_handle_id, + updated_at = excluded.updated_at +` + +type UpsertReviewParams struct { + ID string + SessionID domain.SessionID + ProjectID domain.ProjectID + Harness domain.ReviewerHarness + PRURL string + ReviewerHandleID string + CreatedAt time.Time + UpdatedAt time.Time +} + +func (q *Queries) UpsertReview(ctx context.Context, arg UpsertReviewParams) error { + _, err := q.db.ExecContext(ctx, upsertReview, + arg.ID, + arg.SessionID, + arg.ProjectID, + arg.Harness, + arg.PRURL, + arg.ReviewerHandleID, + arg.CreatedAt, + arg.UpdatedAt, + ) + return err +} diff --git a/backend/internal/storage/sqlite/migrations/0011_add_review_tables.sql b/backend/internal/storage/sqlite/migrations/0011_add_review_tables.sql new file mode 100644 index 00000000..88419a3a --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0011_add_review_tables.sql @@ -0,0 +1,45 @@ +-- Configurable AO code review (issue #192). review holds one row per worker +-- session under review (session_id UNIQUE); a repeat trigger reuses the row. +-- review_run holds the per-pass facts. The reviewer agent posts its review to +-- the PR itself; `ao review submit` records the verdict and body on the run. + +-- +goose Up +-- +goose StatementBegin +CREATE TABLE review ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL UNIQUE REFERENCES sessions (id) ON DELETE CASCADE, + project_id TEXT NOT NULL REFERENCES projects (id), + harness TEXT NOT NULL, + pr_url TEXT NOT NULL DEFAULT '', + -- runtime handle id of the live reviewer pane, reused across passes and + -- exposed so the UI can attach its terminal over /mux. + reviewer_handle_id TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL +); +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TABLE review_run ( + id TEXT PRIMARY KEY, + review_id TEXT NOT NULL REFERENCES review (id) ON DELETE CASCADE, + session_id TEXT NOT NULL REFERENCES sessions (id) ON DELETE CASCADE, + harness TEXT NOT NULL, + pr_url TEXT NOT NULL DEFAULT '', + -- the commit the pass reviewed; lets a repeat trigger for the same head + -- short-circuit to the existing run. + target_sha TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'running', + verdict TEXT NOT NULL DEFAULT '', + body TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL +); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TABLE review_run; +-- +goose StatementEnd +-- +goose StatementBegin +DROP TABLE review; +-- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/review.sql b/backend/internal/storage/sqlite/queries/review.sql new file mode 100644 index 00000000..1151c946 --- /dev/null +++ b/backend/internal/storage/sqlite/queries/review.sql @@ -0,0 +1,31 @@ +-- name: UpsertReview :exec +INSERT INTO review (id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at) +VALUES (?, ?, ?, ?, ?, ?, ?, ?) +ON CONFLICT (session_id) DO UPDATE SET + harness = excluded.harness, + pr_url = excluded.pr_url, + reviewer_handle_id = excluded.reviewer_handle_id, + updated_at = excluded.updated_at; + +-- name: GetReviewBySession :one +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review WHERE session_id = ?; + +-- name: InsertReviewRun :exec +INSERT INTO review_run (id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at) +VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?); + +-- name: UpdateReviewRunResult :execrows +UPDATE review_run SET status = ?, verdict = ?, body = ? WHERE id = ? AND status = 'running'; + +-- name: GetReviewRun :one +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE id = ?; + +-- name: GetReviewRunBySessionAndSHA :one +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? AND target_sha = ? ORDER BY created_at DESC LIMIT 1; + +-- name: ListReviewRunsBySession :many +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? ORDER BY created_at DESC; diff --git a/backend/internal/storage/sqlite/store/review_store.go b/backend/internal/storage/sqlite/store/review_store.go new file mode 100644 index 00000000..36dfc9c7 --- /dev/null +++ b/backend/internal/storage/sqlite/store/review_store.go @@ -0,0 +1,141 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite/gen" +) + +// UpsertReview inserts the per-worker review row, or reuses the existing one +// (session_id is unique) by refreshing its harness/pr_url/updated_at. +func (s *Store) UpsertReview(ctx context.Context, r domain.Review) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() + return s.qw.UpsertReview(ctx, gen.UpsertReviewParams{ + ID: r.ID, + SessionID: r.SessionID, + ProjectID: r.ProjectID, + Harness: r.Harness, + PRURL: r.PRURL, + ReviewerHandleID: r.ReviewerHandleID, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, + }) +} + +// GetReviewBySession returns the review row for a worker session, ok=false if none. +func (s *Store) GetReviewBySession(ctx context.Context, id domain.SessionID) (domain.Review, bool, error) { + row, err := s.qr.GetReviewBySession(ctx, id) + if errors.Is(err, sql.ErrNoRows) { + return domain.Review{}, false, nil + } + if err != nil { + return domain.Review{}, false, fmt.Errorf("get review by session %s: %w", id, err) + } + return reviewFromRow(row), true, nil +} + +// InsertReviewRun records a new review pass. +func (s *Store) InsertReviewRun(ctx context.Context, r domain.ReviewRun) error { + s.writeMu.Lock() + defer s.writeMu.Unlock() + return s.qw.InsertReviewRun(ctx, gen.InsertReviewRunParams{ + ID: r.ID, + ReviewID: r.ReviewID, + SessionID: r.SessionID, + Harness: r.Harness, + PRURL: r.PRURL, + TargetSha: r.TargetSHA, + Status: r.Status, + Verdict: r.Verdict, + Body: r.Body, + CreatedAt: r.CreatedAt, + }) +} + +// UpdateReviewRunResult sets the status/verdict/body of a running review pass. +func (s *Store) UpdateReviewRunResult(ctx context.Context, id string, status domain.ReviewRunStatus, verdict domain.ReviewVerdict, body string) (bool, error) { + s.writeMu.Lock() + defer s.writeMu.Unlock() + n, err := s.qw.UpdateReviewRunResult(ctx, gen.UpdateReviewRunResultParams{ + Status: status, + Verdict: verdict, + Body: body, + ID: id, + }) + if err != nil { + return false, err + } + return n > 0, nil +} + +// GetReviewRun returns one review pass by id. +func (s *Store) GetReviewRun(ctx context.Context, id string) (domain.ReviewRun, bool, error) { + row, err := s.qr.GetReviewRun(ctx, id) + if errors.Is(err, sql.ErrNoRows) { + return domain.ReviewRun{}, false, nil + } + if err != nil { + return domain.ReviewRun{}, false, fmt.Errorf("get review run %s: %w", id, err) + } + return reviewRunFromRow(row), true, nil +} + +// GetReviewRunBySessionAndSHA returns the most recent review pass for a worker +// session at a specific commit, ok=false if none. It lets a repeat trigger for +// the same PR head short-circuit to the existing run. +func (s *Store) GetReviewRunBySessionAndSHA(ctx context.Context, id domain.SessionID, targetSHA string) (domain.ReviewRun, bool, error) { + row, err := s.qr.GetReviewRunBySessionAndSHA(ctx, gen.GetReviewRunBySessionAndSHAParams{SessionID: id, TargetSha: targetSHA}) + if errors.Is(err, sql.ErrNoRows) { + return domain.ReviewRun{}, false, nil + } + if err != nil { + return domain.ReviewRun{}, false, fmt.Errorf("get review run for session %s sha %s: %w", id, targetSHA, err) + } + return reviewRunFromRow(row), true, nil +} + +// ListReviewRunsBySession returns all review passes for a worker session, newest first. +func (s *Store) ListReviewRunsBySession(ctx context.Context, id domain.SessionID) ([]domain.ReviewRun, error) { + rows, err := s.qr.ListReviewRunsBySession(ctx, id) + if err != nil { + return nil, fmt.Errorf("list review runs for session %s: %w", id, err) + } + out := make([]domain.ReviewRun, 0, len(rows)) + for _, row := range rows { + out = append(out, reviewRunFromRow(row)) + } + return out, nil +} + +func reviewFromRow(r gen.Review) domain.Review { + return domain.Review{ + ID: r.ID, + SessionID: r.SessionID, + ProjectID: r.ProjectID, + Harness: r.Harness, + PRURL: r.PRURL, + ReviewerHandleID: r.ReviewerHandleID, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, + } +} + +func reviewRunFromRow(r gen.ReviewRun) domain.ReviewRun { + return domain.ReviewRun{ + ID: r.ID, + ReviewID: r.ReviewID, + SessionID: r.SessionID, + Harness: r.Harness, + PRURL: r.PRURL, + TargetSHA: r.TargetSha, + Status: r.Status, + Verdict: r.Verdict, + Body: r.Body, + CreatedAt: r.CreatedAt, + } +} diff --git a/backend/internal/storage/sqlite/store/review_store_test.go b/backend/internal/storage/sqlite/store/review_store_test.go new file mode 100644 index 00000000..e6643ac4 --- /dev/null +++ b/backend/internal/storage/sqlite/store/review_store_test.go @@ -0,0 +1,111 @@ +package store_test + +import ( + "context" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + rec, err := s.CreateSession(ctx, sampleRecord("mer")) + if err != nil { + t.Fatalf("create session: %v", err) + } + now := time.Now().UTC().Truncate(time.Second) + + // First upsert creates the review row. + if err := s.UpsertReview(ctx, domain.Review{ + ID: "rev-1", SessionID: rec.ID, ProjectID: rec.ProjectID, + Harness: domain.ReviewerClaudeCode, PRURL: "https://example/pr/1", + ReviewerHandleID: "review-mer-1", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("upsert review: %v", err) + } + // Second upsert with the same session reuses the row (session_id UNIQUE), + // refreshing harness/pr_url/reviewer_handle_id but keeping the original id. + if err := s.UpsertReview(ctx, domain.Review{ + ID: "rev-2", SessionID: rec.ID, ProjectID: rec.ProjectID, + Harness: domain.ReviewerHarness("greptile"), PRURL: "https://example/pr/2", + ReviewerHandleID: "review-mer-1b", + CreatedAt: now, UpdatedAt: now.Add(time.Second), + }); err != nil { + t.Fatalf("upsert review (reuse): %v", err) + } + got, ok, err := s.GetReviewBySession(ctx, rec.ID) + if err != nil || !ok { + t.Fatalf("get review: ok=%v err=%v", ok, err) + } + if got.ID != "rev-1" { + t.Fatalf("upsert created a new row, want reuse: id=%q", got.ID) + } + if got.Harness != domain.ReviewerHarness("greptile") || got.PRURL != "https://example/pr/2" || got.ReviewerHandleID != "review-mer-1b" { + t.Fatalf("upsert did not refresh fields: %+v", got) + } + + // A run inserts running and updates to complete/changes_requested. + if err := s.InsertReviewRun(ctx, domain.ReviewRun{ + ID: "run-1", ReviewID: got.ID, SessionID: rec.ID, Harness: domain.ReviewerHarness("greptile"), + PRURL: got.PRURL, TargetSHA: "sha1", Status: domain.ReviewRunRunning, Verdict: domain.VerdictNone, + CreatedAt: now, + }); err != nil { + t.Fatalf("insert run: %v", err) + } + if ok, err := s.UpdateReviewRunResult(ctx, "run-1", domain.ReviewRunComplete, domain.VerdictChangesRequested, "please fix"); err != nil { + t.Fatalf("update run: %v", err) + } else if !ok { + t.Fatal("update run: got ok=false") + } + + gotRun, ok, err := s.GetReviewRun(ctx, "run-1") + if err != nil || !ok { + t.Fatalf("get run: ok=%v err=%v", ok, err) + } + if gotRun.ID != "run-1" || gotRun.SessionID != rec.ID || gotRun.TargetSHA != "sha1" { + t.Fatalf("get run = %+v", gotRun) + } + + bySHA, ok, err := s.GetReviewRunBySessionAndSHA(ctx, rec.ID, "sha1") + if err != nil || !ok { + t.Fatalf("by sha: ok=%v err=%v", ok, err) + } + if bySHA.Status != domain.ReviewRunComplete || bySHA.Verdict != domain.VerdictChangesRequested || bySHA.Body != "please fix" { + t.Fatalf("run result not persisted: %+v", bySHA) + } + if _, ok, _ := s.GetReviewRunBySessionAndSHA(ctx, rec.ID, "other"); ok { + t.Fatal("unexpected run for a different sha") + } + + runs, err := s.ListReviewRunsBySession(ctx, rec.ID) + if err != nil { + t.Fatalf("list runs: %v", err) + } + if len(runs) != 1 || runs[0].ID != "run-1" { + t.Fatalf("list runs = %+v", runs) + } + + if ok, err := s.UpdateReviewRunResult(ctx, "run-1", domain.ReviewRunComplete, domain.VerdictApproved, "again"); err != nil { + t.Fatalf("second update: %v", err) + } else if ok { + t.Fatal("second update completed an already-complete run") + } +} + +func TestReviewGettersMissing(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + if _, ok, err := s.GetReviewBySession(ctx, "mer-1"); err != nil || ok { + t.Fatalf("missing review: ok=%v err=%v", ok, err) + } + if _, ok, err := s.GetReviewRunBySessionAndSHA(ctx, "mer-1", "sha1"); err != nil || ok { + t.Fatalf("missing run: ok=%v err=%v", ok, err) + } + if _, ok, err := s.GetReviewRun(ctx, "run-missing"); err != nil || ok { + t.Fatalf("missing run by id: ok=%v err=%v", ok, err) + } +} diff --git a/backend/sqlc.yaml b/backend/sqlc.yaml index 1813a599..d9a8ad1f 100644 --- a/backend/sqlc.yaml +++ b/backend/sqlc.yaml @@ -92,3 +92,31 @@ sql: go_type: import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" type: "ActivityState" + - column: "review.session_id" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "SessionID" + - column: "review.project_id" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ProjectID" + - column: "review.harness" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ReviewerHarness" + - column: "review_run.session_id" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "SessionID" + - column: "review_run.harness" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ReviewerHarness" + - column: "review_run.status" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ReviewRunStatus" + - column: "review_run.verdict" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ReviewVerdict" diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 93d306ad..17dc2e75 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -143,41 +143,43 @@ export interface paths { patch?: never; trace?: never; }; - "/api/v1/reviews": { + "/api/v1/sessions": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - /** List code-review runs */ - get: operations["listReviews"]; + /** List sessions */ + get: operations["listSessions"]; put?: never; - post?: never; + /** Spawn a new agent session */ + post: operations["spawnSession"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/reviews/{id}/send": { + "/api/v1/sessions/{sessionId}": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - get?: never; + /** Fetch one session */ + get: operations["getSession"]; put?: never; - /** Send a review run's findings to its PR */ - post: operations["sendReview"]; + post?: never; delete?: never; options?: never; head?: never; - patch?: never; + /** Rename a session display name */ + patch: operations["renameSession"]; trace?: never; }; - "/api/v1/reviews/execute": { + "/api/v1/sessions/{sessionId}/activity": { parameters: { query?: never; header?: never; @@ -186,51 +188,49 @@ export interface paths { }; get?: never; put?: never; - /** Start a code-review run for a session's PR */ - post: operations["executeReview"]; + /** Report an agent activity-state signal for a session */ + post: operations["setSessionActivity"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/sessions": { + "/api/v1/sessions/{sessionId}/kill": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - /** List sessions */ - get: operations["listSessions"]; + get?: never; put?: never; - /** Spawn a new agent session */ - post: operations["spawnSession"]; + /** Mark a session terminated and tear down runtime/workspace resources */ + post: operations["killSession"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}": { + "/api/v1/sessions/{sessionId}/pr": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - /** Fetch one session */ - get: operations["getSession"]; + /** List pull requests owned by a session */ + get: operations["listSessionPRs"]; put?: never; post?: never; delete?: never; options?: never; head?: never; - /** Rename a session display name */ - patch: operations["renameSession"]; + patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}/activity": { + "/api/v1/sessions/{sessionId}/pr/claim": { parameters: { query?: never; header?: never; @@ -239,15 +239,15 @@ export interface paths { }; get?: never; put?: never; - /** Report an agent activity-state signal for a session */ - post: operations["setSessionActivity"]; + /** Claim an existing pull request for a session */ + post: operations["claimSessionPR"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}/kill": { + "/api/v1/sessions/{sessionId}/restore": { parameters: { query?: never; header?: never; @@ -256,23 +256,23 @@ export interface paths { }; get?: never; put?: never; - /** Mark a session terminated and tear down runtime/workspace resources */ - post: operations["killSession"]; + /** Restore a terminated session */ + post: operations["restoreSession"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}/pr": { + "/api/v1/sessions/{sessionId}/reviews": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - /** List pull requests owned by a session */ - get: operations["listSessionPRs"]; + /** List a worker's code-review runs */ + get: operations["listReviews"]; put?: never; post?: never; delete?: never; @@ -281,7 +281,7 @@ export interface paths { patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}/pr/claim": { + "/api/v1/sessions/{sessionId}/reviews/submit": { parameters: { query?: never; header?: never; @@ -290,15 +290,15 @@ export interface paths { }; get?: never; put?: never; - /** Claim an existing pull request for a session */ - post: operations["claimSessionPR"]; + /** Record a reviewer's result for a worker's PR */ + post: operations["submitReview"]; delete?: never; options?: never; head?: never; patch?: never; trace?: never; }; - "/api/v1/sessions/{sessionId}/restore": { + "/api/v1/sessions/{sessionId}/reviews/trigger": { parameters: { query?: never; header?: never; @@ -307,8 +307,8 @@ export interface paths { }; get?: never; put?: never; - /** Restore a terminated session */ - post: operations["restoreSession"]; + /** Trigger a code review of a worker's PR */ + post: operations["triggerReview"]; delete?: never; options?: never; head?: never; @@ -422,9 +422,8 @@ export interface components { lastActivityAt: string; state: string; }; - ExecuteReviewInput: { - /** @description Session whose PR to review. */ - sessionId: string; + DomainReviewerConfig: { + harness: string; }; KillSessionResponse: { freed?: boolean; @@ -435,6 +434,7 @@ export interface components { projects: components["schemas"]["ProjectSummary"][]; }; ListReviewsResponse: { + reviewerHandleId: string; reviews: components["schemas"]["ReviewRun"][]; }; ListSessionPRsResponse: { @@ -473,6 +473,7 @@ export interface components { }; orchestrator?: components["schemas"]["RoleOverride"]; postCreate?: string[]; + reviewers?: components["schemas"]["DomainReviewerConfig"][]; sessionPrefix?: string; symlinks?: string[]; worker?: components["schemas"]["RoleOverride"]; @@ -515,23 +516,22 @@ export interface components { session: components["schemas"]["Session"]; sessionId: string; }; - ReviewFinding: { - body: string; - id: string; - line: number; - path: string; - severity: string; - }; - ReviewResponse: { - review: components["schemas"]["ReviewRun"]; - }; ReviewRun: { + body: string; /** Format: date-time */ createdAt: string; - findings: components["schemas"]["ReviewFinding"][]; + harness: string; id: string; + prUrl: string; + reviewId: string; sessionId: string; status: string; + targetSha: string; + verdict: string; + }; + ReviewRunResponse: { + review: components["schemas"]["ReviewRun"]; + reviewerHandleId: string; }; RoleOverride: { agent?: string; @@ -614,6 +614,14 @@ export interface components { projectId: string; prompt?: string; }; + SubmitReviewInput: { + /** @description Review body recorded by AO. Required for changes_requested. */ + body: string; + /** @description Review run id being completed. */ + runId: string; + /** @description Review verdict: approved or changes_requested. */ + verdict: string; + }; WorkspaceRepo: { name: string; relativePath: string; @@ -1160,127 +1168,6 @@ export interface operations { }; }; }; - listReviews: { - parameters: { - query?: never; - header?: never; - path?: never; - cookie?: never; - }; - requestBody?: never; - responses: { - /** @description OK */ - 200: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["ListReviewsResponse"]; - }; - }; - /** @description Not Implemented */ - 501: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - }; - }; - sendReview: { - parameters: { - query?: never; - header?: never; - path: { - /** @description Review run id. */ - id: string; - }; - cookie?: never; - }; - requestBody?: never; - responses: { - /** @description OK */ - 200: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["ReviewResponse"]; - }; - }; - /** @description Not Found */ - 404: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - /** @description Not Implemented */ - 501: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - }; - }; - executeReview: { - parameters: { - query?: never; - header?: never; - path?: never; - cookie?: never; - }; - requestBody: { - content: { - "application/json": components["schemas"]["ExecuteReviewInput"]; - }; - }; - responses: { - /** @description Created */ - 201: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["ReviewResponse"]; - }; - }; - /** @description Bad Request */ - 400: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - /** @description Unprocessable Entity */ - 422: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - /** @description Not Implemented */ - 501: { - headers: { - [name: string]: unknown; - }; - content: { - "application/json": components["schemas"]["APIError"]; - }; - }; - }; - }; listSessions: { parameters: { query?: { @@ -1777,6 +1664,169 @@ export interface operations { }; }; }; + listReviews: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Session identifier, e.g. project-1. */ + sessionId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ListReviewsResponse"]; + }; + }; + /** @description Unprocessable Entity */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; + submitReview: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Session identifier, e.g. project-1. */ + sessionId: string; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SubmitReviewInput"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ReviewRunResponse"]; + }; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Unprocessable Entity */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; + triggerReview: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Session identifier, e.g. project-1. */ + sessionId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ReviewRunResponse"]; + }; + }; + /** @description Created */ + 201: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ReviewRunResponse"]; + }; + }; + /** @description Not Found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Unprocessable Entity */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; rollbackSession: { parameters: { query?: never;