diff --git a/extensions/matrix/src/matrix/client/storage.test.ts b/extensions/matrix/src/matrix/client/storage.test.ts index 6019ce0275c..1d368c90fce 100644 --- a/extensions/matrix/src/matrix/client/storage.test.ts +++ b/extensions/matrix/src/matrix/client/storage.test.ts @@ -112,6 +112,32 @@ describe("matrix client storage paths", () => { expect(fs.existsSync(storagePaths.cryptoPath)).toBe(true); }); + it("continues migrating whichever legacy artifact is still missing", async () => { + const stateDir = setupStateDir(); + const storagePaths = resolveMatrixStoragePaths({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "secret-token", + env: {}, + }); + const legacyRoot = path.join(stateDir, "matrix"); + fs.mkdirSync(storagePaths.rootDir, { recursive: true }); + fs.writeFileSync(storagePaths.storagePath, '{"new":true}'); + fs.mkdirSync(path.join(legacyRoot, "crypto"), { recursive: true }); + + await maybeMigrateLegacyStorage({ + storagePaths, + env: {}, + }); + + expect(maybeCreateMatrixMigrationSnapshotMock).toHaveBeenCalledWith( + expect.objectContaining({ trigger: "matrix-client-fallback" }), + ); + expect(fs.readFileSync(storagePaths.storagePath, "utf8")).toBe('{"new":true}'); + expect(fs.existsSync(path.join(legacyRoot, "crypto"))).toBe(false); + expect(fs.existsSync(storagePaths.cryptoPath)).toBe(true); + }); + it("refuses to migrate legacy storage when the snapshot step fails", async () => { const stateDir = setupStateDir(); const storagePaths = resolveMatrixStoragePaths({ diff --git a/extensions/matrix/src/matrix/client/storage.ts b/extensions/matrix/src/matrix/client/storage.ts index 311f86c17d0..447f66443b0 100644 --- a/extensions/matrix/src/matrix/client/storage.ts +++ b/extensions/matrix/src/matrix/client/storage.ts @@ -185,18 +185,20 @@ export async function maybeMigrateLegacyStorage(params: { storagePaths: MatrixStoragePaths; env?: NodeJS.ProcessEnv; }): Promise { - const hasNewStorage = - fs.existsSync(params.storagePaths.storagePath) || fs.existsSync(params.storagePaths.cryptoPath); - if (hasNewStorage) { - return; - } - const legacy = resolveLegacyStoragePaths(params.env); const hasLegacyStorage = fs.existsSync(legacy.storagePath); const hasLegacyCrypto = fs.existsSync(legacy.cryptoPath); if (!hasLegacyStorage && !hasLegacyCrypto) { return; } + const hasTargetStorage = fs.existsSync(params.storagePaths.storagePath); + const hasTargetCrypto = fs.existsSync(params.storagePaths.cryptoPath); + // Continue partial migrations one artifact at a time; only skip items whose targets already exist. + const shouldMigrateStorage = hasLegacyStorage && !hasTargetStorage; + const shouldMigrateCrypto = hasLegacyCrypto && !hasTargetCrypto; + if (!shouldMigrateStorage && !shouldMigrateCrypto) { + return; + } assertLegacyMigrationAccountSelection({ accountKey: params.storagePaths.accountKey, @@ -210,22 +212,31 @@ export async function maybeMigrateLegacyStorage(params: { }); fs.mkdirSync(params.storagePaths.rootDir, { recursive: true }); const moved: LegacyMoveRecord[] = []; + const skippedExistingTargets: string[] = []; try { - if (hasLegacyStorage) { + if (shouldMigrateStorage) { moveLegacyStoragePathOrThrow({ sourcePath: legacy.storagePath, targetPath: params.storagePaths.storagePath, label: "sync store", moved, }); + } else if (hasLegacyStorage) { + skippedExistingTargets.push( + `- sync store remains at ${legacy.storagePath} because ${params.storagePaths.storagePath} already exists`, + ); } - if (hasLegacyCrypto) { + if (shouldMigrateCrypto) { moveLegacyStoragePathOrThrow({ sourcePath: legacy.cryptoPath, targetPath: params.storagePaths.cryptoPath, label: "crypto store", moved, }); + } else if (hasLegacyCrypto) { + skippedExistingTargets.push( + `- crypto store remains at ${legacy.cryptoPath} because ${params.storagePaths.cryptoPath} already exists`, + ); } } catch (err) { const rollbackError = rollbackLegacyMoves(moved); @@ -242,6 +253,11 @@ export async function maybeMigrateLegacyStorage(params: { .join("\n")}`, ); } + if (skippedExistingTargets.length > 0) { + logger.warn?.( + `matrix: legacy client storage still exists in the flat path because some account-scoped targets already existed.\n${skippedExistingTargets.join("\n")}`, + ); + } } function moveLegacyStoragePathOrThrow(params: { diff --git a/src/gateway/server-startup-matrix-migration.test.ts b/src/gateway/server-startup-matrix-migration.test.ts index 548c6fa571c..95e72bf39dc 100644 --- a/src/gateway/server-startup-matrix-migration.test.ts +++ b/src/gateway/server-startup-matrix-migration.test.ts @@ -127,4 +127,54 @@ describe("runStartupMatrixMigration", () => { ); }); }); + + it("downgrades migration step failures to warnings so startup can continue", async () => { + await withTempHome(async (home) => { + const stateDir = path.join(home, ".openclaw"); + await fs.mkdir(path.join(stateDir, "matrix"), { recursive: true }); + await fs.writeFile(path.join(stateDir, "matrix", "bot-storage.json"), '{"legacy":true}'); + const maybeCreateMatrixMigrationSnapshotMock = vi.fn(async () => ({ + created: true, + archivePath: "/tmp/snapshot.tar.gz", + markerPath: "/tmp/migration-snapshot.json", + })); + const autoMigrateLegacyMatrixStateMock = vi.fn(async () => ({ + migrated: true, + changes: [], + warnings: [], + })); + const autoPrepareLegacyMatrixCryptoMock = vi.fn(async () => { + throw new Error("disk full"); + }); + const warn = vi.fn(); + + await expect( + runStartupMatrixMigration({ + cfg: { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok-123", + }, + }, + } as never, + env: process.env, + deps: { + maybeCreateMatrixMigrationSnapshot: maybeCreateMatrixMigrationSnapshotMock, + autoMigrateLegacyMatrixState: autoMigrateLegacyMatrixStateMock, + autoPrepareLegacyMatrixCrypto: autoPrepareLegacyMatrixCryptoMock, + }, + log: { warn }, + }), + ).resolves.toBeUndefined(); + + expect(maybeCreateMatrixMigrationSnapshotMock).toHaveBeenCalledOnce(); + expect(autoMigrateLegacyMatrixStateMock).toHaveBeenCalledOnce(); + expect(autoPrepareLegacyMatrixCryptoMock).toHaveBeenCalledOnce(); + expect(warn).toHaveBeenCalledWith( + "gateway: legacy Matrix encrypted-state preparation failed during Matrix migration; continuing startup: Error: disk full", + ); + }); + }); }); diff --git a/src/gateway/server-startup-matrix-migration.ts b/src/gateway/server-startup-matrix-migration.ts index 7774b4f3d6c..64a5f4e0721 100644 --- a/src/gateway/server-startup-matrix-migration.ts +++ b/src/gateway/server-startup-matrix-migration.ts @@ -12,6 +12,20 @@ type MatrixMigrationLogger = { warn?: (message: string) => void; }; +async function runBestEffortMatrixMigrationStep(params: { + label: string; + log: MatrixMigrationLogger; + run: () => Promise; +}): Promise { + try { + await params.run(); + } catch (err) { + params.log.warn?.( + `gateway: ${params.label} failed during Matrix migration; continuing startup: ${String(err)}`, + ); + } +} + export async function runStartupMatrixMigration(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; @@ -55,14 +69,24 @@ export async function runStartupMatrixMigration(params: { return; } - await migrateLegacyState({ - cfg: params.cfg, - env, + await runBestEffortMatrixMigrationStep({ + label: "legacy Matrix state migration", log: params.log, + run: () => + migrateLegacyState({ + cfg: params.cfg, + env, + log: params.log, + }), }); - await prepareLegacyCrypto({ - cfg: params.cfg, - env, + await runBestEffortMatrixMigrationStep({ + label: "legacy Matrix encrypted-state preparation", log: params.log, + run: () => + prepareLegacyCrypto({ + cfg: params.cfg, + env, + log: params.log, + }), }); } diff --git a/src/infra/matrix-legacy-crypto.test.ts b/src/infra/matrix-legacy-crypto.test.ts index 90326af874c..2e3d52fdd24 100644 --- a/src/infra/matrix-legacy-crypto.test.ts +++ b/src/infra/matrix-legacy-crypto.test.ts @@ -121,6 +121,54 @@ describe("matrix legacy encrypted-state migration", () => { }); }); + it("warns instead of throwing when recovery-key persistence fails", async () => { + await withTempHome(async (home) => { + const stateDir = path.join(home, ".openclaw"); + const cfg: OpenClawConfig = { + channels: { + matrix: { + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok-123", + }, + }, + }; + const { rootDir } = resolveMatrixAccountStorageRoot({ + stateDir, + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "tok-123", + }); + writeFile(path.join(rootDir, "crypto", "bot-sdk.json"), '{"deviceId":"DEVICE123"}'); + + const result = await autoPrepareLegacyMatrixCrypto({ + cfg, + env: process.env, + deps: { + inspectLegacyStore: async () => ({ + deviceId: "DEVICE123", + roomKeyCounts: { total: 12, backedUp: 12 }, + backupVersion: "1", + decryptionKeyBase64: "YWJjZA==", + }), + writeJsonFileAtomically: async (filePath) => { + if (filePath.endsWith("recovery-key.json")) { + throw new Error("disk full"); + } + writeFile(filePath, JSON.stringify({ ok: true }, null, 2)); + }, + }, + }); + + expect(result.migrated).toBe(false); + expect(result.warnings).toContain( + `Failed writing Matrix recovery key for account "default" (${path.join(rootDir, "recovery-key.json")}): Error: disk full`, + ); + expect(fs.existsSync(path.join(rootDir, "recovery-key.json"))).toBe(false); + expect(fs.existsSync(path.join(rootDir, "legacy-crypto-migration.json"))).toBe(false); + }); + }); + it("prepares flat legacy crypto for the only configured non-default Matrix account", async () => { await withTempHome(async (home) => { const stateDir = path.join(home, ".openclaw"); diff --git a/src/infra/matrix-legacy-crypto.ts b/src/infra/matrix-legacy-crypto.ts index a025e699296..00caf4c10f2 100644 --- a/src/infra/matrix-legacy-crypto.ts +++ b/src/infra/matrix-legacy-crypto.ts @@ -3,7 +3,7 @@ import os from "node:os"; import path from "node:path"; import type { OpenClawConfig } from "../config/config.js"; import { resolveStateDir } from "../config/paths.js"; -import { writeJsonFileAtomically } from "../plugin-sdk/json-store.js"; +import { writeJsonFileAtomically as writeJsonFileAtomicallyImpl } from "../plugin-sdk/json-store.js"; import { resolveConfiguredMatrixAccountIds } from "./matrix-account-selection.js"; import { resolveLegacyMatrixFlatStoreTarget, @@ -70,6 +70,7 @@ type MatrixLegacyCryptoPreparationResult = { type MatrixLegacyCryptoPrepareDeps = { inspectLegacyStore: MatrixLegacyCryptoInspector; + writeJsonFileAtomically: typeof writeJsonFileAtomicallyImpl; }; type MatrixLegacyBotSdkMetadata = { @@ -285,8 +286,9 @@ function loadLegacyCryptoMigrationState(filePath: string): MatrixLegacyCryptoMig async function persistLegacyMigrationState(params: { filePath: string; state: MatrixLegacyCryptoMigrationState; + writeJsonFileAtomically: typeof writeJsonFileAtomicallyImpl; }): Promise { - await writeJsonFileAtomically(params.filePath, params.state); + await params.writeJsonFileAtomically(params.filePath, params.state); } export function detectLegacyMatrixCrypto(params: { @@ -325,6 +327,8 @@ export async function autoPrepareLegacyMatrixCrypto(params: { const warnings = [...detection.warnings]; const changes: string[] = []; let inspectLegacyStore = params.deps?.inspectLegacyStore; + const writeJsonFileAtomically = + params.deps?.writeJsonFileAtomically ?? writeJsonFileAtomicallyImpl; if (!inspectLegacyStore) { try { inspectLegacyStore = await loadMatrixLegacyCryptoInspector({ @@ -394,11 +398,17 @@ export async function autoPrepareLegacyMatrixCrypto(params: { keyId: null, privateKeyBase64: summary.decryptionKeyBase64, }; - await writeJsonFileAtomically(plan.recoveryKeyPath, payload); - changes.push( - `Imported Matrix legacy backup key for account "${plan.accountId}": ${plan.recoveryKeyPath}`, - ); - decryptionKeyImported = true; + try { + await writeJsonFileAtomically(plan.recoveryKeyPath, payload); + changes.push( + `Imported Matrix legacy backup key for account "${plan.accountId}": ${plan.recoveryKeyPath}`, + ); + decryptionKeyImported = true; + } catch (err) { + warnings.push( + `Failed writing Matrix recovery key for account "${plan.accountId}" (${plan.recoveryKeyPath}): ${String(err)}`, + ); + } } else { decryptionKeyImported = true; } @@ -425,6 +435,14 @@ export async function autoPrepareLegacyMatrixCrypto(params: { `Legacy Matrix encrypted state for account "${plan.accountId}" cannot be fully converted automatically because the old rust crypto store does not expose all local room keys for export.`, ); } + // If recovery-key persistence failed, leave the migration state absent so the next startup can retry. + if ( + summary.decryptionKeyBase64 && + !decryptionKeyImported && + !loadStoredRecoveryKey(plan.recoveryKeyPath) + ) { + continue; + } const state: MatrixLegacyCryptoMigrationState = { version: 1, @@ -438,10 +456,20 @@ export async function autoPrepareLegacyMatrixCrypto(params: { detectedAt: new Date().toISOString(), lastError: null, }; - await persistLegacyMigrationState({ filePath: plan.statePath, state }); - changes.push( - `Prepared Matrix legacy encrypted-state migration for account "${plan.accountId}": ${plan.statePath}`, - ); + try { + await persistLegacyMigrationState({ + filePath: plan.statePath, + state, + writeJsonFileAtomically, + }); + changes.push( + `Prepared Matrix legacy encrypted-state migration for account "${plan.accountId}": ${plan.statePath}`, + ); + } catch (err) { + warnings.push( + `Failed writing Matrix legacy encrypted-state migration record for account "${plan.accountId}" (${plan.statePath}): ${String(err)}`, + ); + } } if (changes.length > 0) {