fix(session-manager): added orphaned backup recovery after EPERM rename
- Added `recoverOrphanedBackups` to promote `.jsonl..bak` files back to their primary path when the primary is missing, preventing data loss after a mid-rename crash. - Changed backup filename from dot-prefixed to plain `..bak` so the shared `*.bak` glob can find it on both real and in-memory storage backends. - Surfaced the original EPERM as the error `cause` and included both original and retry messages when rollback also fails.
This commit is contained in:
@@ -942,12 +942,71 @@ function extractFirstUserPrompt(entries: Array<Record<string, unknown>>): string
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Promote orphaned `<basename>.jsonl.<snowflake>.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<void> {
|
||||
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<string, { backup: string; mtimeMs: number }>();
|
||||
for (const backup of backups) {
|
||||
const name = path.basename(backup);
|
||||
// Expect "<primary>.<snowflake>.bak" where <primary> 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<RecentSessionInfo[]> {
|
||||
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 `<basename>.<snowflake>.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<void> {
|
||||
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<SessionInfo[]> {
|
||||
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 {
|
||||
|
||||
@@ -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<void> {
|
||||
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 <basename>.jsonl.<snowflake>.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");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user