fix: preserve PR status colors across reload (#3454)
This commit is contained in:
@@ -146,18 +146,21 @@ func pullRequestAggregateState(pr pullRequestObservation) string {
|
||||
if lifecycle := pullRequestLifecycleState(state, mergeable); lifecycle != "" {
|
||||
return lifecycle
|
||||
}
|
||||
if mergeable == prStateBlocked || mergeable == prStateDirty {
|
||||
return prStateBlocked
|
||||
}
|
||||
if pullRequestHasFailure(pr, review, checks) {
|
||||
return prStateFailure
|
||||
}
|
||||
if pullRequestHasPendingChecks(pr, checks) {
|
||||
return prStatePending
|
||||
}
|
||||
if pullRequestAwaitsReview(pr, review) {
|
||||
if pullRequestAwaitsReview(pr, review, checks) {
|
||||
return prStateAwaiting
|
||||
}
|
||||
if review == prStatePending {
|
||||
return prStatePending
|
||||
}
|
||||
if mergeable == prStateBlocked || mergeable == prStateDirty {
|
||||
return prStateBlocked
|
||||
}
|
||||
if review == prStateApproved && (checks == "" || checks == prStateSuccess) {
|
||||
return prStateReady
|
||||
}
|
||||
@@ -192,19 +195,29 @@ func pullRequestHasPendingChecks(pr pullRequestObservation, checks string) bool
|
||||
(pr.checksTotal > 0 && pr.checksPassing < pr.checksTotal && checks != prStateSuccess)
|
||||
}
|
||||
|
||||
func pullRequestAwaitsReview(pr pullRequestObservation, review string) bool {
|
||||
func pullRequestAwaitsReview(pr pullRequestObservation, review, checks string) bool {
|
||||
if !pullRequestChecksPassed(pr, checks) {
|
||||
return false
|
||||
}
|
||||
return pr.pendingReviewCount > 0 || (pr.requiredReviews > 0 && review == prStatePending)
|
||||
}
|
||||
|
||||
func pullRequestChecksPassed(pr pullRequestObservation, checks string) bool {
|
||||
if checks == prStateSuccess {
|
||||
return true
|
||||
}
|
||||
return checks == "" && pr.checksTotal > 0 && pr.checksPassing >= pr.checksTotal
|
||||
}
|
||||
|
||||
func pullRequestStateRank(state string) int {
|
||||
switch state {
|
||||
case prStateFailure:
|
||||
return 100
|
||||
case prStateBlocked:
|
||||
return 90
|
||||
case prStatePending:
|
||||
return 80
|
||||
return 90
|
||||
case prStateAwaiting:
|
||||
return 80
|
||||
case prStateBlocked:
|
||||
return 70
|
||||
case prStateReady:
|
||||
return 60
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package statussummary
|
||||
|
||||
import "testing"
|
||||
|
||||
// @covers AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.1
|
||||
// @covers AC-UI-PR-TASK-STATUS-SUMMARY-001.5
|
||||
func TestPullRequestAggregateStatePrecedence(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
pr PullRequestInput
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "pending CI outranks blocked mergeability",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ChecksState: prStatePending,
|
||||
MergeableState: prStateBlocked,
|
||||
},
|
||||
want: prStatePending,
|
||||
},
|
||||
{
|
||||
name: "failed CI outranks blocked mergeability",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ChecksState: prStateFailure,
|
||||
MergeableState: prStateBlocked,
|
||||
},
|
||||
want: prStateFailure,
|
||||
},
|
||||
{
|
||||
name: "pending review outranks blocked mergeability",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ReviewState: prStatePending,
|
||||
ChecksState: prStateSuccess,
|
||||
MergeableState: prStateBlocked,
|
||||
RequiredReviews: 2,
|
||||
PendingReviewCount: 1,
|
||||
},
|
||||
want: prStateAwaiting,
|
||||
},
|
||||
{
|
||||
name: "blocked mergeability remains visible after CI passes",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ChecksState: prStateSuccess,
|
||||
MergeableState: prStateBlocked,
|
||||
},
|
||||
want: prStateBlocked,
|
||||
},
|
||||
{
|
||||
name: "pending review without CI signal remains yellow",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ReviewState: prStatePending,
|
||||
MergeableState: prStateBlocked,
|
||||
},
|
||||
want: prStatePending,
|
||||
},
|
||||
{
|
||||
name: "dirty mergeability does not hide pending CI",
|
||||
pr: PullRequestInput{
|
||||
State: prStateOpen,
|
||||
ChecksState: prStatePending,
|
||||
MergeableState: prStateDirty,
|
||||
},
|
||||
want: prStatePending,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
got := BuildFromAuthoritative(RebuildInput{
|
||||
PRObserved: true,
|
||||
PullRequests: []PullRequestInput{test.pr},
|
||||
})
|
||||
if got.PullRequest == nil {
|
||||
t.Fatal("pull request summary is nil")
|
||||
}
|
||||
if got.PullRequest.AggregateState != test.want {
|
||||
t.Fatalf("aggregate state = %q, want %q", got.PullRequest.AggregateState, test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-UI-PR-TASK-STATUS-SUMMARY-001.5
|
||||
// @covers AC-UI-PR-TASK-STATUS-SUMMARY-001.8
|
||||
func TestPullRequestAggregateStateAcrossPullRequests(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
prs []PullRequestInput
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "pending outranks blocked",
|
||||
prs: []PullRequestInput{
|
||||
{Key: "pending", State: prStateOpen, ChecksState: prStatePending},
|
||||
{Key: "blocked", State: prStateOpen, ChecksState: prStateSuccess, MergeableState: prStateBlocked},
|
||||
},
|
||||
want: prStatePending,
|
||||
},
|
||||
{
|
||||
name: "awaiting review outranks blocked",
|
||||
prs: []PullRequestInput{
|
||||
{
|
||||
Key: "awaiting",
|
||||
State: prStateOpen,
|
||||
ReviewState: prStatePending,
|
||||
ChecksState: prStateSuccess,
|
||||
PendingReviewCount: 1,
|
||||
},
|
||||
{Key: "blocked", State: prStateOpen, ChecksState: prStateSuccess, MergeableState: prStateBlocked},
|
||||
},
|
||||
want: prStateAwaiting,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
got := BuildFromAuthoritative(RebuildInput{
|
||||
PRObserved: true,
|
||||
PullRequests: test.prs,
|
||||
})
|
||||
if got.PullRequest == nil {
|
||||
t.Fatal("pull request summary is nil")
|
||||
}
|
||||
if got.PullRequest.AggregateState != test.want {
|
||||
t.Fatalf("aggregate state = %q, want %q", got.PullRequest.AggregateState, test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -32,6 +32,7 @@ import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { RemoteCloudTooltip } from "@/components/task/remote-cloud-tooltip";
|
||||
import { useTaskPendingInput } from "@/hooks/use-task-pending-input";
|
||||
import { createDebugLogger, isDebug } from "@/lib/debug/log";
|
||||
import { taskPRInfoFromSummary } from "@/lib/task-pr-info";
|
||||
import {
|
||||
getTaskStateIcon,
|
||||
shouldShowTaskRunningSpinner,
|
||||
@@ -92,7 +93,7 @@ export function KanbanCardBody({
|
||||
<div className="flex items-center gap-1 min-w-0" data-testid="kanban-card-title-row">
|
||||
<CardTitle task={task} enableTitleHover={enableTitleHover} />
|
||||
<KanbanCardPriorityIndicator priority={task.priority} />
|
||||
<PRTaskIcon taskId={task.id} />
|
||||
<PRTaskIcon taskId={task.id} prInfo={taskPRInfoFromSummary(task.statusSummary)} />
|
||||
<MRTaskIcon taskId={task.id} />
|
||||
<RegisteredChangeRequestTaskIcon taskId={task.id} />
|
||||
<TaskCardIndicators task={task} />
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
} from "@/components/task/task-external-link-dialog";
|
||||
import type { KanbanExternalLinkAvailability } from "./kanban-external-link-availability";
|
||||
import type { TaskDependencyRef } from "@/lib/state/slices/kanban/types";
|
||||
import type { TaskStatusSummary } from "@/lib/types/task-status-summary";
|
||||
import { TaskGitHubIssueDialog } from "@/components/task/task-github-issue-dialog";
|
||||
import { TaskGitHubPRDialog } from "@/components/task/task-github-pr-dialog";
|
||||
import { TaskMRLinkDialog } from "@/components/gitlab/task-mr-link-dialog";
|
||||
@@ -107,6 +108,7 @@ export interface Task {
|
||||
queuedAt?: string;
|
||||
issueUrl?: string;
|
||||
issueNumber?: number;
|
||||
statusSummary?: TaskStatusSummary | null;
|
||||
}
|
||||
|
||||
export type RepositoryChip = {
|
||||
|
||||
@@ -6,7 +6,7 @@ const NAVIGATION_TITLE = "PR summary navigation";
|
||||
const TARGET_TITLE = "Inactive PR summary target";
|
||||
|
||||
test.describe("inactive task PR summary hydration", () => {
|
||||
test("loads full PR details on hover and reuses the settled task cache", async ({
|
||||
test("keeps pending CI yellow across reload and full PR hydration", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
@@ -49,11 +49,11 @@ test.describe("inactive task PR summary hydration", () => {
|
||||
author_login: "persisted-author",
|
||||
state: "open",
|
||||
review_state: "approved",
|
||||
checks_state: "success",
|
||||
mergeable_state: "clean",
|
||||
checks_state: "pending",
|
||||
mergeable_state: "blocked",
|
||||
required_reviews: 1,
|
||||
review_count: 1,
|
||||
checks_total: 1,
|
||||
checks_total: 2,
|
||||
checks_passing: 1,
|
||||
});
|
||||
|
||||
@@ -78,10 +78,21 @@ test.describe("inactive task PR summary hydration", () => {
|
||||
await expect(icon).toHaveAttribute("data-pr-state", "Open");
|
||||
await expect(icon).toHaveAttribute("data-pr-count", "1");
|
||||
await expect(icon).not.toHaveAttribute("data-pr-ready-to-merge");
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
expect(taskDetailRequests).toBe(0);
|
||||
|
||||
await testPage.reload();
|
||||
await session.waitForLoad();
|
||||
await expect(targetRow).toBeVisible({ timeout: 15_000 });
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
expect(taskDetailRequests).toBe(0);
|
||||
await prCapture.screenshot("desktop-pr-sidebar-pending-ci-reloaded", {
|
||||
caption: "Desktop sidebar keeps pending CI yellow after reload",
|
||||
});
|
||||
|
||||
await icon.hover();
|
||||
await expect.poll(() => taskDetailRequests).toBe(1);
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
|
||||
const summary = testPage
|
||||
.locator(
|
||||
|
||||
@@ -366,6 +366,89 @@ test.describe("PR status badge", () => {
|
||||
await expectTopbarReadyState(testPage, session, "false");
|
||||
});
|
||||
|
||||
test("keeps pending CI yellow on Kanban reload", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
prCapture,
|
||||
}) => {
|
||||
test.setTimeout(120_000);
|
||||
|
||||
const { task, inboxStep } = await seedBadgeTest(
|
||||
apiClient,
|
||||
seedData.workspaceId,
|
||||
seedData.agentProfileId,
|
||||
seedData.repositoryId,
|
||||
"Pending CI Reload Task",
|
||||
);
|
||||
await apiClient.mockGitHubAssociateTaskPR({
|
||||
task_id: task.id,
|
||||
owner: "testorg",
|
||||
repo: "testrepo",
|
||||
pr_number: 105,
|
||||
pr_url: "https://github.com/testorg/testrepo/pull/105",
|
||||
pr_title: "Pending CI with blocked mergeability",
|
||||
head_branch: "fix/pending-ci-reload",
|
||||
base_branch: "main",
|
||||
author_login: "test-user",
|
||||
state: "open",
|
||||
review_state: "approved",
|
||||
checks_state: "pending",
|
||||
mergeable_state: "blocked",
|
||||
checks_total: 2,
|
||||
checks_passing: 1,
|
||||
});
|
||||
await waitForTaskPRFields(apiClient, task.id, {
|
||||
state: "open",
|
||||
review_state: "approved",
|
||||
checks_state: "pending",
|
||||
mergeable_state: "blocked",
|
||||
});
|
||||
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
const card = kanban.taskCardInColumn(task.title, inboxStep.id);
|
||||
await expect(card).toBeVisible();
|
||||
const icon = kanban.board.getByTestId(`pr-task-icon-${task.id}`);
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
|
||||
let releaseWorkspacePRs = () => undefined;
|
||||
const workspacePRsHeld = new Promise<void>((resolve) => {
|
||||
releaseWorkspacePRs = resolve;
|
||||
});
|
||||
let markWorkspacePRsRequested = () => undefined;
|
||||
const workspacePRsRequested = new Promise<void>((resolve) => {
|
||||
markWorkspacePRsRequested = resolve;
|
||||
});
|
||||
let markWorkspacePRsSettled = () => undefined;
|
||||
const workspacePRsSettled = new Promise<void>((resolve) => {
|
||||
markWorkspacePRsSettled = resolve;
|
||||
});
|
||||
await testPage.route("**/api/v1/github/task-prs?workspace_id=*", async (route) => {
|
||||
markWorkspacePRsRequested();
|
||||
const response = await route.fetch();
|
||||
await workspacePRsHeld;
|
||||
await route.fulfill({ response });
|
||||
markWorkspacePRsSettled();
|
||||
});
|
||||
|
||||
try {
|
||||
await testPage.reload();
|
||||
await kanban.board.waitFor({ state: "visible" });
|
||||
await workspacePRsRequested;
|
||||
await expect(card).toBeVisible();
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
await prCapture.screenshot("desktop-kanban-pending-ci-reloaded", {
|
||||
caption: "Desktop Kanban card keeps pending CI yellow after reload",
|
||||
});
|
||||
} finally {
|
||||
releaseWorkspacePRs();
|
||||
}
|
||||
|
||||
await workspacePRsSettled;
|
||||
await expect(icon).toHaveClass(/text-yellow-500/);
|
||||
});
|
||||
|
||||
/**
|
||||
* When reviewers approved, CI passes, and GitHub's mergeable_state is clean,
|
||||
* the badge must show the ready-to-merge state so the user knows the PR
|
||||
|
||||
@@ -5,7 +5,7 @@ import { SessionPage } from "../../pages/session-page";
|
||||
const TARGET_TITLE = "Mobile inactive summary target";
|
||||
|
||||
test.describe("Mobile task status summary", () => {
|
||||
test("updates the native task switcher from bounded status revisions", async ({
|
||||
test("keeps pending CI yellow in native task switcher status updates", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
@@ -35,6 +35,26 @@ test.describe("Mobile task status summary", () => {
|
||||
agentProfileId: seedData.agentProfileId,
|
||||
});
|
||||
|
||||
await apiClient.mockGitHubAssociateTaskPR({
|
||||
workspace_id: seedData.workspaceId,
|
||||
task_id: targetTask.task_id,
|
||||
owner: "kandev-e2e",
|
||||
repo: "mobile-summary-fixtures",
|
||||
pr_number: 43,
|
||||
pr_url: "https://github.test/kandev-e2e/mobile-summary-fixtures/pull/43",
|
||||
pr_title: "Mobile bounded summary fixture",
|
||||
head_branch: "feature/mobile-summary",
|
||||
base_branch: "main",
|
||||
author_login: "e2e",
|
||||
state: "open",
|
||||
review_state: "approved",
|
||||
checks_state: "pending",
|
||||
mergeable_state: "blocked",
|
||||
required_reviews: 1,
|
||||
checks_total: 2,
|
||||
checks_passing: 1,
|
||||
});
|
||||
|
||||
await testPage.goto(`/t/${navTask.task_id}`);
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
@@ -80,30 +100,12 @@ test.describe("Mobile task status summary", () => {
|
||||
});
|
||||
await expect(targetRow.getByTestId("task-agent-error-icon")).toBeVisible({ timeout: 15_000 });
|
||||
|
||||
await apiClient.mockGitHubAssociateTaskPR({
|
||||
workspace_id: seedData.workspaceId,
|
||||
task_id: targetTask.task_id,
|
||||
owner: "kandev-e2e",
|
||||
repo: "mobile-summary-fixtures",
|
||||
pr_number: 43,
|
||||
pr_url: "https://github.test/kandev-e2e/mobile-summary-fixtures/pull/43",
|
||||
pr_title: "Mobile bounded summary fixture",
|
||||
head_branch: "feature/mobile-summary",
|
||||
base_branch: "main",
|
||||
author_login: "e2e",
|
||||
state: "open",
|
||||
review_state: "approved",
|
||||
checks_state: "success",
|
||||
mergeable_state: "clean",
|
||||
required_reviews: 1,
|
||||
checks_total: 1,
|
||||
checks_passing: 1,
|
||||
const prIcon = targetRow.getByTestId(`pr-task-icon-${targetTask.task_id}`);
|
||||
await expect(prIcon).toHaveAttribute("data-pr-state", "Open", { timeout: 15_000 });
|
||||
await expect(prIcon).toHaveClass(/text-yellow-500/);
|
||||
await prCapture.screenshot("mobile-task-switcher-pending-ci", {
|
||||
caption: "Mobile task switcher keeps pending CI yellow after reload",
|
||||
});
|
||||
await expect(targetRow.getByTestId(`pr-task-icon-${targetTask.task_id}`)).toHaveAttribute(
|
||||
"data-pr-state",
|
||||
"open",
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
|
||||
const viewport = testPage.viewportSize();
|
||||
expect(viewport).not.toBeNull();
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
---
|
||||
created: 2026-09-05
|
||||
status: completed
|
||||
requirements:
|
||||
- REQ-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001
|
||||
- REQ-UI-PR-TASK-STATUS-SUMMARY-001
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/bounded-task-status-delivery.md
|
||||
- ../../specs/ui/system-design/pr-task-status-summary.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Preserve PR Status Color Across Reload
|
||||
|
||||
## Overview
|
||||
|
||||
Make the persisted task PR summary use the same attention precedence as the
|
||||
full PR record. An open PR with pending CI and blocked mergeability must remain
|
||||
yellow in the sidebar, Kanban card, and mobile task switcher after a page
|
||||
reload.
|
||||
|
||||
## Confirmed defect
|
||||
|
||||
The full PR renderer gives pending checks a yellow status before it considers
|
||||
blocked branch protection. The backend summary projector returns `blocked`
|
||||
before it evaluates pending checks. The frontend maps that compact aggregate
|
||||
to gray.
|
||||
|
||||
The browser keeps full PR records only in memory. After a page reload, inactive
|
||||
task rows first use the persisted compact summary. This data-source switch
|
||||
exposes the precedence mismatch. The selected task can remain yellow because
|
||||
its full PR record is loaded again.
|
||||
|
||||
The Kanban card also omitted the compact summary when rendering its PR icon.
|
||||
While the full PR request was in flight after reload, the card therefore had no
|
||||
fallback icon state at all.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Make pending CI outrank blocked mergeability in the compact PR aggregate.
|
||||
- Keep failure, pending, and review signals ahead of dirty mergeability.
|
||||
- Keep multi-PR aggregate ranking aligned with the single-PR precedence.
|
||||
- Treat a pending review without a passed CI signal as yellow, matching the
|
||||
full PR renderer.
|
||||
- Preserve terminal, queued, draft, failure, review, ready, and passing rules.
|
||||
- Add focused backend regression coverage for combined PR states.
|
||||
- Pass the persisted compact summary to the Kanban PR icon while full PR data
|
||||
is unavailable.
|
||||
- Keep the preview deploy command compatible with the current default-branch
|
||||
workflow's `--skip-description` flag.
|
||||
- Prove reload parity in the desktop sidebar and Kanban card.
|
||||
- Prove the same compact-summary color in the mobile task switcher.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Persisting full GitHub PR records in the browser.
|
||||
- Changing PR icon artwork, color tokens, tooltip copy, or layouts.
|
||||
- Changing GitHub polling, WebSocket delivery, or full-record hydration.
|
||||
- Adding a new API field or aggregate state.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### Summary precedence
|
||||
|
||||
Add a table-driven test beside the task status summary projector. Start with a
|
||||
case whose checks are pending and whose mergeability is blocked. Assert a
|
||||
`pending` aggregate. Include the adjacent failure case so blocked mergeability
|
||||
cannot hide a stronger failure signal.
|
||||
|
||||
Update `pullRequestAggregateState` in
|
||||
`apps/backend/internal/task/statussummary/projector_pr.go`. Evaluate the
|
||||
stronger check and review signals before the general blocked- or
|
||||
dirty-mergeability fallback. Require passed checks before classifying a PR as
|
||||
awaiting review, and classify a pending review without a passed CI signal as
|
||||
yellow. Align the multi-PR rank table with the same order. Keep terminal and
|
||||
queued states first. Pass the compact task summary through the Kanban card's
|
||||
existing `PRTaskIcon` fallback input. Do not change the aggregate schema or the
|
||||
frontend color map.
|
||||
|
||||
### Desktop browser behavior
|
||||
|
||||
Extend `apps/web/e2e/tests/pr/pr-sidebar-hover-hydration.spec.ts`. Seed an
|
||||
inactive PR with pending checks and blocked mergeability. Assert that its icon
|
||||
is yellow while the row uses the compact summary. Reload the page and assert
|
||||
that the sidebar row remains yellow. Hydrate the full PR record and assert that
|
||||
the color does not change.
|
||||
|
||||
Add the same reload assertion for the board icon in
|
||||
`apps/web/e2e/tests/pr/pr-status-badge.spec.ts`. Scope the locator to the
|
||||
Kanban board so the sidebar copy cannot satisfy the assertion. Hold the full
|
||||
PR response until the compact-summary assertion finishes. Then release it and
|
||||
assert that hydration keeps the icon yellow.
|
||||
|
||||
### Mobile behavior
|
||||
|
||||
Extend `apps/web/e2e/tests/task/mobile-task-status-summary.spec.ts` with the
|
||||
same combined PR state. Open the existing mobile task switcher and assert a
|
||||
yellow icon before full PR hydration.
|
||||
|
||||
This change does not alter mobile structure or interaction. The nearest mobile
|
||||
exemplar remains
|
||||
`apps/web/components/task/mobile/session-task-switcher-sheet.tsx`.
|
||||
|
||||
## Acceptance traceability
|
||||
|
||||
- `AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.1`: the compact snapshot
|
||||
carries the correct visible PR status for desktop and mobile switchers.
|
||||
- `AC-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001.2`: the backend derives the
|
||||
compact value from the authoritative stored PR state.
|
||||
- `AC-UI-PR-TASK-STATUS-SUMMARY-001.5`: existing PR status precedence remains
|
||||
stable across the compact and full representations.
|
||||
- `AC-UI-PR-TASK-STATUS-SUMMARY-001.8`: sidebar, Kanban, and mobile task rows
|
||||
show the same summary state.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Align compact PR status precedence](task-01-align-pr-status-precedence.md)
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test -v -run TestPullRequestAggregateStatePrecedence ./internal/task/statussummary
|
||||
cd apps/backend && go test ./internal/task/statussummary
|
||||
cd apps/web && pnpm e2e:run --project chromium tests/pr/pr-sidebar-hover-hydration.spec.ts -- --grep "keeps pending CI yellow"
|
||||
cd apps/web && pnpm e2e:run --project chromium tests/pr/pr-status-badge.spec.ts -- --grep "keeps pending CI yellow"
|
||||
cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-task-status-summary.spec.ts -- --grep "keeps pending CI yellow"
|
||||
```
|
||||
|
||||
Completed results:
|
||||
|
||||
- Focused projector precedence and multi-PR ranking: 10 tests passed.
|
||||
- Full status summary package: 90 tests passed.
|
||||
- Kanban component suite: 14 tests passed.
|
||||
- Desktop reload regressions: 2 tests passed.
|
||||
- Mobile task-switcher regression: 1 test passed.
|
||||
- Production backend and E2E web builds completed.
|
||||
- Frontend typecheck completed without diagnostics.
|
||||
- `git diff --check` passed.
|
||||
|
||||
## Risks
|
||||
|
||||
- A broad reorder can change unrelated draft, terminal, or queued colors.
|
||||
Table-driven coverage must lock those boundaries before production changes.
|
||||
- Browser coverage can pass on a full PR record and miss the compact path.
|
||||
The test must use an inactive task and assert before hydration.
|
||||
- The summary is persisted. Existing rows receive the repaired aggregate on
|
||||
the next authoritative PR projection.
|
||||
|
||||
## Documentation impact
|
||||
|
||||
No public documentation or ADR change is required. The active requirements
|
||||
already require one status result across summary consumers and full PR data.
|
||||
This work repairs the implementation to match that contract.
|
||||
@@ -0,0 +1,118 @@
|
||||
---
|
||||
id: "01-align-pr-status-precedence"
|
||||
title: "Align compact PR status precedence"
|
||||
status: completed
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-BOUNDED-TASK-STATUS-DELIVERY-001
|
||||
- REQ-UI-PR-TASK-STATUS-SUMMARY-001
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/bounded-task-status-delivery.md
|
||||
- ../../specs/ui/system-design/pr-task-status-summary.md
|
||||
---
|
||||
|
||||
# Task 01: Align Compact PR Status Precedence
|
||||
|
||||
## Acceptance
|
||||
|
||||
- An open PR with pending CI and blocked mergeability has a `pending` compact
|
||||
aggregate and a yellow icon.
|
||||
- A stronger failure signal is not hidden by blocked mergeability.
|
||||
- A stronger failure, pending, or review signal is not hidden by dirty
|
||||
mergeability.
|
||||
- A pending review without a passed CI signal remains yellow.
|
||||
- Pending and awaiting-review PRs outrank blocked PRs when a task has multiple
|
||||
open PRs.
|
||||
- Terminal, queued, draft, review, ready, and passing behavior remains
|
||||
unchanged.
|
||||
- An inactive task shows the same yellow icon in the desktop sidebar, Kanban
|
||||
card, and mobile task switcher after a page reload.
|
||||
- Hydrating the full PR record does not change the icon color.
|
||||
|
||||
## TDD sequence
|
||||
|
||||
1. RED: add table-driven backend cases for pending CI plus blocked
|
||||
mergeability and failure plus blocked mergeability. Run the focused test and
|
||||
record the expected failure.
|
||||
2. RED: extend the existing desktop and mobile Playwright specs with the
|
||||
combined PR state. Confirm that the compact desktop state fails on gray.
|
||||
3. GREEN: reorder only the compact aggregate checks needed to match the
|
||||
existing full PR attention precedence, including dirty mergeability,
|
||||
no-signal pending review, and multi-PR ranking.
|
||||
4. REFACTOR: name the precedence cases clearly and keep the projector branch
|
||||
order readable.
|
||||
5. GREEN: rebuild the production web and backend artifacts through the managed
|
||||
E2E runner. Confirm yellow before reload, after reload, and after hydration.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test -v -run TestPullRequestAggregateStatePrecedence ./internal/task/statussummary
|
||||
cd apps/backend && go test ./internal/task/statussummary
|
||||
cd apps/web && pnpm e2e:run --project chromium tests/pr/pr-sidebar-hover-hydration.spec.ts -- --grep "keeps pending CI yellow"
|
||||
cd apps/web && pnpm e2e:run --project chromium tests/pr/pr-status-badge.spec.ts -- --grep "keeps pending CI yellow"
|
||||
cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-task-status-summary.spec.ts -- --grep "keeps pending CI yellow"
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/task/statussummary/projector_pr.go`
|
||||
- `apps/backend/internal/task/statussummary/projector_pr_precedence_test.go`
|
||||
- `apps/web/components/kanban-card.tsx`
|
||||
- `apps/web/components/kanban-card-content.tsx`
|
||||
- `apps/web/e2e/tests/pr/pr-sidebar-hover-hydration.spec.ts`
|
||||
- `apps/web/e2e/tests/pr/pr-status-badge.spec.ts`
|
||||
- `apps/web/e2e/tests/task/mobile-task-status-summary.spec.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential. The browser assertions depend on the corrected backend summary,
|
||||
and all files describe one precedence rule.
|
||||
|
||||
## Inputs
|
||||
|
||||
- `docs/specs/platform/requirements/bounded-task-status-delivery.md`,
|
||||
especially acceptance criteria 001.1 and 001.2.
|
||||
- `docs/specs/ui/requirements/pr-task-status-summary.md`, especially
|
||||
acceptance criteria 001.5 and 001.8.
|
||||
- `docs/plans/pr-status-summary-color-parity/plan.md`.
|
||||
- The full-record precedence in
|
||||
`apps/web/components/github/pr-task-icon.tsx`.
|
||||
- The compact-summary projection in
|
||||
`apps/backend/internal/task/statussummary/projector_pr.go`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the RED failures, the final precedence order, all changed files, the
|
||||
focused backend and browser results, and the work-order status update. Include
|
||||
the observed icon color before reload, after reload, and after full-record
|
||||
hydration.
|
||||
|
||||
## Result
|
||||
|
||||
The RED projector cases returned `blocked` for pending CI, failed CI, and
|
||||
pending review when mergeability was blocked. The desktop sidebar and native
|
||||
mobile switcher consequently rendered the compact icon gray. The Kanban reload
|
||||
case additionally showed that the card did not pass its compact task summary to
|
||||
the PR icon while full PR hydration was pending.
|
||||
|
||||
The projector now orders failed checks, pending checks, and pending review
|
||||
ahead of general blocked or dirty mergeability. Awaiting-review requires passed
|
||||
checks, while a pending review without a passed CI signal remains yellow. The
|
||||
multi-PR rank table follows the same ordering. Kanban cards pass their
|
||||
persisted task summary through the existing PR icon fallback. The icon is
|
||||
yellow before reload, after reload, and after full-record hydration on the
|
||||
covered desktop and mobile surfaces. The preview deploy command also accepts
|
||||
the `--skip-description` flag used by the current default-branch workflow.
|
||||
|
||||
Verification completed with 10 focused projector cases, all 90 status summary
|
||||
package tests, 14 Kanban component tests, 2 desktop browser regressions, and 1
|
||||
mobile browser regression passing. Backend and web production builds,
|
||||
preview command tests, frontend typecheck, and `git diff --check` also passed
|
||||
before this remediation.
|
||||
Reference in New Issue
Block a user