From b80a7fe6c896fe3beccd01dbd3f51a3ba7ceecda Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 08:43:22 -0700 Subject: [PATCH 1/8] docs(git): plan the other consumers of a timed-out worktree list (#1430) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 docs/plans/2026-09-27-worktree-timeout-consumers.md 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..e6bba370e --- /dev/null +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -0,0 +1,86 @@ +# 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. The recorder already catches with + the cwd, and it now warns that this row's repository is unknown. + - Ruling: one retry, never a loop. The recorder awaits this on every + interval open. Cost if wrong: one mis-filed interval, which heals on + the next. +- **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. +- `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. From ca5db472f23276bbc77f6770cfbf5fd0ac941341 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 08:52:01 -0700 Subject: [PATCH 2/8] fix(git): a timed-out worktree list is never read as 'no family' (#1430) Follow-up of #1429. listWorktreesForCwdDetailed is exported and every other consumer acts on timedOut: the conversations family is marked gitTimedOut, not cached, and the picker says siblings may be missing; worktree activity answers { ok: false, timedOut: true } (worktrees.read activityTimedOut, the dump says 'Git timed out'); the agent-activity repo root retries once and then throws instead of filing a worktree's activity under its folder; history chunks skip worktree attribution on a timeout (worktreesForAttribution). Co-Authored-By: Claude Opus 5.5 --- .../agentActivity/AgentActivityRecorder.ts | 7 +++- .../agentActivity/resolveRepoRoot.test.ts | 35 ++++++++++++++++ src/main/agentActivity/resolveRepoRoot.ts | 34 +++++++++++++++ src/main/conversations/catalog/listing.ts | 2 +- src/main/conversations/family.ts | 22 +++++++++- src/main/conversations/service.system.test.ts | 42 +++++++++++++++++++ src/main/conversations/service.ts | 9 ++-- src/main/index.ts | 9 ++-- src/main/ipc/git.ts | 2 +- src/main/ipc/worktreeActivity.test.ts | 39 +++++++++++++++++ src/main/ipc/worktreeActivity.ts | 8 +++- src/preload/api/git.ts | 4 +- .../ui/ConversationsPicker.renderer.test.tsx | 19 +++++++++ .../conversations/ui/ConversationsPicker.tsx | 13 ++++++ .../worktrees/control.renderer.test.ts | 28 +++++++++++++ .../src/features/worktrees/control.ts | 4 +- .../worktrees/lib/formatWorktreeDump.ts | 8 +++- .../worktrees/lib/loadWorktreeDump.ts | 4 ++ .../src/workspace/hook/actions/history.ts | 7 +++- .../workspace/hook/actions/initialHistory.ts | 21 ++++++---- .../worktreesForAttribution.test.ts | 24 +++++++++++ .../work-context/worktreesForAttribution.ts | 16 +++++++ src/shared/conversations/types.ts | 5 ++- 23 files changed, 335 insertions(+), 27 deletions(-) create mode 100644 src/main/agentActivity/resolveRepoRoot.test.ts create mode 100644 src/main/agentActivity/resolveRepoRoot.ts create mode 100644 src/main/ipc/worktreeActivity.test.ts create mode 100644 src/renderer/src/workspace/work-context/worktreesForAttribution.test.ts create mode 100644 src/renderer/src/workspace/work-context/worktreesForAttribution.ts diff --git a/src/main/agentActivity/AgentActivityRecorder.ts b/src/main/agentActivity/AgentActivityRecorder.ts index 97d614639..2260d08e1 100644 --- a/src/main/agentActivity/AgentActivityRecorder.ts +++ b/src/main/agentActivity/AgentActivityRecorder.ts @@ -250,7 +250,12 @@ 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: a rejection is "repository unknown" (git timed out twice). This + // interval is filed under its cwd and that is said; the next asks again. + repoRoot: cwd ? await this.deps.resolveRepoRoot(cwd).catch((error: unknown) => { + console.warn('[agent-activity] repository unknown for this interval, filed under its folder:', error instanceof Error ? error.message : error) + return cwd + }) : '', 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..82bbfff4e --- /dev/null +++ b/src/main/agentActivity/resolveRepoRoot.ts @@ -0,0 +1,34 @@ +/** + * 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's existing catch then uses the cwd for that one interval and + * says so, and the next interval asks again. 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 31750b88c..3e3b99ba6 100644 --- a/src/main/conversations/service.system.test.ts +++ b/src/main/conversations/service.system.test.ts @@ -68,3 +68,45 @@ describe('ConversationService', () => { expect(s.discoveriesForTests()).toBe(1) }) }) + +// #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 374a096a4..3b6b7280e 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..e8975dc5a 100644 --- a/src/main/ipc/git.ts +++ b/src/main/ipc/git.ts @@ -243,7 +243,7 @@ export async function listWorktreesForCwd(cwd: string): Promise { +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)) { 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..16f58b4bf 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,22 @@ 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() + }) +}) diff --git a/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx b/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx index b5cf77943..21a95a1cd 100644 --- a/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx +++ b/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx @@ -37,6 +37,10 @@ const SCOPES: Array<{ id: ConversationScope; label: string }> = [ { id: 'everywhere', label: 'Everywhere' }, ] +// Fixed words (#1430). Only outside 'cwd' scope: there siblings are not +// wanted anyway, so a timed-out sibling list changes nothing. +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 +286,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 !== 'cwd' && ( +
+ {GIT_TIMED_OUT_NOTE} +
+ )}
{ 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/history.ts b/src/renderer/src/workspace/hook/actions/history.ts index d41f1d92c..3fec034eb 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -26,6 +26,7 @@ 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' // Older history loader — called by Feed's scroll handler when the // user scrolls near the top. @@ -109,7 +110,9 @@ 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). + const worktrees = worktreesForAttribution(worktreesResult) let workActivity = runtime.workActivity let workContext = runtime.workContext let oldestMarker: string | null = runtime.historyOldestMarker @@ -130,7 +133,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/initialHistory.ts b/src/renderer/src/workspace/hook/actions/initialHistory.ts index ebcb2fca7..31356120a 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,7 +301,9 @@ 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() @@ -339,13 +342,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 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 } } From 74a5c0b97266df0c2fb49fc2e55878a829af62c5 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 08:55:01 -0700 Subject: [PATCH 3/8] fix(agent-activity): an unresolved repository is recorded as Unknown, never as the worktree folder (#1430, q126) Two git timeouts made the recorder fall back to the cwd as repoRoot; the store persisted it and summarize grouped by it, so a worktree became a repository of its own that later intervals could not fold back. It is now recorded as '' (the existing Unknown), with the cwd still naming the worktree row. Pinned through the real recorder and store. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 18 +++++-- .../AgentActivityRecorder.test.ts | 54 +++++++++++++++++++ .../agentActivity/AgentActivityRecorder.ts | 14 +++-- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md index e6bba370e..feb66b097 100644 --- a/docs/plans/2026-09-27-worktree-timeout-consumers.md +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -49,11 +49,17 @@ says it or stays unknown, and none caches or records the wrong family. `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. The recorder already catches with - the cwd, and it now warns that this row's repository is unknown. + - 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. Cost if wrong: one mis-filed interval, which heals on - the next. + 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 @@ -71,6 +77,10 @@ says it or stays unknown, and none caches or records the wrong family. - 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` diff --git a/src/main/agentActivity/AgentActivityRecorder.test.ts b/src/main/agentActivity/AgentActivityRecorder.test.ts index 3cd497f12..ec741eb0d 100644 --- a/src/main/agentActivity/AgentActivityRecorder.test.ts +++ b/src/main/agentActivity/AgentActivityRecorder.test.ts @@ -474,3 +474,57 @@ 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)) + + 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 2260d08e1..8c8fbc8ab 100644 --- a/src/main/agentActivity/AgentActivityRecorder.ts +++ b/src/main/agentActivity/AgentActivityRecorder.ts @@ -250,11 +250,17 @@ export class AgentActivityRecorder { provider: placement?.kind ?? entry.kind ?? 'unknown', tabId: placement?.tabId ?? null, tabTitle: placement?.tabTitle ?? null, - // #1430: a rejection is "repository unknown" (git timed out twice). This - // interval is filed under its cwd and that is said; the next asks again. + // #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, filed under its folder:', error instanceof Error ? error.message : error) - return cwd + console.warn('[agent-activity] repository unknown for this interval (recorded as Unknown):', error instanceof Error ? error.message : error) + return '' }) : '', cwd, } From 619b3da38db2509f1ab6e15c77028c7936a4bed8 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 10:51:16 -0700 Subject: [PATCH 4/8] fix(git): #1430 review a/b: restart paging on a family change; hand timed-out history to the reconciler a: a page built after git recovered is not appended to one built while it timed out (useConversationList restarts from page 1 when the family changes). a+b: a history chunk read while git timed out is handed to the live reconciler (WorkspaceRefs.worktreeReconcilerRef, handHistoryToReconciler) so a recovered catalog replays it, instead of losing its worktree evidence. b: the picker note only in Repository scope. a: assert the repository- unknown warning. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 28 +++++++++ .../AgentActivityRecorder.test.ts | 3 + .../ui/ConversationsPicker.renderer.test.tsx | 35 +++++++++++ .../conversations/ui/ConversationsPicker.tsx | 7 ++- .../conversations/useConversationList.ts | 21 +++++++ .../agentIndexNavigation.renderer.test.tsx | 1 + .../src/workspace/hook/actions/history.ts | 5 +- .../historyWorktreeTimeout.renderer.test.ts | 59 +++++++++++++++++++ .../actions/initialHistory.renderer.test.tsx | 44 ++++++++++++++ .../workspace/hook/actions/initialHistory.ts | 27 +++++++++ ...essionReplacementHandoff.renderer.test.tsx | 1 + .../actions/testing/paneActionsHarness.tsx | 1 + .../hook/ipc/testing/workspaceRefsForTest.ts | 1 + .../workspace/hook/ipc/useIpcSubscriptions.ts | 5 ++ src/renderer/src/workspace/hook/refs.ts | 10 ++++ 15 files changed, 244 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md index feb66b097..e0e2dac32 100644 --- a/docs/plans/2026-09-27-worktree-timeout-consumers.md +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -94,3 +94,31 @@ says it or stays unknown, and none caches or records the wrong family. `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. diff --git a/src/main/agentActivity/AgentActivityRecorder.test.ts b/src/main/agentActivity/AgentActivityRecorder.test.ts index ec741eb0d..a4addb140 100644 --- a/src/main/agentActivity/AgentActivityRecorder.test.ts +++ b/src/main/agentActivity/AgentActivityRecorder.test.ts @@ -515,6 +515,9 @@ describe('AgentActivityRecorder when git times out resolving the repository', () 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([ 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 16f58b4bf..31dad3c5b 100644 --- a/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx +++ b/src/renderer/src/features/conversations/ui/ConversationsPicker.renderer.test.tsx @@ -429,3 +429,38 @@ describe('ConversationsPicker when git timed out', () => { 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 21a95a1cd..502b72330 100644 --- a/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx +++ b/src/renderer/src/features/conversations/ui/ConversationsPicker.tsx @@ -37,8 +37,9 @@ const SCOPES: Array<{ id: ConversationScope; label: string }> = [ { id: 'everywhere', label: 'Everywhere' }, ] -// Fixed words (#1430). Only outside 'cwd' scope: there siblings are not -// wanted anyway, so a timed-out sibling list changes nothing. +// 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) { @@ -290,7 +291,7 @@ export function ConversationsPicker({ open, focusSearch, workspace, onClose }: P 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 !== 'cwd' && ( + {response?.family.gitTimedOut && scope === 'repository' && (
{GIT_TIMED_OUT_NOTE}
diff --git a/src/renderer/src/features/conversations/useConversationList.ts b/src/renderer/src/features/conversations/useConversationList.ts index 860dabcef..f036998fd 100644 --- a/src/renderer/src/features/conversations/useConversationList.ts +++ b/src/renderer/src/features/conversations/useConversationList.ts @@ -33,6 +33,9 @@ export function useConversationList(params: ConversationListParams): { stale: boolean } { const version = useRef(0) + // The response the NEXT page would be appended to (read inside run, which + // must not depend on it: that would re-create run on every page). + const responseRef = useRef(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/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.ts b/src/renderer/src/workspace/hook/actions/history.ts index 3fec034eb..be70e74ca 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -27,6 +27,7 @@ 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. @@ -111,8 +112,10 @@ export function useHistoryActions( const prepend: Entry[] = [] const worktreesResult = await window.api.gitWorktrees(meta.cwd) // #1430: null = git timed out, family unknown → skip attribution (see - // worktreesForAttribution). + // worktreesForAttribution), and hand the page to the reconciler so a + // recovered catalog can still attribute it (handHistoryToReconciler). const worktrees = worktreesForAttribution(worktreesResult) + if (worktrees === null) handHistoryToReconciler(refs, sessionId, meta.cwd, chunk.entries) let workActivity = runtime.workActivity let workContext = runtime.workContext let oldestMarker: string | null = runtime.historyOldestMarker 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..f876727da --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts @@ -0,0 +1,59 @@ +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('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..86cbe808d 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,47 @@ 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, + } + 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() } + 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 31356120a..ba94e1c91 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.ts +++ b/src/renderer/src/workspace/hook/actions/initialHistory.ts @@ -308,6 +308,7 @@ export async function loadInitialHistoryForSession({ 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] @@ -591,3 +592,29 @@ 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[], +): 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()) + void reconciler.refresh(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/refs.ts b/src/renderer/src/workspace/hook/refs.ts index 058ad5840..d836f6a31 100644 --- a/src/renderer/src/workspace/hook/refs.ts +++ b/src/renderer/src/workspace/hook/refs.ts @@ -10,6 +10,7 @@ import type { } from '@renderer/workspace/types' import type { SessionId, WorkspaceState } from '@renderer/workspace/types' import type { BuiltInMcpDefaultsInput } from '@mcp/shared/types' +import type { LiveWorktreeReconciler } from '@renderer/workspace/work-context/LiveWorktreeReconciler' // ----------------------------------------------------------------------------- // Ref factory for the workspace hook. @@ -42,6 +43,13 @@ export type WorkspaceRefs = { * semantic suffixes must be dropped until a fresh turn_started (the pure * owner's awaiting gate; api_error passes because it is diagnostic). */ historyAwaitingTurnStartRef: MutableRefObject> + /** 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 From 93ff4161701dcb902e9aa6b75f5096647f0b8326 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:14:00 -0700 Subject: [PATCH 5/8] test(git): pin the older-history timeout hand-off and its null guard; drop the dead lister (#1430 review c) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 9 ++++ src/main/ipc/git.ts | 9 ++-- .../hook/actions/history.renderer.test.tsx | 51 +++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md index e0e2dac32..45ad59dd3 100644 --- a/docs/plans/2026-09-27-worktree-timeout-consumers.md +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -122,3 +122,12 @@ says it or stays unknown, and none caches or records the wrong family. 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. diff --git a/src/main/ipc/git.ts b/src/main/ipc/git.ts index e8975dc5a..51fba7867 100644 --- a/src/main/ipc/git.ts +++ b/src/main/ipc/git.ts @@ -237,9 +237,10 @@ 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). */ @@ -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/renderer/src/workspace/hook/actions/history.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx index 61ed5db2a..891762080 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,54 @@ 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). +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 () => { + let runtimes: Record = { + session: { ...emptyRuntime(), hasOlderHistory: true, historyOldestMarker: 'anchor' }, + } + const observed: unknown[][] = [] + 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) => { observed.push(entries.map(e => e.entry)); return projection }, + refresh, + }), + } 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') }) + + expect(outcome).not.toBe('failed') + expect(observed).toEqual([[{ entries: [older], historyMarker: 'older-1' }]]) + 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(runtimes.session?.workActivity).toBeNull() + expect(runtimes.session?.workContext).toBeNull() + }) +}) From 9596a99c981b139c7928cc0ca584179ed653f2c8 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:28:21 -0700 Subject: [PATCH 6/8] fix(history): replay a handed-over chunk on a cached catalog; older pages never override a known context (#1450 verification a/b) a: with a fresh catalog already cached, refresh() answers 'cached' and never notifies, so a timed-out history chunk never repainted. replayCachedCatalog replays against a real cached catalog; the hand-off calls it on 'cached'. Pinned with the recorded codex-0151 window (was main cwd, now worktree-2). b: the older-history loader handed pages over even when the pane knew a newer context; the reconciler treats observed records as newest, so a recovered catalog moved the pane back. Hand over only while the context is unknown, the same recency rule as the answered-git backfill. Pinned. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 24 +++++ .../hook/actions/history.renderer.test.tsx | 91 +++++++++++-------- .../src/workspace/hook/actions/history.ts | 8 +- .../historyWorktreeTimeout.renderer.test.ts | 25 +++++ .../actions/initialHistory.renderer.test.tsx | 3 +- .../workspace/hook/actions/initialHistory.ts | 8 +- src/renderer/src/workspace/hook/refs.ts | 4 +- .../work-context/LiveWorktreeReconciler.ts | 19 ++++ 8 files changed, 141 insertions(+), 41 deletions(-) diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md index 45ad59dd3..6f129de68 100644 --- a/docs/plans/2026-09-27-worktree-timeout-consumers.md +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -131,3 +131,27 @@ says it or stays unknown, and none caches or records the wrong family. - 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. 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 891762080..7e9877b7e 100644 --- a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx @@ -211,49 +211,68 @@ describe('what an older-history request reports', () => { // 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 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) => { observed.push(entries.map(e => e.entry)); 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, 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 () => { - let runtimes: Record = { - session: { ...emptyRuntime(), hasOlderHistory: true, historyOldestMarker: 'anchor' }, - } - const observed: unknown[][] = [] - 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) => { observed.push(entries.map(e => e.entry)); return projection }, - refresh, - }), - } 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') }) + const { outcome, observed, refresh, older, runtime } = await loadOlderPageWhileGitTimesOut() expect(outcome).not.toBe('failed') expect(observed).toEqual([[{ entries: [older], historyMarker: 'older-1' }]]) 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(runtimes.session?.workActivity).toBeNull() - expect(runtimes.session?.workContext).toBeNull() + 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 be70e74ca..8cd15e359 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -114,8 +114,14 @@ export function useHistoryActions( // #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) handHistoryToReconciler(refs, sessionId, meta.cwd, chunk.entries) + if (worktrees === null && !runtime.workContext) handHistoryToReconciler(refs, sessionId, meta.cwd, chunk.entries) let workActivity = runtime.workActivity let workContext = runtime.workContext let oldestMarker: string | null = runtime.historyOldestMarker diff --git a/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts index f876727da..0ea173dfb 100644 --- a/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts @@ -53,6 +53,31 @@ describe('history read while git timed out (#1430 review a/b)', () => { 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('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 86cbe808d..ff5ba1d6c 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/initialHistory.renderer.test.tsx @@ -111,6 +111,7 @@ describe('the initial-history loader when git times out (#1430)', () => { 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 }), @@ -129,7 +130,7 @@ describe('the initial-history loader when git times out (#1430)', () => { const { history } = emptySession() const pane = rehydratedPane() const observe = vi.fn() - pane.refs.worktreeReconcilerRef.current = { observe, refresh: 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 }) => { diff --git a/src/renderer/src/workspace/hook/actions/initialHistory.ts b/src/renderer/src/workspace/hook/actions/initialHistory.ts index ba94e1c91..beef27924 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.ts +++ b/src/renderer/src/workspace/hook/actions/initialHistory.ts @@ -616,5 +616,11 @@ export function handHistoryToReconciler( const reconciler = refs.worktreeReconcilerRef.current if (!reconciler || entries.length === 0) return reconciler.observe(sessionId, cwd, entries.map(entry => ({ entry })), refs.latestRuntimesRef.current[sessionId] ?? emptyRuntime()) - void reconciler.refresh(cwd) + // '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/refs.ts b/src/renderer/src/workspace/hook/refs.ts index d836f6a31..aa6188357 100644 --- a/src/renderer/src/workspace/hook/refs.ts +++ b/src/renderer/src/workspace/hook/refs.ts @@ -49,7 +49,7 @@ export type WorkspaceRefs = { * 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> + worktreeReconcilerRef: MutableRefObject | null> undoStackRef: MutableRefObject bootstrapTimersRef: MutableRefObject>> persistedFeedDebugIdRef: MutableRefObject> @@ -112,7 +112,7 @@ export function useWorkspaceRefs( const seenUuidsRef = useRef>>({}) const historyWindowsRef = useRef>({}) const historyAwaitingTurnStartRef = useRef>(new Set()) - const worktreeReconcilerRef = useRef | null>(null) + const worktreeReconcilerRef = useRef | null>(null) const undoStackRef = useRef(new UndoCloseStack()) const bootstrapTimersRef = useRef>>(new Map()) const pendingAdoptionWindowIdsRef = useRef([]) diff --git a/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts b/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts index bf5a72669..f081b2267 100644 --- a/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts +++ b/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts @@ -198,6 +198,25 @@ 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) + } + forgetSession(sessionId: SessionId): void { this.evidenceBySession.delete(sessionId) } From f5fc3585052307207b19424df1c1e4752659c310 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 11:55:47 -0700 Subject: [PATCH 7/8] test(workspace): the Codex continuity harness carries worktreeReconcilerRef (#1450 CI) useIpcSubscriptions publishes the live reconciler into WorkspaceRefs (round 1); this harness builds refs by hand and lacked the slot, so the publish threw. Also incidental proof the production publish runs. Co-Authored-By: Claude Opus 5.5 --- .../hook/persistence/codexLiveContinuity.renderer.test.tsx | 1 + 1 file changed, 1 insertion(+) 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 Date: Sun, 27 Sep 2026 12:19:39 -0700 Subject: [PATCH 8/8] fix(history): an older page enters the reconciler as the OLDEST evidence (#1450 B6 verify) A scroll-up during a git timeout handed the older page over as if it were the newest, so a recovered catalog moved the pane to the older worktree. observe() takes a position; older pages go to the old end of the window (overflow dropped, never folded over newer baseline evidence). Pinned with the recorded codex-0151 window + three-worktree catalog. Also fixes the stale resolveRepoRoot header (Unknown, not the cwd). Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-worktree-timeout-consumers.md | 23 +++++++++ src/main/agentActivity/resolveRepoRoot.ts | 8 ++-- .../hook/actions/history.renderer.test.tsx | 9 ++-- .../src/workspace/hook/actions/history.ts | 2 +- .../historyWorktreeTimeout.renderer.test.ts | 38 +++++++++++++++ .../workspace/hook/actions/initialHistory.ts | 6 ++- .../work-context/LiveWorktreeReconciler.ts | 48 +++++++++++++++++++ 7 files changed, 126 insertions(+), 8 deletions(-) diff --git a/docs/plans/2026-09-27-worktree-timeout-consumers.md b/docs/plans/2026-09-27-worktree-timeout-consumers.md index 6f129de68..d2d8be004 100644 --- a/docs/plans/2026-09-27-worktree-timeout-consumers.md +++ b/docs/plans/2026-09-27-worktree-timeout-consumers.md @@ -155,3 +155,26 @@ Both findings are gaps in the round-1 hand-off. - 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/resolveRepoRoot.ts b/src/main/agentActivity/resolveRepoRoot.ts index 82bbfff4e..b6aed618b 100644 --- a/src/main/agentActivity/resolveRepoRoot.ts +++ b/src/main/agentActivity/resolveRepoRoot.ts @@ -8,9 +8,11 @@ * 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's existing catch then uses the cwd for that one interval and - * says so, and the next interval asks again. Never a loop — the recorder - * awaits this on every interval open. + * 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. 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 7e9877b7e..37895d710 100644 --- a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx @@ -216,13 +216,14 @@ async function loadOlderPageWhileGitTimesOut(initial: Partial = 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) => { observed.push(entries.map(e => e.entry)); return projection }, + 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(), }), @@ -247,15 +248,17 @@ async function loadOlderPageWhileGitTimesOut(initial: Partial = const { result } = renderHook(() => useHistoryActions(setRuntimes, refs, updateRuntime, ipcSessionFeed)) let outcome: unknown await act(async () => { outcome = await result.current.loadOlderHistory('session') }) - return { outcome, observed, refresh, older, runtime: () => runtimes.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, refresh, older, runtime } = await loadOlderPageWhileGitTimesOut() + 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. diff --git a/src/renderer/src/workspace/hook/actions/history.ts b/src/renderer/src/workspace/hook/actions/history.ts index 8cd15e359..d341484f9 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -121,7 +121,7 @@ export function useHistoryActions( // 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) + 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 diff --git a/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts index 0ea173dfb..d60f6ddd0 100644 --- a/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/historyWorktreeTimeout.renderer.test.ts @@ -78,6 +78,44 @@ describe('history read while git timed out (#1430 review a/b)', () => { 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.ts b/src/renderer/src/workspace/hook/actions/initialHistory.ts index beef27924..32a225291 100644 --- a/src/renderer/src/workspace/hook/actions/initialHistory.ts +++ b/src/renderer/src/workspace/hook/actions/initialHistory.ts @@ -612,10 +612,14 @@ export function handHistoryToReconciler( 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()) + 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 diff --git a/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts b/src/renderer/src/workspace/work-context/LiveWorktreeReconciler.ts index f081b2267..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 @@ -217,6 +230,41 @@ export class LiveWorktreeReconciler { 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) }