diff --git a/apps/api/src/tools/task-board/checks-status.test.ts b/apps/api/src/tools/task-board/checks-status.test.ts index f9c8457d8d..58fd730c12 100644 --- a/apps/api/src/tools/task-board/checks-status.test.ts +++ b/apps/api/src/tools/task-board/checks-status.test.ts @@ -12,6 +12,7 @@ import { extractPreviewUrlFromDeployment, headShaFromPrGet, headShaFromStatus, + isAwaitingPreview, isRateLimitError, extractPreviewUrlFromCheckRuns, extractPreviewUrlFromComments, @@ -642,3 +643,28 @@ describe("previewMatchesHead", () => { expect(previewMatchesHead([])).toBe(true); }); }); + +describe("isAwaitingPreview", () => { + it("only a running-CI card with no preview keeps refreshing", () => { + expect( + isAwaitingPreview({ previewUrl: null, checksStatus: "pending" }), + ).toBe(true); + // Preview found — nothing left to wait for. + expect( + isAwaitingPreview({ + previewUrl: "https://x.vtex.app", + checksStatus: "pending", + }), + ).toBe(false); + // CI settled without ever posting one; refreshing forever would not help. + expect( + isAwaitingPreview({ previewUrl: null, checksStatus: "passing" }), + ).toBe(false); + expect( + isAwaitingPreview({ previewUrl: null, checksStatus: "failing" }), + ).toBe(false); + expect(isAwaitingPreview({ previewUrl: null, checksStatus: null })).toBe( + false, + ); + }); +}); diff --git a/apps/api/src/tools/task-board/pr-cache.test.ts b/apps/api/src/tools/task-board/pr-cache.test.ts index ec433dddf0..c155ff472c 100644 --- a/apps/api/src/tools/task-board/pr-cache.test.ts +++ b/apps/api/src/tools/task-board/pr-cache.test.ts @@ -271,3 +271,82 @@ describe("fetchOrPlaceholder", () => { }); }); }); + +describe("per-entry revalidate override (a value still awaiting something)", () => { + test("fetch: a not-ready stored value goes stale immediately", async () => { + const clock = { now: 0 }; + const cache = await cacheAt(clock); + let calls = 0; + const pending: Promise[] = []; + const read = (ready: boolean) => + cache.fetch({ + namespace: "conn_1", + key: "deployment", + fetchLive: async () => { + calls++; + return { url: ready ? "https://x.vtex.app" : null }; + }, + onRevalidation: (p) => pending.push(p), + // Not ready -> 0, so the very next read revalidates. + revalidateAfterMs: (stored) => + (stored as { url: string | null }).url === null ? 0 : 55_000, + }); + + await read(false); + expect(calls).toBe(1); + + // One tick later the default window (55s) would still be a HIT; the + // override makes it stale, so the deploy's url is picked up on this poll. + clock.now = 1_000; + await read(true); + await Promise.all(pending); + expect(calls).toBe(2); + + // Now that it IS ready, the default window applies again. + clock.now = 2_000; + await read(true); + await Promise.all(pending); + expect(calls).toBe(2); + }); + + test("fetchOrPlaceholder: an incomplete card is never a hit", async () => { + const clock = { now: 0 }; + const cache = new JetStreamKVPrCache( + PR_CARDS_CACHE, + { getJetStream: () => null }, + () => clock.now, + ); + await cache.init(fakeKv()); + + let calls = 0; + const get = (previewUrl: string | null) => + cache.fetchOrPlaceholder<{ previewUrl: string | null }>({ + namespace: "org_1", + key: "task_1", + fetchLive: async () => { + calls++; + return { previewUrl }; + }, + placeholder: { previewUrl: null }, + revalidateAfterMs: (card) => + card.previewUrl === null ? 0 : PR_CARDS_CACHE.revalidateAfterMs, + }); + + await get(null); // placeholder + detached fill + await Bun.sleep(0); + expect(calls).toBe(1); + + // Inside the 30s default window, so unpatched this was a hit and the + // preview waited for the window to age out. + clock.now = 1_000; + await get("https://x.vtex.app"); + await Bun.sleep(0); + expect(calls).toBe(2); + + // Complete card: back to the normal window, no rebuild per poll. + clock.now = 2_000; + await get("https://x.vtex.app"); + await Bun.sleep(0); + expect(calls).toBe(2); + }); +}); diff --git a/apps/api/src/tools/task-board/pr-cache.ts b/apps/api/src/tools/task-board/pr-cache.ts index 54eeb73330..98822e2dc0 100644 --- a/apps/api/src/tools/task-board/pr-cache.ts +++ b/apps/api/src/tools/task-board/pr-cache.ts @@ -97,6 +97,11 @@ export interface PrCacheFetch { /** Receives the background revalidation so the caller can keep its MCP client * open until it settles. */ onRevalidation: (promise: Promise) => void; + /** Per-entry override of the hit window, computed from the STORED value. + * A read whose value says "not ready yet" — a deploy with no url published — + * should go stale fast, so the next poll refetches instead of serving the + * not-ready answer for the full config window. Omit for the default. */ + revalidateAfterMs?: (stored: unknown) => number; } export class JetStreamKVPrCache { @@ -162,7 +167,8 @@ export class JetStreamKVPrCache { async fetch(params: PrCacheFetch): Promise { const { namespace, key: rawKey, fetchLive, onRevalidation } = params; - const { cache, revalidateAfterMs, maxStaleMs } = this.config; + const { cache, maxStaleMs } = this.config; + const defaultRevalidateAfterMs = this.config.revalidateAfterMs; if (!this.kv) { return this.fallback.fetch({ type: "tools/call", @@ -189,6 +195,9 @@ export class JetStreamKVPrCache { return value; } + const revalidateAfterMs = params.revalidateAfterMs + ? params.revalidateAfterMs(stored.value) + : defaultRevalidateAfterMs; if (age > revalidateAfterMs && !this.revalidating.has(key)) { cacheCounter.add(1, { cache, outcome: "stale" }); this.revalidating.add(key); @@ -228,15 +237,22 @@ export class JetStreamKVPrCache { key: string; fetchLive: () => Promise; placeholder: T; + /** Per-entry override of the hit window, computed from the STORED value — + * see {@link PrCacheFetch.revalidateAfterMs}. */ + revalidateAfterMs?: (stored: T) => number; }): Promise<{ value: T; live: boolean }> { const { namespace, key: rawKey, fetchLive, placeholder } = params; - const { cache, revalidateAfterMs, maxStaleMs } = this.config; + const { cache, maxStaleMs } = this.config; const key = this.storageKey(namespace, rawKey); const stored = await this.read(key); const age = stored ? this.now() - stored.storedAt : Number.POSITIVE_INFINITY; const usable = stored != null && age <= maxStaleMs; + const revalidateAfterMs = + stored && params.revalidateAfterMs + ? params.revalidateAfterMs(stored.value as T) + : this.config.revalidateAfterMs; if (usable && age <= revalidateAfterMs) { cacheCounter.add(1, { cache, outcome: "hit" }); diff --git a/apps/api/src/tools/task-board/prs-get.ts b/apps/api/src/tools/task-board/prs-get.ts index 4791398bcd..5bfd9cc105 100644 --- a/apps/api/src/tools/task-board/prs-get.ts +++ b/apps/api/src/tools/task-board/prs-get.ts @@ -49,7 +49,27 @@ export function isRateLimitError(err: unknown): boolean { * only moves to Done once a poll observes `merged`, and a minute of "did my ship * button work?" is exactly the confusion this cache must not introduce. */ -import { getPrCardCache, getPrReadCache } from "./pr-cache"; +import { + getPrCardCache, + getPrReadCache, + PR_CARDS_CACHE, + PR_READS_CACHE, +} from "./pr-cache"; + +/** Hit window for a cached value that is still waiting on something — a deploy + * with no published url, or a card with no preview while checks run. Zero, so + * the next poll always revalidates (detached, off the request path) instead of + * serving the not-ready answer for the full window. */ +const AWAITING_PREVIEW_REVALIDATE_MS = 0; + +/** A card that should keep refreshing: CI is still running and no preview URL + * has been found yet. Once either settles the card caches normally. */ +export function isAwaitingPreview(card: { + previewUrl: string | null; + checksStatus: ChecksStatus; +}): boolean { + return card.previewUrl === null && card.checksStatus === "pending"; +} export function invalidatePrReads(connectionId: string): Promise { return getPrReadCache().invalidate(connectionId); @@ -168,11 +188,13 @@ async function cachedPrRead( args: Record, describe: string, pending: Promise[], + revalidateAfterMs?: (stored: unknown) => number, ): Promise | null> { try { const raw = await getPrReadCache().fetch({ namespace: connectionId, key: JSON.stringify({ name, args }), + revalidateAfterMs, fetchLive: () => retry( async () => { @@ -867,6 +889,16 @@ async function fetchPrStatusExtras( { owner: pr.repoOwner, repo: pr.repoName, sha: headSha }, `${prLabel(pr)} (deployment preview)`, pending, + // An in-flight deploy answers "no environment url yet". Holding that + // for the full read window means the card keeps rebuilding from a + // stale not-ready answer even once GitHub has the url, so the preview + // shows up minutes after the deploy finished. + (stored) => + extractPreviewUrlFromDeployment( + stored as Record | null, + ) === null + ? AWAITING_PREVIEW_REVALIDATE_MS + : PR_READS_CACHE.revalidateAfterMs, ), ); } @@ -1265,6 +1297,15 @@ export const TASK_BOARD_ITEM_PRS_GET = defineTool({ namespace: organizationId, key: taskBoardItemId, fetchLive: assemble, + // A card whose deploy is still running has no preview yet. At the default + // window that card is a cache HIT for 30s, so the url can be a poll or + // two late even after the read above refreshes. Go stale immediately + // instead: the revalidation is detached, so this costs a background + // rebuild per poll on exactly the cards that are still missing something. + revalidateAfterMs: (cards) => + cards.some(isAwaitingPreview) + ? AWAITING_PREVIEW_REVALIDATE_MS + : PR_CARDS_CACHE.revalidateAfterMs, placeholder: linked.map((pr) => ({ url: pr.url, number: pr.number,