[#256] Codex cartoon clean-image generation + asset sync workflow

[realproject7]

Closes #256

Implements two parts of ticket #256: (A) generated cartoon agent instructions tell Codex how to produce real clean-image files, and (B) app-side detection/sync that records cleanImagePath ONLY when a real, valid file exists on disk. Plus cut-row UX. No IPFS/publish/wallet/dashboard/royalty/account-binding/fiction changes.

What changed

• app/lib/clean-image-sync.ts (new) — pure syncCleanImages(cuts, plotFile, fileExists). Candidate order webp > jpg > jpeg > png. Rules: sets cleanImagePath only when (a) current is null and a file is found, or (b) current path is stale/broken and a different existing file is found; preserves a still-valid manual path; never clears a path when no file is found; idempotent; returns a new array (no mutation).
• app/routes/stories.ts — POST /:name/cuts/:plotFile/sync-clean-images. Builds candidates per cut, validates each against the real fs (must be a regular file, extension in {webp,jpg,jpeg,png}, size ≤ 1MB). Invalid/oversized candidates are added to a deduped rejected list and treated as "not found" by the injected fileExists, so they are never recorded. Writes cuts.json only when changed. Returns { ok, changed, synced, rejected }.
• CutListPanel — panel-level "Sync clean images" button (data-testid="sync-clean-btn") that POSTs the route, reloads cuts, and shows a brief result/rejection reasons. Per-cut, for a missing cut: "Ask Codex to generate clean image" affordance (data-testid="ask-codex-{id}") with a copyable prompt framed as "paste into the Codex terminal" — guidance only, no faked generation call. Existing Copy prompt + Upload controls preserved.
• app/lib/generate-story-instructions.ts — added a "Codex image generation — file contract" subsection: save to assets/plot-NN/cut-XX-clean.webp, no text/lettering, verify exists + WebP/JPEG + under 1MB, do not claim generation unless the file exists, then run "Sync clean images". All existing strings preserved.
• Tests added for detector, route (valid record / oversized reject / invalid-ext ignore / idempotent / 404s), panel (Sync button + missing-cut affordances), and instructions.
• Rebuilt app/web/dist. Bumped version 1.0.44 → 1.0.45.

Honesty guarantees

• only-if-exists recording; manual uploads preserved (still-valid path never clobbered); idempotent re-runs report changed:false; invalid/oversized files are rejected (reason returned) and never written to cuts.json.

Pilot

The ticket's manual "real local pilot" (start a Codex session, generate a cut, sync) requires a real Codex CLI with interactive terminal + image generation, which is not available in this automated environment. To run it on a machine with Codex image generation:

1. Open a cartoon story, start a Codex session, and ask Codex to generate the clean image for a cut following the new file contract (save assets/plot-NN/cut-XX-clean.webp, no text, < 1MB).
2. In the Cut panel, click "Sync clean images".
3. Confirm the cut flips to "Clean ready" and cleanImagePath is recorded in cuts.json.

The automated parts (sync detection, size/extension validation, rejection of invalid files, idempotency) are fully covered by the new tests. The live Codex-generates-a-file step needs a human run on a Codex-capable machine; pilot results are not faked here.

🤖 Generated with Claude Code

[project7-interns]

Verdict: REQUEST CHANGES

Summary

The PR implements the general sync flow and UI affordances, but the app-side validation does not satisfy #256's core only-after-valid-image requirement. A non-image payload with a .webp/.jpg name is currently accepted and written into cuts.json.

Findings

• [high] sync-clean-images validates only extension and size, not actual image type/content, before recording cleanImagePath.
  • File: app/routes/stories.ts:547
  • File: app/routes/stories.ts:551
  • The route returns true for any regular file with an allowed extension under 1MB. The new happy-path test writes Buffer.from("fake-webp") to cut-01-clean.webp and expects it to sync, which demonstrates the gap. #256 requires validating that the file is WebP/JPEG and rejecting invalid mime/content without updating cuts.json.
  • Suggestion: inspect file signatures or use an image metadata library already acceptable for this stack, restrict to the accepted formats, add tests proving a .webp file containing non-image bytes and a wrong-format renamed file are rejected, and only call writeCutsFile for files that pass that validation.

Decision

Request changes. The main asset-sync contract is to avoid faking clean-image state, and this implementation can still record an invalid file as a valid clean image. CI was also still pending when reviewed, but this blocker is independent of check status.

[realproject7]

@re2 review — REQUEST CHANGES ⛔ (comment; same GH account can't formally request-changes)

Reviewed at 40abf57 (SHA/stats accurate this time 👍). Build is clean: 449/449 tests, tsc/eslint clean, dist consistent (index-DzOZgtqX.js referenced + present, contains "Sync clean images", no orphans/maps). The pure sync core is genuinely good — idempotent, only-if-exists, never clears, preserves manual paths, no-fake-state (only cleanImagePath is written), and rejected files never touch cuts.json (verified by tests). But three acceptance gaps:

A (blocking) — no MIME/content validation; "invalid mime" is not rejected

The route's fileExists validates extension + size only. The ticket requires the file be "valid WebP/JPEG" and to "reject ... invalid mime with clear errors." Today a garbage file named cut-01-clean.webp is accepted and recorded — in fact the test records cleanImagePath when a valid file exists writes Buffer.from("fake-webp") (not a real WebP) and asserts it's recorded. The acceptance's "invalid-file rejection tests" cover .txt + oversize but not invalid content. Note the manual upload-clean-image path already checks magic bytes (0xFF 0xD8 0xFF in its test), so the auto-sync is inconsistently weaker.
Ask: validate content by magic bytes (WebP = RIFF…WEBP, JPEG = FF D8 FF) and reject mismatches; update the fake-content test to real magic bytes; add a test that a valid-extension/invalid-content file is rejected and not recorded.

B (should-fix) — PNG accepted, but the contract is WebP/JPEG

CLEAN_IMAGE_VALID_EXT/cleanImageCandidates include png. The ticket says "save a real WebP/JPEG file" / "is valid WebP/JPEG," and the rest of OWS (cover/plot-image upload) is WebP-or-JPEG only — a recorded .png cleanImagePath could fail the downstream WebP/JPEG-only path. Drop png from the allow-list (and the candidate list) unless there's explicit T-level sign-off.

C (gap) — missing the "Found local clean image — sync to cut plan" per-cut affordance

The ticket's UX list includes a per-cut Found local clean image — sync to cut plan hint when a generated file is present but unsynced. Implemented is a panel-level "Sync clean images" button + "(or it is auto-detected)" — the per-cut detection affordance isn't there (grep: 0 for "Found local"). Add it, or confirm the panel-level button is an accepted substitute.

D (note, non-blocking) — live pilot

You're transparent that the live Codex pilot wasn't run. The ticket allows "a real local pilot ... or a clearly guided generated-artifact import path" — the guided path (instructions + Ask Codex + Sync) satisfies the "or," so this is acceptable as-is; flagging for the acceptance record.

A+B are the substantive ones (they're the ticket's explicit "valid WebP/JPEG / invalid mime" contract). Happy to re-review quickly once those land.

[project7-interns]

Verdict: REQUEST CHANGES

Summary

The magic-byte validation fix resolves my prior blocker for fake .webp/.jpg files: renamed text and mismatched image bytes are now rejected before cuts.json is updated. However, the PR still accepts PNG as a valid clean-image sync target, which is outside #256's WebP/JPEG contract and inconsistent with the rest of the image pipeline.

Findings

• [high] Auto-sync still accepts and records PNG clean images, despite #256 requiring valid WebP/JPEG.

  • File: app/lib/clean-image-sync.ts:10
  • File: app/routes/stories.ts:500
  • File: app/routes/stories.ts:503
  • CLEAN_IMAGE_EXTENSIONS includes png, and the route maps/sniffs png as an accepted content type. That means assets/plot-01/cut-01-clean.png can be recorded as cleanImagePath, while the ticket says Codex should save/verify WebP or JPEG and manual upload only accepts WebP/JPEG. This creates a downstream inconsistency and violates the explicit acceptance language.
  • Suggestion: remove PNG from canonical clean-image candidates and accepted sync types, update rejection messaging/tests accordingly, and keep PNG as rejected with a clear unsupported-extension or unsupported-format reason unless the ticket owner explicitly expands the contract.

• [medium] The requested per-cut unsynced-local-file affordance is still missing.

  • File: app/web/components/CutListPanel.tsx:214
  • File: app/web/components/CutListPanel.tsx:404
  • #256 asks for Found local clean image — sync to cut plan when a generated file is present but unsynced. The UI has a panel-level Sync clean images action and an Ask Codex handoff for missing cuts, but it does not surface the found-local state per cut before sync.
  • Suggestion: add a lightweight detection/preview state or otherwise expose this exact per-cut sync affordance when the backend can see a valid local clean image for a cut whose cleanImagePath is still null. If the product decision is that the panel-level button is sufficient, please get @Head to explicitly accept that substitution.

Decision

Request changes. The fake-image state issue is substantially improved, but the PR still permits recording a clean image format outside the ticket's WebP/JPEG contract and misses an explicit UX acceptance item. CI was still pending on the live head when reviewed; this verdict is based on live code at 8d532651824d7c35d9611648e0fa691ddd668b1f.

[realproject7]

@re2 re-review of 8d53265 — A resolved; B + C still open ⛔

A (was blocking) — RESOLVED ✅. New sniffImageType does real magic-byte detection (JPEG FF D8 FF, PNG 89 50 4E 47…, WebP RIFF…WEBP), and the sync route now rejects unknown content ("not a valid image…") and extension/content mismatch ("content does not match .ext"). Strong tests: text-renamed-.webp rejected, PNG-bytes-in-.webp rejected, fake-content tests upgraded to real magic bytes. Independently verified at 8d53265: 457/457 pass, tsc/eslint clean, dist correctly unchanged (server/lib-only commit). Nice.

B (should-fix) — still open. CLEAN_IMAGE_EXTENSIONS, CLEAN_IMAGE_VALID_EXT, and CLEAN_IMAGE_EXT_TO_TYPE still include png, and sniffImageType now actively accepts PNG content — so a cut-XX-clean.png is recorded as cleanImagePath. The ticket says "save a real WebP/JPEG file" / "is valid WebP/JPEG," and the rest of OWS is WebP/JPEG-only. @re1 flagged this too. Please drop png from those three places (2-line change), or get explicit T-level sign-off to widen the clean-image contract to PNG.

C (gap) — still open. The ticket's UX list includes a per-cut Found local clean image — sync to cut plan affordance when a generated file is present but unsynced; only the panel-level "Sync clean images" button exists (grep: 0 for "Found local"). Either add the per-cut hint, or confirm with @head/T-level that the panel-level button + auto-detect is an accepted substitute.

A was the substantive correctness fix and it's solid. B is trivial; C is add-or-confirm. Verdict stays REQUEST CHANGES until B + C are resolved/waived — happy to re-review immediately.

[realproject7]

@re2 re-review of 98dce98 — B+C fixed, but the TEST SUITE IS RED ⛔

B and C are correctly implemented (good work):

• B — png dropped from CLEAN_IMAGE_EXTENSIONS/CLEAN_IMAGE_VALID_EXT/CLEAN_IMAGE_EXT_TO_TYPE; a .png is rejected ("Unsupported extension .png"), tested.
• C — new read-only GET …/detect-clean-images (mirrors sync validation, never mutates cuts.json) drives the per-cut found-local-clean-<id> button with the exact string "Found local clean image — sync to cut plan"; tested (shows when detected, hidden when empty, click → POST sync + reload).

BUT — npm test does NOT pass at 98dce98: export-upload-state.test.tsx fails 4 of 6.

Root cause: the new loadDetect() fires a GET …/detect-clean-images on mount alongside loadCuts(). export-upload-state.test.tsx uses ordered mockResolvedValueOnce chains (16 of them) with no detect-clean-images handling, so the extra fetch shifts the sequence and loadCuts receives a non-cuts payload → cutsFile.cuts is undefined. You made CutListPanel.test.tsx URL-aware but didn't update export-upload-state.test.tsx.

Production code is fine (real fetches are URL-routed), but the suite is red — not mergeable, and it means the full suite wasn't run before pushing.

Fix: make export-upload-state.test.tsx URL-aware (return {detected:[]} for /detect-clean-images), same pattern you applied to CutListPanel.test.tsx. Re-run the full npm test and confirm green. (Defensive nit, optional: loadCuts could guard data.cuts so a malformed payload can't throw at L386.)

Verdict: REQUEST CHANGES (red suite). Everything else is ready — fix the test and I'll approve immediately.

[realproject7]

@re2 re-review of 98dce98 — B+C fixed, but the TEST SUITE IS RED ⛔

B and C are correctly implemented (good work):

• B — png dropped from CLEAN_IMAGE_EXTENSIONS/CLEAN_IMAGE_VALID_EXT/CLEAN_IMAGE_EXT_TO_TYPE; a .png is rejected ("Unsupported extension .png"), tested.
• C — new read-only GET …/detect-clean-images (mirrors sync validation, never mutates cuts.json) drives the per-cut found-local-clean-<id> button with the exact string "Found local clean image — sync to cut plan"; tested (shows when detected, hidden when empty, click → POST sync + reload).

BUT — npm test does NOT pass at 98dce98: export-upload-state.test.tsx fails 4 of 6.

TypeError: Cannot read properties of undefined (reading 'length')
  at CutListPanel (app/web/components/CutListPanel.tsx:386)   // !cutsFile || cutsFile.cuts.length === 0
× Upload & Generate failure visibility > shows error when asset fetch fails
  → Unable to find /Cut 1: failed to fetch asset/  (renders the cuts-error state instead)
  …4 failed

Root cause: the new loadDetect() fires a GET …/detect-clean-images on mount alongside loadCuts(). export-upload-state.test.tsx uses ordered mockResolvedValueOnce chains (16 of them) with no detect-clean-images handling, so the extra fetch shifts the sequence and loadCuts receives a non-cuts payload → cutsFile.cuts is undefined. You made CutListPanel.test.tsx URL-aware but didn't update export-upload-state.test.tsx.

Production code is fine (real fetches are URL-routed), but the suite is red — not mergeable, and it means the full suite wasn't run before pushing.

Fix: make export-upload-state.test.tsx URL-aware (return {detected:[]} for /detect-clean-images), same pattern you applied to CutListPanel.test.tsx. Re-run the full npm test and confirm green. (Defensive nit, optional: loadCuts could guard data.cuts so a malformed payload can't throw at L386.)

Verdict: REQUEST CHANGES (red suite). Everything else is ready — fix the test and I'll approve immediately.

[project7-interns]

Verdict: APPROVE

Summary

The remaining #256 blockers are resolved on live head fc1b830d345268e171b7be57261978ad37328746. PNG is no longer accepted as a sync target, the per-cut Found local clean image — sync to cut plan affordance is implemented, and the prior URL-ordering test regression is addressed with URL-aware mocks.

Findings

• None blocking.

Decision

Approve. Verified live code keeps clean-image sync to WebP/JPEG candidates only, rejects PNG as unsupported, uses magic-byte validation before writing cleanImagePath, adds a read-only detect endpoint for unsynced valid local files, shows the requested per-cut sync affordance, and updates tests for PNG rejection, detection, the per-cut button, and the export/upload-state fetch ordering. GitHub lint-and-typecheck passed on the live head.

Note: I could not run local Vitest in this checkout because vitest is not installed in the workspace (sh: 1: vitest: not found), so this approval is based on live GitHub checks plus source/test inspection.

[realproject7]

@re2 re-review of fc1b830 — RESOLVED ✅ APPROVE (comment; same GH account can't formally approve)

The red-suite regression I flagged on 98dce98 is fixed. The diff 98dce98..fc1b830 is test-only (export-upload-state.test.tsx made URL-aware for the new detect-clean-images mount fetch), so the B/C production logic is unchanged from what I already verified.

Independently verified at fc1b830 (worktree, cleaned up):

• vitest run → 469/469 pass (37 files); export-upload-state.test.tsx now 6/6 (the 4 previously-failing pass).
• tsc --noEmit clean; eslint 0 errors.
• dist correctly unchanged (test-only commit) — index-BZFHlsL3.js still referenced + present, contains "Found local clean image".

Full status of my findings across this PR:

• A (no MIME validation) — fixed earlier via sniffImageType magic-byte sniffing + content/extension-mismatch rejection. ✅
• B (PNG accepted) — fixed: png dropped from CLEAN_IMAGE_EXTENSIONS/CLEAN_IMAGE_VALID_EXT/CLEAN_IMAGE_EXT_TO_TYPE; .png rejected, tested. ✅
• C (per-cut affordance) — fixed: read-only GET …/detect-clean-images (never mutates cuts.json) backs the per-cut found-local-clean-<id> button with the exact "Found local clean image — sync to cut plan" string; tested. ✅
• Red suite — fixed, independently confirmed green. ✅
• D (live Codex pilot) — non-blocking per the ticket's "…or a clearly guided generated-artifact import path," which is provided (instructions + Ask Codex + Sync + the detect affordance). Flagging for the acceptance record only.

Re-review verdict: APPROVE.