diff --git a/.dockerignore b/.dockerignore index c73905e0..d93d3a8a 100644 --- a/.dockerignore +++ b/.dockerignore @@ -2,8 +2,11 @@ node_modules **/node_modules .next .next-e2e +.next-* **/.next **/.next-e2e +**/.next-* +!next-env.d.ts docs/.vitepress/dist public/docs .git diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a9eed5f..6f340da6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ 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`. +## [2026-07-14] — v0.15.1 · Manual GitHub retry safety + +### Fixed + +- **Manual GitHub refresh failures now honor provider retry timing.** Rate limits, permission failures, and timeouts persist the real diagnostic and mapping-wide retry circuit instead of falling back to a short collision lease. +- **Release builds exclude every generated Next output directory.** Lifecycle and other named `.next-*` caches no longer inflate Docker contexts or exhaust builder storage. + ## [2026-07-14] — v0.15.0 · Reliable GitHub status recovery ### Added diff --git a/DEVLOG.md b/DEVLOG.md index c4186529..fe2b34a2 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -2,6 +2,51 @@ > Append-only session log. Read at session start. Update at session end. +## 2026-07-14 — Manual GitHub retry persistence and release context hygiene + +Addressed the post-merge Codex review on PR #29 before production deployment. +Manual and MCP-triggered GitHub refreshes now catch provider failures before +returning them, replace the temporary collision lease with the later of the +workspace exponential backoff or GitHub retry/reset header, persist the failure +count and diagnostic, and open the same mapping-wide circuit used by scheduled +reconciliation for rate limits, permissions, and timeouts. Scheduled sweeps +retain their existing single-owner retry accounting. Resource existence is +also checked before the manual lease so missing links still return NOT_FOUND. +The follow-up Codex review on PR #30 identified that a long provider request +could consume a short backoff before persistence; manual retry timing is now +anchored after the provider call finishes. +The second review pass found that a shorter new mapping-wide delay could +replace a longer existing provider reset on sibling resources. Mapping circuits +are now monotonic: only missing or earlier retry gates are extended. +The final review pass identified the non-throwing partial-checks path: PR reads +can succeed while Checks API calls return a settled partial snapshot. Manual +and MCP syncs now promote that metadata into the same persisted mapping-wide +failure circuit and return the updated diagnostic/backoff state immediately. +Generic partial snapshots (pagination limits or transient 5xx responses) are +now distinguished from permission failures and back off only the affected PR; +only auth, rate-limit, and timeout partials open a mapping-wide circuit in both +manual and scheduled reconciliation. +The last concurrency review found that overlapping refreshes could still +shorten the failing resource's own gate. Primary-row retry updates now use the +same atomic extend-only predicate as sibling rows, and failure counts increment +atomically so concurrent errors cannot lose accounting. +Partial-check diagnostics are already counted by the snapshot upsert, so the +subsequent mapping-circuit extension now preserves that count instead of +incrementing it a second time and skipping an exponential-backoff step. +Failure persistence now recognizes the exact collision lease acquired by the +current manual refresh, allowing it to be replaced by a shorter configured +backoff while still preserving any different, later provider gate written by +an overlapping refresh. The Docker ignore file also explicitly re-includes +`next-env.d.ts` for the worker stage's `COPY` contract. + +The first release image attempt also exposed that `.dockerignore` covered +`.next` and `.next-e2e` but not `.next-lifecycle`. All named `.next-*` outputs +are now excluded so local verification caches cannot inflate release contexts +or exhaust Docker storage. + +Verification: focused GitHub reconciliation and completion policy suite (23/23), +lint (existing warnings only), TypeScript typecheck, and `git diff --check`. + ## 2026-07-14 — Resilient linked-PR status reconciliation Added a webhook-first repair loop for native GitHub `IMPLEMENTS` links. The diff --git a/package.json b/package.json index a62f3b0a..939c4693 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "forge", - "version": "0.15.0", + "version": "0.15.1", "private": true, "description": "Forge — a fast, minimalist, keyboard-driven project management platform with pluggable agents.", "license": "MIT", diff --git a/src/server/services/__tests__/github-reconciliation.test.ts b/src/server/services/__tests__/github-reconciliation.test.ts index a5074aa9..14aab91a 100644 --- a/src/server/services/__tests__/github-reconciliation.test.ts +++ b/src/server/services/__tests__/github-reconciliation.test.ts @@ -2,6 +2,8 @@ import { afterAll, afterEach, describe, expect, it } from "vitest"; import { GitHubRequestError } from "@/server/services/github/client"; import { claimGitHubManualSync, + gitHubPartialChecksError, + persistGitHubManualSyncFailure, upsertExternalResource, } from "@/server/services/github/resource-sync"; import { @@ -89,6 +91,39 @@ async function setupResource() { } describe("GitHub status reconciliation", () => { + it("promotes partial checks metadata into a mapping-wide manual failure", () => { + const retryAt = "2026-07-14T13:00:00.000Z"; + expect( + gitHubPartialChecksError({ + checks: { + partial: true, + rateLimited: true, + timedOut: false, + diagnostic: "Checks API rate limit exceeded", + retryAt, + }, + }), + ).toMatchObject({ + message: "Checks API rate limit exceeded", + status: 429, + retryAt: new Date(retryAt), + rateLimited: true, + timedOut: false, + }); + expect(gitHubPartialChecksError({ checks: { partial: false } })).toBeNull(); + expect( + gitHubPartialChecksError({ + checks: { + partial: true, + rateLimited: false, + timedOut: false, + permissionDenied: false, + diagnostic: "Check suites require another page", + }, + }), + ).toMatchObject({ status: 503, rateLimited: false, timedOut: false }); + }); + 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"); @@ -433,6 +468,127 @@ describe("GitHub status reconciliation", () => { ).resolves.toBe(true); }); + it("persists provider retry timing and diagnostics after a manual refresh failure", async () => { + const { fixture, prisma, resource, mapping } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const reset = new Date("2026-07-14T13:00:00.000Z"); + const laterReset = new Date("2026-07-14T14: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 mapping", + state: "open", + connectionMappingId: mapping.id, + }, + }); + const siblingWithLaterGate = await prisma.externalResource.create({ + data: { + workspaceId: fixture.workspace.id, + provider: "GITHUB", + resourceType: "PULL_REQUEST", + repoFullName: "acme/forge", + number: 100, + url: "https://github.com/acme/forge/pull/100", + title: "Same mapping with a later gate", + state: "open", + connectionMappingId: mapping.id, + syncRetryAt: laterReset, + syncLastError: "Existing longer provider reset", + }, + }); + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { syncRetryAt: laterReset }, + }); + + await expect( + persistGitHubManualSyncFailure({ + db: prisma, + workspaceId: fixture.workspace.id, + externalResourceId: resource.id, + connectionMappingId: mapping.id, + currentFailureCount: 0, + now, + baseMinutes: 5, + maxMinutes: 1440, + error: new GitHubRequestError("API rate limit exceeded", 429, reset, true), + }), + ).resolves.toEqual({ retryAt: laterReset, failureCount: 1, mappingWide: true }); + + const [failed, mappedSibling, mappedSiblingWithLaterGate] = await Promise.all([ + prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }), + prisma.externalResource.findUniqueOrThrow({ where: { id: sibling.id } }), + prisma.externalResource.findUniqueOrThrow({ where: { id: siblingWithLaterGate.id } }), + ]); + expect(failed).toMatchObject({ + syncRetryAt: laterReset, + syncFailureCount: 1, + syncLastError: "API rate limit exceeded", + }); + expect(mappedSibling).toMatchObject({ + syncRetryAt: reset, + syncFailureCount: 0, + syncLastError: "API rate limit exceeded", + }); + expect(mappedSiblingWithLaterGate).toMatchObject({ + syncRetryAt: laterReset, + syncFailureCount: 0, + syncLastError: "Existing longer provider reset", + }); + + await expect( + persistGitHubManualSyncFailure({ + db: prisma, + workspaceId: fixture.workspace.id, + externalResourceId: resource.id, + connectionMappingId: mapping.id, + currentFailureCount: 1, + now: new Date("2026-07-14T12:05:00.000Z"), + baseMinutes: 5, + maxMinutes: 1440, + error: new GitHubRequestError("Partial checks already counted", 403, null), + incrementFailureCount: false, + }), + ).resolves.toMatchObject({ failureCount: 1, mappingWide: true }); + await expect( + prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }), + ).resolves.toMatchObject({ syncFailureCount: 1 }); + }); + + it("replaces the current manual lease without shortening an older provider gate", async () => { + const { fixture, prisma, resource, mapping } = await setupResource(); + const now = new Date("2026-07-14T12:00:00.000Z"); + const retryAt = new Date("2026-07-14T12:05:00.000Z"); + const claimedLeaseUntil = new Date("2026-07-14T12:10:00.000Z"); + await prisma.externalResource.update({ + where: { id: resource.id }, + data: { syncRetryAt: claimedLeaseUntil }, + }); + + await expect( + persistGitHubManualSyncFailure({ + db: prisma, + workspaceId: fixture.workspace.id, + externalResourceId: resource.id, + connectionMappingId: mapping.id, + currentFailureCount: 0, + now, + baseMinutes: 5, + maxMinutes: 1440, + error: new GitHubRequestError("Timed out", 408, retryAt, false, true), + claimedLeaseUntil, + }), + ).resolves.toMatchObject({ retryAt, failureCount: 1, mappingWide: true }); + await expect( + prisma.externalResource.findUniqueOrThrow({ where: { id: resource.id } }), + ).resolves.toMatchObject({ syncRetryAt: retryAt }); + }); + 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" }); diff --git a/src/server/services/github/client.ts b/src/server/services/github/client.ts index a182d00e..5759b898 100644 --- a/src/server/services/github/client.ts +++ b/src/server/services/github/client.ts @@ -89,6 +89,7 @@ export type GitHubChecksSnapshot = { partial: boolean; rateLimited: boolean; timedOut: boolean; + permissionDenied: boolean; diagnostic: string | null; retryAt: string | null; headSha: string; @@ -331,6 +332,12 @@ export async function getGitHubPullRequestChecks(args: { const timedOut = failures.some( (failure) => failure.reason instanceof GitHubRequestError && failure.reason.timedOut, ); + const permissionDenied = failures.some( + (failure) => + failure.reason instanceof GitHubRequestError && + !failure.reason.rateLimited && + [401, 403].includes(failure.reason.status), + ); const retryAt = failures .map((failure) => @@ -363,6 +370,7 @@ export async function getGitHubPullRequestChecks(args: { partial, rateLimited, timedOut, + permissionDenied, diagnostic: partial || nullConclusion || statusUnknown ? [ diff --git a/src/server/services/github/reconciliation.ts b/src/server/services/github/reconciliation.ts index 86df0e2a..481ef4d4 100644 --- a/src/server/services/github/reconciliation.ts +++ b/src/server/services/github/reconciliation.ts @@ -277,7 +277,11 @@ export async function sweepGitHubStatusReconciliation( if (checks.rateLimited === true) { result.rateLimited += 1; } - if (checks.partial === true) { + const mappingWidePartial = + checks.rateLimited === true || + checks.timedOut === true || + checks.permissionDenied === true; + if (mappingWidePartial) { // 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. diff --git a/src/server/services/github/resource-sync.ts b/src/server/services/github/resource-sync.ts index 4da69105..ce6257a6 100644 --- a/src/server/services/github/resource-sync.ts +++ b/src/server/services/github/resource-sync.ts @@ -17,6 +17,7 @@ import { getGitHubIssue, getGitHubPullRequest, getGitHubPullRequestChecks, + GitHubRequestError, issueSnapshot, pullRequestSnapshot, } from "@/server/services/github/client"; @@ -65,6 +66,32 @@ function publicResource(row: ExternalResource) { return row; } +export function gitHubPartialChecksError(metadata: unknown): GitHubRequestError | null { + if (!metadata || typeof metadata !== "object" || Array.isArray(metadata)) return null; + const checks = (metadata as Record).checks; + if (!checks || typeof checks !== "object" || Array.isArray(checks)) return null; + const values = checks as Record; + if (values.partial !== true) return null; + const retryAt = + typeof values.retryAt === "string" && Number.isFinite(Date.parse(values.retryAt)) + ? new Date(values.retryAt) + : null; + const rateLimited = values.rateLimited === true; + const timedOut = values.timedOut === true; + const permissionDenied = values.permissionDenied === true; + const message = + typeof values.diagnostic === "string" && values.diagnostic.trim() + ? values.diagnostic + : "GitHub returned partial checks data."; + return new GitHubRequestError( + message, + rateLimited ? 429 : timedOut ? 408 : permissionDenied ? 403 : 503, + retryAt, + rateLimited, + timedOut, + ); +} + export async function resolveGitHubRepoMapping(args: { db: PrismaClient; workspaceId: string; @@ -697,27 +724,13 @@ export async function syncGitHubExternalResource(args: { select: { githubRequestTimeoutSeconds: true, githubManualCooldownSeconds: true, + githubSyncBackoffMinutes: true, + githubSyncMaxBackoffMinutes: 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.", - }); - } - } + let manualLeaseUntil: Date | null = null; const existing = await args.db.externalResource.findFirst({ where: { id: args.externalResourceId, @@ -736,6 +749,24 @@ export async function syncGitHubExternalResource(args: { message: "Linked GitHub resource not found.", }); } + if (!args.skipCollisionGuard) { + const leaseMs = workspace.githubRequestTimeoutSeconds * 12_000 + 5_000; + manualLeaseUntil = new Date(now.getTime() + leaseMs); + const claimed = await claimGitHubManualSync({ + db: args.db, + workspaceId: args.workspaceId, + externalResourceId: args.externalResourceId, + now, + cooldownSeconds: workspace.githubManualCooldownSeconds, + leaseUntil: manualLeaseUntil, + }); + if (!claimed) { + throw new TRPCError({ + code: "TOO_MANY_REQUESTS", + message: "This GitHub link was refreshed recently or is already being refreshed.", + }); + } + } const mapping = existing.connectionMapping?.status === "active" ? (existing.connectionMapping as GitHubMappingWithConnection) @@ -744,15 +775,41 @@ export async function syncGitHubExternalResource(args: { workspaceId: args.workspaceId, repoFullName: existing.repoFullName, }); - const snapshot = await fetchGitHubSnapshot({ - mapping, - 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) => { + let snapshot: GitHubResourceSnapshot; + try { + snapshot = await fetchGitHubSnapshot({ + mapping, + repoFullName: existing.repoFullName, + resourceType: existing.resourceType as GitHubResourceType, + number: existing.number, + requestTimeoutMs: workspace.githubRequestTimeoutSeconds * 1000, + signal: args.signal, + }); + } catch (error) { + // Scheduled reconciliation owns its own retry accounting. Manual/MCP + // refreshes must persist the provider reset/backoff before returning the + // error so a short collision lease cannot accidentally become the retry + // policy and the issue panel retains the real diagnostic. + if (!args.skipCollisionGuard) { + await persistGitHubManualSyncFailure({ + db: args.db, + workspaceId: args.workspaceId, + externalResourceId: existing.id, + connectionMappingId: mapping.id, + currentFailureCount: existing.syncFailureCount, + // Anchor retry timing after the provider call finishes. A request may + // consume the entire configured timeout, which could otherwise make a + // short backoff expire before the failure is persisted. + now: new Date(), + baseMinutes: workspace.githubSyncBackoffMinutes, + maxMinutes: workspace.githubSyncMaxBackoffMinutes, + error, + claimedLeaseUntil: manualLeaseUntil, + }); + } + throw error; + } + let resource = await args.db.$transaction(async (tx) => { const resource = await upsertExternalResource(tx, { workspaceId: args.workspaceId, connectionMappingId: mapping.id, @@ -768,6 +825,30 @@ export async function syncGitHubExternalResource(args: { }); return publicResource(resource); }); + const partialChecksError = gitHubPartialChecksError(snapshot.metadata); + if (!args.skipCollisionGuard && partialChecksError) { + const failureNow = new Date(); + const failure = await persistGitHubManualSyncFailure({ + db: args.db, + workspaceId: args.workspaceId, + externalResourceId: existing.id, + connectionMappingId: mapping.id, + currentFailureCount: existing.syncFailureCount, + now: failureNow, + baseMinutes: workspace.githubSyncBackoffMinutes, + maxMinutes: workspace.githubSyncMaxBackoffMinutes, + error: partialChecksError, + incrementFailureCount: false, + claimedLeaseUntil: manualLeaseUntil, + }); + resource = { + ...resource, + syncAttemptedAt: failureNow, + syncRetryAt: failure.retryAt, + syncFailureCount: failure.failureCount, + syncLastError: partialChecksError.message.slice(0, 2_000), + }; + } if (resource.resourceType === "PULL_REQUEST") { await reconcileGitHubPullRequestCompletion(args.db, { workspaceId: args.workspaceId, @@ -800,3 +881,86 @@ export async function claimGitHubManualSync(args: { }); return claimed.count === 1; } + +export async function persistGitHubManualSyncFailure(args: { + db: PrismaClient; + workspaceId: string; + externalResourceId: string; + connectionMappingId: string | null; + currentFailureCount: number; + now: Date; + baseMinutes: number; + maxMinutes: number; + error: unknown; + incrementFailureCount?: boolean; + claimedLeaseUntil?: Date | null; +}): Promise<{ retryAt: Date; failureCount: number; mappingWide: boolean }> { + const failureCount = args.currentFailureCount + 1; + const exponentialMinutes = Math.min( + Math.max(1, args.maxMinutes), + Math.max(1, args.baseMinutes) * 2 ** Math.max(0, failureCount - 1), + ); + const fallbackRetryAt = new Date(args.now.getTime() + exponentialMinutes * 60_000); + const providerRetryAt = args.error instanceof GitHubRequestError ? args.error.retryAt : null; + const retryAt = + providerRetryAt && providerRetryAt > fallbackRetryAt ? providerRetryAt : fallbackRetryAt; + const message = + args.error instanceof Error ? args.error.message.slice(0, 2_000) : "Unknown GitHub sync error"; + const mappingWide = + args.error instanceof GitHubRequestError && + (args.error.rateLimited || args.error.timedOut || [401, 403].includes(args.error.status)); + + await args.db.$transaction(async (tx) => { + await tx.externalResource.updateMany({ + where: { + id: args.externalResourceId, + workspaceId: args.workspaceId, + provider: GITHUB_PROVIDER, + }, + data: { + syncAttemptedAt: args.now, + ...(args.incrementFailureCount === false ? {} : { syncFailureCount: { increment: 1 } }), + syncLastError: message, + }, + }); + await tx.externalResource.updateMany({ + where: { + id: args.externalResourceId, + workspaceId: args.workspaceId, + provider: GITHUB_PROVIDER, + OR: [ + { syncRetryAt: null }, + { syncRetryAt: { lt: retryAt } }, + ...(args.claimedLeaseUntil ? [{ syncRetryAt: { equals: args.claimedLeaseUntil } }] : []), + ], + }, + data: { syncRetryAt: retryAt }, + }); + if (mappingWide && args.connectionMappingId) { + await tx.externalResource.updateMany({ + where: { + id: { not: args.externalResourceId }, + workspaceId: args.workspaceId, + provider: GITHUB_PROVIDER, + connectionMappingId: args.connectionMappingId, + OR: [{ syncRetryAt: null }, { syncRetryAt: { lt: retryAt } }], + }, + data: { syncRetryAt: retryAt, syncLastError: message }, + }); + } + }); + + const persisted = await args.db.externalResource.findFirst({ + where: { + id: args.externalResourceId, + workspaceId: args.workspaceId, + provider: GITHUB_PROVIDER, + }, + select: { syncRetryAt: true, syncFailureCount: true }, + }); + return { + retryAt: persisted?.syncRetryAt ?? retryAt, + failureCount: persisted?.syncFailureCount ?? failureCount, + mappingWide, + }; +} diff --git a/tests/unit/github-client.test.ts b/tests/unit/github-client.test.ts index b1c4e320..cc1eb5e6 100644 --- a/tests/unit/github-client.test.ts +++ b/tests/unit/github-client.test.ts @@ -35,6 +35,7 @@ describe("GitHub REST client resilience", () => { status: "unknown", conclusion: null, partial: true, + permissionDenied: true, statusCount: 1, }); expect(checks.diagnostic).toContain("Checks permission required"); @@ -123,9 +124,7 @@ describe("GitHub REST client resilience", () => { { status: 200 }, ), ) - .mockResolvedValueOnce( - new Response(JSON.stringify({ total_count: 1 }), { status: 200 }), - ); + .mockResolvedValueOnce(new Response(JSON.stringify({ total_count: 1 }), { status: 200 })); await expect( getGitHubPullRequestChecks({ @@ -167,6 +166,7 @@ describe("GitHub REST client resilience", () => { ).resolves.toMatchObject({ partial: true, rateLimited: true, + permissionDenied: false, conclusion: null, retryAt: new Date(reset * 1000).toISOString(), });