fix(coding-agent/session): suppressed subagent breadcrumbs so --continue resumes the parent session
- Added a `suppressBreadcrumb` option to `SessionManager.open()` so headless opens skip writing the per-TTY `--continue` breadcrumb, and passed it from the subagent opens in `task/executor.ts` and the HTML export open in `export/html/index.ts`, which run in the parent's terminal and were clobbering the breadcrumb with their own artifact-dir session file. - Added `resolveBreadcrumbToInteractiveRoot()` and applied it in `continueRecent()` so already-poisoned breadcrumbs pointing inside a parent's artifacts dir (`<parent>/<agentId>.jsonl`) resolve back up to the top-level interactive session. - Added `subagent-breadcrumb.test.ts` covering both that a subagent open keeps `--continue` on the parent and that a stale subagent-pointing breadcrumb is recovered.
This commit is contained in:
@@ -242,7 +242,7 @@ export async function exportFromFile(inputPath: string, options?: ExportOptions
|
||||
|
||||
let sm: SessionManager;
|
||||
try {
|
||||
sm = await SessionManager.open(inputPath);
|
||||
sm = await SessionManager.open(inputPath, undefined, undefined, { suppressBreadcrumb: true });
|
||||
} catch (err) {
|
||||
if (isEnoent(err)) throw new Error(`File not found: ${inputPath}`);
|
||||
throw err;
|
||||
|
||||
@@ -71,6 +71,27 @@ function artifactsDirectoryFor(sessionFile: string | undefined): string | null {
|
||||
return sessionFile ? sessionFile.slice(0, -JSONL_SUFFIX_LENGTH) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a breadcrumb's recorded session file to its interactive root. Subagent
|
||||
* (and other artifact) sessions live inside a parent session's artifacts dir —
|
||||
* `<parent>.jsonl` strips its suffix to `<parent>/`, and a child writes
|
||||
* `<parent>/<agentId>.jsonl`. A breadcrumb that points at such a child — a
|
||||
* pre-fix poisoned crumb left by a subagent that opened in the parent's TTY, or
|
||||
* any nested artifact — must resolve back up to the top-level session so
|
||||
* `--continue` resumes the real conversation instead of a subagent transcript.
|
||||
*/
|
||||
function resolveBreadcrumbToInteractiveRoot(sessionFile: string): string {
|
||||
let current = path.resolve(sessionFile);
|
||||
// Walk up while the containing dir is itself a session's artifacts dir
|
||||
// (`<dir>.jsonl` exists). Capped to defend against pathological layouts.
|
||||
for (let depth = 0; depth < 8; depth++) {
|
||||
const parentSessionFile = `${path.dirname(current)}.jsonl`;
|
||||
if (!fs.existsSync(parentSessionFile)) return current;
|
||||
current = parentSessionFile;
|
||||
}
|
||||
return current;
|
||||
}
|
||||
|
||||
function emptyUsageStatistics(): UsageStatistics {
|
||||
return { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, premiumRequests: 0, cost: 0 };
|
||||
}
|
||||
@@ -1528,13 +1549,14 @@ export class SessionManager {
|
||||
filePath: string,
|
||||
sessionDir?: string,
|
||||
storage: SessionStorage = new FileSessionStorage(),
|
||||
options?: { initialCwd?: string },
|
||||
options?: { initialCwd?: string; suppressBreadcrumb?: boolean },
|
||||
): Promise<SessionManager> {
|
||||
const loaded = await loadEntriesFromFile(filePath, storage);
|
||||
const header = loaded.find(entry => entry.type === "session") as SessionHeader | undefined;
|
||||
const cwd = header?.cwd ?? options?.initialCwd ?? getProjectDir();
|
||||
const dir = sessionDir ?? path.dirname(path.resolve(filePath));
|
||||
const manager = new SessionManager(cwd, dir, true, storage);
|
||||
manager.#suppressBreadcrumb = options?.suppressBreadcrumb === true;
|
||||
await manager.setSessionFile(filePath);
|
||||
return manager;
|
||||
}
|
||||
@@ -1551,6 +1573,9 @@ export class SessionManager {
|
||||
let chosenSession: string | null | undefined;
|
||||
|
||||
if (breadcrumb) {
|
||||
// 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);
|
||||
const breadcrumbCwd = path.resolve(breadcrumb.cwd);
|
||||
if (breadcrumbCwd === resolvedCwd) {
|
||||
chosenSession = breadcrumb.sessionFile;
|
||||
|
||||
@@ -1927,7 +1927,12 @@ export async function runSubprocess(options: ExecutorOptions): Promise<SingleRes
|
||||
|
||||
const effectiveCwd = worktree ?? cwd;
|
||||
const sessionManager = sessionFile
|
||||
? await awaitAbortable(SessionManager.open(sessionFile, undefined, undefined, { initialCwd: effectiveCwd }))
|
||||
? await awaitAbortable(
|
||||
SessionManager.open(sessionFile, undefined, undefined, {
|
||||
initialCwd: effectiveCwd,
|
||||
suppressBreadcrumb: true,
|
||||
}),
|
||||
)
|
||||
: SessionManager.inMemory(effectiveCwd);
|
||||
if (options.parentArtifactManager) {
|
||||
sessionManager.adoptArtifactManager(options.parentArtifactManager);
|
||||
@@ -2048,7 +2053,9 @@ export async function runSubprocess(options: ExecutorOptions): Promise<SingleRes
|
||||
// (createAgentSession → agent.replaceMessages). Isolated runs are not
|
||||
// resumable (worktree is merged + cleaned) and never get a reviver.
|
||||
reviveSession = async () => {
|
||||
const reopened = await SessionManager.open(sessionFile);
|
||||
const reopened = await SessionManager.open(sessionFile, undefined, undefined, {
|
||||
suppressBreadcrumb: true,
|
||||
});
|
||||
if (options.parentArtifactManager) {
|
||||
reopened.adoptArtifactManager(options.parentArtifactManager);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
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 { readTerminalBreadcrumbEntry } from "@oh-my-pi/pi-coding-agent/session/session-paths";
|
||||
import { getTerminalId } from "@oh-my-pi/pi-tui";
|
||||
import { getConfigRootDir, getTerminalSessionsDir, setAgentDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
import { makeAssistantMessage } from "./helpers";
|
||||
|
||||
const JSONL_SUFFIX = ".jsonl";
|
||||
|
||||
/** Synchronously seed the per-terminal breadcrumb (write is otherwise fire-and-forget). */
|
||||
function writeBreadcrumb(cwd: string, sessionFile: string): void {
|
||||
const terminalId = getTerminalId();
|
||||
if (!terminalId) throw new Error("Expected a terminal id for breadcrumb test");
|
||||
const dir = getTerminalSessionsDir();
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, terminalId), `${cwd}\n${sessionFile}\n`);
|
||||
}
|
||||
|
||||
/** Materialize a subagent session file under the parent's artifacts dir (`<parent>/<id>.jsonl`). */
|
||||
async function writeSubagentSession(parentFile: string, agentId: string, userText: string): Promise<string> {
|
||||
const artifactsDir = parentFile.slice(0, -JSONL_SUFFIX.length);
|
||||
fs.mkdirSync(artifactsDir, { recursive: true });
|
||||
const subFile = path.join(artifactsDir, `${agentId}.jsonl`);
|
||||
// Subagents open in the parent's TTY; suppression is what keeps them off the breadcrumb.
|
||||
const sub = await SessionManager.open(subFile, undefined, undefined, {
|
||||
initialCwd: path.dirname(parentFile),
|
||||
suppressBreadcrumb: true,
|
||||
});
|
||||
sub.appendMessage({ role: "user", content: userText, timestamp: 2 });
|
||||
sub.appendMessage(makeAssistantMessage());
|
||||
await sub.flush();
|
||||
await sub.close();
|
||||
return subFile;
|
||||
}
|
||||
|
||||
describe("SessionManager subagent breadcrumb isolation", () => {
|
||||
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 = "%subagent-breadcrumb-test";
|
||||
testAgentDir = await fsp.mkdtemp(path.join(os.tmpdir(), "omp-subagent-crumb-"));
|
||||
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 });
|
||||
});
|
||||
|
||||
async function createParentSession(): Promise<string> {
|
||||
const main = SessionManager.create(cwd);
|
||||
main.appendMessage({ role: "user", content: "main work", timestamp: 1 });
|
||||
main.appendMessage(makeAssistantMessage());
|
||||
await main.flush();
|
||||
const mainFile = main.getSessionFile();
|
||||
if (!mainFile) throw new Error("Expected persisted parent session file");
|
||||
await main.close();
|
||||
return mainFile;
|
||||
}
|
||||
|
||||
it("keeps --continue on the parent when a subagent opens in the same terminal", async () => {
|
||||
const mainFile = await createParentSession();
|
||||
writeBreadcrumb(cwd, mainFile);
|
||||
|
||||
// A subagent opening its own session must not clobber the terminal breadcrumb.
|
||||
await writeSubagentSession(mainFile, "Worker", "subagent work");
|
||||
|
||||
const crumb = await readTerminalBreadcrumbEntry();
|
||||
expect(crumb?.sessionFile).toBe(mainFile);
|
||||
|
||||
const resumed = await SessionManager.continueRecent(cwd);
|
||||
try {
|
||||
expect(resumed.getSessionFile()).toBe(path.resolve(mainFile));
|
||||
const dump = JSON.stringify(resumed.getEntries());
|
||||
expect(dump).toContain("main work");
|
||||
expect(dump).not.toContain("subagent work");
|
||||
} finally {
|
||||
await resumed.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("recovers a stale breadcrumb that points inside a subagent artifacts dir", async () => {
|
||||
const mainFile = await createParentSession();
|
||||
const subFile = await writeSubagentSession(mainFile, "Worker", "subagent work");
|
||||
|
||||
// Simulate a pre-fix poisoned breadcrumb pointing at the subagent transcript.
|
||||
writeBreadcrumb(cwd, subFile);
|
||||
|
||||
const resumed = await SessionManager.continueRecent(cwd);
|
||||
try {
|
||||
// Redirected up to the interactive root rather than resuming the subagent.
|
||||
expect(resumed.getSessionFile()).toBe(path.resolve(mainFile));
|
||||
const dump = JSON.stringify(resumed.getEntries());
|
||||
expect(dump).toContain("main work");
|
||||
expect(dump).not.toContain("subagent work");
|
||||
} finally {
|
||||
await resumed.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user