diff --git a/AGENTS.md b/AGENTS.md index 37d83ae9c..775d0e1d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,10 @@ ## Coding Principles +- Follow `dev-docs/CACHE_REFRESH_CONVENTIONS.md` for display snapshots, coalesced trailing refreshes, stale-response fencing and authoritative mutation reads. Check existing feature semantics before adding another cache or refresh policy. + - Worktree discovery/create/remove use `workspaces/native-worktrees.ts` and the native OpenCode worktree API. CodeNomad supplies the `.codenomad/worktrees` default, named-branch policy and verified family transactions. Git common-directory identity scopes the native inventory to the opened local repository; opaque worktree identifiers are separate from mutable branch labels. Validate through `scripts/test-opencode-location-native.mjs` with an isolated CLI and `tests/browser/worktrees.test.ts` for selector gestures. +- Worktree inventory snapshots live in `workspaces/worktree-inventory.ts`: display reads serve cached data and lazily revalidate, directory authorization uses validated reads, and family transactions force fresh reads. Invalidation retains display data and fences pending scans; `workspace.worktreesChanged` refreshes existing UI consumers after a changed snapshot is published. Keep selector opening independent of refresh completion and suppress duplicate selection events during inventory reconciliation. - Session pruning is a narrow V2 plugin/RPC exception under `packages/server/src/opencode/session-pruning/`; see `dev-docs/SESSION_PRUNING_RPC.md`. Bundle it with the shared server for both desktop hosts and provision through normal native plugin discovery. RPC registrations follow backend presence; clean shutdown removes that backend's lease and crashes expire. Loading never deletes content. Deletion occurs only on an explicit pruning request, without an extra enable-write switch or beta-number gate. Keep generic RPC proxy access closed. Writes validate actual storage, a fresh daemon-storage identity challenge and the native durable execution claim inside a synchronous SQLite transaction. Run isolated native concurrency/payload and client-cache regressions; tests must never target the shared daemon or a user's database. - Favor KISS by keeping modules narrowly scoped and limiting public APIs to what callers actually need. diff --git a/dev-docs/CACHE_REFRESH_CONVENTIONS.md b/dev-docs/CACHE_REFRESH_CONVENTIONS.md new file mode 100644 index 000000000..e545f0521 --- /dev/null +++ b/dev-docs/CACHE_REFRESH_CONVENTIONS.md @@ -0,0 +1,41 @@ +# Cache and refresh conventions + +Use consistent refresh semantics across features. Keep domain-specific implementations +where their authority, lifetime or runtime differs. + +## Common rules + +- Keep the last successful display snapshot while a passive refresh runs. +- Share concurrent reads for the same identity. During a read, coalesce refresh + requests into one pending follow-up rather than queueing one request per event. +- Invalidate on relevant events or mutations; a TTL expiring does not itself start + work. Avoid background polling merely to keep a hidden view warm. +- A response must still belong to the current identity/generation before publication. + Serialize follow-up reads so an older response cannot overwrite their result. +- Preserve the last good snapshot on refresh failure and permit recovery. Surface + failures for explicit operations rather than reporting stale data as success. +- A successful mutation must be visible to its next dependent read. Display-cache + policies must not weaken ownership checks or transactional revalidation. +- Updating a displayed collection is not a user selection. Preserve its open state, + stable-key keyboard target and inline actions through background refreshes. + +## Current implementations and limits + +| Area | Current behaviour | Implementation | +| --- | --- | --- | +| Providers/models | Retained signals; shared in-flight catalogue load; dirty-bit trailing refresh; instance, location and request-generation checks. No completed-result TTL inside `fetchProviders` itself. | `packages/ui/src/stores/session-api.ts` | +| Git changes | Filesystem events debounce for 100 ms; one passive refresh plus a pending follow-up; hidden tab marked stale; request versions protect status/diff. Server shares concurrent status requests, not completed results. | `useGitChanges.ts`, `filesystem-events.ts`, server `workspaces/git-status.ts` | +| Worktree display | Last successful server snapshot; demand-driven refresh after 10 s or invalidation; one scan per workspace; obsolete scans discarded and followed by validation; UI requests coalesced. | server `workspaces/worktree-inventory.ts`, UI `stores/worktrees.ts` | +| Worktree authority | Validated reads await stale-inventory revalidation; family transactions force scans. Ownership misses can bypass a warm inventory once per directory-cache lifetime. Create/remove requires a validated next display read. | server `workspaces/worktree-directory.ts`, `manager.ts` | +| Render cache | Explicit versioned values scoped to instance/session; no network scheduler or TTL policy. | UI `lib/global-cache.ts` | +| Virtualized lists | Session list, transcript and timeline use `virtua/solid`; virtualization limits rendered rows, not network refreshes. | UI `session-list.tsx`, `virtual-follow-list.tsx`, `message-timeline.tsx` | + +Git updates are regulated, but not incremental: every new server status calculation +runs five Git commands plus untracked-file processing, and the UI also requests +native status. Continuous activity can sustain repeated full calculations and +selected-diff reads. This is not a completed-result cache. + +The worktree cache is in memory. It does not reduce the first native inventory scan, +the cost of mandatory authoritative scans, or all latency in the serial event relay. +An isolated native fixture covers warm-cache create/remove visibility; browser +fixtures cover menu updates, focus, old responses and refresh bursts. diff --git a/packages/server/src/api-types.ts b/packages/server/src/api-types.ts index a3d028749..60fff4963 100644 --- a/packages/server/src/api-types.ts +++ b/packages/server/src/api-types.ts @@ -468,6 +468,7 @@ export type WorkspaceEventType = | "workspace.error" | "workspace.stopped" | "workspace.log" + | "workspace.worktreesChanged" | "sidecar.updated" | "sidecar.removed" | "storage.configChanged" @@ -484,6 +485,7 @@ export type WorkspaceEventPayload = | { type: "workspace.error"; workspace: WorkspaceDescriptor } | { type: "workspace.stopped"; workspaceId: string; reason?: "deleted" | "stopped" } | { type: "workspace.log"; entry: WorkspaceLogEntry } + | { type: "workspace.worktreesChanged"; workspaceId: string } | { type: "sidecar.updated"; sidecar: SideCar } | { type: "sidecar.removed"; sidecarId: string } | { type: "storage.configChanged"; owner: SettingsOwner; value: SettingsBucket } diff --git a/packages/server/src/events/bus.ts b/packages/server/src/events/bus.ts index 637aad1d2..8950c48ab 100644 --- a/packages/server/src/events/bus.ts +++ b/packages/server/src/events/bus.ts @@ -35,6 +35,7 @@ export class EventBus extends EventEmitter { this.on("workspace.error", handler) this.on("workspace.stopped", handler) this.on("workspace.log", handler) + this.on("workspace.worktreesChanged", handler) this.on("sidecar.updated", handler) this.on("sidecar.removed", handler) this.on("storage.configChanged", handler) @@ -51,6 +52,7 @@ export class EventBus extends EventEmitter { this.off("workspace.error", handler) this.off("workspace.stopped", handler) this.off("workspace.log", handler) + this.off("workspace.worktreesChanged", handler) this.off("sidecar.updated", handler) this.off("sidecar.removed", handler) this.off("storage.configChanged", handler) diff --git a/packages/server/src/server/routes/workspaces.ts b/packages/server/src/server/routes/workspaces.ts index 05e91a52b..2c524089f 100644 --- a/packages/server/src/server/routes/workspaces.ts +++ b/packages/server/src/server/routes/workspaces.ts @@ -361,7 +361,7 @@ async function resolveGitWorktreeDirectory( workspaceId: workspace.id, workspacePath: workspace.path, worktreeSlug, - loadWorktrees: async () => (await workspaceManager.getWorktrees(workspace.id)).worktrees, + loadWorktrees: async (refresh) => (await workspaceManager.getWorktrees(workspace.id, refresh ? "fresh" : "validated")).worktrees, logger, }) if (!directory) { diff --git a/packages/server/src/server/routes/worktrees.ts b/packages/server/src/server/routes/worktrees.ts index 34be72e23..5052bd4ba 100644 --- a/packages/server/src/server/routes/worktrees.ts +++ b/packages/server/src/server/routes/worktrees.ts @@ -47,7 +47,6 @@ export function registerWorktreeRoutes(app: FastifyInstance, deps: RouteDeps) { try { const response: WorktreeListResponse = await deps.workspaceManager.getWorktrees(workspace.id) - invalidateWorktreeCache(workspace.id) return response } catch (error) { return handleError(error, reply) @@ -85,7 +84,7 @@ export function registerWorktreeRoutes(app: FastifyInstance, deps: RouteDeps) { return { error: "Workspace is not a Git repository" } } - const catalogue = await deps.workspaceManager.getWorktrees(workspace.id) + const catalogue = await deps.workspaceManager.getWorktrees(workspace.id, "fresh") const source = catalogue.worktrees.find(entry => entry.slug === (body.fromSlug ?? "root")) if (!source) throw new ProjectSessionError("Source worktree not found", 404) const identities = await Promise.all(catalogue.worktrees.map(entry => ( @@ -265,7 +264,7 @@ export function registerWorktreeRoutes(app: FastifyInstance, deps: RouteDeps) { } function strictWorktrees(manager: WorkspaceManager, workspaceId: string) { - return manager.getWorktrees(workspaceId).then(result => result.worktrees).catch((error) => { + return manager.getWorktrees(workspaceId, "fresh").then(result => result.worktrees).catch((error) => { throw new ProjectSessionError(error instanceof Error ? error.message : "Unable to read native worktree inventory", 502) }) } diff --git a/packages/server/src/workspaces/git-status.test.ts b/packages/server/src/workspaces/git-status.test.ts index 365a52f0c..df1a2f150 100644 --- a/packages/server/src/workspaces/git-status.test.ts +++ b/packages/server/src/workspaces/git-status.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict" import { mkdtemp, rm } from "node:fs/promises" import fs from "node:fs/promises" import { tmpdir } from "node:os" +import { syncBuiltinESMExports } from "node:module" import path from "node:path" import { describe, it } from "node:test" @@ -20,13 +21,16 @@ describe("worktree git status singleflight", () => { let canonicalized = 0 let ready!: () => void const bothCanonicalized = new Promise((resolve) => { ready = resolve }) - t.mock.method(fs, "realpath", async (value: string) => { + const canonicalization = t.mock.method(fs, "realpath", async (value: string) => { const result = await realpath(value) canonicalized += 1 if (canonicalized === 2) ready() await bothCanonicalized return result }) + // Native ESM named imports do not see default-export monkey patches until + // synchronized. The CI runner uses ESM even when a local tsx run uses CJS. + syncBuiltinESMExports() const run = async () => { calls += 1 await blocked @@ -49,6 +53,8 @@ describe("worktree git status singleflight", () => { assert.equal(calls, 10) } finally { release() + canonicalization.mock.restore() + syncBuiltinESMExports() await rm(directory, { recursive: true, force: true }) } }) diff --git a/packages/server/src/workspaces/instance-events.ts b/packages/server/src/workspaces/instance-events.ts index 8d23316d0..aea4d5963 100644 --- a/packages/server/src/workspaces/instance-events.ts +++ b/packages/server/src/workspaces/instance-events.ts @@ -4,7 +4,6 @@ import { EventBus } from "../events/bus" import { Logger } from "../logger" import { WorkspaceManager } from "./manager" import { InstanceStreamStatus } from "../api-types" -import { invalidateWorktreeCache } from "./worktree-directory" const RECONNECT_DELAY_MS = 1000 const LOCATION_OWNER_CACHE_MS = 2000 @@ -110,7 +109,7 @@ export class InstanceEventBridge { private async publishEvent(event: OpenCodeEvent) { if (event.type === "worktree.updated") { - invalidateWorktreeCache() + this.options.workspaceManager.invalidateWorktrees() this.locationOwners.clear() } const sessionId = this.sessionId(event) diff --git a/packages/server/src/workspaces/manager.ts b/packages/server/src/workspaces/manager.ts index 46f14ac49..24523e3ea 100644 --- a/packages/server/src/workspaces/manager.ts +++ b/packages/server/src/workspaces/manager.ts @@ -32,6 +32,7 @@ import { import { WslOpenCodeService } from "./wsl-opencode-service" import { invalidateWorktreeCache, isPathOwnedByWorktree, resolveOwnedWorktreePath } from "./worktree-directory" import { listNativeWorktrees, createNativeWorktree, removeNativeWorktree } from "./native-worktrees" +import { WorktreeInventory } from "./worktree-inventory" import { resolveRepoRoot } from "./git-worktrees" import { locationRequestOptions, readLocationRef, sameLocation } from "../opencode/compatibility/location" @@ -317,25 +318,33 @@ export class WorkspaceManager { } } - private readonly worktreeInventoryRequests = new Map>() + private readonly worktreeInventory = new WorktreeInventory({ + load: (id) => this.nativeWorktreeContext(id).then(listNativeWorktrees), + changed: (id) => { + invalidateWorktreeCache(id) + this.options.eventBus.publish({ type: "workspace.worktreesChanged", workspaceId: id }) + }, + failed: (id, error) => this.options.logger.warn({ workspaceId: id, err: error }, "Failed to refresh worktree inventory"), + now: () => this.now(), + }) - async getWorktrees(id: string) { - const pending = this.worktreeInventoryRequests.get(id) - if (pending) return pending - const task = this.nativeWorktreeContext(id).then(listNativeWorktrees) - this.worktreeInventoryRequests.set(id, task) - try { return await task } - finally { if (this.worktreeInventoryRequests.get(id) === task) this.worktreeInventoryRequests.delete(id) } + getWorktrees(id: string, mode: "cached" | "validated" | "fresh" = "cached") { + return this.worktreeInventory.read(id, mode) + } + + invalidateWorktrees(mode: "lazy" | "blocking" = "lazy"): void { + this.worktreeInventory.invalidate(undefined, mode) + invalidateWorktreeCache() } async createWorktree(id: string, branch: string, fromSlug?: string) { try { return await createNativeWorktree(await this.nativeWorktreeContext(id), branch, fromSlug) } - finally { invalidateWorktreeCache() } + finally { this.invalidateWorktrees("blocking") } } async removeWorktree(id: string, serviceDirectory: string, force: boolean) { try { return await removeNativeWorktree(await this.nativeWorktreeContext(id), serviceDirectory, force) } - finally { invalidateWorktreeCache() } + finally { this.invalidateWorktrees("blocking") } } private async ownsHostDirectory(record: WorkspaceRecord, directory: string): Promise { @@ -347,7 +356,7 @@ export class WorkspaceManager { workspaceId: record.id, workspacePath: record.path, directory, - loadWorktrees: async () => (await this.getWorktrees(record.id)).worktrees, + loadWorktrees: async (refresh) => (await this.getWorktrees(record.id, refresh ? "fresh" : "validated")).worktrees, logger: this.options.logger, })) !== null } @@ -372,7 +381,7 @@ export class WorkspaceManager { workspaceId: record.id, workspacePath: record.path, directory: hostDirectory, - loadWorktrees: async () => (await this.getWorktrees(record.id)).worktrees, + loadWorktrees: async (refresh) => (await this.getWorktrees(record.id, refresh ? "fresh" : "validated")).worktrees, logger: this.options.logger, }) } @@ -391,7 +400,7 @@ export class WorkspaceManager { workspaceId: record.id, workspacePath: record.path, candidate, - loadWorktrees: async () => (await this.getWorktrees(record.id)).worktrees, + loadWorktrees: async (refresh) => (await this.getWorktrees(record.id, refresh ? "fresh" : "validated")).worktrees, logger: this.options.logger, }) } @@ -1042,6 +1051,8 @@ export class WorkspaceManager { ): void { if (this.workspaces.get(id) !== record) return this.workspaces.delete(id) + this.worktreeInventory.forget(id) + invalidateWorktreeCache(id) clearWorkspaceSearchCache(record.path) if (publishStopped) this.publishStopped(record, reason) } diff --git a/packages/server/src/workspaces/worktree-directory.test.ts b/packages/server/src/workspaces/worktree-directory.test.ts index e14736d3c..a664cf5c7 100644 --- a/packages/server/src/workspaces/worktree-directory.test.ts +++ b/packages/server/src/workspaces/worktree-directory.test.ts @@ -7,6 +7,7 @@ import path from "node:path" import { test } from "node:test" import { invalidateWorktreeCache, isPathWithinWorktree, resolveOwnedWorktreePath } from "./worktree-directory" import { fixtureCatalogue } from "./__tests__/native-worktree-fixture" +import { WorktreeInventory } from "./worktree-inventory" test("concurrent ownership misses share a refresh and cache negative results until invalidation", async (t) => { const temp = mkdtempSync(path.join(tmpdir(), "codenomad-inventory-misses-")) @@ -79,3 +80,92 @@ test("resolves nested and junction paths to their canonical owning worktree", as assert.equal(isPathWithinWorktree("\\\\wsl.localhost\\Ubuntu\\repo\\Foo", "\\\\wsl.localhost\\Ubuntu\\repo\\Foo\\nested"), true) assert.equal(isPathWithinWorktree("\\\\WSL.LOCALHOST\\ubuntu\\repo\\Foo", "\\\\wsl.localhost\\Ubuntu\\repo\\Foo\\nested"), true) }) + +test("distinct foreign event directories share one ownership refresh until invalidation", async (t) => { + t.mock.timers.enable({ apis: ["Date"], now: Date.now() }) + const temp = mkdtempSync(path.join(tmpdir(), "codenomad-foreign-events-")) + t.after(() => { invalidateWorktreeCache(temp); rmSync(temp, { recursive: true, force: true }) }) + const root = path.join(temp, "repo") + mkdirSync(root) + let loads = 0 + const inventory = [{ slug: "root", directory: root, kind: "root" as const }] + const params = { + workspaceId: temp, workspacePath: root, + loadWorktrees: async () => { loads++; return inventory }, + } + await resolveOwnedWorktreePath({ ...params, directory: root }) + // The global native stream contains unrelated projects and temporary checkouts. + // Its serial router must not reload every repository's inventory for each one. + const foreign = Array.from({ length: 20 }, (_, i) => path.join(temp, `foreign-${i}`)) + for (const directory of foreign) { + mkdirSync(directory) + assert.equal(await resolveOwnedWorktreePath({ ...params, directory }), null) + } + assert.equal(loads, 2, "at most one miss refresh for the current inventory") + assert.equal(await resolveOwnedWorktreePath({ ...params, directory: foreign[0] }), null) + assert.equal(loads, 2) + // A native worktree event invalidates the snapshot, including negative matches. + inventory.push({ slug: "linked", directory: foreign[0], kind: "root" }) + invalidateWorktreeCache(temp) + assert.equal((await resolveOwnedWorktreePath({ ...params, directory: foreign[0] }))?.slug, "linked") + assert.equal(loads, 3) + // An external change without an event is still discovered after the cache TTL. + inventory.push({ slug: "external", directory: foreign[1], kind: "root" }) + t.mock.timers.tick(10_001) + assert.equal((await resolveOwnedWorktreePath({ ...params, directory: foreign[1] }))?.slug, "external") + assert.equal(loads, 4) +}) + +test("an ownership miss bypasses the warm lower inventory cache once", async t => { + const temp = mkdtempSync(path.join(tmpdir(), "codenomad-layered-cache-")) + t.after(() => { invalidateWorktreeCache(temp); rmSync(temp, { recursive: true, force: true }) }) + const root = path.join(temp, "repo") + const nested = path.join(root, "linked") + mkdirSync(nested, { recursive: true }) + const worktrees = [{ slug: "root", directory: root, kind: "root" as const }] + let scans = 0 + const inventory = new WorktreeInventory({ + load: async () => { scans++; return { isGitRepo: true, worktrees: [...worktrees] } }, + changed: () => invalidateWorktreeCache(temp), + failed: () => {}, + }) + const params = { + workspaceId: temp, workspacePath: root, + loadWorktrees: async (refresh?: boolean) => (await inventory.read(temp, refresh ? "fresh" : "validated")).worktrees, + } + assert.equal((await resolveOwnedWorktreePath({ ...params, directory: root }))?.slug, "root") + worktrees.push({ slug: "linked", directory: nested, kind: "root" }) + // No native update event yet. Resolving the nested checkout must not collapse + // it to the parent repository's mutation/deletion identity. + assert.equal((await resolveOwnedWorktreePath({ ...params, directory: nested }))?.slug, "linked") + assert.equal(scans, 2) + for (let i = 0; i < 20; i++) { + assert.equal(await resolveOwnedWorktreePath({ ...params, directory: path.join(temp, `foreign-${i}`) }), null) + } + assert.equal(scans, 2) +}) + +test("an invalidated directory load cannot return obsolete ownership to its awaiting caller", async t => { + const temp = mkdtempSync(path.join(tmpdir(), "codenomad-directory-generation-")) + t.after(() => { invalidateWorktreeCache(temp); rmSync(temp, { recursive: true, force: true }) }) + const root = path.join(temp, "repo") + mkdirSync(root) + let release!: () => void + let started!: () => void + const ready = new Promise(resolve => { started = resolve }) + const blocked = new Promise(resolve => { release = resolve }) + let calls = 0 + const read = resolveOwnedWorktreePath({ + workspaceId: temp, workspacePath: root, directory: root, + loadWorktrees: async () => { + if (++calls !== 1) return [] + started() + await blocked + return [{ slug: "root", directory: root, kind: "root" }] + }, + }) + await ready + invalidateWorktreeCache(temp) + release() + assert.equal(await read, null) +}) diff --git a/packages/server/src/workspaces/worktree-directory.ts b/packages/server/src/workspaces/worktree-directory.ts index 6f5ca3dd0..d26ebe727 100644 --- a/packages/server/src/workspaces/worktree-directory.ts +++ b/packages/server/src/workspaces/worktree-directory.ts @@ -3,10 +3,11 @@ import path from "node:path" import type { LogLike } from "./git-worktrees" import type { WorktreeDescriptor } from "../api-types" -type WorktreeSource = { loadWorktrees: () => Promise } +type WorktreeSource = { loadWorktrees: (refresh?: boolean) => Promise } type WorktreeCacheEntry = { expiresAt: number + refreshedOnMiss: boolean worktrees: Array<{ slug: string; directory: string; normalizedDirectory: string; worktreeDirectory: string }> resolvedDirectories: Map } @@ -25,7 +26,10 @@ async function normalizeDirectoryPath(directory: string): Promise { } } -async function getCachedWorktrees(params: WorktreeSource & { workspaceId: string; workspacePath: string; logger?: LogLike }) { +async function getCachedWorktrees( + params: WorktreeSource & { workspaceId: string; workspacePath: string; logger?: LogLike }, + refresh = false, +): Promise { const cached = worktreeCache.get(params.workspaceId) const now = Date.now() if (cached && cached.expiresAt > now) { @@ -37,9 +41,10 @@ async function getCachedWorktrees(params: WorktreeSource & { workspaceId: string let load!: Promise load = (async () => { - const worktrees = await params.loadWorktrees() + const worktrees = await params.loadWorktrees(refresh) const entry: WorktreeCacheEntry = { expiresAt: Date.now() + WORKTREE_CACHE_TTL_MS, + refreshedOnMiss: false, worktrees: await Promise.all( worktrees.map(async (wt) => ({ slug: wt.slug, @@ -50,7 +55,10 @@ async function getCachedWorktrees(params: WorktreeSource & { workspaceId: string ), resolvedDirectories: new Map(), } - if (worktreeLoads.get(params.workspaceId) === load) worktreeCache.set(params.workspaceId, entry) + // A native snapshot update or mutation can invalidate during load/realpath. + // Never return the obsolete ownership to callers already awaiting this load. + if (worktreeLoads.get(params.workspaceId) !== load) return getCachedWorktrees(params) + worktreeCache.set(params.workspaceId, entry) return entry })() worktreeLoads.set(params.workspaceId, load) @@ -94,7 +102,7 @@ export async function resolveWorktreeDirectory(params: WorktreeSource & { workspacePath: params.workspacePath, logger: params.logger, loadWorktrees: params.loadWorktrees, - }) + }, true) return refreshed.worktrees.find((wt) => wt.slug === params.worktreeSlug)?.directory ?? null } @@ -124,7 +132,7 @@ export async function resolveWorktreeSlugForDirectory(params: WorktreeSource & { workspacePath: params.workspacePath, logger: params.logger, loadWorktrees: params.loadWorktrees, - }) + }, true) return refreshed.worktrees.find((wt) => wt.normalizedDirectory === target)?.slug ?? null } @@ -182,11 +190,13 @@ export async function resolveOwnedWorktreePath(params: WorktreeSource & { let entry = await getCachedWorktrees(params) if (entry.resolvedDirectories.has(target)) return entry.resolvedDirectories.get(target)! let match = find(entry.worktrees) - if (!match || (match.slug === "root" && match.normalizedDirectory !== target)) { - // Several foreign-location events can miss the same snapshot concurrently. - // Refresh that snapshot once; never discard another caller's pending load. + if (!entry.refreshedOnMiss && (!match || (match.slug === "root" && match.normalizedDirectory !== target))) { + // Refresh once for this cache lifetime, including sequential misses from + // distinct foreign event locations. Otherwise every miss discards the last + // negative result and stalls the serial native event bridge on inventory I/O. if (worktreeCache.get(params.workspaceId) === entry) worktreeCache.delete(params.workspaceId) - entry = await getCachedWorktrees(params) + entry = await getCachedWorktrees(params, true) + entry.refreshedOnMiss = true match = find(entry.worktrees) } const resolved = match ? { slug: match.slug, directory: target, worktreeDirectory: match.worktreeDirectory } : null diff --git a/packages/server/src/workspaces/worktree-inventory.test.ts b/packages/server/src/workspaces/worktree-inventory.test.ts new file mode 100644 index 000000000..eb7150365 --- /dev/null +++ b/packages/server/src/workspaces/worktree-inventory.test.ts @@ -0,0 +1,168 @@ +import assert from "node:assert/strict" +import { it } from "node:test" +import type { WorktreeListResponse } from "../api-types" +import { WorktreeInventory } from "./worktree-inventory" + +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + const promise = new Promise((yes, no) => { resolve = yes; reject = no }) + return { promise, resolve, reject } +} + +const snapshot = (branch: string): WorktreeListResponse => ({ + isGitRepo: true, + worktrees: [{ slug: "root", directory: "/repo", kind: "root", branch }], +}) + +function fixture() { + let now = 1 + const scans: ReturnType>[] = [] + const changes: string[] = [] + const failures: unknown[] = [] + const cache = new WorktreeInventory({ + now: () => now, + load: async () => { + const scan = deferred() + scans.push(scan) + return scan.promise + }, + changed: id => { changes.push(id) }, + failed: (_id, error) => { failures.push(error) }, + }) + return { cache, scans, changes, failures, advance: () => { now += 10_001 } } +} + +const turn = () => new Promise(resolve => setImmediate(resolve)) + +async function seed(f: ReturnType) { + const read = f.cache.read("repo") + await turn() + f.scans[0].resolve(snapshot("main")) + await read +} + +it("shares cold reads and retains their result across sequential requests", async () => { + const f = fixture() + const reads = [f.cache.read("repo"), f.cache.read("repo", "validated")] + await turn() + assert.equal(f.scans.length, 1) + f.scans[0].resolve(snapshot("main")) + assert.deepEqual(await Promise.all(reads), [snapshot("main"), snapshot("main")]) + assert.deepEqual(await f.cache.read("repo"), snapshot("main")) + assert.equal(f.scans.length, 1) + assert.deepEqual(f.changes, []) +}) + +it("returns expired data immediately and runs one lazy refresh for all consumers", async () => { + const f = fixture() + await seed(f) + f.advance() + await turn() + assert.equal(f.scans.length, 1, "expiry alone must not start Git") + const reads = await Promise.all(Array.from({ length: 20 }, () => f.cache.read("repo"))) + assert.ok(reads.every(value => value.worktrees[0].branch === "main")) + assert.equal(f.scans.length, 2) + let authorized = false + const validation = f.cache.read("repo", "validated").then(value => { authorized = true; return value }) + await turn() + assert.equal(authorized, false, "authorization must not use expired display data") + f.scans[1].resolve(snapshot("feature")) + assert.deepEqual(await validation, snapshot("feature")) + assert.deepEqual(await f.cache.read("repo"), snapshot("feature")) + assert.deepEqual(f.changes, ["repo"]) + assert.equal(f.scans.length, 2) +}) + +it("invalidates lazily and forces fresh family reads even inside the TTL", async () => { + const f = fixture() + await seed(f) + f.cache.invalidate() + assert.equal(f.scans.length, 1) + assert.deepEqual(await f.cache.read("repo"), snapshot("main")) + const validated = f.cache.read("repo", "validated") + f.scans[1].resolve(snapshot("feature")) + await validated + const fresh = f.cache.read("repo", "fresh") + await turn() + assert.equal(f.scans.length, 3) + f.scans[2].resolve(snapshot("feature")) + await fresh + assert.deepEqual(f.changes, ["repo"], "unchanged scans must not cause an SSE reload loop") +}) + +it("fences a scan overtaken by a mutation and serializes a trailing validation", async () => { + const f = fixture() + await seed(f) + const fresh = f.cache.read("repo", "fresh") + await turn() + f.cache.invalidate("repo") + const afterMutation = f.cache.read("repo", "validated") + assert.equal(f.scans.length, 2) + f.scans[1].resolve(snapshot("obsolete")) + await turn() + assert.deepEqual(f.changes, []) + assert.equal(f.scans.length, 3) + assert.deepEqual(await f.cache.read("repo"), snapshot("main")) + f.scans[2].resolve(snapshot("new")) + assert.deepEqual(await Promise.all([fresh, afterMutation]), [snapshot("new"), snapshot("new")]) + assert.deepEqual(f.changes, ["repo"]) +}) + +it("keeps the last good display snapshot on failure with demand-driven retry backoff", async () => { + const f = fixture() + await seed(f) + f.advance() + await f.cache.read("repo") + await f.cache.read("repo") + const validated = f.cache.read("repo", "validated") + const rejected = assert.rejects(validated, /offline/) + f.scans[1].reject(new Error("offline")) + await rejected + await turn() + assert.deepEqual(await f.cache.read("repo"), snapshot("main")) + assert.equal(f.failures.length, 1) + assert.equal(f.scans.length, 2) + f.advance() + await f.cache.read("repo") + const retry = f.cache.read("repo", "validated") + f.scans[2].resolve(snapshot("recovered")) + await retry + assert.deepEqual(f.changes, ["repo"]) +}) + +it("never republishes a scan after workspace disposal and isolates workspace keys", async () => { + const f = fixture() + await seed(f) + const fresh = f.cache.read("repo", "fresh") + const rejected = assert.rejects(fresh, /disposed/) + await turn() + f.cache.forget("repo") + const other = f.cache.read("other") + await turn() + f.scans[1].resolve(snapshot("obsolete")) + f.scans[2].resolve(snapshot("other")) + await rejected + assert.deepEqual(await other, snapshot("other")) + assert.deepEqual(f.changes, []) + const reopened = f.cache.read("repo") + await turn() + assert.equal(f.scans.length, 4) + f.scans[3].resolve(snapshot("reopened")) + assert.deepEqual(await reopened, snapshot("reopened")) +}) + +it("waits for the post-mutation inventory instead of returning a pre-create display snapshot", async () => { + const f = fixture() + await seed(f) + f.cache.invalidate(undefined, "blocking") + let settled = false + const read = f.cache.read("repo").then(value => { settled = true; return value }) + await turn() + assert.equal(settled, false) + const created = snapshot("main") + created.worktrees.push({ slug: "new", directory: "/repo/new", kind: "worktree" }) + f.scans[1].resolve(created) + assert.deepEqual(await read, created) + assert.deepEqual(f.changes, ["repo"]) +}) diff --git a/packages/server/src/workspaces/worktree-inventory.ts b/packages/server/src/workspaces/worktree-inventory.ts new file mode 100644 index 000000000..2a04a3dc9 --- /dev/null +++ b/packages/server/src/workspaces/worktree-inventory.ts @@ -0,0 +1,91 @@ +import type { WorktreeListResponse } from "../api-types" + +type ReadMode = "cached" | "validated" | "fresh" +type Entry = { + value?: WorktreeListResponse + expiresAt: number + retryAt: number + revision: number + requireValidation?: boolean + pending?: Promise +} + +const MAX_AGE_MS = 10_000 + +/** Demand-driven inventory: display reads keep the last snapshot while refreshing. + * Directory authorization waits for validation; family transactions force a scan. + * No timers, and no rejected/obsolete scan can replace the last good snapshot. + */ +export class WorktreeInventory { + private readonly entries = new Map() + + constructor(private readonly options: { + load: (id: string) => Promise + changed: (id: string) => void + failed: (id: string, error: unknown) => void + now?: () => number + }) {} + + async read(id: string, mode: ReadMode = "cached"): Promise { + let entry = this.entries.get(id) + if (!entry) { + entry = { expiresAt: 0, retryAt: 0, revision: 0 } + this.entries.set(id, entry) + } + const now = this.now() + if (mode !== "fresh" && entry.value && entry.expiresAt > now) return entry.value + if (mode === "cached" && entry.value && !entry.requireValidation) { + if (!entry.pending && entry.retryAt <= now) void this.load(id, entry).catch(error => this.options.failed(id, error)) + return entry.value + } + return this.load(id, entry) + } + + // Invalidation is lazy: retain the display snapshot, but fence any running scan. + invalidate(id?: string, mode: "lazy" | "blocking" = "lazy"): void { + const entries = id === undefined ? this.entries.values() : [this.entries.get(id)] + for (const entry of entries) { + if (!entry) continue + entry.revision += 1 + entry.expiresAt = 0 + entry.retryAt = 0 + if (mode === "blocking") entry.requireValidation = true + } + } + + forget(id: string): void { + this.entries.delete(id) + } + + private load(id: string, entry: Entry): Promise { + if (entry.pending) return entry.pending + const revision = entry.revision + const previous = entry.value + const task = Promise.resolve().then(() => this.options.load(id)).then(async value => { + if (this.entries.get(id) !== entry) throw new Error("Workspace inventory was disposed") + if (revision !== entry.revision) { + // A mutation overtook the scan. Serialize one trailing validation rather + // than publishing obsolete data or starting overlapping Git process fans. + entry.pending = undefined + return this.load(id, entry) + } + entry.value = value + entry.expiresAt = this.now() + MAX_AGE_MS + entry.retryAt = 0 + entry.requireValidation = false + if (previous && JSON.stringify(previous) !== JSON.stringify(value)) this.options.changed(id) + return value + }).catch(error => { + if (revision === entry.revision) entry.retryAt = this.now() + MAX_AGE_MS + throw error + }).finally(() => { + if (entry.pending === task) entry.pending = undefined + }) + entry.pending = task + return task + } + + private now(): number { + return (this.options.now ?? Date.now)() + } +} diff --git a/packages/ui/src/components/worktree-selector.tsx b/packages/ui/src/components/worktree-selector.tsx index 062f3cf85..0b0ea0ecd 100644 --- a/packages/ui/src/components/worktree-selector.tsx +++ b/packages/ui/src/components/worktree-selector.tsx @@ -1,6 +1,6 @@ import { Select } from "@kobalte/core/select" import { Dialog } from "@kobalte/core/dialog" -import { Show, createMemo, createSignal, createUniqueId } from "solid-js" +import { Show, createMemo, createSignal, createUniqueId, untrack } from "solid-js" import { ChevronDown, Copy, FolderOpen, Trash2 } from "lucide-solid" import type { WorktreeDescriptor } from "../../../server/src/api-types" import { getLogger } from "../lib/logger" @@ -148,9 +148,29 @@ export default function WorktreeSelector(props: WorktreeSelectorProps) { const gitRepoStatus = createMemo(() => getGitRepoStatus(props.instanceId)) const worktreesUnavailable = createMemo(() => gitRepoStatus() === false) const dropdownDisabled = createMemo(() => isChildSession() || worktreesUnavailable()) + let listbox: HTMLUListElement | undefined const worktreeOptions = createMemo(() => { const list = getWorktrees(props.instanceId) + // Kobalte recreates option DOM when its collection changes. Keep the user's + // keyboard target (including an inline action) through lazy inventory updates. + const focused = document.activeElement as HTMLElement | null + const option = focused?.closest("[role=option]") + const key = option?.dataset.key + const actionLabel = focused?.closest("button")?.getAttribute("aria-label") + if (key && listbox?.contains(focused) && untrack(isOpen)) { + queueMicrotask(() => { + if (!isOpen() || focused?.isConnected || !listbox?.isConnected) return + if (document.activeElement !== document.body && !listbox.contains(document.activeElement)) return + const replacement = Array.from(listbox.querySelectorAll("[role=option]")).find(item => item.dataset.key === key) + const action = actionLabel ? Array.from(replacement?.querySelectorAll("button") ?? []).find(button => button.getAttribute("aria-label") === actionLabel) : undefined + const target = action ?? replacement + // Update Kobalte's focused key before focusing a nested button; a button + // focus alone leaves the previous option as its keyboard target. + if (action) replacement?.focus({ preventScroll: true }) + ;(target ?? listbox).focus({ preventScroll: true }) + }) + } const mapped: WorktreeOption[] = list.map((wt) => ({ kind: "worktree", key: wt.slug, @@ -317,14 +337,16 @@ export default function WorktreeSelector(props: WorktreeSelectorProps) { open={isOpen()} onOpenChange={(open) => { if (!open) { setIsOpen(false); return } + setIsOpen(true) void reloadWorktrees(props.instanceId).catch(error => log.warn("Failed to refresh worktrees", error)) - .then(() => setIsOpen(true)) }} value={selectedOption() ?? null} onChange={(value) => { void handleChange(value).catch((error) => log.warn("Failed to change worktree", error)) }} options={worktreeOptions()} + // Inventory reconciliation is not a user selection and must not close the menu. + allowDuplicateSelectionEvents={false} optionValue="key" optionTextValue={(opt) => (opt.kind === "action" ? opt.label : opt.raw.label ?? opt.slug)} placeholder={t("sessionList.sort.worktree")} @@ -345,7 +367,17 @@ export default function WorktreeSelector(props: WorktreeSelectorProps) { } return ( - + { if (opt.slug === currentSlug()) setIsOpen(false) }} + onKeyDown={(event) => { + if (opt.slug === currentSlug() && (event.key === "Enter" || event.key === " ")) { + event.preventDefault() + setIsOpen(false) + } + }} + >
@@ -468,7 +500,7 @@ export default function WorktreeSelector(props: WorktreeSelectorProps) { - + diff --git a/packages/ui/src/stores/worktree-ready.test.ts b/packages/ui/src/stores/worktree-ready.test.ts index 67886ca53..999edcb68 100644 --- a/packages/ui/src/stores/worktree-ready.test.ts +++ b/packages/ui/src/stores/worktree-ready.test.ts @@ -7,6 +7,31 @@ import type { Session } from "../types/session.ts" import { sessions, setSessions } from "./session-state.ts" describe("handleWorktreeReady", () => { + it("reports a failed trailing reload and retains the successful initial snapshot", async () => { + const original = serverApi.fetchWorktrees + let release!: () => void + const gate = new Promise(resolve => { release = resolve }) + let calls = 0 + serverApi.fetchWorktrees = async () => { + if (++calls > 1) throw new Error("trailing refresh failed") + await gate + return { isGitRepo: true, worktrees: [{ slug: "root", directory: "/repo", kind: "root" }] } + } + try { + const initial = ensureWorktreesLoaded("failed-trailing-read") + await Promise.resolve() + const reload = reloadWorktrees("failed-trailing-read") + const rejected = assert.rejects(Promise.all([initial, reload]), /trailing refresh failed/) + release() + await rejected + assert.equal(calls, 2) + assert.equal(getWorktrees("failed-trailing-read")[0]?.slug, "root") + } finally { + release() + serverApi.fetchWorktrees = original + } + }) + it("refreshes worktrees", async () => { const calls: string[] = [] @@ -106,6 +131,7 @@ describe("handleWorktreeReady", () => { try { const initial = ensureWorktreesLoaded(instanceId) + await Promise.resolve() const reload = reloadWorktrees(instanceId) await Promise.resolve() @@ -115,8 +141,7 @@ describe("handleWorktreeReady", () => { isGitRepo: true, worktrees: [{ slug: "root", directory: "/repo", kind: "root" }], }) - await initial - await Promise.resolve() + await new Promise(resolve => setImmediate(resolve)) assert.equal(requestCount, 2) @@ -127,7 +152,7 @@ describe("handleWorktreeReady", () => { { slug: "feature", directory: "/repo-feature", kind: "worktree" }, ], }) - await reload + await Promise.all([initial, reload]) assert.deepEqual(getWorktrees(instanceId).map((worktree) => worktree.slug), ["root", "feature"]) } finally { diff --git a/packages/ui/src/stores/worktrees.ts b/packages/ui/src/stores/worktrees.ts index 5c2602dd6..fa2280adf 100644 --- a/packages/ui/src/stores/worktrees.ts +++ b/packages/ui/src/stores/worktrees.ts @@ -1,6 +1,7 @@ import { createSignal } from "solid-js" import type { WorktreeDescriptor } from "../../../server/src/api-types" import { serverApi } from "../lib/api-client" +import { serverEvents } from "../lib/server-events" import { getSessionRoot, sessions } from "./session-state" import { getLogger } from "../lib/logger" import type { WorktreeReadyEvent } from "../lib/sse-manager" @@ -14,6 +15,7 @@ const [worktreesByInstance, setWorktreesByInstance] = createSignal>(new Map()) const worktreeRequests = new Map>() +const pendingWorktreeRefreshes = new Set() const worktreeReadyRefreshes = new Map>() const familyMoveRequests = new Map>() const defaultDirectories = new Map() @@ -21,8 +23,12 @@ const defaultDirectories = new Map() type WorktreeReadyRefresh = (instanceId: string) => Promise async function queueWorktreeRequest(instanceId: string, initial: boolean): Promise { - const previous = worktreeRequests.get(instanceId) - const task = (previous?.catch(() => undefined) ?? Promise.resolve()).then(async () => { + const existing = worktreeRequests.get(instanceId) + if (existing) { + if (!initial) pendingWorktreeRefreshes.add(instanceId) + return existing + } + const load = async (initialRead: boolean) => { try { const response = await serverApi.fetchWorktrees(instanceId) if (response.defaultDirectory) defaultDirectories.set(instanceId, response.defaultDirectory) @@ -39,8 +45,8 @@ async function queueWorktreeRequest(instanceId: string, initial: boolean): Promi return next }) } catch (error) { - log.warn(initial ? "Failed to load worktrees" : "Failed to reload worktrees", { instanceId, error }) - if (!initial) throw error + log.warn(initialRead ? "Failed to load worktrees" : "Failed to reload worktrees", { instanceId, error }) + if (!initialRead) throw error setWorktreesByInstance((prev) => { const next = new Map(prev) @@ -56,6 +62,20 @@ async function queueWorktreeRequest(instanceId: string, initial: boolean): Promi return next }) } + } + // Like the provider/model catalogue, retain one in-flight read and one dirty + // bit. A burst requests one trailing read, not an unbounded HTTP queue. + const task = Promise.resolve().then(async () => { + let initialRead = initial + do { + pendingWorktreeRefreshes.delete(instanceId) + try { + await load(initialRead) + } catch (error) { + if (!pendingWorktreeRefreshes.has(instanceId)) throw error + } + initialRead = false + } while (pendingWorktreeRefreshes.has(instanceId)) }) worktreeRequests.set(instanceId, task) @@ -84,6 +104,15 @@ async function reloadWorktrees(instanceId: string): Promise { await queueWorktreeRequest(instanceId, false) } +serverEvents.on("workspace.worktreesChanged", (event) => { + if (event.type !== "workspace.worktreesChanged") return + const id = event.workspaceId + // Refresh consumers that already requested this inventory. Queue behind an + // older HTTP response so it cannot overwrite the completed background scan. + if (!worktreesByInstance().has(id) && !worktreeRequests.has(id)) return + void reloadWorktrees(id).catch(error => log.warn("Failed to receive refreshed worktrees", { instanceId: id, error })) +}) + async function handleWorktreeReady( instanceId: string, event: WorktreeReadyEvent, diff --git a/packages/ui/tests/browser/fixtures/worktrees.tsx b/packages/ui/tests/browser/fixtures/worktrees.tsx index d9e27fd33..4410531f7 100644 --- a/packages/ui/tests/browser/fixtures/worktrees.tsx +++ b/packages/ui/tests/browser/fixtures/worktrees.tsx @@ -6,7 +6,8 @@ import { serverApi } from "../../../src/lib/api-client" import { sdkManager } from "../../../src/lib/sdk-manager" import { addInstance } from "../../../src/stores/instances" import { setSessions } from "../../../src/stores/session-state" -import { ensureWorktreesLoaded } from "../../../src/stores/worktrees" +import { ensureWorktreesLoaded, reloadWorktrees, getWorktrees } from "../../../src/stores/worktrees" +import { serverEvents } from "../../../src/lib/server-events" import "../../../src/index.css" const id = "worktree-fixture" @@ -43,4 +44,57 @@ render(() =>
, document.getElementById("root")!) await updatePreferences({ locale: "en" }) -;(window as any).fixture = { calls, location: () => session.location.directory } +;(window as any).fixture = { + calls, + location: () => session.location.directory, + worktrees: () => getWorktrees(id), + refreshBurst: async () => { + let release!: () => void + const gate = new Promise(resolve => { release = resolve }) + let requests = 0 + serverApi.fetchWorktrees = async () => { + requests++ + if (requests === 1) await gate + return { isGitRepo: true, worktrees: entries } + } + const first = reloadWorktrees(id) + await Promise.resolve() + const burst = Array.from({ length: 40 }, () => reloadWorktrees(id)) + release() + await Promise.all([first, ...burst]) + return requests + }, + holdRefresh: () => { + let release!: () => void + const pending = new Promise(resolve => { release = resolve }) + serverApi.fetchWorktrees = async () => { + await pending + return { isGitRepo: true, worktrees: entries.map(entry => ({ ...entry })) } + } + ;(window as any).fixture.releaseRefresh = async () => { + release() + await reloadWorktrees(id) + } + }, + backgroundUpdate: async () => { + const old = entries.map(entry => ({ ...entry })) + let release!: () => void + const pending = new Promise(resolve => { release = resolve }) + let requests = 0 + serverApi.fetchWorktrees = async () => { + requests++ + if (requests === 1) { + await pending + return { isGitRepo: true, worktrees: old } + } + return { isGitRepo: true, worktrees: entries } + } + const initial = reloadWorktrees(id) + await Promise.resolve() + entries[1] = { ...entries[1], label: "renamed in background" } + // Exercise the production dispatcher while an older HTTP reply is pending. + ;(serverEvents as any).dispatchBatch([{ type: "workspace.worktreesChanged", workspaceId: id }]) + release() + await initial + }, +} diff --git a/packages/ui/tests/browser/worktrees.test.ts b/packages/ui/tests/browser/worktrees.test.ts index 186807c9a..e053745f7 100644 --- a/packages/ui/tests/browser/worktrees.test.ts +++ b/packages/ui/tests/browser/worktrees.test.ts @@ -61,6 +61,82 @@ test("worktree actions use click/keyboard/touch without selecting or moving the } finally { await context.close() } }) +test("background inventory completion updates the selector after an older HTTP response", async () => { + const page = await browser.newPage() + try { + await prepare(page) + await page.locator(".selector-trigger").click() + await page.getByRole("option", { name: /feature/ }).waitFor() + await page.getByRole("option", { name: /feature/ }).focus() + const focused = await page.evaluate(() => (document.activeElement as HTMLElement)?.dataset.key) + await page.evaluate(() => (window as any).fixture.backgroundUpdate()) + await page.waitForFunction(() => (window as any).fixture.worktrees().some((entry: any) => entry.label === "renamed in background")) + await page.getByRole("option", { name: /renamed in background/ }).waitFor() + assert.ok(focused) + assert.equal(await page.evaluate(() => (document.activeElement as HTMLElement)?.dataset.key), focused) + assert.deepEqual(await page.evaluate(() => (window as any).fixture.calls), []) + assert.equal(await page.evaluate(() => (window as any).fixture.location()), "/repo") + } finally { await page.close() } +}) + +test("selecting the current worktree dismisses the menu without moving the session", async () => { + const page = await browser.newPage() + try { + await prepare(page) + const trigger = page.locator(".selector-trigger") + await trigger.click() + await page.getByRole("option", { name: "Workspace", exact: true }).click() + await page.getByRole("listbox").waitFor({ state: "hidden" }) + assert.deepEqual(await page.evaluate(() => (window as any).fixture.calls), []) + await trigger.focus() + await page.keyboard.press("Enter") + await page.getByRole("listbox").waitFor() + await page.keyboard.press("Enter") + await page.getByRole("listbox").waitFor({ state: "hidden" }) + assert.deepEqual(await page.evaluate(() => (window as any).fixture.calls), []) + } finally { await page.close() } +}) + +test("coalesces a refresh burst into one trailing HTTP read", async () => { + const page = await browser.newPage() + try { + await prepare(page) + assert.equal(await page.evaluate(() => (window as any).fixture.refreshBurst()), 2) + } finally { await page.close() } +}) + +test("keeps an inline action focused through a background rename", async () => { + const page = await browser.newPage() + try { + await prepare(page) + await page.locator(".selector-trigger").click() + await page.getByRole("option", { name: /feature/ }).getByRole("button", { name: "Copy path", exact: true }).focus() + assert.equal(await page.evaluate(() => document.activeElement?.getAttribute("aria-label")), "Copy path", "inline action must be focusable before refresh") + await page.evaluate(() => (window as any).fixture.backgroundUpdate()) + const copy = page.getByRole("option", { name: /renamed in background/ }).getByRole("button", { name: "Copy path", exact: true }) + await copy.waitFor() + assert.equal(await copy.evaluate(element => element === document.activeElement), true, await page.evaluate(() => document.activeElement?.outerHTML)) + await page.keyboard.press("Enter") + await page.waitForFunction(() => (window as any).copied === "/repo/.codenomad/worktrees/feature") + assert.deepEqual(await page.evaluate(() => (window as any).fixture.calls), []) + } finally { await page.close() } +}) + +test("opens cached options before refresh completes and keeps a dismissed menu closed", async () => { + const page = await browser.newPage() + try { + await prepare(page) + await page.evaluate(() => (window as any).fixture.holdRefresh()) + await page.locator(".selector-trigger").click() + await page.getByRole("option", { name: /feature/ }).waitFor() + await page.keyboard.press("Escape") + await page.getByRole("listbox").waitFor({ state: "hidden" }) + await page.evaluate(() => (window as any).fixture.releaseRefresh()) + assert.equal(await page.locator(".selector-trigger").getAttribute("aria-expanded"), "false") + assert.deepEqual(await page.evaluate(() => (window as any).fixture.calls), []) + } finally { await page.close() } +}) + test("creation uses the selected source and the returned stable worktree ID", async () => { const page = await browser.newPage() try { diff --git a/scripts/test-native-worktree-management.mjs b/scripts/test-native-worktree-management.mjs index 7379cbe81..24a73a068 100644 --- a/scripts/test-native-worktree-management.mjs +++ b/scripts/test-native-worktree-management.mjs @@ -3,10 +3,12 @@ import { execFileSync } from "node:child_process" import { mkdir, writeFile, rm, realpath } from "node:fs/promises" import path from "node:path" import { tsImport } from "tsx/esm/api" +import pino from "pino" // Invoked by the isolated daemon fixture only. No service discovery/user state. export async function testNativeWorktreeManagement({ client, root }) { const { listNativeWorktrees, createNativeWorktree, removeNativeWorktree } = await tsImport("../packages/server/src/workspaces/native-worktrees.ts", import.meta.url) + const { WorktreeInventory } = await tsImport("../packages/server/src/workspaces/worktree-inventory.ts", import.meta.url) const repo = path.join(root, "worktree-policy") const external = path.join(root, "agent-created") const clone = path.join(root, "independent-clone") @@ -64,5 +66,74 @@ export async function testNativeWorktreeManagement({ client, root }) { const fromLinked = await listNativeWorktrees({ ...context, workspacePath: external, location: { directory: external } }) assert.equal(fromLinked.defaultDirectory, catalogue.defaultDirectory, "opening a linked checkout must not nest the default parent") assert.equal(fromLinked.worktrees.find(entry => entry.branch === "main").removable, false) + let scans = 0 + const changed = [] + const inventory = new WorktreeInventory({ + load: async () => { assert.ok(++scans < 8, "native refresh events must not cause a scan loop"); return listNativeWorktrees(context) }, + changed: id => changed.push(id), + failed: (_id, error) => { throw error }, + }) + const controller = new AbortController() + let connected + const ready = new Promise(resolve => { connected = resolve }) + const events = (async () => { + for await (const event of client.event.subscribe({ signal: controller.signal })) { + if (event.type === "server.connected") connected() + if (event.type === "worktree.updated") inventory.invalidate() + } + })() + try { + await ready + const cached = await inventory.read("fixture") + assert.equal(await inventory.read("fixture"), cached) + assert.equal(scans, 1, "sequential display reads must reuse the completed native scan") + git(external, "branch", "-m", "agent-renamed") + inventory.invalidate("fixture") + assert.equal(scans, 1, "invalidation must stay lazy") + assert.equal(await inventory.read("fixture"), cached, "display must not wait for background Git") + const updated = await inventory.read("fixture", "validated") + assert.equal(updated.worktrees.find(entry => entry.slug === source.slug).branch, "agent-renamed") + assert.deepEqual(changed, ["fixture"]) + assert.equal(scans, 2) + await inventory.read("fixture", "fresh") + assert.equal(scans, 3, "family validation must bypass a warm display cache") + assert.deepEqual(changed, ["fixture"], "an unchanged scan must not create a reload feedback loop") + console.log("PASS: native worktree cache reuse, lazy rename refresh and forced family validation") + } finally { + controller.abort() + await events.catch(error => { if (!controller.signal.aborted) throw error }) + } + const { WorkspaceManager } = await tsImport("../packages/server/src/workspaces/manager.ts", import.meta.url) + const { EventBus } = await tsImport("../packages/server/src/events/bus.ts", import.meta.url) + // Exercise the production manager's post-mutation invalidation using only + // this fixture's authenticated client. No native discovery/user service. + const manager = new WorkspaceManager({ + rootDir: root, + settings: { getOwner: () => ({ environmentVariables: {} }) }, + binaryResolver: { resolveDefault: () => ({ path: process.execPath, label: "Isolated fixture" }) }, + eventBus: new EventBus(), + logger: pino({ level: "silent" }), + sharedService: { + client: async () => client, + headers: async () => ({}), + validateLocation: async location => client.location.get({ location }), + shutdown: async () => {}, + }, + }) + try { + const { workspace } = await manager.create(repo) + const before = await manager.getWorktrees(workspace.id) + const added = await manager.createWorktree(workspace.id, "cached-create") + const afterCreate = await manager.getWorktrees(workspace.id) + assert.equal(afterCreate.worktrees.length, before.worktrees.length + 1) + assert.ok(afterCreate.worktrees.some(entry => entry.slug === added.slug), "create-and-use must receive the new ID immediately") + await manager.removeWorktree(workspace.id, added.serviceRoot, false) + const afterRemove = await manager.getWorktrees(workspace.id) + assert.equal(afterRemove.worktrees.length, before.worktrees.length) + assert.ok(!afterRemove.worktrees.some(entry => entry.slug === added.slug), "post-delete refresh must not resurrect a cached checkout") + console.log("PASS: production manager warm-cache create/remove read-your-writes") + } finally { + await manager.shutdown() + } console.log("PASS: native worktree discovery/create/remove, clone scope, selected HEAD, default parent, named branches, stable identity, nested paths and dirty/checked-out guards") }