From ecfaf64526a71391d403a2299a698efd06cdd4be Mon Sep 17 00:00:00 2001 From: stain lu <109842185+stainlu@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:07:55 +0800 Subject: [PATCH] fix: align host tilde paths with OS home (#62804) (thanks @stainlu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(tools): expand tilde in host edit/write paths (non-workspace mode) * test: use it.runIf for visible skip when tmpdir is not under home * fix(tools): address Codex P2 review on tilde host edit/write Responds to two P2 findings from chatgpt-codex-connector on #62804: 1. Tests never ran in CI. The it.runIf(tmpdirUnderHome) guard always skipped on Linux runners where os.tmpdir() is /tmp, outside $HOME, so the regression tests reported green without executing. Tmpdirs now use the test-isolated HOME (process.env.HOME from test/test-env.ts) so tests run in every environment and match what expandHomePrefix resolves, keeping them hermetic. 2. Edit recovery path resolution was inconsistent. resolveEditPath inlined os.homedir() for tilde expansion, bypassing OPENCLAW_HOME, while the write/edit operations use expandHomePrefix. Under a custom OPENCLAW_HOME, wrapEditToolWithRecovery's readback targeted a different file than the edit actually touched, so successful edits could be reported as failures. resolveEditPath now uses the same expandHomePrefix helper. * test(tools): verify tilde expansion honors OPENCLAW_HOME override The prior tests covered tilde expansion but only under the default test home, which matches os.homedir(). That passed whether the production code used expandHomePrefix() or inlined os.homedir() — the behaviors only diverge when OPENCLAW_HOME is set to a path outside $HOME. Adds four tests that set OPENCLAW_HOME to a temp dir explicitly outside $HOME and verify that write/mkdir/read/access tilde operations resolve against OPENCLAW_HOME, not os.homedir(). These would fail if pi-tools.read.ts or pi-tools.host-edit.ts reverted to os.homedir(), directly covering the Codex P2 feedback about OPENCLAW_HOME consistency. Uses the same env snapshot/restore pattern as test/helpers/temp-home.ts. * Agents: resolve host tilde paths against OS home * fix: align host tilde paths with OS home (#62804) (thanks @stainlu) * fix: keep the changelog entry in the active block (#62804) (thanks @stainlu) --------- Co-authored-by: Ayaan Zaidi --- CHANGELOG.md | 1 + src/agents/pi-tools.host-edit.ts | 9 +- .../pi-tools.read.host-edit-recovery.test.ts | 57 ++++++ ...pi-tools.read.host-tilde-expansion.test.ts | 171 ++++++++++++++++++ src/agents/pi-tools.read.ts | 14 +- 5 files changed, 242 insertions(+), 10 deletions(-) create mode 100644 src/agents/pi-tools.read.host-tilde-expansion.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f7c2f10a3ba..fdd6277f6f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai - BlueBubbles/catchup: add a per-message retry ceiling (`catchup.maxFailureRetries`, default 10) so a persistently-failing message with a malformed payload no longer wedges the catchup cursor forever. After N consecutive `processMessage` failures against the same GUID, catchup logs a WARN, skips that message on subsequent sweeps, and lets the cursor advance past it. Transient failures still retry from the same point as before. Also fixes a lost-update race in the persistent dedupe file lock that silently dropped inbound GUIDs on concurrent writes, a dedupe file naming migration gap on version upgrade, and a balloon-event bypass that let catchup replay debouncer-coalesced events as standalone messages. (#67426, #66870) Thanks @omarshahine. - Ollama/chat: strip the `ollama/` provider prefix from Ollama chat request model ids so configured refs like `ollama/qwen3:14b-q8_0` stop 404ing against the Ollama API. (#67457) Thanks @suboss87. - QA/Matrix: split the private QA lab runtime into smaller tested modules, add Matrix media contract coverage for image understanding and generated-image delivery, and update the memory-dreaming QA sweep to assert the separate phase-report layout. (#67430) Thanks @gumadeiras. +- Agents/tools: resolve non-workspace host tilde paths against the OS home directory and keep edit recovery aligned with that same path target, so `~/...` host edit/write operations stop failing or reading back the wrong file when `OPENCLAW_HOME` differs. (#62804) Thanks @stainlu. ## 2026.4.15-beta.1 diff --git a/src/agents/pi-tools.host-edit.ts b/src/agents/pi-tools.host-edit.ts index b3fee70e35e..dbe6cb37c06 100644 --- a/src/agents/pi-tools.host-edit.ts +++ b/src/agents/pi-tools.host-edit.ts @@ -1,6 +1,6 @@ -import os from "node:os"; import path from "node:path"; import type { AgentToolResult, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core"; +import { expandHomePrefix, resolveOsHomeDir } from "../infra/home-dir.js"; import { getToolParamsRecord } from "./pi-tools.params.js"; import type { AnyAgentTool } from "./pi-tools.types.js"; @@ -22,12 +22,9 @@ type EditReplacement = { const EDIT_MISMATCH_MESSAGE = "Could not find the exact text in"; const EDIT_MISMATCH_HINT_LIMIT = 800; -/** Resolve path for edit recovery: expand ~ and resolve relative paths against root. */ function resolveEditPath(root: string, pathParam: string): string { - const expanded = - pathParam.startsWith("~/") || pathParam === "~" - ? pathParam.replace(/^~/, os.homedir()) - : pathParam; + const home = resolveOsHomeDir(); + const expanded = home ? expandHomePrefix(pathParam, { home }) : pathParam; return path.isAbsolute(expanded) ? path.resolve(expanded) : path.resolve(root, expanded); } diff --git a/src/agents/pi-tools.read.host-edit-recovery.test.ts b/src/agents/pi-tools.read.host-edit-recovery.test.ts index f6679138897..9a5f9d5758b 100644 --- a/src/agents/pi-tools.read.host-edit-recovery.test.ts +++ b/src/agents/pi-tools.read.host-edit-recovery.test.ts @@ -225,6 +225,63 @@ describe("edit tool recovery hardening", () => { }); }); + it("recovers tilde paths against the OS home even when OPENCLAW_HOME differs", async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-")); + const osHome = path.join(tmpDir, "home"); + const openclawHome = path.join(tmpDir, "openclaw-home"); + await fs.mkdir(osHome, { recursive: true }); + await fs.mkdir(openclawHome, { recursive: true }); + + const previousHome = process.env.HOME; + const previousUserProfile = process.env.USERPROFILE; + const previousOpenclawHome = process.env.OPENCLAW_HOME; + process.env.HOME = osHome; + process.env.USERPROFILE = osHome; + process.env.OPENCLAW_HOME = openclawHome; + + try { + const filePath = path.join(osHome, "demo.txt"); + await fs.writeFile(filePath, "before old text after\n", "utf-8"); + + const tool = createRecoveredEditTool({ + root: tmpDir, + readFile: (absolutePath) => fs.readFile(absolutePath, "utf-8"), + execute: async () => { + await fs.writeFile(filePath, "before new text after\n", "utf-8"); + throw new Error("Simulated post-write failure (e.g. generateDiffString)"); + }, + }); + const result = await tool.execute( + "call-1", + { path: "~/demo.txt", edits: [{ oldText: "old text", newText: "new text" }] }, + undefined, + ); + + expect(result).toMatchObject({ isError: false }); + expect(result.content[0]).toMatchObject({ + type: "text", + text: "Successfully replaced text in ~/demo.txt.", + }); + await expect(fs.access(path.join(openclawHome, "demo.txt"))).rejects.toBeDefined(); + } finally { + if (previousHome === undefined) { + delete process.env.HOME; + } else { + process.env.HOME = previousHome; + } + if (previousUserProfile === undefined) { + delete process.env.USERPROFILE; + } else { + process.env.USERPROFILE = previousUserProfile; + } + if (previousOpenclawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = previousOpenclawHome; + } + } + }); + it("applies the same recovery path to sandboxed edit tools", async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-edit-recovery-")); const filePath = path.join(tmpDir, "demo.txt"); diff --git a/src/agents/pi-tools.read.host-tilde-expansion.test.ts b/src/agents/pi-tools.read.host-tilde-expansion.test.ts new file mode 100644 index 00000000000..b4d5f043236 --- /dev/null +++ b/src/agents/pi-tools.read.host-tilde-expansion.test.ts @@ -0,0 +1,171 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +type CapturedEditOperations = { + readFile: (absolutePath: string) => Promise; + writeFile: (absolutePath: string, content: string) => Promise; + access: (absolutePath: string) => Promise; +}; + +type CapturedWriteOperations = { + mkdir: (dir: string) => Promise; + writeFile: (absolutePath: string, content: string) => Promise; +}; + +const mocks = vi.hoisted(() => ({ + editOps: undefined as CapturedEditOperations | undefined, + writeOps: undefined as CapturedWriteOperations | undefined, +})); + +vi.mock("@mariozechner/pi-coding-agent", async () => { + const actual = await vi.importActual( + "@mariozechner/pi-coding-agent", + ); + return { + ...actual, + createEditTool: (_cwd: string, options?: { operations?: CapturedEditOperations }) => { + mocks.editOps = options?.operations; + return { + name: "edit", + description: "test edit tool", + parameters: { type: "object", properties: {} }, + execute: async () => ({ content: [{ type: "text" as const, text: "ok" }] }), + }; + }, + createWriteTool: (_cwd: string, options?: { operations?: CapturedWriteOperations }) => { + mocks.writeOps = options?.operations; + return { + name: "write", + description: "test write tool", + parameters: { type: "object", properties: {} }, + execute: async () => ({ content: [{ type: "text" as const, text: "ok" }] }), + }; + }, + }; +}); + +const { createHostWorkspaceEditTool, createHostWorkspaceWriteTool } = + await import("./pi-tools.read.js"); + +const osHome = () => process.env.HOME ?? os.homedir(); +const toTildePath = (absolutePath: string) => absolutePath.replace(osHome(), "~"); + +describe("host tool tilde expansion (non-workspace mode)", () => { + const tempDirs: string[] = []; + + const createTempDir = async (prefix: string, parent = osHome()) => { + const dir = await fs.mkdtemp(path.join(parent, prefix)); + tempDirs.push(dir); + return dir; + }; + + beforeEach(() => { + mocks.editOps = undefined; + mocks.writeOps = undefined; + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + mocks.editOps = undefined; + mocks.writeOps = undefined; + while (tempDirs.length > 0) { + await fs.rm(tempDirs.pop()!, { recursive: true, force: true }); + } + }); + + it("edit readFile expands ~ to the OS home directory", async () => { + const dir = await createTempDir("openclaw-tilde-test-edit-"); + const testFile = path.join(dir, "test.txt"); + await fs.writeFile(testFile, "hello", "utf8"); + + createHostWorkspaceEditTool(dir, { workspaceOnly: false }); + const content = await mocks.editOps!.readFile(toTildePath(testFile)); + + expect(content.toString("utf8")).toBe("hello"); + }); + + it("edit access expands ~ to the OS home directory", async () => { + const dir = await createTempDir("openclaw-tilde-test-edit-"); + const testFile = path.join(dir, "test.txt"); + await fs.writeFile(testFile, "hello", "utf8"); + + createHostWorkspaceEditTool(dir, { workspaceOnly: false }); + + await expect(mocks.editOps!.access(toTildePath(testFile))).resolves.toBeUndefined(); + }); + + it("write writeFile expands ~ to the OS home directory", async () => { + const dir = await createTempDir("openclaw-tilde-test-write-"); + const testFile = path.join(dir, "tilde-write-test.txt"); + + createHostWorkspaceWriteTool(dir, { workspaceOnly: false }); + await mocks.writeOps!.writeFile(toTildePath(testFile), "written via tilde"); + + expect(await fs.readFile(testFile, "utf8")).toBe("written via tilde"); + }); + + it("write mkdir expands ~ to the OS home directory", async () => { + const dir = await createTempDir("openclaw-tilde-test-mkdir-"); + const newDir = path.join(dir, "subdir"); + + createHostWorkspaceWriteTool(dir, { workspaceOnly: false }); + await mocks.writeOps!.mkdir(toTildePath(newDir)); + + expect((await fs.stat(newDir)).isDirectory()).toBe(true); + }); + + it("ignores OPENCLAW_HOME for write operations", async () => { + const openclawHome = await createTempDir("openclaw-home-override-", os.tmpdir()); + const dir = await createTempDir("openclaw-tilde-test-write-"); + const testFile = path.join(dir, "os-home-write.txt"); + vi.stubEnv("OPENCLAW_HOME", openclawHome); + + createHostWorkspaceWriteTool(openclawHome, { workspaceOnly: false }); + await mocks.writeOps!.writeFile(toTildePath(testFile), "written via os home"); + + expect(await fs.readFile(testFile, "utf8")).toBe("written via os home"); + await expect(fs.access(path.join(openclawHome, path.basename(testFile)))).rejects.toBeDefined(); + }); + + it("ignores OPENCLAW_HOME for mkdir operations", async () => { + const openclawHome = await createTempDir("openclaw-home-override-", os.tmpdir()); + const dir = await createTempDir("openclaw-tilde-test-mkdir-"); + const newDir = path.join(dir, "os-home-subdir"); + vi.stubEnv("OPENCLAW_HOME", openclawHome); + + createHostWorkspaceWriteTool(openclawHome, { workspaceOnly: false }); + await mocks.writeOps!.mkdir(toTildePath(newDir)); + + expect((await fs.stat(newDir)).isDirectory()).toBe(true); + await expect(fs.access(path.join(openclawHome, path.basename(newDir)))).rejects.toBeDefined(); + }); + + it("ignores OPENCLAW_HOME for readFile operations", async () => { + const openclawHome = await createTempDir("openclaw-home-override-", os.tmpdir()); + const dir = await createTempDir("openclaw-tilde-test-edit-"); + const testFile = path.join(dir, "os-home-read.txt"); + await fs.writeFile(testFile, "OS home content", "utf8"); + vi.stubEnv("OPENCLAW_HOME", openclawHome); + + createHostWorkspaceEditTool(openclawHome, { workspaceOnly: false }); + const content = await mocks.editOps!.readFile(toTildePath(testFile)); + + expect(content.toString("utf8")).toBe("OS home content"); + await expect(fs.access(path.join(openclawHome, path.basename(testFile)))).rejects.toBeDefined(); + }); + + it("ignores OPENCLAW_HOME for access operations", async () => { + const openclawHome = await createTempDir("openclaw-home-override-", os.tmpdir()); + const dir = await createTempDir("openclaw-tilde-test-edit-"); + const testFile = path.join(dir, "os-home-access.txt"); + await fs.writeFile(testFile, "exists", "utf8"); + vi.stubEnv("OPENCLAW_HOME", openclawHome); + + createHostWorkspaceEditTool(openclawHome, { workspaceOnly: false }); + + await expect(mocks.editOps!.access(toTildePath(testFile))).resolves.toBeUndefined(); + await expect(fs.access(path.join(openclawHome, path.basename(testFile)))).rejects.toBeDefined(); + }); +}); diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index 9da1f3b095e..ade0c7e37e0 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -11,6 +11,7 @@ import { readFileWithinRoot, writeFileWithinRoot, } from "../infra/fs-safe.js"; +import { expandHomePrefix, resolveOsHomeDir } from "../infra/home-dir.js"; import { hasEncodedFileUrlSeparator, trySafeFileURLToPath } from "../infra/local-file-access.js"; import { detectMime } from "../media/mime.js"; import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js"; @@ -746,8 +747,13 @@ function createSandboxEditOperations(params: SandboxToolParams) { } as const; } +function expandTildeToOsHome(filePath: string): string { + const home = resolveOsHomeDir(); + return home ? expandHomePrefix(filePath, { home }) : filePath; +} + async function writeHostFile(absolutePath: string, content: string) { - const resolved = path.resolve(absolutePath); + const resolved = path.resolve(expandTildeToOsHome(absolutePath)); await fs.mkdir(path.dirname(resolved), { recursive: true }); await fs.writeFile(resolved, content, "utf-8"); } @@ -759,7 +765,7 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo // When workspaceOnly is false, allow writes anywhere on the host return { mkdir: async (dir: string) => { - const resolved = path.resolve(dir); + const resolved = path.resolve(expandTildeToOsHome(dir)); await fs.mkdir(resolved, { recursive: true }); }, writeFile: writeHostFile, @@ -793,12 +799,12 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool // When workspaceOnly is false, allow edits anywhere on the host return { readFile: async (absolutePath: string) => { - const resolved = path.resolve(absolutePath); + const resolved = path.resolve(expandTildeToOsHome(absolutePath)); return await fs.readFile(resolved); }, writeFile: writeHostFile, access: async (absolutePath: string) => { - const resolved = path.resolve(absolutePath); + const resolved = path.resolve(expandTildeToOsHome(absolutePath)); await fs.access(resolved); }, } as const;