fix(notion-fetch): improve terminal summary validation and child page resolution

Author: luandro

api-server/fetch-job-runner.test.ts

@@ -88,6 +88,15 @@ function createSpawnSuccessProcess(options?: {
   candidateIds?: string[];
   pagesProcessed?: number;
 }) {
+  return createSpawnProcess(
+    JSON.stringify({
+      candidateIds: options?.candidateIds ?? [],
+      pagesProcessed: options?.pagesProcessed ?? 1,
+    })
+  );
+}
+
+function createSpawnProcess(outputLine: string, exitCode = 0) {
   const stdout = new EventEmitter();
   const stderr = new EventEmitter();
   const child = new EventEmitter() as EventEmitter & {
@@ -97,13 +106,9 @@ function createSpawnSuccessProcess(options?: {
   child.stdout = stdout;
   child.stderr = stderr;
 
-  const summary = JSON.stringify({
-    candidateIds: options?.candidateIds ?? [],
-    pagesProcessed: options?.pagesProcessed ?? 1,
-  });
   queueMicrotask(() => {
-    stdout.emit("data", Buffer.from("Progress: 1/1\n" + summary + "\n"));
-    child.emit("close", 0);
+    stdout.emit("data", Buffer.from("Progress: 1/1\n" + outputLine + "\n"));
+    child.emit("close", exitCode);
   });
 
   return child;
@@ -312,4 +317,131 @@ describe("fetch-job-runner", () => {
     expect(mockPrepareContentBranchForFetch).not.toHaveBeenCalled();
     expect(mockNotionPagesUpdate).not.toHaveBeenCalled();
   });
+
+  it("fails when terminal JSON summary is missing", async () => {
+    mockSpawn.mockImplementation(() =>
+      createSpawnProcess("Progress complete without summary")
+    );
+
+    const result = await runFetchJob({
+      type: "fetch-ready",
+      jobId: "job-missing-json",
+      options: {},
+      onProgress: vi.fn(),
+      logger: createLogger(),
+      childEnv: process.env,
+      signal: new AbortController().signal,
+      timeoutMs: 20 * 60 * 1000,
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.terminal.error?.code).toBe("CONTENT_GENERATION_FAILED");
+    expect(result.terminal.pagesProcessed).toBe(0);
+    expect(mockPrepareContentBranchForFetch).not.toHaveBeenCalled();
+  });
+
+  it("fails when terminal JSON summary is inconsistent for fetch-ready", async () => {
+    mockSpawn.mockImplementation(() =>
+      createSpawnSuccessProcess({
+        pagesProcessed: 0,
+        candidateIds: ["page-1"],
+      })
+    );
+
+    const result = await runFetchJob({
+      type: "fetch-ready",
+      jobId: "job-inconsistent-summary",
+      options: {},
+      onProgress: vi.fn(),
+      logger: createLogger(),
+      childEnv: process.env,
+      signal: new AbortController().signal,
+      timeoutMs: 20 * 60 * 1000,
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.terminal.error?.code).toBe("CONTENT_GENERATION_FAILED");
+    expect(result.terminal.pagesProcessed).toBe(0);
+    expect(mockPrepareContentBranchForFetch).not.toHaveBeenCalled();
+  });
+
+  it("passes fetch-ready status filter args to generation script", async () => {
+    const result = await runFetchJob({
+      type: "fetch-ready",
+      jobId: "job-ready-status-filter",
+      options: { dryRun: true },
+      onProgress: vi.fn(),
+      logger: createLogger(),
+      childEnv: process.env,
+      signal: new AbortController().signal,
+      timeoutMs: 20 * 60 * 1000,
+    });
+
+    expect(result.success).toBe(true);
+    expect(mockSpawn).toHaveBeenCalledTimes(1);
+    expect(mockSpawn.mock.calls[0]?.[0]).toBe("bun");
+    expect(mockSpawn.mock.calls[0]?.[1]).toEqual(
+      expect.arrayContaining([
+        "--status-filter",
+        NOTION_PROPERTIES.READY_TO_PUBLISH,
+      ])
+    );
+    expect(mockSpawn.mock.calls[0]?.[1]).toEqual(
+      expect.arrayContaining(["--status-filter", "Ready to publish"])
+    );
+  });
+
+  it("transitions all fetch-ready candidates on happy path", async () => {
+    const candidateIds = ["page-1", "page-2", "page-3"];
+    mockSpawn.mockImplementation(() =>
+      createSpawnSuccessProcess({
+        pagesProcessed: candidateIds.length,
+        candidateIds,
+      })
+    );
+
+    const result = await runFetchJob({
+      type: "fetch-ready",
+      jobId: "job-ready-happy-path",
+      options: {},
+      onProgress: vi.fn(),
+      logger: createLogger(),
+      childEnv: process.env,
+      signal: new AbortController().signal,
+      timeoutMs: 20 * 60 * 1000,
+    });
+
+    expect(result.success).toBe(true);
+    expect(result.terminal.pagesTransitioned).toBe(candidateIds.length);
+    expect(result.terminal.failedPageIds).toEqual([]);
+    expect(mockNotionPagesUpdate).toHaveBeenCalledTimes(candidateIds.length);
+    for (const pageId of candidateIds) {
+      expect(mockNotionPagesUpdate).toHaveBeenCalledWith(
+        expect.objectContaining({
+          page_id: pageId,
+        })
+      );
+    }
+    expect(mockVerifyRemoteHeadMatchesLocal).toHaveBeenCalledTimes(1);
+  });
+
+  it("passes --force and --dry-run through to generation script", async () => {
+    const result = await runFetchJob({
+      type: "fetch-ready",
+      jobId: "job-force-dry-run",
+      options: { force: true, dryRun: true },
+      onProgress: vi.fn(),
+      logger: createLogger(),
+      childEnv: process.env,
+      signal: new AbortController().signal,
+      timeoutMs: 20 * 60 * 1000,
+    });
+
+    expect(result.success).toBe(true);
+    expect(mockSpawn).toHaveBeenCalledTimes(1);
+    expect(mockSpawn.mock.calls[0]?.[0]).toBe("bun");
+    expect(mockSpawn.mock.calls[0]?.[1]).toEqual(
+      expect.arrayContaining(["--force", "--dry-run"])
+    );
+  });
 });

api-server/fetch-job-runner.ts

@@ -207,6 +207,12 @@ async function runGenerationScript(
   if (options.maxPages !== undefined && options.maxPages > 0) {
     args.push("--max-pages", String(options.maxPages));
   }
+  if (options.force) {
+    args.push("--force");
+  }
+  if (options.dryRun) {
+    args.push("--dry-run");
+  }
 
   return await new Promise<string>((resolve, reject) => {
     const child = spawn("bun", args, {
@@ -342,6 +348,72 @@ async function runGenerationScript(
   });
 }
 
+function parseTerminalSummary(
+  output: string,
+  type: "fetch-ready" | "fetch-all"
+): { pagesProcessed: number; candidateIds: string[] } {
+  const parsedOutput = extractLastJsonLine(output) as {
+    candidateIds?: unknown;
+    pagesProcessed?: unknown;
+  } | null;
+
+  if (!parsedOutput || typeof parsedOutput !== "object") {
+    throw new ContentRepoError(
+      "Generation command did not produce a terminal JSON summary",
+      undefined,
+      "CONTENT_GENERATION_FAILED"
+    );
+  }
+
+  const pagesProcessed = parsedOutput.pagesProcessed;
+  if (
+    typeof pagesProcessed !== "number" ||
+    !Number.isFinite(pagesProcessed) ||
+    !Number.isInteger(pagesProcessed) ||
+    pagesProcessed < 0
+  ) {
+    throw new ContentRepoError(
+      "Generation command produced invalid pagesProcessed in terminal JSON summary",
+      undefined,
+      "CONTENT_GENERATION_FAILED"
+    );
+  }
+
+  const rawCandidateIds = parsedOutput.candidateIds;
+  const candidateIds =
+    rawCandidateIds === undefined
+      ? []
+      : Array.isArray(rawCandidateIds) &&
+          rawCandidateIds.every((id) => typeof id === "string")
+        ? rawCandidateIds
+        : null;
+
+  if (candidateIds === null) {
+    throw new ContentRepoError(
+      "Generation command produced invalid candidateIds in terminal JSON summary",
+      undefined,
+      "CONTENT_GENERATION_FAILED"
+    );
+  }
+
+  if (
+    type === "fetch-ready" &&
+    candidateIds.length > 0 &&
+    pagesProcessed === 0
+  ) {
+    throw new ContentRepoError(
+      "Generation summary is inconsistent: candidateIds present but pagesProcessed is zero",
+      undefined,
+      "CONTENT_GENERATION_FAILED"
+    );
+  }
+
+  return {
+    pagesProcessed,
+    candidateIds,
+  };
+}
+
 async function getPageStatus(
   pageId: string,
   signal: AbortSignal,
@@ -431,13 +503,10 @@ export async function runFetchJob({
       timeoutMs
     );
 
-    const parsedOutput = extractLastJsonLine(output) as {
-      candidateIds?: string[];
-      pagesProcessed?: number;
-    } | null;
-    terminal.pagesProcessed = parsedOutput?.pagesProcessed ?? 0;
+    const summary = parseTerminalSummary(output, type);
+    terminal.pagesProcessed = summary.pagesProcessed;
     const transitionCandidates: string[] =
-      type === "fetch-ready" ? (parsedOutput?.candidateIds ?? []) : [];
+      type === "fetch-ready" ? summary.candidateIds : [];
 
     if (terminal.pagesProcessed === 0) {
       if (options.dryRun) {