From dbc199d2db729103b9e31d6db97d04f8a7675dad Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 11 Aug 2026 16:45:15 +0000 Subject: [PATCH] fix(coding-agent): copy local artifacts across handoff session boundary /handoff mints a fresh session via newSession(), producing a new artifactsDir and an empty local/ root. The handoff document routinely references plans and scratch files under '/data/workspaces/can1357__oh-my-pi__8261/.omp-session/2026-08-11T16-39-09-489Z_019ff1b1-31b1-7000-81f5-c540f4ebf43d/local/,' so every reference became a dangling pointer in the new session. The plan approve-and-execute path already copies artifacts across the boundary; handoff did not. Extracted the plan-approve copy helper into a shared copyLocalArtifacts() in local-protocol.ts and invoke it across the handoff session switch (best-effort, since the switch is already committed). Fixes #8261 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/internal-urls/local-protocol.ts | 42 +++++++++++++++++++ .../src/modes/interactive-mode.ts | 40 +----------------- .../src/session/session-handoff.ts | 21 ++++++++++ .../test/agent-session-handoff.test.ts | 31 ++++++++++++++ 5 files changed, 100 insertions(+), 38 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a53f31baf..54fb3015c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `/handoff` losing the previous session's `local://` artifacts (plans, scratch files, research notes): the handoff document referenced files that became unreadable because the new session's `local/` root was empty. Local artifacts are now copied across the handoff session boundary, mirroring the plan approve-and-execute path ([#8261](https://github.com/can1357/oh-my-pi/issues/8261)). + ## [17.2.13] - 2026-08-11 ### Added diff --git a/packages/coding-agent/src/internal-urls/local-protocol.ts b/packages/coding-agent/src/internal-urls/local-protocol.ts index a20724390..942cb98f4 100644 --- a/packages/coding-agent/src/internal-urls/local-protocol.ts +++ b/packages/coding-agent/src/internal-urls/local-protocol.ts @@ -253,6 +253,48 @@ export function resolveLocalRoot(options: LocalProtocolOptions, platform: NodeJS return path.join(os.tmpdir(), "omp-local", safeSessionId(options)); } +/** + * Recursively copy every local:// artifact from one session-scoped root to + * another. Used when a session transition mints a fresh local root (plan + * approve-and-execute, handoff) so plans, scratch files, and research notes the + * carried-forward context references stay readable in the replacement session. + * No-op when the roots match or the source root is absent. + */ +export async function copyLocalArtifacts(sourceRoot: string, destinationRoot: string): Promise { + if (sourceRoot === destinationRoot) return; + + let sourceRootStat: { isDirectory(): boolean }; + try { + sourceRootStat = await fs.lstat(sourceRoot); + } catch (error) { + if (isEnoent(error)) return; + throw error; + } + if (!sourceRootStat.isDirectory()) return; + + await fs.mkdir(destinationRoot, { recursive: true }); + await copyLocalArtifactEntries(sourceRoot, destinationRoot); +} + +async function copyLocalArtifactEntries(sourceDir: string, destinationDir: string): Promise { + const entries = await fs.readdir(sourceDir, { withFileTypes: true }); + for (const entry of entries) { + const sourcePath = path.join(sourceDir, entry.name); + const destinationPath = path.join(destinationDir, entry.name); + + if (entry.isDirectory()) { + await fs.mkdir(destinationPath, { recursive: true }); + await copyLocalArtifactEntries(sourcePath, destinationPath); + continue; + } + + if (entry.isFile()) { + await fs.mkdir(path.dirname(destinationPath), { recursive: true }); + await fs.copyFile(sourcePath, destinationPath); + } + } +} + /** Resolve a local:// URL to an on-disk path under the active session's local root. */ export function resolveLocalUrlToPath( input: string | InternalUrl, diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 0f9d54026..dc4efae91 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -81,7 +81,7 @@ import type { CompactOptions } from "../extensibility/extensions/types"; import type { Skill } from "../extensibility/skills"; import { loadSlashCommands } from "../extensibility/slash-commands"; import type { Goal, GoalModeState } from "../goals/state"; -import { resolveLocalUrlToPath } from "../internal-urls"; +import { copyLocalArtifacts, resolveLocalUrlToPath } from "../internal-urls"; import { LSP_STARTUP_EVENT_CHANNEL, type LspStartupEvent } from "../lsp/startup-events"; import type { MCPManager } from "../mcp"; import { @@ -3167,42 +3167,6 @@ export class InteractiveMode implements InteractiveModeContext { }); } - async #copyLocalArtifactsForFreshSession(sourceRoot: string, destinationRoot: string): Promise { - if (sourceRoot === destinationRoot) return; - - let sourceRootStat: { isDirectory(): boolean }; - try { - sourceRootStat = await fs.lstat(sourceRoot); - } catch (error) { - if (isEnoent(error)) return; - throw error; - } - - if (!sourceRootStat.isDirectory()) return; - - await fs.mkdir(destinationRoot, { recursive: true }); - await this.#copyLocalArtifactEntries(sourceRoot, destinationRoot); - } - - async #copyLocalArtifactEntries(sourceDir: string, destinationDir: string): Promise { - const entries = await fs.readdir(sourceDir, { withFileTypes: true }); - for (const entry of entries) { - const sourcePath = path.join(sourceDir, entry.name); - const destinationPath = path.join(destinationDir, entry.name); - - if (entry.isDirectory()) { - await fs.mkdir(destinationPath, { recursive: true }); - await this.#copyLocalArtifactEntries(sourcePath, destinationPath); - continue; - } - - if (entry.isFile()) { - await fs.mkdir(path.dirname(destinationPath), { recursive: true }); - await fs.copyFile(sourcePath, destinationPath); - } - } - } - async #approvePlan( planContent: string, options: { @@ -3236,7 +3200,7 @@ export class InteractiveMode implements InteractiveModeContext { const oldLocalRoot = this.#resolveLocalRoot(); await this.handleClearCommand(); const newLocalRoot = this.#resolveLocalRoot(); - await this.#copyLocalArtifactsForFreshSession(oldLocalRoot, newLocalRoot); + await copyLocalArtifacts(oldLocalRoot, newLocalRoot); const newLocalPath = resolveLocalUrlToPath(options.planFilePath, { getArtifactsDir: () => this.sessionManager.getArtifactsDir(), getSessionId: () => this.sessionManager.getSessionId(), diff --git a/packages/coding-agent/src/session/session-handoff.ts b/packages/coding-agent/src/session/session-handoff.ts index 662fa06e4..85e15879e 100644 --- a/packages/coding-agent/src/session/session-handoff.ts +++ b/packages/coding-agent/src/session/session-handoff.ts @@ -14,6 +14,7 @@ import { logger, Snowflake } from "@oh-my-pi/pi-utils"; import type { ModelRegistry } from "../config/model-registry"; import type { Settings } from "../config/settings"; import type { ExtensionRunner, SessionBeforeSwitchResult } from "../extensibility/extensions"; +import { copyLocalArtifacts, resolveLocalUrlToPath } from "../internal-urls"; import { obfuscateProviderContext } from "../secrets/message-transform"; import type { SecretObfuscator } from "../secrets/obfuscator"; import type { HandoffResult, SessionHandoffOptions } from "./agent-session-types"; @@ -255,6 +256,15 @@ export class SessionHandoff { // Stop and settle in-flight advisors while the old-session feeds can still // observe message_end, then mute before opening the replacement session. await this.#host.drainAndDetachAdvisorRecorders(); + // Snapshot the outgoing session's local:// root BEFORE newSession() mints a + // fresh session id (and therefore a fresh, empty local root). The handoff + // document routinely references plans/scratch files under local://, so those + // artifacts must follow the session switch or every reference dangles. + const localProtocolOptions = { + getArtifactsDir: () => this.#host.sessionManager.getArtifactsDir(), + getSessionId: () => this.#host.sessionManager.getSessionId(), + }; + const previousLocalRoot = resolveLocalUrlToPath("local://", localProtocolOptions); const bashTransition = this.#host.beginBashSessionTransition(); this.#host.cancelOwnAsyncJobs(); try { @@ -292,6 +302,17 @@ export class SessionHandoff { this.#host.clearPendingNextTurnMessages(); this.#host.resetTodoCycle(); + // Carry local:// artifacts into the replacement session (best-effort: the + // switch is already committed, so a copy failure must not fail the handoff). + try { + const newLocalRoot = resolveLocalUrlToPath("local://", localProtocolOptions); + await copyLocalArtifacts(previousLocalRoot, newLocalRoot); + } catch (error) { + logger.warn("Failed to copy local artifacts into handoff session", { + error: error instanceof Error ? error.message : String(error), + }); + } + // Inject the handoff document as a custom message const handoffContent = createHandoffContext(handoffText); this.#host.sessionManager.appendCustomMessageEntry("handoff", handoffContent, true, undefined, "agent"); diff --git a/packages/coding-agent/test/agent-session-handoff.test.ts b/packages/coding-agent/test/agent-session-handoff.test.ts index d7a4024c5..4bb0feeaa 100644 --- a/packages/coding-agent/test/agent-session-handoff.test.ts +++ b/packages/coding-agent/test/agent-session-handoff.test.ts @@ -1,4 +1,5 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as fs from "node:fs/promises"; import * as path from "node:path"; import { Agent, type AgentMessage, type StreamFn } from "@oh-my-pi/pi-agent-core"; import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; @@ -13,6 +14,7 @@ import { loadExtensionFromFactory, loadExtensions, } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; +import { resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls"; import { SecretObfuscator } from "@oh-my-pi/pi-coding-agent/secrets"; import { AgentSession, type AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; @@ -177,6 +179,35 @@ describe("AgentSession handoff", () => { expect(session.nextToolChoiceDirective()).toBeUndefined(); }); + it("carries local:// artifacts into the handed-off session", async () => { + // Handoff is a continuity operation: the generated document references + // plans/scratch files the old session wrote under its local:// root. The + // fresh session mints a new local root, so the artifacts must be copied + // forward or every reference the handoff document carries dangles. + vi.spyOn(compactionModule, "generateHandoffFromContext").mockResolvedValue("## Goal\nContinue from here"); + const localOptions = { + getArtifactsDir: () => sessionManager.getArtifactsDir(), + getSessionId: () => sessionManager.getSessionId(), + }; + const oldLocalRoot = resolveLocalUrlToPath("local://", localOptions); + const oldPlanPath = resolveLocalUrlToPath("local://my-plan.md", localOptions); + const oldNestedPath = resolveLocalUrlToPath("local://research/notes.txt", localOptions); + await fs.mkdir(path.dirname(oldNestedPath), { recursive: true }); + await Bun.write(oldPlanPath, "# Plan\n\nbody\n"); + await Bun.write(oldNestedPath, "scratch notes"); + + await session.handoff(); + + const newLocalRoot = resolveLocalUrlToPath("local://", localOptions); + expect(newLocalRoot).not.toBe(oldLocalRoot); + expect(await Bun.file(resolveLocalUrlToPath("local://my-plan.md", localOptions)).text()).toBe("# Plan\n\nbody\n"); + expect(await Bun.file(resolveLocalUrlToPath("local://research/notes.txt", localOptions)).text()).toBe( + "scratch notes", + ); + // The source session's artifacts remain untouched on disk. + expect(await Bun.file(oldPlanPath).text()).toBe("# Plan\n\nbody\n"); + }); + it("emits handoff lifecycle hooks on the outgoing and replacement sessions", async () => { // dispose() is terminal: it closes the manager and releases its in-memory // transcript. Reopen the persisted session file for the replacement