diff --git a/packages/cli/__tests__/contract.test.ts b/packages/cli/__tests__/contract.test.ts index bf4f718..78365c4 100644 --- a/packages/cli/__tests__/contract.test.ts +++ b/packages/cli/__tests__/contract.test.ts @@ -235,17 +235,43 @@ describe('--json on every data-emitting command', () => { const r = run(['list', '--json', '--server', 'http://127.0.0.1:1']) expect(r.status).toBe(6) expect(r.stdout).not.toMatch(ANSI) - const doc = JSON.parse(r.stdout) as { ok: boolean; error: string } + const doc = JSON.parse(r.stdout) as { ok: boolean; error: string; url: string } expect(doc.ok).toBe(false) expect(doc.error).toBe('network') + // Error must include the URL it tried, so agents see what failed. + expect(doc.url).toBe('http://127.0.0.1:1/api/flows') }) - it('get with no server → exit 6, JSON error on stdout', () => { + it('get with no server → exit 6, JSON error on stdout, error includes URL', () => { const r = run(['get', 'abc', '--json', '--server', 'http://127.0.0.1:1']) expect(r.status).toBe(6) - const doc = JSON.parse(r.stdout) as { ok: boolean; error: string } + const doc = JSON.parse(r.stdout) as { ok: boolean; error: string; url: string } expect(doc.ok).toBe(false) expect(doc.error).toBe('network') + expect(doc.url).toBe('http://127.0.0.1:1/api/flows/abc') + }) + + it('every network-error JSON includes the URL that failed', () => { + // Lock the contract across all server-touching commands. push and patch + // need valid bodies first or they fail validation before reaching the + // network — for those we use real fixtures. + const cases = [ + { args: ['list', '--json', '--server', 'http://127.0.0.1:1'] }, + { args: ['get', 'x', '--json', '--server', 'http://127.0.0.1:1'] }, + { args: ['remove', 'x', '--json', '--server', 'http://127.0.0.1:1'] }, + { + args: ['push', EXAMPLE_GOOD, '--json', '--server', 'http://127.0.0.1:1'], + }, + ] + for (const c of cases) { + const r = run(c.args) + expect(r.status, `${c.args[0]} should exit 6`).toBe(6) + const doc = JSON.parse(r.stdout) as { error: string; url?: string } + expect(doc.error, `${c.args[0]} should report network error`).toBe('network') + expect(doc.url, `${c.args[0]} should include url in network error`).toMatch( + /^http:\/\/127\.0\.0\.1:1\// + ) + } }) }) diff --git a/packages/cli/src/get.ts b/packages/cli/src/get.ts index 2cb1f4e..481822e 100644 --- a/packages/cli/src/get.ts +++ b/packages/cli/src/get.ts @@ -20,8 +20,9 @@ export function registerGet(program: Command, defaultServer: string): void { .option('-s, --server ', 'Server URL', defaultServer) .option('--json', 'Emit JSON on stdout (machine-readable)') .action(async (flowId: string, opts) => { + const url = `${opts.server}/api/flows/${flowId}` try { - const res = await fetch(`${opts.server}/api/flows/${flowId}`) + const res = await fetch(url) if (res.status === 404) { if (opts.json) emitJson({ ok: false, error: 'not-found', id: flowId }) @@ -31,27 +32,37 @@ export function registerGet(program: Command, defaultServer: string): void { if (!res.ok) { const body = await res.text() - if (opts.json) emitJson({ ok: false, error: 'server', status: res.status, body }) - else errStderr(red(`✗ Server error (${res.status}): ${body}`)) + if (opts.json) emitJson({ ok: false, error: 'server', status: res.status, body, url }) + else errStderr(red(`✗ Server error (${res.status}) at ${url}: ${body}`)) process.exit(ExitCode.NETWORK) } - const flow = (await res.json()) as Record + const flow = (await res.json()) as Record & { + flow?: { nodes?: unknown[] } + } if (opts.json) { - emitJson(flow) + // Add a flat `nodeCount` so the JSON shape lines up with `push --json`. + // Saves agents from having to reach into `.flow.nodes.length`. + const nodeCount = Array.isArray(flow.flow?.nodes) ? flow.flow!.nodes!.length : 0 + emitJson({ ...flow, nodeCount }) return } - // Human mode: print a summary on stderr, the full YAML/JSON on stdout - // so it pipes cleanly to a file. + // Human mode: print a summary on stderr, the full JSON on stdout so + // it pipes cleanly to a file. + const meta = (flow as { meta?: { title?: unknown } }).meta + const title = typeof meta?.title === 'string' ? meta.title : undefined logStderr(dim(`# Flow ${flowId}`)) - if (typeof flow.title === 'string') logStderr(`${bold('Title:')} ${flow.title}`) + if (title) logStderr(`${bold('Title:')} ${title}`) if (typeof flow.version === 'number') logStderr(`${bold('Version:')} v${flow.version}`) process.stdout.write(JSON.stringify(flow, null, 2) + '\n') } catch (err) { - if (opts.json) emitJson({ ok: false, error: 'network', message: errorMessage(err) }) - else errStderr(red(`✗ Connection failed: ${errorMessage(err)}`)) + if (opts.json) { + emitJson({ ok: false, error: 'network', message: errorMessage(err), url }) + } else { + errStderr(red(`✗ Connection failed (${url}): ${errorMessage(err)}`)) + } process.exit(ExitCode.NETWORK) } }) diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 1e89b1d..f696269 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -53,22 +53,70 @@ const program = new Command() program.name('openhop').description('OpenHop — Data Flow Visualization CLI').version('0.1.0') // --- serve --- +// +// Starts both the API server (Fastify on :8787) and the web UI dev server +// (Vite on :8788). The URL printed by `push` points at the web UI port, so +// without the web running the user lands on a 404 — that's what a fresh +// agent's cold-start test surfaced. Use --no-web to opt out (CI, headless). +// +// Note: this command only works inside a from-source checkout of the +// monorepo, because the server and web entries are looked up relative to +// the CLI bundle's dirname. The published `npm i -g openhop` package +// doesn't ship the server/web sources — for production deployments use +// docker-compose or clone the repo. Tracked for v0.2 with a separate +// `@openhop/server` package. program .command('serve') - .description('Start the OpenHop server') - .option('-p, --port ', 'Port to listen on', '8787') + .description('Start the OpenHop API server (:8787) and web UI (:8788)') + .option('-p, --port ', 'API port', '8787') + .option('--no-web', 'Start API only, skip the web UI dev server') .action((opts) => { - const serverEntry = resolve(import.meta.dirname, '../../server/src/index.ts') - logStderr(dim(`Starting OpenHop server on port ${opts.port}...`)) - const child = spawn('npx', ['tsx', serverEntry], { - stdio: 'inherit', + const cliDir = resolve(import.meta.dirname, '..', '..') + const serverEntry = resolve(cliDir, 'server', 'src', 'index.ts') + const webDir = resolve(cliDir, 'web') + + logStderr(dim(`Starting OpenHop API on port ${opts.port}...`)) + // Pipe child stdout/stderr to OUR stderr — the CLI contract says stdout + // is for data only. Without this, Fastify's pino logs and Vite's dev + // banner would pollute parent stdout and break agents that pipe + // `serve` output through jq. + const api = spawn('npx', ['tsx', serverEntry], { + stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, PORT: opts.port }, }) - child.on('error', (err) => { - errStderr(red(`Failed to start server: ${errorMessage(err)}`)) + api.stdout?.pipe(process.stderr) + api.stderr?.pipe(process.stderr) + + let web: ReturnType | null = null + if (opts.web !== false) { + logStderr(dim('Starting OpenHop web UI on port 8788...')) + web = spawn('npm', ['run', 'dev'], { + cwd: webDir, + stdio: ['ignore', 'pipe', 'pipe'], + env: { ...process.env }, + }) + web.stdout?.pipe(process.stderr) + web.stderr?.pipe(process.stderr) + web.on('error', (err) => { + errStderr(red(`Failed to start web UI: ${errorMessage(err)}`)) + // Web failure is non-fatal — API can still run for headless agents. + }) + } + + const shutdown = () => { + api.kill('SIGTERM') + web?.kill('SIGTERM') + } + process.on('SIGINT', shutdown) + process.on('SIGTERM', shutdown) + + api.on('error', (err) => { + errStderr(red(`Failed to start API: ${errorMessage(err)}`)) + web?.kill('SIGTERM') process.exit(ExitCode.GENERIC) }) - child.on('exit', (code) => { + api.on('exit', (code) => { + web?.kill('SIGTERM') process.exit(code ?? ExitCode.SUCCESS) }) }) @@ -105,8 +153,9 @@ program process.exit(ExitCode.VALIDATION) } + const url = `${opts.server}/api/flows` try { - const res = await fetch(`${opts.server}/api/flows`, { + const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'text/yaml' }, body: yamlContent, @@ -115,33 +164,39 @@ program if (!res.ok) { const body = await res.text() if (opts.json) { - emitJson({ ok: false, error: 'server', status: res.status, body }) + emitJson({ ok: false, error: 'server', status: res.status, body, url }) } else { - errStderr(red(`✗ Server error (${res.status}): ${body}`)) + errStderr(red(`✗ Server error (${res.status}) at ${url}: ${body}`)) } process.exit(mapServerStatus(res.status)) } const data = (await res.json()) as { id: string; title: string; version: number } const webUrl = opts.server.replace(/:\d+$/, ':8788') - const url = `${webUrl}/flow/${data.id}` + const flowUrl = `${webUrl}/flow/${data.id}` // Spec asks for nodeCount in the JSON output. We have it locally from // the validated flow, no need to round-trip through the server. const nodeCount = result.data?.flow?.nodes?.length ?? 0 if (opts.json) { - emitJson({ id: data.id, title: data.title, version: data.version, url, nodeCount }) + emitJson({ + id: data.id, + title: data.title, + version: data.version, + url: flowUrl, + nodeCount, + }) } else { logStderr(green('✓ Flow created')) logStderr(` ${bold('ID:')} ${data.id}`) logStderr(` ${bold('Title:')} ${data.title}`) - logStderr(` ${bold('URL:')} ${cyan(url)}`) + logStderr(` ${bold('URL:')} ${cyan(flowUrl)}`) } } catch (err) { if (opts.json) { - emitJson({ ok: false, error: 'network', message: errorMessage(err) }) + emitJson({ ok: false, error: 'network', message: errorMessage(err), url }) } else { - errStderr(red(`✗ Connection failed: ${errorMessage(err)}`)) + errStderr(red(`✗ Connection failed (${url}): ${errorMessage(err)}`)) } process.exit(ExitCode.NETWORK) } @@ -154,11 +209,12 @@ program .option('-s, --server ', 'Server URL', DEFAULT_SERVER) .option('--json', 'Emit JSON on stdout (machine-readable)') .action(async (opts) => { + const url = `${opts.server}/api/flows` try { - const res = await fetch(`${opts.server}/api/flows`) + const res = await fetch(url) if (!res.ok) { - if (opts.json) emitJson({ ok: false, error: 'server', status: res.status }) - else errStderr(red(`✗ Server error (${res.status})`)) + if (opts.json) emitJson({ ok: false, error: 'server', status: res.status, url }) + else errStderr(red(`✗ Server error (${res.status}) at ${url}`)) process.exit(ExitCode.NETWORK) } @@ -204,8 +260,11 @@ program process.stdout.write(row + '\n') } } catch (err) { - if (opts.json) emitJson({ ok: false, error: 'network', message: errorMessage(err) }) - else errStderr(red(`✗ Connection failed: ${errorMessage(err)}`)) + if (opts.json) { + emitJson({ ok: false, error: 'network', message: errorMessage(err), url }) + } else { + errStderr(red(`✗ Connection failed (${url}): ${errorMessage(err)}`)) + } process.exit(ExitCode.NETWORK) } }) @@ -249,8 +308,9 @@ program process.exit(ExitCode.VALIDATION) } + const url = `${opts.server}/api/flows/${flowId}` try { - const res = await fetch(`${opts.server}/api/flows/${flowId}`, { + const res = await fetch(url, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(operations), @@ -259,9 +319,9 @@ program if (!res.ok) { const body = await res.text() if (opts.json) { - emitJson({ ok: false, error: 'server', status: res.status, body }) + emitJson({ ok: false, error: 'server', status: res.status, body, url }) } else { - errStderr(red(`✗ Server error (${res.status}): ${body}`)) + errStderr(red(`✗ Server error (${res.status}) at ${url}: ${body}`)) } process.exit(mapServerStatus(res.status)) } @@ -276,8 +336,11 @@ program logStderr(` ${bold('Version:')} v${data.version}`) } } catch (err) { - if (opts.json) emitJson({ ok: false, error: 'network', message: errorMessage(err) }) - else errStderr(red(`✗ Connection failed: ${errorMessage(err)}`)) + if (opts.json) { + emitJson({ ok: false, error: 'network', message: errorMessage(err), url }) + } else { + errStderr(red(`✗ Connection failed (${url}): ${errorMessage(err)}`)) + } process.exit(ExitCode.NETWORK) } }) @@ -289,8 +352,9 @@ program .option('-s, --server ', 'Server URL', DEFAULT_SERVER) .option('--json', 'Emit JSON on stdout (machine-readable)') .action(async (flowId: string, opts) => { + const url = `${opts.server}/api/flows/${flowId}` try { - const res = await fetch(`${opts.server}/api/flows/${flowId}`, { method: 'DELETE' }) + const res = await fetch(url, { method: 'DELETE' }) if (res.status === 204) { if (opts.json) emitJson({ id: flowId, deleted: true }) @@ -304,13 +368,16 @@ program process.exit(ExitCode.NOT_FOUND) } else { const body = await res.text() - if (opts.json) emitJson({ ok: false, error: 'server', status: res.status, body }) - else errStderr(red(`✗ Server error (${res.status}): ${body}`)) + if (opts.json) emitJson({ ok: false, error: 'server', status: res.status, body, url }) + else errStderr(red(`✗ Server error (${res.status}) at ${url}: ${body}`)) process.exit(ExitCode.NETWORK) } } catch (err) { - if (opts.json) emitJson({ ok: false, error: 'network', message: errorMessage(err) }) - else errStderr(red(`✗ Connection failed: ${errorMessage(err)}`)) + if (opts.json) { + emitJson({ ok: false, error: 'network', message: errorMessage(err), url }) + } else { + errStderr(red(`✗ Connection failed (${url}): ${errorMessage(err)}`)) + } process.exit(ExitCode.NETWORK) } })