From a39f112701722dc56da36ddc7fdabfa7a326e2d6 Mon Sep 17 00:00:00 2001 From: Jiang Hua Date: Thu, 10 Sep 2026 16:03:04 +0800 Subject: [PATCH] fix(desktop): an entered project must not render as lane headers with no rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drill-in read (`projects.project_sessions`) resolved `null` both for "no such project" and for "a newer request superseded this one", and the hook committed that as the project. The sidebar then fell back to the overview node — whose lanes carry no rows by design (`hydrate=False`) — while `hasProjectContent` still counted it as content, because the overview node keeps its `sessionCount`. Entering a project could therefore show every branch header with nothing under it, silently and permanently. - `fetchProjectSessions` reports a superseded read as `ProjectSessionsSuperseded` instead of resolving it (a stale FAILURE is no more evidence than a stale answer), so no caller can mistake a discarded read for an empty project. - The drill-in hook retries a superseded read (bounded) and, once the budget is spent, reports failure — the sidebar already renders that as its Retry affordance rather than as an empty project. - `enteredProjectHasContent` keeps the structure-only fallback from counting as content, so the loading skeleton / empty state renders instead of empty lanes. --- apps/desktop/src/app/chat/sidebar/index.tsx | 6 ++ .../app/chat/sidebar/projects/model.test.ts | 41 +++++++++++- .../src/app/chat/sidebar/projects/model.ts | 22 ++++++ ...sessions-section-new-session-drag.test.tsx | 6 +- .../src/app/chat/sidebar/sessions-section.tsx | 14 +++- .../use-entered-project-sessions.test.ts | 44 +++++++++++- .../sidebar/use-entered-project-sessions.ts | 67 ++++++++++++++----- apps/desktop/src/store/projects.test.ts | 9 ++- apps/desktop/src/store/projects.ts | 21 +++++- 9 files changed, 201 insertions(+), 29 deletions(-) diff --git a/apps/desktop/src/app/chat/sidebar/index.tsx b/apps/desktop/src/app/chat/sidebar/index.tsx index f5b7ed2cb60e0..34c1732bbbf8b 100644 --- a/apps/desktop/src/app/chat/sidebar/index.tsx +++ b/apps/desktop/src/app/chat/sidebar/index.tsx @@ -1046,6 +1046,11 @@ export function ChatSidebar({ [enteredProject, enteredProjectOverlaySessions, removedSessionIds] ) + // True only when the drill-in read answered FOR THIS project. Until it does, + // `enteredProject` is the overview node — same structure, no lane rows — and + // must not be rendered as a project whose sessions are empty. + const enteredProjectHydrated = Boolean(enteredProjectTree && enteredProjectTree.id === overviewEnteredProject?.id) + const scopedRepoPaths = useMemo( () => enteredProject ? enteredProject.repos.map(repo => repo.path).filter((path): path is string => Boolean(path)) : [], @@ -1854,6 +1859,7 @@ export function ChatSidebar({ inProject ? : undefined } projectContent={inProject ? enteredProjectContent : undefined} + projectContentHydrated={inProject ? enteredProjectHydrated : undefined} projectOverview={projectOverview} projectOverviewPreviews={overviewPreviews} projectRepoWorktrees={inProject ? scopedRepoWorktrees : undefined} diff --git a/apps/desktop/src/app/chat/sidebar/projects/model.test.ts b/apps/desktop/src/app/chat/sidebar/projects/model.test.ts index 8035686f381a7..5e77553ac6cb1 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/model.test.ts +++ b/apps/desktop/src/app/chat/sidebar/projects/model.test.ts @@ -1,7 +1,12 @@ import { describe, expect, it } from 'vitest' -import { orderProjectsByIds, sortProjectsForOverview } from './model' -import { NO_PROJECT_ID, type SidebarProjectTree } from './workspace-groups' +import { enteredProjectHasContent, orderProjectsByIds, sortProjectsForOverview } from './model' +import { + NO_PROJECT_ID, + type SidebarProjectTree, + type SidebarSessionGroup, + type SidebarWorkspaceTree +} from './workspace-groups' function makeProject(id: string, sessionCount: number): SidebarProjectTree { return { @@ -76,3 +81,35 @@ describe('sortProjectsForOverview', () => { expect(ids(sortProjectsForOverview(projects, 'active'))).toEqual([NO_PROJECT_ID, 'active', 'scanned']) }) }) + +describe('enteredProjectHasContent', () => { + const lane = (sessions: number): SidebarSessionGroup => ({ + id: 'lane', + label: 'main', + path: '/repos/p', + sessions: Array.from({ length: sessions }, (_, index) => ({ id: `s${index}` }) as never) + }) + + const repo = (lanes: SidebarSessionGroup[]): SidebarWorkspaceTree => ({ + id: 'repo', + label: 'repo', + path: '/repos/p', + groups: lanes, + sessionCount: lanes.reduce((total, group) => total + group.sessions.length, 0) + }) + + // The overview tree ships its lanes WITHOUT rows (`hydrate=False`): rendering + // that fallback as content is what left an entered project showing branch + // headers with nothing under them. + it('refuses the structure-only fallback until rows arrive', () => { + const structureOnly = { ...makeProject('p', 43), repos: [repo([lane(0)])] } + + expect(enteredProjectHasContent(structureOnly, false)).toBe(false) + // …and the very same node, once it carries hydrated rows, is content again. + expect(enteredProjectHasContent({ ...structureOnly, repos: [repo([lane(3)])] }, false)).toBe(true) + // A hydrated project whose rows were all filtered out (pinned) is still + // content: the lanes are real, so the drill-in must keep rendering them. + expect(enteredProjectHasContent({ ...structureOnly, repos: [repo([lane(0)])] }, true)).toBe(true) + expect(enteredProjectHasContent({ ...makeProject('p', 5), repos: [] }, true)).toBe(true) + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/projects/model.ts b/apps/desktop/src/app/chat/sidebar/projects/model.ts index 0836a089b31a0..d1ed5cbb7f43e 100644 --- a/apps/desktop/src/app/chat/sidebar/projects/model.ts +++ b/apps/desktop/src/app/chat/sidebar/projects/model.ts @@ -45,6 +45,28 @@ const projectActivityTime = (project: SidebarProjectTree): number => export const latestProjectSessions = (project: SidebarProjectTree, limit: number): SessionInfo[] => [...projectSessions(project)].sort((a, b) => sessionRecency(b) - sessionRecency(a)).slice(0, limit) +/** + * Whether an entered project's payload can render rows at all. + * + * The overview tree ships every lane WITHOUT rows (`hydrate=False`, the payload + * the sidebar renders immediately on drill-in), so a structure-only node must + * never read as content: treating it as one is what left an entered project + * showing branch headers with nothing under them while the drill-in read was + * still pending or had failed. Rows placed by the live overlay still count — + * those are real sessions, not structure. + */ +export function enteredProjectHasContent(project: SidebarProjectTree | undefined, hydrated: boolean): boolean { + if (!project) { + return false + } + + if (!hydrated) { + return project.repos.some(repo => repo.groups.some(group => group.sessions.length > 0)) + } + + return project.sessionCount > 0 || project.repos.some(repo => repo.groups.length > 0) +} + // Home is a fixture, not a project: it always leads the overview, above the // active project and outside any hand-picked order. const homeFirst = (projects: SidebarProjectTree[]): SidebarProjectTree[] => diff --git a/apps/desktop/src/app/chat/sidebar/sessions-section-new-session-drag.test.tsx b/apps/desktop/src/app/chat/sidebar/sessions-section-new-session-drag.test.tsx index 89c5482aeadd0..e520901973eee 100644 --- a/apps/desktop/src/app/chat/sidebar/sessions-section-new-session-drag.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/sessions-section-new-session-drag.test.tsx @@ -81,7 +81,10 @@ vi.mock('@/i18n', () => ({ }) })) -vi.mock('./projects/model', () => ({ +vi.mock('./projects/model', async importOriginal => ({ + // Everything else stays REAL: the drag tests stub only what they replace, so + // a new export can never silently vanish from the mock (and blow up render). + ...(await importOriginal()), PROJECT_PREVIEW_COUNT: 3, SIDEBAR_GROUP_PAGE: 20, latestProjectSessions: () => [], @@ -268,6 +271,7 @@ describe('project-associated new-session drag sources', () => { {...baseProps()} onNewSessionSplit={onNewSessionSplit} projectContent={project({ repos: [repoA, repoB], sessionCount: 1 })} + projectContentHydrated /> ) diff --git a/apps/desktop/src/app/chat/sidebar/sessions-section.tsx b/apps/desktop/src/app/chat/sidebar/sessions-section.tsx index 393f2126e4f50..d941c03166097 100644 --- a/apps/desktop/src/app/chat/sidebar/sessions-section.tsx +++ b/apps/desktop/src/app/chat/sidebar/sessions-section.tsx @@ -41,6 +41,7 @@ import { SidebarWorkspaceGroup, type SidebarWorkspaceTree } from './projects' +import { enteredProjectHasContent } from './projects/model' import { WorkspaceAddButton } from './projects/workspace-header' import { ReorderableList, useSortableBindings } from './reorderable-list' import { SidebarSessionSkeletons } from './section-states' @@ -140,6 +141,11 @@ interface SidebarSessionsSectionProps { // The entered project's flattened content: main-checkout sessions render // directly (no redundant repo/branch header); only linked worktrees nest. projectContent?: SidebarProjectTree + // Whether `projectContent` is the drill-in read's payload (`hydrated`) rather + // than the overview node the sidebar falls back to while that read is pending + // or failed — the overview's lanes carry no rows, so it must not read as an + // empty project. See `enteredProjectHasContent`. + projectContentHydrated?: boolean // Live git lanes (`git worktree list`) for repos in the entered project — // a VISUAL enhancer only (empty lanes), never session membership. projectRepoWorktrees?: Record @@ -210,6 +216,7 @@ export function SidebarSessionsSection({ projectsLoading = false, onEnterProject, projectContent, + projectContentHydrated = false, projectRepoWorktrees, liveSessions, removedSessionIds, @@ -244,9 +251,10 @@ export function SidebarSessionsSection({ // emits a lane that has sessions, so a lane surviving with zero rows means // they were filtered out (pinned) — the branch is real and must still render. // A genuinely empty project has no lanes at all and keeps its empty state. - const hasProjectContent = Boolean( - projectContent && (projectContent.sessionCount > 0 || projectContent.repos.some(repo => repo.groups.length > 0)) - ) + // That reasoning only holds for the DRILL-IN payload: the overview node the + // sidebar falls back to carries every lane with no rows, so it renders the + // skeleton / empty state instead of branch headers with nothing under them. + const hasProjectContent = enteredProjectHasContent(projectContent, projectContentHydrated) const showEmptyState = forceEmptyState || (!hasGroupedSessions && !hasProjectOverview && !hasProjectContent && sessions.length === 0) diff --git a/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.test.ts b/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.test.ts index 047336035cf5d..81c9872bca4cc 100644 --- a/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.test.ts +++ b/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.test.ts @@ -1,13 +1,25 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, expect, it, vi } from 'vitest' -import { fetchProjectSessions } from '@/store/projects' +import { fetchProjectSessions, ProjectSessionsSuperseded } from '@/store/projects' +import type { SidebarProjectTree } from './projects/workspace-groups' import { useEnteredProjectSessions } from './use-entered-project-sessions' -vi.mock('@/store/projects', () => ({ fetchProjectSessions: vi.fn() })) +vi.mock('@/store/projects', () => ({ + fetchProjectSessions: vi.fn(), + // The hook's guard is `instanceof`, so the test has to share this exact class. + ProjectSessionsSuperseded: class ProjectSessionsSuperseded extends Error {} +})) afterEach(cleanup) +// Long enough for the hook's bounded superseded retries (3 × 150 ms). +const RETRY_TIMEOUT_MS = 3000 + +// Stable identity: the hook re-runs its read whenever `treeRevision` changes, +// so an inline `[]` would restart the effect on every render. +const NO_REVISIONS: never[] = [] + it('ignores departed drill-ins and clears failure on retry', async () => { let failOld!: (error: Error) => void vi.mocked(fetchProjectSessions) @@ -36,3 +48,31 @@ it('ignores departed drill-ins and clears failure on retry', async () => { await waitFor(() => expect(result.current.loading).toBe(false)) expect(result.current.failed).toBe(false) }) + +// A superseded read is not an answer: committing it painted the drill-in as an +// empty project (the overview node's lanes carry no rows), so the entered +// project showed branch headers with nothing under them. +it('retries a superseded read instead of committing it as an empty project', async () => { + const hydrated = { id: 'current', repos: [], sessionCount: 2 } as unknown as SidebarProjectTree + + vi.mocked(fetchProjectSessions) + .mockRejectedValueOnce(new ProjectSessionsSuperseded('superseded')) + .mockRejectedValueOnce(new ProjectSessionsSuperseded('superseded')) + .mockResolvedValue(hydrated) + + const { result } = renderHook(() => useEnteredProjectSessions('current', true, NO_REVISIONS, 'default')) + + await waitFor(() => expect(result.current.project).toBe(hydrated), { timeout: RETRY_TIMEOUT_MS }) + expect(result.current.failed).toBe(false) + expect(result.current.loading).toBe(false) +}) + +it('reports failure — never an empty project — when the read stays superseded', async () => { + vi.mocked(fetchProjectSessions).mockImplementation(() => Promise.reject(new ProjectSessionsSuperseded('superseded'))) + + const { result } = renderHook(() => useEnteredProjectSessions('current', true, NO_REVISIONS, 'default')) + + await waitFor(() => expect(result.current.failed).toBe(true), { timeout: RETRY_TIMEOUT_MS }) + expect(result.current.project).toBe(null) + expect(result.current.loading).toBe(false) +}) diff --git a/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.ts b/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.ts index a86fb130b3424..28e496ebc9d84 100644 --- a/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.ts +++ b/apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.ts @@ -1,9 +1,19 @@ import { useEffect, useState } from 'react' -import { fetchProjectSessions } from '@/store/projects' +import { fetchProjectSessions, ProjectSessionsSuperseded } from '@/store/projects' import type { SidebarProjectTree } from './projects/workspace-groups' +// A superseded answer is not an answer. Committing it as `null` painted the +// entered project as empty — the overview node's lanes carry no rows by design — +// which is how a drill-in ended up showing branch headers with nothing under +// them. The retry budget is spent inside the same effect (the effect also re-runs +// on every tree revision, so a live sidebar re-asks anyway); once it is spent the +// read reports FAILURE, which the sidebar renders as its Retry affordance +// instead of a project that looks empty. +const SUPERSEDED_RETRY_LIMIT = 3 +const SUPERSEDED_RETRY_MS = 150 + // The mounted drill-in owns its outcome. A global error flag lets a departed // project's slow failure overwrite the next project's successful load. export function useEnteredProjectSessions( @@ -23,6 +33,8 @@ export function useEnteredProjectSessions( useEffect(() => { let cancelled = false + let attempts = 0 + let timer: undefined | number setFailed(false) if (!projectId || !ready) { @@ -33,25 +45,48 @@ export function useEnteredProjectSessions( } setLoading(true) - void fetchProjectSessions(projectId) - .then(next => { - if (!cancelled) { - setProject(next) - } - }) - .catch(() => { - if (!cancelled) { + + const run = () => { + void fetchProjectSessions(projectId) + .then(next => { + if (!cancelled) { + setProject(next) + } + }) + .catch(error => { + if (cancelled) { + return + } + + if (error instanceof ProjectSessionsSuperseded && attempts < SUPERSEDED_RETRY_LIMIT) { + attempts += 1 + timer = window.setTimeout(() => { + timer = undefined + run() + }, SUPERSEDED_RETRY_MS) + + return + } + setFailed(true) - } - }) - .finally(() => { - if (!cancelled) { - setLoading(false) - } - }) + }) + .finally(() => { + // A scheduled retry keeps the pending state: the project is still + // being read, so nothing may render as its (empty) content yet. + if (!cancelled && timer === undefined) { + setLoading(false) + } + }) + } + + run() return () => { cancelled = true + + if (timer !== undefined) { + window.clearTimeout(timer) + } } }, [projectId, ready, treeRevision, scope, retryToken]) diff --git a/apps/desktop/src/store/projects.test.ts b/apps/desktop/src/store/projects.test.ts index 4d49435b46906..1f6408e51a08e 100644 --- a/apps/desktop/src/store/projects.test.ts +++ b/apps/desktop/src/store/projects.test.ts @@ -20,6 +20,7 @@ import { fetchProjectSessions, openProjectCreate, pickProjectFolder, + ProjectSessionsSuperseded, projectIdForCwd, projectNameForCwd, refreshProjects, @@ -177,7 +178,9 @@ describe('projects RPC profile forwarding', () => { await refreshProjects() await refreshProjectTree() - await fetchProjectSessions('p_123') + // The read is reported as SUPERSEDED, never as an answer: resolving `null` + // here is what let a caller paint the entered project as empty. + await expect(fetchProjectSessions('p_123')).rejects.toBeInstanceOf(ProjectSessionsSuperseded) expect(request).not.toHaveBeenCalled() setShowAllProfiles(false) @@ -925,7 +928,9 @@ describe('project tree profile isolation', () => { }) expect(profileB?.id).toBe('profile-b') - await expect(pendingDefault).resolves.toBeNull() + // The late answer must be dropped — and reported as superseded, so no + // caller can mistake the discarded read for an empty project. + await expect(pendingDefault).rejects.toBeInstanceOf(ProjectSessionsSuperseded) }) }) diff --git a/apps/desktop/src/store/projects.ts b/apps/desktop/src/store/projects.ts index 9caab3a116834..ed45cf5fd307c 100644 --- a/apps/desktop/src/store/projects.ts +++ b/apps/desktop/src/store/projects.ts @@ -511,12 +511,21 @@ async function refreshProjectTreeAcrossProfiles(): Promise { // membership match exactly. let projectSessionsRefreshGeneration = 0 +/** + * A drill-in read whose answer was thrown away: a newer request superseded it, + * or the active gateway/profile moved under it. That is NOT a statement about + * the project — the caller must neither render it nor read it as "this project + * has no sessions". Returning `null` for both cases is what left an entered + * project showing lane headers with no rows (the overview's lanes carry none). + */ +export class ProjectSessionsSuperseded extends Error {} + export async function fetchProjectSessions(projectId: string): Promise { const generation = ++projectSessionsRefreshGeneration const profile = projectProfile() if (!profile) { - return null + throw new ProjectSessionsSuperseded('no profile scope for the entered project') } let context: ActiveProjectsContext | undefined @@ -531,17 +540,23 @@ export async function fetchProjectSessions(projectId: string): Promise