From 0cbf60acd6bb4a5aebc3472472d582f831461acb Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:13:33 -0700 Subject: [PATCH] Reuse creationKey across move retries when schedule changes When retrying a failed automation move, dtstart is minted fresh each attempt, changing the payload. Previously, operationKey included the full payload, so retries would mint new creationKeys and risk duplicate automations on the destination if the initial create failed in transport. Now key only by the move (source + destination) to ensure stable creationKey across retries. Also fix workspace auto-selection to use authority-scoped worktrees instead of the merged cache, preventing unwanted restoration of source-host workspaces after switching authorities. --- ...utomationsPage.create-destination.test.tsx | 64 +++++++++++++++++++ .../automations/AutomationsPage.tsx | 15 +++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx b/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx index 93ff1adc93b..fdfdb7d7ff0 100644 --- a/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.create-destination.test.tsx @@ -528,6 +528,70 @@ describe('AutomationsPage edit destination', () => { expect(api.automations.delete).toHaveBeenCalled() }) + it('reuses the move creation key on retry even when the schedule changed', async () => { + const automation = makeAutomation({ id: 'a-1' }) + api.automations.list.mockResolvedValue([]) + scopedList([automation]) + runtimeHost([], []) + addRuntimeProject() + // The first create dies in transport after the server may have committed; + // only an identical creationKey on the retry keeps that copy singular. + let failCreate = true + const previous = mocks.callRuntimeRpc.getMockImplementation() + mocks.callRuntimeRpc.mockImplementation( + async (target: unknown, method: string, params: unknown, options: unknown) => { + if (method === 'automation.create') { + if (failCreate) { + failCreate = false + throw new Error('socket hang up') + } + return { automation: makeAutomation({ id: 'a-moved', projectId: RUNTIME_REPO_ID }) } + } + return await previous?.(target, method, params, options) + } + ) + + await renderPage() + await settleHostQueries() + await act(async () => { + void mocks.listPanel?.openEditDialog(listedRow(automation.id)) + }) + await act(async () => { + mocks.editorDialog?.editDestination?.onSelect(RUNTIME_SELF_KEY) + }) + await act(async () => { + mocks.editorDialog?.onDraftChange((current) => ({ + ...(current as Record), + projectId: RUNTIME_REPO_ID, + workspaceMode: 'existing', + workspaceId: RUNTIME_WORKSPACE_ID, + // A changed schedule mints dtstart per attempt — the volatile input that + // must not re-key the move. + preset: 'custom', + customSchedule: 'FREQ=DAILY;BYHOUR=10;BYMINUTE=0' + })) + }) + + const realNow = Date.now.bind(Date) + let clockSkew = 0 + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => realNow() + clockSkew) + try { + await save() + clockSkew = 5000 + await save() + } finally { + nowSpy.mockRestore() + } + + const creationKeys = runtimeCreateCalls().map( + (call) => (call[2] as { creationKey?: string }).creationKey + ) + expect(creationKeys).toHaveLength(2) + expect(creationKeys[0]).toBeTruthy() + expect(creationKeys[1]).toBe(creationKeys[0]) + expect(api.automations.delete).toHaveBeenCalledWith(expect.objectContaining({ id: 'a-1' })) + }) + it('reports a kept original as two live copies, never as a completed move', async () => { const automation = makeAutomation({ id: 'a-1' }) api.automations.list.mockResolvedValue([]) diff --git a/src/renderer/src/components/automations/AutomationsPage.tsx b/src/renderer/src/components/automations/AutomationsPage.tsx index 2f375024371..69d69c815ea 100644 --- a/src/renderer/src/components/automations/AutomationsPage.tsx +++ b/src/renderer/src/components/automations/AutomationsPage.tsx @@ -1345,12 +1345,13 @@ export default function AutomationsPage(): React.JSX.Element { if (!draft.projectId) { return } - const available = worktreesByRepo[draft.projectId] ?? [] - const defaultWorktree = getDefaultWorktree(available) + // Authority-scoped: the merged cache would restore a source-host workspace + // after a host change clears it, which save validation then rejects. + const defaultWorktree = getDefaultWorktree(dialogWorktrees) if (!draft.workspaceId && defaultWorktree) { setDraft((current) => ({ ...current, workspaceId: defaultWorktree.id })) } - }, [draft.projectId, draft.workspaceId, worktreesByRepo]) + }, [dialogWorktrees, draft.projectId, draft.workspaceId]) useEffect(() => { if ( @@ -2026,7 +2027,8 @@ export default function AutomationsPage(): React.JSX.Element { }) setDraft((current) => ({ ...current, name: '', prompt: '' })) await refresh() - // A move creates a new row on another authority; select it by its new id. + // A move retires the edited row key; the else branch selects the new copy + // by id alone and lets the refreshed list resolve its row. if (editingAutomationId && editingRowKey && !moveTarget) { setSelectedAutomationRunPageId(null) setSelectedRowKey(editingRowKey) @@ -2094,7 +2096,10 @@ export default function AutomationsPage(): React.JSX.Element { originalRemoved: false } } - const operationKey = `${source.id}:${target.entry.stableKey}:${JSON.stringify(input)}` + // Keyed by the move, not its payload: dtstart is minted per attempt, so a + // payload-keyed retry would mint a fresh creationKey and let an ambiguous + // create failure schedule a second copy on the destination. + const operationKey = `${source.id}:${target.entry.stableKey}` const creationKey = moveCreationKeysRef.current.get(operationKey) ?? crypto.randomUUID() moveCreationKeysRef.current.set(operationKey, creationKey) const created = toDispatchResult(