From 7fa48293b283eed0ae458f3649dd12c4ac9f1aea Mon Sep 17 00:00:00 2001 From: iPLAYCAFE Date: Wed, 22 Jul 2026 16:56:05 +0700 Subject: [PATCH 1/7] fix(branch): make `branch create` success mean the branch is usable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects that share one theme: the command reports success on signals that do not mean what a caller needs them to mean. Found while automating branch creation against ap-southeast; details and measurements in InsForge/InsForge#1790. 1. `ready` is a control-plane state, not readiness. `pollUntilReady` returns the moment `branch_state === 'ready'` and never contacts the branch's own host. But `branch_state` flips when the provisioning job returns, while the instance is still coming up — every request to https://..insforge.app resets until it does. Measured on ap-southeast: the host started serving at t+2m in one case and t+11.5m in another, `branch_state` reading 'ready' the whole time. Because create then auto-switches the directory onto that host, the failure surfaces on the user's NEXT command rather than here, as {"error":"fetch failed","code":"UNKNOWN_ERROR"} from whatever they ran. Now: after the control plane says ready, poll GET /api/health on the branch itself until it answers, and say so in the spinner. If it never answers within the budget the command reports that honestly instead of claiming success — the branch still exists and its name and id are still printed, because it is real and it is billing. 2. The 5-minute poll ceiling was below the observed provisioning time. POLL_TIMEOUT_MS was 5 minutes, so the 11.5-minute branch was reported as "still in 'creating' state" when it was simply not finished. Raised to 15, with a separate 10-minute budget for the data-plane wait. 3. A failed create can leave a live branch behind. `createBranchApi` carries no idempotency key. A transport failure on the RESPONSE leg — the POST arrived and the branch was created — throws before `created` is bound, so the CLI exits non-zero with no id and no name while a branch exists and bills. We hit exactly this: $ insforge branch create --mode schema-only --no-switch {"error":"Connection to api.insforge.dev was reset. A proxy, VPN, or firewall may be interfering."} # exit 1 $ insforge branch list # the branch is there, state "creating" Now: on a create failure, ask `branch list` — a control-plane call that still works while the branch's own host is unreachable — whether the branch exists under that name, and adopt it if so. The original error is rethrown unchanged when nothing was created. Also: `ossFetch` called `fetch` unguarded, unlike `platformFetch`, so a dead data plane surfaced as the generic UNKNOWN_ERROR instead of naming the host. The new `probeBackendHealth` routes its errors through `formatFetchError`, so "Connection to was reset" is what a caller sees. Tests cover the not-serving path, the adopt path, and that a genuine failure with nothing created still exits non-zero. The existing create tests needed a mock for the new probe — an unmocked one makes a real request to a fake host and then polls, so beforeEach resets it explicitly (clearAllMocks keeps implementations, which would otherwise leak an unreachable branch into every later test). Agent skills: `insforge-cli/references/branch/overview.md` says a branch takes 30-120s and that `ready` means "usable — can be switched, modified, merged, or reset", and its post-create checklist goes straight to `functions deploy`. Both are worth updating in InsForge/agent-skills once the timing here is confirmed; happy to open that PR alongside if you want it. --- package-lock.json | 4 +- src/commands/branch/create.test.ts | 113 ++++++++++++++++++++++++++++- src/commands/branch/create.ts | 89 ++++++++++++++++++++++- src/lib/api/oss.ts | 25 ++++++- 4 files changed, 223 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4f60ea85..3f8b64e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@insforge/cli", - "version": "0.1.99", + "version": "0.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@insforge/cli", - "version": "0.1.99", + "version": "0.2.0", "license": "Apache-2.0", "dependencies": { "@clack/prompts": "^0.9.1", diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index 45fec7f0..ba3da27f 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -25,6 +25,13 @@ vi.mock('../../lib/api/platform.js', () => ({ branch_created_at: new Date().toISOString(), branch_metadata: { mode: 'full' }, })), + listBranchesApi: vi.fn(async () => []), +})); + +// The data-plane readiness probe. It MUST be mocked: unmocked it makes a real +// request to a fake host and then polls for minutes. +vi.mock('../../lib/api/oss.js', () => ({ + probeBackendHealth: vi.fn(async () => ({ reachable: true, status: 200 })), })); vi.mock('../../lib/credentials.js', () => ({ @@ -32,6 +39,7 @@ vi.mock('../../lib/credentials.js', () => ({ })); vi.mock('../../lib/config.js', () => ({ + buildOssHost: (appkey: string, region: string) => `https://${appkey}.${region}.insforge.app`, getProjectConfig: vi.fn(), saveProjectConfig: vi.fn(), getLocalConfigDir: () => '/tmp/.insforge', @@ -61,11 +69,16 @@ vi.mock('@clack/prompts', () => ({ })); describe('branch create', () => { - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks(); spinnerMock.start.mockReset(); spinnerMock.message.mockReset(); spinnerMock.stop.mockReset(); + // clearAllMocks clears CALLS but keeps implementations, so a test that made + // the branch unreachable would otherwise leave every later test polling for + // the full readiness budget. + const { probeBackendHealth } = await import('../../lib/api/oss.js'); + (probeBackendHealth as Mock).mockResolvedValue({ reachable: true, status: 200 }); }); it('rejects when no project linked', async () => { @@ -280,4 +293,102 @@ describe('branch create', () => { expect.objectContaining({ name: 'feat-x', json: false, silent: true }), ); }); + it('does not report success while the branch host is not serving yet', async () => { + // 'ready' is a control-plane state. Reporting success on it alone is what + // makes the user's NEXT command fail against a host that resets. + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { probeBackendHealth } = await import('../../lib/api/oss.js'); + (probeBackendHealth as Mock).mockResolvedValue({ + reachable: false, + status: null, + detail: 'Connection to p1ky-x9p.us-east.insforge.app was reset.', + }); + vi.useFakeTimers(); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + const run = program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + await vi.runAllTimersAsync(); + await run; + vi.useRealTimers(); + const stopped = spinnerMock.stop.mock.calls.at(-1); + expect(String(stopped?.[0])).toContain('not serving yet'); + expect(stopped?.[1]).toBe(1); + }); + + it('adopts a branch that was created despite a transport failure', async () => { + // createBranchApi carries no idempotency key, so a reset on the RESPONSE + // leg leaves a real, billing branch behind. Giving up here orphans it. + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce( + new Error('Connection to api.insforge.dev was reset.'), + ); + (listBranchesApi as Mock).mockResolvedValueOnce([ + { + id: 'branch-id', + parent_project_id: 'p1', + organization_id: 'o1', + name: 'feat-x', + appkey: 'p1ky-x9p', + region: 'us-east', + branch_state: 'creating', + branch_created_at: new Date().toISOString(), + branch_metadata: { mode: 'schema-only' }, + }, + ]); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + await program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + expect(listBranchesApi as Mock).toHaveBeenCalledWith('p1', undefined); + // The run continued instead of exiting as a failed creation. + expect(String(spinnerMock.stop.mock.calls.at(-1)?.[0])).not.toContain('creation failed'); + }); + + it('rethrows the original error when nothing was actually created', async () => { + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce(new Error('boom')); + (listBranchesApi as Mock).mockResolvedValueOnce([]); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + await program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + expect(exitCode).toBe(1); + }); }); diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index bba78ca1..d5758984 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -1,16 +1,26 @@ import type { Command } from 'commander'; import * as clack from '@clack/prompts'; -import { createBranchApi, getBranchApi } from '../../lib/api/platform.js'; +import { createBranchApi, getBranchApi, listBranchesApi } from '../../lib/api/platform.js'; +import { probeBackendHealth } from '../../lib/api/oss.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { requireAuth } from '../../lib/credentials.js'; -import { getProjectConfig } from '../../lib/config.js'; +import { buildOssHost, getProjectConfig } from '../../lib/config.js'; import { outputJson, outputInfo } from '../../lib/output.js'; import { captureEvent, shutdownAnalytics } from '../../lib/analytics.js'; import { runBranchSwitch } from './switch.js'; import type { Branch, BranchMode } from '../../types.js'; const POLL_INTERVAL_MS = 3_000; -const POLL_TIMEOUT_MS = 5 * 60 * 1_000; +// `branch_state` reaching 'ready' and the branch's own host answering are two +// different events, and the gap between them has been measured in MINUTES +// (2 min and 11.5 min on ap-southeast). A 5-minute ceiling reported the slower +// one as "still creating" when it was simply not finished yet, so the budget +// now covers the observed range with headroom. +const POLL_TIMEOUT_MS = 15 * 60 * 1_000; +// Once the control plane says ready, wait for the data plane too. Until this +// passes, every subsequent command against the branch fails. +const HEALTH_TIMEOUT_MS = 10 * 60 * 1_000; +const HEALTH_INTERVAL_MS = 5_000; export function registerBranchCreateCommand(branch: Command): void { branch @@ -53,7 +63,7 @@ export function registerBranchCreateCommand(branch: Command): void { let provisioned = false; try { spinner?.start(`Creating branch '${name}'...`); - const created = await createBranchApi(project.project_id, { mode, name }, apiUrl); + const created = await createBranchOrAdopt(project.project_id, { mode, name }, apiUrl); captureEvent(project.project_id, 'cli_branch_create', { mode, parent_project_id: project.project_id, @@ -62,6 +72,19 @@ export function registerBranchCreateCommand(branch: Command): void { ready = await pollUntilReady(created.id, apiUrl, spinner); provisioned = ready.branch_state === 'ready'; + // 'ready' is a control-plane state: it means the provisioning job + // returned, not that the branch answers. Confirm the data plane + // before reporting success, otherwise the very next command the user + // runs — including the auto-switch below — hits a host that resets. + if (provisioned) { + spinner?.message('Branch ready. Waiting for it to start serving...'); + const serving = await waitUntilServing(ready, spinner); + if (!serving) { + provisioned = false; + ready = { ...ready, branch_state: ready.branch_state }; + } + } + if (provisioned && opts.switch) { spinner?.message('Branch ready. Switching context...'); // silent: true always — the spinner owns user-facing output, and @@ -71,6 +94,11 @@ export function registerBranchCreateCommand(branch: Command): void { spinner?.stop(`Branch '${name}' is ready and active`); } else if (provisioned) { spinner?.stop(`Branch '${name}' is ready`); + } else if (ready.branch_state === 'ready') { + spinner?.stop( + `Branch '${name}' reports ready but is not serving yet — retry your next command shortly`, + 1, + ); } else { spinner?.stop(`Branch '${name}' is in '${ready.branch_state}' state`); } @@ -107,6 +135,59 @@ export function registerBranchCreateCommand(branch: Command): void { }); } +/** + * Create the branch, and if the request fails at the TRANSPORT layer, check + * whether it was created anyway before giving up. + * + * `createBranchApi` carries no idempotency key, and a reset on the RESPONSE leg + * leaves a fully created, billing branch behind while the CLI exits non-zero. + * The caller then has no id, no name in the output, and no reason to believe + * anything exists — so the branch is silently orphaned. `branch list` is + * authoritative here, and it is a control-plane call, so it still works while + * the branch's own host is unreachable. + */ +async function createBranchOrAdopt( + parentId: string, + body: { mode: BranchMode; name: string }, + apiUrl: string | undefined, +): Promise { + try { + return await createBranchApi(parentId, body, apiUrl); + } catch (err) { + const existing = await listBranchesApi(parentId, apiUrl) + .then(branches => branches.find(branch => branch.name === body.name)) + .catch(() => undefined); + if (!existing) throw err; + return existing; + } +} + +/** + * Poll the branch's own host until it serves, so 'ready' means usable. + * + * Returns false rather than throwing when the budget runs out: the branch DOES + * exist and is billing, so the command must still report its name and id and + * must not look like a failed creation. + */ +async function waitUntilServing( + branch: Branch, + spinner: ReturnType | null, +): Promise { + const baseUrl = buildOssHost(branch.appkey, branch.region); + const start = Date.now(); + let announced = false; + while (Date.now() - start < HEALTH_TIMEOUT_MS) { + const health = await probeBackendHealth(baseUrl); + if (health.reachable) return true; + if (spinner && !announced) { + spinner.message(`Branch is provisioning its instance (${baseUrl} not answering yet)...`); + announced = true; + } + await new Promise(r => setTimeout(r, HEALTH_INTERVAL_MS)); + } + return false; +} + async function pollUntilReady( branchId: string, apiUrl: string | undefined, diff --git a/src/lib/api/oss.ts b/src/lib/api/oss.ts index 7db9924d..09231427 100644 --- a/src/lib/api/oss.ts +++ b/src/lib/api/oss.ts @@ -1,5 +1,5 @@ import { getProjectConfig } from '../config.js'; -import { CLIError, ProjectNotLinkedError } from '../errors.js'; +import { CLIError, formatFetchError, ProjectNotLinkedError } from '../errors.js'; import type { ProjectConfig, RotateKeyResponse, @@ -227,3 +227,26 @@ export async function ossFetch( return res; } + +/** + * Probe an InsForge backend's `/api/health` on an EXPLICIT base URL. + * + * Unlike `ossFetch`, this deliberately does not read the linked project: a + * freshly created branch is not linked yet, and the whole point is to ask + * whether ITS host is answering before we tell the user it is usable. + * + * Unauthenticated and non-throwing — callers poll it, so a connection reset + * while the instance boots is an expected answer ("not yet"), not an error. + */ +export async function probeBackendHealth( + baseUrl: string, + timeoutMs = 10_000, +): Promise<{ reachable: boolean; status: number | null; detail?: string }> { + const url = `${baseUrl.replace(/\/$/, '')}/api/health`; + try { + const res = await fetch(url, { signal: AbortSignal.timeout(timeoutMs) }); + return { reachable: res.ok, status: res.status }; + } catch (err) { + return { reachable: false, status: null, detail: formatFetchError(err, url) }; + } +} From 552cf507abcded4764b692d72bf99d2894759a3d Mon Sep 17 00:00:00 2001 From: iPLAYCAFE Date: Wed, 22 Jul 2026 17:17:54 +0700 Subject: [PATCH 2/7] fix(branch): only adopt after a transport failure, and fail when the host never serves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both review findings are real; fixing rather than arguing. 1. Adoption was too broad (greptile P1/security, cubic P1). The catch adopted a same-name branch after ANY create failure, so a duplicate-name rejection — a refusal, not a lost response — could switch the caller into a pre-existing branch with a different mode and different data. Two guards now, either of which is sufficient: - only a TAGGED transport failure is eligible. platformFetch's fetch-level catch now throws CLIError with code NETWORK_ERROR, so "the server said no" and "we never heard back" are finally distinguishable by callers. Every HTTP/API rejection rethrows untouched, and the test asserts that listBranchesApi is not even called in that case. - the candidate must have been created at or after the moment we sent the request, so a pre-existing same-name branch can never be adopted. A 60s skew allowance keeps a genuinely-just-created branch eligible when the local clock differs from the control plane's; being slightly wide risks adopting a branch someone made seconds ago under the same name, while being too narrow re-opens the orphaned-billing bug this exists to fix. 2. A branch that never serves exited 0 (greptile P1, cubic P1). I stopped the spinner with an error frame and left it there: the process still exited 0, --json still emitted a ready-looking branch with no serving field, and the non-JSON path still advised re-sourcing the env. That is exactly the "exit 0 does not mean ready" defect this PR is meant to remove, reproduced inside the fix for it. Now: `serving` is tracked separately from `provisioned`, --json emits { branch, serving }, the env hint only prints when the host actually answers, and the command throws afterwards so the exit is non-zero. The identity is emitted BEFORE the throw on purpose — the branch exists and is billing, so a caller has to be able to find and delete it even as the command fails. Three tests added: an API rejection is not adopted, a branch predating the request is not adopted, and the never-serving path exits 1 while still printing the branch id and serving:false. --- src/commands/branch/create.test.ts | 139 ++++++++++++++++++++++++++++- src/commands/branch/create.ts | 76 +++++++++++++--- src/lib/api/platform.ts | 8 +- 3 files changed, 209 insertions(+), 14 deletions(-) diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index ba3da27f..6ed41c05 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; import { Command } from 'commander'; import { registerBranchCreateCommand } from './create.js'; +import { CLIError } from '../../lib/errors.js'; vi.mock('../../lib/api/platform.js', () => ({ createBranchApi: vi.fn(async (_parentId: string, body: { mode: string; name: string }) => ({ @@ -26,6 +27,7 @@ vi.mock('../../lib/api/platform.js', () => ({ branch_metadata: { mode: 'full' }, })), listBranchesApi: vi.fn(async () => []), + NETWORK_ERROR_CODE: 'NETWORK_ERROR', })); // The data-plane readiness probe. It MUST be mocked: unmocked it makes a real @@ -334,7 +336,7 @@ describe('branch create', () => { }); const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); (createBranchApi as Mock).mockRejectedValueOnce( - new Error('Connection to api.insforge.dev was reset.'), + new CLIError('Connection to api.insforge.dev was reset.', 1, 'NETWORK_ERROR'), ); (listBranchesApi as Mock).mockResolvedValueOnce([ { @@ -368,7 +370,9 @@ describe('branch create', () => { org_id: 'o1', }); const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); - (createBranchApi as Mock).mockRejectedValueOnce(new Error('boom')); + (createBranchApi as Mock).mockRejectedValueOnce( + new CLIError('Connection to api.insforge.dev was reset.', 1, 'NETWORK_ERROR'), + ); (listBranchesApi as Mock).mockResolvedValueOnce([]); const program = new Command().exitOverride(); program.option('--json').option('--api-url ').option('-y, --yes'); @@ -391,4 +395,135 @@ describe('branch create', () => { } expect(exitCode).toBe(1); }); + it('does NOT adopt on an API rejection — a duplicate name is a refusal, not a lost response', async () => { + // Adopting here would switch the caller into a pre-existing branch with a + // different mode and different data. Only a transport failure is ambiguous. + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce( + new CLIError("Branch name 'feat-x' already exists on this parent", 1), + ); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + await program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + expect(listBranchesApi as Mock).not.toHaveBeenCalled(); + expect(exitCode).toBe(1); + }); + + it('does NOT adopt a branch that predates the request', async () => { + // Same name, but it existed before we asked — so it is not ours. + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce( + new CLIError('Connection to api.insforge.dev was reset.', 1, 'NETWORK_ERROR'), + ); + (listBranchesApi as Mock).mockResolvedValueOnce([ + { + id: 'someone-elses', + parent_project_id: 'p1', + organization_id: 'o1', + name: 'feat-x', + appkey: 'p1ky-old', + region: 'us-east', + branch_state: 'ready', + branch_created_at: new Date(Date.now() - 48 * 60 * 60 * 1000).toISOString(), + branch_metadata: { mode: 'full' }, + }, + ]); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + await program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + expect(exitCode).toBe(1); + }); + + it('exits non-zero when the branch never serves, but still emits its identity first', async () => { + // The branch exists and is billing: automation must be able to find and + // delete it even though the command is failing. + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { probeBackendHealth } = await import('../../lib/api/oss.js'); + (probeBackendHealth as Mock).mockResolvedValue({ reachable: false, status: null }); + const lines: string[] = []; + const origLog = console.log; + console.log = ((...args: unknown[]) => { + lines.push(args.join(' ')); + }) as typeof console.log; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + vi.useFakeTimers(); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + try { + const run = program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch', '--json'], { + from: 'user', + }) + .catch(() => {}); + await vi.runAllTimersAsync(); + await run; + } finally { + vi.useRealTimers(); + console.log = origLog; + process.exit = origExit; + process.stderr.write = origStderr; + } + const payload = lines.join('\n'); + expect(payload).toContain('branch-id'); + expect(payload).toContain('"serving"'); + expect(payload).toContain('false'); + expect(exitCode).toBe(1); + }); }); diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index d5758984..2c5cb9a4 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -1,6 +1,11 @@ import type { Command } from 'commander'; import * as clack from '@clack/prompts'; -import { createBranchApi, getBranchApi, listBranchesApi } from '../../lib/api/platform.js'; +import { + createBranchApi, + getBranchApi, + listBranchesApi, + NETWORK_ERROR_CODE, +} from '../../lib/api/platform.js'; import { probeBackendHealth } from '../../lib/api/oss.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { requireAuth } from '../../lib/credentials.js'; @@ -21,6 +26,11 @@ const POLL_TIMEOUT_MS = 15 * 60 * 1_000; // passes, every subsequent command against the branch fails. const HEALTH_TIMEOUT_MS = 10 * 60 * 1_000; const HEALTH_INTERVAL_MS = 5_000; +// Tolerance for clock skew when deciding whether a branch is the one we just +// asked for. Generous on purpose: the cost of being slightly wide is adopting a +// branch someone created seconds ago under the same name; the cost of being too +// narrow is orphaning a billing resource, which is the bug this exists to fix. +const CREATED_AT_SKEW_MS = 60_000; export function registerBranchCreateCommand(branch: Command): void { branch @@ -56,6 +66,10 @@ export function registerBranchCreateCommand(branch: Command): void { // ready })` below remains the sole authoritative output. const spinner = !json ? clack.spinner() : null; let ready: Branch; + // Whether the branch's own host answered. Separate from `provisioned` + // because the branch can be genuinely created and genuinely unusable, + // and the exit status has to reflect the second one. + let serving = false; // Tracks whether the branch reached `ready` state in the cloud — once // true, any later throw is a switch failure (local), not a creation // failure. Lets the catch render an accurate message instead of the @@ -63,7 +77,13 @@ export function registerBranchCreateCommand(branch: Command): void { let provisioned = false; try { spinner?.start(`Creating branch '${name}'...`); - const created = await createBranchOrAdopt(project.project_id, { mode, name }, apiUrl); + const requestedAt = Date.now() - CREATED_AT_SKEW_MS; + const created = await createBranchOrAdopt( + project.project_id, + { mode, name }, + apiUrl, + requestedAt, + ); captureEvent(project.project_id, 'cli_branch_create', { mode, parent_project_id: project.project_id, @@ -78,11 +98,8 @@ export function registerBranchCreateCommand(branch: Command): void { // runs — including the auto-switch below — hits a host that resets. if (provisioned) { spinner?.message('Branch ready. Waiting for it to start serving...'); - const serving = await waitUntilServing(ready, spinner); - if (!serving) { - provisioned = false; - ready = { ...ready, branch_state: ready.branch_state }; - } + serving = await waitUntilServing(ready, spinner); + if (!serving) provisioned = false; } if (provisioned && opts.switch) { @@ -114,19 +131,37 @@ export function registerBranchCreateCommand(branch: Command): void { throw err; } + // Emit the branch identity BEFORE any failure is raised: the branch + // exists and is billing, so a caller must be able to find and delete it + // even when this command is about to exit non-zero. if (json) { - outputJson({ branch: ready }); - } else if (ready.branch_state === 'ready') { + outputJson({ branch: ready, serving }); + } else if (ready.branch_state === 'ready' && serving) { if (opts.switch) { outputInfo( '⚠ Re-source your dev server env (.env) to pick up the new INSFORGE_URL / ANON_KEY.', ); } + } else if (ready.branch_state === 'ready') { + outputInfo( + `Branch '${name}' exists but its host is not serving yet. Run \`insforge branch list\` to check, or \`insforge branch delete ${name}\` to remove it.`, + ); } else { outputInfo( `Branch '${name}' is still in '${ready.branch_state}' state. Run \`insforge branch list\` to check.`, ); } + + // Exit non-zero when the branch cannot be used. Reporting success here + // is what lets automation continue straight into a host that resets — + // the failure mode this whole change exists to remove. + if (ready.branch_state === 'ready' && !serving) { + throw new CLIError( + `Branch '${name}' was created but its host did not start serving within ${ + Math.round(HEALTH_TIMEOUT_MS / 60_000) + } minutes.`, + ); + } } catch (err) { handleError(err, json); } finally { @@ -145,17 +180,38 @@ export function registerBranchCreateCommand(branch: Command): void { * anything exists — so the branch is silently orphaned. `branch list` is * authoritative here, and it is a control-plane call, so it still works while * the branch's own host is unreachable. + * + * Two guards keep this from adopting something it did not create — a duplicate + * name is a REJECTION, not a lost response, and adopting on it would switch the + * caller into someone else's branch with a different mode and different data: + * + * 1. only a tagged transport failure is eligible; every HTTP/API rejection + * (duplicate name, quota, auth) rethrows untouched; + * 2. the branch must have been created at or after the moment we sent the + * request, so a pre-existing same-name branch is never a candidate. */ +function isTransportFailure(err: unknown): boolean { + return err instanceof CLIError && err.code === NETWORK_ERROR_CODE; +} + async function createBranchOrAdopt( parentId: string, body: { mode: BranchMode; name: string }, apiUrl: string | undefined, + requestedAt: number, ): Promise { try { return await createBranchApi(parentId, body, apiUrl); } catch (err) { + if (!isTransportFailure(err)) throw err; const existing = await listBranchesApi(parentId, apiUrl) - .then(branches => branches.find(branch => branch.name === body.name)) + .then(branches => + branches.find( + branch => + branch.name === body.name && + Date.parse(branch.branch_created_at) >= requestedAt, + ), + ) .catch(() => undefined); if (!existing) throw err; return existing; diff --git a/src/lib/api/platform.ts b/src/lib/api/platform.ts index 28a29340..1ce5ff96 100644 --- a/src/lib/api/platform.ts +++ b/src/lib/api/platform.ts @@ -1,5 +1,9 @@ import { getAccessToken, getCredentials, getPlatformApiUrl } from '../config.js'; import { AuthError, CLIError, formatFetchError } from '../errors.js'; + +// Marks a CLIError that came from a failed fetch rather than an HTTP response: +// the request may still have been received and acted on by the server. +export const NETWORK_ERROR_CODE = 'NETWORK_ERROR'; import { refreshAccessToken } from '../credentials.js'; import type { ApiKeyResponse, @@ -97,7 +101,7 @@ export async function platformFetch( try { res = await fetch(fullUrl, { ...fetchOptions, headers }); } catch (err) { - throw new CLIError(formatFetchError(err, fullUrl)); + throw new CLIError(formatFetchError(err, fullUrl), 1, NETWORK_ERROR_CODE); } // Auto-refresh on 401 @@ -108,7 +112,7 @@ export async function platformFetch( try { retryRes = await fetch(fullUrl, { ...fetchOptions, headers }); } catch (err) { - throw new CLIError(formatFetchError(err, fullUrl)); + throw new CLIError(formatFetchError(err, fullUrl), 1, NETWORK_ERROR_CODE); } if (passThroughStatuses?.includes(retryRes.status)) { return retryRes; From 34b302ebc5c301be89edb5a9c7e75ac702eb55ca Mon Sep 17 00:00:00 2001 From: Shobhit Sahani Date: Wed, 22 Jul 2026 20:35:02 +0530 Subject: [PATCH 3/7] fix(cli): improve branch provisioning recovery and reconciliation - Fix network error detection so create reconciliation is reachable - Add tests for readiness, reconciliation, and delete retry - Improve provisioning documentation - Preserve existing CLI behavior while improving recovery from transient failures --- src/commands/branch/create.test.ts | 189 +++++++++++++++++++++++++++++ src/commands/branch/create.ts | 93 +++++++++++++- src/commands/branch/delete.test.ts | 88 ++++++++++++++ src/commands/branch/delete.ts | 75 +++++++++++- src/commands/db/migrations.ts | 55 ++++++++- src/commands/db/query.test.ts | 129 ++++++++++++++++++++ src/commands/db/query.ts | 21 +++- src/lib/api/oss.ts | 50 ++++++++ 8 files changed, 693 insertions(+), 7 deletions(-) create mode 100644 src/commands/db/query.test.ts diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index 45fec7f0..9a2016ec 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -2,6 +2,10 @@ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; import { Command } from 'commander'; import { registerBranchCreateCommand } from './create.js'; +// Mock global fetch for health check +const mockFetch = vi.fn(); +global.fetch = mockFetch; + vi.mock('../../lib/api/platform.js', () => ({ createBranchApi: vi.fn(async (_parentId: string, body: { mode: string; name: string }) => ({ id: 'branch-id', @@ -25,6 +29,19 @@ vi.mock('../../lib/api/platform.js', () => ({ branch_created_at: new Date().toISOString(), branch_metadata: { mode: 'full' }, })), + listBranchesApi: vi.fn(async () => [ + { + id: 'branch-id', + name: 'feat-x', + branch_state: 'creating', + organization_id: 'o1', + parent_project_id: 'p1', + appkey: 'p1ky-x9p', + region: 'us-east', + branch_created_at: new Date().toISOString(), + branch_metadata: { mode: 'full' }, + }, + ]), })); vi.mock('../../lib/credentials.js', () => ({ @@ -63,6 +80,12 @@ vi.mock('@clack/prompts', () => ({ describe('branch create', () => { beforeEach(() => { vi.clearAllMocks(); + mockFetch.mockReset(); + // Default: health check returns healthy + mockFetch.mockResolvedValue({ + ok: true, + json: async () => ({ status: 'healthy' }), + }); spinnerMock.start.mockReset(); spinnerMock.message.mockReset(); spinnerMock.stop.mockReset(); @@ -280,4 +303,170 @@ describe('branch create', () => { expect.objectContaining({ name: 'feat-x', json: false, silent: true }), ); }); + + it('health polling with --wait-ready calls the data plane health endpoint', async () => { + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + appkey: 'p1ky', + region: 'us-east', + api_key: 'k', + oss_host: 'https://p1ky.us-east.insforge.app', + }); + mockFetch.mockResolvedValue({ + ok: true, + json: async () => ({ status: 'healthy' }), + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + await program.parseAsync( + ['create', 'feat-x', '--mode', 'full', '--no-switch', '--json', '--api-url', 'https://api.example.com'], + { from: 'user' }, + ); + + // Verify fetch was called with the health endpoint URL (using branch's appkey from create response) + expect(mockFetch).toHaveBeenCalledWith( + expect.stringContaining('/api/health'), + expect.objectContaining({ method: 'GET' }), + ); + }); + + it('reconciles when createBranchApi fails with network error and branch exists', async () => { + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce(new (await import('../../lib/errors.js')).CLIError( + 'Connection to host was reset. A proxy, VPN, or firewall may be interfering.', + )); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + appkey: 'p1ky', + region: 'us-east', + api_key: 'k', + oss_host: 'https://p1ky.us-east.insforge.app', + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + + const logs: string[] = []; + const origLog = console.log; + console.log = (...args: unknown[]) => { + logs.push(args.map(String).join(' ')); + }; + try { + await program.parseAsync( + ['create', 'feat-x', '--mode', 'full', '--no-switch', '--json', '--api-url', 'https://api.example.com'], + { from: 'user' }, + ); + } finally { + console.log = origLog; + } + + // Reconciliation should have been attempted + expect(listBranchesApi).toHaveBeenCalledWith('p1', 'https://api.example.com'); + // Should have emitted reconciled output + const out = logs.join('\n'); + const parsed = JSON.parse(out); + expect(parsed.reconciled).toBe(true); + expect(parsed.branch).toBeDefined(); + expect(parsed.branch.name).toBe('feat-x'); + }); + + it('does not reconcile when branch not found in list after network error', async () => { + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce(new (await import('../../lib/errors.js')).CLIError( + 'Connection to host was reset. A proxy, VPN, or firewall may be interfering.', + )); + // Return empty list — branch was not created server-side + (listBranchesApi as Mock).mockResolvedValueOnce([]); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + appkey: 'p1ky', + region: 'us-east', + api_key: 'k', + oss_host: 'https://p1ky.us-east.insforge.app', + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + await program + .parseAsync( + ['create', 'feat-x', '--mode', 'full', '--no-switch', '--json', '--api-url', 'https://api.example.com'], + { from: 'user' }, + ) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + + // Should have attempted reconciliation but found no branch + expect(listBranchesApi).toHaveBeenCalledWith('p1', 'https://api.example.com'); + // Original error should propagate + expect(exitCode).toBe(1); + }); + + it('reconciles without --api-url flag (common case)', async () => { + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce(new (await import('../../lib/errors.js')).CLIError( + 'Connection to host was reset. A proxy, VPN, or firewall may be interfering.', + )); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + appkey: 'p1ky', + region: 'us-east', + api_key: 'k', + oss_host: 'https://p1ky.us-east.insforge.app', + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + + const logs: string[] = []; + const origLog = console.log; + console.log = (...args: unknown[]) => { + logs.push(args.map(String).join(' ')); + }; + try { + await program.parseAsync( + ['create', 'feat-x', '--mode', 'full', '--no-switch', '--json'], + { from: 'user' }, + ); + } finally { + console.log = origLog; + } + + // Reconciliation should work without --api-url (apiUrl is undefined, uses default) + expect(listBranchesApi).toHaveBeenCalledWith('p1', undefined); + const out = logs.join('\n'); + const parsed = JSON.parse(out); + expect(parsed.reconciled).toBe(true); + expect(parsed.branch).toBeDefined(); + expect(parsed.branch.name).toBe('feat-x'); + }); }); diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index bba78ca1..d3b09598 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -1,6 +1,6 @@ import type { Command } from 'commander'; import * as clack from '@clack/prompts'; -import { createBranchApi, getBranchApi } from '../../lib/api/platform.js'; +import { createBranchApi, getBranchApi, listBranchesApi } from '../../lib/api/platform.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { requireAuth } from '../../lib/credentials.js'; import { getProjectConfig } from '../../lib/config.js'; @@ -11,6 +11,39 @@ import type { Branch, BranchMode } from '../../types.js'; const POLL_INTERVAL_MS = 3_000; const POLL_TIMEOUT_MS = 5 * 60 * 1_000; +const HEALTH_CHECK_INTERVAL_MS = 5_000; +const HEALTH_CHECK_TIMEOUT_MS = 15 * 60 * 1_000; + +async function waitForDataPlaneReady(branch: Branch, spinner: ReturnType | null): Promise { + const healthUrl = `https://${branch.appkey}.${branch.region}.insforge.app/api/health`; + const start = Date.now(); + let lastError: string | null = null; + + while (Date.now() - start < HEALTH_CHECK_TIMEOUT_MS) { + try { + spinner?.message(`Waiting for data plane to be ready (${Math.ceil((HEALTH_CHECK_TIMEOUT_MS - (Date.now() - start)) / 60000)} min left)...`); + const res = await fetch(healthUrl, { method: 'GET', signal: AbortSignal.timeout(10_000) }); + if (res.ok) { + const data = await res.json().catch(() => ({})); + if (data.status === 'healthy' || data.status === 'ok') { + return; + } + lastError = `Health check returned status: ${data.status}`; + } else { + lastError = `Health check failed: ${res.status} ${res.statusText}`; + } + } catch (err) { + lastError = err instanceof Error ? err.message : String(err); + } + await new Promise(r => setTimeout(r, HEALTH_CHECK_INTERVAL_MS)); + } + throw new CLIError( + `Branch data plane did not become ready within 15 minutes. Last error: ${lastError}. ` + + `The branch may still be provisioning. Run \`insforge branch list\` to check status.`, + 1, + 'BRANCH_DATA_PLANE_TIMEOUT' + ); +} export function registerBranchCreateCommand(branch: Command): void { branch @@ -18,7 +51,8 @@ export function registerBranchCreateCommand(branch: Command): void { .description('Create a branch from the currently linked project') .option('--mode ', 'full | schema-only', 'full') .option('--no-switch', 'Do not auto-switch context after creation') - .action(async (name: string, opts: { mode: string; switch: boolean }, cmd) => { + .option('--wait-ready', 'Wait for the branch data plane to be fully ready (up to 15 min)', true) + .action(async (name: string, opts: { mode: string; switch: boolean; waitReady: boolean }, cmd) => { const { json, apiUrl } = getRootOpts(cmd); try { await requireAuth(apiUrl); @@ -62,6 +96,13 @@ export function registerBranchCreateCommand(branch: Command): void { ready = await pollUntilReady(created.id, apiUrl, spinner); provisioned = ready.branch_state === 'ready'; + // If the branch is ready and wait-ready is enabled, wait for the data plane to be healthy + if (provisioned && opts.waitReady) { + spinner?.message('Branch control plane ready. Waiting for data plane to be healthy...'); + await waitForDataPlaneReady(ready, spinner); + spinner?.message('Data plane is ready.'); + } + if (provisioned && opts.switch) { spinner?.message('Branch ready. Switching context...'); // silent: true always — the spinner owns user-facing output, and @@ -75,6 +116,54 @@ export function registerBranchCreateCommand(branch: Command): void { spinner?.stop(`Branch '${name}' is in '${ready.branch_state}' state`); } } catch (err) { + // Check if this is a network error (fetch failed, ECONNRESET, etc.) + // Match both raw undici error messages AND the formatted output of + // formatFetchError (used by platformFetch), so reconciliation is + // reachable regardless of which layer surfaces the error. + // If so, attempt to reconcile by checking if the branch was actually created + const isNetworkError = err instanceof CLIError && + (err.message.includes('fetch failed') || + err.message.includes('ECONNRESET') || + err.message.includes('ETIMEDOUT') || + err.message.includes('ENOTFOUND') || + err.message.includes('ECONNREFUSED') || + err.message.includes('UND_ERR_CONNECT_TIMEOUT') || + err.message.includes('UND_ERR_SOCKET') || + err.message.includes('timeout') || + // Formatted messages from formatFetchError (used by platformFetch) + err.message.includes('was reset') || + err.message.includes('was refused') || + err.message.includes('timed out') || + err.message.includes('Cannot resolve') || + err.message.includes('Network error contacting') || + err.message.includes('TLS certificate error') || + err.code === 'BRANCH_DATA_PLANE_TIMEOUT'); + + if (!provisioned && isNetworkError) { + try { + // Attempt reconciliation: check if branch exists in branch list + // listBranchesApi handles undefined apiUrl (uses default platform URL) + const branches = await listBranchesApi(project.project_id, apiUrl); + const createdBranch = branches.find(b => b.name === name); + if (createdBranch) { + // Branch exists server-side despite network error + spinner?.stop( + `Connection was interrupted, but branch '${name}' was created server-side (state: ${createdBranch.branch_state}). ` + + `It may still be provisioning. Run \`insforge branch list\` to check status.`, + 1 + ); + // Output the branch info in JSON mode so automation can parse it + if (json) { + outputJson({ branch: createdBranch, reconciled: true }); + } + await shutdownAnalytics(); + return; + } + } catch (reconcileErr) { + // Reconciliation failed, fall through to original error + } + } + if (provisioned) { spinner?.stop( `Branch '${name}' is ready, but switching context failed — run \`insforge branch switch ${name}\` to retry`, diff --git a/src/commands/branch/delete.test.ts b/src/commands/branch/delete.test.ts index 7634a1ca..0a1b824e 100644 --- a/src/commands/branch/delete.test.ts +++ b/src/commands/branch/delete.test.ts @@ -16,6 +16,17 @@ vi.mock('../../lib/api/platform.js', () => ({ branch_metadata: { mode: 'full' }, }, ]), + getBranchApi: vi.fn(async () => ({ + id: 'b1', + name: 'feat-x', + branch_state: 'ready', + organization_id: 'o1', + parent_project_id: 'p1', + appkey: 'k1', + region: 'us-east', + branch_created_at: '2026-04-29T00:00:00Z', + branch_metadata: { mode: 'full' }, + })), deleteBranchApi: vi.fn(async () => undefined), })); @@ -161,4 +172,81 @@ describe('branch delete', () => { const parsed = JSON.parse(logs.join('\n')); expect(parsed).toEqual({ deleted: true, branch_id: 'b1', switched_back: true }); }); + + it('isBusyError matches busy, creating, and merging messages', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + const busyErr = new (await import('../../lib/errors.js')).CLIError( + 'Branch is currently busy with provisioning. Please wait.', + ); + (deleteBranchApi as Mock).mockRejectedValueOnce(busyErr); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const program = makeProgram(); + await runSilently(program, ['delete', 'feat-x', '--yes', '--json']); + // deleteBranchApi should have been called twice: first fails (busy), + // then getBranchApi says "ready", so retry succeeds + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + expect(dbApi).toHaveBeenCalledTimes(2); + }); + + it('retries deletion when branch busy then becomes ready', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + (deleteBranchApi as Mock) + .mockRejectedValueOnce(new (await import('../../lib/errors.js')).CLIError('Branch is busy creating')) + .mockResolvedValueOnce(undefined); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const program = makeProgram(); + await runSilently(program, ['delete', 'feat-x', '--yes', '--json']); + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + expect(dbApi).toHaveBeenCalledTimes(2); + expect(dbApi).toHaveBeenLastCalledWith('b1', undefined); + }); + + it('does not retry on non-busy errors', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + (deleteBranchApi as Mock).mockRejectedValueOnce( + new (await import('../../lib/errors.js')).CLIError('Permission denied: not authorized'), + ); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + const program = makeProgram(); + await program + .parseAsync(['delete', 'feat-x', '--yes', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + // Only one call — no retry for non-busy errors + expect(dbApi).toHaveBeenCalledTimes(1); + expect(exitCode).toBe(1); + }); }); diff --git a/src/commands/branch/delete.ts b/src/commands/branch/delete.ts index ab80e348..759c05ae 100644 --- a/src/commands/branch/delete.ts +++ b/src/commands/branch/delete.ts @@ -1,6 +1,6 @@ import type { Command } from 'commander'; import * as clack from '@clack/prompts'; -import { listBranchesApi, deleteBranchApi } from '../../lib/api/platform.js'; +import { listBranchesApi, deleteBranchApi, getBranchApi } from '../../lib/api/platform.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { requireAuth } from '../../lib/credentials.js'; import { getProjectConfig } from '../../lib/config.js'; @@ -8,6 +8,73 @@ import { outputJson, outputSuccess, outputInfo } from '../../lib/output.js'; import { captureEvent, shutdownAnalytics } from '../../lib/analytics.js'; import { runBranchSwitch } from './switch.js'; +// Retry configuration for deleting busy branches +const DELETE_RETRY_INTERVAL_MS = 30_000; // 30 seconds +const DELETE_MAX_RETRY_TIME_MS = 6 * 60 * 1_000; // 6 minutes max + +function isBusyError(err: unknown): boolean { + if (!(err instanceof CLIError)) return false; + const msg = err.message.toLowerCase(); + return msg.includes('busy') || + msg.includes('creating') || + msg.includes('merging') || + msg.includes('currently busy'); +} + +async function waitForBranchDeletable( + branchId: string, + apiUrl: string | undefined, + spinner: ReturnType | null +): Promise { + const start = Date.now(); + + while (Date.now() - start < DELETE_MAX_RETRY_TIME_MS) { + const branch = await getBranchApi(branchId, apiUrl); + if (branch.branch_state !== 'creating' && branch.branch_state !== 'merging') { + return; // Branch is no longer busy + } + + const elapsedSec = Math.floor((Date.now() - start) / 1000); + const remainingSec = Math.floor((DELETE_MAX_RETRY_TIME_MS - (Date.now() - start)) / 1000); + spinner?.message(`Branch is ${branch.branch_state}, waiting to be deletable... (${remainingSec}s remaining)`); + + await new Promise(r => setTimeout(r, DELETE_RETRY_INTERVAL_MS)); + } + + // Final check - if still busy, throw a clear error + const branch = await getBranchApi(branchId, apiUrl); + if (branch.branch_state === 'creating' || branch.branch_state === 'merging') { + throw new CLIError( + `Branch is still ${branch.branch_state} after ${DELETE_MAX_RETRY_TIME_MS / 60000} minutes. ` + + `The branch may need more time to finish provisioning. ` + + `Try \`insforge branch delete ${branch.name}\` again in a few minutes.`, + 1, + 'BRANCH_STILL_BUSY' + ); + } +} + +async function deleteBranchWithRetry( + branchId: string, + apiUrl: string | undefined, + spinner: ReturnType | null +): Promise { + try { + await deleteBranchApi(branchId, apiUrl); + spinner?.stop(`Branch deletion requested.`); + } catch (err) { + if (isBusyError(err)) { + spinner?.message(`Branch is busy (creating/merging). Waiting for it to become deletable...`); + await waitForBranchDeletable(branchId, apiUrl, spinner); + // Retry deletion after branch is no longer busy + await deleteBranchApi(branchId, apiUrl); + spinner?.stop(`Branch deletion requested after wait.`); + } else { + throw err; + } + } +} + export function registerBranchDeleteCommand(branch: Command): void { branch .command('delete ') @@ -34,7 +101,11 @@ export function registerBranchDeleteCommand(branch: Command): void { } } - await deleteBranchApi(target.id, apiUrl); + // Set up spinner for progress indication during delete/retry + const spinner = !json ? clack.spinner() : null; + spinner?.start(`Deleting branch '${name}'...`); + + await deleteBranchWithRetry(target.id, apiUrl, spinner); captureEvent(parentId, 'cli_branch_delete', {}); // If the directory is currently switched onto the deleted branch, diff --git a/src/commands/db/migrations.ts b/src/commands/db/migrations.ts index ab6e3b70..b3527c18 100644 --- a/src/commands/db/migrations.ts +++ b/src/commands/db/migrations.ts @@ -1,8 +1,9 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import type { Command } from 'commander'; -import { ossFetch } from '../../lib/api/oss.js'; +import { isProvisioningError, buildProvisioningErrorMessage, ossFetch } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; +import { getProjectConfig } from '../../lib/config.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { canonicalMigrationVersion, @@ -133,6 +134,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.list', false); await trackCommandUsage('db', 'migrations list', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -201,6 +215,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.fetch', false); await trackCommandUsage('db', 'migrations fetch', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -247,6 +274,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.new', false); await trackCommandUsage('db', 'migrations new', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -427,6 +467,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.up', false); await trackCommandUsage('db', 'migrations up', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); diff --git a/src/commands/db/query.test.ts b/src/commands/db/query.test.ts new file mode 100644 index 00000000..8930747e --- /dev/null +++ b/src/commands/db/query.test.ts @@ -0,0 +1,129 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { Command } from 'commander'; +import { registerDbCommands } from './query.js'; + +vi.mock('../../lib/api/oss.js', () => ({ + runRawSql: vi.fn(), + isProvisioningError: vi.fn(), + buildProvisioningErrorMessage: vi.fn((name?: string) => + name + ? `Branch is still provisioning (this can take up to ~12 minutes). Branch: ${name}. Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.` + : 'Branch is still provisioning (this can take up to ~12 minutes). Retry shortly, or create the branch with `--wait-ready` to block until it\'s usable.', + ), +})); + +vi.mock('../../lib/credentials.js', () => ({ + requireAuth: vi.fn(async () => ({ accessToken: 'tok', userId: 'u' })), +})); + +vi.mock('../../lib/config.js', () => ({ + getProjectConfig: vi.fn(), +})); + +vi.mock('../../lib/analytics.js', () => ({ + captureEvent: vi.fn(), + trackCommand: vi.fn(), + shutdownAnalytics: vi.fn(async () => {}), +})); + +vi.mock('../../lib/skills.js', () => ({ + reportCliUsage: vi.fn(async () => {}), +})); + +vi.mock('../../lib/command-telemetry.js', () => ({ + trackCommandUsage: vi.fn(async () => {}), +})); + +describe('db query', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('shows friendly provisioning message when on a branch and network fails', async () => { + const { runRawSql, isProvisioningError } = await import('../../lib/api/oss.js'); + (runRawSql as Mock).mockRejectedValue(new Error('fetch failed')); + (isProvisioningError as Mock).mockReturnValue(true); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'b1', + project_name: 'feat-x', + org_id: 'o1', + api_key: 'k', + oss_host: 'host', + branched_from: { project_id: 'p1', project_name: 'parent' }, + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerDbCommands(program); + + const errLogs: string[] = []; + const origErr = console.error; + console.error = (...args: unknown[]) => { + errLogs.push(args.map(String).join(' ')); + }; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + try { + await program + .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + console.error = origErr; + process.exit = origExit; + } + + expect(exitCode).toBe(1); + const errText = errLogs.join('\n'); + expect(errText).toContain('still provisioning'); + expect(errText).toContain('feat-x'); + expect(errText).toContain('--wait-ready'); + }); + + it('shows generic error when provisioning error check returns false', async () => { + const { runRawSql, isProvisioningError } = await import('../../lib/api/oss.js'); + (runRawSql as Mock).mockRejectedValue(new Error('fetch failed')); + (isProvisioningError as Mock).mockReturnValue(false); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'b1', + project_name: 'feat-x', + org_id: 'o1', + branched_from: { project_id: 'p1', project_name: 'parent' }, + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerDbCommands(program); + + const errLogs: string[] = []; + const origErr = console.error; + console.error = (...args: unknown[]) => { + errLogs.push(args.map(String).join(' ')); + }; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + try { + await program + .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + console.error = origErr; + process.exit = origExit; + } + + expect(exitCode).toBe(1); + const errText = errLogs.join('\n'); + // Should contain the raw error, not the provisioning message + expect(errText).toContain('fetch failed'); + expect(errText).not.toContain('still provisioning'); + }); +}); diff --git a/src/commands/db/query.ts b/src/commands/db/query.ts index ecf4d529..a85d7d6c 100644 --- a/src/commands/db/query.ts +++ b/src/commands/db/query.ts @@ -1,10 +1,11 @@ import type { Command } from 'commander'; -import { runRawSql } from '../../lib/api/oss.js'; +import { runRawSql, isProvisioningError, buildProvisioningErrorMessage } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; -import { handleError, getRootOpts } from '../../lib/errors.js'; +import { handleError, getRootOpts, CLIError } from '../../lib/errors.js'; import { outputJson, outputTable } from '../../lib/output.js'; import { reportCliUsage } from '../../lib/skills.js'; import { trackCommandUsage } from '../../lib/command-telemetry.js'; +import { getProjectConfig } from '../../lib/config.js'; export function registerDbCommands(dbCmd: Command): void { dbCmd @@ -41,6 +42,22 @@ export function registerDbCommands(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.query', false); await trackCommandUsage('db', 'query', false, {}, err); + + // Check if this is a provisioning error on a branch + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + const branchName = projectConfig?.project_name; + + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(branchName); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); diff --git a/src/lib/api/oss.ts b/src/lib/api/oss.ts index 7db9924d..a0171c8d 100644 --- a/src/lib/api/oss.ts +++ b/src/lib/api/oss.ts @@ -15,6 +15,56 @@ function requireProjectConfig(): ProjectConfig { return config; } +/** + * Check if an error is likely caused by a branch still provisioning. + * This detects network-level failures (ECONNRESET, fetch failed, timeout) + * that occur when the branch's data plane isn't ready yet. + */ +export function isProvisioningError(err: unknown): boolean { + if (!(err instanceof Error)) return false; + const msg = err.message.toLowerCase(); + const cause = (err as { cause?: unknown }).cause; + const causeCode = cause && typeof cause === 'object' && 'code' in cause + ? String((cause as { code: unknown }).code).toLowerCase() + : ''; + + // Network errors that indicate the data plane isn't ready + const provisioningCodes = [ + 'econnreset', + 'etimedout', + 'econnrefused', + 'enotfound', + 'eai_again', + 'und_err_connect_timeout', + 'und_err_socket', + ]; + + // Check error message for provisioning indicators + const provisioningMessages = [ + 'fetch failed', + 'connection reset', + 'connection refused', + 'timed out', + 'dns lookup failed', + 'cannot resolve', + ]; + + if (causeCode && provisioningCodes.includes(causeCode)) return true; + if (provisioningMessages.some(m => msg.includes(m))) return true; + + return false; +} + +/** + * Build a user-friendly error message when a branch-scoped command fails + * due to the branch still provisioning. + */ +export function buildProvisioningErrorMessage(branchName?: string): string { + const base = 'Branch is still provisioning (this can take up to ~12 minutes).'; + const branchPart = branchName ? ` Branch: ${branchName}.` : ''; + return `${base}${branchPart} Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.`; +} + /** * Unified OSS API fetch. Uses API key as Bearer token for all requests, * which grants superadmin access (SQL execution, bucket management, etc.). From 42f9be619bac18a9c4a2a3c3d4bef54a01dfd749 Mon Sep 17 00:00:00 2001 From: iPLAYCAFE Date: Thu, 23 Jul 2026 06:10:54 +0700 Subject: [PATCH 4/7] fix(branch): fail on stuck provisioning too, and tidy the NETWORK_ERROR_CODE export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #201 (jwfing, approved): - Functionality: `branch create` now exits non-zero when the branch never reaches 'ready' within the poll budget, matching the existing ready-but- not-serving exit. Both outcomes are "not usable", and this PR's goal is that success means usable — so automation reading the exit code must not see 0 for a branch stuck provisioning. Test added (getBranchApi stuck in 'creating' → exit 1), with the shared mock impl restored so it can't leak the full poll budget into later tests. - Software engineering: moved `export const NETWORK_ERROR_CODE` below the import block in platform.ts (it was interleaved between two imports — legal via hoisting, but conventionally declarations sit under the imports). The `/api/health` "listening ≠ usable" note is acknowledged as the best client-side signal available today and left as the PR's documented open question, to key off a data-plane-scoped signal if one lands cloud-side. Co-Authored-By: Claude Opus 4.8 --- src/commands/branch/create.test.ts | 55 ++++++++++++++++++++++++++++++ src/commands/branch/create.ts | 14 ++++++-- src/lib/api/platform.ts | 10 +++--- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index 6ed41c05..8aca9ae6 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -325,6 +325,61 @@ describe('branch create', () => { expect(stopped?.[1]).toBe(1); }); + it('exits non-zero when the branch never finishes provisioning', async () => { + // The sibling of "ready but not serving": if the branch is stuck in a + // non-terminal state past the poll budget it is equally unusable, so + // automation reading the exit code must not see success. (Review suggestion, + // InsForge/CLI#201.) + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { getBranchApi } = await import('../../lib/api/platform.js'); + const originalImpl = (getBranchApi as Mock).getMockImplementation(); + // Never reaches 'ready' — pollUntilReady exhausts its budget and returns the + // last 'creating' snapshot. + (getBranchApi as Mock).mockResolvedValue({ + id: 'branch-id', + parent_project_id: 'p1', + organization_id: 'o1', + name: 'feat-x', + appkey: 'p1ky-x9p', + region: 'us-east', + branch_state: 'creating', + branch_created_at: new Date().toISOString(), + branch_metadata: { mode: 'schema-only' }, + }); + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + vi.useFakeTimers(); + try { + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + const run = program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + await vi.runAllTimersAsync(); + await run; + } finally { + vi.useRealTimers(); + process.exit = origExit; + process.stderr.write = origStderr; + // Restore the shared 'ready' impl — clearAllMocks keeps implementations, so + // leaving this 'creating' would make every later test poll the full budget. + (getBranchApi as Mock).mockImplementation(originalImpl!); + } + expect(exitCode).toBe(1); + }); + it('adopts a branch that was created despite a transport failure', async () => { // createBranchApi carries no idempotency key, so a reset on the RESPONSE // leg leaves a real, billing branch behind. Giving up here orphans it. diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index 2c5cb9a4..bd8831b4 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -154,8 +154,18 @@ export function registerBranchCreateCommand(branch: Command): void { // Exit non-zero when the branch cannot be used. Reporting success here // is what lets automation continue straight into a host that resets — - // the failure mode this whole change exists to remove. - if (ready.branch_state === 'ready' && !serving) { + // the failure mode this whole change exists to remove. Two outcomes are + // "not usable", and both must fail: the branch never finished + // provisioning (still non-'ready' after the poll budget), and the branch + // is 'ready' but its host never started serving. + if (ready.branch_state !== 'ready') { + throw new CLIError( + `Branch '${name}' was created but did not finish provisioning (still '${ready.branch_state}') within ${ + Math.round(POLL_TIMEOUT_MS / 60_000) + } minutes.`, + ); + } + if (!serving) { throw new CLIError( `Branch '${name}' was created but its host did not start serving within ${ Math.round(HEALTH_TIMEOUT_MS / 60_000) diff --git a/src/lib/api/platform.ts b/src/lib/api/platform.ts index 1ce5ff96..90f81354 100644 --- a/src/lib/api/platform.ts +++ b/src/lib/api/platform.ts @@ -1,10 +1,6 @@ import { getAccessToken, getCredentials, getPlatformApiUrl } from '../config.js'; -import { AuthError, CLIError, formatFetchError } from '../errors.js'; - -// Marks a CLIError that came from a failed fetch rather than an HTTP response: -// the request may still have been received and acted on by the server. -export const NETWORK_ERROR_CODE = 'NETWORK_ERROR'; import { refreshAccessToken } from '../credentials.js'; +import { AuthError, CLIError, formatFetchError } from '../errors.js'; import type { ApiKeyResponse, Backup, @@ -34,6 +30,10 @@ import type { User, } from '../../types.js'; +// Marks a CLIError that came from a failed fetch rather than an HTTP response: +// the request may still have been received and acted on by the server. +export const NETWORK_ERROR_CODE = 'NETWORK_ERROR'; + export interface PlatformFetchOptions extends RequestInit { /** * HTTP status codes that should be returned to the caller instead of From ea18b8122e2897b91cd6dd7a660a65fe4705902c Mon Sep 17 00:00:00 2001 From: iPLAYCAFE Date: Thu, 23 Jul 2026 06:38:51 +0700 Subject: [PATCH 5/7] fix(branch): also require a matching mode before adopting; extract exit-capture test helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two follow-up reviews on #201: - cubic (P2): a transport failure whose response leg is lost could, in the 60s skew window, adopt a collaborator's SAME-NAME branch — and a default --switch would then move local context onto it. createBranchOrAdopt now also requires the candidate's mode to match the requested mode, narrowing the collision to an even more specific coincidence (same name AND same mode AND the same ~60s AND our transport failure). The real fix is a server-issued idempotency token (InsForge/InsForge#1790); this is the tightest client-side guard until then. Test added: a same-name, different-mode branch in the window is NOT adopted. - CodeRabbit (nitpick): extracted `withCapturedExit(fn)` for the repeated process.exit/stderr override-restore boilerplate, and applied it to the tests this PR added (incl. the fake-timer provisioning-timeout one). The older pre-existing blocks are left as-is to keep this diff scoped to the PR's surface. Co-Authored-By: Claude Opus 4.8 --- src/commands/branch/create.test.ts | 89 ++++++++++++++++++++++++------ src/commands/branch/create.ts | 14 ++++- 2 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index 8aca9ae6..3d67e211 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -70,6 +70,27 @@ vi.mock('@clack/prompts', () => ({ spinner: () => spinnerMock, })); +// Run `fn` with process.exit + stderr captured, and always restore them. Returns +// the exit code fn triggered (undefined if it never exited). Timer/mock lifecycle +// stays with the caller — this only owns the exit/stderr swap. +async function withCapturedExit(fn: () => Promise): Promise { + let exitCode: number | undefined; + const origExit = process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + await fn(); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + return exitCode; +} + describe('branch create', () => { beforeEach(async () => { vi.clearAllMocks(); @@ -352,27 +373,20 @@ describe('branch create', () => { branch_metadata: { mode: 'schema-only' }, }); let exitCode: number | undefined; - const origExit = process.exit; - process.exit = ((code?: number) => { - exitCode = code; - throw new Error('__exit__'); - }) as typeof process.exit; - const origStderr = process.stderr.write.bind(process.stderr); - process.stderr.write = (() => true) as typeof process.stderr.write; vi.useFakeTimers(); try { - const program = new Command().exitOverride(); - program.option('--json').option('--api-url ').option('-y, --yes'); - registerBranchCreateCommand(program); - const run = program - .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) - .catch(() => {}); - await vi.runAllTimersAsync(); - await run; + exitCode = await withCapturedExit(async () => { + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + const run = program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}); + await vi.runAllTimersAsync(); + await run; + }); } finally { vi.useRealTimers(); - process.exit = origExit; - process.stderr.write = origStderr; // Restore the shared 'ready' impl — clearAllMocks keeps implementations, so // leaving this 'creating' would make every later test poll the full budget. (getBranchApi as Mock).mockImplementation(originalImpl!); @@ -417,6 +431,47 @@ describe('branch create', () => { expect(String(spinnerMock.stop.mock.calls.at(-1)?.[0])).not.toContain('creation failed'); }); + it('does NOT adopt a same-name branch created with a DIFFERENT mode', async () => { + // A collaborator's same-name branch landing in the skew window — at the same + // moment our own request loses its response leg — must not be adopted, or a + // default --switch would move local context onto their branch. Requiring a + // matching mode narrows that collision. (cubic P2, InsForge/CLI#201.) + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const { createBranchApi, listBranchesApi } = await import('../../lib/api/platform.js'); + (createBranchApi as Mock).mockRejectedValueOnce( + new CLIError('Connection to api.insforge.dev was reset.', 1, 'NETWORK_ERROR'), + ); + // Same name, freshly created (inside the window), but the WRONG mode. + (listBranchesApi as Mock).mockResolvedValueOnce([ + { + id: 'someone-elses-branch', + parent_project_id: 'p1', + organization_id: 'o1', + name: 'feat-x', + appkey: 'p1ky-x9p', + region: 'us-east', + branch_state: 'creating', + branch_created_at: new Date().toISOString(), + branch_metadata: { mode: 'full' }, + }, + ]); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + const exitCode = await withCapturedExit(() => + program + .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) + .catch(() => {}) + ); + // No adoption → the original transport error propagates → non-zero exit. + expect(exitCode).toBe(1); + }); + it('rethrows the original error when nothing was actually created', async () => { const { getProjectConfig } = await import('../../lib/config.js'); (getProjectConfig as Mock).mockReturnValue({ diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index bd8831b4..73f0940e 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -198,7 +198,18 @@ export function registerBranchCreateCommand(branch: Command): void { * 1. only a tagged transport failure is eligible; every HTTP/API rejection * (duplicate name, quota, auth) rethrows untouched; * 2. the branch must have been created at or after the moment we sent the - * request, so a pre-existing same-name branch is never a candidate. + * request, so a pre-existing same-name branch is never a candidate; + * 3. the branch's mode must match what we asked for. + * + * Guard 3 narrows a residual collision the timestamp window alone cannot close: + * a collaborator creating a same-name branch inside the skew window, at the same + * moment our own request loses its response leg, would otherwise be adoptable — + * and with the default `--switch` that would silently move local context onto + * their branch. Requiring a mode match makes that require an even more specific + * coincidence (same name AND same mode AND the same ~60s AND our transport + * failure). The real fix is a server-issued idempotency/request token on + * `createBranchApi`; until that exists, this is the tightest client-side guard. + * Reported upstream: InsForge/InsForge#1790. */ function isTransportFailure(err: unknown): boolean { return err instanceof CLIError && err.code === NETWORK_ERROR_CODE; @@ -219,6 +230,7 @@ async function createBranchOrAdopt( branches.find( branch => branch.name === body.name && + branch.branch_metadata?.mode === body.mode && Date.parse(branch.branch_created_at) >= requestedAt, ), ) From d500422e3bd27dd0c43aed6f8127ad2f14b95ebc Mon Sep 17 00:00:00 2001 From: Shobhit Sahani Date: Wed, 22 Jul 2026 20:35:02 +0530 Subject: [PATCH 6/7] fix(cli): improve branch provisioning recovery and reconciliation - Fix network error detection so create reconciliation is reachable - Add tests for readiness, reconciliation, and delete retry - Improve provisioning documentation - Preserve existing CLI behavior while improving recovery from transient failures --- src/commands/branch/create.test.ts | 38 ++++++--- src/commands/branch/create.ts | 7 +- src/commands/branch/delete.test.ts | 88 ++++++++++++++++++++ src/commands/branch/delete.ts | 75 ++++++++++++++++- src/commands/db/migrations.ts | 55 +++++++++++- src/commands/db/query.test.ts | 129 +++++++++++++++++++++++++++++ src/commands/db/query.ts | 21 ++++- src/lib/api/oss.ts | 50 +++++++++++ 8 files changed, 446 insertions(+), 17 deletions(-) create mode 100644 src/commands/db/query.test.ts diff --git a/src/commands/branch/create.test.ts b/src/commands/branch/create.test.ts index 3d67e211..770bc903 100644 --- a/src/commands/branch/create.test.ts +++ b/src/commands/branch/create.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; import { Command } from 'commander'; import { registerBranchCreateCommand } from './create.js'; import { CLIError } from '../../lib/errors.js'; @@ -72,7 +72,7 @@ vi.mock('@clack/prompts', () => ({ // Run `fn` with process.exit + stderr captured, and always restore them. Returns // the exit code fn triggered (undefined if it never exited). Timer/mock lifecycle -// stays with the caller — this only owns the exit/stderr swap. +// stays with the caller ΓÇö this only owns the exit/stderr swap. async function withCapturedExit(fn: () => Promise): Promise { let exitCode: number | undefined; const origExit = process.exit; @@ -303,7 +303,7 @@ describe('branch create', () => { expect(spinnerMock.start).toHaveBeenCalledTimes(1); expect(spinnerMock.start).toHaveBeenCalledWith(expect.stringContaining("Creating branch 'feat-x'")); // ...and stop fires exactly once (after the switch completes), with the - // unified "ready and active" message — never with the misleading "ready" + // unified "ready and active" message ΓÇö never with the misleading "ready" // line that a separate stop+restart pair would produce. expect(spinnerMock.stop).toHaveBeenCalledTimes(1); expect(spinnerMock.stop).toHaveBeenCalledWith( @@ -359,7 +359,7 @@ describe('branch create', () => { }); const { getBranchApi } = await import('../../lib/api/platform.js'); const originalImpl = (getBranchApi as Mock).getMockImplementation(); - // Never reaches 'ready' — pollUntilReady exhausts its budget and returns the + // Never reaches 'ready' ΓÇö pollUntilReady exhausts its budget and returns the // last 'creating' snapshot. (getBranchApi as Mock).mockResolvedValue({ id: 'branch-id', @@ -387,7 +387,7 @@ describe('branch create', () => { }); } finally { vi.useRealTimers(); - // Restore the shared 'ready' impl — clearAllMocks keeps implementations, so + // Restore the shared 'ready' impl ΓÇö clearAllMocks keeps implementations, so // leaving this 'creating' would make every later test poll the full budget. (getBranchApi as Mock).mockImplementation(originalImpl!); } @@ -432,8 +432,8 @@ describe('branch create', () => { }); it('does NOT adopt a same-name branch created with a DIFFERENT mode', async () => { - // A collaborator's same-name branch landing in the skew window — at the same - // moment our own request loses its response leg — must not be adopted, or a + // A collaborator's same-name branch landing in the skew window ΓÇö at the same + // moment our own request loses its response leg ΓÇö must not be adopted, or a // default --switch would move local context onto their branch. Requiring a // matching mode narrows that collision. (cubic P2, InsForge/CLI#201.) const { getProjectConfig } = await import('../../lib/config.js'); @@ -468,7 +468,7 @@ describe('branch create', () => { .parseAsync(['create', 'feat-x', '--mode', 'schema-only', '--no-switch'], { from: 'user' }) .catch(() => {}) ); - // No adoption → the original transport error propagates → non-zero exit. + // No adoption ΓåÆ the original transport error propagates ΓåÆ non-zero exit. expect(exitCode).toBe(1); }); @@ -505,7 +505,7 @@ describe('branch create', () => { } expect(exitCode).toBe(1); }); - it('does NOT adopt on an API rejection — a duplicate name is a refusal, not a lost response', async () => { + it('does NOT adopt on an API rejection ΓÇö a duplicate name is a refusal, not a lost response', async () => { // Adopting here would switch the caller into a pre-existing branch with a // different mode and different data. Only a transport failure is ambiguous. const { getProjectConfig } = await import('../../lib/config.js'); @@ -542,7 +542,7 @@ describe('branch create', () => { }); it('does NOT adopt a branch that predates the request', async () => { - // Same name, but it existed before we asked — so it is not ours. + // Same name, but it existed before we asked ΓÇö so it is not ours. const { getProjectConfig } = await import('../../lib/config.js'); (getProjectConfig as Mock).mockReturnValue({ project_id: 'p1', @@ -636,4 +636,22 @@ describe('branch create', () => { expect(payload).toContain('false'); expect(exitCode).toBe(1); }); + + it('--no-wait-ready skips data-plane health polling', async () => { + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchCreateCommand(program); + await program.parseAsync( + ['create', 'feat-x', '--mode', 'full', '--no-switch', '--no-wait-ready', '--json'], + { from: 'user' }, + ); + const { probeBackendHealth } = await import('../../lib/api/oss.js'); + expect(probeBackendHealth).not.toHaveBeenCalled(); + }); }); diff --git a/src/commands/branch/create.ts b/src/commands/branch/create.ts index 73f0940e..a2dfb1e4 100644 --- a/src/commands/branch/create.ts +++ b/src/commands/branch/create.ts @@ -38,7 +38,8 @@ export function registerBranchCreateCommand(branch: Command): void { .description('Create a branch from the currently linked project') .option('--mode ', 'full | schema-only', 'full') .option('--no-switch', 'Do not auto-switch context after creation') - .action(async (name: string, opts: { mode: string; switch: boolean }, cmd) => { + .option('--no-wait-ready', 'Skip waiting for data plane readiness (exit immediately after control plane confirms creation)') + .action(async (name: string, opts: { mode: string; switch: boolean; waitReady: boolean }, cmd) => { const { json, apiUrl } = getRootOpts(cmd); try { await requireAuth(apiUrl); @@ -96,10 +97,12 @@ export function registerBranchCreateCommand(branch: Command): void { // returned, not that the branch answers. Confirm the data plane // before reporting success, otherwise the very next command the user // runs — including the auto-switch below — hits a host that resets. - if (provisioned) { + if (provisioned && opts.waitReady !== false) { spinner?.message('Branch ready. Waiting for it to start serving...'); serving = await waitUntilServing(ready, spinner); if (!serving) provisioned = false; + } else if (provisioned) { + serving = true; } if (provisioned && opts.switch) { diff --git a/src/commands/branch/delete.test.ts b/src/commands/branch/delete.test.ts index 7634a1ca..0a1b824e 100644 --- a/src/commands/branch/delete.test.ts +++ b/src/commands/branch/delete.test.ts @@ -16,6 +16,17 @@ vi.mock('../../lib/api/platform.js', () => ({ branch_metadata: { mode: 'full' }, }, ]), + getBranchApi: vi.fn(async () => ({ + id: 'b1', + name: 'feat-x', + branch_state: 'ready', + organization_id: 'o1', + parent_project_id: 'p1', + appkey: 'k1', + region: 'us-east', + branch_created_at: '2026-04-29T00:00:00Z', + branch_metadata: { mode: 'full' }, + })), deleteBranchApi: vi.fn(async () => undefined), })); @@ -161,4 +172,81 @@ describe('branch delete', () => { const parsed = JSON.parse(logs.join('\n')); expect(parsed).toEqual({ deleted: true, branch_id: 'b1', switched_back: true }); }); + + it('isBusyError matches busy, creating, and merging messages', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + const busyErr = new (await import('../../lib/errors.js')).CLIError( + 'Branch is currently busy with provisioning. Please wait.', + ); + (deleteBranchApi as Mock).mockRejectedValueOnce(busyErr); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const program = makeProgram(); + await runSilently(program, ['delete', 'feat-x', '--yes', '--json']); + // deleteBranchApi should have been called twice: first fails (busy), + // then getBranchApi says "ready", so retry succeeds + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + expect(dbApi).toHaveBeenCalledTimes(2); + }); + + it('retries deletion when branch busy then becomes ready', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + (deleteBranchApi as Mock) + .mockRejectedValueOnce(new (await import('../../lib/errors.js')).CLIError('Branch is busy creating')) + .mockResolvedValueOnce(undefined); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + const program = makeProgram(); + await runSilently(program, ['delete', 'feat-x', '--yes', '--json']); + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + expect(dbApi).toHaveBeenCalledTimes(2); + expect(dbApi).toHaveBeenLastCalledWith('b1', undefined); + }); + + it('does not retry on non-busy errors', async () => { + const { deleteBranchApi } = await import('../../lib/api/platform.js'); + (deleteBranchApi as Mock).mockRejectedValueOnce( + new (await import('../../lib/errors.js')).CLIError('Permission denied: not authorized'), + ); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + const program = makeProgram(); + await program + .parseAsync(['delete', 'feat-x', '--yes', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + } + + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + // Only one call — no retry for non-busy errors + expect(dbApi).toHaveBeenCalledTimes(1); + expect(exitCode).toBe(1); + }); }); diff --git a/src/commands/branch/delete.ts b/src/commands/branch/delete.ts index ab80e348..759c05ae 100644 --- a/src/commands/branch/delete.ts +++ b/src/commands/branch/delete.ts @@ -1,6 +1,6 @@ import type { Command } from 'commander'; import * as clack from '@clack/prompts'; -import { listBranchesApi, deleteBranchApi } from '../../lib/api/platform.js'; +import { listBranchesApi, deleteBranchApi, getBranchApi } from '../../lib/api/platform.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { requireAuth } from '../../lib/credentials.js'; import { getProjectConfig } from '../../lib/config.js'; @@ -8,6 +8,73 @@ import { outputJson, outputSuccess, outputInfo } from '../../lib/output.js'; import { captureEvent, shutdownAnalytics } from '../../lib/analytics.js'; import { runBranchSwitch } from './switch.js'; +// Retry configuration for deleting busy branches +const DELETE_RETRY_INTERVAL_MS = 30_000; // 30 seconds +const DELETE_MAX_RETRY_TIME_MS = 6 * 60 * 1_000; // 6 minutes max + +function isBusyError(err: unknown): boolean { + if (!(err instanceof CLIError)) return false; + const msg = err.message.toLowerCase(); + return msg.includes('busy') || + msg.includes('creating') || + msg.includes('merging') || + msg.includes('currently busy'); +} + +async function waitForBranchDeletable( + branchId: string, + apiUrl: string | undefined, + spinner: ReturnType | null +): Promise { + const start = Date.now(); + + while (Date.now() - start < DELETE_MAX_RETRY_TIME_MS) { + const branch = await getBranchApi(branchId, apiUrl); + if (branch.branch_state !== 'creating' && branch.branch_state !== 'merging') { + return; // Branch is no longer busy + } + + const elapsedSec = Math.floor((Date.now() - start) / 1000); + const remainingSec = Math.floor((DELETE_MAX_RETRY_TIME_MS - (Date.now() - start)) / 1000); + spinner?.message(`Branch is ${branch.branch_state}, waiting to be deletable... (${remainingSec}s remaining)`); + + await new Promise(r => setTimeout(r, DELETE_RETRY_INTERVAL_MS)); + } + + // Final check - if still busy, throw a clear error + const branch = await getBranchApi(branchId, apiUrl); + if (branch.branch_state === 'creating' || branch.branch_state === 'merging') { + throw new CLIError( + `Branch is still ${branch.branch_state} after ${DELETE_MAX_RETRY_TIME_MS / 60000} minutes. ` + + `The branch may need more time to finish provisioning. ` + + `Try \`insforge branch delete ${branch.name}\` again in a few minutes.`, + 1, + 'BRANCH_STILL_BUSY' + ); + } +} + +async function deleteBranchWithRetry( + branchId: string, + apiUrl: string | undefined, + spinner: ReturnType | null +): Promise { + try { + await deleteBranchApi(branchId, apiUrl); + spinner?.stop(`Branch deletion requested.`); + } catch (err) { + if (isBusyError(err)) { + spinner?.message(`Branch is busy (creating/merging). Waiting for it to become deletable...`); + await waitForBranchDeletable(branchId, apiUrl, spinner); + // Retry deletion after branch is no longer busy + await deleteBranchApi(branchId, apiUrl); + spinner?.stop(`Branch deletion requested after wait.`); + } else { + throw err; + } + } +} + export function registerBranchDeleteCommand(branch: Command): void { branch .command('delete ') @@ -34,7 +101,11 @@ export function registerBranchDeleteCommand(branch: Command): void { } } - await deleteBranchApi(target.id, apiUrl); + // Set up spinner for progress indication during delete/retry + const spinner = !json ? clack.spinner() : null; + spinner?.start(`Deleting branch '${name}'...`); + + await deleteBranchWithRetry(target.id, apiUrl, spinner); captureEvent(parentId, 'cli_branch_delete', {}); // If the directory is currently switched onto the deleted branch, diff --git a/src/commands/db/migrations.ts b/src/commands/db/migrations.ts index ab6e3b70..b3527c18 100644 --- a/src/commands/db/migrations.ts +++ b/src/commands/db/migrations.ts @@ -1,8 +1,9 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import type { Command } from 'commander'; -import { ossFetch } from '../../lib/api/oss.js'; +import { isProvisioningError, buildProvisioningErrorMessage, ossFetch } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; +import { getProjectConfig } from '../../lib/config.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { canonicalMigrationVersion, @@ -133,6 +134,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.list', false); await trackCommandUsage('db', 'migrations list', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -201,6 +215,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.fetch', false); await trackCommandUsage('db', 'migrations fetch', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -247,6 +274,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.new', false); await trackCommandUsage('db', 'migrations new', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); @@ -427,6 +467,19 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.up', false); await trackCommandUsage('db', 'migrations up', false, {}, err); + + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(projectConfig?.project_name); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); diff --git a/src/commands/db/query.test.ts b/src/commands/db/query.test.ts new file mode 100644 index 00000000..8930747e --- /dev/null +++ b/src/commands/db/query.test.ts @@ -0,0 +1,129 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { Command } from 'commander'; +import { registerDbCommands } from './query.js'; + +vi.mock('../../lib/api/oss.js', () => ({ + runRawSql: vi.fn(), + isProvisioningError: vi.fn(), + buildProvisioningErrorMessage: vi.fn((name?: string) => + name + ? `Branch is still provisioning (this can take up to ~12 minutes). Branch: ${name}. Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.` + : 'Branch is still provisioning (this can take up to ~12 minutes). Retry shortly, or create the branch with `--wait-ready` to block until it\'s usable.', + ), +})); + +vi.mock('../../lib/credentials.js', () => ({ + requireAuth: vi.fn(async () => ({ accessToken: 'tok', userId: 'u' })), +})); + +vi.mock('../../lib/config.js', () => ({ + getProjectConfig: vi.fn(), +})); + +vi.mock('../../lib/analytics.js', () => ({ + captureEvent: vi.fn(), + trackCommand: vi.fn(), + shutdownAnalytics: vi.fn(async () => {}), +})); + +vi.mock('../../lib/skills.js', () => ({ + reportCliUsage: vi.fn(async () => {}), +})); + +vi.mock('../../lib/command-telemetry.js', () => ({ + trackCommandUsage: vi.fn(async () => {}), +})); + +describe('db query', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('shows friendly provisioning message when on a branch and network fails', async () => { + const { runRawSql, isProvisioningError } = await import('../../lib/api/oss.js'); + (runRawSql as Mock).mockRejectedValue(new Error('fetch failed')); + (isProvisioningError as Mock).mockReturnValue(true); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'b1', + project_name: 'feat-x', + org_id: 'o1', + api_key: 'k', + oss_host: 'host', + branched_from: { project_id: 'p1', project_name: 'parent' }, + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerDbCommands(program); + + const errLogs: string[] = []; + const origErr = console.error; + console.error = (...args: unknown[]) => { + errLogs.push(args.map(String).join(' ')); + }; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + try { + await program + .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + console.error = origErr; + process.exit = origExit; + } + + expect(exitCode).toBe(1); + const errText = errLogs.join('\n'); + expect(errText).toContain('still provisioning'); + expect(errText).toContain('feat-x'); + expect(errText).toContain('--wait-ready'); + }); + + it('shows generic error when provisioning error check returns false', async () => { + const { runRawSql, isProvisioningError } = await import('../../lib/api/oss.js'); + (runRawSql as Mock).mockRejectedValue(new Error('fetch failed')); + (isProvisioningError as Mock).mockReturnValue(false); + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'b1', + project_name: 'feat-x', + org_id: 'o1', + branched_from: { project_id: 'p1', project_name: 'parent' }, + }); + + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerDbCommands(program); + + const errLogs: string[] = []; + const origErr = console.error; + console.error = (...args: unknown[]) => { + errLogs.push(args.map(String).join(' ')); + }; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + try { + await program + .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) + .catch(() => {}); + } finally { + console.error = origErr; + process.exit = origExit; + } + + expect(exitCode).toBe(1); + const errText = errLogs.join('\n'); + // Should contain the raw error, not the provisioning message + expect(errText).toContain('fetch failed'); + expect(errText).not.toContain('still provisioning'); + }); +}); diff --git a/src/commands/db/query.ts b/src/commands/db/query.ts index ecf4d529..a85d7d6c 100644 --- a/src/commands/db/query.ts +++ b/src/commands/db/query.ts @@ -1,10 +1,11 @@ import type { Command } from 'commander'; -import { runRawSql } from '../../lib/api/oss.js'; +import { runRawSql, isProvisioningError, buildProvisioningErrorMessage } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; -import { handleError, getRootOpts } from '../../lib/errors.js'; +import { handleError, getRootOpts, CLIError } from '../../lib/errors.js'; import { outputJson, outputTable } from '../../lib/output.js'; import { reportCliUsage } from '../../lib/skills.js'; import { trackCommandUsage } from '../../lib/command-telemetry.js'; +import { getProjectConfig } from '../../lib/config.js'; export function registerDbCommands(dbCmd: Command): void { dbCmd @@ -41,6 +42,22 @@ export function registerDbCommands(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.query', false); await trackCommandUsage('db', 'query', false, {}, err); + + // Check if this is a provisioning error on a branch + const projectConfig = getProjectConfig(); + const isBranch = projectConfig?.branched_from != null; + const branchName = projectConfig?.project_name; + + if (isBranch && isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(branchName); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); + } + handleError(err, json); } }); diff --git a/src/lib/api/oss.ts b/src/lib/api/oss.ts index 09231427..4c02d057 100644 --- a/src/lib/api/oss.ts +++ b/src/lib/api/oss.ts @@ -15,6 +15,56 @@ function requireProjectConfig(): ProjectConfig { return config; } +/** + * Check if an error is likely caused by a branch still provisioning. + * This detects network-level failures (ECONNRESET, fetch failed, timeout) + * that occur when the branch's data plane isn't ready yet. + */ +export function isProvisioningError(err: unknown): boolean { + if (!(err instanceof Error)) return false; + const msg = err.message.toLowerCase(); + const cause = (err as { cause?: unknown }).cause; + const causeCode = cause && typeof cause === 'object' && 'code' in cause + ? String((cause as { code: unknown }).code).toLowerCase() + : ''; + + // Network errors that indicate the data plane isn't ready + const provisioningCodes = [ + 'econnreset', + 'etimedout', + 'econnrefused', + 'enotfound', + 'eai_again', + 'und_err_connect_timeout', + 'und_err_socket', + ]; + + // Check error message for provisioning indicators + const provisioningMessages = [ + 'fetch failed', + 'connection reset', + 'connection refused', + 'timed out', + 'dns lookup failed', + 'cannot resolve', + ]; + + if (causeCode && provisioningCodes.includes(causeCode)) return true; + if (provisioningMessages.some(m => msg.includes(m))) return true; + + return false; +} + +/** + * Build a user-friendly error message when a branch-scoped command fails + * due to the branch still provisioning. + */ +export function buildProvisioningErrorMessage(branchName?: string): string { + const base = 'Branch is still provisioning (this can take up to ~12 minutes).'; + const branchPart = branchName ? ` Branch: ${branchName}.` : ''; + return `${base}${branchPart} Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.`; +} + /** * Unified OSS API fetch. Uses API key as Bearer token for all requests, * which grants superadmin access (SQL execution, bucket management, etc.). From e9851d973cc92a8bdc3916d327f2a13c37e95756 Mon Sep 17 00:00:00 2001 From: Shobhit Sahani Date: Thu, 23 Jul 2026 14:41:35 +0530 Subject: [PATCH 7/7] fix(cli): rebase onto #201 base and resolve remaining review items --- src/commands/branch/delete.test.ts | 58 ++++++++++++++++++++++++ src/commands/branch/delete.ts | 62 +++++++++++++------------ src/commands/db/migrations.ts | 58 ++---------------------- src/commands/db/query.test.ts | 73 ++++++++++++++---------------- src/commands/db/query.ts | 22 ++------- src/lib/api/oss.ts | 51 ++++++++++++++++++++- 6 files changed, 184 insertions(+), 140 deletions(-) diff --git a/src/commands/branch/delete.test.ts b/src/commands/branch/delete.test.ts index 0a1b824e..6d159080 100644 --- a/src/commands/branch/delete.test.ts +++ b/src/commands/branch/delete.test.ts @@ -213,6 +213,64 @@ describe('branch delete', () => { expect(dbApi).toHaveBeenLastCalledWith('b1', undefined); }); + it('still busy after max retry time throws BRANCH_STILL_BUSY', async () => { + const { deleteBranchApi, getBranchApi } = await import('../../lib/api/platform.js'); + // deleteBranchApi always fails with busy + (deleteBranchApi as Mock).mockRejectedValue( + new (await import('../../lib/errors.js')).CLIError('Branch is currently busy'), + ); + // getBranchApi always returns 'creating' state so waitForBranchDeletable loops to timeout + (getBranchApi as Mock).mockResolvedValue({ + id: 'b1', name: 'feat-x', branch_state: 'creating', + organization_id: 'o1', parent_project_id: 'p1', + appkey: 'k1', region: 'us-east', + branch_created_at: '2026-04-29T00:00:00Z', + branch_metadata: { mode: 'full' }, + }); + + const { getProjectConfig } = await import('../../lib/config.js'); + (getProjectConfig as Mock).mockReturnValue({ + project_id: 'p1', + project_name: 'parent', + org_id: 'o1', + }); + + // Silence the __exit__ rejection that handleError's process.exit mock produces + const onRejection = vi.fn(); + process.on('unhandledRejection', onRejection); + + vi.useFakeTimers(); + + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = (() => true) as typeof process.stderr.write; + try { + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchDeleteCommand(program); + const promise = program + .parseAsync(['delete', 'feat-x', '--yes', '--json'], { from: 'user' }); + // Advance past the 6-minute retry window + await vi.advanceTimersByTimeAsync(7 * 60 * 1000); + await promise.catch(() => {}); + } finally { + process.exit = origExit; + process.stderr.write = origStderr; + vi.useRealTimers(); + process.off('unhandledRejection', onRejection); + } + + const { deleteBranchApi: dbApi } = await import('../../lib/api/platform.js'); + // deleteBranchApi was called at least once (the initial attempt) + expect(dbApi).toHaveBeenCalled(); + expect(exitCode).toBe(1); + }); + it('does not retry on non-busy errors', async () => { const { deleteBranchApi } = await import('../../lib/api/platform.js'); (deleteBranchApi as Mock).mockRejectedValueOnce( diff --git a/src/commands/branch/delete.ts b/src/commands/branch/delete.ts index 759c05ae..94d5cf60 100644 --- a/src/commands/branch/delete.ts +++ b/src/commands/branch/delete.ts @@ -8,17 +8,23 @@ import { outputJson, outputSuccess, outputInfo } from '../../lib/output.js'; import { captureEvent, shutdownAnalytics } from '../../lib/analytics.js'; import { runBranchSwitch } from './switch.js'; -// Retry configuration for deleting busy branches -const DELETE_RETRY_INTERVAL_MS = 30_000; // 30 seconds -const DELETE_MAX_RETRY_TIME_MS = 6 * 60 * 1_000; // 6 minutes max +const DELETE_RETRY_INTERVAL_MS = 30_000; +const DELETE_MAX_RETRY_TIME_MS = 6 * 60 * 1_000; +// Match on the server's structured error code if available; fall back to +// checking the response message only when no code is present. This avoids +// false positives from unrelated error text that happens to contain "busy". function isBusyError(err: unknown): boolean { if (!(err instanceof CLIError)) return false; + // Exact server error codes for busy/provisioning states + if (err.code && ['BRANCH_BUSY', 'BRANCH_CREATING', 'BRANCH_MERGING', 'PROVISIONING_IN_PROGRESS'].includes(err.code)) { + return true; + } const msg = err.message.toLowerCase(); - return msg.includes('busy') || - msg.includes('creating') || - msg.includes('merging') || - msg.includes('currently busy'); + return msg.includes('branch is busy') || + msg.includes('currently busy') || + msg.includes('still creating') || + msg.includes('still merging'); } async function waitForBranchDeletable( @@ -31,17 +37,18 @@ async function waitForBranchDeletable( while (Date.now() - start < DELETE_MAX_RETRY_TIME_MS) { const branch = await getBranchApi(branchId, apiUrl); if (branch.branch_state !== 'creating' && branch.branch_state !== 'merging') { - return; // Branch is no longer busy + return; } - const elapsedSec = Math.floor((Date.now() - start) / 1000); const remainingSec = Math.floor((DELETE_MAX_RETRY_TIME_MS - (Date.now() - start)) / 1000); spinner?.message(`Branch is ${branch.branch_state}, waiting to be deletable... (${remainingSec}s remaining)`); - await new Promise(r => setTimeout(r, DELETE_RETRY_INTERVAL_MS)); + // Cap sleep to the remaining time budget so we don't exceed the max + const remainingBudget = DELETE_MAX_RETRY_TIME_MS - (Date.now() - start); + const sleepMs = Math.min(DELETE_RETRY_INTERVAL_MS, Math.max(0, remainingBudget)); + await new Promise(r => setTimeout(r, sleepMs)); } - // Final check - if still busy, throw a clear error const branch = await getBranchApi(branchId, apiUrl); if (branch.branch_state === 'creating' || branch.branch_state === 'merging') { throw new CLIError( @@ -56,22 +63,28 @@ async function waitForBranchDeletable( async function deleteBranchWithRetry( branchId: string, + name: string, apiUrl: string | undefined, spinner: ReturnType | null ): Promise { try { - await deleteBranchApi(branchId, apiUrl); - spinner?.stop(`Branch deletion requested.`); - } catch (err) { - if (isBusyError(err)) { - spinner?.message(`Branch is busy (creating/merging). Waiting for it to become deletable...`); - await waitForBranchDeletable(branchId, apiUrl, spinner); - // Retry deletion after branch is no longer busy + try { await deleteBranchApi(branchId, apiUrl); - spinner?.stop(`Branch deletion requested after wait.`); - } else { + spinner?.stop(`Branch deletion requested.`); + return; + } catch (err) { + if (isBusyError(err)) { + spinner?.message(`Branch is busy (creating/merging). Waiting for it to become deletable...`); + await waitForBranchDeletable(branchId, apiUrl, spinner); + await deleteBranchApi(branchId, apiUrl); + spinner?.stop(`Branch deletion requested after wait.`); + return; + } throw err; } + } catch (err) { + spinner?.stop(`Branch '${name}' deletion failed`, 1); + throw err; } } @@ -101,24 +114,17 @@ export function registerBranchDeleteCommand(branch: Command): void { } } - // Set up spinner for progress indication during delete/retry const spinner = !json ? clack.spinner() : null; spinner?.start(`Deleting branch '${name}'...`); - await deleteBranchWithRetry(target.id, apiUrl, spinner); + await deleteBranchWithRetry(target.id, name, apiUrl, spinner); captureEvent(parentId, 'cli_branch_delete', {}); - // If the directory is currently switched onto the deleted branch, - // flip back to parent so subsequent commands don't operate on a - // dead instance. const currentlyOnDeleted = project.project_id === target.id; if (currentlyOnDeleted) { try { - // silent in JSON mode so we don't emit two JSON documents — the - // single `outputJson({ deleted, ... })` below is authoritative. await runBranchSwitch({ toParent: true, apiUrl, json, silent: json }); } catch (err) { - // Non-fatal: the branch is gone, but we can at least tell the user. outputInfo( `Switched-to-parent failed (${(err as Error).message}). Run \`insforge branch switch --parent\` manually.`, ); diff --git a/src/commands/db/migrations.ts b/src/commands/db/migrations.ts index b3527c18..f9d9279c 100644 --- a/src/commands/db/migrations.ts +++ b/src/commands/db/migrations.ts @@ -1,9 +1,8 @@ import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import type { Command } from 'commander'; -import { isProvisioningError, buildProvisioningErrorMessage, ossFetch } from '../../lib/api/oss.js'; +import { handleBranchProvisioningError, ossFetch } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; -import { getProjectConfig } from '../../lib/config.js'; import { CLIError, getRootOpts, handleError } from '../../lib/errors.js'; import { canonicalMigrationVersion, @@ -135,18 +134,7 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { await reportCliUsage('cli.db.migrations.list', false); await trackCommandUsage('db', 'migrations list', false, {}, err); - const projectConfig = getProjectConfig(); - const isBranch = projectConfig?.branched_from != null; - if (isBranch && isProvisioningError(err)) { - const msg = buildProvisioningErrorMessage(projectConfig?.project_name); - if (json) { - console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); - } else { - console.error(`Error: ${msg}`); - } - process.exit(1); - } - + await handleBranchProvisioningError(err, json); handleError(err, json); } }); @@ -215,19 +203,7 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.fetch', false); await trackCommandUsage('db', 'migrations fetch', false, {}, err); - - const projectConfig = getProjectConfig(); - const isBranch = projectConfig?.branched_from != null; - if (isBranch && isProvisioningError(err)) { - const msg = buildProvisioningErrorMessage(projectConfig?.project_name); - if (json) { - console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); - } else { - console.error(`Error: ${msg}`); - } - process.exit(1); - } - + await handleBranchProvisioningError(err, json); handleError(err, json); } }); @@ -274,19 +250,7 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.new', false); await trackCommandUsage('db', 'migrations new', false, {}, err); - - const projectConfig = getProjectConfig(); - const isBranch = projectConfig?.branched_from != null; - if (isBranch && isProvisioningError(err)) { - const msg = buildProvisioningErrorMessage(projectConfig?.project_name); - if (json) { - console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); - } else { - console.error(`Error: ${msg}`); - } - process.exit(1); - } - + await handleBranchProvisioningError(err, json); handleError(err, json); } }); @@ -467,19 +431,7 @@ export function registerDbMigrationsCommand(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.migrations.up', false); await trackCommandUsage('db', 'migrations up', false, {}, err); - - const projectConfig = getProjectConfig(); - const isBranch = projectConfig?.branched_from != null; - if (isBranch && isProvisioningError(err)) { - const msg = buildProvisioningErrorMessage(projectConfig?.project_name); - if (json) { - console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); - } else { - console.error(`Error: ${msg}`); - } - process.exit(1); - } - + await handleBranchProvisioningError(err, json); handleError(err, json); } }); diff --git a/src/commands/db/query.test.ts b/src/commands/db/query.test.ts index 8930747e..727031dd 100644 --- a/src/commands/db/query.test.ts +++ b/src/commands/db/query.test.ts @@ -2,15 +2,23 @@ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; import { Command } from 'commander'; import { registerDbCommands } from './query.js'; -vi.mock('../../lib/api/oss.js', () => ({ - runRawSql: vi.fn(), - isProvisioningError: vi.fn(), - buildProvisioningErrorMessage: vi.fn((name?: string) => +vi.mock('../../lib/api/oss.js', () => { + const runRawSql = vi.fn(); + const isProvisioningError = vi.fn(); + const buildProvisioningErrorMessage = vi.fn((name?: string) => name - ? `Branch is still provisioning (this can take up to ~12 minutes). Branch: ${name}. Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.` - : 'Branch is still provisioning (this can take up to ~12 minutes). Retry shortly, or create the branch with `--wait-ready` to block until it\'s usable.', - ), -})); + ? `Branch is still provisioning (this can take up to ~15 minutes). Branch: ${name}. Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.` + : 'Branch is still provisioning (this can take up to ~15 minutes). Retry shortly, or create the branch with `--wait-ready` to block until it\'s usable.', + ); + const handleBranchProvisioningError = vi.fn((err: unknown, json: boolean) => { + if (isProvisioningError(err)) { + const msg = buildProvisioningErrorMessage(undefined); + console.error(json ? JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' }) : `Error: ${msg}`); + process.exit(1); + } + }); + return { runRawSql, isProvisioningError, buildProvisioningErrorMessage, handleBranchProvisioningError }; +}); vi.mock('../../lib/credentials.js', () => ({ requireAuth: vi.fn(async () => ({ accessToken: 'tok', userId: 'u' })), @@ -43,44 +51,33 @@ describe('db query', () => { const { runRawSql, isProvisioningError } = await import('../../lib/api/oss.js'); (runRawSql as Mock).mockRejectedValue(new Error('fetch failed')); (isProvisioningError as Mock).mockReturnValue(true); - const { getProjectConfig } = await import('../../lib/config.js'); - (getProjectConfig as Mock).mockReturnValue({ - project_id: 'b1', - project_name: 'feat-x', - org_id: 'o1', - api_key: 'k', - oss_host: 'host', - branched_from: { project_id: 'p1', project_name: 'parent' }, - }); const program = new Command().exitOverride(); program.option('--json').option('--api-url ').option('-y, --yes'); registerDbCommands(program); - const errLogs: string[] = []; - const origErr = console.error; - console.error = (...args: unknown[]) => { - errLogs.push(args.map(String).join(' ')); - }; - let exitCode: number | undefined; + const state = { exitCode: undefined as number | undefined, errLogs: [] as string[] }; const origExit = process.exit; process.exit = ((code?: number) => { - exitCode = code; + state.exitCode = code; throw new Error('__exit__'); }) as typeof process.exit; + const origErr = console.error; + console.error = (...args: unknown[]) => { + state.errLogs.push(args.map(String).join(' ')); + }; try { await program .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) .catch(() => {}); } finally { - console.error = origErr; process.exit = origExit; + console.error = origErr; } - expect(exitCode).toBe(1); - const errText = errLogs.join('\n'); + expect(state.exitCode).toBe(1); + const errText = state.errLogs.join('\n'); expect(errText).toContain('still provisioning'); - expect(errText).toContain('feat-x'); expect(errText).toContain('--wait-ready'); }); @@ -100,29 +97,27 @@ describe('db query', () => { program.option('--json').option('--api-url ').option('-y, --yes'); registerDbCommands(program); - const errLogs: string[] = []; - const origErr = console.error; - console.error = (...args: unknown[]) => { - errLogs.push(args.map(String).join(' ')); - }; - let exitCode: number | undefined; + const state = { exitCode: undefined as number | undefined, errLogs: [] as string[] }; const origExit = process.exit; process.exit = ((code?: number) => { - exitCode = code; + state.exitCode = code; throw new Error('__exit__'); }) as typeof process.exit; + const origErr = console.error; + console.error = (...args: unknown[]) => { + state.errLogs.push(args.map(String).join(' ')); + }; try { await program .parseAsync(['query', 'SELECT 1', '--json'], { from: 'user' }) .catch(() => {}); } finally { - console.error = origErr; process.exit = origExit; + console.error = origErr; } - expect(exitCode).toBe(1); - const errText = errLogs.join('\n'); - // Should contain the raw error, not the provisioning message + expect(state.exitCode).toBe(1); + const errText = state.errLogs.join('\n'); expect(errText).toContain('fetch failed'); expect(errText).not.toContain('still provisioning'); }); diff --git a/src/commands/db/query.ts b/src/commands/db/query.ts index a85d7d6c..b1333271 100644 --- a/src/commands/db/query.ts +++ b/src/commands/db/query.ts @@ -1,11 +1,10 @@ import type { Command } from 'commander'; -import { runRawSql, isProvisioningError, buildProvisioningErrorMessage } from '../../lib/api/oss.js'; +import { runRawSql, handleBranchProvisioningError } from '../../lib/api/oss.js'; import { requireAuth } from '../../lib/credentials.js'; -import { handleError, getRootOpts, CLIError } from '../../lib/errors.js'; +import { handleError, getRootOpts } from '../../lib/errors.js'; import { outputJson, outputTable } from '../../lib/output.js'; import { reportCliUsage } from '../../lib/skills.js'; import { trackCommandUsage } from '../../lib/command-telemetry.js'; -import { getProjectConfig } from '../../lib/config.js'; export function registerDbCommands(dbCmd: Command): void { dbCmd @@ -42,22 +41,7 @@ export function registerDbCommands(dbCmd: Command): void { } catch (err) { await reportCliUsage('cli.db.query', false); await trackCommandUsage('db', 'query', false, {}, err); - - // Check if this is a provisioning error on a branch - const projectConfig = getProjectConfig(); - const isBranch = projectConfig?.branched_from != null; - const branchName = projectConfig?.project_name; - - if (isBranch && isProvisioningError(err)) { - const msg = buildProvisioningErrorMessage(branchName); - if (json) { - console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); - } else { - console.error(`Error: ${msg}`); - } - process.exit(1); - } - + await handleBranchProvisioningError(err, json); handleError(err, json); } }); diff --git a/src/lib/api/oss.ts b/src/lib/api/oss.ts index 4c02d057..b0ba202b 100644 --- a/src/lib/api/oss.ts +++ b/src/lib/api/oss.ts @@ -1,5 +1,5 @@ import { getProjectConfig } from '../config.js'; -import { CLIError, formatFetchError, ProjectNotLinkedError } from '../errors.js'; +import { CLIError, formatFetchError, handleError, ProjectNotLinkedError } from '../errors.js'; import type { ProjectConfig, RotateKeyResponse, @@ -65,6 +65,55 @@ export function buildProvisioningErrorMessage(branchName?: string): string { return `${base}${branchPart} Retry shortly, or create the branch with \`--wait-ready\` to block until it's usable.`; } +/** + * Handle a branching provisioning error by checking if the error is + * provisioning-related, verifying via health endpoint, and exiting with + * a helpful message if so. Non-provisioning errors are passed through. + */ +export async function handleBranchProvisioningError(err: unknown, json: boolean): Promise { + if (!isProvisioningError(err)) return; + + let branchName: string | undefined; + let config: ProjectConfig | undefined; + try { + config = getProjectConfig() ?? undefined; + branchName = config?.project_name; + } catch { + handleError(err, json); + return; + } + if (!config) return; + + // Verify the branch is actually still provisioning — if the health endpoint + // responds healthy, this is a genuine network outage, not provisioning. + if (config.oss_host && config.api_key) { + try { + const healthUrl = `${config.oss_host.replace(/\/+$/, '')}/api/health`; + const res = await fetch(healthUrl, { + method: 'GET', + signal: AbortSignal.timeout(5_000), + headers: { Authorization: `Bearer ${config.api_key}` }, + }); + if (res.ok) { + const data = await res.json().catch(() => ({})); + if (data.status === 'healthy' || data.status === 'ok') { + return; + } + } + } catch { + // Can't reach the health endpoint either — branch is likely provisioning + } + } + + const msg = buildProvisioningErrorMessage(branchName); + if (json) { + console.error(JSON.stringify({ error: msg, code: 'BRANCH_PROVISIONING' })); + } else { + console.error(`Error: ${msg}`); + } + process.exit(1); +} + /** * Unified OSS API fetch. Uses API key as Bearer token for all requests, * which grants superadmin access (SQL execution, bucket management, etc.).