From 56686ea03525296b67cde453b3ba810f06d644b5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 4 May 2026 14:58:09 -0700 Subject: [PATCH] fix(worktrees): preserve SSH focus recency and route agent clicks through activation helper (#1410) - Scope pruneLastVisitedTimestamps per-repo so not-yet-hydrated SSH repos retain persisted Cmd+J recency instead of being wiped at startup. - Parse lastVisitedAtByWorktreeId leniently: drop only bad entries rather than failing the whole workspace session on one corrupted timestamp. - Route sidebar agent-tab clicks through activateAndRevealWorktree so cross-repo activation and nav history are not silently skipped. Co-authored-by: Orca --- .../components/sidebar/WorktreeCardAgents.tsx | 26 +++++++---------- .../src/store/slices/worktrees.test.ts | 18 ++++++++++-- src/renderer/src/store/slices/worktrees.ts | 28 ++++++++++++++++--- src/shared/workspace-session-schema.test.ts | 21 ++++++++++++++ src/shared/workspace-session-schema.ts | 26 +++++++++++++++-- 5 files changed, 94 insertions(+), 25 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx index c1dbfb69166..6f731c56bc7 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardAgents.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useMemo } from 'react' import { ChevronDown } from 'lucide-react' import { useAppStore } from '@/store' +import { activateAndRevealWorktree } from '@/lib/worktree-activation' import DashboardAgentRow from '@/components/dashboard/DashboardAgentRow' import { useNow } from '@/components/dashboard/useNow' import { useWorktreeAgentRows } from './useWorktreeAgentRows' @@ -50,11 +51,8 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ }: BodyProps) { const dropAgentStatus = useAppStore((s) => s.dropAgentStatus) const dismissRetainedAgent = useAppStore((s) => s.dismissRetainedAgent) - const setActiveWorktree = useAppStore((s) => s.setActiveWorktree) const setActiveTab = useAppStore((s) => s.setActiveTab) - const setActiveView = useAppStore((s) => s.setActiveView) const acknowledgeAgents = useAppStore((s) => s.acknowledgeAgents) - const markWorktreeVisited = useAppStore((s) => s.markWorktreeVisited) // Why: per-worktree collapse is session-only UI state. Single-primitive // subscription so the card only re-renders when THIS worktree's collapsed @@ -89,24 +87,20 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({ const handleActivateAgentTab = useCallback( (tabId: string, paneKey: string) => { acknowledgeAgents([paneKey]) - setActiveWorktree(worktreeId) - // Why: sidebar agent-tab click is a user-initiated switch; stamp focus - // recency for Cmd+J. See docs/cmd-j-empty-query-ordering.md. - markWorktreeVisited(worktreeId) - setActiveView('terminal') + // Why: route through activateAndRevealWorktree so cross-repo clicks also + // set activeRepoId, record a nav-history entry, clear sidebar filters, + // reveal the card, and stamp focus recency — per the design doc rule + // "Every user-initiated worktree switch must route through + // activateAndRevealWorktree". Bypassing it (direct setActiveWorktree + + // markWorktreeVisited) silently skipped cross-repo activation and + // back/forward history for clicks from inline agent rows. + activateAndRevealWorktree(worktreeId) const tabs = useAppStore.getState().tabsByWorktree[worktreeId] ?? [] if (tabs.some((t) => t.id === tabId)) { setActiveTab(tabId) } }, - [ - worktreeId, - setActiveWorktree, - setActiveTab, - setActiveView, - acknowledgeAgents, - markWorktreeVisited - ] + [worktreeId, setActiveTab, acknowledgeAgents] ) const handleToggleCollapsed = useCallback( diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 4e8a4ccf3b6..57b10152722 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -779,14 +779,28 @@ describe('markWorktreeVisited', () => { expect(store.getState().lastVisitedAtByWorktreeId['wt-1']).toBe(existing) }) - it('pruneLastVisitedTimestamps drops entries for unknown worktree IDs', () => { + it('pruneLastVisitedTimestamps drops entries for unknown worktree IDs within hydrated repos', () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a' }) store.setState({ worktreesByRepo: { repo1: [wt] }, - lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'stale-id': 200 } + lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'repo1::/gone': 200 } } as Partial) store.getState().pruneLastVisitedTimestamps() expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ 'repo1::/a': 100 }) }) + + it('pruneLastVisitedTimestamps preserves entries for not-yet-hydrated repos (e.g. SSH pre-connect)', () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a' }) + store.setState({ + worktreesByRepo: { repo1: [wt] }, + lastVisitedAtByWorktreeId: { 'repo1::/a': 100, 'ssh-repo::/b': 200 } + } as Partial) + store.getState().pruneLastVisitedTimestamps() + expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ + 'repo1::/a': 100, + 'ssh-repo::/b': 200 + }) + }) }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 037f801cd98..e59e277562f 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -584,16 +584,36 @@ export const createWorktreeSlice: StateCreator pruneLastVisitedTimestamps: () => { set((s) => { - const validIds = new Set() - for (const list of Object.values(s.worktreesByRepo)) { + // Why: scope pruning per-repo. SSH-backed repos cannot enumerate + // worktrees until their connection is established, so at hydration + // time worktreesByRepo[sshRepoId] is empty/undefined. If we pruned + // globally based on the union of all repos' worktrees, we would wipe + // every persisted focus-recency entry for SSH worktrees — precisely + // the set this feature exists to preserve. Instead, only drop entries + // whose repo has a populated worktree list: a missing repoId means + // "not yet hydrated" (defer), a repoId with an empty list after a + // successful listing means the worktree really is gone (drop). + // The ssh:state-changed 'connected' handler re-fetches worktrees and + // a follow-up prune runs from the same site if needed. + const validIdsByRepo = new Map>() + for (const [repoId, list] of Object.entries(s.worktreesByRepo)) { + const ids = new Set() for (const w of list) { - validIds.add(w.id) + ids.add(w.id) } + validIdsByRepo.set(repoId, ids) } let changed = false const next: Record = {} for (const [id, ts] of Object.entries(s.lastVisitedAtByWorktreeId)) { - if (validIds.has(id)) { + const repoId = getRepoIdFromWorktreeId(id) + const repoIds = validIdsByRepo.get(repoId) + if (!repoIds) { + // Repo not yet hydrated (e.g. SSH not connected). Keep the entry. + next[id] = ts + continue + } + if (repoIds.has(id)) { next[id] = ts } else { changed = true diff --git a/src/shared/workspace-session-schema.test.ts b/src/shared/workspace-session-schema.test.ts index 0b24fe5eb17..4a25933f4aa 100644 --- a/src/shared/workspace-session-schema.test.ts +++ b/src/shared/workspace-session-schema.test.ts @@ -95,4 +95,25 @@ describe('parseWorkspaceSession', () => { expect(parseWorkspaceSession('garbage').ok).toBe(false) expect(parseWorkspaceSession(42).ok).toBe(false) }) + + it('drops bad lastVisitedAtByWorktreeId entries rather than failing the session', () => { + const result = parseWorkspaceSession({ + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + lastVisitedAtByWorktreeId: { + good: 1_700_000_000_000, + nan: Number.NaN, + infinite: Number.POSITIVE_INFINITY, + negative: -5, + string: 'nope' + } + }) + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value.lastVisitedAtByWorktreeId).toEqual({ good: 1_700_000_000_000 }) + } + }) }) diff --git a/src/shared/workspace-session-schema.ts b/src/shared/workspace-session-schema.ts index 30ebae27044..66b1f3b3c39 100644 --- a/src/shared/workspace-session-schema.ts +++ b/src/shared/workspace-session-schema.ts @@ -207,9 +207,29 @@ export const workspaceSessionStateSchema: z.ZodType = z.o remoteSessionIdsByTabId: z.record(z.string(), z.string()).optional(), // Why: the sort comparator in order-empty-query-worktrees.ts would produce // NaN (undefined sort order) if a corrupted session file carried NaN or - // Infinity here. Constrain to finite non-negative numbers so a bad on-disk - // value is rejected at parse time rather than silently breaking Cmd+J. - lastVisitedAtByWorktreeId: z.record(z.string(), z.number().finite().nonnegative()).optional() + // Infinity here. Parse leniently: drop individual bad entries rather than + // failing the entire session. A strict record() rejection here would cause + // parseWorkspaceSession to fall back to defaults for the ENTIRE session + // (terminals, editors, browsers, layouts) on a single corrupted timestamp + // — a blast radius far larger than "Cmd+J falls back to activity recency", + // which is all this field gates. + lastVisitedAtByWorktreeId: z + .preprocess( + (raw) => { + if (raw == null || typeof raw !== 'object') { + return raw + } + const cleaned: Record = {} + for (const [k, v] of Object.entries(raw as Record)) { + if (typeof v === 'number' && Number.isFinite(v) && v >= 0) { + cleaned[k] = v + } + } + return cleaned + }, + z.record(z.string(), z.number().finite().nonnegative()) + ) + .optional() }) export type ParsedWorkspaceSession =