diff --git a/package-lock.json b/package-lock.json index 46d1601..5792cf3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@withone/cli", - "version": "1.47.6", + "version": "1.47.13", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@withone/cli", - "version": "1.47.6", + "version": "1.47.13", "dependencies": { "@clack/prompts": "^0.9.1", "commander": "^13.1.0", diff --git a/package.json b/package.json index 09c0295..82cc2bd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@withone/cli", - "version": "1.47.6", + "version": "1.47.13", "description": "CLI for managing One", "type": "module", "files": [ diff --git a/skills/one/references/flows.md b/skills/one/references/flows.md index f61eab4..f8dbf2a 100644 --- a/skills/one/references/flows.md +++ b/skills/one/references/flows.md @@ -456,10 +456,12 @@ e.g. if sub-flow `enrich-customer` has a step `load` that returns `{ TEAM: "acme { "id": "analyze", "type": "bash", - "bash": { "command": "cat /tmp/data.json | claude --print 'Analyze this' --output-format json", "timeout": 180000, "parseJson": true } + "bash": { "command": "cat /tmp/data.json | claude --print 'Analyze this' --output-format json", "timeout": 180000, "parseEnvelope": true } } ``` +**Parsing output.** `parseJson: true` parses stdout as JSON (and strips outer code fences). For **`claude --print --output-format json`** steps use **`parseEnvelope: true`** instead — `claude` wraps the model's answer in a CLI envelope `{ "type": "result", "result": "```json\n{...}\n```" }`, and `parseEnvelope` unwraps `.result`, strips the inner code fences, drops any preamble/trailer text, and parses the inner JSON as `$.steps..output`. If the unwrapped payload isn't valid JSON the step fails (no silently-broken data). No more hand-rolled `unwrap()` helpers in code steps. + **Safe interpolation.** Plain `{{$.input.x}}` does string substitution and is **unsafe** for bash — values containing quotes, `$`, backticks, `&`, etc. will break the command (or worse). Use the `q` helper to POSIX-shell-quote the value: ```json @@ -627,8 +629,8 @@ Each substep inside `parallel.steps` must have the full step schema: `id`, `name When raw data needs analysis, use this pattern: 1. `file-write` — save data to temp file (API responses are too large to inline) -2. `bash` — call `claude --print` to analyze (set timeout to 180000+, use `--output-format json`) -3. `code` — parse and structure the AI output for downstream steps +2. `bash` — call `claude --print` to analyze (set timeout to 180000+, use `--output-format json` and `"parseEnvelope": true` so `$.steps..output` is the clean parsed JSON) +3. `code` — structure the AI output for downstream steps (no manual envelope unwrap needed — `parseEnvelope` already did it) ## CLI Commands diff --git a/src/lib/flow-bash-envelope.test.ts b/src/lib/flow-bash-envelope.test.ts new file mode 100644 index 0000000..5da18de --- /dev/null +++ b/src/lib/flow-bash-envelope.test.ts @@ -0,0 +1,81 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { unwrapClaudeEnvelope, executeFlow } from './flow-engine.js'; +import type { OneApi } from './api.js'; +import type { Flow, FlowContext, FlowEvent, FlowStep } from './flow-types.js'; + +// #90/#68: `parseEnvelope` on bash steps unwraps the `claude --print +// --output-format json` envelope — envelope detection + fence strip + preamble +// removal — so flows stop re-implementing it. Bad JSON fails the step. + +const env = (result: string) => JSON.stringify({ type: 'result', result }); + +describe('unwrapClaudeEnvelope (#90)', () => { + it('unwraps a fenced-JSON envelope', () => { + assert.deepEqual(unwrapClaudeEnvelope(env('```json\n{"a":1,"b":2}\n```'), 's'), { a: 1, b: 2 }); + }); + + it('unwraps an envelope whose result is bare JSON (no fences)', () => { + assert.deepEqual(unwrapClaudeEnvelope(env('{"a":1}'), 's'), { a: 1 }); + }); + + it('strips conversational preamble before the JSON', () => { + assert.deepEqual(unwrapClaudeEnvelope(env('Here is the analysis:\n{"score":9}'), 's'), { score: 9 }); + }); + + it('strips trailing text after the JSON', () => { + assert.deepEqual(unwrapClaudeEnvelope(env('{"done":true}\n\nLet me know if you need more!'), 's'), { done: true }); + }); + + it('handles an array payload', () => { + assert.deepEqual(unwrapClaudeEnvelope(env('```json\n[1,2,3]\n```'), 's'), [1, 2, 3]); + }); + + it('accepts a bare result string as stdout (not wrapped in an envelope)', () => { + assert.deepEqual(unwrapClaudeEnvelope('```json\n{"x":true}\n```', 's'), { x: true }); + }); + + it('passes through top-level JSON that is already unwrapped (not the envelope)', () => { + assert.deepEqual(unwrapClaudeEnvelope('{"already":"clean"}', 's'), { already: 'clean' }); + }); + + it('throws (with a snippet) when the unwrapped payload is not valid JSON', () => { + assert.throws( + () => unwrapClaudeEnvelope(env('I could not complete that request.'), 'analyze'), + /parseEnvelope: claude output was not valid JSON/, + ); + }); +}); + +// ── integration through executeFlow + a real bash step ── + +const fakeApi = {} as unknown as OneApi; + +async function runBash(command: string, parseEnvelope: boolean): Promise { + const flow = { + key: 'k', name: 'n', version: '1', inputs: {}, + steps: [{ id: 'llm', name: 'LLM', type: 'bash', bash: { command, parseEnvelope } }] as unknown as FlowStep[], + } as unknown as Flow; + const events: FlowEvent[] = []; + return executeFlow(flow, {}, fakeApi, 'write', [], { allowBash: true, onEvent: e => events.push(e) }); +} + +describe('bash step parseEnvelope — integration (#90)', () => { + it('unwraps a real claude-style envelope from stdout into step output', async () => { + // printf emits the raw envelope JSON; \n inside the result is JSON-escaped. + const envelopeText = '{"type":"result","result":"```json\\n{\\"ok\\":true,\\"n\\":2}\\n```"}'; + const ctx = await runBash(`printf '%s' '${envelopeText}'`, true); + assert.equal(ctx.steps.llm.status, 'success'); + assert.deepEqual(ctx.steps.llm.output, { ok: true, n: 2 }); + // raw stdout is still available on response + assert.match(String((ctx.steps.llm.response as { stdout: string }).stdout), /"type":"result"/); + }); + + it('fails the step when parseEnvelope output is not valid JSON', async () => { + const bad = '{"type":"result","result":"sorry, no JSON here"}'; + await assert.rejects( + () => runBash(`printf '%s' '${bad}'`, true), + /parseEnvelope: claude output was not valid JSON/, + ); + }); +}); diff --git a/src/lib/flow-engine.ts b/src/lib/flow-engine.ts index 97fd154..1ea3107 100644 --- a/src/lib/flow-engine.ts +++ b/src/lib/flow-engine.ts @@ -327,6 +327,70 @@ function stripCodeFences(text: string): string { return match ? match[1].trim() : trimmed; } +/** + * Strip code fences and any conversational preamble/trailer around a JSON + * body, returning just the `{...}`/`[...]` substring. Handles the common LLM + * shapes "Here is the analysis:\n{...}" and "```json\n{...}\n```". If no + * JSON-looking span is found, returns the fence-stripped text unchanged. + */ +function stripToJson(text: string): string { + const s = stripCodeFences(text); + // Slice from the first opening brace/bracket to the last closing one — + // trims both preamble ("Here is the analysis:\n{...}") and any trailer + // ("{...}\n\nLet me know!"). A clean JSON body is returned unchanged. + const candidates = [s.indexOf('{'), s.indexOf('[')].filter(i => i >= 0); + if (candidates.length === 0) return s; + const start = Math.min(...candidates); + const end = Math.max(s.lastIndexOf('}'), s.lastIndexOf(']')); + return end > start ? s.slice(start, end + 1) : s; +} + +/** + * Unwrap a `claude --print --output-format json` envelope and parse the inner + * JSON (#90). Handles the three defensive layers flows otherwise re-implement: + * envelope detection (`{type:'result', result}`), code-fence stripping, and + * preamble removal. Accepts stdout that is the full envelope, a bare result + * string, or already-unwrapped JSON. Throws (with a snippet) when the payload + * isn't valid JSON, so a bad LLM response fails the step instead of passing + * broken data downstream. + */ +export function unwrapClaudeEnvelope(stdout: string, stepId: string): unknown { + const trimmed = stdout.trim(); + let envelope: unknown; + try { + envelope = JSON.parse(trimmed); + } catch { + envelope = trimmed; // not JSON at the top level — treat stdout as the result text + } + + // Drill to the text that should contain the model's JSON. + let resultText: string; + if ( + envelope && typeof envelope === 'object' && !Array.isArray(envelope) && + (envelope as Record).type === 'result' && + typeof (envelope as Record).result === 'string' + ) { + resultText = (envelope as Record).result as string; + } else if (typeof envelope === 'string') { + resultText = envelope; + } else { + // Top-level JSON that isn't the envelope (e.g. claude already emitted bare + // JSON, or a future format) — use it directly. + return envelope; + } + + const payload = stripToJson(resultText); + try { + return JSON.parse(payload); + } catch (err) { + const snippet = payload.length > 200 ? `${payload.slice(0, 200)}…` : payload; + throw new Error( + `Bash step "${stepId}" parseEnvelope: claude output was not valid JSON after unwrapping ` + + `(${err instanceof Error ? err.message : String(err)}). Got: ${snippet}`, + ); + } +} + // ── Step Executors ── async function executeActionStep( @@ -1038,7 +1102,11 @@ async function executeBashStep( maxBuffer: 10 * 1024 * 1024, }); - const output = config.parseJson ? JSON.parse(stripCodeFences(stdout)) : stdout.trim(); + const output = config.parseEnvelope + ? unwrapClaudeEnvelope(stdout, step.id) + : config.parseJson + ? JSON.parse(stripCodeFences(stdout)) + : stdout.trim(); return { status: 'success', output, diff --git a/src/lib/flow-schema.ts b/src/lib/flow-schema.ts index 510dfd5..bef1505 100644 --- a/src/lib/flow-schema.ts +++ b/src/lib/flow-schema.ts @@ -265,6 +265,7 @@ export const FLOW_SCHEMA: FlowSchemaDescriptor = { command: { type: 'string', required: true, description: 'Shell command to execute (supports selectors)' }, timeout: { type: 'number', required: false, description: 'Timeout in ms (default: 30000)' }, parseJson: { type: 'boolean', required: false, description: 'Parse stdout as JSON (default: false). When true, $.steps..output is the parsed object/array; when false, it is the trimmed stdout string.' }, + parseEnvelope: { type: 'boolean', required: false, description: "Unwrap a `claude --print --output-format json` envelope: extract .result, strip code fences + preamble, parse the inner JSON as $.steps..output. Use this (instead of parseJson) for claude --print steps. Fails the step if the unwrapped payload isn't valid JSON." }, cwd: { type: 'string', required: false, description: 'Working directory (supports selectors)' }, env: { type: 'object', required: false, description: 'Additional environment variables' }, }, @@ -272,7 +273,7 @@ export const FLOW_SCHEMA: FlowSchemaDescriptor = { id: 'analyze', name: 'Analyze with Claude', type: 'bash', bash: { command: "cat /tmp/data.json | claude --print 'Analyze this data' --output-format json", - timeout: 180000, parseJson: true, + timeout: 180000, parseEnvelope: true, }, }, }, diff --git a/src/lib/flow-types.ts b/src/lib/flow-types.ts index 3e3dff1..9a15100 100644 --- a/src/lib/flow-types.ts +++ b/src/lib/flow-types.ts @@ -116,6 +116,17 @@ export interface FlowBashConfig { command: string; timeout?: number; parseJson?: boolean; + /** + * Unwrap a `claude --print --output-format json` CLI envelope before using + * the output (#90). The envelope looks like + * `{ "type": "result", "result": "```json\n{...}\n```" }`; with this set the + * step extracts `.result`, strips markdown code fences, drops any preamble + * text, and parses the inner JSON as `$.steps..output`. If the unwrapped + * payload isn't valid JSON the step fails with a clear error rather than + * passing broken data downstream. Implies JSON output (no need to also set + * `parseJson`). + */ + parseEnvelope?: boolean; cwd?: string; env?: Record; } diff --git a/src/lib/guide-content.ts b/src/lib/guide-content.ts index 0976278..30fb6ec 100644 --- a/src/lib/guide-content.ts +++ b/src/lib/guide-content.ts @@ -81,7 +81,7 @@ one --agent flow list # List all workflows - Code steps can reference an external \`.mjs\` module under the flow's \`lib/\` folder (stdin JSON in, stdout JSON out) — keeps JS out of JSON strings and makes flows shareable - 12 step types: action, transform, code, condition, loop, parallel, file-read, file-write, while, flow, paginate, bash - Data wiring via selectors: \`$.input.param\`, \`$.steps.stepId.response\`, \`$.loop.item\` -- AI analysis via bash steps: \`claude --print\` with \`parseJson: true\` +- AI analysis via bash steps: \`claude --print --output-format json\` with \`parseEnvelope: true\` (unwraps the CLI envelope + fences + preamble into clean parsed JSON; use instead of \`parseJson\` for claude steps) - Use \`--allow-bash\` to enable bash steps, \`--mock\` for dry-run with realistic mock responses (uses example data from action schemas) - Use \`--skip-validation\` to bypass input validation on action steps - Use \`--output-file \` to stream the full result to a file instead of stdout — for large results that would otherwise be truncated or hit the JSON string-size limit; stdout (and \`--agent\` output) then carries an \`outputFile\` pointer instead of inline \`steps\`