Skip to content

Commit e9a97f3

Browse files
feat: support multi-pr review runs
1 parent afe0817 commit e9a97f3

16 files changed

Lines changed: 699 additions & 152 deletions

File tree

backend/internal/httpd/apispec/openapi.yaml

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,13 +1243,13 @@ paths:
12431243
content:
12441244
application/json:
12451245
schema:
1246-
$ref: '#/components/schemas/ReviewRunResponse'
1246+
$ref: '#/components/schemas/TriggerReviewResponse'
12471247
description: OK
12481248
"201":
12491249
content:
12501250
application/json:
12511251
schema:
1252-
$ref: '#/components/schemas/ReviewRunResponse'
1252+
$ref: '#/components/schemas/TriggerReviewResponse'
12531253
description: Created
12541254
"404":
12551255
content:
@@ -1632,6 +1632,15 @@ components:
16321632
type: object
16331633
ListReviewsResponse:
16341634
properties:
1635+
items:
1636+
description: Deprecated alias for reviewItems.
1637+
items:
1638+
$ref: '#/components/schemas/PRReviewItem'
1639+
type: array
1640+
reviewItems:
1641+
items:
1642+
$ref: '#/components/schemas/PRReviewItem'
1643+
type: array
16351644
reviewerHandleId:
16361645
type: string
16371646
reviews:
@@ -1641,6 +1650,8 @@ components:
16411650
required:
16421651
- reviewerHandleId
16431652
- reviews
1653+
- reviewItems
1654+
- items
16441655
type: object
16451656
ListSessionPRsResponse:
16461657
properties:
@@ -1772,6 +1783,30 @@ components:
17721783
- id
17731784
- projectId
17741785
type: object
1786+
PRReviewItem:
1787+
properties:
1788+
latestRun:
1789+
$ref: '#/components/schemas/ReviewRun'
1790+
prNumber:
1791+
type: integer
1792+
prUrl:
1793+
type: string
1794+
status:
1795+
enum:
1796+
- needs_review
1797+
- running
1798+
- up_to_date
1799+
- changes_requested
1800+
- ineligible
1801+
type: string
1802+
targetSha:
1803+
type: string
1804+
required:
1805+
- prUrl
1806+
- prNumber
1807+
- targetSha
1808+
- status
1809+
type: object
17751810
Project:
17761811
properties:
17771812
agent:
@@ -2403,6 +2438,37 @@ components:
24032438
- body
24042439
- githubReviewId
24052440
type: object
2441+
TriggerReviewResponse:
2442+
properties:
2443+
createdReviews:
2444+
items:
2445+
$ref: '#/components/schemas/ReviewRun'
2446+
type: array
2447+
items:
2448+
description: Deprecated alias for reviewItems.
2449+
items:
2450+
$ref: '#/components/schemas/PRReviewItem'
2451+
type: array
2452+
review:
2453+
$ref: '#/components/schemas/ReviewRun'
2454+
reviewItems:
2455+
items:
2456+
$ref: '#/components/schemas/PRReviewItem'
2457+
type: array
2458+
reviewerHandleId:
2459+
type: string
2460+
reviews:
2461+
items:
2462+
$ref: '#/components/schemas/ReviewRun'
2463+
type: array
2464+
required:
2465+
- review
2466+
- reviewerHandleId
2467+
- reviews
2468+
- createdReviews
2469+
- reviewItems
2470+
- items
2471+
type: object
24062472
WorkspaceRepo:
24072473
properties:
24082474
name:

backend/internal/httpd/apispec/specgen/build.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,13 @@ var schemaNames = map[string]string{
181181
"ControllersResolveCommentsRequest": "ResolveCommentsRequest",
182182
"ControllersResolveCommentsResponse": "ResolveCommentsResponse",
183183
// httpd/controllers — review wire envelopes
184-
"ControllersListReviewsResponse": "ListReviewsResponse",
185-
"ControllersReviewRunResponse": "ReviewRunResponse",
186-
"ControllersSubmitReviewInput": "SubmitReviewInput",
184+
"ControllersListReviewsResponse": "ListReviewsResponse",
185+
"ControllersReviewRunResponse": "ReviewRunResponse",
186+
"ControllersTriggerReviewResponse": "TriggerReviewResponse",
187+
"ControllersSubmitReviewInput": "SubmitReviewInput",
187188
// domain review entities
188-
"DomainReviewRun": "ReviewRun",
189+
"DomainReviewRun": "ReviewRun",
190+
"ReviewPRReviewItem": "PRReviewItem",
189191
// service/project entities + DTOs
190192
"ProjectProject": "Project",
191193
"ProjectSummary": "ProjectSummary",
@@ -345,8 +347,8 @@ func reviewOperations() []operation {
345347
summary: "Trigger a code review of a worker's PR",
346348
pathParams: []any{controllers.SessionIDParam{}},
347349
resps: []respUnit{
348-
{http.StatusOK, controllers.ReviewRunResponse{}},
349-
{http.StatusCreated, controllers.ReviewRunResponse{}},
350+
{http.StatusOK, controllers.TriggerReviewResponse{}},
351+
{http.StatusCreated, controllers.TriggerReviewResponse{}},
350352
{http.StatusUnprocessableEntity, envelope.APIError{}},
351353
{http.StatusNotFound, envelope.APIError{}},
352354
{http.StatusNotImplemented, envelope.APIError{}},

backend/internal/httpd/controllers/reviews.go

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,39 @@ import (
1010
"github.com/aoagents/agent-orchestrator/backend/internal/domain"
1111
"github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec"
1212
"github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope"
13+
reviewcore "github.com/aoagents/agent-orchestrator/backend/internal/review"
1314
reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review"
1415
)
1516

1617
// ListReviewsResponse is the body of GET /api/v1/sessions/{sessionId}/reviews.
1718
// reviewerHandleId is the live reviewer pane's runtime handle, for the UI to
1819
// attach its terminal over /mux (empty when no reviewer has run).
1920
type ListReviewsResponse struct {
20-
ReviewerHandleID string `json:"reviewerHandleId"`
21-
Reviews []domain.ReviewRun `json:"reviews"`
21+
ReviewerHandleID string `json:"reviewerHandleId"`
22+
Reviews []domain.ReviewRun `json:"reviews"`
23+
ReviewItems []reviewcore.PRReviewItem `json:"reviewItems"`
24+
Items []reviewcore.PRReviewItem `json:"items" description:"Deprecated alias for reviewItems."`
2225
}
2326

24-
// ReviewRunResponse is the body of trigger (200/201) and submit (200). It
25-
// carries the run plus the reviewer pane handle so the UI can attach a terminal.
27+
// ReviewRunResponse is the body of submit (200). It carries the run plus the
28+
// reviewer pane handle so the UI can attach a terminal.
2629
type ReviewRunResponse struct {
2730
Review domain.ReviewRun `json:"review"`
2831
ReviewerHandleID string `json:"reviewerHandleId"`
2932
}
3033

34+
// TriggerReviewResponse is the body of trigger (200/201). review and
35+
// reviewerHandleId are legacy compatibility fields; reviews carries the
36+
// PR-scoped batch of newly created runs and items carries per-PR review state.
37+
type TriggerReviewResponse struct {
38+
Review domain.ReviewRun `json:"review"`
39+
ReviewerHandleID string `json:"reviewerHandleId"`
40+
Reviews []domain.ReviewRun `json:"reviews"`
41+
CreatedReviews []domain.ReviewRun `json:"createdReviews"`
42+
ReviewItems []reviewcore.PRReviewItem `json:"reviewItems"`
43+
Items []reviewcore.PRReviewItem `json:"items" description:"Deprecated alias for reviewItems."`
44+
}
45+
3146
// SubmitReviewInput is the body of POST /api/v1/sessions/{sessionId}/reviews/submit.
3247
type SubmitReviewInput struct {
3348
RunID string `json:"runId" description:"Review run id being completed."`
@@ -62,7 +77,11 @@ func (c *ReviewsController) list(w http.ResponseWriter, r *http.Request) {
6277
if runs == nil {
6378
runs = []domain.ReviewRun{}
6479
}
65-
envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{ReviewerHandleID: res.ReviewerHandleID, Reviews: runs})
80+
items := res.Items
81+
if items == nil {
82+
items = []reviewcore.PRReviewItem{}
83+
}
84+
envelope.WriteJSON(w, http.StatusOK, ListReviewsResponse{ReviewerHandleID: res.ReviewerHandleID, Reviews: runs, ReviewItems: items, Items: items})
6685
}
6786

6887
func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) {
@@ -81,7 +100,22 @@ func (c *ReviewsController) trigger(w http.ResponseWriter, r *http.Request) {
81100
if res.Created {
82101
status = http.StatusCreated
83102
}
84-
envelope.WriteJSON(w, status, ReviewRunResponse{Review: res.Run, ReviewerHandleID: res.ReviewerHandleID})
103+
reviews := res.CreatedRuns
104+
if reviews == nil {
105+
reviews = []domain.ReviewRun{}
106+
}
107+
items := res.Items
108+
if items == nil {
109+
items = []reviewcore.PRReviewItem{}
110+
}
111+
envelope.WriteJSON(w, status, TriggerReviewResponse{
112+
Review: res.Run,
113+
ReviewerHandleID: res.ReviewerHandleID,
114+
Reviews: reviews,
115+
CreatedReviews: reviews,
116+
ReviewItems: items,
117+
Items: items,
118+
})
85119
}
86120

87121
func (c *ReviewsController) submit(w http.ResponseWriter, r *http.Request) {

backend/internal/httpd/controllers/reviews_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ import (
2020

2121
type fakeReviewService struct {
2222
triggerErr error
23+
trigger reviewcore.TriggerResult
24+
list reviewcore.SessionReviews
2325
}
2426

2527
func (f *fakeReviewService) Trigger(context.Context, domain.SessionID) (reviewcore.TriggerResult, error) {
2628
if f.triggerErr != nil {
2729
return reviewcore.TriggerResult{}, f.triggerErr
2830
}
31+
if f.trigger.ReviewerHandleID != "" || f.trigger.Run.ID != "" || f.trigger.Items != nil || f.trigger.CreatedRuns != nil {
32+
return f.trigger, nil
33+
}
2934
return reviewcore.TriggerResult{Run: domain.ReviewRun{ID: "run-1"}, Created: true}, nil
3035
}
3136

@@ -34,7 +39,7 @@ func (f *fakeReviewService) Submit(context.Context, domain.SessionID, string, do
3439
}
3540

3641
func (f *fakeReviewService) List(context.Context, domain.SessionID) (reviewcore.SessionReviews, error) {
37-
return reviewcore.SessionReviews{}, nil
42+
return f.list, nil
3843
}
3944

4045
func newReviewTestServer(t *testing.T, svc reviewsvc.Manager) *httptest.Server {
@@ -59,3 +64,46 @@ func TestReviewsTrigger_MissingReviewerBinaryReturns422WithCause(t *testing.T) {
5964
t.Fatalf("message = %q, want reviewer binary cause", got.Message)
6065
}
6166
}
67+
68+
func TestReviewsListIncludesItems(t *testing.T) {
69+
srv := newReviewTestServer(t, &fakeReviewService{list: reviewcore.SessionReviews{
70+
ReviewerHandleID: "review-mer-1",
71+
Runs: []domain.ReviewRun{{ID: "run-1", PRURL: "https://github.com/o/r/pull/1", TargetSHA: "sha1"}},
72+
Items: []reviewcore.PRReviewItem{{PRURL: "https://github.com/o/r/pull/1", PRNumber: 1, TargetSHA: "sha1", Status: reviewcore.ItemUpToDate}},
73+
}})
74+
75+
body, status, headers := doRequest(t, srv, "GET", "/api/v1/sessions/mer-1/reviews", "")
76+
assertJSON(t, headers)
77+
if status != http.StatusOK {
78+
t.Fatalf("status = %d body=%s", status, body)
79+
}
80+
if !strings.Contains(string(body), `"reviewItems"`) || !strings.Contains(string(body), `"items"`) || !strings.Contains(string(body), `"up_to_date"`) || !strings.Contains(string(body), `"reviewerHandleId":"review-mer-1"`) {
81+
t.Fatalf("body missing review items/handle: %s", body)
82+
}
83+
}
84+
85+
func TestReviewsTriggerIncludesBatchFields(t *testing.T) {
86+
run1 := domain.ReviewRun{ID: "run-1", PRURL: "https://github.com/o/r/pull/1", TargetSHA: "sha1"}
87+
run2 := domain.ReviewRun{ID: "run-2", PRURL: "https://github.com/o/r/pull/2", TargetSHA: "sha2"}
88+
srv := newReviewTestServer(t, &fakeReviewService{trigger: reviewcore.TriggerResult{
89+
Run: run1,
90+
ReviewerHandleID: "review-mer-1",
91+
Created: true,
92+
CreatedRuns: []domain.ReviewRun{run1, run2},
93+
Items: []reviewcore.PRReviewItem{
94+
{PRURL: run1.PRURL, PRNumber: 1, TargetSHA: run1.TargetSHA, Status: reviewcore.ItemRunning, LatestRun: &run1},
95+
{PRURL: run2.PRURL, PRNumber: 2, TargetSHA: run2.TargetSHA, Status: reviewcore.ItemRunning, LatestRun: &run2},
96+
},
97+
}})
98+
99+
body, status, headers := doRequest(t, srv, "POST", "/api/v1/sessions/mer-1/reviews/trigger", "")
100+
assertJSON(t, headers)
101+
if status != http.StatusCreated {
102+
t.Fatalf("status = %d body=%s", status, body)
103+
}
104+
for _, want := range []string{`"reviews"`, `"createdReviews"`, `"reviewItems"`, `"items"`, `"run-1"`, `"run-2"`, `"reviewerHandleId":"review-mer-1"`} {
105+
if !strings.Contains(string(body), want) {
106+
t.Fatalf("body missing %s: %s", want, body)
107+
}
108+
}
109+
}

backend/internal/lifecycle/manager_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,10 @@ func TestApplyReviewResultSendsAndDedupsThroughPRSignature(t *testing.T) {
543543
t.Fatalf("outcome/messages = %q/%v, want sent once", outcome, msg.msgs)
544544
}
545545
got := msg.msgs[0]
546-
if !strings.Contains(got, "[AO reviewer]") || !strings.Contains(got, "fix the bug") || !strings.Contains(got, "98[2J765") {
547-
t.Fatalf("AO review nudge missing label/body/review id: %q", got)
546+
for _, want := range []string{"[AO reviewer]", "PR: " + result.PRURL, "Verdict: changes_requested", "Review body:\nfix the bug", "GitHub review: 98[2J765"} {
547+
if !strings.Contains(got, want) {
548+
t.Fatalf("AO review nudge missing %q: %q", want, got)
549+
}
548550
}
549551
if strings.Contains(got, "\x1b") {
550552
t.Fatalf("AO review nudge should sanitize control bytes: %q", got)

backend/internal/lifecycle/reactions.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,14 @@ func (m *Manager) ApplyReviewResult(ctx context.Context, workerID domain.Session
154154
if m.messenger == nil {
155155
return ReviewDeliveryNoop, nil
156156
}
157-
msg := "[AO reviewer] AO's internal code reviewer requested changes on your PR. Review the feedback below and address it."
157+
msg := fmt.Sprintf("[AO reviewer] AO's internal code reviewer submitted a review.\n\nPR: %s\nVerdict: %s", domain.SanitizeControlChars(r.PRURL), domain.SanitizeControlChars(string(r.Verdict)))
158158
if r.GithubReviewID != "" {
159159
safeReviewID := domain.SanitizeControlChars(r.GithubReviewID)
160-
msg += fmt.Sprintf(" This feedback is GitHub review %s. Once you have addressed it, reply on that review referencing id %s with how you addressed it, then resolve the review comment threads you addressed.", safeReviewID, safeReviewID)
160+
msg += fmt.Sprintf("\nGitHub review: %s", safeReviewID)
161+
msg += fmt.Sprintf("\n\nOnce you have addressed it, reply on GitHub review %s with how you addressed it, then resolve the review comment threads you addressed.", safeReviewID)
161162
}
162163
if r.Body != "" {
163-
msg += "\n\n" + domain.SanitizeControlChars(r.Body)
164+
msg += "\n\nReview body:\n" + domain.SanitizeControlChars(r.Body)
164165
}
165166
key := "review:" + r.PRURL + ":ao:" + r.RunID
166167
sig := strings.Join([]string{r.TargetSHA, r.RunID, r.GithubReviewID, r.Body}, "\x00")

0 commit comments

Comments
 (0)