fix(read): refreshed URL responses on each invocation
Removed process-local URL response reuse so read and URL-backed search paths fetch current content. Added regressions for repeated reads and searches. Fixes #5803
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed repeated URL reads and URL-backed searches returning stale same-session responses instead of refetching the resource ([#5803](https://github.com/can1357/oh-my-pi/issues/5803)).
|
||||
|
||||
## [17.0.2] - 2026-07-17
|
||||
|
||||
### Added
|
||||
|
||||
@@ -7,7 +7,6 @@ import type { FetchImpl, ImageContent, TextContent } from "@oh-my-pi/pi-ai";
|
||||
import { htmlToMarkdown } from "@oh-my-pi/pi-natives";
|
||||
import { type Component, Text } from "@oh-my-pi/pi-tui";
|
||||
import { $which, ptree, truncate } from "@oh-my-pi/pi-utils";
|
||||
import { LRUCache } from "lru-cache/raw";
|
||||
import type { Settings } from "../config/settings";
|
||||
import { readEditableNotebookText } from "../edit/notebook";
|
||||
import type { RenderResultOptions } from "../extensibility/custom-tools/types";
|
||||
@@ -1553,24 +1552,15 @@ export interface ReadUrlToolDetails {
|
||||
meta?: OutputMeta;
|
||||
}
|
||||
|
||||
interface ReadUrlCacheEntry {
|
||||
interface ReadUrlEntry {
|
||||
artifactId?: string;
|
||||
artifactPath?: string;
|
||||
contentPath?: string;
|
||||
details: ReadUrlToolDetails;
|
||||
image?: FetchImagePayload;
|
||||
output: string;
|
||||
content: string;
|
||||
}
|
||||
|
||||
const READ_URL_CACHE_MAX_ENTRIES = 100;
|
||||
const readUrlCache = new LRUCache<string, ReadUrlCacheEntry>({ max: READ_URL_CACHE_MAX_ENTRIES });
|
||||
|
||||
function getReadUrlCacheKey(session: ToolSession, requestedUrl: string, raw: boolean): string {
|
||||
const scope = session.getSessionFile() ?? session.cwd;
|
||||
return `${scope}::${raw ? "raw" : "rendered"}::${normalizeUrl(requestedUrl)}`;
|
||||
}
|
||||
|
||||
async function findArtifactPath(session: ToolSession, artifactId: string): Promise<string | null> {
|
||||
const artifactsDir = session.getArtifactsDir?.();
|
||||
if (!artifactsDir) return null;
|
||||
@@ -1584,25 +1574,6 @@ async function findArtifactPath(session: ToolSession, artifactId: string): Promi
|
||||
}
|
||||
}
|
||||
|
||||
async function readArtifactOutput(session: ToolSession, artifactId: string): Promise<string | null> {
|
||||
const artifactPath = await findArtifactPath(session, artifactId);
|
||||
return artifactPath ? await Bun.file(artifactPath).text() : null;
|
||||
}
|
||||
|
||||
async function materializeReadUrlCacheEntry(
|
||||
session: ToolSession,
|
||||
entry: ReadUrlCacheEntry,
|
||||
): Promise<ReadUrlCacheEntry | null> {
|
||||
if (entry.artifactId) {
|
||||
const artifactOutput = await readArtifactOutput(session, entry.artifactId);
|
||||
if (artifactOutput !== null) {
|
||||
return { ...entry, output: artifactOutput };
|
||||
}
|
||||
}
|
||||
|
||||
return entry.output.length > 0 ? entry : null;
|
||||
}
|
||||
|
||||
async function persistReadUrlArtifact(
|
||||
session: ToolSession,
|
||||
output: string,
|
||||
@@ -1613,7 +1584,7 @@ async function persistReadUrlArtifact(
|
||||
return artifact;
|
||||
}
|
||||
|
||||
async function ensureReadUrlCacheArtifact(session: ToolSession, entry: ReadUrlCacheEntry): Promise<ReadUrlCacheEntry> {
|
||||
async function ensureReadUrlArtifact(session: ToolSession, entry: ReadUrlEntry): Promise<ReadUrlEntry> {
|
||||
if (entry.artifactId && entry.artifactPath) return entry;
|
||||
if (entry.artifactId) {
|
||||
const artifactPath = await findArtifactPath(session, entry.artifactId);
|
||||
@@ -1632,19 +1603,7 @@ function readUrlContentExtension(finalUrl: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
async function ensureReadUrlContentFile(
|
||||
session: ToolSession,
|
||||
entry: ReadUrlCacheEntry,
|
||||
raw: boolean,
|
||||
): Promise<ReadUrlCacheEntry> {
|
||||
if (entry.contentPath) {
|
||||
try {
|
||||
await Bun.file(entry.contentPath).stat();
|
||||
return entry;
|
||||
} catch {
|
||||
// Recreate below when the cached scratch file was removed.
|
||||
}
|
||||
}
|
||||
async function materializeReadUrlContent(session: ToolSession, entry: ReadUrlEntry, raw: boolean): Promise<string> {
|
||||
const root = session.getArtifactsDir?.();
|
||||
if (!root) {
|
||||
throw new ToolError("Cannot search URL output because this session cannot materialize read artifacts.");
|
||||
@@ -1654,20 +1613,16 @@ async function ensureReadUrlContentFile(
|
||||
const hash = Bun.hash(`${raw ? "raw" : "rendered"}:${entry.details.finalUrl}`).toString(36);
|
||||
const contentPath = path.join(dir, `${hash}${readUrlContentExtension(entry.details.finalUrl)}`);
|
||||
await Bun.write(contentPath, entry.content);
|
||||
return { ...entry, contentPath };
|
||||
return contentPath;
|
||||
}
|
||||
|
||||
function cacheReadUrlEntry(session: ToolSession, requestedUrl: string, raw: boolean, entry: ReadUrlCacheEntry): void {
|
||||
readUrlCache.set(getReadUrlCacheKey(session, requestedUrl, raw), entry);
|
||||
readUrlCache.set(getReadUrlCacheKey(session, entry.details.finalUrl, raw), entry);
|
||||
}
|
||||
|
||||
async function buildReadUrlCacheEntry(
|
||||
/** Fetch and render a URL for a read or search operation. */
|
||||
export async function fetchReadUrl(
|
||||
session: ToolSession,
|
||||
params: { path: string; raw?: boolean },
|
||||
signal?: AbortSignal,
|
||||
options?: { ensureArtifact?: boolean },
|
||||
): Promise<ReadUrlCacheEntry> {
|
||||
): Promise<ReadUrlEntry> {
|
||||
const { path: url, raw = false } = params;
|
||||
|
||||
const effectiveTimeout = clampTimeout("fetch", 30);
|
||||
@@ -1708,30 +1663,6 @@ async function buildReadUrlCacheEntry(
|
||||
};
|
||||
}
|
||||
|
||||
export async function loadReadUrlCacheEntry(
|
||||
session: ToolSession,
|
||||
params: { path: string; raw?: boolean },
|
||||
signal?: AbortSignal,
|
||||
options?: { ensureArtifact?: boolean; preferCached?: boolean },
|
||||
): Promise<ReadUrlCacheEntry> {
|
||||
const raw = params.raw ?? false;
|
||||
const cached = readUrlCache.get(getReadUrlCacheKey(session, params.path, raw));
|
||||
if (options?.preferCached && cached) {
|
||||
const prepared = options.ensureArtifact ? await ensureReadUrlCacheArtifact(session, cached) : cached;
|
||||
const materialized = await materializeReadUrlCacheEntry(session, prepared);
|
||||
if (materialized) {
|
||||
cacheReadUrlEntry(session, params.path, raw, materialized);
|
||||
return materialized;
|
||||
}
|
||||
}
|
||||
|
||||
const fresh = await buildReadUrlCacheEntry(session, params, signal, {
|
||||
ensureArtifact: options?.ensureArtifact,
|
||||
});
|
||||
cacheReadUrlEntry(session, params.path, raw, fresh);
|
||||
return fresh;
|
||||
}
|
||||
|
||||
/** Materialize rendered URL body text to a local file for tools that require filesystem paths. */
|
||||
export async function materializeReadUrlToFile(
|
||||
session: ToolSession,
|
||||
@@ -1741,13 +1672,9 @@ export async function materializeReadUrlToFile(
|
||||
if (!session.settings.get("fetch.enabled")) {
|
||||
throw new ToolError("URL reads are disabled by settings.");
|
||||
}
|
||||
const cacheEntry = await loadReadUrlCacheEntry(session, params, signal, { preferCached: true });
|
||||
const materialized = await ensureReadUrlContentFile(session, cacheEntry, params.raw ?? false);
|
||||
cacheReadUrlEntry(session, params.path, params.raw ?? false, materialized);
|
||||
if (!materialized.contentPath) {
|
||||
throw new ToolError("Cannot search URL output because this session cannot materialize read artifacts.");
|
||||
}
|
||||
return { path: materialized.contentPath, details: materialized.details };
|
||||
const entry = await fetchReadUrl(session, params, signal);
|
||||
const contentPath = await materializeReadUrlContent(session, entry, params.raw ?? false);
|
||||
return { path: contentPath, details: entry.details };
|
||||
}
|
||||
|
||||
function buildUrlReadOutput(result: FetchRenderResult, content: string): string {
|
||||
@@ -1768,36 +1695,35 @@ export async function executeReadUrl(
|
||||
params: { path: string; raw?: boolean },
|
||||
signal?: AbortSignal,
|
||||
): Promise<AgentToolResult<ReadUrlToolDetails>> {
|
||||
let cacheEntry = await loadReadUrlCacheEntry(session, params, signal, { preferCached: true });
|
||||
const truncation = truncateHead(cacheEntry.output, {
|
||||
let entry = await fetchReadUrl(session, params, signal);
|
||||
const truncation = truncateHead(entry.output, {
|
||||
maxBytes: DEFAULT_MAX_BYTES,
|
||||
maxLines: FETCH_DEFAULT_MAX_LINES,
|
||||
});
|
||||
const needsArtifact = truncation.truncated;
|
||||
if (needsArtifact && !cacheEntry.artifactId) {
|
||||
cacheEntry = await ensureReadUrlCacheArtifact(session, cacheEntry);
|
||||
cacheReadUrlEntry(session, params.path, params.raw ?? false, cacheEntry);
|
||||
if (needsArtifact && !entry.artifactId) {
|
||||
entry = await ensureReadUrlArtifact(session, entry);
|
||||
}
|
||||
const output = needsArtifact ? truncation.content : cacheEntry.output;
|
||||
const output = needsArtifact ? truncation.content : entry.output;
|
||||
const details: ReadUrlToolDetails = {
|
||||
...cacheEntry.details,
|
||||
truncated: Boolean(cacheEntry.details.truncated || needsArtifact),
|
||||
...entry.details,
|
||||
truncated: Boolean(entry.details.truncated || needsArtifact),
|
||||
};
|
||||
|
||||
const contentBlocks: Array<TextContent | ImageContent> = [{ type: "text", text: output }];
|
||||
if (cacheEntry.image) {
|
||||
contentBlocks.push({ type: "image", data: cacheEntry.image.data, mimeType: cacheEntry.image.mimeType });
|
||||
if (entry.image) {
|
||||
contentBlocks.push({ type: "image", data: entry.image.data, mimeType: entry.image.mimeType });
|
||||
}
|
||||
|
||||
const resultBuilder = toolResult(details).content(contentBlocks).sourceUrl(details.finalUrl);
|
||||
if (needsArtifact) {
|
||||
resultBuilder.truncation(truncation, { direction: "head", artifactId: cacheEntry.artifactId });
|
||||
} else if (cacheEntry.details.truncated) {
|
||||
const outputLines = cacheEntry.output.split("\n").length;
|
||||
const outputBytes = Buffer.byteLength(cacheEntry.output, "utf-8");
|
||||
resultBuilder.truncation(truncation, { direction: "head", artifactId: entry.artifactId });
|
||||
} else if (entry.details.truncated) {
|
||||
const outputLines = entry.output.split("\n").length;
|
||||
const outputBytes = Buffer.byteLength(entry.output, "utf-8");
|
||||
const totalBytes = Math.max(outputBytes + 1, MAX_OUTPUT_CHARS + 1);
|
||||
const totalLines = outputLines + 1;
|
||||
resultBuilder.truncationFromText(cacheEntry.output, {
|
||||
resultBuilder.truncationFromText(entry.output, {
|
||||
direction: "tail",
|
||||
totalLines,
|
||||
totalBytes,
|
||||
|
||||
@@ -78,7 +78,7 @@ import {
|
||||
} from "./conflict-detect";
|
||||
import {
|
||||
executeReadUrl,
|
||||
loadReadUrlCacheEntry,
|
||||
fetchReadUrl,
|
||||
parseReadUrlTarget,
|
||||
type ReadUrlToolDetails,
|
||||
renderReadUrlCall,
|
||||
@@ -2167,15 +2167,12 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
const urlRaw = parsedUrlTarget.raw;
|
||||
const urlRanges = parsedUrlTarget.ranges;
|
||||
if (urlRanges !== undefined && urlRanges.length > 1) {
|
||||
const cached = await loadReadUrlCacheEntry(
|
||||
this.session,
|
||||
{ path: parsedUrlTarget.path, raw: urlRaw },
|
||||
signal,
|
||||
{ ensureArtifact: true, preferCached: true },
|
||||
);
|
||||
return this.#buildInMemoryMultiRangeResult(cached.output, urlRanges, {
|
||||
details: { ...cached.details },
|
||||
sourceUrl: cached.details.finalUrl,
|
||||
const entry = await fetchReadUrl(this.session, { path: parsedUrlTarget.path, raw: urlRaw }, signal, {
|
||||
ensureArtifact: true,
|
||||
});
|
||||
return this.#buildInMemoryMultiRangeResult(entry.output, urlRanges, {
|
||||
details: { ...entry.details },
|
||||
sourceUrl: entry.details.finalUrl,
|
||||
entityLabel: "URL output",
|
||||
raw: urlRaw,
|
||||
immutable: true,
|
||||
@@ -2184,18 +2181,12 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
const urlOffset = parsedUrlTarget.offset;
|
||||
const urlLimit = parsedUrlTarget.limit;
|
||||
if (urlOffset !== undefined || urlLimit !== undefined) {
|
||||
const cached = await loadReadUrlCacheEntry(
|
||||
this.session,
|
||||
{ path: parsedUrlTarget.path, raw: urlRaw },
|
||||
signal,
|
||||
{
|
||||
ensureArtifact: true,
|
||||
preferCached: true,
|
||||
},
|
||||
);
|
||||
return this.#buildInMemoryTextResult(cached.output, urlOffset, urlLimit, {
|
||||
details: { ...cached.details },
|
||||
sourceUrl: cached.details.finalUrl,
|
||||
const entry = await fetchReadUrl(this.session, { path: parsedUrlTarget.path, raw: urlRaw }, signal, {
|
||||
ensureArtifact: true,
|
||||
});
|
||||
return this.#buildInMemoryTextResult(entry.output, urlOffset, urlLimit, {
|
||||
details: { ...entry.details },
|
||||
sourceUrl: entry.details.finalUrl,
|
||||
entityLabel: "URL output",
|
||||
raw: urlRaw,
|
||||
immutable: true,
|
||||
|
||||
@@ -99,6 +99,30 @@ describe("read URL with :raw selector (regression: JSON/feed parsers ignored raw
|
||||
expect(textBlock?.text).toContain('"alpha": 1');
|
||||
});
|
||||
|
||||
it("refetches the same URL on subsequent reads", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
let body = "v1";
|
||||
const loadPage = vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
finalUrl: requestedUrl,
|
||||
contentType: "text/plain",
|
||||
content: body,
|
||||
}));
|
||||
|
||||
const first = await tool.execute("first", { path: "https://example.com/live.txt:raw" });
|
||||
body = "v2";
|
||||
const second = await tool.execute("second", { path: "https://example.com/live.txt:raw" });
|
||||
const firstText = first.content.find(entry => entry.type === "text");
|
||||
const secondText = second.content.find(entry => entry.type === "text");
|
||||
|
||||
expect(firstText?.text).toContain("v1");
|
||||
expect(secondText?.text).toContain("v2");
|
||||
expect(secondText?.text).not.toContain("v1");
|
||||
expect(loadPage).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("returns slices of raw content when :raw is combined with a range", async () => {
|
||||
const session = makeSession(testDir);
|
||||
const tool = new ReadTool(session);
|
||||
|
||||
@@ -61,7 +61,7 @@ describe("search tools with external URL paths", () => {
|
||||
await removeWithRetries(testDir);
|
||||
});
|
||||
|
||||
it("search fetches a URL through the read cache and greps the rendered text", async () => {
|
||||
it("search fetches a URL and greps the rendered text", async () => {
|
||||
stubLoadPage("alpha\nremote needle\nomega\n", "text/plain");
|
||||
const tools = await createTools(createSession(testDir));
|
||||
const tool = tools.find(entry => entry.name === "grep");
|
||||
@@ -77,6 +77,35 @@ describe("search tools with external URL paths", () => {
|
||||
expect(text).not.toContain("Cannot search external URL");
|
||||
});
|
||||
|
||||
it("refetches the same URL before each search", async () => {
|
||||
let body = "first needle\n";
|
||||
const loadPage = vi.spyOn(scrapers, "loadPage").mockImplementation(async requestedUrl => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
finalUrl: requestedUrl,
|
||||
contentType: "text/plain",
|
||||
content: body,
|
||||
}));
|
||||
const tools = await createTools(createSession(testDir));
|
||||
const tool = tools.find(entry => entry.name === "grep");
|
||||
expect(tool).toBeDefined();
|
||||
|
||||
const first = await tool!.execute("search-url-first", {
|
||||
pattern: "first|second",
|
||||
path: "https://example.com/live.txt",
|
||||
});
|
||||
body = "second needle\n";
|
||||
const second = await tool!.execute("search-url-second", {
|
||||
pattern: "first|second",
|
||||
path: "https://example.com/live.txt",
|
||||
});
|
||||
|
||||
expect(resultText(first)).toContain("first needle");
|
||||
expect(resultText(second)).toContain("second needle");
|
||||
expect(resultText(second)).not.toContain("first needle");
|
||||
expect(loadPage).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("search applies URL line-range selectors after materialization", async () => {
|
||||
stubLoadPage("outside before\nremote needle\noutside after\n", "text/plain");
|
||||
const tools = await createTools(createSession(testDir));
|
||||
|
||||
Reference in New Issue
Block a user