Merge PR #5086: fix(agent): isolate memory root resolution (@roboomp)
# Conflicts: # packages/coding-agent/src/internal-urls/memory-protocol.ts
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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<string> {
|
||||
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;
|
||||
|
||||
@@ -751,6 +751,7 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
|
||||
const internalUrlOptions: InternalUrlExpansionOptions = {
|
||||
skills: this.session.skills ?? [],
|
||||
internalRouter: InternalUrlRouter.instance(),
|
||||
cwd: this.session.cwd,
|
||||
localOptions: {
|
||||
getArtifactsDir: this.session.getArtifactsDir,
|
||||
getSessionId: this.session.getSessionId,
|
||||
|
||||
@@ -78,6 +78,66 @@ describe("MemoryProtocolHandler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
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/<path> within memory root", async () => {
|
||||
await withMemoryFixture(async ({ memoryRoot }) => {
|
||||
const skillPath = path.join(memoryRoot, "skills", "demo", "SKILL.md");
|
||||
|
||||
@@ -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<string, { sourcePath?: string; e
|
||||
canHandle: (input: string) => 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" },
|
||||
|
||||
Reference in New Issue
Block a user