From 6fad3772fbe56a0d92aa48b1477d29bc068715d2 Mon Sep 17 00:00:00 2001 From: re2zero Date: Wed, 19 Aug 2026 02:33:34 +0800 Subject: [PATCH] fix(session): only advertise --resume when the session is on disk Persistence is lazy: getSessionFile() returns an allocated path from the start, but the JSONL is only materialized once an assistant message (or an explicit ensureOnDisk()) crosses the persistence gate. The shutdown banner printed the resume hint based solely on the path, so any session that ended before the first assistant message advertised a copy-pasteable command that always fails with Session not found. Gate the hint on the new SessionManager#isSessionOnDisk() (file exists in the active storage backend) and add unit tests. Fixes #8860 --- .../src/modes/interactive-mode.ts | 9 ++++- .../src/session/session-manager.ts | 15 +++++++ .../test/session-manager-on-disk-8860.test.ts | 40 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/session-manager-on-disk-8860.test.ts diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index ec6eb9c6b..6059b4dcc 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -4239,10 +4239,15 @@ export class InteractiveMode implements InteractiveModeContext { popTerminalTitle(); this.stop(); - // Print resumption hint if this is a persisted session + // Print resumption hint only if the session was actually materialized to + // durable storage. Persistence is lazy — a session that exits before its + // first assistant message (or dies early to an auth error, a mid-flight + // Ctrl+C, or a launch-then-quit) never wrote its JSONL, so the path is + // allocated but the file does not exist and `--resume ` would fail + // (issue #8860). const sessionId = this.sessionManager.getSessionId(); const sessionFile = this.sessionManager.getSessionFile(); - if (sessionId && sessionFile) { + if (sessionId && sessionFile && this.sessionManager.isSessionOnDisk()) { process.stderr.write(`\n${chalk.dim(`Resume this session with ${APP_NAME} --resume ${sessionId}`)}\n`); } diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 0bb9fb3e4..8aa0e6aec 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -1950,6 +1950,21 @@ export class SessionManager { return this.#sessionFile; } + /** + * Whether the current session has actually been materialized to durable + * storage (the JSONL exists on disk / in the active storage backend). + * + * Session persistence is lazy: the file is only written once the history + * contains an assistant message (or an explicit {@link ensureOnDisk} + * caller forces it). Until then {@link getSessionFile} returns an allocated + * path that leads nowhere, so a `--resume ` hint built from it would + * always fail. Consumers that advertise a resume command must gate on this + * (issue #8860). + */ + isSessionOnDisk(): boolean { + return !!this.#sessionFile && this.#storage.existsSync(this.#sessionFile); + } + getArtifactsDir(): string | null { if (this.#adoptedArtifactManager) return this.#adoptedArtifactManager.dir; return artifactsDirectoryFor(this.#sessionFile); diff --git a/packages/coding-agent/test/session-manager-on-disk-8860.test.ts b/packages/coding-agent/test/session-manager-on-disk-8860.test.ts new file mode 100644 index 000000000..2eee2146b --- /dev/null +++ b/packages/coding-agent/test/session-manager-on-disk-8860.test.ts @@ -0,0 +1,40 @@ +/** + * Issue #8860 — the exit banner advertises `--resume ` for sessions that + * were never written to disk. Persistence is lazy: `getSessionFile()` returns + * an allocated path from the start, but the JSONL is only materialized once the + * history crosses the persistence gate (first assistant message / explicit + * `ensureOnDisk()`). Consumers advertising a resume command must gate on + * `isSessionOnDisk()` instead of just the allocated path. + */ +import { describe, expect, it } from "bun:test"; +import { join } from "node:path"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { MemorySessionStorage } from "@oh-my-pi/pi-coding-agent/session/session-storage"; + +function freshSession(): SessionManager { + const cwd = join("/tmp", `omp-on-disk-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); + return SessionManager.create(cwd, join(cwd, "sessions"), new MemorySessionStorage()); +} + +describe("SessionManager.isSessionOnDisk (issue #8860)", () => { + it("returns false for a fresh lazy session whose JSONL was never materialized", () => { + const session = freshSession(); + expect(session.getSessionId()).not.toBe(""); + expect(session.getSessionFile()).toBeTruthy(); + // The path is allocated up front, but no file exists yet. + expect(session.isSessionOnDisk()).toBe(false); + }); + + it("returns true once ensureOnDisk() materializes the session file", async () => { + const session = freshSession(); + expect(session.isSessionOnDisk()).toBe(false); + await session.ensureOnDisk(); + expect(session.isSessionOnDisk()).toBe(true); + expect(session.getSessionFile()).toBeTruthy(); + }); + + it("stays false for an in-memory (non-persisting) session", () => { + const session = SessionManager.inMemory(); + expect(session.isSessionOnDisk()).toBe(false); + }); +});