diff --git a/src/commands/compute/deploy.test.ts b/src/commands/compute/deploy.test.ts index 559fd90..b79fa78 100644 --- a/src/commands/compute/deploy.test.ts +++ b/src/commands/compute/deploy.test.ts @@ -1,10 +1,20 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'; import type * as ErrorsModule from '../../lib/errors.js'; +import type * as ConfigModule from '../../lib/config.js'; + +const PROJECT_ID = '6cdb996f-c696-429b-b9a9-d5abd114dce5'; const ossFetchMock = vi.hoisted(() => vi.fn()); vi.mock('../../lib/api/oss.js', () => ({ ossFetch: ossFetchMock })); vi.mock('../../lib/credentials.js', () => ({ requireAuth: vi.fn().mockResolvedValue(undefined) })); vi.mock('../../lib/skills.js', () => ({ reportCliUsage: vi.fn() })); +vi.mock('../../lib/config.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getProjectConfig: () => ({ project_id: '6cdb996f-c696-429b-b9a9-d5abd114dce5' }), + }; +}); vi.mock('../../lib/errors.js', async (importOriginal) => { const actual = await importOriginal(); return { @@ -135,3 +145,83 @@ describe('compute deploy --always-on / --scale-to-zero', () => { ).rejects.toThrow(/mutually exclusive/); }); }); + +describe('compute deploy stale fly-registry image guard', () => { + const OWN_IMAGE = `registry.fly.io/hospet-api-${PROJECT_ID}:cli-1783654786759`; + + function makeCmd() { + const cmd = new Command(); + cmd.exitOverride(); + const compute = cmd.command('compute'); + registerComputeDeployCommand(compute); + return cmd; + } + + beforeEach(() => { + ossFetchMock.mockReset(); + }); + + it('fails fast (before any create call) when a fresh create references its own service registry', async () => { + ossFetchMock.mockResolvedValueOnce({ json: async () => [] }); // list: no existing service + await expect( + makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'deploy', + '--image', OWN_IMAGE, '--name', 'hospet-api', + ]) + ).rejects.toThrow(/deleting a service deletes its registry images/); + // Only the list call went out — no POST that would hang for 60s server-side. + expect(ossFetchMock).toHaveBeenCalledTimes(1); + }); + + it('does not block fresh creates on foreign registry.fly.io images or other registries', async () => { + for (const image of [`registry.fly.io/other-svc-${PROJECT_ID}:v1`, 'redis:7-alpine']) { + ossFetchMock.mockReset(); + ossFetchMock.mockResolvedValueOnce({ json: async () => [] }); + ossFetchMock.mockResolvedValueOnce({ + json: async () => ({ name: 'hospet-api', status: 'creating' }), + }); + await makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'deploy', + '--image', image, '--name', 'hospet-api', + ]); + expect(ossFetchMock).toHaveBeenCalledTimes(2); + } + }); + + it('appends the stale-image hint when an update of an existing service times out', async () => { + const { CLIError } = await vi.importActual('../../lib/errors.js'); + ossFetchMock.mockResolvedValueOnce({ + json: async () => [{ id: 'svc-1', name: 'hospet-api' }], + }); + ossFetchMock.mockRejectedValueOnce( + new CLIError( + 'COMPUTE_CLOUD_UNAVAILABLE: The operation was aborted due to timeout', + 1, + 'COMPUTE_CLOUD_UNAVAILABLE', + 503 + ) + ); + await expect( + makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'deploy', + '--image', OWN_IMAGE, '--name', 'hospet-api', + ]) + ).rejects.toThrow(/Rebuild and push a fresh image by deploying from source/); + }); + + it('leaves timeout errors on non-fly images untouched', async () => { + const { CLIError } = await vi.importActual('../../lib/errors.js'); + ossFetchMock.mockResolvedValueOnce({ json: async () => [] }); + ossFetchMock.mockRejectedValueOnce(new CLIError('OSS request failed: 504', 1, undefined, 504)); + let caught: unknown; + try { + await makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'deploy', + '--image', 'redis:7-alpine', '--name', 'cache', + ]); + } catch (err) { + caught = err; + } + expect(String((caught as Error).message)).not.toContain('Hint:'); + }); +}); diff --git a/src/commands/compute/deploy.ts b/src/commands/compute/deploy.ts index c99be92..4aa32a9 100644 --- a/src/commands/compute/deploy.ts +++ b/src/commands/compute/deploy.ts @@ -2,6 +2,7 @@ import { existsSync } from 'node:fs'; import { join, resolve } from 'node:path'; import type { Command } from 'commander'; import { ossFetch } from '../../lib/api/oss.js'; +import { getProjectConfig } from '../../lib/config.js'; import { requireAuth } from '../../lib/credentials.js'; import { handleError, getRootOpts, CLIError } from '../../lib/errors.js'; import { outputJson, outputSuccess, outputInfo } from '../../lib/output.js'; @@ -12,6 +13,10 @@ import { ensureFlyctlAvailable, flyctlBuildAndPush, } from '../../lib/flyctl.js'; +import { + imageBelongsToOwnService, + withStaleImageHint, +} from '../../lib/fly-registry.js'; // `compute deploy` has two modes: // @@ -148,21 +153,49 @@ export function registerComputeDeployCommand(computeCmd: Command): void { (s) => s.name === opts.name ); + // A registry.fly.io image whose repo is this service's own Fly app + // (`-`) cannot exist when the service doesn't: + // deleting a service destroys the app and its registry images with + // it. Fail fast instead of letting the platform spin in + // MANIFEST_UNKNOWN retries until a timeout that reads as a cloud + // outage. + if (!existing) { + const config = getProjectConfig(); + const projectIds = [ + config?.project_id, + config?.branched_from?.project_id, + ].filter((id): id is string => Boolean(id)); + if (imageBelongsToOwnService(String(opts.image), opts.name, projectIds)) { + throw new CLIError( + `Image ${opts.image} lives in the registry of the Fly app that backs service ` + + `"${opts.name}" — but that service doesn't exist, so the image is gone ` + + `(deleting a service deletes its registry images). Deploying this reference ` + + `will always fail.\n` + + `Rebuild and push a fresh image by deploying from source:\n` + + ` npx @insforge/cli compute deploy --name ${opts.name}` + ); + } + } + let res; - if (existing) { - if (!json) outputInfo(`Found existing service "${opts.name}", updating...`); - const updateBody: Record = { ...body }; - delete updateBody.name; - if (opts.protocol === 'tcp') updateBody.protocol = 'tcp'; - res = await ossFetch(`/api/compute/services/${encodeURIComponent(existing.id)}`, { - method: 'PATCH', - body: JSON.stringify(updateBody), - }); - } else { - res = await ossFetch('/api/compute/services', { - method: 'POST', - body: JSON.stringify(body), - }); + try { + if (existing) { + if (!json) outputInfo(`Found existing service "${opts.name}", updating...`); + const updateBody: Record = { ...body }; + delete updateBody.name; + if (opts.protocol === 'tcp') updateBody.protocol = 'tcp'; + res = await ossFetch(`/api/compute/services/${encodeURIComponent(existing.id)}`, { + method: 'PATCH', + body: JSON.stringify(updateBody), + }); + } else { + res = await ossFetch('/api/compute/services', { + method: 'POST', + body: JSON.stringify(body), + }); + } + } catch (deployErr) { + throw withStaleImageHint(deployErr, String(opts.image), opts.name); } const service = (await res.json()) as Record; diff --git a/src/commands/compute/update.test.ts b/src/commands/compute/update.test.ts new file mode 100644 index 0000000..3102eff --- /dev/null +++ b/src/commands/compute/update.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import type * as ErrorsModule from '../../lib/errors.js'; + +const ossFetchMock = vi.hoisted(() => vi.fn()); +vi.mock('../../lib/api/oss.js', () => ({ ossFetch: ossFetchMock })); +vi.mock('../../lib/credentials.js', () => ({ requireAuth: vi.fn().mockResolvedValue(undefined) })); +vi.mock('../../lib/skills.js', () => ({ reportCliUsage: vi.fn() })); +vi.mock('../../lib/errors.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + handleError: (err: unknown) => { throw err; }, + }; +}); + +import { Command } from 'commander'; +import { registerComputeUpdateCommand } from './update.js'; + +describe('compute update stale fly-registry image hint', () => { + const FLY_IMAGE = 'registry.fly.io/hospet-api-6cdb996f-c696-429b-b9a9-d5abd114dce5:cli-1783654786759'; + + function makeCmd() { + const cmd = new Command(); + cmd.exitOverride(); + const compute = cmd.command('compute'); + registerComputeUpdateCommand(compute); + return cmd; + } + + async function timeoutError() { + const { CLIError } = await vi.importActual('../../lib/errors.js'); + return new CLIError( + 'COMPUTE_CLOUD_UNAVAILABLE: The operation was aborted due to timeout', + 1, + 'COMPUTE_CLOUD_UNAVAILABLE', + 503 + ); + } + + beforeEach(() => { + ossFetchMock.mockReset(); + }); + + it('appends the hint when a PATCH with --image on a fly registry ref times out', async () => { + ossFetchMock.mockRejectedValueOnce(await timeoutError()); + await expect( + makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'update', 'svc-1', '--image', FLY_IMAGE, + ]) + ).rejects.toThrow(/Rebuild and push a fresh image by deploying from source/); + }); + + it('leaves timeouts unhinted when no --image was supplied (stored-image gap)', async () => { + ossFetchMock.mockRejectedValueOnce(await timeoutError()); + let caught: unknown; + try { + await makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'update', 'svc-1', '--memory', '1024', + ]); + } catch (err) { + caught = err; + } + expect(String((caught as Error).message)).not.toContain('Hint:'); + }); + + it('leaves timeouts unhinted for non-fly images', async () => { + ossFetchMock.mockRejectedValueOnce(await timeoutError()); + let caught: unknown; + try { + await makeCmd().parseAsync([ + 'node', 'lim', 'compute', 'update', 'svc-1', '--image', 'redis:7-alpine', + ]); + } catch (err) { + caught = err; + } + expect(String((caught as Error).message)).not.toContain('Hint:'); + }); +}); diff --git a/src/commands/compute/update.ts b/src/commands/compute/update.ts index 6910639..fdabf44 100644 --- a/src/commands/compute/update.ts +++ b/src/commands/compute/update.ts @@ -5,6 +5,7 @@ import { handleError, getRootOpts, CLIError } from '../../lib/errors.js'; import { outputJson, outputSuccess } from '../../lib/output.js'; import { reportCliUsage } from '../../lib/skills.js'; import { trackCommandUsage } from '../../lib/command-telemetry.js'; +import { withStaleImageHint } from '../../lib/fly-registry.js'; const ENV_KEY_REGEX = /^[A-Z_][A-Z0-9_]*$/; @@ -122,10 +123,23 @@ export function registerComputeUpdateCommand(computeCmd: Command): void { ); } - const res = await ossFetch(`/api/compute/services/${encodeURIComponent(id)}`, { - method: 'PATCH', - body: JSON.stringify(body), - }); + let res; + try { + res = await ossFetch(`/api/compute/services/${encodeURIComponent(id)}`, { + method: 'PATCH', + body: JSON.stringify(body), + }); + } catch (updateErr) { + // A timeout on a registry.fly.io image usually means the tag was + // deleted along with a previous service — say so instead of + // "cloud unavailable". Known gap: an update without --image + // relaunches against the service's *stored* image, which can be a + // dead registry.fly.io tag too, but the CLI doesn't know that URL + // without an extra fetch — those timeouts pass through unhinted. + throw opts.image + ? withStaleImageHint(updateErr, String(opts.image)) + : updateErr; + } const service = await res.json() as Record; await trackCommandUsage('compute', 'update', true); diff --git a/src/lib/fly-registry.test.ts b/src/lib/fly-registry.test.ts new file mode 100644 index 0000000..d75cd1d --- /dev/null +++ b/src/lib/fly-registry.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from 'vitest'; +import { CLIError } from './errors.js'; +import { + flyRegistryRepo, + imageBelongsToOwnService, + staleFlyImageHint, + withStaleImageHint, +} from './fly-registry.js'; + +const PROJECT_ID = '6cdb996f-c696-429b-b9a9-d5abd114dce5'; +const OWN_IMAGE = `registry.fly.io/hospet-api-${PROJECT_ID}:cli-1783654786759`; + +describe('flyRegistryRepo', () => { + it('extracts the repo from a plain registry.fly.io ref', () => { + expect(flyRegistryRepo(OWN_IMAGE)).toBe(`hospet-api-${PROJECT_ID}`); + }); + + it('tolerates docker:// and https:// prefixes and digests', () => { + expect(flyRegistryRepo(`docker://registry.fly.io/foo:latest`)).toBe('foo'); + expect(flyRegistryRepo(`https://registry.fly.io/foo@sha256:abc`)).toBe('foo'); + }); + + it('returns null for other registries', () => { + expect(flyRegistryRepo('redis:7-alpine')).toBeNull(); + expect(flyRegistryRepo('ghcr.io/acme/api:v1')).toBeNull(); + expect(flyRegistryRepo('docker.io/library/nginx')).toBeNull(); + // registry.fly.io as a path segment, not the host + expect(flyRegistryRepo('example.com/registry.fly.io/foo')).toBeNull(); + }); +}); + +describe('imageBelongsToOwnService', () => { + it('matches only the exact - repo', () => { + expect(imageBelongsToOwnService(OWN_IMAGE, 'hospet-api', [PROJECT_ID])).toBe(true); + // another service's repo that shares a name prefix must NOT match + expect( + imageBelongsToOwnService( + `registry.fly.io/hospet-api-gateway-${PROJECT_ID}:v1`, + 'hospet-api', + [PROJECT_ID] + ) + ).toBe(false); + // same name, different project — could be a live app elsewhere + expect( + imageBelongsToOwnService(OWN_IMAGE, 'hospet-api', ['00000000-0000-0000-0000-000000000000']) + ).toBe(false); + expect(imageBelongsToOwnService('redis:7-alpine', 'hospet-api', [PROJECT_ID])).toBe(false); + }); +}); + +describe('withStaleImageHint', () => { + const timeoutErr = () => + new CLIError('OSS request failed: 504', 1, undefined, 504); + + it('appends the hint for timeout-ish failures on fly registry images', () => { + const wrapped = withStaleImageHint(timeoutErr(), OWN_IMAGE, 'hospet-api') as CLIError; + expect(wrapped.message).toContain('deleted together with'); + expect(wrapped.message).toContain('--name hospet-api'); + expect(wrapped.statusCode).toBe(504); + }); + + it('matches COMPUTE_CLOUD_UNAVAILABLE by code and 502/503 by status', () => { + for (const err of [ + new CLIError('unavailable', 1, 'COMPUTE_CLOUD_UNAVAILABLE', 503), + new CLIError('bad gateway', 1, undefined, 502), + new CLIError('unavailable', 1, undefined, 503), + ]) { + const wrapped = withStaleImageHint(err, OWN_IMAGE, 'x') as CLIError; + expect(wrapped.message).toContain('Hint:'); + } + }); + + it('passes through non-timeout errors, non-fly images, and non-CLIErrors', () => { + const quota = new CLIError('quota exceeded', 1, 'COMPUTE_QUOTA_EXCEEDED', 403); + expect(withStaleImageHint(quota, OWN_IMAGE, 'x')).toBe(quota); + + const dockerhubTimeout = timeoutErr(); + expect(withStaleImageHint(dockerhubTimeout, 'redis:7', 'x')).toBe(dockerhubTimeout); + + const plain = new Error('boom'); + expect(withStaleImageHint(plain, OWN_IMAGE, 'x')).toBe(plain); + }); + + it('falls back to a placeholder when the service name is unknown', () => { + expect(staleFlyImageHint()).toContain('--name '); + }); +}); diff --git a/src/lib/fly-registry.ts b/src/lib/fly-registry.ts new file mode 100644 index 0000000..23f90d2 --- /dev/null +++ b/src/lib/fly-registry.ts @@ -0,0 +1,75 @@ +// registry.fly.io repositories live and die with the Fly app they belong to: +// deleting a compute service destroys its Fly app AND every image ever pushed +// to that app's registry. A cached `--image registry.fly.io/:` +// reference therefore goes permanently stale the moment the service is +// deleted — and redeploying it makes the platform spin in MANIFEST_UNKNOWN +// retries until the request times out as a misleading COMPUTE_CLOUD_UNAVAILABLE. +// These helpers let the deploy/update commands catch that before (or explain +// it after) the round-trip. + +import { CLIError } from './errors.js'; + +/** Extract the repository name from a registry.fly.io image URL, or null if + * the image lives in any other registry. Tolerates docker://, https://, a + * :tag suffix, and an @sha256 digest. */ +export function flyRegistryRepo(imageUrl: string): string | null { + const m = /^(?:docker:\/\/|https?:\/\/)?registry\.fly\.io\/([^:@/\s]+)/i.exec(imageUrl.trim()); + return m ? m[1] : null; +} + +/** True when the image reference points at the registry of the Fly app that + * backs this very service (`-`). If that service + * does not exist, neither does the app — so the registry is empty and the + * deploy is guaranteed to fail. + * + * The `-` scheme mirrors the OSS backend's makeFlyAppName + * (insforge/backend/src/providers/compute/services.service.ts) — the CLI + * never builds app names itself. If the server ever changes the scheme this + * guard silently no-ops (falls back to the timeout + hint path); it can + * never wrongly block a valid deploy. */ +export function imageBelongsToOwnService( + imageUrl: string, + serviceName: string, + projectIds: string[] +): boolean { + const repo = flyRegistryRepo(imageUrl); + if (!repo) return false; + return projectIds.some((id) => repo === `${serviceName}-${id}`); +} + +/** Appended to deploy/update failures that smell like a vanished registry + * image, so users get "re-push the image" instead of "cloud unavailable". */ +export function staleFlyImageHint(serviceName?: string): string { + return ( + `\nHint: this image lives in registry.fly.io, where images are deleted together with ` + + `their service. If the service was deleted (or this tag came from an older build), ` + + `the image no longer exists and every retry will fail the same way.\n` + + `Rebuild and push a fresh image by deploying from source:\n` + + ` npx @insforge/cli compute deploy --name ${serviceName ?? ''}` + ); +} + +/** Rewrap a deploy/update failure with the stale-image hint when it looks + * like the platform timed out resolving a registry.fly.io image — the + * signature of a manifest that no longer exists. Non-matching errors pass + * through untouched. */ +export function withStaleImageHint( + err: unknown, + imageUrl: string, + serviceName?: string +): unknown { + if (!(err instanceof CLIError)) return err; + if (!flyRegistryRepo(imageUrl)) return err; + const timeoutish = + err.code === 'COMPUTE_CLOUD_UNAVAILABLE' || + err.statusCode === 502 || + err.statusCode === 503 || + err.statusCode === 504; + if (!timeoutish) return err; + return new CLIError( + err.message + staleFlyImageHint(serviceName), + err.exitCode, + err.code, + err.statusCode + ); +}