From 6bbc573c6f2f81b0789acee49ad4e9d4d2cb280c Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 16 Jun 2026 16:05:50 +0200 Subject: [PATCH] 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 (`/.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. --- .../coding-agent/src/export/html/index.ts | 2 +- .../src/session/session-manager.ts | 27 +++- packages/coding-agent/src/task/executor.ts | 11 +- .../subagent-breadcrumb.test.ts | 119 ++++++++++++++++++ 4 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 packages/coding-agent/test/session-manager/subagent-breadcrumb.test.ts diff --git a/packages/coding-agent/src/export/html/index.ts b/packages/coding-agent/src/export/html/index.ts index e2f7222d3..03fe28535 100644 --- a/packages/coding-agent/src/export/html/index.ts +++ b/packages/coding-agent/src/export/html/index.ts @@ -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; diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 3b13ba255..17ee20bbb 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -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 — + * `.jsonl` strips its suffix to `/`, and a child writes + * `/.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 + // (`.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 { 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; diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 27f3b1a9c..35129198b 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -1927,7 +1927,12 @@ export async function runSubprocess(options: ExecutorOptions): Promise { - const reopened = await SessionManager.open(sessionFile); + const reopened = await SessionManager.open(sessionFile, undefined, undefined, { + suppressBreadcrumb: true, + }); if (options.parentArtifactManager) { reopened.adoptArtifactManager(options.parentArtifactManager); } diff --git a/packages/coding-agent/test/session-manager/subagent-breadcrumb.test.ts b/packages/coding-agent/test/session-manager/subagent-breadcrumb.test.ts new file mode 100644 index 000000000..e8219756e --- /dev/null +++ b/packages/coding-agent/test/session-manager/subagent-breadcrumb.test.ts @@ -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 (`/.jsonl`). */ +async function writeSubagentSession(parentFile: string, agentId: string, userText: string): Promise { + 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 { + 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(); + } + }); +});