diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1b0f25aa0..58e02f469 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -281,6 +281,7 @@ - Fixed MCP OAuth dynamic client registration omitting discovered scopes on the RFC 7591 registration body. Providers such as Clerk bind DCR-created clients to only the scopes declared at registration, then reject the subsequent authorize request when it asks for `openid` (from `scopes_supported`). Registration now includes `config.scopes` when present, matching Claude Code and the scopes already sent on authorize. - Fixed `write` treating read-only internal URLs like `memory://root/memory_summary.md` as project-relative filesystem paths, prevented leaks such as `{cwd}/memory:/root/memory_summary.md`, and made `memory://root` prefer the calling session's cwd-specific memory root when multiple agents are live. ([#5075](https://github.com/can1357/oh-my-pi/issues/5075)) +- 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 diff --git a/packages/coding-agent/src/internal-urls/memory-protocol.ts b/packages/coding-agent/src/internal-urls/memory-protocol.ts index 3380d7e04..6b948ec4c 100644 --- a/packages/coding-agent/src/internal-urls/memory-protocol.ts +++ b/packages/coding-agent/src/internal-urls/memory-protocol.ts @@ -29,10 +29,8 @@ export function memoryRootsFromRegistry(): string[] { } function memoryRootsForContext(context?: ResolveContext): string[] { - const roots = memoryRootsFromRegistry(); - if (!context?.cwd) return roots; - const callerRoot = getMemoryRoot(getAgentDir(), context.cwd); - return [callerRoot, ...roots.filter(root => root !== callerRoot)]; + if (context?.cwd) return [getMemoryRoot(getAgentDir(), context.cwd)]; + return memoryRootsFromRegistry(); } function ensureWithinRoot(targetPath: string, rootPath: string): void { @@ -203,10 +201,9 @@ function renderMnemopiMemory(url: InternalUrl, hit: MnemopiScopedMemoryHit): Int /** * Protocol handler for memory:// URLs. - * - * Resolves the caller cwd's memory root first, then walks other active - * sessions' roots. Parent and subagent sharing a cwd see the same file - * regardless of registry 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"; diff --git a/packages/coding-agent/src/tools/bash-skill-urls.ts b/packages/coding-agent/src/tools/bash-skill-urls.ts index 6db86b693..135990b21 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 ccfea33c1..0499e86d3 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -751,6 +751,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 7bbf12e55..9ea65e242 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" },