diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 919b908f375..6db6d79e076 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -210,6 +210,7 @@ import { validateTaskPageGitHubDuplicateTarget, type TaskPageGitHubCloseAction } from '@/components/task-page-github-status-actions' +import { assertTaskPageGitHubDialogStateAuthority } from '@/components/task-page-github-dialog-state-authority' import { sortChecksBySeverity } from '../../../shared/pr-check-severity-order' import { getCheckConclusion, @@ -2834,6 +2835,14 @@ function PRActionsPanel({ } const previousState = localState setStatePending(true) + // Why: without registry authority a search-lagged Tasks refetch silently + // reverts this row to its pre-mutation state (STA-3343). + const authority = assertTaskPageGitHubDialogStateAuthority({ + repoId: item.repoId, + itemId: item.id, + state: nextState, + sourceContext + }) applyStatePatch(nextState) try { await runPullRequestStateUpdate({ @@ -2853,6 +2862,7 @@ function PRActionsPanel({ ) onMutated() } catch (err) { + authority.revert() applyStatePatch(previousState) toast.error( err instanceof Error @@ -2913,6 +2923,13 @@ function PRActionsPanel({ toast.error(result.error) return } + // Why: merge is confirmed here; hold 'merged' against search-lagged refetches. + assertTaskPageGitHubDialogStateAuthority({ + repoId: item.repoId, + itemId: item.id, + state: 'merged', + sourceContext + }) applyStatePatch('merged') if (mergeTarget.kind === 'environment') { notifyWorkItemDetailsMutation( @@ -4175,6 +4192,9 @@ function GHEditSection({ return } 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 run('state', { mutate: () => runIssueUpdate({ @@ -4189,11 +4209,18 @@ function GHEditSection({ : { state: newState } }), onOptimistic: () => { + authority = assertTaskPageGitHubDialogStateAuthority({ + repoId: item.repoId, + itemId: item.id, + state: newState, + sourceContext + }) onStateChange(newState) patchWorkItem(item.id, { state: newState }, item.repoId, { sourceContext }) patchProjectRowIfNeeded({ state: newState }) }, onRevert: () => { + authority?.revert() onStateChange(prevState) patchWorkItem(item.id, { state: prevState }, item.repoId, { sourceContext }) patchProjectRowIfNeeded({ state: prevState }) 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 4aa11354059..2c8c2cac3fc 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 @@ -242,4 +242,23 @@ describe('GitHubItemDialog source host boundaries', () => { expect(checksTab).toContain('item={displayWorkItem ?? workItem}') }) + + it('records state authority for dialog state mutations so stale list refetches cannot revert them (STA-3343)', () => { + const source = componentSource('GitHubItemDialog.tsx') + + // 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()') + + // PR close/reopen + merge: same protection for the shared Tasks list rows. + const actionsSection = sourceBetween( + source, + 'function PRActionsPanel', + 'function CommentReplyForm' + ) + expect(actionsSection.match(/assertTaskPageGitHubDialogStateAuthority\(\{/g)).toHaveLength(2) + expect(actionsSection).toContain('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 new file mode 100644 index 00000000000..63d5a78313c --- /dev/null +++ b/src/renderer/src/components/task-page-github-dialog-state-authority.test.ts @@ -0,0 +1,165 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { GitHubWorkItem } from '../../../shared/types' +import { getTaskSourceCacheScope } from '../../../shared/task-source-context' +import { assertTaskPageGitHubDialogStateAuthority } from './task-page-github-dialog-state-authority' +import { + adoptQuietSearchFieldsForItem, + applyPendingTaskPageGitHubMutationsToItems, + materializeTaskPageItemList, + reapplyPendingTaskPageGitHubMutationsToCache +} from './task-page-github-work-item-mutations' +import { + getLastConfirmedClientValue, + getOrCreateQuietRevalidateState, + resetTaskPageGitHubMutationRegistryForTests, + setLastConfirmedClientValue, + setTaskPageGitHubMutationQueryKey, + taskPageGitHubFamilyDirtyKey, + taskPageGitHubItemKey +} from './task-page-github-work-item-mutation-registry' + +function item(overrides: Partial = {}): GitHubWorkItem { + return { + id: 'issue:1', + type: 'issue', + number: 1, + title: 't', + state: 'open', + url: 'https://github.com/o/r/issues/1', + labels: [], + updatedAt: '2026-01-01T00:00:00Z', + author: 'author', + repoId: 'repo-1', + ...overrides + } +} + +const sourceContext = { + kind: 'task-source' as const, + provider: 'github' as const, + hostId: 'local' as const, + projectId: 'proj', + projectHostSetupId: 'setup', + repoId: 'repo-1' +} + +afterEach(() => resetTaskPageGitHubMutationRegistryForTests()) + +describe('dialog state authority (STA-3343)', () => { + it('without authority a stale search refetch reverts a cache-only patch (external edits adopt)', () => { + // Why: documents the pre-fix dialog behavior — patchWorkItem alone gives the + // row no registry protection, so the lagging search response wins. + const materialized = materializeTaskPageItemList({ + networkItems: [item({ state: 'open' })], + previousItems: [item({ state: 'closed' })], + queryKey: 'q' + }) + expect(materialized[0]?.state).toBe('open') + }) + + it('holds a dialog-confirmed close over a stale search refetch', () => { + setTaskPageGitHubMutationQueryKey('q') + assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + const materialized = materializeTaskPageItemList({ + networkItems: [item({ state: 'open' })], + previousItems: [item({ state: 'closed' })], + queryKey: 'q' + }) + expect(materialized[0]?.state).toBe('closed') + }) + + it('holds the close through the cache re-apply path too', () => { + setTaskPageGitHubMutationQueryKey('q') + assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + const store = new Map([['issue:1', item({ state: 'open' })]]) + reapplyPendingTaskPageGitHubMutationsToCache({ + items: [item({ state: 'open' })], + patchWorkItem: (id, patch) => { + store.set(id, { ...(store.get(id) ?? item()), ...patch }) + } + }) + expect(store.get('issue:1')?.state).toBe('closed') + }) + + it('records authority under the mutation source scope so other scopes are untouched', () => { + setTaskPageGitHubMutationQueryKey('q') + assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed', + sourceContext + }) + const scope = getTaskSourceCacheScope(sourceContext) + expect(getLastConfirmedClientValue(scope, 'repo-1', 'issue:1', 'state')).toBe('closed') + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBeUndefined() + // Why: overlay resolves the remembered scope, so the list row still holds. + const overlaid = applyPendingTaskPageGitHubMutationsToItems([item({ state: 'open' })]) + expect(overlaid[0]?.state).toBe('closed') + }) + + it('marks the state family dirty so quiet revalidation trails the stale row', () => { + setTaskPageGitHubMutationQueryKey('q') + const before = getOrCreateQuietRevalidateState('q').dirtyGeneration + assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + const quiet = getOrCreateQuietRevalidateState('q') + expect(quiet.dirtyGeneration).toBe(before + 1) + expect( + quiet.familyDirtyAt.get( + taskPageGitHubFamilyDirtyKey(taskPageGitHubItemKey('repo-1', 'issue:1'), 'state') + ) + ).toBe(quiet.dirtyGeneration) + }) + + it('releases authority once search reports the confirmed state, so later external edits win', () => { + setTaskPageGitHubMutationQueryKey('q') + assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + const patched: Partial[] = [] + adoptQuietSearchFieldsForItem({ + item: item({ state: 'closed' }), + serverItem: item({ state: 'closed' }), + sourceScope: null, + queryKey: 'q', + fetchStartedAtGeneration: getOrCreateQuietRevalidateState('q').dirtyGeneration, + patchWorkItem: (_id, patch) => patched.push(patch) + }) + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBeUndefined() + const overlaid = applyPendingTaskPageGitHubMutationsToItems([item({ state: 'open' })]) + expect(overlaid[0]?.state).toBe('open') + }) + + it('revert drops fresh authority and restores a pre-existing value', () => { + setTaskPageGitHubMutationQueryKey('q') + const fresh = assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + fresh.revert() + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBeUndefined() + + setLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state', 'open') + const layered = assertTaskPageGitHubDialogStateAuthority({ + repoId: 'repo-1', + itemId: 'issue:1', + state: 'closed' + }) + layered.revert() + expect(getLastConfirmedClientValue(null, 'repo-1', 'issue:1', 'state')).toBe('open') + }) +}) 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 new file mode 100644 index 00000000000..9c2d7132e62 --- /dev/null +++ b/src/renderer/src/components/task-page-github-dialog-state-authority.ts @@ -0,0 +1,53 @@ +import type { GitHubWorkItem } from '../../../shared/types' +import { + getTaskSourceCacheScope, + type TaskSourceContext +} from '../../../shared/task-source-context' +import { + deleteLastConfirmedClientValue, + getLastConfirmedClientValue, + getTaskPageGitHubMutationQueryKey, + markTaskPageGitHubFamiliesDirty, + notifyTaskPageGitHubMutationRegistry, + setLastConfirmedClientValue, + taskPageGitHubItemKey +} from './task-page-github-work-item-mutation-registry' + +function markStateFamilyDirty(repoId: string, itemId: string): void { + const queryKey = getTaskPageGitHubMutationQueryKey() + if (queryKey !== null) { + markTaskPageGitHubFamiliesDirty(queryKey, taskPageGitHubItemKey(repoId, itemId), ['state']) + } +} + +/** + * Dialog state edits patch `workItemsCache` directly with no pending op, so a + * search-lagged list refetch (GitHub search index + `gh` URL cache) silently + * reverted them in the Tasks list (STA-3343). Record the confirmed state in the + * mutation registry so the list fetch paths re-assert it until search catches + * up; quiet adopt releases it on match, so external reverts still win. + */ +export function assertTaskPageGitHubDialogStateAuthority(args: { + repoId: string + itemId: string + state: GitHubWorkItem['state'] + sourceContext?: TaskSourceContext | null +}): { revert: () => void } { + const sourceScope = + args.sourceContext?.provider === 'github' ? getTaskSourceCacheScope(args.sourceContext) : null + const previous = getLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state') + setLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state', args.state) + markStateFamilyDirty(args.repoId, args.itemId) + notifyTaskPageGitHubMutationRegistry() + return { + revert: () => { + if (previous === undefined) { + deleteLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state') + } else { + setLastConfirmedClientValue(sourceScope, args.repoId, args.itemId, 'state', previous) + } + markStateFamilyDirty(args.repoId, args.itemId) + notifyTaskPageGitHubMutationRegistry() + } + } +}