smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): copy local artifacts across handoff session boundary

Browse Source 
/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
This commit is contained in:
roboomp
2026-08-11 16:45:15 +00:00
parent d3b22a0db6
commit dbc199d2db
5 changed files with 100 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -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
packages/coding-agent/src/internal-urls/local-protocol.ts
+42
View File
@@ -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<void> {
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<void> {
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,
packages/coding-agent/src/modes/interactive-mode.ts
+2 -38
View File
@@ -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<void> {
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<void> {
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(),
packages/coding-agent/src/session/session-handoff.ts
+21
View File
@@ -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");
packages/coding-agent/test/agent-session-handoff.test.ts
+31
View File
@@ -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