feat(github): surface pull request merge queue status (#2928)
This commit is contained in:
Carlos Florêncio
@@ -9,9 +9,13 @@ status: complete
|
||||
## Overview
|
||||
|
||||
Extend the existing GitHub merge boundary to use GitHub's asynchronous,
|
||||
queue-aware merge request and return a typed merged-or-queued outcome. Then
|
||||
reuse that outcome in the existing PR merge button across the desktop detail,
|
||||
compact status, and mobile Review surfaces, with targeted unit and E2E proof.
|
||||
queue-aware merge request and return a typed merged-or-queued outcome. Observe
|
||||
the resulting merge-queue entry through the existing GraphQL status sync,
|
||||
persist its state, one-based position, and optional estimated duration, and
|
||||
project it through the existing task and PR status surfaces. Reuse the typed
|
||||
outcome and the persisted status in the existing PR merge button across the
|
||||
desktop detail, compact status, and mobile Review surfaces, with targeted unit
|
||||
and E2E proof.
|
||||
|
||||
## Backend
|
||||
|
||||
@@ -37,6 +41,21 @@ compact status, and mobile Review surfaces, with targeted unit and E2E proof.
|
||||
`{"status":"merged"}` or `{"status":"queued"}` and retain the existing
|
||||
operational-auth and provider-error mapping.
|
||||
|
||||
### Queue observation, persistence, and bounded projection
|
||||
|
||||
- Extend the batched GraphQL pull-request selection and normalized `PRStatus`
|
||||
with `mergeQueueEntry { state position estimatedTimeToMerge }`. A populated
|
||||
GraphQL result replaces all three queue fields atomically; queue-unaware REST
|
||||
and `gh pr view` reads preserve the last complete observation, while an
|
||||
authoritative null or terminal lifecycle state clears it.
|
||||
- Add the queue state, one-based position, and optional estimate in seconds to
|
||||
every `TaskPR` schema, migration, write, restore, and update path. Extend
|
||||
the mock controller so integrated tests can seed the same contract.
|
||||
- Carry queue membership through live and restart-built task status summaries
|
||||
as the bounded `queued` state. Terminal states precede queue membership;
|
||||
queue membership precedes other non-terminal states for one PR, while the
|
||||
existing aggregate ranking keeps a failing sibling above a queued sibling.
|
||||
|
||||
## Frontend
|
||||
|
||||
### Queue-aware merge action
|
||||
@@ -54,6 +73,22 @@ compact status, and mobile Review surfaces, with targeted unit and E2E proof.
|
||||
`apps/web/src/locales/*/github.json`; generate the Traditional Chinese pair
|
||||
through the repository script rather than translating them independently.
|
||||
|
||||
### Queue status color and surfaces
|
||||
|
||||
- Add the normalized queue fields to the web `TaskPR` contract and the shared
|
||||
queued semantic color `text-[#966600]`. Terminal colors remain first; an
|
||||
active queue entry is second, so hydrated failure, draft, dirty, or behind
|
||||
fields cannot overwrite queue presentation. Multi-PR aggregation still ranks
|
||||
a failing sibling above a queued sibling.
|
||||
- Extend the task hover summary, compact desktop popover, phone status drawer,
|
||||
and PR detail panel with translated queue state, one-based position, and an
|
||||
optional estimate rounded up to localized whole minutes. Missing estimates
|
||||
omit only the estimate line. Recognized provider states retain their labels;
|
||||
future non-empty states use generic `Queued` copy.
|
||||
- Keep queue derivation shared across desktop and mobile. Mobile display uses
|
||||
the existing drawer and Review destination; the mobile merge-action test
|
||||
separately verifies the existing 44px action target.
|
||||
|
||||
### Mobile design contract
|
||||
|
||||
- Desktop outcome: the existing PR detail header and compact status surface
|
||||
@@ -92,31 +127,59 @@ compact status, and mobile Review surfaces, with targeted unit and E2E proof.
|
||||
and refresh callback. **File:**
|
||||
`apps/web/components/github/pr-merge-button.test.tsx`. **How:** component tests
|
||||
with mocked API outcomes.
|
||||
- **What:** queue observation, persistence, queue-unaware read preservation,
|
||||
terminal clearing, and bounded live/restart projection. **Files:**
|
||||
`apps/backend/internal/github/*merge_queue_status_test.go`,
|
||||
`apps/backend/internal/github/store_taskpr_schema_drift_test.go`,
|
||||
`apps/backend/internal/task/statussummary/merge_queue_status_test.go`, and
|
||||
`apps/backend/internal/backendapp/status_summary_adapter_test.go`. **How:**
|
||||
focused GraphQL, real-store, and projection tests.
|
||||
- **What:** queue color precedence, failing-sibling aggregation, translated
|
||||
queue metadata, estimate boundaries, terminal queue clearing, and generic
|
||||
fallback copy. **Files:**
|
||||
`apps/web/components/github/pr-task-icon.test.ts`,
|
||||
`pr-task-status-summary.test.ts`, `pr-status-chip.test.tsx`,
|
||||
`pr-merge-queue-status.test.tsx`, `pr-mergeability-row.test.tsx`, and
|
||||
`pr-detail-panel.test.tsx`. **How:** pure formatter and focused component
|
||||
tests for terminal, queued, future-state, missing-estimate, sub-minute, and
|
||||
minute-boundary cases.
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- **Scenario:** eligible direct merge reports merged. **File:**
|
||||
`apps/web/e2e/tests/pr/pr-merge-queue.spec.ts`. **What to verify:** the desktop
|
||||
PR surface exposes the action, submits it, and shows the merged notification.
|
||||
- **Scenario:** queue-required merge reports queued and does not resubmit.
|
||||
**File:** `apps/web/e2e/tests/pr/pr-merge-queue.spec.ts`. **What to verify:**
|
||||
the desktop action remains available for the queue-required state and the
|
||||
accepted queue notification is shown once.
|
||||
- **Scenario:** phone Review surface provides the same queued outcome. **File:**
|
||||
`apps/web/e2e/tests/pr/mobile-pr-merge-queue.spec.ts`. **What to verify:**
|
||||
Review navigation, touch activation, queued notification, a touch-sized
|
||||
action, and no document horizontal overflow.
|
||||
- **Scenario:** an eligible queue-required merge is accepted. **File:**
|
||||
`apps/web/e2e/tests/pr/pr-merge-queue.spec.ts`, action test. **What to
|
||||
verify:** the desktop PR surface exposes `Merge PR`, the merge request
|
||||
returns `{"status":"queued"}`, the success notification appears, and the
|
||||
accepted state suppresses the duplicate action.
|
||||
- **Scenario:** an already queued PR exposes hydrated queue metadata. **File:**
|
||||
`apps/web/e2e/tests/pr/pr-merge-queue.spec.ts`, display test. **What to
|
||||
verify:** task icon color, task hover summary, compact popover, and detail
|
||||
notice show queued state, position, and estimated duration.
|
||||
- **Scenario:** the same accepted action is touch-usable. **File:**
|
||||
`apps/web/e2e/tests/pr/mobile-pr-merge-queue.spec.ts`, action test. **What to
|
||||
verify:** Review navigation, touch activation, queued notification, a
|
||||
minimum 44px action target, and no document horizontal overflow.
|
||||
- **Scenario:** an already queued PR is readable on a phone. **File:**
|
||||
`apps/web/e2e/tests/pr/mobile-pr-merge-queue.spec.ts`, display test. **What to
|
||||
verify:** the existing PR status drawer and full-height Review surface show
|
||||
queued state, position, and estimate without hover or horizontal overflow.
|
||||
|
||||
## Verification Results
|
||||
|
||||
- Backend: `make -C apps/backend test ARGS='./internal/github/...'` passed; the
|
||||
Make target executed the full backend `go test -tags fts5 ./...` suite.
|
||||
- Frontend: 96 focused Vitest tests passed; `pnpm run i18n:check` and
|
||||
`pnpm run typecheck` passed.
|
||||
- Desktop E2E: `pnpm e2e:run --no-build tests/pr/pr-merge-queue.spec.ts`
|
||||
passed (1 test).
|
||||
- Mobile E2E: `pnpm e2e:run --no-build --project mobile-chrome
|
||||
tests/pr/mobile-pr-merge-queue.spec.ts` passed (1 test).
|
||||
Recorded after the remediation run:
|
||||
|
||||
- Backend: `cd apps/backend && go test -tags fts5
|
||||
./internal/github/... ./internal/task/statussummary/...
|
||||
./internal/backendapp` passed 2,193 tests across 3 packages, including the
|
||||
queue precedence regression.
|
||||
- Frontend: the six-file queue-status command passed 160 tests. `pnpm run
|
||||
typecheck` and `pnpm run lint` passed. `pnpm run i18n:check` passed with
|
||||
7,223 referenced keys, 8,779 English entries, 48 orphans, and all five
|
||||
required catalogs complete.
|
||||
- Desktop E2E: the fresh-build `cd apps/web && pnpm e2e:run
|
||||
tests/pr/pr-merge-queue.spec.ts` passed 2 tests in 12.2 seconds. Mobile E2E:
|
||||
`pnpm e2e:run --no-build --project mobile-chrome
|
||||
tests/pr/mobile-pr-merge-queue.spec.ts` passed 2 tests in 9.6 seconds.
|
||||
|
||||
## Implementation Waves And Parallel Candidates
|
||||
|
||||
@@ -129,5 +192,11 @@ Wave 2:
|
||||
Wave 3:
|
||||
- [x] [task-03-desktop-mobile-e2e](task-03-desktop-mobile-e2e.md)
|
||||
|
||||
The detailed queue-observation task package under
|
||||
`docs/plans/github-pr-merge-queue-status/` is the synchronized execution record
|
||||
for the persistence, projection, queue color, metadata, and responsive display
|
||||
work described above. Its E2E task and this parent plan intentionally record
|
||||
the same two desktop and two mobile scenarios.
|
||||
|
||||
Execution is sequential in the primary conversation unless the user explicitly
|
||||
authorizes subagents.
|
||||
|
||||
@@ -25,6 +25,11 @@ spec: "../../specs/github-pr-merge-queue/spec.md"
|
||||
- `apps/backend/internal/github/noop_client.go`
|
||||
- `apps/backend/internal/github/service_pr.go`
|
||||
- `apps/backend/internal/github/controller.go`
|
||||
- `apps/backend/internal/github/graphql.go`
|
||||
- `apps/backend/internal/github/models.go`
|
||||
- `apps/backend/internal/github/store.go`
|
||||
- `apps/backend/internal/github/service_pr_watch.go`
|
||||
- `apps/backend/internal/task/statussummary/projector_pr.go`
|
||||
- Their focused `*_test.go` files
|
||||
|
||||
## Acceptance
|
||||
@@ -35,6 +40,10 @@ spec: "../../specs/github-pr-merge-queue/spec.md"
|
||||
including already-queued idempotency, and unknown results fail closed.
|
||||
- The workspace-scoped HTTP endpoint returns the normalized outcome while
|
||||
retaining auth routing, cache invalidation, validation, and error mapping.
|
||||
- The existing GraphQL status sync observes queue state, one-based position,
|
||||
and optional estimate; persistence preserves queue-unaware reads, clears
|
||||
authoritative exits and terminal states, and the bounded task projection
|
||||
exposes the queued state with multi-PR attention ranking.
|
||||
|
||||
## Verification
|
||||
|
||||
|
||||
@@ -35,11 +35,15 @@ spec: "../../specs/github-pr-merge-queue/spec.md"
|
||||
PR state, and prevent duplicate submission; rejected requests remain retryable.
|
||||
- Desktop, compact, and mobile Review surfaces reuse the same mutation and
|
||||
eligibility behavior with accessible, touch-usable controls.
|
||||
- An active queue entry uses the dedicated `#966600` color before other
|
||||
non-terminal review, check, draft, dirty, or behind states. A failing sibling
|
||||
still wins multi-PR aggregation, terminal colors remain authoritative, and
|
||||
future non-empty queue states use generic queued copy.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && pnpm install --frozen-lockfile && pnpm --filter @kandev/web test -- lib/api/domains/github-api.test.ts components/github/pr-task-icon.test.ts components/github/pr-merge-button.test.tsx && cd web && pnpm run i18n:check && pnpm run typecheck
|
||||
cd apps && pnpm install --frozen-lockfile && pnpm --filter @kandev/web test -- lib/api/domains/github-api.test.ts components/github/pr-task-icon.test.ts components/github/pr-merge-button.test.tsx components/github/pr-task-status-summary.test.ts components/github/pr-status-chip.test.tsx components/github/pr-merge-queue-status.test.tsx && cd web && pnpm run i18n:check && pnpm run typecheck
|
||||
```
|
||||
|
||||
## Dependencies And Risks
|
||||
@@ -52,12 +56,21 @@ cd apps && pnpm install --frozen-lockfile && pnpm --filter @kandev/web test -- l
|
||||
|
||||
## Results
|
||||
|
||||
- `pnpm --filter @kandev/web test -- --run lib/api/domains/github-api.test.ts components/github/pr-task-icon.test.ts` passed: 96 tests.
|
||||
- `pnpm run i18n:check` passed with all supported catalogs complete.
|
||||
- The six-file queue-status command passed 160 tests, including the queue
|
||||
precedence, failing-sibling, and future-state fallback regressions. The
|
||||
existing API and merge-button action tests remain covered by Task 02's
|
||||
frontend suite.
|
||||
- `pnpm run i18n:check` passed with 7,223 referenced keys, 8,779 English
|
||||
entries, 48 orphans, and all supported catalogs complete.
|
||||
- `pnpm run typecheck` passed.
|
||||
- Review remediation keeps GitHub's overloaded `blocked` state visually neutral
|
||||
and labels the pre-submit action `Merge PR`; only the accepted response claims
|
||||
that GitHub added the PR to its merge queue.
|
||||
- Queue state is checked after terminal state and before other non-terminal
|
||||
states in both task-icon and compact-chip status derivation. Multi-PR status
|
||||
ranking still lets a failing sibling dominate a queued sibling.
|
||||
- Future provider queue enums map to the generic queued status rather than an
|
||||
`Unknown` label.
|
||||
- Queue feedback is driven by GitHub's terminal `enqueued` result, not
|
||||
asynchronous `pending` acceptance.
|
||||
- Focused terminal-outcome coverage passes for merged, queued, rejected, and
|
||||
|
||||
@@ -26,18 +26,24 @@ spec: "../../specs/github-pr-merge-queue/spec.md"
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Desktop E2E proves merge-queue acceptance from the existing PR UI.
|
||||
- Component coverage proves immediate merge feedback and duplicate-submission
|
||||
suppression without duplicating those deterministic cases in Playwright.
|
||||
- Mobile E2E reaches the same queued outcome through Review using touch and
|
||||
verifies action size and absence of horizontal document overflow.
|
||||
- The managed runner builds fresh backend/frontend artifacts and both focused
|
||||
projects pass without retries masking failures.
|
||||
- Desktop E2E has separate action and display scenarios. The action scenario
|
||||
proves the existing PR UI exposes `Merge PR`, returns the queued API outcome,
|
||||
shows the success notification, and suppresses the accepted action. The
|
||||
display scenario proves task hover, compact popover, and detail notice expose
|
||||
queued state, position, and estimated duration.
|
||||
- Mobile E2E has separate action and display scenarios. The action scenario
|
||||
reaches the queued outcome through Review using touch, verifies a minimum
|
||||
44px action target, and checks the success notification. The display scenario
|
||||
verifies the existing status drawer and Review surface expose queue metadata
|
||||
without hover.
|
||||
- Both display scenarios retain the absence-of-horizontal-document-overflow
|
||||
assertion, and the managed runner builds fresh backend/frontend artifacts
|
||||
before the focused projects pass without retries masking failures.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && pnpm install --frozen-lockfile && cd web && pnpm e2e:run tests/pr/pr-merge-queue.spec.ts && pnpm e2e:run --project mobile-chrome tests/pr/mobile-pr-merge-queue.spec.ts
|
||||
cd apps && pnpm install --frozen-lockfile && cd web && pnpm e2e:run tests/pr/pr-merge-queue.spec.ts && pnpm e2e:run --no-build --project mobile-chrome tests/pr/mobile-pr-merge-queue.spec.ts
|
||||
```
|
||||
|
||||
## Dependencies And Risks
|
||||
@@ -50,10 +56,15 @@ cd apps && pnpm install --frozen-lockfile && cd web && pnpm e2e:run tests/pr/pr-
|
||||
|
||||
## Results
|
||||
|
||||
- Desktop managed E2E passed with one discovered test.
|
||||
- Mobile Chrome managed E2E passed with one discovered test, including the
|
||||
touch action, minimum target height, and horizontal-overflow assertions.
|
||||
- `pnpm run typecheck` passed after the final E2E helper and spec changes.
|
||||
- Isolated desktop and mobile screenshots were captured under
|
||||
`/tmp/kandev-merge-queue-screenshots-20260817/`; capture-only test hooks were
|
||||
removed and both specs passed again afterward.
|
||||
Recorded after the remediation run:
|
||||
|
||||
- Desktop managed E2E passed with two discovered tests in 12.2 seconds: the
|
||||
merge action and the queued-status display scenario.
|
||||
- Mobile Chrome managed E2E passed with two discovered tests in 9.6 seconds:
|
||||
the Review merge action with the minimum target-height assertion and the
|
||||
drawer/Review queue display scenario. Both retained the horizontal-overflow
|
||||
assertion.
|
||||
- The fresh-build desktop command was followed by the mobile `--no-build`
|
||||
command, so both projects exercised the same newly built artifacts.
|
||||
- Existing queue-status screenshots remain valid for the display scenario;
|
||||
action coverage is behavioral and does not add capture assets.
|
||||
|
||||
Reference in New Issue
Block a user