fix(session): persist /new boundary so autoResume does not resume pre-/new transcript
New-session persistence is lazy: after `/new`, the JSONL is not created until assistant output exists. Exiting before any assistant message left the per-terminal breadcrumb pointing at a not-yet-materialized file, which `readTerminalBreadcrumbEntry` rejected (missing target), so `continueRecent` fell back to `findMostRecentSession` and resurrected the pre-`/new` transcript. `/new` now records a durable `fresh` breadcrumb boundary. The reader returns a fresh breadcrumb even when its target is absent (with `exists:false`), and `continueRecent` honors it by starting fresh instead of falling back. The crumb is re-stamped non-fresh once the session materializes, so a genuinely stale/deleted breadcrumb still falls back to the most-recent session. The breadcrumb write is now synchronous so the fresh->materialized re-stamp cannot reorder. Fixes #5730
This commit is contained in:
roboomp
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `autoResume` crossing an explicit `/new` boundary: after `/new` a new session's JSONL is created lazily (only once assistant output exists), so exiting before any assistant message left the per-terminal breadcrumb pointing at a not-yet-materialized file. `readTerminalBreadcrumbEntry` rejected the missing target and `continueRecent()` fell back to the most-recent session — the pre-`/new` transcript — processing the next prompt with stale context. `/new` now records a durable `fresh` breadcrumb boundary that `continueRecent()` honors (starting fresh) even when the target is absent, while a genuinely stale/deleted breadcrumb still falls back to the most-recent session ([#5730](https://github.com/can1357/oh-my-pi/issues/5730)).
|
||||
|
||||
## [17.0.1] - 2026-07-16
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -445,6 +445,13 @@ export class SessionManager {
|
||||
#inMemoryArtifactCounter = 0;
|
||||
|
||||
#suppressBreadcrumb = false;
|
||||
/**
|
||||
* The last breadcrumb this manager wrote marked a lazy `/new` boundary whose
|
||||
* JSONL is not yet on disk. Cleared (and the crumb re-stamped non-fresh) once
|
||||
* the session materializes, so a materialized-then-deleted session still falls
|
||||
* back to the most-recent session instead of being treated as a fresh crumb.
|
||||
*/
|
||||
#breadcrumbFresh = false;
|
||||
#sessionNameChangedCallbacks = new Set<() => void>();
|
||||
|
||||
private constructor(cwd: string, sessionDir: string, persist: boolean, storage: SessionStorage) {
|
||||
@@ -457,8 +464,18 @@ export class SessionManager {
|
||||
if (persist && sessionDir) this.#storage.ensureDirSync(sessionDir);
|
||||
}
|
||||
|
||||
#rememberBreadcrumb(cwd: string, sessionFile: string): void {
|
||||
if (!this.#suppressBreadcrumb) writeTerminalBreadcrumb(cwd, sessionFile);
|
||||
#rememberBreadcrumb(cwd: string, sessionFile: string, fresh = false): void {
|
||||
this.#breadcrumbFresh = fresh;
|
||||
if (!this.#suppressBreadcrumb) writeTerminalBreadcrumb(cwd, sessionFile, fresh);
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-stamp a fresh `/new` breadcrumb as non-fresh once the session has
|
||||
* materialized on disk. A no-op unless the current breadcrumb is still fresh.
|
||||
*/
|
||||
#materializeBreadcrumb(): void {
|
||||
if (!this.#breadcrumbFresh || !this.#sessionFile) return;
|
||||
this.#rememberBreadcrumb(this.#cwd, this.#sessionFile, false);
|
||||
}
|
||||
|
||||
#clearDiskError(): void {
|
||||
@@ -600,6 +617,7 @@ export class SessionManager {
|
||||
this.#closeWriterEventually();
|
||||
this.#storage.writeTextSync(this.#sessionFile, body);
|
||||
this.#fileIsCurrent = true;
|
||||
this.#materializeBreadcrumb();
|
||||
this.#rewriteRequired = false;
|
||||
this.#hasTitleSlot = true;
|
||||
} catch (err) {
|
||||
@@ -625,6 +643,7 @@ export class SessionManager {
|
||||
async () => {
|
||||
if (await this.#runFencedAtomicRewrite(startEpoch)) {
|
||||
this.#fileIsCurrent = true;
|
||||
this.#materializeBreadcrumb();
|
||||
this.#rewriteRequired = false;
|
||||
this.#hasTitleSlot = true;
|
||||
}
|
||||
@@ -807,7 +826,7 @@ export class SessionManager {
|
||||
this.#sessionFile =
|
||||
forcedSessionFile ??
|
||||
path.join(this.#sessionDir, `${fileSafeTimestamp(timestamp)}_${this.#sessionId}.jsonl`);
|
||||
this.#rememberBreadcrumb(this.#cwd, this.#sessionFile);
|
||||
this.#rememberBreadcrumb(this.#cwd, this.#sessionFile, true);
|
||||
} else {
|
||||
this.#sessionFile = undefined;
|
||||
}
|
||||
@@ -1998,6 +2017,18 @@ export class SessionManager {
|
||||
let chosenSession: string | null | undefined;
|
||||
|
||||
if (breadcrumb) {
|
||||
// A fresh `/new` boundary whose JSONL was never materialized (lazy
|
||||
// new-session persistence, then a process exit before any assistant
|
||||
// output). Honor the boundary: start fresh rather than falling back to
|
||||
// findMostRecentSession(), which would resurrect the pre-`/new`
|
||||
// transcript. A materialized (or genuinely stale/deleted) crumb reports
|
||||
// exists=false only when fresh, so this never masks a real stale crumb.
|
||||
if (breadcrumb.fresh && !breadcrumb.exists) {
|
||||
const manager = new SessionManager(cwd, dir, true, storage);
|
||||
manager.#resetToNewSession();
|
||||
return manager;
|
||||
}
|
||||
|
||||
// Recover stale crumbs: a subagent open (pre-fix) may have pointed this
|
||||
// terminal's breadcrumb at an artifact child; resume the parent instead.
|
||||
breadcrumb.sessionFile = resolveBreadcrumbToInteractiveRoot(breadcrumb.sessionFile);
|
||||
|
||||
@@ -146,28 +146,54 @@ export function computeDefaultSessionDir(
|
||||
* Write a breadcrumb linking the current terminal to a session file.
|
||||
* The breadcrumb contains the cwd and session path so --continue can
|
||||
* find "this terminal's last session" even when running concurrent instances.
|
||||
*
|
||||
* `fresh` marks a `/new` (or freshly-minted) session boundary whose JSONL is
|
||||
* not yet materialized (new-session persistence is lazy until assistant output
|
||||
* exists). A fresh breadcrumb is honored by {@link readTerminalBreadcrumbEntry}
|
||||
* even when its target file is still absent, so relaunch/auto-resume reopens the
|
||||
* post-`/new` session instead of falling back to the pre-`/new` transcript. Once
|
||||
* the session materializes the caller rewrites the breadcrumb with `fresh:false`
|
||||
* so a later external delete is still treated as a genuinely stale crumb.
|
||||
*/
|
||||
export function writeTerminalBreadcrumb(cwd: string, sessionFile: string): void {
|
||||
export function writeTerminalBreadcrumb(cwd: string, sessionFile: string, fresh = false): void {
|
||||
const terminalId = getTerminalId();
|
||||
if (!terminalId) return;
|
||||
|
||||
const breadcrumbDir = getTerminalSessionsDir();
|
||||
const breadcrumbFile = path.join(breadcrumbDir, terminalId);
|
||||
const content = `${cwd}\n${sessionFile}\n`;
|
||||
// Best-effort — don't break session creation if breadcrumb fails
|
||||
Bun.write(breadcrumbFile, content).catch(() => {});
|
||||
const content = fresh ? `${cwd}\n${sessionFile}\nfresh\n` : `${cwd}\n${sessionFile}\n`;
|
||||
// Synchronous + best-effort. Infrequent (session create/switch/reset, never
|
||||
// per-append), and writing in order matters: a lazy `/new` fresh crumb is
|
||||
// re-stamped non-fresh the instant the session materializes, so an async
|
||||
// fire-and-forget could land the two writes out of order and leave a
|
||||
// materialized session marked fresh.
|
||||
try {
|
||||
fs.mkdirSync(breadcrumbDir, { recursive: true });
|
||||
fs.writeFileSync(breadcrumbFile, content);
|
||||
} catch (err) {
|
||||
if (!isEnoent(err)) logger.debug("Terminal breadcrumb write failed", { err });
|
||||
}
|
||||
}
|
||||
|
||||
export interface TerminalBreadcrumb {
|
||||
cwd: string;
|
||||
sessionFile: string;
|
||||
/** The recorded session file exists on disk right now. */
|
||||
exists: boolean;
|
||||
/** Recorded as a `/new` fresh-session boundary whose JSONL may not exist yet. */
|
||||
fresh: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the raw terminal breadcrumb for the current terminal.
|
||||
* Returns the recorded cwd + session file (verified to exist) regardless of
|
||||
* whether the recorded cwd still matches the current one. Callers decide how
|
||||
* to interpret a cwd mismatch (e.g. a moved/renamed worktree).
|
||||
* Returns the recorded cwd + session file regardless of whether the recorded
|
||||
* cwd still matches the current one. Callers decide how to interpret a cwd
|
||||
* mismatch (e.g. a moved/renamed worktree).
|
||||
*
|
||||
* A missing target file yields `null` UNLESS the breadcrumb is a `fresh`
|
||||
* boundary — a lazy `/new` session whose JSONL was never written — in which case
|
||||
* the entry is returned with `exists:false` so the caller can distinguish it
|
||||
* from a genuinely stale/deleted breadcrumb.
|
||||
*/
|
||||
export async function readTerminalBreadcrumbEntry(): Promise<TerminalBreadcrumb | null> {
|
||||
const terminalId = getTerminalId();
|
||||
@@ -181,10 +207,13 @@ export async function readTerminalBreadcrumbEntry(): Promise<TerminalBreadcrumb
|
||||
|
||||
const breadcrumbCwd = lines[0];
|
||||
const sessionFile = lines[1];
|
||||
const fresh = lines[2] === "fresh";
|
||||
|
||||
// Verify the session file still exists
|
||||
const stat = fs.statSync(sessionFile, { throwIfNoEntry: false });
|
||||
if (stat?.isFile()) return { cwd: breadcrumbCwd, sessionFile };
|
||||
const exists = stat?.isFile() === true;
|
||||
// A materialized target resumes normally; a missing target is honored only
|
||||
// for a fresh `/new` boundary (never-written lazy session).
|
||||
if (exists || fresh) return { cwd: breadcrumbCwd, sessionFile, exists, fresh };
|
||||
} catch (err) {
|
||||
if (!isEnoent(err)) logger.debug("Terminal breadcrumb read failed", { err });
|
||||
// Breadcrumb doesn't exist or is corrupt — fall through
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as fsp from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { getConfigRootDir, setAgentDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
import { makeAssistantMessage } from "./helpers";
|
||||
|
||||
describe("SessionManager.continueRecent /new boundary", () => {
|
||||
let testAgentDir: string;
|
||||
let cwd: string;
|
||||
const originalAgentDir = process.env.PI_CODING_AGENT_DIR;
|
||||
const originalTmuxPane = process.env.TMUX_PANE;
|
||||
const fallbackAgentDir = path.join(getConfigRootDir(), "agent");
|
||||
|
||||
beforeEach(async () => {
|
||||
// Deterministic, non-TTY terminal id so breadcrumb read/write is stable.
|
||||
process.env.TMUX_PANE = "%new-boundary-test";
|
||||
testAgentDir = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-new-boundary-"));
|
||||
setAgentDir(testAgentDir);
|
||||
cwd = path.join(testAgentDir, "project");
|
||||
fs.mkdirSync(cwd, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (originalTmuxPane === undefined) delete process.env.TMUX_PANE;
|
||||
else process.env.TMUX_PANE = originalTmuxPane;
|
||||
if (originalAgentDir) {
|
||||
setAgentDir(originalAgentDir);
|
||||
} else {
|
||||
setAgentDir(fallbackAgentDir);
|
||||
delete process.env.PI_CODING_AGENT_DIR;
|
||||
}
|
||||
await fsp.rm(testAgentDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("does not resume the pre-/new transcript when the new session produced no output", async () => {
|
||||
// Persisted old session with recognizable context (assistant output → file on disk).
|
||||
const old = SessionManager.create(cwd);
|
||||
old.appendMessage({ role: "user", content: "pre-new work", timestamp: 1 });
|
||||
old.appendMessage(makeAssistantMessage());
|
||||
await old.flush();
|
||||
const oldFile = old.getSessionFile();
|
||||
if (!oldFile) throw new Error("Expected persisted old session file");
|
||||
await old.close();
|
||||
|
||||
// Resume it, then hit an explicit `/new` boundary and exit before any
|
||||
// assistant output — the new session's JSONL is never materialized (lazy).
|
||||
const resumed = await SessionManager.continueRecent(cwd);
|
||||
expect(JSON.stringify(resumed.getEntries())).toContain("pre-new work");
|
||||
await resumed.newSession();
|
||||
const freshFile = resumed.getSessionFile();
|
||||
if (!freshFile) throw new Error("Expected a fresh session file path");
|
||||
expect(path.resolve(freshFile)).not.toBe(path.resolve(oldFile));
|
||||
expect(fs.existsSync(freshFile)).toBe(false); // lazy: not yet on disk
|
||||
await resumed.close();
|
||||
|
||||
// Relaunch with auto-resume: must NOT fall back to the pre-/new transcript.
|
||||
const relaunched = await SessionManager.continueRecent(cwd);
|
||||
try {
|
||||
const dump = JSON.stringify(relaunched.getEntries());
|
||||
expect(dump).not.toContain("pre-new work");
|
||||
expect(relaunched.getEntries()).toHaveLength(0);
|
||||
// Reopens the fresh session established by `/new`, not the old file.
|
||||
expect(path.resolve(relaunched.getSessionFile() ?? "")).not.toBe(path.resolve(oldFile));
|
||||
} finally {
|
||||
await relaunched.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("still falls back to the most-recent session for a genuinely stale breadcrumb", async () => {
|
||||
// A normal persisted session (survives).
|
||||
const first = SessionManager.create(cwd);
|
||||
first.appendMessage({ role: "user", content: "first session", timestamp: 1 });
|
||||
first.appendMessage(makeAssistantMessage());
|
||||
await first.flush();
|
||||
await first.close();
|
||||
|
||||
// A distinct second session becomes the terminal's breadcrumb target and
|
||||
// materializes on disk (re-stamped non-fresh), then is externally deleted.
|
||||
const second = SessionManager.create(cwd);
|
||||
second.appendMessage({ role: "user", content: "second session", timestamp: 1 });
|
||||
second.appendMessage(makeAssistantMessage());
|
||||
await second.flush();
|
||||
const secondFile = second.getSessionFile();
|
||||
if (!secondFile) throw new Error("Expected persisted second session file");
|
||||
await second.close();
|
||||
await fsp.rm(secondFile, { force: true });
|
||||
|
||||
const relaunched = await SessionManager.continueRecent(cwd);
|
||||
try {
|
||||
// Materialized-then-deleted target (non-fresh) → fall back to the
|
||||
// most-recent surviving session, not a fresh empty one.
|
||||
expect(JSON.stringify(relaunched.getEntries())).toContain("first session");
|
||||
} finally {
|
||||
await relaunched.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user