Clarify fallback issue paths for manual PR creation - #56572
Conversation
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
|
Lean already. Ship. Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "api.github.com"
- "chatgpt.com"
- "github.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff 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
- "github.com"See Network Configuration for more information.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
No actionable changed-line issues found in this PR.
Review notes
The template reordering is consistent across both fallback paths, the protected-files messaging is still present where it matters, and the regression test covers the new body ordering well enough to catch accidental reversions. I did not find a correctness, security, or maintainability problem in the changed lines that justifies blocking this merge.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 2.74 AIC · ⌖ 7.31 AIC · ⊞ 4.6K
Comment /review to run again
There was a problem hiding this comment.
Pull request overview
Clarifies manual pull-request creation paths in fallback issues.
Changes:
- Adds prominent creation links and reassuring guidance.
- Reorders protected-file and permission details.
- Tests protected-file fallback ordering.
Show a summary per file
| File | Description |
|---|---|
actions/setup/md/pr_permission_denied_fallback.md |
Restructures permission fallback guidance. |
actions/setup/md/manifest_protection_create_pr_fallback.md |
Promotes the protected-file recovery path. |
actions/setup/js/create_pull_request.test.cjs |
Verifies protected-file body ordering. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
Reviewed with clarify mode (documentation/copy changes only).
The restructuring is a clear UX improvement:
- Front-loading a
[!TIP]callout with a prominent CTA reduces cognitive friction for users who just need to create their PR. - Downgrading
[!WARNING]→[!NOTE]for protected files is appropriate — the content is informational, not a hard blocker. - The
pr_permission_denied_fallback.mdNOTE is now tighter and avoids redundancy. - Tests correctly assert both content presence and relative ordering.
No blocking issues.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 13.8 AIC · ⌖ 10.6 AIC · ⊞ 6.2K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /codebase-design — commenting on test coverage gap and template asymmetry.
📋 Key Themes & Highlights
Key Themes
- Test coverage gap: the new ordering assertions cover the protected-file fallback but not the permission-denied fallback (
pr_permission_denied_fallback.md). - Template asymmetry: the
> [!NOTE]block inpr_permission_denied_fallback.mddrops the "changes are pushed and ready" reassurance that the parallel template retains.
Positive Highlights
- ✅ Promoting the PR link to a top-level
[!TIP]callout is a clear UX improvement. - ✅ Downgrading
[!WARNING]→[!NOTE]reduces alarm fatigue for a routine situation. - ✅ Good symmetry applied across both fallback templates.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 23.3 AIC · ⌖ 10.6 AIC · ⊞ 7.6K
Comment /matt to run again
✅ Test Quality Sentinel: ApprovePR #56572 — Regression test additions to verify fallback issue body ordering Analysis SummaryThis PR modifies markdown templates for fallback issues and enhances existing test coverage with regression assertions. No new test functions were added—only strategic additions to verify critical behavior. Test ModificationsFile: 4 Assertion Lines Added:
Quality Assessment
Verdict✅ APPROVE — Regression test assertions are well-crafted and provide concrete validation of the new fallback issue structure. The modifications ensure the critical UX improvement (prominent recovery path) cannot regress. Analysis Note: Pre-fetched test data was unavailable; analysis performed against live repository. 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
- "github.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: Score N/A—Regression test only (4 assertion lines added to verify fallback issue body ordering). No new test functions. Classification: High-value design test with no violations. Regression assertions ensure critical UX structure (prominent recovery path) cannot break.
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
|
Category: docs | Risk: low | Score: 36 (impact 18 + urgency 8 + quality 10) | Action: defer | Batch: none
|
1 similar comment
|
Category: docs | Risk: low | Score: 36 (impact 18 + urgency 8 + quality 10) | Action: defer | Batch: none
|
|
Category: bug | Risk: medium | Score: 51 (impact 22 urgency 16 quality 13) | Action: batch_review | Batch: workflow-ux Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"See Network Configuration for more information.
|
|
Category: docs | Risk: low | Score: 40 | Breakdown: impact 20/urgency 8/quality 12 | Action: defer | Batch: none
|
|
Category: docs | Risk: low | Total: 38 (impact 16 + urgency 7 + quality 15)
|
|
Category: docs | Risk: low | Score: 32 | Action: defer | Batch: workflow-fallbacks. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
|
Category: docs | Risk: low | Score: 46/100 (impact 20/50, urgency 10/30, quality 16/20) | Action: batch_review | Note: UX/docs fallback cleanup with approvals and regression tests.
|
|
Category: docs | Risk: low | Score: 34 (impact 12, urgency 8, quality 16) | Action: auto_merge | Batch: workflow-fallbacks
|
|
Category: docs | Risk: low | Score: 39/100 (impact 16/50, urgency 9/30, quality 14/20) | Action: auto_merge | Batch: workflow-fallbacks | CI: unknown
|
|
Category: bug | Risk: low | Score: 49/100 (impact 24/50 urgency 10/30 quality 15/20) | Action: auto_merge | Batch: none | CI: unknown. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
|
|
Category: docs | Risk: low | Score: 41/100 | Action: defer | Batch: workflow-ux. Low-risk messaging update with limited user impact.
|
|
Category: bug | Risk: medium | Score: 49/100 (impact 26/50, urgency 11/30, quality 12/20) | Action: batch_review | Batch: workflow-policy. Fallback behavior is worth a grouped review pass with the other policy changes.
|
|
PR triage: category=docs, risk=low, score=41 (impact 18 + urgency 10 + quality 13). Action: defer. Docs/clarification PR for manual PR fallback paths; low-risk but low-value for immediate review.
|
|
Triage: bug | risk: low | score: 46/100 (impact 22, urgency 12, quality 12).
|
|
🎉 This pull request is included in a new release. Release: |
Protected-file fallback issues buried the PR creation link below the original description and presented it as a warning, making the intended next step unclear.
Prominent recovery path
Readable issue structure
Run: https://github.com/github/gh-aw/actions/runs/33195749972
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.