fix(frontend): complete cancel-finalize contract — clear stale report on cancel

Codex round-2 P2 on PR #700: a tab opened mid-scan loads a previous report,
and a cancelled scan may produce none — so finalizeScan must also CLEAR the
rendered report on cancel, not just skip the reload. Otherwise the Security
tab renders a stale report after a cancel.

Add a clearReport effect to scanFinalizeEffects (true for cancel) and extract
applyScanFinalize() — the ordered side-effect applier — out of the component
so the full contract is test-driven against fake handlers. finalizeScan now
wires its refs/stores into applyScanFinalize.

Full cancel contract now: stopScanPolling + clearScanRunState (spinner off),
clear scanReport (no stale report), refresh server store (status not
'scanning'), 'Scan cancelled' toast, NO success toast, NO report reload.

New behavioral test: tab-open-mid-scan stale report → cancelled via poll →
asserts scanLoading=false, scanReport cleared, store refreshed, no success
toast. plus success/error contract tests.

Related #700
Related MCP-2755

Author: Dumbris

frontend/src/utils/scanState.ts

@@ -102,6 +102,13 @@ export function decideScanReconcile(
 export interface ScanFinalizeEffects {
   /** Set the scan-error banner and stop (failed/errored job). */
   setError: boolean
+  /**
+   * Clear the currently-rendered report. Required for cancellation: a tab opened
+   * mid-scan may have a previous (stale) report loaded, and a cancelled scan may
+   * have produced none — so the stale one must be dropped rather than left on
+   * screen or reloaded (MCP-2755).
+   */
+  clearReport: boolean
   /** Reload the report (only for a successful scan that produced one). */
   reloadReport: boolean
   /**
@@ -130,15 +137,55 @@ export function scanFinalizeEffects(
   wasLoading: boolean,
 ): ScanFinalizeEffects {
   if (decision.isError) {
-    // Failed/errored: surface the error and short-circuit (no reload / refresh / toast).
-    return { setError: true, reloadReport: false, refreshServers: false, successToast: false, cancelledToast: false }
+    // Failed/errored: surface the error and short-circuit (no clear / reload / refresh / toast).
+    return { setError: true, clearReport: false, reloadReport: false, refreshServers: false, successToast: false, cancelledToast: false }
   }
   if (decision.isCancelled) {
-    // Terminal but NOT a success: refresh the store so the cancelled status clears
-    // the spinner, but do NOT reload a report that may be stale or absent.
-    return { setError: false, reloadReport: false, refreshServers: true, successToast: false, cancelledToast: wasLoading }
+    // Terminal but NOT a success: drop any stale report and refresh the store so the
+    // cancelled status clears the spinner — but do NOT reload a report that may be
+    // stale or absent.
+    return { setError: false, clearReport: true, reloadReport: false, refreshServers: true, successToast: false, cancelledToast: wasLoading }
   }
   // Success: refresh the freshest report + store, and announce completion if a
   // scan was actually running in this tab.
-  return { setError: false, reloadReport: true, refreshServers: true, successToast: wasLoading, cancelledToast: false }
+  return { setError: false, clearReport: false, reloadReport: true, refreshServers: true, successToast: wasLoading, cancelledToast: false }
+}
+
+/** Imperative side-effect handlers the host component wires to its own refs/stores. */
+export interface ScanFinalizeHandlers {
+  /** Drop the currently-rendered report (e.g. `scanReport.value = null`). */
+  clearReport: () => void
+  /** Reload the freshest report from the backend. */
+  reloadReport: () => Promise<void> | void
+  /** Refresh the server store so derived status leaves "scanning". */
+  refreshServers: () => Promise<void> | void
+  /** Set the scan-error banner from the terminal payload. */
+  setError: () => void
+  /** Show the "Scan Complete" success toast. */
+  successToast: () => void
+  /** Show the neutral "Scan cancelled" info toast. */
+  cancelledToast: () => void
+}
+
+/**
+ * Apply a finalize decision's side-effects in the one correct order. Extracted from
+ * the (heavy) ServerDetail component so the full cancel-finalize contract is driven
+ * by tests against fake handlers, not just asserted as flags (MCP-2755).
+ *
+ * Order matters: an error short-circuits; otherwise the stale report is cleared
+ * BEFORE any (success-only) reload, the store is refreshed, then toasts fire last.
+ */
+export async function applyScanFinalize(
+  effects: ScanFinalizeEffects,
+  handlers: ScanFinalizeHandlers,
+): Promise<void> {
+  if (effects.setError) {
+    handlers.setError()
+    return
+  }
+  if (effects.clearReport) handlers.clearReport()
+  if (effects.reloadReport) await handlers.reloadReport()
+  if (effects.refreshServers) await handlers.refreshServers()
+  if (effects.successToast) handlers.successToast()
+  if (effects.cancelledToast) handlers.cancelledToast()
 }

frontend/src/views/ServerDetail.vue

@@ -1249,7 +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, scanFinalizeEffects } from '@/utils/scanState'
+import { isTerminalScanStatus, decideScanReconcile, scanFinalizeEffects, applyScanFinalize } from '@/utils/scanState'
 import { selectQuarantinedTools } from '@/utils/toolQuarantine'
 import { oauthSignInState } from '@/utils/health'
 import { computeToolDiffSections } from '@/utils/toolDiff'
@@ -2815,27 +2815,23 @@ function clearScanRunState() {
 // was actually in flight, so reconciling on a fresh tab-open stays silent.
 async function finalizeScan(data: any, decision: { isError: boolean; isCancelled: boolean }) {
   const wasLoading = scanLoading.value
+  // clearScanRunState() stops polling and drops scanLoading + activeScanJobId.
   clearScanRunState()
-  // MCP-2755: derive the side-effects per terminal outcome from a pure, tested
-  // reducer. A cancelled scan is terminal but NOT a success: it may not have
-  // produced a report (so we skip the reload), yet the store MUST still be
-  // refreshed — otherwise, after clearScanRunState() drops scanLoading, the
-  // Security tab keeps deriving "scanning" from the stale store and the spinner
-  // never clears.
+  // MCP-2755: derive the per-outcome side-effects from a pure, tested reducer and
+  // apply them in the one correct order. A cancelled scan is terminal but NOT a
+  // success: it may not have produced a report, and a tab opened mid-scan may have
+  // a previous (stale) report loaded — so we CLEAR the report and refresh the store
+  // (clearing the spinner) WITHOUT reloading or showing the success toast.
   const effects = scanFinalizeEffects(decision, wasLoading)
-  if (effects.setError) {
-    scanError.value = data?.error || 'Scan failed'
-    return
-  }
-  if (effects.reloadReport) await loadScanReport(true, true)
-  if (effects.refreshServers) await serversStore.fetchServers()
-  // server is a computed from the store — no manual reassignment needed.
-  if (effects.successToast) {
-    systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` })
-  }
-  if (effects.cancelledToast) {
-    systemStore.addToast({ type: 'info', title: 'Scan cancelled', message: `Security scan for ${server.value?.name} was cancelled.` })
-  }
+  await applyScanFinalize(effects, {
+    clearReport: () => { scanReport.value = null },
+    reloadReport: () => loadScanReport(true, true),
+    refreshServers: () => serversStore.fetchServers(),
+    setError: () => { scanError.value = data?.error || 'Scan failed' },
+    // server is a computed from the store — no manual reassignment needed.
+    successToast: () => systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` }),
+    cancelledToast: () => systemStore.addToast({ type: 'info', title: 'Scan cancelled', message: `Security scan for ${server.value?.name} was cancelled.` }),
+  })
 }
 
 function startScanPolling() {

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

@@ -1,5 +1,10 @@
-import { describe, it, expect } from 'vitest'
-import { isTerminalScanStatus, decideScanReconcile, scanFinalizeEffects } from '@/utils/scanState'
+import { describe, it, expect, vi } from 'vitest'
+import {
+  isTerminalScanStatus,
+  decideScanReconcile,
+  scanFinalizeEffects,
+  applyScanFinalize,
+} 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
@@ -152,6 +157,16 @@ describe('scanFinalizeEffects (MCP-2755)', () => {
     expect(e.successToast).toBe(false)
   })
 
+  it('cancelled clears any previously-loaded (stale) report', () => {
+    expect(scanFinalizeEffects({ isError: false, isCancelled: true }, true).clearReport).toBe(true)
+    expect(scanFinalizeEffects({ isError: false, isCancelled: true }, false).clearReport).toBe(true)
+  })
+
+  it('success/error do NOT clear the report (success reloads it, error keeps current)', () => {
+    expect(scanFinalizeEffects({ isError: false, isCancelled: false }, true).clearReport).toBe(false)
+    expect(scanFinalizeEffects({ isError: true, isCancelled: false }, true).clearReport).toBe(false)
+  })
+
   it('error: set error and short-circuit (no reload, no toast)', () => {
     const e = scanFinalizeEffects({ isError: true, isCancelled: false }, true)
     expect(e.setError).toBe(true)
@@ -161,3 +176,70 @@ describe('scanFinalizeEffects (MCP-2755)', () => {
     expect(e.cancelledToast).toBe(false)
   })
 })
+
+// MCP-2755 (Codex round-2 P2 on PR #700): the full cancel-finalize contract.
+// Drives the real finalize side-effect applier against fake refs/handlers so the
+// behavior — not just the effect flags — is asserted end to end.
+describe('applyScanFinalize — cancel-finalize contract (MCP-2755)', () => {
+  it('tab opened mid-scan (stale report loaded) → cancelled via poll: spinner+report cleared, store refreshed, no success toast', async () => {
+    // Tab was opened while a scan was running: a previous (stale) report is on
+    // screen and the spinner is up.
+    const state = { scanLoading: true, activeScanJobId: 'job-A', scanReport: { stale: true } as unknown, scanError: '' }
+    const handlers = {
+      clearReport: vi.fn(() => { state.scanReport = null }),
+      reloadReport: vi.fn(async () => { state.scanReport = { fresh: true } }),
+      refreshServers: vi.fn(async () => {}),
+      setError: vi.fn(() => { state.scanError = 'boom' }),
+      successToast: vi.fn(),
+      cancelledToast: vi.fn(),
+    }
+
+    // finalizeScan sequence: snapshot wasLoading, then clearScanRunState() drops
+    // the spinner + tracked job, then apply the per-outcome effects.
+    const wasLoading = state.scanLoading
+    state.scanLoading = false
+    state.activeScanJobId = null
+    const decision = decideScanReconcile(
+      { status: 'cancelled', jobId: 'job-B' }, // a sub-2s/other-tab cancel: id need not match
+      { scanLoading: wasLoading, activeScanJobId: 'job-A' },
+    )
+    await applyScanFinalize(scanFinalizeEffects(decision, wasLoading), handlers)
+
+    expect(state.scanLoading).toBe(false)      // spinner gone
+    expect(state.scanReport).toBeNull()        // stale report cleared, NOT replaced
+    expect(handlers.clearReport).toHaveBeenCalledTimes(1)
+    expect(handlers.reloadReport).not.toHaveBeenCalled() // no report reload
+    expect(handlers.refreshServers).toHaveBeenCalledTimes(1) // store refreshed → status not 'scanning'
+    expect(handlers.successToast).not.toHaveBeenCalled() // no 'Scan Complete'
+    expect(handlers.cancelledToast).toHaveBeenCalledTimes(1)
+    expect(handlers.setError).not.toHaveBeenCalled()
+    expect(state.scanError).toBe('')
+  })
+
+  it('success: reloads the report and refreshes the store (no clear, no cancelled toast)', async () => {
+    const handlers = {
+      clearReport: vi.fn(), reloadReport: vi.fn(async () => {}), refreshServers: vi.fn(async () => {}),
+      setError: vi.fn(), successToast: vi.fn(), cancelledToast: vi.fn(),
+    }
+    await applyScanFinalize(scanFinalizeEffects({ isError: false, isCancelled: false }, true), handlers)
+    expect(handlers.clearReport).not.toHaveBeenCalled()
+    expect(handlers.reloadReport).toHaveBeenCalledTimes(1)
+    expect(handlers.refreshServers).toHaveBeenCalledTimes(1)
+    expect(handlers.successToast).toHaveBeenCalledTimes(1)
+    expect(handlers.cancelledToast).not.toHaveBeenCalled()
+  })
+
+  it('error: only sets the error (no clear/reload/refresh/toast)', async () => {
+    const handlers = {
+      clearReport: vi.fn(), reloadReport: vi.fn(), refreshServers: vi.fn(),
+      setError: vi.fn(), successToast: vi.fn(), cancelledToast: vi.fn(),
+    }
+    await applyScanFinalize(scanFinalizeEffects({ isError: true, isCancelled: false }, true), handlers)
+    expect(handlers.setError).toHaveBeenCalledTimes(1)
+    expect(handlers.clearReport).not.toHaveBeenCalled()
+    expect(handlers.reloadReport).not.toHaveBeenCalled()
+    expect(handlers.refreshServers).not.toHaveBeenCalled()
+    expect(handlers.successToast).not.toHaveBeenCalled()
+    expect(handlers.cancelledToast).not.toHaveBeenCalled()
+  })
+})