break the 22-module seriesAutopilot/creativeDirector import cycle with a project-start sink

server/index.js

@@ -122,6 +122,12 @@ import videoTimelineRoutes from './routes/videoTimeline.js';
 import mediaJobsRoutes from './routes/mediaJobs.js';
 import federatedMediaRoutes from './routes/federatedMedia.js';
 import creativeDirectorRoutes from './routes/creativeDirector.js';
+// Side-effect import (#5920): evaluating the completion hook is what registers the
+// Creative Director project starter on `creativeDirector/projectStartSink.js`, the
+// seam `pipeline/episodeVideo.js` starts a CD project through. The route module
+// above already pulls it in, but the pipeline must not depend on THAT staying true —
+// an unregistered sink throws, so arm it explicitly at boot.
+import './services/creativeDirector/completionHook.js';
 import creativeCommissionRoutes from './routes/creativeCommissions.js';
 import gamesRoutes from './routes/games.js';
 import fableLoomRoutes from './routes/fableLoom.js';

server/services/creative/toolRegistry.test.js

@@ -60,7 +60,7 @@ vi.mock('../writersRoom/evaluator.js', () => ({ runAnalysis: vi.fn(async () => (
 vi.mock('../pipeline/series.js', () => ({ createSeries: vi.fn(async () => ({ id: 'ser1' })) }));
 vi.mock('../pipeline/seriesGenerate.js', () => ({ generateSeriesConcept: vi.fn(async () => ({ name: 'C' })) }));
 vi.mock('../pipeline/textStages.js', () => ({ generateStage: vi.fn(async () => ({ stage: 'st' })) }));
-vi.mock('../pipeline/seriesAutopilot.js', () => ({ startSeriesAutopilot: vi.fn(async () => ({ runId: 'ap1' })) }));
+vi.mock('../pipeline/seriesAutopilot/orchestrator.js', () => ({ startSeriesAutopilot: vi.fn(async () => ({ runId: 'ap1' })) }));
 vi.mock('../pipeline/visualStages.js', () => ({
   renderComicCover: vi.fn(async () => ({ jobId: 'cov1', variant: 'proof' })),
   renderComicBackCover: vi.fn(async () => ({ jobId: 'bcov1', variant: 'proof' })),
@@ -82,7 +82,7 @@ import { dispatchTool as dispatchVoiceTool } from '../voice/tools.js';
 import { getDomainBudgetStatus, recordDomainUsage } from '../domainUsage.js';
 import { createSeries } from '../pipeline/series.js';
 import { generateStage } from '../pipeline/textStages.js';
-import { startSeriesAutopilot } from '../pipeline/seriesAutopilot.js';
+import { startSeriesAutopilot } from '../pipeline/seriesAutopilot/orchestrator.js';
 import { renderComicCover, renderVolumeCover, renderComicPage, refineComicPageRender } from '../pipeline/visualStages.js';
 import { enqueueJob } from '../mediaJobQueue/index.js';
 import { getProject } from '../creativeDirector/local.js';

server/services/creative/tools/pipeline.js

@@ -8,7 +8,7 @@ import { z } from 'zod';
 import { createSeries } from '../../pipeline/series.js';
 import { generateSeriesConcept } from '../../pipeline/seriesGenerate.js';
 import { generateStage } from '../../pipeline/textStages.js';
-import { startSeriesAutopilot } from '../../pipeline/seriesAutopilot.js';
+import { startSeriesAutopilot } from '../../pipeline/seriesAutopilot/orchestrator.js';
 import {
   renderComicCover,
   renderComicBackCover,

server/services/creativeDirector/completionHook.js

@@ -27,6 +27,7 @@ import { join } from 'path';
 import { getProject, updateProject, updateScene, updateRun, recordRun } from './local.js';
 import { enqueueTreatmentTask } from './agentBridge.js';
 import { advanceAfterPlanStepSettled } from './planAdvance.js';
+import { registerCreativeDirectorProjectStarter } from './projectStartSink.js';
 import { dispatchSceneEvaluation } from './sceneEvaluator.js';
 import { runSceneRender } from './sceneRunner.js';
 import { runStitch } from './stitchRunner.js';
@@ -612,6 +613,13 @@ export async function startCreativeDirectorProject(projectId) {
   return advanceAfterSceneSettled(projectId);
 }
 
+// Wire the pipeline-facing seam (#5920). `pipeline/episodeVideo.js` starts the CD
+// project it just built through `projectStartSink.js` rather than importing this
+// module, which is what keeps the two halves out of one import cycle. Registering
+// here (rather than at a call site) means every importer of the completion hook
+// arms the seam — see the sink's header for why boot imports this module.
+registerCreativeDirectorProjectStarter(startCreativeDirectorProject);
+
 // Test-only: clear the module-level in-memory dedup sets so suites that leave
 // a seed-frame defer armed (deferred but never fired the settle event) don't
 // bleed the deferKey into a later test that reuses the same projectId.

server/services/creativeDirector/completionHook.test.js

@@ -41,7 +41,23 @@ vi.mock('../mediaJobQueue/index.js', () => ({
   mediaJobEvents: { on: vi.fn(), off: vi.fn() },
 }));
 
-const { handleCreativeDirectorCompletion, advanceAfterSceneSettled, __resetInflightState } = await import('./completionHook.js');
+const { handleCreativeDirectorCompletion, advanceAfterSceneSettled, startCreativeDirectorProject, __resetInflightState } = await import('./completionHook.js');
+
+import { hasCreativeDirectorProjectStarter, registerCreativeDirectorProjectStarter } from './projectStartSink.js';
+
+// #5920: `pipeline/episodeVideo.js` starts a CD project through the sink instead of
+// importing this module, so importing this module MUST arm the sink. Nothing else
+// registers a starter — if this side effect is ever dropped, every pipeline episode
+// silently stops advancing, and only this assertion notices.
+describe('registers the pipeline-facing project starter (#5920)', () => {
+  it('arms the project-start sink on import', () => {
+    expect(hasCreativeDirectorProjectStarter()).toBe(true);
+    // And it is THIS module's starter, not some other registrant: re-registering the
+    // same function is the sink's one idempotent case, so a no-throw here is an
+    // identity check on what is wired.
+    expect(() => registerCreativeDirectorProjectStarter(startCreativeDirectorProject)).not.toThrow();
+  });
+});
 
 const planTask = (runId = 'run-1') => ({
   id: 'task-1',

server/services/creativeDirector/planAdvance.js

@@ -27,7 +27,11 @@ import { getProject, updateProject, updatePlanStep, recordRun, updateRun } from
 import { enqueuePlanTask } from './agentBridge.js';
 import { dispatchCreativeTool } from '../creative/toolRegistry.js';
 import { listJobs, mediaJobEvents } from '../mediaJobQueue/index.js';
-import { autopilotEvents, isAutopilotActive, AUTOPILOT_TERMINAL_TYPES } from '../pipeline/seriesAutopilot.js';
+// Import the modules that DECLARE these, not the `seriesAutopilot.js` barrel that
+// forwards them (#5920): a barrel re-entry from outside the package pulls the whole
+// autopilot cluster in and is what closed the old import cycle through this file.
+import { autopilotEvents, AUTOPILOT_TERMINAL_TYPES } from '../pipeline/seriesAutopilot/state.js';
+import { isAutopilotActive } from '../pipeline/seriesAutopilot/session.js';
 import { getSeries } from '../pipeline/series.js';
 import { MAX_REPLAN_ROUNDS, PLAN_STEP_TERMINAL_SUCCESS } from '../../lib/creativeDirectorPresets.js';
 import { blockedStageReason, closeDeliverableStreak, exhaustedDeliverableStreak } from './deliverableGate.js';

server/services/creativeDirector/planAdvance.test.js

@@ -222,11 +222,13 @@ vi.mock('../mediaJobQueue/index.js', () => ({
   listJobs: (...a) => mockListJobs(...a),
   mediaJobEvents: { on: vi.fn(), off: vi.fn() },
 }));
-vi.mock('../pipeline/seriesAutopilot.js', () => ({
+vi.mock('../pipeline/seriesAutopilot/state.js', () => ({
   autopilotEvents: ap.autopilotEvents,
-  isAutopilotActive: () => ap.ctl.active,
   AUTOPILOT_TERMINAL_TYPES: ap.AUTOPILOT_TERMINAL_TYPES,
 }));
+vi.mock('../pipeline/seriesAutopilot/session.js', () => ({
+  isAutopilotActive: () => ap.ctl.active,
+}));
 vi.mock('../pipeline/series.js', () => ({
   getSeries: async () => ap.ctl.marker && { autopilot: ap.ctl.marker },
 }));

server/services/creativeDirector/projectStartSink.js

@@ -0,0 +1,71 @@
+/**
+ * Creative Director — project-start sink (#5920).
+ *
+ * The one seam that keeps `server/services/pipeline/*` and
+ * `server/services/creativeDirector/*` out of a shared import cycle.
+ *
+ * The two halves are genuinely bidirectional at RUNTIME: the Creative Director's
+ * tool registry can start a Series Autopilot run, and the pipeline's episode-video
+ * step can start a CD project. Expressed as two static imports that is a 22-module
+ * strongly-connected component (see `serviceImportCycles.test.js`), and in a static
+ * ESM cycle whichever member evaluates first sees `undefined` for the others'
+ * bindings — a boot-time TDZ crash waiting on a module-ordering change.
+ *
+ * Only ONE edge ran pipeline → creativeDirector: `pipeline/episodeVideo.js`
+ * importing `completionHook.js#startCreativeDirectorProject`. This module inverts
+ * it. The pipeline side depends on this leaf (it imports nothing), and the CD side
+ * REGISTERS its starter here at module evaluation, so the arrow now points
+ * creativeDirector → pipeline in both directions and the component dissolves.
+ *
+ * Deferring the import with `await import()` would have hidden the cycle from the
+ * guard without removing the boot-order hazard, so it is deliberately not the fix.
+ *
+ * **Registration is a hard requirement, not best-effort.** `completionHook.js`
+ * registers on import, and `server/index.js` imports it for that side effect at
+ * boot (routes/creativeDirector.js also pulls it in, but boot order is not a
+ * contract to lean on). An unregistered request THROWS rather than silently
+ * dropping the start — a CD project that never advances is invisible, and the
+ * caller logs the throw.
+ */
+
+let startProject = null;
+
+/**
+ * Wire the concrete starter. Idempotent: re-registering the same function is a
+ * no-op (module re-evaluation under a test suite), and registering a DIFFERENT
+ * one throws rather than letting a second registrant silently win.
+ *
+ * @param {(projectId: string) => Promise<unknown>} fn
+ */
+export function registerCreativeDirectorProjectStarter(fn) {
+  if (typeof fn !== 'function') {
+    throw new Error('registerCreativeDirectorProjectStarter: starter must be a function');
+  }
+  if (startProject && startProject !== fn) {
+    throw new Error('registerCreativeDirectorProjectStarter: a different starter is already registered');
+  }
+  startProject = fn;
+}
+
+/** Whether a starter has been wired. Exported for the boot-time guard and tests. */
+export function hasCreativeDirectorProjectStarter() {
+  return startProject !== null;
+}
+
+/**
+ * Start a Creative Director project through the registered starter.
+ *
+ * @param {string} projectId
+ * @returns {Promise<unknown>}
+ */
+export async function requestCreativeDirectorProjectStart(projectId) {
+  if (!startProject) {
+    throw new Error(`Creative Director project start requested before a starter was registered (project ${projectId})`);
+  }
+  return startProject(projectId);
+}
+
+/** Test-only: drop the registration so a suite can assert the unwired behavior. */
+export function __resetCreativeDirectorProjectStarter() {
+  startProject = null;
+}

server/services/creativeDirector/projectStartSink.test.js

@@ -0,0 +1,42 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import {
+  registerCreativeDirectorProjectStarter,
+  hasCreativeDirectorProjectStarter,
+  requestCreativeDirectorProjectStart,
+  __resetCreativeDirectorProjectStarter,
+} from './projectStartSink.js';
+
+// The sink is the seam that keeps `pipeline/*` and `creativeDirector/*` out of a
+// shared static import cycle (#5920). Its whole contract is "the CD side wires a
+// starter, the pipeline side calls it, and an UNWIRED sink is loud" — a silent
+// no-op there would strand a pipeline episode's CD project in `pending` with
+// nothing in the logs.
+describe('creativeDirector/projectStartSink (#5920)', () => {
+  beforeEach(() => __resetCreativeDirectorProjectStarter());
+
+  it('rejects a start request before a starter is registered', async () => {
+    expect(hasCreativeDirectorProjectStarter()).toBe(false);
+    await expect(requestCreativeDirectorProjectStart('proj-1')).rejects.toThrow(/proj-1/);
+  });
+
+  it('delegates to the registered starter and returns its result', async () => {
+    const starter = vi.fn(async (id) => `started:${id}`);
+    registerCreativeDirectorProjectStarter(starter);
+
+    expect(hasCreativeDirectorProjectStarter()).toBe(true);
+    await expect(requestCreativeDirectorProjectStart('proj-2')).resolves.toBe('started:proj-2');
+    expect(starter).toHaveBeenCalledWith('proj-2');
+  });
+
+  it('is idempotent for the same starter but rejects a second, different one', () => {
+    const starter = vi.fn();
+    registerCreativeDirectorProjectStarter(starter);
+    expect(() => registerCreativeDirectorProjectStarter(starter)).not.toThrow();
+    expect(() => registerCreativeDirectorProjectStarter(vi.fn())).toThrow(/already registered/);
+  });
+
+  it('refuses a non-function registrant', () => {
+    expect(() => registerCreativeDirectorProjectStarter(null)).toThrow(/must be a function/);
+    expect(() => registerCreativeDirectorProjectStarter('nope')).toThrow(/must be a function/);
+  });
+});

server/services/pipeline/episodeVideo.js

@@ -5,9 +5,14 @@
  * stitch loop, but reuses the Creative Director machinery instead of
  * duplicating it. We create a CD project with `autoAcceptScenes: true` so
  * no LLM evaluator round-trip runs (the Pipeline already had the human
- * vetting the storyboard scenes), then call into CD's existing
- * `advanceAfterSceneSettled` to kick off the first render. CD's own
- * sceneRunner / completionHook / stitchRunner take it from there.
+ * vetting the storyboard scenes), then ask CD to start the project. CD's own
+ * completionHook / sceneRunner / stitchRunner take it from there.
+ *
+ * That start goes through `creativeDirector/projectStartSink.js` rather than a
+ * direct import of the completion hook (#5920): a static edge pipeline → CD closed
+ * a 22-module import cycle, because CD's tool registry starts a Series Autopilot in
+ * the other direction. The sink is a leaf both halves can depend on; the hook
+ * registers the concrete starter on it.
  *
  * The CD project id is persisted on the issue's `stages.episodeVideo` so
  * the UI can poll `/api/creative-director/:id` to render progress and
@@ -18,7 +23,7 @@ import { getIssue, updateStage, assertStageUnlocked } from './issues.js';
 import { getSeries } from './series.js';
 import { getSeriesCanon } from './seriesCanon.js';
 import { createProject as createCDProject, setTreatment as setCDTreatment } from '../creativeDirector/local.js';
-import { startCreativeDirectorProject } from '../creativeDirector/completionHook.js';
+import { requestCreativeDirectorProjectStart } from '../creativeDirector/projectStartSink.js';
 import { getDefaultVideoModelId, getVideoModels } from '../../lib/mediaModels.js';
 import { buildPlaceByKey } from '../../lib/scenePrompt.js';
 import { getSettings } from '../settings.js';
@@ -232,7 +237,7 @@ export async function startEpisodeVideoForIssue(issueId, options = {}) {
   // Kick off the orchestrator — fire-and-forget so the route can return
   // immediately. Failures land on the CD project's `failureReason` field
   // and surface via the UI's CD project poll, not via this Promise.
-  startCreativeDirectorProject(project.id).catch((err) =>
+  requestCreativeDirectorProjectStart(project.id).catch((err) =>
     console.log(`⚠️ Pipeline episode CD start failed for ${project.id}: ${err.message}`),
   );
 

server/services/pipeline/episodeVideo.test.js

@@ -1,5 +1,6 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { mockNoPeerSync, mockNoPeers } from '../../lib/mockPathsDataRoot.js';
+import { requestCreativeDirectorProjectStart } from '../creativeDirector/projectStartSink.js';
 
 const fileStore = new Map();
 
@@ -35,8 +36,11 @@ vi.mock('../creativeDirector/local.js', () => ({
   }),
 }));
 
-vi.mock('../creativeDirector/completionHook.js', () => ({
-  startCreativeDirectorProject: vi.fn(async () => undefined),
+// #5920 — the CD project is started through the sink, not a direct import of
+// completionHook.js, which is what keeps pipeline/* and creativeDirector/* out of
+// one static import cycle.
+vi.mock('../creativeDirector/projectStartSink.js', () => ({
+  requestCreativeDirectorProjectStart: vi.fn(async () => undefined),
 }));
 
 let mockVideoModels = [
@@ -279,6 +283,9 @@ describe('pipeline episodeVideo helper', () => {
     // restore the picker state — defaults applied since no overrides given.
     expect(refreshed.stages.episodeVideo.aspectRatio).toBe('16:9');
     expect(refreshed.stages.episodeVideo.quality).toBe('standard');
+    // The stage is only useful if the CD project actually starts advancing; the
+    // start is fire-and-forget, so nothing downstream would notice it going missing.
+    expect(requestCreativeDirectorProjectStart).toHaveBeenCalledWith(result.cdProjectId);
   });
 
   it('startEpisodeVideoForIssue persists user-overridden aspectRatio + quality on the stage', async () => {

server/services/pipeline/seriesAutopilot.js

@@ -97,14 +97,17 @@ export * from './seriesAutopilot/orchestrator.js';
 // Export internals for tests. Pulled back together from their new home modules
 // so the existing `__testing` import contract survives the #2842 split.
 //
-// The properties MUST be lazy getters, not eagerly-read values. `session.js`
-// reaches this barrel through a real import cycle
-// (session → autoRunner → episodeVideo → completionHook → planAdvance →
-// seriesAutopilot), so evaluating `providerOverrideOpts` at module-evaluation
-// time throws a TDZ ReferenceError on any cold import of a focused module. The
-// getters defer each read until a test actually touches the key, by which point
-// every binding is initialized. Behavior for callers is unchanged — `__testing`
-// is still a plain object whose keys resolve to the same functions.
+// The properties are lazy getters, not eagerly-read values. They were forced by a
+// real import cycle (session → autoRunner → episodeVideo → completionHook →
+// planAdvance → seriesAutopilot), which made evaluating `providerOverrideOpts` at
+// module-evaluation time a TDZ ReferenceError on any cold import of a focused
+// module. #5920 removed that cycle — `episodeVideo.js` now starts a Creative
+// Director project through `creativeDirector/projectStartSink.js` instead of
+// importing the completion hook — but the getters stay: they cost nothing, they
+// keep the cold-import guard in `seriesAutopilotColdImport.test.js` honest, and a
+// future edge must not silently reintroduce the crash. Behavior for callers is
+// unchanged — `__testing` is still a plain object whose keys resolve to the same
+// functions.
 import * as state from './seriesAutopilot/state.js';
 import * as convergence from './seriesAutopilot/convergence.js';
 import * as session from './seriesAutopilot/session.js';