Skip to content

Commit 37e23d9

Browse files
committed
fix(settings): preserve debug lifecycle ownership
1 parent a9ae62e commit 37e23d9

3 files changed

Lines changed: 186 additions & 77 deletions

File tree

.claude/rules/sim-react-performance.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,15 @@ const [{ id }, { kbName }] = await Promise.all([params, searchParams])
9090

9191
Only keep awaits sequential when a later call genuinely uses an earlier result, or when the ordering is deliberate (rate-limited batches, retry loops, write-then-read).
9292

93+
## Carry exact lifecycle ownership across async boundaries
94+
95+
When asynchronous work can outlive an execution, session, or resource instance, capture its
96+
opaque ownership token before the first `await` and pass that exact token through completion and
97+
error cleanup. Never re-adopt the current owner from delayed cleanup: a replacement may now own
98+
the same scope. End the lifecycle by exact-token match, and clear shared state only when that end
99+
succeeds. Current-owner adoption is reserved for synchronous user actions that explicitly stop
100+
the current lifecycle.
101+
93102
## Prefetch dynamic destination lists on intent
94103

95104
For long lists of dynamic destinations, do not viewport-prefetch every row and do not assume

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88

99
const {
1010
executionStoreState,
11+
idleExecution,
1112
mockCancel,
1213
mockAdoptScopedExecution,
1314
mockBeginScopedExecution,
@@ -89,6 +90,7 @@ const {
8990

9091
return {
9192
executionStoreState,
93+
idleExecution,
9294
mockCancel: vi.fn(),
9395
mockAdoptScopedExecution: vi.fn(),
9496
mockBeginScopedExecution: vi.fn(() => ({})),
@@ -416,9 +418,8 @@ describe('useWorkflowExecution attachment uploads', () => {
416418
beforeEach(() => {
417419
vi.clearAllMocks()
418420
terminalStoreState._hasHydrated = false
419-
executionStoreState.getWorkflowExecution.mockReturnValue(
420-
executionStoreState.workflowExecutions.get('workflow-1')!
421-
)
421+
executionStoreState.workflowExecutions.set('workflow-1', idleExecution)
422+
executionStoreState.getWorkflowExecution.mockReturnValue(idleExecution)
422423
executionStoreState.getCurrentExecutionId.mockReturnValue(null)
423424
mockAdoptScopedExecution.mockReturnValue(undefined)
424425
mockLoadExecutionPointer.mockResolvedValue(null)
@@ -679,6 +680,64 @@ describe('useWorkflowExecution attachment uploads', () => {
679680
unmount()
680681
})
681682

683+
it('does not let delayed debug completion reset a replacement execution', async () => {
684+
const debugPersistenceExecution = {}
685+
const replacementPersistenceExecution = {}
686+
let currentPersistenceExecution: object | undefined = debugPersistenceExecution
687+
let resolveDebugStep: ((result: unknown) => void) | undefined
688+
const continueExecution = vi.fn(
689+
() =>
690+
new Promise((resolve) => {
691+
resolveDebugStep = resolve
692+
})
693+
)
694+
const debugExecution = {
695+
...idleExecution,
696+
status: 'running',
697+
isExecuting: true,
698+
isDebugging: true,
699+
pendingBlocks: ['start'],
700+
executor: { continueExecution },
701+
debugContext: { blockLogs: [] },
702+
}
703+
executionStoreState.workflowExecutions.set('workflow-1', debugExecution)
704+
executionStoreState.getWorkflowExecution.mockReturnValue(debugExecution)
705+
mockAdoptScopedExecution.mockImplementation(() => currentPersistenceExecution)
706+
mockEndScopedExecution.mockImplementation((_workflowId, persistenceExecution) => {
707+
if (persistenceExecution !== currentPersistenceExecution) return false
708+
currentPersistenceExecution = undefined
709+
return true
710+
})
711+
712+
const { result, unmount } = renderWorkflowExecutionHook()
713+
let debugStep: Promise<void>
714+
act(() => {
715+
debugStep = result().handleStepDebug()
716+
})
717+
expect(continueExecution).toHaveBeenCalledOnce()
718+
719+
currentPersistenceExecution = replacementPersistenceExecution
720+
resolveDebugStep?.({ success: true, output: {}, logs: [] })
721+
await act(async () => {
722+
await debugStep
723+
})
724+
725+
expect(mockEndScopedExecution).not.toHaveBeenCalledWith(
726+
'workflow-1',
727+
replacementPersistenceExecution
728+
)
729+
expect(mockClearExecutionPointer).not.toHaveBeenCalled()
730+
expect(executionStoreState.setIsExecuting).not.toHaveBeenCalledWith('workflow-1', false)
731+
expect(executionStoreState.setIsDebugging).not.toHaveBeenCalledWith('workflow-1', false)
732+
expect(executionStoreState.setDebugContext).not.toHaveBeenCalledWith('workflow-1', null)
733+
expect(executionStoreState.setExecutor).not.toHaveBeenCalledWith('workflow-1', null)
734+
expect(executionStoreState.setPendingBlocks).not.toHaveBeenCalledWith('workflow-1', [])
735+
expect(executionStoreState.setActiveBlocks).not.toHaveBeenCalled()
736+
expect(mockRequestJson).not.toHaveBeenCalled()
737+
738+
unmount()
739+
})
740+
682741
it('uses only projected live thinking without changing normal settle behavior', async () => {
683742
mockExecute.mockImplementationOnce(async (options) => {
684743
options.onExecutionId?.('execution-1')

0 commit comments

Comments
 (0)