Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion frontend/src/utils/scanState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -67,6 +73,7 @@ export function decideScanReconcile(
return {
finalize: true,
isError: status === 'failed' || status === 'error',
isCancelled: status === 'cancelled' || status === 'canceled',
resumePolling: false,
}
}
Expand All @@ -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<ScanReconcileDecision, 'isError' | 'isCancelled'>,
wasLoading: boolean,
): 'success' | 'cancelled' | null {
if (!wasLoading || decision.isError) return null
return decision.isCancelled ? 'cancelled' : 'success'
}
12 changes: 9 additions & 3 deletions frontend/src/views/ServerDetail.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand All @@ -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.` })
}
}

Expand Down
39 changes: 36 additions & 3 deletions frontend/tests/unit/scan-state-reconcile.spec.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)', () => {
Expand Down Expand Up @@ -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()
})
})
Loading