feat(web): add CI hover popover on PR top-bar button (#846)
* feat(github): plumb CI popover fields + branch protection cache
Adds RequiredReviews/UnresolvedReviewThreads/ChecksTotal/ChecksPassing
to TaskPR + PRStatus. Extends the GraphQL PR query with reviewThreads
to populate UnresolvedReviewThreads on every poll. New branch_protection.go
holds an in-memory TTL cache + BranchProtectionFetcher interface;
PATClient/GHClient implement REST/CLI lookups so SyncTaskPR can resolve
required_approving_review_count for the PR's base branch. SyncTaskPR
preserves existing counts when a lightweight (GraphQL) sync returns 0
so the popover doesn't flap during the polling cycle.
* feat(github): mock controller hooks for CI popover e2e
Extends associateTaskPR to accept review/check counts, required_reviews,
and unresolved_review_threads, and switches to ReplaceTaskPR so re-POSTing
the same task drives the popover live-refresh path. Adds POST /pr-feedback
to seed PRFeedback (checks/reviews/comments) keyed by a deterministic
synthetic head_sha, and PUT /auth-health to flip the mock client's
authenticated flag for the "Reconnect GitHub" branch. Mock client gains
ReplaceCheckRuns/ReplaceReviews/ReplaceComments helpers and an
authenticated field that defaults to true.
* feat(web): CI hover popover on PR top-bar button
Hovering the PR top-bar button now reveals a popover with
Passed/In-Progress/Failed counts, a workflow-rollup list under the
non-empty buckets (with "open on GitHub" + "+ ctx" affordances on
failed rows), a review-state row ("Approved 1 / 2 required"), and
unresolved-comments count. Click on the button keeps the existing
"open PR detail panel" behavior so shipped tests stay green.
PRFeedback is lazy-fetched on hover-open via a new SWR-style hook
backed by a 20-entry LRU cache slice; the popover refetches whenever
the underlying TaskPR's updated_at changes (the WS handler already
applies poll deltas). Bucket logic + workflow grouping live in a
small util with vitest coverage. The aggregate-popover variant for
multi-PR tasks ships but the multi-PR trigger preserves its existing
DropdownMenu so per-row click → addPRPanel still works.
* test(e2e): pr-topbar-popover scenarios A–F covering CI popover
11 specs covering: count rows + zero-bucket hiding, workflow rollup
with (N/M passed) badge under Failed, review row + unresolved comments,
hover open + leave close, failed-row open/+ctx affordances, header
external link, "No checks have started" empty state, "Reconnect GitHub"
auth-lost branch, and live count updates on a re-associate.
Adds api-client helpers (mockGitHubSeedPRFeedback, mockGitHubSetAuthHealth,
extended mockGitHubAssociateTaskPR) and SessionPage popover accessors.
* fix(github): address bot review comments on CI popover
- branch_protection.go: evict stale entries on read so the cache map
cannot grow unbounded over a long-running deployment; rewrite the
comment to drop the inaccurate singleflight claim (no dedup logic
exists, concurrent misses race harmlessly to set()).
- gh_client.go: replace fragile strings.Contains "HTTP 404" with an
isNotFoundErr helper that tolerates the formats current and future
gh CLI versions use ("HTTP 404", "404 Not Found", "status: 404").
- graphql.go: extend reviewThreads with totalCount and attribute the
un-fetched tail to the unresolved bucket so PRs with >100 threads
don't silently undercount in the popover.
- pr-ci-popover.tsx: drop the dead "icon" initializer in PRReviewRow
(every branch overwrites it). Memoize useAddCheckToContext so
PRWorkflowRow's reference equality stays stable across renders.
- pr-topbar-button.tsx: collapse the duplicate JSDoc on
usePopoverInteractions.
* fix(github): address remaining bot review comments + tests
Real correctness fixes:
- Replace the "0/0 means counts omitted" heuristic with an explicit
PRStatus.ChecksPopulated flag so the rich REST sync can clear stale
totals when checks are removed from a PR upstream.
- Refresh tp.BaseBranch from status.PR on each SyncTaskPR so a
retargeted PR's branch-protection lookup keys off the new branch.
Persist base_branch in UpdateTaskPR.
- Cache 403 from /branches/.../protection (token lacks Administration:
Read scope) the same way as 404 — otherwise every poller tick burns
rate-limit quota retrying. Both PATClient and GHClient.
- usePRCIPopover: refetch on every popover-open transition (not just
on updated_at change), via a wasEnabledRef gate. Restores the
intended SWR semantics that the lastSyncedRef-only guard suppressed.
- groupChecksByWorkflow: prefer the URL of a job matching the group's
bucket so clicking a "failed" workflow row navigates to a failing
job, not an earlier passing one.
Tests:
- branch_protection_test.go: cache hit/miss/TTL/eviction, error
not cached, no-rule cached, client-without-fetcher path.
- pat_client_test.go: TestPATClient_FetchBranchProtection table-test
for 200/200-with-null/404/403/500.
- service_test.go: ChecksPopulated preserve-on-zero, accept real
zero, base-branch retarget, change-publish for the new fields.
* refactor(github): trim associateTaskPR + drop unreachable PRAggregatePopover
- mock_controller.go: extract associateTaskPRRequest struct,
buildTaskPRFromRequest, and ensureMockPRForRequest helpers so the
handler is back under the 80-line Go limit.
- pr-ci-popover.tsx: remove unreachable PRAggregatePopover export.
Multi-PR tasks currently use the existing DropdownMenu trigger
(no hover popover), so the aggregate component was dead code.
* fix(github): drop unused anchorRef + tests for is{NotFound,Forbidden}Err
- pr-topbar-button.tsx: anchorRef was leftover from the dropped
click-to-pin onInteractOutside guard. Removed.
- gh_client_test.go: tests for the new isNotFoundErr / isForbiddenErr
helpers covering each accepted format and a few non-matches so a
future gh CLI wording change is caught.
* fix(github): UnresolvedReviewThreadsPopulated guard + drop unreachable nil checks
- service.go: mirror the ChecksPopulated/preserve dance for
UnresolvedReviewThreads. The REST single-PR poller (getPRStatus)
doesn't fetch review threads, so without this guard every REST
sync would clobber the GraphQL-populated value to 0 and emit a
spurious github.task_pr.updated event. The GraphQL path
(convertBatchedPRResult) sets the new flag.
- branch_protection.go: drop unreachable s == nil / fetcher == nil
checks. fetchRequiredReviews is only invoked from SyncTaskPR on a
service constructed via NewService, which always initialises the
cache. After a successful type assertion, the interface value
cannot be nil; the !ok branch already covers nil-interface inputs.
- service_test.go: new TestSyncTaskPR_UnresolvedThreads_PreservesOnLightweightSync
covers the regression. Existing NewFieldsPublishEvent threads-change
case updated to set UnresolvedReviewThreadsPopulated.
* refactor(web): drop CI popover progress bar — last-updated is enough
The animated progress bar was visual noise; the "updated Ns ago"
timestamp tells the user the same thing without the indicator. Footer
also hides entirely until the first fetch lands so an empty popover
doesn't show a useless "" line.
* fix(web): background-sync PRFeedback so popover is fresh on first hover
The on-hover SWR fetch was the only refresh path, so opening the popover
showed the previously-cached value while a new fetch raced behind it —
visibly stale until the refetch landed. The button icon, by contrast,
updates the moment a github.task_pr.updated WS push lands.
Add a usePRFeedbackBackgroundSync hook mounted at the PRTopbarButton
that subscribes to the same pr.updated_at change signal and refetches
PRFeedback into the cache in the background. The popover hook still
fires an on-open refetch as a safety net for the very first hover
(before any sync). No extra polling — fetches are gated on real
TaskPR changes, matching the icon's cadence.
* feat(web): CI status chip in chat status bar + tweak elapsed label
- New <PRStatusChip> renders a compact colored circle in the chat
status bar (left of the TodoIndicator, just above the input). Hover
opens the same PRCIPopover anchored to the chip's top edge so the
card expands upward — matching the user's "open bottom-up" ask. The
chip mounts the background-sync hook so the popover is fresh on
first hover even when the top-bar PR button isn't visible.
- elapsedLabel: "just now" → "updated just now" so the footer reads
the same regardless of how recently the cache was refreshed.
* fix(web): chip color was missing — use static class map for Tailwind JIT
\`getPRStatusColor().replace("text-", "bg-")\` produced class strings at
runtime, which Tailwind's content scanner doesn't pick up — the chip
rendered with no background. Replace with a static lookup so each
literal `bg-*` class appears in the source and gets compiled into the
bundle.
* fix(web): chip uses currentColor SVG so the dot actually renders
The previous bg-* approach relied on Tailwind compiling the literal
class strings. Even with a static map some palette entries (notably
bg-muted-foreground) didn't surface as a usable utility, so the chip
remained transparent. Apply getPRStatusColor's text-* class to the
button wrapper and fill an SVG circle with currentColor — the text-*
utilities are already compiled (PRTaskIcon uses them) so the dot
is guaranteed to render in the right color.
* fix(web): redesign PR status chip — clipboard icon + status glyph
- Move chip to right of TodoIndicator (was left).
- Render as "[clipboard icon] [status glyph]" instead of a bare circle:
passed → green CircleCheck, failed → red CircleX, in_progress →
yellow Loader2 (spinning), merged/neutral → muted dot. Status icons
reuse already-compiled text-* utilities so colors render reliably.
* fix(github): ReviewCount = approved-reviewer count + ReviewCountsPopulated guard
Two bugs feeding the popover's "Approved (0)" misreport:
- Semantic: REST path set ReviewCount to len(reviews), which double-counts
authors who left multiple reviews and includes COMMENTED rows. Replace
with countApprovedReviewers — distinct authors whose latest review
state is APPROVED. GraphQL converter gets the parallel
countApprovedReviewerNodes helper and also populates PendingReviewCount
from ReviewRequests.TotalCount (was unset, so the chip's "(K requested)"
suffix was always missing).
- Clobber: GraphQL polls used to write ReviewCount/PendingReviewCount=0
on every cycle because the converter never set them. Add
ReviewCountsPopulated to PRStatus and gate SyncTaskPR's writes on it
the same way ChecksPopulated already does, so a partial sync path
preserves the persisted values instead of resetting them to zero.
* fix(web): popover nitpicks (icon, review row, indent, badge)
- Chip icon: ClipboardCheck → ListCheck.
- PRReviewRow always renders (drop the early-return). Fresh PRs with
no approvals + no protection rule now show "Awaiting review" instead
of vanishing.
- Indent workflow rows under the In Progress / Failed group headers
(pl-5) so the hierarchy reads more clearly.
- In-progress workflow badge: "0/1 ran" → "1 running". The old phrasing
hid whether the job had even started.
* feat(web): stacked pass-rate progress bar in CI popover
Mirrors TodoIndicator's progress bar at the top of the CI section, but
split into three coloured segments:
green (passed) | yellow (in_progress) | red (failed)
sized proportionally to each bucket's count. The label reads
"Pass rate · K/N (P%)" so the user gets the same at-a-glance summary
TODOs shows. Skipped/cancelled-but-not-failed jobs aren't counted —
the bar always reflects the actionable picture.
* fix(web): right-align review counts + drop chip hover bg
- Move approval/required counts to the right edge of the review row
("Approved" on the left, "1 / 2" on the right), mirroring the
bucket-row layout. The "(K requested)" suffix becomes a "· K requested"
trailing fragment on the right side.
- PRStatusChip: drop hover:bg-accent/50 so the trigger doesn't grow a
background plate when the popover is open.
* fix(web): switch CI chip icon to IconChecklist
* fix(github): clear bot review-comment backlog
Real correctness fixes:
- pr-status-chip: pending checks / pending review now beat
checks_state === "success" in chipStatus precedence — a PR with
CI green but reviewers outstanding is no longer rendered as
"passed".
- branch_protection: drop the unreachable Reset() method.
- pr-topbar-button: stop double-mounting usePRFeedbackBackgroundSync;
the chip already covers it.
- use-pr-ci-popover: dedupe key combines prFeedbackKey + updated_at
so switching tasks with same-timestamp PRs still refetches.
- use-pr-ci-popover: drop stale comment about the removed progress bar.
Refactor:
- client_helpers + graphql: extract reviewSample +
latestReviewStateByAuthor helper so the three near-identical
per-reviewer dedup loops share one implementation.
Tests:
- countApprovedReviewers + reduceReviewSummary unit tests.
- TestSyncTaskPR_ReviewCounts_PreservesOnLightweightSync +
AcceptsRealZero mirror the existing ChecksPopulated guards.
- groupChecksByWorkflow distinct-URL test verifies the failed-job
URL-selection fix.
- branch_protection TTL tests now use testing/synctest — instant
virtual time, no real-sleep flake on CI.
* fix(web): clicking the CI chip no longer closes the popover
HoverCard treats the trigger button as outside the content's bounding
box, so a click on the chip fired Radix's onPointerDownOutside and
auto-closed the popover. Add a triggerRef + onPointerDownOutside guard
on HoverCardContent that filters out clicks landing on the trigger —
clicking the chip is now a no-op while the popover stays open via hover.
This commit is contained in:
José Almeida
@@ -0,0 +1,119 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// BranchProtection captures the subset of a GitHub branch protection rule that
|
||||
// the CI popover surfaces ("N approvals required to merge"). Stored values
|
||||
// have a fixed TTL so changes to the protection rule on GitHub propagate
|
||||
// without restarting the backend.
|
||||
type BranchProtection struct {
|
||||
RequiredApprovingReviewCount int
|
||||
HasRule bool // false when GitHub returned 404 (no rule)
|
||||
FetchedAt time.Time
|
||||
}
|
||||
|
||||
// branchProtectionCacheTTL is how long a cached protection lookup stays
|
||||
// authoritative before a refetch is allowed. One hour balances API quota
|
||||
// (4-PRs-per-cycle GraphQL polling) against rule-change latency (rare).
|
||||
const branchProtectionCacheTTL = time.Hour
|
||||
|
||||
// branchProtectionCache is an in-memory TTL cache keyed by
|
||||
// "owner/repo@base_branch". Concurrent reads share the RLock; concurrent
|
||||
// cache misses for the same key may each issue their own GitHub fetch and
|
||||
// race to set() the result — values are deterministic so the loser's
|
||||
// write is harmless, and the extra round-trips are bounded by the 1h TTL.
|
||||
// Stale entries are evicted on read so the map cannot grow unbounded over
|
||||
// a long-running process: callers always observe an active TTL window.
|
||||
type branchProtectionCache struct {
|
||||
mu sync.RWMutex
|
||||
m map[string]BranchProtection
|
||||
ttl time.Duration
|
||||
}
|
||||
|
||||
func newBranchProtectionCache() *branchProtectionCache {
|
||||
return &branchProtectionCache{
|
||||
m: make(map[string]BranchProtection),
|
||||
ttl: branchProtectionCacheTTL,
|
||||
}
|
||||
}
|
||||
|
||||
func (c *branchProtectionCache) get(key string) (BranchProtection, bool) {
|
||||
c.mu.RLock()
|
||||
bp, ok := c.m[key]
|
||||
c.mu.RUnlock()
|
||||
if !ok {
|
||||
return BranchProtection{}, false
|
||||
}
|
||||
if time.Since(bp.FetchedAt) <= c.ttl {
|
||||
return bp, true
|
||||
}
|
||||
// Evict the stale entry so the map doesn't grow unbounded across
|
||||
// long-running deployments with many distinct (owner, repo, branch)
|
||||
// keys. Recheck under the write lock so a concurrent set() that just
|
||||
// landed isn't clobbered.
|
||||
c.mu.Lock()
|
||||
if cur, present := c.m[key]; present && time.Since(cur.FetchedAt) > c.ttl {
|
||||
delete(c.m, key)
|
||||
}
|
||||
c.mu.Unlock()
|
||||
return BranchProtection{}, false
|
||||
}
|
||||
|
||||
func (c *branchProtectionCache) set(key string, bp BranchProtection) {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
c.m[key] = bp
|
||||
}
|
||||
|
||||
func branchProtectionKey(owner, repo, branch string) string {
|
||||
return owner + "/" + repo + "@" + branch
|
||||
}
|
||||
|
||||
// BranchProtectionFetcher is implemented by the live GitHub clients (PAT, gh
|
||||
// CLI). The mock client returns a sentinel "no rule" so e2e tests get
|
||||
// deterministic results unless they seed via the mock controller.
|
||||
type BranchProtectionFetcher interface {
|
||||
FetchBranchProtection(ctx context.Context, owner, repo, branch string) (BranchProtection, error)
|
||||
}
|
||||
|
||||
// fetchRequiredReviews returns the branch-protection-derived
|
||||
// RequiredApprovingReviewCount for (owner, repo, branch), or nil if:
|
||||
// - the active client doesn't implement BranchProtectionFetcher,
|
||||
// - the upstream call errors (token lacks scope, network down, etc.),
|
||||
// - the repo has no protection rule on the base branch.
|
||||
//
|
||||
// Result is cached for branchProtectionCacheTTL. nil never gets cached so a
|
||||
// transient error is retried on the next sync.
|
||||
func (s *Service) fetchRequiredReviews(ctx context.Context, owner, repo, branch string) *int {
|
||||
if owner == "" || repo == "" || branch == "" {
|
||||
return nil
|
||||
}
|
||||
key := branchProtectionKey(owner, repo, branch)
|
||||
if bp, ok := s.protectionCache.get(key); ok {
|
||||
if !bp.HasRule {
|
||||
return nil
|
||||
}
|
||||
n := bp.RequiredApprovingReviewCount
|
||||
return &n
|
||||
}
|
||||
fetcher, ok := s.client.(BranchProtectionFetcher)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
bp, err := fetcher.FetchBranchProtection(ctx, owner, repo, branch)
|
||||
if err != nil {
|
||||
// Don't cache errors — next sync retries.
|
||||
return nil
|
||||
}
|
||||
bp.FetchedAt = time.Now().UTC()
|
||||
s.protectionCache.set(key, bp)
|
||||
if !bp.HasRule {
|
||||
return nil
|
||||
}
|
||||
n := bp.RequiredApprovingReviewCount
|
||||
return &n
|
||||
}
|
||||
@@ -0,0 +1,138 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
)
|
||||
|
||||
// fakeBPFetcher is a minimal BranchProtectionFetcher for testing the
|
||||
// service-level cache wiring. It records call counts and returns canned
|
||||
// values per (owner, repo, branch) key.
|
||||
type fakeBPFetcher struct {
|
||||
calls atomic.Int64
|
||||
bp BranchProtection
|
||||
err error
|
||||
}
|
||||
|
||||
func (f *fakeBPFetcher) FetchBranchProtection(_ context.Context, _, _, _ string) (BranchProtection, error) {
|
||||
f.calls.Add(1)
|
||||
return f.bp, f.err
|
||||
}
|
||||
|
||||
// minimal Client surface — only the bits fetchRequiredReviews needs (it
|
||||
// asserts the client implements BranchProtectionFetcher; everything else
|
||||
// can be nil-valued).
|
||||
type fakeBPClient struct {
|
||||
*fakeBPFetcher
|
||||
Client
|
||||
}
|
||||
|
||||
func newServiceWithBPFetcher(f *fakeBPFetcher) *Service {
|
||||
return &Service{
|
||||
client: fakeBPClient{fakeBPFetcher: f},
|
||||
protectionCache: newBranchProtectionCache(),
|
||||
}
|
||||
}
|
||||
|
||||
func TestBranchProtectionCache_GetSet_TTL(t *testing.T) {
|
||||
// synctest.Test advances time in virtual ticks so a 60ms sleep is
|
||||
// instantaneous wall-clock — keeps the suite fast and deterministic
|
||||
// (no real sleeps that flake on heavily loaded CI runners).
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
c := newBranchProtectionCache()
|
||||
c.ttl = 50 * time.Millisecond
|
||||
key := "o/r@main"
|
||||
c.set(key, BranchProtection{HasRule: true, RequiredApprovingReviewCount: 2, FetchedAt: time.Now()})
|
||||
got, ok := c.get(key)
|
||||
if !ok || got.RequiredApprovingReviewCount != 2 {
|
||||
t.Fatalf("fresh entry: got=%+v ok=%v", got, ok)
|
||||
}
|
||||
time.Sleep(60 * time.Millisecond)
|
||||
if _, ok := c.get(key); ok {
|
||||
t.Fatalf("stale entry should not be returned")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestBranchProtectionCache_StaleEntryIsEvicted(t *testing.T) {
|
||||
c := newBranchProtectionCache()
|
||||
c.ttl = 1 * time.Millisecond
|
||||
c.set("o/r@main", BranchProtection{HasRule: true, FetchedAt: time.Now().Add(-time.Hour)})
|
||||
if _, ok := c.get("o/r@main"); ok {
|
||||
t.Fatalf("expected stale miss")
|
||||
}
|
||||
c.mu.RLock()
|
||||
_, present := c.m["o/r@main"]
|
||||
c.mu.RUnlock()
|
||||
if present {
|
||||
t.Fatalf("stale entry should have been deleted from map")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchRequiredReviews_CacheHit(t *testing.T) {
|
||||
f := &fakeBPFetcher{bp: BranchProtection{HasRule: true, RequiredApprovingReviewCount: 3}}
|
||||
s := newServiceWithBPFetcher(f)
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got == nil || *got != 3 {
|
||||
t.Fatalf("first call: %v", got)
|
||||
}
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got == nil || *got != 3 {
|
||||
t.Fatalf("cached call: %v", got)
|
||||
}
|
||||
if calls := f.calls.Load(); calls != 1 {
|
||||
t.Fatalf("fetcher called %d times, want 1 (cache miss + hit)", calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchRequiredReviews_NoRuleIsCached(t *testing.T) {
|
||||
f := &fakeBPFetcher{bp: BranchProtection{HasRule: false}}
|
||||
s := newServiceWithBPFetcher(f)
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got != nil {
|
||||
t.Fatalf("expected nil for no-rule, got %v", got)
|
||||
}
|
||||
// Second call must not hit the network — the negative result is cached
|
||||
// so we don't re-query upstream every poll.
|
||||
_ = s.fetchRequiredReviews(context.Background(), "o", "r", "main")
|
||||
if calls := f.calls.Load(); calls != 1 {
|
||||
t.Fatalf("fetcher called %d times, want 1", calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchRequiredReviews_ErrorIsNotCached(t *testing.T) {
|
||||
f := &fakeBPFetcher{err: errors.New("boom")}
|
||||
s := newServiceWithBPFetcher(f)
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got != nil {
|
||||
t.Fatalf("expected nil on error, got %v", got)
|
||||
}
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got != nil {
|
||||
t.Fatalf("expected nil on retry, got %v", got)
|
||||
}
|
||||
if calls := f.calls.Load(); calls != 2 {
|
||||
t.Fatalf("fetcher called %d times, want 2 (error not cached)", calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchRequiredReviews_ClientWithoutFetcherReturnsNil(t *testing.T) {
|
||||
// noop client doesn't satisfy BranchProtectionFetcher.
|
||||
s := &Service{client: nil, protectionCache: newBranchProtectionCache()}
|
||||
if got := s.fetchRequiredReviews(context.Background(), "o", "r", "main"); got != nil {
|
||||
t.Fatalf("expected nil when client lacks BranchProtectionFetcher, got %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchRequiredReviews_TTLRefetch(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
f := &fakeBPFetcher{bp: BranchProtection{HasRule: true, RequiredApprovingReviewCount: 1}}
|
||||
s := newServiceWithBPFetcher(f)
|
||||
s.protectionCache.ttl = 50 * time.Millisecond
|
||||
_ = s.fetchRequiredReviews(context.Background(), "o", "r", "main")
|
||||
time.Sleep(60 * time.Millisecond)
|
||||
_ = s.fetchRequiredReviews(context.Background(), "o", "r", "main")
|
||||
if calls := f.calls.Load(); calls != 2 {
|
||||
t.Fatalf("fetcher called %d times, want 2 (refetch after TTL)", calls)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -91,17 +91,103 @@ func getPRStatus(ctx context.Context, c Client, owner, repo string, number int)
|
||||
reviewState, pendingReviewCount := deriveReviewSyncState(pr, reviews)
|
||||
total, passing := countCheckResults(checks)
|
||||
return &PRStatus{
|
||||
PR: pr,
|
||||
ReviewState: reviewState,
|
||||
ChecksState: computeOverallCheckStatus(checks),
|
||||
MergeableState: pr.MergeableState,
|
||||
ReviewCount: len(reviews),
|
||||
PR: pr,
|
||||
ReviewState: reviewState,
|
||||
ChecksState: computeOverallCheckStatus(checks),
|
||||
MergeableState: pr.MergeableState,
|
||||
// ReviewCount is the number of distinct reviewers whose latest review
|
||||
// state is APPROVED — it's the value the popover renders as
|
||||
// "Approved (N)" / "Approved N / M required". Counting raw review
|
||||
// entries (`len(reviews)`) double-counts authors who left multiple
|
||||
// reviews and also includes COMMENTED / CHANGES_REQUESTED rows.
|
||||
ReviewCount: countApprovedReviewers(reviews),
|
||||
PendingReviewCount: pendingReviewCount,
|
||||
ChecksTotal: total,
|
||||
ChecksPassing: passing,
|
||||
// REST path actually counted check runs + reviews — distinguish
|
||||
// from the batched-GraphQL path that only carries rollup state.
|
||||
ChecksPopulated: true,
|
||||
ReviewCountsPopulated: true,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// reviewSample is a normalized review row consumed by latestReviewStateByAuthor.
|
||||
// Adapter functions below convert PRReview (REST) and reviewNode (GraphQL)
|
||||
// to this shape so the per-reviewer dedup rule is only spelled out once.
|
||||
type reviewSample struct {
|
||||
author string
|
||||
state string
|
||||
at time.Time
|
||||
}
|
||||
|
||||
// latestReviewStateByAuthor reduces a list of reviews to "one entry per
|
||||
// reviewer, latest binding state wins". COMMENTED and PENDING never
|
||||
// override a prior APPROVED / CHANGES_REQUESTED for the same author so a
|
||||
// reviewer who approved-then-commented still counts as approved.
|
||||
// Anonymous reviewers (deleted users) get a unique synthetic key so each
|
||||
// review still counts independently.
|
||||
func latestReviewStateByAuthor(samples []reviewSample) map[string]reviewSample {
|
||||
byAuthor := make(map[string]reviewSample, len(samples))
|
||||
for _, s := range samples {
|
||||
key := s.author
|
||||
if key == "" {
|
||||
key = "<anon>:" + s.at.UTC().Format(time.RFC3339Nano)
|
||||
}
|
||||
state := strings.ToUpper(s.state)
|
||||
if state != reviewStateApproved && state != reviewStateChangesRequested {
|
||||
if _, ok := byAuthor[key]; ok {
|
||||
continue
|
||||
}
|
||||
}
|
||||
prev, ok := byAuthor[key]
|
||||
if !ok || !s.at.Before(prev.at) {
|
||||
byAuthor[key] = reviewSample{author: s.author, state: state, at: s.at}
|
||||
}
|
||||
}
|
||||
return byAuthor
|
||||
}
|
||||
|
||||
// countApprovedAuthors counts distinct reviewers whose latest binding state is
|
||||
// APPROVED. Shared between the REST and GraphQL count helpers.
|
||||
func countApprovedAuthors(samples []reviewSample) int {
|
||||
count := 0
|
||||
for _, s := range latestReviewStateByAuthor(samples) {
|
||||
if s.state == reviewStateApproved {
|
||||
count++
|
||||
}
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
// reduceReviewSummary collapses the per-author map into a single overall
|
||||
// review state. CHANGES_REQUESTED wins across authors; APPROVED otherwise;
|
||||
// "" when nothing is binding. Shared between REST and GraphQL paths.
|
||||
func reduceReviewSummary(samples []reviewSample) string {
|
||||
hasApproval := false
|
||||
for _, l := range latestReviewStateByAuthor(samples) {
|
||||
switch l.state {
|
||||
case reviewStateChangesRequested:
|
||||
return computedReviewStateChangesRequested
|
||||
case reviewStateApproved:
|
||||
hasApproval = true
|
||||
}
|
||||
}
|
||||
if hasApproval {
|
||||
return computedReviewStateApproved
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// countApprovedReviewers returns the number of distinct authors whose latest
|
||||
// review state is APPROVED. REST-path adapter over the shared helper.
|
||||
func countApprovedReviewers(reviews []PRReview) int {
|
||||
samples := make([]reviewSample, len(reviews))
|
||||
for i, r := range reviews {
|
||||
samples[i] = reviewSample{author: r.Author, state: r.State, at: r.CreatedAt}
|
||||
}
|
||||
return countApprovedAuthors(samples)
|
||||
}
|
||||
|
||||
// countCheckResults returns (total, passing) counts using the same logic as
|
||||
// computeOverallCheckStatus: skipped/neutral and still-running checks are
|
||||
// excluded from the total so the X/Y badge communicates pass rate rather
|
||||
|
||||
@@ -471,3 +471,123 @@ func TestHasChangesRequested(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// --- Reviewer dedup helpers (countApprovedReviewers + reduceReviewSummary) ---
|
||||
|
||||
func TestCountApprovedReviewers(t *testing.T) {
|
||||
t0 := time.Date(2025, 5, 1, 12, 0, 0, 0, time.UTC)
|
||||
mk := func(author, state string, offset time.Duration) PRReview {
|
||||
return PRReview{Author: author, State: state, CreatedAt: t0.Add(offset)}
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
reviews []PRReview
|
||||
want int
|
||||
}{
|
||||
{name: "empty", reviews: []PRReview{}, want: 0},
|
||||
{
|
||||
name: "single approval",
|
||||
reviews: []PRReview{mk("alice", reviewStateApproved, 0)},
|
||||
want: 1,
|
||||
},
|
||||
{
|
||||
name: "dedup same author latest wins",
|
||||
reviews: []PRReview{
|
||||
mk("alice", reviewStateChangesRequested, 0),
|
||||
mk("alice", reviewStateApproved, time.Hour),
|
||||
},
|
||||
want: 1,
|
||||
},
|
||||
{
|
||||
name: "COMMENTED does not replace prior APPROVED",
|
||||
reviews: []PRReview{
|
||||
mk("alice", reviewStateApproved, 0),
|
||||
mk("alice", "COMMENTED", time.Hour),
|
||||
},
|
||||
want: 1,
|
||||
},
|
||||
{
|
||||
name: "PENDING never counts even alone",
|
||||
reviews: []PRReview{mk("alice", "PENDING", 0)},
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "two distinct approvers",
|
||||
reviews: []PRReview{
|
||||
mk("alice", reviewStateApproved, 0),
|
||||
mk("bob", reviewStateApproved, time.Hour),
|
||||
},
|
||||
want: 2,
|
||||
},
|
||||
{
|
||||
name: "approve then change-request reverts",
|
||||
reviews: []PRReview{
|
||||
mk("alice", reviewStateApproved, 0),
|
||||
mk("alice", reviewStateChangesRequested, time.Hour),
|
||||
},
|
||||
want: 0,
|
||||
},
|
||||
{
|
||||
name: "anonymous reviewers counted separately",
|
||||
reviews: []PRReview{
|
||||
mk("", reviewStateApproved, 0),
|
||||
mk("", reviewStateApproved, time.Hour),
|
||||
},
|
||||
want: 2,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := countApprovedReviewers(tc.reviews); got != tc.want {
|
||||
t.Fatalf("countApprovedReviewers = %d, want %d", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestReduceReviewSummary(t *testing.T) {
|
||||
t0 := time.Date(2025, 5, 1, 12, 0, 0, 0, time.UTC)
|
||||
mk := func(author, state string, offset time.Duration) reviewSample {
|
||||
return reviewSample{author: author, state: state, at: t0.Add(offset)}
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
in []reviewSample
|
||||
want string
|
||||
}{
|
||||
{name: "empty", in: nil, want: ""},
|
||||
{
|
||||
name: "single approval",
|
||||
in: []reviewSample{mk("a", reviewStateApproved, 0)},
|
||||
want: computedReviewStateApproved,
|
||||
},
|
||||
{
|
||||
name: "changes requested wins across authors",
|
||||
in: []reviewSample{
|
||||
mk("a", reviewStateApproved, 0),
|
||||
mk("b", reviewStateChangesRequested, time.Hour),
|
||||
},
|
||||
want: computedReviewStateChangesRequested,
|
||||
},
|
||||
{
|
||||
name: "later approval supersedes earlier changes-requested from same author",
|
||||
in: []reviewSample{
|
||||
mk("a", reviewStateChangesRequested, 0),
|
||||
mk("a", reviewStateApproved, time.Hour),
|
||||
},
|
||||
want: computedReviewStateApproved,
|
||||
},
|
||||
{
|
||||
name: "only comments returns empty",
|
||||
in: []reviewSample{mk("a", "COMMENTED", 0)},
|
||||
want: "",
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := reduceReviewSummary(tc.in); got != tc.want {
|
||||
t.Fatalf("reduceReviewSummary = %q, want %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -505,6 +505,65 @@ func (c *GHClient) GetPRStatus(ctx context.Context, owner, repo string, number i
|
||||
return getPRStatus(ctx, c, owner, repo, number)
|
||||
}
|
||||
|
||||
// FetchBranchProtection looks up branch protection via `gh api`. A 404 from
|
||||
// `gh` (no rule) and a 403 (token lacks Administration: Read scope) are both
|
||||
// treated as "no rule we can see" so callers cache the negative result and
|
||||
// don't burn rate-limit quota on every poll. The string match is permissive
|
||||
// across formats so a future `gh` CLI release that tweaks the wording
|
||||
// doesn't silently break the cache.
|
||||
func (c *GHClient) FetchBranchProtection(ctx context.Context, owner, repo, branch string) (BranchProtection, error) {
|
||||
out, err := c.run(ctx, "api",
|
||||
fmt.Sprintf("repos/%s/%s/branches/%s/protection", owner, repo, branch))
|
||||
if err != nil {
|
||||
if isNotFoundErr(err) || isForbiddenErr(err) {
|
||||
return BranchProtection{HasRule: false}, nil
|
||||
}
|
||||
return BranchProtection{}, err
|
||||
}
|
||||
var raw struct {
|
||||
RequiredPullRequestReviews *struct {
|
||||
RequiredApprovingReviewCount int `json:"required_approving_review_count"`
|
||||
} `json:"required_pull_request_reviews"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(out), &raw); err != nil {
|
||||
return BranchProtection{}, fmt.Errorf("decode branch protection: %w", err)
|
||||
}
|
||||
if raw.RequiredPullRequestReviews == nil {
|
||||
return BranchProtection{HasRule: true}, nil
|
||||
}
|
||||
return BranchProtection{
|
||||
HasRule: true,
|
||||
RequiredApprovingReviewCount: raw.RequiredPullRequestReviews.RequiredApprovingReviewCount,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// isNotFoundErr matches the formats the `gh` CLI uses to report a 404 across
|
||||
// recent versions: "HTTP 404", "HTTP 404: ...", and "404 Not Found".
|
||||
// Restrictive enough that an unrelated 404 substring (e.g. inside a JSON
|
||||
// payload accidentally rendered into stderr) is unlikely to false-match.
|
||||
func isNotFoundErr(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
s := err.Error()
|
||||
return strings.Contains(s, "HTTP 404") ||
|
||||
strings.Contains(s, "404 Not Found") ||
|
||||
strings.Contains(s, "status: 404")
|
||||
}
|
||||
|
||||
// isForbiddenErr matches the formats the `gh` CLI uses to report a 403, used
|
||||
// by the branch-protection lookup to silently fall back to "no rule" when
|
||||
// the token lacks Administration: Read scope.
|
||||
func isForbiddenErr(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
s := err.Error()
|
||||
return strings.Contains(s, "HTTP 403") ||
|
||||
strings.Contains(s, "403 Forbidden") ||
|
||||
strings.Contains(s, "status: 403")
|
||||
}
|
||||
|
||||
func (c *GHClient) ListPRFiles(ctx context.Context, owner, repo string, number int) ([]PRFile, error) {
|
||||
out, err := c.run(ctx, "api",
|
||||
fmt.Sprintf("repos/%s/%s/pulls/%d/files", owner, repo, number),
|
||||
|
||||
@@ -1,10 +1,57 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestIsNotFoundErr(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil", nil, false},
|
||||
{"HTTP 404", errors.New("gh api: HTTP 404"), true},
|
||||
{"HTTP 404 with suffix", errors.New("gh: HTTP 404: Not Found (404)"), true},
|
||||
{"404 Not Found", errors.New("404 Not Found"), true},
|
||||
{"status: 404", errors.New("request failed (status: 404)"), true},
|
||||
{"unrelated 500", errors.New("HTTP 500: server error"), false},
|
||||
{"unrelated text", errors.New("connection refused"), false},
|
||||
{"403 not 404", errors.New("HTTP 403: Forbidden"), false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := isNotFoundErr(tc.err); got != tc.want {
|
||||
t.Fatalf("isNotFoundErr(%v) = %v, want %v", tc.err, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsForbiddenErr(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil", nil, false},
|
||||
{"HTTP 403", errors.New("gh api: HTTP 403"), true},
|
||||
{"403 Forbidden", errors.New("403 Forbidden"), true},
|
||||
{"status: 403", errors.New("request failed (status: 403)"), true},
|
||||
{"404 not 403", errors.New("HTTP 404"), false},
|
||||
{"500 not 403", errors.New("HTTP 500"), false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := isForbiddenErr(tc.err); got != tc.want {
|
||||
t.Fatalf("isForbiddenErr(%v) = %v, want %v", tc.err, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseTimePtr(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -94,6 +94,12 @@ type batchedPRResult struct {
|
||||
ReviewRequests struct {
|
||||
TotalCount int `json:"totalCount"`
|
||||
} `json:"reviewRequests"`
|
||||
ReviewThreads struct {
|
||||
TotalCount int `json:"totalCount"`
|
||||
Nodes []struct {
|
||||
IsResolved bool `json:"isResolved"`
|
||||
} `json:"nodes"`
|
||||
} `json:"reviewThreads"`
|
||||
Commits struct {
|
||||
Nodes []struct {
|
||||
Commit struct {
|
||||
@@ -184,6 +190,7 @@ func prFieldsBlock() string {
|
||||
`author { login } createdAt updatedAt mergedAt closedAt ` +
|
||||
`reviews(last: 100) { nodes { state author { login } submittedAt } } ` +
|
||||
`reviewRequests(first: 0) { totalCount } ` +
|
||||
`reviewThreads(first: 100) { totalCount nodes { isResolved } } ` +
|
||||
`commits(last: 1) { nodes { commit { statusCheckRollup { state } } } }`
|
||||
}
|
||||
|
||||
@@ -236,59 +243,61 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int
|
||||
if len(raw.Commits.Nodes) > 0 && raw.Commits.Nodes[0].Commit.StatusCheckRollup != nil {
|
||||
checksState = strings.ToLower(raw.Commits.Nodes[0].Commit.StatusCheckRollup.State)
|
||||
}
|
||||
// Count unresolved threads from the fetched nodes. When the page is
|
||||
// capped (totalCount > nodes), fall back to totalCount for the
|
||||
// resolved-vs-unresolved estimate so the popover doesn't undercount on
|
||||
// busy PRs. The fallback is conservative: we have no way to tell from
|
||||
// the truncated page how many of the un-fetched threads are resolved,
|
||||
// so attribute the unseen tail to "unresolved" — the popover's value
|
||||
// is meant to be actionable, not exact, and over-reporting is safer
|
||||
// than silently hiding open feedback.
|
||||
unresolved := 0
|
||||
for _, t := range raw.ReviewThreads.Nodes {
|
||||
if !t.IsResolved {
|
||||
unresolved++
|
||||
}
|
||||
}
|
||||
if total := raw.ReviewThreads.TotalCount; total > len(raw.ReviewThreads.Nodes) {
|
||||
unresolved += total - len(raw.ReviewThreads.Nodes)
|
||||
}
|
||||
return &PRStatus{
|
||||
PR: pr,
|
||||
ReviewState: reviewState,
|
||||
ChecksState: checksState,
|
||||
MergeableState: pr.MergeableState,
|
||||
PR: pr,
|
||||
ReviewState: reviewState,
|
||||
ChecksState: checksState,
|
||||
MergeableState: pr.MergeableState,
|
||||
ReviewCount: countApprovedReviewerNodes(raw.Reviews.Nodes),
|
||||
PendingReviewCount: raw.ReviewRequests.TotalCount,
|
||||
ReviewCountsPopulated: true,
|
||||
UnresolvedReviewThreads: unresolved,
|
||||
UnresolvedReviewThreadsPopulated: true,
|
||||
}
|
||||
}
|
||||
|
||||
// reviewNodesToSamples converts the GraphQL reviewNode shape to the shared
|
||||
// reviewSample slice used by the dedup helpers in client_helpers.go.
|
||||
func reviewNodesToSamples(nodes []reviewNode) []reviewSample {
|
||||
samples := make([]reviewSample, len(nodes))
|
||||
for i, n := range nodes {
|
||||
samples[i] = reviewSample{author: n.Author.Login, state: n.State, at: n.SubmittedAt}
|
||||
}
|
||||
return samples
|
||||
}
|
||||
|
||||
// countApprovedReviewerNodes returns the number of distinct authors whose
|
||||
// latest review state is APPROVED, on the GraphQL reviewNode shape. Thin
|
||||
// adapter over countApprovedAuthors so REST and GraphQL agree on what
|
||||
// "Approved (N)" means.
|
||||
func countApprovedReviewerNodes(nodes []reviewNode) int {
|
||||
return countApprovedAuthors(reviewNodesToSamples(nodes))
|
||||
}
|
||||
|
||||
// summarizeReviewState collapses the review history to a single
|
||||
// "approved"/"changes_requested"/"" value matching the existing
|
||||
// service-side summary. Per-reviewer dedup: each reviewer's most-recent
|
||||
// review wins, so a CHANGES_REQUESTED followed by an APPROVED from the
|
||||
// same author resolves to APPROVED. CHANGES_REQUESTED beats APPROVED across
|
||||
// distinct reviewers.
|
||||
// "approved"/"changes_requested"/"" value. Per-reviewer dedup: each
|
||||
// reviewer's most-recent binding review wins, so a CHANGES_REQUESTED
|
||||
// followed by APPROVED from the same author resolves to APPROVED.
|
||||
// CHANGES_REQUESTED beats APPROVED across distinct reviewers.
|
||||
func summarizeReviewState(nodes []reviewNode) string {
|
||||
type latest struct {
|
||||
state string
|
||||
at time.Time
|
||||
}
|
||||
byAuthor := map[string]latest{}
|
||||
for _, n := range nodes {
|
||||
// Anonymous reviewers (deleted users) get a unique bucket so each
|
||||
// review still counts independently.
|
||||
key := n.Author.Login
|
||||
if key == "" {
|
||||
key = "<anon>:" + n.SubmittedAt.UTC().Format(time.RFC3339Nano)
|
||||
}
|
||||
state := strings.ToUpper(n.State)
|
||||
// COMMENTED and PENDING are non-binding states and shouldn't replace
|
||||
// a prior APPROVED / CHANGES_REQUESTED for the same reviewer.
|
||||
if state != reviewStateApproved && state != reviewStateChangesRequested {
|
||||
if _, ok := byAuthor[key]; ok {
|
||||
continue
|
||||
}
|
||||
}
|
||||
prev, ok := byAuthor[key]
|
||||
if !ok || !n.SubmittedAt.Before(prev.at) {
|
||||
byAuthor[key] = latest{state: state, at: n.SubmittedAt}
|
||||
}
|
||||
}
|
||||
hasApproval := false
|
||||
for _, l := range byAuthor {
|
||||
switch l.state {
|
||||
case reviewStateChangesRequested:
|
||||
return computedReviewStateChangesRequested
|
||||
case reviewStateApproved:
|
||||
hasApproval = true
|
||||
}
|
||||
}
|
||||
if hasApproval {
|
||||
return computedReviewStateApproved
|
||||
}
|
||||
return ""
|
||||
return reduceReviewSummary(reviewNodesToSamples(nodes))
|
||||
}
|
||||
|
||||
// PATClient.ExecuteGraphQL satisfies GraphQLExecutor by POSTing to /graphql.
|
||||
|
||||
@@ -51,6 +51,8 @@ type repoKey struct {
|
||||
type MockClient struct {
|
||||
mu sync.RWMutex
|
||||
user string
|
||||
authenticated bool
|
||||
authError string
|
||||
prs map[prKey]*PR
|
||||
prsByBranch map[branchKey]*PR
|
||||
orgs []GitHubOrg
|
||||
@@ -67,23 +69,26 @@ type MockClient struct {
|
||||
// NewMockClient creates a new MockClient with default values.
|
||||
func NewMockClient() *MockClient {
|
||||
return &MockClient{
|
||||
user: mockDefaultUser,
|
||||
prs: make(map[prKey]*PR),
|
||||
prsByBranch: make(map[branchKey]*PR),
|
||||
repos: make(map[string][]GitHubRepo),
|
||||
branches: make(map[repoKey][]RepoBranch),
|
||||
reviews: make(map[prKey][]PRReview),
|
||||
comments: make(map[prKey][]PRComment),
|
||||
checks: make(map[checkKey][]CheckRun),
|
||||
files: make(map[prKey][]PRFile),
|
||||
commits: make(map[prKey][]PRCommitInfo),
|
||||
user: mockDefaultUser,
|
||||
authenticated: true,
|
||||
prs: make(map[prKey]*PR),
|
||||
prsByBranch: make(map[branchKey]*PR),
|
||||
repos: make(map[string][]GitHubRepo),
|
||||
branches: make(map[repoKey][]RepoBranch),
|
||||
reviews: make(map[prKey][]PRReview),
|
||||
comments: make(map[prKey][]PRComment),
|
||||
checks: make(map[checkKey][]CheckRun),
|
||||
files: make(map[prKey][]PRFile),
|
||||
commits: make(map[prKey][]PRCommitInfo),
|
||||
}
|
||||
}
|
||||
|
||||
// --- Client interface implementation ---
|
||||
|
||||
func (m *MockClient) IsAuthenticated(context.Context) (bool, error) {
|
||||
return true, nil
|
||||
m.mu.RLock()
|
||||
defer m.mu.RUnlock()
|
||||
return m.authenticated, nil
|
||||
}
|
||||
|
||||
func (m *MockClient) GetAuthenticatedUser(context.Context) (string, error) {
|
||||
@@ -270,6 +275,16 @@ func (m *MockClient) SetUser(username string) {
|
||||
m.user = username
|
||||
}
|
||||
|
||||
// SetAuthHealth toggles the authenticated state for the e2e auth-lost test.
|
||||
// When authenticated=false the popover renders the "Reconnect GitHub" branch.
|
||||
// authError is an opaque message preserved for diagnostics.
|
||||
func (m *MockClient) SetAuthHealth(authenticated bool, authError string) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
m.authenticated = authenticated
|
||||
m.authError = authError
|
||||
}
|
||||
|
||||
// AddPR adds a PR to the mock data store, indexed by owner/repo/number and branch.
|
||||
func (m *MockClient) AddPR(pr *PR) {
|
||||
m.mu.Lock()
|
||||
@@ -327,6 +342,34 @@ func (m *MockClient) AddCheckRuns(owner, repo, ref string, checks []CheckRun) {
|
||||
m.checks[k] = append(m.checks[k], checks...)
|
||||
}
|
||||
|
||||
// ReplaceCheckRuns overwrites the check runs for a ref. Used by the e2e mock
|
||||
// controller so a follow-up seed call yields deterministic state.
|
||||
func (m *MockClient) ReplaceCheckRuns(owner, repo, ref string, checks []CheckRun) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
cp := make([]CheckRun, len(checks))
|
||||
copy(cp, checks)
|
||||
m.checks[checkKey{owner, repo, ref}] = cp
|
||||
}
|
||||
|
||||
// ReplaceReviews overwrites the reviews for a PR.
|
||||
func (m *MockClient) ReplaceReviews(owner, repo string, number int, reviews []PRReview) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
cp := make([]PRReview, len(reviews))
|
||||
copy(cp, reviews)
|
||||
m.reviews[prKey{owner, repo, number}] = cp
|
||||
}
|
||||
|
||||
// ReplaceComments overwrites the comments for a PR.
|
||||
func (m *MockClient) ReplaceComments(owner, repo string, number int, comments []PRComment) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
cp := make([]PRComment, len(comments))
|
||||
copy(cp, comments)
|
||||
m.comments[prKey{owner, repo, number}] = cp
|
||||
}
|
||||
|
||||
// AddPRFiles appends files for a PR.
|
||||
func (m *MockClient) AddPRFiles(owner, repo string, number int, files []PRFile) {
|
||||
m.mu.Lock()
|
||||
@@ -348,6 +391,8 @@ func (m *MockClient) Reset() {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
m.user = mockDefaultUser
|
||||
m.authenticated = true
|
||||
m.authError = ""
|
||||
m.prs = make(map[prKey]*PR)
|
||||
m.prsByBranch = make(map[branchKey]*PR)
|
||||
m.orgs = nil
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
@@ -41,6 +43,8 @@ func (c *MockController) RegisterRoutes(router *gin.Engine) {
|
||||
api.POST("/commits", c.addPRCommits)
|
||||
api.POST("/branches", c.addBranches)
|
||||
api.POST("/task-prs", c.associateTaskPR)
|
||||
api.POST("/pr-feedback", c.seedPRFeedback)
|
||||
api.PUT("/auth-health", c.setAuthHealth)
|
||||
api.DELETE("/reset", c.reset)
|
||||
}
|
||||
|
||||
@@ -188,25 +192,39 @@ func (c *MockController) addBranches(ctx *gin.Context) {
|
||||
ctx.JSON(http.StatusOK, gin.H{"added": len(req.Branches)})
|
||||
}
|
||||
|
||||
// associateTaskPR directly creates a github_task_prs record for E2E testing.
|
||||
// associateTaskPRRequest is the JSON body for the mock-controller's
|
||||
// associateTaskPR endpoint. Pointer fields are optional — leave them nil
|
||||
// to skip the corresponding TaskPR column update.
|
||||
type associateTaskPRRequest struct {
|
||||
TaskID string `json:"task_id"`
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
PRNumber int `json:"pr_number"`
|
||||
PRURL string `json:"pr_url"`
|
||||
PRTitle string `json:"pr_title"`
|
||||
HeadBranch string `json:"head_branch"`
|
||||
BaseBranch string `json:"base_branch"`
|
||||
AuthorLogin string `json:"author_login"`
|
||||
State string `json:"state"`
|
||||
ReviewState string `json:"review_state"`
|
||||
ChecksState string `json:"checks_state"`
|
||||
MergeableState string `json:"mergeable_state"`
|
||||
Additions int `json:"additions"`
|
||||
Deletions int `json:"deletions"`
|
||||
ReviewCount *int `json:"review_count,omitempty"`
|
||||
PendingReviewCount *int `json:"pending_review_count,omitempty"`
|
||||
RequiredReviews *int `json:"required_reviews,omitempty"`
|
||||
ChecksTotal *int `json:"checks_total,omitempty"`
|
||||
ChecksPassing *int `json:"checks_passing,omitempty"`
|
||||
UnresolvedReviewThreads *int `json:"unresolved_review_threads,omitempty"`
|
||||
}
|
||||
|
||||
// associateTaskPR directly creates (or replaces) a github_task_prs record for
|
||||
// E2E testing. Repeating the call with the same (task_id, owner, repo,
|
||||
// pr_number) updates the row so tests can drive the live-refresh
|
||||
// scenario by re-POSTing with new aggregate counts.
|
||||
func (c *MockController) associateTaskPR(ctx *gin.Context) {
|
||||
var req struct {
|
||||
TaskID string `json:"task_id"`
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
PRNumber int `json:"pr_number"`
|
||||
PRURL string `json:"pr_url"`
|
||||
PRTitle string `json:"pr_title"`
|
||||
HeadBranch string `json:"head_branch"`
|
||||
BaseBranch string `json:"base_branch"`
|
||||
AuthorLogin string `json:"author_login"`
|
||||
State string `json:"state"`
|
||||
ReviewState string `json:"review_state"`
|
||||
ChecksState string `json:"checks_state"`
|
||||
MergeableState string `json:"mergeable_state"`
|
||||
Additions int `json:"additions"`
|
||||
Deletions int `json:"deletions"`
|
||||
}
|
||||
var req associateTaskPRRequest
|
||||
if err := ctx.ShouldBindJSON(&req); err != nil {
|
||||
ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"})
|
||||
return
|
||||
@@ -215,6 +233,26 @@ func (c *MockController) associateTaskPR(ctx *gin.Context) {
|
||||
req.State = defaultPRState
|
||||
}
|
||||
now := time.Now().UTC()
|
||||
tp := buildTaskPRFromRequest(&req, now)
|
||||
if err := c.store.ReplaceTaskPR(ctx.Request.Context(), tp); err != nil {
|
||||
ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
c.ensureMockPRForRequest(ctx.Request.Context(), &req, now)
|
||||
// Publish the event so the frontend Zustand store picks up the new PR
|
||||
// without requiring a page reload — mirrors real AssociatePRWithTask.
|
||||
if c.eventBus != nil {
|
||||
event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", tp)
|
||||
if err := c.eventBus.Publish(ctx.Request.Context(), events.GitHubTaskPRUpdated, event); err != nil {
|
||||
c.logger.Debug("mock: failed to publish task PR updated event", zap.Error(err))
|
||||
}
|
||||
}
|
||||
ctx.JSON(http.StatusCreated, tp)
|
||||
}
|
||||
|
||||
// buildTaskPRFromRequest copies the required fields from the JSON body into
|
||||
// a TaskPR and applies the optional pointer fields when present.
|
||||
func buildTaskPRFromRequest(req *associateTaskPRRequest, now time.Time) *TaskPR {
|
||||
tp := &TaskPR{
|
||||
TaskID: req.TaskID,
|
||||
Owner: req.Owner,
|
||||
@@ -233,19 +271,122 @@ func (c *MockController) associateTaskPR(ctx *gin.Context) {
|
||||
Deletions: req.Deletions,
|
||||
CreatedAt: now,
|
||||
}
|
||||
if err := c.store.CreateTaskPR(ctx.Request.Context(), tp); err != nil {
|
||||
ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
if req.ReviewCount != nil {
|
||||
tp.ReviewCount = *req.ReviewCount
|
||||
}
|
||||
if req.PendingReviewCount != nil {
|
||||
tp.PendingReviewCount = *req.PendingReviewCount
|
||||
}
|
||||
if req.RequiredReviews != nil {
|
||||
v := *req.RequiredReviews
|
||||
tp.RequiredReviews = &v
|
||||
}
|
||||
if req.ChecksTotal != nil {
|
||||
tp.ChecksTotal = *req.ChecksTotal
|
||||
}
|
||||
if req.ChecksPassing != nil {
|
||||
tp.ChecksPassing = *req.ChecksPassing
|
||||
}
|
||||
if req.UnresolvedReviewThreads != nil {
|
||||
tp.UnresolvedReviewThreads = *req.UnresolvedReviewThreads
|
||||
}
|
||||
return tp
|
||||
}
|
||||
|
||||
// ensureMockPRForRequest seeds a synthetic PR in the mock client so the lazy
|
||||
// PRFeedback path can resolve check runs by HeadSHA. Skipped when the PR was
|
||||
// already seeded explicitly via addPRs (tests that pin a head_sha for
|
||||
// matching check_runs must not have it overwritten).
|
||||
func (c *MockController) ensureMockPRForRequest(ctx context.Context, req *associateTaskPRRequest, now time.Time) {
|
||||
if existing, _ := c.mock.GetPR(ctx, req.Owner, req.Repo, req.PRNumber); existing != nil {
|
||||
return
|
||||
}
|
||||
// Publish the event so the frontend Zustand store picks up the new PR
|
||||
// without requiring a page reload — mirrors real AssociatePRWithTask.
|
||||
if c.eventBus != nil {
|
||||
event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", tp)
|
||||
if err := c.eventBus.Publish(ctx.Request.Context(), events.GitHubTaskPRUpdated, event); err != nil {
|
||||
c.logger.Debug("mock: failed to publish task PR updated event", zap.Error(err))
|
||||
}
|
||||
c.mock.AddPR(&PR{
|
||||
Number: req.PRNumber,
|
||||
Title: req.PRTitle,
|
||||
URL: req.PRURL,
|
||||
HTMLURL: req.PRURL,
|
||||
State: req.State,
|
||||
HeadBranch: req.HeadBranch,
|
||||
HeadSHA: mockHeadSHA(req.Owner, req.Repo, req.PRNumber),
|
||||
BaseBranch: req.BaseBranch,
|
||||
AuthorLogin: req.AuthorLogin,
|
||||
RepoOwner: req.Owner,
|
||||
RepoName: req.Repo,
|
||||
Additions: req.Additions,
|
||||
Deletions: req.Deletions,
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
})
|
||||
}
|
||||
|
||||
// seedPRFeedback registers checks (and optionally reviews / comments) for a
|
||||
// specific PR. The PR must already exist in the mock client (associateTaskPR
|
||||
// synthesizes one). The check ref is the PR's HeadSHA so getPRFeedback
|
||||
// resolves them via ListCheckRuns.
|
||||
func (c *MockController) seedPRFeedback(ctx *gin.Context) {
|
||||
var req struct {
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
PRNumber int `json:"pr_number"`
|
||||
Checks []CheckRun `json:"checks"`
|
||||
Reviews []PRReview `json:"reviews"`
|
||||
Comments []PRComment `json:"comments"`
|
||||
}
|
||||
ctx.JSON(http.StatusCreated, tp)
|
||||
if err := ctx.ShouldBindJSON(&req); err != nil {
|
||||
ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"})
|
||||
return
|
||||
}
|
||||
if req.Owner == "" || req.Repo == "" || req.PRNumber == 0 {
|
||||
ctx.JSON(http.StatusBadRequest, gin.H{"error": "owner, repo, pr_number are required"})
|
||||
return
|
||||
}
|
||||
headSHA := mockHeadSHA(req.Owner, req.Repo, req.PRNumber)
|
||||
// If associateTaskPR ran first, an underlying PR row exists with this
|
||||
// HeadSHA. Otherwise synthesize a minimal one so getPRFeedback's GetPR
|
||||
// call doesn't fail.
|
||||
if pr, err := c.mock.GetPR(ctx.Request.Context(), req.Owner, req.Repo, req.PRNumber); err != nil || pr == nil {
|
||||
c.mock.AddPR(&PR{
|
||||
Number: req.PRNumber,
|
||||
RepoOwner: req.Owner,
|
||||
RepoName: req.Repo,
|
||||
HeadSHA: headSHA,
|
||||
CreatedAt: time.Now().UTC(),
|
||||
UpdatedAt: time.Now().UTC(),
|
||||
})
|
||||
}
|
||||
// Replace prior seeded checks/reviews/comments so a follow-up call gives
|
||||
// deterministic state; helpful for tests that drive a "then a check
|
||||
// finishes" transition.
|
||||
c.mock.ReplaceCheckRuns(req.Owner, req.Repo, headSHA, req.Checks)
|
||||
c.mock.ReplaceReviews(req.Owner, req.Repo, req.PRNumber, req.Reviews)
|
||||
c.mock.ReplaceComments(req.Owner, req.Repo, req.PRNumber, req.Comments)
|
||||
ctx.JSON(http.StatusOK, gin.H{
|
||||
"checks": len(req.Checks),
|
||||
"reviews": len(req.Reviews),
|
||||
"comments": len(req.Comments),
|
||||
})
|
||||
}
|
||||
|
||||
// setAuthHealth toggles the mock client's authenticated state so e2e tests
|
||||
// can drive the "Reconnect GitHub" branch in the popover.
|
||||
func (c *MockController) setAuthHealth(ctx *gin.Context) {
|
||||
var req struct {
|
||||
Authenticated bool `json:"authenticated"`
|
||||
Error string `json:"error"`
|
||||
}
|
||||
if err := ctx.ShouldBindJSON(&req); err != nil {
|
||||
ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"})
|
||||
return
|
||||
}
|
||||
c.mock.SetAuthHealth(req.Authenticated, req.Error)
|
||||
ctx.JSON(http.StatusOK, gin.H{"authenticated": req.Authenticated})
|
||||
}
|
||||
|
||||
// mockHeadSHA produces a deterministic synthetic head SHA for a (owner, repo,
|
||||
// pr_number) tuple so test seed calls can compute the same key.
|
||||
func mockHeadSHA(owner, repo string, prNumber int) string {
|
||||
return fmt.Sprintf("mocksha-%s-%s-%d", owner, repo, prNumber)
|
||||
}
|
||||
|
||||
func (c *MockController) reset(ctx *gin.Context) {
|
||||
|
||||
@@ -92,8 +92,29 @@ type PRStatus struct {
|
||||
MergeableState string `json:"mergeable_state"` // "clean", "blocked", "behind", "dirty", "has_hooks", "unstable", "draft", "unknown", ""
|
||||
ReviewCount int `json:"review_count"`
|
||||
PendingReviewCount int `json:"pending_review_count"`
|
||||
RequiredReviews *int `json:"required_reviews,omitempty"` // nil when no branch protection rule found
|
||||
ChecksTotal int `json:"checks_total"`
|
||||
ChecksPassing int `json:"checks_passing"`
|
||||
// ChecksPopulated reports whether the sync path actually computed
|
||||
// ChecksTotal / ChecksPassing. The batched GraphQL poller doesn't (it
|
||||
// only carries the rollup state), so SyncTaskPR uses this flag to
|
||||
// decide whether to overwrite the persisted counts. A value of true
|
||||
// with both counts at 0 is a real "no checks" answer; a value of false
|
||||
// means "I didn't look, keep what's there."
|
||||
ChecksPopulated bool `json:"checks_populated,omitempty"`
|
||||
UnresolvedReviewThreads int `json:"unresolved_review_threads"`
|
||||
// UnresolvedReviewThreadsPopulated mirrors ChecksPopulated for the
|
||||
// review-threads field. The REST path (getPRStatus) doesn't fetch
|
||||
// review threads, so it leaves the field at zero; SyncTaskPR uses
|
||||
// this flag to avoid clobbering a non-zero value set by the GraphQL
|
||||
// path during a subsequent REST sync.
|
||||
UnresolvedReviewThreadsPopulated bool `json:"unresolved_review_threads_populated,omitempty"`
|
||||
// ReviewCountsPopulated covers ReviewCount + PendingReviewCount. Both
|
||||
// REST and GraphQL paths compute these now, but historically only the
|
||||
// REST path did, so without the guard a GraphQL poll would clobber
|
||||
// the REST value back to 0 (the popover's "Approved (1)" turning
|
||||
// into "Approved (0)" until a new REST call landed).
|
||||
ReviewCountsPopulated bool `json:"review_counts_populated,omitempty"`
|
||||
}
|
||||
|
||||
// PRSearchPage is a paginated slice of PR search results, with the total
|
||||
@@ -137,31 +158,37 @@ type PRWatch struct {
|
||||
// repository this PR belongs to (multi-repo tasks can have one PR per repo).
|
||||
// Empty for legacy rows persisted before multi-repo support.
|
||||
type TaskPR struct {
|
||||
ID string `json:"id" db:"id"`
|
||||
TaskID string `json:"task_id" db:"task_id"`
|
||||
RepositoryID string `json:"repository_id,omitempty" db:"repository_id"`
|
||||
Owner string `json:"owner" db:"owner"`
|
||||
Repo string `json:"repo" db:"repo"`
|
||||
PRNumber int `json:"pr_number" db:"pr_number"`
|
||||
PRURL string `json:"pr_url" db:"pr_url"`
|
||||
PRTitle string `json:"pr_title" db:"pr_title"`
|
||||
HeadBranch string `json:"head_branch" db:"head_branch"`
|
||||
BaseBranch string `json:"base_branch" db:"base_branch"`
|
||||
AuthorLogin string `json:"author_login" db:"author_login"`
|
||||
State string `json:"state" db:"state"` // open, closed, merged
|
||||
ReviewState string `json:"review_state" db:"review_state"` // approved, changes_requested, pending, ""
|
||||
ChecksState string `json:"checks_state" db:"checks_state"` // success, failure, pending, ""
|
||||
MergeableState string `json:"mergeable_state" db:"mergeable_state"` // clean, blocked, behind, dirty, has_hooks, unstable, draft, unknown, ""
|
||||
ReviewCount int `json:"review_count" db:"review_count"`
|
||||
PendingReviewCount int `json:"pending_review_count" db:"pending_review_count"`
|
||||
CommentCount int `json:"comment_count" db:"comment_count"`
|
||||
Additions int `json:"additions" db:"additions"`
|
||||
Deletions int `json:"deletions" db:"deletions"`
|
||||
CreatedAt time.Time `json:"created_at" db:"created_at"`
|
||||
MergedAt *time.Time `json:"merged_at,omitempty" db:"merged_at"`
|
||||
ClosedAt *time.Time `json:"closed_at,omitempty" db:"closed_at"`
|
||||
LastSyncedAt *time.Time `json:"last_synced_at,omitempty" db:"last_synced_at"`
|
||||
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
|
||||
ID string `json:"id" db:"id"`
|
||||
TaskID string `json:"task_id" db:"task_id"`
|
||||
RepositoryID string `json:"repository_id,omitempty" db:"repository_id"`
|
||||
Owner string `json:"owner" db:"owner"`
|
||||
Repo string `json:"repo" db:"repo"`
|
||||
PRNumber int `json:"pr_number" db:"pr_number"`
|
||||
PRURL string `json:"pr_url" db:"pr_url"`
|
||||
PRTitle string `json:"pr_title" db:"pr_title"`
|
||||
HeadBranch string `json:"head_branch" db:"head_branch"`
|
||||
BaseBranch string `json:"base_branch" db:"base_branch"`
|
||||
AuthorLogin string `json:"author_login" db:"author_login"`
|
||||
State string `json:"state" db:"state"` // open, closed, merged
|
||||
ReviewState string `json:"review_state" db:"review_state"` // approved, changes_requested, pending, ""
|
||||
ChecksState string `json:"checks_state" db:"checks_state"` // success, failure, pending, ""
|
||||
MergeableState string `json:"mergeable_state" db:"mergeable_state"` // clean, blocked, behind, dirty, has_hooks, unstable, draft, unknown, ""
|
||||
ReviewCount int `json:"review_count" db:"review_count"`
|
||||
PendingReviewCount int `json:"pending_review_count" db:"pending_review_count"`
|
||||
// RequiredReviews is the branch protection's required_approving_review_count.
|
||||
// Nil when no protection rule exists or the token lacks scope to read it.
|
||||
RequiredReviews *int `json:"required_reviews,omitempty" db:"required_reviews"`
|
||||
CommentCount int `json:"comment_count" db:"comment_count"`
|
||||
UnresolvedReviewThreads int `json:"unresolved_review_threads" db:"unresolved_review_threads"`
|
||||
ChecksTotal int `json:"checks_total" db:"checks_total"`
|
||||
ChecksPassing int `json:"checks_passing" db:"checks_passing"`
|
||||
Additions int `json:"additions" db:"additions"`
|
||||
Deletions int `json:"deletions" db:"deletions"`
|
||||
CreatedAt time.Time `json:"created_at" db:"created_at"`
|
||||
MergedAt *time.Time `json:"merged_at,omitempty" db:"merged_at"`
|
||||
ClosedAt *time.Time `json:"closed_at,omitempty" db:"closed_at"`
|
||||
LastSyncedAt *time.Time `json:"last_synced_at,omitempty" db:"last_synced_at"`
|
||||
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
|
||||
}
|
||||
|
||||
// RepoFilter identifies a GitHub repository for review watch filtering.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -551,6 +552,37 @@ func parseNextLink(header string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// FetchBranchProtection looks up the branch protection rule on a repo's base
|
||||
// branch via the REST API. A 404 is treated as "no rule configured" and a 403
|
||||
// is treated as "no rule we can see" (token lacks Administration: Read scope).
|
||||
// Both return HasRule=false with no error so the cache stores the negative
|
||||
// result and we don't burn rate-limit quota retrying every poll cycle.
|
||||
// Other errors propagate so callers can decide whether to retry.
|
||||
func (c *PATClient) FetchBranchProtection(ctx context.Context, owner, repo, branch string) (BranchProtection, error) {
|
||||
endpoint := fmt.Sprintf("/repos/%s/%s/branches/%s/protection", owner, repo, branch)
|
||||
var raw struct {
|
||||
RequiredPullRequestReviews *struct {
|
||||
RequiredApprovingReviewCount int `json:"required_approving_review_count"`
|
||||
} `json:"required_pull_request_reviews"`
|
||||
}
|
||||
if err := c.get(ctx, endpoint, &raw); err != nil {
|
||||
var apiErr *GitHubAPIError
|
||||
if errors.As(err, &apiErr) {
|
||||
if apiErr.StatusCode == http.StatusNotFound || apiErr.StatusCode == http.StatusForbidden {
|
||||
return BranchProtection{HasRule: false}, nil
|
||||
}
|
||||
}
|
||||
return BranchProtection{}, err
|
||||
}
|
||||
if raw.RequiredPullRequestReviews == nil {
|
||||
return BranchProtection{HasRule: true}, nil
|
||||
}
|
||||
return BranchProtection{
|
||||
HasRule: true,
|
||||
RequiredApprovingReviewCount: raw.RequiredPullRequestReviews.RequiredApprovingReviewCount,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// patPR is the JSON shape from the GitHub REST API for PRs.
|
||||
type patPR struct {
|
||||
Number int `json:"number"`
|
||||
|
||||
@@ -297,6 +297,80 @@ func TestPATClient_MarksExhaustedFromRateLimitBody(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPATClient_FetchBranchProtection(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
status int
|
||||
body string
|
||||
wantHasRule bool
|
||||
wantRequired int
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "200 with required reviews",
|
||||
status: http.StatusOK,
|
||||
body: `{"required_pull_request_reviews":{"required_approving_review_count":2}}`,
|
||||
wantHasRule: true,
|
||||
wantRequired: 2,
|
||||
},
|
||||
{
|
||||
name: "200 with rule but no required reviews block",
|
||||
status: http.StatusOK,
|
||||
body: `{"required_pull_request_reviews":null}`,
|
||||
wantHasRule: true,
|
||||
},
|
||||
{
|
||||
name: "404 maps to no rule",
|
||||
status: http.StatusNotFound,
|
||||
body: `{"message":"Branch not protected"}`,
|
||||
wantHasRule: false,
|
||||
},
|
||||
{
|
||||
name: "403 (no admin scope) maps to no rule",
|
||||
status: http.StatusForbidden,
|
||||
body: `{"message":"Resource not accessible by integration"}`,
|
||||
wantHasRule: false,
|
||||
},
|
||||
{
|
||||
name: "500 propagates as error",
|
||||
status: http.StatusInternalServerError,
|
||||
body: `{"message":"server error"}`,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
wantPath := "/repos/o/r/branches/main/protection"
|
||||
if r.URL.Path != wantPath {
|
||||
t.Fatalf("path = %q, want %q", r.URL.Path, wantPath)
|
||||
}
|
||||
w.WriteHeader(tc.status)
|
||||
_, _ = w.Write([]byte(tc.body))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
c := newPATClientPointingAt(t, srv.URL)
|
||||
bp, err := c.FetchBranchProtection(context.Background(), "o", "r", "main")
|
||||
if tc.wantErr {
|
||||
if err == nil {
|
||||
t.Fatal("want error, got nil")
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if bp.HasRule != tc.wantHasRule {
|
||||
t.Fatalf("HasRule = %v, want %v", bp.HasRule, tc.wantHasRule)
|
||||
}
|
||||
if bp.RequiredApprovingReviewCount != tc.wantRequired {
|
||||
t.Fatalf("RequiredApprovingReviewCount = %d, want %d",
|
||||
bp.RequiredApprovingReviewCount, tc.wantRequired)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// newPATClientPointingAt builds a PATClient whose underlying HTTP client
|
||||
// reroutes any github API URL to the given test server.
|
||||
func newPATClientPointingAt(t *testing.T, baseURL string) *PATClient {
|
||||
|
||||
@@ -71,21 +71,23 @@ type Service struct {
|
||||
taskEventSubs []bus.Subscription
|
||||
searchCache *ttlCache
|
||||
prStatusCache *ttlCache
|
||||
protectionCache *branchProtectionCache
|
||||
rateTracker *RateTracker
|
||||
}
|
||||
|
||||
// NewService creates a new GitHub service.
|
||||
func NewService(client Client, authMethod string, secrets SecretProvider, store *Store, eventBus bus.EventBus, log *logger.Logger) *Service {
|
||||
return &Service{
|
||||
client: client,
|
||||
authMethod: authMethod,
|
||||
secrets: secrets,
|
||||
store: store,
|
||||
eventBus: eventBus,
|
||||
logger: log,
|
||||
searchCache: newTTLCache(),
|
||||
prStatusCache: newTTLCache(),
|
||||
rateTracker: NewRateTracker(eventBus, log),
|
||||
client: client,
|
||||
authMethod: authMethod,
|
||||
secrets: secrets,
|
||||
store: store,
|
||||
eventBus: eventBus,
|
||||
logger: log,
|
||||
searchCache: newTTLCache(),
|
||||
prStatusCache: newTTLCache(),
|
||||
protectionCache: newBranchProtectionCache(),
|
||||
rateTracker: NewRateTracker(eventBus, log),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -753,6 +755,51 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
|
||||
return err
|
||||
}
|
||||
|
||||
// Some sync paths (notably the batched GraphQL poller) don't populate
|
||||
// ChecksTotal / ChecksPassing — they only carry the rollup state. The
|
||||
// caller sets status.ChecksPopulated=true when it actually counted
|
||||
// checks; otherwise we preserve the persisted values so the popover
|
||||
// doesn't flap to "0/0" between a rich REST sync and a lightweight
|
||||
// GraphQL one. When the populated counter says 0/0 it really is 0/0
|
||||
// (e.g. all workflows were removed from the PR), so we honor it.
|
||||
nextChecksTotal, nextChecksPassing := tp.ChecksTotal, tp.ChecksPassing
|
||||
if status.ChecksPopulated {
|
||||
nextChecksTotal = status.ChecksTotal
|
||||
nextChecksPassing = status.ChecksPassing
|
||||
}
|
||||
// Same Populated/preserve dance for unresolved review threads — the
|
||||
// REST path doesn't fetch them, so blindly writing status.UnresolvedReviewThreads
|
||||
// would clobber the non-zero value set by the GraphQL path on every poll.
|
||||
nextUnresolved := tp.UnresolvedReviewThreads
|
||||
if status.UnresolvedReviewThreadsPopulated {
|
||||
nextUnresolved = status.UnresolvedReviewThreads
|
||||
}
|
||||
// Review counts: only overwrite when the caller actually computed them.
|
||||
// Both REST and GraphQL paths now populate these, but a partial sync
|
||||
// path that doesn't would otherwise reset the popover's "Approved (N)"
|
||||
// to zero.
|
||||
nextReviewCount, nextPendingReviewCount := tp.ReviewCount, tp.PendingReviewCount
|
||||
if status.ReviewCountsPopulated {
|
||||
nextReviewCount = status.ReviewCount
|
||||
nextPendingReviewCount = status.PendingReviewCount
|
||||
}
|
||||
// PRs can be retargeted to a different base branch; pick up the new
|
||||
// branch from status.PR before resolving branch-protection so we don't
|
||||
// indefinitely surface the wrong rule.
|
||||
nextBaseBranch := tp.BaseBranch
|
||||
if status.PR.BaseBranch != "" && status.PR.BaseBranch != tp.BaseBranch {
|
||||
nextBaseBranch = status.PR.BaseBranch
|
||||
}
|
||||
// RequiredReviews comes from branch protection, fetched separately.
|
||||
// Treat nil as "unknown — don't touch"; only write when the caller has it
|
||||
// or our cache resolves the rule for this base branch.
|
||||
nextRequiredReviews := tp.RequiredReviews
|
||||
if status.RequiredReviews != nil {
|
||||
nextRequiredReviews = status.RequiredReviews
|
||||
} else if fetched := s.fetchRequiredReviews(ctx, tp.Owner, tp.Repo, nextBaseBranch); fetched != nil {
|
||||
nextRequiredReviews = fetched
|
||||
}
|
||||
|
||||
changed := tp.State != status.PR.State ||
|
||||
tp.PRTitle != status.PR.Title ||
|
||||
tp.Additions != status.PR.Additions ||
|
||||
@@ -760,8 +807,13 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
|
||||
tp.ReviewState != status.ReviewState ||
|
||||
tp.ChecksState != status.ChecksState ||
|
||||
tp.MergeableState != status.MergeableState ||
|
||||
tp.ReviewCount != status.ReviewCount ||
|
||||
tp.PendingReviewCount != status.PendingReviewCount ||
|
||||
tp.ReviewCount != nextReviewCount ||
|
||||
tp.PendingReviewCount != nextPendingReviewCount ||
|
||||
!intPtrEqual(tp.RequiredReviews, nextRequiredReviews) ||
|
||||
tp.ChecksTotal != nextChecksTotal ||
|
||||
tp.ChecksPassing != nextChecksPassing ||
|
||||
tp.UnresolvedReviewThreads != nextUnresolved ||
|
||||
tp.BaseBranch != nextBaseBranch ||
|
||||
!timeEqual(tp.MergedAt, status.PR.MergedAt) ||
|
||||
!timeEqual(tp.ClosedAt, status.PR.ClosedAt)
|
||||
|
||||
@@ -774,8 +826,13 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu
|
||||
tp.ReviewState = status.ReviewState
|
||||
tp.ChecksState = status.ChecksState
|
||||
tp.MergeableState = status.MergeableState
|
||||
tp.ReviewCount = status.ReviewCount
|
||||
tp.PendingReviewCount = status.PendingReviewCount
|
||||
tp.ReviewCount = nextReviewCount
|
||||
tp.PendingReviewCount = nextPendingReviewCount
|
||||
tp.RequiredReviews = nextRequiredReviews
|
||||
tp.ChecksTotal = nextChecksTotal
|
||||
tp.ChecksPassing = nextChecksPassing
|
||||
tp.UnresolvedReviewThreads = nextUnresolved
|
||||
tp.BaseBranch = nextBaseBranch
|
||||
// CommentCount is no longer updated from polling -- only refreshed on-demand
|
||||
now := time.Now().UTC()
|
||||
tp.LastSyncedAt = &now
|
||||
@@ -804,6 +861,17 @@ func timeEqual(a, b *time.Time) bool {
|
||||
return a.Equal(*b)
|
||||
}
|
||||
|
||||
// intPtrEqual compares two nullable int pointers for equality.
|
||||
func intPtrEqual(a, b *int) bool {
|
||||
if a == nil && b == nil {
|
||||
return true
|
||||
}
|
||||
if a == nil || b == nil {
|
||||
return false
|
||||
}
|
||||
return *a == *b
|
||||
}
|
||||
|
||||
// --- PR info and feedback (live) ---
|
||||
|
||||
// GetPR fetches basic PR details from GitHub.
|
||||
|
||||
@@ -910,6 +910,253 @@ func TestSyncTaskPR_PublishesEventOnMergeableStateChange(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_ChecksPopulated_PreservesOnLightweightSync verifies the
|
||||
// "preserve when batch sync didn't populate" path: a PRStatus with
|
||||
// ChecksPopulated=false must NOT clobber the persisted ChecksTotal /
|
||||
// ChecksPassing, even when status carries 0/0.
|
||||
func TestSyncTaskPR_ChecksPopulated_PreservesOnLightweightSync(t *testing.T) {
|
||||
svc, store, _ := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1",
|
||||
Owner: "owner",
|
||||
Repo: "repo",
|
||||
PRNumber: 1,
|
||||
PRURL: "https://github.com/owner/repo/pull/1",
|
||||
HeadBranch: "feat",
|
||||
BaseBranch: "main",
|
||||
State: "open",
|
||||
ChecksTotal: 10,
|
||||
ChecksPassing: 7,
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "owner", RepoName: "repo"},
|
||||
ChecksTotal: 0,
|
||||
ChecksPassing: 0,
|
||||
ChecksPopulated: false, // batched GraphQL path: didn't count
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, err := store.GetTaskPR(ctx, "t1")
|
||||
if err != nil {
|
||||
t.Fatalf("get: %v", err)
|
||||
}
|
||||
if stored.ChecksTotal != 10 || stored.ChecksPassing != 7 {
|
||||
t.Fatalf("counts clobbered: total=%d passing=%d (want 10/7)", stored.ChecksTotal, stored.ChecksPassing)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_ChecksPopulated_AcceptsRealZero verifies the design fix:
|
||||
// when the rich sync path explicitly reports 0/0 (workflows removed from
|
||||
// the PR), we DO honor the new value instead of clinging to stale totals.
|
||||
func TestSyncTaskPR_ChecksPopulated_AcceptsRealZero(t *testing.T) {
|
||||
svc, store, _ := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open",
|
||||
ChecksTotal: 5, ChecksPassing: 5,
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
ChecksTotal: 0,
|
||||
ChecksPassing: 0,
|
||||
ChecksPopulated: true, // REST path: counted to zero
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, _ := store.GetTaskPR(ctx, "t1")
|
||||
if stored.ChecksTotal != 0 || stored.ChecksPassing != 0 {
|
||||
t.Fatalf("real-zero not honored: total=%d passing=%d", stored.ChecksTotal, stored.ChecksPassing)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_ReviewCounts_PreservesOnLightweightSync covers the same
|
||||
// preserve-on-not-populated dance for ReviewCount/PendingReviewCount as the
|
||||
// checks/threads variants. A partial sync path (status.ReviewCountsPopulated
|
||||
// = false) must not reset the popover's "Approved (N)" line to zero.
|
||||
func TestSyncTaskPR_ReviewCounts_PreservesOnLightweightSync(t *testing.T) {
|
||||
svc, store, eb := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open",
|
||||
ReviewCount: 2, PendingReviewCount: 1,
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
ReviewCount: 0,
|
||||
PendingReviewCount: 0,
|
||||
ReviewCountsPopulated: false,
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, _ := store.GetTaskPR(ctx, "t1")
|
||||
if stored.ReviewCount != 2 || stored.PendingReviewCount != 1 {
|
||||
t.Fatalf("review counts clobbered: review=%d pending=%d (want 2/1)",
|
||||
stored.ReviewCount, stored.PendingReviewCount)
|
||||
}
|
||||
if got := eb.publishedCount(); got != 0 {
|
||||
t.Fatalf("expected no event when nothing changed, got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_ReviewCounts_AcceptsRealZero is the symmetric case: when the
|
||||
// caller actually counted reviewers and reports zero, we honor it (e.g. all
|
||||
// approvals dismissed) instead of clinging to the previous non-zero values.
|
||||
func TestSyncTaskPR_ReviewCounts_AcceptsRealZero(t *testing.T) {
|
||||
svc, store, _ := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open",
|
||||
ReviewCount: 2, PendingReviewCount: 1,
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
ReviewCount: 0,
|
||||
PendingReviewCount: 0,
|
||||
ReviewCountsPopulated: true,
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, _ := store.GetTaskPR(ctx, "t1")
|
||||
if stored.ReviewCount != 0 || stored.PendingReviewCount != 0 {
|
||||
t.Fatalf("real-zero not honored: review=%d pending=%d",
|
||||
stored.ReviewCount, stored.PendingReviewCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_UnresolvedThreads_PreservesOnLightweightSync verifies the
|
||||
// REST single-PR poller (which doesn't fetch review threads) does NOT
|
||||
// clobber the value set by the GraphQL batched poller. Without the
|
||||
// UnresolvedReviewThreadsPopulated guard, every REST sync would flip the
|
||||
// count to 0 and emit a spurious github.task_pr.updated event.
|
||||
func TestSyncTaskPR_UnresolvedThreads_PreservesOnLightweightSync(t *testing.T) {
|
||||
svc, store, eb := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open",
|
||||
UnresolvedReviewThreads: 3,
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
UnresolvedReviewThreads: 0,
|
||||
UnresolvedReviewThreadsPopulated: false, // REST path: didn't look
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, _ := store.GetTaskPR(ctx, "t1")
|
||||
if stored.UnresolvedReviewThreads != 3 {
|
||||
t.Fatalf("threads clobbered: got %d, want 3", stored.UnresolvedReviewThreads)
|
||||
}
|
||||
if got := eb.publishedCount(); got != 0 {
|
||||
t.Fatalf("expected no event when nothing changed, got %d", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_BaseBranchRetarget covers the case where a PR is retargeted
|
||||
// to a different base branch upstream — the new branch must be persisted so
|
||||
// future branch-protection lookups key off the right value.
|
||||
func TestSyncTaskPR_BaseBranchRetarget(t *testing.T) {
|
||||
svc, store, _ := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &TaskPR{
|
||||
TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open",
|
||||
}); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
status := &PRStatus{
|
||||
PR: &PR{
|
||||
Number: 1, State: "open", RepoOwner: "o", RepoName: "r",
|
||||
BaseBranch: "develop",
|
||||
},
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
stored, _ := store.GetTaskPR(ctx, "t1")
|
||||
if stored.BaseBranch != "develop" {
|
||||
t.Fatalf("base branch not updated: got %q want develop", stored.BaseBranch)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncTaskPR_NewFieldsPublishEvent guards that a change to any of the
|
||||
// CI-popover-relevant fields (UnresolvedReviewThreads, RequiredReviews,
|
||||
// ChecksTotal, ChecksPassing) triggers a github.task_pr.updated event so the
|
||||
// frontend popover refreshes via WS.
|
||||
func TestSyncTaskPR_NewFieldsPublishEvent(t *testing.T) {
|
||||
required2 := 2
|
||||
cases := []struct {
|
||||
name string
|
||||
init TaskPR
|
||||
status PRStatus
|
||||
}{
|
||||
{
|
||||
name: "unresolved threads change",
|
||||
init: TaskPR{TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open"},
|
||||
status: PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
UnresolvedReviewThreads: 3,
|
||||
UnresolvedReviewThreadsPopulated: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "required reviews change",
|
||||
init: TaskPR{TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open"},
|
||||
status: PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
RequiredReviews: &required2,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "checks total change",
|
||||
init: TaskPR{TaskID: "t1", Owner: "o", Repo: "r", PRNumber: 1, PRURL: "u",
|
||||
HeadBranch: "feat", BaseBranch: "main", State: "open"},
|
||||
status: PRStatus{
|
||||
PR: &PR{Number: 1, State: "open", RepoOwner: "o", RepoName: "r"},
|
||||
ChecksTotal: 5,
|
||||
ChecksPassing: 3,
|
||||
ChecksPopulated: true,
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
svc, store, eb := setupSyncTest(t)
|
||||
ctx := context.Background()
|
||||
if err := store.CreateTaskPR(ctx, &tc.init); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
if err := svc.SyncTaskPR(ctx, "t1", &tc.status); err != nil {
|
||||
t.Fatalf("sync: %v", err)
|
||||
}
|
||||
if got := eb.publishedCount(); got != 1 {
|
||||
t.Fatalf("expected 1 event, got %d", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTimeEqual(t *testing.T) {
|
||||
now := time.Now()
|
||||
later := now.Add(time.Second)
|
||||
|
||||
@@ -74,7 +74,11 @@ const createTablesSQL = `
|
||||
mergeable_state TEXT NOT NULL DEFAULT '',
|
||||
review_count INTEGER DEFAULT 0,
|
||||
pending_review_count INTEGER DEFAULT 0,
|
||||
required_reviews INTEGER,
|
||||
comment_count INTEGER DEFAULT 0,
|
||||
unresolved_review_threads INTEGER DEFAULT 0,
|
||||
checks_total INTEGER DEFAULT 0,
|
||||
checks_passing INTEGER DEFAULT 0,
|
||||
additions INTEGER DEFAULT 0,
|
||||
deletions INTEGER DEFAULT 0,
|
||||
created_at DATETIME NOT NULL,
|
||||
@@ -163,6 +167,13 @@ func (s *Store) initSchema() error {
|
||||
// Phase 4 (multi-repo): per-repo PR association on github_task_prs.
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN repository_id TEXT NOT NULL DEFAULT ''`)
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_pr_watches ADD COLUMN repository_id TEXT NOT NULL DEFAULT ''`)
|
||||
// CI popover: aggregate counts + branch protection's required_approving_review_count
|
||||
// + unresolved review-threads, surfaced in the PR top-bar hover popover so the
|
||||
// frontend can render the counts row without a second round-trip.
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN required_reviews INTEGER`)
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN unresolved_review_threads INTEGER DEFAULT 0`)
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN checks_total INTEGER DEFAULT 0`)
|
||||
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN checks_passing INTEGER DEFAULT 0`)
|
||||
if err := s.migratePRTablesForMultiRepo(); err != nil {
|
||||
return fmt.Errorf("migrate PR tables for multi-repo: %w", err)
|
||||
}
|
||||
@@ -360,7 +371,11 @@ func (s *Store) migratePRTablesForMultiRepo() error {
|
||||
mergeable_state TEXT NOT NULL DEFAULT '',
|
||||
review_count INTEGER DEFAULT 0,
|
||||
pending_review_count INTEGER DEFAULT 0,
|
||||
required_reviews INTEGER,
|
||||
comment_count INTEGER DEFAULT 0,
|
||||
unresolved_review_threads INTEGER DEFAULT 0,
|
||||
checks_total INTEGER DEFAULT 0,
|
||||
checks_passing INTEGER DEFAULT 0,
|
||||
additions INTEGER DEFAULT 0,
|
||||
deletions INTEGER DEFAULT 0,
|
||||
created_at DATETIME NOT NULL,
|
||||
@@ -580,11 +595,13 @@ func (s *Store) CreateTaskPR(ctx context.Context, tp *TaskPR) error {
|
||||
tp.UpdatedAt = now
|
||||
_, err := s.db.ExecContext(ctx, `
|
||||
INSERT INTO github_task_prs (id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
|
||||
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, comment_count, additions, deletions,
|
||||
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
|
||||
unresolved_review_threads, checks_total, checks_passing, additions, deletions,
|
||||
created_at, merged_at, closed_at, last_synced_at, updated_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
tp.ID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
|
||||
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.CommentCount, tp.Additions, tp.Deletions,
|
||||
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
|
||||
tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
|
||||
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt)
|
||||
return err
|
||||
}
|
||||
@@ -713,11 +730,13 @@ func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR) error {
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx, `
|
||||
INSERT INTO github_task_prs (id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
|
||||
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, comment_count, additions, deletions,
|
||||
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
|
||||
unresolved_review_threads, checks_total, checks_passing, additions, deletions,
|
||||
created_at, merged_at, closed_at, last_synced_at, updated_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
|
||||
tp.ID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
|
||||
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.CommentCount, tp.Additions, tp.Deletions,
|
||||
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
|
||||
tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
|
||||
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -729,13 +748,15 @@ func (s *Store) UpdateTaskPR(ctx context.Context, tp *TaskPR) error {
|
||||
tp.UpdatedAt = time.Now().UTC()
|
||||
_, err := s.db.ExecContext(ctx, `
|
||||
UPDATE github_task_prs SET state = ?, review_state = ?, checks_state = ?, mergeable_state = ?,
|
||||
review_count = ?, pending_review_count = ?, comment_count = ?,
|
||||
additions = ?, deletions = ?, pr_title = ?,
|
||||
review_count = ?, pending_review_count = ?, required_reviews = ?, comment_count = ?,
|
||||
unresolved_review_threads = ?, checks_total = ?, checks_passing = ?,
|
||||
additions = ?, deletions = ?, pr_title = ?, base_branch = ?,
|
||||
merged_at = ?, closed_at = ?, last_synced_at = ?, updated_at = ?
|
||||
WHERE id = ?`,
|
||||
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState,
|
||||
tp.ReviewCount, tp.PendingReviewCount, tp.CommentCount,
|
||||
tp.Additions, tp.Deletions, tp.PRTitle,
|
||||
tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
|
||||
tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing,
|
||||
tp.Additions, tp.Deletions, tp.PRTitle, tp.BaseBranch,
|
||||
tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt, tp.ID)
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,569 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useMemo, useState, type ReactNode } from "react";
|
||||
import {
|
||||
IconCheck,
|
||||
IconCircleCheck,
|
||||
IconCircleDot,
|
||||
IconCircleX,
|
||||
IconExternalLink,
|
||||
IconGitPullRequest,
|
||||
IconMessageCircle,
|
||||
IconPlus,
|
||||
} from "@tabler/icons-react";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useCommentsStore } from "@/lib/state/slices/comments";
|
||||
import type { PRFeedbackComment } from "@/lib/state/slices/comments";
|
||||
import { useGitHubStatus } from "@/hooks/domains/github/use-github-status";
|
||||
import { usePRCIPopover } from "@/hooks/domains/github/use-pr-ci-popover";
|
||||
import {
|
||||
bucketCheck,
|
||||
bucketCheckCounts,
|
||||
groupChecksByWorkflow,
|
||||
type CheckBucket,
|
||||
type WorkflowGroup,
|
||||
} from "@/lib/github/check-buckets";
|
||||
import type { CheckRun, TaskPR } from "@/lib/types/github";
|
||||
|
||||
type CountsView = {
|
||||
passed: number;
|
||||
inProgress: number;
|
||||
failed: number;
|
||||
};
|
||||
|
||||
const CHECK_GROUP_ORDER: CheckBucket[] = ["passed", "in_progress", "failed"];
|
||||
|
||||
function countForBucket(counts: CountsView, kind: CheckBucket): number {
|
||||
if (kind === "passed") return counts.passed;
|
||||
if (kind === "in_progress") return counts.inProgress;
|
||||
return counts.failed;
|
||||
}
|
||||
|
||||
function deriveAggregateCounts(pr: TaskPR): CountsView {
|
||||
// Pre-load: if checks_total is populated, derive a coarse split using
|
||||
// checks_state to attribute the "non-passing" remainder. This is good
|
||||
// enough for the instant render — the lazy PRFeedback fetch refines.
|
||||
const total = pr.checks_total;
|
||||
const passing = pr.checks_passing;
|
||||
const remaining = Math.max(0, total - passing);
|
||||
if (pr.checks_state === "failure") {
|
||||
return { passed: passing, failed: remaining, inProgress: 0 };
|
||||
}
|
||||
if (pr.checks_state === "pending") {
|
||||
return { passed: passing, failed: 0, inProgress: remaining };
|
||||
}
|
||||
if (pr.checks_state === "success") {
|
||||
return { passed: total, failed: 0, inProgress: 0 };
|
||||
}
|
||||
return { passed: passing, failed: 0, inProgress: remaining };
|
||||
}
|
||||
|
||||
function PRCIPopoverHeader({ pr }: { pr: TaskPR }) {
|
||||
const checksUrl = `${pr.pr_url}/checks`;
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-popover-header"
|
||||
className="flex items-center justify-between gap-2 border-b border-border/50 pb-2"
|
||||
>
|
||||
<span className="text-sm font-medium">CI status</span>
|
||||
<a
|
||||
data-testid="pr-popover-external-link"
|
||||
href={checksUrl}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="cursor-pointer text-muted-foreground hover:text-foreground"
|
||||
aria-label="View all checks on GitHub"
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
<IconExternalLink className="h-3.5 w-3.5" />
|
||||
</a>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function CheckGroupIcon({ kind }: { kind: CheckBucket }) {
|
||||
if (kind === "passed") return <IconCircleCheck className="h-3.5 w-3.5 text-emerald-500" />;
|
||||
if (kind === "in_progress")
|
||||
return <IconCircleDot className="h-3.5 w-3.5 text-yellow-500 animate-pulse" />;
|
||||
return <IconCircleX className="h-3.5 w-3.5 text-red-500" />;
|
||||
}
|
||||
|
||||
function bucketLabel(kind: CheckBucket): string {
|
||||
if (kind === "passed") return "Passed";
|
||||
if (kind === "in_progress") return "In progress";
|
||||
return "Failed";
|
||||
}
|
||||
|
||||
function CheckGroupHeader({ kind, count }: { kind: CheckBucket; count: number }) {
|
||||
const label = bucketLabel(kind);
|
||||
return (
|
||||
<div className="flex items-center justify-between gap-2 px-1 py-1">
|
||||
<div className="flex items-center gap-1.5">
|
||||
<CheckGroupIcon kind={kind} />
|
||||
<span className="text-xs font-medium">{label}</span>
|
||||
</div>
|
||||
<span
|
||||
data-testid="pr-check-group-count"
|
||||
className="text-xs tabular-nums text-muted-foreground"
|
||||
>
|
||||
{count}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function PRWorkflowRow({
|
||||
group,
|
||||
onAddAsContext,
|
||||
}: {
|
||||
group: WorkflowGroup;
|
||||
onAddAsContext: ((message: string) => void) | null;
|
||||
}) {
|
||||
// For an in-progress workflow, "0/1 ran" reads as "nothing finished"
|
||||
// and confuses people: did it start? It's clearer to just report how
|
||||
// many jobs are running. (Failed workflows go to a different bucket,
|
||||
// so an in_progress group never has failed jobs.)
|
||||
const badge =
|
||||
group.bucket === "in_progress"
|
||||
? `${group.inProgress} running`
|
||||
: `${group.passed}/${group.total} passed`;
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-workflow-row"
|
||||
data-workflow={group.workflow}
|
||||
data-bucket={group.bucket}
|
||||
className="flex items-center gap-2 px-2 py-1 rounded-sm hover:bg-accent/50 cursor-pointer"
|
||||
onClick={() => {
|
||||
if (group.htmlUrl) window.open(group.htmlUrl, "_blank", "noopener,noreferrer");
|
||||
}}
|
||||
>
|
||||
<span className="text-xs font-medium truncate min-w-0 flex-1" title={group.workflow}>
|
||||
{group.workflow}
|
||||
</span>
|
||||
<span className="text-[10px] text-muted-foreground shrink-0">{badge}</span>
|
||||
{group.htmlUrl && (
|
||||
<Button
|
||||
data-testid="pr-workflow-open"
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
className="h-5 w-5 p-0 cursor-pointer"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
window.open(group.htmlUrl, "_blank", "noopener,noreferrer");
|
||||
}}
|
||||
aria-label={`Open ${group.workflow} on GitHub`}
|
||||
>
|
||||
<IconExternalLink className="h-3 w-3" />
|
||||
</Button>
|
||||
)}
|
||||
{group.bucket === "failed" && onAddAsContext && (
|
||||
<Button
|
||||
data-testid="pr-workflow-add-context"
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
className="h-5 w-5 p-0 cursor-pointer"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onAddAsContext(buildWorkflowMessage(group));
|
||||
}}
|
||||
aria-label={`Add ${group.workflow} failures to chat context`}
|
||||
>
|
||||
<IconPlus className="h-3 w-3" />
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function buildWorkflowMessage(group: WorkflowGroup): string {
|
||||
const failed = group.jobs.filter((j) => bucketCheck(j) === "failed");
|
||||
const lines: string[] = [
|
||||
`### Workflow **${group.workflow}** has ${failed.length} failing job${failed.length !== 1 ? "s" : ""}.`,
|
||||
"",
|
||||
];
|
||||
for (const job of failed) {
|
||||
lines.push(`- **${job.name}** — ${job.conclusion || job.status}`);
|
||||
if (job.output) lines.push(` ${job.output}`);
|
||||
if (job.html_url) lines.push(` ${job.html_url}`);
|
||||
}
|
||||
lines.push("", "Please investigate and fix.");
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
function PRCheckGroup({
|
||||
kind,
|
||||
count,
|
||||
workflows,
|
||||
isLoading,
|
||||
onAddAsContext,
|
||||
}: {
|
||||
kind: CheckBucket;
|
||||
count: number;
|
||||
workflows?: WorkflowGroup[];
|
||||
isLoading: boolean;
|
||||
onAddAsContext: ((message: string) => void) | null;
|
||||
}) {
|
||||
if (count <= 0 && (!workflows || workflows.length === 0)) return null;
|
||||
const showRows = kind !== "passed";
|
||||
const hasWorkflows = !!workflows && workflows.length > 0;
|
||||
return (
|
||||
<div data-testid="pr-check-group" data-kind={kind} className="flex flex-col">
|
||||
<CheckGroupHeader kind={kind} count={count} />
|
||||
{showRows && (
|
||||
<div className="flex flex-col pl-5">
|
||||
{hasWorkflows &&
|
||||
workflows!.map((g) => (
|
||||
<PRWorkflowRow
|
||||
key={`${g.workflow}-${g.bucket}`}
|
||||
group={g}
|
||||
onAddAsContext={kind === "failed" ? onAddAsContext : null}
|
||||
/>
|
||||
))}
|
||||
{!hasWorkflows && isLoading
|
||||
? Array.from({ length: Math.min(2, count || 1) }).map((_, i) => (
|
||||
<div
|
||||
key={`skel-${i}`}
|
||||
data-testid="pr-workflow-row-skeleton"
|
||||
className="h-5 my-0.5 rounded-sm bg-muted animate-pulse"
|
||||
/>
|
||||
))
|
||||
: null}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stacked progress bar mirroring TodoIndicator's bar but split into three
|
||||
* segments — green (passed), yellow (in_progress), red (failed) — each
|
||||
* sized proportionally to its count. The leading "pass rate" label gives
|
||||
* the same at-a-glance number TodoIndicator shows ("6/10 (60%)").
|
||||
*
|
||||
* Skipped/cancelled-but-not-failed jobs aren't counted toward the total
|
||||
* (they never enter any bucket), so the bar always reflects the actionable
|
||||
* picture: how close are we to "all green".
|
||||
*/
|
||||
function PRChecksProgressBar({ counts }: { counts: CountsView }) {
|
||||
const total = counts.passed + counts.inProgress + counts.failed;
|
||||
if (total === 0) return null;
|
||||
const pct = (n: number) => (n / total) * 100;
|
||||
const passedPct = pct(counts.passed);
|
||||
const inProgressPct = pct(counts.inProgress);
|
||||
const failedPct = pct(counts.failed);
|
||||
const completePct = Math.round(passedPct);
|
||||
return (
|
||||
<div data-testid="pr-checks-progress" className="flex flex-col gap-1.5 px-1 pt-1 pb-1.5">
|
||||
<div className="flex items-center justify-between text-xs">
|
||||
<span className="font-medium text-foreground">Pass rate</span>
|
||||
<span className="text-muted-foreground tabular-nums">
|
||||
{counts.passed}/{total} ({completePct}%)
|
||||
</span>
|
||||
</div>
|
||||
<div className="flex h-1.5 w-full overflow-hidden rounded-full bg-muted/70">
|
||||
{passedPct > 0 && (
|
||||
<div
|
||||
data-segment="passed"
|
||||
className="h-full bg-green-500 transition-[width]"
|
||||
style={{ width: `${passedPct}%` }}
|
||||
/>
|
||||
)}
|
||||
{inProgressPct > 0 && (
|
||||
<div
|
||||
data-segment="in_progress"
|
||||
className="h-full bg-yellow-500 transition-[width]"
|
||||
style={{ width: `${inProgressPct}%` }}
|
||||
/>
|
||||
)}
|
||||
{failedPct > 0 && (
|
||||
<div
|
||||
data-segment="failed"
|
||||
className="h-full bg-red-500 transition-[width]"
|
||||
style={{ width: `${failedPct}%` }}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function PRChecksSection({
|
||||
pr,
|
||||
feedback,
|
||||
isFetching,
|
||||
onAddAsContext,
|
||||
}: {
|
||||
pr: TaskPR;
|
||||
feedback: { checks?: CheckRun[] } | null;
|
||||
isFetching: boolean;
|
||||
onAddAsContext: ((message: string) => void) | null;
|
||||
}) {
|
||||
const aggregateCounts = useMemo(() => deriveAggregateCounts(pr), [pr]);
|
||||
|
||||
const { precise, byBucket } = useMemo(() => {
|
||||
// Treat empty `feedback.checks` the same as "feedback not loaded yet" so
|
||||
// we keep showing the aggregate counts. Some mock paths return empty
|
||||
// arrays without errors, and we don't want the popover to flash 0/0/0.
|
||||
if (!feedback?.checks || feedback.checks.length === 0) {
|
||||
return { precise: null as CountsView | null, byBucket: null };
|
||||
}
|
||||
const counts = bucketCheckCounts(feedback.checks);
|
||||
const precise: CountsView = {
|
||||
passed: counts.passed,
|
||||
inProgress: counts.inProgress,
|
||||
failed: counts.failed,
|
||||
};
|
||||
const groups = groupChecksByWorkflow(feedback.checks);
|
||||
const byBucket: Record<CheckBucket, WorkflowGroup[]> = {
|
||||
passed: groups.filter((g) => g.bucket === "passed"),
|
||||
in_progress: groups.filter((g) => g.bucket === "in_progress"),
|
||||
failed: groups.filter((g) => g.bucket === "failed"),
|
||||
};
|
||||
return { precise, byBucket };
|
||||
}, [feedback]);
|
||||
|
||||
const counts = precise ?? aggregateCounts;
|
||||
// "No checks at all": the lazy fetch has settled (not isFetching) and there
|
||||
// are no checks anywhere — neither in PRFeedback nor in the aggregate.
|
||||
const noChecksAtAll =
|
||||
!isFetching && pr.checks_total === 0 && (!feedback || (feedback.checks?.length ?? 0) === 0);
|
||||
if (noChecksAtAll) {
|
||||
return (
|
||||
<div data-testid="pr-checks-section" className="flex flex-col">
|
||||
<div data-testid="pr-checks-empty" className="px-1 py-2 text-xs text-muted-foreground">
|
||||
No checks have started
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<div data-testid="pr-checks-section" className="flex flex-col gap-1">
|
||||
<PRChecksProgressBar counts={counts} />
|
||||
{CHECK_GROUP_ORDER.map((kind) => {
|
||||
const value = countForBucket(counts, kind);
|
||||
return (
|
||||
<PRCheckGroup
|
||||
key={kind}
|
||||
kind={kind}
|
||||
count={value}
|
||||
workflows={byBucket?.[kind]}
|
||||
isLoading={isFetching && !byBucket}
|
||||
onAddAsContext={kind === "failed" ? onAddAsContext : null}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function PRReviewRow({ pr }: { pr: TaskPR }) {
|
||||
const required = pr.required_reviews ?? null;
|
||||
const approved = pr.review_count;
|
||||
const requested = pr.pending_review_count;
|
||||
|
||||
let label: string;
|
||||
let icon: ReactNode;
|
||||
if (pr.review_state === "approved") {
|
||||
icon = <IconCheck className="h-3.5 w-3.5 text-emerald-500" />;
|
||||
label = "Approved";
|
||||
} else if (pr.review_state === "changes_requested") {
|
||||
icon = <IconCircleX className="h-3.5 w-3.5 text-red-500" />;
|
||||
label = "Changes requested";
|
||||
} else {
|
||||
// Default branch covers "no reviews yet", "review pending", and any
|
||||
// unknown state. Always rendered (no early-return) so a fresh PR
|
||||
// still surfaces its review status.
|
||||
icon = <IconCircleDot className="h-3.5 w-3.5 text-muted-foreground" />;
|
||||
label = "Awaiting review";
|
||||
}
|
||||
// Counts live on the right edge, mirroring the check-group rows.
|
||||
// Use "K / M" when a required-minimum is known, otherwise just the
|
||||
// approved count. The "(K requested)" suffix slides in when reviewers
|
||||
// have been pinged but haven't submitted a review.
|
||||
let countText: string;
|
||||
if (required != null) countText = `${approved} / ${required}`;
|
||||
else countText = `${approved}`;
|
||||
if (requested > 0) countText += ` · ${requested} requested`;
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-review-row"
|
||||
className="flex items-center justify-between gap-2 px-1 py-1 text-xs"
|
||||
>
|
||||
<div className="flex items-center gap-1.5 min-w-0">
|
||||
{icon}
|
||||
<span className="truncate">{label}</span>
|
||||
</div>
|
||||
<span className="shrink-0 text-muted-foreground tabular-nums">{countText}</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function PRCommentsRow({ pr }: { pr: TaskPR }) {
|
||||
if (!pr.unresolved_review_threads || pr.unresolved_review_threads <= 0) return null;
|
||||
return (
|
||||
<div data-testid="pr-comments-row" className="flex items-center gap-1.5 px-1 py-1 text-xs">
|
||||
<IconMessageCircle className="h-3.5 w-3.5 text-muted-foreground" />
|
||||
<span>
|
||||
{pr.unresolved_review_threads} unresolved comment
|
||||
{pr.unresolved_review_threads === 1 ? "" : "s"}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function elapsedLabel(elapsed: number | null): string {
|
||||
if (elapsed == null) return "";
|
||||
if (elapsed === 0) return "updated just now";
|
||||
return `updated ${formatElapsedShort(elapsed)} ago`;
|
||||
}
|
||||
|
||||
function PRPopoverFooter({ lastUpdatedAt }: { lastUpdatedAt: number | null }) {
|
||||
// Capture a "now" snapshot only inside the setInterval callback (an event
|
||||
// handler — no impurity in render, no setState during commit). When no
|
||||
// tick has fired yet, fall back to lastUpdatedAt so the rendered elapsed
|
||||
// is 0 ("just now").
|
||||
const [now, setNow] = useState<number | null>(null);
|
||||
useEffect(() => {
|
||||
if (lastUpdatedAt == null) return;
|
||||
const id = setInterval(() => setNow(Date.now()), 10_000);
|
||||
return () => clearInterval(id);
|
||||
}, [lastUpdatedAt]);
|
||||
const elapsed =
|
||||
lastUpdatedAt == null
|
||||
? null
|
||||
: Math.max(0, Math.floor(((now ?? lastUpdatedAt) - lastUpdatedAt) / 1000));
|
||||
if (lastUpdatedAt == null) return null;
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-popover-footer"
|
||||
className="flex items-center justify-end border-t border-border/50 pt-1.5"
|
||||
>
|
||||
<span
|
||||
data-testid="pr-popover-updated-at"
|
||||
className="text-[10px] text-muted-foreground tabular-nums"
|
||||
>
|
||||
{elapsedLabel(elapsed)}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function formatElapsedShort(seconds: number): string {
|
||||
if (seconds < 60) return `${seconds}s`;
|
||||
const minutes = Math.floor(seconds / 60);
|
||||
if (minutes < 60) return `${minutes}m`;
|
||||
const hours = Math.floor(minutes / 60);
|
||||
return `${hours}h`;
|
||||
}
|
||||
|
||||
function ReconnectGitHubBlock() {
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-popover-auth-error"
|
||||
className="flex flex-col items-start gap-1 px-1 py-2 text-xs"
|
||||
>
|
||||
<span className="text-foreground">GitHub authentication lost.</span>
|
||||
<a
|
||||
data-testid="pr-popover-reconnect-link"
|
||||
href="/settings#github"
|
||||
className="cursor-pointer text-primary hover:underline"
|
||||
>
|
||||
Reconnect GitHub
|
||||
</a>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export function PRCIPopover({
|
||||
pr,
|
||||
enabled,
|
||||
onOpenDetailPanel,
|
||||
}: {
|
||||
pr: TaskPR;
|
||||
enabled: boolean;
|
||||
onOpenDetailPanel?: () => void;
|
||||
}) {
|
||||
const ghStatus = useAppStore((s) => s.githubStatus.status);
|
||||
const authLost = ghStatus !== null && !ghStatus.authenticated;
|
||||
// Trigger an initial status load from the same hook the rest of the app uses.
|
||||
useGitHubStatus();
|
||||
const { feedback, isFetching, lastUpdatedAt } = usePRCIPopover(pr, enabled && !authLost);
|
||||
const onAddAsContext = useAddCheckToContext(pr);
|
||||
|
||||
return (
|
||||
<div
|
||||
data-testid="pr-topbar-popover-inner"
|
||||
className="flex flex-col gap-2"
|
||||
onClick={(e) => e.stopPropagation()}
|
||||
>
|
||||
<PRCIPopoverHeader pr={pr} />
|
||||
{authLost ? (
|
||||
<ReconnectGitHubBlock />
|
||||
) : (
|
||||
<>
|
||||
<PRChecksSection
|
||||
pr={pr}
|
||||
feedback={feedback}
|
||||
isFetching={isFetching}
|
||||
onAddAsContext={onAddAsContext}
|
||||
/>
|
||||
<div className="flex flex-col gap-0">
|
||||
<PRReviewRow pr={pr} />
|
||||
<PRCommentsRow pr={pr} />
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
{onOpenDetailPanel && (
|
||||
<Button
|
||||
data-testid="pr-popover-open-detail"
|
||||
size="sm"
|
||||
variant="ghost"
|
||||
className="cursor-pointer justify-start gap-1.5 px-2"
|
||||
onClick={onOpenDetailPanel}
|
||||
>
|
||||
<IconGitPullRequest className="h-3.5 w-3.5" />
|
||||
<span>Open PR details</span>
|
||||
</Button>
|
||||
)}
|
||||
<PRPopoverFooter lastUpdatedAt={lastUpdatedAt} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// --- Add-to-context wiring (mirrors pr-detail-panel.tsx for failed checks) ---
|
||||
function useAddCheckToContext(pr: TaskPR): ((message: string) => void) | null {
|
||||
const sessionId = useAppStore((s) => s.tasks.activeSessionId);
|
||||
const addComment = useCommentsStore((s) => s.addComment);
|
||||
const { toast } = useToast();
|
||||
const prNumber = pr.pr_number;
|
||||
// Always call useCallback (rules-of-hooks) before bailing out, so the
|
||||
// returned callback identity is stable across renders unless its inputs
|
||||
// change. Without memoization the popover would create a new function
|
||||
// every parent render, defeating PRWorkflowRow's cheap reference equality.
|
||||
const handler = useCallback(
|
||||
(message: string) => {
|
||||
if (!sessionId) return;
|
||||
const comment: PRFeedbackComment = {
|
||||
id: `pr-feedback-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`,
|
||||
sessionId,
|
||||
text: message,
|
||||
createdAt: new Date().toISOString(),
|
||||
status: "pending",
|
||||
source: "pr-feedback",
|
||||
prNumber,
|
||||
feedbackType: "check",
|
||||
content: message,
|
||||
};
|
||||
addComment(comment);
|
||||
toast({ description: "Added to chat context" });
|
||||
},
|
||||
[sessionId, prNumber, addComment, toast],
|
||||
);
|
||||
return sessionId ? handler : null;
|
||||
}
|
||||
|
||||
// Multi-PR aggregate popover deferred — multi-PR tasks currently keep the
|
||||
// existing DropdownMenu trigger (no hover popover). The aggregate component
|
||||
// will land alongside that hover variant when it ships, see follow-up.
|
||||
@@ -0,0 +1,118 @@
|
||||
"use client";
|
||||
|
||||
import { useRef } from "react";
|
||||
import {
|
||||
IconCircleCheckFilled,
|
||||
IconCircleXFilled,
|
||||
IconChecklist,
|
||||
IconLoader2,
|
||||
IconPointFilled,
|
||||
} from "@tabler/icons-react";
|
||||
import { HoverCard, HoverCardContent, HoverCardTrigger } from "@kandev/ui/hover-card";
|
||||
import { useTaskPR } from "@/hooks/domains/github/use-task-pr";
|
||||
import { usePRFeedbackBackgroundSync } from "@/hooks/domains/github/use-pr-ci-popover";
|
||||
import { PRCIPopover } from "@/components/github/pr-ci-popover";
|
||||
import { isPRReadyToMerge } from "@/components/github/pr-task-icon";
|
||||
import type { TaskPR } from "@/lib/types/github";
|
||||
|
||||
const HOVER_OPEN_DELAY_MS = 150;
|
||||
const HOVER_CLOSE_DELAY_MS = 150;
|
||||
|
||||
type ChipStatus = "passed" | "failed" | "in_progress" | "merged" | "closed" | "neutral";
|
||||
|
||||
function chipStatus(pr: TaskPR): ChipStatus {
|
||||
if (pr.state === "merged") return "merged";
|
||||
if (pr.state === "closed") return "closed";
|
||||
if (pr.review_state === "changes_requested" || pr.checks_state === "failure") return "failed";
|
||||
// Pending checks / pending review must beat checks_state === "success" so a
|
||||
// PR with all checks green but reviewers still outstanding renders as
|
||||
// in-progress, not passed. Without this order, the chip flips to green the
|
||||
// moment CI finishes and ignores the human gate.
|
||||
if (pr.checks_state === "pending" || pr.review_state === "pending") return "in_progress";
|
||||
if (pr.checks_state === "success") return "passed";
|
||||
return "neutral";
|
||||
}
|
||||
|
||||
/**
|
||||
* Compact CI indicator for the chat status bar — a "CI" prefix icon plus a
|
||||
* status glyph that mirrors the popover's bucket colors:
|
||||
* passed → green check
|
||||
* failed → red X
|
||||
* in progress → yellow spinner
|
||||
* merged → purple dot
|
||||
* neutral → muted dot
|
||||
*
|
||||
* Hovering opens the full PRCIPopover anchored to the top edge so the card
|
||||
* expands upward (the chip sits just above the chat input).
|
||||
*
|
||||
* Returns null when the task has no PR yet.
|
||||
*/
|
||||
export function PRStatusChip({ taskId }: { taskId: string | null }) {
|
||||
const { pr } = useTaskPR(taskId);
|
||||
// Subscribe at the chip level so the cache warms even when the
|
||||
// top-bar PR button isn't mounted (e.g. small viewport that hides it).
|
||||
usePRFeedbackBackgroundSync(pr);
|
||||
if (!pr) return null;
|
||||
return <PRStatusChipInner pr={pr} />;
|
||||
}
|
||||
|
||||
function PRStatusChipInner({ pr }: { pr: TaskPR }) {
|
||||
const status = chipStatus(pr);
|
||||
const triggerRef = useRef<HTMLButtonElement>(null);
|
||||
return (
|
||||
<HoverCard openDelay={HOVER_OPEN_DELAY_MS} closeDelay={HOVER_CLOSE_DELAY_MS}>
|
||||
<HoverCardTrigger asChild>
|
||||
<button
|
||||
ref={triggerRef}
|
||||
type="button"
|
||||
data-testid="pr-status-chip"
|
||||
data-pr-number={pr.pr_number}
|
||||
data-pr-state={pr.state}
|
||||
data-status={status}
|
||||
data-pr-ready-to-merge={isPRReadyToMerge(pr) ? "true" : "false"}
|
||||
aria-label={`Pull request #${pr.pr_number} CI status`}
|
||||
className="cursor-pointer inline-flex items-center gap-1 rounded-md px-1 py-0.5 text-xs"
|
||||
>
|
||||
<IconChecklist className="h-3.5 w-3.5 text-muted-foreground" aria-hidden="true" />
|
||||
<ChipStatusGlyph status={status} />
|
||||
</button>
|
||||
</HoverCardTrigger>
|
||||
<HoverCardContent
|
||||
side="top"
|
||||
align="start"
|
||||
sideOffset={8}
|
||||
className="w-80 p-2.5"
|
||||
onPointerDownOutside={(e) => {
|
||||
// Radix HoverCard treats the trigger as outside the content's
|
||||
// bounding box, so a click on the chip would auto-close the
|
||||
// popover. Filter out trigger clicks so clicking the chip is
|
||||
// a no-op while the popover stays open via hover.
|
||||
if (triggerRef.current && triggerRef.current.contains(e.target as Node)) {
|
||||
e.preventDefault();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<PRCIPopover pr={pr} enabled={true} />
|
||||
</HoverCardContent>
|
||||
</HoverCard>
|
||||
);
|
||||
}
|
||||
|
||||
function ChipStatusGlyph({ status }: { status: ChipStatus }) {
|
||||
switch (status) {
|
||||
case "passed":
|
||||
return <IconCircleCheckFilled className="h-3.5 w-3.5 text-green-500" aria-hidden="true" />;
|
||||
case "failed":
|
||||
return <IconCircleXFilled className="h-3.5 w-3.5 text-red-500" aria-hidden="true" />;
|
||||
case "in_progress":
|
||||
return (
|
||||
<IconLoader2 className="h-3.5 w-3.5 text-yellow-500 animate-spin" aria-hidden="true" />
|
||||
);
|
||||
case "merged":
|
||||
return <IconPointFilled className="h-3.5 w-3.5 text-purple-500" aria-hidden="true" />;
|
||||
case "closed":
|
||||
return <IconPointFilled className="h-3.5 w-3.5 text-red-500" aria-hidden="true" />;
|
||||
default:
|
||||
return <IconPointFilled className="h-3.5 w-3.5 text-muted-foreground" aria-hidden="true" />;
|
||||
}
|
||||
}
|
||||
@@ -27,6 +27,9 @@ function makePR(overrides: Partial<TaskPR> = {}): TaskPR {
|
||||
review_count: 0,
|
||||
pending_review_count: 0,
|
||||
comment_count: 0,
|
||||
unresolved_review_threads: 0,
|
||||
checks_total: 0,
|
||||
checks_passing: 0,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
created_at: "",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { memo } from "react";
|
||||
import { memo, useCallback, useEffect, useRef, useState } from "react";
|
||||
import {
|
||||
IconGitPullRequest,
|
||||
IconCheck,
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
IconChevronDown,
|
||||
} from "@tabler/icons-react";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { Popover, PopoverAnchor, PopoverContent } from "@kandev/ui/popover";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import {
|
||||
DropdownMenu,
|
||||
@@ -20,15 +21,20 @@ import {
|
||||
} from "@kandev/ui/dropdown-menu";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { useTaskPR } from "@/hooks/domains/github/use-task-pr";
|
||||
import { useIsMobile } from "@/hooks/use-mobile";
|
||||
import {
|
||||
aggregatePRStatusColor,
|
||||
getPRStatusColor,
|
||||
isPRReadyToMerge,
|
||||
} from "@/components/github/pr-task-icon";
|
||||
import { prTaskKey } from "@/components/github/pr-detail-panel";
|
||||
import { PRCIPopover } from "@/components/github/pr-ci-popover";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import type { TaskPR } from "@/lib/types/github";
|
||||
|
||||
const POPOVER_OPEN_DELAY_MS = 150;
|
||||
const POPOVER_CLOSE_DELAY_MS = 150;
|
||||
|
||||
function PRStatusIcon({ pr }: { pr: TaskPR }) {
|
||||
// Terminal states take priority
|
||||
if (pr.state === "merged") {
|
||||
@@ -65,33 +71,132 @@ export const PRTopbarButton = memo(function PRTopbarButton() {
|
||||
return <PRMultiButton prs={prs} />;
|
||||
});
|
||||
|
||||
/**
|
||||
* Manages the hover-driven popover lifecycle. Click on the button is left
|
||||
* to the caller — desktop preserves the existing "open the PR detail panel"
|
||||
* behavior, and hover is what reveals the CI popover. On touch devices the
|
||||
* popover is suppressed entirely so the button click falls through to the
|
||||
* existing detail-panel handler.
|
||||
*/
|
||||
function usePopoverInteractions() {
|
||||
const isMobile = useIsMobile();
|
||||
const [open, setOpen] = useState(false);
|
||||
const openTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const closeTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
|
||||
const clearOpen = useCallback(() => {
|
||||
if (openTimer.current) {
|
||||
clearTimeout(openTimer.current);
|
||||
openTimer.current = null;
|
||||
}
|
||||
}, []);
|
||||
const clearClose = useCallback(() => {
|
||||
if (closeTimer.current) {
|
||||
clearTimeout(closeTimer.current);
|
||||
closeTimer.current = null;
|
||||
}
|
||||
}, []);
|
||||
|
||||
const handleEnter = useCallback(() => {
|
||||
if (isMobile) return;
|
||||
clearClose();
|
||||
openTimer.current = setTimeout(() => setOpen(true), POPOVER_OPEN_DELAY_MS);
|
||||
}, [isMobile, clearClose]);
|
||||
|
||||
const handleLeave = useCallback(() => {
|
||||
if (isMobile) return;
|
||||
clearOpen();
|
||||
closeTimer.current = setTimeout(() => setOpen(false), POPOVER_CLOSE_DELAY_MS);
|
||||
}, [isMobile, clearOpen]);
|
||||
|
||||
useEffect(
|
||||
() => () => {
|
||||
clearOpen();
|
||||
clearClose();
|
||||
},
|
||||
[clearOpen, clearClose],
|
||||
);
|
||||
|
||||
return {
|
||||
isMobile,
|
||||
open,
|
||||
onOpenChange: (next: boolean) => {
|
||||
if (!next) {
|
||||
clearOpen();
|
||||
clearClose();
|
||||
setOpen(false);
|
||||
}
|
||||
},
|
||||
handleEnter,
|
||||
handleLeave,
|
||||
};
|
||||
}
|
||||
|
||||
function PRSingleButton({ pr }: { pr: TaskPR }) {
|
||||
const addPRPanel = useDockviewStore((s) => s.addPRPanel);
|
||||
const tooltip = `${pr.owner}/${pr.repo} #${pr.pr_number} — ${pr.pr_title}`;
|
||||
const { isMobile, open, onOpenChange, handleEnter, handleLeave } = usePopoverInteractions();
|
||||
// Background sync lives on PRStatusChip (always mounted in the chat
|
||||
// input area); the chip and this popover share prFeedbackCache so a
|
||||
// single subscription warms both.
|
||||
|
||||
const button = (
|
||||
<Button
|
||||
data-testid="pr-topbar-button"
|
||||
data-pr-number={pr.pr_number}
|
||||
data-pr-state={pr.state}
|
||||
data-pr-ready-to-merge={isPRReadyToMerge(pr) ? "true" : "false"}
|
||||
size="sm"
|
||||
variant="outline"
|
||||
className="cursor-pointer gap-1.5 px-2"
|
||||
onMouseEnter={handleEnter}
|
||||
onMouseLeave={handleLeave}
|
||||
onClick={() => {
|
||||
addPRPanel(prTaskKey(pr));
|
||||
onOpenChange(false);
|
||||
}}
|
||||
>
|
||||
<IconGitPullRequest className={`h-4 w-4 ${getPRStatusColor(pr)}`} />
|
||||
<span className="text-xs font-medium">#{pr.pr_number}</span>
|
||||
<PRStatusIcon pr={pr} />
|
||||
</Button>
|
||||
);
|
||||
|
||||
if (isMobile) {
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>{button}</TooltipTrigger>
|
||||
<TooltipContent>{tooltip}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
data-testid="pr-topbar-button"
|
||||
data-pr-number={pr.pr_number}
|
||||
data-pr-state={pr.state}
|
||||
data-pr-ready-to-merge={isPRReadyToMerge(pr) ? "true" : "false"}
|
||||
size="sm"
|
||||
variant="outline"
|
||||
className="cursor-pointer gap-1.5 px-2"
|
||||
onClick={() => addPRPanel(prTaskKey(pr))}
|
||||
>
|
||||
<IconGitPullRequest className={`h-4 w-4 ${getPRStatusColor(pr)}`} />
|
||||
<span className="text-xs font-medium">#{pr.pr_number}</span>
|
||||
<PRStatusIcon pr={pr} />
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>{tooltip}</TooltipContent>
|
||||
</Tooltip>
|
||||
<Popover open={open} onOpenChange={onOpenChange}>
|
||||
<PopoverAnchor asChild>{button}</PopoverAnchor>
|
||||
<PopoverContent
|
||||
data-testid="pr-topbar-popover"
|
||||
align="end"
|
||||
sideOffset={4}
|
||||
className="w-80"
|
||||
onMouseEnter={handleEnter}
|
||||
onMouseLeave={handleLeave}
|
||||
onOpenAutoFocus={(e) => e.preventDefault()}
|
||||
>
|
||||
<PRCIPopover pr={pr} enabled={open} />
|
||||
</PopoverContent>
|
||||
</Popover>
|
||||
);
|
||||
}
|
||||
|
||||
function PRMultiButton({ prs }: { prs: TaskPR[] }) {
|
||||
// Multi-PR keeps the original dropdown semantics on every form factor —
|
||||
// the popover lives inside the dropdown trigger via hover only, and the
|
||||
// per-PR rows are the click targets that addPRPanel.
|
||||
return <PRMultiDropdown prs={prs} />;
|
||||
}
|
||||
|
||||
function PRMultiDropdown({ prs }: { prs: TaskPR[] }) {
|
||||
const addPRPanel = useDockviewStore((s) => s.addPRPanel);
|
||||
const aggColor = aggregatePRStatusColor(prs);
|
||||
return (
|
||||
|
||||
@@ -5,6 +5,7 @@ import { IconArrowRight, IconGitMerge, IconX } from "@tabler/icons-react";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { TodoIndicator } from "./todo-indicator";
|
||||
import { PRStatusChip } from "@/components/github/pr-status-chip";
|
||||
import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
import { useKeyboardShortcut } from "@/hooks/use-keyboard-shortcut";
|
||||
import { useMessageHandler } from "@/hooks/use-message-handler";
|
||||
@@ -290,6 +291,7 @@ function ChatStatusBar({
|
||||
className="flex items-center gap-1.5 py-1 text-xs text-muted-foreground"
|
||||
>
|
||||
{showTodos && <TodoIndicator todos={todoItems} />}
|
||||
<PRStatusChip taskId={taskId} />
|
||||
{taskId && <PRMergedBanner key={taskId} taskId={taskId} />}
|
||||
{showProceed && (
|
||||
<Tooltip>
|
||||
|
||||
@@ -720,10 +720,58 @@ export class ApiClient {
|
||||
mergeable_state?: string;
|
||||
additions?: number;
|
||||
deletions?: number;
|
||||
review_count?: number;
|
||||
pending_review_count?: number;
|
||||
required_reviews?: number;
|
||||
checks_total?: number;
|
||||
checks_passing?: number;
|
||||
unresolved_review_threads?: number;
|
||||
}): Promise<void> {
|
||||
await this.request("POST", "/api/v1/github/mock/task-prs", data);
|
||||
}
|
||||
|
||||
async mockGitHubSeedPRFeedback(data: {
|
||||
owner: string;
|
||||
repo: string;
|
||||
pr_number: number;
|
||||
checks?: Array<{
|
||||
name: string;
|
||||
source?: string;
|
||||
status?: string;
|
||||
conclusion?: string;
|
||||
html_url?: string;
|
||||
output?: string;
|
||||
started_at?: string | null;
|
||||
completed_at?: string | null;
|
||||
}>;
|
||||
reviews?: Array<{
|
||||
id: number;
|
||||
author: string;
|
||||
author_avatar?: string;
|
||||
state: string;
|
||||
body?: string;
|
||||
created_at?: string;
|
||||
}>;
|
||||
comments?: Array<{
|
||||
id: number;
|
||||
author: string;
|
||||
author_avatar?: string;
|
||||
body: string;
|
||||
path?: string;
|
||||
line?: number;
|
||||
side?: string;
|
||||
comment_type?: string;
|
||||
created_at?: string;
|
||||
updated_at?: string;
|
||||
}>;
|
||||
}): Promise<void> {
|
||||
await this.request("POST", "/api/v1/github/mock/pr-feedback", data);
|
||||
}
|
||||
|
||||
async mockGitHubSetAuthHealth(data: { authenticated: boolean; error?: string }): Promise<void> {
|
||||
await this.request("PUT", "/api/v1/github/mock/auth-health", data);
|
||||
}
|
||||
|
||||
async mockGitHubGetStatus(): Promise<{
|
||||
authenticated: boolean;
|
||||
username: string;
|
||||
|
||||
@@ -428,6 +428,88 @@ export class SessionPage {
|
||||
return this.page.getByTestId("pr-detail-panel");
|
||||
}
|
||||
|
||||
// --- CI hover popover accessors (kept here so the spec stays declarative) ---
|
||||
|
||||
/** The single-PR hover popover content (visible after hovering the topbar button). */
|
||||
prTopbarPopover(): Locator {
|
||||
return this.page.getByTestId("pr-topbar-popover");
|
||||
}
|
||||
|
||||
/** Multi-PR aggregate popover content. */
|
||||
prTopbarPopoverAggregate(): Locator {
|
||||
return this.page.getByTestId("pr-topbar-popover-aggregate");
|
||||
}
|
||||
|
||||
/** A specific bucket group inside the popover by kind. */
|
||||
prCheckGroup(kind: "passed" | "in_progress" | "failed"): Locator {
|
||||
return this.prTopbarPopover().locator(`[data-testid='pr-check-group'][data-kind='${kind}']`);
|
||||
}
|
||||
|
||||
/** Count number rendered inside a bucket group's header. */
|
||||
prCheckGroupCount(kind: "passed" | "in_progress" | "failed"): Locator {
|
||||
return this.prCheckGroup(kind).getByTestId("pr-check-group-count");
|
||||
}
|
||||
|
||||
/** A workflow row by its workflow name (the part before " / " in CheckRun.name). */
|
||||
prWorkflowRow(workflow: string): Locator {
|
||||
return this.prTopbarPopover().locator(
|
||||
`[data-testid='pr-workflow-row'][data-workflow='${workflow}']`,
|
||||
);
|
||||
}
|
||||
|
||||
/** Open-on-GitHub button inside a workflow row. */
|
||||
prWorkflowOpenButton(workflow: string): Locator {
|
||||
return this.prWorkflowRow(workflow).getByTestId("pr-workflow-open");
|
||||
}
|
||||
|
||||
/** "+ ctx" button inside a (failed) workflow row. */
|
||||
prWorkflowAddContextButton(workflow: string): Locator {
|
||||
return this.prWorkflowRow(workflow).getByTestId("pr-workflow-add-context");
|
||||
}
|
||||
|
||||
/** Review state line ("Approved 1 / 2 required" etc.). */
|
||||
prReviewRow(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-review-row");
|
||||
}
|
||||
|
||||
/** Unresolved-comments row inside the popover. */
|
||||
prCommentsRow(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-comments-row");
|
||||
}
|
||||
|
||||
/** Header external-link icon (top-right corner of the popover). */
|
||||
prPopoverExternalLink(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-popover-external-link");
|
||||
}
|
||||
|
||||
/** Footer "updated Ns ago" timestamp text. */
|
||||
prPopoverUpdatedAt(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-popover-updated-at");
|
||||
}
|
||||
|
||||
/** Empty-state row when the PR has no checks yet. */
|
||||
prChecksEmpty(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-checks-empty");
|
||||
}
|
||||
|
||||
/** "Reconnect GitHub" link rendered when auth health is unhealthy. */
|
||||
prPopoverReconnectLink(): Locator {
|
||||
return this.prTopbarPopover().getByTestId("pr-popover-reconnect-link");
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the popover by hovering the topbar button. Waits for the open delay
|
||||
* (~150ms in PRTopbarButton) plus a small buffer.
|
||||
*
|
||||
* To keep the popover open while interacting with rows, the test should
|
||||
* hover the popover content directly afterwards (Playwright hover() over a
|
||||
* row inside the popover keeps the cursor in the open region).
|
||||
*/
|
||||
async hoverPRTopbar(): Promise<void> {
|
||||
await this.prTopbarButton().hover();
|
||||
await expect(this.prTopbarPopover()).toBeVisible({ timeout: 5_000 });
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert the `pr-detail` panel's dockview group contains at least one
|
||||
* `session:{sessionId}` panel — i.e. the PR opened as a tab next to a
|
||||
|
||||
@@ -0,0 +1,450 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { KanbanPage } from "../../pages/kanban-page";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import type { ApiClient } from "../../helpers/api-client";
|
||||
|
||||
const OWNER = "acme";
|
||||
const REPO = "demo";
|
||||
const PR_NUMBER = 42;
|
||||
const PR_URL = `https://github.com/${OWNER}/${REPO}/pull/${PR_NUMBER}`;
|
||||
|
||||
type SeedResult = {
|
||||
workflowId: string;
|
||||
inboxStepId: string;
|
||||
workingStepId: string;
|
||||
doneStepId: string;
|
||||
taskId: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Stand up a workspace + workflow + task that reaches the Done column
|
||||
* immediately (auto-start + on_turn_complete moves it). Returns the IDs the
|
||||
* spec needs to seed PR data + open the task.
|
||||
*/
|
||||
async function seedTask(
|
||||
apiClient: ApiClient,
|
||||
workspaceId: string,
|
||||
agentProfileId: string,
|
||||
repositoryId: string,
|
||||
title: string,
|
||||
): Promise<SeedResult> {
|
||||
const workflow = await apiClient.createWorkflow(workspaceId, `${title} Workflow`);
|
||||
const inbox = await apiClient.createWorkflowStep(workflow.id, "Inbox", 0);
|
||||
const working = await apiClient.createWorkflowStep(workflow.id, "Working", 1);
|
||||
const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2);
|
||||
|
||||
await apiClient.updateWorkflowStep(working.id, {
|
||||
prompt: 'e2e:message("done")\n{{task_prompt}}',
|
||||
events: {
|
||||
on_enter: [{ type: "auto_start_agent" }],
|
||||
on_turn_complete: [{ type: "move_to_step", config: { step_id: done.id } }],
|
||||
},
|
||||
});
|
||||
|
||||
await apiClient.saveUserSettings({
|
||||
workspace_id: workspaceId,
|
||||
workflow_filter_id: workflow.id,
|
||||
enable_preview_on_click: false,
|
||||
});
|
||||
|
||||
await apiClient.mockGitHubReset();
|
||||
await apiClient.mockGitHubSetUser("test-user");
|
||||
|
||||
const task = await apiClient.createTask(workspaceId, title, {
|
||||
workflow_id: workflow.id,
|
||||
workflow_step_id: inbox.id,
|
||||
agent_profile_id: agentProfileId,
|
||||
repository_ids: [repositoryId],
|
||||
});
|
||||
|
||||
return {
|
||||
workflowId: workflow.id,
|
||||
inboxStepId: inbox.id,
|
||||
workingStepId: working.id,
|
||||
doneStepId: done.id,
|
||||
taskId: task.id,
|
||||
};
|
||||
}
|
||||
|
||||
async function associatePR(
|
||||
apiClient: ApiClient,
|
||||
taskId: string,
|
||||
overrides:
|
||||
| Parameters<ApiClient["mockGitHubAssociateTaskPR"]>[0]
|
||||
| Partial<Parameters<ApiClient["mockGitHubAssociateTaskPR"]>[0]> = {},
|
||||
) {
|
||||
await apiClient.mockGitHubAssociateTaskPR({
|
||||
task_id: taskId,
|
||||
owner: OWNER,
|
||||
repo: REPO,
|
||||
pr_number: PR_NUMBER,
|
||||
pr_url: PR_URL,
|
||||
pr_title: "Add CI popover",
|
||||
head_branch: "feat/popover",
|
||||
base_branch: "main",
|
||||
author_login: "test-user",
|
||||
state: "open",
|
||||
additions: 100,
|
||||
deletions: 5,
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
async function openTaskAndWait(
|
||||
testPage: import("@playwright/test").Page,
|
||||
apiClient: ApiClient,
|
||||
seed: SeedResult,
|
||||
title: string,
|
||||
): Promise<SessionPage> {
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
await apiClient.moveTask(seed.taskId, seed.workflowId, seed.workingStepId);
|
||||
await expect(kanban.taskCardInColumn(title, seed.doneStepId)).toBeVisible({ timeout: 45_000 });
|
||||
await kanban.taskCardInColumn(title, seed.doneStepId).click();
|
||||
await expect(testPage).toHaveURL(/\/[st]\//, { timeout: 15_000 });
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.prTopbarButton()).toBeVisible({ timeout: 15_000 });
|
||||
return session;
|
||||
}
|
||||
|
||||
test.describe("PR top-bar CI popover", () => {
|
||||
test("counts row uses TaskPR aggregates and hides zero-count buckets", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Counts Aggregates";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 27,
|
||||
checks_passing: 22,
|
||||
review_state: "approved",
|
||||
review_count: 1,
|
||||
pending_review_count: 0,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
|
||||
await expect(session.prCheckGroupCount("passed")).toHaveText("22");
|
||||
await expect(session.prCheckGroupCount("failed")).toHaveText("5");
|
||||
// No in-progress group when checks_state=failure with full passing count.
|
||||
await expect(session.prCheckGroup("in_progress")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("shows only Passed when all checks succeed (in_progress + failed hidden)", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "All Passed";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "success",
|
||||
checks_total: 22,
|
||||
checks_passing: 22,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
|
||||
await expect(session.prCheckGroup("passed")).toBeVisible();
|
||||
await expect(session.prCheckGroupCount("passed")).toHaveText("22");
|
||||
await expect(session.prCheckGroup("in_progress")).toHaveCount(0);
|
||||
await expect(session.prCheckGroup("failed")).toHaveCount(0);
|
||||
// Passed group is header-only — no workflow rows beneath it.
|
||||
await expect(session.prCheckGroup("passed").getByTestId("pr-workflow-row")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("workflow rollup groups jobs and shows (N/M passed) badge under Failed", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Workflow Rollup";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 6,
|
||||
checks_passing: 4,
|
||||
});
|
||||
await apiClient.mockGitHubSeedPRFeedback({
|
||||
owner: OWNER,
|
||||
repo: REPO,
|
||||
pr_number: PR_NUMBER,
|
||||
checks: [
|
||||
{ name: "Lint / check", status: "completed", conclusion: "failure", html_url: "" },
|
||||
{ name: "E2E / a", status: "completed", conclusion: "failure", html_url: "" },
|
||||
{ name: "E2E / b", status: "completed", conclusion: "success", html_url: "" },
|
||||
{ name: "E2E / c", status: "completed", conclusion: "success", html_url: "" },
|
||||
{ name: "E2E / d", status: "completed", conclusion: "success", html_url: "" },
|
||||
{ name: "E2E / e", status: "completed", conclusion: "success", html_url: "" },
|
||||
],
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
|
||||
await expect(session.prWorkflowRow("Lint")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.prWorkflowRow("E2E")).toBeVisible();
|
||||
await expect(session.prWorkflowRow("Lint")).toContainText("0/1 passed");
|
||||
await expect(session.prWorkflowRow("E2E")).toContainText("4/5 passed");
|
||||
});
|
||||
|
||||
test("review row shows N / M required + unresolved comments count", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Review + Comments";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
review_state: "approved",
|
||||
review_count: 1,
|
||||
pending_review_count: 0,
|
||||
required_reviews: 2,
|
||||
unresolved_review_threads: 3,
|
||||
checks_state: "success",
|
||||
checks_total: 1,
|
||||
checks_passing: 1,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
|
||||
await expect(session.prReviewRow()).toBeVisible();
|
||||
await expect(session.prReviewRow()).toContainText("Approved");
|
||||
await expect(session.prReviewRow()).toContainText("1 / 2");
|
||||
await expect(session.prCommentsRow()).toBeVisible();
|
||||
await expect(session.prCommentsRow()).toContainText("3 unresolved comments");
|
||||
});
|
||||
|
||||
test("comments row hidden when unresolved_review_threads = 0", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "No Unresolved";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
review_state: "approved",
|
||||
review_count: 1,
|
||||
unresolved_review_threads: 0,
|
||||
checks_state: "success",
|
||||
checks_total: 1,
|
||||
checks_passing: 1,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
|
||||
await expect(session.prReviewRow()).toBeVisible();
|
||||
await expect(session.prCommentsRow()).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("hover opens popover; mouse-leave closes it after the close delay", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Hover Open Close";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "success",
|
||||
checks_total: 1,
|
||||
checks_passing: 1,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
|
||||
await session.hoverPRTopbar();
|
||||
// Move the cursor far away from the popover; close timer fires and the
|
||||
// popover unmounts.
|
||||
await testPage.mouse.move(0, 0);
|
||||
await expect(session.prTopbarPopover()).toHaveCount(0, { timeout: 5_000 });
|
||||
});
|
||||
|
||||
test("failed workflow row exposes open + add-to-context buttons", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Failed Affordances";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 1,
|
||||
checks_passing: 0,
|
||||
});
|
||||
await apiClient.mockGitHubSeedPRFeedback({
|
||||
owner: OWNER,
|
||||
repo: REPO,
|
||||
pr_number: PR_NUMBER,
|
||||
checks: [
|
||||
{
|
||||
name: "Lint / check",
|
||||
status: "completed",
|
||||
conclusion: "failure",
|
||||
html_url: "https://example.com/lint-run-1",
|
||||
output: "lint failed",
|
||||
},
|
||||
],
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
// Move into the popover so it stays open while we read the row.
|
||||
await session.prTopbarPopover().hover();
|
||||
|
||||
await expect(session.prWorkflowRow("Lint")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.prWorkflowOpenButton("Lint")).toBeVisible();
|
||||
await expect(session.prWorkflowAddContextButton("Lint")).toBeVisible();
|
||||
});
|
||||
|
||||
test("header external link points at the PR's GitHub /checks tab", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "External Link";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "success",
|
||||
checks_total: 1,
|
||||
checks_passing: 1,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
await expect(session.prPopoverExternalLink()).toHaveAttribute("href", `${PR_URL}/checks`);
|
||||
});
|
||||
|
||||
test("empty state when PR has no checks at all", async ({ testPage, apiClient, seedData }) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "No Checks Yet";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "",
|
||||
checks_total: 0,
|
||||
checks_passing: 0,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
await expect(session.prChecksEmpty()).toBeVisible();
|
||||
await expect(session.prChecksEmpty()).toContainText("No checks have started");
|
||||
});
|
||||
|
||||
test("auth lost surfaces a Reconnect GitHub link", async ({ testPage, apiClient, seedData }) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Reconnect GitHub";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await apiClient.mockGitHubSetAuthHealth({
|
||||
authenticated: false,
|
||||
error: "token expired",
|
||||
});
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 2,
|
||||
checks_passing: 1,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
await expect(session.prPopoverReconnectLink()).toBeVisible();
|
||||
await expect(session.prPopoverReconnectLink()).toHaveAttribute("href", /settings/);
|
||||
});
|
||||
|
||||
test("counts update on a re-associate (live refresh)", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
const title = "Live Refresh";
|
||||
const seed = await seedTask(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
title,
|
||||
);
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 27,
|
||||
checks_passing: 22,
|
||||
});
|
||||
const session = await openTaskAndWait(testPage, apiClient, seed, title);
|
||||
await session.hoverPRTopbar();
|
||||
await session.prTopbarPopover().hover();
|
||||
await expect(session.prCheckGroupCount("passed")).toHaveText("22");
|
||||
|
||||
// Re-POST with a new passing count to simulate a CI tick.
|
||||
await associatePR(apiClient, seed.taskId, {
|
||||
checks_state: "failure",
|
||||
checks_total: 27,
|
||||
checks_passing: 25,
|
||||
});
|
||||
await expect(session.prCheckGroupCount("passed")).toHaveText("25", { timeout: 5_000 });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,104 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { getPRFeedback } from "@/lib/api/domains/github-api";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import type { PRFeedback, TaskPR } from "@/lib/types/github";
|
||||
|
||||
export function prFeedbackKey(pr: { owner: string; repo: string; pr_number: number }): string {
|
||||
return `${pr.owner}/${pr.repo}#${pr.pr_number}`;
|
||||
}
|
||||
|
||||
type Result = {
|
||||
/** Last cached PRFeedback (may be stale while a refetch is in flight). */
|
||||
feedback: PRFeedback | null;
|
||||
/** True while a fetch is in flight. Drives skeleton loading in PRCheckGroup. */
|
||||
isFetching: boolean;
|
||||
/** Wallclock ms when the cache entry was last updated. */
|
||||
lastUpdatedAt: number | null;
|
||||
/** Trigger a refetch immediately (used as a hover-open safety net). */
|
||||
refetch: () => void;
|
||||
};
|
||||
|
||||
/**
|
||||
* Internal: fetch + cache one PR's feedback. Used by the background-sync hook
|
||||
* (always-on, mounted at the top-bar button) and by the popover hook (gated
|
||||
* on hover-open). Keeping the fetch logic shared means the request-counter
|
||||
* dedup is preserved across both call sites.
|
||||
*/
|
||||
function useFeedbackFetch(pr: TaskPR | null) {
|
||||
const setEntry = useAppStore((state) => state.setPRFeedbackCacheEntry);
|
||||
const [isFetching, setIsFetching] = useState(false);
|
||||
const requestRef = useRef(0);
|
||||
const refetch = useCallback(() => {
|
||||
if (!pr) return;
|
||||
const requestId = ++requestRef.current;
|
||||
setIsFetching(true);
|
||||
getPRFeedback(pr.owner, pr.repo, pr.pr_number, { cache: "no-store" })
|
||||
.then((response) => {
|
||||
if (requestRef.current !== requestId) return;
|
||||
if (response) setEntry(prFeedbackKey(pr), response);
|
||||
})
|
||||
.catch(() => {
|
||||
// Swallow errors — the popover keeps showing the stale cached value
|
||||
// (stale-while-revalidate). A future refetch may succeed.
|
||||
})
|
||||
.finally(() => {
|
||||
if (requestRef.current === requestId) setIsFetching(false);
|
||||
});
|
||||
}, [pr, setEntry]);
|
||||
return { refetch, isFetching };
|
||||
}
|
||||
|
||||
/**
|
||||
* Always-on background sync for the active task's PR. Mounted at the
|
||||
* PRTopbarButton so the popover cache stays fresh at the same cadence as
|
||||
* the button icon: every time `pr.updated_at` changes (the WS push that
|
||||
* already drives the icon color), refetch PRFeedback into the cache.
|
||||
*
|
||||
* Without this, hover-open had to wait for the on-demand fetch to land
|
||||
* before showing fresh data — the user sees a stale popover for ~150ms
|
||||
* + network latency.
|
||||
*/
|
||||
export function usePRFeedbackBackgroundSync(pr: TaskPR | null): void {
|
||||
const { refetch } = useFeedbackFetch(pr);
|
||||
// Compound the cache key with the timestamp so that switching the active
|
||||
// task to a different PR (different key) always refetches even when the
|
||||
// two PRs happen to share the same updated_at string. Tracking
|
||||
// updated_at alone would silently skip the new PR's first fetch.
|
||||
const syncKey = pr ? `${prFeedbackKey(pr)}@${pr.updated_at}` : null;
|
||||
const lastSyncedRef = useRef<string | null>(null);
|
||||
useEffect(() => {
|
||||
if (syncKey == null) return;
|
||||
if (lastSyncedRef.current === syncKey) return;
|
||||
lastSyncedRef.current = syncKey;
|
||||
queueMicrotask(refetch);
|
||||
}, [syncKey, refetch]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Popover-side reader: returns the cached feedback + fires an on-demand
|
||||
* refetch whenever the popover transitions from closed to open. The
|
||||
* background-sync hook keeps the cache fresh in the meantime, so this
|
||||
* mostly serves as a safety net for the very first hover (before any
|
||||
* sync has fired).
|
||||
*/
|
||||
export function usePRCIPopover(pr: TaskPR | null, enabled: boolean): Result {
|
||||
const key = pr ? prFeedbackKey(pr) : null;
|
||||
const cached = useAppStore((state) => (key ? (state.prFeedbackCache.byKey[key] ?? null) : null));
|
||||
const { refetch, isFetching } = useFeedbackFetch(pr);
|
||||
|
||||
const wasEnabledRef = useRef(false);
|
||||
useEffect(() => {
|
||||
const opened = enabled && !wasEnabledRef.current;
|
||||
wasEnabledRef.current = enabled;
|
||||
if (opened) queueMicrotask(refetch);
|
||||
}, [enabled, refetch]);
|
||||
|
||||
return {
|
||||
feedback: cached?.feedback ?? null,
|
||||
isFetching,
|
||||
lastUpdatedAt: cached?.lastUpdatedAt ?? null,
|
||||
refetch,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,147 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { bucketCheck, bucketCheckCounts, groupChecksByWorkflow } from "./check-buckets";
|
||||
import type { CheckRun } from "@/lib/types/github";
|
||||
|
||||
function makeCheck(overrides: Partial<CheckRun> = {}): CheckRun {
|
||||
return {
|
||||
name: "Test / unit",
|
||||
source: "check_run",
|
||||
status: "completed",
|
||||
conclusion: "success",
|
||||
html_url: "https://example.com/checks/1",
|
||||
output: "",
|
||||
started_at: null,
|
||||
completed_at: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("bucketCheck", () => {
|
||||
it("buckets success and neutral as Passed", () => {
|
||||
expect(bucketCheck(makeCheck({ conclusion: "success" }))).toBe("passed");
|
||||
expect(bucketCheck(makeCheck({ conclusion: "neutral" }))).toBe("passed");
|
||||
});
|
||||
|
||||
it("buckets failure / cancelled / timed_out / action_required as Failed", () => {
|
||||
expect(bucketCheck(makeCheck({ conclusion: "failure" }))).toBe("failed");
|
||||
expect(bucketCheck(makeCheck({ conclusion: "cancelled" }))).toBe("failed");
|
||||
expect(bucketCheck(makeCheck({ conclusion: "timed_out" }))).toBe("failed");
|
||||
expect(bucketCheck(makeCheck({ conclusion: "action_required" }))).toBe("failed");
|
||||
});
|
||||
|
||||
it("buckets queued / in_progress as In Progress (status takes priority)", () => {
|
||||
expect(bucketCheck(makeCheck({ status: "queued", conclusion: "" }))).toBe("in_progress");
|
||||
expect(bucketCheck(makeCheck({ status: "in_progress", conclusion: "" }))).toBe("in_progress");
|
||||
// Even if a stale conclusion is present, status overrides:
|
||||
expect(bucketCheck(makeCheck({ status: "in_progress", conclusion: "failure" }))).toBe(
|
||||
"in_progress",
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores skipped and stale", () => {
|
||||
expect(bucketCheck(makeCheck({ conclusion: "skipped" }))).toBeNull();
|
||||
expect(bucketCheck(makeCheck({ conclusion: "stale" }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("bucketCheckCounts", () => {
|
||||
it("counts each bucket independently and ignores skipped/stale", () => {
|
||||
const checks = [
|
||||
makeCheck({ conclusion: "success" }),
|
||||
makeCheck({ conclusion: "neutral" }),
|
||||
makeCheck({ conclusion: "cancelled" }),
|
||||
makeCheck({ conclusion: "timed_out" }),
|
||||
makeCheck({ conclusion: "action_required" }),
|
||||
makeCheck({ status: "in_progress", conclusion: "" }),
|
||||
makeCheck({ conclusion: "skipped" }),
|
||||
makeCheck({ conclusion: "stale" }),
|
||||
];
|
||||
expect(bucketCheckCounts(checks)).toEqual({ passed: 2, inProgress: 1, failed: 3 });
|
||||
});
|
||||
});
|
||||
|
||||
describe("groupChecksByWorkflow", () => {
|
||||
it("groups by the part before ' / '", () => {
|
||||
const checks = [
|
||||
makeCheck({ name: "Test / unit", conclusion: "success" }),
|
||||
makeCheck({ name: "Test / e2e", conclusion: "failure" }),
|
||||
makeCheck({ name: "Lint / check", conclusion: "failure" }),
|
||||
];
|
||||
const groups = groupChecksByWorkflow(checks);
|
||||
expect(groups.map((g) => g.workflow)).toEqual(["Test", "Lint"]);
|
||||
const test = groups.find((g) => g.workflow === "Test")!;
|
||||
expect(test.passed).toBe(1);
|
||||
expect(test.failed).toBe(1);
|
||||
expect(test.bucket).toBe("failed");
|
||||
});
|
||||
|
||||
it("treats a name without ' / ' as a single-row workflow", () => {
|
||||
const checks = [makeCheck({ name: "vercel", source: "status_context", conclusion: "success" })];
|
||||
const groups = groupChecksByWorkflow(checks);
|
||||
expect(groups).toHaveLength(1);
|
||||
expect(groups[0].workflow).toBe("vercel");
|
||||
expect(groups[0].bucket).toBe("passed");
|
||||
});
|
||||
|
||||
it("group bucket prioritizes failed > in_progress > passed", () => {
|
||||
const checks = [
|
||||
makeCheck({ name: "Test / a", conclusion: "success" }),
|
||||
makeCheck({ name: "Test / b", status: "in_progress", conclusion: "" }),
|
||||
makeCheck({ name: "Test / c", conclusion: "failure" }),
|
||||
];
|
||||
const [group] = groupChecksByWorkflow(checks);
|
||||
expect(group.bucket).toBe("failed");
|
||||
expect(group.passed).toBe(1);
|
||||
expect(group.inProgress).toBe(1);
|
||||
expect(group.failed).toBe(1);
|
||||
expect(group.total).toBe(3);
|
||||
});
|
||||
|
||||
it("computes (N/M passed) badge components correctly when no failures", () => {
|
||||
const checks = [
|
||||
makeCheck({ name: "Build / linux", status: "in_progress", conclusion: "" }),
|
||||
makeCheck({ name: "Build / mac", conclusion: "success" }),
|
||||
];
|
||||
const [group] = groupChecksByWorkflow(checks);
|
||||
expect(group.bucket).toBe("in_progress");
|
||||
expect(group.passed).toBe(1);
|
||||
expect(group.inProgress).toBe(1);
|
||||
expect(group.total).toBe(2);
|
||||
});
|
||||
|
||||
it("htmlUrl points to first job matching the group's bucket", () => {
|
||||
// Regression for the URL-selection fix in check-buckets.ts: a failed
|
||||
// workflow row should link to a failing job's log, not the first
|
||||
// (potentially passing) job's URL.
|
||||
const checks = [
|
||||
makeCheck({ name: "Build / a", conclusion: "success", html_url: "https://ok" }),
|
||||
makeCheck({ name: "Build / b", conclusion: "failure", html_url: "https://bad" }),
|
||||
makeCheck({ name: "Build / c", conclusion: "failure", html_url: "https://bad-2" }),
|
||||
];
|
||||
const [group] = groupChecksByWorkflow(checks);
|
||||
expect(group.bucket).toBe("failed");
|
||||
// Picks the first failing job's URL.
|
||||
expect(group.htmlUrl).toBe("https://bad");
|
||||
});
|
||||
|
||||
it("htmlUrl falls back to first job's URL when no bucket-match has html_url", () => {
|
||||
const checks = [
|
||||
makeCheck({ name: "Build / a", conclusion: "success", html_url: "https://ok-1" }),
|
||||
makeCheck({ name: "Build / b", conclusion: "success", html_url: "https://ok-2" }),
|
||||
];
|
||||
const [group] = groupChecksByWorkflow(checks);
|
||||
expect(group.bucket).toBe("passed");
|
||||
expect(group.htmlUrl).toBe("https://ok-1");
|
||||
});
|
||||
|
||||
it("ignores skipped jobs in totals", () => {
|
||||
const checks = [
|
||||
makeCheck({ name: "Test / a", conclusion: "success" }),
|
||||
makeCheck({ name: "Test / b", conclusion: "skipped" }),
|
||||
];
|
||||
const [group] = groupChecksByWorkflow(checks);
|
||||
expect(group.total).toBe(1);
|
||||
expect(group.passed).toBe(1);
|
||||
expect(group.bucket).toBe("passed");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,126 @@
|
||||
import type { CheckRun } from "@/lib/types/github";
|
||||
|
||||
export type CheckBucket = "passed" | "in_progress" | "failed";
|
||||
|
||||
/**
|
||||
* Bucket a CheckRun's status/conclusion into Passed / In Progress / Failed.
|
||||
* Returns null for skipped/stale checks — they're ignored entirely in the
|
||||
* popover (see Q11). Order of evaluation matters: status comes before
|
||||
* conclusion so a queued/in_progress run never falls through to the
|
||||
* conclusion mapping (which would be the *previous* run's conclusion when
|
||||
* GitHub re-runs a check).
|
||||
*/
|
||||
export function bucketCheck(check: CheckRun): CheckBucket | null {
|
||||
if (check.status === "queued" || check.status === "in_progress") {
|
||||
return "in_progress";
|
||||
}
|
||||
switch (check.conclusion) {
|
||||
case "success":
|
||||
case "neutral":
|
||||
return "passed";
|
||||
case "failure":
|
||||
case "cancelled":
|
||||
case "timed_out":
|
||||
case "action_required":
|
||||
return "failed";
|
||||
case "skipped":
|
||||
case "stale":
|
||||
return null;
|
||||
case "":
|
||||
case undefined:
|
||||
// Completed checks without a conclusion are unusual; treat them as
|
||||
// passed so we don't accidentally flag them as failed in the popover.
|
||||
return check.status === "completed" ? "passed" : "in_progress";
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export type CheckBucketCounts = {
|
||||
passed: number;
|
||||
inProgress: number;
|
||||
failed: number;
|
||||
};
|
||||
|
||||
export function bucketCheckCounts(checks: CheckRun[]): CheckBucketCounts {
|
||||
const counts: CheckBucketCounts = { passed: 0, inProgress: 0, failed: 0 };
|
||||
for (const c of checks) {
|
||||
const b = bucketCheck(c);
|
||||
if (b === "passed") counts.passed++;
|
||||
else if (b === "in_progress") counts.inProgress++;
|
||||
else if (b === "failed") counts.failed++;
|
||||
}
|
||||
return counts;
|
||||
}
|
||||
|
||||
export type WorkflowGroup = {
|
||||
/** Workflow name (the part before " / " in CheckRun.name); falls back to
|
||||
* the full name for status_context entries. */
|
||||
workflow: string;
|
||||
/** Aggregated bucket — matches bucketCheck applied to the worst job. */
|
||||
bucket: CheckBucket;
|
||||
/** All jobs that belong to this workflow, in input order. */
|
||||
jobs: CheckRun[];
|
||||
/** Count of completed jobs whose conclusion is success/neutral. */
|
||||
passed: number;
|
||||
/** Count of jobs currently running/queued. */
|
||||
inProgress: number;
|
||||
/** Count of jobs in the failed bucket. */
|
||||
failed: number;
|
||||
/** Count of jobs we counted toward total (excludes skipped/stale). */
|
||||
total: number;
|
||||
/** First jobs's html_url — workflow-row click uses this when no job-level
|
||||
* rollup link is available. */
|
||||
htmlUrl: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Group CheckRuns by workflow name. A name like "Test / unit" splits to
|
||||
* workflow="Test", job="unit". A name without " / " (e.g. legacy
|
||||
* status_context like "vercel") is treated as a single-row workflow.
|
||||
*
|
||||
* Group bucket priority: failed > in_progress > passed. A workflow with any
|
||||
* failing job is "failed" even if other jobs passed.
|
||||
*/
|
||||
export function groupChecksByWorkflow(checks: CheckRun[]): WorkflowGroup[] {
|
||||
const order: string[] = [];
|
||||
const map = new Map<string, WorkflowGroup>();
|
||||
for (const check of checks) {
|
||||
const sep = check.name.indexOf(" / ");
|
||||
const workflow = sep >= 0 ? check.name.slice(0, sep) : check.name;
|
||||
let group = map.get(workflow);
|
||||
if (!group) {
|
||||
group = {
|
||||
workflow,
|
||||
bucket: "passed",
|
||||
jobs: [],
|
||||
passed: 0,
|
||||
inProgress: 0,
|
||||
failed: 0,
|
||||
total: 0,
|
||||
htmlUrl: check.html_url,
|
||||
};
|
||||
map.set(workflow, group);
|
||||
order.push(workflow);
|
||||
}
|
||||
group.jobs.push(check);
|
||||
const bucket = bucketCheck(check);
|
||||
if (bucket === null) continue;
|
||||
group.total++;
|
||||
if (bucket === "passed") group.passed++;
|
||||
else if (bucket === "in_progress") group.inProgress++;
|
||||
else if (bucket === "failed") group.failed++;
|
||||
}
|
||||
for (const g of map.values()) {
|
||||
if (g.failed > 0) g.bucket = "failed";
|
||||
else if (g.inProgress > 0) g.bucket = "in_progress";
|
||||
else g.bucket = "passed";
|
||||
// Prefer the URL of the first job that matches the group's bucket so
|
||||
// clicking a "failed" workflow row navigates to a failing job, not an
|
||||
// earlier passing one. Falls back to the first job's URL when no
|
||||
// matching job has html_url set.
|
||||
const target = g.jobs.find((j) => bucketCheck(j) === g.bucket && j.html_url);
|
||||
if (target) g.htmlUrl = target.html_url;
|
||||
}
|
||||
return order.map((w) => map.get(w)!);
|
||||
}
|
||||
@@ -8,8 +8,11 @@ export const defaultGitHubState: GitHubSliceState = {
|
||||
reviewWatches: { items: [], loaded: false, loading: false },
|
||||
issueWatches: { items: [], loaded: false, loading: false },
|
||||
actionPresets: { byWorkspaceId: {}, loading: {} },
|
||||
prFeedbackCache: { byKey: {} },
|
||||
};
|
||||
|
||||
const PR_FEEDBACK_CACHE_LIMIT = 20;
|
||||
|
||||
type ImmerSet = Parameters<
|
||||
StateCreator<GitHubSlice, [["zustand/immer", never]], [], GitHubSlice>
|
||||
>[0];
|
||||
@@ -157,6 +160,31 @@ function createActionPresetActions(
|
||||
};
|
||||
}
|
||||
|
||||
function createPRFeedbackCacheActions(
|
||||
set: ImmerSet,
|
||||
): Pick<GitHubSlice, "setPRFeedbackCacheEntry" | "removePRFeedbackCacheEntry"> {
|
||||
return {
|
||||
setPRFeedbackCacheEntry: (key, feedback) =>
|
||||
set((draft) => {
|
||||
draft.prFeedbackCache.byKey[key] = { feedback, lastUpdatedAt: Date.now() };
|
||||
// Bound cache size: drop the oldest entries when over the limit so a
|
||||
// user opening many PRs doesn't grow the slice unboundedly.
|
||||
const entries = Object.entries(draft.prFeedbackCache.byKey);
|
||||
if (entries.length > PR_FEEDBACK_CACHE_LIMIT) {
|
||||
entries.sort((a, b) => a[1].lastUpdatedAt - b[1].lastUpdatedAt);
|
||||
const drop = entries.length - PR_FEEDBACK_CACHE_LIMIT;
|
||||
for (let i = 0; i < drop; i++) {
|
||||
delete draft.prFeedbackCache.byKey[entries[i][0]];
|
||||
}
|
||||
}
|
||||
}),
|
||||
removePRFeedbackCacheEntry: (key) =>
|
||||
set((draft) => {
|
||||
delete draft.prFeedbackCache.byKey[key];
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
function createRateLimitActions(set: ImmerSet): Pick<GitHubSlice, "applyGitHubRateLimitUpdate"> {
|
||||
return {
|
||||
applyGitHubRateLimitUpdate: (update) =>
|
||||
@@ -187,4 +215,5 @@ export const createGitHubSlice: StateCreator<
|
||||
...createWatchActions(set),
|
||||
...createActionPresetActions(set),
|
||||
...createRateLimitActions(set),
|
||||
...createPRFeedbackCacheActions(set),
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
ReviewWatch,
|
||||
IssueWatch,
|
||||
GitHubActionPresets,
|
||||
PRFeedback,
|
||||
} from "@/lib/types/github";
|
||||
|
||||
export type GitHubStatusState = {
|
||||
@@ -42,6 +43,16 @@ export type ActionPresetsState = {
|
||||
loading: Record<string, boolean>;
|
||||
};
|
||||
|
||||
export type PRFeedbackCacheEntry = {
|
||||
feedback: PRFeedback;
|
||||
lastUpdatedAt: number;
|
||||
};
|
||||
|
||||
export type PRFeedbackCacheState = {
|
||||
/** Keyed by `${owner}/${repo}#${pr_number}` so multi-PR tasks coexist. */
|
||||
byKey: Record<string, PRFeedbackCacheEntry>;
|
||||
};
|
||||
|
||||
export type GitHubSliceState = {
|
||||
githubStatus: GitHubStatusState;
|
||||
taskPRs: TaskPRsState;
|
||||
@@ -49,6 +60,7 @@ export type GitHubSliceState = {
|
||||
reviewWatches: ReviewWatchesState;
|
||||
issueWatches: IssueWatchesState;
|
||||
actionPresets: ActionPresetsState;
|
||||
prFeedbackCache: PRFeedbackCacheState;
|
||||
};
|
||||
|
||||
export type GitHubSliceActions = {
|
||||
@@ -72,6 +84,8 @@ export type GitHubSliceActions = {
|
||||
setActionPresets: (workspaceId: string, presets: GitHubActionPresets) => void;
|
||||
setActionPresetsLoading: (workspaceId: string, loading: boolean) => void;
|
||||
applyGitHubRateLimitUpdate: (update: GitHubRateLimitUpdate) => void;
|
||||
setPRFeedbackCacheEntry: (key: string, feedback: PRFeedback) => void;
|
||||
removePRFeedbackCacheEntry: (key: string) => void;
|
||||
};
|
||||
|
||||
export type GitHubSlice = GitHubSliceState & GitHubSliceActions;
|
||||
|
||||
@@ -17,6 +17,7 @@ import type {
|
||||
ReviewWatch as GitHubReviewWatch,
|
||||
IssueWatch as GitHubIssueWatch,
|
||||
GitHubActionPresets,
|
||||
PRFeedback,
|
||||
} from "@/lib/types/github";
|
||||
import type { SystemHealthResponse } from "@/lib/types/health";
|
||||
import type { UISliceActions as UIA } from "./slices/ui/types";
|
||||
@@ -218,6 +219,7 @@ export type AppState = {
|
||||
reviewWatches: (typeof defaultGitHubState)["reviewWatches"];
|
||||
issueWatches: (typeof defaultGitHubState)["issueWatches"];
|
||||
actionPresets: (typeof defaultGitHubState)["actionPresets"];
|
||||
prFeedbackCache: (typeof defaultGitHubState)["prFeedbackCache"];
|
||||
|
||||
// JIRA slice
|
||||
jiraIssueWatches: (typeof defaultJiraState)["jiraIssueWatches"];
|
||||
@@ -265,6 +267,8 @@ export type AppState = {
|
||||
setActionPresets: (workspaceId: string, presets: GitHubActionPresets) => void;
|
||||
setActionPresetsLoading: (workspaceId: string, loading: boolean) => void;
|
||||
applyGitHubRateLimitUpdate: (update: GitHubRateLimitUpdate) => void;
|
||||
setPRFeedbackCacheEntry: (key: string, feedback: PRFeedback) => void;
|
||||
removePRFeedbackCacheEntry: (key: string) => void;
|
||||
|
||||
// JIRA actions
|
||||
setJiraIssueWatches: (watches: JiraIssueWatch[]) => void;
|
||||
|
||||
@@ -154,7 +154,16 @@ export type TaskPR = {
|
||||
mergeable_state: MergeableState;
|
||||
review_count: number;
|
||||
pending_review_count: number;
|
||||
/** Number of approving reviews required by the base branch protection rule.
|
||||
* Null when no protection rule exists or the token lacks scope to read it. */
|
||||
required_reviews?: number | null;
|
||||
comment_count: number;
|
||||
/** Count of unresolved review threads. Surfaced in the CI hover popover. */
|
||||
unresolved_review_threads: number;
|
||||
/** Aggregate check counts. Used by the CI hover popover to render the
|
||||
* Passed/Failed/In-Progress count rows before the lazy PRFeedback loads. */
|
||||
checks_total: number;
|
||||
checks_passing: number;
|
||||
additions: number;
|
||||
deletions: number;
|
||||
created_at: string;
|
||||
|
||||
Reference in New Issue
Block a user