From 9dc48ec650025de62c51925ac16e2740e9370864 Mon Sep 17 00:00:00 2001 From: genius-0963 Date: Wed, 4 Nov 2026 18:17:42 +0530 Subject: [PATCH 1/3] Fix #3658: display PR number and ensure link renders in PR created UI When pull requests or issues are created or updated, GitHub MCP server returns a MinimalResponse containing the entity ID and URL (e.g. https://github.com/owner/repo/pull/123). The number field is not part of MinimalResponse and is expected to be derived from the URL or fetched separately. Because pr.number was undefined on the result payload: 1. The PR created UI popup never displayed the PR number (#{pr.number}). 2. When the UI was rendered from a completed tool result (such as up-front execution without prior form submission), pr.title and submittedTitle were both empty, causing the anchor element to have empty text and zero width, leaving the user with an invisible, unclickable link. This fix: - Adds parseNumberFromURL in toolResult.ts to extract the PR/issue number from GitHub URLs. - Updates SuccessView in pr-write, pr-edit, and issue-write to derive the number from html_url/url/URL when not present on the result object. - Provides a clean fallback title (Pull request #${prNumber} / Issue #${issueNumber} or toolInput.title) so the link anchor always has visible text and is clickable. - Adds comprehensive unit tests for completedToolResult and parseNumberFromURL. Fixes #3658 --- ui/src/apps/issue-write/App.tsx | 16 +++++++---- ui/src/apps/pr-edit/App.tsx | 16 +++++++---- ui/src/apps/pr-write/App.tsx | 20 +++++++++----- ui/src/lib/toolResult.test.ts | 49 +++++++++++++++++++++++++++++++++ ui/src/lib/toolResult.ts | 11 ++++++++ 5 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 ui/src/lib/toolResult.test.ts diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index 5cfb5d83a4..d3b2f7146b 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -24,7 +24,7 @@ import { } from "@primer/octicons-react"; import { AppProvider } from "../../components/AppProvider"; import { useMcpApp } from "../../hooks/useMcpApp"; -import { completedToolResult } from "../../lib/toolResult"; +import { completedToolResult, parseNumberFromURL } from "../../lib/toolResult"; import { parseUIGetResult } from "../../lib/uiGetResult"; import { MarkdownEditor } from "../../components/MarkdownEditor"; @@ -294,7 +294,11 @@ function SuccessView({ isUpdate: boolean; openLink: (url: string) => Promise; }) { - const issueUrl = issue.html_url || issue.url || issue.URL || "#"; + const rawUrl = issue.html_url || issue.url || issue.URL; + const issueNum = issue.number ?? parseNumberFromURL(rawUrl); + const issueUrl = rawUrl || (owner && repo && issueNum ? `https://github.com/${owner}/${repo}/issues/${issueNum}` : "#"); + const titleText = issue.title || submittedTitle; + const displayTitle = titleText || (issueNum ? `Issue #${issueNum}` : "View issue"); return (
- {issue.title || submittedTitle} - {issue.number && ( + {displayTitle} + {issueNum && titleText && !titleText.includes(`#${issueNum}`) && ( - #{issue.number} + #{issueNum} )} @@ -1281,7 +1285,7 @@ function CreateIssueApp() { issue={shownIssue} owner={owner} repo={repo} - submittedTitle={title} + submittedTitle={title || (toolInput?.title as string) || ""} submittedLabels={selectedLabels} isUpdate={isUpdateMode} openLink={openLink} diff --git a/ui/src/apps/pr-edit/App.tsx b/ui/src/apps/pr-edit/App.tsx index 59e4e5ca09..38142241a3 100644 --- a/ui/src/apps/pr-edit/App.tsx +++ b/ui/src/apps/pr-edit/App.tsx @@ -25,7 +25,7 @@ import { } from "@primer/octicons-react"; import { AppProvider } from "../../components/AppProvider"; import { useMcpApp } from "../../hooks/useMcpApp"; -import { completedToolResult } from "../../lib/toolResult"; +import { completedToolResult, parseNumberFromURL } from "../../lib/toolResult"; import { parseUIGetResult } from "../../lib/uiGetResult"; import { MarkdownEditor } from "../../components/MarkdownEditor"; @@ -167,7 +167,11 @@ function SuccessView({ submittedTitle: string; openLink: (url: string) => Promise; }) { - const prUrl = pr.html_url || pr.url || pr.URL || "#"; + const rawUrl = pr.html_url || pr.url || pr.URL; + const prNumber = pr.number ?? parseNumberFromURL(rawUrl); + const prUrl = rawUrl || (owner && repo && prNumber ? `https://github.com/${owner}/${repo}/pull/${prNumber}` : "#"); + const titleText = pr.title || submittedTitle; + const displayTitle = titleText || (prNumber ? `Pull request #${prNumber}` : "View pull request"); return (
- {pr.title || submittedTitle} - {pr.number && ( + {displayTitle} + {prNumber && titleText && !titleText.includes(`#${prNumber}`) && ( - #{pr.number} + #{prNumber} )} @@ -534,7 +538,7 @@ function EditPRApp() { if (shownPR) { return ( - + ); } diff --git a/ui/src/apps/pr-write/App.tsx b/ui/src/apps/pr-write/App.tsx index c800ae0cdc..1e27a4cecf 100644 --- a/ui/src/apps/pr-write/App.tsx +++ b/ui/src/apps/pr-write/App.tsx @@ -27,7 +27,7 @@ import { } from "@primer/octicons-react"; import { AppProvider } from "../../components/AppProvider"; import { useMcpApp } from "../../hooks/useMcpApp"; -import { completedToolResult } from "../../lib/toolResult"; +import { completedToolResult, parseNumberFromURL } from "../../lib/toolResult"; import { parseUIGetResult } from "../../lib/uiGetResult"; import { MarkdownEditor } from "../../components/MarkdownEditor"; @@ -80,7 +80,11 @@ function SuccessView({ submittedTitle: string; openLink: (url: string) => Promise; }) { - const prUrl = pr.html_url || pr.url || pr.URL || "#"; + const rawUrl = pr.html_url || pr.url || pr.URL; + const prNumber = pr.number ?? parseNumberFromURL(rawUrl); + const prUrl = rawUrl || (owner && repo && prNumber ? `https://github.com/${owner}/${repo}/pull/${prNumber}` : "#"); + const titleText = pr.title || submittedTitle; + const displayTitle = titleText || (prNumber ? `Pull request #${prNumber}` : "View pull request"); return (
- {pr.title || submittedTitle} - {pr.number && ( + {displayTitle} + {prNumber && titleText && !titleText.includes(`#${prNumber}`) && ( - #{pr.number} + #{prNumber} )} @@ -457,6 +461,8 @@ function CreatePRApp() { if (textContent && textContent.type === "text" && textContent.text) { const prData = JSON.parse(textContent.text); setSuccessPR(prData); + const createdPRNumber = prData.number ?? parseNumberFromURL(prData.html_url || prData.url || prData.URL); + const prRef = createdPRNumber ? `Pull request #${createdPRNumber}` : "A new pull request"; // Push the new PR into the model context so subsequent agent // turns can reference it (MCP Apps 2026-01-26 ui/update-model-context). void setModelContext({ @@ -464,7 +470,7 @@ function CreatePRApp() { content: [ { type: "text", - text: `A new pull request was created in ${owner}/${repo} by the user via the create-pull-request view.`, + text: `${prRef} was created in ${owner}/${repo} by the user via the create-pull-request view.`, }, ], }); @@ -480,7 +486,7 @@ function CreatePRApp() { if (shownPR) { return ( - + ); } diff --git a/ui/src/lib/toolResult.test.ts b/ui/src/lib/toolResult.test.ts new file mode 100644 index 0000000000..cc4e5b3efa --- /dev/null +++ b/ui/src/lib/toolResult.test.ts @@ -0,0 +1,49 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { completedToolResult, parseNumberFromURL } from "./toolResult.ts"; + +test("completedToolResult: handles null, error, deferral, and completed results", () => { + assert.equal(completedToolResult(null), null); + assert.equal(completedToolResult({ isError: true, content: [{ type: "text", text: "{}" }] }), null); + assert.equal( + completedToolResult({ + isError: false, + structuredContent: { status: "awaiting_user_submission" }, + content: [{ type: "text", text: "{}" }], + }), + null, + ); + assert.equal(completedToolResult({ isError: false, content: [] }), null); + assert.equal(completedToolResult({ isError: false, content: [{ type: "text", text: "invalid-json" }] }), null); + assert.deepEqual( + completedToolResult({ + isError: false, + content: [{ type: "text", text: '{"id":"123","url":"https://github.com/owner/repo/pull/42"}' }], + }), + { id: "123", url: "https://github.com/owner/repo/pull/42" }, + ); +}); + +test("parseNumberFromURL: extracts PR numbers from various GitHub URLs", () => { + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/123"), 123); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/123/"), 123); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/123/files"), 123); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/123#issuecomment-456"), 123); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/123?diff=unified"), 123); +}); + +test("parseNumberFromURL: extracts issue numbers from various GitHub URLs", () => { + assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/789"), 789); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/789/"), 789); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/789#issuecomment-999"), 789); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/789?foo=bar"), 789); +}); + +test("parseNumberFromURL: returns undefined for non-PR/issue URLs or invalid inputs", () => { + assert.equal(parseNumberFromURL(undefined), undefined); + assert.equal(parseNumberFromURL(""), undefined); + assert.equal(parseNumberFromURL("#"), undefined); + assert.equal(parseNumberFromURL("https://github.com/owner/repo"), undefined); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/notanumber"), undefined); + assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/notanumber"), undefined); +}); diff --git a/ui/src/lib/toolResult.ts b/ui/src/lib/toolResult.ts index cb39015754..0eb5530db6 100644 --- a/ui/src/lib/toolResult.ts +++ b/ui/src/lib/toolResult.ts @@ -35,3 +35,14 @@ export function completedToolResult>( return null; } } + +/** + * Extracts a numeric PR or issue number from a GitHub web URL, e.g. + * "https://github.com/owner/repo/pull/123" -> 123 + * "https://github.com/owner/repo/issues/456" -> 456 + */ +export function parseNumberFromURL(url?: string): number | undefined { + if (!url) return undefined; + const match = url.match(/\/(?:pull|issues)\/(\d+)(?:[/?#]|$)/); + return match ? parseInt(match[1], 10) : undefined; +} From 7c936b85198d899c64dc43756a0cc4accb62a071 Mon Sep 17 00:00:00 2001 From: Saurabh Sungh Date: Wed, 4 Nov 2026 18:43:39 +0530 Subject: [PATCH 2/3] Update regex to match PR number in title text Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ui/src/apps/pr-edit/App.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/apps/pr-edit/App.tsx b/ui/src/apps/pr-edit/App.tsx index 38142241a3..e07def293a 100644 --- a/ui/src/apps/pr-edit/App.tsx +++ b/ui/src/apps/pr-edit/App.tsx @@ -254,7 +254,7 @@ function SuccessView({ }} > {displayTitle} - {prNumber && titleText && !titleText.includes(`#${prNumber}`) && ( + {prNumber && titleText && !new RegExp(`#${prNumber}(?!\\d)`).test(titleText) && ( Date: Thu, 5 Nov 2026 10:45:27 +0530 Subject: [PATCH 3/3] fix(ui): use boundary-aware check for issue and PR numbers in titles Avoid substring false positives where a title containing a longer number (e.g. #1234) incorrectly suppresses the suffix for a shorter reference (#123). Use hasExactIssueReference across pr-write, pr-edit, and issue-write views. --- ui/src/apps/issue-write/App.tsx | 4 ++-- ui/src/apps/pr-edit/App.tsx | 4 ++-- ui/src/apps/pr-write/App.tsx | 4 ++-- ui/src/lib/toolResult.test.ts | 15 ++++++++++++++- ui/src/lib/toolResult.ts | 13 +++++++++++++ 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/ui/src/apps/issue-write/App.tsx b/ui/src/apps/issue-write/App.tsx index d3b2f7146b..d986fa2489 100644 --- a/ui/src/apps/issue-write/App.tsx +++ b/ui/src/apps/issue-write/App.tsx @@ -24,7 +24,7 @@ import { } from "@primer/octicons-react"; import { AppProvider } from "../../components/AppProvider"; import { useMcpApp } from "../../hooks/useMcpApp"; -import { completedToolResult, parseNumberFromURL } from "../../lib/toolResult"; +import { completedToolResult, hasExactIssueReference, parseNumberFromURL } from "../../lib/toolResult"; import { parseUIGetResult } from "../../lib/uiGetResult"; import { MarkdownEditor } from "../../components/MarkdownEditor"; @@ -384,7 +384,7 @@ function SuccessView({ }} > {displayTitle} - {issueNum && titleText && !titleText.includes(`#${issueNum}`) && ( + {issueNum && titleText && !hasExactIssueReference(titleText, issueNum) && ( {displayTitle} - {prNumber && titleText && !new RegExp(`#${prNumber}(?!\\d)`).test(titleText) && ( + {prNumber && titleText && !hasExactIssueReference(titleText, prNumber) && ( {displayTitle} - {prNumber && titleText && !titleText.includes(`#${prNumber}`) && ( + {prNumber && titleText && !hasExactIssueReference(titleText, prNumber) && ( { assert.equal(completedToolResult(null), null); @@ -47,3 +47,16 @@ test("parseNumberFromURL: returns undefined for non-PR/issue URLs or invalid inp assert.equal(parseNumberFromURL("https://github.com/owner/repo/pull/notanumber"), undefined); assert.equal(parseNumberFromURL("https://github.com/owner/repo/issues/notanumber"), undefined); }); + +test("hasExactIssueReference: avoids false matches on longer numbers", () => { + assert.equal(hasExactIssueReference("Fix #1234", 123), false); + assert.equal(hasExactIssueReference("Fix #123", 123), true); + assert.equal(hasExactIssueReference("#123", 123), true); + assert.equal(hasExactIssueReference("Fix (#123)", 123), true); + assert.equal(hasExactIssueReference("Issue #456 is fixed", 456), true); + assert.equal(hasExactIssueReference("Issue #4567 is fixed", 456), false); + assert.equal(hasExactIssueReference("Prefix #456", 456), true); + assert.equal(hasExactIssueReference("Prefix #4567", 456), false); + assert.equal(hasExactIssueReference(undefined, 123), false); + assert.equal(hasExactIssueReference("Fix issue", undefined), false); +}); diff --git a/ui/src/lib/toolResult.ts b/ui/src/lib/toolResult.ts index 0eb5530db6..a9bad8356e 100644 --- a/ui/src/lib/toolResult.ts +++ b/ui/src/lib/toolResult.ts @@ -46,3 +46,16 @@ export function parseNumberFromURL(url?: string): number | undefined { const match = url.match(/\/(?:pull|issues)\/(\d+)(?:[/?#]|$)/); return match ? parseInt(match[1], 10) : undefined; } + +/** + * Checks whether a title contains an exact whole-number reference to a GitHub + * issue or pull request (e.g. "#123", "(#123)", "Fix #123"), avoiding false + * positive matches against longer numbers like "#1234". + */ +export function hasExactIssueReference( + title: string | undefined, + number: number | undefined, +): boolean { + if (!title || number === undefined) return false; + return new RegExp(`(?:^|[^\\d])#${number}(?!\\d)`).test(title); +}