From 96aad47eb6dccdbaf0b81e4e18d594f0896c6503 Mon Sep 17 00:00:00 2001 From: oldschoola Date: Mon, 25 May 2026 23:58:00 -0700 Subject: [PATCH] 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. --- packages/coding-agent/src/dap/session.ts | 33 +++- packages/coding-agent/src/tools/find.ts | 2 +- packages/coding-agent/src/tools/path-utils.ts | 28 ++- .../test/debug/dap-launch-failures.test.ts | 168 ++++++++++++++++++ .../test/edit-patch-unchanged-error.test.ts | 112 ++++++++++++ .../test/tools/find-validate-paths.test.ts | 42 +++++ .../test/tools/split-internal-url-sel.test.ts | 15 +- 7 files changed, 371 insertions(+), 29 deletions(-) create mode 100644 packages/coding-agent/test/edit-patch-unchanged-error.test.ts create mode 100644 packages/coding-agent/test/tools/find-validate-paths.test.ts diff --git a/packages/coding-agent/src/dap/session.ts b/packages/coding-agent/src/dap/session.ts index c238e028d..0dc66c63b 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -108,6 +108,14 @@ function toErrorMessage(value: unknown): string { interface DapStartRequestFailure { rejected: boolean; error?: unknown; + /** + * Resolves (never rejects) when the underlying launch/attach request + * settles either way. Set by {@link trackDapStartRequest} on each call, + * so a single failure object must not be reused across launch attempts. + * Consumed by {@link throwPreferredDapStartError} to bound how long to + * wait for a delayed adapter-side rejection before falling back to the + * cascade error from configurationDone. + */ settled?: Promise; } @@ -146,6 +154,21 @@ async function throwPreferredDapStartError( } throw configurationError; } + +const DEBUGPY_MISSING_MODULE_RE = /No module named ['"]?debugpy['"]?/; + +/** + * Map a generic adapter-side failure into the targeted `pip install debugpy` + * hint when the adapter is debugpy and stderr/the wrapping error mentions + * the missing module. Returns null when the heuristic does not apply, so the + * caller can rethrow the original error untouched. + */ +function mapDebugpyMissingModule(adapterName: string, error: unknown): Error | null { + if (adapterName !== "debugpy") return null; + if (!DEBUGPY_MISSING_MODULE_RE.test(toErrorMessage(error))) return null; + return new Error("adapter 'debugpy' is not available: install with 'pip install debugpy'"); +} + function normalizePath(filePath: string): string { return path.resolve(filePath); } @@ -280,9 +303,8 @@ export class DapSessionManager { return buildSummary(session); } catch (error) { await this.#disposeSession(session); - if (options.adapter.name === "debugpy" && /No module named ['"]?debugpy['"]?/.test(toErrorMessage(error))) { - throw new Error("adapter 'debugpy' is not available: install with 'pip install debugpy'"); - } + const mapped = mapDebugpyMissingModule(options.adapter.name, error); + if (mapped) throw mapped; throw error; } } @@ -339,9 +361,8 @@ export class DapSessionManager { return buildSummary(session); } catch (error) { await this.#disposeSession(session); - if (options.adapter.name === "debugpy" && /No module named ['"]?debugpy['"]?/.test(toErrorMessage(error))) { - throw new Error("adapter 'debugpy' is not available: install with 'pip install debugpy'"); - } + const mapped = mapDebugpyMissingModule(options.adapter.name, error); + if (mapped) throw mapped; throw error; } } diff --git a/packages/coding-agent/src/tools/find.ts b/packages/coding-agent/src/tools/find.ts index b53a1a38f..8f0f2ac87 100644 --- a/packages/coding-agent/src/tools/find.ts +++ b/packages/coding-agent/src/tools/find.ts @@ -59,7 +59,7 @@ const MAX_GLOB_TIMEOUT_MS = 60_000; * Commas inside brace expansion (`{a,b}`) are legitimate glob syntax and * must pass through. */ -function validateFindPathInputs(paths: readonly string[]): void { +export function validateFindPathInputs(paths: readonly string[]): void { for (const entry of paths) { let braceDepth = 0; for (let i = 0; i < entry.length; i++) { diff --git a/packages/coding-agent/src/tools/path-utils.ts b/packages/coding-agent/src/tools/path-utils.ts index e5770ef53..d446628c0 100644 --- a/packages/coding-agent/src/tools/path-utils.ts +++ b/packages/coding-agent/src/tools/path-utils.ts @@ -29,6 +29,13 @@ const INTERNAL_SCHEMES_WITH_SELECTORS: Record = { rule: true, skill: true, }; +// Schemes whose resource URIs are server-defined and may legitimately end +// with selector-shaped tails (e.g. `:raw`, `:conflicts`, `:1-50`, `/:raw`). +// `McpProtocolHandler` resolves by exact URI match (`r.uri === uri`), so +// peeling syntactically can make valid resources unreachable. Keep these +// schemes opaque; selector support for them needs a resolver-aware path that +// tries the exact URI before interpreting any suffix as a read selector. +const OPAQUE_RESOURCE_SCHEMES: ReadonlySet = new Set(["mcp"]); const INTERNAL_URL_SCHEME_RE = /^([a-z][a-z0-9+.-]*):\/\//i; const NARROW_NO_BREAK_SPACE = "\u202F"; const TOP_LEVEL_INTERNAL_URL_PREFIXES = [ @@ -173,27 +180,16 @@ export function splitPathAndSel(rawPath: string): { path: string; sel?: string } * * Falls back to the input unchanged when nothing matches. */ -/** MCP resource URIs are server-defined and may legitimately end with - * selector-shaped tails like `:raw`, `:conflicts`, `:1-50`, or even `/:raw`. - * `McpProtocolHandler` resolves by exact resource URI match (`r.uri === uri`), - * so syntactically peeling a selector here can make valid resources - * unreachable. Keep mcp:// opaque; selector support for MCP resources needs a - * resolver-aware path that can try the exact URI before interpreting a suffix - * as a read selector. */ -function keepOpaqueResourceUri(rawPath: string): { path: string } { - return { path: rawPath }; -} export function splitInternalUrlSel(rawPath: string): { path: string; sel?: string } { const schemeMatch = rawPath.match(INTERNAL_URL_SCHEME_RE); if (!schemeMatch) return { path: rawPath }; const scheme = schemeMatch[1].toLowerCase(); - // `mcp://` resource URIs are server-defined and may legitimately contain - // selector-shaped suffixes. Keep them opaque so exact resource lookup wins. - if (!INTERNAL_SCHEMES_WITH_SELECTORS[scheme]) { - if (scheme === "mcp") return keepOpaqueResourceUri(rawPath); - return { path: rawPath }; - } + // Opaque schemes (mcp://, etc.) carry server-defined resource URIs that may + // legitimately end in selector-shaped tails. Forward verbatim — see + // OPAQUE_RESOURCE_SCHEMES. + if (OPAQUE_RESOURCE_SCHEMES.has(scheme)) return { path: rawPath }; + if (!INTERNAL_SCHEMES_WITH_SELECTORS[scheme]) return { path: rawPath }; const schemeEnd = schemeMatch[0].length; let path = rawPath; diff --git a/packages/coding-agent/test/debug/dap-launch-failures.test.ts b/packages/coding-agent/test/debug/dap-launch-failures.test.ts index af2e5363c..83a133e95 100644 --- a/packages/coding-agent/test/debug/dap-launch-failures.test.ts +++ b/packages/coding-agent/test/debug/dap-launch-failures.test.ts @@ -35,7 +35,9 @@ class FakeDapClient { readonly cwd: string, readonly options: { launchError?: string; + launchErrorDelayMs?: number; attachError?: string; + attachErrorDelayMs?: number; configurationDoneError?: string; rejectStopWaiters?: boolean; }, @@ -62,9 +64,11 @@ class FakeDapClient { async sendRequest(command: string): Promise { if (command === "launch" && this.options.launchError) { + if (this.options.launchErrorDelayMs) await Bun.sleep(this.options.launchErrorDelayMs); throw new Error(this.options.launchError); } if (command === "attach" && this.options.attachError) { + if (this.options.attachErrorDelayMs) await Bun.sleep(this.options.attachErrorDelayMs); throw new Error(this.options.attachError); } if (command === "configurationDone" && this.options.configurationDoneError) { @@ -198,6 +202,90 @@ describe("DAP launch failure handling", () => { expect(message).toContain("ENOENT"); expect(message).toContain(TEST_ADAPTER.name); }); + + it("surfaces 'pip install debugpy' when launch stderr mentions missing module", async () => { + const manager = new DapSessionManager(); + const debugpyAdapter: DapResolvedAdapter = { ...TEST_ADAPTER, name: "debugpy" }; + const fake = new FakeDapClient(debugpyAdapter, process.cwd(), { + launchError: "ImportError: No module named 'debugpy'", + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.launch({ adapter: debugpyAdapter, program: "/bin/echo", cwd: process.cwd() }); + } catch (error) { + expect(error).toBeInstanceOf(Error); + message = (error as Error).message; + } + + expect(message).toContain("pip install debugpy"); + expect(message).toContain("debugpy"); + }); + + it("surfaces 'pip install debugpy' when attach stderr mentions missing module", async () => { + const manager = new DapSessionManager(); + const debugpyAdapter: DapResolvedAdapter = { ...TEST_ADAPTER, name: "debugpy" }; + const fake = new FakeDapClient(debugpyAdapter, process.cwd(), { + attachError: 'ModuleNotFoundError: No module named "debugpy"', + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.attach({ adapter: debugpyAdapter, cwd: process.cwd(), pid: 123 }); + } catch (error) { + message = (error as Error).message; + } + + expect(message).toContain("pip install debugpy"); + }); + + it("does NOT rewrite to 'pip install debugpy' for non-debugpy adapters even when stderr mentions the module", async () => { + const manager = new DapSessionManager(); + const fake = new FakeDapClient(TEST_ADAPTER, process.cwd(), { + launchError: "incidental log line: No module named debugpy was here but the adapter is lldb-dap", + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.launch({ adapter: TEST_ADAPTER, program: "/bin/echo", cwd: process.cwd() }); + } catch (error) { + message = (error as Error).message; + } + + expect(message).not.toContain("pip install debugpy"); + expect(message).toContain("incidental log line"); + }); + + it("prefers a delayed launch failure over the configurationDone cascade", async () => { + // Models real adapter I/O where the launch failure arrives via socket + // several ticks after configurationDone has already rejected. The old + // `await Promise.resolve()` (one microtask) would miss the late launch + // rejection and surface only the configurationDone cascade. + const manager = new DapSessionManager(); + const fake = new FakeDapClient(TEST_ADAPTER, process.cwd(), { + launchError: "launch: 'C:\\repo\\program' is not a valid executable", + launchErrorDelayMs: 10, + configurationDoneError: "configurationDone: Expected process to be stopped.", + }); + spyOn(DapClient, "spawn").mockResolvedValue(fake as unknown as DapClient); + + let message = ""; + try { + await manager.launch({ adapter: TEST_ADAPTER, program: "C:\\repo\\program", cwd: process.cwd() }); + } catch (error) { + message = (error as Error).message; + } + + // The combined error must include the launch failure as the preferred + // error — not just the configurationDone cascade. Both messages are + // present in the combined form (see combineDapStartErrors), but the + // regression-prone case is omitting the launch line entirely. + expect(message).toContain("launch: 'C:\\repo\\program' is not a valid executable"); + expect(message).toContain("configurationDone: Expected process to be stopped."); + }); }); describe("DebugTool launch validation", () => { @@ -221,4 +309,84 @@ describe("DebugTool launch validation", () => { await fs.rm(cwd, { recursive: true, force: true }); } }); + + it("throws targeted 'python not found in PATH' when adapter:'debugpy' is unresolvable for launch", async () => { + const dapModule = await import("../../src/dap"); + const launchSpy = spyOn(dapModule, "selectLaunchAdapter").mockReturnValue(null); + try { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-debugpy-")); + try { + await fs.writeFile(path.join(cwd, "main.py"), "print('hi')"); + const session: ToolSession = { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated({ "debug.enabled": true }), + }; + const tool = new DebugTool(session); + + await expect( + tool.execute("call", { action: "launch", program: "main.py", adapter: "debugpy" }), + ).rejects.toThrow(/debugpy.*python not found in PATH/); + } finally { + await fs.rm(cwd, { recursive: true, force: true }); + } + } finally { + launchSpy.mockRestore(); + } + }); + + it("throws targeted 'python not found in PATH' when adapter:'debugpy' is unresolvable for attach", async () => { + const dapModule = await import("../../src/dap"); + const attachSpy = spyOn(dapModule, "selectAttachAdapter").mockReturnValue(null); + try { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-debugpy-attach-")); + try { + const session: ToolSession = { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated({ "debug.enabled": true }), + }; + const tool = new DebugTool(session); + + await expect(tool.execute("call", { action: "attach", pid: 1234, adapter: "debugpy" })).rejects.toThrow( + /debugpy.*python not found in PATH/, + ); + } finally { + await fs.rm(cwd, { recursive: true, force: true }); + } + } finally { + attachSpy.mockRestore(); + } + }); + + it("falls back to the generic 'No debugger adapter' error when adapter is unspecified", async () => { + const dapModule = await import("../../src/dap"); + const launchSpy = spyOn(dapModule, "selectLaunchAdapter").mockReturnValue(null); + try { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-noadapter-")); + try { + await fs.writeFile(path.join(cwd, "main.py"), "print('hi')"); + const session: ToolSession = { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + settings: Settings.isolated({ "debug.enabled": true }), + }; + const tool = new DebugTool(session); + + await expect(tool.execute("call", { action: "launch", program: "main.py" })).rejects.toThrow( + /No debugger adapter available/, + ); + } finally { + await fs.rm(cwd, { recursive: true, force: true }); + } + } finally { + launchSpy.mockRestore(); + } + }); }); diff --git a/packages/coding-agent/test/edit-patch-unchanged-error.test.ts b/packages/coding-agent/test/edit-patch-unchanged-error.test.ts new file mode 100644 index 000000000..94a2ddeaf --- /dev/null +++ b/packages/coding-agent/test/edit-patch-unchanged-error.test.ts @@ -0,0 +1,112 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { DEFAULT_FUZZY_THRESHOLD, executePatchSingle } from "@oh-my-pi/pi-coding-agent/edit"; +import type { FileDiagnosticsResult } from "@oh-my-pi/pi-coding-agent/lsp"; +import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; + +function makeSession(cwd: string): ToolSession { + return { + cwd, + hasUI: false, + getSessionFile: () => null, + getSessionSpawns: () => "*", + enableLsp: false, + settings: Settings.isolated({ "edit.mode": "patch" }), + getArtifactsDir: () => null, + getSessionId: () => null, + getPlanModeState: () => undefined, + } as unknown as ToolSession; +} + +const noopBeginDeferred = (_p: string) => ({ + onDeferredDiagnostics: () => {}, + signal: new AbortController().signal, + finalize: () => {}, +}); + +/** + * Simulates an LSP host integration that claims success without persisting the + * write — the failure mode the post-write verification block in `patch.ts` is + * defending against. Unlike `writethroughNoop`, this really doesn't touch the + * filesystem. + */ +async function silentlySwallowingWritethrough(): Promise { + return undefined; +} + +let tempDir: string; + +beforeEach(async () => { + resetSettingsForTest(); + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-patch-unchanged-")); + await Settings.init({ inMemory: true, cwd: tempDir }); +}); + +afterEach(async () => { + resetSettingsForTest(); + await fs.rm(tempDir, { recursive: true, force: true }); +}); + +describe("executePatchSingle — post-write verification error path", () => { + test("error message contains the caller-supplied relative path and not the absolute resolvedPath", async () => { + const relPath = "deep/nested/foo.txt"; + await fs.mkdir(path.join(tempDir, "deep", "nested"), { recursive: true }); + await fs.writeFile(path.join(tempDir, relPath), "a\n"); + + let caught: Error | undefined; + try { + await executePatchSingle({ + session: makeSession(tempDir), + path: relPath, + params: { op: "update", diff: "@@\n-a\n+b" }, + allowFuzzy: true, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: silentlySwallowingWritethrough, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + } catch (err) { + caught = err as Error; + } + + expect(caught).toBeInstanceOf(Error); + const message = caught?.message ?? ""; + + // The relative path supplied by the caller must appear in the + // user-facing error — it's what the outer composer in `executeSinglePathEntries` + // uses in its `Error editing ${path}: …` wrapper. + expect(message).toContain(relPath); + + // The absolute resolved path must NOT appear in the user-facing + // message — leaking it embeds `$HOME`/`os.tmpdir()` in the TUI and + // double-embeds the path when the outer composer prepends its own. + // resolvedPath still lives in the structured `context` metadata. + expect(message).not.toContain(tempDir); + }); + + test("ToolError still carries the absolute resolvedPath in its structured context for log correlation", async () => { + const relPath = "foo.txt"; + await fs.writeFile(path.join(tempDir, relPath), "a\n"); + + let caught: Error | undefined; + try { + await executePatchSingle({ + session: makeSession(tempDir), + path: relPath, + params: { op: "update", diff: "@@\n-a\n+b" }, + allowFuzzy: true, + fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD, + writethrough: silentlySwallowingWritethrough, + beginDeferredDiagnosticsForPath: noopBeginDeferred, + }); + } catch (err) { + caught = err as Error; + } + + expect(caught).toBeInstanceOf(Error); + const context = (caught as Error & { context?: { path?: string } }).context; + expect(context?.path).toBe(path.join(tempDir, relPath)); + }); +}); diff --git a/packages/coding-agent/test/tools/find-validate-paths.test.ts b/packages/coding-agent/test/tools/find-validate-paths.test.ts new file mode 100644 index 000000000..db6d6bd63 --- /dev/null +++ b/packages/coding-agent/test/tools/find-validate-paths.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "bun:test"; +import { validateFindPathInputs } from "../../src/tools/find"; + +describe("validateFindPathInputs", () => { + it("accepts a normal array of glob entries", () => { + expect(() => validateFindPathInputs(["src/**/*.ts", "test/**/*.ts"])).not.toThrow(); + }); + + it('rejects comma-joined entries (the `["a,b"]` shape)', () => { + expect(() => validateFindPathInputs(["a.py,b.py"])).toThrow(/paths is an array/); + }); + + it("allows commas inside brace expansion", () => { + expect(() => validateFindPathInputs(["src/{a,b}/*.ts"])).not.toThrow(); + expect(() => validateFindPathInputs(["{foo,bar,baz}.md"])).not.toThrow(); + }); + + it("allows backslash-escaped commas at top level (matches search.ts:containsTopLevelComma)", () => { + // Backslash-escapes a literal comma in a filename — must not trip the + // array-vs-string heuristic. + expect(() => validateFindPathInputs(["weird\\,name.txt"])).not.toThrow(); + expect(() => validateFindPathInputs(["a\\,b\\,c"])).not.toThrow(); + }); + + it("still rejects unescaped top-level commas mixed with escaped ones", () => { + // `a\,b,c` — the second comma is unescaped, so the heuristic should fire. + expect(() => validateFindPathInputs(["a\\,b,c"])).toThrow(/paths is an array/); + }); + + it("allows a trailing backslash without crashing", () => { + // `foo\\` is a backslash at end-of-string; the i+1 validateFindPathInputs(["foo\\"])).not.toThrow(); + }); + + it("treats `\\{a,b}` as an escaped brace, so the inner comma is still top-level", () => { + // Skip-next semantics: the backslash consumes the `{`, so braceDepth stays 0 + // and the unescaped `,` between `a` and `b` rejects. This pins the literal + // behavior of the new escape-skip, which intentionally does NOT model glob + // brace semantics — it only mirrors search.ts's containsTopLevelComma. + expect(() => validateFindPathInputs(["\\{a,b}"])).toThrow(/paths is an array/); + }); +}); diff --git a/packages/coding-agent/test/tools/split-internal-url-sel.test.ts b/packages/coding-agent/test/tools/split-internal-url-sel.test.ts index 49369e3c1..1bc1bcc42 100644 --- a/packages/coding-agent/test/tools/split-internal-url-sel.test.ts +++ b/packages/coding-agent/test/tools/split-internal-url-sel.test.ts @@ -95,16 +95,19 @@ describe("splitInternalUrlSel", () => { }); }); - it("does not peel when the only slash before the colon is part of the `://` separator", () => { - // Guards against degenerate inputs like `mcp://:1-50` — stripping the - // scheme's own slash would emit `mcp:/` as the path. The peeler refuses - // when the resulting path no longer carries a scheme separator. + 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("rejects bare-integer suffixes even with the trailing-slash escape", () => { - // Could still be a port number after a path; require a richer selector form. + 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", });