diff --git a/backend/internal/domain/review.go b/backend/internal/domain/review.go index 8b38a40f..21d83173 100644 --- a/backend/internal/domain/review.go +++ b/backend/internal/domain/review.go @@ -6,10 +6,10 @@ import ( ) // ErrDuplicateReviewRun is returned by InsertReviewRun when a run already exists -// for the same worker session and target commit (the partial unique index from -// migration 0013). It lets the review engine fall back to the recorded run -// instead of surfacing a raw storage error after a reviewer may have launched. -var ErrDuplicateReviewRun = errors.New("domain: review run already exists for session and target sha") +// for the same worker session, PR, and target commit. It lets the review engine +// fall back to the recorded run instead of surfacing a raw storage error after a +// reviewer may have launched. +var ErrDuplicateReviewRun = errors.New("domain: review run already exists for session, PR, and target sha") // 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 diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 3e067d4d..8c38eb4c 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -912,6 +912,14 @@ paths: schema: description: Session identifier, e.g. project-1. type: string + - description: Tracked PR URL to list review runs for. Omit to list all PR review + targets for the session. + in: query + name: prUrl + schema: + description: Tracked PR URL to list review runs for. Omit to list all PR + review targets for the session. + type: string responses: "200": content: @@ -996,6 +1004,11 @@ paths: schema: description: Session identifier, e.g. project-1. type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/TriggerReviewInput' responses: "200": content: @@ -1332,9 +1345,14 @@ components: items: $ref: '#/components/schemas/ReviewRun' type: array + targets: + items: + $ref: '#/components/schemas/ReviewTargetResponse' + type: array required: - reviewerHandleId - reviews + - targets type: object ListSessionPRsResponse: properties: @@ -1644,6 +1662,21 @@ components: - review - reviewerHandleId type: object + ReviewTargetResponse: + properties: + prUrl: + type: string + reviewerHandleId: + type: string + reviews: + items: + $ref: '#/components/schemas/ReviewRun' + type: array + required: + - prUrl + - reviewerHandleId + - reviews + type: object RoleOverride: properties: agent: @@ -1876,6 +1909,13 @@ components: - verdict - body type: object + TriggerReviewInput: + properties: + prUrl: + description: Tracked PR URL to review. Required when the session owns multiple + PRs. + type: string + type: object WorkspaceRepo: properties: name: diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 2aeca734..42197164 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -84,7 +84,11 @@ func Build() ([]byte, error) { // AddReqStructure leaves requestBody.required absent, which // OpenAPI reads as optional. These bodies are mandatory, so force // it — otherwise validators/generators treat the body as skippable. - oc.AddReqStructure(op.reqBody, openapi.WithCustomize(markRequestBodyRequired)) + if op.optionalReqBody { + oc.AddReqStructure(op.reqBody) + } else { + oc.AddReqStructure(op.reqBody, openapi.WithCustomize(markRequestBodyRequired)) + } } for _, resp := range op.resps { opts := []openapi.ContentOption{openapi.WithHTTPStatus(resp.status)} @@ -167,9 +171,12 @@ var schemaNames = map[string]string{ "ControllersResolveCommentsRequest": "ResolveCommentsRequest", "ControllersResolveCommentsResponse": "ResolveCommentsResponse", // httpd/controllers — review wire envelopes - "ControllersListReviewsResponse": "ListReviewsResponse", - "ControllersReviewRunResponse": "ReviewRunResponse", - "ControllersSubmitReviewInput": "SubmitReviewInput", + "ControllersListReviewsResponse": "ListReviewsResponse", + "ControllersListReviewsQuery": "ListReviewsQuery", + "ControllersReviewRunResponse": "ReviewRunResponse", + "ControllersReviewTargetResponse": "ReviewTargetResponse", + "ControllersTriggerReviewInput": "TriggerReviewInput", + "ControllersSubmitReviewInput": "SubmitReviewInput", // domain review entities "DomainReviewRun": "ReviewRun", // service/project entities + DTOs @@ -248,6 +255,7 @@ type operation struct { tag string pathParams []any // path/query param containers (e.g. ProjectIDParam) reqBody any // JSON request body struct, nil when the op takes none + optionalReqBody bool resps []respUnit contentTypes map[int]string // optional non-JSON response content types by status } @@ -297,7 +305,7 @@ func reviewOperations() []operation { { 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{}}, + pathParams: []any{controllers.SessionIDParam{}, controllers.ListReviewsQuery{}}, resps: []respUnit{ {http.StatusOK, controllers.ListReviewsResponse{}}, {http.StatusUnprocessableEntity, envelope.APIError{}}, @@ -306,8 +314,10 @@ func reviewOperations() []operation { }, { 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{}}, + summary: "Trigger a code review of a worker's PR", + pathParams: []any{controllers.SessionIDParam{}}, + reqBody: controllers.TriggerReviewInput{}, + optionalReqBody: true, resps: []respUnit{ {http.StatusOK, controllers.ReviewRunResponse{}}, {http.StatusCreated, controllers.ReviewRunResponse{}}, diff --git a/backend/internal/httpd/controllers/reviews.go b/backend/internal/httpd/controllers/reviews.go index b9fc3c68..4a7b27d1 100644 --- a/backend/internal/httpd/controllers/reviews.go +++ b/backend/internal/httpd/controllers/reviews.go @@ -3,6 +3,7 @@ package controllers import ( "encoding/json" "errors" + "io" "net/http" "github.com/go-chi/chi/v5" @@ -13,12 +14,26 @@ import ( reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review" ) +// ListReviewsQuery selects one PR's review state. Omit prUrl to list every PR +// review target for the session. +type ListReviewsQuery struct { + PRURL string `query:"prUrl,omitempty" description:"Tracked PR URL to list review runs for. Omit to list all PR review targets for the session."` +} + +// ReviewTargetResponse is one PR's review state within a worker session. +type ReviewTargetResponse struct { + PRURL string `json:"prUrl"` + ReviewerHandleID string `json:"reviewerHandleId"` + Reviews []domain.ReviewRun `json:"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 { - ReviewerHandleID string `json:"reviewerHandleId"` - Reviews []domain.ReviewRun `json:"reviews"` + ReviewerHandleID string `json:"reviewerHandleId"` + Reviews []domain.ReviewRun `json:"reviews"` + Targets []ReviewTargetResponse `json:"targets"` } // ReviewRunResponse is the body of trigger (200/201) and submit (200). It @@ -28,6 +43,12 @@ type ReviewRunResponse struct { ReviewerHandleID string `json:"reviewerHandleId"` } +// TriggerReviewInput is the optional body of +// POST /api/v1/sessions/{sessionId}/reviews/trigger. +type TriggerReviewInput struct { + PRURL string `json:"prUrl,omitempty" description:"Tracked PR URL to review. Required when the session owns multiple PRs."` +} + // 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."` @@ -52,7 +73,7 @@ func (c *ReviewsController) list(w http.ResponseWriter, r *http.Request) { apispec.NotImplemented(w, r, "GET", "/api/v1/sessions/{sessionId}/reviews") return } - res, err := c.Svc.List(r.Context(), sessionID(r)) + res, err := c.Svc.List(r.Context(), sessionID(r), r.URL.Query().Get("prUrl")) if err != nil { writeReviewError(w, r, err) return @@ -61,7 +82,7 @@ func (c *ReviewsController) list(w http.ResponseWriter, r *http.Request) { if runs == nil { runs = []domain.ReviewRun{} } - envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{ReviewerHandleID: res.ReviewerHandleID, Reviews: runs}) + envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{ReviewerHandleID: res.ReviewerHandleID, Reviews: runs, Targets: reviewTargetsResponse(res.Targets)}) } func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) { @@ -69,7 +90,12 @@ func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) { apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/reviews/trigger") return } - res, err := c.Svc.Trigger(r.Context(), sessionID(r)) + var in TriggerReviewInput + if err := json.NewDecoder(r.Body).Decode(&in); err != nil && !errors.Is(err, io.EOF) { + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_BODY", "Invalid request body", nil) + return + } + res, err := c.Svc.Trigger(r.Context(), sessionID(r), in.PRURL) if err != nil { writeReviewError(w, r, err) return @@ -83,6 +109,22 @@ func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) { envelope.WriteJSON(w, status, ReviewRunResponse{Review: res.Run, ReviewerHandleID: res.ReviewerHandleID}) } +func reviewTargetsResponse(targets []reviewsvc.ReviewTarget) []ReviewTargetResponse { + out := make([]ReviewTargetResponse, 0, len(targets)) + for _, target := range targets { + runs := target.Runs + if runs == nil { + runs = []domain.ReviewRun{} + } + out = append(out, ReviewTargetResponse{ + PRURL: target.PRURL, + ReviewerHandleID: target.ReviewerHandleID, + Reviews: runs, + }) + } + return out +} + func (c *ReviewsController) submit(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/reviews/submit") diff --git a/backend/internal/httpd/controllers/reviews_test.go b/backend/internal/httpd/controllers/reviews_test.go new file mode 100644 index 00000000..ad422df9 --- /dev/null +++ b/backend/internal/httpd/controllers/reviews_test.go @@ -0,0 +1,104 @@ +package controllers + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + reviewcore "github.com/aoagents/agent-orchestrator/backend/internal/review" +) + +type fakeReviewsService struct { + listWorker domain.SessionID + listPRURL string + triggerWorker domain.SessionID + triggerPRURL string +} + +func (f *fakeReviewsService) Trigger(_ context.Context, workerID domain.SessionID, prURL string) (reviewcore.TriggerResult, error) { + f.triggerWorker = workerID + f.triggerPRURL = prURL + return reviewcore.TriggerResult{ + Run: domain.ReviewRun{ID: "run-1", SessionID: workerID, PRURL: prURL}, + ReviewerHandleID: "reviewer-1", + Created: true, + }, nil +} + +func (f *fakeReviewsService) Submit(_ context.Context, workerID domain.SessionID, runID string, verdict domain.ReviewVerdict, body string) (domain.ReviewRun, error) { + return domain.ReviewRun{ID: runID, SessionID: workerID, Verdict: verdict, Body: body}, nil +} + +func (f *fakeReviewsService) List(_ context.Context, workerID domain.SessionID, prURL string) (reviewcore.SessionReviews, error) { + f.listWorker = workerID + f.listPRURL = prURL + return reviewcore.SessionReviews{ + ReviewerHandleID: "reviewer-1", + Runs: []domain.ReviewRun{{ID: "run-1", SessionID: workerID, PRURL: prURL}}, + Targets: []reviewcore.ReviewTarget{{PRURL: prURL, ReviewerHandleID: "reviewer-1", Runs: []domain.ReviewRun{{ID: "run-1", SessionID: workerID, PRURL: prURL}}}}, + }, nil +} + +func reviewTestRouter(svc *fakeReviewsService) http.Handler { + r := chi.NewRouter() + (&ReviewsController{Svc: svc}).Register(r) + return r +} + +func TestReviewsListPassesPRURLQuery(t *testing.T) { + svc := &fakeReviewsService{} + req := httptest.NewRequest(http.MethodGet, "/sessions/mer-1/reviews?prUrl=https%3A%2F%2Fgithub.com%2Fo%2Fr%2Fpull%2F1", nil) + rec := httptest.NewRecorder() + + reviewTestRouter(svc).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d body=%s", rec.Code, rec.Body.String()) + } + if svc.listWorker != "mer-1" || svc.listPRURL != "https://github.com/o/r/pull/1" { + t.Fatalf("list args worker=%q pr=%q", svc.listWorker, svc.listPRURL) + } + var out ListReviewsResponse + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode response: %v", err) + } + if len(out.Targets) != 1 || out.Targets[0].PRURL != "https://github.com/o/r/pull/1" { + t.Fatalf("targets = %+v", out.Targets) + } +} + +func TestReviewsTriggerAcceptsOptionalPRURLBody(t *testing.T) { + svc := &fakeReviewsService{} + req := httptest.NewRequest(http.MethodPost, "/sessions/mer-1/reviews/trigger", strings.NewReader(`{"prUrl":"https://github.com/o/r/pull/1"}`)) + rec := httptest.NewRecorder() + + reviewTestRouter(svc).ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d body=%s", rec.Code, rec.Body.String()) + } + if svc.triggerWorker != "mer-1" || svc.triggerPRURL != "https://github.com/o/r/pull/1" { + t.Fatalf("trigger args worker=%q pr=%q", svc.triggerWorker, svc.triggerPRURL) + } +} + +func TestReviewsTriggerAcceptsEmptyBody(t *testing.T) { + svc := &fakeReviewsService{} + req := httptest.NewRequest(http.MethodPost, "/sessions/mer-1/reviews/trigger", nil) + rec := httptest.NewRecorder() + + reviewTestRouter(svc).ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d body=%s", rec.Code, rec.Body.String()) + } + if svc.triggerWorker != "mer-1" || svc.triggerPRURL != "" { + t.Fatalf("trigger args worker=%q pr=%q", svc.triggerWorker, svc.triggerPRURL) + } +} diff --git a/backend/internal/review/review.go b/backend/internal/review/review.go index 9b064625..6924ad67 100644 --- a/backend/internal/review/review.go +++ b/backend/internal/review/review.go @@ -30,12 +30,14 @@ var ( // 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) + GetReviewBySessionAndPR(ctx context.Context, id domain.SessionID, prURL string) (domain.Review, bool, error) + ListReviewsBySession(ctx context.Context, id domain.SessionID) ([]domain.Review, 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) + GetReviewRunBySessionPRAndSHA(ctx context.Context, id domain.SessionID, prURL, targetSHA string) (domain.ReviewRun, bool, error) ListReviewRunsBySession(ctx context.Context, id domain.SessionID) ([]domain.ReviewRun, error) + ListReviewRunsBySessionAndPR(ctx context.Context, id domain.SessionID, prURL string) ([]domain.ReviewRun, error) } // Sessions resolves the worker session under review. @@ -134,11 +136,19 @@ type TriggerResult struct { Created bool } -// SessionReviews is a worker's review state: the live reviewer handle plus its -// recorded passes, newest first. +// ReviewTarget is one PR's review state within a worker session. +type ReviewTarget struct { + PRURL string + ReviewerHandleID string + Runs []domain.ReviewRun +} + +// SessionReviews is a worker's review state. ReviewerHandleID and Runs preserve +// the original single-PR response shape; Targets carries the PR-scoped state. type SessionReviews struct { ReviewerHandleID string Runs []domain.ReviewRun + Targets []ReviewTarget } // Trigger starts (or reuses) a review of a worker's PR at its current head: @@ -146,7 +156,7 @@ type SessionReviews struct { // - 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) { +func (e *Engine) Trigger(ctx context.Context, workerID domain.SessionID, prURL string) (TriggerResult, error) { if workerID == "" { return TriggerResult{}, fmt.Errorf("%w: worker session id is required", ErrInvalid) } @@ -172,19 +182,19 @@ func (e *Engine) Trigger(ctx context.Context, workerID domain.SessionID) (Trigge return TriggerResult{}, fmt.Errorf("%w: worker session %q has no workspace to review", ErrInvalid, workerID) } - pr, err := e.workerPR(ctx, workerID) + pr, err := e.workerPR(ctx, workerID, prURL) if err != nil { return TriggerResult{}, err } targetSHA := pr.HeadSHA - review, hasReview, err := e.store.GetReviewBySession(ctx, workerID) + review, hasReview, err := e.store.GetReviewBySessionAndPR(ctx, workerID, pr.URL) 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 { + if existing, ok, err := e.store.GetReviewRunBySessionPRAndSHA(ctx, workerID, pr.URL, targetSHA); err != nil { return TriggerResult{}, err } else if ok { return TriggerResult{Run: existing, ReviewerHandleID: review.ReviewerHandleID, Created: false}, nil @@ -246,12 +256,12 @@ func (e *Engine) Trigger(ctx context.Context, workerID domain.SessionID) (Trigge } if err := e.store.InsertReviewRun(ctx, run); err != nil { // The per-worker lock serialises in-process triggers, but the unique - // index (migration 0013) can still reject a run a concurrent daemon (or - // a pre-lock restart) recorded for this commit. The reviewer is already + // index can still reject a run a concurrent daemon (or a pre-lock + // restart) recorded for this PR commit. The reviewer is already // launched, so don't surface a raw error: re-read the recorded run and // return it as the existing, not-newly-created pass. if errors.Is(err, domain.ErrDuplicateReviewRun) { - if existing, ok, getErr := e.store.GetReviewRunBySessionAndSHA(ctx, workerID, targetSHA); getErr != nil { + if existing, ok, getErr := e.store.GetReviewRunBySessionPRAndSHA(ctx, workerID, pr.URL, targetSHA); getErr != nil { return TriggerResult{}, getErr } else if ok { return TriggerResult{Run: existing, ReviewerHandleID: handleID, Created: false}, nil @@ -307,24 +317,53 @@ func (e *Engine) Submit(ctx context.Context, workerID domain.SessionID, runID st } // 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) { +func (e *Engine) List(ctx context.Context, workerID domain.SessionID, prURL string) (SessionReviews, error) { if workerID == "" { return SessionReviews{}, fmt.Errorf("%w: worker session id is required", ErrInvalid) } + if prURL != "" { + pr, err := e.workerPR(ctx, workerID, prURL) + if err != nil { + return SessionReviews{}, err + } + runs, err := e.store.ListReviewRunsBySessionAndPR(ctx, workerID, pr.URL) + if err != nil { + return SessionReviews{}, err + } + handle := "" + if review, ok, err := e.store.GetReviewBySessionAndPR(ctx, workerID, pr.URL); err != nil { + return SessionReviews{}, err + } else if ok { + handle = review.ReviewerHandleID + } + return SessionReviews{ + ReviewerHandleID: handle, + Runs: runs, + Targets: []ReviewTarget{{PRURL: pr.URL, ReviewerHandleID: handle, Runs: runs}}, + }, nil + } + 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 { + reviews, err := e.store.ListReviewsBySession(ctx, workerID) + if err != nil { return SessionReviews{}, err - } else if ok { - handle = review.ReviewerHandleID } - return SessionReviews{ReviewerHandleID: handle, Runs: runs}, nil + prs, err := e.prs.ListPRsBySession(ctx, workerID) + if err != nil { + return SessionReviews{}, err + } + targets := reviewTargets(prs, reviews, runs) + handle := "" + if len(targets) == 1 { + handle = targets[0].ReviewerHandleID + } + return SessionReviews{ReviewerHandleID: handle, Runs: runs, Targets: targets}, nil } -func (e *Engine) workerPR(ctx context.Context, workerID domain.SessionID) (domain.PullRequest, error) { +func (e *Engine) workerPR(ctx context.Context, workerID domain.SessionID, prURL string) (domain.PullRequest, error) { prs, err := e.prs.ListPRsBySession(ctx, workerID) if err != nil { return domain.PullRequest{}, err @@ -332,9 +371,60 @@ func (e *Engine) workerPR(ctx context.Context, workerID domain.SessionID) (domai if len(prs) == 0 { return domain.PullRequest{}, fmt.Errorf("%w: worker %q has no PR to review", ErrInvalid, workerID) } + if prURL != "" { + for _, pr := range prs { + if pr.URL == prURL { + return pr, nil + } + } + return domain.PullRequest{}, fmt.Errorf("%w: PR %q is not tracked by worker %q", ErrInvalid, prURL, workerID) + } + if len(prs) > 1 { + return domain.PullRequest{}, fmt.Errorf("%w: worker %q has multiple PRs; prUrl is required", ErrInvalid, workerID) + } return prs[0], nil } +func reviewTargets(prs []domain.PullRequest, reviews []domain.Review, runs []domain.ReviewRun) []ReviewTarget { + reviewByPR := make(map[string]domain.Review, len(reviews)) + for _, review := range reviews { + reviewByPR[review.PRURL] = review + } + runsByPR := make(map[string][]domain.ReviewRun) + for _, run := range runs { + runsByPR[run.PRURL] = append(runsByPR[run.PRURL], run) + } + seen := make(map[string]bool, len(prs)+len(reviews)+len(runsByPR)) + targets := make([]ReviewTarget, 0, len(prs)) + for _, pr := range prs { + targets = append(targets, reviewTarget(pr.URL, reviewByPR, runsByPR)) + seen[pr.URL] = true + } + for _, review := range reviews { + if !seen[review.PRURL] { + targets = append(targets, reviewTarget(review.PRURL, reviewByPR, runsByPR)) + seen[review.PRURL] = true + } + } + for prURL := range runsByPR { + if !seen[prURL] { + targets = append(targets, reviewTarget(prURL, reviewByPR, runsByPR)) + } + } + return targets +} + +func reviewTarget(prURL string, reviewByPR map[string]domain.Review, runsByPR map[string][]domain.ReviewRun) ReviewTarget { + target := ReviewTarget{PRURL: prURL, Runs: runsByPR[prURL]} + if review, ok := reviewByPR[prURL]; ok { + target.ReviewerHandleID = review.ReviewerHandleID + } + if target.Runs == nil { + target.Runs = []domain.ReviewRun{} + } + return target +} + // 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. @@ -351,7 +441,7 @@ func (e *Engine) reviewerHarness(ctx context.Context, worker domain.SessionRecor } 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) + existing, ok, err := e.store.GetReviewBySessionAndPR(ctx, worker.ID, prURL) if err != nil { return domain.Review{}, err } @@ -367,7 +457,7 @@ func (e *Engine) upsertReview(ctx context.Context, worker domain.SessionRecord, } if ok { // Reuse the existing row's identity and creation time; UpsertReview - // refreshes harness/pr_url/reviewer_handle_id/updated_at. + // refreshes harness/reviewer_handle_id/updated_at. review.ID = existing.ID review.CreatedAt = existing.CreatedAt } diff --git a/backend/internal/review/review_test.go b/backend/internal/review/review_test.go index 5a0a2eff..fd788e5e 100644 --- a/backend/internal/review/review_test.go +++ b/backend/internal/review/review_test.go @@ -13,25 +13,41 @@ import ( // --- fakes --- type fakeStore struct { - review *domain.Review - runs []domain.ReviewRun + reviews []domain.Review + runs []domain.ReviewRun // insertErr, when set, makes the next InsertReviewRun model a concurrent // writer that already recorded a run for this commit: it records that - // winner (so a follow-up GetReviewRunBySessionAndSHA finds it) and returns + // winner (so a follow-up GetReviewRunBySessionPRAndSHA finds it) and returns // insertErr instead of recording the caller's run. insertErr error } func (f *fakeStore) UpsertReview(_ context.Context, r domain.Review) error { - cp := r - f.review = &cp + for i := range f.reviews { + if f.reviews[i].SessionID == r.SessionID && f.reviews[i].PRURL == r.PRURL { + f.reviews[i] = r + return nil + } + } + f.reviews = append(f.reviews, r) return nil } -func (f *fakeStore) GetReviewBySession(_ context.Context, _ domain.SessionID) (domain.Review, bool, error) { - if f.review == nil { - return domain.Review{}, false, nil +func (f *fakeStore) GetReviewBySessionAndPR(_ context.Context, id domain.SessionID, prURL string) (domain.Review, bool, error) { + for _, review := range f.reviews { + if review.SessionID == id && review.PRURL == prURL { + return review, true, nil + } + } + return domain.Review{}, false, nil +} +func (f *fakeStore) ListReviewsBySession(_ context.Context, id domain.SessionID) ([]domain.Review, error) { + var out []domain.Review + for _, review := range f.reviews { + if review.SessionID == id { + out = append(out, review) + } } - return *f.review, true, nil + return out, nil } func (f *fakeStore) InsertReviewRun(_ context.Context, r domain.ReviewRun) error { if f.insertErr != nil { @@ -65,16 +81,31 @@ func (f *fakeStore) GetReviewRun(_ context.Context, id string) (domain.ReviewRun } return domain.ReviewRun{}, false, nil } -func (f *fakeStore) GetReviewRunBySessionAndSHA(_ context.Context, _ domain.SessionID, sha string) (domain.ReviewRun, bool, error) { +func (f *fakeStore) GetReviewRunBySessionPRAndSHA(_ context.Context, id domain.SessionID, prURL, sha string) (domain.ReviewRun, bool, error) { for i := len(f.runs) - 1; i >= 0; i-- { - if f.runs[i].TargetSHA == sha { + if f.runs[i].SessionID == id && f.runs[i].PRURL == prURL && 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 +func (f *fakeStore) ListReviewRunsBySession(_ context.Context, id domain.SessionID) ([]domain.ReviewRun, error) { + var out []domain.ReviewRun + for _, run := range f.runs { + if run.SessionID == id { + out = append(out, run) + } + } + return out, nil +} +func (f *fakeStore) ListReviewRunsBySessionAndPR(_ context.Context, id domain.SessionID, prURL string) ([]domain.ReviewRun, error) { + var out []domain.ReviewRun + for _, run := range f.runs { + if run.SessionID == id && run.PRURL == prURL { + out = append(out, run) + } + } + return out, nil } type fakeSessions struct { @@ -146,7 +177,11 @@ func newEngineForTest(store Store, sessions Sessions, prs PRs, projects Projects } func prAt(sha string) fakePRs { - return fakePRs{prs: []domain.PullRequest{{URL: "https://github.com/o/r/pull/1", HeadSHA: sha}}} + return prAtURL("https://github.com/o/r/pull/1", sha) +} + +func prAtURL(url, sha string) fakePRs { + return fakePRs{prs: []domain.PullRequest{{URL: url, HeadSHA: sha}}} } // --- tests --- @@ -156,7 +191,7 @@ func TestTriggerSpawnsNewReviewerAndRecordsRunAfterLaunch(t *testing.T) { 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") + res, err := eng.Trigger(context.Background(), "mer-1", "") if err != nil { t.Fatalf("Trigger: %v", err) } @@ -172,8 +207,8 @@ func TestTriggerSpawnsNewReviewerAndRecordsRunAfterLaunch(t *testing.T) { 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) + if len(store.runs) != 1 || len(store.reviews) != 1 || store.reviews[0].ReviewerHandleID != "review-mer-1" { + t.Fatalf("persisted reviews=%+v runs=%+v", store.reviews, store.runs) } } @@ -190,7 +225,7 @@ func TestTriggerConcurrentSameWorkerSpawnsOnce(t *testing.T) { for i := 0; i < n; i++ { go func(i int) { defer wg.Done() - results[i], errs[i] = eng.Trigger(context.Background(), "mer-1") + results[i], errs[i] = eng.Trigger(context.Background(), "mer-1", "") }(i) } wg.Wait() @@ -223,7 +258,7 @@ func TestTriggerFallsBackToExistingRunOnUniqueConflict(t *testing.T) { 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") + res, err := eng.Trigger(context.Background(), "mer-1", "") if err != nil { t.Fatalf("Trigger: %v", err) } @@ -240,13 +275,13 @@ func TestTriggerFallsBackToExistingRunOnUniqueConflict(t *testing.T) { 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}}, + reviews: []domain.Review{{ID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/1", ReviewerHandleID: "review-mer-1"}}, + runs: []domain.ReviewRun{{ID: "run-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/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") + res, err := eng.Trigger(context.Background(), "mer-1", "") if err != nil { t.Fatalf("Trigger: %v", err) } @@ -263,13 +298,13 @@ func TestTriggerIsIdempotentForSameCommit(t *testing.T) { 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}}, + reviews: []domain.Review{{ID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/1", ReviewerHandleID: "review-mer-1"}}, + runs: []domain.ReviewRun{{ID: "run-0", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/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") + res, err := eng.Trigger(context.Background(), "mer-1", "") if err != nil { t.Fatalf("Trigger: %v", err) } @@ -286,13 +321,13 @@ func TestTriggerNotifiesLiveReviewerOnNewCommit(t *testing.T) { 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}}, + reviews: []domain.Review{{ID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/1", ReviewerHandleID: "review-mer-1"}}, + runs: []domain.ReviewRun{{ID: "run-0", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/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 { + if _, err := eng.Trigger(context.Background(), "mer-1", ""); err != nil { t.Fatalf("Trigger: %v", err) } if !launcher.spawned || launcher.notified { @@ -305,11 +340,11 @@ func TestTriggerLaunchFailureRecordsNothing(t *testing.T) { 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 { + 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) + if len(store.runs) != 0 || len(store.reviews) != 0 { + t.Fatalf("nothing should be persisted on launch failure: reviews=%+v runs=%+v", store.reviews, store.runs) } } @@ -319,7 +354,7 @@ func TestTriggerUsesConfiguredReviewerHarness(t *testing.T) { 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") + res, err := eng.Trigger(context.Background(), "mer-1", "") if err != nil { t.Fatalf("Trigger: %v", err) } @@ -331,18 +366,58 @@ func TestTriggerUsesConfiguredReviewerHarness(t *testing.T) { 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) { + 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) { + if _, err := eng.Trigger(context.Background(), "mer-1", ""); !errors.Is(err, ErrInvalid) { + t.Fatalf("err = %v, want ErrInvalid", err) + } + }) + t.Run("multiple prs require selector", func(t *testing.T) { + prs := fakePRs{prs: []domain.PullRequest{ + {URL: "https://github.com/o/r/pull/1", HeadSHA: "sha1"}, + {URL: "https://github.com/o/r2/pull/2", HeadSHA: "sha2"}, + }} + eng := newEngineForTest(&fakeStore{}, fakeSessions{rec: liveWorker(), ok: true}, prs, fakeProjects{}, &fakeLauncher{}) + if _, err := eng.Trigger(context.Background(), "mer-1", ""); !errors.Is(err, ErrInvalid) { + t.Fatalf("err = %v, want ErrInvalid", err) + } + }) + t.Run("unknown selected pr", func(t *testing.T) { + eng := newEngineForTest(&fakeStore{}, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) + if _, err := eng.Trigger(context.Background(), "mer-1", "https://github.com/o/r/pull/404"); !errors.Is(err, ErrInvalid) { t.Fatalf("err = %v, want ErrInvalid", err) } }) } +func TestTriggerUsesSelectedPRAndScopesIdempotencyByPR(t *testing.T) { + store := &fakeStore{ + reviews: []domain.Review{{ID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/api/pull/1", ReviewerHandleID: "review-api"}}, + runs: []domain.ReviewRun{{ID: "run-api", ReviewID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/api/pull/1", TargetSHA: "same-sha", Status: domain.ReviewRunRunning}}, + } + prs := fakePRs{prs: []domain.PullRequest{ + {URL: "https://github.com/o/api/pull/1", HeadSHA: "same-sha"}, + {URL: "https://github.com/o/web/pull/2", HeadSHA: "same-sha"}, + }} + launcher := &fakeLauncher{handle: "review-web"} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prs, fakeProjects{}, launcher) + + res, err := eng.Trigger(context.Background(), "mer-1", "https://github.com/o/web/pull/2") + if err != nil { + t.Fatalf("Trigger: %v", err) + } + if !res.Created || res.Run.PRURL != "https://github.com/o/web/pull/2" || res.Run.ID == "run-api" { + t.Fatalf("selected PR did not get its own run: %+v", res) + } + if !launcher.spawned || launcher.gotSpec.PRURL != "https://github.com/o/web/pull/2" { + t.Fatalf("launcher spec = %+v", launcher.gotSpec) + } +} + 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{}) @@ -376,15 +451,44 @@ func TestSubmitValidationAndOwnership(t *testing.T) { 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"}}, + reviews: []domain.Review{{ID: "rev-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/1", ReviewerHandleID: "review-mer-1"}}, + runs: []domain.ReviewRun{{ID: "run-1", SessionID: "mer-1", PRURL: "https://github.com/o/r/pull/1", TargetSHA: "sha1"}}, } eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prAt("sha1"), fakeProjects{}, &fakeLauncher{}) - got, err := eng.List(context.Background(), "mer-1") + 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 { + if got.ReviewerHandleID != "review-mer-1" || len(got.Runs) != 1 || len(got.Targets) != 1 { t.Fatalf("list = %+v", got) } } + +func TestListCanSelectOnePR(t *testing.T) { + store := &fakeStore{ + reviews: []domain.Review{ + {ID: "rev-api", SessionID: "mer-1", PRURL: "https://github.com/o/api/pull/1", ReviewerHandleID: "review-api"}, + {ID: "rev-web", SessionID: "mer-1", PRURL: "https://github.com/o/web/pull/2", ReviewerHandleID: "review-web"}, + }, + runs: []domain.ReviewRun{ + {ID: "run-api", SessionID: "mer-1", PRURL: "https://github.com/o/api/pull/1", TargetSHA: "sha1"}, + {ID: "run-web", SessionID: "mer-1", PRURL: "https://github.com/o/web/pull/2", TargetSHA: "sha2"}, + }, + } + prs := fakePRs{prs: []domain.PullRequest{ + {URL: "https://github.com/o/api/pull/1", HeadSHA: "sha1"}, + {URL: "https://github.com/o/web/pull/2", HeadSHA: "sha2"}, + }} + eng := newEngineForTest(store, fakeSessions{rec: liveWorker(), ok: true}, prs, fakeProjects{}, &fakeLauncher{}) + + got, err := eng.List(context.Background(), "mer-1", "https://github.com/o/web/pull/2") + if err != nil { + t.Fatalf("List: %v", err) + } + if got.ReviewerHandleID != "review-web" || len(got.Runs) != 1 || got.Runs[0].ID != "run-web" { + t.Fatalf("selected list = %+v", got) + } + if len(got.Targets) != 1 || got.Targets[0].PRURL != "https://github.com/o/web/pull/2" { + t.Fatalf("targets = %+v", got.Targets) + } +} diff --git a/backend/internal/service/review/review.go b/backend/internal/service/review/review.go index 1bfc8e3f..13594220 100644 --- a/backend/internal/service/review/review.go +++ b/backend/internal/service/review/review.go @@ -18,11 +18,14 @@ var ( ErrNotFound = reviewcore.ErrNotFound ) +// ReviewTarget is one PR's review state within a worker session. +type ReviewTarget = reviewcore.ReviewTarget + // Manager is the reviews surface the HTTP controller depends on. type Manager interface { - Trigger(ctx context.Context, workerID domain.SessionID) (reviewcore.TriggerResult, error) + Trigger(ctx context.Context, workerID domain.SessionID, prURL string) (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) + List(ctx context.Context, workerID domain.SessionID, prURL string) (reviewcore.SessionReviews, error) } // Service is the API-facing review service. It delegates to the core engine. @@ -38,8 +41,8 @@ func New(engine *reviewcore.Engine) *Service { } // 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 *Service) Trigger(ctx context.Context, workerID domain.SessionID, prURL string) (reviewcore.TriggerResult, error) { + return s.engine.Trigger(ctx, workerID, prURL) } // Submit records a reviewer's result for a specific worker review pass. @@ -48,6 +51,6 @@ func (s *Service) Submit(ctx context.Context, workerID domain.SessionID, runID s } // 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) +func (s *Service) List(ctx context.Context, workerID domain.SessionID, prURL string) (reviewcore.SessionReviews, error) { + return s.engine.List(ctx, workerID, prURL) } diff --git a/backend/internal/storage/sqlite/gen/review.sql.go b/backend/internal/storage/sqlite/gen/review.sql.go index c075e2b5..4e7571f8 100644 --- a/backend/internal/storage/sqlite/gen/review.sql.go +++ b/backend/internal/storage/sqlite/gen/review.sql.go @@ -12,13 +12,18 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -const getReviewBySession = `-- name: GetReviewBySession :one +const getReviewBySessionAndPR = `-- name: GetReviewBySessionAndPR :one SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at -FROM review WHERE session_id = ? +FROM review WHERE session_id = ? AND pr_url = ? ` -func (q *Queries) GetReviewBySession(ctx context.Context, sessionID domain.SessionID) (Review, error) { - row := q.db.QueryRowContext(ctx, getReviewBySession, sessionID) +type GetReviewBySessionAndPRParams struct { + SessionID domain.SessionID + PRURL string +} + +func (q *Queries) GetReviewBySessionAndPR(ctx context.Context, arg GetReviewBySessionAndPRParams) (Review, error) { + row := q.db.QueryRowContext(ctx, getReviewBySessionAndPR, arg.SessionID, arg.PRURL) var i Review err := row.Scan( &i.ID, @@ -56,18 +61,19 @@ func (q *Queries) GetReviewRun(ctx context.Context, id string) (ReviewRun, error return i, err } -const getReviewRunBySessionAndSHA = `-- name: GetReviewRunBySessionAndSHA :one +const getReviewRunBySessionPRAndSHA = `-- name: GetReviewRunBySessionPRAndSHA :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 +FROM review_run WHERE session_id = ? AND pr_url = ? AND target_sha = ? ORDER BY created_at DESC LIMIT 1 ` -type GetReviewRunBySessionAndSHAParams struct { +type GetReviewRunBySessionPRAndSHAParams struct { SessionID domain.SessionID + PRURL string TargetSha string } -func (q *Queries) GetReviewRunBySessionAndSHA(ctx context.Context, arg GetReviewRunBySessionAndSHAParams) (ReviewRun, error) { - row := q.db.QueryRowContext(ctx, getReviewRunBySessionAndSHA, arg.SessionID, arg.TargetSha) +func (q *Queries) GetReviewRunBySessionPRAndSHA(ctx context.Context, arg GetReviewRunBySessionPRAndSHAParams) (ReviewRun, error) { + row := q.db.QueryRowContext(ctx, getReviewRunBySessionPRAndSHA, arg.SessionID, arg.PRURL, arg.TargetSha) var i ReviewRun err := row.Scan( &i.ID, @@ -157,6 +163,87 @@ func (q *Queries) ListReviewRunsBySession(ctx context.Context, sessionID domain. return items, nil } +const listReviewRunsBySessionAndPR = `-- name: ListReviewRunsBySessionAndPR :many +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? AND pr_url = ? ORDER BY created_at DESC +` + +type ListReviewRunsBySessionAndPRParams struct { + SessionID domain.SessionID + PRURL string +} + +func (q *Queries) ListReviewRunsBySessionAndPR(ctx context.Context, arg ListReviewRunsBySessionAndPRParams) ([]ReviewRun, error) { + rows, err := q.db.QueryContext(ctx, listReviewRunsBySessionAndPR, arg.SessionID, arg.PRURL) + 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 listReviewsBySession = `-- name: ListReviewsBySession :many +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review WHERE session_id = ? ORDER BY updated_at DESC +` + +func (q *Queries) ListReviewsBySession(ctx context.Context, sessionID domain.SessionID) ([]Review, error) { + rows, err := q.db.QueryContext(ctx, listReviewsBySession, sessionID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []Review{} + for rows.Next() { + var i Review + if err := rows.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.Harness, + &i.PRURL, + &i.ReviewerHandleID, + &i.CreatedAt, + &i.UpdatedAt, + ); 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' ` @@ -184,9 +271,8 @@ func (q *Queries) UpdateReviewRunResult(ctx context.Context, arg UpdateReviewRun 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 +ON CONFLICT (session_id, pr_url) DO UPDATE SET harness = excluded.harness, - pr_url = excluded.pr_url, reviewer_handle_id = excluded.reviewer_handle_id, updated_at = excluded.updated_at ` diff --git a/backend/internal/storage/sqlite/migrations/0014_review_per_pr.sql b/backend/internal/storage/sqlite/migrations/0014_review_per_pr.sql new file mode 100644 index 00000000..f574393d --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0014_review_per_pr.sql @@ -0,0 +1,61 @@ +-- Review rows are now scoped to one pull request within a worker session. +-- Workspace sessions can own multiple PRs, so session_id alone is not a +-- stable review identity. Review-run idempotency is likewise scoped to the PR. + +-- +goose Up +-- +goose StatementBegin +DROP INDEX IF EXISTS idx_review_run_session_sha; + +CREATE TABLE review_new ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL REFERENCES sessions (id) ON DELETE CASCADE, + project_id TEXT NOT NULL REFERENCES projects (id), + harness TEXT NOT NULL, + pr_url TEXT NOT NULL DEFAULT '', + reviewer_handle_id TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + UNIQUE (session_id, pr_url) +); + +INSERT INTO review_new (id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at) +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review; + +DROP TABLE review; +ALTER TABLE review_new RENAME TO review; + +CREATE INDEX idx_review_session ON review (session_id); +CREATE UNIQUE INDEX idx_review_run_session_pr_sha + ON review_run (session_id, pr_url, target_sha) WHERE target_sha != ''; +CREATE INDEX idx_review_run_session_pr_created ON review_run (session_id, pr_url, created_at); +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP INDEX IF EXISTS idx_review_run_session_pr_created; +DROP INDEX IF EXISTS idx_review_run_session_pr_sha; +DROP INDEX IF EXISTS idx_review_session; + +CREATE TABLE review_old ( + 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 '', + reviewer_handle_id TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL +); + +INSERT OR IGNORE INTO review_old (id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at) +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review +ORDER BY updated_at DESC; + +DROP TABLE review; +ALTER TABLE review_old RENAME TO review; + +CREATE UNIQUE INDEX idx_review_run_session_sha + ON review_run (session_id, target_sha) WHERE target_sha != ''; +-- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/review.sql b/backend/internal/storage/sqlite/queries/review.sql index 1151c946..d7e61146 100644 --- a/backend/internal/storage/sqlite/queries/review.sql +++ b/backend/internal/storage/sqlite/queries/review.sql @@ -1,15 +1,18 @@ -- 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 +ON CONFLICT (session_id, pr_url) 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 +-- name: GetReviewBySessionAndPR :one SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at -FROM review WHERE session_id = ?; +FROM review WHERE session_id = ? AND pr_url = ?; + +-- name: ListReviewsBySession :many +SELECT id, session_id, project_id, harness, pr_url, reviewer_handle_id, created_at, updated_at +FROM review WHERE session_id = ? ORDER BY updated_at DESC; -- name: InsertReviewRun :exec INSERT INTO review_run (id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at) @@ -22,10 +25,14 @@ UPDATE review_run SET status = ?, verdict = ?, body = ? WHERE id = ? AND status SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at FROM review_run WHERE id = ?; --- name: GetReviewRunBySessionAndSHA :one +-- name: GetReviewRunBySessionPRAndSHA :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; +FROM review_run WHERE session_id = ? AND pr_url = ? 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; + +-- name: ListReviewRunsBySessionAndPR :many +SELECT id, review_id, session_id, harness, pr_url, target_sha, status, verdict, body, created_at +FROM review_run WHERE session_id = ? AND pr_url = ? 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 index b872b33a..238624a1 100644 --- a/backend/internal/storage/sqlite/store/review_store.go +++ b/backend/internal/storage/sqlite/store/review_store.go @@ -10,8 +10,8 @@ import ( "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. +// UpsertReview inserts the per-PR review row, or reuses the existing one for +// the same worker session and PR by refreshing its harness/handle/updated_at. func (s *Store) UpsertReview(ctx context.Context, r domain.Review) error { s.writeMu.Lock() defer s.writeMu.Unlock() @@ -27,20 +27,33 @@ func (s *Store) UpsertReview(ctx context.Context, r domain.Review) error { }) } -// 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) +// GetReviewBySessionAndPR returns the review row for one session PR. +func (s *Store) GetReviewBySessionAndPR(ctx context.Context, id domain.SessionID, prURL string) (domain.Review, bool, error) { + row, err := s.qr.GetReviewBySessionAndPR(ctx, gen.GetReviewBySessionAndPRParams{SessionID: id, PRURL: prURL}) 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 domain.Review{}, false, fmt.Errorf("get review for session %s pr %s: %w", id, prURL, err) } return reviewFromRow(row), true, nil } +// ListReviewsBySession returns all per-PR review rows for a worker session. +func (s *Store) ListReviewsBySession(ctx context.Context, id domain.SessionID) ([]domain.Review, error) { + rows, err := s.qr.ListReviewsBySession(ctx, id) + if err != nil { + return nil, fmt.Errorf("list reviews for session %s: %w", id, err) + } + out := make([]domain.Review, 0, len(rows)) + for _, row := range rows { + out = append(out, reviewFromRow(row)) + } + return out, nil +} + // InsertReviewRun records a new review pass. A unique-constraint hit on the -// (session_id, target_sha) index (migration 0013) is surfaced as the sentinel +// (session_id, pr_url, target_sha) index is surfaced as the sentinel // domain.ErrDuplicateReviewRun so the engine can fall back to the existing run. func (s *Store) InsertReviewRun(ctx context.Context, r domain.ReviewRun) error { s.writeMu.Lock() @@ -58,7 +71,7 @@ func (s *Store) InsertReviewRun(ctx context.Context, r domain.ReviewRun) error { CreatedAt: r.CreatedAt, }) if isSQLiteUnique(err) { - return fmt.Errorf("insert review run for session %s sha %s: %w", r.SessionID, r.TargetSHA, domain.ErrDuplicateReviewRun) + return fmt.Errorf("insert review run for session %s pr %s sha %s: %w", r.SessionID, r.PRURL, r.TargetSHA, domain.ErrDuplicateReviewRun) } return err } @@ -91,16 +104,15 @@ func (s *Store) GetReviewRun(ctx context.Context, id string) (domain.ReviewRun, 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}) +// GetReviewRunBySessionPRAndSHA returns the most recent review pass for one +// session PR at a specific commit. +func (s *Store) GetReviewRunBySessionPRAndSHA(ctx context.Context, id domain.SessionID, prURL, targetSHA string) (domain.ReviewRun, bool, error) { + row, err := s.qr.GetReviewRunBySessionPRAndSHA(ctx, gen.GetReviewRunBySessionPRAndSHAParams{SessionID: id, PRURL: prURL, 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 domain.ReviewRun{}, false, fmt.Errorf("get review run for session %s pr %s sha %s: %w", id, prURL, targetSHA, err) } return reviewRunFromRow(row), true, nil } @@ -118,6 +130,19 @@ func (s *Store) ListReviewRunsBySession(ctx context.Context, id domain.SessionID return out, nil } +// ListReviewRunsBySessionAndPR returns review passes for one session PR. +func (s *Store) ListReviewRunsBySessionAndPR(ctx context.Context, id domain.SessionID, prURL string) ([]domain.ReviewRun, error) { + rows, err := s.qr.ListReviewRunsBySessionAndPR(ctx, gen.ListReviewRunsBySessionAndPRParams{SessionID: id, PRURL: prURL}) + if err != nil { + return nil, fmt.Errorf("list review runs for session %s pr %s: %w", id, prURL, 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, diff --git a/backend/internal/storage/sqlite/store/review_store_test.go b/backend/internal/storage/sqlite/store/review_store_test.go index bf20cd8d..66224be0 100644 --- a/backend/internal/storage/sqlite/store/review_store_test.go +++ b/backend/internal/storage/sqlite/store/review_store_test.go @@ -18,22 +18,23 @@ func TestInsertReviewRunDuplicateSHAMapsToSentinel(t *testing.T) { t.Fatalf("create session: %v", err) } now := time.Now().UTC().Truncate(time.Second) + prURL := "https://example/pr/1" if err := s.UpsertReview(ctx, domain.Review{ ID: "rev-1", SessionID: rec.ID, ProjectID: rec.ProjectID, - Harness: domain.ReviewerClaudeCode, CreatedAt: now, UpdatedAt: now, + Harness: domain.ReviewerClaudeCode, PRURL: prURL, CreatedAt: now, UpdatedAt: now, }); err != nil { t.Fatalf("upsert review: %v", err) } run := domain.ReviewRun{ ID: "run-1", ReviewID: "rev-1", SessionID: rec.ID, Harness: domain.ReviewerClaudeCode, - TargetSHA: "sha1", Status: domain.ReviewRunRunning, Verdict: domain.VerdictNone, CreatedAt: now, + PRURL: prURL, TargetSHA: "sha1", Status: domain.ReviewRunRunning, Verdict: domain.VerdictNone, CreatedAt: now, } if err := s.InsertReviewRun(ctx, run); err != nil { t.Fatalf("first insert: %v", err) } - // A second run for the same (session_id, target_sha) hits the partial unique - // index (migration 0013) and must surface as the sentinel so the engine can + // A second run for the same (session_id, pr_url, target_sha) hits the + // partial unique index and must surface as the sentinel so the engine can // fall back to the existing run. dup := run dup.ID = "run-2" @@ -51,7 +52,7 @@ func TestInsertReviewRunDuplicateSHAMapsToSentinel(t *testing.T) { } } -func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { +func TestReviewUpsertScopesRowsBySessionAndPR(t *testing.T) { s := newTestStore(t) ctx := context.Background() seedProject(t, s, "mer") @@ -61,40 +62,66 @@ func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { } now := time.Now().UTC().Truncate(time.Second) - // First upsert creates the review row. + pr1 := "https://example/pr/1" + pr2 := "https://example/pr/2" + + // First upsert creates the review row for PR 1. if err := s.UpsertReview(ctx, domain.Review{ ID: "rev-1", SessionID: rec.ID, ProjectID: rec.ProjectID, - Harness: domain.ReviewerClaudeCode, PRURL: "https://example/pr/1", + Harness: domain.ReviewerClaudeCode, PRURL: pr1, 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. + // A different PR in the same session gets a distinct review row. 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", + Harness: domain.ReviewerHarness("greptile"), PRURL: pr2, ReviewerHandleID: "review-mer-1b", CreatedAt: now, UpdatedAt: now.Add(time.Second), }); err != nil { - t.Fatalf("upsert review (reuse): %v", err) + t.Fatalf("upsert second review: %v", err) + } + // Upserting the same session+PR reuses that PR's row and keeps its id. + if err := s.UpsertReview(ctx, domain.Review{ + ID: "rev-3", SessionID: rec.ID, ProjectID: rec.ProjectID, + Harness: domain.ReviewerHarness("greptile"), PRURL: pr1, + ReviewerHandleID: "review-mer-1c", + CreatedAt: now, UpdatedAt: now.Add(2 * time.Second), + }); err != nil { + t.Fatalf("upsert first review again: %v", err) } - got, ok, err := s.GetReviewBySession(ctx, rec.ID) + + got, ok, err := s.GetReviewBySessionAndPR(ctx, rec.ID, pr1) 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" { + if got.Harness != domain.ReviewerHarness("greptile") || got.PRURL != pr1 || got.ReviewerHandleID != "review-mer-1c" { t.Fatalf("upsert did not refresh fields: %+v", got) } + reviews, err := s.ListReviewsBySession(ctx, rec.ID) + if err != nil { + t.Fatalf("list reviews: %v", err) + } + if len(reviews) != 2 { + t.Fatalf("reviews = %+v, want 2", reviews) + } // 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, + PRURL: pr1, TargetSHA: "sha1", Status: domain.ReviewRunRunning, Verdict: domain.VerdictNone, + CreatedAt: now, + }); err != nil { + t.Fatalf("insert run: %v", err) + } + if err := s.InsertReviewRun(ctx, domain.ReviewRun{ + ID: "run-2", ReviewID: "rev-2", SessionID: rec.ID, Harness: domain.ReviewerHarness("greptile"), + PRURL: pr2, TargetSHA: "sha1", Status: domain.ReviewRunRunning, Verdict: domain.VerdictNone, CreatedAt: now, }); err != nil { t.Fatalf("insert run: %v", err) @@ -113,14 +140,21 @@ func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { t.Fatalf("get run = %+v", gotRun) } - bySHA, ok, err := s.GetReviewRunBySessionAndSHA(ctx, rec.ID, "sha1") + bySHA, ok, err := s.GetReviewRunBySessionPRAndSHA(ctx, rec.ID, pr1, "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" { + if bySHA.ID != "run-1" || 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 { + bySHA, ok, err = s.GetReviewRunBySessionPRAndSHA(ctx, rec.ID, pr2, "sha1") + if err != nil || !ok { + t.Fatalf("by sha for second pr: ok=%v err=%v", ok, err) + } + if bySHA.ID != "run-2" { + t.Fatalf("same sha on second pr should resolve second run, got %+v", bySHA) + } + if _, ok, _ := s.GetReviewRunBySessionPRAndSHA(ctx, rec.ID, pr1, "other"); ok { t.Fatal("unexpected run for a different sha") } @@ -128,9 +162,16 @@ func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { if err != nil { t.Fatalf("list runs: %v", err) } - if len(runs) != 1 || runs[0].ID != "run-1" { + if len(runs) != 2 { t.Fatalf("list runs = %+v", runs) } + runs, err = s.ListReviewRunsBySessionAndPR(ctx, rec.ID, pr1) + if err != nil { + t.Fatalf("list runs by pr: %v", err) + } + if len(runs) != 1 || runs[0].ID != "run-1" { + t.Fatalf("list runs by pr = %+v", runs) + } if ok, err := s.UpdateReviewRunResult(ctx, "run-1", domain.ReviewRunComplete, domain.VerdictApproved, "again"); err != nil { t.Fatalf("second update: %v", err) @@ -142,10 +183,10 @@ func TestReviewUpsertReusesRowAndRunRoundTrip(t *testing.T) { func TestReviewGettersMissing(t *testing.T) { s := newTestStore(t) ctx := context.Background() - if _, ok, err := s.GetReviewBySession(ctx, "mer-1"); err != nil || ok { + if _, ok, err := s.GetReviewBySessionAndPR(ctx, "mer-1", "https://example/pr/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 { + if _, ok, err := s.GetReviewRunBySessionPRAndSHA(ctx, "mer-1", "https://example/pr/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 { diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 92a84d15..1a808079 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -473,6 +473,7 @@ export interface components { ListReviewsResponse: { reviewerHandleId: string; reviews: components["schemas"]["ReviewRun"][]; + targets: components["schemas"]["ReviewTargetResponse"][]; }; ListSessionPRsResponse: { prs: components["schemas"]["SessionPRFacts"][]; @@ -591,6 +592,11 @@ export interface components { review: components["schemas"]["ReviewRun"]; reviewerHandleId: string; }; + ReviewTargetResponse: { + prUrl: string; + reviewerHandleId: string; + reviews: components["schemas"]["ReviewRun"][]; + }; RoleOverride: { agent?: string; agentConfig?: components["schemas"]["AgentConfig"]; @@ -680,6 +686,10 @@ export interface components { /** @description Review verdict: approved or changes_requested. */ verdict: string; }; + TriggerReviewInput: { + /** @description Tracked PR URL to review. Required when the session owns multiple PRs. */ + prUrl?: string; + }; WorkspaceRepo: { name: string; relativePath: string; @@ -1817,7 +1827,10 @@ export interface operations { }; listReviews: { parameters: { - query?: never; + query?: { + /** @description Tracked PR URL to list review runs for. Omit to list all PR review targets for the session. */ + prUrl?: string; + }; header?: never; path: { /** @description Session identifier, e.g. project-1. */ @@ -1929,7 +1942,11 @@ export interface operations { }; cookie?: never; }; - requestBody?: never; + requestBody?: { + content: { + "application/json": components["schemas"]["TriggerReviewInput"]; + }; + }; responses: { /** @description OK */ 200: {