From f6f7609b6655f8d09f4674dec8c91c6ecc6b2511 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 3 Apr 2026 20:25:49 -0400 Subject: [PATCH] matrix: retry credentials after legacy migration race (#60591) Merged via squash. Prepared head SHA: e050b39de022f15f21de27bd859390ee522b4829 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + .../matrix/src/matrix/credentials-read.ts | 77 ++++++++--- .../matrix/src/matrix/credentials.test.ts | 129 ++++++++++++++++++ 3 files changed, 191 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 955d642e81e..e8d0eea6bf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,7 @@ Docs: https://docs.openclaw.ai - Agents/fallback: persist selected fallback overrides before retry attempts start, prefer persisted overrides during live-session reconciliation, and keep provider-scoped auth-profile failover from snapping retries back to stale primary selections. - Agents/MCP: sort MCP tools deterministically by name so the tools block in API requests is stable across turns, preventing unnecessary prompt-cache busting from non-deterministic `listTools()` order. (#58037) Thanks @bcherny. - Infra/json-file: preserve symlink-backed JSON stores and Windows overwrite fallback when atomically saving small sync JSON state files. (#60589) Thanks @gumadeiras. +- Matrix/credentials: read the current and legacy credential files directly during migration fallback so concurrent legacy rename races still resolve to the stored credentials. (#60591) Thanks @gumadeiras. ## 2026.4.2 diff --git a/extensions/matrix/src/matrix/credentials-read.ts b/extensions/matrix/src/matrix/credentials-read.ts index 1617196a665..4222f78fa2d 100644 --- a/extensions/matrix/src/matrix/credentials-read.ts +++ b/extensions/matrix/src/matrix/credentials-read.ts @@ -21,6 +21,18 @@ export type MatrixStoredCredentials = { lastUsedAt?: string; }; +type MatrixCredentialsSource = "current" | "legacy"; + +type MatrixCredentialsFileLoadResult = + | { + kind: "loaded"; + source: MatrixCredentialsSource; + credentials: MatrixStoredCredentials | null; + } + | { + kind: "missing"; + }; + function resolveStateDir(env: NodeJS.ProcessEnv): string { try { return getMatrixRuntime().state.resolveStateDir(env, os.homedir); @@ -36,7 +48,7 @@ function resolveStateDir(env: NodeJS.ProcessEnv): string { } } -function resolveLegacyMatrixCredentialsPath(env: NodeJS.ProcessEnv): string | null { +function resolveLegacyMatrixCredentialsPath(env: NodeJS.ProcessEnv): string { return path.join(resolveMatrixCredentialsDir(env), "credentials.json"); } @@ -76,6 +88,35 @@ function parseMatrixCredentialsFile(filePath: string): MatrixStoredCredentials | return parsed as MatrixStoredCredentials; } +function loadMatrixCredentialsFile( + filePath: string, + source: MatrixCredentialsSource, +): MatrixCredentialsFileLoadResult { + try { + return { + kind: "loaded", + source, + credentials: parseMatrixCredentialsFile(filePath), + }; + } catch (error) { + if ((error as NodeJS.ErrnoException)?.code === "ENOENT") { + return { kind: "missing" }; + } + throw error; + } +} + +function loadLegacyMatrixCredentialsWithCurrentFallback(params: { + legacyPath: string; + currentPath: string; +}): MatrixCredentialsFileLoadResult { + const legacy = loadMatrixCredentialsFile(params.legacyPath, "legacy"); + if (legacy.kind === "loaded") { + return legacy; + } + return loadMatrixCredentialsFile(params.currentPath, "current"); +} + export function resolveMatrixCredentialsDir( env: NodeJS.ProcessEnv = process.env, stateDir?: string, @@ -96,30 +137,36 @@ export function loadMatrixCredentials( env: NodeJS.ProcessEnv = process.env, accountId?: string | null, ): MatrixStoredCredentials | null { - const credPath = resolveMatrixCredentialsPath(env, accountId); + const currentPath = resolveMatrixCredentialsPath(env, accountId); try { - if (fs.existsSync(credPath)) { - return parseMatrixCredentialsFile(credPath); + const current = loadMatrixCredentialsFile(currentPath, "current"); + if (current.kind === "loaded") { + return current.credentials; } const legacyPath = resolveLegacyMigrationSourcePath(env, accountId); - if (!legacyPath || !fs.existsSync(legacyPath)) { + if (!legacyPath) { return null; } - const parsed = parseMatrixCredentialsFile(legacyPath); - if (!parsed) { + const loaded = loadLegacyMatrixCredentialsWithCurrentFallback({ + legacyPath, + currentPath, + }); + if (loaded.kind !== "loaded" || !loaded.credentials) { return null; } - try { - fs.mkdirSync(path.dirname(credPath), { recursive: true }); - fs.renameSync(legacyPath, credPath); - } catch { - // Keep returning the legacy credentials even if migration fails. + if (loaded.source === "legacy") { + try { + fs.mkdirSync(path.dirname(currentPath), { recursive: true }); + fs.renameSync(legacyPath, currentPath); + } catch { + // Keep returning the legacy credentials even if migration fails. + } } - return parsed; + return loaded.credentials; } catch { return null; } @@ -138,9 +185,7 @@ export function clearMatrixCredentials( continue; } try { - if (fs.existsSync(filePath)) { - fs.unlinkSync(filePath); - } + fs.unlinkSync(filePath); } catch { // ignore } diff --git a/extensions/matrix/src/matrix/credentials.test.ts b/extensions/matrix/src/matrix/credentials.test.ts index dbd69125f82..aa1ed5aa2ae 100644 --- a/extensions/matrix/src/matrix/credentials.test.ts +++ b/extensions/matrix/src/matrix/credentials.test.ts @@ -16,6 +16,7 @@ describe("matrix credentials storage", () => { const tempDirs: string[] = []; afterEach(() => { + vi.restoreAllMocks(); for (const dir of tempDirs.splice(0)) { fs.rmSync(dir, { recursive: true, force: true }); } @@ -113,6 +114,134 @@ describe("matrix credentials storage", () => { expect(fs.existsSync(currentPath)).toBe(true); }); + it("returns migrated credentials when another process moves the legacy file mid-read", () => { + const stateDir = setupStateDir({ + channels: { + matrix: { + accounts: { + ops: {}, + }, + }, + }, + }); + const legacyPath = path.join(stateDir, "credentials", "matrix", "credentials.json"); + const currentPath = resolveMatrixCredentialsPath({}, "ops"); + fs.mkdirSync(path.dirname(legacyPath), { recursive: true }); + fs.writeFileSync( + legacyPath, + JSON.stringify({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "legacy-token", + createdAt: "2026-03-01T10:00:00.000Z", + }), + ); + + const originalReadFileSync = fs.readFileSync.bind(fs); + let moved = false; + const readFileSpy = vi.spyOn(fs, "readFileSync").mockImplementation((( + filePath: fs.PathOrFileDescriptor, + options?: fs.ObjectEncodingOptions | BufferEncoding | null, + ) => { + if (!moved && filePath === legacyPath) { + fs.renameSync(legacyPath, currentPath); + moved = true; + } + return originalReadFileSync(filePath, options as never); + }) as typeof fs.readFileSync); + try { + const loaded = loadMatrixCredentials({}, "ops"); + + expect(loaded?.accessToken).toBe("legacy-token"); + expect(moved).toBe(true); + expect(fs.existsSync(legacyPath)).toBe(false); + expect(fs.existsSync(currentPath)).toBe(true); + } finally { + readFileSpy.mockRestore(); + } + }); + + it("does not rename the legacy path after falling back to already-migrated current credentials", () => { + const stateDir = setupStateDir({ + channels: { + matrix: { + accounts: { + ops: {}, + }, + }, + }, + }); + const legacyPath = path.join(stateDir, "credentials", "matrix", "credentials.json"); + const currentPath = resolveMatrixCredentialsPath({}, "ops"); + fs.mkdirSync(path.dirname(legacyPath), { recursive: true }); + fs.writeFileSync( + legacyPath, + JSON.stringify({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "legacy-token", + createdAt: "2026-03-01T10:00:00.000Z", + }), + ); + + const originalReadFileSync = fs.readFileSync.bind(fs); + const originalRenameSync = fs.renameSync.bind(fs); + const renameSpy = vi.spyOn(fs, "renameSync"); + let migrated = false; + const readFileSpy = vi.spyOn(fs, "readFileSync").mockImplementation((( + filePath: fs.PathOrFileDescriptor, + options?: fs.ObjectEncodingOptions | BufferEncoding | null, + ) => { + if (!migrated && filePath === legacyPath && fs.existsSync(legacyPath)) { + originalRenameSync(legacyPath, currentPath); + fs.writeFileSync( + currentPath, + JSON.stringify({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "current-token", + createdAt: "2026-03-01T10:00:00.000Z", + }), + ); + migrated = true; + try { + return originalReadFileSync(filePath, options as never); + } finally { + fs.writeFileSync( + legacyPath, + JSON.stringify({ + homeserver: "https://matrix.example.org", + userId: "@bot:example.org", + accessToken: "recreated-stale-legacy-token", + createdAt: "2026-03-01T10:00:00.000Z", + }), + ); + } + } + return originalReadFileSync(filePath, options as never); + }) as typeof fs.readFileSync); + + try { + const loaded = loadMatrixCredentials({}, "ops"); + + expect(loaded?.accessToken).toBe("current-token"); + expect(renameSpy).not.toHaveBeenCalled(); + expect( + JSON.parse(fs.readFileSync(currentPath, "utf8")) as { accessToken: string }, + ).toMatchObject({ + accessToken: "current-token", + }); + expect( + JSON.parse(fs.readFileSync(legacyPath, "utf8")) as { accessToken: string }, + ).toMatchObject({ + accessToken: "recreated-stale-legacy-token", + }); + } finally { + readFileSpy.mockRestore(); + renameSpy.mockRestore(); + } + }); + it("does not migrate legacy default credentials during a non-selected account read", () => { const stateDir = setupStateDir({ channels: {