Skip to content

fix: validate parsed PR URLs before extraction - #577

Merged
matt2e merged 1 commit into
mainfrom
invalid-pr-url
Apr 2, 2026
Merged

fix: validate parsed PR URLs before extraction#577
matt2e merged 1 commit into
mainfrom
invalid-pr-url

Conversation

@matt2e

@matt2e matt2e commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate markers before accepting them as GitHub pull request links
  • normalize extracted PR URLs to a canonical form and strip surrounding markdown punctuation
  • add regression tests for placeholder markers and fallback URL extraction

@matt2e
matt2e requested review from baxen and wesbillman as code owners April 2, 2026 05:00
@matt2e
matt2e merged commit 9af5f82 into main Apr 2, 2026
4 checks passed
@matt2e
matt2e deleted the invalid-pr-url branch April 2, 2026 05:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a907e01daa

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

function normalizePrUrl(candidate: string): string | null {
const trimmed = candidate.trim().replace(/^[<`'"\[(]+|[>`'"\]),.?!;:]+$/g, '');
const match = trimmed.match(
/^https:\/\/github\.com\/([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)\/pull\/(\d+)(?:[/?#].*)?$/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse GitHub host case-insensitively in PR URL normalization

normalizePrUrl currently accepts only lowercase https://github.com/..., so valid URLs like https://GitHub.com/org/repo/pull/123 are rejected and extractPrUrl can report no PR URL (or fall back to an unrelated one). Because hostnames are case-insensitive and the prior logic accepted these marker URLs, this introduces a regression in PR detection for some assistant/tool outputs.

Useful? React with 👍 / 👎.

loganj added a commit that referenced this pull request Apr 9, 2026
…ktree-dis

* origin/main: (45 commits)
  feat(penpal): persist recent files activity across restarts (#595)
  feat: persist open windows, tabs, and geometry across app restarts (#588)
  feat(staged): generate next prefill text when generating notes (#594)
  feat(staged): add "Run again" button to action output dialog (#593)
  feat: queue repo sessions in project MCP with async cancellation fix (#591)
  fix(staged): lowercase "building" subtitle for consistency (#592)
  fix(staged): use completedAt for commit prefill timeline ranking (#590)
  fix: add per-branch mutex to prevent worktree setup race condition (#589)
  feat: add diff commit session launcher (#585)
  fix: avoid error state for empty titled code reviews (#587)
  fix: cancel in-flight auto reviews before manual sessions (#584)
  feat(settings): add repo context search (#583)
  fix(staged): sort timeline items by completion time so queued items appear before completed ones (#568)
  fix: drain queued branch sessions from backend lifecycle (#579)
  fix: restore resume session metadata for resumed actions (#580)
  feat(staged): add purple diff comment theme (#578)
  fix(ui): hide branch actions during setup (#572)
  fix: validate parsed PR URLs before extraction (#577)
  fix: centralize staged relative time updates (#576)
  fix: avoid UI freezes when starting background sessions (#575)
  ...

# Conflicts:
#	apps/penpal/internal/watcher/watcher.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant