From e0696b5ac89611832144b1f5d79e11fd021b7b2d Mon Sep 17 00:00:00 2001 From: Ander Date: Mon, 3 Aug 2026 19:02:30 +0200 Subject: [PATCH 1/2] feat(cli): consistent --json success envelope + batch --output/--follow polish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive CLI-consistency pass (the safe half of the coherence review; the exit-code contract change is deferred to its own PR): - every success --json path now leads with a flat `ok` field, matching the established shape (fetch/browser/batch already did). Errors already emit `{ok:false, error}` via printError, so agents can branch on `.ok` uniformly. Touched: account/config/policy/usage/mcp/plugin/trace/browser(info)/batch (estimate) + asset explain/validate. `ok` reflects validity where meaningful (estimate, asset validate). Raw-dump paths left as-is by intent (trace export, eval stored results, browser human-readable fallback). - batch `--output`: fail loudly (INVALID_USAGE) on an unknown format instead of silently dropping it — matching fetch's normalizeOutput (no silent drops). - batch `--follow`: clearer name for "poll until done"; `--wait` kept as a back-compat alias (fetch/extract use `--wait ` for a different meaning). - tests: --output loud-fail (no-network) + estimate --json envelope (ok true/false). Co-Authored-By: Claude Opus 4.8 --- src/cli/asset-command.ts | 4 ++-- src/cli/commands/account.ts | 1 + src/cli/commands/batch.ts | 29 ++++++++++++++++++------- src/cli/commands/browser.ts | 2 +- src/cli/commands/config.ts | 2 +- src/cli/commands/mcp.ts | 4 ++-- src/cli/commands/plugin.ts | 4 ++-- src/cli/commands/policy.ts | 2 +- src/cli/commands/trace.ts | 6 +++--- src/cli/commands/usage.ts | 2 +- tests/batch-command.test.ts | 42 +++++++++++++++++++++++++++++++++++++ 11 files changed, 78 insertions(+), 20 deletions(-) diff --git a/src/cli/asset-command.ts b/src/cli/asset-command.ts index c0dccc6..834b0cf 100644 --- a/src/cli/asset-command.ts +++ b/src/cli/asset-command.ts @@ -173,7 +173,7 @@ function explainCmd(type: AssetType, argv: string[], ctx: RunContext): number { if (!name) throw usageErr(type, "explain "); const asset = requireAsset(type, name); if (ctx.json) { - log.out(JSON.stringify({ ...asset, runnable: assetRunnable(asset) }, null, 2)); + log.out(JSON.stringify({ ok: true, ...asset, runnable: assetRunnable(asset) }, null, 2)); return 0; } log.info(c(ANSI.bold, `${asset.name} (${asset.type}, ${asset.status})`)); @@ -257,7 +257,7 @@ function validateSkill(argv: string[], ctx: RunContext): number { if (/apikey=|ZENROWS_API_KEY=[A-Za-z0-9]/.test(body)) errors.push("SKILL.md may contain a secret"); } if (ctx.json) { - log.out(JSON.stringify({ name, valid: errors.length === 0, errors }, null, 2)); + log.out(JSON.stringify({ ok: errors.length === 0, name, valid: errors.length === 0, errors }, null, 2)); } else if (errors.length === 0) { log.success(`Skill "${name}" is valid.`); } else { diff --git a/src/cli/commands/account.ts b/src/cli/commands/account.ts index 4ea1a5d..39f7b85 100644 --- a/src/cli/commands/account.ts +++ b/src/cli/commands/account.ts @@ -69,6 +69,7 @@ export const account: Command = { if (ctx.json) { log.out(JSON.stringify({ + ok: true, hasKey: auth.hasKey, source: auth.source, account: acct ?? null, diff --git a/src/cli/commands/batch.ts b/src/cli/commands/batch.ts index aa695b1..6d55e03 100644 --- a/src/cli/commands/batch.ts +++ b/src/cli/commands/batch.ts @@ -32,8 +32,8 @@ export const batch: Command = { " --js-render job-level: render JavaScript", " --premium-proxy job-level: use residential IPs", " --proxy-country job-level: geo-target (needs --premium-proxy)", - " --output job-level response_type (markdown|plaintext|pdf)", - " --wait poll until the run finishes", + " --output job-level response_type (markdown|plaintext|pdf|html)", + " --follow poll until the run finishes (alias: --wait)", " --no-signup do not auto-create a Free plan account if no key exists", " status show run status + stats", " results [--status s] list results (successful|failed|all); paginated", @@ -84,7 +84,7 @@ function estimateCmd(rest: string[], ctx: RunContext): number { const v = validateJsonl(file); const est = estimateCredits(v.jobs); if (ctx.json) { - log.out(JSON.stringify({ ...v, estimatedCredits: est.credits }, null, 2)); + log.out(JSON.stringify({ ok: v.errors.length === 0, ...v, estimatedCredits: est.credits }, null, 2)); } else { log.info(c(ANSI.bold, `Job spec: ${file}`)); log.info(`valid jobs: ${v.validJobs}/${v.totalLines}`); @@ -105,10 +105,14 @@ async function createCmd(rest: string[], ctx: RunContext): Promise { "premium-proxy": { type: "boolean" }, "proxy-country": { type: "string" }, output: { type: "string" }, - wait: { type: "boolean" }, + follow: { type: "boolean" }, + wait: { type: "boolean" }, // back-compat alias for --follow "no-signup": { type: "boolean" }, json: { type: "boolean" }, }); + // `--follow` is the clear name (poll until the run finishes); `--wait` is kept + // as an alias because fetch/extract use `--wait ` for a different meaning. + const follow = values.follow === true || values.wait === true; const json = ctx.json || values.json === true; const file = positionals[0]; if (!file) throw needFile(); @@ -163,7 +167,7 @@ async function createCmd(rest: string[], ctx: RunContext): Promise { log.step(`Submitting batch job (${body.tasks.length} tasks, ~${est.credits} credits)…`); try { const job = await createJob(body, { apiKey }); - const finished = values.wait === true ? await waitForJob(job.job_id, { apiKey }) : job; + const finished = follow ? await waitForJob(job.job_id, { apiKey }) : job; const runDir = writeRun({ runId, command: "zenrows batch create", @@ -342,8 +346,19 @@ function normalizeOutput(v?: string): string | undefined { pdf: "pdf", html: "", // raw HTML is the default; no response_type }; - const mapped = map[v.toLowerCase()]; - return mapped ? mapped : undefined; + const key = v.toLowerCase(); + if (!(key in map)) { + // Fail loudly rather than silently dropping an unrecognized format (house rule). + throw new ToolkitError({ + code: "INVALID_USAGE", + message: `Unknown --output format: ${v}.`, + likely_cause: "Only markdown | plaintext | pdf | html are supported for batch --output.", + next_action: "Use --output md|markdown | text|plaintext | pdf | html (html = raw HTML, the default).", + suggested_commands: ["zenrows batch create jobs.jsonl --output markdown"], + }); + } + const mapped = map[key]; + return mapped ? mapped : undefined; // html → undefined (no response_type) } function requireId(id: string | undefined): string { diff --git a/src/cli/commands/browser.ts b/src/cli/commands/browser.ts index 66e7f59..b6d6d8a 100644 --- a/src/cli/commands/browser.ts +++ b/src/cli/commands/browser.ts @@ -243,7 +243,7 @@ function infoCmd(ctx: RunContext): number { const policy = loadPolicy(); assertBrowserAllowed(policy); // throws POLICY_BROWSER_DISABLED (→ exit 2) only if opted out if (ctx.json) { - log.out(JSON.stringify({ capability: "browser", escalationOnly: true, backend: "REST session API (mcp.zenrows.com/browser/sessions)" }, null, 2)); + log.out(JSON.stringify({ ok: true, capability: "browser", escalationOnly: true, backend: "REST session API (mcp.zenrows.com/browser/sessions)" }, null, 2)); return 0; } log.info(c(ANSI.bold, "Browser sessions (escalation-only)")); diff --git a/src/cli/commands/config.ts b/src/cli/commands/config.ts index 744650b..e9db16d 100644 --- a/src/cli/commands/config.ts +++ b/src/cli/commands/config.ts @@ -74,7 +74,7 @@ export const config: Command = { }; function out(cfg: ToolkitConfig, _ctx: RunContext): number { - log.out(JSON.stringify(cfg, null, 2)); + log.out(JSON.stringify({ ok: true, ...cfg }, null, 2)); return 0; } function unknownKey(key?: string): ToolkitError { diff --git a/src/cli/commands/mcp.ts b/src/cli/commands/mcp.ts index d96f124..a11e309 100644 --- a/src/cli/commands/mcp.ts +++ b/src/cli/commands/mcp.ts @@ -44,7 +44,7 @@ export const mcp: Command = { } const { client, snippet } = buildMcpConfig(clientId, transport); if (ctx.json) { - log.out(JSON.stringify({ client: client.id, transport, configFile: client.configFile, autoConfigurable: client.autoConfigurable, snippet }, null, 2)); + log.out(JSON.stringify({ ok: true, client: client.id, transport, configFile: client.configFile, autoConfigurable: client.autoConfigurable, snippet }, null, 2)); return 0; } log.info(c(ANSI.bold, `MCP config for ${client.label} (${transport})`)); @@ -76,7 +76,7 @@ export const mcp: Command = { function statusCmd(ctx: RunContext): number { const cap = loadCapabilities().mcp; if (ctx.json) { - log.out(JSON.stringify({ capability: cap, remote: REMOTE_URL, local: "npx -y @zenrows/mcp", clients: Object.values(MCP_CLIENTS) }, null, 2)); + log.out(JSON.stringify({ ok: true, capability: cap, remote: REMOTE_URL, local: "npx -y @zenrows/mcp", clients: Object.values(MCP_CLIENTS) }, null, 2)); return 0; } log.info(c(ANSI.bold, "Zenrows MCP")); diff --git a/src/cli/commands/plugin.ts b/src/cli/commands/plugin.ts index fedd2af..fb1ed18 100644 --- a/src/cli/commands/plugin.ts +++ b/src/cli/commands/plugin.ts @@ -21,7 +21,7 @@ export const plugin: Command = { const [sub, name] = argv; if (!sub || sub === "list") { if (ctx.json) { - log.out(JSON.stringify({ clients: Object.values(MCP_CLIENTS) }, null, 2)); + log.out(JSON.stringify({ ok: true, clients: Object.values(MCP_CLIENTS) }, null, 2)); return 0; } log.info(c(ANSI.bold, "Installable agent plugins:")); @@ -33,7 +33,7 @@ export const plugin: Command = { if (sub === "status") { const skills = listInstalled("skill"); if (ctx.json) { - log.out(JSON.stringify({ installedSkills: skills }, null, 2)); + log.out(JSON.stringify({ ok: true, installedSkills: skills }, null, 2)); return 0; } log.info(`Installed skills: ${skills.join(", ") || "none"}`); diff --git a/src/cli/commands/policy.ts b/src/cli/commands/policy.ts index df49fe4..bd94cb4 100644 --- a/src/cli/commands/policy.ts +++ b/src/cli/commands/policy.ts @@ -23,7 +23,7 @@ export const policy: Command = { const pol = loadPolicy(); if (!sub || sub === "show") { if (ctx.json) { - log.out(JSON.stringify(pol, null, 2)); + log.out(JSON.stringify({ ok: true, ...pol }, null, 2)); return 0; } for (const [k, v] of Object.entries(pol)) kv(k, Array.isArray(v) ? `[${v.join(", ")}]` : String(v), 24); diff --git a/src/cli/commands/trace.ts b/src/cli/commands/trace.ts index 5c11f3b..95d180c 100644 --- a/src/cli/commands/trace.ts +++ b/src/cli/commands/trace.ts @@ -41,7 +41,7 @@ export const trace: Command = { const rec = loadRun(runId); if (sub === "inspect") { - log.out(JSON.stringify(rec, null, 2)); + log.out(JSON.stringify({ ok: true, ...rec }, null, 2)); return 0; } if (sub === "export") { @@ -52,7 +52,7 @@ export const trace: Command = { if (sub === "replay") { const cmd = rebuildCommand(rec); if (ctx.json) { - log.out(JSON.stringify({ runId, replay: cmd }, null, 2)); + log.out(JSON.stringify({ ok: true, runId, replay: cmd }, null, 2)); } else { log.info("Replay this run with:"); log.out(cmd); @@ -102,7 +102,7 @@ function explain(rec: RunRecord, ctx: RunContext): number { : [rebuildCommand(rec) + " --manual --js-render --premium-proxy"]; if (ctx.json) { - log.out(JSON.stringify({ runId: rec.runId, what_happened: what, likely_failure_reason: reason, evidence: rec, recommended_next_action: nextAction, suggested_commands: suggested }, null, 2)); + log.out(JSON.stringify({ ok: true, runId: rec.runId, what_happened: what, likely_failure_reason: reason, evidence: rec, recommended_next_action: nextAction, suggested_commands: suggested }, null, 2)); return 0; } log.info(c(ANSI.bold, `Trace explain · ${rec.runId}`)); diff --git a/src/cli/commands/usage.ts b/src/cli/commands/usage.ts index 192c4b9..d3ecadb 100644 --- a/src/cli/commands/usage.ts +++ b/src/cli/commands/usage.ts @@ -35,7 +35,7 @@ export const usage: Command = { const u = await fetchUsage(config.apiBase, apiKey); if (ctx.json) { - log.out(JSON.stringify(u, null, 2)); + log.out(JSON.stringify({ ok: true, ...u }, null, 2)); return 0; } diff --git a/tests/batch-command.test.ts b/tests/batch-command.test.ts index 82f8295..6ab938b 100644 --- a/tests/batch-command.test.ts +++ b/tests/batch-command.test.ts @@ -90,3 +90,45 @@ test("batch create submits when the run is within policy caps", async () => { assert.equal(didFetch(), true, "an in-policy run must reach the Batch API"); }); }); + +test("batch create rejects an unknown --output format before any network call", async () => { + await withBatchWorkspace({}, async (didFetch) => { + const file = writeSpec(["https://ok.example/a"]); + const code = await batch.run(["create", file, "--output", "bogus"], ctx); + assert.equal(code, 1); + assert.equal(didFetch(), false, "an unknown --output must fail loudly, not silently drop"); + }); +}); + +/** Capture stdout (log.out) for the duration of `fn`. */ +async function captureOut(fn: () => unknown): Promise { + const orig = process.stdout.write.bind(process.stdout); + let buf = ""; + process.stdout.write = ((s: string | Uint8Array) => { + buf += typeof s === "string" ? s : Buffer.from(s).toString(); + return true; + }) as typeof process.stdout.write; + try { + await fn(); + } finally { + process.stdout.write = orig; + } + return buf; +} + +test("batch estimate --json emits an {ok,...} envelope (ok reflects spec validity)", async () => { + await withBatchWorkspace({}, async () => { + const good = writeSpec(["https://ok.example/a"]); + const okOut = await captureOut(() => batch.run(["estimate", good], ctx)); + const okJson = JSON.parse(okOut) as { ok: boolean; estimatedCredits: number }; + assert.equal(okJson.ok, true); + assert.equal(typeof okJson.estimatedCredits, "number"); + + // A spec with a bad line → ok:false (and the command's exit code is 1). + const bad = join(process.cwd(), "bad.jsonl"); + writeFileSync(bad, '{"url":"https://ok.example/a"}\nnot-json\n'); + const badOut = await captureOut(() => batch.run(["estimate", bad], ctx)); + const badJson = JSON.parse(badOut) as { ok: boolean }; + assert.equal(badJson.ok, false); + }); +}); From 705dcbaf107797686c4be7e6194dca5fee491899 Mon Sep 17 00:00:00 2001 From: Ander Date: Mon, 3 Aug 2026 19:15:04 +0200 Subject: [PATCH 2/2] =?UTF-8?q?refactor(cli):=20drop=20the=20dead=20CAPABI?= =?UTF-8?q?LITY=5FUNAVAILABLE=E2=86=92exit-2=20branch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The top-level catch mapped CAPABILITY_UNAVAILABLE to exit 2, but the capability-gated commands (fetch/extract/batch/browser) all catch internally and return 1, so that branch never fired — the CLI already exits 1 on every error in practice, and no docs promised otherwise. Make the contract honest: any error exits 1. Machine consumers read the precise `error.code` from --json output to tell a denial from a failure, which is finer- grained than a 1-vs-2 exit code would ever be. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 --- src/cli/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index d333b4b..d574602 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -94,7 +94,11 @@ export async function main(rawArgv: string[]): Promise { return await cmd.run(argv, ctx); } catch (err) { printError(err, json); - return err instanceof ToolkitError && err.code === "CAPABILITY_UNAVAILABLE" ? 2 : 1; + // Any error exits 1. We do not overload the exit code to signal error kind: + // the capability-gated commands catch internally and return 1 already, so a + // CAPABILITY_UNAVAILABLE→2 mapping here never fired. Machine consumers read + // the precise `error.code` from --json output instead. + return 1; } }