diff --git a/frontend/src/utils/scanState.ts b/frontend/src/utils/scanState.ts index df08f49c..cb32b189 100644 --- a/frontend/src/utils/scanState.ts +++ b/frontend/src/utils/scanState.ts @@ -44,6 +44,12 @@ export interface ScanReconcileDecision { finalize: boolean /** The finalize is due to a failed/errored job (set error, skip success toast). */ isError: boolean + /** + * The finalize is due to a cancelled job (MCP-2755). Terminal but NOT a success: + * the caller suppresses the "Scan Complete" toast. Report/score rendering is + * left unchanged (pre-existing behavior, out of scope). + */ + isCancelled: boolean /** The job is still running/pending and polling should (continue to) run. */ resumePolling: boolean } @@ -67,6 +73,7 @@ export function decideScanReconcile( return { finalize: true, isError: status === 'failed' || status === 'error', + isCancelled: status === 'cancelled' || status === 'canceled', resumePolling: false, } } @@ -78,13 +85,29 @@ export function decideScanReconcile( input.jobId !== state.activeScanJobId && input.scanPass === 2 ) { - return { finalize: true, isError: false, resumePolling: false } + return { finalize: true, isError: false, isCancelled: false, resumePolling: false } } // Still in flight. return { finalize: false, isError: false, + isCancelled: false, resumePolling: status === 'running' || status === 'pending', } } + +/** + * Which finalize toast to show, if any (MCP-2755). A cancelled scan is terminal but + * NOT a success, so it must NOT fire the "Scan Complete" toast — it gets a neutral + * "Scan cancelled" notice instead. Only fires when a scan was actually in flight in + * this tab (`wasLoading`), so reconciling on a fresh tab-open stays silent. Errors + * use the dedicated error banner, not a toast. + */ +export function finalizeToastKind( + decision: Pick, + wasLoading: boolean, +): 'success' | 'cancelled' | null { + if (!wasLoading || decision.isError) return null + return decision.isCancelled ? 'cancelled' : 'success' +} diff --git a/frontend/src/views/ServerDetail.vue b/frontend/src/views/ServerDetail.vue index 518174fa..e037a0bd 100644 --- a/frontend/src/views/ServerDetail.vue +++ b/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 } from '@/utils/scanState' +import { isTerminalScanStatus, decideScanReconcile, finalizeToastKind } from '@/utils/scanState' import { selectQuarantinedTools } from '@/utils/toolQuarantine' import { oauthSignInState } from '@/utils/health' import { computeToolDiffSections } from '@/utils/toolDiff' @@ -2813,7 +2813,7 @@ function clearScanRunState() { // 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 }) { +async function finalizeScan(data: any, decision: { isError: boolean; isCancelled: boolean }) { const wasLoading = scanLoading.value clearScanRunState() if (decision.isError) { @@ -2823,8 +2823,14 @@ async function finalizeScan(data: any, decision: { isError: boolean }) { await loadScanReport(true, true) await serversStore.fetchServers() // server is a computed from the store — no manual reassignment needed. - if (wasLoading) { + // MCP-2755: a cancelled scan is terminal but NOT a success — suppress the "Scan + // Complete" toast and show a neutral "Scan cancelled" notice instead. Report/score + // rendering is left exactly as before (pre-existing behavior, out of scope). + const toast = finalizeToastKind(decision, wasLoading) + if (toast === 'success') { systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` }) + } else if (toast === 'cancelled') { + systemStore.addToast({ type: 'info', title: 'Scan cancelled', message: `Security scan for ${server.value?.name} was cancelled.` }) } } diff --git a/frontend/tests/unit/scan-state-reconcile.spec.ts b/frontend/tests/unit/scan-state-reconcile.spec.ts index 900d2d42..d91284cf 100644 --- a/frontend/tests/unit/scan-state-reconcile.spec.ts +++ b/frontend/tests/unit/scan-state-reconcile.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { isTerminalScanStatus, decideScanReconcile } from '@/utils/scanState' +import { isTerminalScanStatus, decideScanReconcile, finalizeToastKind } 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 @@ -56,8 +56,19 @@ describe('decideScanReconcile (MCP-2740)', () => { }) }) - it('finalizes a cancelled job', () => { - expect(decideScanReconcile({ status: 'cancelled', jobId: 'job-A' }, loadingState).finalize).toBe(true) + it('finalizes a cancelled job as terminal-non-success (MCP-2755)', () => { + for (const status of ['cancelled', 'canceled']) { + const d = decideScanReconcile({ status, jobId: 'job-A' }, loadingState) + expect(d.finalize).toBe(true) + expect(d.isError).toBe(false) + expect(d.isCancelled).toBe(true) + } + }) + + it('does NOT flag cancellation for completed/failed/running statuses (MCP-2755)', () => { + for (const status of ['completed', 'complete', 'failed', 'error', 'running']) { + expect(decideScanReconcile({ status, jobId: 'job-A' }, loadingState).isCancelled).toBe(false) + } }) it('finalizes when a newer Pass-2 job is running (Pass 1 is done)', () => { @@ -93,3 +104,25 @@ describe('decideScanReconcile (MCP-2740)', () => { // skipPolling=true to suppress this branch and preserve the Pass-1 report. }) }) + +// MCP-2755 (Codex P3 from #698): a cancelled scan is terminal but NOT a success, so +// finalizeScan must NOT fire the "Scan Complete" toast for it. This is the minimal +// fix — report/score rendering is intentionally left as-is (out of scope). +describe('finalizeToastKind (MCP-2755)', () => { + it('a cancelled scan does NOT fire the success toast (shows the cancelled notice instead)', () => { + expect(finalizeToastKind({ isError: false, isCancelled: true }, true)).toBe('cancelled') + }) + + it('a successful scan fires the success toast', () => { + expect(finalizeToastKind({ isError: false, isCancelled: false }, true)).toBe('success') + }) + + it('stays silent when no scan was in flight (reconcile on a fresh tab-open)', () => { + expect(finalizeToastKind({ isError: false, isCancelled: false }, false)).toBeNull() + expect(finalizeToastKind({ isError: false, isCancelled: true }, false)).toBeNull() + }) + + it('fires no toast for an error (it uses the error banner)', () => { + expect(finalizeToastKind({ isError: true, isCancelled: false }, true)).toBeNull() + }) +})