Skip to content

Commit 5bac648

Browse files
authored
fix(frontend): treat cancelled scans as non-success in finalizeScan (MCP-2755) (#700)
Codex P3 from #698: the scan-reconcile reducer marked cancelled/canceled as terminal but not an error, so finalizeScan took the success path and could fire the 'Scan Complete' toast for a scan cancelled from another tab or during a cancel/status race. Minimal fix (per PR #700 review direction to avoid scope creep): - decideScanReconcile now flags cancelled/canceled as isCancelled (terminal, non-success), distinct from isError. - finalizeScan suppresses the success toast for a cancelled scan and shows a neutral 'Scan cancelled' info notice instead, via a small pure finalizeToastKind helper. Report/score rendering is left exactly as before (pre-existing, out of scope) — no scanReport clearing, no cancelled panel, no extra ref. Tests: reducer flags cancelled as terminal-non-success; finalizeToastKind — a cancelled scan does NOT fire the success toast. Related #698 Related MCP-2755
1 parent 90f6e7f commit 5bac648

3 files changed

Lines changed: 69 additions & 7 deletions

File tree

frontend/src/utils/scanState.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ export interface ScanReconcileDecision {
4444
finalize: boolean
4545
/** The finalize is due to a failed/errored job (set error, skip success toast). */
4646
isError: boolean
47+
/**
48+
* The finalize is due to a cancelled job (MCP-2755). Terminal but NOT a success:
49+
* the caller suppresses the "Scan Complete" toast. Report/score rendering is
50+
* left unchanged (pre-existing behavior, out of scope).
51+
*/
52+
isCancelled: boolean
4753
/** The job is still running/pending and polling should (continue to) run. */
4854
resumePolling: boolean
4955
}
@@ -67,6 +73,7 @@ export function decideScanReconcile(
6773
return {
6874
finalize: true,
6975
isError: status === 'failed' || status === 'error',
76+
isCancelled: status === 'cancelled' || status === 'canceled',
7077
resumePolling: false,
7178
}
7279
}
@@ -78,13 +85,29 @@ export function decideScanReconcile(
7885
input.jobId !== state.activeScanJobId &&
7986
input.scanPass === 2
8087
) {
81-
return { finalize: true, isError: false, resumePolling: false }
88+
return { finalize: true, isError: false, isCancelled: false, resumePolling: false }
8289
}
8390

8491
// Still in flight.
8592
return {
8693
finalize: false,
8794
isError: false,
95+
isCancelled: false,
8896
resumePolling: status === 'running' || status === 'pending',
8997
}
9098
}
99+
100+
/**
101+
* Which finalize toast to show, if any (MCP-2755). A cancelled scan is terminal but
102+
* NOT a success, so it must NOT fire the "Scan Complete" toast — it gets a neutral
103+
* "Scan cancelled" notice instead. Only fires when a scan was actually in flight in
104+
* this tab (`wasLoading`), so reconciling on a fresh tab-open stays silent. Errors
105+
* use the dedicated error banner, not a toast.
106+
*/
107+
export function finalizeToastKind(
108+
decision: Pick<ScanReconcileDecision, 'isError' | 'isCancelled'>,
109+
wasLoading: boolean,
110+
): 'success' | 'cancelled' | null {
111+
if (!wasLoading || decision.isError) return null
112+
return decision.isCancelled ? 'cancelled' : 'success'
113+
}

frontend/src/views/ServerDetail.vue

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,7 +1249,7 @@ import type { Server, Tool, ToolApproval, SecurityScanReport } from '@/types'
12491249
import api from '@/services/api'
12501250
import { useSecurityScannerStatus } from '@/composables/useSecurityScannerStatus'
12511251
import { serverDisplayName, scanReportPath } from '@/utils/serverRoute'
1252-
import { isTerminalScanStatus, decideScanReconcile } from '@/utils/scanState'
1252+
import { isTerminalScanStatus, decideScanReconcile, finalizeToastKind } from '@/utils/scanState'
12531253
import { selectQuarantinedTools } from '@/utils/toolQuarantine'
12541254
import { oauthSignInState } from '@/utils/health'
12551255
import { computeToolDiffSections } from '@/utils/toolDiff'
@@ -2813,7 +2813,7 @@ function clearScanRunState() {
28132813
// Finalize the scan UI from an authoritative terminal (or newer Pass-2) status.
28142814
// Idempotent: safe to call repeatedly. Only emits the success toast when a scan
28152815
// was actually in flight, so reconciling on a fresh tab-open stays silent.
2816-
async function finalizeScan(data: any, decision: { isError: boolean }) {
2816+
async function finalizeScan(data: any, decision: { isError: boolean; isCancelled: boolean }) {
28172817
const wasLoading = scanLoading.value
28182818
clearScanRunState()
28192819
if (decision.isError) {
@@ -2823,8 +2823,14 @@ async function finalizeScan(data: any, decision: { isError: boolean }) {
28232823
await loadScanReport(true, true)
28242824
await serversStore.fetchServers()
28252825
// server is a computed from the store — no manual reassignment needed.
2826-
if (wasLoading) {
2826+
// MCP-2755: a cancelled scan is terminal but NOT a success — suppress the "Scan
2827+
// Complete" toast and show a neutral "Scan cancelled" notice instead. Report/score
2828+
// rendering is left exactly as before (pre-existing behavior, out of scope).
2829+
const toast = finalizeToastKind(decision, wasLoading)
2830+
if (toast === 'success') {
28272831
systemStore.addToast({ type: 'success', title: 'Scan Complete', message: `Security scan for ${server.value?.name} finished.` })
2832+
} else if (toast === 'cancelled') {
2833+
systemStore.addToast({ type: 'info', title: 'Scan cancelled', message: `Security scan for ${server.value?.name} was cancelled.` })
28282834
}
28292835
}
28302836

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

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect } from 'vitest'
2-
import { isTerminalScanStatus, decideScanReconcile } from '@/utils/scanState'
2+
import { isTerminalScanStatus, decideScanReconcile, finalizeToastKind } from '@/utils/scanState'
33

44
// MCP-2740: A finished security scan stayed stuck on "Scanning… 5/5 complete"
55
// with the Report button disabled because terminal state was only derived from a
@@ -56,8 +56,19 @@ describe('decideScanReconcile (MCP-2740)', () => {
5656
})
5757
})
5858

59-
it('finalizes a cancelled job', () => {
60-
expect(decideScanReconcile({ status: 'cancelled', jobId: 'job-A' }, loadingState).finalize).toBe(true)
59+
it('finalizes a cancelled job as terminal-non-success (MCP-2755)', () => {
60+
for (const status of ['cancelled', 'canceled']) {
61+
const d = decideScanReconcile({ status, jobId: 'job-A' }, loadingState)
62+
expect(d.finalize).toBe(true)
63+
expect(d.isError).toBe(false)
64+
expect(d.isCancelled).toBe(true)
65+
}
66+
})
67+
68+
it('does NOT flag cancellation for completed/failed/running statuses (MCP-2755)', () => {
69+
for (const status of ['completed', 'complete', 'failed', 'error', 'running']) {
70+
expect(decideScanReconcile({ status, jobId: 'job-A' }, loadingState).isCancelled).toBe(false)
71+
}
6172
})
6273

6374
it('finalizes when a newer Pass-2 job is running (Pass 1 is done)', () => {
@@ -93,3 +104,25 @@ describe('decideScanReconcile (MCP-2740)', () => {
93104
// skipPolling=true to suppress this branch and preserve the Pass-1 report.
94105
})
95106
})
107+
108+
// MCP-2755 (Codex P3 from #698): a cancelled scan is terminal but NOT a success, so
109+
// finalizeScan must NOT fire the "Scan Complete" toast for it. This is the minimal
110+
// fix — report/score rendering is intentionally left as-is (out of scope).
111+
describe('finalizeToastKind (MCP-2755)', () => {
112+
it('a cancelled scan does NOT fire the success toast (shows the cancelled notice instead)', () => {
113+
expect(finalizeToastKind({ isError: false, isCancelled: true }, true)).toBe('cancelled')
114+
})
115+
116+
it('a successful scan fires the success toast', () => {
117+
expect(finalizeToastKind({ isError: false, isCancelled: false }, true)).toBe('success')
118+
})
119+
120+
it('stays silent when no scan was in flight (reconcile on a fresh tab-open)', () => {
121+
expect(finalizeToastKind({ isError: false, isCancelled: false }, false)).toBeNull()
122+
expect(finalizeToastKind({ isError: false, isCancelled: true }, false)).toBeNull()
123+
})
124+
125+
it('fires no toast for an error (it uses the error banner)', () => {
126+
expect(finalizeToastKind({ isError: true, isCancelled: false }, true)).toBeNull()
127+
})
128+
})

0 commit comments

Comments
 (0)