xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix Source Control huge repo warning dismissal leak (#7686)

This commit is contained in:
Neil 2026-07-11 01:29:42 -07:00 committed by GitHub
parent fcdcab7cd4
commit d912482752
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 373 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/renderer/src/components/right-sidebar/SourceControl.tsx
View File

@ -58,6 +58,11 @@ import {
type PrimaryAction,
type RemoteOpKind
} from './source-control-primary-action'
import {
beginHugeRepoWarningProbe,
hasDismissedHugeRepoWarning,
markHugeRepoWarningDismissed
} from '@/lib/source-control-huge-repo-warning-dismissals'
import {
resolveDropdownItems,
type DropdownActionKind,
@ -486,11 +491,6 @@ function rewriteCompareBaseBranchFromCandidate(
const EMPTY_GIT_STATUS_ENTRIES: GitStatusEntry[] = []
const EMPTY_BRANCH_CHANGE_ENTRIES: GitBranchChangeEntry[] = []
// Why: the "too many changes — add folder to .gitignore?" warning shows at most
// once per worktree per session (the analog of a "Don't show again" gate), so a
// repo that stays huge across polls doesn't re-toast every refresh.
const hugeRepoWarningDismissed = new Set<string>()
// Why: directional signifiers ahead of each primary action label. Commit
// (✓) is affirmative; Push (↑) points in the direction data flows; Sync
// (↕) is bidirectional; Publish gets a cloud-up to distinguish the
@ -815,6 +815,7 @@ function SourceControlInner(): React.JSX.Element {
const commitInFlightRef = useRef<Record<string, boolean>>({})
const activeWorktree = useActiveWorktree()
const activeWorktreeId = useAppStore((s) => s.activeWorktreeId)
const activeWorktreeInstanceId = activeWorktree?.instanceId
const activeGroupId = useAppStore((s) =>
activeWorktreeId ? s.activeGroupIdByWorktree[activeWorktreeId] : undefined
)
@ -1321,18 +1322,23 @@ function SourceControlInner(): React.JSX.Element {
if (!repositoryHuge || !activeWorktreeId || !worktreePath || activeConnectionId) {
return
}
if (hugeRepoWarningDismissed.has(activeWorktreeId)) {
const warningProbe = beginHugeRepoWarningProbe({
id: activeWorktreeId,
instanceId: activeWorktreeInstanceId
})
if (hasDismissedHugeRepoWarning(warningProbe)) {
return
}
const worktreeId = activeWorktreeId
let cancelled = false
void window.api.git
.findHugeFoldersToIgnore({ worktreePath })
.then((folders) => {
if (cancelled || folders.length === 0 || hugeRepoWarningDismissed.has(worktreeId)) {
if (cancelled || folders.length === 0 || hasDismissedHugeRepoWarning(warningProbe)) {
return
}
if (!markHugeRepoWarningDismissed(warningProbe)) {
return
}
hugeRepoWarningDismissed.add(worktreeId)
const folderName = folders[0]
toast.warning(
translate(
@ -1347,6 +1353,11 @@ function SourceControlInner(): React.JSX.Element {
'Add to .gitignore'
),
onClick: () => {
// Why: the toast can outlive its worktree; a purged probe must
// not write .gitignore in a same-path replacement.
if (!hasDismissedHugeRepoWarning(warningProbe)) {
return
}
void window.api.git
.appendGitignore({ worktreePath, folderName })
.then(() => refreshActiveGitStatus())
@ -1360,7 +1371,14 @@ function SourceControlInner(): React.JSX.Element {
return () => {
cancelled = true
}
}, [repositoryHuge, activeWorktreeId, worktreePath, activeConnectionId, refreshActiveGitStatus])
}, [
repositoryHuge,
activeWorktreeId,
activeWorktreeInstanceId,
worktreePath,
activeConnectionId,
refreshActiveGitStatus
])
const refreshGitStatusAfterPullRequestGeneration = useCallback(
async (context: PullRequestGenerationContext): Promise<void> => {

86
src/renderer/src/components/right-sidebar/source-control-huge-repo-warning-dismissals.test.ts Normal file
View File

@ -0,0 +1,86 @@
import { afterEach, describe, expect, it } from 'vitest'
import {
HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES,
beginHugeRepoWarningProbe,
clearHugeRepoWarningDismissalsForTests,
forgetHugeRepoWarningDismissalsForWorktrees,
getHugeRepoWarningDismissalCountForTests,
getHugeRepoWarningStateCountForTests,
hasDismissedHugeRepoWarning,
markHugeRepoWarningDismissed
} from '@/lib/source-control-huge-repo-warning-dismissals'
function worktree(id: string, instanceId: string = `${id}-instance`) {
return { id, instanceId }
}
function probe(id: string, instanceId: string = `${id}-instance`) {
return beginHugeRepoWarningProbe(worktree(id, instanceId))
}
describe('source-control huge repo warning dismissals', () => {
afterEach(() => {
clearHugeRepoWarningDismissalsForTests()
})
it('stays capped through prolonged churn while retaining a reused entry', () => {
const retainedProbe = probe('keep')
expect(markHugeRepoWarningDismissed(retainedProbe)).toBe(true)
const churnCount = HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES * 8
for (let i = 0; i < churnCount; i += 1) {
expect(markHugeRepoWarningDismissed(probe(`worktree-${i}`))).toBe(true)
expect(hasDismissedHugeRepoWarning(retainedProbe)).toBe(true)
if (i % HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES === 0) {
expect(getHugeRepoWarningStateCountForTests()).toBeLessThanOrEqual(
HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES
)
}
}
expect(getHugeRepoWarningStateCountForTests()).toBe(HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES)
expect(hasDismissedHugeRepoWarning(retainedProbe)).toBe(true)
expect(hasDismissedHugeRepoWarning(probe('worktree-0'))).toBe(false)
expect(hasDismissedHugeRepoWarning(probe(`worktree-${churnCount - 1}`))).toBe(true)
})
it('does not count duplicate dismissals as new worktree entries', () => {
const repeatedProbe = probe('worktree-a')
expect(markHugeRepoWarningDismissed(repeatedProbe)).toBe(true)
expect(markHugeRepoWarningDismissed(repeatedProbe)).toBe(true)
expect(getHugeRepoWarningDismissalCountForTests()).toBe(1)
expect(hasDismissedHugeRepoWarning(repeatedProbe)).toBe(true)
})
it('preserves visibility toggles but clears an authoritatively removed path', () => {
const originalWorktree = worktree('repo::/reused-path', 'persisted-instance')
const originalProbe = beginHugeRepoWarningProbe(originalWorktree)
expect(markHugeRepoWarningDismissed(originalProbe)).toBe(true)
// Why: visibility filters can temporarily hide a still-live external
// worktree, while an authoritative missing scan proves its lifecycle ended.
forgetHugeRepoWarningDismissalsForWorktrees([])
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe({ ...originalWorktree }))).toBe(
true
)
forgetHugeRepoWarningDismissalsForWorktrees([originalWorktree.id])
// External recreation can reuse persisted metadata and instanceId.
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe({ ...originalWorktree }))).toBe(
false
)
expect(getHugeRepoWarningDismissalCountForTests()).toBe(0)
})
it('rejects a late probe completion after authoritative removal', () => {
const deferredProbe = probe('repo::/removed', 'persisted-instance')
forgetHugeRepoWarningDismissalsForWorktrees([deferredProbe.worktreeId])
const replacementProbe = probe('repo::/removed', 'persisted-instance')
expect(markHugeRepoWarningDismissed(deferredProbe)).toBe(false)
expect(hasDismissedHugeRepoWarning(replacementProbe)).toBe(false)
})
})

128
src/renderer/src/lib/source-control-huge-repo-warning-dismissals.ts Normal file
View File

@ -0,0 +1,128 @@
// Why: keep a generous live/recent session working set while guaranteeing a
// fixed ceiling without trusting visibility-filtered worktree ownership maps.
export const HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES = 1024
export type HugeRepoWarningWorktreeIdentity = {
id: string
instanceId?: string
}
export type HugeRepoWarningProbe = {
readonly worktreeId: string
readonly instanceId: string
readonly lifecycleToken: symbol
}
type HugeRepoWarningWorktreeState = {
instanceId: string
lifecycleToken: symbol
dismissed: boolean
}
const hugeRepoWarningStateByWorktreeId = new Map<string, HugeRepoWarningWorktreeState>()
function refreshHugeRepoWarningState(
worktreeId: string,
state: HugeRepoWarningWorktreeState
): void {
hugeRepoWarningStateByWorktreeId.delete(worktreeId)
hugeRepoWarningStateByWorktreeId.set(worktreeId, state)
while (hugeRepoWarningStateByWorktreeId.size > HUGE_REPO_WARNING_DISMISSAL_MAX_WORKTREES) {
const oldestWorktreeId = hugeRepoWarningStateByWorktreeId.keys().next().value
if (oldestWorktreeId === undefined) {
break
}
hugeRepoWarningStateByWorktreeId.delete(oldestWorktreeId)
}
}
export function beginHugeRepoWarningProbe(
worktree: HugeRepoWarningWorktreeIdentity
): HugeRepoWarningProbe {
const instanceId = worktree.instanceId ?? ''
let state = hugeRepoWarningStateByWorktreeId.get(worktree.id)
if (!state || state.instanceId !== instanceId) {
state = { instanceId, lifecycleToken: Symbol(worktree.id), dismissed: false }
}
refreshHugeRepoWarningState(worktree.id, state)
return {
worktreeId: worktree.id,
instanceId,
lifecycleToken: state.lifecycleToken
}
}
export function hasDismissedHugeRepoWarning(probe: HugeRepoWarningProbe): boolean {
const state = hugeRepoWarningStateByWorktreeId.get(probe.worktreeId)
if (
!state ||
state.lifecycleToken !== probe.lifecycleToken ||
!state.dismissed
) {
return false
}
refreshHugeRepoWarningState(probe.worktreeId, state)
return true
}
export function markHugeRepoWarningDismissed(probe: HugeRepoWarningProbe): boolean {
const state = hugeRepoWarningStateByWorktreeId.get(probe.worktreeId)
if (!state || state.lifecycleToken !== probe.lifecycleToken) {
return false
}
state.dismissed = true
refreshHugeRepoWarningState(probe.worktreeId, state)
return true
}
export function migrateHugeRepoWarningDismissal(
oldWorktreeId: string,
newWorktreeId: string
): void {
if (oldWorktreeId === newWorktreeId) {
return
}
const state = hugeRepoWarningStateByWorktreeId.get(oldWorktreeId)
if (!state) {
return
}
hugeRepoWarningStateByWorktreeId.delete(oldWorktreeId)
// Why: preserve the once-per-worktree choice while invalidating actions that
// captured the path before the rename.
refreshHugeRepoWarningState(newWorktreeId, {
...state,
lifecycleToken: Symbol(newWorktreeId)
})
}
export function forgetHugeRepoWarningDismissalsForWorktrees(
removedWorktreeIds: Iterable<string>
): void {
const removedIds = new Set(removedWorktreeIds)
if (removedIds.size === 0) {
return
}
for (const worktreeId of removedIds) {
// Why: deleting the state also invalidates every outstanding probe token,
// so late async completions cannot resurrect a removed worktree dismissal.
hugeRepoWarningStateByWorktreeId.delete(worktreeId)
}
}
export function clearHugeRepoWarningDismissalsForTests(): void {
hugeRepoWarningStateByWorktreeId.clear()
}
export function getHugeRepoWarningStateCountForTests(): number {
return hugeRepoWarningStateByWorktreeId.size
}
export function getHugeRepoWarningDismissalCountForTests(): number {
let count = 0
for (const state of hugeRepoWarningStateByWorktreeId.values()) {
if (state.dismissed) {
count += 1
}
}
return count
}

119
src/renderer/src/store/slices/worktrees.test.ts
View File

@ -21,6 +21,12 @@ import {
} from '../../runtime/runtime-compatibility-test-fixture'
import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client'
import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host'
import {
beginHugeRepoWarningProbe,
clearHugeRepoWarningDismissalsForTests,
hasDismissedHugeRepoWarning,
markHugeRepoWarningDismissed
} from '@/lib/source-control-huge-repo-warning-dismissals'
vi.mock('sonner', () => ({
toast: {
@ -350,6 +356,7 @@ describe('fetchWorktrees', () => {
beforeEach(() => {
vi.clearAllMocks()
resetRemoteRuntimeMocks()
clearHugeRepoWarningDismissalsForTests()
})
it('does not notify subscribers when the fetched payload is unchanged', async () => {
@ -882,7 +889,7 @@ describe('fetchWorktrees', () => {
expect(getHostedReviewLinkMutationGenerationForTests(surviving.id)).toBeGreaterThan(0)
})
it('purges remembered state for hidden worktrees removed by an authoritative refresh', async () => {
it('retains hidden state until an authoritative refresh removes the worktree', async () => {
const store = createTestStore()
const visible = makeWorktree({
id: 'repo1::/path/visible',
@ -891,6 +898,7 @@ describe('fetchWorktrees', () => {
})
const hidden = makeWorktree({
id: 'repo1::/path/hidden',
instanceId: 'persisted-hidden-instance',
repoId: 'repo1',
path: '/path/hidden'
})
@ -900,7 +908,9 @@ describe('fetchWorktrees', () => {
ownership: 'external',
visible: false
}
mockApi.worktrees.listDetected.mockResolvedValueOnce(makeDetectedResult('repo1', [visible]))
mockApi.worktrees.listDetected
.mockResolvedValueOnce(previousDetected)
.mockResolvedValueOnce(makeDetectedResult('repo1', [visible]))
store.setState({
worktreesByRepo: { repo1: [visible] },
detectedWorktreesByRepo: { repo1: previousDetected },
@ -917,6 +927,11 @@ describe('fetchWorktrees', () => {
[hidden.id]: [{ id: 'tab-hidden', worktreeId: hidden.id }]
}
} as unknown as Partial<AppState>)
markHugeRepoWarningDismissed(beginHugeRepoWarningProbe(hidden))
await store.getState().fetchWorktrees('repo1')
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe(hidden))).toBe(true)
await store.getState().fetchWorktrees('repo1')
@ -925,6 +940,51 @@ describe('fetchWorktrees', () => {
expect(store.getState().rightSidebarExplorerViewByWorktree).toEqual({ [visible.id]: 'files' })
expect(store.getState().tabsByWorktree[hidden.id]).toBeUndefined()
expect(store.getState().sortEpoch).toBe(7)
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe(hidden))).toBe(false)
})
it('clears a hidden dismissal across hydrated fetch-all delete and recreation', async () => {
const store = createTestStore()
const visible = makeWorktree({
id: 'repo1::/path/visible',
repoId: 'repo1',
path: '/path/visible'
})
const hidden = makeWorktree({
id: 'repo1::/path/reused',
instanceId: 'persisted-reused-instance',
repoId: 'repo1',
path: '/path/reused'
})
const hiddenDetected = makeDetectedResult('repo1', [visible, hidden])
hiddenDetected.worktrees[1] = {
...hiddenDetected.worktrees[1],
ownership: 'external',
visible: false
}
const recreatedDetected = makeDetectedResult('repo1', [visible, hidden])
mockApi.worktrees.listDetected
.mockResolvedValueOnce(hiddenDetected)
.mockResolvedValueOnce(makeDetectedResult('repo1', [visible]))
.mockResolvedValueOnce(recreatedDetected)
store.setState({
repos: [
{ id: 'repo1', path: '/repo1', displayName: 'Repo 1', badgeColor: '#000', addedAt: 0 }
],
hasHydratedWorktreePurge: true,
worktreesByRepo: { repo1: [visible] },
detectedWorktreesByRepo: { repo1: hiddenDetected }
} as Partial<AppState>)
markHugeRepoWarningDismissed(beginHugeRepoWarningProbe(hidden))
await store.getState().fetchAllWorktrees()
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe(hidden))).toBe(true)
await store.getState().fetchAllWorktrees()
await store.getState().fetchAllWorktrees()
// The backend can reuse persisted instance metadata for the same path.
expect(hasDismissedHugeRepoWarning(beginHugeRepoWarningProbe(hidden))).toBe(false)
})
it('purges session-only tab keys after an authoritative refresh', async () => {
@ -3215,6 +3275,47 @@ describe('removeWorktree state cleanup', () => {
beforeEach(() => {
vi.clearAllMocks()
resetRemoteRuntimeMocks()
clearHugeRepoWarningDismissalsForTests()
})
it('invalidates huge-repo warning probes after successful explicit removal', async () => {
const store = createTestStore()
const removed = makeWorktree({
id: 'repo1::/path/reused',
instanceId: 'persisted-instance',
repoId: 'repo1',
path: '/path/reused'
})
store.setState({ worktreesByRepo: { repo1: [removed] } } as Partial<AppState>)
const staleProbe = beginHugeRepoWarningProbe(removed)
expect(markHugeRepoWarningDismissed(staleProbe)).toBe(true)
await store.getState().removeWorktree(removed.id)
// The same-path replacement can reuse persisted instance metadata.
const replacementProbe = beginHugeRepoWarningProbe({ ...removed })
expect(hasDismissedHugeRepoWarning(staleProbe)).toBe(false)
expect(markHugeRepoWarningDismissed(staleProbe)).toBe(false)
expect(hasDismissedHugeRepoWarning(replacementProbe)).toBe(false)
})
it('retains huge-repo warning state when explicit removal fails', async () => {
const store = createTestStore()
const retained = makeWorktree({
id: 'repo1::/path/retained',
instanceId: 'retained-instance',
repoId: 'repo1',
path: '/path/retained'
})
store.setState({ worktreesByRepo: { repo1: [retained] } } as Partial<AppState>)
const retainedProbe = beginHugeRepoWarningProbe(retained)
expect(markHugeRepoWarningDismissed(retainedProbe)).toBe(true)
mockApi.worktrees.remove.mockRejectedValueOnce(new Error('delete failed'))
const result = await store.getState().removeWorktree(retained.id)
expect(result).toEqual({ ok: false, error: 'delete failed' })
expect(hasDismissedHugeRepoWarning(retainedProbe)).toBe(true)
})
it('cleans up hosted review link mutation bookkeeping for the removed worktree', async () => {
@ -6543,6 +6644,20 @@ describe('migrateWorktreeIdentity', () => {
vi.clearAllMocks()
resetRemoteRuntimeMocks()
resetHostedReviewLinkMutationGenerationForTests()
clearHugeRepoWarningDismissalsForTests()
})
it('carries a dismissal across rename while invalidating the old-path probe', () => {
const store = createTestStore()
const staleProbe = beginHugeRepoWarningProbe({ id: OLD, instanceId: 'persisted-instance' })
expect(markHugeRepoWarningDismissed(staleProbe)).toBe(true)
store.getState().migrateWorktreeIdentity(OLD, NEW)
const renamedProbe = beginHugeRepoWarningProbe({ id: NEW, instanceId: 'persisted-instance' })
expect(hasDismissedHugeRepoWarning(staleProbe)).toBe(false)
expect(markHugeRepoWarningDismissed(staleProbe)).toBe(false)
expect(hasDismissedHugeRepoWarning(renamedProbe)).toBe(true)
})
it('re-keys worktree-scoped maps, pointers, the Set, and openFiles old->new', () => {

14
src/renderer/src/store/slices/worktrees.ts
View File

@ -47,6 +47,10 @@ import { forgetAgentStartupDeliveriesForTabs } from '@/lib/agent-startup-deliver
import { branchName } from '@/lib/git-utils'
import { markInputQuietSchedulerInput, scheduleAfterInputQuiet } from '@/lib/input-quiet-scheduler'
import { clearSessionCommitDraftForWorktree } from '@/lib/source-control-commit-draft-session'
import {
forgetHugeRepoWarningDismissalsForWorktrees,
migrateHugeRepoWarningDismissal
} from '@/lib/source-control-huge-repo-warning-dismissals'
import { showLocalBaseRefUpdateSuggestionToast } from '@/components/sidebar/local-base-ref-suggestion-toast'
import { showPreservedBranchToast } from '@/components/sidebar/preserved-branch-toast'
import { translate } from '@/i18n/i18n'
@ -1882,6 +1886,9 @@ function buildWorktreeRenameState(
function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial<AppState> {
const worktreeIdSet = new Set(worktreeIds)
pruneHostedReviewLinkMutationGenerations(worktreeIdSet)
// Why: every authoritative and explicit purge converges here, including
// fetchAllWorktrees; centralizing prevents a deleted path inheriting UI state.
forgetHugeRepoWarningDismissalsForWorktrees(worktreeIdSet)
// Collect every tab id (and removed file id) we are about to orphan.
const doomedTabIds = new Set<string>()
@ -3227,6 +3234,10 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
{ timeoutMs: 60_000 }
))
// Why: invalidate stale probes as soon as deletion is authoritative, so
// an old toast cannot mutate a same-path replacement during UI teardown.
forgetHugeRepoWarningDismissalsForWorktrees([worktreeId])
const worktreeDisplayName = worktreeBeforeRemoval?.displayName?.trim()
if (worktreeDisplayName) {
try {
@ -4784,6 +4795,9 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
if (oldWorktreeId === newWorktreeId) {
return
}
// Why: invalidate pre-rename toast actions before publishing the new path,
// while carrying the dismissal forward for the same logical worktree.
migrateHugeRepoWarningDismissal(oldWorktreeId, newWorktreeId)
set((s) => buildWorktreeRenameState(s, oldWorktreeId, newWorktreeId))
migrateHostedReviewLinkMutationGeneration(oldWorktreeId, newWorktreeId)
}