Repository navigation
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
|
| Layer / File(s) | Summary |
|---|---|
Unresolved mention fallback links resources/js/components/linkified-text.tsx, tests/Unit/LinkifiedText.test.tsx, tests/Unit/CommitPoll.test.tsx, tests/Unit/ComposerPreview.test.tsx |
LinkifiedText links unresolved mentions to /@handle by default. Setting linksEveryMention={false} leaves them as text. The tests expect fallback links in commit polls and mention previews. |
Commit body and chat controls resources/js/components/commit-body.tsx, resources/js/components/chat/message-parts.tsx, .ai/guidelines/resources.js.blade.php, KNOWLEDGE_BASE.md |
CommitBody forwards linksEveryMention through quoted and unquoted content. Chat disables fallback links. The prefetch rule excludes unresolved handles, and the knowledge base documents mention behavior. |
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Feature
Possibly related issues
- [RFC] Orgs #938: This issue proposes organization profiles and mentions; this change links unresolved handles to profile routes.
Possibly related PRs
- dev-null-sh/dev-null.sh#1156: Adds handle-based identity and mention extraction used by unresolved mention links.
- dev-null-sh/dev-null.sh#1348: Changes commit mention rendering, which this PR extends with fallback links.
- dev-null-sh/dev-null.sh#1406: Adds the
/@handleroutes targeted by these fallback links.
Suggested reviewers: xheaven, jubianchi, nunomaduro
Merge Risk: 
Minimal · previous review 1d519
-
Unresolved mentions gain profile links while chat keeps them as text. No concrete merge-blocking risk remains.
Security architecture risk: 
Minimal · up to 1d519
-
The new links preserve existing authentication, validation, and rate-limit controls. Guests cannot initiate GitHub lookups, chat retains its previous behavior, and viewing rendered text does not itself initiate a lookup.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — An author can place a valid handle in rendered text and induce a visiting member to navigate into the existing lookup flow. The traced effect is profile lookup and possible unclaimed-profile persistence, not arbitrary URL selection or acquisition of profile ownership.
Trust Boundaries and Controls
- observed — Server-side authentication, lookup-specific throttling, and request validation protect the provider-spending POST. Guest lookup suppression is enforced by the controller; nofollow is not the authorization boundary.
-
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title says mentions of anyone now link to their page, and that the page looks them up on GitHub. The changeset does this: LinkifiedText, CommitBody and the message-parts call site add a linksEveryMention option, which defaults to true and renders unknown
@handlementions as links to /@handle. The changeset also adds rel="nofollow" and no-prefetch handling, and updates the guidelines and the KNOWLEDGE_BASE docs. The title covers the main change. It does not mention the opt-out for remotes or the nofollow and no-prefetch details, which is acceptable for a title.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 7 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
1d51903 to
e45c1b3
Compare
Summary
Stacked on #1407. In commits and replies, a mention of someone who isn't on /dev/null used to stay plain text. Now any
@handlethat isn't a known member or organization links to/@handle, where #1406's page looks the account up on GitHub for members, and asks guests to sign in.Nothing is stored and nobody is notified: the link is all there is.
Changes
LinkifiedTextlinks these mentions withrel="nofollow", the same style as other mentions, and without prefetch. Guests get a missing page, and a feed would otherwise ask for it on every hover. I added this case to the never-prefetch list of the frontend guidelines.MessageTextpasseslinksEveryMention={false}.Decisions
Visual evidence
None: the link looks exactly like a mention of a member. The
LinkifiedText,CommitPollandComposerPreviewtests cover the markup.Limitations
Summary by CodeRabbit
New Features
Documentation
Bug Fixes