From 7a67726826f12c8b1bb8b5a05234148500233485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ovidiu=20M=C4=83ru=C8=99?= Date: Sat, 6 Jun 2026 05:50:17 +0000 Subject: [PATCH] fix: stabilize checkout pr cache keys --- apps/daemon/src/app-github-quota.test.ts | 46 ++++++ apps/daemon/src/checkout-pr-primer.test.ts | 2 +- apps/daemon/src/extra-routes.ts | 8 +- apps/daemon/src/github-provider-state.test.ts | 6 +- apps/daemon/src/provider-cache.ts | 4 +- apps/daemon/src/provider-refresh-job.test.ts | 9 +- apps/daemon/src/provider-refresh-job.ts | 12 +- .../src/workspaces-pr-state-route.test.ts | 136 ++++++++++++++++++ 8 files changed, 206 insertions(+), 17 deletions(-) diff --git a/apps/daemon/src/app-github-quota.test.ts b/apps/daemon/src/app-github-quota.test.ts index f28dc404..033e35f3 100644 --- a/apps/daemon/src/app-github-quota.test.ts +++ b/apps/daemon/src/app-github-quota.test.ts @@ -156,6 +156,52 @@ exit 1 ROUTE_TEST_TIMEOUT_MS, ); + it( + "starts GitHub cooldown from a nearly exhausted quota response before it reaches zero", + async () => { + const fixture = createFixture(); + const fakeGh = path.join(fixture.config.dataDir, "fake-gh"); + fs.writeFileSync( + fakeGh, + `#!/usr/bin/env bash +if [ "$1" = "api" ] && [ "$2" = "rate_limit" ]; then + cat <<'JSON' +{"resources":{"core":{"limit":5000,"used":25,"remaining":4975,"reset":4102444800},"graphql":{"limit":5000,"used":4950,"remaining":50,"reset":4102444800},"search":{"limit":30,"used":0,"remaining":30,"reset":4102444800}}} +JSON + exit 0 +fi +exit 1 +`, + { mode: 0o755 }, + ); + fixture.config.providers.github.enabled = true; + fixture.config.providers.github.command = fakeGh; + const { server } = await createDaemonApp(fixture); + const baseUrl = await listen(server); + try { + const body = await getJson<{ + quota: { + status: string; + reason: string | null; + cooldownUntil: string | null; + resources: Array<{ name: string; remaining: number; percentUsed: number }>; + }; + }>(`${baseUrl}/api/integrations/github/quota`); + expect(body.quota.status).toBe("degraded"); + expect(body.quota.reason).toContain("GitHub graphql quota nearly exhausted"); + expect(body.quota.cooldownUntil).toBeTruthy(); + expect(body.quota.resources.find((resource) => resource.name === "graphql")).toMatchObject({ + remaining: 50, + percentUsed: 99, + }); + } finally { + clearGhCooldown(); + await closeServer(server); + } + }, + ROUTE_TEST_TIMEOUT_MS, + ); + it( "injects versionControl.cooldownUntil into provider-summary while gh is in cooldown (review #6)", async () => { diff --git a/apps/daemon/src/checkout-pr-primer.test.ts b/apps/daemon/src/checkout-pr-primer.test.ts index 63a9ab53..dd66065d 100644 --- a/apps/daemon/src/checkout-pr-primer.test.ts +++ b/apps/daemon/src/checkout-pr-primer.test.ts @@ -154,7 +154,7 @@ describe("checkout PR primer", () => { expect.objectContaining({ id: "ws_a" }), expect.objectContaining({ id: "co_api" }), expect.objectContaining({ id: "repo_a" }), - "vc:ws_a:checkout:co_api:2026-06-05T00:00:00.000Z", + "vc:ws_a:checkout:co_api", { intent: "automatic", force: true, staleWhileRevalidate: true }, ); expect(fetchVersionControl).not.toHaveBeenCalled(); diff --git a/apps/daemon/src/extra-routes.ts b/apps/daemon/src/extra-routes.ts index b268efe0..c7e082eb 100644 --- a/apps/daemon/src/extra-routes.ts +++ b/apps/daemon/src/extra-routes.ts @@ -21,6 +21,7 @@ type AsyncRoute = ( const execFileAsync = promisify(execFile); const GITHUB_QUOTA_CACHE_TTL_MS = 60_000; +const GITHUB_QUOTA_COOLDOWN_PERCENT_USED = 99; export function registerWorkspaceExtraRoutes(input: { app: express.Express; @@ -497,12 +498,15 @@ async function readGitHubQuota( function quotaCooldownFromResources(quota: GitHubQuotaSummary): { until: number; reason: string } | null { const exhausted = quota.resources.find( - (resource) => (resource.name === "graphql" || resource.name === "core") && resource.remaining === 0, + (resource) => + (resource.name === "graphql" || resource.name === "core") && + (resource.remaining === 0 || resource.percentUsed >= GITHUB_QUOTA_COOLDOWN_PERCENT_USED), ); if (!exhausted?.resetAt) return null; const resetMs = Date.parse(exhausted.resetAt); if (!Number.isFinite(resetMs) || resetMs <= Date.now()) return null; - const reason = `GitHub ${exhausted.name} quota exhausted until ${exhausted.resetAt}`; + const state = exhausted.remaining === 0 ? "exhausted" : "nearly exhausted"; + const reason = `GitHub ${exhausted.name} quota ${state} until ${exhausted.resetAt}`; const until = setGhCooldown(reason, Math.max(1, resetMs - Date.now())); return { until, reason }; } diff --git a/apps/daemon/src/github-provider-state.test.ts b/apps/daemon/src/github-provider-state.test.ts index b71915b8..e5f32160 100644 --- a/apps/daemon/src/github-provider-state.test.ts +++ b/apps/daemon/src/github-provider-state.test.ts @@ -277,7 +277,7 @@ describe("GitHub provider state service", () => { makeWorkspace({ kind: "root" }), checkout, makeRepo(), - "vc:w1:checkout:co_api:2026-05-25T00:00:00Z", + "vc:w1:checkout:co_api", { intent: "interactive", force: true, staleWhileRevalidate: false }, ); @@ -304,7 +304,7 @@ describe("GitHub provider state service", () => { makeWorkspace({ kind: "root" }), checkout, makeRepo(), - "vc:w1:checkout:co_api:2026-05-25T00:00:00Z", + "vc:w1:checkout:co_api", { intent: "interactive", force: true, staleWhileRevalidate: false }, ); @@ -338,7 +338,7 @@ describe("GitHub provider state service", () => { makeWorkspace({ kind: "root" }), checkout, makeRepo(), - "vc:w1:checkout:co_api:2026-05-25T00:00:00Z", + "vc:w1:checkout:co_api", { intent: "automatic" }, ); diff --git a/apps/daemon/src/provider-cache.ts b/apps/daemon/src/provider-cache.ts index aef3e4f1..e144852a 100644 --- a/apps/daemon/src/provider-cache.ts +++ b/apps/daemon/src/provider-cache.ts @@ -293,8 +293,8 @@ export function ciCacheKey(id: string, updatedAt: string): string { return `ci:${id}:${updatedAt}`; } -export function checkoutVcCacheKey(workspaceId: string, checkoutId: string, checkoutUpdatedAt: string): string { - return `vc:${workspaceId}:checkout:${checkoutId}:${checkoutUpdatedAt}`; +export function checkoutVcCacheKey(workspaceId: string, checkoutId: string, _checkoutUpdatedAt: string): string { + return `vc:${workspaceId}:checkout:${checkoutId}`; } export function issueCacheKey(issueKey: string): string { diff --git a/apps/daemon/src/provider-refresh-job.test.ts b/apps/daemon/src/provider-refresh-job.test.ts index feaeb7b3..3cc126df 100644 --- a/apps/daemon/src/provider-refresh-job.test.ts +++ b/apps/daemon/src/provider-refresh-job.test.ts @@ -367,9 +367,9 @@ describe("startProviderRefreshJob", () => { const callPaths = (deps.providers.collectGitHubVersionControlSummary as ReturnType).mock.calls.map( (call) => call[0], ); - expect(callPaths).toEqual(["/tmp/w1", "/tmp/w1/co_api"]); - expect(deps.cache.get("vc:w1:checkout:co_api:2026-05-25T00:00:00Z")?.value).toBeDefined(); - expect(deps.cache.get("vc:w1:checkout:co_archived:2026-05-25T00:00:00Z")).toBeUndefined(); + expect(callPaths).toEqual(["/tmp/w1/co_api"]); + expect(deps.cache.get("vc:w1:checkout:co_api")?.value).toBeDefined(); + expect(deps.cache.get("vc:w1:checkout:co_archived")).toBeUndefined(); job.stop(); }); @@ -409,11 +409,12 @@ describe("startProviderRefreshJob", () => { }; const job = startProviderRefreshJob({ ...deps, github, tickIntervalMs: 0, jitterMaxMs: 0 }); await job.runTickForTest(); + expect(github.fetchVersionControl).not.toHaveBeenCalled(); expect(github.fetchCheckoutVersionControl).toHaveBeenCalledWith( workspace, checkout, repo, - "vc:w1:checkout:co_api:2026-05-25T00:00:00Z", + "vc:w1:checkout:co_api", { intent: "automatic" }, ); job.stop(); diff --git a/apps/daemon/src/provider-refresh-job.ts b/apps/daemon/src/provider-refresh-job.ts index a320bd48..56d98138 100644 --- a/apps/daemon/src/provider-refresh-job.ts +++ b/apps/daemon/src/provider-refresh-job.ts @@ -120,11 +120,13 @@ export function startProviderRefreshJob(deps: ProviderRefreshDeps): ProviderRefr const ciMs = deps.config.providerRefresh.intervals.ciMs ?? prCiMs; const vcKey = vcCacheKey(workspace.id, workspace.updatedAt); const ciKey = ciCacheKey(workspace.id, workspace.updatedAt); - if (isStale(vcKey, prCiMs)) { - items.push({ kind: "vc", workspaceId: workspace.id, cacheKey: vcKey, ttlMs: prCiMs, rootPath: workspace.path }); - } - if (shouldFetchGithubCi(deps.store, workspace) && isStale(ciKey, ciMs)) { - items.push({ kind: "ci", workspaceId: workspace.id, cacheKey: ciKey, ttlMs: ciMs, rootPath: workspace.path }); + if (workspace.kind !== "root") { + if (isStale(vcKey, prCiMs)) { + items.push({ kind: "vc", workspaceId: workspace.id, cacheKey: vcKey, ttlMs: prCiMs, rootPath: workspace.path }); + } + if (shouldFetchGithubCi(deps.store, workspace) && isStale(ciKey, ciMs)) { + items.push({ kind: "ci", workspaceId: workspace.id, cacheKey: ciKey, ttlMs: ciMs, rootPath: workspace.path }); + } } for (const checkout of listActiveWorkspaceCheckouts(deps.store, workspace.id)) { const checkoutVcKey = checkoutVcCacheKey(workspace.id, checkout.id, checkout.updatedAt); diff --git a/apps/daemon/src/workspaces-pr-state-route.test.ts b/apps/daemon/src/workspaces-pr-state-route.test.ts index fa2d9518..d40a7963 100644 --- a/apps/daemon/src/workspaces-pr-state-route.test.ts +++ b/apps/daemon/src/workspaces-pr-state-route.test.ts @@ -279,4 +279,140 @@ describe("GET /api/workspaces/pr-state", () => { await closeServer(server); } }); + + it( + "keeps checkout PR summaries visible after PR binding updates bump checkout.updatedAt", + { timeout: 30_000 }, + async () => { + const fixture = createFixture(dirs); + const timestamp = "2026-06-04T00:00:00.000Z"; + fixture.store.insertRepo({ + id: "repo_1", + name: "Repo", + rootPath: path.join(fixture.config.dataDir, "repo"), + defaultBranch: "main", + defaultRemote: "origin", + worktreeParent: path.join(fixture.config.dataDir, "worktrees"), + setupHookIds: [], + teardownHookIds: [], + providerIds: [], + deployHookCommand: null, + createdAt: timestamp, + updatedAt: timestamp, + archivedAt: null, + }); + fixture.store.insertWorkspace({ + id: "ws_structured", + repoId: null, + name: "Structured", + path: path.join(fixture.config.dataDir, "structured"), + rootPath: path.join(fixture.config.dataDir, "structured"), + mode: "structured", + branch: "home", + baseBranch: "main", + source: "scratch", + kind: "root", + lifecyclePhase: "implementation", + parentIssue: null, + prUrl: null, + issueKey: null, + issueTitle: null, + issueUrl: null, + slackThreadUrl: null, + section: "backlog", + pinned: false, + lifecycle: "ready", + dirty: false, + namespaceId: null, + createdAt: timestamp, + updatedAt: timestamp, + archivedAt: null, + }); + fixture.store.insertWorkspaceCheckout({ + id: "co_api", + workspaceId: "ws_structured", + repoId: "repo_1", + name: "api", + path: path.join(fixture.config.dataDir, "structured", "api"), + branch: "feature/api", + baseBranch: "main", + issue: null, + intendedPr: null, + stackParentCheckoutId: null, + inferredPurpose: "implementation", + gateStatus: "not_started", + createdAt: timestamp, + updatedAt: timestamp, + archivedAt: null, + }); + const cachedAtMs = Date.now(); + fs.writeFileSync( + `${fixture.config.dataDir}/provider-cache.json`, + JSON.stringify({ + version: 1, + savedAt: new Date().toISOString(), + entries: [ + [ + checkoutVcCacheKey("ws_structured", "co_api", timestamp), + { + expiresAt: Date.now() + 60_000, + cachedAt: cachedAtMs, + value: { + providerId: "github-gh", + status: "healthy", + reason: null, + checkedAt: timestamp, + defaultBranch: "main", + currentBranch: "feature/api", + remotes: [], + pullRequest: { + number: 42, + title: "Ship API", + url: "https://github.example.test/org/repo/pull/42", + state: "OPEN", + draft: false, + reviewDecision: null, + checks: [], + additions: null, + deletions: null, + reviewers: [], + commits: [], + headRefName: "feature/api", + parentPr: null, + mergeable: "unknown", + allowedMergeStrategies: [], + mergeStateStatus: null, + headSha: "abc123", + }, + }, + }, + ], + ], + }), + ); + fixture.store.updateWorkspaceCheckoutPr("co_api", { + provider: "github", + number: 42, + url: "https://github.example.test/org/repo/pull/42", + state: "open", + headSha: "abc123", + baseRef: "main", + fetchedAt: "2026-06-04T00:01:00.000Z", + checksGreen: null, + mergeStateStatus: null, + hasConflicts: null, + }); + + const { server } = await createDaemonApp(fixture); + const baseUrl = await listen(server); + try { + const body = await getJson<{ + checkoutPrState: Record>; + }>(`${baseUrl}/api/workspaces/pr-state`); + expect(body.checkoutPrState.ws_structured?.co_api?.pullRequest).toMatchObject({ number: 42 }); + } finally { + await closeServer(server); + } + }, + ); });