diff --git a/packages/baoyu-codex-imagegen/src/main.ts b/packages/baoyu-codex-imagegen/src/main.ts index ac66035..6827e11 100755 --- a/packages/baoyu-codex-imagegen/src/main.ts +++ b/packages/baoyu-codex-imagegen/src/main.ts @@ -6,7 +6,7 @@ import process from "node:process"; import { setTimeout as delay } from "node:timers/promises"; import { GenError, type CliOptions, type GenerateResult } from "./types.ts"; import { runCodexExec } from "./spawn.ts"; -import { findCpToTarget, verifyImageGenWasInvoked, verifyOutput } from "./validator.ts"; +import { hasImageGenEvidence, verifyImageGenWasInvoked, verifyOutput } from "./validator.ts"; import { cacheKey, lookupCache, storeCache, FileLock } from "./cache.ts"; import { JsonLogger } from "./logger.ts"; @@ -125,7 +125,7 @@ function buildInstruction(prompt: string, opts: CliOptions): string { const refHint = opts.refImages.length > 0 ? `\nREFERENCE IMAGES (attached above): ${opts.refImages.length} image(s) provided for style/composition guidance.\n` : ""; - return `You have an internal tool called image_gen for image generation. Use it. + return `You have an internal tool called image_gen for image generation. You MUST call it before doing anything else. TASK: Generate an image with the spec below, then save to disk. @@ -137,12 +137,15 @@ OUTPUT PATH: ${opts.outputPath} ${refHint} STEPS: 1. Call image_gen with the prompt and aspect ratio above${opts.refImages.length > 0 ? " (using the attached reference images for guidance)" : ""}. -2. Move or copy the resulting image from Codex default location ($CODEX_HOME/generated_images/...) to: ${opts.outputPath} +2. Move or copy ONLY the image produced by that image_gen call from Codex default location ($CODEX_HOME/generated_images/...) to: ${opts.outputPath} 3. Verify with: ls -la ${opts.outputPath} 4. Reply with ONLY this JSON line (no markdown fences, no other text): {"status":"ok","path":"${opts.outputPath}","bytes":} HARD CONSTRAINTS: +- Do NOT search for, find, inspect, reuse, or copy any pre-existing files from $CODEX_HOME/generated_images/ or any other directory. +- Do NOT run ls/find/rg/grep/glob over $CODEX_HOME/generated_images/ before image_gen has been called. +- You MUST call image_gen first. Only after image_gen completes may you copy the newly created file from this turn. - Do NOT use curl, wget, Python, or any external API. - Do NOT use bash to fabricate an image; only image_gen produces real pixels. - Use ONLY the image_gen internal tool.`; @@ -175,13 +178,16 @@ async function attemptGenerate( throw new GenError("agent_refused", "No thread id in event stream"); } - // verify: image_gen was actually invoked (check $CODEX_HOME/generated_images/{threadId}/) + // verify image_gen ran in THIS thread. A PNG in this thread's + // generated_images dir is the real signal (image_gen does not surface as a + // stream item); the stream check is a forward-compatible fallback. The #185 + // shortcut (copying an unrelated history image) yields neither. const ver = await verifyImageGenWasInvoked(run.threadId); - if (!ver.ok) { - // secondary verify: did tool_calls include cp/mv from generated_images to our target - if (!findCpToTarget(run.toolCalls, opts.outputPath)) { - throw new GenError("no_image_gen_tool_use", `image_gen was not invoked: ${ver.reason}`); - } + if (!hasImageGenEvidence(run.toolCalls, ver.ok)) { + throw new GenError( + "no_image_gen_tool_use", + `image_gen was not invoked (no image_gen event in stream; ${ver.reason})`, + ); } // verify output diff --git a/packages/baoyu-codex-imagegen/src/parser.test.ts b/packages/baoyu-codex-imagegen/src/parser.test.ts index 2ce939f..1ce2996 100644 --- a/packages/baoyu-codex-imagegen/src/parser.test.ts +++ b/packages/baoyu-codex-imagegen/src/parser.test.ts @@ -33,17 +33,26 @@ test("parseEventStream tolerates malformed lines", () => { expect(r.usage?.input).toBe(1); }); -test("hasImageGenInvocation finds shell calls touching generated_images", () => { +test("hasImageGenInvocation does not infer image_gen from shell copies", () => { const r = parseEventStream(REAL_PoC_STREAM); - // image_gen itself is not an event; inferred via generated_images cp path - // this test only verifies parser behavior; driver logic lives in validator const hasCp = r.toolCalls.some((tc) => tc.command?.includes("generated_images")); expect(hasCp).toBe(true); + expect(hasImageGenInvocation(r.toolCalls)).toBe(false); }); -test("hasImageGenInvocation (proper) returns false when no image_gen tool", () => { +test("hasImageGenInvocation detects real image_gen tool calls only", () => { expect(hasImageGenInvocation([{ id: "1", tool: "shell", status: "completed" }])).toBe(false); expect( hasImageGenInvocation([{ id: "1", tool: "image_gen", status: "completed" }]), ).toBe(true); }); + +test("parseEventStream normalizes an image_generation item to image_gen", () => { + const stream = `{"type":"thread.started","thread_id":"t1"} +{"type":"item.started","item":{"id":"g0","type":"image_generation","status":"in_progress"}} +{"type":"item.completed","item":{"id":"g0","type":"image_generation","status":"completed"}} +{"type":"item.completed","item":{"id":"m0","type":"agent_message","text":"done"}}`; + const r = parseEventStream(stream); + expect(r.toolCalls.some((tc) => tc.tool === "image_gen")).toBe(true); + expect(hasImageGenInvocation(r.toolCalls)).toBe(true); +}); diff --git a/packages/baoyu-codex-imagegen/src/validator.test.ts b/packages/baoyu-codex-imagegen/src/validator.test.ts index 3e38656..8acd579 100644 --- a/packages/baoyu-codex-imagegen/src/validator.test.ts +++ b/packages/baoyu-codex-imagegen/src/validator.test.ts @@ -2,11 +2,52 @@ import { test, expect } from "bun:test"; import { mkdtemp, writeFile, rm, mkdir } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; -import { verifyOutput, verifyImageGenWasInvoked, findCpToTarget } from "./validator.ts"; +import { verifyOutput, verifyImageGenWasInvoked, hasImageGenEvidence } from "./validator.ts"; +import { parseEventStream } from "./parser.ts"; import { GenError } from "./types.ts"; const PNG_HEADER = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +// Condensed + sanitized capture of a REAL successful `codex exec --json` run +// (thread 019edc1c…, 16:9 maple-tree image, single attempt). The image_gen +// tool leaves NO stream item — success shows only reasoning / command_execution +// / agent_message, and a `cp` from generated_images//ig_*.png. +const REAL_SUCCESS_STREAM = `{"type":"thread.started","thread_id":"019edc1c-e7a3-74f0-a276-13bea71d32d6"} +{"type":"turn.started"} +{"type":"item.completed","item":{"id":"r0","type":"reasoning"}} +{"type":"item.completed","item":{"id":"r1","type":"reasoning"}} +{"type":"item.completed","item":{"id":"m0","type":"agent_message","text":"Image generation is complete. Locating the newly produced image and copying it to the requested path."}} +{"type":"item.started","item":{"id":"c0","type":"command_execution","command":"ls $CODEX_HOME/generated_images","status":"in_progress"}} +{"type":"item.completed","item":{"id":"c0","type":"command_execution","command":"ls $CODEX_HOME/generated_images","exit_code":1,"status":"failed"}} +{"type":"item.started","item":{"id":"c1","type":"command_execution","command":"cp $CODEX_HOME/generated_images/019edc1c-e7a3-74f0-a276-13bea71d32d6/ig_03eda661.png /Users/x/out/maple.png","status":"in_progress"}} +{"type":"item.completed","item":{"id":"c1","type":"command_execution","command":"cp $CODEX_HOME/generated_images/019edc1c-e7a3-74f0-a276-13bea71d32d6/ig_03eda661.png /Users/x/out/maple.png","exit_code":0,"status":"completed"}} +{"type":"item.completed","item":{"id":"m1","type":"agent_message","text":"{\\"status\\":\\"ok\\",\\"path\\":\\"/Users/x/out/maple.png\\",\\"bytes\\":1317377}"}} +{"type":"turn.completed","usage":{"input_tokens":49489,"cached_input_tokens":34432,"output_tokens":2463,"reasoning_output_tokens":1990}}`; + +test("real success stream carries no image_gen item — gating on the stream alone would false-negative (#185)", () => { + const r = parseEventStream(REAL_SUCCESS_STREAM); + expect(r.threadId).toBe("019edc1c-e7a3-74f0-a276-13bea71d32d6"); + // success path tools: reasoning / shell / agent_message — never image_gen + expect(r.toolCalls.map((tc) => tc.tool).sort()).toEqual([ + "agent_message", + "agent_message", + "reasoning", + "reasoning", + "shell", + "shell", + ]); + expect(r.toolCalls.some((tc) => tc.tool === "image_gen")).toBe(false); + // filesystem evidence (PNG in this thread's dir) is what must let it through + expect(hasImageGenEvidence(r.toolCalls, true)).toBe(true); + // with no filesystem evidence and no stream item, it is rejected (the shortcut) + expect(hasImageGenEvidence(r.toolCalls, false)).toBe(false); +}); + +test("hasImageGenEvidence accepts a real image_gen stream item even without a dir PNG", () => { + expect(hasImageGenEvidence([{ id: "1", tool: "image_gen", status: "completed" }], false)).toBe(true); + expect(hasImageGenEvidence([{ id: "1", tool: "shell", status: "completed" }], false)).toBe(false); +}); + test("verifyOutput passes for valid PNG", async () => { const dir = await mkdtemp(path.join(tmpdir(), "cig-val-")); try { @@ -73,26 +114,3 @@ test("verifyImageGenWasInvoked true when PNG exists in thread dir", async () => await rm(tempHome, { recursive: true, force: true }); } }); - -test("findCpToTarget detects cp from generated_images", () => { - expect( - findCpToTarget( - [ - { - id: "1", - tool: "shell", - status: "completed", - command: "cp ~/.codex/generated_images/thread/ig_x.png /tmp/out.png", - }, - ], - "/tmp/out.png", - ), - ).toBe(true); - - expect( - findCpToTarget( - [{ id: "1", tool: "shell", status: "completed", command: "ls /tmp" }], - "/tmp/out.png", - ), - ).toBe(false); -}); diff --git a/packages/baoyu-codex-imagegen/src/validator.ts b/packages/baoyu-codex-imagegen/src/validator.ts index 487ad81..fe83752 100644 --- a/packages/baoyu-codex-imagegen/src/validator.ts +++ b/packages/baoyu-codex-imagegen/src/validator.ts @@ -3,6 +3,7 @@ import { homedir } from "node:os"; import path from "node:path"; import { GenError } from "./types.ts"; import type { ToolCall } from "./types.ts"; +import { hasImageGenInvocation } from "./parser.ts"; const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); @@ -23,15 +24,15 @@ export async function verifyImageGenWasInvoked(threadId: string | null): Promise } } -export function findCpToTarget(toolCalls: ToolCall[], target: string): boolean { - return toolCalls.some( - (tc) => - tc.tool === "shell" && - typeof tc.command === "string" && - (tc.command.includes(target) || tc.command.includes(path.basename(target))) && - /\b(cp|mv|cat)\b/.test(tc.command) && - tc.command.includes("generated_images"), - ); +// Real evidence that image_gen ran in THIS thread. Codex's image_gen tool does +// not surface as a stream item, so a successful run shows only reasoning/shell/ +// agent_message — `dirHasImage` (a PNG in this thread's generated_images dir) is +// what proves it. The stream check is kept as a forward-compatible signal in +// case a future Codex version emits the item. The #185 shortcut (copying an +// unrelated history image, which lives under a different thread id) yields +// neither, so it is correctly rejected. +export function hasImageGenEvidence(toolCalls: ToolCall[], dirHasImage: boolean): boolean { + return dirHasImage || hasImageGenInvocation(toolCalls); } export async function verifyOutput(outputPath: string): Promise<{ bytes: number }> { diff --git a/skills/baoyu-image-gen/scripts/codex-imagegen/main.ts b/skills/baoyu-image-gen/scripts/codex-imagegen/main.ts index ac66035..6827e11 100755 --- a/skills/baoyu-image-gen/scripts/codex-imagegen/main.ts +++ b/skills/baoyu-image-gen/scripts/codex-imagegen/main.ts @@ -6,7 +6,7 @@ import process from "node:process"; import { setTimeout as delay } from "node:timers/promises"; import { GenError, type CliOptions, type GenerateResult } from "./types.ts"; import { runCodexExec } from "./spawn.ts"; -import { findCpToTarget, verifyImageGenWasInvoked, verifyOutput } from "./validator.ts"; +import { hasImageGenEvidence, verifyImageGenWasInvoked, verifyOutput } from "./validator.ts"; import { cacheKey, lookupCache, storeCache, FileLock } from "./cache.ts"; import { JsonLogger } from "./logger.ts"; @@ -125,7 +125,7 @@ function buildInstruction(prompt: string, opts: CliOptions): string { const refHint = opts.refImages.length > 0 ? `\nREFERENCE IMAGES (attached above): ${opts.refImages.length} image(s) provided for style/composition guidance.\n` : ""; - return `You have an internal tool called image_gen for image generation. Use it. + return `You have an internal tool called image_gen for image generation. You MUST call it before doing anything else. TASK: Generate an image with the spec below, then save to disk. @@ -137,12 +137,15 @@ OUTPUT PATH: ${opts.outputPath} ${refHint} STEPS: 1. Call image_gen with the prompt and aspect ratio above${opts.refImages.length > 0 ? " (using the attached reference images for guidance)" : ""}. -2. Move or copy the resulting image from Codex default location ($CODEX_HOME/generated_images/...) to: ${opts.outputPath} +2. Move or copy ONLY the image produced by that image_gen call from Codex default location ($CODEX_HOME/generated_images/...) to: ${opts.outputPath} 3. Verify with: ls -la ${opts.outputPath} 4. Reply with ONLY this JSON line (no markdown fences, no other text): {"status":"ok","path":"${opts.outputPath}","bytes":} HARD CONSTRAINTS: +- Do NOT search for, find, inspect, reuse, or copy any pre-existing files from $CODEX_HOME/generated_images/ or any other directory. +- Do NOT run ls/find/rg/grep/glob over $CODEX_HOME/generated_images/ before image_gen has been called. +- You MUST call image_gen first. Only after image_gen completes may you copy the newly created file from this turn. - Do NOT use curl, wget, Python, or any external API. - Do NOT use bash to fabricate an image; only image_gen produces real pixels. - Use ONLY the image_gen internal tool.`; @@ -175,13 +178,16 @@ async function attemptGenerate( throw new GenError("agent_refused", "No thread id in event stream"); } - // verify: image_gen was actually invoked (check $CODEX_HOME/generated_images/{threadId}/) + // verify image_gen ran in THIS thread. A PNG in this thread's + // generated_images dir is the real signal (image_gen does not surface as a + // stream item); the stream check is a forward-compatible fallback. The #185 + // shortcut (copying an unrelated history image) yields neither. const ver = await verifyImageGenWasInvoked(run.threadId); - if (!ver.ok) { - // secondary verify: did tool_calls include cp/mv from generated_images to our target - if (!findCpToTarget(run.toolCalls, opts.outputPath)) { - throw new GenError("no_image_gen_tool_use", `image_gen was not invoked: ${ver.reason}`); - } + if (!hasImageGenEvidence(run.toolCalls, ver.ok)) { + throw new GenError( + "no_image_gen_tool_use", + `image_gen was not invoked (no image_gen event in stream; ${ver.reason})`, + ); } // verify output diff --git a/skills/baoyu-image-gen/scripts/codex-imagegen/validator.ts b/skills/baoyu-image-gen/scripts/codex-imagegen/validator.ts index 487ad81..fe83752 100644 --- a/skills/baoyu-image-gen/scripts/codex-imagegen/validator.ts +++ b/skills/baoyu-image-gen/scripts/codex-imagegen/validator.ts @@ -3,6 +3,7 @@ import { homedir } from "node:os"; import path from "node:path"; import { GenError } from "./types.ts"; import type { ToolCall } from "./types.ts"; +import { hasImageGenInvocation } from "./parser.ts"; const PNG_MAGIC = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); @@ -23,15 +24,15 @@ export async function verifyImageGenWasInvoked(threadId: string | null): Promise } } -export function findCpToTarget(toolCalls: ToolCall[], target: string): boolean { - return toolCalls.some( - (tc) => - tc.tool === "shell" && - typeof tc.command === "string" && - (tc.command.includes(target) || tc.command.includes(path.basename(target))) && - /\b(cp|mv|cat)\b/.test(tc.command) && - tc.command.includes("generated_images"), - ); +// Real evidence that image_gen ran in THIS thread. Codex's image_gen tool does +// not surface as a stream item, so a successful run shows only reasoning/shell/ +// agent_message — `dirHasImage` (a PNG in this thread's generated_images dir) is +// what proves it. The stream check is kept as a forward-compatible signal in +// case a future Codex version emits the item. The #185 shortcut (copying an +// unrelated history image, which lives under a different thread id) yields +// neither, so it is correctly rejected. +export function hasImageGenEvidence(toolCalls: ToolCall[], dirHasImage: boolean): boolean { + return dirHasImage || hasImageGenInvocation(toolCalls); } export async function verifyOutput(outputPath: string): Promise<{ bytes: number }> {