From a24b7183503d96dc27a1c131f2d3b89a91ddc98e Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 15 May 2026 17:25:59 +0200 Subject: [PATCH 1/2] fix(local): recover persona writer output from disk when stdout truncates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit claude --print buffers a full response then emits it. For large workflow artifacts the LLM's max_output_tokens cap kicks in and the JSON gets cut off mid-emit — observed in production as ~38KB of valid JSON ending at `"language": "typescript",` with no closing braces and no `artifact.content`. The parser then throws "Workforce persona response must be structured JSON or include fenced TypeScript artifact and JSON metadata blocks" and Ricky reports a parse-error, even though the writer typically *did* succeed at writing the workflow source to disk via the Write tool (reliable now that bypass-permissions ships through the resolver — see fix/headless-claude-bypass-permissions). This commit adds a freshness-gated disk fallback so the parser can recover from a truncated stdout without trusting stale on-disk leftovers: - `PersonaResponseParseOptions.writerInvokedAtMs`: caller pins a `Date.now()`-style timestamp *before* spawning the writer subprocess. - `PersonaResponseParseOptions.statFile`: stat-by-path override for deterministic tests; defaults to `fs.statSync` (ENOENT → undefined). - `recoverArtifactFromTruncatedOutput`: only fires after every other parse path has rejected the stdout. It reads `expectedPath` from disk ONLY when (a) `writerInvokedAtMs` is set AND (b) the file's `mtimeMs` is strictly greater than that timestamp — proving the file is the current writer's output, not a stale artifact from a prior attempt. - The recovered content still has to pass `validateArtifactContent` (the existing `workflow(...)` builder check). A junk file on disk cannot smuggle itself past the structural validation. - `writeWorkflowWithWorkforcePersona` captures `writerInvokedAtMs` before its `sendMessage` call and threads it into the parser, opting in. Other callers of `parsePersonaWorkflowResponse` that don't pass the timestamp keep their prior behavior — the fallback never fires. Coverage: 5 new tests in `workforce-persona-writer.test.ts`: - recovers from truncated stdout when the file on disk is fresh - does NOT fall back to a stale file (mtime older than writerInvokedAtMs) - does NOT fall back when writerInvokedAtMs is absent - does NOT fall back when no file exists at expectedPath - does NOT fall back when the on-disk file fails structural validation 151 of 151 generation-suite tests pass. Pre-existing typecheck errors in `scheduled-agent.ts` are unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workforce-persona-writer.test.ts | 84 ++++++++++++++++++ .../generation/workforce-persona-writer.ts | 88 ++++++++++++++++++- 2 files changed, 171 insertions(+), 1 deletion(-) diff --git a/src/product/generation/workforce-persona-writer.test.ts b/src/product/generation/workforce-persona-writer.test.ts index 5f25ddc2..02341b70 100644 --- a/src/product/generation/workforce-persona-writer.test.ts +++ b/src/product/generation/workforce-persona-writer.test.ts @@ -275,6 +275,90 @@ describe('workforce persona workflow writer', () => { } }); + describe('truncated-stdout disk fallback', () => { + // The claude CLI buffers a full --print response then emits it; for + // large workflow artifacts the LLM's max_output_tokens cap kicks in + // and the JSON gets cut off mid-emit. The writer typically *did* + // succeed at writing the workflow to disk via the Write tool (now that + // bypass-permissions makes that reliable in headless mode), so the + // parser should fall back to reading the file rather than hard-failing. + + const truncatedJsonOutput = [ + 'I need to author a workflow that ships this PR. Let me produce the structured JSON response.', + '', + '```json', + '{', + ' "artifact": {', + ' "path": "workflows/generated/persona.ts",', + ' "language": "typescript",', + // cut off here — no `content` field, no closing braces + ].join('\n'); + + it('recovers from truncated stdout by reading a fresh file at expectedPath', () => { + const artifactPath = 'workflows/generated/persona.ts'; + const writerInvokedAtMs = 100; + const parsed = parsePersonaWorkflowResponse(truncatedJsonOutput, artifactPath, { + repoRoot: '/tmp/repo', + writerInvokedAtMs, + statFile: (path) => path.endsWith('persona.ts') ? { mtimeMs: writerInvokedAtMs + 1_000 } : undefined, + readFileText: () => workflowSource(), + }); + expect(parsed.responseFormat).toBe('structured-json'); + expect(parsed.content).toContain('workflow("persona")'); + expect(parsed.metadata).toMatchObject({ recoveredFromDisk: true }); + }); + + it('does NOT fall back to a STALE file (mtime older than writerInvokedAtMs)', () => { + const artifactPath = 'workflows/generated/persona.ts'; + const writerInvokedAtMs = 1_000; + expect(() => + parsePersonaWorkflowResponse(truncatedJsonOutput, artifactPath, { + repoRoot: '/tmp/repo', + writerInvokedAtMs, + statFile: () => ({ mtimeMs: writerInvokedAtMs - 1 }), + readFileText: () => workflowSource(), + }), + ).toThrow(/structured JSON or include fenced TypeScript artifact/); + }); + + it('does NOT fall back when `writerInvokedAtMs` is absent (preserves prior behavior for callers that have not opted in)', () => { + const artifactPath = 'workflows/generated/persona.ts'; + expect(() => + parsePersonaWorkflowResponse(truncatedJsonOutput, artifactPath, { + repoRoot: '/tmp/repo', + // writerInvokedAtMs intentionally omitted + statFile: () => ({ mtimeMs: Date.now() }), + readFileText: () => workflowSource(), + }), + ).toThrow(/structured JSON or include fenced TypeScript artifact/); + }); + + it('does NOT fall back when no file exists at expectedPath (stat returns undefined)', () => { + const artifactPath = 'workflows/generated/persona.ts'; + expect(() => + parsePersonaWorkflowResponse(truncatedJsonOutput, artifactPath, { + repoRoot: '/tmp/repo', + writerInvokedAtMs: 100, + statFile: () => undefined, + readFileText: () => workflowSource(), + }), + ).toThrow(/structured JSON or include fenced TypeScript artifact/); + }); + + it('does NOT fall back when the on-disk file fails structural validation (re-raises original parser error)', () => { + const artifactPath = 'workflows/generated/persona.ts'; + expect(() => + parsePersonaWorkflowResponse(truncatedJsonOutput, artifactPath, { + repoRoot: '/tmp/repo', + writerInvokedAtMs: 100, + statFile: () => ({ mtimeMs: 200 }), + // File on disk is not a valid workflow (no `workflow(` call) — fallback should refuse it. + readFileText: () => 'export const broken = 1;', + }), + ).toThrow(/structured JSON or include fenced TypeScript artifact/); + }); + }); + it('invokes the spawned harness non-interactively (no TUI flag, structured-response contract)', async () => { const base = generate({ spec: spec({ diff --git a/src/product/generation/workforce-persona-writer.ts b/src/product/generation/workforce-persona-writer.ts index 9a927209..b176fb28 100644 --- a/src/product/generation/workforce-persona-writer.ts +++ b/src/product/generation/workforce-persona-writer.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto'; -import { readFileSync } from 'node:fs'; +import { readFileSync, statSync } from 'node:fs'; import { mkdir, readFile, writeFile } from 'node:fs/promises'; import { dirname, isAbsolute, join, resolve } from 'node:path'; @@ -158,6 +158,25 @@ export interface WorkforcePersonaWriterResult { export interface PersonaResponseParseOptions { repoRoot?: string; readFileText?: (path: string) => string; + /** + * Optional `Date.now()`-style ms timestamp captured *before* the writer + * subprocess was spawned. When set, the parser's "stdout was truncated + * but a file exists on disk at expectedPath" fallback only fires for a + * file whose mtime is newer than this — i.e. one this writer call + * produced — so the parser cannot accidentally accept a stale leftover + * artifact from a prior attempt as the current writer's output. + * + * Without this timestamp the parser treats the file as untrusted and + * skips the disk fallback (preserving prior behavior). + */ + writerInvokedAtMs?: number; + /** + * Optional stat-by-path used by the disk fallback. Defaults to + * `fs.statSync(path)` swallowing ENOENT to `undefined`. Override for + * deterministic tests of the freshness check without touching the real + * filesystem. + */ + statFile?: (path: string) => { mtimeMs: number } | undefined; } export interface ResolvedWorkforcePersonaContext { @@ -210,6 +229,10 @@ export async function writeWorkflowWithWorkforcePersona( ): Promise { const workflowName = options.workflowName ?? workflowNameFromOutputPath(options.outputPath); const relevantFiles = await resolveRelevantFiles(options.repoRoot, spec, options.relevantFiles); + // Pin "writer invoked at" before the spawn so the parser's disk fallback + // can prove a file at outputPath was actually written by THIS run (mtime + // newer than this timestamp) rather than left over from a prior attempt. + const writerInvokedAtMs = Date.now(); const resolver = options.resolver ?? defaultWorkforcePersonaResolver; const resolved = await resolver( options.personaIntentCandidates ?? WORKFORCE_PERSONA_INTENT_CANDIDATES, @@ -275,6 +298,7 @@ export async function writeWorkflowWithWorkforcePersona( try { parsed = parsePersonaWorkflowResponse(result.output, options.outputPath, { repoRoot: options.repoRoot, + writerInvokedAtMs: writerInvokedAtMs, }); } catch (error) { await dumpDebug('parse-error'); @@ -955,11 +979,73 @@ export function parsePersonaWorkflowResponse( return validateStructuredResponse(embedded, expectedPath, 'structured-json', options); } + // Last-resort disk fallback. claude --print buffers a full response then + // returns; for large workflow artifacts the LLM's max_output_tokens cap + // kicks in and the JSON gets cut off mid-emit (observed: 38KB of valid + // JSON ending at `"language": "typescript",` with no closing braces). In + // that case the writer typically *did* succeed at writing the workflow + // source to disk via the Write tool — there's just no parseable stdout + // wrapper for the parser to read. Trust the file on disk only when (a) + // the caller provided `writerInvokedAtMs` so we can prove the file is + // fresh, and (b) the file's mtime is strictly after that timestamp. + const recovered = recoverArtifactFromTruncatedOutput(expectedPath, options); + if (recovered) { + try { + validateArtifactContent(recovered); + return { + content: recovered, + metadata: { recoveredFromDisk: true, reason: 'stdout-truncated-or-unparseable' }, + responseFormat: 'structured-json', + }; + } catch { + // Recovered content failed structural validation; fall through to + // the original parser-failure error so the caller sees the real + // truncation symptom rather than a misleading "no workflow()" error. + } + } + throw new WorkforcePersonaWriterError( 'Workforce persona response must be structured JSON or include fenced TypeScript artifact and JSON metadata blocks.', ); } +/** + * Reads the artifact from disk as a last-resort fallback when the writer's + * stdout could not be parsed. Only fires when the caller pinned a + * `writerInvokedAtMs` timestamp AND the file at `expectedPath` was modified + * strictly after it — proving the file is the current writer's output, not + * a stale leftover from a prior attempt. + */ +function recoverArtifactFromTruncatedOutput( + expectedPath: string, + options: PersonaResponseParseOptions, +): string | undefined { + if (!options.repoRoot || options.writerInvokedAtMs === undefined) return undefined; + const expectedResolved = isAbsolute(expectedPath) + ? expectedPath + : resolve(options.repoRoot, expectedPath); + + const statFn = options.statFile ?? defaultStatFile; + const stat = statFn(expectedResolved); + if (!stat) return undefined; + if (stat.mtimeMs <= options.writerInvokedAtMs) return undefined; + + try { + return options.readFileText?.(expectedResolved) ?? readFileSync(expectedResolved, 'utf8'); + } catch { + return undefined; + } +} + +function defaultStatFile(path: string): { mtimeMs: number } | undefined { + try { + const stats = statSync(path); + return { mtimeMs: stats.mtimeMs }; + } catch { + return undefined; + } +} + /** * Walks `text` looking for the first top-level balanced `{ ... }` object * and parses it. String literals (including nested escape sequences) and From 4595f612d17db2cf56cf8f2af12c09d527a0bc2e Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 15 May 2026 17:46:57 +0200 Subject: [PATCH 2/2] review: capture writerInvokedAtMs immediately before sendMessage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both coderabbit and cubic-dev-ai flagged the same issue: pinning `writerInvokedAtMs = Date.now()` at the top of `writeWorkflowWithWorkforcePersona` runs *before* the `await resolver(...)` and `buildWorkflowPersonaTask(...)` calls. The resolver loads personas (filesystem + bundle merge) and the task builder reads spec files, so the window between the original capture and the actual `sendMessage` spawn can stretch into multiple seconds. A stale leftover file at `outputPath` whose mtime lands inside that window would falsely satisfy the freshness check and impersonate the current writer's output. Moves the capture to the line immediately above `resolved.context. sendMessage(task, ...)` — after all preceding awaits, before any code that could plausibly write to outputPath — so the timestamp now strictly precedes the spawn. The comment is rewritten to be explicit about why "immediately before" matters. All 55 writer-suite tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/product/generation/workforce-persona-writer.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/product/generation/workforce-persona-writer.ts b/src/product/generation/workforce-persona-writer.ts index b176fb28..ff653aaa 100644 --- a/src/product/generation/workforce-persona-writer.ts +++ b/src/product/generation/workforce-persona-writer.ts @@ -229,10 +229,6 @@ export async function writeWorkflowWithWorkforcePersona( ): Promise { const workflowName = options.workflowName ?? workflowNameFromOutputPath(options.outputPath); const relevantFiles = await resolveRelevantFiles(options.repoRoot, spec, options.relevantFiles); - // Pin "writer invoked at" before the spawn so the parser's disk fallback - // can prove a file at outputPath was actually written by THIS run (mtime - // newer than this timestamp) rather than left over from a prior attempt. - const writerInvokedAtMs = Date.now(); const resolver = options.resolver ?? defaultWorkforcePersonaResolver; const resolved = await resolver( options.personaIntentCandidates ?? WORKFORCE_PERSONA_INTENT_CANDIDATES, @@ -250,6 +246,13 @@ export async function writeWorkflowWithWorkforcePersona( }); const promptDigest = digest(task); const selection = resolved.context.selection; + // Pin "writer invoked at" immediately before the spawn so the parser's + // disk fallback can prove a file at outputPath was actually written by + // THIS sendMessage window (mtime > this timestamp) rather than left + // over from a prior attempt. Capturing earlier — e.g. before the + // resolver and task-builder awaits — opens a multi-second window in + // which a stale leftover could falsely satisfy the freshness check. + const writerInvokedAtMs = Date.now(); const run = resolved.context.sendMessage(task, { workingDirectory: options.repoRoot, name: `ricky-workflow-writer-${promptDigest.slice(0, 12)}`,