diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index 420e49007..25ed5e13c 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -90,11 +90,13 @@ shows the PR is dirty/conflicting). If the branch is merely behind but cleanly mergeable, skip the rebase and let the merge handle it — needless rebases burn iterations and CI. -**Bot pings by iteration.** First push (Phase 0 / initial PR) → `@copilot review -but do not make fixes`. Subsequent fix-iteration re-pushes → `@codex review`. For -a >250-file diff, also ping `@greptile` and `@coderabbit` (separate comments). -Phase 1 still waits for the review signal to settle before fixing. This is the -playbook's Phase 4 rule — defer to it for exact bodies. +**Bot pings by iteration.** Never ping GitHub Copilot and never treat Copilot as +an expected review signal; quota exhaustion otherwise leaves the loop waiting +forever. Initial PR pushes do not need a direct review ping. Subsequent +fix-iteration re-pushes → `@codex review`. For a >250-file diff, also ping +`@greptile` and `@coderabbit` (separate comments). Phase 1 still waits for the +expected review signals to settle before fixing. This is the playbook's Phase 4 +rule — defer to it for exact bodies. **Merge needs admin.** `main` is ruleset-guarded — `gh pr merge --squash` will show BLOCKED. Retry with `gh pr merge --admin --squash`; the ruleset's @@ -147,8 +149,8 @@ self-resume signal. Either: ## The loop (summary — full detail in the playbook) - **Phase 0 (first run):** safety rails (clean tree, GitHub origin, refuse - `main`) → commit → push → open PR (`ade`, gh fallback) → `@copilot` ping → - write state, schedule first wake. + `main`) → commit → push → open PR (`ade`, gh fallback) → write state → + schedule first wake. - **Phase 1 — Poll:** wait for BOTH CI terminal AND review bots terminal. Return a structured summary (merged / conflicting / ciFailed / newComments). Don't fix on a partial signal. diff --git a/apps/ade-cli/README.md b/apps/ade-cli/README.md index 36e487017..504e0b139 100644 --- a/apps/ade-cli/README.md +++ b/apps/ade-cli/README.md @@ -371,7 +371,8 @@ ade search --status --text # index doc counts, ade prs create --lane lane-id --base main --title "Fix checkout flow" --text # prints GitHub + ADE PR URLs ade prs create --lane lane-id --base main --close-linear-issue-on-merge ade prs list-open --text -ade prs github-snapshot --include-external-closed +ade prs github-snapshot --include-external-closed --history-page-limit 4 +ade prs github-snapshot --include-state-counts --no-revalidate ade prs checks pr-id --text ade prs comments pr-id --text ade run defs --text diff --git a/apps/ade-cli/src/bootstrap.ts b/apps/ade-cli/src/bootstrap.ts index 404119a11..3c7086319 100644 --- a/apps/ade-cli/src/bootstrap.ts +++ b/apps/ade-cli/src/bootstrap.ts @@ -1301,8 +1301,12 @@ export async function createAdeRuntime(args: { // by pushPublisherService.start() (declared below), so it stays empty and inert // when push publishing is not running. const pushPrNotificationSubscribers = new Set<(notification: PushPrNotification) => void>(); + let syncService: ReturnType | null = null; const emitPrEvent = (event: PrEventPayload): void => { pushEvent("runtime", { type: "pr_event", event }); + if (event.type === "prs-updated") { + syncService?.notifyPrsUpdated(); + } if (event.type === "pr-notification" && pushPrNotificationSubscribers.size > 0) { const notification: PushPrNotification = { kind: event.kind, @@ -1571,7 +1575,6 @@ export async function createAdeRuntime(args: { } let externalSessionsService: ReturnType | null = null; - let syncService: ReturnType | null = null; if (resolvedArgs.syncRuntime?.enabled && agentChatService) { const { createSyncService } = await import("./services/sync/syncService"); syncService = createSyncService({ diff --git a/apps/ade-cli/src/cli.test.ts b/apps/ade-cli/src/cli.test.ts index 9e89c2e48..b7935efb4 100644 --- a/apps/ade-cli/src/cli.test.ts +++ b/apps/ade-cli/src/cli.test.ts @@ -4944,14 +4944,19 @@ describe("ADE CLI", () => { }); }); - it("forwards PR GitHub snapshot full-history flag to the runtime action", () => { + it("forwards bounded PR GitHub snapshot options to the runtime action", () => { const snapshot = buildCliPlan([ "prs", "github-snapshot", "--include-external-closed", + "--history-page-limit", + "4", + "--include-state-counts", + "--no-revalidate", ]); expect(snapshot.kind).toBe("execute"); if (snapshot.kind !== "execute") return; + expect(snapshot.label).toBe("PR GitHub snapshot"); expect(snapshot.steps[0]?.params).toEqual({ name: "run_ade_action", arguments: { @@ -4960,9 +4965,19 @@ describe("ADE CLI", () => { args: { force: false, includeExternalClosed: true, + historyPageLimit: 4, + includeStateCounts: true, + revalidate: false, }, }, }); + + expect(() => buildCliPlan([ + "prs", + "github-snapshot", + "--history-page-limit", + "0", + ])).toThrow("--history-page-limit must be a positive integer."); }); it("maps discoverable git status, sync, and conflict helpers to existing actions", () => { diff --git a/apps/ade-cli/src/cli.ts b/apps/ade-cli/src/cli.ts index 32a447749..2429c102e 100644 --- a/apps/ade-cli/src/cli.ts +++ b/apps/ade-cli/src/cli.ts @@ -1525,8 +1525,10 @@ const HELP_BY_COMMAND: Record = { $ ade prs link --lane --url Map an existing GitHub PR to a lane $ ade prs checks --text Show check status $ ade prs comments --text Show unresolved review work - $ ade prs github-snapshot --include-external-closed - Include closed external PR history in the GitHub snapshot + $ ade prs github-snapshot --include-external-closed --history-page-limit 4 + Include bounded closed PR history in the GitHub snapshot + $ ade prs github-snapshot --include-state-counts --no-revalidate + Include exact state totals without a background refresh $ ade prs resolve-thread --thread Resolve a review thread $ ade prs labels set ready-to-merge Replace labels $ ade prs reviewers request alice bob Request reviewers @@ -5776,6 +5778,19 @@ function buildPrPlan(args: string[]): CliPlan { if (readFlag(args, ["--include-external-closed", "--include-closed-external"])) { snapshotArgs.includeExternalClosed = true; } + const historyPageLimit = readIntOption(args, ["--history-page-limit", "--history-pages"]); + if (historyPageLimit !== undefined) { + if (historyPageLimit <= 0) { + throw new CliUsageError("--history-page-limit must be a positive integer."); + } + snapshotArgs.historyPageLimit = historyPageLimit; + } + if (readFlag(args, ["--include-state-counts", "--state-counts"])) { + snapshotArgs.includeStateCounts = true; + } + if (readFlag(args, ["--no-revalidate"])) { + snapshotArgs.revalidate = false; + } return { kind: "execute", label: "PR GitHub snapshot", diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index d1de1b9c4..a9264495d 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -2946,6 +2946,53 @@ async function connectPeer( return { ws, ...tracked }; } +describe("PR snapshot invalidation push", () => { + beforeEach(() => { + publishMock.mockReset(); + spawnMock.mockReset(); + bonjourDestroyMock.mockReset(); + bonjourConstructorMock.mockReset(); + spawnMock.mockImplementation(() => ({ kill: vi.fn(), once: vi.fn(), unref: vi.fn() })); + }); + + it("broadcasts one lightweight prs_updated envelope to an authenticated peer", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const base = createHostArgs(projectRoot, []); + const host = createSyncHostService({ + ...base, + deviceRegistryService: { + ...base.deviceRegistryService, + upsertPeerMetadata: vi.fn(), + }, + } as unknown as Parameters[0]); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + peer = await connectPeer(port, host.getBootstrapToken(), "ios-pr-invalidation"); + const envelopeCount = peer.envelopes.length; + + host.broadcastPrsUpdated(); + + const envelope = await waitForValue( + () => peer?.envelopes.slice(envelopeCount).find((entry) => entry.type === "prs_updated"), + "prs_updated push", + ); + const payload = envelope.payload as { updatedAt?: string }; + expect(envelope.type).toBe("prs_updated"); + expect(payload).toEqual({ updatedAt: expect.any(String) }); + expect(Number.isNaN(Date.parse(payload.updatedAt ?? ""))).toBe(false); + } finally { + try { + peer?.ws.close(); + } catch { + // ignore + } + await host.dispose(); + cleanup(); + } + }); +}); + describe("outbound changeset ack retries", () => { beforeEach(() => { publishMock.mockReset(); diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index 335e846c9..51f904536 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -5962,6 +5962,20 @@ export function createSyncHostService(args: SyncHostServiceArgs) { broadcastBrainStatus(); }, + /** + * Tell connected controllers that the host's cached GitHub projection has + * changed. The payload is intentionally tiny; clients fetch the newest + * projected snapshot once, while the normal CRR stream continues to carry + * mapped PR rows. This covers unmapped webhook updates without polling. + */ + broadcastPrsUpdated(): void { + const payload = { updatedAt: nowIso() }; + for (const peer of peers) { + if (!peer.authenticated || peer.ws.readyState !== WebSocket.OPEN) continue; + send(peer.ws, "prs_updated", payload); + } + }, + async broadcastProjectCatalog(): Promise { const payload = await buildProjectCatalogPayload(); if (bonjourPort != null) { diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts index f6a75ca2b..fba4c1e12 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts @@ -112,6 +112,43 @@ function makePairingConnectInfo( } describe("createSyncRemoteCommandService", () => { + it("forwards bounded GitHub history pagination for mobile PR lists", async () => { + const getGithubSnapshot = vi.fn().mockResolvedValue({ repoPullRequests: [] }); + const { service } = createService({ prService: { getGithubSnapshot } }); + + await service.execute(makePayload("prs.getGitHubSnapshot", { + includeExternalClosed: true, + historyPageLimit: 4.8, + revalidate: false, + includeStateCounts: true, + })); + + expect(getGithubSnapshot).toHaveBeenCalledWith({ + force: false, + includeExternalClosed: true, + historyPageLimit: 4, + revalidate: false, + includeStateCounts: true, + }); + }); + + it("serves unmapped PR detail through one coordinate-based mobile command", async () => { + const detail = { snapshot: { detail: null, checks: [] }, reviewThreads: [], actionRuns: [], activity: [] }; + const getMobileGithubDetail = vi.fn().mockResolvedValue(detail); + const { service } = createService({ prService: { getMobileGithubDetail } }); + + await expect(service.execute(makePayload("prs.getMobileGithubDetail", { + repoOwner: "arul28", + repoName: "ADE", + githubPrNumber: 849, + }))).resolves.toEqual(detail); + expect(getMobileGithubDetail).toHaveBeenCalledWith({ + repoOwner: "arul28", + repoName: "ADE", + githubPrNumber: 849, + }); + }); + it("advertises the complete paired-client analytics command contract", async () => { const flush = vi.fn(async () => true); const { service } = createService({ productAnalyticsService: { flush } }); diff --git a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts index a965db757..8b95109ea 100644 --- a/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts +++ b/apps/ade-cli/src/services/sync/syncRemoteCommandService.ts @@ -4646,6 +4646,11 @@ function registerPrAndDeeplinkRemoteCommands({ args, register }: RemoteCommandRe args.prService.getGithubSnapshot({ force: payload.force === true, includeExternalClosed: payload.includeExternalClosed === true, + revalidate: payload.revalidate !== false, + includeStateCounts: payload.includeStateCounts === true, + ...(typeof payload.historyPageLimit === "number" && Number.isFinite(payload.historyPageLimit) + ? { historyPageLimit: Math.max(1, Math.floor(payload.historyPageLimit)) } + : {}), })); register("prs.getReviewThreads", { viewerAllowed: true }, async (payload) => args.prService.getReviewThreads(requirePrId(payload, "prs.getReviewThreads"))); register("prs.getActionRuns", { viewerAllowed: true }, async (payload) => args.prService.getActionRuns(requirePrId(payload, "prs.getActionRuns"))); @@ -4665,6 +4670,8 @@ function registerPrAndDeeplinkRemoteCommands({ args, register }: RemoteCommandRe register("prs.getReviewsByGithub", { viewerAllowed: true }, async (payload) => args.prService.getReviewsByGithub(requirePrGithubCoords(payload, "prs.getReviewsByGithub"))); register("prs.getCommentsByGithub", { viewerAllowed: true }, async (payload) => args.prService.getCommentsByGithub(requirePrGithubCoords(payload, "prs.getCommentsByGithub"))); register("prs.getReviewThreadsByGithub", { viewerAllowed: true }, async (payload) => args.prService.getReviewThreadsByGithub(requirePrGithubCoords(payload, "prs.getReviewThreadsByGithub"))); + register("prs.getMobileGithubDetail", { viewerAllowed: true }, async (payload) => + args.prService.getMobileGithubDetail(requirePrGithubCoords(payload, "prs.getMobileGithubDetail"))); register("prs.createFromLane", { viewerAllowed: true, queueable: true }, async (payload) => args.prService.createFromLane(parseCreatePrArgs(payload))); register("prs.createQueue", { viewerAllowed: true, queueable: true }, async (payload) => args.prService.createQueuePrs(parseCreateQueuePrsArgs(payload))); register("prs.linkToLane", { viewerAllowed: true, queueable: true }, async (payload) => args.prService.linkToLane(parseLinkPrToLaneArgs(payload))); diff --git a/apps/ade-cli/src/services/sync/syncService.ts b/apps/ade-cli/src/services/sync/syncService.ts index 93d763b9a..9b3a891eb 100644 --- a/apps/ade-cli/src/services/sync/syncService.ts +++ b/apps/ade-cli/src/services/sync/syncService.ts @@ -1715,6 +1715,10 @@ export function createSyncService(args: SyncServiceArgs) { hostService?.handlePtyExit(event); }, + notifyPrsUpdated(): void { + hostService?.broadcastPrsUpdated(); + }, + getHostService(): SyncHostService | null { return hostService; }, diff --git a/apps/desktop/src/main/services/adeActions/registry.test.ts b/apps/desktop/src/main/services/adeActions/registry.test.ts index 650ddf705..748888f2e 100644 --- a/apps/desktop/src/main/services/adeActions/registry.test.ts +++ b/apps/desktop/src/main/services/adeActions/registry.test.ts @@ -310,9 +310,10 @@ describe("ADE_ACTION_ALLOWLIST shape", () => { expect(actions).toContain("listDeleteProgress"); }); - it("exposes pr.listPrsByLane for runtime-backed drawer PR pills", () => { + it("exposes runtime-backed PR reads", () => { const actions = ADE_ACTION_ALLOWLIST.pr ?? []; expect(actions).toContain("listPrsByLane"); + expect(actions).toContain("getMobileGithubDetail"); }); it("exposes ade_project.clearLocalData for runtime-backed cleanup", () => { diff --git a/apps/desktop/src/main/services/adeActions/registry.ts b/apps/desktop/src/main/services/adeActions/registry.ts index 3dd9131d9..af4e2063e 100644 --- a/apps/desktop/src/main/services/adeActions/registry.ts +++ b/apps/desktop/src/main/services/adeActions/registry.ts @@ -418,6 +418,7 @@ export const ADE_ACTION_ALLOWLIST: Partial { })); }); + it("fetches all PR state totals in one GraphQL request when mobile asks for counts", async () => { + const githubService = makeGithubService({ + getStatus: vi.fn(async () => makeGithubStatus()), + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === "/graphql") { + return { + data: { + data: { + repository: { + open: { totalCount: 4 }, + merged: { totalCount: 834 }, + closed: { totalCount: 17 }, + }, + }, + }, + }; + } + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + return { data: [makeGitHubPull({ number: 321, title: "Live PR" })] }; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + const snapshot = await service.getGithubSnapshot({ force: true, includeStateCounts: true }); + + expect(snapshot.history?.repoPullRequestCounts).toEqual({ + open: 4, + merged: 834, + closed: 17, + }); + expect(githubService.apiRequest).toHaveBeenCalledWith(expect.objectContaining({ + method: "POST", + path: "/graphql", + })); + }); + + it("retries a projected snapshot invalidated while exact state totals are loading", async () => { + let resolveStaleCounts!: (value: unknown) => void; + const staleCounts = new Promise((resolve) => { + resolveStaleCounts = resolve; + }); + let resolveFreshCounts!: (value: unknown) => void; + const freshCounts = new Promise((resolve) => { + resolveFreshCounts = resolve; + }); + let countRequests = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => makeGithubStatus()), + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === "/graphql") { + countRequests += 1; + return countRequests === 1 ? staleCounts : freshCounts; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const db = makeMockDb(); + db.all.mockImplementation((sql: string) => + String(sql).includes("from github_pr_projections") ? [makeGithubProjectionRow()] : []); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + const staleRequest = service.getGithubSnapshot({ includeStateCounts: true, revalidate: false }); + await flushMicrotasks(); + service.invalidateGithubSnapshot(); + const freshRequest = service.getGithubSnapshot({ includeStateCounts: true, revalidate: false }); + await flushMicrotasks(); + + resolveFreshCounts({ + data: { data: { repository: { + open: { totalCount: 2 }, merged: { totalCount: 20 }, closed: { totalCount: 3 }, + } } }, + }); + await expect(freshRequest).resolves.toMatchObject({ + history: { repoPullRequestCounts: { open: 2, merged: 20, closed: 3 } }, + }); + + resolveStaleCounts({ + data: { data: { repository: { + open: { totalCount: 1 }, merged: { totalCount: 10 }, closed: { totalCount: 1 }, + } } }, + }); + await expect(staleRequest).resolves.toMatchObject({ + history: { repoPullRequestCounts: { open: 2, merged: 20, closed: 3 } }, + }); + await expect(service.getGithubSnapshot({ includeStateCounts: true, revalidate: false })).resolves.toMatchObject({ + history: { repoPullRequestCounts: { open: 2, merged: 20, closed: 3 } }, + }); + expect(countRequests).toBe(2); + }); + it("serves a webhook projection before live GitHub revalidation", async () => { const githubService = makeGithubService({ getStatus: vi.fn(async () => makeGithubStatus()), @@ -1138,6 +1230,27 @@ describe("prService.getGithubSnapshot", () => { } }); + it("returns stale cached data without starting GitHub work when revalidation is disabled", async () => { + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(Date.parse("2026-01-01T00:00:00Z")); + const githubService = makeGithubService({ + getStatus: vi.fn(async () => makeGithubStatus()), + apiRequest: vi.fn(async () => ({ data: [makeGitHubPull({ title: "Cached PR" })] })), + }); + const { service } = buildService({ githubService, laneService: makeLaneService([]) }); + + try { + await service.getGithubSnapshot(); + nowSpy.mockReturnValue(Date.parse("2026-01-01T00:00:00Z") + GITHUB_SNAPSHOT_TTL_MS_FOR_TEST + 1); + + const stale = await service.getGithubSnapshot({ revalidate: false }); + expect(stale.repoPullRequests[0]?.title).toBe("Cached PR"); + await flushMicrotasks(30); + expect(githubService.apiRequest).toHaveBeenCalledTimes(1); + } finally { + nowSpy.mockRestore(); + } + }); + it("keeps GitHub tab snapshots scoped to the current repo", async () => { const githubService = makeGithubService({ getStatus: vi.fn(async () => makeGithubStatus()), @@ -1841,6 +1954,74 @@ describe("prService.ingestGithubWebhook", () => { prs: [], })); }); + + it("invalidates exact state totals and refreshes them on the projected webhook read", async () => { + let countRequests = 0; + const githubService = makeGithubService({ + getStatus: vi.fn(async () => makeGithubStatus()), + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === "/graphql") { + countRequests += 1; + return { + data: { + data: { + repository: { + open: { totalCount: 1 }, + merged: { totalCount: countRequests }, + closed: { totalCount: 0 }, + }, + }, + }, + }; + } + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + return { data: [makeGitHubPull()] }; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const projectionRow = { + project_id: "proj-1", repo_owner: REPO.owner, repo_name: REPO.name, + github_pr_number: 404, github_node_id: "PR_projection_404", + github_url: "https://github.com/test-owner/test-repo/pull/404", + title: "Projected PR", state: "merged", is_draft: 0, + base_branch: "main", head_branch: "feature/unmapped", + head_repo_owner: REPO.owner, head_repo_name: REPO.name, + head_sha: "head-sha", base_sha: "base-sha", author: "octocat", + labels_json: "[]", is_bot: 0, comment_count: 0, + created_at: "2026-05-01T00:00:00Z", updated_at: "2026-05-02T00:00:00Z", + synced_at: "2026-05-02T00:00:01Z", last_event_name: "pull_request", + last_delivery_id: "delivery-counts", + }; + const db = makeMockDb(); + db.all.mockImplementation((sql: string) => + String(sql).includes("from github_pr_projections") ? [projectionRow] : []); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + const first = await service.getGithubSnapshot({ force: true, includeStateCounts: true }); + expect(first.history?.repoPullRequestCounts?.merged).toBe(1); + + await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-count-refresh", + payload: { + action: "closed", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + pull_request: makeUnmappedBranchPull({ state: "closed", merged_at: "2026-05-03T00:00:00Z" }), + }, + }); + + const refreshed = await service.getGithubSnapshot({ + includeStateCounts: true, + revalidate: false, + }); + expect(refreshed.history?.repoPullRequestCounts?.merged).toBe(2); + expect(countRequests).toBe(2); + }); }); describe("prService.listWithConflicts", () => { @@ -2658,6 +2839,53 @@ describe("prService coordinate-based detail (unmapped PRs)", () => { // requireRow would have thrown; reaching here proves the row was never required. }); + + it("returns a complete unmapped header and marks failed sidecars as unavailable", async () => { + const db = makeMockDb(); + db.get.mockImplementation(() => null); + db.all.mockImplementation(() => []); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === "/repos/up-owner/up-repo/pulls/77") { + return { + data: makeGitHubPull({ + number: 77, + title: "Unmapped header", + body: "Unmapped body", + html_url: "https://github.com/up-owner/up-repo/pull/77", + base: { ref: "main" }, + head: { ref: "fork-feature", sha: "head-sha" }, + }), + }; + } + throw new Error(`Unavailable test sidecar: ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const result = await service.getMobileGithubDetail({ + repoOwner: "up-owner", + repoName: "up-repo", + githubPrNumber: 77, + }); + + expect(result.item).toEqual(expect.objectContaining({ + title: "Unmapped header", + githubPrNumber: 77, + githubUrl: "https://github.com/up-owner/up-repo/pull/77", + baseBranch: "main", + headBranch: "fork-feature", + })); + expect(result.snapshot.detail?.body).toBe("Unmapped body"); + expect(result.unavailableParts).toEqual(expect.arrayContaining([ + "action_runs", + "comments", + "commits", + "files", + "review_threads", + "timeline", + ])); + }); }); describe("prService merge contexts", () => { diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index 888384d29..ffc8a62af 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -78,6 +78,7 @@ import type { PrCreateCapabilities, PrCreateLaneEligibility, PrMobileSnapshot, + PrMobileGithubDetailSnapshot, PrStackInfo, PrStackMember, PrStackMemberRole, @@ -1990,7 +1991,10 @@ export function createPrService({ cachedGithubSnapshotAt = 0; cachedGithubSnapshotIncludesClosed = false; cachedGithubSnapshotHistoryPageLimit = 0; + cachedGithubSnapshotIncludesStateCounts = false; githubSnapshotCacheEpoch += 1; + githubPrStateCountsByRepo.clear(); + githubPrStateCountsInFlight.clear(); }; const pruneExpiredHotRefreshes = (nowMs = Date.now()): void => { @@ -4062,6 +4066,57 @@ export function createPrService({ return payload.data; }; + const githubPrStateCountsByRepo = new Map(); + const githubPrStateCountsInFlight = new Map>(); + const githubPrStateCountsKey = (repo: GitHubRepoRef): string => + repoRefKey(repo) ?? `${repo.owner.toLowerCase()}/${repo.name.toLowerCase()}`; + + const fetchGithubPrStateCounts = async (repo: GitHubRepoRef): Promise => { + const key = githubPrStateCountsKey(repo); + const existing = githubPrStateCountsInFlight.get(key); + if (existing) return await existing; + const requestEpoch = githubSnapshotCacheEpoch; + + const request = (async () => { + const data = await graphqlRequest<{ + repository?: { + open?: { totalCount?: number | null } | null; + merged?: { totalCount?: number | null } | null; + closed?: { totalCount?: number | null } | null; + } | null; + }>( + `query AdePullRequestStateCounts($owner: String!, $name: String!) { + repository(owner: $owner, name: $name) { + open: pullRequests(states: OPEN) { totalCount } + merged: pullRequests(states: MERGED) { totalCount } + closed: pullRequests(states: CLOSED) { totalCount } + } + }`, + { owner: repo.owner, name: repo.name }, + ); + if (!data.repository) { + throw new Error(`GitHub repository not found: ${repo.owner}/${repo.name}`); + } + const counts = { + open: Math.max(0, Number(data.repository.open?.totalCount ?? 0)), + merged: Math.max(0, Number(data.repository.merged?.totalCount ?? 0)), + closed: Math.max(0, Number(data.repository.closed?.totalCount ?? 0)), + }; + if (requestEpoch === githubSnapshotCacheEpoch) { + githubPrStateCountsByRepo.set(key, counts); + } + return counts; + })(); + githubPrStateCountsInFlight.set(key, request); + try { + return await request; + } finally { + if (githubPrStateCountsInFlight.get(key) === request) { + githubPrStateCountsInFlight.delete(key); + } + } + }; + const fetchAllPages = async (args: { path: string; query?: Record; @@ -4448,11 +4503,17 @@ export function createPrService({ }; }; - const bestEffort = async (label: string, task: Promise, fallback: T): Promise => { + const bestEffort = async ( + label: string, + task: Promise, + fallback: T, + onError?: (error: unknown) => void, + ): Promise => { try { return await task; } catch (error) { logger.warn("prs.best_effort_failed", { label, error: getErrorMessage(error) }); + onError?.(error); return fallback; } }; @@ -4892,14 +4953,15 @@ export function createPrService({ return await getReviewsByCoords(coordsFromRow(row)); }; - const getCommentsByCoords = async (coords: PrGithubCoords): Promise => { + const getCommentsByCoords = async (coords: PrGithubCoords, strict = false): Promise => { const repo: GitHubRepoRef = { owner: coords.repoOwner, name: coords.repoName }; const prNumber = Number(coords.githubPrNumber); - const [issueComments, reviewComments] = await Promise.all([ - fetchIssueComments(repo, prNumber).catch(() => []), - fetchReviewComments(repo, prNumber).catch(() => []) - ]); + const [issueComments, reviewComments] = await Promise.all( + strict + ? [fetchIssueComments(repo, prNumber), fetchReviewComments(repo, prNumber)] + : [fetchIssueComments(repo, prNumber).catch(() => []), fetchReviewComments(repo, prNumber).catch(() => [])] + ); return [...issueComments, ...reviewComments].sort((a, b) => { const aTs = a.createdAt ? Date.parse(a.createdAt) : Number.NaN; @@ -4920,24 +4982,33 @@ export function createPrService({ * uses to key the row in `PrDetail.prId` — it has no DB row. We never call * `requireRow` here so unmapped PRs can load. */ - const getDetailSnapshotByCoords = async (coords: PrGithubCoords, prId: string): Promise => { + const prDetailFromGithubPull = (data: any, prId: string): PrDetail => ({ + prId, + body: normalizePrDetailBody(asString(data?.body)), + labels: Array.isArray(data?.labels) ? data.labels.map(toLabel) : [], + assignees: Array.isArray(data?.assignees) ? data.assignees.map(toUser) : [], + requestedReviewers: Array.isArray(data?.requested_reviewers) ? data.requested_reviewers.map(toUser) : [], + requestedTeams: Array.isArray(data?.requested_teams) ? data.requested_teams.map(toTeam) : [], + author: toUser(data?.user), + isDraft: Boolean(data?.draft), + milestone: asString(data?.milestone?.title) || null, + linkedIssues: [], + }); + + const getGithubPullDetailByCoords = async ( + coords: PrGithubCoords, + prId: string, + ): Promise<{ detail: PrDetail; rawPull: any }> => { const repo: GitHubRepoRef = { owner: coords.repoOwner, name: coords.repoName }; const { data } = await githubService.apiRequest({ method: "GET", path: `/repos/${repo.owner}/${repo.name}/pulls/${Number(coords.githubPrNumber)}` }); - return { - prId, - body: normalizePrDetailBody(asString(data?.body)), - labels: Array.isArray(data?.labels) ? data.labels.map(toLabel) : [], - assignees: Array.isArray(data?.assignees) ? data.assignees.map(toUser) : [], - requestedReviewers: Array.isArray(data?.requested_reviewers) ? data.requested_reviewers.map(toUser) : [], - requestedTeams: Array.isArray(data?.requested_teams) ? data.requested_teams.map(toTeam) : [], - author: toUser(data?.user), - isDraft: Boolean(data?.draft), - milestone: asString(data?.milestone?.title) || null, - linkedIssues: [] - }; + return { detail: prDetailFromGithubPull(data, prId), rawPull: data }; + }; + + const getDetailSnapshotByCoords = async (coords: PrGithubCoords, prId: string): Promise => { + return (await getGithubPullDetailByCoords(coords, prId)).detail; }; const getDetailSnapshot = async (prId: string): Promise => { @@ -7991,24 +8062,28 @@ export function createPrService({ force?: boolean; includeExternalClosed?: boolean; historyPageLimit?: number; + revalidate?: boolean; + includeStateCounts?: boolean; }; const GITHUB_SNAPSHOT_TTL_MS = 120_000; const GITHUB_OPEN_SNAPSHOT_MAX_PAGES = 10; const GITHUB_HISTORY_INITIAL_PAGE_LIMIT = 2; const GITHUB_HISTORY_MAX_PAGE_LIMIT = 10; - const GITHUB_PROJECTION_CLOSED_RETAIN_LIMIT = 300; + const GITHUB_PROJECTION_CLOSED_RETAIN_LIMIT = 1_000; const GITHUB_WEBHOOK_DELIVERY_RETAIN_LIMIT = 1_000; const GITHUB_WEBHOOK_ERROR_DELIVERY_RETAIN_LIMIT = 100; let cachedGithubSnapshot: GitHubPrSnapshot | null = null; let cachedGithubSnapshotAt = 0; let cachedGithubSnapshotIncludesClosed = false; let cachedGithubSnapshotHistoryPageLimit = 0; + let cachedGithubSnapshotIncludesStateCounts = false; let githubSnapshotCacheEpoch = 0; let githubSnapshotInFlight: { request: Promise; includeExternalClosed: boolean; historyPageLimit: number; + includeStateCounts: boolean; } | null = null; const normalizeGithubHistoryPageLimit = (options: GithubSnapshotOptions = {}): number => { @@ -8022,10 +8097,11 @@ export function createPrService({ }; const githubSnapshotRequestSatisfies = ( - request: { includeExternalClosed: boolean; historyPageLimit: number } | null, + request: { includeExternalClosed: boolean; historyPageLimit: number; includeStateCounts: boolean } | null, options: GithubSnapshotOptions, ): boolean => { if (!request) return false; + if (options.includeStateCounts === true && !request.includeStateCounts) return false; if (options.includeExternalClosed !== true) return true; return request.includeExternalClosed && request.historyPageLimit >= normalizeGithubHistoryPageLimit(options); @@ -8036,12 +8112,14 @@ export function createPrService({ capturedAt = Date.now(), includeExternalClosed = false, historyPageLimit = 0, + includeStateCounts = false, ): void => { if (snapshot.repo) activeGithubRepo = snapshot.repo; cachedGithubSnapshot = snapshot; cachedGithubSnapshotAt = capturedAt; cachedGithubSnapshotIncludesClosed = includeExternalClosed; cachedGithubSnapshotHistoryPageLimit = includeExternalClosed ? historyPageLimit : 0; + cachedGithubSnapshotIncludesStateCounts = includeStateCounts; }; const clearGithubSnapshotAuthCache = (): void => { @@ -8174,6 +8252,58 @@ export function createPrService({ }; }; + const gitHubItemFromRawPull = ( + rawPr: any, + fallbackRepo: GitHubRepoRef, + metadata?: GithubSnapshotMetadata, + scope: "repo" | "external" = "repo", + ): GitHubPrListItem => { + const rawRepo = rawPullBaseRepo(rawPr, fallbackRepo) ?? fallbackRepo; + const repoOwner = rawRepo.owner; + const repoName = rawRepo.name; + const githubPrNumber = Number(rawPr?.number) || 0; + const linkedPrRow = metadata?.linkedPrByRepoKey.get(repoPrKey(repoOwner, repoName, githubPrNumber)) ?? null; + const workflowRow = linkedPrRow ? metadata?.workflowByPrId.get(linkedPrRow.id) ?? null : null; + const groupRow = linkedPrRow ? metadata?.groupByPrId.get(linkedPrRow.id) ?? null : null; + + return { + id: asString(rawPr?.node_id) || syntheticGithubPrId({ repoOwner, repoName, githubPrNumber }), + scope, + repoOwner, + repoName, + githubPrNumber, + githubUrl: asString(rawPr?.html_url) || `https://github.com/${repoOwner}/${repoName}/pull/${githubPrNumber}`, + title: asString(rawPr?.title) || `PR #${githubPrNumber}`, + state: toGitHubState(rawPr), + isDraft: Boolean(rawPr?.draft), + baseBranch: asString(rawPr?.base?.ref) || null, + headBranch: asString(rawPr?.head?.ref) || null, + headRepoOwner: rawPullHeadOwner(rawPr) || null, + headRepoName: rawPullHeadRepoName(rawPr) || null, + author: asString(rawPr?.user?.login) || null, + createdAt: asString(rawPr?.created_at) || nowIso(), + updatedAt: asString(rawPr?.updated_at) || asString(rawPr?.created_at) || nowIso(), + linkedPrId: linkedPrRow?.id ?? null, + linkedGroupId: asString(workflowRow?.linked_group_id).trim() || groupRow?.group_id || null, + linkedLaneId: linkedPrRow?.lane_id ?? null, + linkedLaneName: linkedPrRow ? (metadata?.laneById.get(linkedPrRow.lane_id)?.name ?? linkedPrRow.lane_id) : null, + adeKind: deriveGithubSnapshotAdeKind(workflowRow, groupRow, linkedPrRow), + workflowDisplayState: workflowRow ? parseWorkflowDisplayState(workflowRow.workflow_display_state) : null, + cleanupState: workflowRow ? parseCleanupState(workflowRow.cleanup_state) : null, + labels: Array.isArray(rawPr?.labels) + ? rawPr.labels + .filter((label: any) => label?.name) + .map((label: any) => ({ + name: String(label.name), + color: String(label.color || "cccccc"), + description: label.description != null ? String(label.description) : null, + })) + : [], + isBot: asString(rawPr?.user?.type).toLowerCase() === "bot", + commentCount: Number(rawPr?.comments) || 0, + }; + }; + const mergeGithubPrItems = ( liveItems: GitHubPrListItem[], projectedItems: GitHubPrListItem[], @@ -8204,7 +8334,14 @@ export function createPrService({ projectionRows = listGithubPrProjectionRows(repo, options); } if (projectionRows.length === 0) return null; - const repoPullRequestCounts = countGithubPrProjectionRows(repo); + const projectionCounts = countGithubPrProjectionRows(repo); + const repoPullRequestCounts = options.includeStateCounts === true + ? await bestEffort( + "buildProjectedGithubSnapshot.fetchGithubPrStateCounts", + fetchGithubPrStateCounts(repo), + githubPrStateCountsByRepo.get(githubPrStateCountsKey(repo)) ?? projectionCounts, + ) + : githubPrStateCountsByRepo.get(githubPrStateCountsKey(repo)) ?? projectionCounts; const historyPageLimit = normalizeGithubHistoryPageLimit(options); return { repo, @@ -8250,49 +8387,13 @@ export function createPrService({ ? historyPageLimit : GITHUB_OPEN_SNAPSHOT_MAX_PAGES; - const toGitHubItem = (rawPr: any, scope: "repo" | "external"): GitHubPrListItem => { - const rawRepo = rawPullBaseRepo(rawPr, repo) ?? repo; - const repoOwner = rawRepo.owner; - const repoName = rawRepo.name; - const githubPrNumber = Number(rawPr?.number) || 0; - const linkedPrRow = metadata.linkedPrByRepoKey.get(repoPrKey(repoOwner, repoName, githubPrNumber)) ?? null; - const workflowRow = linkedPrRow ? metadata.workflowByPrId.get(linkedPrRow.id) ?? null : null; - const groupRow = linkedPrRow ? metadata.groupByPrId.get(linkedPrRow.id) ?? null : null; - - return { - id: asString(rawPr?.node_id) || `${scope}-${repoOwner}-${repoName}-${githubPrNumber}`, - scope, - repoOwner, - repoName, - githubPrNumber, - githubUrl: asString(rawPr?.html_url) || "", - title: asString(rawPr?.title) || `PR #${githubPrNumber}`, - state: toGitHubState(rawPr), - isDraft: Boolean(rawPr?.draft), - baseBranch: asString(rawPr?.base?.ref) || null, - headBranch: asString(rawPr?.head?.ref) || null, - headRepoOwner: rawPullHeadOwner(rawPr) || null, - headRepoName: rawPullHeadRepoName(rawPr) || null, - author: asString(rawPr?.user?.login) || null, - createdAt: asString(rawPr?.created_at) || nowIso(), - updatedAt: asString(rawPr?.updated_at) || asString(rawPr?.created_at) || nowIso(), - linkedPrId: linkedPrRow?.id ?? null, - linkedGroupId: asString(workflowRow?.linked_group_id).trim() || groupRow?.group_id || null, - linkedLaneId: linkedPrRow?.lane_id ?? null, - linkedLaneName: linkedPrRow ? (metadata.laneById.get(linkedPrRow.lane_id)?.name ?? linkedPrRow.lane_id) : null, - adeKind: deriveGithubSnapshotAdeKind(workflowRow, groupRow, linkedPrRow), - workflowDisplayState: workflowRow ? parseWorkflowDisplayState(workflowRow.workflow_display_state) : null, - cleanupState: workflowRow ? parseCleanupState(workflowRow.cleanup_state) : null, - labels: Array.isArray(rawPr?.labels) - ? rawPr.labels - .filter((l: any) => l?.name) - .map((l: any) => ({ name: String(l.name), color: String(l.color || "cccccc"), description: l.description != null ? String(l.description) : null })) - : [], - isBot: asString(rawPr?.user?.type).toLowerCase() === "bot", - commentCount: Number(rawPr?.comments) || 0, - }; - }; - + const stateCountsPromise = options.includeStateCounts === true + ? bestEffort( + "getGithubSnapshot.fetchGithubPrStateCounts", + fetchGithubPrStateCounts(repo), + null as GitHubPrProjectionStateCounts | null, + ) + : Promise.resolve(null); let repoPullRequestsRaw = await fetchAllPages({ path: `/repos/${repo.owner}/${repo.name}/pulls`, query: { @@ -8329,11 +8430,15 @@ export function createPrService({ if (autoMappedCount > 0 || backfilled) { metadata = await loadGithubSnapshotMetadata(); } - const liveRepoPullRequests = repoPullRequestsRaw.map((rawPr) => toGitHubItem(rawPr, "repo")); + const liveRepoPullRequests = repoPullRequestsRaw.map((rawPr) => + gitHubItemFromRawPull(rawPr, repo, metadata, "repo")); const projectedRepoPullRequests = listGithubPrProjectionRows(repo, options) .map((row) => gitHubItemFromProjection(row, metadata, "repo")); const repoPullRequests = mergeGithubPrItems(liveRepoPullRequests, projectedRepoPullRequests); - const repoPullRequestCounts = countGithubPrProjectionRows(repo); + const liveStateCounts = await stateCountsPromise; + const repoPullRequestCounts = liveStateCounts + ?? githubPrStateCountsByRepo.get(githubPrStateCountsKey(repo)) + ?? countGithubPrProjectionRows(repo); const syncedAt = nowIso(); return { @@ -8367,10 +8472,14 @@ export function createPrService({ const requestEpoch = githubSnapshotCacheEpoch; const includeExternalClosed = requestOptions.includeExternalClosed === true; const historyPageLimit = normalizeGithubHistoryPageLimit(requestOptions); + const includeStateCounts = requestOptions.includeStateCounts === true + || (precheckedGithubStatus.repo != null + && githubPrStateCountsByRepo.has(githubPrStateCountsKey(precheckedGithubStatus.repo))); let inFlight!: { request: Promise; includeExternalClosed: boolean; historyPageLimit: number; + includeStateCounts: boolean; }; const request = getGithubSnapshotUncached(precheckedGithubStatus, requestOptions) .then((snapshot) => { @@ -8379,7 +8488,15 @@ export function createPrService({ githubSnapshotInFlight === inFlight && requestEpoch === githubSnapshotCacheEpoch; if (canPublishSnapshot) { - publishGithubSnapshot(snapshot, capturedAt, includeExternalClosed, historyPageLimit); + const hasExactStateCounts = snapshot.repo != null + && githubPrStateCountsByRepo.has(githubPrStateCountsKey(snapshot.repo)); + publishGithubSnapshot( + snapshot, + capturedAt, + includeExternalClosed, + historyPageLimit, + hasExactStateCounts, + ); } return snapshot; }) @@ -8388,7 +8505,7 @@ export function createPrService({ githubSnapshotInFlight = null; } }); - inFlight = { request, includeExternalClosed, historyPageLimit }; + inFlight = { request, includeExternalClosed, historyPageLimit, includeStateCounts }; githubSnapshotInFlight = inFlight; return request; }; @@ -8397,6 +8514,7 @@ export function createPrService({ const requestedHistoryPageLimit = normalizeGithubHistoryPageLimit(options); const cachedSnapshotSatisfiesRequest = cachedGithubSnapshot !== null + && (options.includeStateCounts !== true || cachedGithubSnapshotIncludesStateCounts) && (!needsClosedHistory || ( cachedGithubSnapshotIncludesClosed && cachedGithubSnapshotHistoryPageLimit >= requestedHistoryPageLimit @@ -8415,7 +8533,8 @@ export function createPrService({ historyPageLimit: cachedGithubSnapshotHistoryPageLimit || GITHUB_HISTORY_INITIAL_PAGE_LIMIT, } : options; - if (!githubSnapshotRequestSatisfies(githubSnapshotInFlight, revalidationOptions)) { + if (options.revalidate !== false + && !githubSnapshotRequestSatisfies(githubSnapshotInFlight, revalidationOptions)) { void startSnapshotRequest(githubStatus, revalidationOptions).catch((error) => { logger.warn("prs.github_snapshot_revalidation_failed", { error: error instanceof Error ? error.message : String(error), @@ -8425,16 +8544,28 @@ export function createPrService({ return cachedSnapshot; } if (!force) { + const projectedRequestEpoch = githubSnapshotCacheEpoch; const projectedSnapshot = await buildProjectedGithubSnapshot(githubStatus, options); + if (projectedRequestEpoch !== githubSnapshotCacheEpoch) { + return getGithubSnapshot(options); + } if (projectedSnapshot) { publishGithubSnapshot( projectedSnapshot, Date.now(), options.includeExternalClosed === true, requestedHistoryPageLimit, + projectedSnapshot.repo != null + && githubPrStateCountsByRepo.has(githubPrStateCountsKey(projectedSnapshot.repo)), ); - if (!githubSnapshotRequestSatisfies(githubSnapshotInFlight, options)) { - void startSnapshotRequest(githubStatus, options).catch((error) => { + const revalidationOptions = options.includeStateCounts === true + && projectedSnapshot.repo != null + && githubPrStateCountsByRepo.has(githubPrStateCountsKey(projectedSnapshot.repo)) + ? { ...options, includeStateCounts: false } + : options; + if (options.revalidate !== false + && !githubSnapshotRequestSatisfies(githubSnapshotInFlight, revalidationOptions)) { + void startSnapshotRequest(githubStatus, revalidationOptions).catch((error) => { logger.warn("prs.github_snapshot_revalidation_failed", { error: error instanceof Error ? error.message : String(error), }); @@ -10726,6 +10857,52 @@ export function createPrService({ async getMobileSnapshot(): Promise { return await buildMobileSnapshot(); + }, + + async getMobileGithubDetail(coords: PrGithubCoords): Promise { + const repo: GitHubRepoRef = { owner: coords.repoOwner, name: coords.repoName }; + const prNumber = Number(coords.githubPrNumber); + const unavailableParts = new Set(); + const mobilePart = (part: string, request: Promise, fallback: T): Promise => { + return bestEffort( + `getMobileGithubDetail.${part}`, + request, + fallback, + () => unavailableParts.add(part), + ); + }; + const [anchor, status, checks, reviews, comments, files, commits, reviewThreads, actionRuns, timelineEvents] = + await Promise.all([ + // The detail record is the identity anchor for the entire screen. A + // failed anchor must reject instead of returning a misleading blank + // success; sidecars remain independently best-effort. + getGithubPullDetailByCoords(coords, syntheticGithubPrId(coords)), + mobilePart("status", getStatusByCoords(coords), null), + mobilePart("checks", getChecksByCoords(coords), [] as PrCheck[]), + mobilePart("reviews", getReviewsByCoords(coords), [] as PrReview[]), + mobilePart("comments", getCommentsByCoords(coords, true), [] as PrComment[]), + mobilePart("files", getFilesSnapshotByCoords(coords), [] as PrFile[]), + mobilePart("commits", getCommitsSnapshotByCoords(coords), [] as PrCommit[]), + mobilePart("review_threads", fetchReviewThreads(repo, prNumber), [] as PrReviewThread[]), + mobilePart("action_runs", getActionRunsByCoords(coords), [] as PrActionRun[]), + mobilePart("timeline", fetchAllPages({ + path: `/repos/${repo.owner}/${repo.name}/issues/${prNumber}/timeline`, + }), [] as any[]), + ]); + const rawPull = anchor.rawPull; + const item = gitHubItemFromRawPull(rawPull, repo); + if (["comments", "reviews", "checks", "timeline"].some((part) => unavailableParts.has(part))) { + unavailableParts.add("activity"); + } + + return { + item, + snapshot: { detail: anchor.detail, status, checks, reviews, comments, files, commits }, + reviewThreads, + actionRuns, + activity: buildActivityEvents(comments, reviews, checks, timelineEvents), + unavailableParts: [...unavailableParts].sort(), + }; } }; @@ -10736,7 +10913,10 @@ export function createPrService({ let lanes: LaneSummary[] = []; try { - lanes = await laneService.list({ includeArchived: false, includeStatus: true }); + // Mobile snapshot cards only need lane identity/topology. Full git status + // turns a single controller read into N worktree probes and was the main + // source of multi-second PR-tab loads on larger projects. + lanes = await laneService.list({ includeArchived: false, includeStatus: false }); } catch (error) { logger.warn("prs.mobile_snapshot.lanes_failed", { error: error instanceof Error ? error.message : String(error), diff --git a/apps/desktop/src/shared/syncMobileCompatibility.ts b/apps/desktop/src/shared/syncMobileCompatibility.ts index 174c4f5b1..abe77f6d1 100644 --- a/apps/desktop/src/shared/syncMobileCompatibility.ts +++ b/apps/desktop/src/shared/syncMobileCompatibility.ts @@ -11,6 +11,7 @@ export const MOBILE_SYNC_REQUIRED_REMOTE_COMMAND_ACTIONS = [ "lanes.getDetail", "work.updateSessionMeta", "prs.getMobileSnapshot", + "prs.getMobileGithubDetail", "work.runQuickCommand", "work.startCliSession", "work.sendToSession", diff --git a/apps/desktop/src/shared/types/prs.ts b/apps/desktop/src/shared/types/prs.ts index ee24423f7..d8bc2a2de 100644 --- a/apps/desktop/src/shared/types/prs.ts +++ b/apps/desktop/src/shared/types/prs.ts @@ -2105,3 +2105,26 @@ export type PrMobileSnapshot = { /** Mobile clients should surface a "host offline" banner when this is false. */ live: boolean; }; + +/** + * One batched detail payload for a GitHub PR that does not have a local + * `pull_requests` row yet. Mobile uses this instead of issuing a command per + * sidecar, keeping the unmapped detail screen at one controller round trip. + */ +export type PrMobileGithubDetailSnapshot = { + item: GitHubPrListItem; + snapshot: { + detail: PrDetail | null; + status: PrStatus | null; + checks: PrCheck[]; + reviews: PrReview[]; + comments: PrComment[]; + files: PrFile[]; + commits: PrCommit[]; + }; + reviewThreads: PrReviewThread[]; + actionRuns: PrActionRun[]; + activity: PrActivityEvent[]; + /** Optional sidecars that failed to load and must not be treated as true zeroes. */ + unavailableParts: string[]; +}; diff --git a/apps/desktop/src/shared/types/sync.ts b/apps/desktop/src/shared/types/sync.ts index 763feaea9..a4d4d7b09 100644 --- a/apps/desktop/src/shared/types/sync.ts +++ b/apps/desktop/src/shared/types/sync.ts @@ -1305,6 +1305,7 @@ export type SyncRemoteCommandAction = | "prs.cancelQueueAutomation" | "prs.reorderQueue" | "prs.getMobileSnapshot" + | "prs.getMobileGithubDetail" | "sync.getWebPairingInfo" | "sync.getDesktopPairingInfo" | "modelPicker.getFavorites" @@ -1413,6 +1414,7 @@ export type SyncChatSubscribeEnvelope = SyncEnvelopeWithPayload<"chat_subscribe" export type SyncChatUnsubscribeEnvelope = SyncEnvelopeWithPayload<"chat_unsubscribe", SyncChatUnsubscribePayload>; export type SyncChatEventEnvelope = SyncEnvelopeWithPayload<"chat_event", SyncChatEventPayload>; export type SyncBrainStatusEnvelope = SyncEnvelopeWithPayload<"brain_status", SyncBrainStatusPayload>; +export type SyncPrsUpdatedEnvelope = SyncEnvelopeWithPayload<"prs_updated", { updatedAt: string }>; export type SyncRosterSubscribeEnvelope = SyncEnvelopeWithPayload<"roster_subscribe", SyncRosterSubscribePayload>; export type SyncRosterUnsubscribeEnvelope = SyncEnvelopeWithPayload<"roster_unsubscribe", SyncRosterUnsubscribePayload>; export type SyncRosterSnapshotEnvelope = SyncEnvelopeWithPayload<"roster_snapshot", SyncRosterSnapshotPayload>; @@ -1476,6 +1478,7 @@ export type SyncEnvelope = | SyncChatUnsubscribeEnvelope | SyncChatEventEnvelope | SyncBrainStatusEnvelope + | SyncPrsUpdatedEnvelope | SyncRosterSubscribeEnvelope | SyncRosterUnsubscribeEnvelope | SyncRosterSnapshotEnvelope diff --git a/apps/ios/ADE/Models/RemoteModels.swift b/apps/ios/ADE/Models/RemoteModels.swift index b75e08e4f..436ea0e48 100644 --- a/apps/ios/ADE/Models/RemoteModels.swift +++ b/apps/ios/ADE/Models/RemoteModels.swift @@ -3726,6 +3726,21 @@ struct GitHubPrSnapshot: Codable, Equatable { var repoPullRequests: [GitHubPrListItem] var externalPullRequests: [GitHubPrListItem] var syncedAt: String + var history: GitHubPrSnapshotHistory? = nil +} + +struct GitHubPrSnapshotHistory: Codable, Equatable { + var includeExternalClosed: Bool + var pageLimit: Int + var repoPullRequestsLoaded: Int + var repoPullRequestsMayHaveMore: Bool + var repoPullRequestCounts: GitHubPrSnapshotCounts? +} + +struct GitHubPrSnapshotCounts: Codable, Equatable { + var open: Int + var closed: Int + var merged: Int } struct PrReviewThreadComment: Codable, Identifiable, Equatable { @@ -3796,6 +3811,18 @@ struct PrActivityEvent: Codable, Identifiable, Equatable { var metadata: [String: RemoteJSONValue]? } +/// One host round trip for the full GitHub detail surface when a PR has not +/// been mapped into ADE yet. Keeping the sidecars together avoids mobile +/// command fan-out and makes the detail screen usable before lane mapping. +struct PrMobileGithubDetailSnapshot: Codable, Equatable { + var item: GitHubPrListItem + var snapshot: PullRequestSnapshot + var reviewThreads: [PrReviewThread] + var actionRuns: [PrActionRun] + var activity: [PrActivityEvent] + var unavailableParts: [String] +} + struct PrDeployment: Codable, Identifiable, Equatable { var id: String var environment: String diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index ec7cbcb7a..e77d495cd 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -1848,11 +1848,26 @@ func makeLanesReparentArgs( return args } -func makePrGithubSnapshotArgs(force: Bool = false, includeExternalClosed: Bool = false) -> [String: Any] { +func makePrGithubSnapshotArgs( + force: Bool = false, + includeExternalClosed: Bool = false, + historyPageLimit: Int? = nil, + revalidate: Bool = true, + includeStateCounts: Bool = false +) -> [String: Any] { var args: [String: Any] = ["force": force] + if !revalidate { + args["revalidate"] = false + } + if includeStateCounts { + args["includeStateCounts"] = true + } if includeExternalClosed { args["includeExternalClosed"] = true } + if let historyPageLimit { + args["historyPageLimit"] = max(1, historyPageLimit) + } return args } @@ -2005,6 +2020,9 @@ final class SyncService: ObservableObject { @Published private(set) var workProjectionRevision = 0 @Published private(set) var filesProjectionRevision = 0 @Published private(set) var prsProjectionRevision = 0 + /// Host-pushed invalidation for GitHub projection changes that do not touch + /// replicated `pull_requests` rows (notably unmapped PR webhooks). + @Published private(set) var prsRemoteRevision = 0 @Published private(set) var proofArtifactsProjectionRevision = 0 private let iso8601WithFractionalSecondsFormatter: ISO8601DateFormatter = { @@ -2081,6 +2099,10 @@ final class SyncService: ObservableObject { /// view side; the entry's `loadedAt` gates whether the refresh actually /// re-fetches the sidecar fan-out. @Published private(set) var prDetailCache: [String: PrDetailWarmEntry] = [:] + /// Expensive unmapped-detail reads are single-flight per GitHub coordinate. + /// SwiftUI task cancellation does not cancel an already-sent host command, + /// so every later revision joins the same service-owned request. + private var prMobileGithubDetailInFlight: [String: Task] = [:] /// Repo-scoped GitHub PR list shared by the Lanes and Work tabs so a lane (a /// branch) can surface a PR opened directly on GitHub — not only PRs mapped @@ -5765,14 +5787,69 @@ final class SyncService: ObservableObject { try await sendDecodableCommand(action: "prs.getMobileSnapshot", as: PrMobileSnapshot.self) } - func fetchGitHubPullRequestSnapshot(force: Bool = false, includeExternalClosed: Bool = false) async throws -> GitHubPrSnapshot { + func fetchGitHubPullRequestSnapshot( + force: Bool = false, + includeExternalClosed: Bool = false, + historyPageLimit: Int? = nil, + revalidate: Bool = true, + includeStateCounts: Bool = false + ) async throws -> GitHubPrSnapshot { try await sendDecodableCommand( action: "prs.getGitHubSnapshot", - args: makePrGithubSnapshotArgs(force: force, includeExternalClosed: includeExternalClosed), + args: makePrGithubSnapshotArgs( + force: force, + includeExternalClosed: includeExternalClosed, + historyPageLimit: historyPageLimit, + revalidate: revalidate, + includeStateCounts: includeStateCounts + ), as: GitHubPrSnapshot.self ) } + func fetchPrMobileGithubDetail( + repoOwner: String, + repoName: String, + githubPrNumber: Int + ) async throws -> PrMobileGithubDetailSnapshot { + guard supportsRemoteAction("prs.getMobileGithubDetail") else { + throw NSError( + domain: "ADE", + code: 17, + userInfo: [ + NSLocalizedDescriptionKey: "Full GitHub PR details are not available on this machine version. Update ADE on the machine and reconnect.", + "ADEErrorCode": "unsupported_action", + ] + ) + } + let requestKey = "\(repoOwner.lowercased())/\(repoName.lowercased())#\(githubPrNumber)" + if let inFlight = prMobileGithubDetailInFlight[requestKey] { + return try await inFlight.value + } + + let task = Task { @MainActor [weak self] in + guard let self else { throw CancellationError() } + return try await self.sendDecodableCommand( + action: "prs.getMobileGithubDetail", + args: [ + "repoOwner": repoOwner, + "repoName": repoName, + "githubPrNumber": githubPrNumber, + ], + as: PrMobileGithubDetailSnapshot.self + ) + } + prMobileGithubDetailInFlight[requestKey] = task + do { + let result = try await task.value + prMobileGithubDetailInFlight[requestKey] = nil + return result + } catch { + prMobileGithubDetailInFlight[requestKey] = nil + throw error + } + } + /// Best-effort refresh of the shared `laneGithubPrItems` cache used by the /// Lanes and Work tabs to tag lanes with GitHub PRs opened outside ADE. /// Throttled (skips when refreshed within `minInterval` unless `force`), a @@ -11176,6 +11253,12 @@ final class SyncService: ObservableObject { } } resolve(requestId: requestId, result: .success(payload)) + case "prs_updated": + // Coalesce at the view task boundary. The host sends only a tiny + // invalidation signal; PRsTabView then performs one cached/projected + // snapshot read, never a GitHub poll per event. + prsRemoteRevision += 1 + resolve(requestId: requestId, result: .success(payload)) case "heartbeat": if let dict = payload as? [String: Any], (dict["kind"] as? String) == "ping" { sendEnvelope(type: "heartbeat", requestId: requestId, payload: [ diff --git a/apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift b/apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift index d614a67df..2d120f25b 100644 --- a/apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift +++ b/apps/ios/ADE/Views/PRs/PrDetailOverviewTab.swift @@ -319,11 +319,13 @@ private struct PrSummaryCommitRow: View { /// Amber banner shown at the top of the thread when the PR is not mapped to an /// ADE lane. Primary action is auto-map ("Create lane from PR branch", gated on -/// host support); the secondary action opens the PR on GitHub. (Linking to an -/// existing lane lives on the root unmapped-PR sheet.) +/// host support); mapping to an existing lane and opening GitHub stay available +/// without leaving the full detail view. struct PrUnmappedThreadBanner: View { let canAutoMap: Bool + let canMap: Bool let onAutoMap: () -> Void + let onMap: () -> Void let onOpenInGitHub: () -> Void var body: some View { @@ -345,33 +347,34 @@ struct PrUnmappedThreadBanner: View { Spacer(minLength: 0) } + if canAutoMap { + Button(action: onAutoMap) { + Label("Create lane from PR branch", systemImage: "arrow.triangle.branch") + .font(.subheadline.weight(.semibold)) + .foregroundStyle(.white) + .frame(maxWidth: .infinity, minHeight: 44) + .background(ADEColor.accentDeep, in: RoundedRectangle(cornerRadius: 11, style: .continuous)) + } + .buttonStyle(.plain) + } + HStack(spacing: 8) { - if canAutoMap { - Button(action: onAutoMap) { - Label("Create lane from PR branch", systemImage: "arrow.triangle.branch") - .font(.system(size: 12, weight: .semibold)) - .foregroundStyle(.white) - .frame(maxWidth: .infinity) - .padding(.vertical, 9) - .background(ADEColor.accentDeep, in: RoundedRectangle(cornerRadius: 11, style: .continuous)) + if canMap { + Button(action: onMap) { + Label("Map to lane", systemImage: "link") + .font(.subheadline.weight(.semibold)) + .foregroundStyle(ADEColor.accent) + .frame(maxWidth: .infinity, minHeight: 44) + .background(ADEColor.accent.opacity(0.12), in: RoundedRectangle(cornerRadius: 11, style: .continuous)) } .buttonStyle(.plain) } Button(action: onOpenInGitHub) { Label("Open in GitHub", systemImage: "arrow.up.right.square") - .font(.system(size: 12, weight: .semibold)) - .foregroundStyle(ADEColor.accent) - .frame(maxWidth: canAutoMap ? nil : .infinity) - .padding(.horizontal, canAutoMap ? 12 : 0) - .padding(.vertical, 9) - .background( - RoundedRectangle(cornerRadius: 11, style: .continuous) - .fill(ADEColor.accent.opacity(0.12)) - ) - .overlay( - RoundedRectangle(cornerRadius: 11, style: .continuous) - .strokeBorder(ADEColor.accent.opacity(0.35), lineWidth: 0.5) - ) + .font(.subheadline.weight(.semibold)) + .foregroundStyle(ADEColor.textSecondary) + .frame(maxWidth: .infinity, minHeight: 44) + .background(ADEColor.textMuted.opacity(0.10), in: RoundedRectangle(cornerRadius: 11, style: .continuous)) } .buttonStyle(.plain) } diff --git a/apps/ios/ADE/Views/PRs/PrDetailScreen.swift b/apps/ios/ADE/Views/PRs/PrDetailScreen.swift index 9a891fe88..205b97511 100644 --- a/apps/ios/ADE/Views/PRs/PrDetailScreen.swift +++ b/apps/ios/ADE/Views/PRs/PrDetailScreen.swift @@ -8,17 +8,20 @@ struct PrDetailView: View { let transitionNamespace: Namespace.ID? let requestedRepoOwner: String? let requestedRepoName: String? + let availableLanes: [LaneSummary] init( prId: String, transitionNamespace: Namespace.ID?, requestedRepoOwner: String? = nil, - requestedRepoName: String? = nil + requestedRepoName: String? = nil, + availableLanes: [LaneSummary] = [] ) { self.prId = prId self.transitionNamespace = transitionNamespace self.requestedRepoOwner = requestedRepoOwner self.requestedRepoName = requestedRepoName + self.availableLanes = availableLanes } @State private var pr: PullRequestListItem? @@ -31,6 +34,7 @@ struct PrDetailView: View { @State private var aiSummary: AiReviewSummary? @State private var groupMembers: [PrGroupMemberSummary] = [] @State private var capabilities: PrActionCapabilities? + @State private var unavailableDetailParts: [String] = [] @State private var selectedTab: PrDetailTab = .overview @State private var mergeMethod: PrMergeMethodOption = .squash @State private var reviewerInput = "" @@ -41,6 +45,7 @@ struct PrDetailView: View { @State private var cleanupConfirmationPresented = false @State private var filesWorkspaceId: String? @State private var stackPresentation: PrStackPresentation? + @State private var laneLinkItem: GitHubPrListItem? @State private var editorSheet: PrDetailEditorSheet? @State private var mergeMethodSheetPresented: Bool = false @State private var actionsSheetPresented: Bool = false @@ -146,7 +151,11 @@ struct PrDetailView: View { } private var routedPrNumber: Int? { - Self.prNumber(fromRouteId: prId) + routedGitHubCoordinates?.githubPrNumber ?? Self.prNumber(fromRouteId: prId) + } + + private var routedGitHubCoordinates: (repoOwner: String, repoName: String, githubPrNumber: Int)? { + prGitHubCoordinates(fromRouteId: prId) } private var requestedRepoScope: PrDetailRouteScope? { @@ -162,7 +171,7 @@ struct PrDetailView: View { } private var hasActionablePrId: Bool { - pr != nil || snapshot != nil || githubItem?.linkedPrId != nil + pr != nil || githubItem?.linkedPrId != nil } private var isAwaitingInitialPrDetail: Bool { @@ -361,10 +370,17 @@ struct PrDetailView: View { private var canAutoMapCurrentPr: Bool { isLive && !isDetailBusy && syncService.supportsRemoteAction("prs.createLaneFromPrBranch") + && githubItem?.scope != "external" && !currentPr.repoOwner.isEmpty && !currentPr.repoName.isEmpty && currentPr.githubPrNumber > 0 } + private var canMapCurrentPr: Bool { + isLive && !isDetailBusy && githubItem != nil + && githubItem?.scope != "external" + && syncService.supportsRemoteAction("prs.linkToLane") + } + /// GitHub-style requirement checklist for the merge rail + merge sheet. /// Mirrors desktop's `buildMergeChecklist`, driven by the structured /// merge-state fields. @@ -510,6 +526,18 @@ struct PrDetailView: View { .prListRow() } + if !unavailableDetailParts.isEmpty { + ADENoticeCard( + title: "Some PR data is unavailable", + message: "ADE could not refresh \(unavailableDetailParts.map { $0.replacingOccurrences(of: "_", with: " ") }.joined(separator: ", ")). The visible data is partial, not a zero result.", + icon: "exclamationmark.arrow.triangle.2.circlepath", + tint: ADEColor.warning, + actionTitle: "Retry", + action: { Task { await retryPrDetailLoad() } } + ) + .prListRow() + } + if isAwaitingInitialPrDetail { ADECardSkeleton(rows: 4) .prListRow() @@ -591,7 +619,7 @@ struct PrDetailView: View { await reload(refreshRemote: true) } .adeNavigationZoomTransition(id: transitionNamespace == nil ? nil : "pr-container-\(prId)", in: transitionNamespace) - .task(id: syncService.prsProjectionRevision) { + .task(id: "\(syncService.prsProjectionRevision):\(syncService.prsRemoteRevision)") { // Seed from the warm cache first so an instant render is shown and, when // the cached entry is fresh, `hasLoadedLiveSidecars` is set BEFORE the // gate below evaluates. Doing this inside `.task` (rather than relying on @@ -659,6 +687,29 @@ struct PrDetailView: View { PrStackSheet(groupId: presentation.id, groupName: presentation.groupName) .environmentObject(syncService) } + .sheet(item: $laneLinkItem) { item in + PrLaneLinkSheet( + item: item, + lanes: availableLanes, + canLink: canMapCurrentPr + ) { laneId in + runPrAction( + "Linking pull request", + action: { + try await syncService.linkPullRequestToLane( + laneId: laneId, + prUrlOrNumber: item.githubUrl.isEmpty ? "\(item.githubPrNumber)" : item.githubUrl + ) + }, + onSuccess: { laneLinkItem = nil } + ) + } onOpenGitHub: { + openGitHub(urlString: item.githubUrl) + } + .presentationDetents([.large]) + .presentationDragIndicator(.hidden) + .presentationBackground(.clear) + } .sheet(isPresented: $actionsSheetPresented) { prActionsSheet .presentationDetents([.height(560)]) @@ -922,7 +973,13 @@ struct PrDetailView: View { if !isCurrentPrMapped { PrUnmappedThreadBanner( canAutoMap: canAutoMapCurrentPr, + canMap: canMapCurrentPr, onAutoMap: autoMapCurrentPr, + onMap: { + if let githubItem { + laneLinkItem = githubItem + } + }, onOpenInGitHub: { openGitHub(urlString: currentPr.githubUrl) } ) .prListRow() @@ -1309,11 +1366,12 @@ struct PrDetailView: View { @MainActor private func reload(refreshRemote: Bool = false, includeLiveSidecars: Bool? = nil) async { let requestedPrNumber = routedPrNumber - let shouldFetchLiveSidecars = isLive && ((includeLiveSidecars ?? refreshRemote) || requestedPrNumber != nil) + let routeCoordinates = routedGitHubCoordinates + let shouldFetchLiveSidecars = isLive && (includeLiveSidecars ?? (refreshRemote || requestedPrNumber != nil)) do { var refreshError: Error? - if refreshRemote { + if refreshRemote, routeCoordinates == nil { do { if requestedPrNumber == nil { try await syncService.refreshPullRequestSnapshots(prId: effectivePrId) @@ -1325,9 +1383,11 @@ struct PrDetailView: View { } } let listItems = try await syncService.fetchPullRequestListItems() - var fallbackGitHubItem: GitHubPrListItem? - if shouldFetchLiveSidecars && requestedPrNumber != nil { - fallbackGitHubItem = await fetchGitHubFallbackItem(requestedPrNumber: requestedPrNumber) + var fallbackGitHubItem: GitHubPrListItem? = githubItem + if shouldFetchLiveSidecars && requestedPrNumber != nil && routeCoordinates == nil { + if fallbackGitHubItem == nil { + fallbackGitHubItem = await fetchGitHubFallbackItem(requestedPrNumber: requestedPrNumber) + } } pr = prDetailRouteListItem( @@ -1339,6 +1399,52 @@ struct PrDetailView: View { requestedRepoName: requestedRepoScope?.repoName ) let snapshotPrId = pr?.id ?? (requestedPrNumber == nil ? prId : nil) + + if pr == nil, let routeCoordinates { + githubItem = fallbackGitHubItem + if shouldFetchLiveSidecars { + let mobileDetail = try await syncService.fetchPrMobileGithubDetail( + repoOwner: routeCoordinates.repoOwner, + repoName: routeCoordinates.repoName, + githubPrNumber: routeCoordinates.githubPrNumber + ) + let unavailable = Set(mobileDetail.unavailableParts) + let previousSnapshot = snapshot + let incomingSnapshot = mobileDetail.snapshot + githubItem = mobileDetail.item + snapshot = prMergeMobileGithubSnapshot( + incoming: incomingSnapshot, + previous: previousSnapshot, + unavailableParts: mobileDetail.unavailableParts + ) + if !unavailable.contains("review_threads") { + reviewThreads = mobileDetail.reviewThreads + } + if !unavailable.contains("action_runs") { + actionRuns = mobileDetail.actionRuns + } + if !unavailable.contains("activity") { + activityEvents = mobileDetail.activity + } + unavailableDetailParts = mobileDetail.unavailableParts + deployments = [] + aiSummary = nil + capabilities = nil + groupMembers = [] + // Partial failures retain the last good values above and use the + // normal 25-second freshness window as retry backoff. Explicit Retry + // still bypasses the gate immediately. + hasLoadedLiveSidecars = true + } + errorMessage = refreshError?.localizedDescription + recomputeDerivedModels() + if shouldFetchLiveSidecars { + storeWarmCache() + } + hasAttemptedInitialLoad = true + return + } + if let snapshotPrId { snapshot = try await syncService.fetchPullRequestSnapshot(prId: snapshotPrId) } else { @@ -1393,6 +1499,7 @@ struct PrDetailView: View { } else { groupMembers = [] } + unavailableDetailParts = [] errorMessage = refreshError?.localizedDescription } catch { errorMessage = error.localizedDescription @@ -1432,6 +1539,7 @@ struct PrDetailView: View { aiSummary = entry.aiSummary groupMembers = entry.groupMembers capabilities = entry.capabilities + unavailableDetailParts = entry.unavailableParts // Treat the cache as a successful prior load so the UI shows content (not // the skeleton/unavailable states) while the background refresh runs. hasAttemptedInitialLoad = true @@ -1461,6 +1569,7 @@ struct PrDetailView: View { aiSummary: aiSummary, groupMembers: groupMembers, capabilities: capabilities, + unavailableParts: unavailableDetailParts, loadedAt: Date() ), for: prId @@ -1476,7 +1585,10 @@ struct PrDetailView: View { @MainActor private func fetchGitHubFallbackItem(requestedPrNumber: Int?) async -> GitHubPrListItem? { - guard let github = try? await syncService.fetchGitHubPullRequestSnapshot() else { return nil } + guard let github = try? await syncService.fetchGitHubPullRequestSnapshot( + includeExternalClosed: true, + historyPageLimit: 2 + ) else { return nil } let routeScope = requestedRepoScope return repoScopedGitHubPullRequests(from: github) .first { diff --git a/apps/ios/ADE/Views/PRs/PrHelpers.swift b/apps/ios/ADE/Views/PRs/PrHelpers.swift index d036ab752..24b3f3682 100644 --- a/apps/ios/ADE/Views/PRs/PrHelpers.swift +++ b/apps/ios/ADE/Views/PRs/PrHelpers.swift @@ -59,6 +59,26 @@ func shouldFetchPrDetailLiveSidecars(hasLoadedLiveSidecars: Bool, refreshRemote: refreshRemote || !hasLoadedLiveSidecars } +/// Applies a partial aggregate response without turning unavailable sidecars +/// into authoritative empty values. Fresh fields replace the cache; failed +/// fields retain the last known good value and remain visibly marked partial. +func prMergeMobileGithubSnapshot( + incoming: PullRequestSnapshot, + previous: PullRequestSnapshot?, + unavailableParts: [String] +) -> PullRequestSnapshot { + let unavailable = Set(unavailableParts) + return PullRequestSnapshot( + detail: incoming.detail, + status: unavailable.contains("status") ? previous?.status : incoming.status, + checks: unavailable.contains("checks") ? (previous?.checks ?? []) : incoming.checks, + reviews: unavailable.contains("reviews") ? (previous?.reviews ?? []) : incoming.reviews, + comments: unavailable.contains("comments") ? (previous?.comments ?? []) : incoming.comments, + files: unavailable.contains("files") ? (previous?.files ?? []) : incoming.files, + commits: unavailable.contains("commits") ? previous?.commits : incoming.commits + ) +} + func parsePullRequestPatch(_ patch: String) -> [PrDiffDisplayLine] { guard !patch.isEmpty else { return [] } @@ -258,6 +278,135 @@ func repoScopedGitHubPullRequests(from snapshot: GitHubPrSnapshot?) -> [GitHubPr snapshot?.repoPullRequests ?? [] } +func prSyntheticGitHubId(repoOwner: String, repoName: String, githubPrNumber: Int) -> String { + "gh:\(repoOwner)/\(repoName)#\(githubPrNumber)" +} + +func prSyntheticGitHubId(for item: GitHubPrListItem) -> String { + prSyntheticGitHubId( + repoOwner: item.repoOwner, + repoName: item.repoName, + githubPrNumber: item.githubPrNumber + ) +} + +func prGitHubCoordinates(fromRouteId routeId: String) -> (repoOwner: String, repoName: String, githubPrNumber: Int)? { + guard routeId.hasPrefix("gh:") else { return nil } + let locator = routeId.dropFirst(3) + guard let hashIndex = locator.lastIndex(of: "#"), + let slashIndex = locator[.. 0 else { return nil } + return (owner, repo, number) +} + +private func prGitHubIdentityKey(repoOwner: String, repoName: String, githubPrNumber: Int) -> String { + "\(repoOwner.lowercased())/\(repoName.lowercased())#\(githubPrNumber)" +} + +/// Reconciles the bounded live GitHub projection with replicated ADE PR rows. +/// The host projection is authoritative for rich GitHub metadata, while the +/// local rows keep linked and terminal PRs visible during reconnects or while a +/// webhook-backed projection refresh is still in flight. +func prReconcileGitHubPullRequests( + snapshotItems: [GitHubPrListItem], + mappedPrs: [PullRequestListItem] +) -> [GitHubPrListItem] { + var result = snapshotItems + var indexByIdentity: [String: Int] = [:] + indexByIdentity.reserveCapacity(snapshotItems.count + mappedPrs.count) + for (index, item) in result.enumerated() { + indexByIdentity[ + prGitHubIdentityKey( + repoOwner: item.repoOwner, + repoName: item.repoName, + githubPrNumber: item.githubPrNumber + ) + ] = index + } + + for mapped in mappedPrs { + let key = prGitHubIdentityKey( + repoOwner: mapped.repoOwner, + repoName: mapped.repoName, + githubPrNumber: mapped.githubPrNumber + ) + if let index = indexByIdentity[key] { + var item = result[index] + item.linkedPrId = mapped.id + item.linkedGroupId = mapped.linkedGroupId + item.linkedLaneId = mapped.laneId + item.linkedLaneName = mapped.laneName + item.adeKind = mapped.adeKind + item.workflowDisplayState = mapped.workflowDisplayState + item.cleanupState = mapped.cleanupState + // Let the newest projection own state in either direction. This keeps a + // terminal replicated row visible behind an older open-only response, + // while also allowing a freshly reopened local row to override a stale + // closed projection. If timestamps are unavailable, retain the previous + // terminal-row safety behavior. + let mappedUpdatedAt = prParsedDate(mapped.updatedAt) + let projectedUpdatedAt = prParsedDate(item.updatedAt) + let mappedStateIsNewer = if let mappedUpdatedAt, let projectedUpdatedAt { + mappedUpdatedAt >= projectedUpdatedAt + } else { + mapped.state == "merged" || mapped.state == "closed" + } + if mappedStateIsNewer { + item.state = mapped.state + item.isDraft = mapped.state == "draft" + item.title = mapped.title + item.githubUrl = mapped.githubUrl + item.baseBranch = mapped.baseBranch + item.headBranch = mapped.headBranch + item.updatedAt = mapped.updatedAt + } + result[index] = item + continue + } + + let item = GitHubPrListItem( + id: prSyntheticGitHubId( + repoOwner: mapped.repoOwner, + repoName: mapped.repoName, + githubPrNumber: mapped.githubPrNumber + ), + scope: "repo", + repoOwner: mapped.repoOwner, + repoName: mapped.repoName, + githubPrNumber: mapped.githubPrNumber, + githubUrl: mapped.githubUrl, + title: mapped.title, + state: mapped.state, + isDraft: mapped.state == "draft", + baseBranch: mapped.baseBranch, + headBranch: mapped.headBranch, + headRepoOwner: mapped.repoOwner, + headRepoName: mapped.repoName, + author: nil, + createdAt: mapped.createdAt, + updatedAt: mapped.updatedAt, + linkedPrId: mapped.id, + linkedGroupId: mapped.linkedGroupId, + linkedLaneId: mapped.laneId, + linkedLaneName: mapped.laneName, + adeKind: mapped.adeKind, + workflowDisplayState: mapped.workflowDisplayState, + cleanupState: mapped.cleanupState, + labels: [], + isBot: false, + commentCount: 0 + ) + indexByIdentity[key] = result.count + result.append(item) + } + + return result +} + // MARK: - GitHub PR list filter/sort/count (free functions) // // These mirror the predicates that used to live as private methods on diff --git a/apps/ios/ADE/Views/PRs/PrModels.swift b/apps/ios/ADE/Views/PRs/PrModels.swift index c421fa23d..8b909dd85 100644 --- a/apps/ios/ADE/Views/PRs/PrModels.swift +++ b/apps/ios/ADE/Views/PRs/PrModels.swift @@ -404,6 +404,7 @@ struct PrDetailWarmEntry { var aiSummary: AiReviewSummary? var groupMembers: [PrGroupMemberSummary] var capabilities: PrActionCapabilities? + var unavailableParts: [String] var loadedAt: Date } diff --git a/apps/ios/ADE/Views/PRs/PrRowCard.swift b/apps/ios/ADE/Views/PRs/PrRowCard.swift index 936ff5de6..a27760339 100644 --- a/apps/ios/ADE/Views/PRs/PrRowCard.swift +++ b/apps/ios/ADE/Views/PRs/PrRowCard.swift @@ -5,7 +5,6 @@ struct PrRowCard: View { let transitionNamespace: Namespace.ID? let isSelectedTransitionSource: Bool let onShowStack: (String, String?) -> Void - let onLink: (() -> Void)? init( pr: PullRequestListItem, @@ -17,46 +16,64 @@ struct PrRowCard: View { self.transitionNamespace = transitionNamespace self.isSelectedTransitionSource = isSelectedTransitionSource self.onShowStack = onShowStack - self.onLink = nil } init( item: GitHubPrListItem, linkedPr: PullRequestListItem? = nil, transitionNamespace: Namespace.ID? = nil, - isSelectedTransitionSource: Bool = false, - onLink: (() -> Void)? = nil + isSelectedTransitionSource: Bool = false ) { self.data = Data(item: item, linkedPr: linkedPr) self.transitionNamespace = transitionNamespace self.isSelectedTransitionSource = isSelectedTransitionSource self.onShowStack = { _, _ in } - self.onLink = onLink } var body: some View { let stateColors = PrRowDesktopPalette.stateColors(data.state) - VStack(alignment: .leading, spacing: 6) { - primaryRow(stateColors: stateColors) - if !data.visibleLabels.isEmpty { - labelsRow - } - if data.showsBranchRow { - branchRow - } - statsRow - if !data.isUnmapped, let warnMessage = data.warnMessage { - PrWarnBanner(text: warnMessage) + HStack(alignment: .top, spacing: 12) { + Image(systemName: stateSymbol) + .font(.body.weight(.semibold)) + .foregroundStyle(stateColors.text) + .frame(width: 22, height: 22) + .padding(.top, 1) + + VStack(alignment: .leading, spacing: 7) { + HStack(alignment: .firstTextBaseline, spacing: 8) { + Text(data.title) + .font(.body.weight(.semibold)) + .foregroundStyle(PrsGlass.textPrimary) + .lineLimit(2) + .truncationMode(.tail) + .frame(maxWidth: .infinity, alignment: .leading) + .adeMatchedGeometry(id: isSelectedTransitionSource ? "pr-title-\(data.id)" : nil, in: transitionNamespace) + + if !data.timeAgo.isEmpty { + Text(data.timeAgo) + .font(.caption2.monospaced()) + .foregroundStyle(PrsGlass.textMuted) + .lineLimit(1) + .fixedSize() + } + } + + metadataRow + signalsRow + + if !data.isUnmapped, let warnMessage = data.warnMessage { + PrWarnBanner(text: warnMessage) + } } } - .padding(.horizontal, 14) - .padding(.vertical, 9) + .padding(.horizontal, 16) + .padding(.vertical, 13) .frame(maxWidth: .infinity, alignment: .leading) .background { if isSelectedTransitionSource { LinearGradient( - colors: [stateColors.background, Color.white.opacity(0.02)], + colors: [stateColors.background, Color.white.opacity(0.015)], startPoint: .leading, endPoint: .trailing ) @@ -71,12 +88,13 @@ struct PrRowCard: View { } .overlay(alignment: .bottom) { Rectangle() - .fill(Color.white.opacity(0.04)) + .fill(Color.white.opacity(0.055)) .frame(height: 1) + .padding(.leading, 50) } .adeMatchedTransitionSource(id: isSelectedTransitionSource ? "pr-container-\(data.id)" : nil, in: transitionNamespace) .accessibilityElement(children: .combine) - .accessibilityLabel("PR #\(data.prNumber): \(data.title), state \(data.state)") + .accessibilityLabel(accessibilitySummary) .adeInspectable( "PR.List.Row", metadata: [ @@ -90,307 +108,117 @@ struct PrRowCard: View { ) } - private func primaryRow(stateColors: PrRowDesktopPalette.StateColors) -> some View { - HStack(alignment: .center, spacing: 6) { - authorAvatar(borderColor: stateColors.background) - - if data.isBot { - Text("bot") - .font(.system(size: 9, weight: .bold)) - .textCase(.uppercase) - .foregroundStyle(PrsGlass.textMuted) - .padding(.horizontal, 5) - .padding(.vertical, 1) - .background( - RoundedRectangle(cornerRadius: 3, style: .continuous) - .fill(Color.white.opacity(0.06)) - ) - } - - Text("#\(data.prNumber)") - .font(.system(size: 11, weight: .bold, design: .monospaced)) - .foregroundStyle(stateColors.text) - - Text(data.title) - .font(.system(size: 12, weight: .semibold)) - .foregroundStyle(PrsGlass.textPrimary) - .lineLimit(1) - .truncationMode(.tail) - .frame(maxWidth: .infinity, alignment: .leading) - .adeMatchedGeometry(id: isSelectedTransitionSource ? "pr-title-\(data.id)" : nil, in: transitionNamespace) - - if let ci = data.ciIndicator { - Image(systemName: ci.symbol) - .font(.system(size: 14, weight: .semibold)) - .foregroundStyle(ci.color) - .accessibilityLabel(ci.title) - } - - if !data.timeAgo.isEmpty { - PrMonoText(text: data.timeAgo, color: PrsGlass.textMuted, size: 10) - .lineLimit(1) - } - - if data.commentCount > 0 { - HStack(spacing: 3) { - Image(systemName: "text.bubble") - .font(.system(size: 12)) - Text("\(data.commentCount)") - .font(.system(size: 10, weight: .regular, design: .monospaced)) - } - .foregroundStyle(PrsGlass.textMuted) - } + private var stateSymbol: String { + switch data.state { + case "merged": return "arrow.triangle.merge" + case "closed": return "xmark.circle" + case "draft": return "circle.dashed" + default: return "arrow.triangle.pull" } } - private var labelsRow: some View { - HStack(spacing: 4) { - ForEach(data.visibleLabels) { label in - Text(label.name) - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(prLabelTextColor(label.color)) - .padding(.horizontal, 8) - .padding(.vertical, 1) - .background( - Capsule(style: .continuous) - .fill(Color(hex: label.color)) - ) - .lineLimit(1) - } - if data.labelOverflowCount > 0 { - Text("+\(data.labelOverflowCount)") - .font(.system(size: 10)) - .foregroundStyle(PrsGlass.textMuted) - } + private var accessibilitySummary: String { + var parts = ["Pull request \(data.prNumber)", data.title, "State \(data.state)"] + if let author = data.authorLogin, !author.isEmpty { parts.append("Author \(author)") } + if let head = data.headBranch, let base = data.baseBranch { parts.append("\(head) into \(base)") } + if let ci = data.ciIndicator { parts.append(ci.title) } + if let review = data.reviewIndicator { parts.append(review.label) } + if data.commentCount > 0 { parts.append("\(data.commentCount) comments") } + if data.isUnmapped { + parts.append("Not mapped to an ADE lane") + } else if let lane = data.laneLabel { + parts.append("Lane \(lane)") } - .padding(.leading, 30) + if !data.timeAgo.isEmpty { parts.append("Updated \(data.timeAgo)") } + return parts.joined(separator: ", ") } - private var branchRow: some View { - HStack(spacing: 4) { - if let head = data.headBranch { - PrMonoText(text: head, color: PrsGlass.textMuted, size: 10) - .lineLimit(1) - .truncationMode(.tail) + private var metadataRow: some View { + HStack(spacing: 5) { + Text("#\(data.prNumber)") + .foregroundStyle(PrRowDesktopPalette.stateColors(data.state).text) + if let author = data.authorLogin, !author.isEmpty { + Text("·") + Text("@\(author)") } - if data.headBranch != nil, data.baseBranch != nil { - Text("→") - .font(.system(size: 10, weight: .regular, design: .monospaced)) - .foregroundStyle(PrsGlass.textSecondary.opacity(0.55)) + if data.isBot { + Text("bot") + .fontWeight(.semibold) } - if let base = data.baseBranch { - PrMonoText(text: base, color: PrsGlass.textMuted, size: 10) + if data.isExternal { + Text("·") + Text("\(data.repoOwner)/\(data.repoName)") .lineLimit(1) } + Spacer(minLength: 0) } - .padding(.leading, 30) + .font(.caption.monospaced()) + .foregroundStyle(PrsGlass.textMuted) } - private var statsRow: some View { - HStack(spacing: 6) { - if data.showsStateBadge { - Text(titleCase(data.state)) - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(PrRowDesktopPalette.stateColors(data.state).text) - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(PrRowDesktopPalette.stateColors(data.state).background) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(PrRowDesktopPalette.stateColors(data.state).border, lineWidth: 1) - ) - } - - if let adeKind = data.adeKindBadgeLabel, let style = PrRowDesktopPalette.adeKindStyle(data.adeKind) { - Text(adeKind) - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(style.text) - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(style.background) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(style.border, lineWidth: 1) - ) - } - - if data.isExternal { - PrMonoText( - text: "\(data.repoOwner)/\(data.repoName)", - color: PrsGlass.textMuted, - size: 10 - ) - .lineLimit(1) - } - - if let laneLabel = data.laneLabel { + @ViewBuilder + private var signalsRow: some View { + HStack(spacing: 12) { + if let head = data.headBranch, let base = data.baseBranch { HStack(spacing: 4) { - Circle() - .fill(data.laneTint ?? PrsGlass.textSecondary) - .frame(width: 6, height: 6) - Text(laneLabel) - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(data.laneTint ?? PrsGlass.textSecondary) + Text(head) + .lineLimit(1) + .truncationMode(.middle) + Text("→") + Text(base) .lineLimit(1) } - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(Color.white.opacity(0.04)) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(Color.white.opacity(0.08), lineWidth: 1) - ) - } else if data.isUnmapped { - Text("unmapped") - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(PrsGlass.draftTop) - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.10)) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.18), lineWidth: 1) - ) + .font(.caption2.monospaced()) + .foregroundStyle(PrsGlass.textSecondary) + .layoutPriority(1) + } + + Spacer(minLength: 0) + + if let ci = data.ciIndicator { + Image(systemName: ci.symbol) + .foregroundStyle(ci.color) + .accessibilityLabel(ci.title) } if let review = data.reviewIndicator { - Text(review.label) - .font(.system(size: 10, weight: .medium)) + Image(systemName: reviewSymbol) .foregroundStyle(review.color) - .padding(.horizontal, 6) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 4, style: .continuous) - .fill(review.color.opacity(0.10)) - ) + .accessibilityLabel(review.label) } - if data.additions > 0 || data.deletions > 0 { - HStack(spacing: 4) { - Text("+\(data.additions)") - .foregroundStyle(PrsGlass.openTop) - Text("-\(data.deletions)") - .foregroundStyle(PrsGlass.closedTop) - } - .font(.system(size: 10, weight: .regular, design: .monospaced)) + if data.commentCount > 0 { + Label("\(data.commentCount)", systemImage: "text.bubble") + .labelStyle(.titleAndIcon) } - if data.cleanupRequired { - Text("cleanup") - .font(.system(size: 10, weight: .semibold)) + if data.isUnmapped { + Label("Unmapped", systemImage: "link.badge.plus") .foregroundStyle(PrsGlass.draftTop) - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(PrsGlass.draftTop.opacity(0.10)) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(PrsGlass.draftTop.opacity(0.18), lineWidth: 1) - ) + } else if let laneLabel = data.laneLabel { + Label(laneLabel, systemImage: "rectangle.stack") + .lineLimit(1) } - if data.adeKind == "queue", let groupId = data.stackGroupId { + if let groupId = data.stackGroupId, let groupCount = data.stackGroupCount, groupCount > 0 { Button { onShowStack(groupId, data.stackGroupName) } label: { - Text("open queue") - .font(.system(size: 10, weight: .semibold)) - .foregroundStyle(PrsGlass.externalTop) - .padding(.horizontal, 7) - .padding(.vertical, 2) - .background( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .fill(PrsGlass.externalTop.opacity(0.10)) - ) - .overlay( - RoundedRectangle(cornerRadius: 5, style: .continuous) - .stroke(PrsGlass.externalTop.opacity(0.18), lineWidth: 1) - ) - } - .buttonStyle(.plain) - } else if let groupId = data.stackGroupId, let groupCount = data.stackGroupCount, groupCount > 0 { - Button { - onShowStack(groupId, data.stackGroupName) - } label: { - HStack(spacing: 3) { - Image(systemName: "list.number") - .font(.system(size: 9, weight: .bold)) - Text("\(groupCount)") - .font(.system(size: 10, weight: .semibold, design: .monospaced)) - } - .foregroundStyle(PrsGlass.textSecondary) - .padding(.horizontal, 6) - .padding(.vertical, 2) - .background { - Capsule(style: .continuous) - .fill(Color.white.opacity(0.06)) - } - .overlay { - Capsule(style: .continuous) - .stroke(Color.white.opacity(0.10), lineWidth: 0.5) - } + Label("\(groupCount)", systemImage: "list.number") } .buttonStyle(.plain) .accessibilityLabel("Open stack of \(groupCount) pull requests") } - - Spacer(minLength: 0) - - if data.isUnmapped, let onLink { - Button(action: onLink) { - Image(systemName: "link") - .font(.system(size: 13, weight: .semibold)) - .foregroundStyle(PrsGlass.textMuted) - } - .buttonStyle(.plain) - .accessibilityLabel("Link pull request to a lane") - } } - .padding(.leading, 30) + .font(.caption2.weight(.medium)) + .foregroundStyle(PrsGlass.textMuted) } - @ViewBuilder - private func authorAvatar(borderColor: Color) -> some View { - if let author = data.authorLogin { - AsyncImage(url: URL(string: "https://avatars.githubusercontent.com/\(author)?size=64")) { phase in - switch phase { - case .success(let image): - image - .resizable() - .scaledToFill() - default: - Circle() - .fill(Color.white.opacity(0.05)) - } - } - .frame(width: 22, height: 22) - .clipShape(Circle()) - .overlay { - Circle() - .stroke(borderColor, lineWidth: 1.5) - } - } else { - Circle() - .fill(Color.white.opacity(0.05)) - .overlay { - Circle() - .stroke(Color.white.opacity(0.08), lineWidth: 1) - } - .frame(width: 22, height: 22) + private var reviewSymbol: String { + switch data.reviewStatus { + case "approved": return "checkmark.bubble" + case "changes_requested": return "exclamationmark.bubble" + default: return "bubble.left.and.exclamationmark.bubble.right" } } } @@ -398,90 +226,33 @@ struct PrRowCard: View { private enum PrRowDesktopPalette { struct StateColors { let background: Color - let border: Color let text: Color } - struct AdeKindStyle { - let text: Color - let background: Color - let border: Color - } - static func stateColors(_ state: String) -> StateColors { switch state { case "open": return StateColors( background: Color(red: 0x3B / 255, green: 0x82 / 255, blue: 0xF6 / 255, opacity: 0.10), - border: Color(red: 0x3B / 255, green: 0x82 / 255, blue: 0xF6 / 255, opacity: 0.20), text: Color(red: 0x60 / 255, green: 0xA5 / 255, blue: 0xFA / 255) ) case "draft": return StateColors( background: Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.10), - border: Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.20), text: Color(red: 0xFB / 255, green: 0xBF / 255, blue: 0x24 / 255) ) case "merged": return StateColors( background: Color(red: 0x22 / 255, green: 0xC5 / 255, blue: 0x5E / 255, opacity: 0.10), - border: Color(red: 0x22 / 255, green: 0xC5 / 255, blue: 0x5E / 255, opacity: 0.20), text: Color(red: 0x4A / 255, green: 0xDE / 255, blue: 0x80 / 255) ) default: return StateColors( background: Color(red: 0xA1 / 255, green: 0xA1 / 255, blue: 0xAA / 255, opacity: 0.08), - border: Color(red: 0xA1 / 255, green: 0xA1 / 255, blue: 0xAA / 255, opacity: 0.15), text: Color(red: 0xA1 / 255, green: 0xA1 / 255, blue: 0xAA / 255) ) } } - - static func adeKindStyle(_ adeKind: String?) -> AdeKindStyle? { - switch adeKind { - case "integration": - return AdeKindStyle( - text: Color(red: 0xFB / 255, green: 0xBF / 255, blue: 0x24 / 255), - background: Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.14), - border: Color(red: 0xF5 / 255, green: 0x9E / 255, blue: 0x0B / 255, opacity: 0.22) - ) - case "queue": - return AdeKindStyle( - text: Color(red: 0x60 / 255, green: 0xA5 / 255, blue: 0xFA / 255), - background: Color(red: 0x3B / 255, green: 0x82 / 255, blue: 0xF6 / 255, opacity: 0.14), - border: Color(red: 0x3B / 255, green: 0x82 / 255, blue: 0xF6 / 255, opacity: 0.22) - ) - default: - return nil - } - } -} - -private func prLabelTextColor(_ hexColor: String) -> Color { - let hex = hexColor.trimmingCharacters(in: CharacterSet(charactersIn: "#")) - guard hex.count >= 6, - let value = Int(hex.prefix(6), radix: 16) else { - return PrsGlass.textPrimary - } - let r = Double((value >> 16) & 0xFF) / 255 - let g = Double((value >> 8) & 0xFF) / 255 - let b = Double(value & 0xFF) / 255 - let luminance = (0.299 * r) + (0.587 * g) + (0.114 * b) - return luminance > 0.5 - ? Color(red: 0x1A / 255, green: 0x1A / 255, blue: 0x2E / 255) - : PrsGlass.textPrimary -} - -private extension Color { - init(hex: String) { - let hex = hex.trimmingCharacters(in: CharacterSet(charactersIn: "#")) - let value = Int(hex.prefix(6), radix: 16) ?? 0 - self.init( - red: Double((value >> 16) & 0xFF) / 255, - green: Double((value >> 8) & 0xFF) / 255, - blue: Double(value & 0xFF) / 255 - ) - } } extension PrRowCard { @@ -490,56 +261,28 @@ extension PrRowCard { let prNumber: Int let title: String let state: String - let adeKind: String? - let createdAt: String let updatedAt: String let headBranch: String? let baseBranch: String? let authorLogin: String? let isBot: Bool - let labels: [PrLabel] let commentCount: Int let repoOwner: String let repoName: String let isExternal: Bool let isUnmapped: Bool let laneLabel: String? - let laneTint: Color? let checksStatus: String? let reviewStatus: String? - let additions: Int - let deletions: Int - let cleanupRequired: Bool let warnMessage: String? let stackGroupId: String? let stackGroupName: String? let stackGroupCount: Int? - var visibleLabels: [PrLabel] { - Array(labels.prefix(4)) - } - - var labelOverflowCount: Int { - max(0, labels.count - 4) - } - var timeAgo: String { prRelativeTime(updatedAt) } - var showsBranchRow: Bool { - headBranch != nil && baseBranch != nil - } - - var showsStateBadge: Bool { - state != "open" && state != "draft" - } - - var adeKindBadgeLabel: String? { - guard let adeKind, adeKind != "single" else { return nil } - return adeKind - } - struct CIIndicator { let symbol: String let color: Color @@ -582,26 +325,19 @@ extension PrRowCard { self.prNumber = pr.githubPrNumber self.title = pr.title self.state = pr.state - self.adeKind = pr.adeKind - self.createdAt = pr.createdAt self.updatedAt = pr.updatedAt self.headBranch = pr.headBranch self.baseBranch = pr.baseBranch self.authorLogin = nil self.isBot = false - self.labels = [] self.commentCount = 0 self.repoOwner = pr.repoOwner self.repoName = pr.repoName self.isExternal = false self.isUnmapped = false self.laneLabel = pr.laneName ?? pr.laneId - self.laneTint = ADEColor.tintPRs self.checksStatus = pr.checksStatus == "none" ? nil : pr.checksStatus self.reviewStatus = pr.reviewStatus == "none" ? nil : pr.reviewStatus - self.additions = pr.additions - self.deletions = pr.deletions - self.cleanupRequired = pr.cleanupState == "required" self.warnMessage = Self.warnMessage( workflowDisplayState: pr.workflowDisplayState, checksStatus: pr.checksStatus, @@ -621,26 +357,19 @@ extension PrRowCard { self.prNumber = item.githubPrNumber self.title = item.title self.state = item.isDraft ? "draft" : item.state - self.adeKind = item.adeKind - self.createdAt = item.createdAt self.updatedAt = item.updatedAt self.headBranch = item.headBranch self.baseBranch = item.baseBranch self.authorLogin = item.author self.isBot = item.isBot - self.labels = item.labels self.commentCount = item.commentCount self.repoOwner = item.repoOwner self.repoName = item.repoName self.isExternal = item.scope == "external" self.isUnmapped = unmapped self.laneLabel = item.linkedLaneName ?? item.linkedLaneId ?? linkedPr?.laneName ?? linkedPr?.laneId - self.laneTint = ADEColor.tintPRs self.checksStatus = linkedPr?.checksStatus == "none" ? nil : linkedPr?.checksStatus self.reviewStatus = linkedPr?.reviewStatus == "none" ? nil : linkedPr?.reviewStatus - self.additions = linkedPr?.additions ?? 0 - self.deletions = linkedPr?.deletions ?? 0 - self.cleanupRequired = item.cleanupState == "required" self.warnMessage = unmapped ? nil : Self.warnMessage( diff --git a/apps/ios/ADE/Views/PRs/PrRowCardPreviews.swift b/apps/ios/ADE/Views/PRs/PrRowCardPreviews.swift index a4fff1756..d254e4031 100644 --- a/apps/ios/ADE/Views/PRs/PrRowCardPreviews.swift +++ b/apps/ios/ADE/Views/PRs/PrRowCardPreviews.swift @@ -134,8 +134,8 @@ private enum PrRowCardPreviewData { ScrollView { VStack(spacing: 0) { PrRowCard(item: github559, linkedPr: linkedPr559) - PrRowCard(item: github346, onLink: {}) - PrRowCard(item: github425, onLink: {}) + PrRowCard(item: github346) + PrRowCard(item: github425) } .padding(.horizontal, 16) } @@ -163,8 +163,7 @@ private enum PrRowCardPreviewData { #Preview("Unmapped bot PR #346") { PrRowCard( - item: PrRowCardPreviewData.github346, - onLink: {} + item: PrRowCardPreviewData.github346 ) .padding(.horizontal, 16) .background(PrsLiquidBackdrop()) diff --git a/apps/ios/ADE/Views/PRs/PrsRootScreen.swift b/apps/ios/ADE/Views/PRs/PrsRootScreen.swift index 294b1ad28..cf9bc3e2e 100644 --- a/apps/ios/ADE/Views/PRs/PrsRootScreen.swift +++ b/apps/ios/ADE/Views/PRs/PrsRootScreen.swift @@ -16,6 +16,7 @@ struct PRsTabView: View { @State private var mobileSnapshot: PrMobileSnapshot? @State private var githubSnapshot: GitHubPrSnapshot? @State private var githubExternalHistoryLoaded = false + @State private var githubHistoryPageLimit = 2 // Set synchronously on the @MainActor before any awaited fetch so concurrent // callers (the `onChange` filter handler and the projection-reload `task`) // do not both pass the `!githubExternalHistoryLoaded` guard and issue @@ -38,16 +39,13 @@ struct PRsTabView: View { /// when the snapshot or a filter input changes — see `recomputeGitHubDerived` /// — instead of on every `body` pass. @State private var githubDerived: PrGitHubDerivedList = .empty - @State private var githubDetailRequest: PrGitHubLaneLinkRequest? @State private var laneLinkRequest: PrGitHubLaneLinkRequest? - @State private var pendingLaneLinkRequest: PrGitHubLaneLinkRequest? @State private var autoMapRequest: PrAutoMapRequest? @State private var autoMapPreflight: PrAutoMapPreflight? @State private var autoMapPreflightLoading = false @State private var autoMapBlockingMessage: String? @SceneStorage("ade.prs.rootSurface") private var rootSurfaceRawValue = PrRootSurface.github.rawValue @SceneStorage("ade.prs.workflowFilter") private var workflowFilterRawValue = PrWorkflowKindFilter.all.rawValue - @SceneStorage("ade.prs.githubStatusFilter") private var githubStatusFilterRawValue = PrGitHubStatusFilter.open.rawValue /// Primary headline selector for the GitHub surface (desktop parity): three /// status categories — Open (folds draft), Merged, Closed. @SceneStorage("ade.prs.githubCategory") private var githubCategoryRawValue = PrGitHubCategory.open.rawValue @@ -104,13 +102,6 @@ struct PRsTabView: View { ) } - private var selectedGitHubStatusFilter: Binding { - Binding( - get: { PrGitHubStatusFilter(rawValue: githubStatusFilterRawValue) ?? .open }, - set: { githubStatusFilterRawValue = $0.rawValue } - ) - } - /// Primary three-way category selector (Open / Merged / Closed). Folds draft /// into Open. Drives both the headline tabs and the list derivation. private var selectedGitHubCategory: Binding { @@ -134,21 +125,11 @@ struct PRsTabView: View { ) } - private var filteredPrs: [PullRequestListItem] { - let query = searchText.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() - let status = selectedGitHubStatusFilter.wrappedValue - let scope = selectedGitHubScopeFilter.wrappedValue - return prs.filter { item in - matchesCachedPrStatus(item, status: status) - && matchesCachedPrScope(item, scope: scope) - && matchesCachedPrSearch(item, query: query) - } - } - /// Total repo-scoped GitHub PR count (pre-filter). Cheap pass-through; the /// expensive derivations are memoized in `githubDerived`. private var allGitHubPrsCount: Int { - githubDerived.allCount + let counts = githubCategoryCounts + return counts.open + counts.merged + counts.closed } private var githubSnapshotNeedsExternalHistory: Bool { @@ -173,9 +154,12 @@ struct PRsTabView: View { /// the filter/sort/count cost. private func recomputeGitHubDerived() { let next = prComputeGitHubDerivedList( - items: repoScopedGitHubPullRequests(from: githubSnapshot), + items: prReconcileGitHubPullRequests( + snapshotItems: repoScopedGitHubPullRequests(from: githubSnapshot), + mappedPrs: prs + ), query: searchText, - status: selectedGitHubStatusFilter.wrappedValue, + status: .all, scope: selectedGitHubScopeFilter.wrappedValue, sort: selectedGitHubSort.wrappedValue, category: selectedGitHubCategory.wrappedValue @@ -211,10 +195,20 @@ struct PRsTabView: View { } /// Per-category counts for the three headline tabs, scoped by the active - /// scope filter (ADE / External / All). Memoized in `githubDerived` so a - /// `body` pass never re-scans the snapshot. + /// scope filter (ADE / External / All). Host history counts are a cheap + /// projection floor; never let them undercount rows already visible on the + /// phone after local reconciliation. private var githubCategoryCounts: PrGitHubCategoryCounts { - githubDerived.categoryCounts + if selectedGitHubScopeFilter.wrappedValue == .all, + let counts = githubSnapshot?.history?.repoPullRequestCounts + { + return PrGitHubCategoryCounts( + open: max(counts.open, githubDerived.categoryCounts.open), + merged: max(counts.merged, githubDerived.categoryCounts.merged), + closed: max(counts.closed, githubDerived.categoryCounts.closed) + ) + } + return githubDerived.categoryCounts } /// Prefer the unified `PrMobileSnapshot.workflowCards` payload when available; fall back to the @@ -291,6 +285,10 @@ struct PRsTabView: View { isActive ? syncService.prsProjectionRevision : nil } + private var prsRemoteReloadKey: Int? { + isActive ? syncService.prsRemoteRevision : nil + } + private var prNavigationRequestKey: String? { guard isActive else { return nil } return syncService.requestedPrNavigation?.id @@ -447,6 +445,15 @@ struct PRsTabView: View { guard !Task.isCancelled, prsProjectionReloadKey == revision else { return } lastHandledPrsProjectionRevision = revision } + .task(id: prsRemoteReloadKey) { + guard let revision = prsRemoteReloadKey, revision > 0 else { return } + // Webhook bursts can contain several related deliveries. Coalesce them + // into one bounded snapshot read instead of turning each event into a + // GitHub request from the phone. + try? await Task.sleep(for: .milliseconds(300)) + guard !Task.isCancelled, prsRemoteReloadKey == revision else { return } + await reloadGitHubProjection() + } .task(id: prNavigationRequestKey) { guard prNavigationRequestKey != nil else { return } await handleRequestedPrNavigation() @@ -455,16 +462,12 @@ struct PRsTabView: View { guard selectedGitHubCategory.wrappedValue != .open else { return } Task { await loadGitHubExternalHistoryIfNeeded() } } - .onChange(of: githubStatusFilterRawValue) { _, _ in - guard selectedGitHubStatusFilter.wrappedValue != .open else { return } - Task { await loadGitHubExternalHistoryIfNeeded() } - } // Memoize the GitHub-list derivations: recompute only when the snapshot // or a filter/search input actually changes, never inside a body pass. .onChange(of: githubSnapshot) { _, _ in recomputeGitHubDerived() } + .onChange(of: prs) { _, _ in recomputeGitHubDerived() } .onChange(of: searchText) { _, _ in recomputeGitHubDerived() } .onChange(of: githubCategoryRawValue) { _, _ in recomputeGitHubDerived() } - .onChange(of: githubStatusFilterRawValue) { _, _ in recomputeGitHubDerived() } .onChange(of: githubScopeFilterRawValue) { _, _ in recomputeGitHubDerived() } .onChange(of: githubSortRawValue) { _, _ in recomputeGitHubDerived() } .onAppear { recomputeGitHubDerived() } @@ -480,7 +483,8 @@ struct PRsTabView: View { prId: prId, transitionNamespace: ADEMotion.allowsMatchedGeometry(reduceMotion: reduceMotion) ? prTransitionNamespace : nil, requestedRepoOwner: routeScope?.repoOwner, - requestedRepoName: routeScope?.repoName + requestedRepoName: routeScope?.repoName, + availableLanes: lanes ) .environmentObject(syncService) } @@ -493,27 +497,6 @@ struct PRsTabView: View { PrStackSheet(groupId: presentation.id, groupName: presentation.groupName) .environmentObject(syncService) } - .sheet(item: $githubDetailRequest, onDismiss: presentPendingLaneLinkRequest) { request in - PrGitHubReadDetailSheet( - item: request.item, - canLink: canLinkGitHubPullRequests, - canAutoMap: canAutoMapGitHubPullRequests, - onLink: { - pendingLaneLinkRequest = request - githubDetailRequest = nil - }, - onAutoMap: { - let item = request.item - githubDetailRequest = nil - // Defer the auto-map sheet to after the read sheet dismisses so the - // two don't fight for the presentation slot. - DispatchQueue.main.async { presentAutoMap(for: item) } - }, - onOpenGitHub: { - openGitHub(urlString: request.item.githubUrl) - } - ) - } .sheet(item: $laneLinkRequest) { request in PrLaneLinkSheet( item: request.item, @@ -552,19 +535,13 @@ struct PRsTabView: View { } } - private func presentPendingLaneLinkRequest() { - guard let request = pendingLaneLinkRequest else { return } - pendingLaneLinkRequest = nil - laneLinkRequest = request - } - /// Count shown in the hero-header chip — matches whichever surface the user /// is currently viewing, so we don't flash "GitHub 42" in the title while /// the workflows surface is showing 3 cards. private var heroCount: Int { switch selectedRootSurface.wrappedValue { case .github: - return githubSnapshot == nil ? filteredPrs.count : filteredGitHubPrs.count + return filteredGitHubPrs.count case .workflows: return groupedWorkflowCards.count } @@ -838,7 +815,7 @@ struct PRsTabView: View { .prListRow() } - if prsStatus.phase == .ready && filteredPrs.isEmpty && filteredGitHubPrs.isEmpty { + if prsStatus.phase == .ready && filteredGitHubPrs.isEmpty { ADEEmptyStateView( symbol: searchText.isEmpty ? "arrow.triangle.pull" : "magnifyingglass", title: searchText.isEmpty ? "No pull requests for these filters" : "No PRs match this search", @@ -849,28 +826,7 @@ struct PRsTabView: View { .prListRow() } - if githubSnapshot == nil, !filteredPrs.isEmpty { - ForEach(filteredPrs) { pr in - NavigationLink(value: pr.id) { - PrRowCard( - pr: pr, - transitionNamespace: ADEMotion.allowsMatchedGeometry(reduceMotion: reduceMotion) ? prTransitionNamespace : nil, - isSelectedTransitionSource: selectedPrTransitionId == pr.id - ) { groupId, groupName in - stackPresentation = PrStackPresentation(id: groupId, groupName: groupName) - } - } - .simultaneousGesture(TapGesture().onEnded { - selectedPrTransitionId = pr.id - }) - .swipeActions(edge: .trailing, allowsFullSwipe: false) { - rowSwipeActions(for: pr) - } - .prListRowCard() - } - } - - if githubSnapshot != nil { + if githubSnapshot != nil || !prs.isEmpty { let repoItems = githubDerived.repoItems let externalItems = githubDerived.externalItems if !repoItems.isEmpty { @@ -900,6 +856,33 @@ struct PRsTabView: View { .prListRowCard() } } + if selectedGitHubCategory.wrappedValue != .open, + githubSnapshot?.history?.repoPullRequestsMayHaveMore == true, + githubHistoryPageLimit < 10 + { + Button { + Task { await loadMoreGitHubHistory() } + } label: { + HStack(spacing: 8) { + if isLoadingExternalHistory { + ProgressView() + .controlSize(.small) + } else { + Image(systemName: "arrow.down.circle") + } + Text(isLoadingExternalHistory ? "Loading more pull requests…" : "Load more pull requests") + .font(.system(size: 13, weight: .semibold)) + Spacer(minLength: 0) + } + .foregroundStyle(PrsGlass.textSecondary) + .padding(.horizontal, 14) + .padding(.vertical, 12) + .prGlassCard(cornerRadius: 12, shadow: false) + } + .buttonStyle(.plain) + .disabled(isLoadingExternalHistory) + .prListRow() + } } } @@ -932,7 +915,7 @@ struct PRsTabView: View { } } else if item.scope == "external" { Button { - openGitHub(urlString: item.githubUrl) + openGitHubDetail(item) } label: { PrRowCard(item: item) } @@ -943,21 +926,16 @@ struct PRsTabView: View { } } else { Button { - githubDetailRequest = PrGitHubLaneLinkRequest(item: item) + openGitHubDetail(item) } label: { PrRowCard( item: item, - linkedPr: linkedPullRequest(for: item), - onLink: canLinkGitHubPullRequests - ? { laneLinkRequest = PrGitHubLaneLinkRequest(item: item) } - : nil + linkedPr: linkedPullRequest(for: item) ) } .buttonStyle(.plain) .swipeActions(edge: .trailing, allowsFullSwipe: false) { - Button("Review") { - githubDetailRequest = PrGitHubLaneLinkRequest(item: item) - } + Button("Review") { openGitHubDetail(item) } .tint(ADEColor.warning) if canAutoMapGitHubPullRequests { Button("Create lane") { @@ -977,6 +955,35 @@ struct PRsTabView: View { } } + @MainActor + private func openGitHubDetail(_ item: GitHubPrListItem) { + let routeId = prSyntheticGitHubId(for: item) + if let routeScope = PrDetailRouteScope(repoOwner: item.repoOwner, repoName: item.repoName) { + prDetailRouteScopes[routeId] = routeScope + } + // Seed the title and identity for an instant transition. `distantPast` + // deliberately marks this entry stale so detail performs exactly one + // aggregate live fetch as soon as it appears. + syncService.storePrDetailWarmEntry( + PrDetailWarmEntry( + pr: nil, + githubItem: item, + snapshot: nil, + reviewThreads: [], + actionRuns: [], + activityEvents: [], + deployments: [], + aiSummary: nil, + groupMembers: [], + capabilities: nil, + unavailableParts: [], + loadedAt: .distantPast + ), + for: routeId + ) + path.append(routeId) + } + @ViewBuilder private var workflowsSurfaceRows: some View { PrsWorkflowFilterPills( @@ -1148,49 +1155,6 @@ struct PRsTabView: View { return groups } - private func matchesCachedPrStatus(_ item: PullRequestListItem, status: PrGitHubStatusFilter) -> Bool { - switch status { - case .all: - return true - case .open: - return item.state == "open" - case .draft: - return item.state == "draft" - case .merged: - return item.state == "merged" - case .closed: - return item.state == "closed" - } - } - - private func matchesCachedPrScope(_ item: PullRequestListItem, scope: PrGitHubScopeFilter) -> Bool { - switch scope { - case .all, .ade: - return true - case .external: - return false - } - } - - private func matchesCachedPrSearch(_ item: PullRequestListItem, query: String) -> Bool { - guard !query.isEmpty else { return true } - let haystack = [ - item.title, - item.headBranch, - item.baseBranch, - item.laneName, - item.repoOwner, - item.repoName, - item.adeKind, - item.workflowDisplayState, - "#\(item.githubPrNumber)", - "\(item.githubPrNumber)", - ] - .compactMap { $0?.lowercased() } - .joined(separator: " ") - return haystack.contains(query) - } - private var laneContextNotice: ADENoticeCard? { guard let laneContextLaneId else { return nil } let laneName = lanes.first(where: { $0.id == laneContextLaneId })?.name ?? "lane context" @@ -1219,9 +1183,10 @@ struct PRsTabView: View { do { var refreshError: Error? if refreshRemote { + async let refreshPrs: Void = syncService.refreshPullRequestSnapshots() + async let refreshLanes: Void = syncService.refreshLaneSnapshots() do { - try await syncService.refreshPullRequestSnapshots() - try await syncService.refreshLaneSnapshots() + _ = try await (refreshPrs, refreshLanes) } catch { refreshError = error } @@ -1260,7 +1225,9 @@ struct PRsTabView: View { let githubSnapshotTask = Task { try? await syncService.fetchGitHubPullRequestSnapshot( force: refreshRemote, - includeExternalClosed: effectiveIncludeExternalClosed + includeExternalClosed: effectiveIncludeExternalClosed, + historyPageLimit: effectiveIncludeExternalClosed ? githubHistoryPageLimit : nil, + includeStateCounts: true ) } nextMobileSnapshot = await mobileSnapshotTask.value @@ -1338,7 +1305,11 @@ struct PRsTabView: View { if isLoadingExternalHistory { return } isLoadingExternalHistory = true defer { isLoadingExternalHistory = false } - if let nextGithubSnapshot = try? await syncService.fetchGitHubPullRequestSnapshot(includeExternalClosed: true) { + if let nextGithubSnapshot = try? await syncService.fetchGitHubPullRequestSnapshot( + includeExternalClosed: true, + historyPageLimit: githubHistoryPageLimit, + includeStateCounts: true + ) { githubExternalHistoryLoaded = true if githubSnapshot != nextGithubSnapshot { githubSnapshot = nextGithubSnapshot @@ -1346,6 +1317,40 @@ struct PRsTabView: View { } } + @MainActor + private func loadMoreGitHubHistory() async { + guard isLive, !isLoadingExternalHistory else { return } + let nextLimit = min(10, githubHistoryPageLimit + 2) + guard nextLimit > githubHistoryPageLimit else { return } + isLoadingExternalHistory = true + defer { isLoadingExternalHistory = false } + if let nextSnapshot = try? await syncService.fetchGitHubPullRequestSnapshot( + includeExternalClosed: true, + historyPageLimit: nextLimit, + includeStateCounts: true + ) { + githubHistoryPageLimit = nextSnapshot.history?.pageLimit ?? nextLimit + githubExternalHistoryLoaded = true + if githubSnapshot != nextSnapshot { + githubSnapshot = nextSnapshot + } + } + } + + @MainActor + private func reloadGitHubProjection() async { + guard isLive else { return } + let includeHistory = githubSnapshotShouldIncludeExternalClosed + if let nextSnapshot = try? await syncService.fetchGitHubPullRequestSnapshot( + includeExternalClosed: includeHistory, + historyPageLimit: includeHistory ? githubHistoryPageLimit : nil, + revalidate: false, + includeStateCounts: true + ), githubSnapshot != nextSnapshot { + githubSnapshot = nextSnapshot + } + } + @MainActor private func handleRequestedPrNavigation() async { guard let request = syncService.requestedPrNavigation else { return } @@ -1393,11 +1398,7 @@ struct PRsTabView: View { } path.append(prId) case .github(let item): - if item.scope == "external" { - openGitHub(urlString: item.githubUrl) - } else { - githubDetailRequest = PrGitHubLaneLinkRequest(item: item) - } + openGitHubDetail(item) case .unresolved: errorMessage = "That pull request is not available on this phone yet." } @@ -1800,150 +1801,6 @@ struct PRsTabView: View { } } -private struct PrGitHubReadDetailSheet: View { - @Environment(\.dismiss) private var dismiss - let item: GitHubPrListItem - let canLink: Bool - var canAutoMap: Bool = false - let onLink: () -> Void - var onAutoMap: (() -> Void)? = nil - let onOpenGitHub: () -> Void - - private var stateLabel: String { - item.isDraft ? "draft" : item.state - } - - private var headBranch: String { - item.headBranch?.isEmpty == false ? item.headBranch! : "unknown branch" - } - - private var baseBranch: String { - item.baseBranch?.isEmpty == false ? item.baseBranch! : "unknown base" - } - - var body: some View { - PrLiquidSheetShell( - title: "PR details", - trailingLabel: "Done", - onTrailing: { dismiss() } - ) { - VStack(alignment: .leading, spacing: 14) { - // Hero: number + state + EXTERNAL chip + title. - VStack(alignment: .leading, spacing: 12) { - HStack(spacing: 8) { - Text("#\(item.githubPrNumber)") - .font(.system(size: 22, weight: .bold, design: .monospaced)) - .foregroundStyle(PrGlassPalette.success) - .shadow(color: PrGlassPalette.success.opacity(0.45), radius: 8) - - PrTagChip(label: stateLabel, color: prStateTint(stateLabel)) - - if item.scope == "external" { - PrExternalInfoChip() - } - - Spacer(minLength: 0) - } - - Text(item.title) - .font(.system(size: 17, weight: .semibold)) - .foregroundStyle(PrsGlass.textPrimary) - .tracking(-0.2) - .fixedSize(horizontal: false, vertical: true) - } - .frame(maxWidth: .infinity, alignment: .leading) - - VStack(spacing: 8) { - PrGlassMonoRow( - eyebrow: "Repo", - value: "\(item.repoOwner)/\(item.repoName)", - icon: "chevron.left.slash.chevron.right" - ) - PrGlassMonoRow( - eyebrow: "Branches", - value: "\(headBranch) → \(baseBranch)", - icon: "arrow.triangle.branch" - ) - PrGlassMonoRow( - eyebrow: "Author", - value: (item.author?.isEmpty == false) ? "@\(item.author!)" : "unknown", - icon: "person.fill" - ) - PrGlassMonoRow( - eyebrow: "Updated", - value: prRelativeTime(item.updatedAt), - icon: "clock" - ) - } - - // Unmapped / linked-lane CTA card. - HStack(alignment: .top, spacing: 12) { - ZStack { - Circle() - .fill(PrGlassPalette.purple.opacity(0.22)) - Image(systemName: "link.badge.plus") - .font(.system(size: 15, weight: .semibold)) - .foregroundStyle(PrGlassPalette.purpleBright) - } - .frame(width: 34, height: 34) - - VStack(alignment: .leading, spacing: 4) { - Text("Not linked to an ADE lane") - .font(.system(size: 13, weight: .semibold)) - .foregroundStyle(PrsGlass.textPrimary) - Text("Review the branch and author, then link this PR into a lane to track it inside ADE.") - .font(.system(size: 11)) - .foregroundStyle(PrsGlass.textSecondary) - .fixedSize(horizontal: false, vertical: true) - } - - Spacer(minLength: 0) - } - .padding(14) - .frame(maxWidth: .infinity, alignment: .leading) - .prGlassCard(cornerRadius: 14, tint: PrGlassPalette.purple.opacity(0.55), shadow: false) - - VStack(spacing: 10) { - // Primary auto-map CTA: create a lane straight from the PR branch. - // Hidden entirely on hosts that don't expose the action so older - // machines fall back to the link flow with no dead control. - if canAutoMap, let onAutoMap { - Button { - onAutoMap() - } label: { - Label("Create lane from PR branch", systemImage: "arrow.triangle.branch") - } - .buttonStyle(PrGlassPrimaryButtonStyle()) - } - - // When the auto-map CTA is present it owns the primary slot, so the - // link flow renders as the secondary (outline) action; otherwise the - // link button stays primary (legacy behavior). - Group { - if canAutoMap, onAutoMap != nil { - Button { onLink() } label: { Label("Map to lane…", systemImage: "link") } - .buttonStyle(PrGlassOutlineButtonStyle()) - } else { - Button { onLink() } label: { Label("Link to lane", systemImage: "link") } - .buttonStyle(PrGlassPrimaryButtonStyle()) - } - } - .disabled(!canLink) - - Button { - onOpenGitHub() - } label: { - Label("Open on GitHub", systemImage: "arrow.up.right.square") - } - .buttonStyle(PrGlassOutlineButtonStyle()) - } - .padding(.top, 4) - } - .padding(16) - } - } -} - // MARK: - Auto-map confirmation sheet (create lane from PR branch) // // Mirrors desktop's `CreateLaneFromPrBranchDialog`: a compact summary of the @@ -2045,7 +1902,7 @@ private struct PrAutoMapSheet: View { } } -private struct PrLaneLinkSheet: View { +struct PrLaneLinkSheet: View { @Environment(\.dismiss) private var dismiss let item: GitHubPrListItem let lanes: [LaneSummary] diff --git a/apps/ios/ADE/Views/PRs/PrsRootScreenPreviews.swift b/apps/ios/ADE/Views/PRs/PrsRootScreenPreviews.swift index c44286002..8c9503863 100644 --- a/apps/ios/ADE/Views/PRs/PrsRootScreenPreviews.swift +++ b/apps/ios/ADE/Views/PRs/PrsRootScreenPreviews.swift @@ -201,20 +201,17 @@ private struct PrsGitHubRootPreviewScreen: View { .prListRowCard() PrRowCard( - item: PrsRootPreviewData.github346, - onLink: {} + item: PrsRootPreviewData.github346 ) .prListRowCard() PrRowCard( - item: PrsRootPreviewData.github425, - onLink: {} + item: PrsRootPreviewData.github425 ) .prListRowCard() PrRowCard( - item: PrsRootPreviewData.github344, - onLink: {} + item: PrsRootPreviewData.github344 ) .prListRowCard() } diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index d3c037f31..cab1169fd 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -1110,10 +1110,20 @@ final class ADETests: XCTestCase { let defaultArgs = makePrGithubSnapshotArgs(force: false, includeExternalClosed: false) XCTAssertEqual(defaultArgs["force"] as? Bool, false) XCTAssertNil(defaultArgs["includeExternalClosed"]) + XCTAssertNil(defaultArgs["revalidate"]) - let historyArgs = makePrGithubSnapshotArgs(force: true, includeExternalClosed: true) + let historyArgs = makePrGithubSnapshotArgs( + force: true, + includeExternalClosed: true, + historyPageLimit: 4, + revalidate: false, + includeStateCounts: true + ) XCTAssertEqual(historyArgs["force"] as? Bool, true) XCTAssertEqual(historyArgs["includeExternalClosed"] as? Bool, true) + XCTAssertEqual(historyArgs["historyPageLimit"] as? Int, 4) + XCTAssertEqual(historyArgs["revalidate"] as? Bool, false) + XCTAssertEqual(historyArgs["includeStateCounts"] as? Bool, true) } func testProjectScopedOutboundEnvelopeTypesIncludeActiveProjectId() { @@ -4914,6 +4924,7 @@ final class ADETests: XCTestCase { XCTAssertEqual(service.hostCompatibilityMissingActions, ["commandRouting"]) XCTAssertFalse(service.supportsRemoteAction("usage.getAdeStats")) XCTAssertFalse(service.supportsRemoteAction("analytics.setClientEnabled")) + XCTAssertFalse(service.supportsRemoteAction("prs.getMobileGithubDetail")) XCTAssertFalse(service.supportsChatRemoteAction("chat.cancelScheduledWork", sessionId: "chat-legacy")) XCTAssertFalse(service.supportsChatRemoteAction("chat.setScheduledWorkPaused", sessionId: "chat-legacy")) do { @@ -4928,6 +4939,19 @@ final class ADETests: XCTestCase { } catch { XCTAssertEqual((error as NSError).code, 15) } + do { + _ = try await service.fetchPrMobileGithubDetail( + repoOwner: "arul28", + repoName: "ADE", + githubPrNumber: 849 + ) + XCTFail("A legacy host must reject aggregate PR detail before transport") + } catch { + let nsError = error as NSError + XCTAssertEqual(nsError.domain, "ADE") + XCTAssertEqual(nsError.code, 17) + XCTAssertEqual(nsError.userInfo["ADEErrorCode"] as? String, "unsupported_action") + } let analyticsOptOutAcknowledged = await service.setProductAnalyticsClientEnabled(false) XCTAssertTrue(analyticsOptOutAcknowledged) } @@ -5042,6 +5066,51 @@ final class ADETests: XCTestCase { } } + @MainActor + func testMobileGithubDetailStaysGatedWhenCompatibleHostOmitsAction() async throws { + let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) + try service.applyHelloPayloadForTesting([ + "brain": [ + "deviceId": "host-1", + "deviceName": "Mac Studio", + ], + "features": [ + "projectCatalog": false, + "commandRouting": [ + "mode": "allowlisted", + "actions": [[ + "action": "prs.getGitHubSnapshot", + "policy": ["viewerAllowed": true, "queueable": false], + ]], + ], + "mobileCompatibility": [ + "contractVersion": 1, + "mode": "limited", + "requiredActions": ["prs.getMobileGithubDetail"], + "missingActions": ["prs.getMobileGithubDetail"], + ], + ], + ]) + + XCTAssertEqual(service.connectionState, .connected) + XCTAssertEqual(service.hostCompatibilityMode, .limited) + XCTAssertEqual(service.hostCompatibilityMissingActions, ["prs.getMobileGithubDetail"]) + XCTAssertFalse(service.supportsRemoteAction("prs.getMobileGithubDetail")) + do { + _ = try await service.fetchPrMobileGithubDetail( + repoOwner: "arul28", + repoName: "ADE", + githubPrNumber: 849 + ) + XCTFail("An advertised host without the action must reject aggregate PR detail before transport") + } catch { + let nsError = error as NSError + XCTAssertEqual(nsError.domain, "ADE") + XCTAssertEqual(nsError.code, 17) + XCTAssertEqual(nsError.userInfo["ADEErrorCode"] as? String, "unsupported_action") + } + } + func testMobileAdeUsageStatsDecodesPayloadWithoutNewOptionalBreakdowns() throws { let json = """ { @@ -8844,6 +8913,7 @@ final class ADETests: XCTestCase { aiSummary: nil, groupMembers: [], capabilities: nil, + unavailableParts: [], loadedAt: Date(timeIntervalSince1970: 0) ) diff --git a/apps/ios/ADETests/PrMergeMergeStateTests.swift b/apps/ios/ADETests/PrMergeMergeStateTests.swift index 7f1496f86..2ed8a2dfb 100644 --- a/apps/ios/ADETests/PrMergeMergeStateTests.swift +++ b/apps/ios/ADETests/PrMergeMergeStateTests.swift @@ -382,8 +382,134 @@ final class PrMergeMergeStateTests: XCTestCase { XCTAssertEqual(admin, "gh pr merge 42 --squash --admin --repo arul28/ADE") } + // MARK: - Mobile GitHub projection reliability + + func testGitHubSnapshotDecodesProjectionHistoryCounts() throws { + let snapshot = try JSONDecoder().decode(GitHubPrSnapshot.self, from: Data(#""" + { + "repo": { "owner": "arul28", "name": "ADE", "defaultBranch": "main" }, + "viewerLogin": "arul28", + "repoPullRequests": [], + "externalPullRequests": [], + "syncedAt": "2026-07-17T12:00:00Z", + "history": { + "includeExternalClosed": false, + "pageLimit": 0, + "repoPullRequestsLoaded": 2, + "repoPullRequestsMayHaveMore": false, + "repoPullRequestCounts": { "open": 2, "closed": 17, "merged": 834 } + } + } + """#.utf8)) + + XCTAssertEqual(snapshot.history?.repoPullRequestCounts?.open, 2) + XCTAssertEqual(snapshot.history?.repoPullRequestCounts?.merged, 834) + XCTAssertEqual(snapshot.history?.repoPullRequestCounts?.closed, 17) + } + + func testReconcileKeepsMappedTerminalPrVisibleWhenProjectionOmitsIt() { + let mapped = mappedPr(state: "merged") + let reconciled = prReconcileGitHubPullRequests(snapshotItems: [], mappedPrs: [mapped]) + + XCTAssertEqual(reconciled.count, 1) + XCTAssertEqual(reconciled[0].state, "merged") + XCTAssertEqual(reconciled[0].linkedPrId, mapped.id) + XCTAssertEqual(reconciled[0].linkedLaneId, mapped.laneId) + } + + func testReconcileLetsReplicatedTerminalStateOverrideStaleOpenProjection() { + let mapped = mappedPr(state: "closed") + let reconciled = prReconcileGitHubPullRequests( + snapshotItems: [githubItem(state: "open")], + mappedPrs: [mapped] + ) + + XCTAssertEqual(reconciled.count, 1) + XCTAssertEqual(reconciled[0].state, "closed") + XCTAssertEqual(reconciled[0].linkedPrId, mapped.id) + } + + func testReconcileLetsNewerReplicatedReopenOverrideStaleClosedProjection() { + let mapped = mappedPr(state: "open") + let reconciled = prReconcileGitHubPullRequests( + snapshotItems: [githubItem(state: "closed")], + mappedPrs: [mapped] + ) + + XCTAssertEqual(reconciled.count, 1) + XCTAssertEqual(reconciled[0].state, "open") + XCTAssertEqual(reconciled[0].updatedAt, mapped.updatedAt) + } + + func testSyntheticGitHubRouteRoundTripsCoordinates() { + let route = prSyntheticGitHubId(repoOwner: "arul28", repoName: "ADE", githubPrNumber: 849) + let coords = prGitHubCoordinates(fromRouteId: route) + + XCTAssertEqual(route, "gh:arul28/ADE#849") + XCTAssertEqual(coords?.repoOwner, "arul28") + XCTAssertEqual(coords?.repoName, "ADE") + XCTAssertEqual(coords?.githubPrNumber, 849) + } + + func testPartialMobileDetailRetainsLastGoodSidecars() { + let previous = PullRequestSnapshot( + detail: nil, + status: nil, + checks: [check(name: "CI", conclusion: "success")], + reviews: [], + comments: [], + files: [PrFile(filename: "old.swift", status: "modified", additions: 1, deletions: 0, patch: nil, previousFilename: nil)], + commits: nil + ) + let incoming = PullRequestSnapshot( + detail: nil, + status: nil, + checks: [], + reviews: [], + comments: [], + files: [PrFile(filename: "new.swift", status: "added", additions: 2, deletions: 0, patch: nil, previousFilename: nil)], + commits: nil + ) + + let merged = prMergeMobileGithubSnapshot( + incoming: incoming, + previous: previous, + unavailableParts: ["checks"] + ) + + XCTAssertEqual(merged.checks.map(\.name), ["CI"]) + XCTAssertEqual(merged.files.map(\.filename), ["new.swift"]) + } + // MARK: - Helpers + private func githubItem(state: String) -> GitHubPrListItem { + GitHubPrListItem( + id: "node-849", scope: "repo", repoOwner: "arul28", repoName: "ADE", + githubPrNumber: 849, githubUrl: "https://github.com/arul28/ADE/pull/849", + title: "Clean up the PR list", state: state, isDraft: false, + baseBranch: "main", headBranch: "feature/pr-list", headRepoOwner: "arul28", + headRepoName: "ADE", author: "arul28", createdAt: "2026-07-17T10:00:00Z", + updatedAt: "2026-07-17T11:00:00Z", linkedPrId: nil, linkedGroupId: nil, + linkedLaneId: nil, linkedLaneName: nil, adeKind: nil, workflowDisplayState: nil, + cleanupState: nil, labels: [], isBot: false, commentCount: 3 + ) + } + + private func mappedPr(state: String) -> PullRequestListItem { + PullRequestListItem( + id: "pr-849", laneId: "lane-849", laneName: "PR list", projectId: "project-1", + repoOwner: "arul28", repoName: "ADE", githubPrNumber: 849, + githubUrl: "https://github.com/arul28/ADE/pull/849", title: "Clean up the PR list", + state: state, baseBranch: "main", headBranch: "feature/pr-list", + checksStatus: "passing", reviewStatus: "approved", additions: 12, deletions: 4, + lastSyncedAt: "2026-07-17T12:00:00Z", createdAt: "2026-07-17T10:00:00Z", + updatedAt: "2026-07-17T12:00:00Z", adeKind: "single", linkedGroupId: nil, + linkedGroupType: nil, linkedGroupName: nil, linkedGroupPosition: nil, + linkedGroupCount: 0, workflowDisplayState: "complete", cleanupState: "none" + ) + } + private func check(name: String, conclusion: String) -> PrCheck { PrCheck(name: name, status: "completed", conclusion: conclusion, detailsUrl: nil, startedAt: nil, completedAt: nil) } diff --git a/docs/features/pull-requests/README.md b/docs/features/pull-requests/README.md index 8f4d05b8c..26840182e 100644 --- a/docs/features/pull-requests/README.md +++ b/docs/features/pull-requests/README.md @@ -79,8 +79,9 @@ Service files (`apps/desktop/src/main/services/prs/`): | File | Responsibility | |------|---------------| | `prService.ts` | PR CRUD, GitHub sync, merge context, draft descriptions, check/review/comment hydration, cached detail snapshots (`listSnapshots`), commit snapshots (`getCommits`), integration proposals, merge-into-existing-lane adoption, merge bypass, post-merge cleanup, standalone PR branch cleanup (`cleanupBranch`), deployment listing, review-thread reply/resolve/react mutations for the timeline, the aggregate `getMobileSnapshot` that powers the iOS PRs tab, and `listOpenPullRequests` — a paginated `/repos/{owner}/{name}/pulls?state=open` fetch returning `BranchPullRequest[]` for the lane-creation branch picker. `getForLane(laneId)` resolves through `getDisplayCandidateForCurrentLaneBranch`: it returns the best PR whose head branch matches the lane's current branch ref, considering both mapped `pull_requests` rows and unmapped `github_pr_projections` rows (folded in as synthetic `gh:owner/repo#num` summaries with `unmapped: true`), ranked open/draft → merged → closed then most-recently-updated / created / highest PR number, so a freshly merged PR still shows in lane-scoped UI instead of disappearing the moment GitHub flips the state — and a lane whose PR was created outside ADE still badges from the projection alone. A primary lane whose branch equals its base is excluded. `listPrsByLane()` walks `laneService.list` and applies the same candidate selection over one shared read of mapped rows + projection rows. `getGitHubSnapshot` fetches repo PRs, backfills same-repo lane PR rows by branch, and performs a capped per-branch fallback (`head=:`) for active lane branches missing from the repo snapshot window so old merged/closed externally-created PRs can still badge lanes. On PR open, `publishLinearPrCardsForLane` combines the lane's own Linear references with `collectLinearPrIssueReferencesForLaneSessions(laneId)` — issues attached only to a chat/CLI session in the lane (via `laneService.listLinearIssuesForLaneSessions`, authoritative for sessions whose lane mirror never landed) — deduped via `dedupeLinearPrIssueReferences`, so a session-only issue still gets a PR attachment. When the optional live-status round-trip is enabled (`getLinearLiveStatusService`, gated by `ADE_LINEAR_LIVE_STATUS_ROUNDTRIP=1`) it also posts a PR-link comment back to each linked issue. See [Linear integration](../linear-integration/README.md#session-scoped-issue-attachment-and-cli-context-injection). `computeStatus` / `getStatusByGithub` fetch the authoritative GitHub merge box over GraphQL (`mergeStateStatus`, `reviewDecision`, required/approving review counts, `viewerPermission` for bypass) and fold it into `PrStatus`; `getStatusByGithub` does the same for unmapped GitHub-tab PRs keyed only on `owner/repo#num` coords. `land` takes an editable commit title/body (`commit_title`/`commit_message`, `--subject`/`--body` on the admin retry; ignored for `rebase`) and an `expectedHeadSha` stale-head guard, and `updateBranch` brings a behind branch up to date via GitHub's `update-branch` API (`merge` strategy) or ADE's local lane rebase + force-with-lease push (`rebase` strategy, conflict-aware). Review-thread reply/resolve/react mutations work on unmapped GitHub-tab PRs through synthetic `gh:owner/repo#num` ids (`parseSyntheticGithubPrId` resolves the repo; `assertThreadBelongsToPr` still verifies thread ownership). Commit rows carry an avatar URL — the linked GitHub avatar when present, else a Gravatar identicon derived from the commit-author email. | -| `prService.mobileSnapshot.test.ts` | Coverage for the mobile snapshot builder: stack chaining, capability gates, per-lane create eligibility, workflow-card aggregation | -| `prService.mergeInto.test.ts` | Coverage for integration proposals that preview or adopt an existing merge target lane, including dirty-worktree handling and drift metadata. | +| `prService.test.ts` | Feature-level service coverage, including mobile snapshot aggregation, paged GitHub history and exact state totals, webhook invalidation, unmapped mobile detail, and integration proposal behavior. | +| `prMergeQueue.test.ts` | Merge-queue transition and landing coverage. | +| `prAsync.test.ts` | Shared bounded-concurrency and async helper coverage. | | `prPollingService.ts` | 60 s polling loop, fingerprint-based change detection, notification emission. Writes `last_polled_at` per PR so callers can run delta polls on the next tick. The ADE daemon owns an instance (created + started + disposed in `apps/ade-cli/src/bootstrap.ts`) so background polling and PR events run for runtime-bound windows; the desktop main process still owns one for local-bound windows. When zero PRs are tracked yet, the forced full-snapshot `discoverLanePullRequests` fetch is throttled to a 10-minute cadence instead of running every tick — user-driven surfaces discover PRs on their own reads anyway | | `prSummaryService.ts` | AI PR summary generator; caches `PrAiSummary` per `(prId, headSha)` in `pull_request_ai_summaries` so pushes invalidate the cache | | `queueLandingService.ts` | Merge queue state machine (`ALLOWED_TRANSITIONS`), landing loop, auto-resolve on conflicts | @@ -90,6 +91,12 @@ Service files (`apps/desktop/src/main/services/prs/`): | `prRebaseResolver.ts` | Builds rebase-resolution prompts, launches chat session | | `resolverUtils.ts` | Shared permission-mode mapping, recent commit reading, comment noise filter, and the `looksLikeResolutionAck` heuristic that flags resolved-looking replies on unresolved review threads | +`prService.ts` also owns the coordinate-based `getMobileGithubDetail` +aggregate for unmapped PRs and the opt-in repository state-count query used by +mobile. Count requests are single-flight and epoch-guarded, and snapshot +invalidation clears their cache so an older response cannot repopulate a newer +webhook generation. + Renderer components (`apps/desktop/src/renderer/components/prs/`): | File | Responsibility | @@ -154,6 +161,11 @@ Shared contracts: | `apps/desktop/src/shared/prMarkdownText.ts` | `normalizeEscapedMarkdownNewlines(text)` — unescapes literal `\n` / `\r\n` / `\r` / `\t` sequences that arrive in PR bodies after GitHub round-trips them through JSON. Used by `PrMarkdown` before handing the string to ReactMarkdown so escaped newlines render as paragraph breaks. | | `apps/desktop/src/shared/ipc.ts` / `apps/desktop/src/preload/preload.ts` | PR IPC constants and renderer bridge for proposal simulation, update, commit, resolver, cleanup, and read flows. Read-heavy PR tab calls route to the remote runtime only for remote-bound windows and use in-process IPC for local-bound windows. Local PR/session push subscriptions are multiplexed so multiple renderer subscribers share one IPC listener per channel. | +For mobile, `types/prs.ts` also defines paged GitHub history metadata +(`pageLimit`, `repoPullRequestsMayHaveMore`, `repoPullRequestCounts`) and +`PrMobileGithubDetailSnapshot`, whose `unavailableParts` distinguishes a failed +optional sidecar from an authoritative empty result. + ## Core model `PrSummary` (selected fields, full type in `src/shared/types.ts`): @@ -263,6 +275,14 @@ Caching layers: Snapshot contents include `labels` (name, color, description), `isBot`, and `commentCount` fields so filters can run locally. +History-enabled snapshots also carry `repoPullRequestsMayHaveMore` and the +applied `pageLimit`. The mobile caller opts into +`history.repoPullRequestCounts`: one GraphQL query fetches exact Open / Merged / +Closed totals independent of the loaded row window, with cached projection +counts as the best-effort fallback. iOS uses those totals for the All-scope +category tabs and requests additional terminal-history pages only when the +user taps Load more; ADE/External scope counts are derived from the rows +already reconciled on-device. PR rows in `tabs/GitHubTab.tsx` and queue member rows in `tabs/QueueTab.tsx` render the linked lane's color through `LaneAccentDot` (resolved from the @@ -791,7 +811,9 @@ Builder responsibilities: - **Create eligibility** (`buildCreateCapabilities`) — enumerates non-primary, non-archived lanes, marks lanes as ineligible when an open/draft PR already exists, and resolves the default base branch - through `resolveStableLaneBaseBranch`. + through `resolveStableLaneBaseBranch`. The aggregate lane read is + metadata-only (`includeStatus: false`); it never probes every worktree's git + status just to paint the PR tab. - **Workflow cards** (`buildWorkflowCards`) — pulls non-terminal queue entries from `queue_landing_state` joined with `pr_groups`, active integration proposals via `listIntegrationWorkflows({ view: @@ -817,19 +839,50 @@ through the existing command surface (`prs.createFromLane`, PRs with `source lane -> target lane` titles and no AI-generated title/body step; the explicit `prs.draftDescription` action remains available to callers that request PR-description drafting directly. -The mobile client calls `getMobileSnapshot` on open and re-fetches on -focus or after a successful mutation. +The mobile client calls `getMobileSnapshot` on open and re-fetches on focus or +after a successful mutation. Unmapped GitHub projections are local-only on the +host, so webhook changes also emit a tiny `prs_updated` sync invalidation. +iOS coalesces event bursts, then performs one cached/projected GitHub snapshot +read with background revalidation disabled; it does not poll GitHub on a timer +or run one request per PR. The list +reconciles that projection with replicated `pull_requests` rows so mapped PRs +remain visible offline and a terminal local state cannot fall back to a stale +Open row. +Open / Merged / Closed totals come from one batched GraphQL count query cached +with the snapshot; row history remains independently paged, so accurate tab +counts do not require downloading the entire repository history. + +The iOS list row intentionally follows GitHub's information hierarchy instead +of rendering every field as a badge: state symbol and title first, then PR +number/author/repository, followed by a compact branch, lane, check, review, +and comment signal row. It uses local symbols rather than network-fetched +avatars and precomputes filtered/reconciled rows when inputs change, keeping +scroll-time view work bounded. The mobile PR **detail** screen (`PrDetailView`, a single-column adaptation of the desktop Timeline+Rails layout) pulls its per-PR action sidecars — review threads, activity feed, action runs, deployments, AI summary, and this snapshot's capabilities — separately, and keeps them -live while open through a warm-cache freshness gate keyed on the sync -projection revision: webhook-driven host updates rewrite the replicated -snapshot rows, the changeset pump bumps the projection, and the detail -screen re-fetches the sidecars at most once every 25 s. See +live while open through a warm-cache freshness gate keyed on both the +replicated PR revision and the lightweight remote GitHub revision. Mapped-row +changes arrive through the changeset stream; local-only projection changes use +`prs_updated`. The detail screen re-fetches sidecars at most once every 25 s. +See [iOS companion → PR detail screen](../sync-and-multi-device/ios-companion.md#pr-detail-screen). +An unmapped PR uses the stable synthetic id `gh:owner/repo#number` and the +`prs.getMobileGithubDetail` aggregate. That command returns the core snapshot, +fresh list/header identity, review threads, action runs, and activity in one +controller round trip. Phone requests are single-flight per PR, and failed +optional sidecars are named explicitly so the phone preserves the last good +value and shows partial-data retry UI instead of caching an empty result as a +true zero. Partial aggregates use the normal 25 s freshness window as retry +backoff; explicit Retry bypasses it. The phone renders the same +description/files/checks/timeline it would for a mapped PR while keeping +mutation controls locked until the PR is mapped. +The same banner offers both create-from-branch and map-to-existing-lane actions, +so mapping does not require backing out to the list. + The mobile detail header is intentionally compact: a plain back chevron, centered PR title with `#number · lane · branch`, and a plain ellipsis actions button. The old large PR hero is replaced by a small summary section diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index 5518b9df2..40397d9b0 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -329,7 +329,8 @@ apps/ios/ │ │ │ # PrDetailActivityTab (timeline builder + │ │ │ # commit-group folding), PrDetailOverviewTab, │ │ │ # PrMergeGateCard (PrGlassPalette tokens), -│ │ │ # PrHelpers, PrListRowModifier, +│ │ │ # PrHelpers, PrModels, PrRowCard, +│ │ │ # PrListRowModifier, │ │ │ # PrWorkflowCards, PrStackSheet, │ │ │ # CreatePrWizardView, PrRebaseScreen, │ │ │ # PrTargetBranchPickerDropdown, @@ -366,6 +367,8 @@ apps/ios/ ├── AccountEmailAuthFlowTests.swift # returning email sign-in, new-email │ # sign-up fallback, exact error codes ├── PairingAndDpopTests.swift # smart-URL QR parse + DPoP proof tests + ├── PrMergeMergeStateTests.swift # PR merge state, history/count decoding, + │ # reconciliation, partial-detail retention └── SSHBootstrapTests.swift # key parsing, fingerprint, bootstrap policy ``` @@ -1315,7 +1318,11 @@ refresh is in flight. Projection reloads are keyed by narrow revision counters: `lanesProjectionRevision`, `laneDetailProjectionRevision`, `workProjectionRevision`, `filesProjectionRevision`, -`prsProjectionRevision`, and `proofArtifactsProjectionRevision`. +`prsProjectionRevision`, `prsRemoteRevision`, and +`proofArtifactsProjectionRevision`. `prsProjectionRevision` tracks replicated +mapped-PR changes; `prsRemoteRevision` is the coalescing key for the host's +lightweight `prs_updated` invalidation when local-only GitHub projections +change. Top-level tabs and detail screens observe only the revision that maps to their data, so a chat transcript changeset no longer causes Files, Lanes, and PRs to all re-query together. @@ -1386,6 +1393,23 @@ the lane-PR list, `PrDetailView` synthesizes a fallback list item from the repo-scoped GitHub row + snapshot so the hero card never collapses into a `Pull request / @unknown` placeholder. +The list reconciles the bounded GitHub projection with the replicated mapped-PR +cache, so linked rows remain available offline and a replicated merged/closed +state wins over a stale Open projection. For the All scope, Open / Merged / +Closed tabs read exact repository totals from one cached GraphQL count query; +ADE and External scope counts remain local to the reconciled row set. Terminal +history pages independently in bounded two-page increments, up to ten pages. A +tiny host `prs_updated` envelope invalidates the list after webhook changes; +the view coalesces bursts for 300 ms and reads the newest projected snapshot +once with row revalidation disabled, so freshness requires neither a timer nor +one request per PR. Manual refresh runs PR and lane refreshes concurrently. + +`PrRowCard` keeps scroll-time rendering deliberately compact: a state symbol, +two-line title, age, identity line, and one signal line for branch/lane/checks/ +reviews/comments. It does not fetch author avatars from the network. Filtering, +sorting, counts, and mapped-row reconciliation are recomputed only when their +inputs change rather than during each SwiftUI `body` pass. + ### PR detail screen Source: `apps/ios/ADE/Views/PRs/PrDetailScreen.swift` (the `PrDetailView` @@ -1405,7 +1429,8 @@ Reading order (desktop parity, folded to one column): status, diff totals, and an expandable commit list whose rows jump to the matching timeline anchor; 2. an unmapped-PR banner (`PrUnmappedThreadBanner`) when the PR has no ADE - lane, offering auto-map (`prs.createLaneFromPrBranch`) or Open in GitHub; + lane, offering auto-map (`prs.createLaneFromPrBranch`), map to an existing + lane (`prs.linkToLane`), or Open in GitHub; 3. the AI summary card (`PrAiSummaryCard`, +/- totals from the file list); 4. the PR description (`PrThreadDescriptionCard`); 5. a chronological event feed — one row per timeline event or folded @@ -1433,6 +1458,18 @@ display items, the unresolved/resolved thread split, and the synthesized fallback PR — are precomputed once per data change in `recomputeDerivedModels()`, never inside `body`. +Unmapped rows navigate to this full screen with a synthetic +`gh:owner/repo#number` route instead of opening the old metadata-only sheet. +The warm cache seeds the row title immediately, then +`prs.getMobileGithubDetail` loads detail/status/checks/reviews/comments/files, +commits, review threads, action runs, and activity in one sync command. The +description, files, checks, and timeline therefore remain readable before lane +mapping; lane-dependent mutation controls stay locked. Optional sub-read +failures are returned in `unavailableParts`; the phone preserves the previous +successful field values, displays a partial-data retry notice, and uses the +normal 25 s freshness window as retry backoff. An explicit retry bypasses that +window. + **Palette.** The PR surfaces use `PrGlassPalette` (in `PrMergeGateCard.swift`) and `PrsGlass` (in `PrListRowModifier.swift`), which are now flat and adaptive light/dark and map to the desktop CSS tokens: `ink` = @@ -1445,21 +1482,21 @@ the previous stacked radial-gradient / `.plusLighter` backdrop was dropped because it forced expensive re-compositing under every scroll frame. **Freshness.** `PrDetailView` re-fetches its action sidecars (review threads, -activity feed, action runs, deployments, AI summary, capabilities) on a -`.task(id: syncService.prsProjectionRevision)`, throttled by a warm-cache -freshness window (`detailFreshnessWindow = 25 s`). It first seeds from the -service warm cache for an instant render; when the cached entry is younger -than 25 s the revision-driven reload skips the cold sidecar fan-out (8+ -network calls) and only refreshes the cheap local projection, and when the -window has lapsed the next projection bump re-fetches the sidecars too. This -is what keeps an open detail screen live under webhook-relay-driven host -updates: a GitHub webhook lands on the host, the host's hot poll rewrites -the replicated snapshot rows, the changeset pump bumps `prsProjectionRevision` -here, and the gate turns that into a throttled sidecar refresh (at most once -per 25 s) instead of a one-shot load. Pull-to-refresh and the explicit retry -path bypass the window. Detail actions (merge / close / reopen / comment / -edit) route through `SyncService.runDurablePrAction` so their spinners -survive a tab switch + remount. +activity feed, action runs, deployments, AI summary, capabilities) on a task +keyed by both the replicated PR projection revision and the lightweight remote +GitHub projection revision, throttled by a warm-cache freshness window +(`detailFreshnessWindow = 25 s`). It first seeds from the service warm cache for +an instant render; when the cached entry is younger than 25 s the +revision-driven reload skips the cold sidecar fan-out (8+ network calls) and +only refreshes the cheap local projection, and when the window has lapsed the +next revision bump re-fetches sidecars too. Mapped PR changes arrive through +the replicated changeset stream and bump `prsProjectionRevision`; local-only +GitHub projection changes arrive through `prs_updated` and bump +`prsRemoteRevision`. Both paths therefore share one throttled refresh gate +instead of creating a poll loop. Pull-to-refresh and the explicit retry path +bypass the window. Detail actions (merge / close / reopen / comment / edit) +route through `SyncService.runDurablePrAction` so their spinners survive a tab +switch + remount. ## Command policy from the runtime diff --git a/docs/playbooks/ship-lane.md b/docs/playbooks/ship-lane.md index b64476adb..6fdde6eb7 100644 --- a/docs/playbooks/ship-lane.md +++ b/docs/playbooks/ship-lane.md @@ -19,7 +19,7 @@ Run this playbook once per lane, when the code on the branch is done (or nearly - **Rebase budget rebate.** A rebase, merge-from-main, or conflict-resolution pass moves the current iteration count down by 2 before the next cap check, with a floor of 0. Example: if the lane is on iteration 4 and must rebase because `main` moved, record the rebase and continue as iteration 2. - **Scoped checks.** Never run the full test suite between iterations. For CI, fix and rerun only the failing test file(s) or failing check target. For review-only changes, rerun only directly affected existing tests, plus the narrow package typecheck/lint when the touched surface needs it. - **One push per iteration. Wait for BOTH signals before fixing anything.** Never push a CI-only fix while review bots are still running, and never push a review-only fix while CI is still running. Both signals must be **terminal** before the iteration commits — that is, every required check has a final conclusion AND every expected review bot has posted (or its status check has settled). This is not just an efficiency rule: **review-comment fixes routinely introduce new CI failures**, so applying them on a partial signal means the next push fails and you've thrown away the prior CI cycle. Wait for both, then dispatch ci-fix-agent and review-fix-agent in parallel with full knowledge of both, and combine their edits into one commit. If only one signal has landed when you wake, do not iterate — reschedule and sleep. -- **Default wait is 12 minutes.** After any push, schedule the next poll ~720s out (unless CI hasn't started at all — then 270s to stay in cache). 12 minutes is the **floor** that lets both CI shards and the slower review bots (Greptile, Copilot) finish. Re-entering before that almost always shows a partial state and produces the wrong decision. Only schedule shorter (270s) in Phase 0 immediately post-push to observe CI has kicked off. +- **Default wait is 12 minutes.** After any push, schedule the next poll ~720s out (unless CI hasn't started at all — then 270s to stay in cache). 12 minutes is the **floor** that lets both CI shards and the slower review bots (especially Greptile) finish. Re-entering before that almost always shows a partial state and produces the wrong decision. Only schedule shorter (270s) in Phase 0 immediately post-push to observe CI has kicked off. - **Token-idle waits.** Waiting is done by the agent's native scheduler/resume primitive, or by a shell `sleep` followed by one-shot checks. Between wake-ups, agents should be asleep, not consuming model context or tokens. - **Idempotent resume.** All state lives in `.ade/shipLane/.json`. A re-invocation reads that file and picks up where it left off. @@ -190,7 +190,8 @@ Record in the state file which path was used (`prCreatedVia: "ade" | "gh"`). If ### 0.4 Post initial bot pings -See Phase 4 rules — always `@copilot review but do not make fixes`; add `@greptile` and `@coderabbit` if the diff touches more than 250 files. +See Phase 4 rules. Do not ping GitHub Copilot. Add `@greptile` and `@coderabbit` +only when the diff touches more than 250 files. ### 0.5 Write initial state @@ -221,7 +222,7 @@ This is a one-shot poll. Do not use `gh pr checks --watch`, shell `while` loops, **Wait for both CI and review bots before iterating.** The poll must treat these as two independent signals and only report "ready to fix" when **both** are terminal: - **CI terminal** = every required check has a final conclusion (`success`, `failure`, `cancelled`, `skipped`, `neutral`). If any required check is still `QUEUED` or `IN_PROGRESS`, CI is not terminal. -- **Review bots terminal** = every expected review bot has either posted its review (`gh api repos/.../pulls/{N}/reviews` contains a submission newer than `lastPushSha`'s commit time for each bot) **or** its status check entry has settled. Expected bots for this repo include **Greptile** (appears as the `Greptile Review` status check — wait for it to leave `pending`), **CodeRabbit** (posts a status check and/or review), and **Copilot** (posts an issue comment after being pinged; allow ~3–5 min from the ping). Greptile in particular is slow enough that the 12-minute wait is driven primarily by it. +- **Review bots terminal** = every expected review bot has either posted its review (`gh api repos/.../pulls/{N}/reviews` contains a submission newer than `lastPushSha`'s commit time for each bot) **or** its status check entry has settled. Expected bots for this repo are **Greptile** (appears as the `Greptile Review` status check — wait for it to leave `pending`) and **CodeRabbit** (posts a status check and/or review). GitHub Copilot is neither pinged nor expected: quota exhaustion can stop it without producing a normal review response, so its events never gate the loop. Greptile in particular is slow enough that the 12-minute wait is driven primarily by it. If either signal is still in flight, return "still waiting" and let Phase 5 reschedule. **Do not push a fix on a partial signal.** @@ -281,7 +282,7 @@ Filter out any comment whose `id` is in `addressedCommentIds`. "newComments": [ { "id": 987700, - "author": "copilot-pull-request-reviewer", + "author": "human-reviewer", "body": "Consider guarding against null here.", "path": "apps/desktop/src/main/services/x.ts", "line": 42, @@ -292,7 +293,7 @@ Filter out any comment whose `id` is in `addressedCommentIds`. } ``` -`reviewBotsRunning` is `true` whenever `pendingReviewBots` is non-empty. Populate `pendingReviewBots` with any expected bot that has neither posted a review newer than `lastPushSha` nor has a settled status check (e.g., Greptile status still `pending`, Copilot has not commented within ~5 min of the ping). +`reviewBotsRunning` is `true` whenever `pendingReviewBots` is non-empty. Populate `pendingReviewBots` with any expected bot that has neither posted a review newer than `lastPushSha` nor has a settled status check (for example, Greptile status is still `pending`). `behindMain` is derived from `mergeStateStatus` being `BEHIND` or `DIRTY`, or from `git merge-base --is-ancestor origin/main HEAD` returning non-zero. @@ -583,14 +584,10 @@ There is no second force-finalize. Iteration 6 is one shot at landing the lane. ## Phase 4 — Post-push bot pings -Runs after **any** push. The ping depends on whether this is the initial PR push -or a later fix iteration: +Runs after **any** push. Never ping GitHub Copilot. The ping depends on whether +this is the initial PR push or a later fix iteration: -- **Initial push** (Phase 0, PR just created): - -```bash -gh pr comment "$PR_NUMBER" --body "@copilot review but do not make fixes" -``` +- **Initial push** (Phase 0, PR just created): no direct review ping. - **Subsequent fix-iteration re-pushes:** @@ -646,7 +643,7 @@ Agent-CLI-agnostic guidance. Pick the right primitive for the harness: Cadence (applies once you've picked a primitive): - Just pushed, neither CI nor review has started yet → **270 seconds** (stay in prompt cache; next poll only confirms things have kicked off) -- CI running OR review bots still pending → **720 seconds** (12 min). This is the spec floor: CI shards typically finish in 3–5 min, Greptile in 5–10 min, Copilot within a few minutes of its ping. 12 min is what lets **both** land before the next poll. +- CI running OR review bots still pending → **720 seconds** (12 min). This is the spec floor: CI shards typically finish in 3–5 min and Greptile in 5–10 min. 12 min is what lets **both** land before the next poll. - CI terminal AND review bots terminal, now waiting on human review → **1800 seconds** (30 min; cost-efficient) - Unknown → **720 seconds** default