From a0dcf76177cd48a72a5167072fddecafdd9d1b59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Catriel=20M=C3=BCller?= Date: Mon, 11 May 2026 13:18:49 -0300 Subject: [PATCH] fix(cli): strip trailing sentence punctuation from URL matches in normalizeUrls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit \S+ was greedily consuming trailing punctuation (. ! , ;) into the URL match, causing new URL() to mutate clean ASCII URLs — e.g. "see https://example.com." became "see https://example.com./". Fix: strip trailing sentence punctuation before parsing, extract and replace only the hostname (not href) to avoid adding trailing slashes to bare origins, and restore the stripped tail afterward. --- packages/opencode/src/kilocode/util/url.ts | 22 ++++++- .../opencode/test/kilocode/util/url.test.ts | 57 +++++++++++++++---- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/kilocode/util/url.ts b/packages/opencode/src/kilocode/util/url.ts index c9e152c374e..f6e3c72e3b8 100644 --- a/packages/opencode/src/kilocode/util/url.ts +++ b/packages/opencode/src/kilocode/util/url.ts @@ -6,11 +6,31 @@ * * Example: "curl https://аpitest.com/status" (Cyrillic а) * → "curl https://xn--pitest-2nf.com/status" + * + * Trailing sentence punctuation (. , ! ? ; :) that \S+ would otherwise consume + * into the URL match is stripped before parsing and left in place afterward, so + * plain-text prose like "see https://example.com." is returned unchanged. + * + * Only the hostname is replaced, not the full href, to avoid side-effects such + * as new URL() appending a trailing slash to bare origins. */ export function normalizeUrls(text: string) { return text.replace(/https?:\/\/\S+/g, (match) => { + // Strip trailing sentence punctuation that \S+ greedily consumes but that + // is almost certainly not part of the URL (e.g. "visit https://x.com."). + const stripped = match.replace(/[.,!?;:)"'\]>]+$/, "") + const tail = match.slice(stripped.length) try { - return new URL(match).href + const parsed = new URL(stripped) + // Extract the raw hostname from the stripped string so we can replace + // only that part — using href would add a trailing slash to bare origins. + const afterScheme = stripped.indexOf("//") + 2 + const slashPos = stripped.indexOf("/", afterScheme) + const rawHost = slashPos === -1 ? stripped.slice(afterScheme) : stripped.slice(afterScheme, slashPos) + const colon = rawHost.indexOf(":") + const rawHostname = colon === -1 ? rawHost : rawHost.slice(0, colon) + if (rawHostname === parsed.hostname) return match // plain ASCII — nothing to change + return stripped.replace(rawHostname, parsed.hostname) + tail } catch { return match } diff --git a/packages/opencode/test/kilocode/util/url.test.ts b/packages/opencode/test/kilocode/util/url.test.ts index 40a28dc6c27..876ce571bcc 100644 --- a/packages/opencode/test/kilocode/util/url.test.ts +++ b/packages/opencode/test/kilocode/util/url.test.ts @@ -14,7 +14,7 @@ describe("normalizeUrls", () => { test("converts mixed-script hostname to punycode", () => { // Mix of Latin and Cyrillic in the same label - const input = "https://\u0430pitest.com" + const input = "https://\u0430pitest.com/path" expect(normalizeUrls(input)).not.toContain("\u0430") }) @@ -33,20 +33,57 @@ describe("normalizeUrls", () => { }) describe("plain ASCII URLs are unchanged", () => { - test("leaves a clean https URL untouched", () => { + test("leaves a URL with a path untouched", () => { const url = "https://apitest.com/status" expect(normalizeUrls(url)).toBe(url) }) - test("leaves a clean http URL untouched", () => { + test("leaves a URL with path and query string untouched", () => { const url = "http://example.com/foo?bar=1&baz=2" expect(normalizeUrls(url)).toBe(url) }) - test("leaves localhost URL untouched", () => { + test("leaves a localhost URL with port untouched", () => { const url = "http://localhost:3000/api" expect(normalizeUrls(url)).toBe(url) }) + + test("leaves a bare origin untouched (no trailing slash added)", () => { + // Regression: new URL("https://example.com").href === "https://example.com/" + // The old implementation using href would mutate bare origins by adding "/". + const url = "https://example.com" + expect(normalizeUrls(url)).toBe(url) + }) + }) + + describe("trailing sentence punctuation is not consumed into the URL", () => { + test("period at end of sentence is not consumed (was: adds trailing slash)", () => { + // "see https://example.com." — the period ends the sentence, not the URL. + // Old behaviour (bug): returned "see https://example.com./" + expect(normalizeUrls("see https://example.com.")).toBe("see https://example.com.") + }) + + test("exclamation mark at end of sentence is not consumed", () => { + expect(normalizeUrls("visit https://example.com!")).toBe("visit https://example.com!") + }) + + test("comma after URL in a list is not consumed", () => { + expect(normalizeUrls("check https://example.com, then continue")).toBe( + "check https://example.com, then continue", + ) + }) + + test("closing parenthesis after URL is not consumed", () => { + expect(normalizeUrls("(see https://example.com)")).toBe("(see https://example.com)") + }) + + test("trailing punctuation after an IDN URL is stripped correctly and punycode applied", () => { + // Trailing period on a homograph URL: period is sentence punctuation, not part of the URL. + const input = "see https://\u0430pitest.com." + const result = normalizeUrls(input) + expect(result).toBe("see https://xn--pitest-2nf.com.") + expect(result).not.toContain("\u0430") + }) }) describe("URL embedded in a bash command string", () => { @@ -57,7 +94,7 @@ describe("normalizeUrls", () => { expect(result).not.toContain("\u0430") }) - test("preserves the non-URL parts of the command", () => { + test("preserves flags and pipe around the URL", () => { const input = "curl -sSf https://\u0430pitest.com/status | bash" const result = normalizeUrls(input) expect(result).toMatch(/^curl -sSf /) @@ -71,7 +108,7 @@ describe("normalizeUrls", () => { expect(result).not.toContain("\u0430") }) - test("leaves plain-ASCII command entirely unchanged", () => { + test("leaves a plain-ASCII command entirely unchanged", () => { const input = "curl -sSf https://kilo.ai/update.sh | bash" expect(normalizeUrls(input)).toBe(input) }) @@ -92,18 +129,16 @@ describe("normalizeUrls", () => { expect(normalizeUrls(text)).toBe(text) }) - test("handles a URL with a path, query, and fragment", () => { + test("preserves path, query string, and fragment after IDN conversion", () => { const input = "https://\u0430pitest.com/path?q=1#anchor" const result = normalizeUrls(input) expect(result).toMatch(/xn--/) expect(result).toContain("/path?q=1#anchor") }) - test("preserves a URL that fails to parse (e.g. malformed) verbatim", () => { - // A URL that new URL() cannot parse should pass through untouched. + test("preserves a URL that fails to parse verbatim", () => { const malformed = "https://[unclosed" - const result = normalizeUrls(malformed) - expect(result).toBe(malformed) + expect(normalizeUrls(malformed)).toBe(malformed) }) }) })