Skip visit writes for active worktree reselect (#4417)
This commit is contained in:
parent
36384f17f4
commit
9769b85ae7
|
|
@ -142,3 +142,15 @@ The follow-up preserves the existing state reference when all derived active
|
|||
fields, unread state, and first-activation bookkeeping are unchanged. A
|
||||
regression test subscribes to the store and asserts that reselecting the
|
||||
already-active reconciled worktree does not notify subscribers.
|
||||
|
||||
## Follow-up: Activation Helper Visit Writes
|
||||
|
||||
After the store-level no-op fix, the higher-level `activateAndRevealWorktree`
|
||||
helper could still restamp focus-recency and append navigation history for a
|
||||
plain reselect of the already-active worktree in terminal view. That path did
|
||||
not change the visible workspace, but the recency stamp is part of the persisted
|
||||
session payload and can still wake the session writer.
|
||||
|
||||
The follow-up skips only that true no-op visit write. Activations that switch
|
||||
repo, leave another app view, or carry startup/setup/default-tab work still
|
||||
record the visit, and the sidebar reveal still runs for the no-op case.
|
||||
|
|
|
|||
|
|
@ -34,7 +34,125 @@ function makeWorktree(): Worktree {
|
|||
}
|
||||
}
|
||||
|
||||
function seedAlreadyActiveWorktree(
|
||||
worktree: Worktree,
|
||||
overrides: Partial<ReturnType<typeof useAppStore.getState>> = {}
|
||||
): {
|
||||
markWorktreeVisited: ReturnType<typeof vi.fn>
|
||||
recordWorktreeVisit: ReturnType<typeof vi.fn>
|
||||
revealWorktreeInSidebar: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
const markWorktreeVisited = vi.fn()
|
||||
const recordWorktreeVisit = vi.fn()
|
||||
const revealWorktreeInSidebar = vi.fn()
|
||||
|
||||
useAppStore.setState({
|
||||
repos: [
|
||||
{
|
||||
id: worktree.repoId,
|
||||
path: '/workspace/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: '#000000',
|
||||
addedAt: 0
|
||||
}
|
||||
],
|
||||
worktreesByRepo: { [worktree.repoId]: [worktree] },
|
||||
activeRepoId: worktree.repoId,
|
||||
activeView: 'terminal',
|
||||
activeWorktreeId: worktree.id,
|
||||
activeTabId: 'tab-1',
|
||||
activeTabType: 'terminal',
|
||||
tabsByWorktree: {
|
||||
[worktree.id]: [
|
||||
{
|
||||
id: 'tab-1',
|
||||
ptyId: 'pty-1',
|
||||
worktreeId: worktree.id,
|
||||
title: 'Terminal 1',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 1
|
||||
}
|
||||
]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-1'] },
|
||||
unifiedTabsByWorktree: {
|
||||
[worktree.id]: [
|
||||
{
|
||||
id: 'tab-1',
|
||||
entityId: 'tab-1',
|
||||
groupId: 'group-1',
|
||||
worktreeId: worktree.id,
|
||||
contentType: 'terminal',
|
||||
label: 'Terminal 1',
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 1
|
||||
}
|
||||
]
|
||||
},
|
||||
groupsByWorktree: {
|
||||
[worktree.id]: [
|
||||
{
|
||||
id: 'group-1',
|
||||
worktreeId: worktree.id,
|
||||
activeTabId: 'tab-1',
|
||||
tabOrder: ['tab-1']
|
||||
}
|
||||
]
|
||||
},
|
||||
activeGroupIdByWorktree: { [worktree.id]: 'group-1' },
|
||||
activeTabTypeByWorktree: { [worktree.id]: 'terminal' },
|
||||
everActivatedWorktreeIds: new Set([worktree.id]),
|
||||
openFiles: [],
|
||||
browserTabsByWorktree: {},
|
||||
activeFileIdByWorktree: {},
|
||||
activeBrowserTabIdByWorktree: {},
|
||||
activeTabIdByWorktree: { [worktree.id]: 'tab-1' },
|
||||
tabBarOrderByWorktree: {},
|
||||
settings: {
|
||||
agentCmdOverrides: {},
|
||||
setupScriptLaunchMode: 'new-tab'
|
||||
} as unknown as ReturnType<typeof useAppStore.getState>['settings'],
|
||||
markWorktreeVisited,
|
||||
recordWorktreeVisit,
|
||||
refreshGitHubForWorktreeIfStale: vi.fn(),
|
||||
revealWorktreeInSidebar,
|
||||
...overrides
|
||||
})
|
||||
|
||||
return { markWorktreeVisited, recordWorktreeVisit, revealWorktreeInSidebar }
|
||||
}
|
||||
|
||||
describe('activateAndRevealWorktree created agent reopen', () => {
|
||||
it('does not restamp focus recency when reselecting the already-active terminal worktree', () => {
|
||||
const worktree = makeWorktree()
|
||||
const { markWorktreeVisited, recordWorktreeVisit, revealWorktreeInSidebar } =
|
||||
seedAlreadyActiveWorktree(worktree)
|
||||
|
||||
const result = activateAndRevealWorktree(worktree.id)
|
||||
|
||||
expect(result).toEqual({ primaryTabId: null })
|
||||
expect(markWorktreeVisited).not.toHaveBeenCalled()
|
||||
expect(recordWorktreeVisit).not.toHaveBeenCalled()
|
||||
expect(revealWorktreeInSidebar).toHaveBeenCalledWith(worktree.id)
|
||||
})
|
||||
|
||||
it('records a visit when activating the same worktree changes the current view', () => {
|
||||
const worktree = makeWorktree()
|
||||
const { markWorktreeVisited, recordWorktreeVisit } = seedAlreadyActiveWorktree(worktree, {
|
||||
activeView: 'tasks'
|
||||
})
|
||||
|
||||
const result = activateAndRevealWorktree(worktree.id)
|
||||
|
||||
expect(result).toEqual({ primaryTabId: null })
|
||||
expect(markWorktreeVisited).toHaveBeenCalledWith(worktree.id)
|
||||
expect(recordWorktreeVisit).toHaveBeenCalledWith(worktree.id)
|
||||
})
|
||||
|
||||
it('reopens an empty worktree with the agent selected at creation time', () => {
|
||||
const worktree = makeWorktree()
|
||||
const revealWorktreeInSidebar = vi.fn()
|
||||
|
|
|
|||
|
|
@ -154,6 +154,16 @@ export function activateAndRevealWorktree(
|
|||
if (!wt) {
|
||||
return false
|
||||
}
|
||||
const hasActivationWork = Boolean(
|
||||
opts?.startup || opts?.setup || opts?.defaultTabs || opts?.issueCommand
|
||||
)
|
||||
// Why: a plain reselect of the already-visible workspace should still reveal
|
||||
// the sidebar row, but it must not restamp focus recency and wake persistence.
|
||||
const isPlainAlreadyActiveTerminal =
|
||||
!hasActivationWork &&
|
||||
state.activeRepoId === wt.repoId &&
|
||||
state.activeWorktreeId === worktreeId &&
|
||||
state.activeView === 'terminal'
|
||||
|
||||
// 1. Set activeRepoId if crossing repos
|
||||
if (wt.repoId !== state.activeRepoId) {
|
||||
|
|
@ -181,14 +191,16 @@ export function activateAndRevealWorktree(
|
|||
// flips, so the recency stamp must land with the same guarantee. Separate
|
||||
// from recordWorktreeVisit (nav-history) and from worktree.lastActivityAt
|
||||
// (background signal) on purpose — see docs/cmd-j-empty-query-ordering.md.
|
||||
state.markWorktreeVisited(worktreeId)
|
||||
if (!isPlainAlreadyActiveTerminal) {
|
||||
state.markWorktreeVisited(worktreeId)
|
||||
}
|
||||
|
||||
// Why: activateAndRevealWorktree always ends in 'terminal' view (step 2),
|
||||
// and Settings/Tasks transitions do not pass through this function, so no
|
||||
// view-guard is needed here. The guard skips re-recording when the caller
|
||||
// is goBackWorktree/goForwardWorktree, which mutate the history index
|
||||
// directly instead of treating the target as a new visit.
|
||||
if (!state.isNavigatingHistory) {
|
||||
if (!isPlainAlreadyActiveTerminal && !state.isNavigatingHistory) {
|
||||
state.recordWorktreeVisit(worktreeId)
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue