diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 9e1076e4a..7b7617685 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -942,12 +942,71 @@ function extractFirstUserPrompt(entries: Array>): string return undefined; } +/** + * Promote orphaned `.jsonl..bak` backups created by + * `#replaceSessionFileAfterEperm` back to their primary path when the primary + * is missing. This runs once per session-dir scan, before the main `*.jsonl` + * glob, so a crash between the two renames in the EPERM-rewrite path does not + * leave the user's last good state stranded outside the loader's view. + * + * Exported for testing. + */ +export async function recoverOrphanedBackups(sessionDir: string, storage: SessionStorage): Promise { + let backups: string[]; + try { + backups = storage.listFilesSync(sessionDir, "*.bak"); + } catch { + return; + } + if (backups.length === 0) return; + // For each primary path, pick the newest backup (highest mtime) as the recovery source. + const candidates = new Map(); + for (const backup of backups) { + const name = path.basename(backup); + // Expect "..bak" where ends in ".jsonl". + if (!name.endsWith(".bak")) continue; + const trimmed = name.slice(0, -".bak".length); + const dotIdx = trimmed.lastIndexOf("."); + if (dotIdx <= 0) continue; + const primaryName = trimmed.slice(0, dotIdx); + if (!primaryName.endsWith(".jsonl")) continue; + const primaryPath = path.join(sessionDir, primaryName); + let mtimeMs = 0; + try { + mtimeMs = storage.statSync(backup).mtimeMs; + } catch { + continue; + } + const existing = candidates.get(primaryPath); + if (!existing || mtimeMs > existing.mtimeMs) { + candidates.set(primaryPath, { backup, mtimeMs }); + } + } + for (const [primaryPath, { backup }] of candidates) { + if (storage.existsSync(primaryPath)) continue; + try { + await storage.rename(backup, primaryPath); + logger.warn("Recovered orphaned session backup", { + sessionFile: primaryPath, + backupPath: backup, + }); + } catch (err) { + logger.warn("Failed to recover orphaned session backup", { + sessionFile: primaryPath, + backupPath: backup, + error: toError(err).message, + }); + } + } +} + /** * Reads all session files from the directory and returns them sorted by mtime (newest first). * Uses low-level file I/O to efficiently read only the first 4KB of each file * to extract the JSON header and first user message without loading entire session logs into memory. */ async function getSortedSessions(sessionDir: string, storage: SessionStorage): Promise { + await recoverOrphanedBackups(sessionDir, storage); try { const files: string[] = storage.listFilesSync(sessionDir, "*.jsonl"); const sessions: RecentSessionInfo[] = []; @@ -2149,10 +2208,14 @@ export class SessionManager { } // Windows can reject overwrite-style rename with EPERM even after our own writer is closed. // Move the old session file aside first so a failed retry can roll back to the last good file. + // The backup uses a plain `..bak` name (no leading dot) so that if the + // process crashes between the two renames, `recoverOrphanedBackups` can find it via the + // shared `*.bak` glob on both real and in-memory storage backends and promote it back to + // the primary on the next session-dir scan. async #replaceSessionFileAfterEperm(tempPath: string, targetPath: string, renameError: unknown): Promise { const dir = path.resolve(targetPath, ".."); - const backupPath = path.join(dir, `.${path.basename(targetPath)}.${Snowflake.next()}.bak`); + const backupPath = path.join(dir, `${path.basename(targetPath)}.${Snowflake.next()}.bak`); try { await this.storage.rename(targetPath, backupPath); } catch (err) { @@ -2167,13 +2230,14 @@ export class SessionManager { await this.storage.rename(tempPath, targetPath); } catch (err) { const replaceError = toError(err); + const originalError = toError(renameError); try { await this.storage.rename(backupPath, targetPath); } catch (rollbackErr) { const rollbackError = toError(rollbackErr); throw new Error( - `Failed to replace session file after EPERM (${replaceError.message}); rollback from ${backupPath} also failed: ${rollbackError.message}`, - { cause: replaceError }, + `Failed to replace session file after EPERM (original: ${originalError.message}; retry: ${replaceError.message}); rollback from ${backupPath} also failed: ${rollbackError.message}`, + { cause: originalError }, ); } throw replaceError; @@ -3244,6 +3308,7 @@ export class SessionManager { ): Promise { const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); try { + await recoverOrphanedBackups(dir, storage); const files = storage.listFilesSync(dir, "*.jsonl"); return await collectSessionsFromFiles(files, storage); } catch { diff --git a/packages/coding-agent/test/session-manager/rewrite-rename-eperm.test.ts b/packages/coding-agent/test/session-manager/rewrite-rename-eperm.test.ts index 99c24d4d5..60f67109d 100644 --- a/packages/coding-agent/test/session-manager/rewrite-rename-eperm.test.ts +++ b/packages/coding-agent/test/session-manager/rewrite-rename-eperm.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { recoverOrphanedBackups, SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { MemorySessionStorage } from "@oh-my-pi/pi-coding-agent/session/session-storage"; class FsCodeError extends Error { @@ -59,3 +59,99 @@ describe("SessionManager rewrite EPERM replacement fallback", () => { await expect(session.flush()).resolves.toBeUndefined(); }); }); + +describe("SessionManager rewrite EPERM rollback failure", () => { + it("preserves the original EPERM as the thrown error's cause when rollback also fails", async () => { + class DoubleFailStorage extends MemorySessionStorage { + failureMode = false; + tempRenameAttempts = 0; + + rename(source: string, target: string): Promise { + if (!this.failureMode) return super.rename(source, target); + // Every temp -> target rename fails with EPERM (both the upstream attempt in + // #replaceSessionFile and the retry inside #replaceSessionFileAfterEperm). + if (source.includes(".tmp") && target.endsWith(".jsonl")) { + this.tempRenameAttempts++; + const tag = this.tempRenameAttempts === 1 ? "original" : "retry"; + return Promise.reject(new FsCodeError("EPERM", `EPERM ${tag}: rename '${source}' -> '${target}'`)); + } + // The rollback rename (backup -> target) fails with a distinct code. + if (source.endsWith(".bak") && target.endsWith(".jsonl")) { + return Promise.reject(new FsCodeError("EIO", `EIO rollback: rename '${source}' -> '${target}'`)); + } + return super.rename(source, target); + } + } + + const storage = new DoubleFailStorage(); + const session = SessionManager.create("/cwd", "/sessions", storage); + await session.ensureOnDisk(); + storage.failureMode = true; + const sessionFile = session.getSessionFile(); + if (!sessionFile) throw new Error("Expected session file"); + + let thrown: Error | undefined; + try { + await session.setSessionName("doomed", "user"); + } catch (err) { + thrown = err as Error; + } + if (!thrown) throw new Error("Expected setSessionName to reject"); + // Message text MUST surface both the retry failure and the rollback failure. + expect(thrown.message).toContain("rollback"); + expect(thrown.message).toContain("EIO rollback"); + expect(thrown.message).toContain("EPERM retry"); + // `cause` MUST be the original upstream EPERM that started the fallback path, + // not the second/retry failure or the rollback failure. + const cause = thrown.cause as Error | undefined; + expect(cause).toBeInstanceOf(Error); + expect(cause?.message).toContain("EPERM original"); + }); +}); + +describe("recoverOrphanedBackups", () => { + it("promotes an orphaned .jsonl..bak back to the primary path when the primary is missing", async () => { + const storage = new MemorySessionStorage(); + const dir = "/sessions/proj"; + const primary = `${dir}/session-abc.jsonl`; + const backup = `${primary}.1700000000000.bak`; + storage.writeTextSync(backup, '{"type":"session","id":"abc"}\n'); + + await recoverOrphanedBackups(dir, storage); + + expect(storage.existsSync(primary)).toBe(true); + expect(storage.existsSync(backup)).toBe(false); + expect(storage.readTextSync(primary)).toBe('{"type":"session","id":"abc"}\n'); + }); + + it("leaves the backup alone when the primary already exists", async () => { + const storage = new MemorySessionStorage(); + const dir = "/sessions/proj"; + const primary = `${dir}/session-xyz.jsonl`; + const backup = `${primary}.1700000000000.bak`; + storage.writeTextSync(primary, '{"type":"session","id":"xyz","keep":true}\n'); + storage.writeTextSync(backup, '{"type":"session","id":"xyz","stale":true}\n'); + + await recoverOrphanedBackups(dir, storage); + + expect(storage.readTextSync(primary)).toContain('"keep":true'); + expect(storage.existsSync(backup)).toBe(true); + }); + + it("picks the newest backup when multiple orphans exist for the same primary", async () => { + const storage = new MemorySessionStorage(); + const dir = "/sessions/proj"; + const primary = `${dir}/session-multi.jsonl`; + const older = `${primary}.100.bak`; + const newer = `${primary}.200.bak`; + storage.writeTextSync(older, "older"); + // Force the newer backup to have a strictly higher mtime so recovery is deterministic. + await Bun.sleep(5); + storage.writeTextSync(newer, "newer"); + + await recoverOrphanedBackups(dir, storage); + + expect(storage.existsSync(primary)).toBe(true); + expect(storage.readTextSync(primary)).toBe("newer"); + }); +});