mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 00:02:37 +00:00
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 <help@stably.ai>
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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<AppState>)
|
||||
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<AppState>)
|
||||
store.getState().pruneLastVisitedTimestamps()
|
||||
expect(store.getState().lastVisitedAtByWorktreeId).toEqual({
|
||||
'repo1::/a': 100,
|
||||
'ssh-repo::/b': 200
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -584,16 +584,36 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
|
||||
|
||||
pruneLastVisitedTimestamps: () => {
|
||||
set((s) => {
|
||||
const validIds = new Set<string>()
|
||||
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<string, Set<string>>()
|
||||
for (const [repoId, list] of Object.entries(s.worktreesByRepo)) {
|
||||
const ids = new Set<string>()
|
||||
for (const w of list) {
|
||||
validIds.add(w.id)
|
||||
ids.add(w.id)
|
||||
}
|
||||
validIdsByRepo.set(repoId, ids)
|
||||
}
|
||||
let changed = false
|
||||
const next: Record<string, number> = {}
|
||||
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
|
||||
|
||||
@@ -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 })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -207,9 +207,29 @@ export const workspaceSessionStateSchema: z.ZodType<WorkspaceSessionState> = 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<string, number> = {}
|
||||
for (const [k, v] of Object.entries(raw as Record<string, unknown>)) {
|
||||
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 =
|
||||
|
||||
Reference in New Issue
Block a user