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
This commit is contained in:
Devin Foley
2026-04-26 17:19:23 -07:00
committed by GitHub
parent 54ab0d24cd
commit 8145141c55
4 changed files with 24 additions and 26 deletions
+9 -8
View File
@@ -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", () => {
+9 -7
View File
@@ -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();
+2 -11
View File
@@ -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");
+4
View File
@@ -37,6 +37,10 @@ export type ParsedMentionChip =
const iconMaskCache = new Map<string, string>();
export function parseMentionChipHref(href: string): ParsedMentionChip | null {
if (/^https?:\/\//i.test(href.trim())) {
return null;
}
const issue = parseIssueReferenceHref(href);
if (issue) {
return {