From d3655f215b1af51c07786949fa751ad00d20e0c1 Mon Sep 17 00:00:00 2001 From: Bailey Dixon Date: Tue, 14 Jul 2026 16:43:40 -0400 Subject: [PATCH 1/2] feat: reconcile stale GitHub PR status --- CHANGELOG.md | 8 + DEVLOG.md | 24 ++ .../migration.sql | 25 ++ prisma/schema.prisma | 22 ++ .../w/[slug]/settings/connections/page.tsx | 3 + .../issue-detail/github-links-panel.tsx | 23 +- .../settings/github-reconciliation-policy.tsx | 175 ++++++++++++ src/server/routers/workspace.ts | 30 ++ .../__tests__/github-reconciliation.test.ts | 262 ++++++++++++++++++ src/server/services/github/client.ts | 140 +++++++++- src/server/services/github/reconciliation.ts | 214 ++++++++++++++ src/server/services/github/resource-sync.ts | 93 ++++++- src/server/services/github/webhook.ts | 23 +- src/server/worker.ts | 23 ++ tests/unit/github-client.test.ts | 60 ++++ 15 files changed, 1112 insertions(+), 13 deletions(-) create mode 100644 prisma/migrations/0102_github_status_reconciliation/migration.sql create mode 100644 src/components/settings/github-reconciliation-policy.tsx create mode 100644 src/server/services/__tests__/github-reconciliation.test.ts create mode 100644 src/server/services/github/reconciliation.ts create mode 100644 tests/unit/github-client.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 46772935..b7c2fb56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ date, grouped by `Added` / `Changed` / `Fixed` / `Removed`. ## [Unreleased] +### Added + +- **Linked implementation PRs now self-heal after missed GitHub webhooks.** A settings-driven stale-only worker refreshes PR lifecycle and aggregate checks with bounded batches, restart-safe leases, provider-aware rate-limit backoff, and visible retry diagnostics. + +### Fixed + +- **GitHub status recovery cannot certify stale or partial evidence.** New PR heads invalidate cached checks, late check events for older heads are ignored, partial GitHub App permissions remain non-blocking, and merged PRs without confirmed checks back off instead of polling hot or closing issues. + ## [2026-07-14] — v0.14.0 · Proactive completion handoff ### Added diff --git a/DEVLOG.md b/DEVLOG.md index 63ff2145..f5638281 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -2,6 +2,30 @@ > Append-only session log. Read at session start. Update at session end. +## 2026-07-14 — Resilient linked-PR status reconciliation + +Added a webhook-first repair loop for native GitHub `IMPLEMENTS` links. The +maintenance worker now scans only stale, non-terminal PR snapshots in bounded +per-workspace batches, claims each row with an expiring database lease, and +refreshes PR lifecycle plus both GitHub check suites and legacy combined commit +statuses. Workspace admins can configure enablement, stale age, batch size, and +initial/maximum backoff from Connections settings. + +Persisted provider attempt, retry, failure, diagnostic, and terminal state on +`ExternalResource`. GitHub rate-limit headers override exponential retry; +missing/paused mappings are re-resolved by repository; inaccessible resources +back off without being unlinked; closed PRs and confirmed-green merged PRs stop +polling; reopened PRs become eligible again. Partial Checks/status permissions +no longer prevent a readable PR from linking, but partial evidence cannot +certify completion. Head changes clear cached checks and late check webhooks for +an older head are ignored. Repeated unchanged refreshes produce no issue +activity or duplicate completion cards. + +Verification: Prisma migration 0102 applied locally; lint passed with existing +repository warnings only; typecheck passed; focused GitHub reconciliation, +client, check aggregation, completion, and head-invalidation coverage passed +(**13/13**); and `git diff --check` passed. + ## 2026-07-13 — AXI-102 patient-wait spam + expandable Command Center cards Closed the AXI-102 stall-comment loop introduced by approval-expiry recovery. diff --git a/prisma/migrations/0102_github_status_reconciliation/migration.sql b/prisma/migrations/0102_github_status_reconciliation/migration.sql new file mode 100644 index 00000000..88720eff --- /dev/null +++ b/prisma/migrations/0102_github_status_reconciliation/migration.sql @@ -0,0 +1,25 @@ +ALTER TABLE "Workspace" + ADD COLUMN "githubSyncEnabled" BOOLEAN NOT NULL DEFAULT true, + ADD COLUMN "githubSyncStaleMinutes" INTEGER NOT NULL DEFAULT 15, + ADD COLUMN "githubSyncBatchSize" INTEGER NOT NULL DEFAULT 25, + ADD COLUMN "githubSyncBackoffMinutes" INTEGER NOT NULL DEFAULT 5, + ADD COLUMN "githubSyncMaxBackoffMinutes" INTEGER NOT NULL DEFAULT 1440; + +ALTER TABLE "ExternalResource" + ADD COLUMN "syncAttemptedAt" TIMESTAMP(3), + ADD COLUMN "syncRetryAt" TIMESTAMP(3), + ADD COLUMN "syncFailureCount" INTEGER NOT NULL DEFAULT 0, + ADD COLUMN "syncLastError" TEXT, + ADD COLUMN "syncTerminalAt" TIMESTAMP(3); + +CREATE INDEX "ExternalResource_workspaceId_provider_resourceType_syncTerminalAt_lastSyncedAt_idx" + ON "ExternalResource"("workspaceId", "provider", "resourceType", "syncTerminalAt", "lastSyncedAt"); +CREATE INDEX "ExternalResource_syncRetryAt_idx" ON "ExternalResource"("syncRetryAt"); + +-- Closed unmerged PRs are final. Merged PRs remain eligible until Forge has +-- refreshed aggregate checks at least once under the new reconciliation path. +UPDATE "ExternalResource" +SET "syncTerminalAt" = COALESCE("lastSyncedAt", "updatedAt") +WHERE "provider" = 'GITHUB' + AND "resourceType" = 'PULL_REQUEST' + AND "state" = 'closed'; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 98d0c570..da3d1999 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -889,6 +889,17 @@ model Workspace { /// DONE-category status used by completion recommendations/automation. /// Null keeps completion candidates disabled until an admin chooses one. completionStatusId String? + /// Self-heal missed GitHub webhooks by polling stale linked implementation + /// PRs. Webhooks remain the primary path; this only reconciles stale rows. + githubSyncEnabled Boolean @default(true) + /// Minimum age of a successful GitHub snapshot before it may be polled. + githubSyncStaleMinutes Int @default(15) + /// Maximum stale GitHub resources processed for this workspace per sweep. + githubSyncBatchSize Int @default(25) + /// Initial retry delay for provider failures; retries grow exponentially. + githubSyncBackoffMinutes Int @default(5) + /// Maximum retry delay for repeated provider failures. + githubSyncMaxBackoffMinutes Int @default(1440) /// Issues with `updatedAt` older than this threshold (in days) are /// considered "stalled" by the dashboard suggestions bucket. Settings- /// driven so admins can tune the workspace's idle definition without @@ -2185,6 +2196,15 @@ model ExternalResource { externalCreatedAt DateTime? externalUpdatedAt DateTime? lastSyncedAt DateTime? + /// Last background/manual provider request attempt, successful or not. + syncAttemptedAt DateTime? + /// Retry gate after rate limiting, auth/mapping errors, or provider failure. + syncRetryAt DateTime? + syncFailureCount Int @default(0) + syncLastError String? @db.Text + /// Set once this resource no longer needs background reconciliation. + /// Open/draft PRs and merged PRs without passing aggregate checks stay null. + syncTerminalAt DateTime? createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -2196,6 +2216,8 @@ model ExternalResource { @@index([workspaceId, provider, repoFullName]) @@index([workspaceId, provider, externalNodeId]) @@index([connectionMappingId]) + @@index([workspaceId, provider, resourceType, syncTerminalAt, lastSyncedAt]) + @@index([syncRetryAt]) } /// Join between a Forge issue and an external resource. The link kind is diff --git a/src/app/(app)/w/[slug]/settings/connections/page.tsx b/src/app/(app)/w/[slug]/settings/connections/page.tsx index 76a4b39e..5f8c4265 100644 --- a/src/app/(app)/w/[slug]/settings/connections/page.tsx +++ b/src/app/(app)/w/[slug]/settings/connections/page.tsx @@ -25,6 +25,7 @@ import { Input } from "@/components/ui/input"; import { Confirm, QuickForm } from "@/components/ui/modal"; import { Card } from "@/components/settings/card"; import { EmptyState } from "@/components/settings/empty-state"; +import { GitHubReconciliationPolicy } from "@/components/settings/github-reconciliation-policy"; import { Section } from "@/components/ui"; import { trpc } from "@/lib/trpc"; @@ -443,6 +444,8 @@ export default function ConnectionsMappingPage() {

+ + {!connectionsLoading && !hasAnyConnection && ( synced {relativeTime(resource.lastSyncedAt)} )} + {resource.syncLastError && ( +
+ + + GitHub refresh delayed + {resource.syncRetryAt + ? ` · retry ${formatDate(resource.syncRetryAt, undefined, { + month: "short", + day: "numeric", + hour: "numeric", + minute: "2-digit", + })}` + : ""} + +
+ )} + + + + ); +} + +function ToggleRow({ + label, + hint, + checked, + onChange, +}: { + label: string; + hint: string; + checked: boolean; + onChange: (checked: boolean) => void; +}) { + return ( + + ); +} + +function NumberField({ + label, + suffix, + value, + min, + max, + onChange, +}: { + label: string; + suffix: string; + value: number; + min: number; + max: number; + onChange: (value: number) => void; +}) { + return ( + + ); +} diff --git a/src/server/routers/workspace.ts b/src/server/routers/workspace.ts index 6ba3f478..a537fdcc 100644 --- a/src/server/routers/workspace.ts +++ b/src/server/routers/workspace.ts @@ -138,6 +138,11 @@ export const workspaceRouter = router({ reviewStatusId: true, completionAutomation: true, completionStatusId: true, + githubSyncEnabled: true, + githubSyncStaleMinutes: true, + githubSyncBatchSize: true, + githubSyncBackoffMinutes: true, + githubSyncMaxBackoffMinutes: true, stalledThresholdDays: true, agentHeartbeatWarnMinutes: true, agentHeartbeatCriticalMinutes: true, @@ -307,6 +312,11 @@ export const workspaceRouter = router({ reviewStatusId: z.string().nullable().optional(), completionAutomation: z.nativeEnum(CompletionAutomation).optional(), completionStatusId: z.string().nullable().optional(), + githubSyncEnabled: z.boolean().optional(), + githubSyncStaleMinutes: z.number().int().min(0).max(10080).optional(), + githubSyncBatchSize: z.number().int().min(0).max(250).optional(), + githubSyncBackoffMinutes: z.number().int().min(1).max(1440).optional(), + githubSyncMaxBackoffMinutes: z.number().int().min(1).max(10080).optional(), assignmentEngagementMode: z.nativeEnum(EngagementMode).optional(), mentionEngagementPolicy: z.nativeEnum(MentionEngagementPolicy).optional(), mentionDefaultMode: z.nativeEnum(EngagementMode).optional(), @@ -315,6 +325,26 @@ export const workspaceRouter = router({ ) .mutation(async ({ ctx, input }) => { const data = { ...input }; + if ( + input.githubSyncBackoffMinutes !== undefined || + input.githubSyncMaxBackoffMinutes !== undefined + ) { + const current = await ctx.db.workspace.findUniqueOrThrow({ + where: { id: ctx.workspaceId }, + select: { + githubSyncBackoffMinutes: true, + githubSyncMaxBackoffMinutes: true, + }, + }); + const base = input.githubSyncBackoffMinutes ?? current.githubSyncBackoffMinutes; + const max = input.githubSyncMaxBackoffMinutes ?? current.githubSyncMaxBackoffMinutes; + if (max < base) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "GitHub maximum backoff must be at least the initial backoff.", + }); + } + } // Validate startedStatusId belongs to this workspace and is in the // IN_PROGRESS category. Setting null to disable is fine. if (input.startedStatusId) { diff --git a/src/server/services/__tests__/github-reconciliation.test.ts b/src/server/services/__tests__/github-reconciliation.test.ts new file mode 100644 index 00000000..5c8a1b1f --- /dev/null +++ b/src/server/services/__tests__/github-reconciliation.test.ts @@ -0,0 +1,262 @@ +import { afterAll, afterEach, describe, expect, it } from "vitest"; +import { GitHubRequestError } from "@/server/services/github/client"; +import { upsertExternalResource } from "@/server/services/github/resource-sync"; +import { sweepGitHubStatusReconciliation } from "@/server/services/github/reconciliation"; +import { + createIssue, + createWorkspaceFixture, + disconnectPrisma, + getPrisma, + type TestFixture, +} from "@/server/routers/__tests__/helpers"; + +const fixtures: TestFixture[] = []; + +afterEach(async () => { + while (fixtures.length) await fixtures.pop()!.cleanup(); +}); + +afterAll(async () => { + await disconnectPrisma(); +}); + +async function setupResource() { + const fixture = await createWorkspaceFixture({ keyPrefix: "GR" }); + fixtures.push(fixture); + const prisma = getPrisma(); + const issue = await createIssue(fixture, { statusCategory: "IN_PROGRESS" }); + await prisma.workspace.update({ + where: { id: fixture.workspace.id }, + data: { + githubSyncEnabled: true, + githubSyncStaleMinutes: 15, + githubSyncBatchSize: 10, + githubSyncBackoffMinutes: 5, + githubSyncMaxBackoffMinutes: 1440, + }, + }); + const resource = await prisma.externalResource.create({ + data: { + workspaceId: fixture.workspace.id, + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 42, + url: "https://github.com/acme/forge/pull/42", + title: "Ship it", + state: "open", + lastSyncedAt: new Date("2026-07-14T10:00:00.000Z"), + }, + }); + await prisma.externalResourceLink.create({ + data: { + workspaceId: fixture.workspace.id, + issueId: issue.id, + externalResourceId: resource.id, + kind: "IMPLEMENTS", + createdById: fixture.user.id, + }, + }); + return { fixture, prisma, resource }; +} + +describe("GitHub status reconciliation", () => { + it("polls only stale native implementation links and is a no-op once fresh", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + let calls = 0; + const syncResource = async () => { + calls += 1; + return prisma.externalResource.update({ + where: { id: resource.id }, + data: { + lastSyncedAt: now, + syncAttemptedAt: now, + syncRetryAt: null, + syncFailureCount: 0, + syncLastError: null, + }, + }); + }; + + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).resolves.toMatchObject({ inspected: 1, reconciled: 1, failed: 0 }); + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).resolves.toMatchObject({ inspected: 0, reconciled: 0 }); + expect(calls).toBe(1); + expect(await prisma.activityEvent.count({ where: { workspaceId: fixture.workspace.id } })).toBe( + 0, + ); + }); + + it("persists rate-limit backoff and skips the resource until reset", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const reset = new Date("2026-07-14T13:00:00.000Z"); + let calls = 0; + const syncResource = async (): Promise => { + calls += 1; + throw new GitHubRequestError("API rate limit exceeded", 429, reset); + }; + + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).resolves.toMatchObject({ inspected: 1, failed: 1, rateLimited: 1 }); + const failed = await prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }); + expect(failed.syncRetryAt).toEqual(reset); + expect(failed.syncFailureCount).toBe(1); + await sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now: new Date("2026-07-14T12:30:00.000Z"), + syncResource, + }); + expect(calls).toBe(1); + }); + + it("backs off merged PRs with no checks and re-enables terminal rows when reopened", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const syncResource = async () => + prisma.externalResource.update({ + where: { id: resource.id }, + data: { + state: "merged", + metadata: { checks: { status: "unknown", conclusion: null } }, + lastSyncedAt: now, + syncRetryAt: null, + syncFailureCount: 0, + }, + }); + + await sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }); + const held = await prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }); + expect(held.syncFailureCount).toBe(1); + expect(held.syncRetryAt).toEqual(new Date("2026-07-14T12:05:00.000Z")); + + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { state: "closed", syncTerminalAt: now }, + }); + const reopened = await upsertExternalResource(prisma, { + workspaceId: fixture.workspace.id, + snapshot: { + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 42, + url: resource.url, + title: resource.title, + state: "open", + labels: [], + assignees: [], + metadata: {}, + }, + }); + expect(reopened.syncTerminalAt).toBeNull(); + expect(reopened.syncFailureCount).toBe(0); + + await upsertExternalResource(prisma, { + workspaceId: fixture.workspace.id, + snapshot: { + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 42, + url: resource.url, + title: resource.title, + state: "open", + labels: [], + assignees: [], + metadata: { + head: { sha: "old-head" }, + checks: { status: "completed", conclusion: "success", headSha: "old-head" }, + }, + }, + }); + const newHead = await upsertExternalResource(prisma, { + workspaceId: fixture.workspace.id, + snapshot: { + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 42, + url: resource.url, + title: resource.title, + state: "open", + labels: [], + assignees: [], + metadata: { head: { sha: "new-head" } }, + }, + }); + expect(newHead.metadata).not.toHaveProperty("checks"); + }); + + it("does not inspect disabled workspaces, terminal PRs, or an active retry lease", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + let calls = 0; + const syncResource = async () => { + calls += 1; + return prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }); + }; + + await prisma.workspace.update({ + where: { id: fixture.workspace.id }, + data: { githubSyncEnabled: false }, + }); + expect( + await sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).toMatchObject({ workspaces: 0, inspected: 0 }); + + await prisma.workspace.update({ + where: { id: fixture.workspace.id }, + data: { githubSyncEnabled: true }, + }); + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { syncTerminalAt: now }, + }); + expect( + await sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).toMatchObject({ inspected: 0 }); + + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { syncTerminalAt: null, syncRetryAt: new Date("2026-07-14T12:02:00.000Z") }, + }); + expect( + await sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).toMatchObject({ inspected: 0 }); + expect(calls).toBe(0); + }); +}); diff --git a/src/server/services/github/client.ts b/src/server/services/github/client.ts index b492774e..735e484b 100644 --- a/src/server/services/github/client.ts +++ b/src/server/services/github/client.ts @@ -62,6 +62,46 @@ type GitHubListReposResponse = { repositories?: GitHubRepoResponse[]; }; +type GitHubCheckSuite = { + status?: string | null; + conclusion?: string | null; +}; + +type GitHubCheckSuitesResponse = { + total_count?: number; + check_suites?: GitHubCheckSuite[]; +}; + +type GitHubCombinedStatusResponse = { + state?: "error" | "failure" | "pending" | "success"; + total_count?: number; +}; + +export type GitHubChecksSnapshot = { + status: "completed" | "pending" | "unknown"; + conclusion: string | null; + suiteCount: number; + statusCount: number; + updatedAt: string; + source: "reconciliation"; + partial: boolean; + diagnostic: string | null; + retryAt: string | null; + headSha: string; +}; + +export class GitHubRequestError extends Error { + constructor( + message: string, + readonly status: number, + readonly retryAt: Date | null, + readonly rateLimited: boolean = status === 429, + ) { + super(message); + this.name = "GitHubRequestError"; + } +} + export type GitHubInstallationResponse = { id: number; account?: { login?: string | null; type?: string | null } | null; @@ -120,7 +160,19 @@ async function githubRequest( json && typeof json === "object" && "message" in json && typeof json.message === "string" ? json.message : `GitHub API returned HTTP ${res.status}.`; - throw new Error(message); + const retryAfter = Number(res.headers.get("retry-after")); + const rateLimitReset = Number(res.headers.get("x-ratelimit-reset")); + const retryAt = + Number.isFinite(retryAfter) && retryAfter > 0 + ? new Date(Date.now() + retryAfter * 1000) + : Number.isFinite(rateLimitReset) && rateLimitReset > 0 + ? new Date(rateLimitReset * 1000) + : null; + const rateLimited = + res.status === 429 || + (res.status === 403 && + (res.headers.has("retry-after") || res.headers.get("x-ratelimit-remaining") === "0")); + throw new GitHubRequestError(message, res.status, retryAt, rateLimited); } return json as T; } @@ -138,9 +190,7 @@ function labels(labels: GitHubLabel[] | undefined): Array<{ name: string; color: } function assignees(users: GitHubUser[] | undefined): Array<{ login: string }> { - return (users ?? []) - .map((u) => ({ login: u.login ?? "" })) - .filter((u) => u.login.length > 0); + return (users ?? []).map((u) => ({ login: u.login ?? "" })).filter((u) => u.login.length > 0); } export async function getGitHubIssue(args: { @@ -167,6 +217,88 @@ export async function getGitHubPullRequest(args: { ); } +/** + * Read both GitHub Checks and legacy commit statuses for a PR head. A passing + * result is only returned when every discovered signal is complete and green; + * zero discovered signals stays unknown and cannot certify completion. + */ +export async function getGitHubPullRequestChecks(args: { + installationId: string | number; + owner: string; + repo: string; + headSha: string; +}): Promise { + const ref = encodeURIComponent(args.headSha); + const [suiteResult, statusResult] = await Promise.allSettled([ + githubRequest( + args.installationId, + `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/commits/${ref}/check-suites?filter=latest&per_page=100`, + ), + githubRequest( + args.installationId, + `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/commits/${ref}/status`, + ), + ]); + const suites = suiteResult.status === "fulfilled" ? suiteResult.value : {}; + const statuses = statusResult.status === "fulfilled" ? statusResult.value : {}; + const rows = suites.check_suites ?? []; + const suiteCount = suites.total_count ?? rows.length; + const statusCount = statuses.total_count ?? 0; + const discovered = suiteCount + statusCount; + const pending = + rows.some((suite) => suite.status !== "completed") || statuses.state === "pending"; + const failedSuite = rows.find( + (suite) => + suite.status === "completed" && + !!suite.conclusion && + !["success", "neutral", "skipped"].includes(suite.conclusion), + ); + const failedStatus = statuses.state === "failure" || statuses.state === "error"; + const conclusion = + failedSuite?.conclusion ?? (failedStatus ? (statuses.state ?? "failure") : null); + const failures = [suiteResult, statusResult].filter( + (result): result is PromiseRejectedResult => result.status === "rejected", + ); + const incompleteSuites = suiteCount > rows.length; + const partial = failures.length > 0 || incompleteSuites; + const retryAt = + failures + .map((failure) => + failure.reason instanceof GitHubRequestError ? failure.reason.retryAt : null, + ) + .filter((value): value is Date => value instanceof Date) + .sort((a, b) => b.getTime() - a.getTime())[0] ?? null; + + return { + status: + conclusion !== null + ? "completed" + : partial || discovered === 0 + ? "unknown" + : pending + ? "pending" + : "completed", + conclusion: conclusion ?? (!partial && discovered > 0 && !pending ? "success" : null), + suiteCount, + statusCount, + updatedAt: new Date().toISOString(), + source: "reconciliation", + partial, + diagnostic: partial + ? [ + ...failures.map((failure) => + failure.reason instanceof Error ? failure.reason.message : "GitHub checks unavailable", + ), + ...(incompleteSuites ? ["More than 100 check suites require another page."] : []), + ] + .join("; ") + .slice(0, 2_000) + : null, + retryAt: retryAt?.toISOString() ?? null, + headSha: args.headSha, + }; +} + export async function listGitHubInstallationRepos(args: { installationId: string | number; }): Promise { diff --git a/src/server/services/github/reconciliation.ts b/src/server/services/github/reconciliation.ts new file mode 100644 index 00000000..8936a2c5 --- /dev/null +++ b/src/server/services/github/reconciliation.ts @@ -0,0 +1,214 @@ +import "server-only"; +import type { PrismaClient } from "@prisma/client"; +import { logger } from "@/server/logger"; +import { GitHubRequestError } from "@/server/services/github/client"; +import { syncGitHubExternalResource } from "@/server/services/github/resource-sync"; + +const PROVIDER = "GITHUB"; +const RESOURCE_TYPE = "PULL_REQUEST"; +const LEASE_MS = 2 * 60_000; + +export type GitHubReconciliationResult = { + workspaces: number; + inspected: number; + reconciled: number; + failed: number; + rateLimited: number; + skipped: number; +}; + +type SyncResource = typeof syncGitHubExternalResource; + +function retryAtForFailure(args: { + error: unknown; + failureCount: number; + baseMinutes: number; + maxMinutes: number; + now: Date; +}): { retryAt: Date; rateLimited: boolean; message: string } { + const error = args.error; + const providerRetryAt = error instanceof GitHubRequestError ? error.retryAt : null; + const rateLimited = error instanceof GitHubRequestError && error.rateLimited; + const exponentialMinutes = Math.min( + args.maxMinutes, + args.baseMinutes * 2 ** Math.max(0, args.failureCount - 1), + ); + const fallback = new Date(args.now.getTime() + exponentialMinutes * 60_000); + return { + retryAt: providerRetryAt && providerRetryAt > fallback ? providerRetryAt : fallback, + rateLimited, + message: error instanceof Error ? error.message.slice(0, 2_000) : "Unknown GitHub sync error", + }; +} + +function checksMetadata(value: unknown): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) return {}; + const checks = (value as Record).checks; + return checks && typeof checks === "object" && !Array.isArray(checks) + ? (checks as Record) + : {}; +} + +/** + * Reconcile stale native IMPLEMENTS links. GitHub webhooks stay primary; this + * sweep is a bounded repair loop for missed delivery, permission recovery, and + * worker restarts. A short DB lease prevents overlapping workers/manual jobs + * from polling the same row at once and naturally expires after a crash. + */ +export async function sweepGitHubStatusReconciliation( + db: PrismaClient, + options: { + workspaceId?: string; + now?: Date; + syncResource?: SyncResource; + } = {}, +): Promise { + const now = options.now ?? new Date(); + const syncResource = options.syncResource ?? syncGitHubExternalResource; + const result: GitHubReconciliationResult = { + workspaces: 0, + inspected: 0, + reconciled: 0, + failed: 0, + rateLimited: 0, + skipped: 0, + }; + const workspaces = await db.workspace.findMany({ + where: { + deletedAt: null, + githubSyncEnabled: true, + ...(options.workspaceId ? { id: options.workspaceId } : {}), + }, + select: { + id: true, + githubSyncStaleMinutes: true, + githubSyncBatchSize: true, + githubSyncBackoffMinutes: true, + githubSyncMaxBackoffMinutes: true, + }, + }); + + for (const workspace of workspaces) { + result.workspaces += 1; + if (workspace.githubSyncBatchSize <= 0 || workspace.githubSyncStaleMinutes <= 0) { + continue; + } + const staleBefore = new Date(now.getTime() - workspace.githubSyncStaleMinutes * 60_000); + const candidates = await db.externalResource.findMany({ + where: { + workspaceId: workspace.id, + provider: PROVIDER, + resourceType: RESOURCE_TYPE, + syncTerminalAt: null, + OR: [{ lastSyncedAt: null }, { lastSyncedAt: { lte: staleBefore } }], + AND: [{ OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: now } }] }], + links: { + some: { + workspaceId: workspace.id, + kind: "IMPLEMENTS", + issue: { workspaceId: workspace.id, deletedAt: null }, + }, + }, + }, + select: { id: true, syncFailureCount: true }, + orderBy: [{ lastSyncedAt: "asc" }, { id: "asc" }], + take: workspace.githubSyncBatchSize, + }); + + for (const candidate of candidates) { + result.inspected += 1; + const leaseUntil = new Date(now.getTime() + LEASE_MS); + const claimed = await db.externalResource.updateMany({ + where: { + id: candidate.id, + workspaceId: workspace.id, + syncTerminalAt: null, + OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: now } }], + }, + data: { syncAttemptedAt: now, syncRetryAt: leaseUntil }, + }); + if (claimed.count === 0) { + result.skipped += 1; + continue; + } + + try { + const resource = await syncResource({ + db, + workspaceId: workspace.id, + externalResourceId: candidate.id, + actor: { actorId: null }, + }); + const checks = checksMetadata(resource.metadata); + // A merged PR with zero/partially-readable checks must remain held, but + // exponential retry keeps repositories with no CI from being polled at + // the normal stale cadence forever. Provider retry headers win. + if (resource.state === "merged" && checks.status === "unknown") { + const failureCount = candidate.syncFailureCount + 1; + const exponential = retryAtForFailure({ + error: new Error( + typeof checks.diagnostic === "string" + ? checks.diagnostic + : "No aggregate GitHub checks are available for this merged PR.", + ), + failureCount, + baseMinutes: Math.max(1, workspace.githubSyncBackoffMinutes), + maxMinutes: Math.max(1, workspace.githubSyncMaxBackoffMinutes), + now, + }); + const providerRetryAt = + typeof checks.retryAt === "string" ? new Date(checks.retryAt) : null; + await db.externalResource.updateMany({ + where: { id: candidate.id, workspaceId: workspace.id }, + data: { + syncFailureCount: failureCount, + syncLastError: exponential.message, + syncRetryAt: + providerRetryAt && + Number.isFinite(providerRetryAt.getTime()) && + providerRetryAt > exponential.retryAt + ? providerRetryAt + : exponential.retryAt, + }, + }); + } + result.reconciled += 1; + } catch (error) { + const failureCount = candidate.syncFailureCount + 1; + const retry = retryAtForFailure({ + error, + failureCount, + baseMinutes: Math.max(1, workspace.githubSyncBackoffMinutes), + maxMinutes: Math.max(1, workspace.githubSyncMaxBackoffMinutes), + now, + }); + await db.externalResource.updateMany({ + where: { id: candidate.id, workspaceId: workspace.id }, + data: { + syncAttemptedAt: now, + syncRetryAt: retry.retryAt, + syncFailureCount: failureCount, + syncLastError: retry.message, + }, + }); + result.failed += 1; + if (retry.rateLimited) result.rateLimited += 1; + logger.warn( + { + err: error, + workspaceId: workspace.id, + externalResourceId: candidate.id, + retryAt: retry.retryAt, + failureCount, + }, + "github-reconciliation: resource sync failed", + ); + } + } + } + + if (result.inspected > 0 || result.failed > 0) { + logger.info(result, "github-reconciliation: sweep complete"); + } + return result; +} diff --git a/src/server/services/github/resource-sync.ts b/src/server/services/github/resource-sync.ts index a87711fe..34e91c96 100644 --- a/src/server/services/github/resource-sync.ts +++ b/src/server/services/github/resource-sync.ts @@ -16,6 +16,7 @@ import { reconcileGitHubPullRequestCompletion } from "@/server/services/completi import { getGitHubIssue, getGitHubPullRequest, + getGitHubPullRequestChecks, issueSnapshot, pullRequestSnapshot, } from "@/server/services/github/client"; @@ -127,7 +128,19 @@ export async function fetchGitHubSnapshotForParsed( repo: parsed.repo, number: parsed.number, }); - return pullRequestSnapshot(parsed.repoFullName, pr); + const snapshot = pullRequestSnapshot(parsed.repoFullName, pr); + if (pr.head?.sha) { + snapshot.metadata = { + ...(snapshot.metadata as Record), + checks: await getGitHubPullRequestChecks({ + installationId, + owner: parsed.owner, + repo: parsed.repo, + headSha: pr.head.sha, + }), + }; + } + return snapshot; } const issue = await getGitHubIssue({ installationId, @@ -142,7 +155,19 @@ export async function fetchGitHubSnapshotForParsed( repo: parsed.repo, number: parsed.number, }); - return pullRequestSnapshot(parsed.repoFullName, pr); + const snapshot = pullRequestSnapshot(parsed.repoFullName, pr); + if (pr.head?.sha) { + snapshot.metadata = { + ...(snapshot.metadata as Record), + checks: await getGitHubPullRequestChecks({ + installationId, + owner: parsed.owner, + repo: parsed.repo, + headSha: pr.head.sha, + }), + }; + } + return snapshot; } return issueSnapshot(parsed.repoFullName, issue); } @@ -187,7 +212,7 @@ export async function upsertExternalResource( number: args.snapshot.number, }, }, - select: { metadata: true }, + select: { metadata: true, syncFailureCount: true }, }); const existingMetadata = existing?.metadata && typeof existing.metadata === "object" && !Array.isArray(existing.metadata) @@ -203,6 +228,45 @@ export async function upsertExternalResource( ...existingMetadata, ...snapshotMetadata, }; + const existingHead = + existingMetadata.head && + typeof existingMetadata.head === "object" && + !Array.isArray(existingMetadata.head) + ? (existingMetadata.head as Record) + : {}; + const snapshotHead = + snapshotMetadata.head && + typeof snapshotMetadata.head === "object" && + !Array.isArray(snapshotMetadata.head) + ? (snapshotMetadata.head as Record) + : {}; + if ( + typeof existingHead.sha === "string" && + typeof snapshotHead.sha === "string" && + existingHead.sha !== snapshotHead.sha && + !("checks" in snapshotMetadata) + ) { + delete metadata.checks; + } + const checks = + metadata.checks && typeof metadata.checks === "object" && !Array.isArray(metadata.checks) + ? (metadata.checks as Record) + : {}; + const passingChecks = + checks.status === "completed" && + typeof checks.conclusion === "string" && + ["success", "neutral", "skipped"].includes(checks.conclusion); + const checksDiagnostic = + checks.partial === true && typeof checks.diagnostic === "string" + ? checks.diagnostic.slice(0, 2_000) + : null; + const checksRetryAt = + typeof checks.retryAt === "string" && Number.isFinite(Date.parse(checks.retryAt)) + ? new Date(checks.retryAt) + : null; + const terminal = + args.snapshot.resourceType === "PULL_REQUEST" && + (args.snapshot.state === "closed" || (args.snapshot.state === "merged" && passingChecks)); const row = await db.externalResource.upsert({ where: { workspaceId_provider_repoFullName_resourceType_number: { @@ -233,6 +297,11 @@ export async function upsertExternalResource( externalCreatedAt: args.snapshot.externalCreatedAt ?? undefined, externalUpdatedAt: args.snapshot.externalUpdatedAt ?? undefined, lastSyncedAt: now, + syncAttemptedAt: now, + syncRetryAt: checksDiagnostic ? checksRetryAt : null, + syncFailureCount: checksDiagnostic ? 1 : 0, + syncLastError: checksDiagnostic, + syncTerminalAt: terminal ? now : undefined, }, update: { connectionMappingId: args.connectionMappingId ?? undefined, @@ -249,6 +318,11 @@ export async function upsertExternalResource( externalCreatedAt: args.snapshot.externalCreatedAt ?? undefined, externalUpdatedAt: args.snapshot.externalUpdatedAt ?? undefined, lastSyncedAt: now, + syncAttemptedAt: now, + syncRetryAt: checksDiagnostic ? checksRetryAt : null, + syncFailureCount: checksDiagnostic ? (existing?.syncFailureCount ?? 0) + 1 : 0, + syncLastError: checksDiagnostic, + syncTerminalAt: terminal ? now : null, }, }); return row; @@ -601,13 +675,20 @@ export async function syncGitHubExternalResource(args: { }, }, }); - if (!existing || !existing.connectionMapping) { + if (!existing) { throw new TRPCError({ code: "NOT_FOUND", - message: "Linked GitHub resource with mapping not found.", + message: "Linked GitHub resource not found.", }); } - const mapping = existing.connectionMapping as GitHubMappingWithConnection; + const mapping = + existing.connectionMapping?.status === "active" + ? (existing.connectionMapping as GitHubMappingWithConnection) + : await resolveGitHubRepoMapping({ + db: args.db, + workspaceId: args.workspaceId, + repoFullName: existing.repoFullName, + }); const snapshot = await fetchGitHubSnapshot({ mapping, repoFullName: existing.repoFullName, diff --git a/src/server/services/github/webhook.ts b/src/server/services/github/webhook.ts index e14c3e0e..cac236cf 100644 --- a/src/server/services/github/webhook.ts +++ b/src/server/services/github/webhook.ts @@ -39,11 +39,13 @@ type GitHubWebhookPayload = { pull_request?: GitHubPullResponse; review?: { state?: string | null }; check_suite?: { + head_sha?: string | null; conclusion?: string | null; status?: string | null; pull_requests?: Array<{ number?: number | null; url?: string | null }>; }; check_run?: { + head_sha?: string | null; conclusion?: string | null; status?: string | null; pull_requests?: Array<{ number?: number | null; url?: string | null }>; @@ -384,7 +386,7 @@ async function processCheckEvent(args: { resourceType: "PULL_REQUEST", number: { in: numbers }, }, - select: { id: true, number: true, metadata: true }, + select: { id: true, number: true, state: true, metadata: true }, }); if (resources.length === 0) return 0; @@ -401,6 +403,14 @@ async function processCheckEvent(args: { metadata.checks && typeof metadata.checks === "object" && !Array.isArray(metadata.checks) ? (metadata.checks as Record) : {}; + const head = + metadata.head && typeof metadata.head === "object" && !Array.isArray(metadata.head) + ? (metadata.head as Record) + : {}; + const eventHeadSha = args.payload.check_suite?.head_sha ?? args.payload.check_run?.head_sha; + if (eventHeadSha && typeof head.sha === "string" && head.sha !== eventHeadSha) { + continue; + } // A check_suite conclusion is aggregate. A successful check_run only // proves one job passed, so it must not certify the whole PR; failures // remain safe to persist immediately. @@ -421,8 +431,19 @@ async function processCheckEvent(args: { updatedAt: new Date().toISOString(), event: args.payload.check_suite ? "check_suite" : "check_run", ...(args.payload.check_run ? { lastRunConclusion: conclusion } : {}), + ...(eventHeadSha ? { headSha: eventHeadSha } : {}), }, } as Prisma.InputJsonValue, + syncAttemptedAt: new Date(), + syncRetryAt: null, + syncFailureCount: 0, + syncLastError: null, + syncTerminalAt: + resource.state === "merged" && + aggregateConclusion !== null && + ["success", "neutral", "skipped"].includes(aggregateConclusion) + ? new Date() + : null, }, }); processed += 1; diff --git a/src/server/worker.ts b/src/server/worker.ts index 4d7e0ebc..379e6c95 100644 --- a/src/server/worker.ts +++ b/src/server/worker.ts @@ -29,6 +29,7 @@ import { ingestRunsDispatch } from "@/server/services/dispatch/run-dispatcher"; import { purgeExpiredSessionKeys } from "@/server/services/api-key-purge"; import { sweepIdleEphemeralAgents } from "@/server/services/ephemeral-idle"; import { sweepCompletionCandidates } from "@/server/services/completion-candidate"; +import { sweepGitHubStatusReconciliation } from "@/server/services/github/reconciliation"; import { logger } from "@/server/logger"; import { webhookQueue, maintenanceQueue } from "@/server/queues"; @@ -60,6 +61,8 @@ const EPHEMERAL_IDLE_SWEEP_INTERVAL_MS = 5 * 60_000; const EPHEMERAL_IDLE_SWEEP_JOB_ID = "ephemeral-idle-sweep"; const COMPLETION_CANDIDATE_SWEEP_INTERVAL_MS = 5 * 60_000; const COMPLETION_CANDIDATE_SWEEP_JOB_ID = "completion-candidate-sweep"; +const GITHUB_RECONCILIATION_SWEEP_INTERVAL_MS = 5 * 60_000; +const GITHUB_RECONCILIATION_SWEEP_JOB_ID = "github-reconciliation-sweep"; export { webhookQueue, maintenanceQueue }; export const webhookEvents = new QueueEvents("webhooks", { connection }); @@ -321,6 +324,9 @@ export const maintenanceWorker = new Worker( case "completion-candidate-sweep": { return sweepCompletionCandidates(db); } + case "github-reconciliation-sweep": { + return sweepGitHubStatusReconciliation(db); + } case "required-ack-check": { const eventId = job.data?.agentAssignedEventId as string | undefined; if (!eventId) return null; @@ -581,6 +587,20 @@ export async function registerCompletionCandidateSweepJob(): Promise { ); } +/** Webhook repair loop for stale native GitHub implementation links. */ +export async function registerGitHubReconciliationSweepJob(): Promise { + await maintenanceQueue.add( + "github-reconciliation-sweep", + {}, + { + jobId: GITHUB_RECONCILIATION_SWEEP_JOB_ID, + repeat: { every: GITHUB_RECONCILIATION_SWEEP_INTERVAL_MS }, + removeOnComplete: { age: 3600, count: 100 }, + removeOnFail: { age: 86_400, count: 50 }, + }, + ); +} + // Auto-register recurring jobs when this module loads (i.e. when // `pnpm worker` boots). Fire-and-forget — a Redis outage at boot should // not crash the worker; BullMQ will retry internally on the next op. @@ -620,6 +640,9 @@ void registerEphemeralIdleSweepJob().catch((err) => { void registerCompletionCandidateSweepJob().catch((err) => { logger.warn({ err }, "failed to register completion-candidate-sweep job"); }); +void registerGitHubReconciliationSweepJob().catch((err) => { + logger.warn({ err }, "failed to register github-reconciliation-sweep job"); +}); if (import.meta.url === `file://${process.argv[1]}`) { logger.info("workers running"); diff --git a/tests/unit/github-client.test.ts b/tests/unit/github-client.test.ts new file mode 100644 index 00000000..1cf4b972 --- /dev/null +++ b/tests/unit/github-client.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/server/services/github/installation-token", () => ({ + resolveInstallationToken: vi.fn().mockResolvedValue("installation-token"), +})); + +import { + getGitHubPullRequest, + getGitHubPullRequestChecks, + GitHubRequestError, +} from "@/server/services/github/client"; + +describe("GitHub REST client resilience", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("keeps partial check data unavailable instead of failing a readable PR", async () => { + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ message: "Checks permission required" }), { status: 403 }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ state: "success", total_count: 1 }), { status: 200 }), + ); + + const checks = await getGitHubPullRequestChecks({ + installationId: 1, + owner: "acme", + repo: "forge", + headSha: "abc123", + }); + + expect(checks).toMatchObject({ + status: "unknown", + conclusion: null, + partial: true, + statusCount: 1, + }); + expect(checks.diagnostic).toContain("Checks permission required"); + }); + + it("preserves GitHub rate-limit reset timing on request errors", async () => { + const reset = Math.floor(Date.now() / 1000) + 600; + vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( + new Response(JSON.stringify({ message: "API rate limit exceeded" }), { + status: 429, + headers: { "x-ratelimit-reset": String(reset) }, + }), + ); + + await expect( + getGitHubPullRequest({ installationId: 1, owner: "acme", repo: "forge", number: 42 }), + ).rejects.toMatchObject({ + name: "GitHubRequestError", + status: 429, + retryAt: new Date(reset * 1000), + } satisfies Partial); + }); +}); From b3d4c12441e0cb00907f0e4cf8b5b6bf731f9724 Mon Sep 17 00:00:00 2001 From: Bailey Dixon Date: Tue, 14 Jul 2026 17:27:45 -0400 Subject: [PATCH 2/2] fix: harden GitHub status reconciliation --- CHANGELOG.md | 4 +- DEVLOG.md | 37 ++- package.json | 2 +- .../migration.sql | 5 + prisma/schema.prisma | 8 + .../settings/github-reconciliation-policy.tsx | 59 ++++- src/server/routers/workspace.ts | 8 + .../__tests__/completion-candidate.test.ts | 4 +- .../__tests__/github-reconciliation.test.ts | 243 +++++++++++++++++- src/server/services/completion-candidate.ts | 4 +- src/server/services/github/client.ts | 219 ++++++++++++---- src/server/services/github/reconciliation.ts | 197 ++++++++++++-- src/server/services/github/resource-sync.ts | 83 +++++- src/server/services/github/webhook.ts | 47 ++-- tests/e2e/dashboard-layout.spec.ts | 10 +- tests/unit/github-check-conclusion.test.ts | 41 +-- tests/unit/github-client.test.ts | 153 +++++++++++ 17 files changed, 975 insertions(+), 149 deletions(-) create mode 100644 prisma/migrations/0103_github_reconciliation_safety/migration.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index b7c2fb56..6a9eed5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The dashboard's **What's New** rail reads this file at request time entries. Keep entries terse — one line per item under each version date, grouped by `Added` / `Changed` / `Fixed` / `Removed`. -## [Unreleased] +## [2026-07-14] — v0.15.0 · Reliable GitHub status recovery ### Added @@ -18,6 +18,8 @@ date, grouped by `Added` / `Changed` / `Fixed` / `Removed`. ### Fixed - **GitHub status recovery cannot certify stale or partial evidence.** New PR heads invalidate cached checks, late check events for older heads are ignored, partial GitHub App permissions remain non-blocking, and merged PRs without confirmed checks back off instead of polling hot or closing issues. +- **Check webhooks are refresh hints, not repository-wide success.** Forge now paginates every check suite, combines legacy commit status, distrusts incomplete or malformed aggregates, and only completion evidence read directly from GitHub can close work. +- **Provider failures cannot monopolize maintenance.** Configurable request timeouts, sweep budgets, manual-refresh cooldowns, dormant closed-PR rechecks, atomic leases, and mapping-wide backoff contain timeouts, permission failures, and rate limits while allowing other workspaces to continue. ## [2026-07-14] — v0.14.0 · Proactive completion handoff diff --git a/DEVLOG.md b/DEVLOG.md index f5638281..c4186529 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -14,17 +14,32 @@ initial/maximum backoff from Connections settings. Persisted provider attempt, retry, failure, diagnostic, and terminal state on `ExternalResource`. GitHub rate-limit headers override exponential retry; missing/paused mappings are re-resolved by repository; inaccessible resources -back off without being unlinked; closed PRs and confirmed-green merged PRs stop -polling; reopened PRs become eligible again. Partial Checks/status permissions -no longer prevent a readable PR from linking, but partial evidence cannot -certify completion. Head changes clear cached checks and late check webhooks for -an older head are ignored. Repeated unchanged refreshes produce no issue -activity or duplicate completion cards. - -Verification: Prisma migration 0102 applied locally; lint passed with existing -repository warnings only; typecheck passed; focused GitHub reconciliation, -client, check aggregation, completion, and head-invalidation coverage passed -(**13/13**); and `git diff --check` passed. +back off without being unlinked; closed PRs enter a slow configurable re-probe +cadence so a missed reopen event self-heals; and confirmed-green merged PRs stop +polling. Partial Checks/status permissions no longer prevent a readable PR from +linking, but partial evidence cannot certify completion. Head changes clear +cached checks and late check webhooks for an older head are ignored. Repeated +unchanged refreshes produce no issue activity or duplicate completion cards. + +The safety review removed webhook conclusions as aggregate completion evidence: +check-suite and check-run deliveries now mark a resource dirty, while the repair +loop paginates every check suite and combines legacy commit status before it can +trust success. Completed suites without conclusions, malformed aggregates, and +partially readable endpoints stay unresolved. Configurable request timeouts, +shared-worker wall budgets, atomic manual-refresh cooldowns, exact stale-row +claims, and persistent mapping-wide circuits contain permission failures, +timeouts, and rate limits without blocking later workspaces. + +Verification: Prisma migrations 0102 and 0103 applied locally; lint passed with +existing repository warnings only; typecheck passed; focused GitHub +reconciliation, client, check aggregation, completion, timeout, circuit-breaker, +dormant-reopen, cooldown, and lease-race coverage passed (**22/22**); the full +Vitest suite passed **1,259 tests** with one intentional live-connector skip; a +fresh production build passed; and the full serial Playwright suite passed +**38/38**. The browser gate also exposed a stale dashboard assertion that +hard-coded two rail columns despite the existing work-count threshold; the +assertion now follows the rendered layout mode and passed both alone and in the +full suite. ## 2026-07-13 — AXI-102 patient-wait spam + expandable Command Center cards diff --git a/package.json b/package.json index c43a6f4b..a62f3b0a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "forge", - "version": "0.14.0", + "version": "0.15.0", "private": true, "description": "Forge — a fast, minimalist, keyboard-driven project management platform with pluggable agents.", "license": "MIT", diff --git a/prisma/migrations/0103_github_reconciliation_safety/migration.sql b/prisma/migrations/0103_github_reconciliation_safety/migration.sql new file mode 100644 index 00000000..d38a073e --- /dev/null +++ b/prisma/migrations/0103_github_reconciliation_safety/migration.sql @@ -0,0 +1,5 @@ +ALTER TABLE "Workspace" + ADD COLUMN "githubRequestTimeoutSeconds" INTEGER NOT NULL DEFAULT 10, + ADD COLUMN "githubSweepBudgetSeconds" INTEGER NOT NULL DEFAULT 45, + ADD COLUMN "githubClosedReprobeMinutes" INTEGER NOT NULL DEFAULT 1440, + ADD COLUMN "githubManualCooldownSeconds" INTEGER NOT NULL DEFAULT 30; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index da3d1999..9ca4077c 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -900,6 +900,14 @@ model Workspace { githubSyncBackoffMinutes Int @default(5) /// Maximum retry delay for repeated provider failures. githubSyncMaxBackoffMinutes Int @default(1440) + /// Per-request timeout for GitHub REST calls made by the repair loop. + githubRequestTimeoutSeconds Int @default(10) + /// Maximum wall time a GitHub repair sweep may occupy the shared worker. + githubSweepBudgetSeconds Int @default(45) + /// Slow re-probe cadence for closed-unmerged PRs so missed reopen events heal. + githubClosedReprobeMinutes Int @default(1440) + /// Minimum interval between operator/agent manual refresh requests per PR. + githubManualCooldownSeconds Int @default(30) /// Issues with `updatedAt` older than this threshold (in days) are /// considered "stalled" by the dashboard suggestions bucket. Settings- /// driven so admins can tune the workspace's idle definition without diff --git a/src/components/settings/github-reconciliation-policy.tsx b/src/components/settings/github-reconciliation-policy.tsx index 65293b00..63ce3ba7 100644 --- a/src/components/settings/github-reconciliation-policy.tsx +++ b/src/components/settings/github-reconciliation-policy.tsx @@ -17,6 +17,10 @@ export function GitHubReconciliationPolicy({ isAdmin }: { isAdmin: boolean }) { batchSize: 25, backoffMinutes: 5, maxBackoffMinutes: 1440, + requestTimeoutSeconds: 10, + sweepBudgetSeconds: 45, + closedReprobeMinutes: 1440, + manualCooldownSeconds: 30, }); useEffect(() => { @@ -27,6 +31,10 @@ export function GitHubReconciliationPolicy({ isAdmin }: { isAdmin: boolean }) { batchSize: workspace.githubSyncBatchSize, backoffMinutes: workspace.githubSyncBackoffMinutes, maxBackoffMinutes: workspace.githubSyncMaxBackoffMinutes, + requestTimeoutSeconds: workspace.githubRequestTimeoutSeconds, + sweepBudgetSeconds: workspace.githubSweepBudgetSeconds, + closedReprobeMinutes: workspace.githubClosedReprobeMinutes, + manualCooldownSeconds: workspace.githubManualCooldownSeconds, }); }, [workspace]); @@ -46,7 +54,7 @@ export function GitHubReconciliationPolicy({ isAdmin }: { isAdmin: boolean }) { setPolicy((value) => ({ ...value, enabled }))} /> @@ -86,6 +94,51 @@ export function GitHubReconciliationPolicy({ isAdmin }: { isAdmin: boolean }) { } /> +
+ Safety limits +
+ + setPolicy((value) => ({ ...value, requestTimeoutSeconds })) + } + /> + + setPolicy((value) => ({ ...value, sweepBudgetSeconds })) + } + /> + + setPolicy((value) => ({ ...value, closedReprobeMinutes })) + } + /> + + setPolicy((value) => ({ ...value, manualCooldownSeconds })) + } + /> +
+

Rate-limit reset headers override retry timing. Inaccessible PRs back off instead of @@ -102,6 +155,10 @@ export function GitHubReconciliationPolicy({ isAdmin }: { isAdmin: boolean }) { githubSyncBatchSize: policy.batchSize, githubSyncBackoffMinutes: policy.backoffMinutes, githubSyncMaxBackoffMinutes: policy.maxBackoffMinutes, + githubRequestTimeoutSeconds: policy.requestTimeoutSeconds, + githubSweepBudgetSeconds: policy.sweepBudgetSeconds, + githubClosedReprobeMinutes: policy.closedReprobeMinutes, + githubManualCooldownSeconds: policy.manualCooldownSeconds, }) } > diff --git a/src/server/routers/workspace.ts b/src/server/routers/workspace.ts index a537fdcc..c85efc57 100644 --- a/src/server/routers/workspace.ts +++ b/src/server/routers/workspace.ts @@ -143,6 +143,10 @@ export const workspaceRouter = router({ githubSyncBatchSize: true, githubSyncBackoffMinutes: true, githubSyncMaxBackoffMinutes: true, + githubRequestTimeoutSeconds: true, + githubSweepBudgetSeconds: true, + githubClosedReprobeMinutes: true, + githubManualCooldownSeconds: true, stalledThresholdDays: true, agentHeartbeatWarnMinutes: true, agentHeartbeatCriticalMinutes: true, @@ -317,6 +321,10 @@ export const workspaceRouter = router({ githubSyncBatchSize: z.number().int().min(0).max(250).optional(), githubSyncBackoffMinutes: z.number().int().min(1).max(1440).optional(), githubSyncMaxBackoffMinutes: z.number().int().min(1).max(10080).optional(), + githubRequestTimeoutSeconds: z.number().int().min(1).max(60).optional(), + githubSweepBudgetSeconds: z.number().int().min(5).max(300).optional(), + githubClosedReprobeMinutes: z.number().int().min(60).max(43200).optional(), + githubManualCooldownSeconds: z.number().int().min(1).max(3600).optional(), assignmentEngagementMode: z.nativeEnum(EngagementMode).optional(), mentionEngagementPolicy: z.nativeEnum(MentionEngagementPolicy).optional(), mentionDefaultMode: z.nativeEnum(EngagementMode).optional(), diff --git a/src/server/services/__tests__/completion-candidate.test.ts b/src/server/services/__tests__/completion-candidate.test.ts index 89e02d59..e75d3d91 100644 --- a/src/server/services/__tests__/completion-candidate.test.ts +++ b/src/server/services/__tests__/completion-candidate.test.ts @@ -61,7 +61,9 @@ async function linkedPullRequest( url: "https://github.com/acme/forge/pull/42", title: "Complete issue", state, - metadata: conclusion ? { checks: { status: "completed", conclusion } } : {}, + metadata: conclusion + ? { checks: { status: "completed", conclusion, source: "api-aggregate" } } + : {}, }, }); await prisma.externalResourceLink.create({ diff --git a/src/server/services/__tests__/github-reconciliation.test.ts b/src/server/services/__tests__/github-reconciliation.test.ts index 5c8a1b1f..a5074aa9 100644 --- a/src/server/services/__tests__/github-reconciliation.test.ts +++ b/src/server/services/__tests__/github-reconciliation.test.ts @@ -1,7 +1,13 @@ import { afterAll, afterEach, describe, expect, it } from "vitest"; import { GitHubRequestError } from "@/server/services/github/client"; -import { upsertExternalResource } from "@/server/services/github/resource-sync"; -import { sweepGitHubStatusReconciliation } from "@/server/services/github/reconciliation"; +import { + claimGitHubManualSync, + upsertExternalResource, +} from "@/server/services/github/resource-sync"; +import { + claimGitHubReconciliationCandidate, + sweepGitHubStatusReconciliation, +} from "@/server/services/github/reconciliation"; import { createIssue, createWorkspaceFixture, @@ -33,6 +39,27 @@ async function setupResource() { githubSyncBatchSize: 10, githubSyncBackoffMinutes: 5, githubSyncMaxBackoffMinutes: 1440, + githubRequestTimeoutSeconds: 10, + githubSweepBudgetSeconds: 45, + githubClosedReprobeMinutes: 1440, + githubManualCooldownSeconds: 30, + }, + }); + const connection = await prisma.connection.create({ + data: { + ownerId: fixture.user.id, + provider: "GITHUB", + label: "GitHub test", + status: "CONNECTED", + config: { installationId: "1" }, + }, + }); + const mapping = await prisma.connectionMapping.create({ + data: { + workspaceId: fixture.workspace.id, + connectionId: connection.id, + kind: "repo", + target: "acme/forge", }, }); const resource = await prisma.externalResource.create({ @@ -45,6 +72,7 @@ async function setupResource() { url: "https://github.com/acme/forge/pull/42", title: "Ship it", state: "open", + connectionMappingId: mapping.id, lastSyncedAt: new Date("2026-07-14T10:00:00.000Z"), }, }); @@ -57,7 +85,7 @@ async function setupResource() { createdById: fixture.user.id, }, }); - return { fixture, prisma, resource }; + return { fixture, prisma, resource, mapping }; } describe("GitHub status reconciliation", () => { @@ -259,4 +287,213 @@ describe("GitHub status reconciliation", () => { ).toMatchObject({ inspected: 0 }); expect(calls).toBe(0); }); + + it("slowly re-probes closed PRs so a missed reopen webhook self-heals", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { + state: "closed", + syncTerminalAt: new Date("2026-07-12T10:00:00.000Z"), + }, + }); + const syncResource = async () => + prisma.externalResource.update({ + where: { id: resource.id }, + data: { + state: "open", + syncTerminalAt: null, + lastSyncedAt: now, + syncRetryAt: null, + metadata: { + checks: { + source: "api-aggregate", + status: "pending", + conclusion: null, + partial: false, + }, + }, + }, + }); + + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).resolves.toMatchObject({ inspected: 1, reconciled: 1 }); + expect( + (await prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } })).state, + ).toBe("open"); + }); + + it("backs off partial open PR snapshots and opens a rate-limit circuit", async () => { + const { fixture, prisma, resource, mapping } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const retryAt = new Date("2026-07-14T13:00:00.000Z"); + const sibling = await prisma.externalResource.create({ + data: { + workspaceId: fixture.workspace.id, + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 99, + url: "https://github.com/acme/forge/pull/99", + title: "Same installation", + state: "open", + connectionMappingId: mapping.id, + }, + }); + const syncResource = async () => + prisma.externalResource.update({ + where: { id: resource.id }, + data: { + lastSyncedAt: now, + syncRetryAt: null, + syncFailureCount: 0, + metadata: { + checks: { + source: "api-aggregate", + status: "unknown", + conclusion: null, + partial: true, + rateLimited: true, + retryAt: retryAt.toISOString(), + diagnostic: "API rate limit exceeded", + }, + }, + }, + }); + + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now, + syncResource, + }), + ).resolves.toMatchObject({ + reconciled: 1, + partial: 1, + rateLimited: 1, + circuitBroken: true, + }); + const row = await prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }); + expect(row.syncRetryAt).toEqual(retryAt); + expect(row.syncFailureCount).toBe(1); + expect( + (await prisma.externalResource.findUniqueOrThrow({ where: { id: sibling.id } })).syncRetryAt, + ).toEqual(retryAt); + }); + + it("rechecks snapshot freshness in the lease claim to close manual-sync races", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const candidate = await prisma.externalResource.findUniqueOrThrow({ + where: { id: resource.id }, + }); + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { lastSyncedAt: now }, + }); + + await expect( + claimGitHubReconciliationCandidate({ + db: prisma, + workspaceId: fixture.workspace.id, + candidate, + now, + staleBefore: new Date("2026-07-14T11:45:00.000Z"), + dormantBefore: new Date("2026-07-13T12:00:00.000Z"), + leaseUntil: new Date("2026-07-14T12:01:00.000Z"), + }), + ).resolves.toBe(false); + }); + + it("limits manual refreshes with one atomic cooldown/lease claim", async () => { + const { fixture, prisma, resource } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const input = { + db: prisma, + workspaceId: fixture.workspace.id, + externalResourceId: resource.id, + now, + cooldownSeconds: 30, + leaseUntil: new Date("2026-07-14T12:00:10.000Z"), + }; + await expect(claimGitHubManualSync(input)).resolves.toBe(true); + await expect(claimGitHubManualSync(input)).resolves.toBe(false); + await expect( + claimGitHubManualSync({ + ...input, + now: new Date("2026-07-14T12:00:31.000Z"), + leaseUntil: new Date("2026-07-14T12:00:41.000Z"), + }), + ).resolves.toBe(true); + }); + + it("stops starting resources after the workspace sweep budget is exhausted", async () => { + const { fixture, prisma, resource } = await setupResource(); + const issue = await createIssue(fixture, { statusCategory: "IN_PROGRESS" }); + const second = await prisma.externalResource.create({ + data: { + workspaceId: fixture.workspace.id, + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 43, + url: "https://github.com/acme/forge/pull/43", + title: "Second PR", + state: "open", + lastSyncedAt: new Date("2026-07-14T10:00:00.000Z"), + }, + }); + await prisma.externalResourceLink.create({ + data: { + workspaceId: fixture.workspace.id, + issueId: issue.id, + externalResourceId: second.id, + kind: "IMPLEMENTS", + }, + }); + let clockMs = 0; + let calls = 0; + const syncResource = async ({ externalResourceId }: { externalResourceId: string }) => { + calls += 1; + clockMs += 50_000; + return prisma.externalResource.update({ + where: { id: externalResourceId }, + data: { + lastSyncedAt: new Date("2026-07-14T12:00:00.000Z"), + syncRetryAt: null, + metadata: { + checks: { + source: "api-aggregate", + status: "pending", + conclusion: null, + partial: false, + }, + }, + }, + }); + }; + + await expect( + sweepGitHubStatusReconciliation(prisma, { + workspaceId: fixture.workspace.id, + now: new Date("2026-07-14T12:00:00.000Z"), + clock: () => clockMs, + syncResource: syncResource as never, + }), + ).resolves.toMatchObject({ budgetExhausted: true, reconciled: 1 }); + expect(calls).toBe(1); + const rows = await prisma.externalResource.findMany({ + where: { id: { in: [resource.id, second.id] } }, + select: { lastSyncedAt: true }, + }); + expect( + rows.filter((row) => row.lastSyncedAt?.getTime() === Date.parse("2026-07-14T12:00:00.000Z")), + ).toHaveLength(1); + }); }); diff --git a/src/server/services/completion-candidate.ts b/src/server/services/completion-candidate.ts index 81a374e5..40abd7bc 100644 --- a/src/server/services/completion-candidate.ts +++ b/src/server/services/completion-candidate.ts @@ -43,7 +43,9 @@ function metadataRecord(value: unknown): Record { function checkConclusion(metadata: unknown): string | null { const checks = metadataRecord(metadataRecord(metadata).checks); - return typeof checks.conclusion === "string" ? checks.conclusion : null; + return checks.source === "api-aggregate" && typeof checks.conclusion === "string" + ? checks.conclusion + : null; } function checksArePassing(conclusion: string | null): boolean { diff --git a/src/server/services/github/client.ts b/src/server/services/github/client.ts index 735e484b..a182d00e 100644 --- a/src/server/services/github/client.ts +++ b/src/server/services/github/client.ts @@ -4,6 +4,8 @@ import { resolveInstallationToken } from "@/server/services/github/installation- import type { GitHubResourceSnapshot } from "@/server/services/github/types"; const GITHUB_API_BASE = "https://api.github.com"; +const DEFAULT_GITHUB_REQUEST_TIMEOUT_MS = 10_000; +const MAX_CHECK_SUITE_PAGES = 10; type GitHubUser = { login?: string | null }; type GitHubLabel = { name?: string | null; color?: string | null }; @@ -83,8 +85,10 @@ export type GitHubChecksSnapshot = { suiteCount: number; statusCount: number; updatedAt: string; - source: "reconciliation"; + source: "api-aggregate"; partial: boolean; + rateLimited: boolean; + timedOut: boolean; diagnostic: string | null; retryAt: string | null; headSha: string; @@ -96,6 +100,7 @@ export class GitHubRequestError extends Error { readonly status: number, readonly retryAt: Date | null, readonly rateLimited: boolean = status === 429, + readonly timedOut: boolean = status === 408, ) { super(message); this.name = "GitHubRequestError"; @@ -110,16 +115,48 @@ export type GitHubInstallationResponse = { events?: string[]; }; -async function githubAppRequest(path: string): Promise { +function requestSignal(signal: AbortSignal | null | undefined, timeoutMs: number): AbortSignal { + const timeout = AbortSignal.timeout(Math.max(1, timeoutMs)); + return signal ? AbortSignal.any([signal, timeout]) : timeout; +} + +function timeoutError(timeoutMs: number): GitHubRequestError { + return new GitHubRequestError( + `GitHub API request timed out after ${timeoutMs}ms.`, + 408, + null, + false, + true, + ); +} + +async function githubAppRequest( + path: string, + timeoutMs = DEFAULT_GITHUB_REQUEST_TIMEOUT_MS, +): Promise { const jwt = await createGitHubAppJwt(); - const res = await fetch(`${GITHUB_API_BASE}${path}`, { - headers: { - accept: "application/vnd.github+json", - authorization: `Bearer ${jwt}`, - "user-agent": "forge-github-app", - "x-github-api-version": "2022-11-28", - }, - }); + let res: Response; + try { + res = await fetch(`${GITHUB_API_BASE}${path}`, { + signal: requestSignal(null, timeoutMs), + headers: { + accept: "application/vnd.github+json", + authorization: `Bearer ${jwt}`, + "user-agent": "forge-github-app", + "x-github-api-version": "2022-11-28", + }, + }); + } catch (error) { + if ( + error && + typeof error === "object" && + "name" in error && + (error.name === "TimeoutError" || error.name === "AbortError") + ) { + throw timeoutError(timeoutMs); + } + throw error; + } const json = (await res.json().catch(() => ({}))) as { message?: string }; if (!res.ok) { throw new Error(json.message || `GitHub API returned HTTP ${res.status}.`); @@ -131,21 +168,36 @@ async function githubRequest( installationId: string | number, path: string, init: RequestInit = {}, + timeoutMs = DEFAULT_GITHUB_REQUEST_TIMEOUT_MS, ): Promise { // Prefer a configured GithubApp's credentials for this installation, falling // back to the global env app — so linking works off the same app a workspace // set up in Settings → GitHub Apps. const token = await resolveInstallationToken(installationId); - const res = await fetch(`${GITHUB_API_BASE}${path}`, { - ...init, - headers: { - accept: "application/vnd.github+json", - authorization: `Bearer ${token}`, - "user-agent": "forge-github-app", - "x-github-api-version": "2022-11-28", - ...(init.headers ?? {}), - }, - }); + let res: Response; + try { + res = await fetch(`${GITHUB_API_BASE}${path}`, { + ...init, + signal: requestSignal(init.signal, timeoutMs), + headers: { + accept: "application/vnd.github+json", + authorization: `Bearer ${token}`, + "user-agent": "forge-github-app", + "x-github-api-version": "2022-11-28", + ...(init.headers ?? {}), + }, + }); + } catch (error) { + if ( + error && + typeof error === "object" && + "name" in error && + (error.name === "TimeoutError" || error.name === "AbortError") + ) { + throw timeoutError(timeoutMs); + } + throw error; + } const text = await res.text(); let json: unknown = {}; if (text) { @@ -172,7 +224,7 @@ async function githubRequest( res.status === 429 || (res.status === 403 && (res.headers.has("retry-after") || res.headers.get("x-ratelimit-remaining") === "0")); - throw new GitHubRequestError(message, res.status, retryAt, rateLimited); + throw new GitHubRequestError(message, res.status, retryAt, rateLimited, false); } return json as T; } @@ -198,10 +250,14 @@ export async function getGitHubIssue(args: { owner: string; repo: string; number: number; + requestTimeoutMs?: number; + signal?: AbortSignal; }): Promise { return githubRequest( args.installationId, `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/issues/${args.number}`, + { signal: args.signal }, + args.requestTimeoutMs, ); } @@ -210,10 +266,14 @@ export async function getGitHubPullRequest(args: { owner: string; repo: string; number: number; + requestTimeoutMs?: number; + signal?: AbortSignal; }): Promise { return githubRequest( args.installationId, `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/pulls/${args.number}`, + { signal: args.signal }, + args.requestTimeoutMs, ); } @@ -227,26 +287,30 @@ export async function getGitHubPullRequestChecks(args: { owner: string; repo: string; headSha: string; + requestTimeoutMs?: number; + signal?: AbortSignal; }): Promise { const ref = encodeURIComponent(args.headSha); - const [suiteResult, statusResult] = await Promise.allSettled([ - githubRequest( - args.installationId, - `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/commits/${ref}/check-suites?filter=latest&per_page=100`, - ), + const suiteResult = await settle(() => listAllCheckSuites({ ...args, ref })); + // Deliberately serial: GitHub recommends avoiding concurrent REST requests + // to reduce secondary-rate-limit pressure. + const statusResult = await settle(() => githubRequest( args.installationId, `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/commits/${ref}/status`, + { signal: args.signal }, + args.requestTimeoutMs, ), - ]); - const suites = suiteResult.status === "fulfilled" ? suiteResult.value : {}; - const statuses = statusResult.status === "fulfilled" ? statusResult.value : {}; + ); + const suites: GitHubCheckSuitesResponse & { truncated?: boolean } = + suiteResult.status === "fulfilled" ? suiteResult.value : {}; + const statuses: GitHubCombinedStatusResponse = + statusResult.status === "fulfilled" ? statusResult.value : {}; const rows = suites.check_suites ?? []; const suiteCount = suites.total_count ?? rows.length; const statusCount = statuses.total_count ?? 0; const discovered = suiteCount + statusCount; - const pending = - rows.some((suite) => suite.status !== "completed") || statuses.state === "pending"; + const pending = rows.some((suite) => suite.status !== "completed"); const failedSuite = rows.find( (suite) => suite.status === "completed" && @@ -257,10 +321,16 @@ export async function getGitHubPullRequestChecks(args: { const conclusion = failedSuite?.conclusion ?? (failedStatus ? (statuses.state ?? "failure") : null); const failures = [suiteResult, statusResult].filter( - (result): result is PromiseRejectedResult => result.status === "rejected", + (result): result is { status: "rejected"; reason: unknown } => result.status === "rejected", ); - const incompleteSuites = suiteCount > rows.length; + const incompleteSuites = suiteResult.status === "fulfilled" && suiteResult.value.truncated; const partial = failures.length > 0 || incompleteSuites; + const rateLimited = failures.some( + (failure) => failure.reason instanceof GitHubRequestError && failure.reason.rateLimited, + ); + const timedOut = failures.some( + (failure) => failure.reason instanceof GitHubRequestError && failure.reason.timedOut, + ); const retryAt = failures .map((failure) => @@ -269,36 +339,91 @@ export async function getGitHubPullRequestChecks(args: { .filter((value): value is Date => value instanceof Date) .sort((a, b) => b.getTime() - a.getTime())[0] ?? null; + const nullConclusion = rows.some( + (suite) => suite.status === "completed" && suite.conclusion == null, + ); + const statusPending = statusCount > 0 && statuses.state === "pending"; + const statusUnknown = statusCount > 0 && statuses.state == null; + const unresolved = pending || nullConclusion || statusPending || statusUnknown; + return { status: conclusion !== null ? "completed" - : partial || discovered === 0 + : partial || discovered === 0 || nullConclusion || statusUnknown ? "unknown" - : pending + : unresolved ? "pending" : "completed", - conclusion: conclusion ?? (!partial && discovered > 0 && !pending ? "success" : null), + conclusion: conclusion ?? (!partial && discovered > 0 && !unresolved ? "success" : null), suiteCount, statusCount, updatedAt: new Date().toISOString(), - source: "reconciliation", + source: "api-aggregate", partial, - diagnostic: partial - ? [ - ...failures.map((failure) => - failure.reason instanceof Error ? failure.reason.message : "GitHub checks unavailable", - ), - ...(incompleteSuites ? ["More than 100 check suites require another page."] : []), - ] - .join("; ") - .slice(0, 2_000) - : null, + rateLimited, + timedOut, + diagnostic: + partial || nullConclusion || statusUnknown + ? [ + ...failures.map((failure) => + failure.reason instanceof Error + ? failure.reason.message + : "GitHub checks unavailable", + ), + ...(incompleteSuites + ? [`More than ${MAX_CHECK_SUITE_PAGES * 100} check suites require another page.`] + : []), + ...(nullConclusion ? ["A completed check suite has no conclusion."] : []), + ...(statusUnknown + ? ["GitHub returned commit statuses without an aggregate state."] + : []), + ] + .join("; ") + .slice(0, 2_000) + : null, retryAt: retryAt?.toISOString() ?? null, headSha: args.headSha, }; } +type Settled = { status: "fulfilled"; value: T } | { status: "rejected"; reason: unknown }; + +async function settle(fn: () => Promise): Promise> { + try { + return { status: "fulfilled", value: await fn() }; + } catch (reason) { + return { status: "rejected", reason }; + } +} + +async function listAllCheckSuites(args: { + installationId: string | number; + owner: string; + repo: string; + ref: string; + requestTimeoutMs?: number; + signal?: AbortSignal; +}): Promise { + const check_suites: GitHubCheckSuite[] = []; + let total = 0; + for (let page = 1; page <= MAX_CHECK_SUITE_PAGES; page += 1) { + const response = await githubRequest( + args.installationId, + `/repos/${encodeURIComponent(args.owner)}/${encodeURIComponent(args.repo)}/commits/${args.ref}/check-suites?per_page=100&page=${page}`, + { signal: args.signal }, + args.requestTimeoutMs, + ); + const rows = response.check_suites ?? []; + check_suites.push(...rows); + total = response.total_count ?? check_suites.length; + if (check_suites.length >= total || rows.length < 100) { + return { total_count: total, check_suites, truncated: false }; + } + } + return { total_count: total, check_suites, truncated: check_suites.length < total }; +} + export async function listGitHubInstallationRepos(args: { installationId: string | number; }): Promise { diff --git a/src/server/services/github/reconciliation.ts b/src/server/services/github/reconciliation.ts index 8936a2c5..86df0e2a 100644 --- a/src/server/services/github/reconciliation.ts +++ b/src/server/services/github/reconciliation.ts @@ -15,6 +15,9 @@ export type GitHubReconciliationResult = { failed: number; rateLimited: number; skipped: number; + partial: number; + budgetExhausted: boolean; + circuitBroken: boolean; }; type SyncResource = typeof syncGitHubExternalResource; @@ -49,6 +52,53 @@ function checksMetadata(value: unknown): Record { : {}; } +type ReconciliationCandidate = { + id: string; + state: string; + lastSyncedAt: Date | null; + syncTerminalAt: Date | null; + syncFailureCount: number; + connectionMappingId: string | null; +}; + +export async function claimGitHubReconciliationCandidate(args: { + db: PrismaClient; + workspaceId: string; + candidate: ReconciliationCandidate; + now: Date; + staleBefore: Date; + dormantBefore: Date; + leaseUntil: Date; +}): Promise { + const candidateEligibility = args.candidate.syncTerminalAt + ? { + state: "closed", + syncTerminalAt: args.candidate.syncTerminalAt, + AND: [{ syncTerminalAt: { lte: args.dormantBefore } }], + } + : { + syncTerminalAt: null, + lastSyncedAt: args.candidate.lastSyncedAt, + AND: [ + { + OR: [{ lastSyncedAt: null }, { lastSyncedAt: { lte: args.staleBefore } }], + }, + ], + }; + const claimed = await args.db.externalResource.updateMany({ + where: { + id: args.candidate.id, + workspaceId: args.workspaceId, + provider: PROVIDER, + resourceType: RESOURCE_TYPE, + ...candidateEligibility, + OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: args.now } }], + }, + data: { syncAttemptedAt: args.now, syncRetryAt: args.leaseUntil }, + }); + return claimed.count === 1; +} + /** * Reconcile stale native IMPLEMENTS links. GitHub webhooks stay primary; this * sweep is a bounded repair loop for missed delivery, permission recovery, and @@ -61,10 +111,13 @@ export async function sweepGitHubStatusReconciliation( workspaceId?: string; now?: Date; syncResource?: SyncResource; + clock?: () => number; } = {}, ): Promise { const now = options.now ?? new Date(); const syncResource = options.syncResource ?? syncGitHubExternalResource; + const clock = options.clock ?? Date.now; + const sweepStartedMs = clock(); const result: GitHubReconciliationResult = { workspaces: 0, inspected: 0, @@ -72,6 +125,9 @@ export async function sweepGitHubStatusReconciliation( failed: 0, rateLimited: 0, skipped: 0, + partial: 0, + budgetExhausted: false, + circuitBroken: false, }; const workspaces = await db.workspace.findMany({ where: { @@ -85,22 +141,47 @@ export async function sweepGitHubStatusReconciliation( githubSyncBatchSize: true, githubSyncBackoffMinutes: true, githubSyncMaxBackoffMinutes: true, + githubRequestTimeoutSeconds: true, + githubSweepBudgetSeconds: true, + githubClosedReprobeMinutes: true, }, }); + // The scheduled job spans workspaces, so it needs one deterministic hard + // ceiling as well as each workspace's local budget. The largest configured + // budget preserves an administrator's explicit allowance without letting + // the job consume the sum of every tenant's allowance. + const globalDeadlineMs = + sweepStartedMs + + Math.max(0, ...workspaces.map((workspace) => workspace.githubSweepBudgetSeconds)) * 1000; + let globalBudgetExhausted = false; for (const workspace of workspaces) { + let workspaceCircuitBroken = false; result.workspaces += 1; if (workspace.githubSyncBatchSize <= 0 || workspace.githubSyncStaleMinutes <= 0) { continue; } const staleBefore = new Date(now.getTime() - workspace.githubSyncStaleMinutes * 60_000); + const dormantBefore = new Date(now.getTime() - workspace.githubClosedReprobeMinutes * 60_000); + const workspaceDeadlineMs = Math.min( + globalDeadlineMs, + clock() + workspace.githubSweepBudgetSeconds * 1000, + ); const candidates = await db.externalResource.findMany({ where: { workspaceId: workspace.id, provider: PROVIDER, resourceType: RESOURCE_TYPE, - syncTerminalAt: null, - OR: [{ lastSyncedAt: null }, { lastSyncedAt: { lte: staleBefore } }], + OR: [ + { + syncTerminalAt: null, + OR: [{ lastSyncedAt: null }, { lastSyncedAt: { lte: staleBefore } }], + }, + { + state: "closed", + syncTerminalAt: { lte: dormantBefore }, + }, + ], AND: [{ OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: now } }] }], links: { some: { @@ -110,24 +191,39 @@ export async function sweepGitHubStatusReconciliation( }, }, }, - select: { id: true, syncFailureCount: true }, - orderBy: [{ lastSyncedAt: "asc" }, { id: "asc" }], + select: { + id: true, + state: true, + lastSyncedAt: true, + syncTerminalAt: true, + syncFailureCount: true, + connectionMappingId: true, + }, + orderBy: [{ syncTerminalAt: "asc" }, { lastSyncedAt: "asc" }, { id: "asc" }], take: workspace.githubSyncBatchSize, }); for (const candidate of candidates) { + const currentMs = clock(); + const remainingMs = workspaceDeadlineMs - currentMs; + if (remainingMs <= 0) { + result.budgetExhausted = true; + globalBudgetExhausted = currentMs >= globalDeadlineMs; + break; + } result.inspected += 1; - const leaseUntil = new Date(now.getTime() + LEASE_MS); - const claimed = await db.externalResource.updateMany({ - where: { - id: candidate.id, - workspaceId: workspace.id, - syncTerminalAt: null, - OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: now } }], - }, - data: { syncAttemptedAt: now, syncRetryAt: leaseUntil }, + const leaseMs = Math.max(LEASE_MS, workspace.githubSweepBudgetSeconds * 1000 + 5_000); + const leaseUntil = new Date(now.getTime() + leaseMs); + const claimed = await claimGitHubReconciliationCandidate({ + db, + workspaceId: workspace.id, + candidate, + now, + staleBefore, + dormantBefore, + leaseUntil, }); - if (claimed.count === 0) { + if (!claimed) { result.skipped += 1; continue; } @@ -138,18 +234,23 @@ export async function sweepGitHubStatusReconciliation( workspaceId: workspace.id, externalResourceId: candidate.id, actor: { actorId: null }, + skipCollisionGuard: true, + signal: AbortSignal.timeout(Math.max(1, remainingMs)), }); const checks = checksMetadata(resource.metadata); - // A merged PR with zero/partially-readable checks must remain held, but - // exponential retry keeps repositories with no CI from being polled at - // the normal stale cadence forever. Provider retry headers win. - if (resource.state === "merged" && checks.status === "unknown") { + const untrustedChecks = + checks.source !== "api-aggregate" || + checks.status === "unknown" || + checks.partial === true; + // Unknown/partial checks on any PR state use exponential/provider + // backoff. This includes open/draft resources with missing permissions. + if (untrustedChecks && resource.state !== "closed") { const failureCount = candidate.syncFailureCount + 1; const exponential = retryAtForFailure({ error: new Error( typeof checks.diagnostic === "string" ? checks.diagnostic - : "No aggregate GitHub checks are available for this merged PR.", + : "No trusted aggregate GitHub checks are available for this PR.", ), failureCount, baseMinutes: Math.max(1, workspace.githubSyncBackoffMinutes), @@ -158,21 +259,47 @@ export async function sweepGitHubStatusReconciliation( }); const providerRetryAt = typeof checks.retryAt === "string" ? new Date(checks.retryAt) : null; + const nextRetryAt = + providerRetryAt && + Number.isFinite(providerRetryAt.getTime()) && + providerRetryAt > exponential.retryAt + ? providerRetryAt + : exponential.retryAt; await db.externalResource.updateMany({ where: { id: candidate.id, workspaceId: workspace.id }, data: { syncFailureCount: failureCount, syncLastError: exponential.message, - syncRetryAt: - providerRetryAt && - Number.isFinite(providerRetryAt.getTime()) && - providerRetryAt > exponential.retryAt - ? providerRetryAt - : exponential.retryAt, + syncRetryAt: nextRetryAt, }, }); + result.partial += 1; + if (checks.rateLimited === true) { + result.rateLimited += 1; + } + if (checks.partial === true) { + // Permission, timeout, or rate-limit failures generally affect the + // installation/mapping, not one PR. Open a persisted circuit for + // every resource on that mapping before ending this sweep. + if (resource.connectionMappingId) { + await db.externalResource.updateMany({ + where: { + workspaceId: workspace.id, + provider: PROVIDER, + connectionMappingId: resource.connectionMappingId, + }, + data: { + syncRetryAt: nextRetryAt, + syncLastError: exponential.message, + }, + }); + } + result.circuitBroken = true; + workspaceCircuitBroken = true; + } } result.reconciled += 1; + if (workspaceCircuitBroken) break; } catch (error) { const failureCount = candidate.syncFailureCount + 1; const retry = retryAtForFailure({ @@ -193,6 +320,20 @@ export async function sweepGitHubStatusReconciliation( }); result.failed += 1; if (retry.rateLimited) result.rateLimited += 1; + const mappingWideFailure = + retry.rateLimited || + (error instanceof GitHubRequestError && + (error.timedOut || [401, 403].includes(error.status))); + if (mappingWideFailure && candidate.connectionMappingId) { + await db.externalResource.updateMany({ + where: { + workspaceId: workspace.id, + provider: PROVIDER, + connectionMappingId: candidate.connectionMappingId, + }, + data: { syncRetryAt: retry.retryAt, syncLastError: retry.message }, + }); + } logger.warn( { err: error, @@ -203,8 +344,14 @@ export async function sweepGitHubStatusReconciliation( }, "github-reconciliation: resource sync failed", ); + if (mappingWideFailure) { + result.circuitBroken = true; + workspaceCircuitBroken = true; + break; + } } } + if (globalBudgetExhausted) break; } if (result.inspected > 0 || result.failed > 0) { diff --git a/src/server/services/github/resource-sync.ts b/src/server/services/github/resource-sync.ts index 34e91c96..4da69105 100644 --- a/src/server/services/github/resource-sync.ts +++ b/src/server/services/github/resource-sync.ts @@ -119,6 +119,7 @@ export async function resolveGitHubRepoMapping(args: { export async function fetchGitHubSnapshotForParsed( parsed: ParsedGitHubUrl, mapping: GitHubMappingWithConnection, + options: { requestTimeoutMs?: number; signal?: AbortSignal } = {}, ): Promise { const installationId = githubInstallationId(mapping.connection); if (parsed.type === "PULL_REQUEST") { @@ -127,6 +128,8 @@ export async function fetchGitHubSnapshotForParsed( owner: parsed.owner, repo: parsed.repo, number: parsed.number, + requestTimeoutMs: options.requestTimeoutMs, + signal: options.signal, }); const snapshot = pullRequestSnapshot(parsed.repoFullName, pr); if (pr.head?.sha) { @@ -137,6 +140,8 @@ export async function fetchGitHubSnapshotForParsed( owner: parsed.owner, repo: parsed.repo, headSha: pr.head.sha, + requestTimeoutMs: options.requestTimeoutMs, + signal: options.signal, }), }; } @@ -147,6 +152,8 @@ export async function fetchGitHubSnapshotForParsed( owner: parsed.owner, repo: parsed.repo, number: parsed.number, + requestTimeoutMs: options.requestTimeoutMs, + signal: options.signal, }); if (issue.pull_request) { const pr = await getGitHubPullRequest({ @@ -154,6 +161,8 @@ export async function fetchGitHubSnapshotForParsed( owner: parsed.owner, repo: parsed.repo, number: parsed.number, + requestTimeoutMs: options.requestTimeoutMs, + signal: options.signal, }); const snapshot = pullRequestSnapshot(parsed.repoFullName, pr); if (pr.head?.sha) { @@ -164,6 +173,8 @@ export async function fetchGitHubSnapshotForParsed( owner: parsed.owner, repo: parsed.repo, headSha: pr.head.sha, + requestTimeoutMs: options.requestTimeoutMs, + signal: options.signal, }), }; } @@ -177,6 +188,8 @@ export async function fetchGitHubSnapshot(args: { repoFullName: string; resourceType: GitHubResourceType; number: number; + requestTimeoutMs?: number; + signal?: AbortSignal; }): Promise { const repo = splitRepoFullName(args.repoFullName); return fetchGitHubSnapshotForParsed( @@ -190,6 +203,7 @@ export async function fetchGitHubSnapshot(args: { : `https://github.com/${repo.repoFullName}/issues/${args.number}`, }, args.mapping, + { requestTimeoutMs: args.requestTimeoutMs, signal: args.signal }, ); } @@ -253,6 +267,7 @@ export async function upsertExternalResource( ? (metadata.checks as Record) : {}; const passingChecks = + checks.source === "api-aggregate" && checks.status === "completed" && typeof checks.conclusion === "string" && ["success", "neutral", "skipped"].includes(checks.conclusion); @@ -427,7 +442,13 @@ export async function linkGitHubUrlToIssue(args: { mappingId: args.mappingId, repoFullName: parsed.repoFullName, }); - const snapshot = await fetchGitHubSnapshotForParsed(parsed, mapping); + const workspace = await args.db.workspace.findUniqueOrThrow({ + where: { id: args.workspaceId }, + select: { githubRequestTimeoutSeconds: true }, + }); + const snapshot = await fetchGitHubSnapshotForParsed(parsed, mapping, { + requestTimeoutMs: workspace.githubRequestTimeoutSeconds * 1000, + }); const result = await args.db.$transaction(async (tx) => { const resource = await upsertExternalResource(tx, { workspaceId: args.workspaceId, @@ -503,13 +524,19 @@ export async function importGitHubIssue(args: { mappingId: args.mappingId, repoFullName, }); + const workspace = await args.db.workspace.findUniqueOrThrow({ + where: { id: args.workspaceId }, + select: { githubRequestTimeoutSeconds: true }, + }); + const requestTimeoutMs = workspace.githubRequestTimeoutSeconds * 1000; const snapshot = parsed - ? await fetchGitHubSnapshotForParsed(parsed, mapping) + ? await fetchGitHubSnapshotForParsed(parsed, mapping, { requestTimeoutMs }) : await fetchGitHubSnapshot({ mapping, repoFullName, resourceType: args.resourceType ?? "ISSUE", number, + requestTimeoutMs, }); const resource = await upsertExternalResource(args.db, { workspaceId: args.workspaceId, @@ -662,7 +689,35 @@ export async function syncGitHubExternalResource(args: { workspaceId: string; externalResourceId: string; actor: ActorMeta; + skipCollisionGuard?: boolean; + signal?: AbortSignal; }): Promise { + const workspace = await args.db.workspace.findFirst({ + where: { id: args.workspaceId, deletedAt: null }, + select: { + githubRequestTimeoutSeconds: true, + githubManualCooldownSeconds: true, + }, + }); + if (!workspace) throw new TRPCError({ code: "NOT_FOUND", message: "Workspace not found." }); + const now = new Date(); + if (!args.skipCollisionGuard) { + const leaseMs = workspace.githubRequestTimeoutSeconds * 12_000 + 5_000; + const claimed = await claimGitHubManualSync({ + db: args.db, + workspaceId: args.workspaceId, + externalResourceId: args.externalResourceId, + now, + cooldownSeconds: workspace.githubManualCooldownSeconds, + leaseUntil: new Date(now.getTime() + leaseMs), + }); + if (!claimed) { + throw new TRPCError({ + code: "TOO_MANY_REQUESTS", + message: "This GitHub link was refreshed recently or is already being refreshed.", + }); + } + } const existing = await args.db.externalResource.findFirst({ where: { id: args.externalResourceId, @@ -694,6 +749,8 @@ export async function syncGitHubExternalResource(args: { repoFullName: existing.repoFullName, resourceType: existing.resourceType as GitHubResourceType, number: existing.number, + requestTimeoutMs: workspace.githubRequestTimeoutSeconds * 1000, + signal: args.signal, }); const resource = await args.db.$transaction(async (tx) => { const resource = await upsertExternalResource(tx, { @@ -721,3 +778,25 @@ export async function syncGitHubExternalResource(args: { } return resource; } + +export async function claimGitHubManualSync(args: { + db: PrismaClient; + workspaceId: string; + externalResourceId: string; + now: Date; + cooldownSeconds: number; + leaseUntil: Date; +}): Promise { + const cooldownBefore = new Date(args.now.getTime() - args.cooldownSeconds * 1000); + const claimed = await args.db.externalResource.updateMany({ + where: { + id: args.externalResourceId, + workspaceId: args.workspaceId, + provider: GITHUB_PROVIDER, + OR: [{ syncAttemptedAt: null }, { syncAttemptedAt: { lte: cooldownBefore } }], + AND: [{ OR: [{ syncRetryAt: null }, { syncRetryAt: { lte: args.now } }] }], + }, + data: { syncAttemptedAt: args.now, syncRetryAt: args.leaseUntil }, + }); + return claimed.count === 1; +} diff --git a/src/server/services/github/webhook.ts b/src/server/services/github/webhook.ts index cac236cf..35318619 100644 --- a/src/server/services/github/webhook.ts +++ b/src/server/services/github/webhook.ts @@ -340,15 +340,20 @@ function completedCheckPullRequestNumbers(payload: GitHubWebhookPayload): number return [...out]; } -export function aggregateGitHubCheckConclusion(args: { +export function githubCheckWebhookHint(args: { event: "check_suite" | "check_run"; conclusion: string | null; - existingConclusion: unknown; -}): string | null { - const successful = - args.conclusion !== null && ["success", "neutral", "skipped"].includes(args.conclusion); - if (args.event === "check_suite" || !successful) return args.conclusion; - return typeof args.existingConclusion === "string" ? args.existingConclusion : null; + headSha?: string | null; +}) { + return { + status: "dirty", + conclusion: null, + source: "webhook-hint", + updatedAt: new Date().toISOString(), + event: args.event, + observedConclusion: args.conclusion, + ...(args.headSha ? { headSha: args.headSha } : {}), + } as const; } async function processCheckEvent(args: { @@ -411,13 +416,13 @@ async function processCheckEvent(args: { if (eventHeadSha && typeof head.sha === "string" && head.sha !== eventHeadSha) { continue; } - // A check_suite conclusion is aggregate. A successful check_run only - // proves one job passed, so it must not certify the whole PR; failures - // remain safe to persist immediately. - const aggregateConclusion = aggregateGitHubCheckConclusion({ + // A webhook check_suite is only one suite, not repository-wide CI. + // Persist it as a dirty hint; the worker must fetch every suite plus the + // combined commit status before completion can trust a conclusion. + const webhookHint = githubCheckWebhookHint({ event: args.payload.check_suite ? "check_suite" : "check_run", conclusion, - existingConclusion: existingChecks.conclusion, + headSha: eventHeadSha, }); await tx.externalResource.update({ where: { id: resource.id }, @@ -426,24 +431,12 @@ async function processCheckEvent(args: { ...metadata, checks: { ...existingChecks, - status: "completed", - conclusion: aggregateConclusion, - updatedAt: new Date().toISOString(), - event: args.payload.check_suite ? "check_suite" : "check_run", + ...webhookHint, ...(args.payload.check_run ? { lastRunConclusion: conclusion } : {}), - ...(eventHeadSha ? { headSha: eventHeadSha } : {}), }, } as Prisma.InputJsonValue, - syncAttemptedAt: new Date(), - syncRetryAt: null, - syncFailureCount: 0, - syncLastError: null, - syncTerminalAt: - resource.state === "merged" && - aggregateConclusion !== null && - ["success", "neutral", "skipped"].includes(aggregateConclusion) - ? new Date() - : null, + lastSyncedAt: null, + syncTerminalAt: null, }, }); processed += 1; diff --git a/tests/e2e/dashboard-layout.spec.ts b/tests/e2e/dashboard-layout.spec.ts index 5f8757ca..a9603b45 100644 --- a/tests/e2e/dashboard-layout.spec.ts +++ b/tests/e2e/dashboard-layout.spec.ts @@ -42,8 +42,12 @@ test("dashboard keeps priority work bounded and reflows secondary modules", asyn await expect(flow).toBeVisible(); await expect(flow.getByText("Pipeline", { exact: true }).first()).toBeVisible(); - const priorityGrid = cockpit.locator(`[data-dashboard-columns="2"]`); + const priorityGrid = cockpit.locator(`[data-dashboard-columns]`).first(); await expect(priorityGrid).toBeVisible(); + const configuredPriorityColumns = Number( + await priorityGrid.getAttribute("data-dashboard-columns"), + ); + expect([1, 2]).toContain(configuredPriorityColumns); await expect .poll(() => priorityGrid @@ -55,7 +59,9 @@ test("dashboard keeps priority work bounded and reflows secondary modules", asyn const tracks = getComputedStyle(node).gridTemplateColumns; return tracks === "none" ? 1 : tracks.split(" ").length; }); - expect(priorityColumns).toBe(state.priorityColumns); + expect(priorityColumns).toBe( + state.name === "desktop" ? configuredPriorityColumns : state.priorityColumns, + ); await expect(cockpit.getByTestId("dashboard-pulse")).toBeVisible(); await expect(cockpit.getByTestId("dashboard-schedule")).toBeVisible(); diff --git a/tests/unit/github-check-conclusion.test.ts b/tests/unit/github-check-conclusion.test.ts index 03486e72..aa1f6b89 100644 --- a/tests/unit/github-check-conclusion.test.ts +++ b/tests/unit/github-check-conclusion.test.ts @@ -1,41 +1,28 @@ import { describe, expect, it } from "vitest"; -import { aggregateGitHubCheckConclusion } from "@/server/services/github/webhook"; +import { githubCheckWebhookHint } from "@/server/services/github/webhook"; -describe("aggregateGitHubCheckConclusion", () => { - it("accepts aggregate suite success as completion evidence", () => { +describe("githubCheckWebhookHint", () => { + it("treats a successful suite as a dirty hint, never aggregate completion evidence", () => { expect( - aggregateGitHubCheckConclusion({ + githubCheckWebhookHint({ event: "check_suite", conclusion: "success", - existingConclusion: null, }), - ).toBe("success"); + ).toMatchObject({ + status: "dirty", + conclusion: null, + source: "webhook-hint", + observedConclusion: "success", + }); }); - it("does not treat one successful check run as aggregate success", () => { + it("also treats an individual failure as a non-terminal refresh hint", () => { expect( - aggregateGitHubCheckConclusion({ - event: "check_run", - conclusion: "success", - existingConclusion: null, - }), - ).toBeNull(); - }); - - it("keeps a known suite result across later successful jobs and records failures", () => { - expect( - aggregateGitHubCheckConclusion({ - event: "check_run", - conclusion: "success", - existingConclusion: "success", - }), - ).toBe("success"); - expect( - aggregateGitHubCheckConclusion({ + githubCheckWebhookHint({ event: "check_run", conclusion: "failure", - existingConclusion: "success", + headSha: "abc123", }), - ).toBe("failure"); + ).toMatchObject({ conclusion: null, observedConclusion: "failure", headSha: "abc123" }); }); }); diff --git a/tests/unit/github-client.test.ts b/tests/unit/github-client.test.ts index 1cf4b972..b1c4e320 100644 --- a/tests/unit/github-client.test.ts +++ b/tests/unit/github-client.test.ts @@ -40,6 +40,138 @@ describe("GitHub REST client resilience", () => { expect(checks.diagnostic).toContain("Checks permission required"); }); + it("aggregates every check-suite page and ignores empty legacy status pending", async () => { + const firstPage = Array.from({ length: 100 }, () => ({ + status: "completed", + conclusion: "success", + })); + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ total_count: 101, check_suites: firstPage }), { + status: 200, + }), + ) + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + total_count: 101, + check_suites: [{ status: "completed", conclusion: "neutral" }], + }), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ state: "pending", total_count: 0 }), { status: 200 }), + ); + + const checks = await getGitHubPullRequestChecks({ + installationId: 1, + owner: "acme", + repo: "forge", + headSha: "abc123", + }); + + expect(checks).toMatchObject({ + status: "completed", + conclusion: "success", + suiteCount: 101, + statusCount: 0, + partial: false, + source: "api-aggregate", + }); + const urls = vi.mocked(globalThis.fetch).mock.calls.map(([url]) => String(url)); + expect(urls[0]).not.toContain("filter=latest"); + expect(urls[1]).toContain("page=2"); + }); + + it("keeps a completed suite with a null conclusion untrusted", async () => { + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + total_count: 2, + check_suites: [ + { status: "completed", conclusion: "success" }, + { status: "completed", conclusion: null }, + ], + }), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ state: "success", total_count: 1 }), { status: 200 }), + ); + + await expect( + getGitHubPullRequestChecks({ + installationId: 1, + owner: "acme", + repo: "forge", + headSha: "abc123", + }), + ).resolves.toMatchObject({ status: "unknown", conclusion: null }); + }); + + it("keeps malformed legacy status aggregates untrusted", async () => { + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response( + JSON.stringify({ + total_count: 1, + check_suites: [{ status: "completed", conclusion: "success" }], + }), + { status: 200 }, + ), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ total_count: 1 }), { status: 200 }), + ); + + await expect( + getGitHubPullRequestChecks({ + installationId: 1, + owner: "acme", + repo: "forge", + headSha: "abc123", + }), + ).resolves.toMatchObject({ + status: "unknown", + conclusion: null, + diagnostic: "GitHub returned commit statuses without an aggregate state.", + }); + }); + + it("reports partial rate limits accurately even when another checks endpoint succeeds", async () => { + const reset = Math.floor(Date.now() / 1000) + 600; + vi.spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ message: "API rate limit exceeded" }), { + status: 403, + headers: { + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": String(reset), + }, + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify({ state: "success", total_count: 1 }), { status: 200 }), + ); + + await expect( + getGitHubPullRequestChecks({ + installationId: 1, + owner: "acme", + repo: "forge", + headSha: "abc123", + }), + ).resolves.toMatchObject({ + partial: true, + rateLimited: true, + conclusion: null, + retryAt: new Date(reset * 1000).toISOString(), + }); + }); + it("preserves GitHub rate-limit reset timing on request errors", async () => { const reset = Math.floor(Date.now() / 1000) + 600; vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( @@ -57,4 +189,25 @@ describe("GitHub REST client resilience", () => { retryAt: new Date(reset * 1000), } satisfies Partial); }); + + it("aborts GitHub requests at the configured timeout", async () => { + vi.spyOn(globalThis, "fetch").mockImplementationOnce( + (_url, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => + reject(new DOMException("Timed out", "TimeoutError")), + ); + }), + ); + + await expect( + getGitHubPullRequest({ + installationId: 1, + owner: "acme", + repo: "forge", + number: 42, + requestTimeoutMs: 5, + }), + ).rejects.toMatchObject({ status: 408, timedOut: true, rateLimited: false }); + }); });