From aebf14b0f800b32d10557b5c44656d252f142e14 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Mon, 3 Aug 2026 21:27:20 +0800 Subject: [PATCH 1/4] refactor(storage): make the project catalog part of the operational database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The project catalog decides how every session is organized, yet it stayed in its own `projects.json` when the rest of the File stores were retired. That left the only copy of every project name, relink alias and archive state outside `createOperationalStateBackup`, which captures `runtime.sqlite` plus the Artifact tree and nothing else — a restore silently dropped all of it, and no test could notice because the state was not in the database the validator checks. Schema 21 adds `projects`, `project_locations` and `project_aliases`, keyed by the random project id the renderer already contracts on, with `identity` as a unique index rather than the primary key so `data-project-id` keeps carrying an opaque id instead of a filesystem path. `session_metadata` gains a `project_id` column lifted out of `payload_json`, which makes grouping by project a SQL query for the first time; the column stays two-valued because `json_type` already distinguishes "no project" from "never resolved" for the one caller that needs it. Only persistence moved. Read-modify-write under a serial queue, whole-catalog validation on every write and each method's semantics are untouched, so all 17 catalog contract tests carry over unchanged. `projects.json` is imported once and renamed rather than deleted, so a failed upgrade stays inspectable; this is not legacy-format support but recovery of state this refactor would otherwise strand. The legacy-schema rewind fixtures are brought back in step with the migration list: #1994 left six `DROP TABLE` statements duplicated inside a single `exec`, which failed with `no such table` on the second one, and the new tables need their own teardown. Backup validation failures now carry their cause in the message — with integrity, foreign-key, ~60 required tables and Artifact reconciliation behind one error, a bare "is invalid" tells an operator nothing once `cause` is dropped from a log. --- .../agent-graph-intent-claims.test.ts | 6 +- .../agent-graph-supervisor-wakes.test.ts | 6 +- .../__tests__/sqlite-runtime-store.test.ts | 1 - .../sqlite-session-metadata-store.test.ts | 18 +- .../storage/src/operational-state-backup.ts | 16 +- packages/storage/src/project-catalog.ts | 196 ++++++++++++++++-- .../src/sqlite-session-metadata-schema.ts | 49 ++++- .../src/sqlite-session-metadata-store.ts | 33 ++- 8 files changed, 297 insertions(+), 28 deletions(-) diff --git a/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts b/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts index 227360ed62..2d79414d81 100644 --- a/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts +++ b/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts @@ -136,12 +136,16 @@ describe('SQLite agent graph intent claims', () => { const legacy = new DatabaseSync(path); legacy.exec(` - DROP TABLE session_messages; DROP INDEX session_metadata_tombstones_by_retirement_unit; ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; + DROP INDEX session_metadata_by_project; + ALTER TABLE session_metadata DROP COLUMN project_id; + DROP TABLE project_aliases; + DROP TABLE project_locations; + DROP TABLE projects; DROP TABLE session_messages; DROP TABLE agent_graph_supervisor_wake_attempts; DROP TABLE agent_graph_supervisor_wakes; diff --git a/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts b/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts index 0774a9ec4c..32c5944d24 100644 --- a/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts +++ b/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts @@ -223,12 +223,16 @@ describe('SQLite Agent Graph supervisor wakes', () => { const v11 = new DatabaseSync(path); v11.exec(` - DROP TABLE session_messages; DROP INDEX session_metadata_tombstones_by_retirement_unit; ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; + DROP INDEX session_metadata_by_project; + ALTER TABLE session_metadata DROP COLUMN project_id; + DROP TABLE project_aliases; + DROP TABLE project_locations; + DROP TABLE projects; DROP TABLE session_messages; `); v11 diff --git a/packages/storage/src/__tests__/sqlite-runtime-store.test.ts b/packages/storage/src/__tests__/sqlite-runtime-store.test.ts index ce5584dc5c..f4880a21ad 100644 --- a/packages/storage/src/__tests__/sqlite-runtime-store.test.ts +++ b/packages/storage/src/__tests__/sqlite-runtime-store.test.ts @@ -48,7 +48,6 @@ describe('SqliteRuntimeStore', () => { const legacy = new DatabaseSync(dbPath); legacy.exec(` - DROP TABLE headless_task_run_events; DROP TABLE runtime_workspace_heads; DROP TABLE runtime_workspace_versions; DROP TABLE runtime_workspace_epochs; diff --git a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts index 723ad99039..49f5ecf893 100644 --- a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts +++ b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts @@ -231,12 +231,16 @@ describe('SqliteSessionMetadataStore', () => { const v12 = new DatabaseSync(path); v12.exec(` - DROP TABLE session_messages; DROP INDEX session_metadata_tombstones_by_retirement_unit; ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; + DROP INDEX session_metadata_by_project; + ALTER TABLE session_metadata DROP COLUMN project_id; + DROP TABLE project_aliases; + DROP TABLE project_locations; + DROP TABLE projects; DROP TABLE session_messages; UPDATE session_metadata_schema SET version = 12 @@ -995,11 +999,15 @@ describe('SqliteSessionMetadataStore', () => { // Rewind to the pre-provenance shape a shipped database would have. const v13 = new DatabaseSync(path); v13.exec(` - DROP TABLE session_messages; DROP INDEX session_metadata_tombstones_by_retirement_unit; ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; + DROP INDEX session_metadata_by_project; + ALTER TABLE session_metadata DROP COLUMN project_id; + DROP TABLE project_aliases; + DROP TABLE project_locations; + DROP TABLE projects; DROP TABLE session_messages; ALTER TABLE sandbox_boundary_log DROP COLUMN turn_id; ALTER TABLE sandbox_boundary_log DROP COLUMN run_id; @@ -1504,12 +1512,16 @@ describe('SqliteSessionMetadataStore', () => { const v4 = new DatabaseSync(path); v4.exec(` - DROP TABLE session_messages; DROP INDEX session_metadata_tombstones_by_retirement_unit; ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; + DROP INDEX session_metadata_by_project; + ALTER TABLE session_metadata DROP COLUMN project_id; + DROP TABLE project_aliases; + DROP TABLE project_locations; + DROP TABLE projects; DROP TABLE session_messages; DROP TABLE agent_graph_supervisor_wake_attempts; DROP TABLE agent_graph_supervisor_wakes; diff --git a/packages/storage/src/operational-state-backup.ts b/packages/storage/src/operational-state-backup.ts index ce5c05fcf5..fb3988d184 100644 --- a/packages/storage/src/operational-state-backup.ts +++ b/packages/storage/src/operational-state-backup.ts @@ -394,6 +394,9 @@ function validateSqlite(path: string, files: readonly OperationalBackupFile[]): 'session_catalog_projection', 'session_catalog_label_projection', 'session_messages', + 'projects', + 'project_locations', + 'project_aliases', 'core_agent_runs', 'core_agent_run_events', 'core_agent_run_projections', @@ -507,9 +510,16 @@ function validateSqlite(path: string, files: readonly OperationalBackupFile[]): database.close(); } } catch (error) { - throw new OperationalBackupError('corrupt_backup', 'Backup runtime.sqlite is invalid', { - cause: error, - }); + // The reason has to reach the message: validation covers integrity, + // foreign keys, schema versions, ~60 required tables, Artifact payload + // reconciliation and message decoding, and a bare "is invalid" leaves an + // operator with no way to tell those apart in a log that dropped `cause`. + const reason = error instanceof Error ? error.message : String(error); + throw new OperationalBackupError( + 'corrupt_backup', + `Backup runtime.sqlite is invalid: ${reason}`, + { cause: error }, + ); } } diff --git a/packages/storage/src/project-catalog.ts b/packages/storage/src/project-catalog.ts index 9fe69b247f..eaf56ead9c 100644 --- a/packages/storage/src/project-catalog.ts +++ b/packages/storage/src/project-catalog.ts @@ -1,10 +1,14 @@ import { execFile } from 'node:child_process'; import { randomUUID } from 'node:crypto'; -import { mkdir, readFile, realpath, rename, stat, writeFile } from 'node:fs/promises'; +import { readFile, realpath, rename, stat } from 'node:fs/promises'; import { basename, dirname, join, normalize, resolve } from 'node:path'; import { promisify } from 'node:util'; import type { ProjectLocation, ProjectRecord } from '@maka/core'; import { hasEnclosingGitEntry } from './git-entry.js'; +import { + acquireOperationalStateDatabase, + type OperationalStateDatabaseLease, +} from './operational-state-store.js'; export type { ProjectLocation, ProjectRecord } from '@maka/core'; @@ -38,6 +42,13 @@ export interface ProjectRelinkContext { export interface ProjectCatalog { list(): Promise; register(path: string): Promise; + /** + * Resolve a path recorded by an existing session rather than chosen by the + * user. Unlike `register`, the directory may already be gone — a session + * outlives the folder it ran in — so a missing path still yields a stable + * folder identity instead of failing. + */ + resolveHistoricalPath(path: string, usedAt?: number): Promise; select(projectId: string): Promise<{ project: ProjectRecord; path: string }>; touch(projectId: string, path?: string): Promise; relink( @@ -74,20 +85,35 @@ export function createProjectCatalog( deps: { now?: () => number; createId?: () => string; + databaseLease?: OperationalStateDatabaseLease; } = {}, ): ProjectCatalog { - return new FileProjectCatalog( + return new SqliteProjectCatalog( + deps.databaseLease ?? acquireOperationalStateDatabase(storageRoot), join(storageRoot, 'projects.json'), deps.now ?? Date.now, deps.createId ?? randomUUID, ); } -class FileProjectCatalog implements ProjectCatalog { +/** + * The project catalog is operational state: it decides how every session is + * organized, and it has to survive backup and restore alongside the sessions + * it groups. Keeping it in its own JSON file left it outside the operational + * database — and therefore outside `createOperationalStateBackup`, which only + * captures `runtime.sqlite` plus the Artifact tree. + * + * Only the persistence layer moved. Read-modify-write under a serial queue, + * the whole-catalog validation on every write, and each method's semantics are + * unchanged, so the catalog contract tests carry over as-is. + */ +class SqliteProjectCatalog implements ProjectCatalog { private queue: Promise = Promise.resolve(); + private legacyImport: Promise | undefined; constructor( - private readonly path: string, + private readonly lease: OperationalStateDatabaseLease, + private readonly legacyPath: string, private readonly now: () => number, private readonly createId: () => string, ) {} @@ -111,6 +137,29 @@ class FileProjectCatalog implements ProjectCatalog { return this.upsertResolvedProject(resolved, this.now()); } + async resolveHistoricalPath(path: string, usedAt: number = this.now()): Promise { + let resolved: ResolvedProjectLocation; + try { + resolved = await resolveProjectLocation({ path }); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + let pathIsMissing = false; + try { + await stat(path); + } catch (pathError) { + pathIsMissing = (pathError as NodeJS.ErrnoException).code === 'ENOENT'; + } + if (!pathIsMissing) throw error; + const canonicalPath = normalize(resolve(path)); + resolved = { + canonicalPath, + identity: `folder:${canonicalPath}`, + kind: 'folder', + }; + } + return this.upsertResolvedProject(resolved, usedAt); + } + private async upsertResolvedProject( resolved: ResolvedProjectLocation, timestamp: number, @@ -359,22 +408,135 @@ class FileProjectCatalog implements ProjectCatalog { } private async read(): Promise { - try { - return normalizeProjectCatalogFile(JSON.parse(await readFile(this.path, 'utf8'))); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return { schemaVersion: 1, projects: [] }; - } - throw error; - } + await this.importLegacyCatalogOnce(); + return this.selectCatalog(); } private async write(file: ProjectCatalogFile): Promise { - const normalized = normalizeProjectCatalogFile(file); - await mkdir(dirname(this.path), { recursive: true }); - const tempPath = `${this.path}.${process.pid}.${Date.now()}.tmp`; - await writeFile(tempPath, `${JSON.stringify(normalized, null, 2)}\n`, 'utf8'); - await rename(tempPath, this.path); + this.replaceCatalog(normalizeProjectCatalogFile(file)); + } + + private selectCatalog(): ProjectCatalogFile { + return this.lease.transaction('read', () => { + const database = this.lease.database; + const locations = new Map(); + for (const row of database + .prepare( + `SELECT project_id, path, is_worktree, last_used_at + FROM project_locations + ORDER BY project_id, path`, + ) + .all() as Array>) { + const bucket = locations.get(row.project_id as string) ?? []; + bucket.push({ + path: row.path as string, + isWorktree: row.is_worktree === 1, + lastUsedAt: row.last_used_at as number, + }); + locations.set(row.project_id as string, bucket); + } + const aliases = new Map(); + for (const row of database + .prepare('SELECT alias, project_id FROM project_aliases ORDER BY project_id, alias') + .all() as Array>) { + const bucket = aliases.get(row.project_id as string) ?? []; + bucket.push(row.alias as string); + aliases.set(row.project_id as string, bucket); + } + const projects = ( + database + .prepare( + `SELECT project_id, identity, name, last_used_at, archived_at + FROM projects + ORDER BY project_id`, + ) + .all() as Array> + ).map((row): PersistedProject => { + const id = row.project_id as string; + const projectAliases = aliases.get(id); + return { + id, + ...(projectAliases && projectAliases.length > 0 ? { aliases: projectAliases } : {}), + name: row.name as string, + identity: row.identity as string, + locations: locations.get(id) ?? [], + lastUsedAt: row.last_used_at as number, + ...(row.archived_at === null ? {} : { archivedAt: row.archived_at as number }), + }; + }); + return { schemaVersion: 1, projects }; + }); + } + + private replaceCatalog(file: ProjectCatalogFile): void { + this.lease.transaction('write', () => { + const database = this.lease.database; + // Catalogs are small and every mutation already rewrote the whole file, + // so a full replace keeps the previous read-modify-write semantics + // exactly, now with transactional atomicity instead of temp-file rename. + database.exec('DELETE FROM project_aliases'); + database.exec('DELETE FROM project_locations'); + database.exec('DELETE FROM projects'); + const insertProject = database.prepare( + `INSERT INTO projects(project_id, identity, name, last_used_at, archived_at) + VALUES (?, ?, ?, ?, ?)`, + ); + const insertLocation = database.prepare( + `INSERT INTO project_locations(project_id, path, is_worktree, last_used_at) + VALUES (?, ?, ?, ?)`, + ); + const insertAlias = database.prepare( + 'INSERT INTO project_aliases(alias, project_id) VALUES (?, ?)', + ); + for (const project of file.projects) { + insertProject.run( + project.id, + project.identity, + project.name, + project.lastUsedAt, + project.archivedAt ?? null, + ); + for (const location of project.locations) { + insertLocation.run( + project.id, + location.path, + location.isWorktree ? 1 : 0, + location.lastUsedAt, + ); + } + for (const alias of project.aliases ?? []) insertAlias.run(alias, project.id); + } + }); + } + + /** + * `projects.json` predates the operational database and was left behind when + * the rest of the File stores were retired, so it still holds the only copy + * of every project name, relink alias and archive state. Importing it once is + * not legacy-format support: it recovers state this refactor would otherwise + * strand. The file is renamed rather than deleted so a failed upgrade stays + * inspectable, and a malformed file fails closed without touching SQLite. + */ + private importLegacyCatalogOnce(): Promise { + this.legacyImport ??= (async () => { + let raw: string; + try { + raw = await readFile(this.legacyPath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; + throw error; + } + const imported = normalizeProjectCatalogFile(JSON.parse(raw)); + const occupied = this.lease.transaction( + 'read', + () => + this.lease.database.prepare('SELECT 1 AS found FROM projects LIMIT 1').get() !== + undefined, + ); + if (!occupied) this.replaceCatalog(imported); + await rename(this.legacyPath, `${this.legacyPath}.imported`); + })(); + return this.legacyImport; } private withQueue(operation: () => Promise): Promise { diff --git a/packages/storage/src/sqlite-session-metadata-schema.ts b/packages/storage/src/sqlite-session-metadata-schema.ts index 3e3f119211..658f65458e 100644 --- a/packages/storage/src/sqlite-session-metadata-schema.ts +++ b/packages/storage/src/sqlite-session-metadata-schema.ts @@ -1,6 +1,6 @@ import type { DatabaseSync } from 'node:sqlite'; -export const SQLITE_SESSION_METADATA_SCHEMA_VERSION = 20; +export const SQLITE_SESSION_METADATA_SCHEMA_VERSION = 21; export const SQLITE_AGENT_GRAPH_CONTROL_TABLES = [ 'agent_graph_intent_claims', @@ -798,6 +798,53 @@ const MIGRATIONS: ReadonlyMap = new Map([ ON session_messages(session_id, message_ts, sequence); `, ], + [ + 21, + ` + CREATE TABLE projects ( + project_id TEXT PRIMARY KEY, + identity TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + last_used_at INTEGER NOT NULL, + archived_at INTEGER + ); + + CREATE TABLE project_locations ( + project_id TEXT NOT NULL, + path TEXT NOT NULL, + is_worktree INTEGER NOT NULL CHECK (is_worktree IN (0, 1)), + last_used_at INTEGER NOT NULL, + PRIMARY KEY(project_id, path), + FOREIGN KEY(project_id) REFERENCES projects(project_id) ON DELETE CASCADE + ); + + CREATE TABLE project_aliases ( + alias TEXT PRIMARY KEY, + project_id TEXT NOT NULL, + FOREIGN KEY(project_id) REFERENCES projects(project_id) ON DELETE CASCADE + ); + + CREATE INDEX project_aliases_by_project + ON project_aliases(project_id, alias); + + CREATE INDEX projects_by_recency + ON projects(archived_at, last_used_at DESC, project_id); + + -- Project membership was already carried inside the header payload; lifting + -- it into its own column is what makes "group by project" a SQL query + -- instead of a full-table scan decoded in JS. NULL covers both "explicitly + -- no project" and "never resolved" — the two are distinguished by + -- json_type(payload_json, '$.projectId'), which is only needed by the + -- backfill path, so the column stays free of that redundancy. + ALTER TABLE session_metadata ADD COLUMN project_id TEXT; + + UPDATE session_metadata + SET project_id = json_extract(payload_json, '$.projectId'); + + CREATE INDEX session_metadata_by_project + ON session_metadata(project_id, last_message_at DESC, session_id); + `, + ], ]); export function configureSqliteSessionMetadataDatabase(db: DatabaseSync): void { diff --git a/packages/storage/src/sqlite-session-metadata-store.ts b/packages/storage/src/sqlite-session-metadata-store.ts index 999702e18f..41b6e20e15 100644 --- a/packages/storage/src/sqlite-session-metadata-store.ts +++ b/packages/storage/src/sqlite-session-metadata-store.ts @@ -1161,6 +1161,33 @@ export class SqliteSessionMetadataStore { return rows.map(decodeRecord); } + /** + * Sessions whose project membership was never resolved. + * + * `projectId` is deliberately three-valued: a project id means resolved, + * `null` means the user chose no project, and an absent key means nobody has + * decided yet. Only the third state may be backfilled, and SQL can tell them + * apart through `json_type` — `null` reports `'null'` while an absent key + * reports SQL NULL. Scoping the query this way keeps startup proportional to + * the sessions that still need work rather than to the whole catalog. + */ + async listSessionsWithUnresolvedProject(): Promise> { + this.assertOpen(); + const rows = this.db + .prepare(` + SELECT session_id AS id, json_extract(payload_json, '$.cwd') AS cwd + FROM session_metadata + WHERE json_type(payload_json, '$.projectId') IS NULL + ORDER BY session_id + `) + .all() as Array<{ id?: unknown; cwd?: unknown }>; + return rows.flatMap((row) => + typeof row.id === 'string' && typeof row.cwd === 'string' && row.cwd.length > 0 + ? [{ id: row.id, cwd: row.cwd }] + : [], + ); + } + async listCatalogPage( filter: SessionListFilter, cursor: SessionMetadataCatalogCursor | undefined, @@ -2587,9 +2614,10 @@ export class SqliteSessionMetadataStore { backend, llm_connection_slug, model, + project_id, metadata_version, committed_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `) .run( header.id, @@ -2617,6 +2645,7 @@ export class SqliteSessionMetadataStore { header.backend, header.llmConnectionSlug, header.model, + header.projectId ?? null, metadataVersion, committedAt, ); @@ -2827,6 +2856,7 @@ export class SqliteSessionMetadataStore { backend = ?, llm_connection_slug = ?, model = ?, + project_id = ?, metadata_version = ?, committed_at = ? WHERE session_id = ? AND metadata_version = ? @@ -2849,6 +2879,7 @@ export class SqliteSessionMetadataStore { next.backend, next.llmConnectionSlug, next.model, + next.projectId ?? null, metadataVersion, committedAt, sessionId, From 13dd17cd2dc63e293fe573027dab52a371770486 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Mon, 3 Aug 2026 21:27:30 +0800 Subject: [PATCH 2/4] fix: resolve project membership for sessions that never had it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CreateSessionInput.projectId` is documented as three-valued — an id, an explicit `null` for "no project", and absent meaning "main resolves it automatically" (packages/core/src/runtime-inputs.ts:28-30). #1994 deleted the code that did that resolving along with the File stores, so the third state became permanent: any session whose project was never decided could no longer acquire one, and the sidebar's "group by project" view collapsed every such session into 未归属项目. The e2e case that renames a project failed deterministically as a result, because the ungrouped bucket has no project actions to click. Resolution is restored, scoped by SQL to the sessions that still need it instead of walking every header, and skipping the explicit `null` so a user who detached a session keeps that choice. A session whose directory is gone still resolves — a session outlives the folder it ran in — while a session that fails to resolve is left alone and retried next start rather than frozen into a wrong group. Verified against the failure this fixes: e2e/sidebar-navigation.spec.ts goes from a reproducible 30s timeout to 7/7, with the fixture untouched — the 66 seeded sessions now find their project from `cwd` on their own. --- apps/desktop/src/main/app-lifecycle.ts | 19 +++ .../project-session-backfill.test.ts | 115 ++++++++++++++++++ packages/storage/src/execution-stores.ts | 2 + packages/storage/src/index.ts | 1 + .../storage/src/project-session-backfill.ts | 44 +++++++ packages/storage/src/session-store.ts | 7 ++ 6 files changed, 188 insertions(+) create mode 100644 packages/storage/src/__tests__/project-session-backfill.test.ts create mode 100644 packages/storage/src/project-session-backfill.ts diff --git a/apps/desktop/src/main/app-lifecycle.ts b/apps/desktop/src/main/app-lifecycle.ts index 30e9134ee8..8f9c699c3d 100644 --- a/apps/desktop/src/main/app-lifecycle.ts +++ b/apps/desktop/src/main/app-lifecycle.ts @@ -11,6 +11,7 @@ import type { ShellRunProcessManager, } from '@maka/runtime'; import type { McpClientManager } from '@maka/mcp'; +import { backfillSessionProjects } from '@maka/storage'; import type { createConnectionStore, createProjectCatalog, @@ -183,6 +184,23 @@ export function wireAppLifecycle(deps: AppLifecycleDeps): void { } } + async function resolveSessionProjectsOnStartup(): Promise { + try { + const result = await backfillSessionProjects({ + sessions: sessionStore, + catalog: projectCatalog, + }); + if (result.failed > 0) { + console.error(`[projects] could not resolve ${result.failed} session project(s)`); + } + if (result.resolved > 0) emitSessionsChanged('migrated'); + } catch (error) { + // Best-effort: an unresolved project only affects sidebar grouping, and + // the sessions themselves must still reach the renderer. + console.error('[projects] session project resolution failed:', error); + } + } + async function ensureBootstrapConnection(): Promise { await mkdir(workspaceRoot, { recursive: true }); if ((await connectionStore.list()).length > 0) return; @@ -301,6 +319,7 @@ export function wireAppLifecycle(deps: AppLifecycleDeps): void { await step('keep-awake', () => keepSystemAwake.apply(resolved.system.keepSystemAwake)); } await step('usage readiness', () => ensureUsageReady()); + await step('project resolution', () => resolveSessionProjectsOnStartup()); await step('session recovery', () => recoverInterruptedSessionsOnStartup()); let botRegistryReady = false; if (settings) { diff --git a/packages/storage/src/__tests__/project-session-backfill.test.ts b/packages/storage/src/__tests__/project-session-backfill.test.ts new file mode 100644 index 0000000000..e2f2a0da9f --- /dev/null +++ b/packages/storage/src/__tests__/project-session-backfill.test.ts @@ -0,0 +1,115 @@ +import assert from 'node:assert/strict'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { createProjectCatalog } from '../project-catalog.js'; +import { backfillSessionProjects } from '../project-session-backfill.js'; +import { createSessionStore } from '../session-store.js'; + +function sessionInput(cwd: string) { + return { + cwd, + backend: 'fake' as const, + llmConnectionSlug: 'fixture', + model: 'fixture-model', + permissionMode: 'execute' as const, + }; +} + +async function withWorkspace( + run: (context: { + sessions: ReturnType; + catalog: ReturnType; + projectPath: string; + }) => Promise, +): Promise { + const root = await mkdtemp(join(tmpdir(), 'maka-project-backfill-')); + const projectPath = await mkdtemp(join(tmpdir(), 'maka-project-backfill-cwd-')); + const workspace = join(root, 'workspace'); + const sessions = createSessionStore(workspace); + const catalog = createProjectCatalog(workspace); + try { + await run({ sessions, catalog, projectPath }); + } finally { + await sessions.close?.(); + await rm(root, { recursive: true, force: true }); + await rm(projectPath, { recursive: true, force: true }); + } +} + +test('a session that never resolved a project is grouped by its working directory', async () => { + await withWorkspace(async ({ sessions, catalog, projectPath }) => { + const session = await sessions.create({ + ...sessionInput(projectPath), + name: 'session without a project', + }); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + undefined, + 'precondition: the session starts with no resolved project', + ); + + const result = await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual(result, { resolved: 1, failed: 0 }); + const projects = await catalog.list(); + assert.equal(projects.length, 1); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + projects[0]!.id, + 'the session must join the project derived from its cwd', + ); + }); +}); + +test('a session detached from every project keeps that choice', async () => { + await withWorkspace(async ({ sessions, catalog, projectPath }) => { + const session = await sessions.create({ + ...sessionInput(projectPath), + name: 'session with no project by choice', + projectId: null, + }); + + const result = await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual(result, { resolved: 0, failed: 0 }); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + null, + 'an explicit "no project" is a user decision, not missing data', + ); + assert.deepEqual(await catalog.list(), []); + }); +}); + +test('a session whose directory is gone still resolves to a stable project', async () => { + await withWorkspace(async ({ sessions, catalog, projectPath }) => { + const session = await sessions.create({ + ...sessionInput(projectPath), + name: 'session in a deleted folder', + }); + await rm(projectPath, { recursive: true, force: true }); + + const result = await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual(result, { resolved: 1, failed: 0 }); + const projects = await catalog.list(); + assert.equal(projects.length, 1); + assert.equal(projects[0]!.available, false, 'the project exists but its directory does not'); + assert.equal((await sessions.readHeaderSnapshot(session.id)).projectId, projects[0]!.id); + }); +}); + +test('backfill is idempotent and leaves resolved sessions untouched', async () => { + await withWorkspace(async ({ sessions, catalog, projectPath }) => { + await sessions.create({ ...sessionInput(projectPath), name: 'session' }); + + const first = await backfillSessionProjects({ sessions, catalog }); + const second = await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual(first, { resolved: 1, failed: 0 }); + assert.deepEqual(second, { resolved: 0, failed: 0 }, 'a second start finds nothing to do'); + assert.equal((await catalog.list()).length, 1, 'no duplicate project is created'); + }); +}); diff --git a/packages/storage/src/execution-stores.ts b/packages/storage/src/execution-stores.ts index be241a6f23..caff5b9013 100644 --- a/packages/storage/src/execution-stores.ts +++ b/packages/storage/src/execution-stores.ts @@ -339,6 +339,8 @@ async function createExecutionStoresForWrite run(() => sessionStore.listCatalogPage(filter, cursor, limit, expectedRevision)), listHeaders: () => run(() => sessionStore.listHeaders()), + listSessionsWithUnresolvedProject: () => + run(() => sessionStore.listSessionsWithUnresolvedProject()), listForRecovery: () => run(() => sessionStore.listForRecovery()), readHeaderSnapshot: (sessionId) => run(() => sessionStore.readHeaderSnapshot(sessionId)), readHeaderRecordSnapshot: (sessionId) => diff --git a/packages/storage/src/index.ts b/packages/storage/src/index.ts index 42ccd5cbf3..9f5840a024 100644 --- a/packages/storage/src/index.ts +++ b/packages/storage/src/index.ts @@ -122,6 +122,7 @@ export * from './workspace-identity.js'; export * from './memory-bundle-store.js'; export * from './long-term-memory-store.js'; export * from './project-catalog.js'; +export * from './project-session-backfill.js'; export * from './git-worktree-child-executor.js'; export * from './git-workspace-service.js'; export * from './session-bundle-policy.js'; diff --git a/packages/storage/src/project-session-backfill.ts b/packages/storage/src/project-session-backfill.ts new file mode 100644 index 0000000000..8c4f365e44 --- /dev/null +++ b/packages/storage/src/project-session-backfill.ts @@ -0,0 +1,44 @@ +import type { ProjectCatalog } from './project-catalog.js'; +import type { SessionStore } from './session-store.js'; + +export interface ProjectSessionBackfillResult { + resolved: number; + failed: number; +} + +/** + * Give every session that never had its project resolved a membership. + * + * `projectId` is three-valued — an id, an explicit `null` for "no project", or + * absent for "never decided" — and only the last state is backfilled here, so + * a user who deliberately detached a session keeps that choice. + * + * Resolution costs one `git` subprocess plus a few `realpath` calls per + * session, which is why the work is scoped to unresolved sessions by SQL + * rather than filtered in memory: the cost is paid once per session for the + * lifetime of the workspace, and later startups find nothing to do. + * + * A session whose path can no longer be resolved is left unresolved rather + * than forced into a project, so a transient failure is retried next start + * instead of being frozen into a wrong grouping. + */ +export async function backfillSessionProjects(input: { + sessions: Pick; + catalog: Pick; +}): Promise { + const pending = await input.sessions.listSessionsWithUnresolvedProject(); + let resolved = 0; + let failed = 0; + + for (const session of pending) { + try { + const project = await input.catalog.resolveHistoricalPath(session.cwd); + await input.sessions.updateHeader(session.id, { projectId: project.id }); + resolved += 1; + } catch { + failed += 1; + } + } + + return { resolved, failed }; +} diff --git a/packages/storage/src/session-store.ts b/packages/storage/src/session-store.ts index 342f2f7403..665eef1dc2 100644 --- a/packages/storage/src/session-store.ts +++ b/packages/storage/src/session-store.ts @@ -151,6 +151,8 @@ export interface SessionStore { list(filter?: SessionListFilter): Promise; /** Enumerate durable metadata without reading transcript bodies. */ listHeaders(): Promise; + /** Sessions whose project membership was never decided, with their working directory. */ + listSessionsWithUnresolvedProject(): Promise>; listForRecovery(): Promise; /** Read only the durable header without triggering connection-lock self-healing. */ readHeaderSnapshot(sessionId: string): Promise; @@ -531,6 +533,11 @@ class SqliteSessionStore implements SessionAuthorityStore { .sort((a, b) => a.id.localeCompare(b.id)); } + async listSessionsWithUnresolvedProject(): Promise> { + await this.ensureReady(); + return this.metadata.listSessionsWithUnresolvedProject(); + } + async readHeaderSnapshot(sessionId: string): Promise { return (await this.readHeaderRecordSnapshot(sessionId)).header; } From 390e61f47bdea7d3adf63b2cc1888e64715ee401 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Mon, 3 Aug 2026 22:24:16 +0800 Subject: [PATCH 3/4] fix(storage): make project resolution atomic, stable and correctly scoped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The follow-up review of the SQLite project-authority change found that the rebuilt catalog was correct in structure but wrong in three ways that only show up on a real upgrade, plus one path that never had a consumer. Backfill input. `listSessionsWithUnresolvedProject` returned only id and cwd, so every project was created with "now" as its timestamp and `preferredPath` fell out of session-id order rather than real activity. It also returned subagent sessions, whose disposable Git worktrees then became — and outranked — the user's own project locations. Both are one query: carry the session's last activity and exclude rows that have a subagent parent. Sessions are grouped by directory before resolution so an upgrade pays one Git probe per project instead of one per session. Identity. A directory that no longer exists was canonicalized with `normalize` alone, so on macOS the same folder resolved to `/var/...` after deletion and `/private/var/...` before it, splitting one project in two. The nearest surviving ancestor is now resolved with `realpath` and the missing segments re-appended. Concurrency. Every mutation rewrites the whole catalog, and the read and the write sat in separate transactions, so a second window's rename or archive was replayed away with no error. The synchronous mutations now read, change and rewrite inside one `BEGIN IMMEDIATE`; `select` and `relink` await the filesystem mid-change and re-derive their commit from the state under the lock. Dead weight. `session_metadata.project_id` and its index had no reader — three-valued membership can only be answered by `json_type` on the payload — so the column, the recency index and the unused `databaseLease` dependency are gone rather than kept as a second source of truth. Also: a failed `projects.json` import no longer takes the read path down with it, the set-aside file is timestamped so a second attempt cannot overwrite the first, the catalog releases its database lease, and project resolution runs after session recovery instead of ahead of it. Tests cover what the reviews found untested: the backup/restore round trip that is this change's whole premise, the legacy import in both its success and failure branches, identity stability across deletion, recency order, subagent exclusion, unresolvable directories, and the concurrent lost update. Each was confirmed to fail against the unfixed code. The e2e fixture resolves projects while seeding, so sidebar tests assert on a settled state instead of racing the startup resolver. --- apps/desktop/src/main/app-lifecycle.ts | 8 +- apps/desktop/src/main/boot.ts | 5 +- apps/desktop/src/main/e2e-fixture.ts | 24 ++ .../agent-graph-intent-claims.test.ts | 2 - .../agent-graph-supervisor-wakes.test.ts | 2 - .../operational-state-backup.test.ts | 32 +- .../src/__tests__/project-catalog.test.ts | 107 +++++- .../project-session-backfill.test.ts | 123 +++++- .../sqlite-session-metadata-store.test.ts | 6 - packages/storage/src/project-catalog.ts | 349 +++++++++++------- .../storage/src/project-session-backfill.ts | 58 ++- packages/storage/src/session-store.ts | 7 +- .../src/sqlite-session-metadata-schema.ts | 17 - .../src/sqlite-session-metadata-store.ts | 41 +- 14 files changed, 566 insertions(+), 215 deletions(-) diff --git a/apps/desktop/src/main/app-lifecycle.ts b/apps/desktop/src/main/app-lifecycle.ts index 8f9c699c3d..c0627b4a2c 100644 --- a/apps/desktop/src/main/app-lifecycle.ts +++ b/apps/desktop/src/main/app-lifecycle.ts @@ -190,8 +190,8 @@ export function wireAppLifecycle(deps: AppLifecycleDeps): void { sessions: sessionStore, catalog: projectCatalog, }); - if (result.failed > 0) { - console.error(`[projects] could not resolve ${result.failed} session project(s)`); + for (const failure of result.failures) { + console.error(`[projects] could not resolve ${failure.cwd}: ${failure.reason}`); } if (result.resolved > 0) emitSessionsChanged('migrated'); } catch (error) { @@ -319,8 +319,10 @@ export function wireAppLifecycle(deps: AppLifecycleDeps): void { await step('keep-awake', () => keepSystemAwake.apply(resolved.system.keepSystemAwake)); } await step('usage readiness', () => ensureUsageReady()); - await step('project resolution', () => resolveSessionProjectsOnStartup()); await step('session recovery', () => recoverInterruptedSessionsOnStartup()); + // After recovery: an interrupted session must come back before the sidebar + // learns how to group it, and resolution costs a git probe per directory. + await step('project resolution', () => resolveSessionProjectsOnStartup()); let botRegistryReady = false; if (settings) { const resolved = settings; diff --git a/apps/desktop/src/main/boot.ts b/apps/desktop/src/main/boot.ts index 4114ff7a6b..b8275cce4c 100644 --- a/apps/desktop/src/main/boot.ts +++ b/apps/desktop/src/main/boot.ts @@ -243,7 +243,10 @@ async function confirmDesktopStorageRootRepair(): Promise { const keepSystemAwake = createKeepSystemAwakeController(powerSaveBlocker); const store = createSessionStore(workspaceRoot); const agentGraphControlStore = createAgentGraphControlStore(workspaceRoot); -const projectCatalog = createProjectCatalog(workspaceRoot); +const projectCatalog = createProjectCatalog(workspaceRoot, { + onLegacyImportFailure: (error) => + console.error('[projects] projects.json could not be imported:', error), +}); const worktreeChildExecutor = createGitWorktreeChildExecutor({ storageRoot: workspaceRoot }); const planStore = createSqlitePlanStore(workspaceRoot); const executionStoreWiring = await openDesktopExecutionStoreWiring(workspaceRoot); diff --git a/apps/desktop/src/main/e2e-fixture.ts b/apps/desktop/src/main/e2e-fixture.ts index 7a280fd2fa..69b3b2e871 100644 --- a/apps/desktop/src/main/e2e-fixture.ts +++ b/apps/desktop/src/main/e2e-fixture.ts @@ -1,5 +1,10 @@ import { mkdir, rm } from 'node:fs/promises'; import type { UiLocale, E2eFixtureScenario, E2eFixtureState } from '@maka/core'; +import { + backfillSessionProjects, + createProjectCatalog, + createSessionStore, +} from '@maka/storage'; import { resolveStorageRoot } from '@maka/storage/root-authority'; import type { CredentialStore } from './credential-store.js'; import { @@ -761,4 +766,23 @@ export async function seedE2eFixture(input: { await writeSession(input.workspaceRoot, seed.header, seed.messages); } } + await seedSessionProjects(input.workspaceRoot); +} + +/** + * Resolve every seeded session's project here rather than leaving it to the + * startup backfill. The fixture is meant to hand the renderer a settled state: + * the app resolves projects in background startup, concurrently with window + * creation, so a test that asserts on project grouping would otherwise be + * racing the resolver instead of exercising the sidebar. + */ +async function seedSessionProjects(workspaceRoot: string): Promise { + const sessions = createSessionStore(workspaceRoot); + const catalog = createProjectCatalog(workspaceRoot); + try { + await backfillSessionProjects({ sessions, catalog }); + } finally { + await sessions.close?.(); + catalog.close(); + } } diff --git a/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts b/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts index 2d79414d81..2bdf59f3fb 100644 --- a/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts +++ b/packages/storage/src/__tests__/agent-graph-intent-claims.test.ts @@ -141,8 +141,6 @@ describe('SQLite agent graph intent claims', () => { ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; - DROP INDEX session_metadata_by_project; - ALTER TABLE session_metadata DROP COLUMN project_id; DROP TABLE project_aliases; DROP TABLE project_locations; DROP TABLE projects; diff --git a/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts b/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts index 32c5944d24..c43fa6edda 100644 --- a/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts +++ b/packages/storage/src/__tests__/agent-graph-supervisor-wakes.test.ts @@ -228,8 +228,6 @@ describe('SQLite Agent Graph supervisor wakes', () => { ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; - DROP INDEX session_metadata_by_project; - ALTER TABLE session_metadata DROP COLUMN project_id; DROP TABLE project_aliases; DROP TABLE project_locations; DROP TABLE projects; diff --git a/packages/storage/src/__tests__/operational-state-backup.test.ts b/packages/storage/src/__tests__/operational-state-backup.test.ts index 5734b5602e..7fb733fb07 100644 --- a/packages/storage/src/__tests__/operational-state-backup.test.ts +++ b/packages/storage/src/__tests__/operational-state-backup.test.ts @@ -1,9 +1,10 @@ import assert from 'node:assert/strict'; -import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, realpath, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test } from 'node:test'; import { createSqliteArtifactStore } from '../artifact-store.js'; +import { createProjectCatalog } from '../project-catalog.js'; import { createSessionStore } from '../session-store.js'; import { createOperationalStateBackup, @@ -17,9 +18,21 @@ test('backs up and restores runtime.sqlite plus artifact bytes', async () => { const stateRoot = join(base, 'state'); const backupRoot = join(base, 'backup'); const restoreRoot = join(base, 'restore'); + const projectPath = join(base, 'project'); + await mkdir(projectPath); const sessions = createSessionStore(stateRoot); try { + // The project catalog decides how every session is grouped, and its name, + // relink aliases and archive state exist nowhere else. Restoring sessions + // without it would silently reorganize the user's whole sidebar. + const catalog = createProjectCatalog(stateRoot, { now: () => 5 }); + const project = await catalog.register(projectPath); + await catalog.rename(project.id, 'Renamed Project'); + await catalog.archive(project.id); + catalog.close(); + const session = await sessions.create({ + projectId: project.id, cwd: '/tmp/cwd', backend: 'fake', llmConnectionSlug: 'fake', @@ -54,14 +67,31 @@ test('backs up and restores runtime.sqlite plus artifact bytes', async () => { await restoreOperationalStateBackup({ backupRoot, destinationRoot: restoreRoot }); const restored = createSessionStore(restoreRoot); + const restoredCatalog = createProjectCatalog(restoreRoot); try { assert.equal((await restored.readMessages(session.id))[0]?.id, 'message-1'); assert.equal( await readFile(join(restoreRoot, 'artifacts', artifact.relativePath), 'utf8'), 'artifact', ); + assert.equal( + (await restored.readHeaderSnapshot(session.id)).projectId, + project.id, + 'a restored session still belongs to the project it was grouped under', + ); + assert.deepEqual(await restoredCatalog.list(), [ + { + id: project.id, + name: 'Renamed Project', + locations: [{ path: await realpath(projectPath), isWorktree: false }], + archivedAt: 5, + available: true, + preferredPath: await realpath(projectPath), + }, + ]); } finally { await restored.close?.(); + restoredCatalog.close(); } } finally { await rm(base, { recursive: true, force: true }); diff --git a/packages/storage/src/__tests__/project-catalog.test.ts b/packages/storage/src/__tests__/project-catalog.test.ts index 0f9cf5fa6a..a64f723bc9 100644 --- a/packages/storage/src/__tests__/project-catalog.test.ts +++ b/packages/storage/src/__tests__/project-catalog.test.ts @@ -40,7 +40,9 @@ test('a Git probe failure cannot persistently downgrade a repository to a folder await execFileAsync('git', ['init', '--quiet'], { cwd: repository }); await assert.rejects(() => registerProjectWithoutGit(repository, storage)); - await assert.rejects(() => readFile(join(storage, 'projects.json')), { code: 'ENOENT' }); + // Nothing may be recorded: a folder identity written here would outlive the + // probe failure and permanently split the repository from its worktrees. + assert.deepEqual(await createProjectCatalog(storage).list(), []); } finally { await rm(base, { recursive: true, force: true }); } @@ -227,6 +229,34 @@ test('a missing project directory remains in the catalog as unavailable', async } }); +test('two catalogs changing one project at the same time keep both changes', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-concurrent-')); + try { + const workspace = join(base, 'workspace'); + const storage = join(base, 'storage'); + await mkdir(workspace); + const first = createProjectCatalog(storage, { now: () => 1_000 }); + const second = createProjectCatalog(storage, { now: () => 2_000 }); + const project = await first.register(workspace); + // Both catalogs settle their one-time legacy-import probe first, so the two + // mutations below really do overlap instead of queueing behind that I/O. + await Promise.all([first.list(), second.list()]); + + // Each catalog rewrites the whole table; without holding the write lock + // across its own read, the later writer replays a stale copy and the other + // window's edit disappears with no error anywhere. + await Promise.all([second.archive(project.id), first.rename(project.id, 'Renamed')]); + + const [merged] = await first.list(); + assert.equal(merged?.name, 'Renamed', 'the rename must survive the concurrent archive'); + assert.equal(merged?.archivedAt, 2_000, 'the archive must survive the concurrent rename'); + first.close(); + second.close(); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('relinking an unavailable project preserves its id and adopts the new directory', async () => { const base = await mkdtemp(join(tmpdir(), 'maka-project-relink-')); try { @@ -418,7 +448,7 @@ test('selecting a project returns its most recent available location and rejects } }); -test('a malformed project catalog fails closed without overwriting it', async () => { +test('a malformed legacy catalog is reported and preserved without blocking the catalog', async () => { const base = await mkdtemp(join(tmpdir(), 'maka-project-corrupt-')); try { const workspace = join(base, 'workspace'); @@ -428,10 +458,79 @@ test('a malformed project catalog fails closed without overwriting it', async () await mkdir(storage); const original = '{"schemaVersion":1,"projects":[{}]}\n'; await writeFile(catalogPath, original, 'utf8'); - const catalog = createProjectCatalog(storage); + const failures: unknown[] = []; + const catalog = createProjectCatalog(storage, { + onLegacyImportFailure: (error) => failures.push(error), + }); - await assert.rejects(() => catalog.register(workspace), /Invalid project catalog/); + // SQLite is the authority: a legacy file that cannot be read must not take + // the catalog down with it, and it must stay on disk to recover by hand. + const project = await catalog.register(workspace); + + assert.equal((await catalog.list()).length, 1); + assert.equal((await catalog.list())[0]?.id, project.id); assert.equal(await readFile(catalogPath, 'utf8'), original); + assert.equal(failures.length, 1); + assert.match(String(failures[0]), /Invalid project catalog/); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + +test('a legacy catalog is imported once and then set aside', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-import-')); + try { + const storage = join(base, 'storage'); + await mkdir(storage); + await writeFile( + join(storage, 'projects.json'), + JSON.stringify({ + schemaVersion: 1, + projects: [ + { + id: 'legacy-project', + aliases: ['merged-away'], + name: 'Renamed By Hand', + identity: 'folder:/gone', + locations: [{ path: '/gone', isWorktree: false, lastUsedAt: 5 }], + lastUsedAt: 7, + archivedAt: 9, + }, + ], + }), + 'utf8', + ); + const failures: unknown[] = []; + const catalog = createProjectCatalog(storage, { + now: () => 1_000, + onLegacyImportFailure: (error) => failures.push(error), + }); + + const projects = await catalog.list(); + + assert.deepEqual(failures, []); + // The user's name, relink aliases and archive state only ever lived in this + // file; losing them on upgrade would be indistinguishable from data loss. + assert.equal(projects.length, 1); + assert.equal(projects[0]?.id, 'legacy-project'); + assert.equal(projects[0]?.name, 'Renamed By Hand'); + assert.deepEqual(projects[0]?.aliases, ['merged-away']); + assert.equal(projects[0]?.archivedAt, 9); + await assert.rejects(() => readFile(join(storage, 'projects.json'), 'utf8'), { + code: 'ENOENT', + }); + const setAside = JSON.parse( + await readFile(join(storage, 'projects.json.imported-1000'), 'utf8'), + ) as { projects: Array<{ id: string }> }; + assert.deepEqual( + setAside.projects.map((project) => project.id), + ['legacy-project'], + 'the imported file is kept verbatim so a bad upgrade stays recoverable', + ); + + // A catalog opened later must not re-import and must not lose the state. + catalog.close(); + assert.equal((await createProjectCatalog(storage).list()).length, 1); } finally { await rm(base, { recursive: true, force: true }); } diff --git a/packages/storage/src/__tests__/project-session-backfill.test.ts b/packages/storage/src/__tests__/project-session-backfill.test.ts index e2f2a0da9f..77e4ef2cc2 100644 --- a/packages/storage/src/__tests__/project-session-backfill.test.ts +++ b/packages/storage/src/__tests__/project-session-backfill.test.ts @@ -1,12 +1,16 @@ import assert from 'node:assert/strict'; -import { mkdtemp, rm } from 'node:fs/promises'; +import { execFile } from 'node:child_process'; +import { mkdir, mkdtemp, realpath, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test } from 'node:test'; +import { promisify } from 'node:util'; import { createProjectCatalog } from '../project-catalog.js'; import { backfillSessionProjects } from '../project-session-backfill.js'; import { createSessionStore } from '../session-store.js'; +const execFileAsync = promisify(execFile); + function sessionInput(cwd: string) { return { cwd, @@ -22,6 +26,7 @@ async function withWorkspace( sessions: ReturnType; catalog: ReturnType; projectPath: string; + base: string; }) => Promise, ): Promise { const root = await mkdtemp(join(tmpdir(), 'maka-project-backfill-')); @@ -30,9 +35,10 @@ async function withWorkspace( const sessions = createSessionStore(workspace); const catalog = createProjectCatalog(workspace); try { - await run({ sessions, catalog, projectPath }); + await run({ sessions, catalog, projectPath, base: root }); } finally { await sessions.close?.(); + catalog.close(); await rm(root, { recursive: true, force: true }); await rm(projectPath, { recursive: true, force: true }); } @@ -52,7 +58,7 @@ test('a session that never resolved a project is grouped by its working director const result = await backfillSessionProjects({ sessions, catalog }); - assert.deepEqual(result, { resolved: 1, failed: 0 }); + assert.deepEqual(result, { resolved: 1, failures: [] }); const projects = await catalog.list(); assert.equal(projects.length, 1); assert.equal( @@ -73,7 +79,7 @@ test('a session detached from every project keeps that choice', async () => { const result = await backfillSessionProjects({ sessions, catalog }); - assert.deepEqual(result, { resolved: 0, failed: 0 }); + assert.deepEqual(result, { resolved: 0, failures: [] }); assert.equal( (await sessions.readHeaderSnapshot(session.id)).projectId, null, @@ -83,8 +89,9 @@ test('a session detached from every project keeps that choice', async () => { }); }); -test('a session whose directory is gone still resolves to a stable project', async () => { +test('a deleted directory resolves to the same project it had while it existed', async () => { await withWorkspace(async ({ sessions, catalog, projectPath }) => { + const before = await catalog.register(projectPath); const session = await sessions.create({ ...sessionInput(projectPath), name: 'session in a deleted folder', @@ -93,11 +100,14 @@ test('a session whose directory is gone still resolves to a stable project', asy const result = await backfillSessionProjects({ sessions, catalog }); - assert.deepEqual(result, { resolved: 1, failed: 0 }); + assert.deepEqual(result, { resolved: 1, failures: [] }); const projects = await catalog.list(); - assert.equal(projects.length, 1); + // On macOS the temp root is a symlink, so a path canonicalized without + // realpath would mint a second project for the very same directory. + assert.equal(projects.length, 1, 'the vanished directory must not mint a second project'); + assert.equal(projects[0]!.id, before.id); assert.equal(projects[0]!.available, false, 'the project exists but its directory does not'); - assert.equal((await sessions.readHeaderSnapshot(session.id)).projectId, projects[0]!.id); + assert.equal((await sessions.readHeaderSnapshot(session.id)).projectId, before.id); }); }); @@ -108,8 +118,101 @@ test('backfill is idempotent and leaves resolved sessions untouched', async () = const first = await backfillSessionProjects({ sessions, catalog }); const second = await backfillSessionProjects({ sessions, catalog }); - assert.deepEqual(first, { resolved: 1, failed: 0 }); - assert.deepEqual(second, { resolved: 0, failed: 0 }, 'a second start finds nothing to do'); + assert.deepEqual(first, { resolved: 1, failures: [] }); + assert.deepEqual(second, { resolved: 0, failures: [] }, 'a second start finds nothing to do'); assert.equal((await catalog.list()).length, 1, 'no duplicate project is created'); }); }); + +test('projects are ordered by when their sessions were last active, not by upgrade order', async () => { + await withWorkspace(async ({ sessions, catalog, base }) => { + const older = join(base, 'older-project'); + const newer = join(base, 'newer-project'); + await mkdir(older); + await mkdir(newer); + // Names chosen so session-id order cannot accidentally produce the + // expected result: recency has to come from the sessions' own timestamps. + const olderSession = await sessions.create({ ...sessionInput(older), name: 'older' }); + const newerSession = await sessions.create({ ...sessionInput(newer), name: 'newer' }); + await sessions.updateHeader(olderSession.id, { lastMessageAt: 1_000 }); + await sessions.updateHeader(newerSession.id, { lastMessageAt: 9_000 }); + + await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual( + (await catalog.list()).map((project) => project.name), + ['newer-project', 'older-project'], + 'an upgrade must rebuild the real recency order, not flatten it to "now"', + ); + }); +}); + +test('a subagent worktree never becomes one of the user project locations', async () => { + await withWorkspace(async ({ sessions, catalog, base }) => { + const repository = join(base, 'repository'); + const worktree = join(base, 'subagent-worktree'); + await mkdir(repository); + await execFileAsync('git', ['init', '--quiet', '-b', 'main'], { cwd: repository }); + await execFileAsync('git', ['commit', '--quiet', '--allow-empty', '-m', 'root'], { + cwd: repository, + env: { + ...process.env, + GIT_AUTHOR_NAME: 'maka', + GIT_AUTHOR_EMAIL: 'maka@example.com', + GIT_COMMITTER_NAME: 'maka', + GIT_COMMITTER_EMAIL: 'maka@example.com', + }, + }); + await execFileAsync('git', ['worktree', 'add', '--quiet', worktree, '-b', 'child'], { + cwd: repository, + }); + const parent = await sessions.create({ ...sessionInput(repository), name: 'parent' }); + await sessions.create({ + ...sessionInput(worktree), + name: 'subagent', + subagentParent: { + kind: 'subagent' as const, + lifecycle: 'foreground' as const, + parentSessionId: parent.id, + spawnedBy: { parentRunId: 'run-1', parentTurnId: 'turn-1', toolCallId: 'call-1' }, + }, + }); + + const result = await backfillSessionProjects({ sessions, catalog }); + + // The child inherits its parent's project when spawned; its scratch + // worktree is disposable and must never outrank the real checkout. + assert.deepEqual(result, { resolved: 1, failures: [] }); + const projects = await catalog.list(); + assert.equal(projects.length, 1); + assert.deepEqual( + projects[0]!.locations.map((location) => location.path), + [await realpath(repository)], + 'a subagent scratch worktree is not a place the user works', + ); + assert.equal(projects[0]!.preferredPath, await realpath(repository)); + }); +}); + +test('a working directory that cannot be resolved is reported instead of guessed', async () => { + await withWorkspace(async ({ sessions, projectPath }) => { + const session = await sessions.create({ ...sessionInput(projectPath), name: 'session' }); + + const result = await backfillSessionProjects({ + sessions, + catalog: { + resolveHistoricalPath: () => Promise.reject(new Error('git exploded')), + }, + }); + + assert.deepEqual(result, { + resolved: 0, + failures: [{ cwd: projectPath, reason: 'git exploded' }], + }); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + undefined, + 'an unresolved session stays unresolved so the next start can retry', + ); + }); +}); diff --git a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts index 49f5ecf893..5f5938badf 100644 --- a/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts +++ b/packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts @@ -236,8 +236,6 @@ describe('SqliteSessionMetadataStore', () => { ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; - DROP INDEX session_metadata_by_project; - ALTER TABLE session_metadata DROP COLUMN project_id; DROP TABLE project_aliases; DROP TABLE project_locations; DROP TABLE projects; @@ -1003,8 +1001,6 @@ describe('SqliteSessionMetadataStore', () => { ALTER TABLE session_metadata_tombstones DROP COLUMN cleanup_pending; ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; - DROP INDEX session_metadata_by_project; - ALTER TABLE session_metadata DROP COLUMN project_id; DROP TABLE project_aliases; DROP TABLE project_locations; DROP TABLE projects; @@ -1517,8 +1513,6 @@ describe('SqliteSessionMetadataStore', () => { ALTER TABLE session_metadata_tombstones DROP COLUMN retirement_unit_id; DROP TABLE session_create_claims; DROP TABLE sandbox_boundary_log; - DROP INDEX session_metadata_by_project; - ALTER TABLE session_metadata DROP COLUMN project_id; DROP TABLE project_aliases; DROP TABLE project_locations; DROP TABLE projects; diff --git a/packages/storage/src/project-catalog.ts b/packages/storage/src/project-catalog.ts index eaf56ead9c..30b1049d35 100644 --- a/packages/storage/src/project-catalog.ts +++ b/packages/storage/src/project-catalog.ts @@ -59,6 +59,8 @@ export interface ProjectCatalog { rename(projectId: string, name: string): Promise; archive(projectId: string): Promise; restore(projectId: string): Promise; + /** Release this catalog's share of the operational database. */ + close(): void; } interface PersistedProject { @@ -85,14 +87,16 @@ export function createProjectCatalog( deps: { now?: () => number; createId?: () => string; - databaseLease?: OperationalStateDatabaseLease; + /** Report a `projects.json` that could not be imported; the catalog still opens. */ + onLegacyImportFailure?: (error: unknown) => void; } = {}, ): ProjectCatalog { return new SqliteProjectCatalog( - deps.databaseLease ?? acquireOperationalStateDatabase(storageRoot), + acquireOperationalStateDatabase(storageRoot), join(storageRoot, 'projects.json'), deps.now ?? Date.now, deps.createId ?? randomUUID, + deps.onLegacyImportFailure ?? (() => {}), ); } @@ -116,8 +120,13 @@ class SqliteProjectCatalog implements ProjectCatalog { private readonly legacyPath: string, private readonly now: () => number, private readonly createId: () => string, + private readonly onLegacyImportFailure: (error: unknown) => void, ) {} + close(): void { + this.lease.close(); + } + async list(): Promise { let projects: PersistedProject[] = []; await this.withQueue(async () => { @@ -150,7 +159,7 @@ class SqliteProjectCatalog implements ProjectCatalog { pathIsMissing = (pathError as NodeJS.ErrnoException).code === 'ENOENT'; } if (!pathIsMissing) throw error; - const canonicalPath = normalize(resolve(path)); + const canonicalPath = await canonicalizeMissingPath(path); resolved = { canonicalPath, identity: `folder:${canonicalPath}`, @@ -166,25 +175,25 @@ class SqliteProjectCatalog implements ProjectCatalog { ): Promise { let registered: PersistedProject | undefined; await this.withQueue(async () => { - const file = await this.read(); - const locationPath = - resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; - const existing = file.projects.find((project) => project.identity === resolved.identity); - if (existing) { - const location = existing.locations.find((item) => item.path === locationPath); - if (location) { - location.lastUsedAt = Math.max(location.lastUsedAt, timestamp); - location.isWorktree = resolved.git?.isWorktree ?? false; - } else { - existing.locations.push({ - path: locationPath, - isWorktree: resolved.git?.isWorktree ?? false, - lastUsedAt: timestamp, - }); + registered = await this.mutate((file) => { + const locationPath = + resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; + const existing = file.projects.find((project) => project.identity === resolved.identity); + if (existing) { + const location = existing.locations.find((item) => item.path === locationPath); + if (location) { + location.lastUsedAt = Math.max(location.lastUsedAt, timestamp); + location.isWorktree = resolved.git?.isWorktree ?? false; + } else { + existing.locations.push({ + path: locationPath, + isWorktree: resolved.git?.isWorktree ?? false, + lastUsedAt: timestamp, + }); + } + existing.lastUsedAt = Math.max(existing.lastUsedAt, timestamp); + return existing; } - existing.lastUsedAt = Math.max(existing.lastUsedAt, timestamp); - registered = existing; - } else { const project: PersistedProject = { id: this.createId(), name: defaultProjectName(resolved), @@ -199,9 +208,8 @@ class SqliteProjectCatalog implements ProjectCatalog { lastUsedAt: timestamp, }; file.projects.push(project); - registered = project; - } - await this.write(file); + return project; + }); }); if (!registered) { throw new Error(`Failed to register project: ${resolved.canonicalPath}`); @@ -213,34 +221,34 @@ class SqliteProjectCatalog implements ProjectCatalog { let selected: PersistedProject | undefined; let selectedPath: string | undefined; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - if (project.archivedAt !== undefined) { - throw new Error(`Project is archived: ${projectId}`); - } - const availableLocations = ( - await Promise.all( - project.locations.map(async (location) => ({ - location, - available: await isDirectory(location.path), - })), - ) - ) - .filter((entry) => entry.available) - .sort( - (a, b) => - b.location.lastUsedAt - a.location.lastUsedAt || - a.location.path.localeCompare(b.location.path), - ); - const location = availableLocations[0]?.location; - if (!location) throw new Error(`Project is unavailable: ${projectId}`); - const timestamp = this.now(); - location.lastUsedAt = timestamp; - project.lastUsedAt = timestamp; - selected = project; - selectedPath = location.path; - await this.write(file); + // Probing the filesystem cannot happen inside the write transaction, so + // availability is decided first and the choice is re-validated under it. + const existing = findProjectById((await this.read()).projects, projectId); + if (!existing) throw new Error(`No such project: ${projectId}`); + const availablePaths = new Set( + ( + await Promise.all( + existing.locations.map(async (location) => + (await isDirectory(location.path)) ? location.path : undefined, + ), + ) + ).filter((path): path is string => path !== undefined), + ); + [selected, selectedPath] = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + if (project.archivedAt !== undefined) { + throw new Error(`Project is archived: ${projectId}`); + } + const location = project.locations + .filter((item) => availablePaths.has(item.path)) + .sort((a, b) => b.lastUsedAt - a.lastUsedAt || a.path.localeCompare(b.path))[0]; + if (!location) throw new Error(`Project is unavailable: ${projectId}`); + const timestamp = this.now(); + location.lastUsedAt = timestamp; + project.lastUsedAt = timestamp; + return [project, location.path] as const; + }); }); if (!selected || !selectedPath) throw new Error(`Failed to select project: ${projectId}`); return { project: await this.present(selected), path: selectedPath }; @@ -255,22 +263,22 @@ class SqliteProjectCatalog implements ProjectCatalog { : undefined; let touched: PersistedProject | undefined; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - const location = resolvedPath - ? project.locations.find((item) => item.path === resolvedPath) - : [...project.locations].sort( - (a, b) => b.lastUsedAt - a.lastUsedAt || a.path.localeCompare(b.path), - )[0]; - if (resolvedPath && !location) { - throw new ProjectPathMismatchError(projectId, resolvedPath); - } - const timestamp = this.now(); - if (location) location.lastUsedAt = timestamp; - project.lastUsedAt = timestamp; - touched = project; - await this.write(file); + touched = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + const location = resolvedPath + ? project.locations.find((item) => item.path === resolvedPath) + : [...project.locations].sort( + (a, b) => b.lastUsedAt - a.lastUsedAt || a.path.localeCompare(b.path), + )[0]; + if (resolvedPath && !location) { + throw new ProjectPathMismatchError(projectId, resolvedPath); + } + const timestamp = this.now(); + if (location) location.lastUsedAt = timestamp; + project.lastUsedAt = timestamp; + return project; + }); }); if (!touched) throw new Error(`Failed to touch project: ${projectId}`); return this.present(touched); @@ -284,50 +292,62 @@ class SqliteProjectCatalog implements ProjectCatalog { const resolved = await resolveProjectLocation({ path }); const timestamp = this.now(); let relinked: PersistedProject | undefined; + const locationPath = + resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - const conflict = file.projects.find( - (item) => item.id !== project.id && item.identity === resolved.identity, + // `beforeCommit` moves real directories, so it cannot run inside the + // write transaction. It is given the state it will act on, and the + // commit below re-derives everything from the catalog as it stands then. + const preview = await this.read(); + const previewProject = findProjectById(preview.projects, projectId); + if (!previewProject) throw new Error(`No such project: ${projectId}`); + const previewConflict = preview.projects.find( + (item) => item.id !== previewProject.id && item.identity === resolved.identity, ); - if (conflict && !beforeCommit) { - throw new Error(`Project path already belongs to project: ${conflict.id}`); + if (previewConflict && !beforeCommit) { + throw new Error(`Project path already belongs to project: ${previewConflict.id}`); } - const locationPath = - resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; await beforeCommit?.({ - projectId: project.id, - projectAliases: [...(project.aliases ?? [])], + projectId: previewProject.id, + projectAliases: [...(previewProject.aliases ?? [])], destinationPath: locationPath, - previousLocations: project.locations.map((location) => ({ ...location })), - ...(conflict + previousLocations: previewProject.locations.map((location) => ({ ...location })), + ...(previewConflict ? { - conflictingProjectId: conflict.id, - conflictingProjectAliases: [...(conflict.aliases ?? [])], + conflictingProjectId: previewConflict.id, + conflictingProjectAliases: [...(previewConflict.aliases ?? [])], } : {}), }); - if (conflict) { - project.aliases = [ - ...new Set([...(project.aliases ?? []), conflict.id, ...(conflict.aliases ?? [])]), + relinked = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + const conflict = file.projects.find( + (item) => item.id !== project.id && item.identity === resolved.identity, + ); + if (conflict && !beforeCommit) { + throw new Error(`Project path already belongs to project: ${conflict.id}`); + } + if (conflict) { + project.aliases = [ + ...new Set([...(project.aliases ?? []), conflict.id, ...(conflict.aliases ?? [])]), + ]; + file.projects = file.projects.filter((item) => item.id !== conflict.id); + } + project.identity = resolved.identity; + project.locations = [ + { + path: locationPath, + isWorktree: resolved.git?.isWorktree ?? false, + lastUsedAt: timestamp, + }, + ...(conflict?.locations + .filter((location) => location.path !== locationPath) + .map((location) => ({ ...location })) ?? []), ]; - file.projects = file.projects.filter((item) => item.id !== conflict.id); - } - project.identity = resolved.identity; - project.locations = [ - { - path: locationPath, - isWorktree: resolved.git?.isWorktree ?? false, - lastUsedAt: timestamp, - }, - ...(conflict?.locations - .filter((location) => location.path !== locationPath) - .map((location) => ({ ...location })) ?? []), - ]; - project.lastUsedAt = Math.max(timestamp, conflict?.lastUsedAt ?? 0); - relinked = project; - await this.write(file); + project.lastUsedAt = Math.max(timestamp, conflict?.lastUsedAt ?? 0); + return project; + }); }); if (!relinked) throw new Error(`Failed to relink project: ${projectId}`); return this.present(relinked); @@ -338,12 +358,12 @@ class SqliteProjectCatalog implements ProjectCatalog { if (!trimmed) throw new TypeError('Project name cannot be empty.'); let renamed: PersistedProject | undefined; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - project.name = trimmed; - renamed = project; - await this.write(file); + renamed = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + project.name = trimmed; + return project; + }); }); if (!renamed) throw new Error(`Failed to rename project: ${projectId}`); return this.present(renamed); @@ -352,13 +372,12 @@ class SqliteProjectCatalog implements ProjectCatalog { async archive(projectId: string): Promise { let archived: PersistedProject | undefined; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - const timestamp = this.now(); - project.archivedAt = timestamp; - archived = project; - await this.write(file); + archived = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + project.archivedAt = this.now(); + return project; + }); }); if (!archived) throw new Error(`Failed to archive project: ${projectId}`); return this.present(archived); @@ -367,12 +386,12 @@ class SqliteProjectCatalog implements ProjectCatalog { async restore(projectId: string): Promise { let restored: PersistedProject | undefined; await this.withQueue(async () => { - const file = await this.read(); - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - delete project.archivedAt; - restored = project; - await this.write(file); + restored = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + delete project.archivedAt; + return project; + }); }); if (!restored) throw new Error(`Failed to restore project: ${projectId}`); return this.present(restored); @@ -412,8 +431,26 @@ class SqliteProjectCatalog implements ProjectCatalog { return this.selectCatalog(); } - private async write(file: ProjectCatalogFile): Promise { - this.replaceCatalog(normalizeProjectCatalogFile(file)); + /** + * Read, change and rewrite the catalog inside one `BEGIN IMMEDIATE`. + * + * The serial queue only orders callers within a process, so a second Maka + * window reading the catalog between this one's read and write would rewrite + * the whole table from its own stale copy and silently discard the change — + * a rename or archive would simply revert. Holding SQLite's write lock across + * the whole read-modify-write makes the loser wait rather than clobber. + * + * Only mutations that are entirely synchronous can run here; `select` and + * `relink` await the filesystem mid-change and keep the two-phase form. + */ + private async mutate(change: (file: ProjectCatalogFile) => T): Promise { + await this.importLegacyCatalogOnce(); + return this.lease.transaction('write', () => { + const file = this.selectCatalog(); + const result = change(file); + this.replaceCatalog(normalizeProjectCatalogFile(file)); + return result; + }); } private selectCatalog(): ProjectCatalogFile { @@ -515,26 +552,37 @@ class SqliteProjectCatalog implements ProjectCatalog { * of every project name, relink alias and archive state. Importing it once is * not legacy-format support: it recovers state this refactor would otherwise * strand. The file is renamed rather than deleted so a failed upgrade stays - * inspectable, and a malformed file fails closed without touching SQLite. + * inspectable, and a malformed file leaves SQLite untouched. + * + * An import that cannot complete — unreadable file, malformed contents, a + * read-only disk — is reported and then dropped rather than rethrown. SQLite + * is the authority now, so failing the import must not take the read path + * down with it; `projects.json` is still on disk to recover from by hand. */ private importLegacyCatalogOnce(): Promise { this.legacyImport ??= (async () => { - let raw: string; try { - raw = await readFile(this.legacyPath, 'utf8'); + let raw: string; + try { + raw = await readFile(this.legacyPath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; + throw error; + } + const imported = normalizeProjectCatalogFile(JSON.parse(raw)); + const occupied = this.lease.transaction( + 'read', + () => + this.lease.database.prepare('SELECT 1 AS found FROM projects LIMIT 1').get() !== + undefined, + ); + if (!occupied) this.replaceCatalog(imported); + // Timestamped so a second upgrade attempt cannot overwrite the only + // remaining copy of a catalog that failed to import the first time. + await rename(this.legacyPath, `${this.legacyPath}.imported-${this.now()}`); } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; - throw error; + this.onLegacyImportFailure(error); } - const imported = normalizeProjectCatalogFile(JSON.parse(raw)); - const occupied = this.lease.transaction( - 'read', - () => - this.lease.database.prepare('SELECT 1 AS found FROM projects LIMIT 1').get() !== - undefined, - ); - if (!occupied) this.replaceCatalog(imported); - await rename(this.legacyPath, `${this.legacyPath}.imported`); })(); return this.legacyImport; } @@ -648,6 +696,33 @@ function isTimestamp(value: unknown): value is number { return typeof value === 'number' && Number.isFinite(value) && value >= 0; } +/** + * Canonicalize a path that no longer exists. + * + * `realpath` fails outright on a missing path, but its job — resolving symlinks + * — still matters here: on macOS `/tmp` is a link to `/private/tmp`, so naive + * normalization gives a deleted directory a different identity than the same + * directory had while it existed, splitting one project in two. Resolving the + * nearest surviving ancestor and re-appending the missing segments keeps the + * identity stable across the moment the directory disappears. + */ +async function canonicalizeMissingPath(path: string): Promise { + const absolute = normalize(resolve(path)); + const missingSegments: string[] = []; + let candidate = absolute; + for (;;) { + try { + return normalize(join(await realpath(candidate), ...missingSegments)); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + const parent = dirname(candidate); + if (parent === candidate) return absolute; + missingSegments.unshift(basename(candidate)); + candidate = parent; + } +} + async function isDirectory(path: string): Promise { try { return (await stat(path)).isDirectory(); diff --git a/packages/storage/src/project-session-backfill.ts b/packages/storage/src/project-session-backfill.ts index 8c4f365e44..e06c37f5f7 100644 --- a/packages/storage/src/project-session-backfill.ts +++ b/packages/storage/src/project-session-backfill.ts @@ -3,7 +3,7 @@ import type { SessionStore } from './session-store.js'; export interface ProjectSessionBackfillResult { resolved: number; - failed: number; + failures: Array<{ cwd: string; reason: string }>; } /** @@ -13,32 +13,54 @@ export interface ProjectSessionBackfillResult { * absent for "never decided" — and only the last state is backfilled here, so * a user who deliberately detached a session keeps that choice. * - * Resolution costs one `git` subprocess plus a few `realpath` calls per - * session, which is why the work is scoped to unresolved sessions by SQL - * rather than filtered in memory: the cost is paid once per session for the - * lifetime of the workspace, and later startups find nothing to do. - * - * A session whose path can no longer be resolved is left unresolved rather - * than forced into a project, so a transient failure is retried next start - * instead of being frozen into a wrong grouping. + * Sessions are grouped by working directory before resolution: an upgrade + * typically holds many sessions per project, and resolution costs a `git` + * subprocess plus a catalog write each time. Each directory carries the latest + * activity of the sessions that share it, so the rebuilt catalog keeps its real + * recency order rather than collapsing every project to the upgrade's timestamp. */ export async function backfillSessionProjects(input: { - sessions: Pick; + sessions: Pick< + SessionStore, + 'listSessionsWithUnresolvedProject' | 'updateHeader' | 'readHeaderSnapshot' + >; catalog: Pick; }): Promise { const pending = await input.sessions.listSessionsWithUnresolvedProject(); + const byDirectory = new Map(); + for (const session of pending) { + const group = byDirectory.get(session.cwd); + if (group) { + group.usedAt = Math.max(group.usedAt, session.usedAt); + group.sessionIds.push(session.id); + } else { + byDirectory.set(session.cwd, { usedAt: session.usedAt, sessionIds: [session.id] }); + } + } + let resolved = 0; - let failed = 0; + const failures: Array<{ cwd: string; reason: string }> = []; - for (const session of pending) { + for (const [cwd, group] of byDirectory) { + let projectId: string; try { - const project = await input.catalog.resolveHistoricalPath(session.cwd); - await input.sessions.updateHeader(session.id, { projectId: project.id }); - resolved += 1; - } catch { - failed += 1; + projectId = (await input.catalog.resolveHistoricalPath(cwd, group.usedAt)).id; + } catch (error) { + failures.push({ cwd, reason: error instanceof Error ? error.message : String(error) }); + continue; + } + for (const sessionId of group.sessionIds) { + try { + // Re-check instead of writing blind: the user can detach a session + // while this runs, and that decision must win over a stale plan. + if ((await input.sessions.readHeaderSnapshot(sessionId)).projectId !== undefined) continue; + await input.sessions.updateHeader(sessionId, { projectId }); + resolved += 1; + } catch (error) { + failures.push({ cwd, reason: error instanceof Error ? error.message : String(error) }); + } } } - return { resolved, failed }; + return { resolved, failures }; } diff --git a/packages/storage/src/session-store.ts b/packages/storage/src/session-store.ts index 665eef1dc2..cd6e7eaf1b 100644 --- a/packages/storage/src/session-store.ts +++ b/packages/storage/src/session-store.ts @@ -9,6 +9,7 @@ import { SessionMetadataVersionConflictError, type SqliteSessionMetadataStore, type StableSessionCreateProbe, + type UnresolvedProjectSession, type VersionedSessionIdentity, } from './sqlite-session-metadata-store.js'; import { isDiscardableConversationCopy } from './session-conversation-copy.js'; @@ -151,8 +152,8 @@ export interface SessionStore { list(filter?: SessionListFilter): Promise; /** Enumerate durable metadata without reading transcript bodies. */ listHeaders(): Promise; - /** Sessions whose project membership was never decided, with their working directory. */ - listSessionsWithUnresolvedProject(): Promise>; + /** Sessions whose project membership was never decided, newest activity last. */ + listSessionsWithUnresolvedProject(): Promise; listForRecovery(): Promise; /** Read only the durable header without triggering connection-lock self-healing. */ readHeaderSnapshot(sessionId: string): Promise; @@ -533,7 +534,7 @@ class SqliteSessionStore implements SessionAuthorityStore { .sort((a, b) => a.id.localeCompare(b.id)); } - async listSessionsWithUnresolvedProject(): Promise> { + async listSessionsWithUnresolvedProject(): Promise { await this.ensureReady(); return this.metadata.listSessionsWithUnresolvedProject(); } diff --git a/packages/storage/src/sqlite-session-metadata-schema.ts b/packages/storage/src/sqlite-session-metadata-schema.ts index 658f65458e..a9b9282bcf 100644 --- a/packages/storage/src/sqlite-session-metadata-schema.ts +++ b/packages/storage/src/sqlite-session-metadata-schema.ts @@ -826,23 +826,6 @@ const MIGRATIONS: ReadonlyMap = new Map([ CREATE INDEX project_aliases_by_project ON project_aliases(project_id, alias); - - CREATE INDEX projects_by_recency - ON projects(archived_at, last_used_at DESC, project_id); - - -- Project membership was already carried inside the header payload; lifting - -- it into its own column is what makes "group by project" a SQL query - -- instead of a full-table scan decoded in JS. NULL covers both "explicitly - -- no project" and "never resolved" — the two are distinguished by - -- json_type(payload_json, '$.projectId'), which is only needed by the - -- backfill path, so the column stays free of that redundancy. - ALTER TABLE session_metadata ADD COLUMN project_id TEXT; - - UPDATE session_metadata - SET project_id = json_extract(payload_json, '$.projectId'); - - CREATE INDEX session_metadata_by_project - ON session_metadata(project_id, last_message_at DESC, session_id); `, ], ]); diff --git a/packages/storage/src/sqlite-session-metadata-store.ts b/packages/storage/src/sqlite-session-metadata-store.ts index 41b6e20e15..b5242fa848 100644 --- a/packages/storage/src/sqlite-session-metadata-store.ts +++ b/packages/storage/src/sqlite-session-metadata-store.ts @@ -162,6 +162,17 @@ export interface VersionedSessionIdentity { readonly expectedVersion: number; } +/** + * A session whose project membership was never decided, carried together with + * the moment it was last active so that resolving it later reconstructs the + * catalog's real recency order instead of collapsing every project to "now". + */ +export interface UnresolvedProjectSession { + readonly id: string; + readonly cwd: string; + readonly usedAt: number; +} + export type SessionRemovalProbe = | { readonly kind: 'present'; readonly record: SessionMetadataRecord } | { readonly kind: 'removed' } @@ -1171,19 +1182,31 @@ export class SqliteSessionMetadataStore { * reports SQL NULL. Scoping the query this way keeps startup proportional to * the sessions that still need work rather than to the whole catalog. */ - async listSessionsWithUnresolvedProject(): Promise> { + async listSessionsWithUnresolvedProject(): Promise { this.assertOpen(); + // `json_type` distinguishes an absent `projectId` (never decided) from an + // explicit JSON `null` (detached on purpose); only the former is pending. + // Subagent sessions are excluded: they inherit their parent's project when + // spawned, and their working directory is often a throwaway worktree that + // must never become one of the user's project locations. const rows = this.db .prepare(` - SELECT session_id AS id, json_extract(payload_json, '$.cwd') AS cwd + SELECT + session_id AS id, + json_extract(payload_json, '$.cwd') AS cwd, + COALESCE(last_message_at, last_used_at) AS used_at FROM session_metadata WHERE json_type(payload_json, '$.projectId') IS NULL - ORDER BY session_id + AND subagent_parent_session_id IS NULL + ORDER BY used_at, session_id `) - .all() as Array<{ id?: unknown; cwd?: unknown }>; + .all() as Array<{ id?: unknown; cwd?: unknown; used_at?: unknown }>; return rows.flatMap((row) => - typeof row.id === 'string' && typeof row.cwd === 'string' && row.cwd.length > 0 - ? [{ id: row.id, cwd: row.cwd }] + typeof row.id === 'string' && + typeof row.cwd === 'string' && + row.cwd.length > 0 && + typeof row.used_at === 'number' + ? [{ id: row.id, cwd: row.cwd, usedAt: row.used_at }] : [], ); } @@ -2614,10 +2637,9 @@ export class SqliteSessionMetadataStore { backend, llm_connection_slug, model, - project_id, metadata_version, committed_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `) .run( header.id, @@ -2645,7 +2667,6 @@ export class SqliteSessionMetadataStore { header.backend, header.llmConnectionSlug, header.model, - header.projectId ?? null, metadataVersion, committedAt, ); @@ -2856,7 +2877,6 @@ export class SqliteSessionMetadataStore { backend = ?, llm_connection_slug = ?, model = ?, - project_id = ?, metadata_version = ?, committed_at = ? WHERE session_id = ? AND metadata_version = ? @@ -2879,7 +2899,6 @@ export class SqliteSessionMetadataStore { next.backend, next.llmConnectionSlug, next.model, - next.projectId ?? null, metadataVersion, committedAt, sessionId, From 5caa985332b2ee732995fe2ca2cb87f14d56b34f Mon Sep 17 00:00:00 2001 From: AstroHan Date: Mon, 3 Aug 2026 22:42:29 +0800 Subject: [PATCH 4/4] fix(storage): fence project assignment and relink against concurrent decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviews of the previous commit found the same thing from different angles: its concurrency guarantees were asserted in comments but not implemented. The backfill claimed a user detaching a session mid-resolution would win, but read the header and wrote it in two separate transactions, so a detach landing between them was overwritten back to a resolved project. `updateHeaderVersioned` already exists for exactly this; the listing now carries each row's metadata revision and the assignment is fenced by it. The unreliable extra read is gone rather than kept alongside the fence — a version conflict simply means someone decided first, and a session that is still unresolved is retried next start. `relink` re-derived its merge from the catalog as it stood at commit time. That kept the catalog self-consistent while leaving it inconsistent with the world: `beforeCommit` reassigns the sessions of the project being merged away, and those writes cannot be re-derived. If the merge target moved while the callback ran, the sessions had already been handed to the wrong owner. The commit now refuses when the conflict it would merge is not the one the callback was shown. Relink was already retryable — a throwing callback leaves the catalog untouched — so this hands the decision back instead of committing a half-true one. A historical working directory whose ancestor was replaced by a plain file raises ENOTDIR, not ENOENT, and was left permanently unresolved even though walking one level further up canonicalizes it exactly like a deleted directory. Both mean "cannot reach this path"; they are now handled together. Finally, `mutate` made `withQueue` redundant for every fully synchronous mutation: the SQLite write lock does that job now. The queue is kept only where it still earns its place — `select` and `relink`, which await the filesystem or a caller's callback mid-change. Removing it elsewhere deletes five `Failed to …` branches that `mutate` had already made unreachable. Each new test was confirmed to fail against the unfixed code. --- .../src/__tests__/project-catalog.test.ts | 50 +++++ .../project-session-backfill.test.ts | 50 ++++- packages/storage/src/project-catalog.ts | 187 +++++++++--------- .../storage/src/project-session-backfill.ts | 33 ++-- .../src/sqlite-session-metadata-store.ts | 19 +- 5 files changed, 232 insertions(+), 107 deletions(-) diff --git a/packages/storage/src/__tests__/project-catalog.test.ts b/packages/storage/src/__tests__/project-catalog.test.ts index a64f723bc9..805ce975f7 100644 --- a/packages/storage/src/__tests__/project-catalog.test.ts +++ b/packages/storage/src/__tests__/project-catalog.test.ts @@ -257,6 +257,56 @@ test('two catalogs changing one project at the same time keep both changes', asy } }); +test('a relink whose merge target changes mid-flight fails instead of half-committing', async () => { + const base = await mkdtemp(join(tmpdir(), 'maka-project-relink-race-')); + try { + const storage = join(base, 'storage'); + const home = join(base, 'home'); + const shared = join(base, 'shared'); + const elsewhere = join(base, 'elsewhere'); + await Promise.all([mkdir(home), mkdir(shared), mkdir(elsewhere)]); + const first = createProjectCatalog(storage); + const second = createProjectCatalog(storage); + const moving = await first.register(home); + const target = await first.register(shared); + await Promise.all([first.list(), second.list()]); + + let releaseCallback!: () => void; + let callbackStarted!: () => void; + const gate = new Promise((release) => { + releaseCallback = release; + }); + const started = new Promise((resolve) => { + callbackStarted = resolve; + }); + let observed: string | undefined; + const relink = first.relink(moving.id, shared, async (context) => { + observed = context.conflictingProjectId; + callbackStarted(); + await gate; + }); + await started; + + // The callback was told to move `target`'s sessions onto `moving`. While it + // is doing that, the other window moves `target` somewhere else entirely. + await second.relink(target.id, elsewhere); + releaseCallback(); + + assert.equal(observed, target.id, 'precondition: the callback planned a merge'); + await assert.rejects(() => relink, /retry/); + const projects = await first.list(); + assert.deepEqual( + projects.map((project) => project.id).sort(), + [moving.id, target.id].sort(), + 'neither project may be merged away after the plan went stale', + ); + first.close(); + second.close(); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + test('relinking an unavailable project preserves its id and adopts the new directory', async () => { const base = await mkdtemp(join(tmpdir(), 'maka-project-relink-')); try { diff --git a/packages/storage/src/__tests__/project-session-backfill.test.ts b/packages/storage/src/__tests__/project-session-backfill.test.ts index 77e4ef2cc2..e2c7e071e3 100644 --- a/packages/storage/src/__tests__/project-session-backfill.test.ts +++ b/packages/storage/src/__tests__/project-session-backfill.test.ts @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { execFile } from 'node:child_process'; -import { mkdir, mkdtemp, realpath, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test } from 'node:test'; @@ -194,6 +194,54 @@ test('a subagent worktree never becomes one of the user project locations', asyn }); }); +test('a session detached while resolution is running keeps the user decision', async () => { + await withWorkspace(async ({ sessions, catalog, projectPath }) => { + const session = await sessions.create({ ...sessionInput(projectPath), name: 'session' }); + + const result = await backfillSessionProjects({ + sessions, + catalog: { + // Resolution is where the real work — a git probe — happens, so this is + // exactly the window in which a user can still act on the session. + resolveHistoricalPath: async (path, usedAt) => { + await sessions.updateHeader(session.id, { projectId: null }); + return catalog.resolveHistoricalPath(path, usedAt); + }, + }, + }); + + assert.deepEqual(result, { resolved: 0, failures: [] }); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + null, + 'a detach made after the plan was formed must still win', + ); + }); +}); + +test('a working directory reachable through a replaced ancestor still resolves', async () => { + await withWorkspace(async ({ sessions, catalog, base }) => { + const ancestor = join(base, 'ancestor'); + const cwd = join(ancestor, 'project'); + await mkdir(cwd, { recursive: true }); + const before = await catalog.register(cwd); + const session = await sessions.create({ ...sessionInput(cwd), name: 'session' }); + // The directory is gone and something else now occupies its parent, so + // walking up reports ENOTDIR rather than ENOENT. + await rm(ancestor, { recursive: true, force: true }); + await writeFile(ancestor, 'not a directory', 'utf8'); + + const result = await backfillSessionProjects({ sessions, catalog }); + + assert.deepEqual(result, { resolved: 1, failures: [] }); + assert.equal( + (await sessions.readHeaderSnapshot(session.id)).projectId, + before.id, + 'a replaced ancestor must not strand the session outside its project', + ); + }); +}); + test('a working directory that cannot be resolved is reported instead of guessed', async () => { await withWorkspace(async ({ sessions, projectPath }) => { const session = await sessions.create({ ...sessionInput(projectPath), name: 'session' }); diff --git a/packages/storage/src/project-catalog.ts b/packages/storage/src/project-catalog.ts index 30b1049d35..7c5f7672a3 100644 --- a/packages/storage/src/project-catalog.ts +++ b/packages/storage/src/project-catalog.ts @@ -30,6 +30,22 @@ export function isProjectPathMismatchError(error: unknown): error is ProjectPath return error instanceof ProjectPathMismatchError; } +/** + * The catalog changed underneath a relink while its `beforeCommit` callback was + * still reassigning sessions, so the merge the caller was told to prepare is no + * longer the merge that would be committed. Relink is already retryable — its + * callback throwing leaves the catalog untouched — so failing here hands the + * decision back rather than committing a half-true one. + */ +export class ProjectRelinkContentionError extends Error { + readonly name = 'ProjectRelinkContentionError'; + readonly code = 'project_relink_contention'; + + constructor(readonly projectId: string) { + super(`Project changed while relinking, retry: ${projectId}`); + } +} + export interface ProjectRelinkContext { projectId: string; projectAliases: string[]; @@ -128,10 +144,7 @@ class SqliteProjectCatalog implements ProjectCatalog { } async list(): Promise { - let projects: PersistedProject[] = []; - await this.withQueue(async () => { - projects = (await this.read()).projects; - }); + const projects = (await this.read()).projects; projects.sort( (a, b) => Number(a.archivedAt !== undefined) - Number(b.archivedAt !== undefined) || @@ -151,12 +164,12 @@ class SqliteProjectCatalog implements ProjectCatalog { try { resolved = await resolveProjectLocation({ path }); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + if (!isUnreachablePathError(error)) throw error; let pathIsMissing = false; try { await stat(path); } catch (pathError) { - pathIsMissing = (pathError as NodeJS.ErrnoException).code === 'ENOENT'; + pathIsMissing = isUnreachablePathError(pathError); } if (!pathIsMissing) throw error; const canonicalPath = await canonicalizeMissingPath(path); @@ -173,47 +186,41 @@ class SqliteProjectCatalog implements ProjectCatalog { resolved: ResolvedProjectLocation, timestamp: number, ): Promise { - let registered: PersistedProject | undefined; - await this.withQueue(async () => { - registered = await this.mutate((file) => { - const locationPath = - resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; - const existing = file.projects.find((project) => project.identity === resolved.identity); - if (existing) { - const location = existing.locations.find((item) => item.path === locationPath); - if (location) { - location.lastUsedAt = Math.max(location.lastUsedAt, timestamp); - location.isWorktree = resolved.git?.isWorktree ?? false; - } else { - existing.locations.push({ - path: locationPath, - isWorktree: resolved.git?.isWorktree ?? false, - lastUsedAt: timestamp, - }); - } - existing.lastUsedAt = Math.max(existing.lastUsedAt, timestamp); - return existing; + const registered = await this.mutate((file) => { + const locationPath = + resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; + const existing = file.projects.find((project) => project.identity === resolved.identity); + if (existing) { + const location = existing.locations.find((item) => item.path === locationPath); + if (location) { + location.lastUsedAt = Math.max(location.lastUsedAt, timestamp); + location.isWorktree = resolved.git?.isWorktree ?? false; + } else { + existing.locations.push({ + path: locationPath, + isWorktree: resolved.git?.isWorktree ?? false, + lastUsedAt: timestamp, + }); } - const project: PersistedProject = { - id: this.createId(), - name: defaultProjectName(resolved), - identity: resolved.identity, - locations: [ - { - path: locationPath, - isWorktree: resolved.git?.isWorktree ?? false, - lastUsedAt: timestamp, - }, - ], - lastUsedAt: timestamp, - }; - file.projects.push(project); - return project; - }); + existing.lastUsedAt = Math.max(existing.lastUsedAt, timestamp); + return existing; + } + const project: PersistedProject = { + id: this.createId(), + name: defaultProjectName(resolved), + identity: resolved.identity, + locations: [ + { + path: locationPath, + isWorktree: resolved.git?.isWorktree ?? false, + lastUsedAt: timestamp, + }, + ], + lastUsedAt: timestamp, + }; + file.projects.push(project); + return project; }); - if (!registered) { - throw new Error(`Failed to register project: ${resolved.canonicalPath}`); - } return this.present(registered); } @@ -261,26 +268,22 @@ class SqliteProjectCatalog implements ProjectCatalog { ? resolved.git!.worktreeRoot : resolved.canonicalPath : undefined; - let touched: PersistedProject | undefined; - await this.withQueue(async () => { - touched = await this.mutate((file) => { - const project = findProjectById(file.projects, projectId); - if (!project) throw new Error(`No such project: ${projectId}`); - const location = resolvedPath - ? project.locations.find((item) => item.path === resolvedPath) - : [...project.locations].sort( - (a, b) => b.lastUsedAt - a.lastUsedAt || a.path.localeCompare(b.path), - )[0]; - if (resolvedPath && !location) { - throw new ProjectPathMismatchError(projectId, resolvedPath); - } - const timestamp = this.now(); - if (location) location.lastUsedAt = timestamp; - project.lastUsedAt = timestamp; - return project; - }); + const touched = await this.mutate((file) => { + const project = findProjectById(file.projects, projectId); + if (!project) throw new Error(`No such project: ${projectId}`); + const location = resolvedPath + ? project.locations.find((item) => item.path === resolvedPath) + : [...project.locations].sort( + (a, b) => b.lastUsedAt - a.lastUsedAt || a.path.localeCompare(b.path), + )[0]; + if (resolvedPath && !location) { + throw new ProjectPathMismatchError(projectId, resolvedPath); + } + const timestamp = this.now(); + if (location) location.lastUsedAt = timestamp; + project.lastUsedAt = timestamp; + return project; }); - if (!touched) throw new Error(`Failed to touch project: ${projectId}`); return this.present(touched); } @@ -295,9 +298,11 @@ class SqliteProjectCatalog implements ProjectCatalog { const locationPath = resolved.kind === 'git' ? resolved.git!.worktreeRoot : resolved.canonicalPath; await this.withQueue(async () => { - // `beforeCommit` moves real directories, so it cannot run inside the - // write transaction. It is given the state it will act on, and the - // commit below re-derives everything from the catalog as it stands then. + // `beforeCommit` reassigns the sessions of the project being merged away, + // so it cannot run inside the write transaction. It is shown the state it + // will act on, and the commit below refuses to proceed if that state no + // longer holds: re-deriving instead would leave the catalog consistent + // while the sessions the callback already moved point at the wrong owner. const preview = await this.read(); const previewProject = findProjectById(preview.projects, projectId); if (!previewProject) throw new Error(`No such project: ${projectId}`); @@ -328,6 +333,9 @@ class SqliteProjectCatalog implements ProjectCatalog { if (conflict && !beforeCommit) { throw new Error(`Project path already belongs to project: ${conflict.id}`); } + if (conflict?.id !== previewConflict?.id) { + throw new ProjectRelinkContentionError(projectId); + } if (conflict) { project.aliases = [ ...new Set([...(project.aliases ?? []), conflict.id, ...(conflict.aliases ?? [])]), @@ -356,45 +364,36 @@ class SqliteProjectCatalog implements ProjectCatalog { async rename(projectId: string, name: string): Promise { const trimmed = name.trim(); if (!trimmed) throw new TypeError('Project name cannot be empty.'); - let renamed: PersistedProject | undefined; - await this.withQueue(async () => { - renamed = await this.mutate((file) => { + return this.present( + await this.mutate((file) => { const project = findProjectById(file.projects, projectId); if (!project) throw new Error(`No such project: ${projectId}`); project.name = trimmed; return project; - }); - }); - if (!renamed) throw new Error(`Failed to rename project: ${projectId}`); - return this.present(renamed); + }), + ); } async archive(projectId: string): Promise { - let archived: PersistedProject | undefined; - await this.withQueue(async () => { - archived = await this.mutate((file) => { + return this.present( + await this.mutate((file) => { const project = findProjectById(file.projects, projectId); if (!project) throw new Error(`No such project: ${projectId}`); project.archivedAt = this.now(); return project; - }); - }); - if (!archived) throw new Error(`Failed to archive project: ${projectId}`); - return this.present(archived); + }), + ); } async restore(projectId: string): Promise { - let restored: PersistedProject | undefined; - await this.withQueue(async () => { - restored = await this.mutate((file) => { + return this.present( + await this.mutate((file) => { const project = findProjectById(file.projects, projectId); if (!project) throw new Error(`No such project: ${projectId}`); delete project.archivedAt; return project; - }); - }); - if (!restored) throw new Error(`Failed to restore project: ${projectId}`); - return this.present(restored); + }), + ); } private async present(project: PersistedProject): Promise { @@ -714,7 +713,7 @@ async function canonicalizeMissingPath(path: string): Promise { try { return normalize(join(await realpath(candidate), ...missingSegments)); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + if (!isUnreachablePathError(error)) throw error; } const parent = dirname(candidate); if (parent === candidate) return absolute; @@ -723,6 +722,16 @@ async function canonicalizeMissingPath(path: string): Promise { } } +/** + * A path that cannot be reached, whether because a segment is gone (`ENOENT`) + * or because one of its ancestors is now a plain file (`ENOTDIR`). Both mean + * the same thing to a historical working directory: keep walking up. + */ +function isUnreachablePathError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code; + return code === 'ENOENT' || code === 'ENOTDIR'; +} + async function isDirectory(path: string): Promise { try { return (await stat(path)).isDirectory(); diff --git a/packages/storage/src/project-session-backfill.ts b/packages/storage/src/project-session-backfill.ts index e06c37f5f7..62aad4ab4d 100644 --- a/packages/storage/src/project-session-backfill.ts +++ b/packages/storage/src/project-session-backfill.ts @@ -1,5 +1,6 @@ import type { ProjectCatalog } from './project-catalog.js'; -import type { SessionStore } from './session-store.js'; +import type { SessionAuthorityStore } from './session-store.js'; +import { SessionMetadataVersionConflictError } from './sqlite-session-metadata-store.js'; export interface ProjectSessionBackfillResult { resolved: number; @@ -18,23 +19,28 @@ export interface ProjectSessionBackfillResult { * subprocess plus a catalog write each time. Each directory carries the latest * activity of the sessions that share it, so the rebuilt catalog keeps its real * recency order rather than collapsing every project to the upgrade's timestamp. + * + * Each assignment is fenced by the metadata revision the session was listed at. + * This runs during startup while a window is already open, so a user detaching + * a session mid-resolution would otherwise be overwritten by a plan made before + * they decided; the fenced write simply loses instead. */ export async function backfillSessionProjects(input: { sessions: Pick< - SessionStore, - 'listSessionsWithUnresolvedProject' | 'updateHeader' | 'readHeaderSnapshot' + SessionAuthorityStore, + 'listSessionsWithUnresolvedProject' | 'updateHeaderVersioned' >; catalog: Pick; }): Promise { const pending = await input.sessions.listSessionsWithUnresolvedProject(); - const byDirectory = new Map(); + const byDirectory = new Map(); for (const session of pending) { const group = byDirectory.get(session.cwd); if (group) { group.usedAt = Math.max(group.usedAt, session.usedAt); - group.sessionIds.push(session.id); + group.sessions.push(session); } else { - byDirectory.set(session.cwd, { usedAt: session.usedAt, sessionIds: [session.id] }); + byDirectory.set(session.cwd, { usedAt: session.usedAt, sessions: [session] }); } } @@ -49,14 +55,15 @@ export async function backfillSessionProjects(input: { failures.push({ cwd, reason: error instanceof Error ? error.message : String(error) }); continue; } - for (const sessionId of group.sessionIds) { + for (const session of group.sessions) { try { - // Re-check instead of writing blind: the user can detach a session - // while this runs, and that decision must win over a stale plan. - if ((await input.sessions.readHeaderSnapshot(sessionId)).projectId !== undefined) continue; - await input.sessions.updateHeader(sessionId, { projectId }); + await input.sessions.updateHeaderVersioned(session.id, { projectId }, session.revision); resolved += 1; } catch (error) { + // Someone changed the session first. Whatever they decided outranks a + // plan formed before it, and a still-unresolved session is retried on + // the next start, so this is not a failure to report. + if (error instanceof SessionMetadataVersionConflictError) continue; failures.push({ cwd, reason: error instanceof Error ? error.message : String(error) }); } } @@ -64,3 +71,7 @@ export async function backfillSessionProjects(input: { return { resolved, failures }; } + +type UnresolvedSession = Awaited< + ReturnType +>[number]; diff --git a/packages/storage/src/sqlite-session-metadata-store.ts b/packages/storage/src/sqlite-session-metadata-store.ts index b5242fa848..fa05e88d35 100644 --- a/packages/storage/src/sqlite-session-metadata-store.ts +++ b/packages/storage/src/sqlite-session-metadata-store.ts @@ -163,14 +163,19 @@ export interface VersionedSessionIdentity { } /** - * A session whose project membership was never decided, carried together with - * the moment it was last active so that resolving it later reconstructs the + * A session whose project membership was never decided. + * + * `usedAt` is the moment it was last active, so resolving it later rebuilds the * catalog's real recency order instead of collapsing every project to "now". + * `revision` is the metadata version this row was read at, so the write that + * assigns a project can fence itself against anything that touched the session + * in between — including a user detaching it while resolution is still running. */ export interface UnresolvedProjectSession { readonly id: string; readonly cwd: string; readonly usedAt: number; + readonly revision: number; } export type SessionRemovalProbe = @@ -1194,19 +1199,21 @@ export class SqliteSessionMetadataStore { SELECT session_id AS id, json_extract(payload_json, '$.cwd') AS cwd, - COALESCE(last_message_at, last_used_at) AS used_at + COALESCE(last_message_at, last_used_at) AS used_at, + metadata_version AS revision FROM session_metadata WHERE json_type(payload_json, '$.projectId') IS NULL AND subagent_parent_session_id IS NULL ORDER BY used_at, session_id `) - .all() as Array<{ id?: unknown; cwd?: unknown; used_at?: unknown }>; + .all() as Array<{ id?: unknown; cwd?: unknown; used_at?: unknown; revision?: unknown }>; return rows.flatMap((row) => typeof row.id === 'string' && typeof row.cwd === 'string' && row.cwd.length > 0 && - typeof row.used_at === 'number' - ? [{ id: row.id, cwd: row.cwd, usedAt: row.used_at }] + typeof row.used_at === 'number' && + typeof row.revision === 'number' + ? [{ id: row.id, cwd: row.cwd, usedAt: row.used_at, revision: row.revision }] : [], ); }