Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 189 additions & 0 deletions src/commands/branch/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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', () => ({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 <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 <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 <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 <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');
});
});
93 changes: 91 additions & 2 deletions src/commands/branch/create.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -11,14 +11,48 @@ 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<typeof clack.spinner> | null): Promise<void> {
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;
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
}
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
.command('create <name>')
.description('Create a branch from the currently linked project')
.option('--mode <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)
Comment thread
greptile-apps[bot] marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Every branch create now performs the data-plane health wait by default, potentially blocking for 15 minutes and changing the existing command behavior; the declared positive flag also provides no way to disable that default. The option should default to false/undefined so the health wait only runs when --wait-ready is supplied.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/branch/create.ts, line 54:

<comment>Every `branch create` now performs the data-plane health wait by default, potentially blocking for 15 minutes and changing the existing command behavior; the declared positive flag also provides no way to disable that default. The option should default to false/undefined so the health wait only runs when `--wait-ready` is supplied.</comment>

<file context>
@@ -11,14 +11,48 @@ import type { Branch, BranchMode } from '../../types.js';
     .option('--mode <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);
</file context>
Suggested change
.option('--wait-ready', 'Wait for the branch data plane to be fully ready (up to 15 min)', true)
.option('--wait-ready', 'Wait for the branch data plane to be fully ready (up to 15 min)')

.action(async (name: string, opts: { mode: string; switch: boolean; waitReady: boolean }, cmd) => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const { json, apiUrl } = getRootOpts(cmd);
try {
await requireAuth(apiUrl);
Expand Down Expand Up @@ -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);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
spinner?.message('Data plane is ready.');
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

if (provisioned && opts.switch) {
spinner?.message('Branch ready. Switching context...');
// silent: true always — the spinner owns user-facing output, and
Expand All @@ -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');
Comment on lines +179 to +200

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

BRANCH_DATA_PLANE_TIMEOUT in isNetworkError is unreachable here, and the timeout produces a misleading message.

waitForDataPlaneReady only runs after provisioned becomes true (Line 100), so its BRANCH_DATA_PLANE_TIMEOUT error can only surface with provisioned === true. But reconciliation is gated on !provisioned (Line 142), so this err.code check never contributes. Worse, that timeout then falls through to the if (provisioned) branch (Line 167), telling the user "switching context failed — run insforge branch switch to retry", which misdescribes a data-plane readiness timeout. Consider handling the timeout case explicitly so the surfaced guidance matches the actual failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/branch/create.ts` around lines 119 - 140, Update the branch
creation error handling around isNetworkError and the provisioned branch to
handle BRANCH_DATA_PLANE_TIMEOUT explicitly. Since this error occurs after
provisioning, remove it from the reconciliation-only network-error check and
surface dedicated data-plane readiness timeout guidance instead of the generic
context-switch message.


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);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
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;
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}
} 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`,
Expand Down
Loading