From d31e4785bcfeb24b9618cfc6297898fce6b87547 Mon Sep 17 00:00:00 2001 From: David Berlin Date: Tue, 7 Jul 2026 14:22:59 +0300 Subject: [PATCH] fix(deploy): skip deploy creation when the source zip is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When deploying with `--upload-source-zip`, the deploy record was created before the source zip was built. If the zip came back empty ("zip error: Nothing to do!" — everything matched an ignore pattern or the working tree was empty), the deploy was left dangling in the UI (the build path didn't even cancel it) and the command failed with a generic error. Now the source zip is built *first*; the deploy is only created when it is non-empty. When empty, the command exits with code 12 (mirroring zip's own "nothing to do" exit code) so callers can detect this non-failure outcome. `uploadSourceZip` is split into `createSourceZip` / `uploadSourceZipFile` (+ `cleanupSourceZip`), and a typed `EmptySourceZipError` is introduced. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/deploy/deploy.ts | 63 ++- src/utils/deploy/upload-source-zip.ts | 184 ++++---- .../utils/deploy/upload-source-zip.test.ts | 394 ++++-------------- 3 files changed, 239 insertions(+), 402 deletions(-) diff --git a/src/commands/deploy/deploy.ts b/src/commands/deploy/deploy.ts index 2fbcb3211b0..20af763a883 100644 --- a/src/commands/deploy/deploy.ts +++ b/src/commands/deploy/deploy.ts @@ -40,7 +40,7 @@ import { } from '../../utils/command-helpers.js' import { DEFAULT_CONCURRENT_HASH, DEFAULT_DEPLOY_TIMEOUT } from '../../utils/deploy/constants.js' import { type DeployEvent, deploySite } from '../../utils/deploy/deploy-site.js' -import { uploadSourceZip } from '../../utils/deploy/upload-source-zip.js' +import { EmptySourceZipError, createSourceZip, uploadSourceZip } from '../../utils/deploy/upload-source-zip.js' import { getEnvelopeEnv } from '../../utils/env/index.js' import { getFunctionsManifestPath, getInternalFunctionsDir } from '../../utils/functions/index.js' import { isEmpty } from '../../utils/object-utilities.js' @@ -297,6 +297,33 @@ const SEC_TO_MILLISEC = 1e3 // 100 bytes const SYNC_FILE_LIMIT = 1e2 +// Exit code used when a source-zip deploy has nothing to upload (the source is +// empty after applying ignore patterns). Mirrors `zip`'s own "nothing to do" +// exit code (12) so upstream tooling can distinguish it from a real failure. +const EMPTY_SOURCE_ZIP_EXIT_CODE = 12 + +// Builds the source zip *before* the deploy is created, so an empty source +// exits (code 12) without ever creating a dangling deploy. Returns the zip path, +// or undefined when source-zip upload isn't requested. +const buildSourceZipOrExit = async ( + enabled: boolean | undefined, + sourceDir: string | undefined, + statusCb: (status: DeployEvent) => void, +): Promise => { + if (!enabled || !sourceDir) { + return undefined + } + try { + return await createSourceZip({ sourceDir, statusCb }) + } catch (error) { + if (error instanceof EmptySourceZipError) { + warn(error.message) + return exit(EMPTY_SOURCE_ZIP_EXIT_CODE) + } + throw error + } +} + // Helper function to generate copy-pasteable deploy command const generateDeployCommand = ( options: DeployOptionValues, @@ -579,6 +606,12 @@ const runDeploy = async ({ // We won't have a deploy ID if we run the command with `--no-build`. // In this case, we must create the deploy. if (!deployId) { + const sourceZipPath = await buildSourceZipOrExit( + options.uploadSourceZip, + site.root, + silent ? () => {} : deployProgressCb(), + ) + if (deployToProduction) { await prepareProductionDeploy({ siteData, api, options, command }) } @@ -594,17 +627,13 @@ const runDeploy = async ({ const createDeployResponse = await api.createSiteDeploy({ siteId, title, body: createDeployBody }) deployId = createDeployResponse.id as string - if ( - options.uploadSourceZip && - createDeployResponse.source_zip_upload_url && - createDeployResponse.source_zip_filename - ) { - uploadSourceZipResult = await uploadSourceZip({ - sourceDir: site.root, + if (sourceZipPath && createDeployResponse.source_zip_upload_url && createDeployResponse.source_zip_filename) { + await uploadSourceZip({ + zipPath: sourceZipPath, uploadUrl: createDeployResponse.source_zip_upload_url, - filename: createDeployResponse.source_zip_filename, statusCb: silent ? () => {} : deployProgressCb(), }) + uploadSourceZipResult = { sourceZipFileName: createDeployResponse.source_zip_filename } } } @@ -1361,6 +1390,12 @@ export const deploy = async (options: DeployOptionValues, command: BaseCommand) let results = {} as Awaited> if (options.build) { + const sourceZipPath = await buildSourceZipOrExit( + options.uploadSourceZip, + site.root, + options.json || options.silent ? () => {} : deployProgressCb(), + ) + if (deployToProduction) { await prepareProductionDeploy({ siteData, api, options, command }) } @@ -1386,16 +1421,10 @@ export const deploy = async (options: DeployOptionValues, command: BaseCommand) const skewProtectionToken = deployMetadata.skew_protection_token let sourceZipFileName: string | undefined - if ( - options.uploadSourceZip && - deployMetadata.source_zip_upload_url && - deployMetadata.source_zip_filename && - site.root - ) { + if (sourceZipPath && deployMetadata.source_zip_upload_url && deployMetadata.source_zip_filename) { await uploadSourceZip({ - sourceDir: site.root, + zipPath: sourceZipPath, uploadUrl: deployMetadata.source_zip_upload_url, - filename: deployMetadata.source_zip_filename, statusCb: options.json || options.silent ? () => {} : deployProgressCb(), }) sourceZipFileName = deployMetadata.source_zip_filename diff --git a/src/utils/deploy/upload-source-zip.ts b/src/utils/deploy/upload-source-zip.ts index bae8842796c..a7ab323d100 100644 --- a/src/utils/deploy/upload-source-zip.ts +++ b/src/utils/deploy/upload-source-zip.ts @@ -1,6 +1,5 @@ -import { mkdir, readFile } from 'node:fs/promises' +import { readFile, rm } from 'node:fs/promises' import { join, dirname } from 'node:path' -import type { PathLike } from 'node:fs' import { platform } from 'node:os' import execa, { ExecaError } from 'execa' @@ -10,11 +9,18 @@ import { log, warn } from '../command-helpers.js' import { temporaryDirectory } from '../temporary-file.js' import type { DeployEvent } from './status-cb.js' -interface UploadSourceZipOptions { - sourceDir: string - uploadUrl: string - filename: string - statusCb?: (status: DeployEvent) => void +/** + * Thrown when the source zip would be empty — every file matched an ignore + * pattern (or the directory is empty), so `zip` exits with code 12 ("nothing to + * do"). This is not a failure: there is simply nothing to deploy. Callers decide + * how to surface it (the deploy command exits with a dedicated code so upstream + * tooling can detect it). + */ +export class EmptySourceZipError extends Error { + constructor() { + super('Source zip is empty: no files to deploy after applying ignore patterns') + this.name = 'EmptySourceZipError' + } } const DEFAULT_IGNORE_PATTERNS = [ @@ -40,55 +46,99 @@ const DEFAULT_IGNORE_PATTERNS = [ '.temp*', ] -const createSourceZip = async ({ +// `zip` exits with code 12 ("nothing to do") when no files were added to the +// archive (everything matched an exclude, or the directory is empty). +const isEmptyZipError = (error: unknown): boolean => { + const err = error as { exitCode?: number; all?: string; stderr?: string } | null + if (!err) { + return false + } + return err.exitCode === 12 || /nothing to do/iu.test(err.all ?? err.stderr ?? '') +} + +/** + * Creates the source zip in a temporary directory and returns its path. The + * caller uploads it via `uploadSourceZip`, which removes the temporary directory + * afterwards. On any failure (including an empty archive) the directory is + * removed here before throwing. + * + * Throws `EmptySourceZipError` when the archive would be empty, so the caller + * can avoid creating a deploy for a source that has nothing to upload. + */ +export const createSourceZip = async ({ sourceDir, - filename, - statusCb, + statusCb = () => {}, }: { sourceDir: string - filename: string - statusCb: (status: DeployEvent) => void -}) => { - // Check for Windows - this feature is not supported on Windows - if (platform() === 'win32') { - throw new Error('Source zip upload is not supported on Windows') - } + statusCb?: (status: DeployEvent) => void +}): Promise => { + const zipPath = join(temporaryDirectory(), 'source.zip') - const tmpDir = temporaryDirectory() - const zipPath = join(tmpDir, filename) + try { + // Check for Windows - this feature is not supported on Windows + if (platform() === 'win32') { + throw new Error('Source zip upload is not supported on Windows') + } - // Ensure the directory for the zip file exists - // The filename from the API includes a subdirectory path (e.g., 'workspace-snapshots/source-xxx.zip') - // While temporaryDirectory() creates a new empty directory, the subdirectory within it doesn't exist - // so we need to create it before the zip command can write the file - await mkdir(dirname(zipPath), { recursive: true }) + statusCb({ + type: 'source-zip-upload', + msg: `Creating source zip...`, + phase: 'start', + }) - statusCb({ - type: 'source-zip-upload', - msg: `Creating source zip...`, - phase: 'start', - }) + // Create exclusion list for zip command + const excludeArgs = DEFAULT_IGNORE_PATTERNS.flatMap((pattern) => ['-x', pattern]) - // Create exclusion list for zip command - const excludeArgs = DEFAULT_IGNORE_PATTERNS.flatMap((pattern) => ['-x', pattern]) + // Use system zip command to create the archive + try { + await execa('zip', ['-r', '-q', zipPath, '.', ...excludeArgs], { + all: true, + cwd: sourceDir, + stdio: ['ignore', 'pipe', 'pipe'], + }) + } catch (baseErr) { + // An empty archive is an expected outcome, not a failure — let the caller decide. + if (isEmptyZipError(baseErr)) { + throw new EmptySourceZipError() + } - // Use system zip command to create the archive - try { - await execa('zip', ['-r', '-q', zipPath, '.', ...excludeArgs], { - all: true, - cwd: sourceDir, - stdio: ['ignore', 'pipe', 'pipe'], - }) - } catch (_baseErr) { - let message = 'zip command failed' - if (_baseErr instanceof Error && 'command' in _baseErr) { - const baseErr = _baseErr as ExecaError - message = `${baseErr.shortMessage}\n\n${baseErr.all ?? ''}` + let message = 'zip command failed' + if (baseErr instanceof Error && 'command' in baseErr) { + const execaErr = baseErr as ExecaError + message = `${execaErr.shortMessage}\n\n${execaErr.all ?? ''}` + } + throw new Error(message, { cause: baseErr }) + } + + return zipPath + } catch (error) { + // Nothing usable was produced; drop the temp directory we created. + await removeSourceZipDir(zipPath) + + // An empty source zip is not a reported failure. + if (error instanceof EmptySourceZipError) { + throw error } - throw new Error(message, { cause: _baseErr }) + + const errorMsg = error instanceof Error ? error.message : String(error) + statusCb({ + type: 'source-zip-upload', + msg: `Failed to create source zip: ${errorMsg}`, + phase: 'error', + }) + warn(`Failed to create source zip: ${errorMsg}`) + throw error } +} - return zipPath +// Removes the temporary directory that holds a source zip. Best-effort: +// cleanup failures are ignored. +const removeSourceZipDir = async (zipPath: string): Promise => { + try { + await rm(dirname(zipPath), { recursive: true, force: true }) + } catch { + // Ignore cleanup errors + } } const uploadZipToS3 = async (zipPath: string, uploadUrl: string, statusCb: (status: DeployEvent) => void) => { @@ -115,35 +165,22 @@ const uploadZipToS3 = async (zipPath: string, uploadUrl: string, statusCb: (stat } } +/** + * Uploads a source zip previously created by `createSourceZip` and removes its + * temporary directory afterwards. + */ export const uploadSourceZip = async ({ - sourceDir, + zipPath, uploadUrl, - filename, statusCb = () => {}, -}: UploadSourceZipOptions): Promise<{ sourceZipFileName: string }> => { - let zipPath: PathLike | undefined - +}: { + zipPath: string + uploadUrl: string + statusCb?: (status: DeployEvent) => void +}): Promise => { try { - // Create zip from source directory - try { - zipPath = await createSourceZip({ sourceDir, filename, statusCb }) - } catch (error) { - const errorMsg = error instanceof Error ? error.message : String(error) - statusCb({ - type: 'source-zip-upload', - msg: `Failed to create source zip: ${errorMsg}`, - phase: 'error', - }) - warn(`Failed to create source zip: ${errorMsg}`) - throw error - } - - let sourceZipFileName: string - - // Upload to S3 try { await uploadZipToS3(zipPath, uploadUrl, statusCb) - sourceZipFileName = filename } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error) statusCb({ @@ -162,16 +199,7 @@ export const uploadSourceZip = async ({ }) log(`✔ Source code uploaded`) - - return { sourceZipFileName } } finally { - // Clean up temporary zip file - if (zipPath) { - try { - await (await import('node:fs/promises')).unlink(zipPath as unknown as PathLike) - } catch { - // Ignore cleanup errors - } - } + await removeSourceZipDir(zipPath) } } diff --git a/tests/unit/utils/deploy/upload-source-zip.test.ts b/tests/unit/utils/deploy/upload-source-zip.test.ts index 1bb93c588ee..fb8e2a76f95 100644 --- a/tests/unit/utils/deploy/upload-source-zip.test.ts +++ b/tests/unit/utils/deploy/upload-source-zip.test.ts @@ -15,8 +15,7 @@ vi.mock('execa', () => ({ vi.mock('fs/promises', () => ({ readFile: vi.fn(), - unlink: vi.fn(), - mkdir: vi.fn(), + rm: vi.fn(), })) vi.mock('../../../../src/utils/command-helpers.js', () => ({ @@ -33,162 +32,52 @@ vi.mock('os', () => ({ platform: vi.fn().mockReturnValue('darwin'), })) -describe('uploadSourceZip', () => { - beforeEach(() => { - vi.clearAllMocks() - }) +const resolveExeca = async () => { + const mockExeca = await import('execa') + // @ts-expect-error(types): the execa overloads are awkward to satisfy in a mock + vi.mocked(mockExeca.default).mockImplementation(() => Promise.resolve({} as ExecaReturnValue)) +} - test('creates zip and uploads successfully', async () => { - // Ensure OS platform mock returns non-Windows +const rejectExeca = async (error: Error) => { + const mockExeca = await import('execa') + // @ts-expect-error(types): the execa overloads are awkward to satisfy in a mock + vi.mocked(mockExeca.default).mockImplementation(() => Promise.reject(error)) +} + +describe('createSourceZip', () => { + beforeEach(async () => { + vi.clearAllMocks() const mockOs = await import('os') vi.mocked(mockOs.platform).mockReturnValue('darwin') - - // Import after mocks are set up - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - - // Setup mocks using vi.mocked() - const mockFetch = await import('node-fetch') - const mockExeca = await import('execa') - const mockFs = await import('fs/promises') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - vi.mocked(mockFetch.default).mockResolvedValue({ - ok: true, - status: 200, - statusText: 'OK', - } as unknown as Response) - - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return Promise.resolve({} as ExecaReturnValue) - }) - - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockCommandHelpers.log).mockImplementation(() => {}) vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') + }) - const mockStatusCb = vi.fn() + test('creates the zip and returns its path', async () => { + await resolveExeca() + const { createSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') + const mockExeca = await import('execa') - await uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }) + const statusCb = vi.fn() + const zipPath = await createSourceZip({ sourceDir: '/test/source', statusCb }) + expect(zipPath).toBe(join('/tmp/test-temp-dir', 'source.zip')) expect(mockExeca.default).toHaveBeenCalledWith( 'zip', - expect.arrayContaining(['-r', '-q', expect.stringMatching(/test-source\.zip$/), '.']), + expect.arrayContaining(['-r', '-q', join('/tmp/test-temp-dir', 'source.zip'), '.']), expect.objectContaining({ cwd: '/test/source' }), ) - - expect(mockFetch.default).toHaveBeenCalledWith( - 'https://s3.example.com/upload-url', - expect.objectContaining({ - method: 'PUT', - body: Buffer.from('mock zip content'), - }), - ) - - expect(mockStatusCb).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'source-zip-upload', - msg: 'Creating source zip...', - phase: 'start', - }), + expect(statusCb).toHaveBeenCalledWith( + expect.objectContaining({ type: 'source-zip-upload', msg: 'Creating source zip...', phase: 'start' }), ) - - expect(mockFs.unlink).toHaveBeenCalledWith(expect.stringMatching(/test-source\.zip$/)) - expect(mockCommandHelpers.log).toHaveBeenCalledWith('✔ Source code uploaded') }) - test('handles upload failure correctly', async () => { - // Ensure OS platform mock returns non-Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - - const mockFetch = await import('node-fetch') + test('includes the default exclusion patterns', async () => { + await resolveExeca() + const { createSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') const mockExeca = await import('execa') - const mockFs = await import('fs/promises') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - vi.mocked(mockFetch.default).mockResolvedValue({ - ok: false, - status: 403, - statusText: 'Forbidden', - } as unknown as Response) - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return Promise.resolve({} as ExecaReturnValue) - }) - - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockCommandHelpers.warn).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') - - const mockStatusCb = vi.fn() - - await expect( - uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }), - ).rejects.toThrow('Failed to upload zip: Forbidden') - - expect(mockStatusCb).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'source-zip-upload', - phase: 'error', - msg: expect.stringContaining('Failed to upload source zip') as unknown as string, - }), - ) - - expect(mockCommandHelpers.warn).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source zip')) - }) - - test('includes proper exclusion patterns in zip command', async () => { - // Ensure OS platform mock returns non-Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - - const mockFetch = await import('node-fetch') - const mockExeca = await import('execa') - const mockFs = await import('fs/promises') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - vi.mocked(mockFetch.default).mockResolvedValue({ - ok: true, - status: 200, - statusText: 'OK', - } as unknown as Response) - - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return Promise.resolve({} as ExecaReturnValue) - }) - - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockCommandHelpers.log).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') - - const mockStatusCb = vi.fn() - - await uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }) + await createSourceZip({ sourceDir: '/test/source' }) expect(mockExeca.default).toHaveBeenCalledWith( 'zip', @@ -197,213 +86,104 @@ describe('uploadSourceZip', () => { ) }) - test('throws error on Windows platform', async () => { - // Mock OS platform to return Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('win32') + test('throws EmptySourceZipError when zip exits 12 (nothing to do)', async () => { + await rejectExeca( + Object.assign(new Error('Command failed with exit code 12'), { + exitCode: 12, + all: 'zip error: Nothing to do! (source.zip)', + }), + ) + const { createSourceZip, EmptySourceZipError } = await import('../../../../src/utils/deploy/upload-source-zip.js') + const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') + const mockFs = await import('fs/promises') - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') + await expect(createSourceZip({ sourceDir: '/test/source' })).rejects.toBeInstanceOf(EmptySourceZipError) + // An empty zip is not a reported failure. + expect(mockCommandHelpers.warn).not.toHaveBeenCalled() + // The temp directory is cleaned up even though nothing was produced. + expect(mockFs.rm).toHaveBeenCalledWith('/tmp/test-temp-dir', { recursive: true, force: true }) + }) - const mockStatusCb = vi.fn() + test('reports and rethrows other zip failures', async () => { + await rejectExeca(new Error('zip command failed')) + const { createSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') + const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - await expect( - uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }), - ).rejects.toThrow('Source zip upload is not supported on Windows') + const statusCb = vi.fn() + await expect(createSourceZip({ sourceDir: '/test/source', statusCb })).rejects.toThrow('zip command failed') - // Should call error status callback - expect(mockStatusCb).toHaveBeenCalledWith( + expect(statusCb).toHaveBeenCalledWith( expect.objectContaining({ type: 'source-zip-upload', phase: 'error', - msg: 'Failed to create source zip: Source zip upload is not supported on Windows', + msg: 'Failed to create source zip: zip command failed', }), ) + expect(mockCommandHelpers.warn).toHaveBeenCalledWith('Failed to create source zip: zip command failed') }) - test('handles zip creation failure correctly', async () => { - // Ensure OS platform mock returns non-Windows + test('throws on Windows platform', async () => { const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - - const mockExeca = await import('execa') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - // Mock execFile to simulate failure - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return Promise.reject(new Error('zip command failed')) - }) - - vi.mocked(mockCommandHelpers.warn).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') - - const mockStatusCb = vi.fn() + vi.mocked(mockOs.platform).mockReturnValue('win32') - await expect( - uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }), - ).rejects.toThrow('zip command failed') + const { createSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - expect(mockStatusCb).toHaveBeenCalledWith( + const statusCb = vi.fn() + await expect(createSourceZip({ sourceDir: '/test/source', statusCb })).rejects.toThrow( + 'Source zip upload is not supported on Windows', + ) + expect(statusCb).toHaveBeenCalledWith( expect.objectContaining({ type: 'source-zip-upload', phase: 'error', - msg: 'Failed to create source zip: zip command failed', + msg: 'Failed to create source zip: Source zip upload is not supported on Windows', }), ) - - expect(mockCommandHelpers.warn).toHaveBeenCalledWith('Failed to create source zip: zip command failed') }) +}) - test('cleans up zip file even when upload fails', async () => { - // Ensure OS platform mock returns non-Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - - const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - - const mockFetch = await import('node-fetch') - const mockExeca = await import('execa') +describe('uploadSourceZip', () => { + beforeEach(async () => { + vi.clearAllMocks() const mockFs = await import('fs/promises') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - // Mock successful zip creation but failed upload - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return Promise.resolve({} as ExecaReturnValue) - }) - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockFetch.default).mockResolvedValue({ - ok: false, - status: 500, - statusText: 'Internal Server Error', - } as unknown as import('node-fetch').Response) - - vi.mocked(mockCommandHelpers.warn).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') - vi.mocked(mockFs.unlink).mockResolvedValue(undefined) - - const mockStatusCb = vi.fn() - - await expect( - uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - statusCb: mockStatusCb, - }), - ).rejects.toThrow('Failed to upload zip: Internal Server Error') - - // Should still attempt cleanup - expect(mockFs.unlink).toHaveBeenCalledWith(expect.stringMatching(/test-source\.zip$/)) + vi.mocked(mockFs.rm).mockResolvedValue(undefined) }) - test('handles no status callback gracefully', async () => { - // Ensure OS platform mock returns non-Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - + test('uploads the zip and removes its temp dir', async () => { const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - const mockFetch = await import('node-fetch') - const mockExeca = await import('execa') const mockFs = await import('fs/promises') const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') - - vi.mocked(mockFetch.default).mockResolvedValue({ - ok: true, - status: 200, - statusText: 'OK', - json: vi.fn().mockResolvedValue({ url: 'https://test-source-zip-url.com' }), - } as unknown as import('node-fetch').Response) - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return {} as ExecaReturnValue - }) + vi.mocked(mockFetch.default).mockResolvedValue({ ok: true, status: 200, statusText: 'OK' } as unknown as Response) - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockCommandHelpers.log).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') + const statusCb = vi.fn() + await uploadSourceZip({ zipPath: '/tmp/test-temp-dir/source.zip', uploadUrl: 'https://s3/upload', statusCb }) - // Should not throw when no status callback provided - const result = await uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'test-source.zip', - // No statusCb provided - should use default empty function - }) - - expect(result).toHaveProperty('sourceZipFileName') + expect(mockFetch.default).toHaveBeenCalledWith( + 'https://s3/upload', + expect.objectContaining({ method: 'PUT', body: Buffer.from('mock zip content') }), + ) + expect(mockFs.rm).toHaveBeenCalledWith('/tmp/test-temp-dir', { recursive: true, force: true }) + expect(mockCommandHelpers.log).toHaveBeenCalledWith('✔ Source code uploaded') }) - test('creates subdirectories when filename includes path', async () => { - // Ensure OS platform mock returns non-Windows - const mockOs = await import('os') - vi.mocked(mockOs.platform).mockReturnValue('darwin') - + test('throws on upload failure but still removes its temp dir', async () => { const { uploadSourceZip } = await import('../../../../src/utils/deploy/upload-source-zip.js') - const mockFetch = await import('node-fetch') - const mockExeca = await import('execa') const mockFs = await import('fs/promises') - const mockCommandHelpers = await import('../../../../src/utils/command-helpers.js') - const mockTempFile = await import('../../../../src/utils/temporary-file.js') vi.mocked(mockFetch.default).mockResolvedValue({ - ok: true, - status: 200, - statusText: 'OK', + ok: false, + status: 500, + statusText: 'Internal Server Error', } as unknown as Response) - // @ts-expect-error(ndhoule): getting the type on this fairly challenging - vi.mocked(mockExeca.default).mockImplementation((..._args) => { - return {} as ExecaReturnValue - }) - - vi.mocked(mockFs.readFile).mockResolvedValue(Buffer.from('mock zip content')) - vi.mocked(mockFs.mkdir).mockResolvedValue(undefined) - vi.mocked(mockCommandHelpers.log).mockImplementation(() => {}) - vi.mocked(mockTempFile.temporaryDirectory).mockReturnValue('/tmp/test-temp-dir') - - const mockStatusCb = vi.fn() - - // Test with a filename that includes a subdirectory path (like the API provides) - await uploadSourceZip({ - sourceDir: '/test/source', - uploadUrl: 'https://s3.example.com/upload-url', - filename: 'workspace-snapshots/source-abc123-def456.zip', - statusCb: mockStatusCb, - }) - - // Should create the subdirectory before attempting zip creation - expect(mockFs.mkdir).toHaveBeenCalledWith(join('/tmp/test-temp-dir', 'workspace-snapshots'), { recursive: true }) + await expect( + uploadSourceZip({ zipPath: '/tmp/test-temp-dir/source.zip', uploadUrl: 'https://s3/upload' }), + ).rejects.toThrow('Failed to upload zip: Internal Server Error') - // Should still call zip command with the full path - expect(mockExeca.default).toHaveBeenCalledWith( - 'zip', - expect.arrayContaining([ - '-r', - '-q', - join('/tmp/test-temp-dir', 'workspace-snapshots', 'source-abc123-def456.zip'), - '.', - ]), - expect.objectContaining({ cwd: '/test/source' }), - ) + expect(mockFs.rm).toHaveBeenCalledWith('/tmp/test-temp-dir', { recursive: true, force: true }) }) })