Merge pull request #2244 from atomantic/claim/issue-2241

refactor: wrap comic page-render + refine-render as creative tools (route-level persistence) (#2241)

Author: atomantic

.changelog/NEXT.md

@@ -118,6 +118,8 @@
 
 - **[issue-2220] Creative render tools with route-level persistence (CDO Phase 2 slice).** The Creative Director orchestrator can now render comic-issue and volume (season) covers — `pipeline_renderComicCover`, `pipeline_renderComicBackCover`, `pipeline_renderVolumeCover`, `pipeline_renderVolumeBackCover` are registered creative tools. These render jobs need more than a bare enqueue: the media-job filename hook attaches the finished image only if the target cover slot already carries the returned `jobId`, and that slot write used to live in the route factory (`routes/pipeline/covers.js#makeCoverRenderHandler`), so wrapping the bare `enqueueComicCover`-style service in a tool would silently drop orchestrated covers. The enqueue+persist flow (build the in-flight render slot, script-gate, write it onto the record) now lives in shared service entry points — `renderComicCover` / `renderComicBackCover` (→ `stages.comicPages.{cover|backCover}` via the serialized `updateStageWithLatest` series write tail) and `renderVolumeCover` / `renderVolumeBackCover` (→ `series.seasons[].{cover|backCover}` via `updateSeasonOnSeries`) in `visualStages.js` — so the four cover-render routes and the four orchestrator tools share ONE code path. The persist keeps the script-gate semantics (absent `coverScript` preserves the persisted concept, empty string clears it) and serializes against a concurrent blur-save on the same record. Tools charge as `render` against the daily action budget. Cover render is the primary deliverable; wrapping the remaining route-persisted render actions (comic page render / refine-render) as tools stays tracked on #2220.
 
+- **[issue-2241] Creative comic-page render + refine-render tools with route-level persistence (CDO Phase 2 slice).** Completes the #2220 audit clause the covers slice (#2234) deferred: the Creative Director orchestrator can now render and refine full comic pages — `pipeline_renderComicPage` and `pipeline_refineComicPageRender` are registered creative tools. Like the covers, these need more than a bare enqueue — the media-job filename hook attaches the finished image only if `stages.comicPages.pages[pageIndex]`'s active variant slot already carries the returned `jobId`, and that slot write used to live in the route handler (`routes/pipeline/issues.js`), so wrapping the bare `enqueueVisualComicPage` / `refineComicPageRender` in a tool would silently drop orchestrated pages. The enqueue+persist flow now lives in shared service entry points — `renderComicPage` and `refineComicPageRender` (both persist onto `pages[pageIndex].{proofImage|finalImage}` through a shared `persistComicPageSlot` helper over the serialized `updateStageWithLatest` issue write tail) in `visualStages.js` — so the page render + refine-render routes and the two orchestrator tools share ONE code path, mirroring the `renderComicCover` / `makeCoverRenderHandler` structure exactly. The persist runs inside the write tail's compute step so a concurrent page edit or sibling render can't be reverted by a stale snapshot. Tools charge as `render` against the daily action budget. (`server/services/pipeline/visualStages.js`, `server/routes/pipeline/issues.js`, `server/services/creative/tools/pipeline.js`)
+
 - **`getSettings()` now serves from an in-memory cache instead of re-reading settings.json on every call.** `getSettings()` is invoked 100+ times across the codebase (several per request), each previously doing a fresh filesystem read + JSON.parse of app-wide, rarely-changing data. It now memoizes the parsed settings, populated lazily on first read and refreshed off the `settings:updated` event that every `save()`/`updateSettings()`/`updateSettingsWith()` already emits — so app-driven writes keep the cache coherent while write paths still read fresh off disk inside their write queue for the read-merge-write, preserving write-ordering guarantees. Mirrors the existing `settings:updated`-invalidated cache in `sharing/annotationIdentity.js`. Alongside it, a few independent per-file read loops are now parallelized (`digital-twin-analysis.validateCompleteness`, editorial series-health's series+settings load) and hot page derivations memoized (`Review` grouping/sorting, `Tribe` care-queue), cutting redundant work on socket-driven re-renders.
 
 - **Branch & PR reconciliation now runs on any managed app as a Chief-of-Staff scheduled task — not just PortOS.** The branch reconciler (finish this machine's unfinished local git work: delete fully-merged orphaned branches + their worktrees, open PRs for pushed-but-unopened work, resolve merge conflicts, drive the review loop, and auto-merge when a PR is green) used to be a PortOS-only feature with its own daily cron, `/api/branch-reconcile` route, `settings.branchReconcile` toggle, and a dedicated Settings tab — hardwired to the PortOS checkout. It is now a first-class per-app CoS task type, **`branch-reconcile`**, configured like every other improvement task (per-app enable, provider/model, cadence, cooldown, budget) under Chief of Staff → task scheduling. It runs `perpetual` (drain-until-done): each dispatch runs the deterministic, peer-safe cleanup + classification against the *app's* repo, dispatches the coordinator agent only while actionable in-flight branches remain, then parks on a daily recheck — so no agent is burned on a no-op run. A progress-signature convergence guard ensures the drain only re-dispatches when the actionable set actually *changed* since the last run (a branch advanced NEEDS_PR→IN_REVIEW→merged, or a new/removed branch): a cycle that leaves the same branches in the same states (a branch judged "not ready", a PR blocked on human review or red CI) parks instead of re-spawning an identical coordinator back-to-back. The peer-safety guarantee is unchanged (only local `refs/heads/` are ever touched; a federated peer's branch exists here only as a remote-tracking ref and is structurally invisible), and the per-behavior toggles (`cleanupMerged` / `openPr` / `resolveConflicts` / `autoMerge`) are now per-app task-metadata overrides. Off by default; enabling it on an app is the user's explicit consent to let it drive PRs on a schedule. The old dedicated scheduler, route, settings key, and Settings tab are removed; a migration (`scripts/migrations/170-branch-reconcile-to-cos-task.js`) carries a previously-enabled `branchReconcile` opt-in into the new task (cron→recheckCron, actions→per-app metadata) scoped to the PortOS app, and drops the dead settings key — so an install that had it on keeps reconciling instead of silently stopping. (`server/services/branchReconcile.js`, `server/services/cosTaskGenerator.js`, `server/services/taskSchedule.js`, `server/services/taskPromptDefaults/prompts.js`)

server/routes/pipeline.test.js

@@ -107,6 +107,48 @@ vi.mock('../services/pipeline/visualStages.js', async () => {
     };
   };
 
+  // Full-page render+persist. Enqueues then lands the in-flight slot on
+  // stages.comicPages.pages[pageIndex] through the real issue write tail (#2241)
+  // so the route tests exercise the persist end-to-end (mirrors renderComicPage).
+  const persistComicPage = async (issueId, options) => {
+    const pageIndex = options.pageIndex;
+    const variant = options?.target === 'final' ? 'final' : 'proof';
+    const jobId = `page-job-${++uuidCounter}`;
+    const prompt = `comic-page prompt for page ${pageIndex + 1}`;
+    const fromProof = variant === 'final' && options?.useProofAsBase === true;
+    const slotKey = slotKeyForVariant(variant);
+    const slotRecord = buildSlot({ slotKey, jobId, prompt, width: options?.width, height: options?.height, fromProof });
+    const { issue, stage } = await issuesSvc.updateStageWithLatest(issueId, 'comicPages', (current) => {
+      const pages = Array.isArray(current?.pages) ? current.pages : [];
+      const next = [...pages];
+      next[pageIndex] = { ...pages[pageIndex], [slotKey]: slotRecord };
+      return { status: 'edited', pages: next };
+    });
+    return { jobId, mode: 'local', prompt, pageIndex, variant, fromProof, issue, stage };
+  };
+
+  // Comic-page AI refine + i2i re-render (issue #1534) + persist (#2241). Lands
+  // the refined render on the matching variant slot via the real write tail.
+  const persistComicPageRefine = async (issueId, options) => {
+    const pageIndex = options.pageIndex;
+    const variant = options?.target === 'final' ? 'final' : 'proof';
+    const jobId = `page-refine-job-${++uuidCounter}`;
+    const prompt = `refined page prompt for page ${pageIndex + 1}`;
+    const slotKey = slotKeyForVariant(variant);
+    const slotRecord = buildSlot({ slotKey, jobId, prompt, width: options?.width, height: options?.height });
+    const { issue, stage } = await issuesSvc.updateStageWithLatest(issueId, 'comicPages', (current) => {
+      const pages = Array.isArray(current?.pages) ? current.pages : [];
+      const next = [...pages];
+      next[pageIndex] = { ...pages[pageIndex], [slotKey]: slotRecord };
+      return { status: 'edited', pages: next };
+    });
+    return {
+      jobId, mode: 'local', prompt, pageIndex, variant,
+      changes: ['warmed the lighting'], runId: 'run-mock-page-refine', providerId: 'mock-provider',
+      issue, stage,
+    };
+  };
+
   const persistVolumeCover = async (seriesId, seasonId, target, scriptField, body) => {
     const variant = body?.target === 'final' ? 'final' : 'proof';
     const jobId = `vol-${target === 'cover' ? 'cover' : 'backcover'}-job-${++uuidCounter}`;
@@ -134,17 +176,10 @@ vi.mock('../services/pipeline/visualStages.js', async () => {
     mode: 'local',
     prompt: `style, ${opts.description}`,
   })),
-  enqueueVisualComicPage: vi.fn(async (_issueId, opts) => ({
-    jobId: `page-job-${++uuidCounter}`,
-    mode: 'local',
-    // Match the real service contract: pageNumber is 1-based (pageIndex + 1).
-    prompt: `comic-page prompt for page ${opts.pageIndex + 1}`,
-    pageIndex: opts.pageIndex,
-    // Forward proof/final variant + i2i flag so the route's slotKey + slot
-    // record reflect the schema-validated body (target defaults to 'proof').
-    variant: opts?.target === 'final' ? 'final' : 'proof',
-    fromProof: opts?.target === 'final' && opts?.useProofAsBase === true,
-  })),
+  // Full-page render+persist entry point (#2241) — enqueues AND lands the
+  // in-flight slot on stages.comicPages.pages[pageIndex] through the real issue
+  // write tail, driven through persistComicPage above.
+  renderComicPage: vi.fn((issueId, opts) => persistComicPage(issueId, opts)),
   // Front cover render+persist. Lands the in-flight slot on
   // stages.comicPages.cover through the real issue write tail (#2220).
   renderComicCover: vi.fn((issueId, body) => persistComicCover(issueId, 'cover', 'coverScript', body || {})),
@@ -201,19 +236,10 @@ vi.mock('../services/pipeline/visualStages.js', async () => {
     changes: ['mock change'],
     providerId: 'mock-provider',
   })),
-  // Comic-page AI refine + i2i re-render (issue #1534). Returns the shape the
-  // route lands on the matching variant slot via slotKeyForVariant +
-  // buildRenderSlot — variant defaults to 'proof' unless the body forces one.
-  refineComicPageRender: vi.fn(async (_issueId, opts) => ({
-    jobId: `page-refine-job-${++uuidCounter}`,
-    mode: 'local',
-    prompt: `refined page prompt for page ${opts.pageIndex + 1}`,
-    pageIndex: opts.pageIndex,
-    variant: opts?.target === 'final' ? 'final' : 'proof',
-    changes: ['warmed the lighting'],
-    runId: 'run-mock-page-refine',
-    providerId: 'mock-provider',
-  })),
+  // Comic-page AI refine + i2i re-render (issue #1534) + persist (#2241).
+  // Enqueues AND lands the refined render on the matching variant slot through
+  // the real write tail — variant defaults to 'proof' unless the body forces one.
+  refineComicPageRender: vi.fn((issueId, opts) => persistComicPageRefine(issueId, opts)),
   };
 });
 
@@ -1696,8 +1722,9 @@ describe('pipeline routes', () => {
       .send({});
     expect(r.status).toBe(404);
     expect(r.body.code || r.body.error).toMatch(/PIPELINE_COMIC_PAGE_NOT_FOUND|out of range/i);
-    // Enqueue is never reached when the page doesn't exist.
-    expect(visualStages.enqueueVisualComicPage).not.toHaveBeenCalled();
+    // The render+persist service is never reached when the page doesn't exist
+    // (the route pre-validates the page index before dispatching).
+    expect(visualStages.renderComicPage).not.toHaveBeenCalled();
   });
 
   // ---- comicPages/cover/render ----

server/routes/pipeline/issues.js

@@ -21,15 +21,14 @@ import { generateStage } from '../../services/pipeline/textStages.js';
 import * as autoRunner from '../../services/pipeline/autoRunner.js';
 import {
   enqueueVisualImage,
-  enqueueVisualComicPage,
+  renderComicPage,
   enqueueStoryboardSceneVideo,
   enqueueStoryboardShotStartFrame,
   refineComicPanelPrompt,
   refineComicPageRender,
   refineStoryboardScenePrompt,
   generateComicPanelImagePrompts,
   generateStoryboardSceneImagePrompts,
-  buildRenderSlot,
 } from '../../services/pipeline/visualStages.js';
 import { IMAGE_PROMPT_CANDIDATE_MAX } from '../../services/pipeline/refineHelpers.js';
 import {
@@ -38,7 +37,7 @@ import {
 } from '../../services/universeCanon.js';
 import { getSeriesCanon } from '../../services/pipeline/seriesCanon.js';
 import { startEpisodeVideoForIssue } from '../../services/pipeline/episodeVideo.js';
-import { COMIC_PAGE_VARIANTS, slotKeyForVariant } from '../../services/pipeline/owners.js';
+import { COMIC_PAGE_VARIANTS } from '../../services/pipeline/owners.js';
 import { ASPECT_RATIOS, QUALITIES } from '../../lib/creativeDirectorPresets.js';
 import { IMAGE_GEN_MODE } from '../../services/imageGen/modes.js';
 import { extractScenes, scenesFromBeatSheet, SOURCE_KIND } from '../../lib/sceneExtractor.js';
@@ -878,38 +877,16 @@ router.post('/issues/:id/stages/comicPages/pages/:pageIndex/render', asyncHandle
     );
   }
 
-  const result = await enqueueVisualComicPage(req.params.id, { pageIndex, ...body })
+  // renderComicPage enqueues AND persists the in-flight slot onto
+  // pages[pageIndex].{proofImage|finalImage} through the serialized issue write
+  // tail (#2241) — the same shared entry point the CDO orchestrator tool calls,
+  // so an orchestrated page completes exactly like a user-driven one. The
+  // persist runs inside updateStageWithLatest's computeFn so the slot lands on
+  // the freshest persisted pages array (a concurrent page edit or sibling
+  // render can't be reverted by a stale snapshot).
+  const result = await renderComicPage(req.params.id, { pageIndex, ...body })
     .catch((err) => { throw mapServiceError(err); });
-
-  // The splice happens inside updateStageWithLatest's computeFn so the
-  // slot lands on the freshest persisted pages array — a concurrent page
-  // edit or sibling render that wrote between our enqueue and persist
-  // would otherwise be reverted by a stale snapshot.
-  const slotKey = slotKeyForVariant(result.variant);
-  const slotRecord = buildRenderSlot({
-    slotKey, jobId: result.jobId, prompt: result.prompt,
-    width: body.width, height: body.height, fromProof: result.fromProof,
-  });
-  const { issue: updatedIssue, stage } = await issuesSvc.updateStageWithLatest(
-    req.params.id,
-    'comicPages',
-    (currentStage) => {
-      const currentPages = Array.isArray(currentStage?.pages) ? currentStage.pages : [];
-      if (!currentPages[pageIndex]) {
-        throw new ServerError(
-          `pageIndex ${pageIndex} out of range — comicPages has ${currentPages.length} page${currentPages.length === 1 ? '' : 's'}`,
-          { status: 404, code: 'PIPELINE_COMIC_PAGE_NOT_FOUND' },
-        );
-      }
-      const nextPages = [...currentPages];
-      nextPages[pageIndex] = {
-        ...currentPages[pageIndex],
-        [slotKey]: slotRecord,
-      };
-      return { status: 'edited', pages: nextPages };
-    },
-  ).catch((err) => { throw mapServiceError(err); });
-  res.json({ ...result, issue: updatedIssue, stage });
+  res.json(result);
 }));
 
 // AI prompt-refine + image-to-image re-render for a small correction to an
@@ -939,34 +916,14 @@ router.post('/issues/:id/stages/comicPages/pages/:pageIndex/refine-render', asyn
     );
   }
 
+  // refineComicPageRender adjusts the page's stored render prompt (LLM),
+  // re-renders i2i from the page's existing image, AND persists the new slot on
+  // the matching variant through the serialized issue write tail (#2241) — same
+  // shared code path the CDO orchestrator tool uses. A concurrent edit or
+  // sibling render can't be reverted by a stale snapshot.
   const result = await refineComicPageRender(req.params.id, { pageIndex, ...body })
     .catch((err) => { throw mapServiceError(err); });
-
-  const slotKey = slotKeyForVariant(result.variant);
-  const slotRecord = buildRenderSlot({
-    slotKey, jobId: result.jobId, prompt: result.prompt,
-    width: body.width, height: body.height,
-  });
-  const { issue: updatedIssue, stage } = await issuesSvc.updateStageWithLatest(
-    req.params.id,
-    'comicPages',
-    (currentStage) => {
-      const currentPages = Array.isArray(currentStage?.pages) ? currentStage.pages : [];
-      if (!currentPages[pageIndex]) {
-        throw new ServerError(
-          `pageIndex ${pageIndex} out of range — comicPages has ${currentPages.length} page${currentPages.length === 1 ? '' : 's'}`,
-          { status: 404, code: 'PIPELINE_COMIC_PAGE_NOT_FOUND' },
-        );
-      }
-      const nextPages = [...currentPages];
-      nextPages[pageIndex] = {
-        ...currentPages[pageIndex],
-        [slotKey]: slotRecord,
-      };
-      return { status: 'edited', pages: nextPages };
-    },
-  ).catch((err) => { throw mapServiceError(err); });
-  res.json({ ...result, issue: updatedIssue, stage });
+  res.json(result);
 }));
 
 // AI-driven prompt refinement for a single comic panel. Uses