fix: harden multi-file response detection so merge can't fail silently (#6516)

Co-authored-by: Reece Browne <[email protected]>

frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts

@@ -21,6 +21,7 @@ import {
   StirlingFileStub,
 } from "@app/types/fileContext";
 import { FILE_EVENTS } from "@app/services/errorUtils";
+import { zipFileService } from "@app/services/zipFileService";
 import { getFilenameWithoutExtension } from "@app/utils/fileUtils";
 import {
   createChildStub,
@@ -274,29 +275,36 @@ export const useToolOperation = <TParams>(
               responseType: "blob",
             });
 
-            // Multi-file responses are typically ZIP files that need extraction, but some may return single PDFs
+            const responseBlob: Blob = response.data;
+            const contentTypeHeader = response.headers?.["content-type"];
+
             if (config.responseHandler) {
-              // Use custom responseHandler for multi-file (handles ZIP extraction)
               processedFiles = await config.responseHandler(
-                response.data,
+                responseBlob,
                 filesForAPI,
               );
             } else if (
-              response.data.type === "application/pdf" ||
-              (response.headers &&
-                response.headers["content-type"] === "application/pdf")
+              await zipFileService.isZipResponse(
+                responseBlob,
+                typeof contentTypeHeader === "string"
+                  ? contentTypeHeader
+                  : undefined,
+              )
             ) {
-              // Single PDF response (e.g. split with merge option) - add prefix to first original filename
-              const filename = `${config.filePrefix}${filesForAPI[0]?.name || "document.pdf"}`;
-              const singleFile = new File([response.data], filename, {
-                type: "application/pdf",
-              });
-              processedFiles = [singleFile];
+              processedFiles = await extractZipFiles(responseBlob);
             } else {
-              // Default: assume ZIP response for multi-file endpoints
-              // Note: extractZipFiles will check preferences.autoUnzip setting
-              processedFiles = await extractZipFiles(response.data);
+              const filename = `${config.filePrefix}${filesForAPI[0]?.name || "document.pdf"}`;
+              processedFiles = [
+                new File([responseBlob], filename, { type: "application/pdf" }),
+              ];
             }
+
+            if (processedFiles.length === 0) {
+              throw new Error(
+                "The server processed the request but returned no files.",
+              );
+            }
+
             // Assume all inputs succeeded together unless server provided an error earlier
             successSourceIds = validFiles.map((f) => f.fileId);
             break;

frontend/editor/src/core/services/zipFileService.test.ts

@@ -0,0 +1,122 @@
+import { describe, expect, test } from "vitest";
+import { zipFileService } from "@app/services/zipFileService";
+
+// jsdom's Blob.slice(...).arrayBuffer() returns an empty buffer in this
+// version, so a real Blob would never reach the magic-byte branch under test.
+// Duck-type the parts isZipResponse actually touches.
+function fakeBlob(bytes: number[], type = ""): Blob {
+  const u8 = new Uint8Array(bytes);
+  const slice = (start: number, end?: number) =>
+    fakeBlob(Array.from(u8.slice(start, end)), type);
+  return {
+    size: u8.byteLength,
+    type,
+    slice,
+    arrayBuffer: async () =>
+      u8.buffer.slice(u8.byteOffset, u8.byteOffset + u8.byteLength),
+  } as unknown as Blob;
+}
+
+const PDF_BYTES = [0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x37]; // "%PDF-1.7"
+
+describe("zipFileService.isZipResponse", () => {
+  describe("signature is authoritative", () => {
+    test("PK\\x03\\x04 -> ZIP", async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob([0x50, 0x4b, 0x03, 0x04, 0x00, 0x00]),
+        ),
+      ).toBe(true);
+    });
+
+    test("PK\\x05\\x06 (empty archive) -> ZIP", async () => {
+      expect(
+        await zipFileService.isZipResponse(fakeBlob([0x50, 0x4b, 0x05, 0x06])),
+      ).toBe(true);
+    });
+
+    test("PK\\x07\\x08 (spanned marker) -> ZIP", async () => {
+      expect(
+        await zipFileService.isZipResponse(fakeBlob([0x50, 0x4b, 0x07, 0x08])),
+      ).toBe(true);
+    });
+
+    test("%PDF signature overrides a ZIP Content-Type", async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(PDF_BYTES),
+          "application/zip",
+        ),
+      ).toBe(false);
+    });
+  });
+
+  // Bug-trigger Content-Types: previously misrouted PDFs into ZIP extraction.
+  describe("bug-trigger Content-Types must not misroute a PDF", () => {
+    test('"application/pdf;charset=UTF-8" + %PDF -> NOT ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(PDF_BYTES),
+          "application/pdf;charset=UTF-8",
+        ),
+      ).toBe(false);
+    });
+
+    test('"application/octet-stream" + %PDF -> NOT ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(PDF_BYTES),
+          "application/octet-stream",
+        ),
+      ).toBe(false);
+    });
+
+    test('"APPLICATION/PDF" + %PDF -> NOT ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(PDF_BYTES),
+          "APPLICATION/PDF",
+        ),
+      ).toBe(false);
+    });
+  });
+
+  describe("Content-Type fallback when signature is inconclusive", () => {
+    const UNKNOWN_BYTES = [0xff, 0xff, 0xff, 0xff];
+
+    test('unknown bytes + "application/zip" -> ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(UNKNOWN_BYTES),
+          "application/zip",
+        ),
+      ).toBe(true);
+    });
+
+    test('unknown bytes + "application/x-zip-compressed" -> ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(UNKNOWN_BYTES),
+          "application/x-zip-compressed",
+        ),
+      ).toBe(true);
+    });
+
+    test('unknown bytes + "application/json" -> NOT ZIP', async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(UNKNOWN_BYTES),
+          "application/json",
+        ),
+      ).toBe(false);
+    });
+
+    test("unknown bytes, no hint, blob.type 'application/zip' -> ZIP", async () => {
+      expect(
+        await zipFileService.isZipResponse(
+          fakeBlob(UNKNOWN_BYTES, "application/zip"),
+        ),
+      ).toBe(true);
+    });
+  });
+});

frontend/editor/src/core/services/zipFileService.ts

@@ -303,6 +303,37 @@ export class ZipFileService {
     return hasValidType || hasValidExtension;
   }
 
+  // Signature-first ZIP detection for HTTP response bodies. Content-Type alone
+  // is unreliable: proxies and some backends ship merged PDFs as
+  // "application/pdf;charset=UTF-8" or "application/octet-stream", which an
+  // exact-equality check would misroute into ZIP extraction.
+  public async isZipResponse(
+    blob: Blob,
+    contentTypeHint?: string,
+  ): Promise<boolean> {
+    try {
+      const sig = new Uint8Array(await blob.slice(0, 4).arrayBuffer());
+      if (
+        sig[0] === 0x50 &&
+        sig[1] === 0x4b &&
+        (sig[2] === 0x03 || sig[2] === 0x05 || sig[2] === 0x07)
+      ) {
+        return true;
+      }
+      if (
+        sig[0] === 0x25 &&
+        sig[1] === 0x50 &&
+        sig[2] === 0x44 &&
+        sig[3] === 0x46
+      ) {
+        return false;
+      }
+    } catch {
+      // Unreadable blob - fall back to Content-Type.
+    }
+    return (contentTypeHint || blob.type || "").toLowerCase().includes("zip");
+  }
+
   /**
    * Check if a filename indicates a PDF file
    */

frontend/editor/src/core/tests/stubbed/merge-response-handling.spec.ts

@@ -0,0 +1,71 @@
+import type { Page, Route } from "@playwright/test";
+import { test, expect } from "@app/tests/helpers/stub-test-base";
+import {
+  uploadFiles,
+  switchToEditorIfViewerMode,
+  runToolAndWaitForReview,
+} from "@app/tests/helpers/ui-helpers";
+import path from "path";
+import fs from "fs";
+
+// Regression coverage: the merge endpoint used to misroute a PDF into JSZip
+// when the Content-Type wasn't an exact `application/pdf`, producing a bogus
+// `result.zip` instead of the merged file. The UI fix uses signature-based
+// detection - %PDF wins regardless of Content-Type.
+
+const FIXTURES_DIR = path.join(__dirname, "../test-fixtures");
+const SAMPLE_PDF = path.join(FIXTURES_DIR, "sample.pdf");
+const SAMPLE_PDF_BYTES = fs.readFileSync(SAMPLE_PDF);
+
+async function mockMergeWithContentType(page: Page, contentType: string) {
+  await page.route("**/api/v1/general/merge-pdfs", (route: Route) =>
+    route.fulfill({
+      status: 200,
+      contentType,
+      headers: {
+        "Content-Disposition": 'attachment; filename="merged_unsigned.pdf"',
+      },
+      body: SAMPLE_PDF_BYTES,
+    }),
+  );
+}
+
+async function runMergeAndOpenReview(page: Page) {
+  await page.goto("/merge");
+  await uploadFiles(page, [SAMPLE_PDF, SAMPLE_PDF]);
+  await switchToEditorIfViewerMode(page);
+  await runToolAndWaitForReview(page);
+}
+
+test.describe("Merge multi-file response detection", () => {
+  test('"application/pdf;charset=UTF-8" -> merged PDF, not result.zip', async ({
+    page,
+  }) => {
+    await mockMergeWithContentType(page, "application/pdf;charset=UTF-8");
+    await runMergeAndOpenReview(page);
+
+    const review = page.locator('[data-testid="review-panel-container"]');
+    await expect(review).toBeVisible();
+    await expect(review.getByText(/result\.zip/i)).toHaveCount(0);
+  });
+
+  test('"application/octet-stream" -> merged PDF, not result.zip', async ({
+    page,
+  }) => {
+    await mockMergeWithContentType(page, "application/octet-stream");
+    await runMergeAndOpenReview(page);
+
+    const review = page.locator('[data-testid="review-panel-container"]');
+    await expect(review).toBeVisible();
+    await expect(review.getByText(/result\.zip/i)).toHaveCount(0);
+  });
+
+  test('"APPLICATION/PDF" -> merged PDF, not result.zip', async ({ page }) => {
+    await mockMergeWithContentType(page, "APPLICATION/PDF");
+    await runMergeAndOpenReview(page);
+
+    const review = page.locator('[data-testid="review-panel-container"]');
+    await expect(review).toBeVisible();
+    await expect(review.getByText(/result\.zip/i)).toHaveCount(0);
+  });
+});

frontend/editor/src/core/utils/automationExecutor.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, test } from "vitest";
+import { processMultiFileResponse } from "@app/utils/automationExecutor";
+
+// Regression coverage for the automation-side mirror of the merge bug:
+// non-canonical Content-Types previously misrouted a PDF into ZIP extraction
+// and yielded a bogus `automation_*.zip`.
+
+const PDF_BYTES = new Uint8Array([
+  0x25,
+  0x50,
+  0x44,
+  0x46,
+  0x2d,
+  0x31,
+  0x2e,
+  0x37, // "%PDF-1.7"
+  0x0a,
+  0x25,
+  0xe2,
+  0xe3,
+  0xcf,
+  0xd3,
+  0x0a,
+]);
+
+const inputFiles = [
+  new File(["fake"], "input.pdf", { type: "application/pdf" }),
+];
+
+async function run(contentType: string) {
+  return processMultiFileResponse(
+    new Blob([PDF_BYTES]),
+    { "content-type": contentType },
+    inputFiles,
+    "automated_",
+    false,
+  );
+}
+
+describe("processMultiFileResponse (automation execution)", () => {
+  test('PDF body + "application/octet-stream" -> PDF, not .zip', async () => {
+    const result = await run("application/octet-stream");
+    expect(result.length).toBe(1);
+    expect(result[0].name).not.toMatch(/\.zip$/);
+  });
+
+  test('PDF body + "application/pdf;charset=UTF-8" -> PDF, not .zip', async () => {
+    const result = await run("application/pdf;charset=UTF-8");
+    expect(result.length).toBe(1);
+    expect(result[0].name).not.toMatch(/\.zip$/);
+  });
+
+  test('PDF body + "APPLICATION/PDF" -> PDF, not .zip', async () => {
+    const result = await run("APPLICATION/PDF");
+    expect(result.length).toBe(1);
+    expect(result[0].name).not.toMatch(/\.zip$/);
+  });
+});

frontend/editor/src/core/utils/automationExecutor.ts

@@ -4,34 +4,32 @@ import { ToolId } from "@app/types/toolId";
 import { AUTOMATION_CONSTANTS } from "@app/constants/automation";
 import { AutomationFileProcessor } from "@app/utils/automationFileProcessor";
 import { ToolType } from "@app/hooks/tools/shared/useToolOperation";
+import { zipFileService } from "@app/services/zipFileService";
 import { processResponse } from "@app/utils/toolResponseProcessor";
 
-/**
- * Process multi-file tool response (handles ZIP or single PDF responses)
- */
-const processMultiFileResponse = async (
+export const processMultiFileResponse = async (
   responseData: Blob,
   responseHeaders: any,
   files: File[],
   filePrefix: string,
   preserveBackendFilename?: boolean,
 ): Promise<File[]> => {
-  // Multi-file responses are typically ZIP files, but may be single files (e.g. split with merge=true)
-  if (
-    responseData.type === "application/pdf" ||
-    (responseHeaders && responseHeaders["content-type"] === "application/pdf")
-  ) {
-    // Single PDF response - use processResponse to respect preserveBackendFilename
-    const processedFiles = await processResponse(
+  const contentTypeHeader = responseHeaders?.["content-type"];
+  const looksLikeZip = await zipFileService.isZipResponse(
+    responseData,
+    typeof contentTypeHeader === "string" ? contentTypeHeader : undefined,
+  );
+
+  if (!looksLikeZip) {
+    return processResponse(
       responseData,
       files,
       filePrefix,
       undefined,
       preserveBackendFilename ? responseHeaders : undefined,
     );
-    return processedFiles;
-  } else {
-    // ZIP response
+  }
+
   const result =
     await AutomationFileProcessor.extractAutomationZipFiles(responseData);
 
@@ -39,19 +37,15 @@ const processMultiFileResponse = async (
     console.warn(`⚠️ File processing warnings:`, result.errors);
   }
 
-    // Apply prefix to files, replacing any existing prefix
-    const processedFiles =
-      filePrefix && !preserveBackendFilename
-        ? result.files.map((file) => {
+  if (!filePrefix || preserveBackendFilename) {
+    return result.files;
+  }
+  return result.files.map((file) => {
     const nameWithoutPrefix = file.name.replace(/^[^_]*_/, "");
     return new File([file], `${filePrefix}${nameWithoutPrefix}`, {
       type: file.type,
     });
-          })
-        : result.files;
-
-    return processedFiles;
-  }
+  });
 };
 
 /**

frontend/editor/src/core/utils/automationFileProcessor.ts

@@ -19,18 +19,6 @@ export interface AutomationProcessingResult {
 }
 
 export class AutomationFileProcessor {
-  /**
-   * Check if a blob is a ZIP file by examining its header
-   */
-  static isZipFile(blob: Blob): boolean {
-    // This is a simple check - in a real implementation you might want to read the first few bytes
-    // For now, we'll rely on the extraction attempt and fallback
-    return (
-      blob.type === "application/zip" ||
-      blob.type === "application/x-zip-compressed"
-    );
-  }
-
   /**
    * Extract files from a ZIP blob during automation execution, with fallback for non-ZIP files
    * Extracts all file types (PDFs, images, etc.) except HTML files which stay zipped

frontend/editor/src/desktop/services/apiClientSetup.test.ts

@@ -0,0 +1,175 @@
+import type { AxiosInstance } from "axios";
+import { describe, expect, test, vi, beforeEach, afterEach } from "vitest";
+import { OPEN_SIGN_IN_EVENT } from "@app/constants/signInEvents";
+
+// Exercise the error-interceptor logic against a hand-rolled axios-like
+// client; transitive imports are mocked out so this is a pure unit test.
+
+vi.mock("@app/components/toast", () => ({ alert: vi.fn() }));
+vi.mock("@core/services/apiClientSetup", () => ({
+  setupApiInterceptors: vi.fn(),
+  getAuthHeaders: vi.fn(),
+}));
+vi.mock("@app/services/tauriBackendService", () => ({
+  tauriBackendService: {
+    isOnline: true,
+    getBackendStatus: () => "healthy",
+    getBackendPort: () => 8080,
+  },
+}));
+vi.mock("@app/constants/backendErrors", () => ({
+  createBackendNotReadyError: () => new Error("backend not ready"),
+}));
+vi.mock("@app/services/operationRouter", () => ({
+  operationRouter: {
+    isSelfHostedMode: vi.fn().mockResolvedValue(true),
+    isSaaSMode: vi.fn().mockResolvedValue(false),
+    getBaseUrl: vi.fn().mockResolvedValue("https://example.test"),
+    shouldSkipBackendReadyCheck: vi.fn().mockResolvedValue(false),
+  },
+}));
+vi.mock("@app/services/authService", () => ({
+  authService: {
+    awaitRefreshIfInProgress: vi.fn().mockResolvedValue(undefined),
+    getAuthToken: vi.fn().mockResolvedValue(null),
+    refreshToken: vi.fn().mockResolvedValue(false),
+    refreshSupabaseToken: vi.fn().mockResolvedValue(false),
+  },
+}));
+vi.mock("@app/services/connectionModeService", () => ({
+  connectionModeService: {
+    getServerConfig: vi.fn().mockResolvedValue({ url: "https://example.test" }),
+  },
+}));
+vi.mock("@app/constants/connection", () => ({
+  STIRLING_SAAS_URL: "https://saas.test",
+  STIRLING_SAAS_BACKEND_API_URL: "https://api.saas.test",
+}));
+vi.mock("@app/i18n", () => ({
+  default: { t: (_key: string, fallback: string) => fallback || _key },
+}));
+
+import { setupApiInterceptors } from "@app/services/apiClientSetup";
+
+type Interceptor = (value: unknown) => unknown;
+
+type Handlers = {
+  request: Interceptor[];
+  responseFulfilled: Interceptor[];
+  responseRejected: Interceptor[];
+};
+
+type MockClient = {
+  interceptors: {
+    request: { use: (onFulfilled: Interceptor) => void };
+    response: {
+      use: (onFulfilled: Interceptor, onRejected: Interceptor) => void;
+    };
+  };
+  request: ReturnType<typeof vi.fn>;
+};
+
+function makeMockClient(): { client: MockClient; handlers: Handlers } {
+  const handlers: Handlers = {
+    request: [],
+    responseFulfilled: [],
+    responseRejected: [],
+  };
+  const client: MockClient = {
+    interceptors: {
+      request: {
+        use: (onFulfilled) => {
+          handlers.request.push(onFulfilled);
+        },
+      },
+      response: {
+        use: (onFulfilled, onRejected) => {
+          handlers.responseFulfilled.push(onFulfilled);
+          handlers.responseRejected.push(onRejected);
+        },
+      },
+    },
+    // Must exist on the mock; not exercised by these tests.
+    request: vi.fn(),
+  };
+  return { client, handlers };
+}
+
+async function triggerErrorInterceptor(
+  handlers: Handlers,
+  error: unknown,
+): Promise<void> {
+  const handler = handlers.responseRejected[0];
+  expect(handler).toBeTypeOf("function");
+  // The interceptor re-rejects after handling; swallow it.
+  await Promise.resolve(handler(error)).catch(() => {});
+}
+
+describe("desktop apiClientSetup - 401 silent-path", () => {
+  let events: Event[];
+  let listener: (e: Event) => void;
+
+  beforeEach(() => {
+    events = [];
+    listener = (e: Event) => events.push(e);
+    window.addEventListener(OPEN_SIGN_IN_EVENT, listener);
+  });
+
+  afterEach(() => {
+    window.removeEventListener(OPEN_SIGN_IN_EVENT, listener);
+  });
+
+  test("POST 401 without Authorization dispatches sign-in modal event", async () => {
+    const { client, handlers } = makeMockClient();
+    setupApiInterceptors(client as unknown as AxiosInstance);
+    await triggerErrorInterceptor(handlers, {
+      response: { status: 401 },
+      config: {
+        method: "post",
+        url: "/api/v1/general/merge-pdfs",
+        headers: {},
+      },
+    });
+    expect(events).toHaveLength(1);
+    expect(events[0].type).toBe(OPEN_SIGN_IN_EVENT);
+  });
+
+  test("GET 401 without Authorization stays silent (background probe)", async () => {
+    const { client, handlers } = makeMockClient();
+    setupApiInterceptors(client as unknown as AxiosInstance);
+    await triggerErrorInterceptor(handlers, {
+      response: { status: 401 },
+      config: {
+        method: "get",
+        url: "/api/v1/config/endpoint-enabled",
+        headers: {},
+      },
+    });
+    expect(events).toHaveLength(0);
+  });
+
+  test("401 on auth probe (/api/v1/auth/me) never dispatches the modal", async () => {
+    const { client, handlers } = makeMockClient();
+    setupApiInterceptors(client as unknown as AxiosInstance);
+    await triggerErrorInterceptor(handlers, {
+      response: { status: 401 },
+      config: { method: "get", url: "/api/v1/auth/me", headers: {} },
+    });
+    expect(events).toHaveLength(0);
+  });
+
+  test("401 with skipAuthRedirect set never dispatches the modal", async () => {
+    const { client, handlers } = makeMockClient();
+    setupApiInterceptors(client as unknown as AxiosInstance);
+    await triggerErrorInterceptor(handlers, {
+      response: { status: 401 },
+      config: {
+        method: "post",
+        url: "/api/v1/general/merge-pdfs",
+        headers: {},
+        skipAuthRedirect: true,
+      },
+    });
+    expect(events).toHaveLength(0);
+  });
+});

frontend/editor/src/desktop/services/apiClientSetup.ts

@@ -188,10 +188,18 @@ export function setupApiInterceptors(client: AxiosInstance): void {
         if (originalRequest.skipAuthRedirect) {
           return Promise.reject(error);
         }
-        // If no Authorization header was sent, the user was never authenticated —
-        // the 401 is expected (e.g. endpoint availability checks when not signed in).
-        // Don't attempt a refresh or open the sign-in modal in that case.
+        // No token attached: stay silent for background GETs (endpoint probes
+        // before sign-in), but surface the sign-in modal for mutations so the
+        // user isn't left with a no-op click.
         if (!originalRequest.headers.Authorization) {
+          const method = (originalRequest.method || "get").toLowerCase();
+          if (method !== "get") {
+            window.dispatchEvent(
+              new CustomEvent(OPEN_SIGN_IN_EVENT, {
+                detail: { locked: false },
+              }),
+            );
+          }
           return Promise.reject(error);
         }
         originalRequest._retry = true;