ci: use shared PR validation action from shared-ai-standards#24
ci: use shared PR validation action from shared-ai-standards#24andreiships-bot merged 1 commit intodevfrom
Conversation
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
andreiships-bot
left a comment
There was a problem hiding this comment.
Single-pass review (Claude)
Overall: The core integration is correct and well-reasoned:
- SHA-pinned shared action (
4a7766058a3981a4cd54b440c594aae1d8121c77) rather than@main— correct continue-on-error: trueon the shared action step — correctskipped/cancelledoutcomes treated as pass (intentional infra-failure protection) — correct- Optional chaining fix
c.body?.includes(markerText)incheck-standards— correct title-typesinput alongside the shared action's regex — correct usage- Job-level
ifskip replaced with in-script TEAM_MEMBERS check — correct
One real bug found in the new check-compliance job (see inline comment).
Replace inline title regex check with shared composite action from andreiships/shared-ai-standards. The shared action validates PR titles against conventional commit format; this workflow manages labels and linked-issue enforcement on top. - Fixes c.body?.includes (null-safety for PRs with no body) - Treats skipped/cancelled title-check outcome as pass to avoid false positives when the shared action is unreachable - Adds core.setFailed() so check-standards job fails visibly
02a78b1 to
cc2fde0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
andreiships-bot
left a comment
There was a problem hiding this comment.
Single-pass review (Claude)
Overall: Clean, well-scoped refactor. The integration is correct and the safety properties (infra-failure protection, null-body guard, explicit core.setFailed) are all improvements over the inline version. The prior review comment about check-compliance is now obsolete -- that job is not present in this version of the PR.
Findings:
-
Behavior change: regex is now stricter (informational, not a blocker). The removed inline regex
/^(feat|fix|docs|chore|refactor|test)\s*(\([a-zA-Z0-9-]+\))?\s*:/allowed a space before(scope)(e.g.feat (scope): ...) and did not require content after the colon. The shared action regex^(type)(\([\w-]+\))?!?:\s+.+$is stricter: no space before(,!breaking-change marker supported, requires at least one non-space character after:. This is a net improvement but is a subtle behavior change. Worth documenting in CONTRIBUTING.md. -
citype excluded from contributor title check (informational).title-types: feat,fix,docs,chore,refactor,testdoes not includeci. If an external contributor ever opens aci:-prefixed PR, it would getneeds:title. Confirm this is intentional. -
SHA pin comment says
# main(nitpick). SHA pinning is the correct practice. Just ensure the SHA gets bumped deliberately when the shared action receives updates. -
skip-usersinput not forwarded to shared action (design note, not a bug). The job-levelif:already filters skip-users before any steps run, so the shared action built-inskip-usersinput is unused. Correct by design.
No blockers. The core.setFailed() additions are a genuine improvement -- the original would silently pass the job even after posting a needs:title/needs:issue label. The optional-chaining fix on c.body?.includes() is also correct.
Gemini Deep ReviewSummaryThis PR improves CI by replacing a local regex-based title check with a shared GitHub Action for validating PR standards, which is a great move for consistency and maintainability. The changes to fail the workflow on violations ( Findings[gemini-1] blocking: P0 | .github/workflows/pr-standards.yml:26 | The - uses: andreiships/shared-ai-standards/.github/actions/pr-validation @41b2466c374cddf778ed0ea5bb2d08db7d9939d7 # main
+ uses: andreiships/shared-ai-standards/.github/actions/pr-validation@41b2466c374cddf778ed0ea5bb2d08db7d9939d7 # main
PR MetadataSuggested PR Title: Questions
Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes |
Codex ReviewSummaryThis PR is focused and improves maintainability by delegating title validation to the shared action while preserving local label/issue enforcement. I found one reliability issue in failure-mode handling that should be fixed before merge. Findings[FINDING-1] issue: P1 | .github/workflows/pr-standards.yml:87 | Code Quality
Architecture
PR MetadataSuggested PR Title: Recommendation[ ] Approve | [ ] Approve with changes | [x] Request changes Escalate to Gemini?[ ] Yes - [reason] | [x] No |
Dual-Review Summary (Round 1)
Convergence: ❌ Not achieved (2 blocking findings) Rounds: 1 Findings[Codex #1] P1 -
[Gemini #1] P0 -
|
Summary
Replaces the inline PR title validation step with the shared composite action from
andreiships/shared-ai-standards, reducing duplication and keeping validation logic in one place.Changes
andreiships/shared-ai-standards/.github/actions/pr-validation@<sha>c.body?.includes()optional chaining (wasc.body.includes()— would throw on null body)skipped/cancelledtitle-check outcomes treated as pass (prevents infra failures triggeringneeds:title)Test Plan
needs:titlelabel added, comment posted, job failsneeds:issuelabel addeddocs:/refactor:PR → issue check skippedRelated
Depends on
andreiships/shared-ai-standardsPR #3 (merged).