Files
oh-my-pi/packages/coding-agent/test/tools/split-internal-url-sel.test.ts
T
oldschoola 96aad47eb6 fix(coding-agent): address PR #1388 review feedback
Refactor:
- session.ts: extract mapDebugpyMissingModule helper; replace the duplicated
  inline check in launch/attach catch blocks. Add jsdoc on DapStartRequestFailure.settled
  documenting per-call ownership and how throwPreferredDapStartError consumes it.
- path-utils.ts: replace the no-op keepOpaqueResourceUri branch with an
  OPAQUE_RESOURCE_SCHEMES Set so the structure carries the intent. Functionally
  equivalent; new opaque schemes become a one-line Set change.

Tests:
- dap-launch-failures: cover the debugpy stderr -> 'pip install debugpy'
  rewrite for launch and attach, plus a negative case (non-debugpy adapter
  with the substring in stderr is left untouched).
- dap-launch-failures: model the delayed-launch-failure case the new
  settled-race in throwPreferredDapStartError defends against. FakeDapClient
  gains optional launchErrorDelayMs/attachErrorDelayMs.
- dap-launch-failures (DebugTool): assert adapter:'debugpy' early-throw
  surfaces 'python not found in PATH' on both launch and attach when
  selectLaunchAdapter/selectAttachAdapter return null, and the unspecified-adapter
  path still falls back to the generic 'No debugger adapter' error.
- find.ts: export validateFindPathInputs and pin the new backslash-escape
  semantics (\, no longer trips the comma-joined heuristic) plus the
  existing brace-expansion and rejection paths.
- patch.ts: cover the post-write verification error message. The user-facing
  ToolError must contain the caller-supplied relative path and not the
  absolute resolvedPath (which still lives in the structured context for
  log correlation).
- split-internal-url-sel: reword two mcp:// test comments that described a
  'peeler refuses' guard that doesn't exist; rename the tests to reflect the
  actual opaque-scheme rule.
2026-05-26 01:54:09 -07:00

127 lines
5.2 KiB
TypeScript

import { describe, expect, it } from "bun:test";
import { splitInternalUrlSel } from "@oh-my-pi/pi-coding-agent/tools/path-utils";
describe("splitInternalUrlSel", () => {
it("returns the input unchanged when there is no selector tail", () => {
expect(splitInternalUrlSel("artifact://3")).toEqual({ path: "artifact://3" });
expect(splitInternalUrlSel("agent://reviewer_0")).toEqual({ path: "agent://reviewer_0" });
expect(splitInternalUrlSel("memory://root")).toEqual({ path: "memory://root" });
});
it("peels a single line-range selector", () => {
expect(splitInternalUrlSel("artifact://3:1-100")).toEqual({ path: "artifact://3", sel: "1-100" });
expect(splitInternalUrlSel("artifact://3:50+150")).toEqual({ path: "artifact://3", sel: "50+150" });
expect(splitInternalUrlSel("artifact://3:50-")).toEqual({ path: "artifact://3", sel: "50-" });
});
it("peels a `raw` selector", () => {
expect(splitInternalUrlSel("artifact://3:raw")).toEqual({ path: "artifact://3", sel: "raw" });
});
it("peels compound `raw:range` selectors in either order", () => {
expect(splitInternalUrlSel("artifact://3:raw:1-100")).toEqual({
path: "artifact://3",
sel: "raw:1-100",
});
expect(splitInternalUrlSel("artifact://3:1-100:raw")).toEqual({
path: "artifact://3",
sel: "1-100:raw",
});
});
it("peels the malformed `:-N` selector that the strict splitter misses (the original bug)", () => {
expect(splitInternalUrlSel("artifact://3:raw:-100")).toEqual({
path: "artifact://3",
sel: "raw:-100",
});
expect(splitInternalUrlSel("artifact://3:-100")).toEqual({ path: "artifact://3", sel: "-100" });
});
it("peels selectors from skill URLs with namespaced hosts", () => {
expect(splitInternalUrlSel("skill://superpowers:brainstorming:1-5")).toEqual({
path: "skill://superpowers:brainstorming",
sel: "1-5",
});
});
it("does not peel chunks that are not selector-shaped", () => {
// `name` is part of the host, not a selector.
expect(splitInternalUrlSel("skill://plugin:name")).toEqual({ path: "skill://plugin:name" });
});
it("stops at the scheme separator `://`", () => {
expect(splitInternalUrlSel("agent://1-50")).toEqual({ path: "agent://1-50" });
});
it("keeps bare-integer suffixes on mcp:// URLs (could be a port)", () => {
expect(splitInternalUrlSel("mcp://some/resource:1234")).toEqual({
path: "mcp://some/resource:1234",
});
});
it("treats mcp:// URIs as opaque by default — selector-shaped suffixes are NOT peeled", () => {
// MCP resource URIs are server-defined and may legitimately end with `:raw`,
// `:1-50`, etc. Without an explicit escape the URI must be forwarded verbatim
// to the protocol handler so server-defined resources remain reachable.
expect(splitInternalUrlSel("mcp://server/resource:1-50")).toEqual({
path: "mcp://server/resource:1-50",
});
expect(splitInternalUrlSel("mcp://server/resource:raw")).toEqual({
path: "mcp://server/resource:raw",
});
expect(splitInternalUrlSel("mcp://server/resource:L10")).toEqual({
path: "mcp://server/resource:L10",
});
expect(splitInternalUrlSel("mcp://server/resource:conflicts")).toEqual({
path: "mcp://server/resource:conflicts",
});
});
it("keeps escaped selector-shaped mcp:// suffixes opaque too", () => {
// MCP resource URIs are exact server-defined IDs. A resource may
// legitimately end in `/:raw` or `/:1-50`; splitting before resolution
// would make that resource unreachable with no exact-URI escape hatch.
expect(splitInternalUrlSel("mcp://server/resource/:1-50")).toEqual({
path: "mcp://server/resource/:1-50",
});
expect(splitInternalUrlSel("mcp://server/resource/:raw")).toEqual({
path: "mcp://server/resource/:raw",
});
expect(splitInternalUrlSel("mcp://server/resource/:L10")).toEqual({
path: "mcp://server/resource/:L10",
});
expect(splitInternalUrlSel("mcp://server/resource/:conflicts")).toEqual({
path: "mcp://server/resource/:conflicts",
});
});
it("keeps `mcp://:1-50`-shaped degenerate inputs opaque", () => {
// All mcp:// inputs are forwarded verbatim (see OPAQUE_RESOURCE_SCHEMES in
// path-utils.ts). This covers the edge case where the only `:` candidate
// for peeling sits next to the scheme's own `://`, so any peeler that ever
// replaces the opaque-scheme guard must still refuse these inputs.
expect(splitInternalUrlSel("mcp://:1-50")).toEqual({ path: "mcp://:1-50" });
expect(splitInternalUrlSel("mcp://:raw")).toEqual({ path: "mcp://:raw" });
});
it("keeps mcp:// trailing-slash bare-integer suffixes opaque", () => {
// `:1234` could be a port number after a path. The opaque-scheme rule keeps
// it verbatim either way; if mcp ever moves to selector-aware peeling, a
// richer selector form should be required before treating `:N` as a tail.
expect(splitInternalUrlSel("mcp://server/resource/:1234")).toEqual({
path: "mcp://server/resource/:1234",
});
});
it("returns the input unchanged for non-URL strings", () => {
expect(splitInternalUrlSel("/abs/path:1-50")).toEqual({ path: "/abs/path:1-50" });
expect(splitInternalUrlSel("plain-text")).toEqual({ path: "plain-text" });
});
it("does not peel for unknown schemes", () => {
expect(splitInternalUrlSel("http://example.com:1-50")).toEqual({
path: "http://example.com:1-50",
});
});
});