Merge PR #6811: fix(tools): refuse write targets shaped as a read-selector list (@roboomp)

This commit is contained in:
can1357
2026-07-28 10:59:36 +02:00
3 changed files with 55 additions and 0 deletions
+1
View File
@@ -99,6 +99,7 @@
- Fixed `/live` sideband WebSockets ignoring standard proxy environment variables and `NO_PROXY`, which left proxied sessions stuck while the rest of the Codex connection succeeded ([#6770](https://github.com/can1357/oh-my-pi/issues/6770)).
- Fixed the bash tool's `kill` builtin rejecting numeric signals and multiple process operands, stopping after the first failed target, and defaulting to `SIGKILL` instead of the standard `SIGTERM`. Negative PID operands (process groups per `kill(2)`) and the `--` end-of-options marker are now handled instead of being misparsed as signals ([#6779](https://github.com/can1357/oh-my-pi/issues/6779)).
- Fixed `learned.md` saves growing a blank line on every write (trailing-newline split artifact) and hoisting all headings/prose above all bullets, which re-scoped lessons under the wrong heading in hand-organized files. Saves are now byte-idempotent and preserve mixed Markdown ordering: non-list lines keep their positions, new lessons insert newest-first at the head of the first bullet run, and dedupe/cap operate on bullet lines in place.
- Fixed the `write` tool treating a semicolon-joined list of read selectors (e.g. `a.txt:1-2;b/c.txt:3-4`) as a filesystem path, silently creating a nested directory tree when a read-only step mis-dispatched a multi-file `read` as `write`. Such targets are now refused regardless of `content`, since no real write targets a `;`-list whose every segment carries a read selector ([#6809](https://github.com/can1357/oh-my-pi/issues/6809)).
## [17.1.5] - 2026-07-27
+31
View File
@@ -160,7 +160,38 @@ function throwReadSelectorMisfire(target: string, sel: string): never {
);
}
/**
* Recognize a semicolon-joined list of read-tool selectors mis-dispatched as a
* single write target — the multi-file read expression the scout emitted in
* issue #6809 (`a.txt:1-2;b/c.txt:3-4`). Every `;`-segment must be non-empty and
* carry its own read selector ({@link splitPathAndSel} peels a `:N-M`, `:raw`,
* or `:conflicts` tail). No real call targets such a list: `read` accepts one
* path, `write` writes one file. Unlike {@link readSelectorForEmptyWrite} this
* fires regardless of `content` — the non-empty-content escape hatch exists for
* a lone selector-shaped *filename*, never a `;`-list, and honoring it here
* silently creates a nested directory tree (`a.txt:1-2;b/`) in the workspace.
*/
function readSelectorListMisfire(target: string): number | undefined {
if (!target.includes(";")) return undefined;
const segments = target.split(";");
if (segments.length < 2) return undefined;
for (const segment of segments) {
const trimmed = segment.trim();
if (trimmed.length === 0 || splitPathAndSel(trimmed).sel === undefined) return undefined;
}
return segments.length;
}
function throwReadSelectorListMisfire(target: string, count: number): never {
throw new ToolError(
`write target '${target}' is a semicolon-joined list of ${count} read-tool selectors, not a filesystem path — refusing to create it. ` +
`write creates a single file; issue one read() per path to read these ranges (e.g. read({ path: "<one path>:<range>" })).`,
);
}
async function assertNotReadSelectorMisfire(target: string, content: string, cwd: string): Promise<void> {
const listCount = readSelectorListMisfire(target);
if (listCount !== undefined) throwReadSelectorListMisfire(target, listCount);
const sel = readSelectorForEmptyWrite(target, content);
if (sel === undefined) return;
if ((await probeLiteralPathExists(target, cwd)) !== "missing") return;
@@ -103,4 +103,27 @@ describe("write refuses read-selector misfires", () => {
expect(entries.get(member)).toEqual(new Uint8Array());
await fs.rm(dir, { recursive: true, force: true });
});
it("rejects a semicolon-joined selector list with non-empty content and creates nothing", async () => {
const dir = await makeWorkspace();
const write = new WriteTool(session(dir));
const target = "a.txt:1-2;b/c.txt:3-4";
await expect(write.execute("c", { path: target, content: "{}" })).rejects.toThrow(
/semicolon-joined list of 2 read-tool selectors/,
);
expect(await Bun.file(path.join(dir, target)).exists()).toBe(false);
expect(await Bun.file(path.join(dir, "a.txt:1-2;b/c.txt:3-4")).exists()).toBe(false);
expect(await fs.readdir(dir)).toEqual(["src"]);
await fs.rm(dir, { recursive: true, force: true });
});
it("still writes a real path that merely contains a semicolon (no per-segment selectors)", async () => {
const dir = await makeWorkspace();
const write = new WriteTool(session(dir));
const target = "notes;draft.txt";
const res = await write.execute("c", { path: target, content: "hi" });
expect(res.isError).toBeUndefined();
expect(await Bun.file(path.join(dir, target)).text()).toBe("hi");
await fs.rm(dir, { recursive: true, force: true });
});
});