fix(logs,workspace): prevent cancelled status overwrite on race and move impersonation banner (#4617)

- Guard completeWithError against overwriting a cancelled execution status — cancel route writes cancelled to DB optimistically, but a block error racing the 500ms Redis check could finalize with failed before the engine detects cancellation
- Add tests covering the guard: cancelled DB status skips the write, non-cancelled proceeds normally, DB failure falls through to cost-only fallback, and subsequent attempts are deduped after guard marks session complete
- Move ImpersonationBanner from workspace root into components/ folder
This commit is contained in:
Waleed
2026-05-15 12:14:37 -07:00
committed by GitHub
parent c9118e775b
commit d3d8f9c861
4 changed files with 87 additions and 1 deletions
@@ -1,8 +1,8 @@
import { redirect } from 'next/navigation'
import { ToastProvider } from '@/components/emcn'
import { getSession } from '@/lib/auth'
import { ImpersonationBanner } from '@/app/workspace/[workspaceId]/components/impersonation-banner'
import { NavTour } from '@/app/workspace/[workspaceId]/components/product-tour'
import { ImpersonationBanner } from '@/app/workspace/[workspaceId]/impersonation-banner'
import { GlobalCommandsProvider } from '@/app/workspace/[workspaceId]/providers/global-commands-provider'
import { ProviderModelsLoader } from '@/app/workspace/[workspaceId]/providers/provider-models-loader'
import { SettingsLoader } from '@/app/workspace/[workspaceId]/providers/settings-loader'
@@ -453,6 +453,75 @@ describe('LoggingSession completion retries', () => {
})
})
describe('completeWithError cancelled-status guard', () => {
beforeEach(() => {
vi.clearAllMocks()
dbMocks.updateWhere.mockResolvedValue(undefined)
dbMocks.execute.mockResolvedValue(undefined)
})
it('skips writing failed and marks session complete when DB status is already cancelled', async () => {
dbMocks.selectLimit.mockResolvedValue([{ status: 'cancelled' }])
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
await session.safeCompleteWithError({ error: { message: 'block errored mid-cancel' } })
expect(completeWorkflowExecutionMock).not.toHaveBeenCalled()
expect(session.hasCompleted()).toBe(true)
})
it('writes failed when DB status is running (no cancel in flight)', async () => {
dbMocks.selectLimit.mockResolvedValue([{ status: 'running' }])
completeWorkflowExecutionMock.mockResolvedValue({})
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
await session.safeCompleteWithError({ error: { message: 'genuine block failure' } })
expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
expect.objectContaining({ status: 'failed' })
)
expect(session.hasCompleted()).toBe(true)
})
it('writes failed when no execution log exists yet', async () => {
dbMocks.selectLimit.mockResolvedValue([])
completeWorkflowExecutionMock.mockResolvedValue({})
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
await session.safeCompleteWithError({ error: { message: 'pre-log error' } })
expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
expect.objectContaining({ status: 'failed' })
)
})
it('deduplicates all subsequent completion attempts after guard early-return', async () => {
dbMocks.selectLimit.mockResolvedValue([{ status: 'cancelled' }])
completeWorkflowExecutionMock.mockResolvedValue({})
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
await session.safeCompleteWithError({ error: { message: 'error 1' } })
await session.safeCompleteWithError({ error: { message: 'error 2' } })
await session.safeComplete({ finalOutput: { ok: true } })
expect(completeWorkflowExecutionMock).not.toHaveBeenCalled()
expect(session.hasCompleted()).toBe(true)
})
it('falls through to cost-only fallback when the DB check itself throws', async () => {
dbMocks.selectLimit.mockRejectedValueOnce(new Error('DB connection lost'))
completeWorkflowExecutionMock.mockResolvedValue({})
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
await session.safeCompleteWithError({ error: { message: 'block failed' } })
expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
expect.objectContaining({ finalizationPath: 'force_failed' })
)
expect(session.hasCompleted()).toBe(true)
})
})
describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => {
beforeEach(() => {
vi.clearAllMocks()
@@ -544,6 +544,23 @@ export class LoggingSession {
this.completing = true
try {
const currentLog = await db
.select({ status: workflowExecutionLogs.status })
.from(workflowExecutionLogs)
.where(
and(
eq(workflowExecutionLogs.workflowId, this.workflowId),
eq(workflowExecutionLogs.executionId, this.executionId)
)
)
.limit(1)
.then((rows) => rows[0])
if (currentLog?.status === 'cancelled') {
this.completed = true
return
}
const { endedAt, totalDurationMs, error, traceSpans, skipCost } = params
const endTime = endedAt ? new Date(endedAt) : new Date()