Repository navigation
ci: adopt native Pages reports with template 2.1 - #3
Conversation
📝 SummarySummary by CodeRabbit
WalkthroughThe reports workflow adds a pull request trigger and changes its reusable workflow call and permissions. The tests workflow changes its permissions and sets a required-status input based on the actor. ChangesReports workflow
Tests workflow
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: 🔵 Low · up to Later Reports runs may use changed upstream workflow code. This warrants awareness but does not currently block merging. Pre-merge checks |
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4634d96a3f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🔇 Additional comments (3)
.github/workflows/tests.yml (1)
16-16: 🩺 Stability & AvailabilityThe concern is refuted.
.github/workflows/tests.ymltriggers only onpushandworkflow_dispatch; it has nopull_requestorpull_request_targettrigger. Therefore, fork pull-request runs do not reach this caller, so the proposed status-write failure is not applicable..github/workflows/reports.yml (1)
5-6: LGTM!Also applies to: 11-11
.github/workflows/reports.yml-17-17 (1)
17-17: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Pin the reusable workflow to a commit SHA.
@maincan resolve to different workflow code on later runs. The caller permitspages: writeandid-token: write, so an upstream change can alter code running with those permissions. Pin the reviewed commit SHA and update it deliberately. GitHub identifies a commit SHA as the safest reference for stability and security. (docs.github.com)Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
bbf5f2ba-2f21-430e-beba-5843b0dd8944
📒 Files selected for processing (2)
.github/workflows/reports.yml.github/workflows/tests.yml
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Routes Reports through the shared native Pages workflow so API documentation uses the PHP Fast Forward template design system. Pull requests build a preview artifact; pushes to
mainand manual runs retain their existing entry points. Caller permissions cap the callee atcontents: read, with Pages and OIDC writes reserved for its isolated deployment job, and no inherited secrets.Uses the shared workflow introduced by the merged php-fast-forward/dev-tools#363, now available at
.github/workflows/reports-native.yml@main. The callee checks thatfast-forward/phpdoc-bootstrap-templateis at least2.1.0, records its installed version and reference, and compares the generated identity assets with the selected template using SHA-256.Validation:
actionlint .github/workflows/reports.yml .github/workflows/tests.ymlandgit diff --checkpassed after fetching the explicitmainrefspec and merging the GitHub-confirmed current base. The diff contains only the Reports caller. Feature-branch Reports builds exercise the shared workflow; the isolated deployment job runs only formain.