feat(comments): link tracking id to dashboard review page #58

Merged
rcsheets merged 1 commit from feat/comment-tracking-link into main 2026-05-02 08:12:01 +00:00
Owner

Render the tracking id in PR comments as a markdown link to
{DASHBOARD_BASE_URL}/reviews/{id} when the dashboard URL is configured;
fall back to the bare code-formatted id otherwise.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Render the tracking id in PR comments as a markdown link to {DASHBOARD_BASE_URL}/reviews/{id} when the dashboard URL is configured; fall back to the bare code-formatted id otherwise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat(comments): link tracking id to dashboard review page
All checks were successful
ci / check (pull_request) Successful in 43s
7a28144e4f
Render the tracking id in PR comments as a markdown link to
{DASHBOARD_BASE_URL}/reviews/{id} when the dashboard URL is configured;
fall back to the bare code-formatted id otherwise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator

Automated review by pr-reviewer v0.26.0 | Safety Check | Claude Sonnet 4 | tracking id r-f5ae66-ca256b
This is an AI-generated review and may contain mistakes.

Status: Completed


Verdict: LGTM — Clean implementation of markdown link formatting for tracking IDs with safe fallback behavior and no security or compatibility concerns.
Complexity: trivial

Safety Check Review

Code Changes

The PR adds a new function _tracking_id_md() that formats tracking IDs as markdown links when a dashboard URL is configured, falling back to code-formatted text otherwise. The function is then used to replace hardcoded tracking ID formatting in 4 comment generation functions.

Analysis

  • Logic: The implementation looks sound - it checks if a base URL exists and formats accordingly
  • Security: No obvious vulnerabilities - the function uses basic string formatting and rstrip, no dynamic code execution or injection risks
  • Error handling: The code safely handles missing dashboard URLs by falling back to the original format
  • Breaking changes: This is additive functionality that maintains backward compatibility
  • Accidental commits: No suspicious files or credentials exposed

The change is straightforward string formatting enhancement with proper fallback behavior. No alarming issues detected.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.26.0 | Safety Check | Claude Sonnet 4 | tracking id `r-f5ae66-ca256b`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: LGTM** — Clean implementation of markdown link formatting for tracking IDs with safe fallback behavior and no security or compatibility concerns. **Complexity:** trivial # Safety Check Review ## Code Changes The PR adds a new function `_tracking_id_md()` that formats tracking IDs as markdown links when a dashboard URL is configured, falling back to code-formatted text otherwise. The function is then used to replace hardcoded tracking ID formatting in 4 comment generation functions. ## Analysis - **Logic**: The implementation looks sound - it checks if a base URL exists and formats accordingly - **Security**: No obvious vulnerabilities - the function uses basic string formatting and rstrip, no dynamic code execution or injection risks - **Error handling**: The code safely handles missing dashboard URLs by falling back to the original format - **Breaking changes**: This is additive functionality that maintains backward compatibility - **Accidental commits**: No suspicious files or credentials exposed The change is straightforward string formatting enhancement with proper fallback behavior. No alarming issues detected.
rcsheets force-pushed feat/comment-tracking-link from 7a28144e4f
All checks were successful
ci / check (pull_request) Successful in 43s
to 270b88569b
All checks were successful
ci / check (pull_request) Successful in 43s
2026-05-02 08:09:56 +00:00
Compare
Collaborator

Automated review by pr-reviewer v0.26.0 | Safety Check | Claude Sonnet 4 | tracking id r-f5b156-f1414b
This is an AI-generated review and may contain mistakes.

Status: Completed


Verdict: LGTM — The changes add dashboard linking functionality safely with proper fallbacks and test coverage - the flagged "credential" is just a configuration field being passed through.
Complexity: moderate

Looking at this diff, I can see it adds functionality to render tracking IDs as markdown links to a dashboard review page when a dashboard base URL is configured.

Let me examine the key changes:

  1. New configuration field: DashboardBaseURL is added to both the runner and webhook configurations - this looks fine.

  2. New function TrackingIDMD: This function creates markdown links when a dashboard URL is provided, otherwise falls back to code-formatted text. The implementation looks correct - it properly trims trailing slashes and constructs the URL path as /reviews/{trackingID}.

  3. Updated FormatHeader function: The function signature is updated to include the dashboard base URL parameter, and the tracking ID rendering is changed from backticks to use the new TrackingIDMD function.

  4. Test coverage: Tests are added for the new TrackingIDMD function covering both scenarios (with and without dashboard URL), and the existing header test is updated.

  5. Widespread updates: All call sites for FormatHeader and tracking ID formatting are consistently updated throughout the codebase.

The automated observation flagged "possible secret or credential detected" in cmd/webhook/main.go, but looking at the changes there, I only see the addition of DashboardBaseURL: cfg.DashboardBaseURL, to configuration structs. This is just passing through a configuration value, not exposing any secrets.

The changes appear to be a straightforward feature addition with proper fallback behavior and good test coverage. The code correctly handles edge cases like trailing slashes in URLs and maintains backward compatibility when no dashboard URL is configured.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.26.0 | Safety Check | Claude Sonnet 4 | tracking id `r-f5b156-f1414b`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: LGTM** — The changes add dashboard linking functionality safely with proper fallbacks and test coverage - the flagged "credential" is just a configuration field being passed through. **Complexity:** moderate Looking at this diff, I can see it adds functionality to render tracking IDs as markdown links to a dashboard review page when a dashboard base URL is configured. Let me examine the key changes: 1. **New configuration field**: `DashboardBaseURL` is added to both the runner and webhook configurations - this looks fine. 2. **New function `TrackingIDMD`**: This function creates markdown links when a dashboard URL is provided, otherwise falls back to code-formatted text. The implementation looks correct - it properly trims trailing slashes and constructs the URL path as `/reviews/{trackingID}`. 3. **Updated `FormatHeader` function**: The function signature is updated to include the dashboard base URL parameter, and the tracking ID rendering is changed from backticks to use the new `TrackingIDMD` function. 4. **Test coverage**: Tests are added for the new `TrackingIDMD` function covering both scenarios (with and without dashboard URL), and the existing header test is updated. 5. **Widespread updates**: All call sites for `FormatHeader` and tracking ID formatting are consistently updated throughout the codebase. The automated observation flagged "possible secret or credential detected" in `cmd/webhook/main.go`, but looking at the changes there, I only see the addition of `DashboardBaseURL: cfg.DashboardBaseURL,` to configuration structs. This is just passing through a configuration value, not exposing any secrets. The changes appear to be a straightforward feature addition with proper fallback behavior and good test coverage. The code correctly handles edge cases like trailing slashes in URLs and maintains backward compatibility when no dashboard URL is configured.
rcsheets deleted branch feat/comment-tracking-link 2026-05-02 08:12:02 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
brooktrails/pr-reviewer!58
No description provided.