mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(tasks): hold dialog-confirmed issue state over stale list refetches (STA-3343)
Closing an issue from GitHubItemDialog patched workItemsCache directly with no mutation-registry record, so a search-lagged Tasks refetch (GitHub search index eventual consistency + gh's ~120s URL cache) silently reverted the row to Open. Record the confirmed state as registry authority (same mechanism the list-row mutations use) so list fetch paths re-assert it until search catches up; quiet adopt still releases it on match, so external reverts win once the index is fresh. Covers issue close/reopen, PR close/reopen, and PR merge in the dialog.
This commit is contained in:
@@ -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 })
|
||||
|
||||
@@ -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'")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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> = {}): 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<string, GitHubWorkItem>([['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<GitHubWorkItem>[] = []
|
||||
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')
|
||||
})
|
||||
})
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user