From f26fffb8e014edc1362f6d90c6a757d6fade8762 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 10 Jul 2026 14:48:24 +0000 Subject: [PATCH] fix(agent): isolated memory root resolution Resolved file-backed memory://root URLs from the calling session cwd before falling back to the global registry. Passed the caller cwd through bash internal-URL expansion so redirected memory paths cannot pick another live agent root. Fixes #5079 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/internal-urls/memory-protocol.ts | 18 +++--- .../coding-agent/src/tools/bash-skill-urls.ts | 5 +- packages/coding-agent/src/tools/bash.ts | 1 + .../internal-urls/memory-protocol.test.ts | 60 +++++++++++++++++++ .../test/tools/bash-skill-urls.test.ts | 30 +++++++++- 6 files changed, 109 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 04ae4eb4d..e3f5af53b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `memory://root` resolution leaking between live agents with different working directories by resolving file-backed memory roots from the calling session cwd before falling back to the global session registry. ([#5079](https://github.com/can1357/oh-my-pi/issues/5079)) + ## [16.4.0] - 2026-07-10 ### Breaking Changes diff --git a/packages/coding-agent/src/internal-urls/memory-protocol.ts b/packages/coding-agent/src/internal-urls/memory-protocol.ts index c6c83ffd5..c0698d0ab 100644 --- a/packages/coding-agent/src/internal-urls/memory-protocol.ts +++ b/packages/coding-agent/src/internal-urls/memory-protocol.ts @@ -6,7 +6,7 @@ import { getMnemopiSessionState, type MnemopiScopedMemoryHit, type MnemopiSessio import { AgentRegistry } from "../registry/agent-registry"; import { buildDirectoryResource } from "./filesystem-resource"; import { validateRelativePath } from "./skill-protocol"; -import type { InternalResource, InternalUrl, ProtocolHandler, UrlCompletion } from "./types"; +import type { InternalResource, InternalUrl, ProtocolHandler, ResolveContext, UrlCompletion } from "./types"; const DEFAULT_MEMORY_FILE = "memory_summary.md"; const MEMORY_NAMESPACE = "root"; @@ -28,6 +28,11 @@ export function memoryRootsFromRegistry(): string[] { return roots; } +function memoryRootsForContext(context?: ResolveContext): string[] { + if (context?.cwd) return [getMemoryRoot(getAgentDir(), context.cwd)]; + return memoryRootsFromRegistry(); +} + function ensureWithinRoot(targetPath: string, rootPath: string): void { if (targetPath !== rootPath && !targetPath.startsWith(`${rootPath}${path.sep}`)) { throw new Error("memory:// URL escapes memory root"); @@ -196,16 +201,15 @@ function renderMnemopiMemory(url: InternalUrl, hit: MnemopiScopedMemoryHit): Int /** * Protocol handler for memory:// URLs. - * - * Walks every active session's memory root. Worktree-based subagents have - * their own root; first one containing the file wins. Parent and subagent - * sharing a cwd see the same file regardless of order. + * Resolves file-backed roots against the calling session cwd when provided. + * Contextless callers fall back to the live-session registry for legacy + * cross-session lookups. */ export class MemoryProtocolHandler implements ProtocolHandler { readonly scheme = "memory"; readonly immutable = true; - async resolve(url: InternalUrl): Promise { + async resolve(url: InternalUrl, context?: ResolveContext): Promise { const namespace = url.rawHost || url.hostname; if (!namespace) { throw new Error("memory:// URL requires a namespace: memory://root or memory://"); @@ -230,7 +234,7 @@ export class MemoryProtocolHandler implements ProtocolHandler { ); } - const roots = memoryRootsFromRegistry(); + const roots = memoryRootsForContext(context); if (roots.length === 0) { throw new Error( "Memory artifacts are not available for this project yet. Run a session with memories enabled first.", diff --git a/packages/coding-agent/src/tools/bash-skill-urls.ts b/packages/coding-agent/src/tools/bash-skill-urls.ts index 585882a9c..07b2c55e2 100644 --- a/packages/coding-agent/src/tools/bash-skill-urls.ts +++ b/packages/coding-agent/src/tools/bash-skill-urls.ts @@ -27,6 +27,7 @@ export interface InternalUrlExpansionOptions { noEscape?: boolean; internalRouter?: InternalUrlResolver; localOptions?: LocalProtocolOptions; + cwd?: string; ensureLocalParentDirs?: boolean; } @@ -175,6 +176,7 @@ async function resolveInternalUrlToPath( internalRouter?: InternalUrlResolver, localOptions?: LocalProtocolOptions, ensureLocalParentDirs?: boolean, + cwd?: string, ): Promise { const url = normalizeLocalScheme(rawUrl); const scheme = extractScheme(url); @@ -208,7 +210,7 @@ async function resolveInternalUrlToPath( let resource: InternalResource; try { - resource = await internalRouter.resolve(url, { pathOnly: true }); + resource = await internalRouter.resolve(url, { cwd, pathOnly: true }); } catch (error) { const message = error instanceof Error ? error.message : String(error); throw new ToolError(`Failed to resolve ${scheme}:// URL in bash command: ${url}\n${message}`); @@ -268,6 +270,7 @@ export async function expandInternalUrls(command: string, options: InternalUrlEx options.internalRouter, options.localOptions, options.ensureLocalParentDirs, + options.cwd, ); } catch { continue; diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 14e67375b..3f16e8ea5 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -741,6 +741,7 @@ export class BashTool implements AgentTool { }); }); + it("resolves memory://root against the caller cwd when multiple sessions are live", async () => { + const cleanupRoot = await fs.mkdtemp(path.join(os.tmpdir(), "memory-protocol-isolation-")); + const previousAgentDir = getAgentDir(); + try { + const agentDir = path.join(cleanupRoot, "agent"); + setAgentDir(agentDir); + + const firstCwd = path.join(cleanupRoot, "first-project"); + const secondCwd = path.join(cleanupRoot, "second-project"); + await fs.mkdir(firstCwd, { recursive: true }); + await fs.mkdir(secondCwd, { recursive: true }); + + const firstMemoryRoot = getMemoryRoot(agentDir, firstCwd); + const secondMemoryRoot = getMemoryRoot(agentDir, secondCwd); + await fs.mkdir(firstMemoryRoot, { recursive: true }); + await fs.mkdir(secondMemoryRoot, { recursive: true }); + + const firstSummary = "first registered session summary"; + const secondSummary = "second session cwd summary"; + await Bun.write(path.join(firstMemoryRoot, "memory_summary.md"), firstSummary); + await Bun.write(path.join(secondMemoryRoot, "memory_summary.md"), secondSummary); + + AgentRegistry.global().register({ + id: "first-session", + displayName: "first-session", + kind: "main", + session: { + sessionManager: { + getCwd: () => firstCwd, + getArtifactsDir: () => null, + getSessionId: () => "first-session", + }, + } as unknown as AgentSession, + sessionFile: null, + }); + AgentRegistry.global().register({ + id: "second-session", + displayName: "second-session", + kind: "main", + session: { + sessionManager: { + getCwd: () => secondCwd, + getArtifactsDir: () => null, + getSessionId: () => "second-session", + }, + } as unknown as AgentSession, + sessionFile: null, + }); + + const router = InternalUrlRouter.instance(); + const resource = await router.resolve("memory://root", { cwd: secondCwd }); + + expect(resource.content).toBe(secondSummary); + expect(resource.content).not.toBe(firstSummary); + } finally { + setAgentDir(previousAgentDir); + await removeWithRetries(cleanupRoot); + } + }); + it("resolves memory://root/ within memory root", async () => { await withMemoryFixture(async ({ memoryRoot }) => { const skillPath = path.join(memoryRoot, "skills", "demo", "SKILL.md"); diff --git a/packages/coding-agent/test/tools/bash-skill-urls.test.ts b/packages/coding-agent/test/tools/bash-skill-urls.test.ts index 153eef880..de7bfdbc5 100644 --- a/packages/coding-agent/test/tools/bash-skill-urls.test.ts +++ b/packages/coding-agent/test/tools/bash-skill-urls.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "bun:test"; import * as path from "node:path"; import type { Skill } from "@oh-my-pi/pi-coding-agent/extensibility/skills"; -import { resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls"; +import { type ResolveContext, resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls"; import { expandInternalUrls, expandSkillUrls } from "@oh-my-pi/pi-coding-agent/tools/bash-skill-urls"; import { ToolError } from "@oh-my-pi/pi-coding-agent/tools/tool-errors"; @@ -24,6 +24,7 @@ function createInternalRouter(resources: Record boolean; resolve: ( input: string, + context?: ResolveContext, ) => Promise<{ url: string; content: string; contentType: "text/plain"; sourcePath?: string; immutable: boolean }>; } { return { @@ -169,6 +170,33 @@ describe("expandInternalUrls", () => { ); }); + it("passes caller cwd to the router when expanding memory URLs", async () => { + const cwd = "/tmp/session-b"; + const sourcePath = "/tmp/session-b-memory/memory_summary.md"; + let observedCwd: string | undefined; + let observedPathOnly: boolean | undefined; + const router = { + canHandle: (input: string) => input === "memory://root/memory_summary.md", + resolve: async (input: string, context?: ResolveContext) => { + observedCwd = context?.cwd; + observedPathOnly = context?.pathOnly; + return { + url: input, + content: "", + contentType: "text/plain" as const, + sourcePath, + immutable: true, + }; + }, + }; + + await expect( + expandInternalUrls("cat memory://root/memory_summary.md", { skills: [], internalRouter: router, cwd }), + ).resolves.toBe(`cat ${shellEscape(sourcePath)}`); + expect(observedCwd).toBe(cwd); + expect(observedPathOnly).toBe(true); + }); + it("expands quoted non-skill URLs and shell-escapes quotes in paths", async () => { const router = createInternalRouter({ "artifact://7": { sourcePath: "/tmp/artifacts/with'quote.log" },