Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/desktop/src/app/chat/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)) : [],
Expand Down Expand Up @@ -1854,6 +1859,7 @@ export function ChatSidebar({
inProject ? <ProjectBackRow label={s.projects.back} onClick={exitProjectScope} /> : undefined
}
projectContent={inProject ? enteredProjectContent : undefined}
projectContentHydrated={inProject ? enteredProjectHydrated : undefined}
projectOverview={projectOverview}
projectOverviewPreviews={overviewPreviews}
projectRepoWorktrees={inProject ? scopedRepoWorktrees : undefined}
Expand Down
41 changes: 39 additions & 2 deletions apps/desktop/src/app/chat/sidebar/projects/model.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
})
})
22 changes: 22 additions & 0 deletions apps/desktop/src/app/chat/sidebar/projects/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import('./projects/model')>()),
PROJECT_PREVIEW_COUNT: 3,
SIDEBAR_GROUP_PAGE: 20,
latestProjectSessions: () => [],
Expand Down Expand Up @@ -268,6 +271,7 @@ describe('project-associated new-session drag sources', () => {
{...baseProps()}
onNewSessionSplit={onNewSessionSplit}
projectContent={project({ repos: [repoA, repoB], sessionCount: 1 })}
projectContentHydrated
/>
)

Expand Down
14 changes: 11 additions & 3 deletions apps/desktop/src/app/chat/sidebar/sessions-section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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<string, HermesGitWorktree[]>
Expand Down Expand Up @@ -210,6 +216,7 @@ export function SidebarSessionsSection({
projectsLoading = false,
onEnterProject,
projectContent,
projectContentHydrated = false,
projectRepoWorktrees,
liveSessions,
removedSessionIds,
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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)
})
67 changes: 51 additions & 16 deletions apps/desktop/src/app/chat/sidebar/use-entered-project-sessions.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand All @@ -23,6 +33,8 @@ export function useEnteredProjectSessions(

useEffect(() => {
let cancelled = false
let attempts = 0
let timer: undefined | number
setFailed(false)

if (!projectId || !ready) {
Expand All @@ -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])

Expand Down
9 changes: 7 additions & 2 deletions apps/desktop/src/store/projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
fetchProjectSessions,
openProjectCreate,
pickProjectFolder,
ProjectSessionsSuperseded,
projectIdForCwd,
projectNameForCwd,
refreshProjects,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})
})

Expand Down
21 changes: 18 additions & 3 deletions apps/desktop/src/store/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,21 @@ async function refreshProjectTreeAcrossProfiles(): Promise<void> {
// 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<SidebarProjectTree | null> {
const generation = ++projectSessionsRefreshGeneration
const profile = projectProfile()

if (!profile) {
return null
throw new ProjectSessionsSuperseded('no profile scope for the entered project')
}

let context: ActiveProjectsContext | undefined
Expand All @@ -531,17 +540,23 @@ export async function fetchProjectSessions(projectId: string): Promise<SidebarPr
)

if (generation !== projectSessionsRefreshGeneration || !stillOnProjectsContext(context)) {
return null
throw new ProjectSessionsSuperseded('answer superseded before it landed')
}

return res.project ?? null
} catch (error) {
if (error instanceof ProjectSessionsSuperseded) {
throw error
}

if (
generation !== projectSessionsRefreshGeneration ||
profile !== projectProfile() ||
(context && !stillOnProjectsContext(context))
) {
return null
// A stale FAILURE is no more evidence than a stale answer: the request
// that replaced it owns the outcome.
throw new ProjectSessionsSuperseded('superseded request failed')
}

throw error
Expand Down