diff --git a/packages/agent-core-v2/src/app/workspaceRegistry/workspaceRegistryService.ts b/packages/agent-core-v2/src/app/workspaceRegistry/workspaceRegistryService.ts index 9baaebdb98..16e1547697 100644 --- a/packages/agent-core-v2/src/app/workspaceRegistry/workspaceRegistryService.ts +++ b/packages/agent-core-v2/src/app/workspaceRegistry/workspaceRegistryService.ts @@ -1,26 +1,33 @@ /** * `workspaceRegistry` domain (L1) — `IWorkspaceRegistry` implementation. * - * Process-wide catalog of known workspaces, durable: an in-memory cache is - * loaded once from `IWorkspacePersistence` (`/workspaces.json`, the - * v1-compatible file shared with agent-core) and every mutation writes back - * through it. Loading has two paths: + * Process-wide catalog of known workspaces, durable in + * `/workspaces.json` (the v1-compatible file shared with + * agent-core). The service keeps NO in-memory write cache: every operation + * is a fresh read-modify-write against the file, serialized through a + * promise-chain mutex. This is required, not just tidy — the same file is + * written concurrently by other processes (the v1 TUI registers session cwds + * via `touchWorkspaceRegistry`, which also re-reads the file on every call), + * so a write-through cache would clobber external additions and tombstones + * with stale state. Atomic renames at the persistence layer plus fresh + * read-modify-write on both engines shrink the lost-update window to a + * single read-modify-write, and the next session-index merge heals anything + * still lost there. * - * 1. No usable catalog file → one-shot rebuild from the legacy - * `/session_index.jsonl` (one workspace per distinct absolute - * `workDir`), then persisted. - * 2. Catalog loaded → a one-time merge from the same session index adds every - * workDir the file does not know about yet (e.g. sessions created by the - * v1 TUI since the last merge), then persisted if anything changed. + * Once per process, the first operation triggers the startup sync with the + * legacy `/session_index.jsonl`: + * + * 1. No usable catalog file → one-shot rebuild (one workspace per distinct + * absolute `workDir`), persisted. + * 2. Catalog loaded → only workDirs the file does not know about yet are + * added (e.g. sessions created by the v1 TUI since the last sync), + * persisted if anything changed. * * Deletion is soft: `delete` drops the entry but records the id in * `deleted_workspace_ids`, and the merge never resurrects a tombstoned id. * An explicit `createOrTouch` clears the tombstone — the user opening the * folder again is a stronger signal than the historical index. * - * All access is serialized through a promise-chain mutex so - * load/rebuild/merge/mutations never race. - * * `createOrTouch` is the single choke point every workspace/session creation * funnels through, so it owns the root-existence contract: the root must be * an existing directory on the host filesystem, otherwise it throws @@ -39,7 +46,7 @@ import { IHostFileSystem } from '#/os/interface/hostFileSystem'; import { IFileSystemStorageService } from '#/persistence/interface/storage'; import { IWorkspaceRegistry, type Workspace, type WorkspaceUpdate } from './workspaceRegistry'; -import { IWorkspacePersistence } from './workspacePersistence'; +import { IWorkspacePersistence, type WorkspaceCatalog } from './workspacePersistence'; const SESSION_INDEX_SCOPE = ''; const SESSION_INDEX_KEY = 'session_index.jsonl'; @@ -55,8 +62,8 @@ interface SessionIndexLine { export class WorkspaceRegistryService implements IWorkspaceRegistry { declare readonly _serviceBrand: undefined; - private cache: Map | undefined; - private deletedIds: Set | undefined; + /** Whether the once-per-process session-index sync already ran. */ + private merged = false; private opQueue: Promise = Promise.resolve(); constructor( @@ -67,21 +74,23 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry { list(): Promise { return this.runExclusive(async () => { - const cache = await this.ensureLoaded(); - return dedupeByRoot(cache); + await this.ensureMerged(); + const catalog = await this.loadCatalog(); + const byId = new Map(catalog.workspaces.map((ws) => [ws.id, ws])); + return dedupeByRoot(byId); }); } get(id: string): Promise { return this.runExclusive(async () => { - const cache = await this.ensureLoaded(); - return cache.get(id); + await this.ensureMerged(); + const catalog = await this.loadCatalog(); + return catalog.workspaces.find((ws) => ws.id === id); }); } createOrTouch(root: string, name?: string): Promise { return this.runExclusive(async () => { - const cache = await this.ensureLoaded(); let stat; try { stat = await this.hostFs.stat(root); @@ -95,8 +104,12 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry { if (!stat.isDirectory) { throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `workspace root ${root} is not a directory`); } + await this.ensureMerged(); + const catalog = await this.loadCatalog(); + const byId = new Map(catalog.workspaces.map((ws) => [ws.id, ws])); + const deletedIds = new Set(catalog.deletedIds); const id = encodeWorkDirKey(root); - const existing = cache.get(id); + const existing = byId.get(id); const now = Date.now(); const ws: Workspace = existing !== undefined @@ -108,73 +121,84 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry { createdAt: now, lastOpenedAt: now, }; - cache.set(id, ws); + byId.set(id, ws); // An explicit add clears any prior deletion tombstone. - this.deletedIds?.delete(id); - await this.persist(); + deletedIds.delete(id); + await this.store.save({ workspaces: [...byId.values()], deletedIds: [...deletedIds] }); return ws; }); } update(id: string, patch: WorkspaceUpdate): Promise { return this.runExclusive(async () => { - const cache = await this.ensureLoaded(); - const existing = cache.get(id); + await this.ensureMerged(); + const catalog = await this.loadCatalog(); + const existing = catalog.workspaces.find((ws) => ws.id === id); if (existing === undefined) return undefined; const updated: Workspace = { ...existing, ...(patch.name !== undefined ? { name: patch.name } : {}), }; - cache.set(id, updated); - await this.persist(); + await this.store.save({ + workspaces: catalog.workspaces.map((ws) => (ws.id === id ? updated : ws)), + deletedIds: catalog.deletedIds, + }); return updated; }); } delete(id: string): Promise { return this.runExclusive(async () => { - const cache = await this.ensureLoaded(); - cache.delete(id); + await this.ensureMerged(); + const catalog = await this.loadCatalog(); // Soft delete: tombstone the id so the session-index merge cannot // resurrect it, even if sessions still reference the workDir. - this.deletedIds?.add(id); - await this.persist(); + await this.store.save({ + workspaces: catalog.workspaces.filter((ws) => ws.id !== id), + deletedIds: [...new Set([...catalog.deletedIds, id])], + }); }); } - private async ensureLoaded(): Promise> { - if (this.cache !== undefined) return this.cache; + /** Once-per-process startup sync with the legacy session index (see the + * file header). Runs inside the op mutex, so it cannot interleave with a + * mutation's read-modify-write. */ + private async ensureMerged(): Promise { + if (this.merged) return; const loaded = await this.store.load(); if (loaded === undefined) { const rebuilt = await this.rebuildFromSessionIndex(); - this.cache = rebuilt; - this.deletedIds = new Set(); - await this.persist(); - return rebuilt; + await this.store.save({ workspaces: [...rebuilt.values()], deletedIds: [] }); + this.merged = true; + return; } - const cache = new Map(loaded.workspaces.map((ws) => [ws.id, ws])); + const byId = new Map(loaded.workspaces.map((ws) => [ws.id, ws])); const deletedIds = new Set(loaded.deletedIds); - this.cache = cache; - this.deletedIds = deletedIds; - if (await this.mergeFromSessionIndex(cache, deletedIds)) { - await this.persist(); + if (await this.mergeFromSessionIndex(byId, deletedIds)) { + await this.store.save({ workspaces: [...byId.values()], deletedIds: [...deletedIds] }); } - return cache; + this.merged = true; + } + + /** Read the current catalog; a missing or malformed file is an empty + * catalog (mirrors v1's tolerant read). */ + private async loadCatalog(): Promise { + return (await this.store.load()) ?? { workspaces: [], deletedIds: [] }; } /** Add every distinct workDir from the legacy session index that the * catalog does not know about yet. Tombstoned ids are skipped, so a * soft-deleted workspace stays deleted. Returns whether anything changed. */ private async mergeFromSessionIndex( - cache: Map, + byId: Map, deletedIds: ReadonlySet, ): Promise { let changed = false; const now = Date.now(); for (const workDir of await this.readSessionIndexWorkDirs()) { const id = encodeWorkDirKey(workDir); - if (cache.has(id) || deletedIds.has(id)) continue; - cache.set(id, { + if (byId.has(id) || deletedIds.has(id)) continue; + byId.set(id, { id, root: workDir, name: basename(workDir), @@ -218,18 +242,6 @@ export class WorkspaceRegistryService implements IWorkspaceRegistry { return workDirs; } - private async persist(): Promise { - const cache = this.cache; - const deletedIds = this.deletedIds; - if (cache === undefined || deletedIds === undefined) { - throw new Error('workspace registry mutated before load completed'); - } - await this.store.save({ - workspaces: [...cache.values()], - deletedIds: [...deletedIds], - }); - } - private runExclusive(op: () => Promise): Promise { const next = this.opQueue.then(op, op); this.opQueue = next.then( @@ -262,9 +274,9 @@ function parseSessionIndexLine(line: string): SessionIndexLine | undefined { } } -function dedupeByRoot(cache: ReadonlyMap): Workspace[] { +function dedupeByRoot(byId: ReadonlyMap): Workspace[] { const byRoot = new Map(); - for (const ws of cache.values()) { + for (const ws of byId.values()) { const existing = byRoot.get(ws.root); if (existing === undefined) { byRoot.set(ws.root, ws); diff --git a/packages/agent-core-v2/test/app/workspaceRegistry/workspaceRegistryService.test.ts b/packages/agent-core-v2/test/app/workspaceRegistry/workspaceRegistryService.test.ts index 8ca191850b..1c9f8abbc3 100644 --- a/packages/agent-core-v2/test/app/workspaceRegistry/workspaceRegistryService.test.ts +++ b/packages/agent-core-v2/test/app/workspaceRegistry/workspaceRegistryService.test.ts @@ -260,6 +260,101 @@ describe('WorkspaceRegistryService (file-backed)', () => { expect((await restart().list()).map((w) => w.id)).toEqual([a.id]); }); + it('createOrTouch preserves external additions and tombstones written after load', async () => { + const dirA = join(homeDir, 'dir-a'); + const dirB = join(homeDir, 'dir-b'); + const dirC = join(homeDir, 'dir-c'); + await fsp.mkdir(dirA); + await fsp.mkdir(dirC); + const registry = build(); + await registry.createOrTouch(dirA); + + // Simulate a v1 writer touching the file after the v2 registry already + // ran an operation: a new workspace entry plus an unrelated tombstone. + const onDisk = await readWorkspacesJson(); + onDisk.workspaces[encodeWorkDirKey(dirB)] = { + root: dirB, + name: 'dir-b', + created_at: '2024-01-01T00:00:00.000Z', + last_opened_at: '2024-01-01T00:00:00.000Z', + }; + await fsp.writeFile( + join(homeDir, 'workspaces.json'), + JSON.stringify({ + version: 1, + workspaces: onDisk.workspaces, + deleted_workspace_ids: ['wd_external_tombstone'], + }), + 'utf8', + ); + + await registry.createOrTouch(dirC); + + const after = await readWorkspacesJson(); + expect(Object.keys(after.workspaces).toSorted()).toEqual( + [encodeWorkDirKey(dirA), encodeWorkDirKey(dirB), encodeWorkDirKey(dirC)].toSorted(), + ); + expect(after.deleted_workspace_ids).toEqual(['wd_external_tombstone']); + // Reads also see the external entry without a restart. + expect((await registry.list()).map((w) => w.id)).toContain(encodeWorkDirKey(dirB)); + }); + + it('delete adds its tombstone on top of the current file state', async () => { + const dirA = join(homeDir, 'dir-a'); + await fsp.mkdir(dirA); + const registry = build(); + const a = await registry.createOrTouch(dirA); + + const onDisk = await readWorkspacesJson(); + await fsp.writeFile( + join(homeDir, 'workspaces.json'), + JSON.stringify({ + version: 1, + workspaces: onDisk.workspaces, + deleted_workspace_ids: ['wd_external_tombstone'], + }), + 'utf8', + ); + + await registry.delete(a.id); + + const after = await readWorkspacesJson(); + expect(after.workspaces[a.id]).toBeUndefined(); + expect((after.deleted_workspace_ids as string[]).toSorted()).toEqual( + ['wd_external_tombstone', a.id].toSorted(), + ); + }); + + it('update renames the current file entry and misses externally removed ids', async () => { + const dirA = join(homeDir, 'dir-a'); + await fsp.mkdir(dirA); + const registry = build(); + const a = await registry.createOrTouch(dirA); + + // External rename on disk: the update must start from it, not stale state. + const onDisk = await readWorkspacesJson(); + const entry = onDisk.workspaces[a.id]; + if (entry === undefined) throw new Error('seed entry missing'); + onDisk.workspaces[a.id] = { ...entry, name: 'external-name' }; + await fsp.writeFile( + join(homeDir, 'workspaces.json'), + JSON.stringify({ version: 1, workspaces: onDisk.workspaces, deleted_workspace_ids: [] }), + 'utf8', + ); + + const renamed = await registry.update(a.id, { name: 'local-name' }); + expect(renamed?.name).toBe('local-name'); + expect(renamed?.lastOpenedAt).toBe(Date.parse(entry.last_opened_at)); + + // External removal: update reports the id as gone instead of resurrecting. + await fsp.writeFile( + join(homeDir, 'workspaces.json'), + JSON.stringify({ version: 1, workspaces: {}, deleted_workspace_ids: [] }), + 'utf8', + ); + expect(await registry.update(a.id, { name: 'whatever' })).toBeUndefined(); + }); + it('writes through on update and delete', async () => { const created = await build().createOrTouch(homeDir, 'proj'); await build().update(created.id, { name: 'renamed' }); diff --git a/packages/agent-core/src/rpc/core-impl.ts b/packages/agent-core/src/rpc/core-impl.ts index 36b67239a8..dc7a102a71 100644 --- a/packages/agent-core/src/rpc/core-impl.ts +++ b/packages/agent-core/src/rpc/core-impl.ts @@ -48,7 +48,7 @@ import { } from '../session/provider-manager'; import { SessionAPIImpl } from '../session/rpc'; import { normalizeWorkDir, SessionStore } from '../session/store/index'; -import { touchWorkspaceRegistry } from '../services/workspace/workspaceRegistryFile'; +import { touchWorkspaceRegistry } from '../session/store/workspace-registry-file'; import { noopTelemetryClient, withTelemetryContext, diff --git a/packages/agent-core/src/services/AGENTS.md b/packages/agent-core/src/services/AGENTS.md index 8f6d7c79ff..f6e9730d2d 100644 --- a/packages/agent-core/src/services/AGENTS.md +++ b/packages/agent-core/src/services/AGENTS.md @@ -96,7 +96,7 @@ no new suffixes get reintroduced. | `logger/` | `logger.ts` | (adapter lives in server) | `ILogService` | | `fileStore/` | `fileStore.ts` | `fileStoreService.ts` | `IFileStore` | | `fs/` | `fs.ts`, `fsSearch.ts`, `fsGit.ts`, `fsWatcher.ts`, `fsPathSafety.ts` | `fsService.ts`, `fsSearchService.ts`, `fsGitService.ts`, `fsWatcherService.ts` | `IFsService`, `IFsSearchService`, `IFsGitService`, `IFsWatcher` | -| `workspace/` | `workspaceRegistry.ts`, `workspaceFs.ts` | `workspaceRegistryService.ts`, `workspaceRegistryFile.ts`, `workspaceFsService.ts` | `IWorkspaceRegistry`, `IWorkspaceFsService` | +| `workspace/` | `workspaceRegistry.ts`, `workspaceFs.ts` | `workspaceRegistryService.ts`, `workspaceFsService.ts` | `IWorkspaceRegistry`, `IWorkspaceFsService` | | `config/` | `config.ts` | `configService.ts` | `IConfigService` | | `session/` | `session.ts` | `sessionService.ts` | `ISessionService` | | `message/` | `message.ts` | `messageService.ts` | `IMessageService` | diff --git a/packages/agent-core/src/services/workspace/index.ts b/packages/agent-core/src/services/workspace/index.ts index 39575e67a4..0da7cfa797 100644 --- a/packages/agent-core/src/services/workspace/index.ts +++ b/packages/agent-core/src/services/workspace/index.ts @@ -5,7 +5,6 @@ export { type WorkspacePatch, } from './workspaceRegistry'; export { WorkspaceRegistryService, detectGit } from './workspaceRegistryService'; -export { touchWorkspaceRegistry } from './workspaceRegistryFile'; export { IWorkspaceFsService, WorkspaceFsNotAbsoluteError, diff --git a/packages/agent-core/src/services/workspace/workspaceRegistryService.ts b/packages/agent-core/src/services/workspace/workspaceRegistryService.ts index 8913eb61d2..ee2fb894a7 100644 --- a/packages/agent-core/src/services/workspace/workspaceRegistryService.ts +++ b/packages/agent-core/src/services/workspace/workspaceRegistryService.ts @@ -24,7 +24,7 @@ import { writeWorkspaceRegistryFile, type WorkspaceRegistryEntry, type WorkspaceRegistryFile, -} from './workspaceRegistryFile'; +} from '../../session/store/workspace-registry-file'; type WorkspaceRegistryEvent = | { type: 'event.workspace.created'; workspace: Workspace } diff --git a/packages/agent-core/src/services/workspace/workspaceRegistryFile.ts b/packages/agent-core/src/session/store/workspace-registry-file.ts similarity index 93% rename from packages/agent-core/src/services/workspace/workspaceRegistryFile.ts rename to packages/agent-core/src/session/store/workspace-registry-file.ts index 7b7164598a..8671b1c44d 100644 --- a/packages/agent-core/src/services/workspace/workspaceRegistryFile.ts +++ b/packages/agent-core/src/session/store/workspace-registry-file.ts @@ -1,8 +1,11 @@ /** * `workspaces.json` file format and atomic access — the on-disk contract of - * the known-workspaces catalog, shared by `WorkspaceRegistryService` (which - * adds locking and events on top) and by in-process callers that only need a - * best-effort touch (e.g. `KimiCore` registering the cwd on session creation). + * the known-workspaces catalog, shared by `WorkspaceRegistryService` (the + * services-layer facade, which adds locking and events on top) and by + * in-process runtime callers that only need a best-effort touch (e.g. + * `KimiCore` registering the cwd on session creation). It lives next to + * `session-index.ts` because the runtime must not import back into + * `services/` (see `src/services/AGENTS.md`). * * The layout is the v1-compatible `{ version, workspaces, deleted_workspace_ids }` * document at `/workspaces.json`; agent-core-v2 reads and writes the @@ -13,7 +16,7 @@ import { promises as fsp } from 'node:fs'; import { dirname, join } from 'node:path'; import { basename as posixBasename } from 'pathe'; -import { encodeWorkDirKey, normalizeWorkDir } from '../../session/store'; +import { encodeWorkDirKey, normalizeWorkDir } from '#/session/store/workdir-key'; const WORKSPACE_REGISTRY_FILE = 'workspaces.json'; const WORKSPACE_REGISTRY_VERSION = 1; diff --git a/packages/agent-core/test/services/workspace-registry.test.ts b/packages/agent-core/test/services/workspace-registry.test.ts index e2d974b09f..f670d980f3 100644 --- a/packages/agent-core/test/services/workspace-registry.test.ts +++ b/packages/agent-core/test/services/workspace-registry.test.ts @@ -10,7 +10,7 @@ import type { IEnvironmentService } from '../../src/services/environment/environ import type { IEventService } from '../../src/services/event/event'; import type { ILogService } from '../../src/services/logger/logger'; import { WorkspaceRegistryService } from '../../src/services/workspace/workspaceRegistryService'; -import { touchWorkspaceRegistry } from '../../src/services/workspace/workspaceRegistryFile'; +import { touchWorkspaceRegistry } from '../../src/session/store/workspace-registry-file'; import { appendSessionIndexEntry } from '../../src/session/store/session-index'; import { encodeWorkDirKey, normalizeWorkDir } from '../../src/session/store/workdir-key';