From 67abdb1828616e7a2314bd6bc8b423cb2b0d0253 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 18 Jun 2026 19:37:17 +0200 Subject: [PATCH] feat(agent): prevented internal URLs from appearing in file summaries - Added an exclusion check to `extractFileOpsFromMessage` to ignore paths containing URL schemes (e.g., `artifact://`, `conflict://`, `https://`). - Implemented `isUrlSchemePath` helper to identify non-filesystem resources that cannot be re-grounded by the agent. - Added comprehensive unit tests to verify exclusion logic for various scheme-prefixed paths. --- packages/agent/src/compaction/utils.ts | 28 ++++++++- .../agent/test/compaction-file-ops.test.ts | 61 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/packages/agent/src/compaction/utils.ts b/packages/agent/src/compaction/utils.ts index 82ef3cab8..e897c7bca 100644 --- a/packages/agent/src/compaction/utils.ts +++ b/packages/agent/src/compaction/utils.ts @@ -76,6 +76,23 @@ export function stripReadSelector(path: string): string { return splitReadSelector(path).path; } +/** + * A real filesystem path never contains a `scheme://` URL. Tool-call paths that + * do — `conflict://1`, `artifact://3`, `local://ctx.md`, `history://…`, + * `issue://12`, `https://…`, and the tolerated `file.ts:conflict://1` prefix + * form — are session-scoped or remote resources, not files the post-compaction + * agent can re-ground on. Keep them out of the `` summary. + */ +const URL_SCHEME_RE = /[a-z][a-z0-9+.-]*:\/\//i; + +/** + * Whether `path` references a `scheme://` URL (internal URI or web URL) rather + * than a filesystem path that belongs in the compaction `` summary. + */ +export function isUrlSchemePath(path: string): boolean { + return URL_SCHEME_RE.test(path); +} + /** * Extract file operations from tool calls in an assistant message. */ @@ -94,6 +111,10 @@ export function extractFileOpsFromMessage(message: AgentMessage, fileOps: FileOp const path = typeof args.path === "string" ? args.path : undefined; if (!path) continue; + // Internal URIs (conflict://, artifact://, local://, history://, …) and + // web URLs are not re-groundable files — keep them out of ``. + if (isUrlSchemePath(path)) continue; + switch (block.name) { case "read": fileOps.read.add(stripReadSelector(path)); @@ -113,8 +134,11 @@ export function extractFileOpsFromMessage(message: AgentMessage, fileOps: FileOp * Returns readFiles (files only read, not modified) and modifiedFiles. */ export function computeFileLists(fileOps: FileOperations): { readFiles: string[]; modifiedFiles: string[] } { - const modified = new Set([...fileOps.edited, ...fileOps.written]); - const readOnly = [...fileOps.read].filter(f => !modified.has(f)).sort(); + // Drop any `scheme://` URLs (e.g. legacy `conflict://`/`artifact://` entries + // rehydrated straight into `fileOps` from a pre-fix compaction summary) — only + // real files belong in ``. New tool-call scans are already filtered. + const modified = new Set([...fileOps.edited, ...fileOps.written].filter(f => !isUrlSchemePath(f))); + const readOnly = [...fileOps.read].filter(f => !isUrlSchemePath(f) && !modified.has(f)).sort(); const modifiedFiles = [...modified].sort(); return { readFiles: readOnly, modifiedFiles }; } diff --git a/packages/agent/test/compaction-file-ops.test.ts b/packages/agent/test/compaction-file-ops.test.ts index ea8562c26..8e4448dd7 100644 --- a/packages/agent/test/compaction-file-ops.test.ts +++ b/packages/agent/test/compaction-file-ops.test.ts @@ -4,6 +4,7 @@ import { createFileOps, extractFileOpsFromMessage, formatFileOperations, + isUrlSchemePath, stripReadSelector, } from "../src/compaction/utils"; import { createAssistantMessage } from "./helpers"; @@ -12,6 +13,10 @@ function readCall(id: string, path: string) { return { type: "toolCall" as const, id, name: "read", arguments: { path } }; } +function writeCall(id: string, path: string) { + return { type: "toolCall" as const, id, name: "write", arguments: { path } }; +} + describe("stripReadSelector", () => { it("strips line-range and raw selectors in every supported shape", () => { expect(stripReadSelector("src/foo.ts:50")).toBe("src/foo.ts"); @@ -64,6 +69,41 @@ describe("extractFileOpsFromMessage", () => { expect(readFiles).toEqual([]); expect(modifiedFiles).toEqual(["src/login.ts"]); }); + + it("skips internal URLs and web URLs so they never enter ", () => { + const fileOps = createFileOps(); + const message = createAssistantMessage([ + readCall("r1", "src/keep.ts"), + readCall("r2", "artifact://7"), + readCall("r3", "local://ctx.md"), + readCall("r4", "https://example.com/page"), + writeCall("w1", "conflict://1"), + writeCall("w2", "conflict://*"), + // Tolerated `:conflict://N` prefix typo form the write tool accepts. + writeCall("w3", "src/login.ts:conflict://3"), + { type: "toolCall" as const, id: "e1", name: "edit", arguments: { path: "agent://abc" } }, + ]); + extractFileOpsFromMessage(message, fileOps); + const { readFiles, modifiedFiles } = computeFileLists(fileOps); + expect(readFiles).toEqual(["src/keep.ts"]); + expect(modifiedFiles).toEqual([]); + }); +}); + +describe("computeFileLists", () => { + it("drops scheme:// URLs rehydrated from legacy compaction details", () => { + const fileOps = createFileOps(); + // Simulate a pre-fix summary's details.readFiles/modifiedFiles fed straight + // into fileOps without going through extractFileOpsFromMessage. + fileOps.read.add("src/read-only.ts"); + fileOps.read.add("artifact://7"); + fileOps.edited.add("src/edited.ts"); + fileOps.edited.add("conflict://1"); + fileOps.written.add("local://ctx.md"); + const { readFiles, modifiedFiles } = computeFileLists(fileOps); + expect(readFiles).toEqual(["src/read-only.ts"]); + expect(modifiedFiles).toEqual(["src/edited.ts"]); + }); }); describe("formatFileOperations", () => { @@ -83,3 +123,24 @@ describe("formatFileOperations", () => { expect(rendered).toBe(["", "c.ts (Write)", ""].join("\n")); }); }); + +describe("isUrlSchemePath", () => { + it("flags internal URIs and web URLs", () => { + expect(isUrlSchemePath("conflict://1")).toBe(true); + expect(isUrlSchemePath("conflict://*")).toBe(true); + expect(isUrlSchemePath("artifact://7")).toBe(true); + expect(isUrlSchemePath("local://ctx.md")).toBe(true); + expect(isUrlSchemePath("history://AuthLoader")).toBe(true); + expect(isUrlSchemePath("https://example.com/page")).toBe(true); + // Prefixed conflict typo form — scheme appears after a colon, not at start. + expect(isUrlSchemePath("src/login.ts:conflict://3")).toBe(true); + }); + + it("leaves real filesystem paths untouched", () => { + expect(isUrlSchemePath("src/foo.ts")).toBe(false); + expect(isUrlSchemePath("C:/Users/me/file.ts")).toBe(false); + expect(isUrlSchemePath("db.sqlite:users")).toBe(false); + expect(isUrlSchemePath("archive.zip:dir/file.ts")).toBe(false); + expect(isUrlSchemePath("docs/compaction.md:100-170:raw")).toBe(false); + }); +});