bkinnightskytw
/
agent-orchestrator
mirror of https://github.com/ComposioHQ/agent-orchestrator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: deep-link PR attention actions

This commit is contained in:
whoisasx 2026-06-26 17:38:26 +05:30
parent afe0817765
commit 59eb4c2152
28 changed files with 835 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
backend/internal/adapters/scm/github/observer_provider.go
View File

@ -33,6 +33,8 @@ const (
githubReviewCommentLimitPerThread = 5
// githubReviewThreadMaxPages bounds the explicit older-thread fallback.
githubReviewThreadMaxPages = 2
// githubReviewSummaryLimit bounds submitted decisive reviews used for summary links.
githubReviewSummaryLimit = 20
)
// ParseRepository normalizes a GitHub remote/origin URL into a provider-neutral
@ -156,12 +158,12 @@ func (p *Provider) FetchFailedCheckLogTail(ctx context.Context, repo ports.SCMRe
// FetchReviewThreads fetches review threads separately from the fast PR/CI path.
func (p *Provider) FetchReviewThreads(ctx context.Context, ref ports.SCMPRRef) (ports.SCMReviewObservation, error) {
latest, decision, pi, err := p.fetchReviewThreadPage(ctx, ref, "")
latest, reviews, decision, pi, err := p.fetchReviewThreadPage(ctx, ref, "", true)
if err != nil {
return ports.SCMReviewObservation{}, err
}
if !boolv(pi["hasPreviousPage"]) {
return ports.SCMReviewObservation{Decision: decision, Threads: latest}, nil
return ports.SCMReviewObservation{Decision: decision, Reviews: reviews, Threads: latest}, nil
}
out := latest
startCursor := str(pi["startCursor"])
@ -175,7 +177,7 @@ func (p *Provider) FetchReviewThreads(ctx context.Context, ref ports.SCMPRRef) (
p.logger.Warn("github scm: review thread page is partial but missing start cursor",
"repo", repoFullName(ref.Repo), "pr", ref.Number)
} else {
older, _, olderPI, err := p.fetchReviewThreadPage(ctx, ref, startCursor)
older, _, _, olderPI, err := p.fetchReviewThreadPage(ctx, ref, startCursor, false)
if err != nil {
return ports.SCMReviewObservation{}, err
}
@ -190,7 +192,7 @@ func (p *Provider) FetchReviewThreads(ctx context.Context, ref ports.SCMPRRef) (
}
}
}
return ports.SCMReviewObservation{Decision: decision, Threads: out, Partial: true}, nil
return ports.SCMReviewObservation{Decision: decision, Reviews: reviews, Threads: out, Partial: true}, nil
}
type restListPull struct {
@ -518,38 +520,76 @@ func mergeabilityObservation(providerMergeable, providerMergeState, ci, review s
return out
}
func (p *Provider) fetchReviewThreadPage(ctx context.Context, ref ports.SCMPRRef, beforeCursor string) ([]ports.SCMReviewThreadObservation, string, map[string]any, error) {
query := buildReviewThreadsQuery(ref, beforeCursor)
func (p *Provider) fetchReviewThreadPage(ctx context.Context, ref ports.SCMPRRef, beforeCursor string, includeReviews bool) ([]ports.SCMReviewThreadObservation, []ports.SCMReviewSummaryObservation, string, map[string]any, error) {
query := buildReviewThreadsQuery(ref, beforeCursor, includeReviews)
data, err := p.client.doGraphQL(ctx, query, nil)
if err != nil {
return nil, "", nil, err
return nil, nil, "", nil, err
}
repoData, _ := data["repo"].(map[string]any)
pr, _ := repoData["pullRequest"].(map[string]any)
if pr == nil {
return nil, "", nil, fmt.Errorf("%w: pull request not found in review response", ErrNotFound)
return nil, nil, "", nil, fmt.Errorf("%w: pull request not found in review response", ErrNotFound)
}
decision := string(reviewDecisionFromGraphQL(pr))
reviewSummaries, _ := pr["reviewSummaries"].(map[string]any)
reviews := make([]ports.SCMReviewSummaryObservation, 0, len(nodes(reviewSummaries["nodes"])))
for _, review := range nodes(reviewSummaries["nodes"]) {
summary := scmReviewSummaryFromGraphQL(review)
if summary.ID == "" && summary.URL == "" {
continue
}
reviews = append(reviews, summary)
}
threads, _ := pr["reviewThreads"].(map[string]any)
out := make([]ports.SCMReviewThreadObservation, 0, len(nodes(threads["nodes"])))
for _, th := range nodes(threads["nodes"]) {
out = append(out, scmThreadFromGraphQL(th))
}
pi, _ := threads["pageInfo"].(map[string]any)
return out, decision, pi, nil
return out, reviews, decision, pi, nil
}
func buildReviewThreadsQuery(ref ports.SCMPRRef, beforeCursor string) string {
func buildReviewThreadsQuery(ref ports.SCMPRRef, beforeCursor string, includeReviews bool) string {
before := "null"
if beforeCursor != "" {
before = graphQLString(beforeCursor)
}
reviewSelection := ""
if includeReviews {
reviewSelection = fmt.Sprintf(" reviewSummaries: reviews(last:%d, states:[APPROVED,CHANGES_REQUESTED]){ nodes{ id state url submittedAt author{ login __typename } } }", githubReviewSummaryLimit)
}
return fmt.Sprintf(`query{
repo: repository(owner:%s,name:%s){ pullRequest(number:%d){ reviewDecision reviewThreads(last:%d, before:%s){ nodes{
repo: repository(owner:%s,name:%s){ pullRequest(number:%d){ reviewDecision%s reviewThreads(last:%d, before:%s){ nodes{
id isResolved path line
comments(first:%d){ nodes{ id body url author{ login __typename } } }
} pageInfo{ hasPreviousPage startCursor } } } }
}`, graphQLString(ref.Repo.Owner), graphQLString(ref.Repo.Name), ref.Number, githubReviewThreadPageSize, before, githubReviewCommentLimitPerThread)
}`, graphQLString(ref.Repo.Owner), graphQLString(ref.Repo.Name), ref.Number, reviewSelection, githubReviewThreadPageSize, before, githubReviewCommentLimitPerThread)
}
func scmReviewSummaryFromGraphQL(review map[string]any) ports.SCMReviewSummaryObservation {
author, _ := review["author"].(map[string]any)
return ports.SCMReviewSummaryObservation{
ID: str(review["id"]),
Author: str(author["login"]),
State: string(reviewStateFromGraphQL(review["state"])),
URL: str(review["url"]),
IsBot: isBotAuthor(author),
SubmittedAt: parseGitHubTime(str(review["submittedAt"])),
}
}
func reviewStateFromGraphQL(state any) domain.ReviewDecision {
switch strings.ToUpper(strings.TrimSpace(str(state))) {
case "APPROVED":
return domain.ReviewApproved
case "CHANGES_REQUESTED":
return domain.ReviewChangesRequest
case "REVIEW_REQUIRED":
return domain.ReviewRequired
default:
return domain.ReviewNone
}
}
func scmThreadFromGraphQL(th map[string]any) ports.SCMReviewThreadObservation {

20
backend/internal/adapters/scm/github/provider_test.go
View File

@ -1369,6 +1369,9 @@ func TestFetchReviewThreadsUsesLatestWindowWithoutFallbackWhenOldestResolved(t *
if !strings.Contains(string(body), "reviewThreads(last:50, before:null)") {
t.Fatalf("review query should fetch latest 50, body=%s", body)
}
if !strings.Contains(string(body), "reviews(last:20, states:[APPROVED,CHANGES_REQUESTED])") {
t.Fatalf("review query should fetch decisive review summaries, body=%s", body)
}
if !strings.Contains(string(body), "comments(first:5)") {
t.Fatalf("review query should cap comments per thread, body=%s", body)
}
@ -1376,8 +1379,17 @@ func TestFetchReviewThreadsUsesLatestWindowWithoutFallbackWhenOldestResolved(t *
_ = json.NewEncoder(w).Encode(map[string]any{
"data": map[string]any{"repo": map[string]any{"pullRequest": map[string]any{
"reviewDecision": "CHANGES_REQUESTED",
"reviewSummaries": map[string]any{"nodes": []any{map[string]any{
"id": "review-1",
"state": "CHANGES_REQUESTED",
"url": "https://github.com/o/r/pull/1#pullrequestreview-1",
"submittedAt": "2026-06-15T00:00:00Z",
"author": map[string]any{"login": "alice", "__typename": "User"},
}}},
"reviewThreads": map[string]any{
"nodes": []any{map[string]any{"id": "latest-resolved", "path": "main.go", "line": 1, "isResolved": true, "comments": map[string]any{"nodes": []any{}}}},
"nodes": []any{map[string]any{"id": "latest-resolved", "path": "main.go", "line": 1, "isResolved": true, "comments": map[string]any{"nodes": []any{map[string]any{
"id": "comment-1", "body": "fix", "url": "https://github.com/o/r/pull/1#discussion_r1", "author": map[string]any{"login": "alice", "__typename": "User"},
}}}}},
"pageInfo": map[string]any{"hasPreviousPage": true, "startCursor": "latest-start"},
},
}}},
@ -1397,6 +1409,12 @@ func TestFetchReviewThreadsUsesLatestWindowWithoutFallbackWhenOldestResolved(t *
if len(review.Threads) != 1 || review.Threads[0].ID != "latest-resolved" {
t.Fatalf("threads = %#v", review.Threads)
}
if len(review.Reviews) != 1 || review.Reviews[0].Author != "alice" || review.Reviews[0].URL != "https://github.com/o/r/pull/1#pullrequestreview-1" {
t.Fatalf("reviews = %#v", review.Reviews)
}
if len(review.Threads[0].Comments) != 1 || review.Threads[0].Comments[0].URL != "https://github.com/o/r/pull/1#discussion_r1" {
t.Fatalf("thread comments = %#v", review.Threads[0].Comments)
}
}
func TestFetchReviewThreadsFetchesOneOlderPageWhenOldestUnresolved(t *testing.T) {

10
backend/internal/domain/pr.go
View File

@ -107,6 +107,16 @@ type PullRequestReviewThread struct {
UpdatedAt time.Time
}
// PullRequestReview is one submitted provider review for a pull request.
type PullRequestReview struct {
ID string
Author string
State ReviewDecision
URL string
IsBot bool
SubmittedAt time.Time
}
// CIState is the aggregate CI status of a PR.
type CIState string

4
backend/internal/httpd/apispec/openapi.yaml
View File

@ -2253,10 +2253,14 @@ components:
properties:
count:
type: integer
isBot:
type: boolean
links:
items:
$ref: '#/components/schemas/SessionPRReviewCommentLink'
type: array
reviewUrl:
type: string
reviewerId:
type: string
required:

4
backend/internal/httpd/controllers/dto.go
View File

@ -294,6 +294,8 @@ type SessionPRUnresolvedReviewer struct {
ReviewerID string `json:"reviewerId"`
Count int `json:"count"`
Links []SessionPRReviewCommentLink `json:"links"`
ReviewURL string `json:"reviewUrl,omitempty"`
IsBot bool `json:"isBot,omitempty"`
}
// SessionPRReviewCommentLink points to one unresolved review comment.
@ -365,7 +367,7 @@ func newSessionPRReviewSummary(in sessionsvc.PRReviewSummary) SessionPRReviewSum
for _, link := range reviewer.Links {
links = append(links, SessionPRReviewCommentLink{URL: link.URL, File: link.File, Line: link.Line})
}
reviewers = append(reviewers, SessionPRUnresolvedReviewer{ReviewerID: reviewer.ReviewerID, Count: reviewer.Count, Links: links})
reviewers = append(reviewers, SessionPRUnresolvedReviewer{ReviewerID: reviewer.ReviewerID, Count: reviewer.Count, Links: links, ReviewURL: reviewer.ReviewURL, IsBot: reviewer.IsBot})
}
return SessionPRReviewSummary{Decision: in.Decision, HasUnresolvedHumanComments: in.HasUnresolvedHumanComments, UnresolvedBy: reviewers}
}

4
backend/internal/httpd/controllers/sessions_test.go
View File

@ -192,6 +192,7 @@ func (f *fakeSessionService) ListPRSummaries(_ context.Context, id domain.Sessio
UnresolvedBy: []sessionsvc.PRUnresolvedReviewer{{
ReviewerID: "reviewer-a",
Count: 1,
ReviewURL: "https://github.com/aoagents/agent-orchestrator/pull/142#pullrequestreview-1",
Links: []sessionsvc.PRReviewCommentLink{{URL: "https://github.com/aoagents/agent-orchestrator/pull/142#discussion_r1", File: "main.go", Line: 12}},
}},
},
@ -844,6 +845,7 @@ func TestSessionsAPI_PRRoutes(t *testing.T) {
UnresolvedBy []struct {
ReviewerID string `json:"reviewerId"`
Count int `json:"count"`
ReviewURL string `json:"reviewUrl"`
Links []struct {
URL string `json:"url"`
File string `json:"file"`
@ -869,7 +871,7 @@ func TestSessionsAPI_PRRoutes(t *testing.T) {
if checks := listed.PRs[0].CI.FailingChecks; len(checks) != 1 || checks[0].Name != "unit" || checks[0].LogTail != "" {
t.Fatalf("failing checks = %#v", checks)
}
if reviewers := listed.PRs[0].Review.UnresolvedBy; len(reviewers) != 1 || reviewers[0].ReviewerID != "reviewer-a" || reviewers[0].Links[0].Body != "" {
if reviewers := listed.PRs[0].Review.UnresolvedBy; len(reviewers) != 1 || reviewers[0].ReviewerID != "reviewer-a" || reviewers[0].ReviewURL == "" || reviewers[0].Links[0].Body != "" {
t.Fatalf("reviewers = %#v", reviewers)
}
if merge := listed.PRs[0].Mergeability; merge.State != "conflicting" || len(merge.ConflictFiles) != 0 || merge.PRURL == "" {

33
backend/internal/observe/scm/observer.go
View File

@ -53,7 +53,7 @@ type Store interface {
UpsertProject(ctx context.Context, row domain.ProjectRecord) error
ListPRsBySession(ctx context.Context, sessionID domain.SessionID) ([]domain.PullRequest, error)
ListChecks(ctx context.Context, prURL string) ([]domain.PullRequestCheck, error)
WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error
WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error
}
// Lifecycle is the provider-neutral lifecycle notification sink.
@ -344,8 +344,8 @@ func (o *Observer) Poll(ctx context.Context) error {
prRefreshOK[key] = true
continue
}
finalPR, finalChecks, finalThreads, finalComments := domainFromObservation(subj.session.ID, prepared, local, opts, now)
pr, checks, threads, comments := finalPR, finalChecks, finalThreads, finalComments
finalPR, finalChecks, finalReviews, finalThreads, finalComments := domainFromObservation(subj.session.ID, prepared, local, opts, now)
pr, checks, reviews, threads, comments := finalPR, finalChecks, finalReviews, finalThreads, finalComments
// Lifecycle is allowed to run only after the observed facts are durable,
// but semantic hashes are the observer's acknowledgement cursor. Keep
// changed hashes at their local values until lifecycle succeeds; if the
@ -362,9 +362,9 @@ func (o *Observer) Poll(ctx context.Context) error {
if prepared.Changed.Review {
pendingOpts.preserveLocalReviewHash = true
}
pr, checks, threads, comments = domainFromObservation(subj.session.ID, prepared, local, pendingOpts, now)
pr, checks, reviews, threads, comments = domainFromObservation(subj.session.ID, prepared, local, pendingOpts, now)
}
if err := o.store.WriteSCMObservation(ctx, pr, checks, threads, comments, reviewMode); err != nil {
if err := o.store.WriteSCMObservation(ctx, pr, checks, reviews, threads, comments, reviewMode); err != nil {
o.logger.Error("scm observer: DB write failed", "session", subj.session.ID, "pr", pr.URL, "err", err)
markRepoRefreshFailed(subj.repo)
continue
@ -375,7 +375,7 @@ func (o *Observer) Poll(ctx context.Context) error {
markRepoRefreshFailed(subj.repo)
continue
}
if err := o.store.WriteSCMObservation(ctx, finalPR, finalChecks, nil, nil, ports.ReviewWritePreserve); err != nil {
if err := o.store.WriteSCMObservation(ctx, finalPR, finalChecks, nil, nil, nil, ports.ReviewWritePreserve); err != nil {
o.logger.Error("scm observer: DB lifecycle acknowledgement failed", "session", subj.session.ID, "pr", finalPR.URL, "err", err)
markRepoRefreshFailed(subj.repo)
continue
@ -586,7 +586,7 @@ func (o *Observer) discoverNewPRs(ctx context.Context, sessionRepos []sessionRep
// reads every PR of the session from the store. Without this write, an
// open sibling/child discovered in the same poll would not yet be
// durable, and the session could terminate while that PR is still open.
if err := o.store.WriteSCMObservation(ctx, known, nil, nil, nil, ports.ReviewWritePreserve); err != nil {
if err := o.store.WriteSCMObservation(ctx, known, nil, nil, nil, nil, ports.ReviewWritePreserve); err != nil {
o.logger.Error("scm observer: persist discovered PR failed", "session", sr.session.ID, "pr", known.URL, "err", err)
if markRepoFailed != nil {
markRepoFailed(repo)
@ -793,6 +793,7 @@ func (o *Observer) refreshReviews(ctx context.Context, subjects map[string]*subj
if review.Decision != "" {
obs.Review.Decision = review.Decision
}
obs.Review.Reviews = review.Reviews
obs.Review.Threads = review.Threads
obs.Review.Partial = review.Partial
obs.ObservedAt = now
@ -850,7 +851,7 @@ func (o *Observer) prepareForPersistence(obs ports.SCMObservation, local domain.
return obs
}
func domainFromObservation(sessionID domain.SessionID, obs ports.SCMObservation, local domain.PullRequest, opts persistenceOptions, now time.Time) (domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReviewThread, []domain.PullRequestComment) {
func domainFromObservation(sessionID domain.SessionID, obs ports.SCMObservation, local domain.PullRequest, opts persistenceOptions, now time.Time) (domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReview, []domain.PullRequestReviewThread, []domain.PullRequestComment) {
metadataHash := metadataSemanticHash(obs)
if opts.preserveLocalMetadataHash {
metadataHash = local.MetadataHash
@ -924,6 +925,17 @@ func domainFromObservation(sessionID domain.SessionID, obs ports.SCMObservation,
for _, ch := range obs.CI.Checks {
checks = append(checks, domain.PullRequestCheck{Name: ch.Name, CommitHash: obs.CI.HeadSHA, Status: domain.PRCheckStatus(ch.Status), Conclusion: ch.Conclusion, URL: ch.URL, Details: ch.ProviderID, LogTail: ch.LogTail, CreatedAt: now})
}
reviews := make([]domain.PullRequestReview, 0, len(obs.Review.Reviews))
for _, review := range obs.Review.Reviews {
reviews = append(reviews, domain.PullRequestReview{
ID: review.ID,
Author: review.Author,
State: domain.ReviewDecision(firstNonEmpty(review.State, string(domain.ReviewNone))),
URL: review.URL,
IsBot: review.IsBot,
SubmittedAt: firstTime(review.SubmittedAt, now),
})
}
threads := make([]domain.PullRequestReviewThread, 0, len(obs.Review.Threads))
commentCount := 0
for _, th := range obs.Review.Threads {
@ -936,7 +948,7 @@ func domainFromObservation(sessionID domain.SessionID, obs ports.SCMObservation,
comments = append(comments, domain.PullRequestComment{ThreadID: th.ID, ID: c.ID, Author: c.Author, File: th.Path, Line: th.Line, Body: c.Body, URL: c.URL, Resolved: th.Resolved, IsBot: c.IsBot || th.IsBot, CreatedAt: now})
}
}
return pr, checks, threads, comments
return pr, checks, reviews, threads, comments
}
func observationFromLocal(repo ports.SCMRepo, pr domain.PullRequest, checks []domain.PullRequestCheck) ports.SCMObservation {
@ -1119,10 +1131,11 @@ func ciSemanticHash(ci ports.SCMCIObservation) string {
func reviewSemanticHash(review ports.SCMReviewObservation) string {
type reviewHashPayload struct {
Decision string
Reviews []ports.SCMReviewSummaryObservation
Threads []ports.SCMReviewThreadObservation
Partial bool `json:",omitempty"`
}
return stableHash(reviewHashPayload{Decision: review.Decision, Threads: review.Threads, Partial: review.Partial})
return stableHash(reviewHashPayload{Decision: review.Decision, Reviews: review.Reviews, Threads: review.Threads, Partial: review.Partial})
}
func threadSemanticHash(th ports.SCMReviewThreadObservation) string {

16
backend/internal/observe/scm/observer_test.go
View File

@ -41,6 +41,7 @@ type fakeStore struct {
type fakeWrite struct {
pr domain.PullRequest
checks []domain.PullRequestCheck
reviews []domain.PullRequestReview
comments []domain.PullRequestComment
reviewMode ports.ReviewWriteMode
}
@ -94,13 +95,13 @@ func (s *fakeStore) ListChecks(_ context.Context, prURL string) ([]domain.PullRe
return append([]domain.PullRequestCheck(nil), s.checks[prURL]...), nil
}
func (s *fakeStore) WriteSCMObservation(_ context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error {
func (s *fakeStore) WriteSCMObservation(_ context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error {
s.mu.Lock()
defer s.mu.Unlock()
if s.writeErr != nil {
return s.writeErr
}
s.writes = append(s.writes, fakeWrite{pr: pr, checks: append([]domain.PullRequestCheck(nil), checks...), comments: append([]domain.PullRequestComment(nil), comments...), reviewMode: reviewMode})
s.writes = append(s.writes, fakeWrite{pr: pr, checks: append([]domain.PullRequestCheck(nil), checks...), reviews: append([]domain.PullRequestReview(nil), reviews...), comments: append([]domain.PullRequestComment(nil), comments...), reviewMode: reviewMode})
return nil
}
@ -229,7 +230,7 @@ func testObs(num int) ports.SCMObservation {
func knownPR(num int) domain.PullRequest {
obs := testObs(num)
pr, _, _, _ := domainFromObservation("p-1", obs, domain.PullRequest{}, persistenceOptions{}, time.Unix(1, 0).UTC())
pr, _, _, _, _ := domainFromObservation("p-1", obs, domain.PullRequest{}, persistenceOptions{}, time.Unix(1, 0).UTC())
return pr
}
@ -731,7 +732,11 @@ func TestPoll_ReviewHashDrivesPersistenceAndLifecycle(t *testing.T) {
local.ReviewHash = "old"
local.Review = domain.ReviewChangesRequest
store.prs["p-1"] = []domain.PullRequest{local}
review := ports.SCMReviewObservation{Decision: string(domain.ReviewChangesRequest), Threads: []ports.SCMReviewThreadObservation{{ID: "t1", Path: "f.go", Line: 2, Comments: []ports.SCMReviewCommentObservation{{ID: "c1", Author: "ann", Body: "fix this"}}}}}
review := ports.SCMReviewObservation{
Decision: string(domain.ReviewChangesRequest),
Reviews: []ports.SCMReviewSummaryObservation{{ID: "review-1", Author: "ann", State: string(domain.ReviewChangesRequest), URL: "https://github.com/o/r/pull/1#pullrequestreview-1", SubmittedAt: time.Unix(199, 0).UTC()}},
Threads: []ports.SCMReviewThreadObservation{{ID: "t1", Path: "f.go", Line: 2, Comments: []ports.SCMReviewCommentObservation{{ID: "c1", Author: "ann", Body: "fix this"}}}},
}
provider := &fakeProvider{repoGuards: map[string]ports.SCMGuardResult{prKey(testRepo, 0): {ETag: "repo", NotModified: true}}, observations: map[string]ports.SCMObservation{}, reviews: map[string]ports.SCMReviewObservation{prKey(testRepo, 1): review}}
lc := &fakeLifecycle{}
obs := newTestObserver(store, provider, lc, time.Unix(200, 0).UTC())
@ -742,6 +747,9 @@ func TestPoll_ReviewHashDrivesPersistenceAndLifecycle(t *testing.T) {
if len(store.writes) == 0 || len(store.writes[0].comments) != 1 {
t.Fatalf("review change not persisted: %#v", store.writes)
}
if len(store.writes[0].reviews) != 1 || store.writes[0].reviews[0].URL != "https://github.com/o/r/pull/1#pullrequestreview-1" {
t.Fatalf("review summaries not persisted: %#v", store.writes[0].reviews)
}
if len(store.writes) != 2 {
t.Fatalf("review change with lifecycle should write held-back facts then acknowledgement, got %d writes", len(store.writes))
}

4
backend/internal/ports/outbound.go
View File

@ -37,13 +37,13 @@ const (
// whether review facts are preserved, replaced with a complete snapshot, or
// merged as a bounded partial window.
type SCMWriter interface {
WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ReviewWriteMode) error
WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ReviewWriteMode) error
}
// PRClaimer atomically moves (or creates) a PR row for a target session and
// persists the live SCM facts observed for that PR in the same transaction.
type PRClaimer interface {
ClaimPR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ReviewWriteMode, allowActiveTakeover bool) (ClaimOutcome, error)
ClaimPR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ReviewWriteMode, allowActiveTakeover bool) (ClaimOutcome, error)
}
// ErrPRClaimedByActiveSession is returned by PRClaimer.ClaimPR when takeover is

18
backend/internal/ports/scm_observations.go
View File

@ -184,6 +184,8 @@ type SCMCheckObservation struct {
type SCMReviewObservation struct {
// Decision is AO's normalized review decision.
Decision string
// Reviews contains submitted review summaries fetched on the slower review cadence.
Reviews []SCMReviewSummaryObservation
// Threads contains normalized review threads fetched on the slower review cadence.
Threads []SCMReviewThreadObservation
// Partial is true when the provider intentionally fetched and persisted a
@ -192,6 +194,22 @@ type SCMReviewObservation struct {
Partial bool
}
// SCMReviewSummaryObservation is one submitted review with its provider summary URL.
type SCMReviewSummaryObservation struct {
// ID is the provider's stable submitted-review identifier.
ID string
// Author is the provider login/name of the reviewer.
Author string
// State is AO's normalized review decision for this review.
State string
// URL is a provider link to the submitted review summary.
URL string
// IsBot is true when the provider identifies the reviewer as a bot.
IsBot bool
// SubmittedAt is the provider's review submission timestamp.
SubmittedAt time.Time
}
// SCMReviewThreadObservation is a normalized review thread with comments.
type SCMReviewThreadObservation struct {
// ID is the provider's stable review thread identifier.

24
backend/internal/service/session/claim_pr.go
View File

@ -113,8 +113,8 @@ func (s *Service) ClaimPR(ctx context.Context, id domain.SessionID, ref string,
return ClaimPRResult{}, err
}
now := s.clock().UTC()
pr, checks, threads, comments := claimRowsFromSCM(id, obs, now)
outcome, err := s.prClaimer.ClaimPR(ctx, pr, checks, threads, comments, reviewMode, opts.AllowTakeover)
pr, checks, reviews, threads, comments := claimRowsFromSCM(id, obs, now)
outcome, err := s.prClaimer.ClaimPR(ctx, pr, checks, reviews, threads, comments, reviewMode, opts.AllowTakeover)
if err != nil {
return ClaimPRResult{}, err
}
@ -163,6 +163,7 @@ func (s *Service) enrichClaimReviews(ctx context.Context, ref ports.SCMPRRef, ob
obs.Review.Decision = review.Decision
}
obs.Review.Threads = review.Threads
obs.Review.Reviews = review.Reviews
obs.Review.Partial = review.Partial
if review.Partial {
return ports.ReviewWriteMerge, nil
@ -181,7 +182,7 @@ func scmRepoForClaim(provider scmProvider, projectOrigin, prURL string) (ports.S
return ports.SCMRepo{Provider: "github", Host: "github.com", Owner: owner, Name: name, Repo: owner + "/" + name}, nil
}
func claimRowsFromSCM(sessionID domain.SessionID, obs ports.SCMObservation, now time.Time) (domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReviewThread, []domain.PullRequestComment) {
func claimRowsFromSCM(sessionID domain.SessionID, obs ports.SCMObservation, now time.Time) (domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReview, []domain.PullRequestReviewThread, []domain.PullRequestComment) {
observedAt := obs.ObservedAt
if observedAt.IsZero() {
observedAt = now
@ -226,6 +227,21 @@ func claimRowsFromSCM(sessionID domain.SessionID, obs ports.SCMObservation, now
for _, ch := range obs.CI.Checks {
checks = append(checks, domain.PullRequestCheck{Name: ch.Name, CommitHash: obs.CI.HeadSHA, Status: domain.PRCheckStatus(ch.Status), Conclusion: ch.Conclusion, URL: ch.URL, Details: ch.ProviderID, LogTail: ch.LogTail, CreatedAt: now})
}
reviews := make([]domain.PullRequestReview, 0, len(obs.Review.Reviews))
for _, review := range obs.Review.Reviews {
submittedAt := review.SubmittedAt
if submittedAt.IsZero() {
submittedAt = now
}
reviews = append(reviews, domain.PullRequestReview{
ID: review.ID,
Author: review.Author,
State: domain.ReviewDecision(firstNonEmpty(review.State, string(domain.ReviewNone))),
URL: review.URL,
IsBot: review.IsBot,
SubmittedAt: submittedAt,
})
}
threads := make([]domain.PullRequestReviewThread, 0, len(obs.Review.Threads))
commentCount := 0
for _, th := range obs.Review.Threads {
@ -238,7 +254,7 @@ func claimRowsFromSCM(sessionID domain.SessionID, obs ports.SCMObservation, now
comments = append(comments, domain.PullRequestComment{ThreadID: th.ID, ID: c.ID, Author: c.Author, File: th.Path, Line: th.Line, Body: c.Body, URL: c.URL, Resolved: th.Resolved, IsBot: c.IsBot || th.IsBot, CreatedAt: now})
}
}
return pr, checks, threads, comments
return pr, checks, reviews, threads, comments
}
func sessionmanagerAPIError(code, message string) error {

67
backend/internal/service/session/pr_summary.go
View File

@ -62,6 +62,8 @@ type PRUnresolvedReviewer struct {
ReviewerID string
Count int
Links []PRReviewCommentLink
ReviewURL string
IsBot bool
}
// PRReviewCommentLink points to one unresolved review comment.
@ -107,17 +109,21 @@ func (s *Service) ListPRSummaries(ctx context.Context, id domain.SessionID) ([]P
if err != nil {
return nil, err
}
reviews, err := s.store.ListPRReviews(ctx, pr.URL)
if err != nil {
return nil, err
}
comments, err := s.store.ListPRComments(ctx, pr.URL)
if err != nil {
return nil, err
}
out = append(out, summarizePR(pr, checks, threads, comments))
out = append(out, summarizePR(pr, checks, reviews, threads, comments))
}
sortPRSummaries(out)
return out, nil
}
func summarizePR(pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment) PRSummary {
func summarizePR(pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment) PRSummary {
return PRSummary{
URL: pr.URL,
HTMLURL: firstNonEmpty(pr.HTMLURL, pr.URL),
@ -134,7 +140,7 @@ func summarizePR(pr domain.PullRequest, checks []domain.PullRequestCheck, thread
Deletions: pr.Deletions,
ChangedFiles: pr.ChangedFiles,
CI: summarizeCI(pr, checks),
Review: summarizeReview(pr, comments),
Review: summarizeReview(pr, comments, reviews),
Mergeability: summarizeMergeability(pr, threads),
UpdatedAt: pr.UpdatedAt,
ObservedAt: pr.ObservedAt,
@ -166,7 +172,7 @@ func summarizeCI(pr domain.PullRequest, checks []domain.PullRequestCheck) PRCISu
return out
}
func summarizeReview(pr domain.PullRequest, comments []domain.PullRequestComment) PRReviewSummary {
func summarizeReview(pr domain.PullRequest, comments []domain.PullRequestComment, reviews []domain.PullRequestReview) PRReviewSummary {
out := PRReviewSummary{Decision: reviewOrNone(pr.Review)}
if pr.Merged || pr.Closed {
return out
@ -174,6 +180,7 @@ func summarizeReview(pr domain.PullRequest, comments []domain.PullRequestComment
byReviewer := map[string]int{}
order := []string{}
links := map[string][]PRReviewCommentLink{}
isBot := map[string]bool{}
for _, c := range comments {
if c.Resolved || c.IsBot {
continue
@ -186,24 +193,74 @@ func summarizeReview(pr domain.PullRequest, comments []domain.PullRequestComment
order = append(order, reviewer)
}
byReviewer[reviewer]++
isBot[reviewer] = c.IsBot
links[reviewer] = append(links[reviewer], PRReviewCommentLink{
URL: c.URL,
File: c.File,
Line: c.Line,
})
}
reviewURLByAuthor := map[string]string{}
for reviewer, review := range latestChangesRequestedReviews(reviews) {
if _, ok := byReviewer[reviewer]; !ok {
order = append(order, reviewer)
}
reviewURLByAuthor[reviewer] = review.URL
isBot[reviewer] = review.IsBot
}
sort.Strings(order)
for _, reviewer := range order {
out.UnresolvedBy = append(out.UnresolvedBy, PRUnresolvedReviewer{
ReviewerID: reviewer,
Count: byReviewer[reviewer],
Links: links[reviewer],
ReviewURL: reviewURLByAuthor[reviewer],
IsBot: isBot[reviewer],
})
}
out.HasUnresolvedHumanComments = len(out.UnresolvedBy) > 0
for _, reviewer := range out.UnresolvedBy {
if reviewer.Count > 0 && !reviewer.IsBot {
out.HasUnresolvedHumanComments = true
break
}
}
return out
}
func latestChangesRequestedReviews(reviews []domain.PullRequestReview) map[string]domain.PullRequestReview {
latestByReviewer := map[string]domain.PullRequestReview{}
for _, review := range reviews {
if review.State != domain.ReviewChangesRequest && review.State != domain.ReviewApproved {
continue
}
reviewer := strings.TrimSpace(review.Author)
if reviewer == "" {
reviewer = "unknown"
}
current, ok := latestByReviewer[reviewer]
if !ok || reviewAfter(review, current) {
latestByReviewer[reviewer] = review
}
}
out := map[string]domain.PullRequestReview{}
for reviewer, review := range latestByReviewer {
if review.State == domain.ReviewChangesRequest {
out[reviewer] = review
}
}
return out
}
func reviewAfter(a, b domain.PullRequestReview) bool {
if a.SubmittedAt.IsZero() || b.SubmittedAt.IsZero() {
return a.SubmittedAt.IsZero() == b.SubmittedAt.IsZero() && a.ID > b.ID
}
if a.SubmittedAt.Equal(b.SubmittedAt) {
return a.ID > b.ID
}
return a.SubmittedAt.After(b.SubmittedAt)
}
func summarizeMergeability(pr domain.PullRequest, _ []domain.PullRequestReviewThread) PRMergeabilitySummary {
return PRMergeabilitySummary{
State: mergeabilityOrUnknown(pr.Mergeability),

1
backend/internal/service/session/service.go
View File

@ -25,6 +25,7 @@ type Store interface {
ListPRFactsForSession(ctx context.Context, id domain.SessionID) ([]domain.PRFacts, error)
ListPRsBySession(ctx context.Context, sessionID domain.SessionID) ([]domain.PullRequest, error)
ListChecks(ctx context.Context, prURL string) ([]domain.PullRequestCheck, error)
ListPRReviews(ctx context.Context, prURL string) ([]domain.PullRequestReview, error)
ListPRReviewThreads(ctx context.Context, prURL string) ([]domain.PullRequestReviewThread, error)
ListPRComments(ctx context.Context, prURL string) ([]domain.PullRequestComment, error)
GetProject(ctx context.Context, id string) (domain.ProjectRecord, bool, error)

13
backend/internal/service/session/service_test.go
View File

@ -25,6 +25,7 @@ type fakeStore struct {
pr map[domain.SessionID]domain.PRFacts
projects map[string]domain.ProjectRecord
checks map[string][]domain.PullRequestCheck
reviews map[string][]domain.PullRequestReview
threads map[string][]domain.PullRequestReviewThread
comments map[string][]domain.PullRequestComment
num int
@ -36,6 +37,7 @@ func newFakeStore() *fakeStore {
pr: map[domain.SessionID]domain.PRFacts{},
projects: map[string]domain.ProjectRecord{},
checks: map[string][]domain.PullRequestCheck{},
reviews: map[string][]domain.PullRequestReview{},
threads: map[string][]domain.PullRequestReviewThread{},
comments: map[string][]domain.PullRequestComment{},
}
@ -118,6 +120,10 @@ func (f *fakeStore) ListChecks(_ context.Context, prURL string) ([]domain.PullRe
return append([]domain.PullRequestCheck(nil), f.checks[prURL]...), nil
}
func (f *fakeStore) ListPRReviews(_ context.Context, prURL string) ([]domain.PullRequestReview, error) {
return append([]domain.PullRequestReview(nil), f.reviews[prURL]...), nil
}
func (f *fakeStore) ListPRReviewThreads(_ context.Context, prURL string) ([]domain.PullRequestReviewThread, error) {
return append([]domain.PullRequestReviewThread(nil), f.threads[prURL]...), nil
}
@ -567,7 +573,7 @@ type errorFreeClaimOutcome struct {
ports.ClaimOutcome
}
func (f fakePRClaimer) ClaimPR(context.Context, domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReviewThread, []domain.PullRequestComment, ports.ReviewWriteMode, bool) (ports.ClaimOutcome, error) {
func (f fakePRClaimer) ClaimPR(context.Context, domain.PullRequest, []domain.PullRequestCheck, []domain.PullRequestReview, []domain.PullRequestReviewThread, []domain.PullRequestComment, ports.ReviewWriteMode, bool) (ports.ClaimOutcome, error) {
return f.out.ClaimOutcome, f.err
}
@ -685,6 +691,9 @@ func TestListPRSummariesOmitsRawLogsAndReviewBodies(t *testing.T) {
{Name: "unit", Status: domain.PRCheckFailed, Conclusion: "failure", URL: "https://github.com/acme/repo/actions/runs/1", LogTail: "panic: secret"},
{Name: "lint", Status: domain.PRCheckPassed, Conclusion: "success", URL: "https://github.com/acme/repo/actions/runs/2"},
}
stList.reviews[prURL] = []domain.PullRequestReview{
{ID: "review-1", Author: "reviewer-a", State: domain.ReviewChangesRequest, URL: "https://github.com/acme/repo/pull/7#pullrequestreview-1", SubmittedAt: now.Add(-30 * time.Second)},
}
stList.comments[prURL] = []domain.PullRequestComment{
{Author: "reviewer-a", File: "main.go", Line: 12, Body: "raw body must stay private", URL: "https://github.com/acme/repo/pull/7#discussion_r1"},
{Author: "ci-bot", File: "main.go", Line: 13, Body: "bot body", URL: "https://github.com/acme/repo/pull/7#discussion_r2", IsBot: true},
@ -710,6 +719,8 @@ func TestListPRSummariesOmitsRawLogsAndReviewBodies(t *testing.T) {
}
if reviewer := pr.Review.UnresolvedBy[0]; reviewer.ReviewerID != "reviewer-a" || reviewer.Count != 2 || len(reviewer.Links) != 2 {
t.Fatalf("reviewer = %+v", reviewer)
} else if reviewer.ReviewURL != "https://github.com/acme/repo/pull/7#pullrequestreview-1" {
t.Fatalf("review url = %q", reviewer.ReviewURL)
}
if pr.Mergeability.State != domain.MergeConflicting || len(pr.Mergeability.ConflictFiles) != 0 || !containsString(pr.Mergeability.Reasons, "conflicts") {
t.Fatalf("mergeability = %+v", pr.Mergeability)

10
backend/internal/storage/sqlite/gen/models.go
View File

@ -101,6 +101,16 @@ type PRComment struct {
IsBot int64
}
type PRReview struct {
PRURL string
ReviewID string
Author string
State string
URL string
IsBot int64
SubmittedAt time.Time
}
type PRReviewThread struct {
PRURL string
ThreadID string

90
backend/internal/storage/sqlite/gen/pr_reviews.sql.go Normal file
View File

@ -0,0 +1,90 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.31.1
// source: pr_reviews.sql
package gen
import (
"context"
"time"
)
const deletePRReviews = `-- name: DeletePRReviews :exec
DELETE FROM pr_reviews WHERE pr_url = ?
`
func (q *Queries) DeletePRReviews(ctx context.Context, prUrl string) error {
_, err := q.db.ExecContext(ctx, deletePRReviews, prUrl)
return err
}
const listPRReviews = `-- name: ListPRReviews :many
SELECT pr_url, review_id, author, state, url, is_bot, submitted_at
FROM pr_reviews WHERE pr_url = ? ORDER BY submitted_at, review_id
`
func (q *Queries) ListPRReviews(ctx context.Context, prUrl string) ([]PRReview, error) {
rows, err := q.db.QueryContext(ctx, listPRReviews, prUrl)
if err != nil {
return nil, err
}
defer rows.Close()
items := []PRReview{}
for rows.Next() {
var i PRReview
if err := rows.Scan(
&i.PRURL,
&i.ReviewID,
&i.Author,
&i.State,
&i.URL,
&i.IsBot,
&i.SubmittedAt,
); 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 upsertPRReview = `-- name: UpsertPRReview :exec
INSERT INTO pr_reviews (pr_url, review_id, author, state, url, is_bot, submitted_at)
VALUES (?, ?, ?, ?, ?, ?, ?)
ON CONFLICT (pr_url, review_id) DO UPDATE SET
author = excluded.author,
state = excluded.state,
url = excluded.url,
is_bot = excluded.is_bot,
submitted_at = excluded.submitted_at
`
type UpsertPRReviewParams struct {
PRURL string
ReviewID string
Author string
State string
URL string
IsBot int64
SubmittedAt time.Time
}
func (q *Queries) UpsertPRReview(ctx context.Context, arg UpsertPRReviewParams) error {
_, err := q.db.ExecContext(ctx, upsertPRReview,
arg.PRURL,
arg.ReviewID,
arg.Author,
arg.State,
arg.URL,
arg.IsBot,
arg.SubmittedAt,
)
return err
}

22
backend/internal/storage/sqlite/migrations/0020_pr_reviews.sql Normal file
View File

@ -0,0 +1,22 @@
-- Summary: persist provider review submissions so the UI can link to review summaries.
-- +goose Up
-- +goose StatementBegin
CREATE TABLE pr_reviews (
pr_url TEXT NOT NULL REFERENCES pr (url) ON DELETE CASCADE,
review_id TEXT NOT NULL,
author TEXT NOT NULL DEFAULT '',
state TEXT NOT NULL DEFAULT 'none'
CHECK (state IN ('none', 'approved', 'changes_requested', 'review_required')),
url TEXT NOT NULL DEFAULT '',
is_bot INTEGER NOT NULL DEFAULT 0,
submitted_at TIMESTAMP NOT NULL,
PRIMARY KEY (pr_url, review_id)
);
CREATE INDEX idx_pr_reviews_lookup ON pr_reviews (pr_url, submitted_at);
-- +goose StatementEnd
-- +goose Down
-- +goose StatementBegin
DROP INDEX IF EXISTS idx_pr_reviews_lookup;
DROP TABLE IF EXISTS pr_reviews;
-- +goose StatementEnd

16
backend/internal/storage/sqlite/queries/pr_reviews.sql Normal file
View File

@ -0,0 +1,16 @@
-- name: UpsertPRReview :exec
INSERT INTO pr_reviews (pr_url, review_id, author, state, url, is_bot, submitted_at)
VALUES (?, ?, ?, ?, ?, ?, ?)
ON CONFLICT (pr_url, review_id) DO UPDATE SET
author = excluded.author,
state = excluded.state,
url = excluded.url,
is_bot = excluded.is_bot,
submitted_at = excluded.submitted_at;
-- name: DeletePRReviews :exec
DELETE FROM pr_reviews WHERE pr_url = ?;
-- name: ListPRReviews :many
SELECT pr_url, review_id, author, state, url, is_bot, submitted_at
FROM pr_reviews WHERE pr_url = ? ORDER BY submitted_at, review_id;

26
backend/internal/storage/sqlite/store/pr_cdc_test.go
View File

@ -64,17 +64,17 @@ func TestPRReviewThreadsCDC_EmitsOnInsertAndResolvedTransition(t *testing.T) {
now := time.Now().UTC().Truncate(time.Second)
pr := domain.PullRequest{URL: "https://example/pr/9", SessionID: rec.ID, Number: 9, UpdatedAt: now}
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{{
ThreadID: "t1", Path: "main.go", Line: 7, IsBot: true, SemanticHash: "v1", UpdatedAt: now,
}}, nil, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
}
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{{
ThreadID: "t1", Path: "main.go", Line: 8, Resolved: true, IsBot: true, SemanticHash: "v2", UpdatedAt: now.Add(time.Second),
}}, nil, ports.ReviewWriteMerge); err != nil {
t.Fatal(err)
}
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{{
ThreadID: "t1", Path: "main.go", Line: 9, Resolved: true, IsBot: true, SemanticHash: "v3", UpdatedAt: now.Add(2 * time.Second),
}}, nil, ports.ReviewWriteMerge); err != nil {
t.Fatal(err)
@ -138,7 +138,7 @@ func TestPRReviewThreadsCDC_EmitsResolvedOnReplacePoll(t *testing.T) {
// First poll: seed via Replace (no Partial pagination). Same shape as the
// real GitHub provider path when the review-thread listing fits in one page.
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{{
ThreadID: "t1", Path: "main.go", Line: 7, IsBot: true, SemanticHash: "v1", UpdatedAt: now,
}}, nil, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
@ -148,7 +148,7 @@ func TestPRReviewThreadsCDC_EmitsResolvedOnReplacePoll(t *testing.T) {
// On the buggy code this fired the INSERT trigger again (because the
// DELETE-all removed the row first) and the UPDATE trigger never saw the
// resolved transition.
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{{
ThreadID: "t1", Path: "main.go", Line: 7, Resolved: true, IsBot: true, SemanticHash: "v2", UpdatedAt: now.Add(time.Second),
}}, nil, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
@ -197,13 +197,13 @@ func TestPRReviewThreadsReplace_PrunesOrphansWithoutReinserting(t *testing.T) {
now := time.Now().UTC().Truncate(time.Second)
pr := domain.PullRequest{URL: "https://example/pr/56", SessionID: rec.ID, Number: 56, UpdatedAt: now}
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{
{ThreadID: "keep", Path: "a.go", Line: 1, IsBot: true, SemanticHash: "k1", UpdatedAt: now},
{ThreadID: "drop", Path: "b.go", Line: 2, IsBot: true, SemanticHash: "d1", UpdatedAt: now},
}, nil, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
}
if err := s.WriteSCMObservation(ctx, pr, nil, []domain.PullRequestReviewThread{
if err := s.WriteSCMObservation(ctx, pr, nil, nil, []domain.PullRequestReviewThread{
{ThreadID: "keep", Path: "a.go", Line: 1, Resolved: true, IsBot: true, SemanticHash: "k2", UpdatedAt: now.Add(time.Second)},
}, nil, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
@ -283,7 +283,7 @@ func TestClaimPR_CreatesMovesAndGuardsActiveOwner(t *testing.T) {
url := "https://github.com/acme/repo/pull/42"
pr := domain.PullRequest{URL: url, SessionID: first.ID, Number: 42, CI: domain.CIPassing, Mergeability: domain.MergeMergeable, UpdatedAt: time.Now().UTC()}
out, err := s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, true)
out, err := s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, true)
if err != nil {
t.Fatalf("initial claim: %v", err)
}
@ -296,7 +296,7 @@ func TestClaimPR_CreatesMovesAndGuardsActiveOwner(t *testing.T) {
}
pr.SessionID = second.ID
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, false); !errors.Is(err, ports.ErrPRClaimedByActiveSession) {
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, false); !errors.Is(err, ports.ErrPRClaimedByActiveSession) {
t.Fatalf("no-takeover err = %v, want ErrPRClaimedByActiveSession", err)
}
got, _, _ = s.GetPR(ctx, url)
@ -304,7 +304,7 @@ func TestClaimPR_CreatesMovesAndGuardsActiveOwner(t *testing.T) {
t.Fatalf("active-owner refusal moved row to %s", got.SessionID)
}
out, err = s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, true)
out, err = s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, true)
if err != nil {
t.Fatalf("takeover: %v", err)
}
@ -333,7 +333,7 @@ func TestClaimPRCreatedCDCUsesClaimReviewDecision(t *testing.T) {
Review: domain.ReviewChangesRequest,
UpdatedAt: time.Now().UTC(),
}
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, true); err != nil {
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, true); err != nil {
t.Fatal(err)
}
@ -361,7 +361,7 @@ func TestClaimPR_TakesOverTerminatedOwnerAndEmitsSessionChangedCDC(t *testing.T)
second, _ := s.CreateSession(ctx, sampleRecord("mer"))
url := "https://github.com/acme/repo/pull/99"
pr := domain.PullRequest{URL: url, SessionID: first.ID, Number: 99, CI: domain.CIPassing, UpdatedAt: time.Now().UTC()}
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, true); err != nil {
if _, err := s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, true); err != nil {
t.Fatal(err)
}
first.IsTerminated = true
@ -371,7 +371,7 @@ func TestClaimPR_TakesOverTerminatedOwnerAndEmitsSessionChangedCDC(t *testing.T)
}
pr.SessionID = second.ID
out, err := s.ClaimPR(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve, false)
out, err := s.ClaimPR(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve, false)
if err != nil {
t.Fatalf("terminated takeover: %v", err)
}

2
backend/internal/storage/sqlite/store/pr_facts_test.go
View File

@ -28,7 +28,7 @@ func TestListPRFactsForSessionProjectsAllPRsNewestFirst(t *testing.T) {
// historical PR. Distinct updated_at so newest-first ordering is observable.
write := func(pr domain.PullRequest) {
t.Helper()
if err := s.WriteSCMObservation(ctx, pr, nil, nil, nil, ports.ReviewWritePreserve); err != nil {
if err := s.WriteSCMObservation(ctx, pr, nil, nil, nil, nil, ports.ReviewWritePreserve); err != nil {
t.Fatalf("write %s: %v", pr.URL, err)
}
}

66
backend/internal/storage/sqlite/store/pr_store.go
View File

@ -36,7 +36,7 @@ var (
// not touch pr_review_threads: those rows are owned by WriteSCMObservation's
// slower review-thread refresh path.
func (s *Store) WritePR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, comments []domain.PullRequestComment) error {
return s.writePR(ctx, pr, checks, nil, comments, ports.ReviewWritePreserve, true)
return s.writePR(ctx, pr, checks, nil, nil, comments, ports.ReviewWritePreserve, true)
}
// WriteSCMObservation persists a provider-neutral SCM observation in one write
@ -44,14 +44,14 @@ func (s *Store) WritePR(ctx context.Context, pr domain.PullRequest, checks []dom
// and comments are preserved, replaced, or merged according to reviewMode
// because review polling runs at a slower and sometimes intentionally bounded
// cadence than metadata/CI polling.
func (s *Store) WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error {
return s.writePR(ctx, pr, checks, threads, comments, reviewMode, false)
func (s *Store) WriteSCMObservation(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode) error {
return s.writePR(ctx, pr, checks, reviews, threads, comments, reviewMode, false)
}
// ClaimPR moves (or creates) a PR row to pr.SessionID and applies the live SCM
// observation in the same transaction. The session_id update is what fires the
// pr_session_changed CDC trigger added in migration 0005.
func (s *Store) ClaimPR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, allowActiveTakeover bool) (ports.ClaimOutcome, error) {
func (s *Store) ClaimPR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, allowActiveTakeover bool) (ports.ClaimOutcome, error) {
s.writeMu.Lock()
defer s.writeMu.Unlock()
var outcome ports.ClaimOutcome
@ -73,20 +73,20 @@ func (s *Store) ClaimPR(ctx context.Context, pr domain.PullRequest, checks []dom
}); err != nil {
return err
}
return writePRRows(ctx, q, pr, checks, threads, comments, reviewMode, false, false)
return writePRRows(ctx, q, pr, checks, reviews, threads, comments, reviewMode, false, false)
})
return outcome, err
}
func (s *Store) writePR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, replaceLegacyComments bool) error {
func (s *Store) writePR(ctx context.Context, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, replaceLegacyComments bool) error {
s.writeMu.Lock()
defer s.writeMu.Unlock()
return s.inTx(ctx, "write pr observation", func(q *gen.Queries) error {
return writePRRows(ctx, q, pr, checks, threads, comments, reviewMode, replaceLegacyComments, true)
return writePRRows(ctx, q, pr, checks, reviews, threads, comments, reviewMode, replaceLegacyComments, true)
})
}
func writePRRows(ctx context.Context, q *gen.Queries, pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, replaceLegacyComments, rejectReassignment bool) error {
func writePRRows(ctx context.Context, q *gen.Queries, pr domain.PullRequest, checks []domain.PullRequestCheck, reviews []domain.PullRequestReview, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment, reviewMode ports.ReviewWriteMode, replaceLegacyComments, rejectReassignment bool) error {
if rejectReassignment {
existing, err := q.GetPR(ctx, pr.URL)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
@ -114,11 +114,21 @@ func writePRRows(ctx context.Context, q *gen.Queries, pr domain.PullRequest, che
if err := q.DeletePRComments(ctx, pr.URL); err != nil {
return err
}
if err := q.DeletePRReviews(ctx, pr.URL); err != nil {
return err
}
} else if replaceLegacyComments {
if err := q.DeleteLegacyPRComments(ctx, pr.URL); err != nil {
return err
}
}
if reviewMode == ports.ReviewWriteReplace || reviewMode == ports.ReviewWriteMerge {
for _, review := range reviews {
if err := q.UpsertPRReview(ctx, genReviewParams(pr.URL, review)); err != nil {
return fmt.Errorf("review %q: %w", review.ID, err)
}
}
}
if reviewMode == ports.ReviewWriteReplace || reviewMode == ports.ReviewWriteMerge {
for _, th := range threads {
if err := q.UpsertPRReviewThread(ctx, genReviewThreadParams(pr.URL, th)); err != nil {
@ -282,6 +292,19 @@ func (s *Store) ListPRReviewThreads(ctx context.Context, prURL string) ([]domain
return out, nil
}
// ListPRReviews returns a PR's submitted reviews, oldest first.
func (s *Store) ListPRReviews(ctx context.Context, prURL string) ([]domain.PullRequestReview, error) {
rows, err := s.qr.ListPRReviews(ctx, prURL)
if err != nil {
return nil, fmt.Errorf("list pr reviews %s: %w", prURL, err)
}
out := make([]domain.PullRequestReview, 0, len(rows))
for _, review := range rows {
out = append(out, reviewFromGen(review))
}
return out, nil
}
// ---- domain <-> gen mapping ----
// prState collapses the PR's bools into the single pr.state column value.
@ -481,6 +504,33 @@ func reviewThreadFromGen(th gen.PRReviewThread) domain.PullRequestReviewThread {
}
}
func genReviewParams(prURL string, review domain.PullRequestReview) gen.UpsertPRReviewParams {
id := review.ID
if id == "" {
id = review.URL
}
return gen.UpsertPRReviewParams{
PRURL: prURL,
ReviewID: id,
Author: review.Author,
State: string(reviewOrDefault(review.State)),
URL: review.URL,
IsBot: boolInt(review.IsBot),
SubmittedAt: review.SubmittedAt,
}
}
func reviewFromGen(review gen.PRReview) domain.PullRequestReview {
return domain.PullRequestReview{
ID: review.ReviewID,
Author: review.Author,
State: domain.ReviewDecision(review.State),
URL: review.URL,
IsBot: review.IsBot != 0,
SubmittedAt: review.SubmittedAt,
}
}
func boolInt(v bool) int64 {
if v {
return 1

15
backend/internal/storage/sqlite/store/store_test.go
View File

@ -412,10 +412,11 @@ func TestWriteSCMObservationPersistsMetadataChecksReviewsAndComments(t *testing.
UpdatedAt: now, ObservedAt: now, CIObservedAt: now, ReviewObservedAt: now,
}
checks := []domain.PullRequestCheck{{Name: "build", CommitHash: "h1", Status: domain.PRCheckFailed, Conclusion: "failure", URL: "ci", Details: "99", LogTail: "boom", CreatedAt: now}}
reviews := []domain.PullRequestReview{{ID: "review-1", Author: "reviewer", State: domain.ReviewChangesRequest, URL: "https://github.com/o/r/pull/1#pullrequestreview-1", SubmittedAt: now}}
threads := []domain.PullRequestReviewThread{{ThreadID: "t1", Path: "main.go", Line: 7, SemanticHash: "th", UpdatedAt: now}}
comments := []domain.PullRequestComment{{ThreadID: "t1", ID: "c1", Author: "reviewer", File: "main.go", Line: 7, Body: "fix", URL: "comment", CreatedAt: now}}
if err := s.WriteSCMObservation(ctx, pr, checks, threads, comments, ports.ReviewWriteReplace); err != nil {
if err := s.WriteSCMObservation(ctx, pr, checks, reviews, threads, comments, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
}
got, ok, err := s.GetPR(ctx, pr.URL)
@ -433,6 +434,10 @@ func TestWriteSCMObservationPersistsMetadataChecksReviewsAndComments(t *testing.
if len(gotThreads) != 1 || gotThreads[0].ThreadID != "t1" || gotThreads[0].SemanticHash != "th" {
t.Fatalf("threads not persisted: %+v", gotThreads)
}
gotReviews, _ := s.ListPRReviews(ctx, pr.URL)
if len(gotReviews) != 1 || gotReviews[0].ID != "review-1" || gotReviews[0].URL != "https://github.com/o/r/pull/1#pullrequestreview-1" {
t.Fatalf("reviews not persisted: %+v", gotReviews)
}
gotComments, _ := s.ListPRComments(ctx, pr.URL)
if len(gotComments) != 1 || gotComments[0].ThreadID != "t1" || gotComments[0].URL != "comment" {
t.Fatalf("comments not persisted: %+v", gotComments)
@ -455,7 +460,7 @@ func TestWriteSCMObservationMergeUpdatesFetchedReviewWindow(t *testing.T) {
{ThreadID: "older", ID: "older-c1", Author: "ann", Body: "old", CreatedAt: now},
{ThreadID: "latest", ID: "latest-c1", Author: "bob", Body: "before", CreatedAt: now},
}
if err := s.WriteSCMObservation(ctx, pr, nil, initialThreads, initialComments, ports.ReviewWriteReplace); err != nil {
if err := s.WriteSCMObservation(ctx, pr, nil, nil, initialThreads, initialComments, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
}
@ -467,7 +472,7 @@ func TestWriteSCMObservationMergeUpdatesFetchedReviewWindow(t *testing.T) {
{ThreadID: "latest", ID: "latest-c2", Author: "bob", Body: "after", CreatedAt: now.Add(time.Second)},
{ThreadID: "new", ID: "new-c1", Author: "cat", Body: "new", CreatedAt: now.Add(time.Second)},
}
if err := s.WriteSCMObservation(ctx, pr, nil, mergedThreads, mergedComments, ports.ReviewWriteMerge); err != nil {
if err := s.WriteSCMObservation(ctx, pr, nil, nil, mergedThreads, mergedComments, ports.ReviewWriteMerge); err != nil {
t.Fatal(err)
}
@ -524,7 +529,7 @@ func TestWritePRPreservesSCMReviewThreads(t *testing.T) {
threads := []domain.PullRequestReviewThread{{ThreadID: "t1", Path: "main.go", Line: 7, SemanticHash: "thread-v1", UpdatedAt: now}}
comments := []domain.PullRequestComment{{ThreadID: "t1", ID: "c1", Author: "reviewer", Body: "scm", URL: "https://example/comment/c1", CreatedAt: now}}
if err := s.WriteSCMObservation(ctx, pr, nil, threads, comments, ports.ReviewWriteReplace); err != nil {
if err := s.WriteSCMObservation(ctx, pr, nil, nil, threads, comments, ports.ReviewWriteReplace); err != nil {
t.Fatal(err)
}
legacyComments := []domain.PullRequestComment{
@ -575,7 +580,7 @@ func TestWritePRPreservesSCMReviewThreads(t *testing.T) {
mergedThreads := []domain.PullRequestReviewThread{{ThreadID: "t1", Path: "main.go", Line: 8, Resolved: true, SemanticHash: "thread-v2", UpdatedAt: now.Add(2 * time.Second)}}
mergedComments := []domain.PullRequestComment{{ThreadID: "t1", ID: "c2", Author: "reviewer", Body: "updated", URL: "https://example/comment/c2", CreatedAt: now.Add(2 * time.Second)}}
if err := s.WriteSCMObservation(ctx, pr, nil, mergedThreads, mergedComments, ports.ReviewWriteMerge); err != nil {
if err := s.WriteSCMObservation(ctx, pr, nil, nil, mergedThreads, mergedComments, ports.ReviewWriteMerge); err != nil {
t.Fatal(err)
}
gotComments, err = s.ListPRComments(ctx, pr.URL)

2
frontend/src/api/schema.ts
View File

@ -797,7 +797,9 @@ export interface components {
};
SessionPRUnresolvedReviewer: {
count: number;
isBot?: boolean;
links: components["schemas"]["SessionPRReviewCommentLink"][];
reviewUrl?: string;
reviewerId: string;
};
SessionPreviewResponse: {

108
frontend/src/renderer/__tests__/integration/pr-hydration.test.tsx
View File

@ -73,6 +73,100 @@ function respondWithProjectAndPRs() {
});
}
function respondWithAttentionPR() {
getMock.mockImplementation(async (url: string) => {
if (url === "/api/v1/projects") {
return { data: { projects: [{ id: "proj-1", name: "my-app", path: "/repo/my-app" }] }, error: undefined };
}
if (url === "/api/v1/sessions/sess-1/pr") {
return {
data: {
sessionId: "sess-1",
prs: [
{
url: "https://github.com/aoagents/ReverbCode/pull/278",
htmlUrl: "https://github.com/aoagents/ReverbCode/pull/278",
number: 278,
title: "fix the bug",
state: "open",
provider: "github",
repo: "aoagents/ReverbCode",
author: "worker",
sourceBranch: "fix/bug",
targetBranch: "main",
headSha: "abc123",
additions: 1,
deletions: 1,
changedFiles: 1,
ci: { state: "passing", failingChecks: [] },
review: {
decision: "changes_requested",
hasUnresolvedHumanComments: true,
unresolvedBy: [
{
reviewerId: "reviewer-a",
count: 1,
reviewUrl: "https://github.com/aoagents/ReverbCode/pull/278#pullrequestreview-1",
links: [
{
url: "https://github.com/aoagents/ReverbCode/pull/278#discussion_r1",
file: "main.go",
line: 12,
},
],
},
],
},
mergeability: {
state: "conflicting",
reasons: ["conflicts"],
prUrl: "https://github.com/aoagents/ReverbCode/pull/278",
conflictFiles: [],
},
updatedAt: "2026-06-10T16:15:04Z",
observedAt: "2026-06-10T16:15:04Z",
ciObservedAt: "2026-06-10T16:15:04Z",
reviewObservedAt: "2026-06-10T16:15:04Z",
},
],
},
error: undefined,
};
}
if (url === "/api/v1/sessions") {
return {
data: {
sessions: [
{
id: "sess-1",
projectId: "proj-1",
displayName: "fix the bug",
harness: "claude-code",
status: "changes_requested",
isTerminated: false,
updatedAt: "2026-06-10T16:15:04Z",
prs: [
{
number: 278,
state: "open",
url: "https://github.com/aoagents/ReverbCode/pull/278",
ci: "passing",
review: "changes_requested",
mergeability: "conflicting",
reviewComments: true,
updatedAt: "2026-06-10T16:15:04Z",
},
],
},
],
},
error: undefined,
};
}
throw new Error(`unexpected GET ${url}`);
});
}
function renderWithProviders(node: ReactNode) {
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
render(<QueryClientProvider client={queryClient}>{node}</QueryClientProvider>);
@ -93,6 +187,20 @@ describe("PR hydration for a normal project (#251)", () => {
expect(screen.queryByText("no PR yet")).not.toBeInTheDocument();
});
it("links Board attention states to PR fallback and merge conflict targets", async () => {
respondWithAttentionPR();
renderWithProviders(<SessionsBoard />);
expect(await screen.findByRole("link", { name: "PR" })).toHaveAttribute(
"href",
"https://github.com/aoagents/ReverbCode/pull/278",
);
expect(screen.getByRole("link", { name: "conflicts" })).toHaveAttribute(
"href",
"https://github.com/aoagents/ReverbCode/pull/278/conflicts",
);
});
it("lists every session PR on the PR page instead of being empty", async () => {
renderWithProviders(<PullRequestsPage />);

4
frontend/src/renderer/components/SessionInspector.tsx
View File

@ -15,7 +15,7 @@ import { apiClient, apiErrorMessage } from "../lib/api-client";
import { workspaceQueryKey } from "../hooks/useWorkspaceQuery";
import { formatTimeCompact } from "../lib/format-time";
import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary";
import { prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display";
import { prBrowserUrl, prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display";
import type { SessionStatus, WorkspaceSession } from "../types/workspace";
import { sortedPRs, workerDisplayStatus } from "../types/workspace";
import { BrowserPanelView } from "./BrowserPanel";
@ -219,7 +219,7 @@ function PRSummaryCard({ pr }: { pr: SessionPRSummary }) {
{pr.state}
</Badge>
<a
href={pr.htmlUrl || pr.url}
href={prBrowserUrl(pr)}
target="_blank"
rel="noopener noreferrer"
className="ml-auto inline-flex items-center gap-0.5 text-[11px] font-medium text-accent hover:underline"

21
frontend/src/renderer/components/SessionsBoard.tsx
View File

@ -1,4 +1,4 @@
import { useState } from "react";
import { useState, type KeyboardEvent } from "react";
import { useQueryClient } from "@tanstack/react-query";
import { useNavigate } from "@tanstack/react-router";
import { Plus } from "lucide-react";
@ -263,11 +263,22 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: (
const branch = session.branch || "";
const showBranch = branch !== "" && !sameLabel(branch, session.title) && !sameLabel(branch, session.id);
const prSummaries = sessionPRDisplaySummaries(session, useSessionScmSummary(session.id).data);
const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
if (event.target !== event.currentTarget) {
return;
}
if (event.key === "Enter" || event.key === " ") {
event.preventDefault();
onOpen();
}
};
return (
<button
<div
className="w-full rounded-[7px] border border-border bg-surface text-left transition-colors hover:border-border-strong"
onClick={onOpen}
type="button"
onKeyDown={handleKeyDown}
role="button"
tabIndex={0}
>
<div className="flex items-center gap-2 px-[13px] pb-[9px] pt-3">
<span className={cn("inline-flex items-center gap-1.5 text-[11px] font-medium", badge.className)}>
@ -303,7 +314,7 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: (
"no PR yet"
)}
</div>
</button>
</div>
);
}
@ -316,7 +327,7 @@ function BoardPRSummary({ className, pr }: { className?: string; pr: SessionPRSu
</span>
{diffSummary ? <span className="truncate">{diffSummary}</span> : null}
<PRStatusStrip pr={pr} />
<PRAttentionPanel className="mt-1.5 pt-1.5" interactiveLinks={false} maxItems={2} pr={pr} />
<PRAttentionPanel className="mt-1.5 pt-1.5" maxItems={2} pr={pr} />
</div>
);
}

159
frontend/src/renderer/lib/pr-display.test.ts
View File

@ -1,6 +1,6 @@
import { describe, expect, it } from "vitest";
import type { SessionPRSummary } from "../hooks/useSessionScmSummary";
import { prAttentionItems, prDiffSummary, prStatusRows } from "./pr-display";
import { prAttentionItems, prBrowserUrl, prDiffSummary, prStatusRows } from "./pr-display";
const summary = (overrides: Partial<SessionPRSummary> = {}): SessionPRSummary => ({
url: "https://github.com/acme/repo/pull/7",
@ -56,6 +56,19 @@ describe("prDiffSummary", () => {
});
});
describe("prBrowserUrl", () => {
it("normalizes issue-shaped GitHub PR URLs to the pull request page", () => {
expect(
prBrowserUrl(
summary({
url: "https://github.com/acme/repo/issues/7",
htmlUrl: "https://github.com/acme/repo/issues/7",
}),
),
).toBe("https://github.com/acme/repo/pull/7");
});
});
describe("prAttentionItems", () => {
it("returns no attention for clean open PRs", () => {
expect(prAttentionItems(summary())).toEqual([]);
@ -100,6 +113,150 @@ describe("prAttentionItems", () => {
});
});
it("links failing CI checks to their provider URLs", () => {
const items = prAttentionItems(
summary({
ci: {
state: "failing",
failingChecks: [
{ name: "unit", status: "failed", conclusion: "failure", url: "https://checks.example/unit" },
{ name: "lint", status: "failed", conclusion: "failure", url: "https://checks.example/lint" },
{ name: "build", status: "failed", conclusion: "failure", url: "https://checks.example/build" },
{ name: "types", status: "failed", conclusion: "failure", url: "https://checks.example/types" },
],
},
}),
);
const ciItem = items.find((item) => item.kind === "ci_failing");
expect(ciItem?.links).toEqual([
{ label: "unit", href: "https://checks.example/unit", title: "failure" },
{ label: "lint", href: "https://checks.example/lint", title: "failure" },
{ label: "build", href: "https://checks.example/build", title: "failure" },
]);
expect(ciItem?.overflowLabel).toBe("+1 check");
});
it("prefers the submitted review summary over inline comments", () => {
const items = prAttentionItems(
summary({
review: {
decision: "changes_requested",
hasUnresolvedHumanComments: true,
unresolvedBy: [
{
reviewerId: "alice",
count: 2,
reviewUrl: "https://github.com/acme/repo/pull/7#pullrequestreview-1",
links: [
{ url: "https://github.com/acme/repo/pull/7#discussion_r1", file: "main.go", line: 12 },
{ url: "https://github.com/acme/repo/pull/7#discussion_r2", file: "test.go", line: 20 },
],
},
],
},
}),
);
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({
label: "alice +1",
href: "https://github.com/acme/repo/pull/7#pullrequestreview-1",
title: "Open requested-changes review from alice",
});
});
it("falls back to the first inline comment when no review summary exists", () => {
const items = prAttentionItems(
summary({
review: {
decision: "changes_requested",
hasUnresolvedHumanComments: true,
unresolvedBy: [
{
reviewerId: "alice",
count: 2,
links: [
{ url: "https://github.com/acme/repo/pull/7#discussion_r1", file: "main.go", line: 12 },
{ url: "https://github.com/acme/repo/pull/7#discussion_r2", file: "test.go", line: 20 },
],
},
],
},
}),
);
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({
label: "alice +1",
href: "https://github.com/acme/repo/pull/7#discussion_r1",
title: "2 unresolved comments from alice",
});
});
it("falls back to the PR page when review summary and inline comment URLs are missing", () => {
const items = prAttentionItems(
summary({
url: "https://github.com/acme/repo/issues/7",
htmlUrl: "https://github.com/acme/repo/issues/7",
review: {
decision: "changes_requested",
hasUnresolvedHumanComments: true,
unresolvedBy: [{ reviewerId: "alice", count: 1, links: [] }],
},
}),
);
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({
label: "alice",
href: "https://github.com/acme/repo/pull/7",
title: "Open pull request for alice",
});
});
it("shows bot reviewers with a bot label", () => {
const items = prAttentionItems(
summary({
review: {
decision: "changes_requested",
hasUnresolvedHumanComments: false,
unresolvedBy: [
{
reviewerId: "copilot",
count: 0,
reviewUrl: "https://github.com/acme/repo/pull/7#pullrequestreview-2",
isBot: true,
links: [],
},
],
},
}),
);
expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({
label: "copilot bot",
href: "https://github.com/acme/repo/pull/7#pullrequestreview-2",
title: "Open requested-changes review from copilot bot",
});
});
it("links merge conflicts to GitHub's conflict resolution page", () => {
const items = prAttentionItems(
summary({
url: "https://github.com/acme/repo/issues/7",
htmlUrl: "https://github.com/acme/repo/issues/7",
mergeability: {
state: "conflicting",
reasons: [],
prUrl: "https://github.com/acme/repo/issues/7",
},
}),
);
expect(items.find((item) => item.kind === "merge_conflict")?.links[0]).toMatchObject({
label: "conflicts",
href: "https://github.com/acme/repo/pull/7/conflicts",
});
});
it("suppresses attention once the PR is closed or merged", () => {
expect(
prAttentionItems(

101
frontend/src/renderer/lib/pr-display.ts
View File

@ -46,6 +46,10 @@ export function comparePRDisplaySummaries(a: SessionPRSummary, b: SessionPRSumma
return prStateRank[a.state] - prStateRank[b.state] || a.number - b.number;
}
export function prBrowserUrl(pr: SessionPRSummary): string {
return prBaseUrl(pr) ?? pr.htmlUrl ?? pr.url;
}
export function sessionPRDisplaySummaries(
session: WorkspaceSession,
summaries: SessionPRSummary[] = [],
@ -175,11 +179,10 @@ export function prAttentionItems(pr: SessionPRSummary): PRAttentionItem[] {
}
if (pr.review.decision === "changes_requested" || pr.review.hasUnresolvedHumanComments) {
const reviewers = pr.review.unresolvedBy.slice(0, 3);
const links = reviewers.map((reviewer) => ({
label: reviewerLabel(reviewer),
href: reviewer.links.find((link) => link.url)?.url || undefined,
title: `${reviewer.count} unresolved ${pluralize("comment", reviewer.count)}`,
}));
const links = reviewers.map((reviewer) => reviewAttentionLink(pr, reviewer));
if (links.length === 0 && pr.review.decision === "changes_requested") {
links.push({ label: "PR", href: prBrowserUrl(pr), title: "Open pull request" });
}
items.push({
kind: "review_changes_requested",
title: "Address requested changes",
@ -331,18 +334,23 @@ function mergeAttention(
fallback: string,
tone: PRDisplayTone,
): PRAttentionItem {
const href =
kind === "merge_conflict" ? mergeConflictUrl(pr) : pr.mergeability.prUrl || pr.htmlUrl || pr.url || undefined;
const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({
label: file.path,
href: file.url || pr.mergeability.prUrl || undefined,
href: file.url || href,
title: kind === "merge_conflict" ? "Open merge conflicts" : undefined,
}));
const reasonLinks =
fileLinks.length > 0
fileLinks.length > 0 || kind === "merge_conflict"
? []
: pr.mergeability.reasons.slice(0, 3).map((reason) => ({
label: mergeReasonLabel(reason),
href: pr.mergeability.prUrl || undefined,
href,
}));
const links = fileLinks.length > 0 ? fileLinks : reasonLinks;
const fallbackLink =
kind === "merge_conflict" && href ? [{ label: "conflicts", href, title: "Open merge conflicts" }] : [];
const links = fileLinks.length > 0 ? fileLinks : reasonLinks.length > 0 ? reasonLinks : fallbackLink;
return {
kind,
title,
@ -356,11 +364,78 @@ function mergeAttention(
};
}
function reviewerLabel(reviewer: SessionPRSummary["review"]["unresolvedBy"][number]): string {
if (reviewer.count <= 1) {
return reviewer.reviewerId;
function mergeConflictUrl(pr: SessionPRSummary): string | undefined {
return prSubpageUrl(pr, "conflicts") ?? pr.mergeability.prUrl ?? prBrowserUrl(pr);
}
function prBaseUrl(pr: SessionPRSummary): string | undefined {
return prURL(pr);
}
function prSubpageUrl(pr: SessionPRSummary, subpage: "conflicts"): string | undefined {
const base = prURL(pr);
return base ? `${base}/${subpage}` : undefined;
}
function prURL(pr: SessionPRSummary): string | undefined {
const raw = pr.htmlUrl || pr.mergeability.prUrl || pr.url;
if (!raw) {
return undefined;
}
return `${reviewer.reviewerId} +${reviewer.count - 1}`;
try {
const url = new URL(raw);
const match = url.pathname.match(/^(\/[^/]+\/[^/]+)\/(?:pull|issues)\/(\d+)(?:\/.*)?$/);
if (!match) {
return undefined;
}
url.pathname = `${match[1]}/pull/${match[2]}`;
url.search = "";
url.hash = "";
return url.toString();
} catch {
return undefined;
}
}
function reviewerLabel(reviewer: SessionPRSummary["review"]["unresolvedBy"][number]): string {
const name = reviewerDisplayName(reviewer);
if (reviewer.count <= 1) {
return name;
}
return `${name} +${reviewer.count - 1}`;
}
function reviewerDisplayName(reviewer: SessionPRSummary["review"]["unresolvedBy"][number]): string {
return reviewer.isBot ? `${reviewer.reviewerId} bot` : reviewer.reviewerId;
}
function reviewAttentionLink(
pr: SessionPRSummary,
reviewer: SessionPRSummary["review"]["unresolvedBy"][number],
): PRAttentionLink {
const inlineURL = reviewer.links.find((link) => link.url)?.url;
if (reviewer.reviewUrl) {
return {
label: reviewerLabel(reviewer),
href: reviewer.reviewUrl,
title: `Open requested-changes review from ${reviewerDisplayName(reviewer)}`,
};
}
if (inlineURL) {
return {
label: reviewerLabel(reviewer),
href: inlineURL,
title:
reviewer.count > 0
? `${reviewer.count} unresolved ${pluralize("comment", reviewer.count)} from ${reviewerDisplayName(reviewer)}`
: `Open review comments from ${reviewerDisplayName(reviewer)}`,
};
}
return {
label: reviewerLabel(reviewer),
href: prBrowserUrl(pr),
title: `Open pull request for ${reviewerDisplayName(reviewer)}`,
};
}
function mergeReasonLabel(reason: string): string {