From ad54797612df1d2cf22fc3fdb7fcdc520f919ed4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 5 Aug 2026 19:05:41 -0700 Subject: [PATCH] fix(tasks): preserve newer state authority on rollback --- .../src/components/GitHubItemDialog.tsx | 16 +++++++----- ...github-item-dialog-source-boundary.test.ts | 4 +-- ...page-github-dialog-state-authority.test.ts | 26 +++++++++++++++++-- ...task-page-github-dialog-state-authority.ts | 8 +++++- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 6db6d79e076..0e50c3bbc50 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -2862,8 +2862,9 @@ function PRActionsPanel({ ) onMutated() } catch (err) { - authority.revert() - applyStatePatch(previousState) + if (authority.revert()) { + applyStatePatch(previousState) + } toast.error( err instanceof Error ? err.message @@ -4194,7 +4195,7 @@ function GHEditSection({ const prevState = localState // Why: without registry authority a search-lagged Tasks refetch silently // reverts this row to its pre-mutation state (STA-3343). - let authority: { revert: () => void } | null = null + let authority: { revert: () => boolean } | null = null run('state', { mutate: () => runIssueUpdate({ @@ -4220,10 +4221,11 @@ function GHEditSection({ patchProjectRowIfNeeded({ state: newState }) }, onRevert: () => { - authority?.revert() - onStateChange(prevState) - patchWorkItem(item.id, { state: prevState }, item.repoId, { sourceContext }) - patchProjectRowIfNeeded({ state: prevState }) + if (authority?.revert()) { + onStateChange(prevState) + patchWorkItem(item.id, { state: prevState }, item.repoId, { sourceContext }) + patchProjectRowIfNeeded({ state: prevState }) + } }, onSuccess: () => { useAppStore.getState().recordFeatureInteraction('github-tasks') diff --git a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts index 2c8c2cac3fc..a773910d332 100644 --- a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts +++ b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts @@ -249,7 +249,7 @@ describe('GitHubItemDialog source host boundaries', () => { // Issue close/reopen: assert on optimistic apply, revert on failure. const editSection = sourceBetween(source, 'function GHEditSection', 'const closeAsDuplicate') expect(editSection).toContain('assertTaskPageGitHubDialogStateAuthority({') - expect(editSection).toContain('authority?.revert()') + expect(editSection).toContain('if (authority?.revert())') // PR close/reopen + merge: same protection for the shared Tasks list rows. const actionsSection = sourceBetween( @@ -258,7 +258,7 @@ describe('GitHubItemDialog source host boundaries', () => { 'function CommentReplyForm' ) expect(actionsSection.match(/assertTaskPageGitHubDialogStateAuthority\(\{/g)).toHaveLength(2) - expect(actionsSection).toContain('authority.revert()') + expect(actionsSection).toContain('if (authority.revert())') expect(actionsSection).toContain("state: 'merged'") }) }) diff --git a/src/renderer/src/components/task-page-github-dialog-state-authority.test.ts b/src/renderer/src/components/task-page-github-dialog-state-authority.test.ts index 63d5a78313c..2f4d1646d21 100644 --- a/src/renderer/src/components/task-page-github-dialog-state-authority.test.ts +++ b/src/renderer/src/components/task-page-github-dialog-state-authority.test.ts @@ -9,6 +9,7 @@ import { reapplyPendingTaskPageGitHubMutationsToCache } from './task-page-github-work-item-mutations' import { + deleteLastConfirmedClientValue, getLastConfirmedClientValue, getOrCreateQuietRevalidateState, resetTaskPageGitHubMutationRegistryForTests, @@ -150,7 +151,7 @@ describe('dialog state authority (STA-3343)', () => { itemId: 'issue:1', state: 'closed' }) - fresh.revert() + expect(fresh.revert()).toBe(true) expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBeUndefined() setLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state', 'open') @@ -159,7 +160,28 @@ describe('dialog state authority (STA-3343)', () => { itemId: 'issue:1', state: 'closed' }) - layered.revert() + expect(layered.revert()).toBe(true) expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBe('open') }) + + it('does not roll back authority released by search or superseded by a newer state', () => { + setTaskPageGitHubMutationQueryKey('q') + const released = assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + deleteLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state') + expect(released.revert()).toBe(false) + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBeUndefined() + + const superseded = assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + setLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state', 'merged') + expect(superseded.revert()).toBe(false) + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBe('merged') + }) }) diff --git a/src/renderer/src/components/task-page-github-dialog-state-authority.ts b/src/renderer/src/components/task-page-github-dialog-state-authority.ts index 9c2d7132e62..a372762e141 100644 --- a/src/renderer/src/components/task-page-github-dialog-state-authority.ts +++ b/src/renderer/src/components/task-page-github-dialog-state-authority.ts @@ -32,7 +32,7 @@ export function assertTaskPageGitHubDialogStateAuthority(args: { itemId: string state: GitHubWorkItem['state'] sourceContext?: TaskSourceContext | null -}): { revert: () => void } { +}): { revert: () => boolean } { const sourceScope = args.sourceContext?.provider === 'github' ? getTaskSourceCacheScope(args.sourceContext) : null const previous = getLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state') @@ -41,6 +41,11 @@ export function assertTaskPageGitHubDialogStateAuthority(args: { notifyTaskPageGitHubMutationRegistry() return { revert: () => { + const current = getLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state') + // A matching search adopt or newer mutation owns the state now. + if (current !== args.state) { + return false + } if (previous === undefined) { deleteLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state') } else { @@ -48,6 +53,7 @@ export function assertTaskPageGitHubDialogStateAuthority(args: { } markStateFamilyDirty(args.repoId, args.itemId) notifyTaskPageGitHubMutationRegistry() + return true } } }