Skip to content

fix(cli): cold-start agent findings — serve+web, get nodeCount, error URLs #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
naorsabag merged 3 commits into from
Apr 27, 2026
Merged

fix(cli): cold-start agent findings — serve+web, get nodeCount, error URLs #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2cbe458
fix(cli): cold-start agent findings — serve+web, get nodeCount, error…
naorsabag Apr 27, 2026
4cfc3bb
fix(cli): remove duplicated url declaration in get.ts (lint)
naorsabag Apr 27, 2026
0df5e87
fix(cli): serve pipes child logs to stderr, not inherited stdout
naorsabag Apr 27, 2026
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
32 changes: 29 additions & 3 deletions packages/cli/__tests__/contract.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\//
)
}
})
})

Expand Down
31 changes: 21 additions & 10 deletions packages/cli/src/get.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ export function registerGet(program: Command, defaultServer: string): void {
.option('-s, --server <url>', '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 })
Expand All @@ -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<string, unknown>
const flow = (await res.json()) as Record<string, unknown> & {
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)
}
})
Expand Down
131 changes: 99 additions & 32 deletions packages/cli/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>', 'Port to listen on', '8787')
.description('Start the OpenHop API server (:8787) and web UI (:8788)')
.option('-p, --port <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<typeof spawn> | 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 },
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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)
})
})
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand All @@ -154,11 +209,12 @@ program
.option('-s, --server <url>', '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)
}

Expand Down Expand Up @@ -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)
}
})
Expand Down Expand Up @@ -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),
Expand All @@ -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))
}
Expand All @@ -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)
}
})
Expand All @@ -289,8 +352,9 @@ program
.option('-s, --server <url>', '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 })
Expand All @@ -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)
}
})
Expand Down
Loading