fix(frontend): security scan UI stuck on 'Scanning…' though backend job completed (MCP-2740) (#698)

* fix(frontend): reconcile security-scan UI against authoritative backend status

The Server Detail → Security tab could stay stuck on "Scanning… N/N complete"
with the Report button disabled indefinitely while the backend job was already
`completed`. `scanLoading` was only cleared inside a live poll tick whose job id
matched `activeScanJobId`. A sub-2s scan can finalize (and be replaced by a
Pass-2 job, or have its id drift) before the first 2s tick, so the matched-job
branch never fired and the spinner never cleared.

Derive terminal state from the authoritative backend status on every status
fetch, independent of `activeScanJobId` / `scan_pass` bookkeeping:

- New pure, unit-tested reducer `frontend/src/utils/scanState.ts`
  (`isTerminalScanStatus`, `decideScanReconcile`).
- Poll tick and initial `loadScanReport` both finalize on a terminal status
  (or a newer running Pass-2 job) regardless of job-id mismatch.
- `loadScanReport` clears a stale `scanLoading` when the latest job is already
  terminal (handles a scan that finished while the component was unmounted).
- Render-independent safety-net watcher so the spinner can never outlive a
  terminal status.
- Add `data-test` hooks (scan-button, scan-progress, scan-report-link) for
  robust Playwright assertions.

Verified with vitest unit tests and a Playwright sweep against the built
(embedded) UI that reproduces the sub-2s / mismatched-job-id race; report under
specs/mcp-2740-scan-ui-stuck/verification/.

Related MCP-2740

* fix(MCP-2740): skip polling on post-finalize reload to prevent Pass-2 re-hiding Pass-1 report

loadScanReport(true) called from finalizeScan and the safety-net watcher
could restart polling when the backend was already running a Pass-2 job,
re-enabling scanLoading and hiding the just-completed Pass-1 report.

Fix: add skipPolling parameter to loadScanReport; callers that invoke it
after clearScanRunState pass skipPolling=true so a concurrent Pass-2 job
does not hijack the scan-loading state.

Adds unit test documenting the post-finalize cleared-state invariant.

Codex P2 finding on PR #698.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>

Author: Dumbris

frontend/src/utils/scanState.ts

@@ -0,0 +1,90 @@
+// Security-scan UI state reconciliation (MCP-2740).
+//
+// The Server Detail → Security tab previously cleared its "Scanning…" spinner
+// (`scanLoading`) only inside a live poll tick whose job id matched the tracked
+// `activeScanJobId`. A scan can finish in <2s — finalizing (and being replaced by
+// a Pass-2 job) before the first 2s tick — so the matched-job branch never fired
+// and the UI stayed stuck on "Scanning… N/N complete" with the Report button
+// disabled indefinitely. The backend job, meanwhile, was already `completed`.
+//
+// The fix derives terminal state from the authoritative backend status on EVERY
+// status fetch, independent of `activeScanJobId` / `scan_pass` bookkeeping. This
+// module holds that decision as a pure, unit-testable reducer.
+
+export const TERMINAL_SCAN_STATUSES = [
+  'completed',
+  'complete',
+  'failed',
+  'error',
+  'cancelled',
+  'canceled',
+] as const
+
+/** True when a backend scan status means the job is finished (no longer running). */
+export function isTerminalScanStatus(status?: string | null): boolean {
+  return !!status && (TERMINAL_SCAN_STATUSES as readonly string[]).includes(status)
+}
+
+export interface ScanReconcileInput {
+  /** Backend job status (e.g. "running", "completed", "failed"). */
+  status?: string | null
+  /** Backend job id for the polled status. */
+  jobId?: string | null
+  /** Scan pass number (1 or 2); may be absent on older payloads. */
+  scanPass?: number | null
+}
+
+export interface ScanReconcileState {
+  scanLoading: boolean
+  activeScanJobId: string | null
+}
+
+export interface ScanReconcileDecision {
+  /** Finalize the UI: stop polling, clear the spinner, surface the report. */
+  finalize: boolean
+  /** The finalize is due to a failed/errored job (set error, skip success toast). */
+  isError: boolean
+  /** The job is still running/pending and polling should (continue to) run. */
+  resumePolling: boolean
+}
+
+/**
+ * Decide how the UI should reconcile against an authoritative backend scan status.
+ *
+ * Pure and idempotent: a terminal status always finalizes, regardless of whether
+ * the polled job id matches the tracked active job or which scan pass it is. A
+ * newer Pass-2 job that is merely running also finalizes, because that means the
+ * tracked Pass-1 work is done.
+ */
+export function decideScanReconcile(
+  input: ScanReconcileInput,
+  state: ScanReconcileState,
+): ScanReconcileDecision {
+  const status = input.status ?? null
+
+  // Authoritative terminal status — finalize regardless of job-id / scan_pass.
+  if (isTerminalScanStatus(status)) {
+    return {
+      finalize: true,
+      isError: status === 'failed' || status === 'error',
+      resumePolling: false,
+    }
+  }
+
+  // A different, newer Pass-2 job is running → the tracked Pass-1 job is done.
+  if (
+    state.activeScanJobId &&
+    input.jobId &&
+    input.jobId !== state.activeScanJobId &&
+    input.scanPass === 2
+  ) {
+    return { finalize: true, isError: false, resumePolling: false }
+  }
+
+  // Still in flight.
+  return {
+    finalize: false,
+    isError: false,
+    resumePolling: status === 'running' || status === 'pending',
+  }
+}

frontend/src/views/ServerDetail.vue

@@ -966,6 +966,7 @@
                   @click="startSecurityScan"
                   :disabled="scanLoading || !dockerAvailable"
                   class="btn btn-primary"
+                  data-test="scan-button"
                 >
                   <span v-if="scanLoading" class="loading loading-spinner loading-xs"></span>
                   <svg v-else class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24">
@@ -1011,7 +1012,7 @@
             </div>
 
             <!-- Scan Progress (visible during active scan) -->
-            <div v-if="scanLoading" class="space-y-3">
+            <div v-if="scanLoading" class="space-y-3" data-test="scan-progress">
               <template v-if="scanProgress && scanProgress.total > 0">
                 <div class="flex items-center justify-between text-sm">
                   <span class="font-medium">Scanning with {{ scanProgress.total }} scanner{{ scanProgress.total !== 1 ? 's' : '' }}...</span>
@@ -1172,7 +1173,7 @@
 
               <!-- Action buttons -->
               <div class="flex gap-3">
-                <router-link v-if="scanReport.job_id" :to="scanReportPath(scanReport.job_id)" class="btn btn-primary btn-sm">
+                <router-link v-if="scanReport.job_id" :to="scanReportPath(scanReport.job_id)" class="btn btn-primary btn-sm" data-test="scan-report-link">
                   View Full Report &rarr;
                 </router-link>
               </div>
@@ -1248,6 +1249,7 @@ import type { Server, Tool, ToolApproval, SecurityScanReport } from '@/types'
 import api from '@/services/api'
 import { useSecurityScannerStatus } from '@/composables/useSecurityScannerStatus'
 import { serverDisplayName, scanReportPath } from '@/utils/serverRoute'
+import { isTerminalScanStatus, decideScanReconcile } from '@/utils/scanState'
 import { selectQuarantinedTools } from '@/utils/toolQuarantine'
 import { oauthSignInState } from '@/utils/health'
 import { computeToolDiffSections } from '@/utils/toolDiff'
@@ -2748,7 +2750,7 @@ async function loadScannerNames() {
   }
 }
 
-async function loadScanReport(force = false) {
+async function loadScanReport(force = false, skipPolling = false) {
   if (!server.value) return
   // Only load if we have a previous scan (skip check when force-loading after scan completion)
   if (!force && !server.value.security_scan?.last_scan_at && !scanReport.value) return
@@ -2772,11 +2774,24 @@ async function loadScanReport(force = false) {
     }
     if (statusRes.success && statusRes.data) {
       scanStatus.value = statusRes.data
-      // If scan is still running (e.g., page reload during scan), resume polling
-      if (statusRes.data.status === 'running' || statusRes.data.status === 'pending') {
+      const decision = decideScanReconcile(
+        { status: statusRes.data.status, jobId: statusRes.data.id, scanPass: statusRes.data.scan_pass },
+        { scanLoading: scanLoading.value, activeScanJobId: activeScanJobId.value },
+      )
+      if (decision.resumePolling && !skipPolling) {
+        // Scan still running (e.g., page reload during scan) — resume polling.
+        // skipPolling=true when called post-finalize: a concurrent Pass-2 job must
+        // not re-enable scanLoading and hide the just-completed Pass-1 report.
         activeScanJobId.value = statusRes.data.id
         scanLoading.value = true
         startScanPolling()
+      } else if (decision.finalize && scanLoading.value) {
+        // MCP-2740: authoritative status is terminal but the spinner is still up
+        // (stale flag from a sub-2s scan or one that finished while unmounted).
+        // Clear it here — no recursive report reload, we already have the freshest
+        // report above.
+        clearScanRunState()
+        if (decision.isError) scanError.value = statusRes.data.error || 'Scan failed'
       }
     }
   } catch (err) {
@@ -2786,6 +2801,33 @@ async function loadScanReport(force = false) {
   }
 }
 
+// Clear the live-scan run state (spinner, polling timer, tracked job id). Pure
+// state reset — does NOT reload the report (callers that need it call separately),
+// which keeps it safe to invoke from inside loadScanReport without recursion.
+function clearScanRunState() {
+  stopScanPolling()
+  scanLoading.value = false
+  activeScanJobId.value = null
+}
+
+// Finalize the scan UI from an authoritative terminal (or newer Pass-2) status.
+// Idempotent: safe to call repeatedly. Only emits the success toast when a scan
+// was actually in flight, so reconciling on a fresh tab-open stays silent.
+async function finalizeScan(data: any, decision: { isError: boolean }) {
+  const wasLoading = scanLoading.value
+  clearScanRunState()
+  if (decision.isError) {
+    scanError.value = data?.error || 'Scan failed'
+    return
+  }
+  await loadScanReport(true, true)
+  await serversStore.fetchServers()
+  // server is a computed from the store — no manual reassignment needed.
+  if (wasLoading) {
+    systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` })
+  }
+}
+
 function startScanPolling() {
   stopScanPolling()
   scanPollTimer = setInterval(async () => {
@@ -2795,38 +2837,16 @@ function startScanPolling() {
       if (statusResp.success && statusResp.data) {
         // Update scan status for live progress display
         scanStatus.value = statusResp.data
-        const jobId = statusResp.data.id
-        const status = statusResp.data.status
-
-        // Only react to the active job (Pass 1). Ignore completed Pass 2 from previous runs.
-        if (activeScanJobId.value && jobId !== activeScanJobId.value) {
-          // Different job — could be Pass 2 starting after Pass 1 completed.
-          if (statusResp.data.scan_pass === 2) {
-            // Pass 2 started or completed — Pass 1 is done. Finish polling.
-            stopScanPolling()
-            scanLoading.value = false
-            activeScanJobId.value = null
-            await loadScanReport(true)
-            await serversStore.fetchServers()
-            // server is a computed from the store — no manual reassignment needed.
-            systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` })
-          }
-          return
-        }
-
-        if (status === 'completed' || status === 'complete') {
-          stopScanPolling()
-          scanLoading.value = false
-          activeScanJobId.value = null
-          await loadScanReport(true)
-          await serversStore.fetchServers()
-          // server is a computed from the store — no manual reassignment needed.
-          systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` })
-        } else if (status === 'failed' || status === 'error') {
-          stopScanPolling()
-          scanLoading.value = false
-          activeScanJobId.value = null
-          scanError.value = statusResp.data.error || 'Scan failed'
+        const decision = decideScanReconcile(
+          { status: statusResp.data.status, jobId: statusResp.data.id, scanPass: statusResp.data.scan_pass },
+          { scanLoading: scanLoading.value, activeScanJobId: activeScanJobId.value },
+        )
+        // MCP-2740: finalize the instant the backend reports a terminal status (or a
+        // newer Pass-2 job), regardless of activeScanJobId / scan_pass — a sub-2s scan
+        // can finalize before the polled id ever matches, which previously left the
+        // "Scanning…" spinner stuck and the Report button disabled forever.
+        if (decision.finalize) {
+          await finalizeScan(statusResp.data, decision)
         }
       }
     } catch {
@@ -2892,6 +2912,17 @@ async function cancelSecurityScan() {
   }
 }
 
+// Safety net (MCP-2740): the spinner must never outlive a terminal backend status.
+// If any path sets scanStatus to a terminal status while scanLoading is still true,
+// reconcile here so the UI can't get stuck on "Scanning…" regardless of poll-timer
+// lifecycle or job-id bookkeeping.
+watch(() => scanStatus.value?.status, (status) => {
+  if (scanLoading.value && isTerminalScanStatus(status)) {
+    clearScanRunState()
+    if (!scanReport.value) loadScanReport(true, true)
+  }
+})
+
 
 // Server detail hints
 const serverDetailHints = computed<Hint[]>(() => {

frontend/tests/unit/scan-state-reconcile.spec.ts

@@ -0,0 +1,95 @@
+import { describe, it, expect } from 'vitest'
+import { isTerminalScanStatus, decideScanReconcile } from '@/utils/scanState'
+
+// MCP-2740: A finished security scan stayed stuck on "Scanning… 5/5 complete"
+// with the Report button disabled because terminal state was only derived from a
+// live poll tick whose job id matched `activeScanJobId`. A fast scan can finalize
+// (and be replaced by a Pass-2 job, or just have its id drift) before the first
+// 2s tick, so the matched-job branch never fires and `scanLoading` stays true
+// forever. The reconciliation below derives terminal state from the authoritative
+// backend status on EVERY status fetch, regardless of job-id / scan_pass bookkeeping.
+
+describe('isTerminalScanStatus (MCP-2740)', () => {
+  it('treats every backend terminal status as terminal', () => {
+    for (const s of ['completed', 'complete', 'failed', 'error', 'cancelled', 'canceled']) {
+      expect(isTerminalScanStatus(s)).toBe(true)
+    }
+  })
+
+  it('treats live/unknown statuses as non-terminal', () => {
+    for (const s of ['running', 'pending', '', undefined, null]) {
+      expect(isTerminalScanStatus(s as any)).toBe(false)
+    }
+  })
+})
+
+describe('decideScanReconcile (MCP-2740)', () => {
+  const loadingState = { scanLoading: true, activeScanJobId: 'job-A' }
+
+  it('finalizes a completed job whose id matches the active job', () => {
+    const d = decideScanReconcile({ status: 'completed', jobId: 'job-A', scanPass: 1 }, loadingState)
+    expect(d.finalize).toBe(true)
+    expect(d.isError).toBe(false)
+    expect(d.resumePolling).toBe(false)
+  })
+
+  it('finalizes a completed job whose id DIFFERS from the active job (the stuck-UI bug)', () => {
+    // Previously: jobId !== activeScanJobId && scan_pass !== 2 → early return, never finalized.
+    const d = decideScanReconcile({ status: 'completed', jobId: 'job-B', scanPass: 1 }, loadingState)
+    expect(d.finalize).toBe(true)
+    expect(d.isError).toBe(false)
+  })
+
+  it('finalizes a completed job even when scan_pass is absent', () => {
+    const d = decideScanReconcile({ status: 'complete', jobId: 'job-B' }, loadingState)
+    expect(d.finalize).toBe(true)
+  })
+
+  it('finalizes and flags an error for failed/error status', () => {
+    expect(decideScanReconcile({ status: 'failed', jobId: 'job-A' }, loadingState)).toMatchObject({
+      finalize: true,
+      isError: true,
+    })
+    expect(decideScanReconcile({ status: 'error', jobId: 'job-A' }, loadingState)).toMatchObject({
+      finalize: true,
+      isError: true,
+    })
+  })
+
+  it('finalizes a cancelled job', () => {
+    expect(decideScanReconcile({ status: 'cancelled', jobId: 'job-A' }, loadingState).finalize).toBe(true)
+  })
+
+  it('finalizes when a newer Pass-2 job is running (Pass 1 is done)', () => {
+    const d = decideScanReconcile({ status: 'running', jobId: 'job-B', scanPass: 2 }, loadingState)
+    expect(d.finalize).toBe(true)
+    expect(d.isError).toBe(false)
+  })
+
+  it('keeps polling for a still-running active job', () => {
+    const d = decideScanReconcile({ status: 'running', jobId: 'job-A', scanPass: 1 }, loadingState)
+    expect(d.finalize).toBe(false)
+    expect(d.resumePolling).toBe(true)
+  })
+
+  it('keeps polling for a pending job', () => {
+    const d = decideScanReconcile({ status: 'pending', jobId: 'job-A' }, loadingState)
+    expect(d.finalize).toBe(false)
+    expect(d.resumePolling).toBe(true)
+  })
+
+  // Codex P2 (PR #698): after Pass-1 finalizes, loadScanReport(true) is called to
+  // refresh the report. At that point scanLoading=false and activeScanJobId=null.
+  // If the backend is already running Pass-2, decideScanReconcile correctly returns
+  // resumePolling=true — but the caller must pass skipPolling=true so it does NOT
+  // re-enable scanLoading and hide the just-completed Pass-1 report.
+  // This test documents that invariant: the cleared state + running Pass-2 → resumePolling.
+  it('returns resumePolling for a running Pass-2 seen after scan state was cleared (post-finalize)', () => {
+    const clearedState = { scanLoading: false, activeScanJobId: null }
+    const d = decideScanReconcile({ status: 'running', jobId: 'job-B', scanPass: 2 }, clearedState)
+    expect(d.finalize).toBe(false)
+    expect(d.resumePolling).toBe(true)
+    // Callers that invoke loadScanReport after clearScanRunState MUST pass
+    // skipPolling=true to suppress this branch and preserve the Pass-1 report.
+  })
+})