You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by as...@apache.org on 2026/09/09 07:13:08 UTC
(maka) branch main updated: fix(storage): reclaim upgrade residue safely (#4922)
This is an automated email from the ASF dual-hosted git repository.
Astro-Han pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/maka.git
The following commit(s) were added to refs/heads/main by this push:
new 331f1edbad fix(storage): reclaim upgrade residue safely (#4922)
331f1edbad is described below
commit 331f1edbad6aacf996e342133cdd2e3606cf0149
Author: Xiao Liu <li...@qq.com>
AuthorDate: Wed Sep 9 15:13:03 2026 +0800
fix(storage): reclaim upgrade residue safely (#4922)
Reuse artifact removal identity and real-parent containment for upgrade residue cleanup. Preserve bounded pagination and retain unsafe or failed cleanup notes without deleting live aliases.
Fixes #4910
Generated-by: Codex
---
.../storage/src/__tests__/artifact-stores.test.ts | 186 ++++++++++++++++++++-
packages/storage/src/artifact-store.ts | 57 ++++++-
packages/storage/src/sqlite-artifact-metadata.ts | 13 ++
3 files changed, 251 insertions(+), 5 deletions(-)
diff --git a/packages/storage/src/__tests__/artifact-stores.test.ts b/packages/storage/src/__tests__/artifact-stores.test.ts
index d4399dbfe8..a5920ee0b9 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,
@@ -309,6 +309,145 @@ 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;
+
+ assert.deepEqual(await store.reclaimUpgradeResidue({ maxPaths: 64 }), {
+ nextAfter: null,
+ processedPaths: 1,
+ failedPaths: 1,
+ });
+
+ 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, 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,
+ artifactId,
+ 'session-1',
+ 1,
+ status,
+ relativePath,
+ JSON.stringify({
+ id: artifactId,
+ 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]);
+
+ assert.deepEqual(await store.reclaimUpgradeResidue({ maxPaths: 64 }), {
+ nextAfter: null,
+ processedPaths: 1,
+ failedPaths: 0,
+ });
+
+ 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(
() =>
@@ -471,3 +610,46 @@ async function withTemporaryRoot(
}
type TrackArtifactWriter = <T extends { close(): void }>(writer: T) => T;
+
+async function createSymlinkOrSkip(t: TestContext, target: string, path: string): Promise<boolean> {
+ 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<boolean> {
+ 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 6bb8dea74f..f3da7b016a 100644
--- a/packages/storage/src/artifact-store.ts
+++ b/packages/storage/src/artifact-store.ts
@@ -506,6 +506,7 @@ class SqliteArtifactStore implements ArtifactAuthorityStore {
const directories = new Set<string>();
const discharged: string[] = [];
let failedPaths = 0;
+ let realArtifactRoot: string | undefined;
try {
for (const relativePath of selected) {
if (
@@ -515,10 +516,33 @@ class SqliteArtifactStore implements ArtifactAuthorityStore {
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))) {
+ failedPaths += 1;
+ continue;
+ }
+ const artifactIds = new Set([
+ ...artifactIdsFromUpgradeOrphanPath(relativePath),
+ ...artifactIdsFromUpgradeOrphanPath(entry.unlinkPath),
+ ]);
+ if (
+ artifactIds.size > 0 &&
+ (await this.hasClaimedRemovalIdentityUnlocked(
+ [...artifactIds],
+ 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)) {
failedPaths += 1;
@@ -539,6 +563,19 @@ class SqliteArtifactStore implements ArtifactAuthorityStore {
});
}
+ private async hasClaimedRemovalIdentityUnlocked(
+ artifactIds: readonly string[],
+ comparisonIdentity: string,
+ ): Promise<boolean> {
+ for (const relativePath of this.metadataRepository.readRelativePathsByCaseFoldedArtifactIds(
+ artifactIds,
+ )) {
+ const entry = await resolveArtifactRemovalEntry(this.artifactRoot, relativePath);
+ if (entry?.comparisonIdentity === comparisonIdentity) return true;
+ }
+ return false;
+ }
+
private async replayExistingArtifactUnlocked(
existing: ArtifactRecord,
input: CreateArtifactInput,
@@ -1213,6 +1250,20 @@ function symlinkEntryIdentity(entryStat: BigIntStats): string {
].join(':');
}
+function artifactIdsFromUpgradeOrphanPath(relativePath: string): readonly string[] {
+ const name = basename(relativePath);
+ const artifactIds: string[] = [];
+ for (
+ let separator = name.indexOf('-');
+ separator > 0;
+ separator = name.indexOf('-', separator + 1)
+ ) {
+ const artifactId = name.slice(0, separator);
+ if (isCanonicalArtifactEntityId(artifactId)) artifactIds.push(artifactId);
+ }
+ return artifactIds;
+}
+
function isInsideOrSamePath(root: string, target: string): boolean {
if (target === root) return true;
const rel = relative(root, target);
diff --git a/packages/storage/src/sqlite-artifact-metadata.ts b/packages/storage/src/sqlite-artifact-metadata.ts
index db128968d9..df4094ef66 100644
--- a/packages/storage/src/sqlite-artifact-metadata.ts
+++ b/packages/storage/src/sqlite-artifact-metadata.ts
@@ -110,6 +110,19 @@ class SqliteArtifactMetadataRepository {
);
}
+ 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 COLLATE NOCASE IN (${placeholders})`,
+ )
+ .all(...artifactIds) as Array<{ relative_path: string }>;
+ return rows.map((row) => row.relative_path);
+ }
+
forgetUpgradeOrphanPaths(relativePaths: readonly string[]): void {
this.assertOpen();
this.#lease.transaction('write', () => {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org