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
This commit is contained in:
re2zero
2026-08-19 02:33:34 +08:00
parent 8500092296
commit 6fad3772fb
3 changed files with 62 additions and 2 deletions
@@ -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 <id>` 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`);
}
@@ -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 <id>` 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);
@@ -0,0 +1,40 @@
/**
* Issue #8860 — the exit banner advertises `--resume <id>` 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);
});
});