From 6ba0aa86954c488c9c7106b2a510c6907cc9c492 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 17 Sep 2026 21:55:40 +0800 Subject: [PATCH 1/9] fix(cli): retry deploy POST on transport failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transient socket reset (ECONNRESET on a stale pooled keep-alive connection, or any edge/network blip) during the finalize-deploy POST failed the entire deploy even though no response ever arrived — so the platform could not have started processing. Retry the fetch with 1s/5s/10s backoff (max 3 retries). HTTP responses of any status still surface verbatim since a server answer means the request may have been accepted. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 42 +++++++- .../void/test/unit/direct-r2-client.test.ts | 102 ++++++++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index f52eb356e..27d36da48 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -329,6 +329,17 @@ export type UploadUrlAssetRequest = DeployAssetUploadRequest; */ export type ManifestOnlyDeployPayload = DeployUploadPayload; +/** + * Retry budget for the deploy POST's transport phase. A rejected `fetch` + * means no HTTP response ever arrived (e.g. ECONNRESET on a stale pooled + * keep-alive socket after the long R2-upload window), so the platform + * cannot have emitted a deploy event yet — replaying is safe. Once a + * Response exists, even a 4xx/5xx, the server answered: surface it + * verbatim rather than risk processing the same deploy twice. + */ +const DEPLOY_POST_MAX_RETRIES = 3; +const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; + /** * HTTP client for the Void platform API. */ @@ -577,12 +588,29 @@ export class PlatformClient { const cliLog = this.cliLog; cliLog?.info('deploy_post', { url }); + // `fetch` reports transport failures as TypeError ('fetch failed') — + // the only retryable case per the DEPLOY_POST_* contract above. + // Anything else (e.g. AbortError) is not transient and fails fast. let res: Response; - try { - res = await fetch(url, init); - } catch (err) { - cliLog?.error('deploy_fetch_rejected', err, { url }); - throw err; + let attempt = 0; + while (true) { + try { + res = await fetch(url, init); + break; + } catch (err) { + if (!(err instanceof TypeError) || attempt >= DEPLOY_POST_MAX_RETRIES) { + cliLog?.error('deploy_fetch_rejected', err, { url, attempts: attempt + 1 }); + throw err; + } + cliLog?.warn('deploy_fetch_retry', { + url, + attempt: attempt + 1, + delayMs: DEPLOY_POST_BACKOFF_MS[attempt], + error: err instanceof Error ? err.message : String(err), + }); + await sleep(DEPLOY_POST_BACKOFF_MS[attempt]); + attempt++; + } } // `cf-ray` and `cf-cache-status` are the correlation keys that platform @@ -1350,3 +1378,7 @@ export class PlatformClient { return res.json(); } } + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 979f03799..55ed9d8ef 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -238,6 +238,108 @@ describe('PlatformClient.finalizeDeploy', () => { }); }); + it('retries a transport-level fetch rejection with backoff, then streams events', async () => { + vi.useFakeTimers(); + fetchMock + .mockRejectedValueOnce(new TypeError('fetch failed', { cause: new Error('read ECONNRESET') })) + .mockResolvedValueOnce( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + + const cliLog = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + path: '/tmp/test.jsonl', + close: vi.fn(), + }; + const client = new PlatformClient(token, { apiUrl, cliLog }); + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(events).toEqual([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]); + expect(cliLog.warn).toHaveBeenCalledWith( + 'deploy_fetch_retry', + expect.objectContaining({ attempt: 1, delayMs: 1000 }), + ); + expect(cliLog.error).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('throws the original error after exhausting transport retries', async () => { + vi.useFakeTimers(); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + + const cliLog = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + path: '/tmp/test.jsonl', + close: vi.fn(), + }; + const client = new PlatformClient(token, { apiUrl, cliLog }); + try { + const consume = (async () => { + for await (const _event of client.finalizeDeploy(projectId, makePayload())) { + // unreachable — every attempt rejects + } + })(); + const assertion = expect(consume).rejects.toBeInstanceOf(TypeError); + await vi.runAllTimersAsync(); + await assertion; + + // 1 initial attempt + 3 retries. + expect(fetchMock).toHaveBeenCalledTimes(4); + const retries = cliLog.warn.mock.calls.filter(([msg]) => msg === 'deploy_fetch_retry'); + expect(retries.map(([, fields]) => fields?.delayMs)).toEqual([1000, 5000, 10000]); + expect(cliLog.error).toHaveBeenCalledWith( + 'deploy_fetch_rejected', + expect.any(TypeError), + expect.objectContaining({ + url: `${apiUrl}/projects/${projectId}/deploy`, + attempts: 4, + }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it('does not retry non-transport rejections (e.g. AbortError)', async () => { + fetchMock.mockRejectedValueOnce(new DOMException('The operation was aborted.', 'AbortError')); + const client = makeClient(); + const gen = client.finalizeDeploy(projectId, makePayload()); + await expect(gen.next()).rejects.toBeInstanceOf(DOMException); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('does not retry HTTP error responses — the server answered', async () => { + fetchMock.mockResolvedValue(new Response('busted', { status: 503 })); + const client = makeClient(); + const gen = client.finalizeDeploy(projectId, makePayload()); + await expect(gen.next()).rejects.toMatchObject({ + name: 'PlatformApiError', + status: 503, + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + it('serializes workerFiles as base64 in the JSON body', async () => { fetchMock.mockResolvedValueOnce( ndjsonStreamResponse([ From 8d651891c67239386e7ca37c08af7b70f3fd59b2 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 17 Sep 2026 22:34:23 +0800 Subject: [PATCH 2/9] fix(cli): suppress deploy POST retry when an earlier attempt registered a deployment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rejected fetch does not prove the request never arrived — the connection may have dropped after delivery. Before replaying the non-idempotent POST, check for a deployment created inside a 120s window; finding one means the earlier attempt reached the platform, so the retry is suppressed (reported via a terminal error event naming the deployment) instead of bouncing off the server's 409 concurrency guard or registering a duplicate deploy. The check is best-effort: on lookup failure the retry still runs and the server's guard remains the backstop. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 70 ++++++- .../void/test/unit/direct-r2-client.test.ts | 197 +++++++++++++++--- 2 files changed, 235 insertions(+), 32 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 27d36da48..8f90a0fb6 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -330,15 +330,21 @@ export type UploadUrlAssetRequest = DeployAssetUploadRequest; export type ManifestOnlyDeployPayload = DeployUploadPayload; /** - * Retry budget for the deploy POST's transport phase. A rejected `fetch` - * means no HTTP response ever arrived (e.g. ECONNRESET on a stale pooled - * keep-alive socket after the long R2-upload window), so the platform - * cannot have emitted a deploy event yet — replaying is safe. Once a - * Response exists, even a 4xx/5xx, the server answered: surface it - * verbatim rather than risk processing the same deploy twice. + * Retry contract for the deploy POST's transport phase. + * + * - Only `fetch` rejections (`TypeError` — no HTTP response) are retried. + * A Response of any status means the server answered; surface it + * verbatim rather than replay a request it may have accepted. + * - A rejected fetch does NOT prove the request never arrived — the + * connection may have dropped after delivery. Before each retry we look + * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS; finding + * one means the earlier attempt reached the platform, so the retry is + * suppressed instead of bouncing off the server's concurrency guard + * (409) or registering a duplicate deployment. */ const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; +const DEPLOY_POST_DEDUPE_WINDOW_MS = 120_000; /** * HTTP client for the Void platform API. @@ -570,7 +576,7 @@ export class PlatformClient { payload: ManifestOnlyDeployPayload, ): AsyncGenerator { const url = `${this.baseUrl}/projects/${projectId}/deploy`; - yield* this.streamDeployRequest(url, { + yield* this.streamDeployRequest(url, projectId, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -584,7 +590,11 @@ export class PlatformClient { * POST + NDJSON-stream-reader used by `finalizeDeploy()`. Captures * `cf-ray` + `cf-cache-status` for postmortem correlation. */ - private async *streamDeployRequest(url: string, init: RequestInit): AsyncGenerator { + private async *streamDeployRequest( + url: string, + projectId: string, + init: RequestInit, + ): AsyncGenerator { const cliLog = this.cliLog; cliLog?.info('deploy_post', { url }); @@ -602,6 +612,23 @@ export class PlatformClient { cliLog?.error('deploy_fetch_rejected', err, { url, attempts: attempt + 1 }); throw err; } + const existing = await this.findRecentDeployment(projectId, DEPLOY_POST_DEDUPE_WINDOW_MS); + if (existing) { + cliLog?.warn('deploy_retry_suppressed', { + url, + deploymentId: existing.id, + status: existing.status, + }); + yield { + event: 'error', + step: 'finalize', + message: + `an earlier attempt of this deploy may have created deployment ${existing.id} ` + + `(status: ${existing.status}) — not retrying to avoid a duplicate deploy; ` + + 'inspect with `void project status` before redeploying', + }; + return; + } cliLog?.warn('deploy_fetch_retry', { url, attempt: attempt + 1, @@ -683,6 +710,33 @@ export class PlatformClient { } } + /** + * Best-effort lookup for a deployment created within `windowMs` — the + * deploy retry path uses it to detect that an earlier attempt reached + * the platform before the connection dropped. Returns `undefined` on + * any failure or ambiguity: the retried POST then still runs, with the + * server's own concurrency guard (409) as the backstop. + */ + private async findRecentDeployment( + projectId: string, + windowMs: number, + ): Promise { + try { + const { deployments } = await this.listDeployments(projectId); + const newest = deployments[0]; + if (!newest) { + return undefined; + } + const createdMs = Date.parse(newest.created_at); + if (Number.isNaN(createdMs)) { + return undefined; + } + return Date.now() - createdMs <= windowMs ? newest : undefined; + } catch { + return undefined; + } + } + async listSecrets(projectId: string): Promise> { const res = await fetch(`${this.baseUrl}/projects/${projectId}/secrets`, { headers: { diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 55ed9d8ef..3f94135dc 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -1,5 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test'; -import { PlatformClient, type ManifestOnlyDeployPayload } from '../../src/cli/client.ts'; +import { + PlatformClient, + type DeploymentResponse, + type ManifestOnlyDeployPayload, +} from '../../src/cli/client.ts'; import type { DeployManifest } from '../../src/cli/types.ts'; const fetchMock = vi.fn(); @@ -238,17 +242,8 @@ describe('PlatformClient.finalizeDeploy', () => { }); }); - it('retries a transport-level fetch rejection with backoff, then streams events', async () => { - vi.useFakeTimers(); - fetchMock - .mockRejectedValueOnce(new TypeError('fetch failed', { cause: new Error('read ECONNRESET') })) - .mockResolvedValueOnce( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); - - const cliLog = { + function makeCliLog() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), @@ -256,6 +251,53 @@ describe('PlatformClient.finalizeDeploy', () => { path: '/tmp/test.jsonl', close: vi.fn(), }; + } + + function makeDeployment(overrides: Partial = {}): DeploymentResponse { + return { + id: 'dpl_existing', + status: 'pending', + source: 'manual', + commit: null, + manifest: null, + error_message: null, + retained: false, + applied_migrations: null, + created_at: new Date().toISOString(), + deployed_at: null, + ...overrides, + }; + } + + function deploymentListResponse(deployments: Array): Response { + return jsonResponse({ deployments, activeDeploymentId: null }); + } + + const deployPostCalls = () => fetchMock.mock.calls.filter(([u]) => String(u).endsWith('/deploy')); + const deploymentListCalls = () => + fetchMock.mock.calls.filter(([u]) => String(u).includes('/deployments')); + + it('retries a transport-level fetch rejection with backoff, then streams events', async () => { + vi.useFakeTimers(); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([])); + } + posts++; + if (posts === 1) { + return Promise.reject( + new TypeError('fetch failed', { cause: new Error('read ECONNRESET') }), + ); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const cliLog = makeCliLog(); const client = new PlatformClient(token, { apiUrl, cliLog }); try { const events: Array = []; @@ -267,7 +309,8 @@ describe('PlatformClient.finalizeDeploy', () => { await vi.runAllTimersAsync(); await consume; - expect(fetchMock).toHaveBeenCalledTimes(2); + expect(deployPostCalls()).toHaveLength(2); + expect(deploymentListCalls()).toHaveLength(1); expect(events).toEqual([ { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, ]); @@ -283,16 +326,14 @@ describe('PlatformClient.finalizeDeploy', () => { it('throws the original error after exhausting transport retries', async () => { vi.useFakeTimers(); - fetchMock.mockRejectedValue(new TypeError('fetch failed')); + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([])); + } + return Promise.reject(new TypeError('fetch failed')); + }); - const cliLog = { - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - path: '/tmp/test.jsonl', - close: vi.fn(), - }; + const cliLog = makeCliLog(); const client = new PlatformClient(token, { apiUrl, cliLog }); try { const consume = (async () => { @@ -304,8 +345,9 @@ describe('PlatformClient.finalizeDeploy', () => { await vi.runAllTimersAsync(); await assertion; - // 1 initial attempt + 3 retries. - expect(fetchMock).toHaveBeenCalledTimes(4); + // 1 initial attempt + 3 retries, one dedupe GET per retry. + expect(deployPostCalls()).toHaveLength(4); + expect(deploymentListCalls()).toHaveLength(3); const retries = cliLog.warn.mock.calls.filter(([msg]) => msg === 'deploy_fetch_retry'); expect(retries.map(([, fields]) => fields?.delayMs)).toEqual([1000, 5000, 10000]); expect(cliLog.error).toHaveBeenCalledWith( @@ -321,6 +363,113 @@ describe('PlatformClient.finalizeDeploy', () => { } }); + it('suppresses the retry when an earlier attempt already created a deployment', async () => { + const existing = makeDeployment({ id: 'dpl_existing', status: 'in-progress' }); + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([existing])); + } + return Promise.reject(new TypeError('fetch failed')); + }); + + const cliLog = makeCliLog(); + const client = new PlatformClient(token, { apiUrl, cliLog }); + const events: Array = []; + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + + // The POST is never replayed — the platform keeps its only deployment. + expect(deployPostCalls()).toHaveLength(1); + expect(deploymentListCalls()).toHaveLength(1); + expect(events).toEqual([ + { + event: 'error', + step: 'finalize', + message: expect.stringContaining('dpl_existing'), + }, + ]); + expect(cliLog.warn).toHaveBeenCalledWith( + 'deploy_retry_suppressed', + expect.objectContaining({ deploymentId: 'dpl_existing', status: 'in-progress' }), + ); + }); + + it('still retries when the newest deployment is older than the dedupe window', async () => { + vi.useFakeTimers(); + const stale = makeDeployment({ + created_at: new Date(Date.now() - 10 * 60_000).toISOString(), + }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([stale])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + + it('still retries when the recent-deployment check itself fails', async () => { + vi.useFakeTimers(); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.reject(new TypeError('fetch failed')); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + it('does not retry non-transport rejections (e.g. AbortError)', async () => { fetchMock.mockRejectedValueOnce(new DOMException('The operation was aborted.', 'AbortError')); const client = makeClient(); From df5c1c6e4244fa02b06b430cbc1297ef2d7fab52 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 10:22:36 +0800 Subject: [PATCH 3/9] fix(cli): correlate dedupe candidate by git commit before suppressing retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A recency-only match could wrongly attribute an unrelated deploy (a manual or CI deploy racing ours) to the lost attempt and abort the retry. The deployments table stores the payload's git commit, so require the candidate row's commit to equal ours — null===null counts since ownership cannot be disproved without metadata. A commit mismatch means the row is not ours and the POST is still replayed. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 38 +++++++---- .../void/test/unit/direct-r2-client.test.ts | 63 +++++++++++++++++++ 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 8f90a0fb6..20c68953f 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -337,10 +337,12 @@ export type ManifestOnlyDeployPayload = DeployUploadPayload; * verbatim rather than replay a request it may have accepted. * - A rejected fetch does NOT prove the request never arrived — the * connection may have dropped after delivery. Before each retry we look - * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS; finding - * one means the earlier attempt reached the platform, so the retry is - * suppressed instead of bouncing off the server's concurrency guard - * (409) or registering a duplicate deployment. + * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS whose + * recorded git commit equals this payload's (a timestamp alone could + * match an unrelated deploy racing ours). A correlated row means the + * earlier attempt reached the platform, so the retry is suppressed + * instead of bouncing off the server's concurrency guard (409) or + * registering a duplicate deployment. */ const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; @@ -576,7 +578,7 @@ export class PlatformClient { payload: ManifestOnlyDeployPayload, ): AsyncGenerator { const url = `${this.baseUrl}/projects/${projectId}/deploy`; - yield* this.streamDeployRequest(url, projectId, { + yield* this.streamDeployRequest(url, projectId, payload.commit ?? null, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -593,6 +595,7 @@ export class PlatformClient { private async *streamDeployRequest( url: string, projectId: string, + dedupeCommit: string | null, init: RequestInit, ): AsyncGenerator { const cliLog = this.cliLog; @@ -612,7 +615,11 @@ export class PlatformClient { cliLog?.error('deploy_fetch_rejected', err, { url, attempts: attempt + 1 }); throw err; } - const existing = await this.findRecentDeployment(projectId, DEPLOY_POST_DEDUPE_WINDOW_MS); + const existing = await this.findRecentDeployment( + projectId, + DEPLOY_POST_DEDUPE_WINDOW_MS, + dedupeCommit, + ); if (existing) { cliLog?.warn('deploy_retry_suppressed', { url, @@ -711,15 +718,20 @@ export class PlatformClient { } /** - * Best-effort lookup for a deployment created within `windowMs` — the - * deploy retry path uses it to detect that an earlier attempt reached - * the platform before the connection dropped. Returns `undefined` on - * any failure or ambiguity: the retried POST then still runs, with the - * server's own concurrency guard (409) as the backstop. + * Best-effort lookup for a deployment created within `windowMs` AND + * carrying `expectedCommit` — the deploy retry path uses it to detect + * that an earlier attempt reached the platform before the connection + * dropped. Commit equality is the correlation signal: a recency-only + * match could be an unrelated deploy racing ours. `null === null` + * counts (no commit metadata on either side, so ownership cannot be + * disproved). Returns `undefined` on any failure or ambiguity: the + * retried POST then still runs, with the server's own concurrency + * guard (409) as the backstop. */ private async findRecentDeployment( projectId: string, windowMs: number, + expectedCommit: string | null, ): Promise { try { const { deployments } = await this.listDeployments(projectId); @@ -731,7 +743,9 @@ export class PlatformClient { if (Number.isNaN(createdMs)) { return undefined; } - return Date.now() - createdMs <= windowMs ? newest : undefined; + return Date.now() - createdMs <= windowMs && newest.commit === expectedCommit + ? newest + : undefined; } catch { return undefined; } diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 3f94135dc..148b01b56 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -364,6 +364,7 @@ describe('PlatformClient.finalizeDeploy', () => { }); it('suppresses the retry when an earlier attempt already created a deployment', async () => { + // makeDeployment().commit === null matches makePayload().commit === null. const existing = makeDeployment({ id: 'dpl_existing', status: 'in-progress' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { @@ -395,6 +396,68 @@ describe('PlatformClient.finalizeDeploy', () => { ); }); + it('suppresses when the recent deployment carries the same commit as the payload', async () => { + const existing = makeDeployment({ id: 'dpl_same', commit: 'sha_ours' }); + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([existing])); + } + return Promise.reject(new TypeError('fetch failed')); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + const events: Array = []; + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_same'), + }); + }); + + it('retries when the recent deployment carries a different commit than the payload', async () => { + vi.useFakeTimers(); + // A racing deploy's row — not ours, so the POST must still be replayed. + const unrelated = makeDeployment({ id: 'dpl_theirs', commit: 'sha_theirs' }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([unrelated])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + it('still retries when the newest deployment is older than the dedupe window', async () => { vi.useFakeTimers(); const stale = makeDeployment({ From e9a6c42859a09960d62124e16ba9aeac8d31b842 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 10:39:18 +0800 Subject: [PATCH 4/9] fix(cli): match in-flight commit-less placeholder rows in deploy dedupe createDeployment inserts the deployment row as a concurrency claim with commit=NULL; updateDeploymentManifest fills it only after R2 inventory. For a deploy carrying a commit, our own lost attempt therefore reads commit:null during its first seconds and the equality check missed it, letting the replay hit the 409 guard. Now also treat a still-in-flight null-commit row as a possible placeholder when the payload has a commit (an unrelated in-flight deploy would reject the replay with 409 anyway). Failed/canceled rows are dead attempts and no longer suppress. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 55 ++++++--- .../void/test/unit/direct-r2-client.test.ts | 109 +++++++++++++++++- 2 files changed, 143 insertions(+), 21 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 20c68953f..ddc6706e4 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -337,12 +337,13 @@ export type ManifestOnlyDeployPayload = DeployUploadPayload; * verbatim rather than replay a request it may have accepted. * - A rejected fetch does NOT prove the request never arrived — the * connection may have dropped after delivery. Before each retry we look - * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS whose - * recorded git commit equals this payload's (a timestamp alone could - * match an unrelated deploy racing ours). A correlated row means the - * earlier attempt reached the platform, so the retry is suppressed - * instead of bouncing off the server's concurrency guard (409) or - * registering a duplicate deployment. + * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS that + * could be our earlier attempt: same git commit as this payload, or a + * still-in-flight commit-less placeholder row (the server writes the + * commit only after R2 inventory). A correlated row means the earlier + * attempt reached the platform, so the retry is suppressed instead of + * bouncing off the server's concurrency guard (409) or registering a + * duplicate deployment. */ const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; @@ -718,15 +719,24 @@ export class PlatformClient { } /** - * Best-effort lookup for a deployment created within `windowMs` AND - * carrying `expectedCommit` — the deploy retry path uses it to detect - * that an earlier attempt reached the platform before the connection - * dropped. Commit equality is the correlation signal: a recency-only - * match could be an unrelated deploy racing ours. `null === null` - * counts (no commit metadata on either side, so ownership cannot be - * disproved). Returns `undefined` on any failure or ambiguity: the - * retried POST then still runs, with the server's own concurrency - * guard (409) as the backstop. + * Best-effort lookup for a deployment created within `windowMs` that + * could be OUR earlier attempt — the deploy retry path uses it to + * detect that the lost POST reached the platform before the connection + * dropped. A candidate must be recent and either: + * + * - carry `expectedCommit` (`null === null` counts — ownership cannot + * be disproved without metadata on either side), or + * - be a still-in-flight `commit: null` row while we sent a commit: + * `createDeployment` inserts the row as a concurrency claim with + * NULL metadata and `updateDeploymentManifest` fills it only after + * R2 inventory, so our own attempt reads `commit: null` for its + * first seconds. Suppressing here is safe — an unrelated in-flight + * deploy would reject the replay with 409 anyway. + * + * A `failed`/`canceled` row is a dead attempt and never suppresses: + * replaying is exactly what we want. Returns `undefined` on any + * failure or ambiguity: the retried POST then still runs, with the + * server's own concurrency guard (409) as the backstop. */ private async findRecentDeployment( projectId: string, @@ -740,12 +750,19 @@ export class PlatformClient { return undefined; } const createdMs = Date.parse(newest.created_at); - if (Number.isNaN(createdMs)) { + if (Number.isNaN(createdMs) || Date.now() - createdMs > windowMs) { return undefined; } - return Date.now() - createdMs <= windowMs && newest.commit === expectedCommit - ? newest - : undefined; + if (newest.status === 'failed' || newest.status === 'canceled') { + return undefined; + } + if (newest.commit === expectedCommit) { + return newest; + } + if (expectedCommit !== null && newest.commit === null && newest.status !== 'live') { + return newest; + } + return undefined; } catch { return undefined; } diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 148b01b56..c6cceab8f 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -365,7 +365,7 @@ describe('PlatformClient.finalizeDeploy', () => { it('suppresses the retry when an earlier attempt already created a deployment', async () => { // makeDeployment().commit === null matches makePayload().commit === null. - const existing = makeDeployment({ id: 'dpl_existing', status: 'in-progress' }); + const existing = makeDeployment({ id: 'dpl_existing', status: 'uploading' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { return Promise.resolve(deploymentListResponse([existing])); @@ -392,7 +392,7 @@ describe('PlatformClient.finalizeDeploy', () => { ]); expect(cliLog.warn).toHaveBeenCalledWith( 'deploy_retry_suppressed', - expect.objectContaining({ deploymentId: 'dpl_existing', status: 'in-progress' }), + expect.objectContaining({ deploymentId: 'dpl_existing', status: 'uploading' }), ); }); @@ -458,6 +458,111 @@ describe('PlatformClient.finalizeDeploy', () => { } }); + it('suppresses when an in-flight placeholder row has no commit yet', async () => { + // The server inserts the deployment as a concurrency claim with + // commit=NULL and only fills it after R2 inventory — our own attempt + // reads commit:null for its first seconds. + const placeholder = makeDeployment({ id: 'dpl_placeholder', commit: null, status: 'pending' }); + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([placeholder])); + } + return Promise.reject(new TypeError('fetch failed')); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + const events: Array = []; + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_placeholder'), + }); + }); + + it('retries when the only recent commit-less deployment is already live', async () => { + vi.useFakeTimers(); + // A live null-commit row is an unrelated manual deploy — it cannot be + // our placeholder (our commit is written before reaching live). + const unrelated = makeDeployment({ id: 'dpl_live', commit: null, status: 'live' }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([unrelated])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + + it('retries when the same-commit deployment already failed', async () => { + vi.useFakeTimers(); + // A dead attempt cannot conflict — the retried POST must run. + const dead = makeDeployment({ id: 'dpl_dead', commit: 'sha_ours', status: 'failed' }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([dead])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + it('still retries when the newest deployment is older than the dedupe window', async () => { vi.useFakeTimers(); const stale = makeDeployment({ From d526b9924ded4ccbec25a57d6e40275a6a0706fb Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 11:10:08 +0800 Subject: [PATCH 5/9] fix(cli): suppress deploy retry only on in-flight rows, not commit match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit equality cannot prove ownership of a deployment row — same-SHA redeploys and commit-less manual deploys collide — so matching a recent row on commit could suppress a POST that never reached the server, silently dropping the deploy the user asked for. Suppress only when a row created inside the window has a non-terminal status: getActiveDeployment treats any such row as an active deploy and the replay would be rejected with 409 regardless of ownership, so suppression is equivalent to retrying and produces the clearer error. Terminal rows never suppress — a completed attempt instead replays as an identical re-deploy, the benign side of the ambiguity. This also drops the payload-commit plumbing, which no longer discriminates. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 94 ++++++++----------- .../void/test/unit/direct-r2-client.test.ts | 83 ++++++++++------ 2 files changed, 96 insertions(+), 81 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index ddc6706e4..7c3544ca5 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -337,13 +337,16 @@ export type ManifestOnlyDeployPayload = DeployUploadPayload; * verbatim rather than replay a request it may have accepted. * - A rejected fetch does NOT prove the request never arrived — the * connection may have dropped after delivery. Before each retry we look - * for a deployment created inside DEPLOY_POST_DEDUPE_WINDOW_MS that - * could be our earlier attempt: same git commit as this payload, or a - * still-in-flight commit-less placeholder row (the server writes the - * commit only after R2 inventory). A correlated row means the earlier - * attempt reached the platform, so the retry is suppressed instead of - * bouncing off the server's concurrency guard (409) or registering a - * duplicate deployment. + * for an IN-FLIGHT deployment row created inside + * DEPLOY_POST_DEDUPE_WINDOW_MS. Any such row — ours or a concurrent + * deploy's — makes the replay hit the server's concurrency guard + * (409), so suppressing is equivalent to retrying and yields a better + * error. Terminal rows (`live`/`failed`/`canceled`) never suppress: + * attributing them to our attempt needs a server-persisted request id + * we don't have, and misattribution would drop a deploy the user + * asked for. A completed attempt of ours instead gets replayed as an + * identical re-deploy — rare (a deploy finishing within seconds) and + * the benign side of the ambiguity. */ const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; @@ -579,7 +582,7 @@ export class PlatformClient { payload: ManifestOnlyDeployPayload, ): AsyncGenerator { const url = `${this.baseUrl}/projects/${projectId}/deploy`; - yield* this.streamDeployRequest(url, projectId, payload.commit ?? null, { + yield* this.streamDeployRequest(url, projectId, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -596,7 +599,6 @@ export class PlatformClient { private async *streamDeployRequest( url: string, projectId: string, - dedupeCommit: string | null, init: RequestInit, ): AsyncGenerator { const cliLog = this.cliLog; @@ -616,11 +618,7 @@ export class PlatformClient { cliLog?.error('deploy_fetch_rejected', err, { url, attempts: attempt + 1 }); throw err; } - const existing = await this.findRecentDeployment( - projectId, - DEPLOY_POST_DEDUPE_WINDOW_MS, - dedupeCommit, - ); + const existing = await this.findInFlightDeployment(projectId, DEPLOY_POST_DEDUPE_WINDOW_MS); if (existing) { cliLog?.warn('deploy_retry_suppressed', { url, @@ -631,8 +629,9 @@ export class PlatformClient { event: 'error', step: 'finalize', message: - `an earlier attempt of this deploy may have created deployment ${existing.id} ` + - `(status: ${existing.status}) — not retrying to avoid a duplicate deploy; ` + + `deployment ${existing.id} is already in progress (status: ${existing.status}) — ` + + 'it may be an earlier attempt of this deploy or a concurrent one; ' + + 'not retrying to avoid a duplicate deploy; ' + 'inspect with `void project status` before redeploying', }; return; @@ -719,50 +718,39 @@ export class PlatformClient { } /** - * Best-effort lookup for a deployment created within `windowMs` that - * could be OUR earlier attempt — the deploy retry path uses it to - * detect that the lost POST reached the platform before the connection - * dropped. A candidate must be recent and either: + * Best-effort lookup for a deployment created within `windowMs` whose + * status is non-terminal. The deploy retry path uses it to decide + * whether replaying the lost POST could conflict: the server's + * `getActiveDeployment` guard treats any non-terminal row + * (`live`/`failed`/`canceled` are the terminal set) as an active + * deploy and answers 409, so whenever such a row exists the replay is + * guaranteed redundant — suppressing is equivalent to retrying and + * produces the clearer error. * - * - carry `expectedCommit` (`null === null` counts — ownership cannot - * be disproved without metadata on either side), or - * - be a still-in-flight `commit: null` row while we sent a commit: - * `createDeployment` inserts the row as a concurrency claim with - * NULL metadata and `updateDeploymentManifest` fills it only after - * R2 inventory, so our own attempt reads `commit: null` for its - * first seconds. Suppressing here is safe — an unrelated in-flight - * deploy would reject the replay with 409 anyway. - * - * A `failed`/`canceled` row is a dead attempt and never suppresses: - * replaying is exactly what we want. Returns `undefined` on any - * failure or ambiguity: the retried POST then still runs, with the - * server's own concurrency guard (409) as the backstop. + * Ownership of the row is deliberately NOT inferred (commit equality + * is non-unique: same-SHA redeploys, commit-less manual deploys): a + * terminal row never suppresses, so a deploy the user asked for is + * always submitted. Returns `undefined` on any failure: the retried + * POST then still runs, with the server's guard as the backstop. */ - private async findRecentDeployment( + private async findInFlightDeployment( projectId: string, windowMs: number, - expectedCommit: string | null, ): Promise { try { const { deployments } = await this.listDeployments(projectId); - const newest = deployments[0]; - if (!newest) { - return undefined; - } - const createdMs = Date.parse(newest.created_at); - if (Number.isNaN(createdMs) || Date.now() - createdMs > windowMs) { - return undefined; - } - if (newest.status === 'failed' || newest.status === 'canceled') { - return undefined; - } - if (newest.commit === expectedCommit) { - return newest; - } - if (expectedCommit !== null && newest.commit === null && newest.status !== 'live') { - return newest; - } - return undefined; + const now = Date.now(); + return deployments.find((deployment) => { + if ( + deployment.status === 'live' || + deployment.status === 'failed' || + deployment.status === 'canceled' + ) { + return false; + } + const createdMs = Date.parse(deployment.created_at); + return Number.isFinite(createdMs) && now - createdMs <= windowMs; + }); } catch { return undefined; } diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index c6cceab8f..55de55b69 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -396,7 +396,7 @@ describe('PlatformClient.finalizeDeploy', () => { ); }); - it('suppresses when the recent deployment carries the same commit as the payload', async () => { + it('suppresses when an in-flight deployment carries the same commit as the payload', async () => { const existing = makeDeployment({ id: 'dpl_same', commit: 'sha_ours' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { @@ -419,43 +419,29 @@ describe('PlatformClient.finalizeDeploy', () => { }); }); - it('retries when the recent deployment carries a different commit than the payload', async () => { - vi.useFakeTimers(); - // A racing deploy's row — not ours, so the POST must still be replayed. + it('suppresses when an unrelated deployment is already in flight', async () => { + // Commit does not disambiguate: any in-flight row makes the replay + // hit the server's 409 guard, so suppressing is the same outcome. const unrelated = makeDeployment({ id: 'dpl_theirs', commit: 'sha_theirs' }); - let posts = 0; fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { return Promise.resolve(deploymentListResponse([unrelated])); } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); + return Promise.reject(new TypeError('fetch failed')); }); const client = makeClient(); const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); + const events: Array = []; + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); } + + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_theirs'), + }); }); it('suppresses when an in-flight placeholder row has no commit yet', async () => { @@ -524,6 +510,47 @@ describe('PlatformClient.finalizeDeploy', () => { } }); + it('retries when a same-commit deployment is already live', async () => { + vi.useFakeTimers(); + // Same-SHA redeploys are common (dirty tree, config-only): a live row + // may be an earlier SUCCESSFUL deploy of ours or an unrelated one — + // either way replaying submits the deploy the user asked for. + const prior = makeDeployment({ id: 'dpl_prior', commit: 'sha_ours', status: 'live' }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([prior])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + const payload = { ...makePayload(), commit: 'sha_ours' }; + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }); + it('retries when the same-commit deployment already failed', async () => { vi.useFakeTimers(); // A dead attempt cannot conflict — the retried POST must run. From 21271b7a6f2e456a9f4855869fc8ea931ca98cf0 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 11:19:41 +0800 Subject: [PATCH 6/9] fix(cli): recheck dedupe after backoff and skip reconcilable statuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two suppression gaps found on review: - The deployment lookup ran before the backoff sleep, so a placeholder inserted while the lost POST's precheck was still running could be missed entirely and the replay hit the 409 the dedupe exists to avoid. The lookup now runs after the sleep, immediately before the replay. - `cutover`/`compensating` rows are not guaranteed to 409: the precheck can reconcile them (completed compensation → failed, marker-verified cutover → live) and accept the new deploy, so suppressing on them could discard the requested deployment. Suppression now uses an explicit status allowlist (pending/uploading/migrating/prerendering/ canceling) — only rows whose replay is guaranteed redundant. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 63 ++++--- .../void/test/unit/direct-r2-client.test.ts | 173 +++++++++++++----- 2 files changed, 165 insertions(+), 71 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 7c3544ca5..483154374 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -351,6 +351,19 @@ export type ManifestOnlyDeployPayload = DeployUploadPayload; const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; const DEPLOY_POST_DEDUPE_WINDOW_MS = 120_000; +/** + * Non-terminal deployment statuses whose fresh row makes a replayed + * deploy POST hit the server's concurrency guard unconditionally. + * `cutover`/`compensating` are deliberately absent: the precheck can + * reconcile those and accept the deploy, so the replay must run. + */ +const DEPLOY_DEDUPE_BLOCKING_STATUSES = new Set([ + 'pending', + 'uploading', + 'migrating', + 'prerendering', + 'canceling', +]); /** * HTTP client for the Void platform API. @@ -618,6 +631,18 @@ export class PlatformClient { cliLog?.error('deploy_fetch_rejected', err, { url, attempts: attempt + 1 }); throw err; } + cliLog?.warn('deploy_fetch_retry', { + url, + attempt: attempt + 1, + delayMs: DEPLOY_POST_BACKOFF_MS[attempt], + error: err instanceof Error ? err.message : String(err), + }); + await sleep(DEPLOY_POST_BACKOFF_MS[attempt]); + attempt++; + // Recheck only now — right before the replay. The lost POST's + // placeholder may not have existed at rejection time (precheck + // still running), and the backoff gives the server time to + // insert it. const existing = await this.findInFlightDeployment(projectId, DEPLOY_POST_DEDUPE_WINDOW_MS); if (existing) { cliLog?.warn('deploy_retry_suppressed', { @@ -636,14 +661,6 @@ export class PlatformClient { }; return; } - cliLog?.warn('deploy_fetch_retry', { - url, - attempt: attempt + 1, - delayMs: DEPLOY_POST_BACKOFF_MS[attempt], - error: err instanceof Error ? err.message : String(err), - }); - await sleep(DEPLOY_POST_BACKOFF_MS[attempt]); - attempt++; } } @@ -719,19 +736,23 @@ export class PlatformClient { /** * Best-effort lookup for a deployment created within `windowMs` whose - * status is non-terminal. The deploy retry path uses it to decide - * whether replaying the lost POST could conflict: the server's - * `getActiveDeployment` guard treats any non-terminal row - * (`live`/`failed`/`canceled` are the terminal set) as an active - * deploy and answers 409, so whenever such a row exists the replay is - * guaranteed redundant — suppressing is equivalent to retrying and - * produces the clearer error. + * status makes a replayed POST redundant — the deploy retry path uses + * it to decide whether suppressing is equivalent to retrying. + * + * A row counts only when its status guarantees the server's precheck + * rejects the replay with 409 (`getActiveDeployment` → + * `ConcurrentDeployError` for any fresh active row). `cutover` and + * `compensating` are excluded: the precheck can reconcile those + * (completed compensation → failed, marker-verified cutover → live) + * and accept the new deploy, so a replay is NOT guaranteed redundant + * and must reach the server. Terminal rows (`live`/`failed`/ + * `canceled`) likewise never block a replay. * * Ownership of the row is deliberately NOT inferred (commit equality * is non-unique: same-SHA redeploys, commit-less manual deploys): a - * terminal row never suppresses, so a deploy the user asked for is - * always submitted. Returns `undefined` on any failure: the retried - * POST then still runs, with the server's guard as the backstop. + * deploy the user asked for is always submitted. Returns `undefined` + * on any failure: the retried POST then still runs, with the server's + * guard as the backstop. */ private async findInFlightDeployment( projectId: string, @@ -741,11 +762,7 @@ export class PlatformClient { const { deployments } = await this.listDeployments(projectId); const now = Date.now(); return deployments.find((deployment) => { - if ( - deployment.status === 'live' || - deployment.status === 'failed' || - deployment.status === 'canceled' - ) { + if (!DEPLOY_DEDUPE_BLOCKING_STATUSES.has(deployment.status)) { return false; } const createdMs = Date.parse(deployment.created_at); diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 55de55b69..0cfc322f1 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -364,7 +364,7 @@ describe('PlatformClient.finalizeDeploy', () => { }); it('suppresses the retry when an earlier attempt already created a deployment', async () => { - // makeDeployment().commit === null matches makePayload().commit === null. + vi.useFakeTimers(); const existing = makeDeployment({ id: 'dpl_existing', status: 'uploading' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { @@ -375,28 +375,37 @@ describe('PlatformClient.finalizeDeploy', () => { const cliLog = makeCliLog(); const client = new PlatformClient(token, { apiUrl, cliLog }); - const events: Array = []; - for await (const event of client.finalizeDeploy(projectId, makePayload())) { - events.push(event); - } + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; - // The POST is never replayed — the platform keeps its only deployment. - expect(deployPostCalls()).toHaveLength(1); - expect(deploymentListCalls()).toHaveLength(1); - expect(events).toEqual([ - { - event: 'error', - step: 'finalize', - message: expect.stringContaining('dpl_existing'), - }, - ]); - expect(cliLog.warn).toHaveBeenCalledWith( - 'deploy_retry_suppressed', - expect.objectContaining({ deploymentId: 'dpl_existing', status: 'uploading' }), - ); + // The POST is never replayed — the platform keeps its only deployment. + expect(deployPostCalls()).toHaveLength(1); + expect(deploymentListCalls()).toHaveLength(1); + expect(events).toEqual([ + { + event: 'error', + step: 'finalize', + message: expect.stringContaining('dpl_existing'), + }, + ]); + expect(cliLog.warn).toHaveBeenCalledWith( + 'deploy_retry_suppressed', + expect.objectContaining({ deploymentId: 'dpl_existing', status: 'uploading' }), + ); + } finally { + vi.useRealTimers(); + } }); it('suppresses when an in-flight deployment carries the same commit as the payload', async () => { + vi.useFakeTimers(); const existing = makeDeployment({ id: 'dpl_same', commit: 'sha_ours' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { @@ -407,21 +416,30 @@ describe('PlatformClient.finalizeDeploy', () => { const client = makeClient(); const payload = { ...makePayload(), commit: 'sha_ours' }; - const events: Array = []; - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_same'), - }); + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_same'), + }); + } finally { + vi.useRealTimers(); + } }); it('suppresses when an unrelated deployment is already in flight', async () => { - // Commit does not disambiguate: any in-flight row makes the replay - // hit the server's 409 guard, so suppressing is the same outcome. + vi.useFakeTimers(); + // Commit does not disambiguate: any blocking-status row makes the + // replay hit the server's 409 guard, so suppressing is equivalent. const unrelated = makeDeployment({ id: 'dpl_theirs', commit: 'sha_theirs' }); fetchMock.mockImplementation((url: string) => { if (String(url).includes('/deployments')) { @@ -432,19 +450,28 @@ describe('PlatformClient.finalizeDeploy', () => { const client = makeClient(); const payload = { ...makePayload(), commit: 'sha_ours' }; - const events: Array = []; - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_theirs'), - }); + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_theirs'), + }); + } finally { + vi.useRealTimers(); + } }); it('suppresses when an in-flight placeholder row has no commit yet', async () => { + vi.useFakeTimers(); // The server inserts the deployment as a concurrency claim with // commit=NULL and only fills it after R2 inventory — our own attempt // reads commit:null for its first seconds. @@ -458,18 +485,68 @@ describe('PlatformClient.finalizeDeploy', () => { const client = makeClient(); const payload = { ...makePayload(), commit: 'sha_ours' }; - const events: Array = []; - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, payload)) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_placeholder'), - }); + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_placeholder'), + }); + } finally { + vi.useRealTimers(); + } }); + it.each(['cutover', 'compensating'])( + 'retries when the recent deployment is %s — the server can reconcile it', + async (status) => { + vi.useFakeTimers(); + // The precheck can finish a marker-verified cutover or a completed + // compensation and then accept the new deploy — the replay must run. + const reconcilable = makeDeployment({ id: 'dpl_reconcilable', status }); + let posts = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('/deployments')) { + return Promise.resolve(deploymentListResponse([reconcilable])); + } + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); + } + return Promise.resolve( + ndjsonStreamResponse([ + { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, + ]), + ); + }); + + const client = makeClient(); + try { + const events: Array = []; + const consume = (async () => { + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); + } + })(); + await vi.runAllTimersAsync(); + await consume; + + expect(deployPostCalls()).toHaveLength(2); + expect(events[0]).toMatchObject({ event: 'done' }); + } finally { + vi.useRealTimers(); + } + }, + ); + it('retries when the only recent commit-less deployment is already live', async () => { vi.useFakeTimers(); // A live null-commit row is an unrelated manual deploy — it cannot be From 25884e55cf7ebe2e53e4a2134fd6ea616aaeb556 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 11:33:06 +0800 Subject: [PATCH 7/9] fix(cli): parse deployment timestamps as UTC before deduping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Postgres `timestamp` mode:'string' values (e.g. `2026-09-18 12:00:00`) carry no zone marker, so `Date.parse` read them as local time: in UTC+N a fresh row looked N hours old and the dedupe missed it; in UTC−N a stale row could appear to be in the future and wrongly suppress. Normalize offset-less values as UTC, mirroring the platform's `databaseTimestampMs` helper. Tests now emit the real wire format. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 21 +++++++++++++++++-- .../void/test/unit/direct-r2-client.test.ts | 5 +++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 483154374..859500bd6 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -365,6 +365,23 @@ const DEPLOY_DEDUPE_BLOCKING_STATUSES = new Set([ 'canceling', ]); +/** + * Parse a PostgreSQL `timestamp` column value (`mode: 'string'` emits + * `2026-09-18 12:00:00` — no zone marker, stored in UTC). `Date.parse` + * alone would read it as LOCAL time, skewing comparisons by the host's + * offset. Mirrors the platform's `databaseTimestampMs` helper. + */ +function databaseTimestampMs(value: string): number | null { + const withSeparator = value.includes('T') ? value : value.replace(' ', 'T'); + const withZone = /[+-]\d{2}$/.test(withSeparator) + ? `${withSeparator}:00` + : /(?:Z|[+-]\d{2}:?\d{2})$/i.test(withSeparator) + ? withSeparator + : `${withSeparator}Z`; + const parsed = new Date(withZone).getTime(); + return Number.isFinite(parsed) ? parsed : null; +} + /** * HTTP client for the Void platform API. */ @@ -765,8 +782,8 @@ export class PlatformClient { if (!DEPLOY_DEDUPE_BLOCKING_STATUSES.has(deployment.status)) { return false; } - const createdMs = Date.parse(deployment.created_at); - return Number.isFinite(createdMs) && now - createdMs <= windowMs; + const createdMs = databaseTimestampMs(deployment.created_at); + return createdMs !== null && now - createdMs <= windowMs; }); } catch { return undefined; diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 0cfc322f1..94c810166 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -263,7 +263,8 @@ describe('PlatformClient.finalizeDeploy', () => { error_message: null, retained: false, applied_migrations: null, - created_at: new Date().toISOString(), + // Postgres `timestamp` mode:'string' wire format — no zone marker. + created_at: new Date().toISOString().replace('T', ' ').slice(0, 19), deployed_at: null, ...overrides, }; @@ -670,7 +671,7 @@ describe('PlatformClient.finalizeDeploy', () => { it('still retries when the newest deployment is older than the dedupe window', async () => { vi.useFakeTimers(); const stale = makeDeployment({ - created_at: new Date(Date.now() - 10 * 60_000).toISOString(), + created_at: new Date(Date.now() - 10 * 60_000).toISOString().replace('T', ' ').slice(0, 19), }); let posts = 0; fetchMock.mockImplementation((url: string) => { From 04d3929f3c3516521219816f2173255ea9868a6b Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 11:49:14 +0800 Subject: [PATCH 8/9] fix(cli): let the server's precheck arbitrate deploy retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client-side GET-then-decide dedupe can never be authoritative — the deployment row can transition between the lookup and the replay, so any suppression rule carries an unavoidable race that can drop the deploy the user asked for. Remove the /deployments lookup, status allowlist, and timestamp-window machinery entirely. The retried POST now always reaches the server's precheck — the only atomic, project-locked verdict. A 409 conflict ({error, deployment}) is surfaced as a structured error event naming the deployment id instead of a bare HTTP failure. Residual: a lost attempt completing within seconds replays as an identical re-deploy — the benign side of not being able to prove ownership client-side. Addresses Codex review on PR #327. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 142 ++---- .../void/test/unit/direct-r2-client.test.ts | 418 ++---------------- 2 files changed, 78 insertions(+), 482 deletions(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index 859500bd6..b56af6e1a 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -336,51 +336,17 @@ export type ManifestOnlyDeployPayload = DeployUploadPayload; * A Response of any status means the server answered; surface it * verbatim rather than replay a request it may have accepted. * - A rejected fetch does NOT prove the request never arrived — the - * connection may have dropped after delivery. Before each retry we look - * for an IN-FLIGHT deployment row created inside - * DEPLOY_POST_DEDUPE_WINDOW_MS. Any such row — ours or a concurrent - * deploy's — makes the replay hit the server's concurrency guard - * (409), so suppressing is equivalent to retrying and yields a better - * error. Terminal rows (`live`/`failed`/`canceled`) never suppress: - * attributing them to our attempt needs a server-persisted request id - * we don't have, and misattribution would drop a deploy the user - * asked for. A completed attempt of ours instead gets replayed as an - * identical re-deploy — rare (a deploy finishing within seconds) and - * the benign side of the ambiguity. + * connection may have dropped after delivery. Arbitration is left to + * the server's precheck, the only atomic (project-locked) verdict: if + * an earlier attempt or a concurrent deploy is still in flight, the + * replay is rejected with 409 and surfaced as a deploy conflict + * naming the deployment id. A client-side GET-then-decide lookup can + * never be authoritative (the row can transition between lookup and + * replay), so no dedupe query is made — the residual case is the lost + * attempt completing within seconds and being redeployed identically. */ const DEPLOY_POST_MAX_RETRIES = 3; const DEPLOY_POST_BACKOFF_MS = [1000, 5000, 10000]; -const DEPLOY_POST_DEDUPE_WINDOW_MS = 120_000; -/** - * Non-terminal deployment statuses whose fresh row makes a replayed - * deploy POST hit the server's concurrency guard unconditionally. - * `cutover`/`compensating` are deliberately absent: the precheck can - * reconcile those and accept the deploy, so the replay must run. - */ -const DEPLOY_DEDUPE_BLOCKING_STATUSES = new Set([ - 'pending', - 'uploading', - 'migrating', - 'prerendering', - 'canceling', -]); - -/** - * Parse a PostgreSQL `timestamp` column value (`mode: 'string'` emits - * `2026-09-18 12:00:00` — no zone marker, stored in UTC). `Date.parse` - * alone would read it as LOCAL time, skewing comparisons by the host's - * offset. Mirrors the platform's `databaseTimestampMs` helper. - */ -function databaseTimestampMs(value: string): number | null { - const withSeparator = value.includes('T') ? value : value.replace(' ', 'T'); - const withZone = /[+-]\d{2}$/.test(withSeparator) - ? `${withSeparator}:00` - : /(?:Z|[+-]\d{2}:?\d{2})$/i.test(withSeparator) - ? withSeparator - : `${withSeparator}Z`; - const parsed = new Date(withZone).getTime(); - return Number.isFinite(parsed) ? parsed : null; -} /** * HTTP client for the Void platform API. @@ -656,28 +622,6 @@ export class PlatformClient { }); await sleep(DEPLOY_POST_BACKOFF_MS[attempt]); attempt++; - // Recheck only now — right before the replay. The lost POST's - // placeholder may not have existed at rejection time (precheck - // still running), and the backoff gives the server time to - // insert it. - const existing = await this.findInFlightDeployment(projectId, DEPLOY_POST_DEDUPE_WINDOW_MS); - if (existing) { - cliLog?.warn('deploy_retry_suppressed', { - url, - deploymentId: existing.id, - status: existing.status, - }); - yield { - event: 'error', - step: 'finalize', - message: - `deployment ${existing.id} is already in progress (status: ${existing.status}) — ` + - 'it may be an earlier attempt of this deploy or a concurrent one; ' + - 'not retrying to avoid a duplicate deploy; ' + - 'inspect with `void project status` before redeploying', - }; - return; - } } } @@ -694,7 +638,36 @@ export class PlatformClient { }); if (!res.ok) { - await throwPlatformApiError('Deploy failed', res); + const body = await res.text(); + if (res.status === 409) { + // The precheck's authoritative verdict — a deployment is already + // in flight (possibly the earlier attempt of this same deploy, + // or a concurrent one). Surface it as a structured error event + // naming the deployment rather than a bare HTTP failure. + let parsed: { error?: string; deployment?: string } | null = null; + try { + parsed = JSON.parse(body) as typeof parsed; + } catch { + // Non-JSON 409 — fall through to the generic error below. + } + if (parsed?.deployment) { + cliLog?.warn('deploy_conflict', { + url, + deploymentId: parsed.deployment, + error: parsed.error, + }); + yield { + event: 'error', + step: 'finalize', + message: + `${parsed.error ?? 'deploy in progress'}: deployment ${parsed.deployment} — ` + + 'it may be an earlier attempt of this deploy or a concurrent one; ' + + 'inspect with `void project status` before redeploying', + }; + return; + } + } + throw new PlatformApiError('Deploy failed', res.status, body); } const reader = res.body!.getReader(); @@ -751,45 +724,6 @@ export class PlatformClient { } } - /** - * Best-effort lookup for a deployment created within `windowMs` whose - * status makes a replayed POST redundant — the deploy retry path uses - * it to decide whether suppressing is equivalent to retrying. - * - * A row counts only when its status guarantees the server's precheck - * rejects the replay with 409 (`getActiveDeployment` → - * `ConcurrentDeployError` for any fresh active row). `cutover` and - * `compensating` are excluded: the precheck can reconcile those - * (completed compensation → failed, marker-verified cutover → live) - * and accept the new deploy, so a replay is NOT guaranteed redundant - * and must reach the server. Terminal rows (`live`/`failed`/ - * `canceled`) likewise never block a replay. - * - * Ownership of the row is deliberately NOT inferred (commit equality - * is non-unique: same-SHA redeploys, commit-less manual deploys): a - * deploy the user asked for is always submitted. Returns `undefined` - * on any failure: the retried POST then still runs, with the server's - * guard as the backstop. - */ - private async findInFlightDeployment( - projectId: string, - windowMs: number, - ): Promise { - try { - const { deployments } = await this.listDeployments(projectId); - const now = Date.now(); - return deployments.find((deployment) => { - if (!DEPLOY_DEDUPE_BLOCKING_STATUSES.has(deployment.status)) { - return false; - } - const createdMs = databaseTimestampMs(deployment.created_at); - return createdMs !== null && now - createdMs <= windowMs; - }); - } catch { - return undefined; - } - } - async listSecrets(projectId: string): Promise> { const res = await fetch(`${this.baseUrl}/projects/${projectId}/secrets`, { headers: { diff --git a/packages/void/test/unit/direct-r2-client.test.ts b/packages/void/test/unit/direct-r2-client.test.ts index 94c810166..6b82986aa 100644 --- a/packages/void/test/unit/direct-r2-client.test.ts +++ b/packages/void/test/unit/direct-r2-client.test.ts @@ -1,9 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test'; -import { - PlatformClient, - type DeploymentResponse, - type ManifestOnlyDeployPayload, -} from '../../src/cli/client.ts'; +import { PlatformClient, type ManifestOnlyDeployPayload } from '../../src/cli/client.ts'; import type { DeployManifest } from '../../src/cli/types.ts'; const fetchMock = vi.fn(); @@ -253,27 +249,6 @@ describe('PlatformClient.finalizeDeploy', () => { }; } - function makeDeployment(overrides: Partial = {}): DeploymentResponse { - return { - id: 'dpl_existing', - status: 'pending', - source: 'manual', - commit: null, - manifest: null, - error_message: null, - retained: false, - applied_migrations: null, - // Postgres `timestamp` mode:'string' wire format — no zone marker. - created_at: new Date().toISOString().replace('T', ' ').slice(0, 19), - deployed_at: null, - ...overrides, - }; - } - - function deploymentListResponse(deployments: Array): Response { - return jsonResponse({ deployments, activeDeploymentId: null }); - } - const deployPostCalls = () => fetchMock.mock.calls.filter(([u]) => String(u).endsWith('/deploy')); const deploymentListCalls = () => fetchMock.mock.calls.filter(([u]) => String(u).includes('/deployments')); @@ -281,10 +256,7 @@ describe('PlatformClient.finalizeDeploy', () => { it('retries a transport-level fetch rejection with backoff, then streams events', async () => { vi.useFakeTimers(); let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([])); - } + fetchMock.mockImplementation(() => { posts++; if (posts === 1) { return Promise.reject( @@ -311,7 +283,8 @@ describe('PlatformClient.finalizeDeploy', () => { await consume; expect(deployPostCalls()).toHaveLength(2); - expect(deploymentListCalls()).toHaveLength(1); + // No client-side dedupe GET — the server's precheck is authoritative. + expect(deploymentListCalls()).toHaveLength(0); expect(events).toEqual([ { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, ]); @@ -327,12 +300,7 @@ describe('PlatformClient.finalizeDeploy', () => { it('throws the original error after exhausting transport retries', async () => { vi.useFakeTimers(); - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([])); - } - return Promise.reject(new TypeError('fetch failed')); - }); + fetchMock.mockImplementation(() => Promise.reject(new TypeError('fetch failed'))); const cliLog = makeCliLog(); const client = new PlatformClient(token, { apiUrl, cliLog }); @@ -346,9 +314,9 @@ describe('PlatformClient.finalizeDeploy', () => { await vi.runAllTimersAsync(); await assertion; - // 1 initial attempt + 3 retries, one dedupe GET per retry. + // 1 initial attempt + 3 retries — no dedupe GETs. expect(deployPostCalls()).toHaveLength(4); - expect(deploymentListCalls()).toHaveLength(3); + expect(deploymentListCalls()).toHaveLength(0); const retries = cliLog.warn.mock.calls.filter(([msg]) => msg === 'deploy_fetch_retry'); expect(retries.map(([, fields]) => fields?.delayMs)).toEqual([1000, 5000, 10000]); expect(cliLog.error).toHaveBeenCalledWith( @@ -364,14 +332,19 @@ describe('PlatformClient.finalizeDeploy', () => { } }); - it('suppresses the retry when an earlier attempt already created a deployment', async () => { + it('surfaces the server 409 as an error event when a deploy is already in progress', async () => { vi.useFakeTimers(); - const existing = makeDeployment({ id: 'dpl_existing', status: 'uploading' }); - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([existing])); + // The replay hits the authoritative concurrency guard — possibly the + // earlier attempt of this same deploy, or a concurrent one. + let posts = 0; + fetchMock.mockImplementation(() => { + posts++; + if (posts === 1) { + return Promise.reject(new TypeError('fetch failed')); } - return Promise.reject(new TypeError('fetch failed')); + return Promise.resolve( + jsonResponse({ error: 'deploy in progress', deployment: 'dpl_conflict' }, 409), + ); }); const cliLog = makeCliLog(); @@ -386,361 +359,50 @@ describe('PlatformClient.finalizeDeploy', () => { await vi.runAllTimersAsync(); await consume; - // The POST is never replayed — the platform keeps its only deployment. - expect(deployPostCalls()).toHaveLength(1); - expect(deploymentListCalls()).toHaveLength(1); + expect(deployPostCalls()).toHaveLength(2); expect(events).toEqual([ { event: 'error', step: 'finalize', - message: expect.stringContaining('dpl_existing'), + message: expect.stringContaining('dpl_conflict'), }, ]); expect(cliLog.warn).toHaveBeenCalledWith( - 'deploy_retry_suppressed', - expect.objectContaining({ deploymentId: 'dpl_existing', status: 'uploading' }), + 'deploy_conflict', + expect.objectContaining({ deploymentId: 'dpl_conflict' }), ); } finally { vi.useRealTimers(); } }); - it('suppresses when an in-flight deployment carries the same commit as the payload', async () => { - vi.useFakeTimers(); - const existing = makeDeployment({ id: 'dpl_same', commit: 'sha_ours' }); - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([existing])); - } - return Promise.reject(new TypeError('fetch failed')); - }); - - const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_same'), - }); - } finally { - vi.useRealTimers(); - } - }); - - it('suppresses when an unrelated deployment is already in flight', async () => { - vi.useFakeTimers(); - // Commit does not disambiguate: any blocking-status row makes the - // replay hit the server's 409 guard, so suppressing is equivalent. - const unrelated = makeDeployment({ id: 'dpl_theirs', commit: 'sha_theirs' }); - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([unrelated])); - } - return Promise.reject(new TypeError('fetch failed')); - }); - - const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_theirs'), - }); - } finally { - vi.useRealTimers(); - } - }); - - it('suppresses when an in-flight placeholder row has no commit yet', async () => { - vi.useFakeTimers(); - // The server inserts the deployment as a concurrency claim with - // commit=NULL and only fills it after R2 inventory — our own attempt - // reads commit:null for its first seconds. - const placeholder = makeDeployment({ id: 'dpl_placeholder', commit: null, status: 'pending' }); - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([placeholder])); - } - return Promise.reject(new TypeError('fetch failed')); - }); - - const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(1); - expect(events[0]).toMatchObject({ - event: 'error', - message: expect.stringContaining('dpl_placeholder'), - }); - } finally { - vi.useRealTimers(); - } - }); - - it.each(['cutover', 'compensating'])( - 'retries when the recent deployment is %s — the server can reconcile it', - async (status) => { - vi.useFakeTimers(); - // The precheck can finish a marker-verified cutover or a completed - // compensation and then accept the new deploy — the replay must run. - const reconcilable = makeDeployment({ id: 'dpl_reconcilable', status }); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([reconcilable])); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); - }); - - const client = makeClient(); - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, makePayload())) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); - } - }, - ); - - it('retries when the only recent commit-less deployment is already live', async () => { - vi.useFakeTimers(); - // A live null-commit row is an unrelated manual deploy — it cannot be - // our placeholder (our commit is written before reaching live). - const unrelated = makeDeployment({ id: 'dpl_live', commit: null, status: 'live' }); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([unrelated])); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); - }); - - const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); - } - }); - - it('retries when a same-commit deployment is already live', async () => { - vi.useFakeTimers(); - // Same-SHA redeploys are common (dirty tree, config-only): a live row - // may be an earlier SUCCESSFUL deploy of ours or an unrelated one — - // either way replaying submits the deploy the user asked for. - const prior = makeDeployment({ id: 'dpl_prior', commit: 'sha_ours', status: 'live' }); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([prior])); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); - }); + it('surfaces a first-attempt 409 without retrying', async () => { + fetchMock.mockResolvedValue( + jsonResponse({ error: 'deploy in progress', deployment: 'dpl_busy' }, 409), + ); const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); + const events: Array = []; + for await (const event of client.finalizeDeploy(projectId, makePayload())) { + events.push(event); } - }); - it('retries when the same-commit deployment already failed', async () => { - vi.useFakeTimers(); - // A dead attempt cannot conflict — the retried POST must run. - const dead = makeDeployment({ id: 'dpl_dead', commit: 'sha_ours', status: 'failed' }); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([dead])); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); + expect(deployPostCalls()).toHaveLength(1); + expect(events[0]).toMatchObject({ + event: 'error', + message: expect.stringContaining('dpl_busy'), }); - - const client = makeClient(); - const payload = { ...makePayload(), commit: 'sha_ours' }; - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, payload)) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); - } }); - it('still retries when the newest deployment is older than the dedupe window', async () => { - vi.useFakeTimers(); - const stale = makeDeployment({ - created_at: new Date(Date.now() - 10 * 60_000).toISOString().replace('T', ' ').slice(0, 19), - }); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.resolve(deploymentListResponse([stale])); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); - }); - + it('throws the platform error for a 409 without a deployment id', async () => { + fetchMock.mockResolvedValue(new Response('conflict', { status: 409 })); const client = makeClient(); - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, makePayload())) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); - } - }); - - it('still retries when the recent-deployment check itself fails', async () => { - vi.useFakeTimers(); - let posts = 0; - fetchMock.mockImplementation((url: string) => { - if (String(url).includes('/deployments')) { - return Promise.reject(new TypeError('fetch failed')); - } - posts++; - if (posts === 1) { - return Promise.reject(new TypeError('fetch failed')); - } - return Promise.resolve( - ndjsonStreamResponse([ - { event: 'done', deployment: 'd1', url: 'https://x', urls: [], assets: 0, workers: 0 }, - ]), - ); + const gen = client.finalizeDeploy(projectId, makePayload()); + await expect(gen.next()).rejects.toMatchObject({ + name: 'PlatformApiError', + status: 409, }); - - const client = makeClient(); - try { - const events: Array = []; - const consume = (async () => { - for await (const event of client.finalizeDeploy(projectId, makePayload())) { - events.push(event); - } - })(); - await vi.runAllTimersAsync(); - await consume; - - expect(deployPostCalls()).toHaveLength(2); - expect(events[0]).toMatchObject({ event: 'done' }); - } finally { - vi.useRealTimers(); - } + expect(fetchMock).toHaveBeenCalledTimes(1); }); it('does not retry non-transport rejections (e.g. AbortError)', async () => { From b6832dd1e166c55def003bc9c34608d9c6075c14 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 18 Sep 2026 13:39:36 +0800 Subject: [PATCH 9/9] fix(cli): avoid self-referential typeof narrowing in 409 body parse `parsed = JSON.parse(body) as typeof parsed` let the declared type query resolve to the narrowed `null` under stricter checking, collapsing the variable to `never` and failing CI typecheck (TS2339 at the conflict handling site). Parse into `unknown` and narrow explicitly instead. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- packages/void/src/cli/client.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/void/src/cli/client.ts b/packages/void/src/cli/client.ts index b56af6e1a..3b1040c75 100644 --- a/packages/void/src/cli/client.ts +++ b/packages/void/src/cli/client.ts @@ -646,7 +646,10 @@ export class PlatformClient { // naming the deployment rather than a bare HTTP failure. let parsed: { error?: string; deployment?: string } | null = null; try { - parsed = JSON.parse(body) as typeof parsed; + const raw: unknown = JSON.parse(body); + if (raw !== null && typeof raw === 'object') { + parsed = raw as { error?: string; deployment?: string }; + } } catch { // Non-JSON 409 — fall through to the generic error below. }