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.
This commit is contained in:
@@ -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 `<files>` 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 `<files>` 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 `<files>`.
|
||||
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 `<files>`. 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 };
|
||||
}
|
||||
|
||||
@@ -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 <files>", () => {
|
||||
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 `<file>: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(["<files>", "c.ts (Write)", "</files>"].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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user