diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md new file mode 100644 index 000000000..d2d8be004 --- /dev/null +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -0,0 +1,180 @@ +# A timed-out worktree list is never read as "no family" (#1430) + +Size: short plan. It is a follow-up of #1429, which is merged; each +consumer's fix is small and bounded. + +## Outcome + +When `git worktree list` times out, none of the four remaining consumers +treats the empty answer as "this checkout has no siblings". Each either +says it or stays unknown, and none caches or records the wrong family. + +## Evidence (verified 2026-09-27 on origin/main after #1429, do not re-derive) + +- `src/main/ipc/git.ts`: `listWorktreesForCwd` returns `[]` on a timeout. + `listWorktreesForCwdDetailed` returns `{ worktrees, timedOut }` but is not + exported. Timed-out results are never cached (#1429). +- **Conversations:** `family.ts` `resolveFamily` falls back to the cwd + alone when the list is empty, so from a linked checkout the main + checkout's conversations drop out. `service.ts` `discover` caches that + family's discovery for `DISCOVERY_FRESH_MS` (3 s). The response + (`ConversationListResponse.family`) says nothing. +- **Worktree activity:** `ipc/worktreeActivity.ts` throws "not a git + worktree" on `[]` and answers `{ ok: false }`, which `loadWorktreeDump` + shows as "Agent activity: unavailable", the same as a non-repository. +- **Agent activity repo root:** `index.ts` `resolveRepoRoot` is + `listWorktreesForCwd(cwd).then(w => w[0]?.path ?? cwd)`. On a timeout it + records the cwd as the repo root in `AgentActivityRecorder` context, so + a worktree's activity is filed under the worktree, not its repository. +- **Renderer history:** `initialHistory.ts:301-303` and `history.ts:111-112` + map any non-ok `gitWorktrees` answer (including `timedOut`) to `[]`, and + feed it into `ingestWorktreeRawEvent`, which attributes against an empty + family. + +## Change + +- `git.ts`: export `listWorktreesForCwdDetailed`. +- **Conversations:** + - the `ListWorktrees` dependency returns `{ worktrees, timedOut }`; + - `RepositoryFamily.gitTimedOut: boolean`; + - a discovery whose family timed out is returned but NOT kept as + `this.discovery`, so the next request asks git again; + - `ConversationListResponse.family.gitTimedOut?: true`, and the picker + shows a muted line: "Git didn't answer in time. Conversations from this + repository's other worktrees may be missing." +- **Worktree activity:** `{ ok: false, timedOut: true }` on a timeout, and + the preload type matches. `loadWorktreeDump` gets `activityTimedOut`, and + the dump line reads "unavailable (Git timed out)". +- **Repo root:** `resolveRepoRootAfterGit(listDetailed, cwd)` lives in + `agentActivity/`. + - It retries once when the list timed out: the git queue is the usual + cause, and it drains. + - If it times out again it throws `RepoRootUnknown`. The recorder then + records the interval's repository as UNKNOWN (`''`, the store's + existing "no repository" value, which the summary labels Unknown) and + warns. The worktree row keeps its `cwd`, and the next interval asks git + again. + - Steering q126: the first version fell back to the cwd. The store + persisted it, and summarize grouped by it, so a worktree folder became + a repository of its own that no later interval could fold back. It + never healed. + - Ruling: one retry, never a loop. The recorder awaits this on every + interval open. +- **Renderer history:** a `gitWorktrees` answer with `timedOut` skips + worktree attribution for that chunk, and `workActivity`/`workContext` + stay as they were. "Unknown" stays unknown; the live reconciler fills it + in later. A non-repository (`ok: false` without `timedOut`) keeps + today's `[]`. + +## Tests (fail-first, with #1429's execFile timeout fake where main reads git) + +- `family`/`service`: a timed-out list gives `gitTimedOut` on the family + and response, and a second request re-asks git (it isn't cached). + Before: a cwd-only family, cached. +- `worktreeActivity` IPC: timeout → `{ ok: false, timedOut: true }`. + Before: `{ ok: false }`. +- `resolveRepoRootAfterGit`: + - timeout then success → the main checkout; + - two timeouts → throws; + - a success first → no retry. +- `AgentActivityRecorder` (steering q126), the real interval path and a real + store: two timeouts, then success. The first interval is under Unknown, + the second under the repository, and there is never a bucket keyed by the + worktree folder. Before: a `/dev/agent-code/.worktrees/fix` repository. +- `loadWorktreeDump` / `formatWorktreeDump`: the activity line says the + git timeout. +- `initialHistory`: a `timedOut` worktrees answer leaves `workActivity` + untouched. Before: it was ingested against `[]`. + +## Verification + +`npx tsc -b` and the scoped vitest runs. The app is not launched. + +## Out of scope + +`listWorktreesForCwd`'s other callers (MCP read paths) already go through +#1429's surfaces. + +## Review round 1 (a, b: FIX-BEFORE-MERGE), each fix fail-first + +- **a (major): pages from two families.** Page 1 was built while git timed + out (the cwd alone), and page 2 after git recovered (the whole + repository). Appending lost the rows the recovered order puts before the + cursor, and page 2's family cleared the warning. + - Fix: `useConversationList` compares the page's family (root, roots, + `gitTimedOut`) with what it appends to. On a change it discards the page + and reloads page 1. + - Pinned: a picker test drives ArrowDown paging across recovery. +- **a + b (major): skipped history lost its worktree evidence.** Skipping + attribution on a timeout meant the reconciler, which replays only what it + observed, never saw the chunk. A quiet session stayed on the launch folder + after git recovered. + - Fix: `WorkspaceRefs.worktreeReconcilerRef` publishes the live + reconciler. Both history loaders hand a timed-out chunk to it + (`handHistoryToReconciler`: `observe` plus `refresh`), and its bounded + window replays the chunk when a later refresh gets the catalog. Failed + probes are not cached, so the next refresh retries. + - Pinned: the real reconciler with the recorded `codex-0151` window + reaches `.../worktree-2` after git recovers, and a loader-level test + shows the timed-out chunk is handed over. Removing that call turns it + red. +- **b (minor): the note showed in Everywhere,** where the family removes no + rows. It now shows only in Repository scope. Pinned. +- **a (surviving mutation): the repository-unknown warning.** It is now + asserted. +- **c (MERGE-READY), minors:** + - The older-history loader's hand-off and its null guard are pinned by a + `loadOlderHistory` test with git timing out. Both of c's mutations are + now red. + - The dead `listWorktreesForCwd` export is removed. + - The body's counts are corrected. + - Residual, accepted: the production publish of `worktreeReconcilerRef` + in `useIpcSubscriptions` is unasserted, because mounting that hook is + heavy. Every loader and reconciler test injects the ref. + +## Verification pass (a, b: FIX-BEFORE-MERGE), each fix fail-first + +Both findings are gaps in the round-1 hand-off. + +- **a (major): a fresh cached catalog never repainted.** A live event had + already cached the catalog, and only the history's own `gitWorktrees` + call timed out. `refresh()` answered `cached` and never called + `onCatalogReady`, so the handed-over chunk sat in the window. + - Fix: `LiveWorktreeReconciler.replayCachedCatalog(cwd)` replays the + retained evidence against a real cached catalog. It does nothing for + the empty placeholder an in-flight probe writes. + `handHistoryToReconciler` calls it when `refresh` answers `cached`. + - Pinned: the recorded `codex-0151` window, with the catalog loaded + first, reaches `worktree-2`. Before the fix it stayed on the main + checkout. +- **b (major): an older page could replace a newer context.** The + reconciler appends what it observes as the newest evidence. + - Fix: the older-history loader hands a page over only while + `workContext` is unknown. This is the same recency rule as its + answered-git backfill. + - Pinned: a pane with a known context hands nothing over and keeps it. + - Ruling: initial history needs no such guard. It is the transcript's + tail, the newest evidence there is. + +## Manager verification (B6 at f5fc3585: FIX), fixed fail-first + +- **A scroll-up during a git timeout outranked the newest chunk.** The + sequence: the initial history (worktree-1) is handed over during a + timeout, then the user scrolls up while git still times out, and the + older page (worktree-2) is handed over too. It was appended as the + newest evidence, and once git recovered the pane landed on worktree-2. + - Fix: `observe(..., position)`. An older page enters at the OLD end of + the window (`retainOlder`), under the same 2 × limit bound. + - Overflow is the oldest evidence there is, so it is dropped rather than + folded when a catalog is cached. The folded baseline holds newer + records, and folding on top of them would make the page newest again. + - `handHistoryToReconciler(..., 'older')` is used by the older-history + loader. + - Pinned: the recorded `codex-0151` window as the older page, with the + same records moved to worktree-1 and 1 h later as the newest chunk, + and the recorded three-worktree catalog. It now lands on worktree-1. + - Mutations: appending instead of prepending, and the loader passing + newest, are each 1 red. +- The `resolveRepoRoot.ts` header now says Unknown, not the cwd. +- Filed separately by B6, out of scope here: a git failure that is not a + timeout is still read as "not a repository". diff --git a/src/main/agentActivity/AgentActivityRecorder.test.ts b/src/main/agentActivity/AgentActivityRecorder.test.ts index 3cd497f12..a4addb140 100644 --- a/src/main/agentActivity/AgentActivityRecorder.test.ts +++ b/src/main/agentActivity/AgentActivityRecorder.test.ts @@ -474,3 +474,60 @@ describe('AgentActivityRecorder', () => { expect(summary.totals.agentMs).toBe(10 * MINUTE) }) }) + +// #1430 / steering q126: git timing out twice made resolveRepoRoot throw, the +// recorder's catch answered the CWD, and the store persisted the worktree folder +// as the interval's repository. summarize groups by that stored key, so the +// worktree became a repository of its own and a later successful interval could +// never fold the earlier one back. An unresolved repository is recorded as +// UNKNOWN ('' — the store's existing "no repository" value, labelled Unknown), +// never as a false one; the cwd still names the worktree row. +describe('AgentActivityRecorder when git times out resolving the repository', () => { + it('files the interval under Unknown, never under the worktree folder, and later intervals under the repository', async () => { + const { resolveRepoRootAfterGit } = await import('@main/agentActivity/resolveRepoRoot.js') + let gitTimingOut = true + const manager = new EventEmitter() + const recorder = new AgentActivityRecorder({ + manager: manager as unknown as Pick, + store: new AgentActivityStore(dir), + // The REAL retry-once policy over a git lister that times out, then answers. + resolveRepoRoot: cwd => resolveRepoRootAfterGit(async () => gitTimingOut + ? { worktrees: [], timedOut: true } + : { worktrees: [{ path: '/dev/agent-code' }, { path: '/dev/agent-code/.worktrees/fix' }], timedOut: false }, cwd), + identityOf: () => undefined, + }) + recorders.push(recorder) + await recorder.start() + recorder.updateWorkspace(windows(), { 'name-1': 'Ada' }) + manager.emit('started', { sessionId: 'child', kind: 'codex' }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + manager.emit('semantic-event', { sessionId: 'child', event: { type: 'stream_phase', phase: 'responding' } }) + vi.setSystemTime(T0 + HOUR) + manager.emit('semantic-event', { sessionId: 'child', event: { type: 'stream_phase', phase: 'idle' } }) + await vi.waitFor(async () => expect((await recorder.summary('24h')).totals.agentMs).toBe(HOUR)) + + gitTimingOut = false + vi.setSystemTime(T0 + 2 * HOUR) + manager.emit('semantic-event', { sessionId: 'child', event: { type: 'stream_phase', phase: 'responding' } }) + vi.setSystemTime(T0 + 3 * HOUR) + manager.emit('semantic-event', { sessionId: 'child', event: { type: 'stream_phase', phase: 'idle' } }) + vi.setSystemTime(T0 + 4 * HOUR) + await vi.waitFor(async () => expect((await recorder.summary('24h')).totals.agentMs).toBe(2 * HOUR)) + + // The unknown interval is said once in main's log, not silently filed + // (review a: the warning was not asserted). + expect(warn).toHaveBeenCalledWith(expect.stringContaining('repository unknown for this interval'), expect.anything()) + const [project] = (await recorder.summary('24h')).projects + const repositories = project!.repositories.map(r => [r.repoRoot, r.label, r.agentMs, r.worktrees.map(w => w.cwd)]) + expect(repositories).toEqual(expect.arrayContaining([ + ['', 'Unknown', HOUR, ['/dev/agent-code/.worktrees/fix']], + ['/dev/agent-code', 'agent-code', HOUR, ['/dev/agent-code/.worktrees/fix']], + ])) + // The false repository — the worktree folder as its own repository — never exists. + expect(project!.repositories.some(r => r.repoRoot === '/dev/agent-code/.worktrees/fix')).toBe(false) + } finally { + warn.mockRestore() + } + }) +}) diff --git a/src/main/agentActivity/AgentActivityRecorder.ts b/src/main/agentActivity/AgentActivityRecorder.ts index 97d614639..8c8fbc8ab 100644 --- a/src/main/agentActivity/AgentActivityRecorder.ts +++ b/src/main/agentActivity/AgentActivityRecorder.ts @@ -250,7 +250,18 @@ export class AgentActivityRecorder { provider: placement?.kind ?? entry.kind ?? 'unknown', tabId: placement?.tabId ?? null, tabTitle: placement?.tabTitle ?? null, - repoRoot: cwd ? await this.deps.resolveRepoRoot(cwd).catch(() => cwd) : '', + // #1430 / steering q126: a rejection means the repository is UNKNOWN (git + // timed out twice). It is recorded as '' — the store's existing "no + // repository" value, which the summary labels Unknown — and NEVER as the + // cwd. The cwd used to be persisted as this interval's repository: the + // store keeps it, summarize groups by it, so a worktree folder became a + // repository of its own that no later, correctly resolved interval could + // fold back. `cwd` below still names the worktree row, and the next + // interval asks git again. + repoRoot: cwd ? await this.deps.resolveRepoRoot(cwd).catch((error: unknown) => { + console.warn('[agent-activity] repository unknown for this interval (recorded as Unknown):', error instanceof Error ? error.message : error) + return '' + }) : '', cwd, } } diff --git a/src/main/agentActivity/resolveRepoRoot.test.ts b/src/main/agentActivity/resolveRepoRoot.test.ts new file mode 100644 index 000000000..a8dd846df --- /dev/null +++ b/src/main/agentActivity/resolveRepoRoot.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it, vi } from 'vitest' + +import { RepoRootUnknown, resolveRepoRootAfterGit } from './resolveRepoRoot' + +// #1430: a timed-out `git worktree list` used to file an agent's activity under +// its worktree folder instead of its repository (resolveRepoRoot read `[]` as +// "not a repository" and answered the cwd). +const MAIN = '/repo' +const WORKTREE = '/repo/.worktrees/feature' +const answered = { worktrees: [{ path: MAIN }, { path: WORKTREE }], timedOut: false } +const timeout = { worktrees: [], timedOut: true } + +describe('resolveRepoRootAfterGit', () => { + it('answers the main checkout when git answers', async () => { + const list = vi.fn(async () => answered) + expect(await resolveRepoRootAfterGit(list, WORKTREE)).toBe(MAIN) + expect(list).toHaveBeenCalledTimes(1) + }) + + it('retries a timeout once, and files under the repository when the retry answers', async () => { + const list = vi.fn().mockResolvedValueOnce(timeout).mockResolvedValueOnce(answered) + expect(await resolveRepoRootAfterGit(list, WORKTREE)).toBe(MAIN) + expect(list).toHaveBeenCalledTimes(2) + }) + + it('throws after two timeouts instead of answering the worktree folder', async () => { + const list = vi.fn(async () => timeout) + await expect(resolveRepoRootAfterGit(list, WORKTREE)).rejects.toBeInstanceOf(RepoRootUnknown) + expect(list).toHaveBeenCalledTimes(2) + }) + + it('keeps the folder for a real non-repository (git answered, no worktrees)', async () => { + expect(await resolveRepoRootAfterGit(async () => ({ worktrees: [], timedOut: false }), '/tmp/plain')).toBe('/tmp/plain') + }) +}) diff --git a/src/main/agentActivity/resolveRepoRoot.ts b/src/main/agentActivity/resolveRepoRoot.ts new file mode 100644 index 000000000..b6aed618b --- /dev/null +++ b/src/main/agentActivity/resolveRepoRoot.ts @@ -0,0 +1,36 @@ +/** + * The repository an agent's activity is filed under: the main checkout, i.e. + * the first entry of `git worktree list` (#1430). + * + * WHY a timeout is retried once and then THROWN rather than answered with the + * cwd: the recorder files each interval under the root this returns, so a + * timed-out (empty) list used to file a worktree's activity under the worktree + * itself — a silent, persisted mis-grouping. A timeout is almost always the + * git queue being busy (#1429's shared queue of five), which drains, so one + * retry usually answers. Two timeouts are "unknown", and unknown is thrown: + * the recorder records that one interval's repository as UNKNOWN (`''`, the + * store's "no repository" value) and warns, and the next interval asks again. + * Not the cwd (steering q126): the store persisted it, and a worktree folder + * became a repository of its own that no later interval could fold back. + * Never a loop — the recorder awaits this on every interval open. + * + * A non-repository (git answered, no worktrees) is not a timeout: the cwd is + * then the honest root, exactly as before. + */ +export class RepoRootUnknown extends Error { + constructor(cwd: string) { + super(`git worktree list timed out twice for ${cwd}`) + this.name = 'RepoRootUnknown' + } +} + +export async function resolveRepoRootAfterGit( + listDetailed: (cwd: string) => Promise<{ worktrees: ReadonlyArray<{ path: string }>; timedOut: boolean }>, + cwd: string, +): Promise { + for (let attempt = 0; attempt < 2; attempt += 1) { + const { worktrees, timedOut } = await listDetailed(cwd) + if (!timedOut) return worktrees[0]?.path ?? cwd + } + throw new RepoRootUnknown(cwd) +} diff --git a/src/main/conversations/catalog/listing.ts b/src/main/conversations/catalog/listing.ts index bfbc7b158..5d4427009 100644 --- a/src/main/conversations/catalog/listing.ts +++ b/src/main/conversations/catalog/listing.ts @@ -67,7 +67,7 @@ export function buildListing(input: BuildListingInput): ConversationListResponse total, hiddenChildren, nextCursor: start + limit < visible.length && last ? encodeCursor(last) : null, - family: { repoRoot: family.root, roots: family.roots }, + family: { repoRoot: family.root, roots: family.roots, ...(family.gitTimedOut ? { gitTimedOut: true as const } : {}) }, timing: { ms: Math.max(0, Date.now() - input.startedAt) }, } } diff --git a/src/main/conversations/family.ts b/src/main/conversations/family.ts index 5c5c095e7..bf76ea899 100644 --- a/src/main/conversations/family.ts +++ b/src/main/conversations/family.ts @@ -37,11 +37,24 @@ export type RepositoryFamily = { * project directory name from the literal cwd, so a lowercased root * would name a directory that does not exist. */ rawRoots: string[] + /** `git worktree list` timed out (#1430), so the siblings are UNKNOWN, not + * absent: `roots` fell back to the cwd alone and may be missing the main + * checkout and other worktrees. The service does not cache such a family, + * and the picker says so. False when git answered (or is simply not a + * repository here). */ + gitTimedOut: boolean matches(candidate: string | null | undefined): boolean } +/** A plain list means git answered. The detailed form (main's + * listWorktreesForCwdDetailed) can also say the list timed out (#1430); + * both are accepted so fixtures that hand in a known list stay as they are. */ +export type ListedWorktrees = + | ReadonlyArray<{ path: string }> + | { worktrees: ReadonlyArray<{ path: string }>; timedOut: boolean } + export type FamilyDeps = { - listWorktrees(cwd: string): Promise> + listWorktrees(cwd: string): Promise } /** `path.resolve` collapses `..` and trailing slashes; darwin and win32 file @@ -76,8 +89,12 @@ export async function resolveFamily( const cwdForms = forms(cwd) const cwdRoots = cwdForms.map(normalizeCwd) let worktreeForms: string[][] = [] + let gitTimedOut = false try { - worktreeForms = (await deps.listWorktrees(cwd)).map(w => forms(w.path)) + const listed = await deps.listWorktrees(cwd) + const worktrees = 'timedOut' in listed ? listed.worktrees : listed + gitTimedOut = 'timedOut' in listed && listed.timedOut === true + worktreeForms = worktrees.map(w => forms(w.path)) } catch { worktreeForms = [] } @@ -122,6 +139,7 @@ export async function resolveFamily( root, roots, rawRoots, + gitTimedOut, matches(candidate) { if (scope === 'everywhere') return true if (!candidate) return false diff --git a/src/main/conversations/service.system.test.ts b/src/main/conversations/service.system.test.ts index 52ea3ef6f..26c4a3461 100644 --- a/src/main/conversations/service.system.test.ts +++ b/src/main/conversations/service.system.test.ts @@ -109,3 +109,44 @@ describe('ConversationService', () => { }) }) +// #1430 (found by #1429 review b): `git worktree list` timing out answered `[]`, +// resolveFamily fell back to the cwd alone, and the service cached that guess +// for DISCOVERY_FRESH_MS. From a linked checkout the main checkout's +// conversations dropped out of the repository picker and nothing said so. +describe('ConversationService when git times out listing worktrees', () => { + it('says the family is incomplete, does not cache it, and recovers when git answers', async () => { + const porcelain = await corpusWorktreesPorcelain() + const worktrees = porcelain.split('\n').filter(l => l.startsWith('worktree ')).map(l => ({ path: l.slice('worktree '.length) })) + let timedOut = true + const calls: string[] = [] + const claudeHistory = new ClaudeHistoryIndex(join(corpus.claudeConfigDir, 'history.jsonl')) + const s = new ConversationService({ + sources: [ + new ClaudeConversationSource({ projectsDir: join(corpus.claudeConfigDir, 'projects'), history: claudeHistory }), + new CodexConversationSource({ codexHome: corpus.codexHome }), + new OpencodeConversationSource({ dataDir: corpus.opencodeDataDir }), + ], + ledger: null, + // The shape main's listWorktreesForCwdDetailed answers. + listWorktrees: async cwd => { + calls.push(cwd) + return timedOut ? { worktrees: [], timedOut: true } : { worktrees, timedOut: false } + }, + claudeHistory, + }) + const cwd = '/fixture/repo/.worktrees/extension-platform' + + const slow = await s.list({ cwd, scope: 'repository', limit: 500 }) + expect(slow.family.gitTimedOut).toBe(true) + // Asked again at once: a guessed family is never served from the cache. + await s.list({ cwd, scope: 'repository', limit: 500 }) + expect(calls).toHaveLength(2) + + timedOut = false + const answered = await s.list({ cwd, scope: 'repository', limit: 500 }) + expect(answered.family.gitTimedOut).toBeUndefined() + expect(answered.family.repoRoot).toBe('/fixture/repo') + // The main checkout's rows are back: the guessed family was missing some. + expect(answered.total).toBeGreaterThan(slow.total) + }) +}) diff --git a/src/main/conversations/service.ts b/src/main/conversations/service.ts index 2b069d1d3..44af639ac 100644 --- a/src/main/conversations/service.ts +++ b/src/main/conversations/service.ts @@ -11,7 +11,7 @@ import { performanceService } from '@main/performance/PerformanceService.js' import { buildListing, HIDDEN_KINDS } from './catalog/listing.js' import { normalizeConversation } from './catalog/normalize.js' import { unwrapUserText } from './catalog/unwrap.js' -import { resolveFamily, type RepositoryFamily } from './family.js' +import { resolveFamily, type ListedWorktrees, type RepositoryFamily } from './family.js' import type { ConversationLedger } from './ledger/ledger.js' import type { LedgerRow } from './ledger/types.js' import { ClaudeConversationSource } from './sources/claude.js' @@ -56,7 +56,7 @@ const SEARCH_BYTES_PER_ROW = 128 * 1024 type Discovery = { at: number; key: string; family: RepositoryFamily; sources: SourceConversation[] } -export type ListWorktrees = (cwd: string) => Promise> +export type ListWorktrees = (cwd: string) => Promise export class ConversationService { private discovery: Discovery | null = null @@ -95,7 +95,10 @@ export class ConversationService { return [] as SourceConversation[] }))) const discovery: Discovery = { at: Date.now(), key, family, sources: perSource.flat() } - this.discovery = discovery + // #1430: a family built while git timed out is a guess (the cwd alone), so it answers this + // request and is not kept — the next one asks git again instead of serving the guess for + // DISCOVERY_FRESH_MS. + if (!family.gitTimedOut) this.discovery = discovery this.discoveries++ span.end({ rows: discovery.sources.length }) return discovery diff --git a/src/main/index.ts b/src/main/index.ts index 146e8d5fd..f32814656 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -119,7 +119,8 @@ import { WorkspaceFileStore } from '@main/storage/workspaceFileStore.js' import type { PersistedWindow } from '@main/storage/workspaceFile.js' import { ConversationLedger, readAgentNameAssignments } from '@main/conversations/ledger/ledger.js' import { createConversationService } from '@main/conversations/service.js' -import { listWorktreesForCwd } from '@main/ipc/git.js' +import { listWorktreesForCwdDetailed } from '@main/ipc/git.js' +import { resolveRepoRootAfterGit } from '@main/agentActivity/resolveRepoRoot.js' import { AGENT_NAMES_FILE } from '@main/agentNames/ipc.js' import { RemoteWorkspaceProjection } from '@main/remote/workspaceProjection.js' import { tldrIdentitiesInUse } from '@main/tldr/identitiesInUse.js' @@ -1430,7 +1431,8 @@ async function startApp(): Promise { store: new AgentActivityStore(AGENT_ACTIVITY_DIR), // The first worktree entry is the main checkout, so every worktree of one // repository folds into it (the conversations picker's family rule). - resolveRepoRoot: cwd => listWorktreesForCwd(cwd).then(worktrees => worktrees[0]?.path ?? cwd), + // #1430: a timed-out list is retried once, then thrown (see resolveRepoRootAfterGit). + resolveRepoRoot: cwd => resolveRepoRootAfterGit(listWorktreesForCwdDetailed, cwd), identityOf: sessionId => builtInMcpHost.sessionTldrIdentity(sessionId), }) const projectActivity = (windows: readonly PersistedWindow[]) => { @@ -1547,7 +1549,8 @@ async function startApp(): Promise { // the ledger, so it is constructed after the ledger. The control host above // was built before the workspace store opened and holds a getter for it; // its handlers only run on requests, long after this line. - const conversationService = createConversationService({ ledger: conversationLedger, listWorktrees: listWorktreesForCwd }) + // The detailed lister (#1430): a timed-out list is a family the service must not cache. + const conversationService = createConversationService({ ledger: conversationLedger, listWorktrees: listWorktreesForCwdDetailed }) registerAllIpc({ manager, sessionFeedTap: feedTap, diff --git a/src/main/ipc/git.ts b/src/main/ipc/git.ts index 3ffd69f38..51fba7867 100644 --- a/src/main/ipc/git.ts +++ b/src/main/ipc/git.ts @@ -237,13 +237,14 @@ function parseWorktreePorcelain(out: string): WorktreeIdentity[] { return worktrees } -export async function listWorktreesForCwd(cwd: string): Promise { - return (await listWorktreesForCwdDetailed(cwd)).worktrees -} +// No plain `listWorktreesForCwd` any more (#1430, review c): it answered `[]` +// for a timeout as well as for "not a repository", and every consumer that +// read it that way was moved to the detailed answer below. A new caller that +// only needs the list must still decide what a timeout means to it. /** The list plus whether `git worktree list` timed out, which an empty list * alone cannot tell apart from "not a git repository" (#1250 row 11). */ -async function listWorktreesForCwdDetailed(cwd: string): Promise<{ worktrees: WorktreeIdentity[]; timedOut: boolean }> { +export async function listWorktreesForCwdDetailed(cwd: string): Promise<{ worktrees: WorktreeIdentity[]; timedOut: boolean }> { const now = Date.now() const cached = worktreeListCache.get(cwd) if (cached && (!cached.settled || cached.expiresAt > now)) { @@ -564,7 +565,7 @@ async function inspectSubmodule( export function registerGitIpc(): void { // WHY both worktree handlers carry `gitMissing` exactly like git:status - // (#508 review): a machine without a working git makes listWorktreesForCwd + // (#508 review): a machine without a working git makes the worktree lister // return [] (the '' swallow), which these handlers collapse into a bare // { ok:false } — and the Worktrees panel then renders "not a git // repository", the same lie A5 fixed on GitBar. The flag is read AFTER diff --git a/src/main/ipc/worktreeActivity.test.ts b/src/main/ipc/worktreeActivity.test.ts new file mode 100644 index 000000000..8a6974e76 --- /dev/null +++ b/src/main/ipc/worktreeActivity.test.ts @@ -0,0 +1,39 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// #1430: the REAL worktree-activity handler; Electron's ipcMain is captured and +// the git lister is the edge. A timed-out `git worktree list` used to throw +// "not a git worktree" here and answer `{ ok: false }` — indistinguishable from +// a non-repository, and shown as a missing activity index. +const handlers = new Map Promise>() +vi.mock('electron', () => ({ ipcMain: { handle: (channel: string, handler: (...args: unknown[]) => Promise) => handlers.set(channel, handler) } })) +const git = vi.hoisted(() => ({ listWorktreesForCwdDetailed: vi.fn() })) +vi.mock('@main/ipc/git.js', () => git) + +import { registerWorktreeActivityIpc } from './worktreeActivity' + +const getSummary = vi.fn(async () => ({ summaries: [], status: { lastIndexedAt: null } })) +beforeEach(() => { + handlers.clear() + getSummary.mockClear() + registerWorktreeActivityIpc({ getSummary } as never) +}) +const summary = (cwd: string) => handlers.get('worktree-activity:summary')!({}, cwd, false) + +describe('worktree-activity:summary', () => { + it('says a git timeout instead of answering "not a repository"', async () => { + git.listWorktreesForCwdDetailed.mockResolvedValue({ worktrees: [], timedOut: true }) + expect(await summary('/repo')).toEqual({ ok: false, timedOut: true }) + expect(getSummary).not.toHaveBeenCalled() + }) + + it('keeps a plain non-repository a plain failure', async () => { + git.listWorktreesForCwdDetailed.mockResolvedValue({ worktrees: [], timedOut: false }) + expect(await summary('/tmp/plain')).toEqual({ ok: false }) + }) + + it('summarises a repository git answered for', async () => { + git.listWorktreesForCwdDetailed.mockResolvedValue({ worktrees: [{ path: '/repo' }], timedOut: false }) + expect(await summary('/repo')).toMatchObject({ ok: true, summaries: [] }) + expect(getSummary).toHaveBeenCalledWith({ worktrees: [{ path: '/repo' }], refresh: false }) + }) +}) diff --git a/src/main/ipc/worktreeActivity.ts b/src/main/ipc/worktreeActivity.ts index b7d54482c..ca5ff1771 100644 --- a/src/main/ipc/worktreeActivity.ts +++ b/src/main/ipc/worktreeActivity.ts @@ -1,6 +1,6 @@ import { ipcMain } from 'electron' -import { listWorktreesForCwd } from '@main/ipc/git.js' +import { listWorktreesForCwdDetailed } from '@main/ipc/git.js' import type { WorktreeActivityIndex } from '@main/worktreeActivity/WorktreeActivityIndex.js' export function registerWorktreeActivityIpc(index: WorktreeActivityIndex): void { @@ -8,7 +8,11 @@ export function registerWorktreeActivityIpc(index: WorktreeActivityIndex): void 'worktree-activity:summary', async (_evt, cwd: string, refresh?: boolean) => { try { - const worktrees = await listWorktreesForCwd(cwd) + const { worktrees, timedOut } = await listWorktreesForCwdDetailed(cwd) + // #1430: an empty list from a TIMED-OUT git is "unknown", not "not a + // repository". Said as its own answer so the panel and agents reading + // it (worktrees.read) do not report the activity index as missing. + if (timedOut) return { ok: false as const, timedOut: true as const } if (worktrees.length === 0) throw new Error('not a git worktree') const result = await index.getSummary({ worktrees, diff --git a/src/preload/api/git.ts b/src/preload/api/git.ts index e2c80ef5c..b65ac5654 100644 --- a/src/preload/api/git.ts +++ b/src/preload/api/git.ts @@ -43,7 +43,9 @@ export const gitApi = { summaries: WorktreeActivitySummary[] status: WorktreeActivityIndexStatus } - | { ok: false } + // `timedOut` (#1430): git worktree list did not answer in time; the + // activity is unknown, not absent. + | { ok: false; timedOut?: true } > => ipcRenderer.invoke('worktree-activity:summary', cwd, refresh), gitStatus: (cwd: string): Promise => diff --git a/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx b/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx index 7756ecae8..31dad3c5b 100644 --- a/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx +++ b/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx @@ -410,3 +410,57 @@ describe('ConversationsPicker', () => { }) }) + +// #1430: git timed out listing the repository's worktrees, so main built the +// list from this folder alone. The rows are complete for this folder but may +// be missing the siblings' conversations — said in a muted status line. +describe('ConversationsPicker when git timed out', () => { + it('says conversations from other worktrees may be missing', async () => { + install(vi.fn(async () => response({ family: { repoRoot: '/fixture/repo/.worktrees/extension-platform', roots: ['/fixture/repo/.worktrees/extension-platform'], gitTimedOut: true } }))) + render() + const note = await screen.findByText("Git didn't answer in time. Conversations from this repository's other worktrees may be missing.") + expect(note).toHaveAttribute('role', 'status') + }) + + it('says nothing when git answered', async () => { + install() + render() + await screen.findByText('Project context bootstrapping') + expect(screen.queryByText(/Git didn't answer in time/)).toBeNull() + }) +}) + +// #1430 review a/b. +describe('ConversationsPicker paging across a git timeout (#1430)', () => { + it('starts over from page 1 when git recovers between pages, instead of appending a page from another family', async () => { + const timedOut = { repoRoot: '/fixture/repo/.worktrees/extension-platform', roots: ['/fixture/repo/.worktrees/extension-platform'], gitTimedOut: true as const } + const recovered = { repoRoot: '/fixture/repo', roots: ['/fixture/repo', '/fixture/repo/.worktrees/extension-platform'] } + const worktreeRow = row({ nativeId: 'feature-first', label: 'feature first', cwd: '/fixture/repo/.worktrees/extension-platform' }) + const mainNewest = row({ nativeId: 'main-newest', label: 'main newest' }) + const list = vi.fn(async (request: { cursor?: string | null }) => { + if (list.mock.calls.length === 1) return response({ rows: [worktreeRow], total: 1, hiddenChildren: 0, nextCursor: 'after-feature', family: timedOut }) + if (request.cursor) return response({ rows: [row({ nativeId: 'main-second', label: 'main second' })], total: 3, hiddenChildren: 0, nextCursor: null, family: recovered }) + return response({ rows: [mainNewest, worktreeRow], total: 2, hiddenChildren: 0, nextCursor: null, family: recovered }) + }) + install(list) + render() + // Page 1 (built while git timed out) is on screen; moving down pages in. + expect(await screen.findByText('feature first')).toBeInTheDocument() + expect(screen.getByText(/Git didn't answer in time/)).toBeInTheDocument() + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'ArrowDown' }) + // The recovered page 1 replaces the timed-out one; the row page 1 missed is there. + expect(await screen.findByText('main newest')).toBeInTheDocument() + expect(list.mock.calls.map(c => c[0].cursor ?? null)).toEqual([null, 'after-feature', null]) + await waitFor(() => expect(list.mock.calls.at(-1)?.[0]).toMatchObject({ cursor: null })) + expect(screen.queryByText('main second')).toBeNull() + expect(screen.queryByText(/Git didn't answer in time/)).toBeNull() + }) + + it('says nothing about missing worktrees in Everywhere, where the family removes no rows', async () => { + install(vi.fn(async () => response({ family: { repoRoot: null, roots: [], gitTimedOut: true } }))) + render() + await screen.findByText('Project context bootstrapping') + fireEvent.click(screen.getByRole('button', { name: 'Everywhere' })) + await waitFor(() => expect(screen.queryByText(/Git didn't answer in time/)).toBeNull()) + }) +}) diff --git a/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx b/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx index b5cf77943..502b72330 100644 --- a/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx +++ b/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx @@ -37,6 +37,11 @@ const SCOPES: Array<{ id: ConversationScope; label: string }> = [ { id: 'everywhere', label: 'Everywhere' }, ] +// Fixed words (#1430). Only in 'repository' scope (review b): 'cwd' never +// wants siblings, and 'everywhere' matches every folder, so a timed-out +// sibling list cannot remove a row from either. +export const GIT_TIMED_OUT_NOTE = "Git didn't answer in time. Conversations from this repository's other worktrees may be missing." + export function ConversationsPicker({ open, focusSearch, workspace, onClose }: Props) { const [query, setQuery] = useState('') const [scope, setScope] = useState('repository') @@ -282,6 +287,15 @@ export function ConversationsPicker({ open, focusSearch, workspace, onClose }: P {banner &&
{banner}
} + {/* #1430: git timed out listing this repository's worktrees, so the + list was built from this folder alone (and main did not cache it). + Said, because rows are MISSING, not absent: a muted status line, + not an error, since the next open asks git again. */} + {response?.family.gitTimedOut && scope === 'repository' && ( +
+ {GIT_TIMED_OUT_NOTE} +
+ )}
(null) const [response, setResponse] = useState(null) // The parameters `response` was fetched for, set with it. const [responseKey, setResponseKey] = useState(null) @@ -57,6 +60,18 @@ export function useConversationList(params: ConversationListParams): { includeChildren, query: query.trim() || undefined, cursor, limit: PAGE, }) if (request !== version.current) return + // #1430 review a: a page is only an append of the pages before it when + // both came from the SAME family. Page 1 built while git timed out + // (the cwd alone) and a page 2 built after git recovered (the whole + // repository) interleave differently: the recovered order can place + // rows BEFORE the cursor that page 1 never had, so appending loses them + // for good — and page 2's family (no gitTimedOut) would clear the + // warning while rows are missing. Start over from page 1 instead. + const current = responseRef.current + if (cursor && current && !sameFamily(current.family, next.family)) { + void run(null) + return + } setResponse(prev => (cursor && prev ? { ...next, rows: [...prev.rows, ...next.rows] } : next)) setResponseKey(requestKey) } catch { @@ -73,6 +88,7 @@ export function useConversationList(params: ConversationListParams): { const hasResponse = useRef(false) hasResponse.current = response !== null + responseRef.current = response useEffect(() => { if (!open) { version.current += 1 @@ -98,3 +114,8 @@ export function useConversationList(params: ConversationListParams): { return { response, loading, error, needsPane, loadMore, stale } } + +/** Same family = same repository root, same roots, same git-timeout answer. */ +function sameFamily(a: ConversationListResponse['family'], b: ConversationListResponse['family']): boolean { + return a.repoRoot === b.repoRoot && a.gitTimedOut === b.gitTimedOut && a.roots.length === b.roots.length && a.roots.every((root, i) => root === b.roots[i]) +} diff --git a/src/renderer/src/features/worktrees/control.renderer.test.ts b/src/renderer/src/features/worktrees/control.renderer.test.ts index 9d9751994..385699376 100644 --- a/src/renderer/src/features/worktrees/control.renderer.test.ts +++ b/src/renderer/src/features/worktrees/control.renderer.test.ts @@ -36,3 +36,31 @@ describe('worktrees.read and a git timeout', () => { expect(await read({ ok: false, gitMissing: false })).toMatchObject({ gitUnavailable: true, gitMissing: false, gitTimedOut: false }) }) }) + +// #1430: git answered the status, but listing worktrees for the activity index +// then timed out. That used to read as "activity unavailable", the same as a +// missing index; it now says the git timeout, to agents and in the dump. +describe('worktrees.read when the activity lookup times out', () => { + async function readWithActivity(activity: unknown) { + const gitWorktreeStatus = vi.fn(async () => ({ ok: true, worktrees: [] })) + Object.defineProperty(window, 'api', { configurable: true, value: { gitWorktreeStatus, worktreeActivitySummary: vi.fn(async () => activity) } }) + useAppStore.setState({ workspaceState: { ...originalStore.workspaceState, sessions: { agent: { cwd: '/repo', kind: 'claude' } } } } as never) + const workspace = { state: { tabs: [], sessions: {}, pinnedSessionIds: [] }, runtimes: {} } as unknown as Workspace + const [capability] = worktreeControlCapabilities(() => workspace) + const result = await capability!.execute({ sessionId: 'agent' }, {} as never) + expect(result.ok).toBe(true) + return (result as { value: Record }).value + } + + it('says a git timeout apart from a missing activity index', async () => { + expect(await readWithActivity({ ok: false, timedOut: true })).toMatchObject({ activityUnavailable: true, activityTimedOut: true }) + expect(await readWithActivity({ ok: false })).toMatchObject({ activityUnavailable: true, activityTimedOut: false }) + }) + + it('and the text dump says it too', async () => { + const { formatWorktreeDump } = await import('./lib/formatWorktreeDump') + const base = { cwd: '/repo', generatedAt: 0, rows: [], indexStatus: null, gitUnavailable: false, gitMissing: false, activityUnavailable: true } + expect(formatWorktreeDump({ ...base, activityTimedOut: true } as never)).toContain('- Agent activity: unavailable (Git timed out)') + expect(formatWorktreeDump(base as never)).toContain('- Agent activity: unavailable\n') + }) +}) diff --git a/src/renderer/src/features/worktrees/control.ts b/src/renderer/src/features/worktrees/control.ts index 443a1cc67..a48aebb33 100644 --- a/src/renderer/src/features/worktrees/control.ts +++ b/src/renderer/src/features/worktrees/control.ts @@ -8,7 +8,7 @@ export function worktreeControlCapabilities(getWorkspace: () => Workspace) { return [defineCapability({ id: 'worktrees.read', title: 'Read worktree status and agent activity', execution: 'window', effect: 'read', target: { kind: 'session', field: 'sessionId' }, description: 'Read the Worktrees panel data for an exact agent’s repository, including branch/path, Git status, indexed activity and associated live agents. Bounded pages use a revision; changing Git/activity state requires a fresh read. Explicitly reports missing Git, a Git timeout (gitTimedOut: slow, not a verdict on the repository), non-repository and activity-index unavailability. Does not create, delete or change worktrees, or wake agents.', input: z.object({ sessionId: z.string(), refreshActivity: z.boolean().default(false), ...pageInput }).strict(), - output: pageSchema(z.json()).extend({ cwd: z.string(), generatedAt: z.number(), gitUnavailable: z.boolean(), gitMissing: z.boolean(), gitTimedOut: z.boolean(), activityUnavailable: z.boolean(), indexStatus: z.json().nullable() }), + output: pageSchema(z.json()).extend({ cwd: z.string(), generatedAt: z.number(), gitUnavailable: z.boolean(), gitMissing: z.boolean(), gitTimedOut: z.boolean(), activityUnavailable: z.boolean(), activityTimedOut: z.boolean(), indexStatus: z.json().nullable() }), handler: async input => { const workspace = getWorkspace(), meta = useAppStore.getState().workspaceState.sessions[input.sessionId] if (!meta) throw new ControlError('unavailable', 'Agent no longer exists') @@ -18,6 +18,8 @@ export function worktreeControlCapabilities(getWorkspace: () => Workspace) { // #1429 review a, b: without it a timed-out list reached agents as // "not a repository", the exact misreading the panel now avoids. gitTimedOut: dump.gitTimedOut === true, activityUnavailable: dump.activityUnavailable, + // #1430: the activity index was not missing; git timed out listing worktrees. + activityTimedOut: dump.activityTimedOut === true, indexStatus: z.json().parse(JSON.parse(JSON.stringify(dump.indexStatus))) } }, })] diff --git a/src/renderer/src/features/worktrees/lib/formatWorktreeDump.ts b/src/renderer/src/features/worktrees/lib/formatWorktreeDump.ts index ebcc738ad..7093bee50 100644 --- a/src/renderer/src/features/worktrees/lib/formatWorktreeDump.ts +++ b/src/renderer/src/features/worktrees/lib/formatWorktreeDump.ts @@ -45,7 +45,7 @@ export function formatWorktreeDump(dump: WorktreeDump): string { lines.push(`- Patch-equivalent: ${countRows(dump.rows, row => row.category === 'patch-equivalent')}`) lines.push(`- Cleanup/merged: ${countRows(dump.rows, row => row.category === 'cleanup-merged')}`) lines.push(`- Detached: ${countRows(dump.rows, row => row.detached)}`) - lines.push(`- Agent activity: ${dump.activityUnavailable ? 'unavailable' : 'available'}`) + lines.push(`- Agent activity: ${activityLabel(dump)}`) if (dump.indexStatus?.lastIndexedAt) { // Same #495 A15 rationale as the Generated line above. lines.push(`- Activity index updated: ${new Date(dump.indexStatus.lastIndexedAt).toISOString()}`) @@ -143,3 +143,9 @@ export function providerLabel(kind: SessionKind): string { function formatLiveAgent(agent: WorktreeDumpRow['liveAgents'][number]): string { return `${providerLabel(agent.kind)} ${agent.live ? 'active' : 'open'} in "${agent.tabTitle}"` } + +/** #1430: a git timeout is said as one, never as a missing activity index. */ +function activityLabel(dump: WorktreeDump): string { + if (!dump.activityUnavailable) return 'available' + return dump.activityTimedOut ? 'unavailable (Git timed out)' : 'unavailable' +} diff --git a/src/renderer/src/features/worktrees/lib/loadWorktreeDump.ts b/src/renderer/src/features/worktrees/lib/loadWorktreeDump.ts index d4d8f84dd..609098924 100644 --- a/src/renderer/src/features/worktrees/lib/loadWorktreeDump.ts +++ b/src/renderer/src/features/worktrees/lib/loadWorktreeDump.ts @@ -39,6 +39,9 @@ export type WorktreeDump = { * (#1250 row 11). */ gitTimedOut?: boolean activityUnavailable: boolean + /** activityUnavailable because git timed out while the activity index asked + * for this repository's worktrees (#1430), not because there is no index. */ + activityTimedOut?: boolean } export async function loadWorktreeDump(params: { @@ -92,6 +95,7 @@ export async function loadWorktreeDump(params: { gitUnavailable: false, gitMissing: false, activityUnavailable: !activityResult.ok, + activityTimedOut: !activityResult.ok && 'timedOut' in activityResult && activityResult.timedOut === true, } } diff --git a/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx index e97bac952..7ae60a6c6 100644 --- a/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx @@ -44,6 +44,7 @@ function makeRefs(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, + worktreeReconcilerRef: { current: null }, undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx index 61ed5db2a..37895d710 100644 --- a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx @@ -206,3 +206,76 @@ describe('what an older-history request reports', () => { expect(answer).toBe('skipped') }) }) + +// #1430 review c: the older-history loader's half of the round-1 fix was +// untested. A page read while `git worktree list` timed out must reach the live +// reconciler (so a recovered catalog replays it), and must NOT be attributed +// against a null family (ingestWorktreeRawEvent would throw on it). +async function loadOlderPageWhileGitTimesOut(initial: Partial = {}) { + let runtimes: Record = { + session: { ...emptyRuntime(), hasOlderHistory: true, historyOldestMarker: 'anchor', ...initial }, + } + const observed: unknown[][] = [] + const positions: Array = [] + const refresh = vi.fn(async () => 'failed' as const) + const refs = { + stateRef: ref({ sessions: { session: { kind: 'claude', cwd: '/tmp/project', providerSessionId: 'provider-session' } } }), + latestRuntimesRef: ref(runtimes), + seenUuidsRef: ref({}), + worktreeReconcilerRef: ref({ + observe: (_s: string, _c: string, entries: Array<{ entry: unknown }>, projection: unknown, position?: string) => { observed.push(entries.map(e => e.entry)); positions.push(position); return projection }, + refresh, + replayCachedCatalog: vi.fn(), + }), + } as unknown as WorkspaceRefs + const setRuntimes: WorkspaceSetRuntimes = next => { + runtimes = typeof next === 'function' ? next(runtimes) : next + refs.latestRuntimesRef.current = runtimes + } + const updateRuntime = (id: string, patch: Partial) => { + setRuntimes(prev => ({ ...prev, [id]: { ...prev[id]!, ...patch } })) + } + const older = { + type: 'assistant', + uuid: 'older-1', + timestamp: '2026-09-20T09:05:00.000Z', + message: { role: 'assistant', content: [{ type: 'tool_use', id: 'toolu_1', name: 'Write', input: { file_path: '/tmp/project/.worktrees/x/a.ts' } }] }, + } + Object.defineProperty(window, 'api', { configurable: true, value: { + loadOlderHistory: vi.fn().mockResolvedValue({ entries: [{ entries: [older], historyMarker: 'older-1' }], hasMore: false }), + gitWorktrees: vi.fn(async () => ({ ok: false, gitMissing: false, timedOut: true })), + } }) + const { result } = renderHook(() => useHistoryActions(setRuntimes, refs, updateRuntime, ipcSessionFeed)) + let outcome: unknown + await act(async () => { outcome = await result.current.loadOlderHistory('session') }) + return { outcome, observed, positions, refresh, older, runtime: () => runtimes.session } +} + +describe('an older page read while git timed out (#1430)', () => { + it('hands the page to the reconciler, asks it to refresh, and attributes nothing', async () => { + const { outcome, observed, positions, refresh, older, runtime } = await loadOlderPageWhileGitTimesOut() + + expect(outcome).not.toBe('failed') + expect(observed).toEqual([[{ entries: [older], historyMarker: 'older-1' }]]) + // As the OLDEST evidence, never the newest (#1450 B6 verify). + expect(positions).toEqual(['older']) + expect(refresh).toHaveBeenCalledWith('/tmp/project') + // Nothing was attributed against the unknown family: no activity folded + // from the page, no work context derived from it. + expect(runtime()?.workActivity).toBeNull() + expect(runtime()?.workContext).toBeNull() + }) + + it('never hands an older page over when the pane already knows a newer context (#1450 verification b)', async () => { + // The answered-git path backfills only an UNKNOWN context: older records + // must never replace fresher evidence. Handing the page to the reconciler + // appended it as if it were the newest evidence, so a recovered catalog + // moved the pane back to the older worktree. + const known = { worktreePath: '/tmp/project/.worktrees/newer' } as unknown as SessionRuntime['workContext'] + const { observed, refresh, runtime } = await loadOlderPageWhileGitTimesOut({ workContext: known }) + + expect(observed).toEqual([]) + expect(refresh).not.toHaveBeenCalled() + expect(runtime()?.workContext).toBe(known) + }) +}) diff --git a/src/renderer/src/workspace/hook/actions/history.ts b/src/renderer/src/workspace/hook/actions/history.ts index d41f1d92c..d341484f9 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -26,6 +26,8 @@ import type { WorkspaceSetRuntimes } from '@renderer/workspace/hook/context' import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' import * as perf from '@renderer/performance/client' import type { SessionFeed } from '@shared/sessionFeed/SessionFeed' +import { worktreesForAttribution } from '@renderer/workspace/work-context/worktreesForAttribution' +import { handHistoryToReconciler } from '@renderer/workspace/hook/actions/initialHistory' // Older history loader — called by Feed's scroll handler when the // user scrolls near the top. @@ -109,7 +111,17 @@ export function useHistoryActions( } const prepend: Entry[] = [] const worktreesResult = await window.api.gitWorktrees(meta.cwd) - const worktrees = worktreesResult.ok ? worktreesResult.worktrees : [] + // #1430: null = git timed out, family unknown → skip attribution (see + // worktreesForAttribution), and hand the page to the reconciler so a + // recovered catalog can still attribute it (handHistoryToReconciler). + // + // Only while the context is still UNKNOWN, the same recency rule as + // the answered-git backfill below (#1450 verification b): the + // reconciler appends what it observes as the NEWEST evidence, so an + // older page handed over while the pane already knew a newer + // worktree moved it back to the older one once git recovered. + const worktrees = worktreesForAttribution(worktreesResult) + if (worktrees === null && !runtime.workContext) handHistoryToReconciler(refs, sessionId, meta.cwd, chunk.entries, 'older') let workActivity = runtime.workActivity let workContext = runtime.workContext let oldestMarker: string | null = runtime.historyOldestMarker @@ -130,7 +142,7 @@ export function useHistoryActions( // Older-history pagination walks records that predate the current // tail. Use them only to backfill an unknown badge; never let old // worktree evidence replace fresher live/current context. - if (!workContext) { + if (!workContext && worktrees !== null) { workActivity = ingestWorktreeRawEvent({ state: workActivity, raw: rawEntry, diff --git a/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts new file mode 100644 index 000000000..d60f6ddd0 --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts @@ -0,0 +1,122 @@ +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' + +import { describe, expect, it } from 'vitest' + +import { emptyRuntime, type SessionRuntime } from '@renderer/session-runtime/state' +import { LiveWorktreeReconciler } from '@renderer/workspace/work-context/LiveWorktreeReconciler' +import { handHistoryToReconciler } from './initialHistory' + +// #1430 review a/b: a history chunk read while `git worktree list` timed out +// was skipped for worktree attribution, and nothing else ever saw it: the live +// reconciler replays only what it observed, so a quiet session whose writes were +// in a linked worktree stayed on the launch folder after git recovered. The +// loaders now hand such a chunk to the reconciler (handHistoryToReconciler), +// whose window replays it when the catalog answers. +// +// Recorded data: the Codex worktree window (13 records, main +// /fixture/project-1, the agent's actual worktree .../worktree-2) and the +// recorded `git worktree list` identities. +const fixtureDir = resolve(process.cwd(), 'testing', 'fixtures', 'worktree-live-attribution') +const codex = JSON.parse(readFileSync(resolve(fixtureDir, 'codex-0151-worktree-window.json'), 'utf8')) as { + git: { main: { path: string }; ui?: { path: string; branch: string } } + records: unknown[] +} +const catalog = (JSON.parse(readFileSync(resolve(fixtureDir, 'git-worktree-identities.json'), 'utf8')) as { + worktrees: Array<{ path: string; branch: string; detached: boolean }> +}).worktrees.map(worktree => ({ ...worktree, head: null })) + +describe('history read while git timed out (#1430 review a/b)', () => { + it('reaches the worktree the agent wrote in once git answers', async () => { + let gitAnswers = false + let runtime: SessionRuntime = emptyRuntime() + let reconciler!: LiveWorktreeReconciler + reconciler = new LiveWorktreeReconciler({ + loadWorktrees: async () => gitAnswers ? { ok: true, worktrees: catalog } : { ok: false, gitMissing: false, timedOut: true } as never, + onCatalogReady: cwd => { + const projection = reconciler.project({ sessionId: 'resumed', cwd, projection: runtime }) + runtime = { ...runtime, ...projection } + }, + }) + const refs = { worktreeReconcilerRef: { current: reconciler }, latestRuntimesRef: { current: { resumed: runtime } } } + + // The initial history load: git timed out, so the chunk is handed over. + handHistoryToReconciler(refs as never, 'resumed', codex.git.main.path, codex.records) + // The refresh the loader asked for still sees git time out; a failed probe + // is not cached, so the next refresh (any live batch or catalog event) + // asks again — and git has recovered by then. + expect(await reconciler.refresh(codex.git.main.path)).toBe('failed') + expect(runtime.workContext).toBeNull() + gitAnswers = true + expect(await reconciler.refresh(codex.git.main.path)).toBe('ready') + + expect(runtime.workContext?.worktreePath).toBe(codex.git.ui?.path) + }) + + it('repaints at once when the reconciler already holds a fresh catalog (#1450 verification a)', async () => { + // A live event loaded the catalog before this pane's history arrived; only + // the history's own gitWorktrees call timed out. refresh() then answers + // 'cached' and never calls onCatalogReady, so without a replay the chunk + // sat in the window and the pane stayed on the launch folder. + let runtime: SessionRuntime = emptyRuntime() + let reconciler!: LiveWorktreeReconciler + reconciler = new LiveWorktreeReconciler({ + loadWorktrees: async () => ({ ok: true, worktrees: catalog }), + onCatalogReady: cwd => { + const projection = reconciler.project({ sessionId: 'resumed', cwd, projection: runtime }) + runtime = { ...runtime, ...projection } + }, + }) + expect(await reconciler.refresh(codex.git.main.path)).toBe('ready') + expect(runtime.workContext?.worktreePath).not.toBe(codex.git.ui?.path) + const refs = { worktreeReconcilerRef: { current: reconciler }, latestRuntimesRef: { current: { resumed: runtime } } } + + handHistoryToReconciler(refs as never, 'resumed', codex.git.main.path, codex.records) + await Promise.resolve() + await Promise.resolve() + + expect(runtime.workContext?.worktreePath).toBe(codex.git.ui?.path) + }) + + it('an older page read during the timeout never outranks the newest chunk (#1450 B6 verify)', async () => { + // B6's sequence: the initial history (newest) is handed over while git + // times out; the user scrolls up while it still times out, and the older + // page is handed over too; git recovers. The older page must stay OLDER + // evidence: the pane belongs where the newest records put it. + // + // Older page = the recorded worktree-2 window. Newest chunk = the same + // recorded records moved to worktree-1 and 1 h later (only cwd and + // timestamp change, so the record shape is the recorded one). + const worktree1 = catalog.find(w => w.path.endsWith('/worktree-1'))!.path + const newest = codex.records.map(record => { + const r = structuredClone(record) as { timestamp?: string; payload?: { item?: { cwd?: string } } } + if (r.timestamp) r.timestamp = new Date(Date.parse(r.timestamp) + 3_600_000).toISOString() + if (r.payload?.item?.cwd) r.payload.item.cwd = `file://${worktree1}` + return r + }) + let gitAnswers = false + let runtime: SessionRuntime = emptyRuntime() + let reconciler!: LiveWorktreeReconciler + reconciler = new LiveWorktreeReconciler({ + loadWorktrees: async () => gitAnswers ? { ok: true, worktrees: catalog } : { ok: false, gitMissing: false, timedOut: true } as never, + onCatalogReady: cwd => { + const projection = reconciler.project({ sessionId: 'resumed', cwd, projection: runtime }) + runtime = { ...runtime, ...projection } + }, + }) + const refs = { worktreeReconcilerRef: { current: reconciler }, latestRuntimesRef: { current: { resumed: runtime } } } + + handHistoryToReconciler(refs as never, 'resumed', codex.git.main.path, newest) + expect(await reconciler.refresh(codex.git.main.path)).toBe('failed') + handHistoryToReconciler(refs as never, 'resumed', codex.git.main.path, codex.records, 'older') + expect(await reconciler.refresh(codex.git.main.path)).toBe('failed') + gitAnswers = true + expect(await reconciler.refresh(codex.git.main.path)).toBe('ready') + + expect(runtime.workContext?.worktreePath).toBe(worktree1) + }) + + it('does nothing without a reconciler or without records', () => { + expect(() => handHistoryToReconciler({ worktreeReconcilerRef: { current: null }, latestRuntimesRef: { current: {} } } as never, 's', '/x', [{}])).not.toThrow() + }) +}) diff --git a/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx index 965cf8898..ff5ba1d6c 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx @@ -97,3 +97,48 @@ describe('the initial-history loader under failure and repetition', () => { expect(gated).toHaveBeenCalledTimes(3) }) }) + +// #1430 review a/b: when `git worktree list` times out during the initial +// load, the chunk is not attributed against an empty family — and it is not +// dropped either: the loader hands its records to the live reconciler, whose +// window replays them once git answers (handHistoryToReconciler). +describe('the initial-history loader when git times out (#1430)', () => { + it('hands the chunk to the worktree reconciler and asks it to refresh', async () => { + const { history } = emptySession() + const pane = rehydratedPane() + const observed: unknown[][] = [] + const refresh = vi.fn(async () => 'failed' as const) + pane.refs.worktreeReconcilerRef.current = { + observe: (_sessionId, _cwd, entries, projection) => { observed.push(entries.map(e => e.entry)); return projection }, + refresh, + replayCachedCatalog: vi.fn(), + } + scope.extendApi({ + gitWorktrees: async () => ({ ok: false, gitMissing: false, timedOut: true }), + loadInitialHistory: async (request: { cwd: string; providerSessionId: string; limit: number }) => { + const chunk = await history.loadInitialHistory(request) + return { ...chunk, entries: [{ type: 'recorded-row', n: 1 }, ...chunk.entries] } + }, + }) + await loadInitialHistoryForSession({ sessionId: SESSION_ID, meta: pane.meta, refs: pane.refs, setRuntimes: pane.setRuntimes }) + expect(observed).toHaveLength(1) + expect(observed[0]![0]).toEqual({ type: 'recorded-row', n: 1 }) + expect(refresh).toHaveBeenCalledWith(pane.meta.cwd) + }) + + it('does not hand anything over when git answered', async () => { + const { history } = emptySession() + const pane = rehydratedPane() + const observe = vi.fn() + pane.refs.worktreeReconcilerRef.current = { observe, refresh: vi.fn(), replayCachedCatalog: vi.fn() } + scope.extendApi({ + gitWorktrees: async () => ({ ok: true, worktrees: [] }), + loadInitialHistory: async (request: { cwd: string; providerSessionId: string; limit: number }) => { + const chunk = await history.loadInitialHistory(request) + return { ...chunk, entries: [{ type: 'recorded-row', n: 1 }, ...chunk.entries] } + }, + }) + await loadInitialHistoryForSession({ sessionId: SESSION_ID, meta: pane.meta, refs: pane.refs, setRuntimes: pane.setRuntimes }) + expect(observe).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/workspace/hook/actions/initialHistory.ts b/src/renderer/src/workspace/hook/actions/initialHistory.ts index ebcb2fca7..32a225291 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.ts +++ b/src/renderer/src/workspace/hook/actions/initialHistory.ts @@ -1,6 +1,7 @@ import { DEFAULT_PROVIDER, isAgentProviderKind } from '@shared/types/providerKind' import type { Entry } from '@shared/types/transcript' import { emptyRuntime } from '@renderer/session-runtime/state' +import { worktreesForAttribution } from '@renderer/workspace/work-context/worktreesForAttribution' import type { SessionRuntime } from '@renderer/session-runtime/state' import type { SessionId, SessionMeta } from '@renderer/workspace/types' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' @@ -300,11 +301,14 @@ export async function loadInitialHistoryForSession({ historyRead, window.api.gitWorktrees(meta.cwd), ]) - const worktrees = worktreesResult.ok ? worktreesResult.worktrees : [] + // #1430: null = git timed out, family unknown → skip attribution for this + // chunk (see worktreesForAttribution). + const worktrees = worktreesForAttribution(worktreesResult) if (superseded()) { span.end({ fetched: chunk.entries.length, hasMore: chunk.hasMore, superseded: true }) return settleSuperseded() } + if (worktrees === null) handHistoryToReconciler(refs, sessionId, meta.cwd, chunk.entries) setRuntimes(prev => { const current = prev[sessionId] @@ -339,13 +343,15 @@ export async function loadInitialHistoryForSession({ const toolResultIndex = current.toolResultIndex for (const [rawIndex, raw] of chunk.entries.entries()) { - workActivity = ingestWorktreeRawEvent({ - state: workActivity, - raw, - worktrees, - sessionCwd: meta.cwd, - }) - workContext = deriveAgentWorkContext(workActivity) + if (worktrees !== null) { + workActivity = ingestWorktreeRawEvent({ + state: workActivity, + raw, + worktrees, + sessionCwd: meta.cwd, + }) + workContext = deriveAgentWorkContext(workActivity) + } const { entries: mapped, historyMarker: marker } = mapper.map(raw) // Marker policy (site-owned): the FIRST kept line of the @@ -586,3 +592,39 @@ export function reconcileStuckTranscriptLoads({ } return reKicked } + +/** + * A history chunk read while `git worktree list` timed out (#1430 review a/b). + * + * WHY hand it to the live reconciler instead of skipping it: skipping avoided + * a wrong attribution (against an empty family) but threw the chunk's worktree + * evidence away for good — the reconciler only replays what it observed, so a + * quiet session whose writes were in a linked worktree stayed on the launch + * folder after git recovered. observe() keeps the chunk's RELEVANT records in + * its bounded window (deferred while no catalog is cached) and refresh() asks + * git again; when the catalog lands, onCatalogReady replays the window against + * it and repaints the pane. Outside any setState updater, because observe is + * a side effect and an updater may run twice. The current runtime is the + * baseline, as for a live batch. + */ +export function handHistoryToReconciler( + refs: Pick, + sessionId: SessionId, + cwd: string, + entries: readonly unknown[], + // 'older' for an older-history page: it enters the reconciler's window as + // the OLDEST evidence, so a scroll-up during a git timeout never outranks + // the newest chunk (#1450 B6 verify). + position: 'newest' | 'older' = 'newest', +): void { + const reconciler = refs.worktreeReconcilerRef.current + if (!reconciler || entries.length === 0) return + reconciler.observe(sessionId, cwd, entries.map(entry => ({ entry })), refs.latestRuntimesRef.current[sessionId] ?? emptyRuntime(), position) + // 'cached' means a fresh catalog was already there, so no onCatalogReady is + // coming: replay now or the chunk waits for an unrelated event (#1450 + // verification a). 'ready' already replayed; 'failed' is retried by the + // next refresh, which notifies when git answers. + void reconciler.refresh(cwd).then(outcome => { + if (outcome === 'cached') reconciler.replayCachedCatalog(cwd) + }) +} diff --git a/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx index ffdf3e637..14151d937 100644 --- a/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx @@ -71,6 +71,7 @@ describe('renderer session replacement handoff', () => { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, + worktreeReconcilerRef: { current: null }, undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx b/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx index ae67475c5..a0fa57d73 100644 --- a/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx +++ b/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx @@ -43,6 +43,7 @@ export function makeRefs(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, + worktreeReconcilerRef: { current: null }, undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts b/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts index 981814b13..534d7adf8 100644 --- a/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts +++ b/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts @@ -22,6 +22,7 @@ export function makeWorkspaceRefsForTest(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, + worktreeReconcilerRef: { current: null } as never, undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts index 256b8b013..d6004812c 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts @@ -616,6 +616,10 @@ export function useIpcSubscriptions( refs.seenUuidsRef.current, ) }, MEMORY_GAUGE_INTERVAL_MS) + // Published for the history loaders (#1430 review a/b): a chunk read while + // git timed out is handed here instead of being dropped. See + // WorkspaceRefs.worktreeReconcilerRef. + refs.worktreeReconcilerRef.current = worktreeReconciler const refreshWorktrees = (cwd: string | null | undefined): void => { // The reconciler owns failure/retry/coalescing. This listener intentionally // does not await decorative Git metadata and therefore cannot delay a @@ -2830,6 +2834,7 @@ export function useIpcSubscriptions( }) return () => { + if (refs.worktreeReconcilerRef.current === worktreeReconciler) refs.worktreeReconcilerRef.current = null worktreeReconciler.dispose() window.clearInterval(orphanSweepTimer) window.clearInterval(memoryGaugeTimer) diff --git a/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx b/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx index dd18bdbd0..709d2dd4f 100644 --- a/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx @@ -219,6 +219,7 @@ function makeRefs(state: WorkspaceState, runtimes: Record> + /** The live worktree reconciler, owned by useIpcSubscriptions' effect and + * published here (null outside it). #1430 review a/b: a history chunk read + * while `git worktree list` timed out cannot be attributed, and dropping it + * lost its worktree evidence for good. The loaders hand such a chunk to + * the reconciler instead, whose bounded window replays it when the + * catalog answers. */ + worktreeReconcilerRef: MutableRefObject | null> undoStackRef: MutableRefObject bootstrapTimersRef: MutableRefObject>> persistedFeedDebugIdRef: MutableRefObject> @@ -104,6 +112,7 @@ export function useWorkspaceRefs( const seenUuidsRef = useRef>>({}) const historyWindowsRef = useRef>({}) const historyAwaitingTurnStartRef = useRef>(new Set()) + const worktreeReconcilerRef = useRef | null>(null) const undoStackRef = useRef(new UndoCloseStack()) const bootstrapTimersRef = useRef>>(new Map()) const pendingAdoptionWindowIdsRef = useRef([]) @@ -141,6 +150,7 @@ export function useWorkspaceRefs( seenUuidsRef, historyWindowsRef, historyAwaitingTurnStartRef, + worktreeReconcilerRef, // Latest screen per session — mirrored from state into a ref so // the Enter handler in TileLeaf can capture a baseline diff --git a/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts b/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts index bf5a72669..9fc2e5f72 100644 --- a/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts +++ b/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts @@ -109,6 +109,12 @@ export class LiveWorktreeReconciler { cwd: string, entries: ReadonlyArray<{ entry: unknown }>, projection: WorktreeRuntimeProjection, + // 'older' = an older-history page (#1450 B6 verify). Its records predate + // everything already retained, so they enter at the OLD end of the + // window. Appended like a live batch, a scroll-up during a git timeout + // became the newest evidence and moved the pane to the older worktree + // once git recovered: folding is in window order, and the last write wins. + position: 'newest' | 'older' = 'newest', ): WorktreeRuntimeProjection { if (this.disposed) return projection let evidence = this.evidenceBySession.get(sessionId) @@ -160,6 +166,13 @@ export class LiveWorktreeReconciler { const relevantRaw = entries .map(({ entry }) => entry) .filter(entry => extractWorktreeActivityEvents(entry, this.now()).length > 0) + if (position === 'older') { + this.retainOlder(cwd, evidence, relevantRaw) + this.evidenceBySession.set(sessionId, evidence) + const next = this.rebuild(cwd, evidence) + evidence.lastEmitted = next + return next + } evidence.recentRaw.push(...relevantRaw) // Length is not a generation: after eviction this window stays at 500 // while its contents keep changing. Irrelevant transport batches do not @@ -198,6 +211,60 @@ export class LiveWorktreeReconciler { return next } + /** + * Replay retained evidence against the catalog already cached for `cwd`. + * + * WHY (#1450 verification a): refresh() notifies only when git ANSWERS. A + * history chunk handed over while its own gitWorktrees call timed out can + * land after a live event already cached a fresh catalog; refresh() then + * answers 'cached' and never notifies, so the chunk sat in the window and + * the pane stayed on the launch folder until an unrelated event. Only with + * a real catalog (refreshedAt > 0): the placeholder an in-flight probe + * writes is empty, and replaying against it is the wrong-family read #1430 + * removes. + */ + replayCachedCatalog(cwd: string): void { + if (this.disposed) return + const cached = this.cache.get(cwd) + if (!cached || cached.refreshedAt <= 0) return + this.onCatalogReady(cwd) + } + + /** + * Put an older page's relevant records at the OLD end of the retained + * evidence, under the same 2 × recentRawLimit bound as live batches. + * + * Order of age, oldest first: deferredRaw, then recentRaw. The page predates + * both, so it goes in front of whichever is non-empty first. What does not + * fit is the oldest evidence there is, so it is what gets dropped: + * - With a cached catalog the baseline already holds folded records that are + * NEWER than this page, and folding the page on top of them would make it + * the newest again. So the overflow is dropped, never folded; the window + * is full of newer evidence anyway. + * - Without a catalog the overflow moves into deferredRaw's front, and what + * deferredRaw cannot hold is counted in droppedBeforeCatalog, as for live + * evictions. + */ + private retainOlder(cwd: string, evidence: SessionEvidence, relevantRaw: unknown[]): void { + if (relevantRaw.length === 0) return + evidence.revision += 1 + if (evidence.deferredRaw.length > 0) { + evidence.deferredRaw.unshift(...relevantRaw) + } else { + evidence.recentRaw.unshift(...relevantRaw) + if (evidence.recentRaw.length > this.recentRawLimit) { + const overflow = evidence.recentRaw.splice(0, evidence.recentRaw.length - this.recentRawLimit) + const cached = this.cache.get(cwd) + if (!(cached && cached.refreshedAt > 0)) evidence.deferredRaw.unshift(...overflow) + } + } + if (evidence.deferredRaw.length > this.recentRawLimit) { + const overflow = evidence.deferredRaw.length - this.recentRawLimit + evidence.deferredRaw.splice(0, overflow) + evidence.droppedBeforeCatalog += overflow + } + } + forgetSession(sessionId: SessionId): void { this.evidenceBySession.delete(sessionId) } diff --git a/src/renderer/src/workspace/work-context/worktreesForAttribution.test.ts b/src/renderer/src/workspace/work-context/worktreesForAttribution.test.ts new file mode 100644 index 000000000..bc395540c --- /dev/null +++ b/src/renderer/src/workspace/work-context/worktreesForAttribution.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' + +import { worktreesForAttribution } from './worktreesForAttribution' + +// #1430: history chunks attributed a TIMED-OUT worktree list as `[]` (no +// worktrees), recording the pane's work as outside any worktree. The three +// answers `git:worktrees` gives (see src/preload/api/git.ts) map to three +// different things. +describe('worktreesForAttribution', () => { + const worktrees = [{ path: '/repo' }, { path: '/repo/.worktrees/a' }] + + it('attributes against the list git answered', () => { + expect(worktreesForAttribution({ ok: true, worktrees })).toBe(worktrees) + }) + + it('skips attribution when git timed out: the family is unknown, not empty', () => { + expect(worktreesForAttribution({ ok: false, gitMissing: false, timedOut: true } as never)).toBeNull() + }) + + it('attributes against no worktrees for a real non-repository or missing git', () => { + expect(worktreesForAttribution({ ok: false, gitMissing: false } as never)).toEqual([]) + expect(worktreesForAttribution({ ok: false, gitMissing: true } as never)).toEqual([]) + }) +}) diff --git a/src/renderer/src/workspace/work-context/worktreesForAttribution.ts b/src/renderer/src/workspace/work-context/worktreesForAttribution.ts new file mode 100644 index 000000000..278ecbe20 --- /dev/null +++ b/src/renderer/src/workspace/work-context/worktreesForAttribution.ts @@ -0,0 +1,16 @@ +/** + * Which worktree list a history chunk's events are attributed against (#1430). + * + * WHY a TIMED-OUT list answers `null` (skip attribution) and not `[]`: `[]` + * is what a non-repository really has, and attributing against it is right + * there. For a timeout the family is UNKNOWN — attributing against `[]` would + * record the pane's work as outside any worktree, a wrong answer the live + * reconciler then has to overwrite. Skipping leaves the pane's work context as + * it was; the live reconciler fills it in when git answers. + */ +export function worktreesForAttribution( + result: { ok: true; worktrees: W[] } | { ok: false; timedOut?: boolean }, +): W[] | null { + if (result.ok) return result.worktrees + return 'timedOut' in result && result.timedOut === true ? null : [] +} diff --git a/src/shared/conversations/types.ts b/src/shared/conversations/types.ts index f629fd256..17300b67e 100644 --- a/src/shared/conversations/types.ts +++ b/src/shared/conversations/types.ts @@ -77,7 +77,10 @@ export type ConversationListResponse = { total: number hiddenChildren: number nextCursor: string | null - family: { repoRoot: string | null; roots: string[] } + /** `gitTimedOut` (#1430): `git worktree list` did not answer in time, so the + * repository's other worktrees could not be included; the rows may be + * missing some. Absent when git answered. */ + family: { repoRoot: string | null; roots: string[]; gitTimedOut?: true } timing: { ms: number } }