From 9052cb808cb7a5af0f8185b8228afe2b05b66286 Mon Sep 17 00:00:00 2001 From: liuxiaocs7 Date: Sun, 6 Sep 2026 19:17:10 +0800 Subject: [PATCH 1/2] fix(storage): reclaim upgrade residue safely Resolve upgrade orphan paths through the same removal identity used by ordinary artifact purges. Keep out-of-root entries pending and discharge aliases of live artifacts without deleting their bytes. Fixes #4910 Generated-by: Codex --- .../src/__tests__/artifact-stores.test.ts | 178 +++++++++++++++++- packages/storage/src/artifact-store.ts | 21 ++- 2 files changed, 194 insertions(+), 5 deletions(-) diff --git a/packages/storage/src/__tests__/artifact-stores.test.ts b/packages/storage/src/__tests__/artifact-stores.test.ts index 2bb2a687fa..cc6daf5d44 100644 --- a/packages/storage/src/__tests__/artifact-stores.test.ts +++ b/packages/storage/src/__tests__/artifact-stores.test.ts @@ -18,11 +18,11 @@ */ import assert from 'node:assert/strict'; -import { mkdir, mkdtemp, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rename, rm, stat, symlink, writeFile } from 'node:fs/promises'; import { DatabaseSync } from 'node:sqlite'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { after, describe, test } from 'node:test'; +import { after, describe, test, type TestContext } from 'node:test'; import { authenticateInteractiveArtifactStoreWriter, openInteractiveArtifactStoreForWrite, @@ -243,6 +243,137 @@ describe('interactive artifact store authority', () => { }); }); + test('does not follow a replaced parent directory while reclaiming upgrade residue', async (t) => { + const outsideRoot = await mkdtemp(join(tmpdir(), 'maka-artifact-upgrade-outside-')); + try { + await withInteractiveOwner(async (owner, root, track) => { + const initial = await openInteractiveArtifactStoreForWrite(owner.lease); + initial.close(); + const relativePath = 'session-1/retired-payload.txt'; + const db = new DatabaseSync(join(root, 'runtime.sqlite')); + db.exec(` + DROP TABLE artifact_records; + CREATE TABLE artifact_records ( + storage_key TEXT PRIMARY KEY, artifact_id TEXT NOT NULL, + session_id TEXT NOT NULL, created_at INTEGER NOT NULL CHECK(created_at >= 0), + status TEXT NOT NULL CHECK(status IN ('live', 'deleted')), + relative_path TEXT NOT NULL, record_json TEXT NOT NULL + ); + CREATE UNIQUE INDEX artifact_records_relative_path ON artifact_records(relative_path); + UPDATE operational_schema_migrations SET version = 1 WHERE scope = 'artifact'; + `); + db.prepare('INSERT INTO artifact_records VALUES (?, ?, ?, ?, ?, ?, ?)').run( + 'retired', + 'retired', + 'session-1', + 1, + 'live', + relativePath, + JSON.stringify({ + id: 'retired', + sessionId: 'session-1', + turnId: 'turn-1', + createdAt: 1, + name: 'payload.txt', + kind: 'file', + sizeBytes: 8, + relativePath, + source: 'provider_request_capture', + status: 'live', + }), + ); + db.close(); + + const artifactRoot = join(root, 'artifacts'); + const sessionRoot = join(artifactRoot, 'session-1'); + await mkdir(sessionRoot, { recursive: true }); + await writeFile(join(artifactRoot, relativePath), 'original', 'utf8'); + const store = track(await openInteractiveArtifactStoreForWrite(owner.lease)); + const displacedSessionRoot = join(artifactRoot, 'displaced-session-1'); + await rename(sessionRoot, displacedSessionRoot); + const outsidePath = join(outsideRoot, 'retired-payload.txt'); + await writeFile(outsidePath, 'external', 'utf8'); + if (!(await createSymlinkOrSkip(t, outsideRoot, sessionRoot))) return; + + await store.reclaimUpgradeResidue(); + + assert.equal(await readFile(outsidePath, 'utf8'), 'external'); + assert.equal( + await readFile(join(displacedSessionRoot, 'retired-payload.txt'), 'utf8'), + 'original', + ); + assert.deepEqual(readUpgradeOrphanPaths(root), [relativePath]); + }); + } finally { + await rm(outsideRoot, { recursive: true, force: true }); + } + }); + + test('does not reclaim an upgrade orphan path that aliases a live artifact', async (t) => { + await withInteractiveOwner(async (owner, root, track) => { + if (!(await isCaseInsensitiveFilesystem(root))) { + t.skip('requires a case-insensitive filesystem'); + return; + } + + const initial = await openInteractiveArtifactStoreForWrite(owner.lease); + initial.close(); + const liveRelativePath = 'session-1/shared-Payload.txt'; + const orphanRelativePath = 'session-1/shared-payload.txt'; + const db = new DatabaseSync(join(root, 'runtime.sqlite')); + db.exec(` + DROP TABLE artifact_records; + CREATE TABLE artifact_records ( + storage_key TEXT PRIMARY KEY, artifact_id TEXT NOT NULL, + session_id TEXT NOT NULL, created_at INTEGER NOT NULL CHECK(created_at >= 0), + status TEXT NOT NULL CHECK(status IN ('live', 'deleted')), + relative_path TEXT NOT NULL, record_json TEXT NOT NULL + ); + CREATE UNIQUE INDEX artifact_records_relative_path ON artifact_records(relative_path); + UPDATE operational_schema_migrations SET version = 1 WHERE scope = 'artifact'; + `); + for (const [storageKey, status, relativePath, name] of [ + ['live', 'live', liveRelativePath, 'Payload.txt'], + ['retired', 'deleted', orphanRelativePath, 'payload.txt'], + ] as const) { + db.prepare('INSERT INTO artifact_records VALUES (?, ?, ?, ?, ?, ?, ?)').run( + storageKey, + 'shared', + 'session-1', + 1, + status, + relativePath, + JSON.stringify({ + id: 'shared', + sessionId: 'session-1', + turnId: 'turn-1', + createdAt: 1, + name, + kind: 'file', + sizeBytes: 10, + relativePath, + source: 'user_upload', + status, + }), + ); + } + db.close(); + + await mkdir(join(root, 'artifacts', 'session-1'), { recursive: true }); + await writeFile(join(root, 'artifacts', liveRelativePath), 'live bytes', 'utf8'); + const store = track(await openInteractiveArtifactStoreForWrite(owner.lease)); + assert.deepEqual(readUpgradeOrphanPaths(root), [orphanRelativePath]); + + await store.reclaimUpgradeResidue(); + + assert.deepEqual(await store.readTextInSession('session-1', 'shared'), { + ok: true, + text: 'live bytes', + }); + assert.deepEqual(readUpgradeOrphanPaths(root), []); + }); + }); + test('requires authentic leases and writer facades', async () => { await assert.rejects( () => @@ -405,3 +536,46 @@ async function withTemporaryRoot( } type TrackArtifactWriter = (writer: T) => T; + +async function createSymlinkOrSkip(t: TestContext, target: string, path: string): Promise { + try { + await symlink(target, path, process.platform === 'win32' ? 'junction' : 'dir'); + return true; + } catch (error) { + const code = (error as { code?: unknown }).code; + if (process.platform === 'win32' && (code === 'EPERM' || code === 'EACCES')) { + t.skip('Windows symlink creation requires elevated privileges or Developer Mode'); + return false; + } + throw error; + } +} + +function readUpgradeOrphanPaths(root: string): string[] { + const database = new DatabaseSync(join(root, 'runtime.sqlite'), { readOnly: true }); + try { + return database + .prepare('SELECT relative_path FROM artifact_upgrade_orphan_paths ORDER BY relative_path') + .all() + .map((row) => (row as { relative_path: string }).relative_path); + } finally { + database.close(); + } +} + +async function isCaseInsensitiveFilesystem(directory: string): Promise { + const probe = join(directory, '.maka-case-sensitivity-probe'); + const alias = join(directory, '.MAKA-CASE-SENSITIVITY-PROBE'); + await writeFile(probe, 'probe', { flag: 'wx' }); + try { + return await stat(alias).then( + () => true, + (error: unknown) => { + if ((error as { code?: unknown }).code === 'ENOENT') return false; + throw error; + }, + ); + } finally { + await rm(probe, { force: true }); + } +} diff --git a/packages/storage/src/artifact-store.ts b/packages/storage/src/artifact-store.ts index 45f565193b..00f5035c94 100644 --- a/packages/storage/src/artifact-store.ts +++ b/packages/storage/src/artifact-store.ts @@ -487,18 +487,33 @@ class SqliteArtifactStore implements ArtifactAuthorityStore { const recorded = this.metadataRepository.readUpgradeOrphanPaths(); if (recorded.length === 0) return; const claimed = new Set(this.records.map((record) => record.relativePath)); + const claimedEntries = await this.resolveRemovalEntriesUnlocked(this.records); + const claimedIdentities = new Set( + claimedEntries.flatMap((entry) => (entry ? [entry.comparisonIdentity] : [])), + ); const directories = new Set(); const discharged: string[] = []; + let realArtifactRoot: string | undefined; try { for (const relativePath of recorded) { if (claimed.has(relativePath) || !isSafeRelativeArtifactPath(relativePath)) { discharged.push(relativePath); continue; } - const target = join(this.artifactRoot, relativePath); + const entry = await resolveArtifactRemovalEntry(this.artifactRoot, relativePath); + if (!entry) { + discharged.push(relativePath); + continue; + } + realArtifactRoot ??= await ensureRealDirectory(this.artifactRoot); + if (!isInsideOrSamePath(realArtifactRoot, dirname(entry.unlinkPath))) continue; + if (claimedIdentities.has(entry.comparisonIdentity)) { + discharged.push(relativePath); + continue; + } try { - await unlink(target); - directories.add(dirname(target)); + await unlink(entry.unlinkPath); + directories.add(dirname(entry.unlinkPath)); } catch (error) { if (!isNotFound(error)) continue; } From 96412b7e6cfa7a1a8da13abccb94fea726c8f20a Mon Sep 17 00:00:00 2001 From: liuxiaocs7 Date: Tue, 8 Sep 2026 11:59:22 +0800 Subject: [PATCH 2/2] fix(storage): preserve bounded safe residue cleanup Keep the upstream paginated maintenance contract while resolving orphan entries through filesystem identity and querying only plausible live aliases.\n\nGenerated-by: Codex --- .../storage/src/__tests__/artifact-stores.test.ts | 14 +++++++------- packages/storage/src/artifact-store.ts | 2 +- packages/storage/src/sqlite-artifact-metadata.ts | 7 +++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/storage/src/__tests__/artifact-stores.test.ts b/packages/storage/src/__tests__/artifact-stores.test.ts index 7941677007..a5920ee0b9 100644 --- a/packages/storage/src/__tests__/artifact-stores.test.ts +++ b/packages/storage/src/__tests__/artifact-stores.test.ts @@ -388,7 +388,7 @@ describe('interactive artifact store authority', () => { const initial = await openInteractiveArtifactStoreForWrite(owner.lease); initial.close(); - const liveRelativePath = 'session-1/shared-Payload.txt'; + const liveRelativePath = 'session-1/SHARED-Payload.txt'; const orphanRelativePath = 'session-1/shared-payload.txt'; const db = new DatabaseSync(join(root, 'runtime.sqlite')); db.exec(` @@ -402,19 +402,19 @@ describe('interactive artifact store authority', () => { CREATE UNIQUE INDEX artifact_records_relative_path ON artifact_records(relative_path); UPDATE operational_schema_migrations SET version = 1 WHERE scope = 'artifact'; `); - for (const [storageKey, status, relativePath, name] of [ - ['live', 'live', liveRelativePath, 'Payload.txt'], - ['retired', 'deleted', orphanRelativePath, 'payload.txt'], + for (const [storageKey, artifactId, status, relativePath, name] of [ + ['live', 'SHARED', 'live', liveRelativePath, 'Payload.txt'], + ['retired', 'shared', 'deleted', orphanRelativePath, 'payload.txt'], ] as const) { db.prepare('INSERT INTO artifact_records VALUES (?, ?, ?, ?, ?, ?, ?)').run( storageKey, - 'shared', + artifactId, 'session-1', 1, status, relativePath, JSON.stringify({ - id: 'shared', + id: artifactId, sessionId: 'session-1', turnId: 'turn-1', createdAt: 1, @@ -440,7 +440,7 @@ describe('interactive artifact store authority', () => { failedPaths: 0, }); - assert.deepEqual(await store.readTextInSession('session-1', 'shared'), { + assert.deepEqual(await store.readTextInSession('session-1', 'SHARED'), { ok: true, text: 'live bytes', }); diff --git a/packages/storage/src/artifact-store.ts b/packages/storage/src/artifact-store.ts index 5645d00e5f..f3da7b016a 100644 --- a/packages/storage/src/artifact-store.ts +++ b/packages/storage/src/artifact-store.ts @@ -567,7 +567,7 @@ class SqliteArtifactStore implements ArtifactAuthorityStore { artifactIds: readonly string[], comparisonIdentity: string, ): Promise { - for (const relativePath of this.metadataRepository.readRelativePathsByArtifactIds( + for (const relativePath of this.metadataRepository.readRelativePathsByCaseFoldedArtifactIds( artifactIds, )) { const entry = await resolveArtifactRemovalEntry(this.artifactRoot, relativePath); diff --git a/packages/storage/src/sqlite-artifact-metadata.ts b/packages/storage/src/sqlite-artifact-metadata.ts index 8442e66ca2..df4094ef66 100644 --- a/packages/storage/src/sqlite-artifact-metadata.ts +++ b/packages/storage/src/sqlite-artifact-metadata.ts @@ -110,12 +110,15 @@ class SqliteArtifactMetadataRepository { ); } - readRelativePathsByArtifactIds(artifactIds: readonly string[]): string[] { + readRelativePathsByCaseFoldedArtifactIds(artifactIds: readonly string[]): string[] { this.assertOpen(); if (artifactIds.length === 0) return []; const placeholders = artifactIds.map(() => '?').join(', '); const rows = this.#lease.database - .prepare(`SELECT relative_path FROM artifact_records WHERE artifact_id IN (${placeholders})`) + .prepare( + `SELECT relative_path FROM artifact_records + WHERE artifact_id COLLATE NOCASE IN (${placeholders})`, + ) .all(...artifactIds) as Array<{ relative_path: string }>; return rows.map((row) => row.relative_path); }