diff --git a/PolyPilot.Tests/GitHubReferenceLinkerTests.cs b/PolyPilot.Tests/GitHubReferenceLinkerTests.cs new file mode 100644 index 0000000000..43e0c88ccd --- /dev/null +++ b/PolyPilot.Tests/GitHubReferenceLinkerTests.cs @@ -0,0 +1,283 @@ +using PolyPilot.Services; + +namespace PolyPilot.Tests; + +public class GitHubReferenceLinkerTests +{ + // --- Fully-qualified references (owner/repo#123) --- + + [Fact] + public void QualifiedRef_LinkedWithoutRepoContext() + { + var html = "

See PureWeen/PolyPilot#42 for details

"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.Contains("href=\"https://github.com/PureWeen/PolyPilot/issues/42\"", result); + Assert.Contains("target=\"_blank\"", result); + Assert.Contains(">PureWeen/PolyPilot#42", result); + } + + [Fact] + public void MultipleQualifiedRefs_AllLinked() + { + var html = "

See owner/repo#1 and other/project#999

"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.Contains("href=\"https://github.com/owner/repo/issues/1\"", result); + Assert.Contains("href=\"https://github.com/other/project/issues/999\"", result); + } + + // --- Bare references (#123) --- + + [Fact] + public void BareRef_NotLinkedWithoutRepoUrl() + { + var html = "

Fix for #123

"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.DoesNotContain("href=", result); + Assert.Contains("#123", result); + } + + [Fact] + public void BareRef_LinkedWithRepoUrl() + { + var html = "

Fix for #123

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/PureWeen/PolyPilot"); + Assert.Contains("href=\"https://github.com/PureWeen/PolyPilot/issues/123\"", result); + Assert.Contains(">#123", result); + } + + [Fact] + public void BareRef_WorksWithDotGitUrl() + { + var html = "

#456

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo.git"); + Assert.Contains("href=\"https://github.com/owner/repo/issues/456\"", result); + } + + [Fact] + public void BareRef_WorksWithSshUrl() + { + var html = "

#789

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "git@github.com:owner/repo.git"); + Assert.Contains("href=\"https://github.com/owner/repo/issues/789\"", result); + } + + // --- Skip tags (content inside , ,
 should NOT be linked) ---
+
+    [Fact]
+    public void SkipsExistingLinks()
+    {
+        var html = "

#123

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + // The #123 inside the existing should not be double-linked + Assert.DoesNotContain("href=\"https://github.com/owner/repo/issues/123\"", result); + } + + [Fact] + public void SkipsCodeBlocks() + { + var html = "#123"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + Assert.DoesNotContain("href=", result); + } + + [Fact] + public void SkipsPreBlocks() + { + var html = "
owner/repo#42
"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.DoesNotContain("href=", result); + } + + [Fact] + public void SkipsNestedCodeInParagraph() + { + var html = "

Use #123 in your code, but see #456 for context

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + // #123 in code should NOT be linked + Assert.Contains("#123", result); + // #456 in text should be linked + Assert.Contains("href=\"https://github.com/owner/repo/issues/456\"", result); + } + + // --- HTML entity safety --- + + [Fact] + public void SkipsHtmlEntities() + { + // { is the HTML entity for { + var html = "

Use { for braces

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + // Should NOT linkify the 123 from { + Assert.DoesNotContain("href=", result); + } + + [Fact] + public void SkipsUrlFragments() + { + var html = "

Navigate to /path#123

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + // /path#123 looks like a URL fragment, not a GitHub ref + Assert.DoesNotContain("href=", result); + } + + // --- Mixed content --- + + [Fact] + public void MixedQualifiedAndBareRefs() + { + var html = "

See PureWeen/PolyPilot#42 and also #100

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/other/repo"); + // Qualified ref links to the specified repo + Assert.Contains("href=\"https://github.com/PureWeen/PolyPilot/issues/42\"", result); + // Bare ref links to the context repo + Assert.Contains("href=\"https://github.com/other/repo/issues/100\"", result); + } + + [Fact] + public void TextBeforeAndAfterRefsPreserved() + { + var html = "

This is about #42 and more stuff

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + Assert.Contains("This is about ", result); + Assert.Contains(" and more stuff", result); + } + + // --- Edge cases --- + + [Fact] + public void EmptyStringReturnsEmpty() + { + Assert.Equal("", GitHubReferenceLinker.LinkifyReferences("")); + } + + [Fact] + public void NullStringReturnsNull() + { + Assert.Null(GitHubReferenceLinker.LinkifyReferences(null!)); + } + + [Fact] + public void NoRefsReturnsOriginal() + { + var html = "

No references here

"; + Assert.Equal(html, GitHubReferenceLinker.LinkifyReferences(html)); + } + + [Fact] + public void PlainTextWithoutHtml() + { + var text = "Fix #42 please"; + var result = GitHubReferenceLinker.LinkifyReferences(text, "https://github.com/owner/repo"); + Assert.Contains("href=\"https://github.com/owner/repo/issues/42\"", result); + } + + // --- ExtractOwnerRepo tests --- + + [Fact] + public void ExtractOwnerRepo_HttpsUrl() + { + Assert.Equal("PureWeen/PolyPilot", GitHubReferenceLinker.ExtractOwnerRepo("https://github.com/PureWeen/PolyPilot")); + } + + [Fact] + public void ExtractOwnerRepo_HttpsUrlWithGit() + { + Assert.Equal("owner/repo", GitHubReferenceLinker.ExtractOwnerRepo("https://github.com/owner/repo.git")); + } + + [Fact] + public void ExtractOwnerRepo_SshUrl() + { + Assert.Equal("owner/repo", GitHubReferenceLinker.ExtractOwnerRepo("git@github.com:owner/repo.git")); + } + + [Fact] + public void ExtractOwnerRepo_SshUrlNoGit() + { + Assert.Equal("owner/repo", GitHubReferenceLinker.ExtractOwnerRepo("git@github.com:owner/repo")); + } + + [Fact] + public void ExtractOwnerRepo_NullReturnsNull() + { + Assert.Null(GitHubReferenceLinker.ExtractOwnerRepo(null)); + } + + [Fact] + public void ExtractOwnerRepo_EmptyReturnsNull() + { + Assert.Null(GitHubReferenceLinker.ExtractOwnerRepo("")); + } + + // --- Markdown-rendered HTML patterns --- + + [Fact] + public void MarkdownRenderedParagraph_RefsLinked() + { + // Simulates what Markdig produces for "Fix for #123" + var html = "

Fix for #123

\n"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + Assert.Contains("href=\"https://github.com/owner/repo/issues/123\"", result); + } + + [Fact] + public void MarkdownRenderedList_RefsLinked() + { + var html = "
    \n
  • Fixed #10
  • \n
  • Closed #20
  • \n
\n"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + Assert.Contains("href=\"https://github.com/owner/repo/issues/10\"", result); + Assert.Contains("href=\"https://github.com/owner/repo/issues/20\"", result); + } + + [Fact] + public void DoesNotLinkInsideExistingMarkdownLinks() + { + // Markdig converts [text](url) to
text + var html = "

See #42

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + // #42 inside the existing should NOT be re-linked + var hrefCount = result.Split("href=").Length - 1; + Assert.Equal(1, hrefCount); + } + + [Fact] + public void RefInInlineCode_NotLinked() + { + // Markdig converts `#123` to #123 + var html = "

Use #123 for the issue

"; + var result = GitHubReferenceLinker.LinkifyReferences(html, "https://github.com/owner/repo"); + Assert.Contains("#123", result); + Assert.DoesNotContain("href=\"https://github.com/owner/repo/issues/123\"", result); + } + + [Fact] + public void QualifiedRef_OwnerWithDotsAndDashes() + { + var html = "

my-org.name/my-repo.js#42

"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.Contains("href=\"https://github.com/my-org.name/my-repo.js/issues/42\"", result); + } + + [Fact] + public void DoubleDotInOwnerRepo_NotLinked() + { + // Avoid matching file paths like ../../file#123 + var html = "

See ../repo#42

"; + var result = GitHubReferenceLinker.LinkifyReferences(html); + Assert.DoesNotContain("href=", result); + } + + [Fact] + public void ExtractOwnerRepo_MaliciousSshUrl_ReturnsNull() + { + var result = GitHubReferenceLinker.ExtractOwnerRepo("git@github.com:org/repo\" onclick=\"alert(1).git"); + Assert.Null(result); + } + + [Fact] + public void ExtractOwnerRepo_MaliciousHttpsUrl_ReturnsNull() + { + var result = GitHubReferenceLinker.ExtractOwnerRepo("https://github.com/org/repo\"onclick=\"alert(1)"); + Assert.Null(result); + } +} diff --git a/PolyPilot.Tests/PolyPilot.Tests.csproj b/PolyPilot.Tests/PolyPilot.Tests.csproj index 9d59fe8618..a915e77b64 100644 --- a/PolyPilot.Tests/PolyPilot.Tests.csproj +++ b/PolyPilot.Tests/PolyPilot.Tests.csproj @@ -59,6 +59,7 @@ + diff --git a/PolyPilot/Components/ChatMessageItem.razor b/PolyPilot/Components/ChatMessageItem.razor index 19909faf6d..07c86d5cc1 100644 --- a/PolyPilot/Components/ChatMessageItem.razor +++ b/PolyPilot/Components/ChatMessageItem.razor @@ -45,7 +45,7 @@ { AI } -
@((MarkupString)ChatMessageList.RenderMarkdown(Message.Content))
+
@((MarkupString)ChatMessageList.RenderMarkdown(Message.Content, RepoUrl))
@if (!Compact && !IsStreaming) {
@Message.Timestamp.ToShortTimeString()@(!string.IsNullOrEmpty(Message.Model) ? $" ยท {Message.Model}" : "")
@@ -178,7 +178,7 @@ case ChatMessageType.System:
-
@((MarkupString)ChatMessageList.RenderMarkdown(Message.Content))
+
@((MarkupString)ChatMessageList.RenderMarkdown(Message.Content, RepoUrl))
break; @@ -250,6 +250,7 @@ [Parameter] public bool Compact { get; set; } [Parameter] public bool IsStreaming { get; set; } [Parameter] public string? UserAvatarUrl { get; set; } + [Parameter] public string? RepoUrl { get; set; } private static string? GetImageSource(ChatMessage msg) => msg.ImageDataUri ?? (!string.IsNullOrEmpty(msg.ImagePath) ? ChatMessageList.FileToDataUri(msg.ImagePath) : null); diff --git a/PolyPilot/Components/ChatMessageList.razor b/PolyPilot/Components/ChatMessageList.razor index f77029c8fa..5188e2efde 100644 --- a/PolyPilot/Components/ChatMessageList.razor +++ b/PolyPilot/Components/ChatMessageList.razor @@ -12,7 +12,7 @@ { @foreach (var msg in Messages.ToList()) { - + } @* Current turn tool activity feed โ€” skip activities already in history *@ @@ -72,7 +72,7 @@ { AI } -
@((MarkupString)RenderMarkdown(StreamingContent))
+
@((MarkupString)RenderMarkdown(StreamingContent, RepoUrl))
} @@ -113,6 +113,7 @@ [Parameter] public DateTime? ProcessingStartedAt { get; set; } [Parameter] public int ToolCallCount { get; set; } [Parameter] public int ProcessingPhase { get; set; } + [Parameter] public string? RepoUrl { get; set; } private string GetProcessingStatus() { @@ -170,10 +171,11 @@ @"(?"; return match.Value; }); + html = PolyPilot.Services.GitHubReferenceLinker.LinkifyReferences(html, repoUrl); // LRU eviction: remove oldest when at capacity if (_markdownCache.Count >= MarkdownCacheMaxSize) { @@ -195,8 +198,8 @@ _markdownCacheLru.RemoveFirst(); } } - _markdownCache[content] = html; - _markdownCacheLru.AddLast(content); + _markdownCache[cacheKey] = html; + _markdownCacheLru.AddLast(cacheKey); return html; } catch { return System.Net.WebUtility.HtmlEncode(content).Replace("\n", "
"); } diff --git a/PolyPilot/Components/ExpandedSessionView.razor b/PolyPilot/Components/ExpandedSessionView.razor index e91f6e3b1a..78566ac193 100644 --- a/PolyPilot/Components/ExpandedSessionView.razor +++ b/PolyPilot/Components/ExpandedSessionView.razor @@ -143,7 +143,8 @@ Compact="false" UserAvatarUrl="@UserAvatarUrl" Layout="@Layout" - Style="@Style" /> + Style="@Style" + RepoUrl="@RepoUrl" /> @if (!string.IsNullOrEmpty(Error)) @@ -290,6 +291,7 @@ [Parameter] public IReadOnlyList AvailableModels { get; set; } = Array.Empty(); [Parameter] public List? PendingImages { get; set; } [Parameter] public string? UserAvatarUrl { get; set; } + [Parameter] public string? RepoUrl { get; set; } [Parameter] public ChatLayout Layout { get; set; } = ChatLayout.Default; [Parameter] public ChatStyle Style { get; set; } = ChatStyle.Normal; [Parameter] public bool IsLoadingHistory { get; set; } diff --git a/PolyPilot/Components/Pages/Dashboard.razor b/PolyPilot/Components/Pages/Dashboard.razor index 2b6cb4c77c..fb78cb0ebc 100644 --- a/PolyPilot/Components/Pages/Dashboard.razor +++ b/PolyPilot/Components/Pages/Dashboard.razor @@ -139,6 +139,7 @@ AvailableModels="availableModels" PendingImages="@(pendingImagesBySession.TryGetValue(session.Name, out var pi2) ? pi2 : null)" UserAvatarUrl="@CopilotService.GitHubAvatarUrl" + RepoUrl="@CopilotService.GetRepoUrlForSession(session.Name)" Layout="@CopilotService.ChatLayout" Style="@CopilotService.ChatStyle" FontSize="fontSize" @@ -308,6 +309,7 @@ Error="@(errorBySession.TryGetValue(session.Name, out var errG) ? errG : null)" PendingImages="@(pendingImagesBySession.TryGetValue(session.Name, out var pi) ? pi : new())" UserAvatarUrl="@CopilotService.GitHubAvatarUrl" + RepoUrl="@CopilotService.GetRepoUrlForSession(session.Name)" Layout="@CopilotService.ChatLayout" Style="@CopilotService.ChatStyle" IsRenaming="@(cardRenamingSession == session.Name)" diff --git a/PolyPilot/Components/SessionCard.razor b/PolyPilot/Components/SessionCard.razor index c842bbd571..ca4bfba92f 100644 --- a/PolyPilot/Components/SessionCard.razor +++ b/PolyPilot/Components/SessionCard.razor @@ -99,7 +99,8 @@ Compact="true" UserAvatarUrl="@UserAvatarUrl" Layout="@Layout" - Style="@Style" /> + Style="@Style" + RepoUrl="@RepoUrl" /> @if (!string.IsNullOrEmpty(Error)) @@ -164,6 +165,7 @@ [Parameter] public string? Error { get; set; } [Parameter] public List PendingImages { get; set; } = new(); [Parameter] public string? UserAvatarUrl { get; set; } + [Parameter] public string? RepoUrl { get; set; } [Parameter] public ChatLayout Layout { get; set; } = ChatLayout.Default; [Parameter] public ChatStyle Style { get; set; } = ChatStyle.Normal; diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 85b70c44ff..569024f700 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -609,6 +609,37 @@ private object ApplySort(AgentSessionInfo session, Dictionary Organization.Sessions.FirstOrDefault(m => m.SessionName == sessionName); + /// + /// Resolves the GitHub repo URL for a session (via WorktreeId or group RepoId). + /// Returns null if the session has no associated repo. + /// + public string? GetRepoUrlForSession(string sessionName) + { + var meta = GetSessionMeta(sessionName); + if (meta == null) return null; + + // Try via WorktreeId first + if (meta.WorktreeId != null) + { + var wt = _repoManager.Worktrees.FirstOrDefault(w => w.Id == meta.WorktreeId); + if (wt != null) + { + var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == wt.RepoId); + if (repo != null) return repo.Url; + } + } + + // Fall back to group's RepoId + var group = Organization.Groups.FirstOrDefault(g => g.Id == meta.GroupId); + if (group?.RepoId != null) + { + var repo = _repoManager.Repositories.FirstOrDefault(r => r.Id == group.RepoId); + if (repo != null) return repo.Url; + } + + return null; + } + ///
/// Check whether a session belongs to a multi-agent group. /// Used by the watchdog to apply the longer timeout for orchestrated workers. diff --git a/PolyPilot/Services/GitHubReferenceLinker.cs b/PolyPilot/Services/GitHubReferenceLinker.cs new file mode 100644 index 0000000000..7693270df1 --- /dev/null +++ b/PolyPilot/Services/GitHubReferenceLinker.cs @@ -0,0 +1,195 @@ +using System.Text; +using System.Text.RegularExpressions; + +namespace PolyPilot.Services; + +/// +/// Converts GitHub issue/PR references in HTML to clickable links. +/// Handles both fully-qualified (owner/repo#123) and bare (#123) references. +/// +public static class GitHubReferenceLinker +{ + // Allowlist for owner/repo path segments โ€” prevents HTML attribute injection + private static readonly Regex SafeOwnerRepoRegex = new( + @"^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$", + RegexOptions.Compiled); + + // Tags whose content should NOT be processed + private static readonly HashSet SkipTags = new(StringComparer.OrdinalIgnoreCase) + { "a", "code", "pre", "script", "style" }; + + // Matches an HTML tag (opening, closing, or self-closing) + private static readonly Regex TagRegex = new( + @"<(/?)(\w+)([^>]*)>", + RegexOptions.Compiled); + + /// + /// Converts GitHub references in HTML to clickable links. + /// + /// HTML string to process. + /// + /// Optional GitHub repo URL (e.g., "https://github.com/owner/repo") for resolving bare #123 refs. + /// When null, only fully-qualified owner/repo#123 refs are linked. + /// + public static string LinkifyReferences(string html, string? repoUrl = null) + { + if (string.IsNullOrEmpty(html)) return html; + + var ownerRepo = ExtractOwnerRepo(repoUrl); + var sb = new StringBuilder(html.Length + 64); + var skipDepth = new Dictionary(StringComparer.OrdinalIgnoreCase); + int pos = 0; + + foreach (Match tagMatch in TagRegex.Matches(html)) + { + // Process text before this tag + if (tagMatch.Index > pos) + { + var text = html.Substring(pos, tagMatch.Index - pos); + if (IsInsideSkipTag(skipDepth)) + sb.Append(text); + else + sb.Append(LinkifyText(text, ownerRepo)); + } + + // Append the tag itself + sb.Append(tagMatch.Value); + + // Track skip tag depth + var tagName = tagMatch.Groups[2].Value; + if (SkipTags.Contains(tagName)) + { + var isClosing = tagMatch.Groups[1].Value == "/"; + var isSelfClosing = tagMatch.Groups[3].Value.TrimEnd().EndsWith("/"); + + if (!isClosing && !isSelfClosing) + { + skipDepth.TryGetValue(tagName, out var depth); + skipDepth[tagName] = depth + 1; + } + else if (isClosing) + { + skipDepth.TryGetValue(tagName, out var depth); + if (depth > 0) skipDepth[tagName] = depth - 1; + } + } + + pos = tagMatch.Index + tagMatch.Length; + } + + // Process remaining text after last tag + if (pos < html.Length) + { + var text = html.Substring(pos); + if (IsInsideSkipTag(skipDepth)) + sb.Append(text); + else + sb.Append(LinkifyText(text, ownerRepo)); + } + + return sb.ToString(); + } + + // Combined regex: qualified ref first (higher priority), then bare ref. + // Using alternation ensures each position is matched at most once. + private static readonly Regex CombinedRefRegex = new( + @"([A-Za-z0-9][A-Za-z0-9._-]*/[A-Za-z0-9][A-Za-z0-9._-]*)#(\d+)|#(\d+)", + RegexOptions.Compiled); + + /// + /// Linkifies GitHub references in a plain text segment (not inside skip tags). + /// Uses a single-pass regex to avoid re-matching refs inside already-created links. + /// + internal static string LinkifyText(string text, string? ownerRepo) + { + return CombinedRefRegex.Replace(text, match => + { + var startIdx = match.Index; + + // Skip if preceded by & (HTML entities like {) + if (startIdx > 0 && text[startIdx - 1] == '&') + return match.Value; + + if (match.Groups[1].Success) + { + // Qualified ref: owner/repo#123 + var refOwnerRepo = match.Groups[1].Value; + var number = match.Groups[2].Value; + + // Don't match file paths with double dots or trailing dots + if (refOwnerRepo.Contains("..") || refOwnerRepo.EndsWith(".")) + return match.Value; + + var url = $"https://github.com/{refOwnerRepo}/issues/{number}"; + return $"{match.Value}"; + } + else if (match.Groups[3].Success && ownerRepo != null) + { + // Bare ref: #123 + var number = match.Groups[3].Value; + + // Skip if preceded by a word character or / (URL fragments, file paths) + if (startIdx > 0 && (char.IsLetterOrDigit(text[startIdx - 1]) || text[startIdx - 1] == '/' || text[startIdx - 1] == '.')) + return match.Value; + + var url = $"https://github.com/{ownerRepo}/issues/{number}"; + return $"{match.Value}"; + } + + return match.Value; + }); + } + + /// + /// Extracts "owner/repo" from a GitHub URL. + /// Handles formats: https://github.com/owner/repo, https://github.com/owner/repo.git + /// + internal static string? ExtractOwnerRepo(string? repoUrl) + { + if (string.IsNullOrEmpty(repoUrl)) return null; + + try + { + // Handle git@ SSH URLs + if (repoUrl.StartsWith("git@", StringComparison.OrdinalIgnoreCase)) + { + var colonIdx = repoUrl.IndexOf(':'); + if (colonIdx > 0) + { + var path = repoUrl.Substring(colonIdx + 1); + path = path.TrimEnd('/'); + if (path.EndsWith(".git", StringComparison.OrdinalIgnoreCase)) + path = path[..^4]; + if (!SafeOwnerRepoRegex.IsMatch(path)) return null; + return path; + } + } + + // Handle HTTPS URLs + if (Uri.TryCreate(repoUrl, UriKind.Absolute, out var uri)) + { + var path = uri.AbsolutePath.Trim('/'); + if (path.EndsWith(".git", StringComparison.OrdinalIgnoreCase)) + path = path[..^4]; + + var parts = path.Split('/'); + if (parts.Length >= 2) + { + var result = $"{parts[0]}/{parts[1]}"; + if (!SafeOwnerRepoRegex.IsMatch(result)) return null; + return result; + } + } + } + catch { /* Malformed URL โ€” return null */ } + + return null; + } + + private static bool IsInsideSkipTag(Dictionary skipDepth) + { + foreach (var kvp in skipDepth) + if (kvp.Value > 0) return true; + return false; + } +}