fix(write): rejected unknown uri-like targets
- Blocked malformed and unregistered URI-like paths before filesystem resolution. - Suggested canonical xd:// spelling while preserving explicitly escaped local paths. - Added regression coverage for xdt://, xd:/, and xd/ near misses. Fixes #6123
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed near-miss `xd://` write targets silently creating filesystem paths instead of surfacing a corrective URI error ([#6123](https://github.com/can1357/oh-my-pi/issues/6123)).
|
||||
|
||||
## [17.0.5] - 2026-07-18
|
||||
|
||||
### Added
|
||||
|
||||
@@ -47,7 +47,7 @@ For independent per-item chains (review → verify, fetch → extract → score)
|
||||
schema: FINDINGS_SCHEMA,
|
||||
});
|
||||
return await parallel(found.findings.map((f) => async () => ({
|
||||
...f,
|
||||
…f,
|
||||
verdict: await agent(
|
||||
`Refute if you can (default refuted when unsure): ${f.title}`,
|
||||
{ label: `verify:${f.file}`, schema: VERDICT_SCHEMA },
|
||||
@@ -57,8 +57,6 @@ For independent per-item chains (review → verify, fetch → extract → score)
|
||||
phase("Review");
|
||||
const results = await parallel(DIMENSIONS.map((d) => async () => reviewAndVerify(d)));
|
||||
const confirmed = results.flat().filter((f) => f.verdict.is_real);
|
||||
|
||||
|
||||
Reach for `pipeline()` only when a stage genuinely needs ALL of the previous stage first — dedup/merge across the whole set, early-exit on zero, or "compare against the other findings" — because its inter-stage barrier makes every item wait for the slowest peer:
|
||||
|
||||
**Python (`eval`, Python backend):**
|
||||
@@ -80,8 +78,6 @@ Reach for `pipeline()` only when a stage genuinely needs ALL of the previous sta
|
||||
const verdicts = await parallel(findings.map((f) => async () =>
|
||||
await agent(verifyPrompt(f), { schema: VERDICT_SCHEMA }),
|
||||
));
|
||||
|
||||
|
||||
Use ordinary code between calls to flatten/map/filter; don't add a barrier just for that. Nested `parallel()` pools each cap independently, so keep total fan-out sane.
|
||||
</structure>
|
||||
|
||||
|
||||
@@ -85,6 +85,33 @@ import { renderXdevCall, renderXdevResult, type XdevDispatch } from "./xdev";
|
||||
|
||||
const LOOSE_HASHLINE_HEADER_RE = /^\s*\[[^#\r\n]+#[^ \t\r\n]*\]\s*$/;
|
||||
const EXECUTABLE_NOTICE = "[Notice: Made executable via chmod +x]";
|
||||
const URI_LIKE_WRITE_PATH_RE = /^([a-z][a-z0-9+.-]*):\/{1,2}(.*)$/i;
|
||||
const XD_MISSING_DELIMITER_RE = /^xd\/+(.*)$/i;
|
||||
const XD_SCHEME_NEAR_MISSES: Record<string, true> = { dx: true, xdd: true, xdt: true };
|
||||
|
||||
function assertWriteTargetAddressable(target: string, router: InternalUrlRouter): void {
|
||||
const trimmed = target.trim();
|
||||
if (path.win32.isAbsolute(trimmed) || router.canHandle(trimmed)) return;
|
||||
|
||||
const missingDelimiter = trimmed.match(XD_MISSING_DELIMITER_RE);
|
||||
if (missingDelimiter) {
|
||||
throw new ToolError(
|
||||
`Unknown URI-like write target '${trimmed}'. Did you mean 'xd://${missingDelimiter[1]}'? Prefix the path with './' to write it as a filesystem path.`,
|
||||
);
|
||||
}
|
||||
|
||||
const uriLike = trimmed.match(URI_LIKE_WRITE_PATH_RE);
|
||||
if (!uriLike) return;
|
||||
|
||||
const scheme = uriLike[1]!.toLowerCase();
|
||||
const canonicalScheme = router.getHandler(scheme) ? scheme : XD_SCHEME_NEAR_MISSES[scheme] ? "xd" : undefined;
|
||||
const suggestion = canonicalScheme
|
||||
? ` Did you mean '${canonicalScheme}://${uriLike[2]}'?`
|
||||
: " Tool devices use 'xd://<tool>'.";
|
||||
throw new ToolError(
|
||||
`Unknown URI-like write target '${trimmed}'.${suggestion} Prefix the path with './' to write it as a filesystem path.`,
|
||||
);
|
||||
}
|
||||
|
||||
const BULK_DIRECTIVE_RE = /^#?(\d+)\s*[:=]\s*(@ours|@theirs|@base|@both)$/;
|
||||
/**
|
||||
@@ -987,6 +1014,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
// Strip hashline display prefixes ([PATH#HASH] + LINE:) if the model copied them from read output
|
||||
const { text: cleanContent, stripped } = stripWriteContent(this.session, content);
|
||||
const internalRouter = InternalUrlRouter.instance();
|
||||
assertWriteTargetAddressable(path, internalRouter);
|
||||
if (internalRouter.canHandle(path)) {
|
||||
const parsed = parseInternalUrl(path);
|
||||
const scheme = parsed.protocol.replace(/:$/, "").toLowerCase();
|
||||
|
||||
@@ -86,6 +86,32 @@ describe("read and write route xd:// device URLs", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects near-miss xd addresses before filesystem fallback", async () => {
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-near-miss-"));
|
||||
try {
|
||||
const tools = await createTools(xdevSession(tempDir));
|
||||
const write = tools.find(entry => entry.name === "write");
|
||||
expect(write).toBeDefined();
|
||||
|
||||
for (const target of ["xdt://web_search", "xd:/web_search", "xd/web_search"]) {
|
||||
await expect(write!.execute(`write-${target}`, { path: target, content: "{}" })).rejects.toThrow(
|
||||
"Did you mean 'xd://web_search'?",
|
||||
);
|
||||
}
|
||||
expect(await Bun.file(path.join(tempDir, "xdt:/web_search")).exists()).toBe(false);
|
||||
expect(await Bun.file(path.join(tempDir, "xd/web_search")).exists()).toBe(false);
|
||||
|
||||
const escaped = await write!.execute("write-explicit-path", {
|
||||
path: "./xd/web_search",
|
||||
content: "intentional file",
|
||||
});
|
||||
expect(escaped.isError).toBeUndefined();
|
||||
expect(await Bun.file(path.join(tempDir, "xd/web_search")).text()).toBe("intentional file");
|
||||
} finally {
|
||||
await removeWithRetries(tempDir);
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves function-valued device approvals per payload and fails closed on bad content", async () => {
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-approval-"));
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user