From 8145141c55f1a16f36be4787fe3bb46340d63553 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 26 Apr 2026 17:19:23 -0700 Subject: [PATCH] Fix external issue URL rewriting in markdown (#4558) ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - Issue and comment rendering is part of the board UI where humans supervise and inspect agent work. > - External Paperclip issue URLs can appear in comments as references to other runs, review threads, or remote test environments. > - Those links must preserve their full destination, including origin, port, and `#comment-...` fragments, or the operator is taken to the wrong place. > - The bug here was that absolute `http(s)` issue URLs were being normalized into internal `/issues/...` routes in the markdown path. > - This pull request stops rewriting absolute URLs while keeping internal issue-reference behavior for relative paths and identifiers. > - The benefit is that authored external links now navigate exactly where the operator expects, especially for remote test and comment-deep-link workflows. ## What Changed - Stopped `ui/src/lib/issue-reference.ts` from treating absolute `http(s)` URLs as internal issue paths. - Added defense-in-depth in `ui/src/lib/mention-chips.ts` so absolute `http(s)` URLs are never reclassified as issue mention chips. - Updated `ui/src/lib/issue-reference.test.ts` to cover absolute Paperclip URLs with preserved origin, port, and comment hash. - Updated `ui/src/components/MarkdownBody.test.tsx` to assert the reported URL renders as an external link, not an internal `/issues/...` href. ## Verification - `pnpm exec vitest run ui/src/lib/issue-reference.test.ts ui/src/components/MarkdownBody.test.tsx` - Expected result: `2` files passed, `37` tests passed. - Manual spot-check from the issue report path: a URL like `http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-...` should remain an external link with its full destination preserved. ## Risks - Low risk. The change narrows when Paperclip rewrites URLs, so the main risk is if some existing workflow depended on absolute `http(s)` Paperclip URLs being converted into internal issue links. The added regression coverage is aimed at preventing that from regressing silently. ## Model Used - OpenAI Codex local agent via Paperclip `codex_local` - Backing model family: GPT-5-based Codex runtime - Exact backend model ID/version: not exposed by this adapter/runtime surface - Context window: not exposed by this adapter/runtime surface - Capabilities used: tool use, shell command execution, code editing, git operations, and local test execution ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] I will address all Greptile and reviewer comments before requesting merge --- ui/src/components/MarkdownBody.test.tsx | 17 +++++++++-------- ui/src/lib/issue-reference.test.ts | 16 +++++++++------- ui/src/lib/issue-reference.ts | 13 ++----------- ui/src/lib/mention-chips.ts | 4 ++++ 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/ui/src/components/MarkdownBody.test.tsx b/ui/src/components/MarkdownBody.test.tsx index 8f64bf8f..0cae828d 100644 --- a/ui/src/components/MarkdownBody.test.tsx +++ b/ui/src/components/MarkdownBody.test.tsx @@ -187,16 +187,17 @@ describe("MarkdownBody", () => { expect(html).not.toContain('aria-label="Issue PAP-1271: PAP-1271"'); }); - it("rewrites full issue URLs to internal issue links", () => { - const html = renderMarkdown("See http://localhost:3100/PAP/issues/PAP-1179.", [ - { identifier: "PAP-1179", status: "blocked" }, + it("preserves absolute issue URLs as external links", () => { + const url = "http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d"; + const html = renderMarkdown(`See ${url}.`, [ + { identifier: "PAPA-115", status: "blocked" }, ]); - expect(html).toContain('href="/issues/PAP-1179"'); - expect(html).toContain("text-red-600"); - expect(html).toContain(">http://localhost:3100/PAP/issues/PAP-1179<"); - expect(html).toContain('data-mention-kind="issue"'); - expect(html).not.toContain("paperclip-mention-chip--issue"); + expect(html).toContain(`href="${url}"`); + expect(html).toContain('target="_blank"'); + expect(html).toContain("lucide-external-link"); + expect(html).not.toContain('href="/issues/PAPA-115"'); + expect(html).not.toContain("paperclip-markdown-issue-ref"); }); it("linkifies plain internal issue paths in markdown text", () => { diff --git a/ui/src/lib/issue-reference.test.ts b/ui/src/lib/issue-reference.test.ts index cd0f038a..54224bfb 100644 --- a/ui/src/lib/issue-reference.test.ts +++ b/ui/src/lib/issue-reference.test.ts @@ -9,8 +9,9 @@ describe("issue-reference", () => { expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull(); }); - it("extracts issue ids from full issue URLs", () => { - expect(parseIssuePathIdFromPath("http://localhost:3100/PAP/issues/PAP-1179")).toBe("PAP-1179"); + it("does not treat full issue URLs as internal issue paths", () => { + expect(parseIssuePathIdFromPath("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull(); + expect(parseIssuePathIdFromPath("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull(); }); it("does not treat GitHub issue URLs as internal Paperclip issue links", () => { @@ -24,15 +25,11 @@ describe("issue-reference", () => { expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull(); }); - it("normalizes bare identifiers, issue URLs, and issue scheme links into internal links", () => { + it("normalizes bare identifiers, relative issue paths, and issue scheme links into internal links", () => { expect(parseIssueReferenceFromHref("pap-1271")).toEqual({ issuePathId: "PAP-1271", href: "/issues/PAP-1271", }); - expect(parseIssueReferenceFromHref("http://localhost:3100/PAP/issues/PAP-1179")).toEqual({ - issuePathId: "PAP-1179", - href: "/issues/PAP-1179", - }); expect(parseIssueReferenceFromHref("/PAP/issues/pap-1180")).toEqual({ issuePathId: "PAP-1180", href: "/issues/PAP-1180", @@ -54,6 +51,11 @@ describe("issue-reference", () => { }); }); + it("preserves absolute Paperclip issue URLs so origin, port, and hash are not lost", () => { + expect(parseIssueReferenceFromHref("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull(); + expect(parseIssueReferenceFromHref("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull(); + }); + it("ignores literal route placeholder paths", () => { expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull(); expect(parseIssueReferenceFromHref("http://localhost:3100/api/issues/:id")).toBeNull(); diff --git a/ui/src/lib/issue-reference.ts b/ui/src/lib/issue-reference.ts index cbef86ff..616bb876 100644 --- a/ui/src/lib/issue-reference.ts +++ b/ui/src/lib/issue-reference.ts @@ -11,18 +11,9 @@ const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\ export function parseIssuePathIdFromPath(pathOrUrl: string | null | undefined): string | null { if (!pathOrUrl) return null; - let pathname = pathOrUrl.trim(); + const pathname = pathOrUrl.trim(); if (!pathname) return null; - - if (/^https?:\/\//i.test(pathname)) { - try { - const url = new URL(pathname); - if (url.hostname === "github.com" || url.hostname === "www.github.com") return null; - pathname = url.pathname; - } catch { - return null; - } - } + if (/^https?:\/\//i.test(pathname)) return null; const segments = pathname.split("/").filter(Boolean); const issueIndex = segments.findIndex((segment) => segment === "issues"); diff --git a/ui/src/lib/mention-chips.ts b/ui/src/lib/mention-chips.ts index d2dbe1c7..b12311b4 100644 --- a/ui/src/lib/mention-chips.ts +++ b/ui/src/lib/mention-chips.ts @@ -37,6 +37,10 @@ export type ParsedMentionChip = const iconMaskCache = new Map(); export function parseMentionChipHref(href: string): ParsedMentionChip | null { + if (/^https?:\/\//i.test(href.trim())) { + return null; + } + const issue = parseIssueReferenceHref(href); if (issue) { return {