Explicitly test for console warnings & errors (#6502)

# Description of Changes
Disallow warnings and errors from being thrown in the browser console
during tests unless explicitly expected in the test. Also adds a
Playwright test to prod around some main UI areas and checks that no
warnings/errors have been thrown.

frontend/editor/src/core/tests/convert/ConvertIntegration.test.tsx

@@ -29,6 +29,8 @@ import { PreferencesProvider } from "@app/contexts/PreferencesContext";
 import { I18nextProvider } from "react-i18next";
 import i18n from "@app/i18n/config";
 import { createTestStirlingFile } from "@app/tests/utils/testFileHelpers";
+import { expectConsole } from "@app/tests/failOnConsole";
+import { fileStorage } from "@app/services/fileStorage";
 import { StirlingFile } from "@app/types/fileContext";
 import { MantineProvider } from "@mantine/core";
 
@@ -78,6 +80,7 @@ vi.mock("../../services/fileStorage", () => ({
         thumbnail: thumbnail,
       });
     }),
+    storeStirlingFile: vi.fn().mockResolvedValue(undefined),
     getAllFileMetadata: vi.fn().mockResolvedValue([]),
     cleanup: vi.fn().mockResolvedValue(undefined),
   },
@@ -209,6 +212,10 @@ describe("Convert Tool Integration Tests", () => {
       expect(result.current.downloadFilename).toBe("test.png");
       expect(result.current.isLoading).toBe(false);
       expect(result.current.errorMessage).toBe(null);
+
+      // The output file must be persisted via fileStorage.storeStirlingFile
+      // so downstream tools see it in the registry.
+      expect(fileStorage.storeStirlingFile).toHaveBeenCalled();
     });
 
     test("should handle API error responses correctly", async () => {
@@ -571,6 +578,9 @@ describe("Convert Tool Integration Tests", () => {
 
   describe("File Upload Integration", () => {
     test("should handle multiple file uploads correctly", async () => {
+      // Test mocks only one apiClient.post response; the second file hits an
+      // undefined response and production warns about the conversion failure.
+      expectConsole.warn(/Failed to convert file test2\.pdf/);
       const mockBlob = new Blob(["zip-content"], { type: "application/zip" });
       (mockedApiClient.post as Mock).mockResolvedValueOnce({ data: mockBlob });
 

frontend/editor/src/core/tests/convert/ConvertSmartDetectionIntegration.test.tsx

@@ -28,6 +28,8 @@ import {
   createTestStirlingFile,
   createTestFilesWithId,
 } from "@app/tests/utils/testFileHelpers";
+import { allowConsole, expectConsole } from "@app/tests/failOnConsole";
+import { fileStorage } from "@app/services/fileStorage";
 import { MantineProvider } from "@mantine/core";
 
 // Mock axios (for static methods like CancelToken, isCancel)
@@ -76,6 +78,7 @@ vi.mock("../../services/fileStorage", () => ({
         thumbnail: thumbnail,
       });
     }),
+    storeStirlingFile: vi.fn().mockResolvedValue(undefined),
     getAllFileMetadata: vi.fn().mockResolvedValue([]),
     cleanup: vi.fn().mockResolvedValue(undefined),
   },
@@ -169,6 +172,10 @@ describe("Convert Tool - Smart Detection Integration Tests", () => {
           responseType: "blob",
         },
       );
+
+      // The output file must be persisted via fileStorage.storeStirlingFile
+      // so downstream tools see it in the registry.
+      expect(fileStorage.storeStirlingFile).toHaveBeenCalled();
     });
 
     test("should handle unknown file type with file-to-pdf fallback", async () => {
@@ -496,6 +503,12 @@ describe("Convert Tool - Smart Detection Integration Tests", () => {
     });
 
     test("should send correct PDF/A parameters for pdf-to-pdfa conversion", async () => {
+      // Test files aren't registered in the FileContext stub registry, so
+      // production warns about the stub/output mismatch - incidental to what
+      // this test asserts.
+      allowConsole.warn(
+        /\[useToolOperation\] Mismatch successInputStubs vs outputs/,
+      );
       const { result: paramsResult } = renderHook(
         () => useConvertParameters(),
         {
@@ -638,6 +651,9 @@ describe("Convert Tool - Smart Detection Integration Tests", () => {
 
   describe("Error Scenarios in Smart Detection", () => {
     test("should handle partial failures in multi-file processing", async () => {
+      // Production warns when an individual file in a batch fails; the test
+      // deliberately drives one mock rejection to exercise that path.
+      expectConsole.warn(/Failed to convert file doc2\.xyz/);
       const { result: paramsResult } = renderHook(
         () => useConvertParameters(),
         {

frontend/editor/src/core/tests/failOnConsole.test.ts

@@ -0,0 +1,78 @@
+import { describe, expect, it } from "vitest";
+import {
+  allowConsole,
+  expectConsole,
+  formatArgs,
+  matches,
+} from "@app/tests/failOnConsole";
+
+describe("failOnConsole", () => {
+  describe("matches", () => {
+    it("treats a string matcher as a substring test", () => {
+      expect(matches("foo", "the foo bar")).toBe(true);
+      expect(matches("foo", "no match here")).toBe(false);
+    });
+
+    it("treats a RegExp matcher as a .test() call", () => {
+      expect(matches(/^\[Tag\]/, "[Tag] something")).toBe(true);
+      expect(matches(/^\[Tag\]/, "prefix [Tag] something")).toBe(false);
+    });
+
+    it("string match is case-sensitive", () => {
+      expect(matches("Foo", "foo")).toBe(false);
+    });
+  });
+
+  describe("formatArgs", () => {
+    it("joins string args with spaces", () => {
+      expect(formatArgs(["one", "two", "three"])).toBe("one two three");
+    });
+
+    it("stringifies plain objects with JSON", () => {
+      expect(formatArgs([{ a: 1, b: "x" }])).toBe('{"a":1,"b":"x"}');
+    });
+
+    it("uses the stack for Error instances when present", () => {
+      const err = new Error("boom");
+      const result = formatArgs(["context:", err]);
+      expect(result).toContain("context:");
+      expect(result).toContain("boom");
+    });
+
+    it("falls back to String() when JSON.stringify throws (circular ref)", () => {
+      const circular: Record<string, unknown> = { name: "loop" };
+      circular.self = circular;
+      const out = formatArgs([circular]);
+      expect(out).toBe("[object Object]");
+    });
+
+    it("handles non-string, non-Error primitives", () => {
+      expect(formatArgs([42, true, null])).toBe("42 true null");
+    });
+  });
+
+  describe("expectConsole / allowConsole integration", () => {
+    it("expectConsole.error absorbs a matching call and satisfies the expectation", () => {
+      expectConsole.error(/expected sentinel/);
+      console.error("an expected sentinel was emitted");
+      // No assertion failure here means: the call was absorbed AND
+      // afterEach won't throw because the expectation matched.
+    });
+
+    it("allowConsole.warn absorbs a matching call without asserting it fires", () => {
+      allowConsole.warn(/optional sentinel/);
+      // Intentionally do NOT call console.warn. The test should still
+      // pass because allowConsole is non-required.
+    });
+
+    it("allowConsole.warn absorbs the call when it does fire too", () => {
+      allowConsole.warn(/optional sentinel that does fire/);
+      console.warn("optional sentinel that does fire now");
+    });
+
+    it("supports a string matcher (substring)", () => {
+      expectConsole.warn("substring sentinel inside");
+      console.warn("a substring sentinel inside a longer message");
+    });
+  });
+});

frontend/editor/src/core/tests/failOnConsole.ts

@@ -0,0 +1,152 @@
+import { afterEach, beforeEach, vi, type MockInstance } from "vitest";
+
+type ConsoleMethod = "error" | "warn";
+type Matcher = string | RegExp;
+
+const WATCHED: ConsoleMethod[] = ["error", "warn"];
+
+type Capture = { method: ConsoleMethod; args: unknown[] };
+
+type Expectation = {
+  method: ConsoleMethod;
+  matcher: Matcher;
+  /**
+   * `true` for `expectConsole` (a contract: the test fails if no matching
+   * call fires); `false` for `allowConsole` (incidental noise: absorbed if
+   * it happens, ignored if it doesn't).
+   */
+  required: boolean;
+  matched: boolean;
+};
+
+const captured: Capture[] = [];
+const expectations: Expectation[] = [];
+
+/**
+ * Vitest setup that fails any test whose code calls console.error or
+ * console.warn. The goal is to keep the browser console clean during normal
+ * usage of the app: React key warnings, act() warnings, deprecation
+ * notices, and runtime errors should all surface as test failures rather
+ * than silently scrolling past in CI.
+ *
+ * If a test legitimately needs to drive a log:
+ *   - Use `expectConsole.error(/pattern/)` when the log is part of the
+ *     contract under test. The matching call is absorbed AND the test
+ *     fails if it never fires, so the log can't silently disappear later.
+ *   - Use `allowConsole.warn(/pattern/)` when the log is incidental
+ *     (third-party noise, an artefact of a fake JWT, etc.). The matching
+ *     call is absorbed; nothing is asserted.
+ *
+ * Anything that doesn't match an expectation/allowance still fails the
+ * test as before.
+ */
+export function installFailOnConsole(): void {
+  const spies: MockInstance[] = [];
+
+  beforeEach(() => {
+    captured.length = 0;
+    expectations.length = 0;
+    spies.length = 0;
+    for (const method of WATCHED) {
+      const original = console[method].bind(console);
+      const spy = vi
+        .spyOn(console, method)
+        .mockImplementation((...args: unknown[]) => {
+          const text = formatArgs(args);
+          const match = expectations.find(
+            (e) => e.method === method && matches(e.matcher, text),
+          );
+          if (match) {
+            match.matched = true;
+            return;
+          }
+          captured.push({ method, args });
+          original(...args);
+        });
+      spies.push(spy);
+    }
+  });
+
+  afterEach(() => {
+    for (const spy of spies.splice(0)) {
+      spy.mockRestore();
+    }
+
+    const unmet = expectations.filter((e) => e.required && !e.matched);
+    const localCaptured = captured.splice(0);
+    expectations.length = 0;
+
+    if (unmet.length > 0) {
+      const lines = unmet
+        .map((e) => `  console.${e.method}: ${formatMatcher(e.matcher)}`)
+        .join("\n");
+      throw new Error(
+        `Expected console output never fired:\n${lines}\n\n` +
+          "If production stopped logging this, either restore the log or " +
+          "update/remove the expectConsole call.",
+      );
+    }
+    if (localCaptured.length === 0) return;
+    const lines = localCaptured
+      .map((c) => `  console.${c.method}: ${formatArgs(c.args)}`)
+      .join("\n");
+    throw new Error(
+      `Test produced unexpected console output:\n${lines}\n\n` +
+        "If the log is expected, capture it with expectConsole.error/warn " +
+        "(contract) or allowConsole.error/warn (incidental) from " +
+        "@app/tests/failOnConsole.",
+    );
+  });
+}
+
+/**
+ * Register a required console expectation. The test passes only if at
+ * least one matching call fires; matching calls are absorbed instead of
+ * failing the test.
+ */
+export const expectConsole = {
+  error: (matcher: Matcher) => register("error", matcher, true),
+  warn: (matcher: Matcher) => register("warn", matcher, true),
+};
+
+/**
+ * Register an optional console allowance. Matching calls are absorbed;
+ * the test does NOT fail if no matching call fires. Use for incidental
+ * noise (fake-JWT decode warnings, third-party library logs in dev, etc.).
+ */
+export const allowConsole = {
+  error: (matcher: Matcher) => register("error", matcher, false),
+  warn: (matcher: Matcher) => register("warn", matcher, false),
+};
+
+function register(method: ConsoleMethod, matcher: Matcher, required: boolean) {
+  expectations.push({ method, matcher, required, matched: false });
+}
+
+/** Exported for the failOnConsole unit tests; not part of the public API. */
+export function matches(matcher: Matcher, text: string): boolean {
+  return typeof matcher === "string"
+    ? text.includes(matcher)
+    : matcher.test(text);
+}
+
+function formatMatcher(matcher: Matcher): string {
+  return matcher instanceof RegExp
+    ? matcher.toString()
+    : JSON.stringify(matcher);
+}
+
+/** Exported for the failOnConsole unit tests; not part of the public API. */
+export function formatArgs(args: unknown[]): string {
+  return args.map(formatArg).join(" ");
+}
+
+function formatArg(value: unknown): string {
+  if (value instanceof Error) return value.stack ?? value.message;
+  if (typeof value === "string") return value;
+  try {
+    return JSON.stringify(value);
+  } catch {
+    return String(value);
+  }
+}

frontend/editor/src/core/tests/stubbed/console-clean.spec.ts

@@ -0,0 +1,114 @@
+import { test, expect } from "@app/tests/helpers/stub-test-base";
+import { errors, type ConsoleMessage, type Page } from "@playwright/test";
+
+/**
+ * Smoke test: standard usage of the app must not produce any
+ * console.error, console.warn, or uncaught page errors.
+ *
+ * Each test disables the fixture's auto-goto (via `test.use({ autoGoto:
+ * false })`), attaches listeners inline with `attachListeners(page)` BEFORE
+ * navigating, walks a representative route, lets it settle, then asserts
+ * the captured buffer is empty.
+ *
+ * If you have a legitimate reason a warning fires on a given route
+ * (third-party library noise we cannot influence, etc.), filter it via the
+ * `IGNORED` allowlist below, but the default expectation is that the
+ * console stays clean. Do not add entries casually; prefer fixing the
+ * underlying issue.
+ */
+
+type ConsoleEntry = {
+  type: "error" | "warn" | "pageerror";
+  text: string;
+  location?: string;
+};
+
+const IGNORED: RegExp[] = [
+  // Add entries here only with a comment explaining why the warning is
+  // unavoidable. Default: keep this list empty.
+];
+
+function shouldIgnore(text: string): boolean {
+  return IGNORED.some((re) => re.test(text));
+}
+
+function attachListeners(page: Page): ConsoleEntry[] {
+  const entries: ConsoleEntry[] = [];
+  page.on("console", (msg: ConsoleMessage) => {
+    const type = msg.type();
+    if (type !== "error" && type !== "warning") return;
+    const text = msg.text();
+    if (shouldIgnore(text)) return;
+    const loc = msg.location();
+    entries.push({
+      type: type === "warning" ? "warn" : "error",
+      text,
+      location: loc.url
+        ? `${loc.url}:${loc.lineNumber}:${loc.columnNumber}`
+        : undefined,
+    });
+  });
+  page.on("pageerror", (err) => {
+    if (shouldIgnore(err.message)) return;
+    entries.push({ type: "pageerror", text: err.stack ?? err.message });
+  });
+  return entries;
+}
+
+function formatEntries(entries: ConsoleEntry[]): string {
+  return entries
+    .map(
+      (e) =>
+        `  [${e.type}] ${e.text}${e.location ? `\n    at ${e.location}` : ""}`,
+    )
+    .join("\n");
+}
+
+async function expectCleanConsole(entries: ConsoleEntry[]) {
+  expect(
+    entries,
+    `Page produced unexpected console output:\n${formatEntries(entries)}`,
+  ).toEqual([]);
+}
+
+// ─── Routes to sweep ────────────────────────────────────────────────────────
+//
+// One entry per route we want to guarantee is console-clean on load. Mirrors
+// the most common user entry points; expand cautiously - every entry adds CI
+// time and triage surface for new warnings.
+
+const ROUTES: { name: string; path: string }[] = [
+  { name: "landing", path: "/" },
+  { name: "files", path: "/files" },
+  { name: "compress", path: "/compress" },
+  { name: "split", path: "/split" },
+  { name: "merge", path: "/merge" },
+  { name: "convert", path: "/convert" },
+  { name: "rotate", path: "/rotate" },
+  { name: "addPageNumbers", path: "/add-page-numbers" },
+];
+
+// Disable the fixture's auto-goto so we can attach listeners before any
+// navigation happens. Otherwise listeners miss early load-time noise.
+test.use({ autoGoto: false });
+
+test.describe("Console hygiene: representative routes load cleanly", () => {
+  for (const route of ROUTES) {
+    test(`${route.name} (${route.path})`, async ({ page }) => {
+      const entries = attachListeners(page);
+      await page.goto(route.path, { waitUntil: "domcontentloaded" });
+      // Give async effects (i18n load, lazy chunks, posthog init) a beat to
+      // surface anything they were going to log.
+      await page
+        .waitForLoadState("networkidle", { timeout: 10_000 })
+        .catch((err) => {
+          // networkidle can flake on third-party CDNs; treat ONLY the
+          // timeout as benign and still run the console assertion on what
+          // we captured. Anything else (frame detached, navigation abort,
+          // etc.) is a real problem and should fail the test.
+          if (!(err instanceof errors.TimeoutError)) throw err;
+        });
+      await expectCleanConsole(entries);
+    });
+  }
+});