From 5d6fcff2f175c24abf094e37afcc80f3301e57a7 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 14 Jul 2026 19:11:48 +0000 Subject: [PATCH] fix(eval): delegated python uri reads to host resolver - Routed non-local URI reads through the session read tool. - Preserved offset and limit as host line selectors. - Covered artifact delegation with the shipped Python prelude. Fixes #5353 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/eval/py/__tests__/prelude.test.ts | 59 +++++++++++++++++++ packages/coding-agent/src/eval/py/prelude.py | 28 ++++++++- .../src/prompts/system/tan-context-switch.md | 2 +- .../coding-agent/src/prompts/tools/eval.md | 2 +- .../agent-session-prune-persistence.test.ts | 4 +- 6 files changed, 91 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9fe2494f3..2e376f13a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -32,6 +32,7 @@ ### Fixed +- Fixed Python eval `read()` rejecting every non-`local://` URI before the host resolver could handle it; URI reads now delegate through the session `read` tool with `offset`/`limit` preserved as line selectors, including spilled `artifact://` output ([#5353](https://github.com/can1357/oh-my-pi/issues/5353)). - Fixed `/tan` and `/fork` clones cold-missing the provider prompt cache: the per-turn supersede/useless-result prune rewrote the live context without persisting it, so file-based forks and resume rebuilt a divergent (un-pruned) prefix and re-wrote the entire cache - Fixed `/tan` pinning the clone's prompt-cache key to the parent's session id instead of the parent's effective cache key, dropping shard affinity when the parent was itself a fork or tan - Fixed inconsistent history rendering when toggling the display setting for compacted items diff --git a/packages/coding-agent/src/eval/py/__tests__/prelude.test.ts b/packages/coding-agent/src/eval/py/__tests__/prelude.test.ts index 7ff55a61f..20f961f9a 100644 --- a/packages/coding-agent/src/eval/py/__tests__/prelude.test.ts +++ b/packages/coding-agent/src/eval/py/__tests__/prelude.test.ts @@ -1,6 +1,30 @@ import { describe, expect, it } from "bun:test"; import { PYTHON_PRELUDE } from "../prelude"; +const pythonPath = Bun.env.PYTHON ?? "python3"; + +async function runPrelude( + code: string, + env: Record, +): Promise<{ stdout: string; stderr: string; exitCode: number }> { + const prelude = PYTHON_PRELUDE.replace( + "from __future__ import annotations", + "from __future__ import annotations\n__omp_display = lambda *args, **kwargs: None", + ); + const script = `${prelude}\n${code}`; + const proc = Bun.spawn([pythonPath, "-c", script], { + stdout: "pipe", + stderr: "pipe", + env: { ...process.env, ...env }, + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + describe("python prelude", () => { it("exposes read(path, offset?, limit?) with positional optional args", () => { // The eval docs advertise `read(path, offset?=1, limit?=None)`. A @@ -17,6 +41,41 @@ describe("python prelude", () => { expect(signature).toContain("limit"); }); + it("delegates artifact URI reads through the host read tool with line selectors", async () => { + const requests: unknown[] = []; + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch: async request => { + requests.push(await request.json()); + return Response.json({ + ok: true, + value: { text: "artifact contents", details: { resolvedPath: "/tmp/21.txt" } }, + }); + }, + }); + + try { + const result = await runPrelude(`print(read("artifact://21", 3, 2))`, { + PI_TOOL_BRIDGE_URL: server.url.toString(), + PI_TOOL_BRIDGE_TOKEN: "test-token", + PI_TOOL_BRIDGE_SESSION: "test-session", + }); + + expect(result).toEqual({ stdout: "artifact contents\n", stderr: "", exitCode: 0 }); + expect(requests).toEqual([ + { + session: "test-session", + run: null, + name: "read", + args: { path: "artifact://21:3-4" }, + }, + ]); + } finally { + server.stop(true); + } + }); + it("exposes isolation artifacts on the agent() handle node", () => { // agent(..., handle=True) is the only escape hatch for // recovering apply=False patch/branch/nested artifacts (the bare diff --git a/packages/coding-agent/src/eval/py/prelude.py b/packages/coding-agent/src/eval/py/prelude.py index 663b2a07a..2681caa8b 100644 --- a/packages/coding-agent/src/eval/py/prelude.py +++ b/packages/coding-agent/src/eval/py/prelude.py @@ -57,6 +57,29 @@ if "__omp_prelude_loaded__" not in globals(): _OMP_INTERNAL_URL_RE = re.compile(r"^([a-z][a-z0-9+.-]*)://(.*)$", re.IGNORECASE) + def _should_delegate_read(path: str | Path) -> bool: + return ( + isinstance(path, str) + and _OMP_INTERNAL_URL_RE.match(path) is not None + and not path.lower().startswith("local://") + ) + + def _with_read_line_selector(path: str, offset: int, limit: int | None) -> str | None: + if offset <= 1 and limit is None: + return path + if limit is not None and limit <= 0: + return None + start = max(1, offset) + if limit is None: + return f"{path}:{start}-" + return f"{path}:{start}-{start + limit - 1}" + + def _read_tool_text(path: str) -> str: + result = _bridge_call("read", {"path": path}) + if isinstance(result, dict) and "text" in result: + return result["text"] + return result + def _resolve_omp_path(path: str | Path) -> Path: """Map a helper path to a real filesystem Path. @@ -94,7 +117,10 @@ if "__omp_prelude_loaded__" not in globals(): return Path(resolved) def read(path: str | Path, offset: int = 1, limit: int | None = None) -> str: - """Read file contents. offset/limit are 1-indexed line numbers.""" + """Read file or read-tool URI contents. offset/limit are 1-indexed lines.""" + if _should_delegate_read(path): + tool_path = _with_read_line_selector(path, offset, limit) + return "" if tool_path is None else _read_tool_text(tool_path) p = _resolve_omp_path(path) data = p.read_text(encoding="utf-8") lines = data.splitlines(keepends=True) diff --git a/packages/coding-agent/src/prompts/system/tan-context-switch.md b/packages/coding-agent/src/prompts/system/tan-context-switch.md index 55468b15a..88cd57291 100644 --- a/packages/coding-agent/src/prompts/system/tan-context-switch.md +++ b/packages/coding-agent/src/prompts/system/tan-context-switch.md @@ -1,5 +1,5 @@ -The conversation above belongs to your parent session. +The conversation above belongs to your parent session. You are a fork created solely to handle the user's request below. Your parent agent is still working on the original task — that responsibility is diff --git a/packages/coding-agent/src/prompts/tools/eval.md b/packages/coding-agent/src/prompts/tools/eval.md index 81ea84b70..4fa76ffa7 100644 --- a/packages/coding-agent/src/prompts/tools/eval.md +++ b/packages/coding-agent/src/prompts/tools/eval.md @@ -28,7 +28,7 @@ display(value) → None print(value, ...) → None Text output. read(path, offset?=1, limit?=None) → str - File as text; offset/limit 1-indexed lines. Accepts `local://…`. + File/resource text; offset/limit = 1-indexed lines. `local://…` works everywhere; Python/JS also accept top-level `read` URI schemes. write(path, content) → str Write file (creates parents) → resolved path. `local://…` persists across turns/subagents. env(key?=None, value?=None) → str | None | dict diff --git a/packages/coding-agent/test/agent-session-prune-persistence.test.ts b/packages/coding-agent/test/agent-session-prune-persistence.test.ts index 3b78c3657..438f0de4e 100644 --- a/packages/coding-agent/test/agent-session-prune-persistence.test.ts +++ b/packages/coding-agent/test/agent-session-prune-persistence.test.ts @@ -114,7 +114,7 @@ describe("AgentSession per-turn prune persistence", () => { const message = session.agent.state.messages.find( candidate => candidate.role === "toolResult" && candidate.toolCallId === BIG_CALL_ID, ); - if (!message || message.role !== "toolResult" || !Array.isArray(message.content)) { + if (message?.role !== "toolResult" || !Array.isArray(message.content)) { throw new Error("Expected the seeded tool result in live agent state"); } const text = message.content.find(block => block.type === "text"); @@ -156,7 +156,7 @@ describe("AgentSession per-turn prune persistence", () => { const rebuilt = reloaded .buildSessionContext() .messages.find(candidate => candidate.role === "toolResult" && candidate.toolCallId === BIG_CALL_ID); - if (!rebuilt || rebuilt.role !== "toolResult" || !Array.isArray(rebuilt.content)) { + if (rebuilt?.role !== "toolResult" || !Array.isArray(rebuilt.content)) { throw new Error("Expected the seeded tool result in the from-disk rebuild"); } const rebuiltText = rebuilt.content.find(block => block.type === "text");