Merge pull request #4142 from sdornan/claude/scan-page-missing-active-display-30873c

fix(v2): show a running scan on pages loaded mid-scan

Author: gantoine

frontend/src/v2/composables/useScanLifecycle/index.test.ts

@@ -0,0 +1,220 @@
+import { mount } from "@vue/test-utils";
+import { createPinia, setActivePinia } from "pinia";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { defineComponent, reactive } from "vue";
+import type { ScanStats } from "@/__generated__";
+import taskApi from "@/services/api/task";
+import storeScanning from "@/stores/scanning";
+import { installScanLifecycle } from "./index";
+
+// Minimal socket stand-in: records handlers so tests can fire events, and
+// stays "connected" so `useSocketEvent` never tries to dial out.
+const handlers = new Map<string, (payload: unknown) => void>();
+vi.mock("@/services/socket", () => ({
+  default: {
+    connected: true,
+    connect: vi.fn(),
+    on: (event: string, handler: (payload: unknown) => void) => {
+      handlers.set(event, handler);
+    },
+    off: vi.fn(),
+  },
+}));
+
+vi.mock("@/services/api/task", () => ({
+  default: { getTaskStatus: vi.fn() },
+}));
+
+// Platform lookups are incidental here: `getPlatform` for platforms the
+// scanning store doesn't know yet, `getPlatforms` for the post-scan reconcile.
+vi.mock("@/services/api/platform", () => ({
+  default: {
+    getPlatform: vi.fn(() => Promise.resolve({ data: { id: 1 } })),
+    getPlatforms: vi.fn(() => Promise.resolve({ data: [] })),
+  },
+}));
+
+// Reactive so the composable's `watch` on `authStore.user` fires when the
+// user arrives after install, which is the real flow: AppLayout installs
+// while /users/me is still in flight.
+const authState = reactive({
+  user: { id: 1, oauth_scopes: ["tasks.run"] } as {
+    id: number;
+    oauth_scopes: string[];
+  } | null,
+});
+vi.mock("@/stores/auth", () => ({
+  default: () => authState,
+}));
+
+const getTaskStatus = vi.mocked(taskApi.getTaskStatus);
+
+/** Drain pending microtasks so the reconcile's promise chain has settled. */
+const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0));
+
+function makeStats(overrides: Partial<ScanStats> = {}): ScanStats {
+  return {
+    total_platforms: 0,
+    total_roms: 0,
+    scanned_platforms: 0,
+    new_platforms: 0,
+    identified_platforms: 0,
+    scanned_roms: 0,
+    new_roms: 0,
+    identified_roms: 0,
+    scanned_firmware: 0,
+    new_firmware: 0,
+    ...overrides,
+  };
+}
+
+function runningScanTask(stats: ScanStats | null) {
+  return {
+    task_name: "scan_platforms",
+    task_id: "job-1",
+    status: "started",
+    task_type: "scan",
+    created_at: null,
+    enqueued_at: null,
+    started_at: null,
+    ended_at: null,
+    meta: { scan_stats: stats },
+  };
+}
+
+// The lifecycle uses `inject` and `onScopeDispose`, so it needs a host
+// component instance. Tracked so `afterEach` can unmount it: the auth state
+// is reactive and shared, so a leaked host would keep watching it and
+// reconcile again during later tests.
+let host: ReturnType<typeof mount> | null = null;
+
+function install() {
+  host = mount(
+    defineComponent({
+      setup() {
+        installScanLifecycle();
+        return () => null;
+      },
+    }),
+  );
+}
+
+function fire(event: string, payload: unknown) {
+  handlers.get(event)?.(payload);
+}
+
+describe("installScanLifecycle", () => {
+  beforeEach(() => {
+    setActivePinia(createPinia());
+    handlers.clear();
+    getTaskStatus.mockReset();
+    getTaskStatus.mockResolvedValue({ data: [] } as never);
+    authState.user = { id: 1, oauth_scopes: ["tasks.run"] };
+  });
+
+  afterEach(() => {
+    host?.unmount();
+    host = null;
+  });
+
+  it("treats a stats event as proof a scan is running", () => {
+    install();
+    const scanning = storeScanning();
+    expect(scanning.scanning).toBe(false);
+
+    fire("scan:update_stats", makeStats({ scanned_roms: 12 }));
+
+    expect(scanning.scanning).toBe(true);
+    expect(scanning.scanStats.scanned_roms).toBe(12);
+  });
+
+  it("reconciles with a running scan job on install", async () => {
+    getTaskStatus.mockResolvedValue({
+      data: [runningScanTask(makeStats({ scanned_roms: 40, total_roms: 100 }))],
+    } as never);
+
+    install();
+    await flushPromises();
+
+    const scanning = storeScanning();
+    expect(scanning.scanning).toBe(true);
+    expect(scanning.scanStats.scanned_roms).toBe(40);
+    expect(scanning.scanStats.total_roms).toBe(100);
+  });
+
+  it("reconciles once the user arrives after install", async () => {
+    // The real flow: AppLayout installs while /users/me is still in flight,
+    // so `user` is null at install and the watch has to catch the arrival.
+    authState.user = null;
+    getTaskStatus.mockResolvedValue({
+      data: [runningScanTask(makeStats({ scanned_roms: 7 }))],
+    } as never);
+
+    install();
+    await flushPromises();
+    expect(getTaskStatus).not.toHaveBeenCalled();
+
+    authState.user = { id: 1, oauth_scopes: ["tasks.run"] };
+    await flushPromises();
+
+    expect(getTaskStatus).toHaveBeenCalledTimes(1);
+    const scanning = storeScanning();
+    expect(scanning.scanning).toBe(true);
+    expect(scanning.scanStats.scanned_roms).toBe(7);
+  });
+
+  it("stays idle when no scan job is running", async () => {
+    getTaskStatus.mockResolvedValue({
+      data: [{ ...runningScanTask(makeStats()), status: "finished" }],
+    } as never);
+
+    install();
+    await flushPromises();
+
+    expect(storeScanning().scanning).toBe(false);
+  });
+
+  it("skips the reconcile without the tasks.run scope", async () => {
+    authState.user = { id: 1, oauth_scopes: ["platforms.write"] };
+
+    install();
+    await flushPromises();
+
+    expect(getTaskStatus).not.toHaveBeenCalled();
+    expect(storeScanning().scanning).toBe(false);
+  });
+
+  it("does not resurrect a scan that ended while the reconcile was in flight", async () => {
+    let resolveStatus!: (value: unknown) => void;
+    getTaskStatus.mockReturnValue(
+      new Promise((resolve) => {
+        resolveStatus = resolve;
+      }) as never,
+    );
+
+    install();
+    fire("scan:done", makeStats({ scanned_roms: 100 }));
+    resolveStatus({ data: [runningScanTask(makeStats({ scanned_roms: 40 }))] });
+    await flushPromises();
+
+    const scanning = storeScanning();
+    expect(scanning.scanning).toBe(false);
+    expect(scanning.scanStats.scanned_roms).toBe(100);
+  });
+
+  it("lets live stats win over the job's snapshot", async () => {
+    let resolveStatus!: (value: unknown) => void;
+    getTaskStatus.mockReturnValue(
+      new Promise((resolve) => {
+        resolveStatus = resolve;
+      }) as never,
+    );
+
+    install();
+    fire("scan:update_stats", makeStats({ scanned_roms: 90 }));
+    resolveStatus({ data: [runningScanTask(makeStats({ scanned_roms: 40 }))] });
+    await flushPromises();
+
+    expect(storeScanning().scanStats.scanned_roms).toBe(90);
+  });
+});

frontend/src/v2/composables/useScanLifecycle/index.ts

@@ -18,14 +18,20 @@
 //   * `scan:done_ko`           — scan errored; surface the message as a
 //                                snackbar and flip `scanning` off.
 //
+// Events alone can't tell a tab that loads mid-scan what's going on, so
+// install also reconciles against the running RQ job — see
+// `reconcileWithRunningScan` below.
+//
 // `useSocketEvent` is the typed subscription wrapper that auto-cleans up
 // on unmount; since AppLayout never unmounts during normal use the
 // listeners effectively live for the session.
 import { debounce } from "lodash";
 import type { Emitter } from "mitt";
-import { inject } from "vue";
-import type { ScanStats } from "@/__generated__";
+import { inject, watch } from "vue";
+import type { ScanStats, ScanTaskStatusResponse } from "@/__generated__";
 import platformApi from "@/services/api/platform";
+import taskApi from "@/services/api/task";
+import storeAuth from "@/stores/auth";
 import storePlatforms from "@/stores/platforms";
 import storeRoms, { type SimpleRom } from "@/stores/roms";
 import storeScanning, { type ScanningPlatform } from "@/stores/scanning";
@@ -38,6 +44,7 @@ export function installScanLifecycle() {
   const romsStore = storeRoms();
   const platformsStore = storePlatforms();
   const galleryRomsStore = storeGalleryRoms();
+  const authStore = storeAuth();
   const emitter = inject<Emitter<Events>>("emitter");
 
   useSocketEvent<ScanningPlatform>(
@@ -162,11 +169,18 @@ export function installScanLifecycle() {
     processRomUpdates();
   });
 
+  // Stats are the only event a scan emits continuously: `scanning_platform`
+  // fires once per platform, and `scanning_rom` only for ROMs the scan
+  // actually adds, so an update scan over a settled library can go a long
+  // while emitting nothing else. Flipping `scanning` here is what lets a tab
+  // that missed the start of the scan catch up on the next tick.
   useSocketEvent<ScanStats>("scan:update_stats", (stats) => {
+    scanningStore.setScanning(true);
     scanningStore.setScanStats(stats);
   });
 
   useSocketEvent<ScanStats>("scan:done", (stats) => {
+    markScanEnded();
     scanningStore.setScanStats(stats);
     scanningStore.setScanning(false);
     // Reconcile against the backend once the scan settles: pick up anything
@@ -181,6 +195,7 @@ export function installScanLifecycle() {
   });
 
   useSocketEvent<string>("scan:done_ko", (msg) => {
+    markScanEnded();
     scanningStore.setScanning(false);
     emitter?.emit("snackbarShow", {
       msg: `Scan failed: ${msg}`,
@@ -189,4 +204,56 @@ export function installScanLifecycle() {
       timeout: 6000,
     });
   });
+
+  // Reconcile with the scan the server is actually running. Without this a
+  // tab that loads mid-scan (refresh, second tab, another device) shows the
+  // /scan empty state and an armed "Start scan" button until an event lands.
+  //
+  // `/tasks/status` needs `tasks.run`, the same scope the `scan` socket
+  // handler gates on, so anyone who could have started this scan can read it
+  // back. Users without it stay purely event-driven.
+  let sawScanEnd = false;
+  function markScanEnded() {
+    sawScanEnd = true;
+  }
+
+  // Reconciles once per eligible user: collapsing the source to an id keeps
+  // unrelated profile updates from re-firing it, and re-arms if the scope
+  // shows up later.
+  watch(
+    () =>
+      authStore.user?.oauth_scopes.includes("tasks.run")
+        ? authStore.user.id
+        : null,
+    (userId) => {
+      if (userId === null) return;
+      sawScanEnd = false;
+      reconcileWithRunningScan();
+    },
+    { immediate: true },
+  );
+
+  function reconcileWithRunningScan() {
+    taskApi
+      .getTaskStatus()
+      .then(({ data }) => {
+        // A terminal event that landed while the request was in flight means
+        // the job we asked about is already over; don't resurrect it. Same for
+        // stats already streaming in, which are fresher than the job's meta.
+        if (sawScanEnd || scanningStore.scanning) return;
+        const running = data.find(
+          (task): task is ScanTaskStatusResponse =>
+            task.task_type === "scan" && task.status === "started",
+        );
+        if (!running) return;
+        scanningStore.setScanning(true);
+        // The per-platform live log only ever lived in the originating tab's
+        // memory, so the panel list fills in from the next platform onward.
+        // `meta` is typed as required but this is a JSON boundary: a missing
+        // snapshot just means no counters yet, not "no scan".
+        if (running.meta?.scan_stats)
+          scanningStore.setScanStats(running.meta.scan_stats);
+      })
+      .catch((error) => console.error(error));
+  }
 }