From ea9bb52fe73b8be4dc0ce3d095c3fcc285e5c0e4 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 16:28:44 +0300 Subject: [PATCH] fix(sync): stop directory cache thrashing when a project is expanded Expanding a project with more worktrees and sessions than MAX_DIR_STORES put the sidebar into an endless request loop (#1472). Every sidebar row calls ensureChild during render, but the pin that protects the directory is only taken in an effect after commit. ensureChild marked the directory and ran eviction synchronously, so directories that were actively rendering looked unpinned and were disposed. The next render recreated them with a loading status, which issued another bootstrap request, and the cycle repeated for as long as the project stayed expanded. Raising the limit only moves the cliff, so the limit is now a soft target instead: a directory touched within a grace window is never an overflow victim. A burst of live directories overflows the cache briefly rather than thrashing, while idle-time eviction still bounds it. Eviction is also coalesced into one deferred pass per tick, so a render that mounts many rows no longer sorts and scans every directory once per row, and a whole commit's pin effects settle before anything is considered for disposal. Releasing the final consumer stays synchronous, since that is an explicit lifecycle edge. The idle profiler gains --expand-projects to reach this state. Not yet verified end to end: reproducing the loop needs many worktrees under one project, which this development environment does not have. --- packages/ui/src/sync/child-store.ts | 26 ++++- packages/ui/src/sync/eviction-thrash.test.ts | 110 +++++++++++++++++++ packages/ui/src/sync/eviction.ts | 9 +- packages/ui/src/sync/types.ts | 13 +++ scripts/profile-idle.mjs | 17 ++- 5 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 packages/ui/src/sync/eviction-thrash.test.ts diff --git a/packages/ui/src/sync/child-store.ts b/packages/ui/src/sync/child-store.ts index cfd23584..0b0142de 100644 --- a/packages/ui/src/sync/child-store.ts +++ b/packages/ui/src/sync/child-store.ts @@ -1,6 +1,6 @@ import { create, type StoreApi } from "zustand" import type { DirState, State } from "./types" -import { INITIAL_STATE, MAX_DIR_STORES, DIR_IDLE_TTL_MS } from "./types" +import { INITIAL_STATE, MAX_DIR_STORES, DIR_IDLE_TTL_MS, EVICTION_GRACE_MS } from "./types" import { pickDirectoriesToEvict, canDisposeDirectory, hasPendingBlockingRequests } from "./eviction" import { readDirCache, persistVcs, persistProjectMeta, persistIcon, persistSessions } from "./persist-cache" import { normalizePath } from "@/lib/pathNormalization" @@ -250,6 +250,7 @@ export class ChildStoreManager { readonly children = new Map>() private readonly lifecycle = new Map() private readonly pins = new Map() + private evictionScheduled = false private readonly disposers = new Map void>() private readonly registrySubscribers = new Set<() => void>() private readonly bootstrapSubscribers = new Set<() => void>() @@ -308,7 +309,25 @@ export class ChildStoreManager { mark(directory: string) { if (!directory) return this.lifecycle.set(directory, { lastAccessAt: Date.now() }) - this.runEviction(directory) + this.scheduleEviction() + } + + /** + * Coalesce eviction into one pass per tick. + * + * `ensureChild` runs during render, once per sidebar row, and used to sort + * and scan every directory synchronously on each call. Deferring the pass + * also lets a whole render commit — and with it every pin effect — settle + * before anything is considered for disposal. + */ + private scheduleEviction() { + if (this.evictionScheduled || this.disposed) return + this.evictionScheduled = true + queueMicrotask(() => { + this.evictionScheduled = false + if (this.disposed) return + this.runEviction() + }) } pin(directory: string) { @@ -327,6 +346,8 @@ export class ChildStoreManager { return } this.pins.delete(normalizedDirectory) + // Releasing the final consumer is an explicit lifecycle edge, not a render- + // path access, so this pass stays synchronous. this.runEviction() } @@ -621,6 +642,7 @@ export class ChildStoreManager { pins: new Set(stores.filter((d) => this.pinned(d))), max: MAX_DIR_STORES, ttl: DIR_IDLE_TTL_MS, + graceMs: EVICTION_GRACE_MS, now: Date.now(), hasPendingBlockingRequests: (dir) => this.hasPendingBlockingRequestsForDirectory(dir), }).filter((d) => d !== skip) diff --git a/packages/ui/src/sync/eviction-thrash.test.ts b/packages/ui/src/sync/eviction-thrash.test.ts new file mode 100644 index 00000000..da316215 --- /dev/null +++ b/packages/ui/src/sync/eviction-thrash.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, test } from "bun:test" + +import { pickDirectoriesToEvict } from "./eviction" +import { DIR_IDLE_TTL_MS, EVICTION_GRACE_MS, MAX_DIR_STORES } from "./types" + +/** + * Regression coverage for the sidebar cache-thrash loop (issue #1472). + * + * Expanding a project with many worktrees mounts a sidebar row per directory. + * Each row calls `ensureChild` during render, while the pin that protects it is + * only taken in an effect after commit. With more live directories than the + * store limit, overflow eviction therefore disposed directories that were + * actively being rendered; the next render recreated them with a `loading` + * status, which issued another bootstrap request, and the cycle repeated + * indefinitely. + * + * The fix treats the limit as a soft target: a directory touched within the + * grace window is never an overflow victim, so a burst of live directories + * overflows the cache briefly instead of thrashing. Idle directories stay + * evictable, which is what keeps the cache bounded. + */ + +const buildState = (directories: string[], lastAccessAt: number) => + new Map(directories.map((directory) => [directory, { lastAccessAt }])) + +const directories = (count: number, prefix = "/repo/worktree-") => + Array.from({ length: count }, (_, index) => `${prefix}${index}`) + +describe("directory eviction under sidebar expansion", () => { + test("does not evict directories that are being accessed right now", () => { + const now = 1_000_000 + const stores = directories(MAX_DIR_STORES + 25) + + const evicted = pickDirectoriesToEvict({ + stores, + state: buildState(stores, now), + pins: new Set(), + max: MAX_DIR_STORES, + ttl: DIR_IDLE_TTL_MS, + graceMs: EVICTION_GRACE_MS, + now, + }) + + expect(evicted).toEqual([]) + }) + + test("still evicts overflow once directories fall outside the grace window", () => { + const now = 1_000_000 + const live = directories(MAX_DIR_STORES, "/repo/live-") + const stale = directories(5, "/repo/stale-") + const state = new Map([ + ...buildState(live, now), + ...buildState(stale, now - EVICTION_GRACE_MS - 1), + ]) + + const evicted = pickDirectoriesToEvict({ + stores: [...live, ...stale], + state, + pins: new Set(), + max: MAX_DIR_STORES, + ttl: DIR_IDLE_TTL_MS, + graceMs: EVICTION_GRACE_MS, + now, + }) + + expect([...evicted].sort()).toEqual([...stale].sort()) + }) + + test("still evicts directories idle past the TTL even inside the limit", () => { + const now = 1_000_000 + const active = directories(3, "/repo/active-") + const abandoned = directories(2, "/repo/abandoned-") + const state = new Map([ + ...buildState(active, now), + ...buildState(abandoned, now - DIR_IDLE_TTL_MS - 1), + ]) + + const evicted = pickDirectoriesToEvict({ + stores: [...active, ...abandoned], + state, + pins: new Set(), + max: MAX_DIR_STORES, + ttl: DIR_IDLE_TTL_MS, + graceMs: EVICTION_GRACE_MS, + now, + }) + + expect([...evicted].sort()).toEqual([...abandoned].sort()) + }) + + test("never evicts pinned or blocked directories regardless of overflow", () => { + const now = 1_000_000 + const stores = directories(MAX_DIR_STORES + 10) + const stale = now - EVICTION_GRACE_MS - 1 + + const evicted = pickDirectoriesToEvict({ + stores, + state: buildState(stores, stale), + pins: new Set([stores[0]]), + max: MAX_DIR_STORES, + ttl: DIR_IDLE_TTL_MS, + graceMs: EVICTION_GRACE_MS, + now, + hasPendingBlockingRequests: (directory) => directory === stores[1], + }) + + expect(evicted).not.toContain(stores[0]) + expect(evicted).not.toContain(stores[1]) + }) +}) diff --git a/packages/ui/src/sync/eviction.ts b/packages/ui/src/sync/eviction.ts index 723149a4..cb9a65a5 100644 --- a/packages/ui/src/sync/eviction.ts +++ b/packages/ui/src/sync/eviction.ts @@ -26,6 +26,7 @@ export function hasPendingBlockingRequests(state: State | undefined): boolean { export function pickDirectoriesToEvict(input: EvictPlan) { const overflow = Math.max(0, input.stores.length - input.max) let pendingOverflow = overflow + const graceMs = input.graceMs ?? 0 const sorted = input.stores .filter((dir) => !input.pins.has(dir)) .filter((dir) => !input.hasPendingBlockingRequests?.(dir)) @@ -34,8 +35,14 @@ export function pickDirectoriesToEvict(input: EvictPlan) { const output: string[] = [] for (const dir of sorted) { const last = input.state.get(dir)?.lastAccessAt ?? 0 - const idle = input.now - last >= input.ttl + const age = input.now - last + const idle = age >= input.ttl if (!idle && pendingOverflow <= 0) continue + // A directory touched moments ago is almost certainly still mounted and + // merely waiting for its pin effect to run. Evicting it starts the + // recreate/bootstrap loop this grace window exists to prevent; going over + // the limit for a while is the cheaper failure. + if (!idle && age < graceMs) continue output.push(dir) if (pendingOverflow > 0) pendingOverflow -= 1 } diff --git a/packages/ui/src/sync/types.ts b/packages/ui/src/sync/types.ts index edb88e1a..22f1301a 100644 --- a/packages/ui/src/sync/types.ts +++ b/packages/ui/src/sync/types.ts @@ -96,6 +96,7 @@ export type EvictPlan = { pins: Set max: number ttl: number + graceMs?: number now: number hasPendingBlockingRequests?: (directory: string) => boolean } @@ -110,6 +111,18 @@ export type DisposeCheck = { } export const MAX_DIR_STORES = 30 +/** + * Directories touched within this window are never overflow-eviction victims. + * + * Sidebar rows call `ensureChild` during render but only take their pin in an + * effect after commit. Without a grace window, expanding a project with more + * worktrees than `MAX_DIR_STORES` evicted directories that were actively + * rendering, which recreated them, which issued another bootstrap request, in + * an endless loop (issue #1472). The limit is therefore a soft target: a burst + * of live directories overflows briefly rather than thrashing, and the cache is + * bounded by idle-time eviction instead. + */ +export const EVICTION_GRACE_MS = 30 * 1000 export const DIR_IDLE_TTL_MS = 20 * 60 * 1000 export const SESSION_CACHE_LIMIT = 40 diff --git a/scripts/profile-idle.mjs b/scripts/profile-idle.mjs index dbe47c14..98dc67a3 100644 --- a/scripts/profile-idle.mjs +++ b/scripts/profile-idle.mjs @@ -42,6 +42,9 @@ Options: --then-tab After settling, navigate to this tab without a reload, then record. Use it to measure what a surface keeps doing after the user leaves it. + --expand-projects Expand every project in the sidebar before + recording, which mounts a row per worktree and + session directory --panel Open the context panel on this surface before recording (chat, preview, terminal, git, pr, notes, file, diff, plan, context, browser, walkthrough). @@ -70,6 +73,7 @@ const parseArgs = (argv) => { tab: null, thenTab: null, panels: [], + expandProjects: false, duration: 30, settle: 15, output: null, @@ -94,6 +98,7 @@ const parseArgs = (argv) => { else if (value === "--tab") options.tab = argv[++index] else if (value === "--then-tab") options.thenTab = argv[++index] else if (value === "--panel") options.panels.push(argv[++index]) + else if (value === "--expand-projects") options.expandProjects = true else if (value === "--label") options.label = argv[++index] else if (value === "--duration") options.duration = Number(argv[++index]) else if (value === "--settle") options.settle = Number(argv[++index]) @@ -351,8 +356,16 @@ const main = async () => { await client.send("Page.navigate", { url: options.url }) await loaded - if (options.panels.length > 0) { - await seedContextPanel(client, options.panels, options.session) + if (options.expandProjects) { + // The sidebar persists the ids of collapsed projects, so an empty list + // expands everything. This is the state in issue #1472: one mounted + // directory-bound row per worktree and session. + await evaluateValue(client, `localStorage.setItem("oc.sessions.projectCollapse", "[]")`) + console.log("Expanded every project in the sidebar.") + } + + if (options.panels.length > 0 || options.expandProjects) { + if (options.panels.length > 0) await seedContextPanel(client, options.panels, options.session) const reloaded = client.once("Page.loadEventFired", 60_000) await client.send("Page.reload", { ignoreCache: false }) await reloaded