Make safe-output run summary entries regular and always linked - #51478
Conversation
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis.
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
There was a problem hiding this comment.
Pull request overview
Regularizes safe-output summaries with consistent target links, label formatting, and collapsible framing.
Changes:
- Adds target URL discovery and derived issue/PR links.
- Normalizes label objects into readable names.
- Adds collapsible summary framing and coverage for key result shapes.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/safe_output_summary.cjs |
Implements target formatting, label normalization, and collapsible summaries. |
actions/setup/js/safe_output_summary.test.cjs |
Tests links, labels, URL safety, and framing. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The changes are clean and well-tested.
- Entity URL extraction is centralized via priority-ordered field lists
- XSS is guarded via the
https?://regex informatLink— non-http URLs render as plain text (tested) - Graceful fallback:
buildEntityUrlconstructs links from repo+number when no URL is reported <details>wrapping is consistent with other summary sections- Label normalization correctly handles both plain strings and GitHub label objects
No blocking issues.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 21.6 AIC · ⌖ 8.94 AIC · ⊞ 5.5K
|
@copilot Address the above review comments |
There was a problem hiding this comment.
Generated by ✂️ Ponytail Reviewer for #51478 · auto · 41.3 AIC · ⌖ 4.76 AIC · ⊞ 6.8K
Comment /ponytail to run again
Test Quality Analysis 🧪PR #51478: "Make safe-output run summary entries regular and always linked" SummaryThis PR adds 7 new behavioral tests to Key Metrics
Test Coverage BreakdownDetailed Test Classifications
Strengths ✅
Notes
Recommendation✅ APPROVE — This PR demonstrates strong test quality. All new tests are behavioral contracts covering the design's key invariants, including security boundaries. The security-focused protocol validation test is particularly commendable. Test inflation is healthy, and no coding violations detected.
|
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 57.4 AIC · ⌖ 7.77 AIC · ⊞ 7.1K
Comment /matt to run again
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Addressed in |
|
🎉 This pull request is included in a new release. Release: |
The safe-output section of the workflow run summary rendered inconsistently: entries whose handlers report entity fields under non-canonical names (e.g.
close_pull_request→pull_request_url,reply_to_pull_request_review_comment→reply_url) showed nothing but a heading, labels rendered as[object Object], and the section was a bare##heading rather than a collapsible block like the threat-detection sections.All changes are in
actions/setup/js/safe_output_summary.cjs.Uniform entity link
**URL:**/**Location:**/**Project URL:**lines with a single**Target:**line.url,html_url,issue_url,pull_request_url,discussion_url,comment_url,reply_url,review_url,item_url,commit_url,projectUrl,run_url, …), so every handler shape produces a link without each handler having to normalize its result.repo+number(e.g.add_labels), the link is derived as<GITHUB_SERVER_URL>/owner/repo/issues/<n>, which GitHub redirects to the PR when the number is a PR.http(s)URLs are linkified; anything else degrades to plain text.Labels
{name}), preferring handler-reportedlabelsAdded/labelsover the raw message labels. Fixes**Labels:** [object Object].Section framing
<details>with a status emoji and counts in the<summary>, matching the threat-detection sections. TheProcessed N …/Status:/ item-count lines are unchanged, just moved inside.Before / after for the same run:
Tests in
actions/setup/js/safe_output_summary.test.cjscover close-PR links, review-comment reply links, label-object normalization, derived entity URLs, non-httpURL handling, and the collapsible wrapper.