refactor(pr-validation): modularize workflow into composites#164
Merged
refactor(pr-validation): modularize workflow into composites#164
Conversation
…c/validate/ Extract all inline business logic from pr-validation.yml into 7 reusable composite actions under src/validate/. Add dry_run input, fix script injection risks (use env vars instead of direct interpolation), fix notify ref for external callers, and update conventions to prohibit workflow_dispatch on reusable workflows due to injection risk.
|
Caution Review failedPull request was closed or merged during review WalkthroughReplaces legacy PR‑validation docs with a reusable two‑tier Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
🔍 Lint Analysis
❌ Failures (4)YAML Lint
Pinned Actions
Markdown Link CheckNo annotation details available — view full logs. Spelling Check
|
🛡️ CodeQL Analysis ResultsLanguages analyzed: Found 7 issue(s): 7 Medium
🔍 View full scan logs | 🛡️ Security tab |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-validation.yml (1)
233-236:⚠️ Potential issue | 🟠 MajorDon’t send a green Slack alert when changelog validation fails.
statusandfailed_jobsonly inspect three jobs today. Ifpr-changelogfails, the workflow still reports success to Slack.🔧 Proposed fix
- status: ${{ (needs.pr-source-branch.result == 'failure' || needs.pr-title.result == 'failure' || needs.pr-description.result == 'failure') && 'failure' || 'success' }} + status: ${{ (needs.pr-source-branch.result == 'failure' || needs.pr-title.result == 'failure' || needs.pr-description.result == 'failure' || needs.pr-changelog.result == 'failure') && 'failure' || 'success' }} workflow_name: "PR Validation" - failed_jobs: ${{ needs.pr-source-branch.result == 'failure' && 'Source Branch, ' || '' }}${{ needs.pr-title.result == 'failure' && 'PR Title, ' || '' }}${{ needs.pr-description.result == 'failure' && 'PR Description' || '' }} + failed_jobs: ${{ needs.pr-source-branch.result == 'failure' && 'Source Branch, ' || '' }}${{ needs.pr-title.result == 'failure' && 'PR Title, ' || '' }}${{ needs.pr-description.result == 'failure' && 'PR Description, ' || '' }}${{ needs.pr-changelog.result == 'failure' && 'Changelog' || '' }}As per coding guidelines, Slack notification should pass
statusbased on the required job results.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 233 - 236, The Slack notification currently bases status and failed_jobs only on needs.pr-source-branch, needs.pr-title and needs.pr-description; update both expressions to include needs.pr-changelog.result so a failing changelog produces a non-green alert and is listed in failed_jobs. Concretely, extend the status conditional to OR in needs.pr-changelog.result == 'failure' (alongside the other three) and append a needs.pr-changelog.result == 'failure' && 'PR Changelog' (or similar label) into the failed_jobs concatenation so the changelog failure is reflected in the message; update the status and failed_jobs fields in the same block where those keys are defined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 69-73: Replace the hard-coded use of github.token in the
auto-labeler job with the new MANAGE_TOKEN secret so callers can supply the
elevated token; specifically update the auto-labeler step that currently
references github.token to instead read the MANAGE_TOKEN secret (and fall back
to github.token if undefined), and do the same replacement for the second
occurrence noted (the other auto-labeler usage around the later block), ensuring
the job uses secrets.MANAGE_TOKEN (with fallback) while leaving
SLACK_WEBHOOK_URL unchanged.
In `@docs/pr-validation.md`:
- Around line 137-147: Update the jobs table in docs/pr-validation.md to reflect
the draft-PR guard used in the workflow: for pr-source-branch, pr-labels,
pr-changelog, and notify include the draft check alongside their existing
condition (e.g. prepend or append "github.event.pull_request.draft != true &&"
so the Condition cell matches the workflow gating), leaving other rows unchanged
(pr-title, pr-size, pr-description, pr-metadata, pr-checks-summary remain
as-is).
- Around line 41-58: The example workflow "PR Validation" uses a floating
reference "uses:
LerianStudio/github-actions-shared-workflows/.github/workflows/pr-validation.yml@v1.x.x";
replace that floating `@v1.x.x` with `@develop` for test/example snippets or with a
concrete release tag (e.g. `@vX.Y.Z`) for production examples, and make the same
replacement pattern for the other usage snippets called out (the blocks covering
lines 62-79, 83-92, 96-103) so all examples follow the guideline of using
`@develop/`@feat/<branch> for testing and `@vX.Y.Z` for production.
In `@src/validate/pr-changelog/action.yml`:
- Around line 1-55: Add a companion README.md for this composite action that
documents the inputs (github-token, base-ref, dry-run), the outputs (updated),
behavior (how the changelog check in step id "changelog" sets outputs.updated
based on git diff against origin/${BASE_REF}), the comment step (the "Comment if
CHANGELOG not updated" step using actions/github-script@v8) including skip-label
behavior (labels "skip-changelog" and "dependencies" prevent commenting), and
provide usage examples showing how to call the action with inputs and interpret
the updated output and dry-run mode.
- Line 40: The workflow currently uses a floating tag
"actions/github-script@v8"; replace that with a specific, immutable reference
(e.g., a concrete semver tag like actions/github-script@v8.0.0 or a commit SHA)
so the action version is pinned and won't unexpectedly change; update the uses:
entry in action.yml to the chosen exact tag/sha.
In `@src/validate/pr-description/action.yml`:
- Line 18: Replace the floating major tag "uses: actions/github-script@v8" with
a pinned version to ensure reproducible builds; update the "uses" entry in
src/validate/pr-description/action.yml to reference a specific tag or commit SHA
(e.g., a concrete semver tag like actions/github-script@v8.0.0 or the exact
commit SHA) instead of "@v8".
- Around line 1-49: The action is missing a companion README.md; add
src/validate/pr-description/README.md that documents the workflow name "Validate
PR Description", the inputs (github-token, min-length) including default for
min-length, the behavior of the two steps ("Check description length" which
fails via core.setFailed when body.length < MIN_LENGTH and "Check for required
sections" which warns when requiredSections like 'Description' and 'Type of
Change' are missing), and include usage examples showing how to call the
composite action with inputs and expected outcomes (fail vs warning).
- Line 33: The second usage of the GitHub Action is unpinned ("uses:
actions/github-script@v8"), which repeats the same pinning issue; replace that
loose tag with a fully pinned reference (use the specific commit SHA used for
the first actions/github-script usage in this repo) so the line becomes "uses:
actions/github-script@<commit-sha>" to ensure reproducible, secure runs.
In `@src/validate/pr-labels/action.yml`:
- Around line 1-21: Add a companion README.md alongside action.yml that
documents this composite action: summarise what "Auto-label PR" does, list and
describe the inputs (github-token, config-path and their required/default
values), explain why actions/labeler was chosen, and include usage examples
showing how to call the action from workflows with the inputs (example must pin
the external action to a stable tag like actions/labeler@vX.Y.Z rather than
`@main`); ensure the examples demonstrate passing repo-token and
configuration-path and mention sync-labels behavior so consumers can copy/paste
a correct invocation.
- Line 17: Pin the GitHub Action usage by replacing the floating tag
"actions/labeler@v6" in validate/pr-labels/action.yml with a specific release
tag or commit SHA (e.g., "actions/labeler@v6.0.1" or a full SHA) to satisfy
repository pinning policy; then update README.md to add a short rationale for
choosing actions/labeler (why this action was selected, version
stability/maintenance info, and any config notes) so reviewers can see the
decision provenance.
In `@src/validate/pr-labels/README.md`:
- Line 8: Update src/validate/pr-labels/README.md to include a short rationale
for choosing the actions/labeler action: explain selection criteria (ease of
maintenance, GitHub-native support, simple YAML mapping for file globs), why
actions/labeler met those criteria over alternatives (custom script, other
third-party actions), and note any trade-offs (limited complex logic vs.
simplicity) and maintenance expectations; reference the existing labeler
configuration file (actions/labeler) so readers can see the mapping and where to
change rules.
In `@src/validate/pr-metadata/action.yml`:
- Line 14: The workflow currently references a floating major tag
"actions/github-script@v8" in validate/pr-metadata/action.yml; replace that
floating tag with a specific release tag (for example
"actions/github-script@v8.0.0") so the action is pinned to a fixed
version—update the uses entry in action.yml accordingly and commit the change.
- Around line 1-42: Add a README.md alongside the action
(src/validate/pr-metadata/README.md) that documents the composite action name
"Validate PR Metadata", the required input "github-token", and explains that the
steps "Check for assignee" and "Check for linked issues" only emit warnings
(they do not fail the workflow) and how they detect missing assignees and linked
issues (assignees.length === 0 and searching for keywords
['closes','fixes','resolves','relates to'] in the PR body); include example
usage snippet showing how to pass github-token and a note about
localization/case-insensitivity of keyword matching.
- Line 25: The second usage of the GitHub Action is unpinned ("uses:
actions/github-script@v8"); change it to the same fixed pin used elsewhere (use
an exact tag or commit SHA instead of a floating major) so both occurrences
reference the identical pinned version (e.g., replace "actions/github-script@v8"
with the exact tag/sha used in the other action usage).
In `@src/validate/pr-size/action.yml`:
- Line 55: The workflow currently references a floating tag "uses:
actions/github-script@v8"; replace that floating major tag with a fixed,
immutable version (a specific semver tag like "v8.x.y" or a full commit SHA) so
the action no longer floats. Locate the line containing the uses declaration
(the "actions/github-script" reference) in src/validate/pr-size/action.yml and
update it to a pinned tag, then commit the change.
- Line 87: The second usage of the GitHub Action is unpinned ("uses:
actions/github-script@v8") — replace that floating major tag with a pinned
reference (either an exact released tag or commit SHA) to prevent unexpected
breaking changes; update the "uses: actions/github-script@v8" entry to the same
fixed version/sha used elsewhere in the workflow (or choose a specific stable
release) so both occurrences are consistent and reproducible.
- Around line 34-38: The CHANGED_LINES variable can be empty when git diff
--shortstat returns nothing, causing the numeric test in the if [
"$CHANGED_LINES" -lt 50 ] to fail; fix by ensuring CHANGED_LINES is set to 0
when empty—after computing CHANGED_LINES from git diff/awk, add a fallback like
CHANGED_LINES=${CHANGED_LINES:-0} (or check -z and set CHANGED_LINES=0) before
echoing to GITHUB_OUTPUT and before the numeric comparison so the if test always
receives a valid integer.
In `@src/validate/pr-source-branch/action.yml`:
- Around line 8-11: The allowed-branches input currently only recognizes the
specific suffix "/*" for prefix matching (per the comment about lines 47-59);
update the branch-matching logic that consumes the "allowed-branches" input so
that any pattern containing "*" is treated as a wildcard (not just "/*").
Implement this by converting each allowlist token into a proper regex (escape
literal chars, replace "*" with ".*", anchor start/end as appropriate) and use
that regex to test the PR source branch; ensure tokens like "release-*" and "*"
behave as documented.
In `@src/validate/pr-title/action.yml`:
- Around line 1-47: Add a companion README.md for the Validate PR Title
composite action that documents the action purpose and chosen marketplace action
(amannn/action-semantic-pull-request@v6), lists and explains all inputs
(github-token, types with the default newline-separated list, scopes,
require-scope) and their defaults, shows how to call the action (examples of
workflow usage with GITHUB_TOKEN and overriding types/scopes/requireScope and
subjectPattern), and explains the subjectPattern/subjectPatternError behavior so
consumers know expected PR title format and how to customize it.
- Line 36: The workflow currently references the floating tag
amannn/action-semantic-pull-request@v6; update that usage to a specific release
tag (for example amannn/action-semantic-pull-request@v6.1.1) so the action is
pinned; locate the uses entry for amannn/action-semantic-pull-request in
action.yml and replace the floating `@v6` with the chosen fixed version tag.
In `@src/validate/pr-title/README.md`:
- Line 8: Update the README.md for the validate/pr-title composite to include a
brief rationale for choosing the action-semantic-pull-request action: state the
selection criteria (e.g., native support for Conventional Commits,
configurability for repo policies, active maintenance/community adoption, CI
performance and stability, and compatible license), mention any evaluated
alternatives and why they were rejected, and note how
action-semantic-pull-request satisfies the composite's requirements (integration
with GitHub Actions and ease of use); reference the action by name
("action-semantic-pull-request") and the composite (validate/pr-title) so
readers can map the rationale to the implementation.
---
Outside diff comments:
In @.github/workflows/pr-validation.yml:
- Around line 233-236: The Slack notification currently bases status and
failed_jobs only on needs.pr-source-branch, needs.pr-title and
needs.pr-description; update both expressions to include
needs.pr-changelog.result so a failing changelog produces a non-green alert and
is listed in failed_jobs. Concretely, extend the status conditional to OR in
needs.pr-changelog.result == 'failure' (alongside the other three) and append a
needs.pr-changelog.result == 'failure' && 'PR Changelog' (or similar label) into
the failed_jobs concatenation so the changelog failure is reflected in the
message; update the status and failed_jobs fields in the same block where those
keys are defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0693d298-73f4-47b1-8fe1-0aa737b04485
📒 Files selected for processing (20)
.claude/commands/gha.md.claude/commands/workflow.md.github/labels.yml.github/workflows/pr-validation.ymldocs/pr-validation-workflow.mddocs/pr-validation.mdsrc/validate/pr-changelog/README.mdsrc/validate/pr-changelog/action.ymlsrc/validate/pr-description/README.mdsrc/validate/pr-description/action.ymlsrc/validate/pr-labels/README.mdsrc/validate/pr-labels/action.ymlsrc/validate/pr-metadata/README.mdsrc/validate/pr-metadata/action.ymlsrc/validate/pr-size/README.mdsrc/validate/pr-size/action.ymlsrc/validate/pr-source-branch/README.mdsrc/validate/pr-source-branch/action.ymlsrc/validate/pr-title/README.mdsrc/validate/pr-title/action.yml
💤 Files with no reviewable changes (1)
- docs/pr-validation-workflow.md
- Fix code-injection: move needs.*.result and inputs.dry_run to env vars
in pr-checks-summary job (use process.env instead of ${{ }} interpolation)
- Wire MANAGE_TOKEN into auto-labeler job (was hardcoded to github.token)
- Include pr-changelog in Slack notification status and failed_jobs
- Handle empty git diff output in pr-size (CHANGED_LINES defaults to 0)
- Support all * wildcard patterns in pr-source-branch (not just /*)
- Fix broken markdown links in docs (add -workflow suffix)
- Fix docs examples to use @v1.2.3 placeholder instead of @v1.x.x
- Update jobs table with non-draft condition for all gated jobs
bedatty
dismissed
coderabbitai[bot]’s stale review
All actionable comments addressed in commit a8aebf0. Remaining items are false positives or not applicable per repo conventions.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/validate/pr-source-branch/action.yml (1)
47-59:⚠️ Potential issue | 🟠 MajorThe matcher still rejects non-suffix
*patterns.Lines 49-55 only special-case
pattern.endsWith('*'). Patterns such as*-candidateorrelease-*/hotfixstill fall back to exact matching, so this does not actually support all*wildcard patterns.Suggested fix
- let isAllowed = false; - for (const pattern of allowedBranches) { - if (pattern.endsWith('*')) { - const prefix = pattern.slice(0, -1); - if (sourceBranch.startsWith(prefix)) { - isAllowed = true; - break; - } - } else if (pattern === sourceBranch) { - isAllowed = true; - break; - } - } + const toRegExp = (pattern) => + new RegExp( + '^' + + pattern + .split('*') + .map((segment) => segment.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + .join('.*') + + '$' + ); + + const isAllowed = allowedBranches.some((pattern) => + toRegExp(pattern).test(sourceBranch) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validate/pr-source-branch/action.yml` around lines 47 - 59, The current loop in the allowedBranches matcher only handles suffix wildcards via pattern.endsWith('*') (in the block that sets isAllowed), so patterns like "*-candidate" or "release-*/hotfix" fail; update the matching in the loop over allowedBranches to treat any pattern containing '*' as a wildcard pattern by converting the pattern into a proper regex (escape literal chars, replace '*' with '.*', anchor start/end as needed) and test sourceBranch against that regex instead of only checking startsWith; keep exact equality for patterns without '*' so the behavior of the existing equality branch (pattern === sourceBranch) remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 243-245: The Slack payload logic currently builds status and
failed_jobs using only pr-source-branch, pr-title, pr-description, and
pr-changelog; include pr-size, pr-labels, and pr-metadata in both the overall
status expression (the || conditions used to compute status) and the failed_jobs
concatenation so any failures there flip status to 'failure' and append their
names to failed_jobs; update the status expression that sets the status variable
and extend the failed_jobs construction to add ${{ needs.pr-size.result ==
'failure' && 'Size, ' || '' }}, ${{ needs.pr-labels.result == 'failure' &&
'Labels, ' || '' }}, and ${{ needs.pr-metadata.result == 'failure' && 'Metadata'
|| '' }} (or similar) so Slack accurately reflects failures for those jobs.
In `@docs/pr-validation.md`:
- Around line 44-47: The workflow triggers only on pull_request types [opened,
synchronize, reopened, ready_for_review] but must also run when PR metadata
changes; update the pull_request.types list to include edited, assigned,
unassigned, labeled, and unlabeled so validation actions (the pull_request
trigger in the workflow) run when title/description/assignees/labels change.
In `@src/validate/pr-size/action.yml`:
- Around line 65-75: The catch block around github.rest.issues.removeLabel is
currently swallowing all errors; change it to only ignore 404 (label not found)
and let other errors surface. In the catch for the loop that calls
github.rest.issues.removeLabel (labels, issue_number, owner/repo from context),
inspect the thrown error's status (e.g. error.status or error.statusCode) and if
it's 404 continue/ignore, otherwise rethrow or propagate the error so 403/5xx do
not get silently ignored.
---
Duplicate comments:
In `@src/validate/pr-source-branch/action.yml`:
- Around line 47-59: The current loop in the allowedBranches matcher only
handles suffix wildcards via pattern.endsWith('*') (in the block that sets
isAllowed), so patterns like "*-candidate" or "release-*/hotfix" fail; update
the matching in the loop over allowedBranches to treat any pattern containing
'*' as a wildcard pattern by converting the pattern into a proper regex (escape
literal chars, replace '*' with '.*', anchor start/end as needed) and test
sourceBranch against that regex instead of only checking startsWith; keep exact
equality for patterns without '*' so the behavior of the existing equality
branch (pattern === sourceBranch) remains intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb30cb75-67a5-4bdb-9df9-9274dfe33b7a
📒 Files selected for processing (4)
.github/workflows/pr-validation.ymldocs/pr-validation.mdsrc/validate/pr-size/action.ymlsrc/validate/pr-source-branch/action.yml
bedatty
dismissed
coderabbitai[bot]’s stale review
All comments addressed. 1 valid (Slack notification), 1 nice-to-have (removeLabel catch), 1 dismissed (docs event types). Fixes pending in next commit.
…branch refs for testing
|
This PR is very large (21 files, 1847 lines changed). Consider breaking it into smaller PRs for easier review. |
lerian-studio
added
documentation
.github/workflows/pr-validation.yml
Outdated
| id: check_branch | ||
| uses: actions/github-script@v8 | ||
| - name: Validate source branch | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Validate PR title format | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-title@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| - name: Comment on large PR | ||
| if: steps.size.outputs.size == 'XL' | ||
| uses: actions/github-script@v8 | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-size@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| - name: Check description length | ||
| uses: actions/github-script@v8 | ||
| - name: Validate PR description | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-description@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| steps: | ||
| - name: Check for assignee | ||
| uses: actions/github-script@v8 | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-labels@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| - name: Check for linked issues | ||
| uses: actions/github-script@v8 | ||
| - name: Check assignee and linked issues | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-metadata@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
.github/workflows/pr-validation.yml
Outdated
| - name: Summary | ||
| uses: actions/github-script@v8 | ||
| - name: PR Checks Summary | ||
| uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-checks-summary@refactor/modularize-pr-validation |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/pr-validation.yml (1)
219-221:⚠️ Potential issue | 🟠 MajorSlack status still undercounts failed jobs.
statusandfailed_jobsstill omitpr-size,pr-labels, andpr-metadata;pr-checks-summaryis also a direct dependency but never counted. A failure in any of those jobs will still sendsuccess.🔧 Suggested fix
- status: ${{ (needs.pr-source-branch.result == 'failure' || needs.pr-title.result == 'failure' || needs.pr-description.result == 'failure' || needs.pr-changelog.result == 'failure') && 'failure' || 'success' }} + status: ${{ (needs.pr-source-branch.result == 'failure' || needs.pr-title.result == 'failure' || needs.pr-size.result == 'failure' || needs.pr-description.result == 'failure' || needs.pr-labels.result == 'failure' || needs.pr-metadata.result == 'failure' || needs.pr-changelog.result == 'failure' || needs.pr-checks-summary.result == 'failure') && 'failure' || 'success' }}Extend
failed_jobswith the same missing job set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 219 - 221, Update the status and failed_jobs expressions so they include the missing jobs: pr-size, pr-labels, pr-metadata, and pr-checks-summary. Specifically modify the status assignment (variable name "status") to OR in checks for needs.pr-size.result, needs.pr-labels.result, needs.pr-metadata.result, and needs.pr-checks-summary.result alongside the existing checks, and extend the failed_jobs concatenation (variable name "failed_jobs") to append human-readable names for any of those jobs that failed (e.g., 'PR Size, ', 'PR Labels, ', 'PR Metadata, ', 'PR Checks Summary' using the same conditional pattern as the existing parts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 89-90: Replace the transient feature branch ref used in the
composite calls: locate all uses entries referencing
LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@refactor/modularize-pr-validation
(and the other occurrences of `@refactor/modularize-pr-validation`) and change the
ref to `@develop` so they read .../pr-source-branch@develop; update every instance
(the uses lines around the workflow steps) in the PR validation workflow to
ensure the internal composite points to the stable develop ref before merging.
---
Duplicate comments:
In @.github/workflows/pr-validation.yml:
- Around line 219-221: Update the status and failed_jobs expressions so they
include the missing jobs: pr-size, pr-labels, pr-metadata, and
pr-checks-summary. Specifically modify the status assignment (variable name
"status") to OR in checks for needs.pr-size.result, needs.pr-labels.result,
needs.pr-metadata.result, and needs.pr-checks-summary.result alongside the
existing checks, and extend the failed_jobs concatenation (variable name
"failed_jobs") to append human-readable names for any of those jobs that failed
(e.g., 'PR Size, ', 'PR Labels, ', 'PR Metadata, ', 'PR Checks Summary' using
the same conditional pattern as the existing parts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e537678c-eca9-4a1b-a535-ca0d59aaee15
📒 Files selected for processing (2)
.github/workflows/pr-validation.ymlsrc/validate/pr-checks-summary/action.yml
lerian-studio
added
size/XL
|
This PR is very large (23 files, 1899 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/slack-notify-workflow.md (1)
63-69:⚠️ Potential issue | 🔴 CriticalFix YAML syntax error:
secretsmust be at same level aswith.Line 69 incorrectly nests
secrets: inheritinside thewith:block. In GitHub Actions syntax,secretsis a top-level job key that must be at the same indentation level aswith:anduses:.🐛 Correct indentation
notify: uses: LerianStudio/github-actions-shared-workflows/.github/workflows/slack-notify.yml@v1.0.0 with: status: "success" workflow_name: "Release" custom_message: "Version v1.2.3 has been released! 🎉" - secrets: inherit + secrets: inherit🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/slack-notify-workflow.md` around lines 63 - 69, The notify job in the workflow incorrectly nests the secrets key inside the with block; move the secrets: inherit key out of the with: mapping so that secrets is a sibling of uses: and with: (i.e., at the same indentation level as uses and with) for the notify job to conform to GitHub Actions syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/slack-notify-workflow.md`:
- Around line 63-69: The notify job in the workflow incorrectly nests the
secrets key inside the with block; move the secrets: inherit key out of the
with: mapping so that secrets is a sibling of uses: and with: (i.e., at the same
indentation level as uses and with) for the notify job to conform to GitHub
Actions syntax.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8dff6b9f-dc56-4892-842f-bf7381da4d39
📒 Files selected for processing (2)
docs/slack-notify-workflow.mdsrc/validate/pr-checks-summary/README.md
Consolidate 9 parallel jobs into 4 with a 2-tier architecture: - Tier 1 (blocking-checks): title, source-branch, description — no checkout, fail-fast - Tier 2 (advisory-checks): metadata, size, labels, changelog — shared checkout, only runs if Tier 1 passes Reduces runner cost (9 → 4 runners, 3 checkouts → 1) while providing faster feedback on blocking validation failures.
lerian-studio
added
size/XL
|
This PR is very large (23 files, 2008 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 124-145: The collect step (id: collect) currently writes
SOURCE_BRANCH, TITLE, DESCRIPTION to GITHUB_OUTPUT but does not emit them when
dry_run is enabled; update the shell block in the collect step to check the
dry_run input/variable and, when true, print verbose ::notice:: lines for each
resolved value (SOURCE_BRANCH, TITLE, DESCRIPTION and any computed FAILED) so
callers can preview results without side effects; apply the same change to the
other analogous block (the second validation-results emission) so both places
log every resolved input and computed variable under dry_run.
- Around line 89-92: Add workflow-level boolean outputs following the
"has_<noun>" convention so callers can branch on this reusable workflow: add
outputs such as has_source_branch, has_title, has_description (string values
'true' or 'false') derived from the existing step outputs
(steps.collect.outputs.source_branch, steps.collect.outputs.title,
steps.collect.outputs.description) and likewise add corresponding has_* outputs
for the other group referenced around the later block (the outputs derived at
the second location, e.g., where steps.collect.outputs.* are used at the 154-158
region). Ensure the values are set to the string 'true' when the corresponding
source value is present/non-empty and 'false' otherwise so workflow_call.outputs
can be consumed by callers.
In `@src/validate/pr-description/action.yml`:
- Around line 38-47: The current logic treats a missing "Type of Change" section
as a warning (warnings.push) so the check can pass; change that to be blocking
by pushing an error instead: when typeMatch is falsy, call errors.push(...) with
a clear message (same or updated text) rather than warnings.push(...). Update
the block that references typeMatch/typeSection/checked to ensure both the
unchecked-case and the missing-section-case add to errors so the PR validation
fails when the "Type of Change" section is absent or has no checked boxes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 32dcca62-b58d-4478-8597-c86e617e0aac
📒 Files selected for processing (4)
.github/workflows/pr-validation.ymlsrc/validate/pr-description/README.mdsrc/validate/pr-description/action.ymlsrc/validate/pr-title/action.yml
Replace the warning-only assignee and linked issues checks with an actionable auto-assign: if no assignee is set, assign the PR author automatically. Bot accounts are skipped.
lerian-studio
added
size/XL
|
This PR is very large (23 files, 1983 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/validate/pr-metadata/action.yml`:
- Around line 17-22: The code assumes pr.assignees is always present; update the
check to be null-safe by verifying assignees is an array before accessing length
(e.g., use Array.isArray(pr.assignees) && pr.assignees.length > 0 or
pr.assignees?.length > 0) so that the branch safely handles undefined/null
webhook payloads; change the condition around the variable pr (from
context.payload.pull_request) accordingly and keep the existing logging and
return behavior when assignees exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26af9b1d-4462-4477-8806-038e83f1dc93
📒 Files selected for processing (2)
src/validate/pr-metadata/README.mdsrc/validate/pr-metadata/action.yml
- Check current labels before removing/adding — skip entirely if the correct size label is already set - Only remove stale size labels that actually exist on the PR - Remove the XL comment (generic noise on every sync)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/validate/pr-size/action.yml (1)
72-82: 🧹 Nitpick | 🔵 TrivialAdd try-catch to handle race-condition 404s on removeLabel.
The check-before-remove pattern reduces 404 likelihood, but a race condition (label removed between payload fetch and API call) would still throw and fail the step before
addLabelsexecutes. Wrap theremoveLabelcall in a try-catch that ignores 404:Suggested fix
for (const label of allSizeLabels) { if (currentLabels.includes(label)) { console.log(`Removing stale label '${label}'`); + try { await github.rest.issues.removeLabel({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, name: label }); + } catch (error) { + if (error.status !== 404) throw error; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validate/pr-size/action.yml` around lines 72 - 82, Wrap the await github.rest.issues.removeLabel(...) call inside a try-catch so transient race-condition 404s are ignored and do not abort the step; inside the catch check the error status (err.status or err?.status) and if it equals 404 simply continue, otherwise rethrow or log and throw so real errors surface; apply this change around the loop that iterates allSizeLabels/currentLabels so addLabels still runs afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/validate/pr-size/action.yml`:
- Around line 72-82: Wrap the await github.rest.issues.removeLabel(...) call
inside a try-catch so transient race-condition 404s are ignored and do not abort
the step; inside the catch check the error status (err.status or err?.status)
and if it equals 404 simply continue, otherwise rethrow or log and throw so real
errors surface; apply this change around the loop that iterates
allSizeLabels/currentLabels so addLabels still runs afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1c1352e-fed7-4961-8f3e-f4de4defb459
📒 Files selected for processing (1)
src/validate/pr-size/action.yml
…y SHA - Remove pr-changelog from workflow, summary, and inputs — CHANGELOG.md is auto-generated by semantic-release - Pin actions/github-script@v8 and actions/checkout@v6 to commit SHAs across all validate composites
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/validate/pr-labels/action.yml`:
- Line 17: The YAML lint error is due to insufficient spacing before the inline
comment on the uses line; in src/validate/pr-labels/action.yml locate the line
containing "uses: actions/labeler@634933edcd8ababfe52f92936142cc22ac488b1b" and
insert two spaces immediately before the "#" comment delimiter so the inline
comment is preceded by exactly two spaces (e.g., ensure " # v6") to satisfy
yamllint's "too few spaces before comment" rule.
- Around line 17-20: The composite action step that uses
actions/labeler@634933edcd8ababfe52f92936142cc22ac488b1b (the "uses" entry in
src/validate/pr-labels/action.yml) is incompatible with your current
.github/labeler.yml v5 schema; fix by either updating .github/labeler.yml to the
v6 schema (convert any-glob-to-any-file rules into nested any/all conditions per
actions/labeler v6 docs) or change the "uses: actions/labeler@..." to a
v5-compatible tag/commit (pin to the official v5 release) so the existing
labeler configuration is recognized. Ensure the configuration-path input remains
${ inputs.config-path } and test on a sample PR to confirm labels apply.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b4283cb-f464-4ac0-b7fb-57f72bb49e37
📒 Files selected for processing (1)
src/validate/pr-labels/action.yml
Display results as two tables (Blocking / Advisory) instead of flat lines. Skipped checks now use ⏭️ instead of⚠️ for clarity.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (6)
src/validate/pr-size/action.yml (1)
71-82:⚠️ Potential issue | 🟡 MinorFilter
removeLabelerrors to only ignore 404.The catch block silently ignores all errors including 403 (permission) and 5xx (server). Only 404 (label not attached) should be ignored.
} catch (error) { - // Label doesn't exist, ignore + if (error.status !== 404) { + throw error; + } }Per the past review conversation, this was acknowledged as a valid finding to apply.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validate/pr-size/action.yml` around lines 71 - 82, The removal loop over allSizeLabels uses github.rest.issues.removeLabel and currently swallows all errors; change the error handling around the await github.rest.issues.removeLabel call so that you catch errors, inspect error.status (or error.statusCode) and only ignore HTTP 404 (label not attached), while rethrowing or logging any other status (e.g., 403, 5xx); update the try/catch around the loop or each remove call (referencing allSizeLabels, currentLabels, github.rest.issues.removeLabel and context) to implement this filtered-ignore behavior.src/validate/pr-metadata/action.yml (1)
19-19: 🧹 Nitpick | 🔵 TrivialAdd null-safety for
pr.assignees.If the webhook payload is malformed,
pr.assigneescould beundefined, causingpr.assignees.lengthto throw.- if (pr.assignees.length > 0) { + if (pr.assignees?.length > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validate/pr-metadata/action.yml` at line 19, Ensure access to pr.assignees is null-safe: replace direct use of pr.assignees.length with a guard such as Array.isArray(pr.assignees) && pr.assignees.length > 0 or use optional chaining like (pr.assignees?.length || 0) > 0 so malformed webhook payloads (undefined assignees) do not throw; update the conditional that currently reads "if (pr.assignees.length > 0)" accordingly.docs/pr-validation.md (2)
196-200:⚠️ Potential issue | 🟡 MinorUpdate stale related-workflow links.
These links use the old
*-workflow.mdpattern. Per past review, they should be updated to match current doc naming.-- [Go CI](./go-ci-workflow.md) — Continuous integration testing -- [Go Security](./go-security-workflow.md) — Security scanning -- [PR Security Scan](./pr-security-scan-workflow.md) — Security scanning for PRs +- [Go CI](./go-ci.md) — Continuous integration testing +- [Go Security](./go-security.md) — Security scanning +- [PR Security Scan](./pr-security-scan.md) — Security scanning for PRs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pr-validation.md` around lines 196 - 200, Update the three stale related-workflow links that use the old "*-workflow.md" pattern—replace "./go-ci-workflow.md", "./go-security-workflow.md", and "./pr-security-scan-workflow.md" with the current doc filenames (follow the repo's new naming convention) while keeping the link text ("Go CI", "Go Security", "PR Security Scan") intact so the markdown under "Related Workflows" points to the updated documents.
26-26: 🧹 Nitpick | 🔵 TrivialAdd language specifiers to fenced code blocks.
Lines 26, 132, and 188 have fenced code blocks without language specifiers (MD040).
-``` +```text pr-validation.yml (reusable workflow)Also applies to: 132-132, 188-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pr-validation.md` at line 26, Update the fenced code blocks that lack language specifiers by adding an explicit language tag (e.g., ```text or ```yaml) to each block; specifically modify the blocks containing "pr-validation.yml (reusable workflow)" and the other two similar fenced blocks so they include the appropriate language specifier (use text for plain text snippets or yaml for YAML content) to satisfy MD040.src/validate/pr-description/action.yml (1)
45-47:⚠️ Potential issue | 🟠 MajorMissing "Type of Change" section treated as warning, not error.
This was flagged in a previous review. The
elsebranch (line 46) pushes towarnings, noterrors, so a PR missing the entire "Type of Change" section passes validation. If this is a Tier 1 blocking check, missing required sections should fail.} else { - warnings.push('Missing "Type of Change" section.'); + errors.push('Missing "Type of Change" section. Please add the section and check at least one option.'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/validate/pr-description/action.yml` around lines 45 - 47, The "Type of Change" missing branch currently pushes to warnings (warnings.push('Missing \"Type of Change\" section.')) which lets PRs pass; change that branch to push to errors instead (errors.push(...)) so a missing "Type of Change" section triggers a failing validation; update any related message text if needed and ensure downstream logic that checks the errors array (errors, warnings) treats this as a blocking Tier 1 check..github/workflows/pr-validation.yml (1)
96-97:⚠️ Potential issue | 🟠 MajorReplace feature-branch refs with
@developbefore merge.All composite refs still use
@refactor/modularize-pr-validation. This branch will be deleted after merge, breaking the workflow.This was flagged in a previous review - ensure refs are updated to
@developin the final commit.Also applies to: 106-106, 116-116, 157-157, 164-164, 171-171, 181-181, 204-204, 219-219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 96 - 97, Replace the hard-coded feature branch ref used in composite action refs with the stable develop ref: find occurrences of "LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@refactor/modularize-pr-validation" (and other composite refs that end with "@refactor/modularize-pr-validation") and change them to use "@develop" instead; ensure you update every instance flagged (including the repeats at the other occurrences) so no composite action points to the soon-to-be-deleted feature branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 9-10: The workflow currently only exposes the workflow_call
trigger; add a workflow_dispatch trigger alongside workflow_call so the reusable
workflow can be manually triggered as required by guidelines—update the
top-level triggers to include both workflow_call and workflow_dispatch (i.e.,
add a workflow_dispatch key next to workflow_call in the workflow definition).
- Around line 161-166: The Check PR metadata step (id: metadata) calls the
pr-metadata composite which performs writes (e.g.,
github.rest.issues.addAssignees) but currently runs during dry-run; either gate
the step with the same condition used by pr-labels (add an if: !inputs.dry_run
to the step) or modify the pr-metadata composite
(LerianStudio/github-actions-shared-workflows/src/validate/pr-metadata) to
accept a dry-run input and early-return when dry-run is enabled; update the
workflow step to pass that dry-run input into the composite if you choose the
latter.
- Around line 122-131: Replace direct interpolation of steps.*.outcome inside
the shell run by mapping those outcomes into environment variables and then
reading the env vars inside the script: define env: SOURCE_BRANCH: ${{
steps.source-branch.outcome }}, TITLE: ${{ steps.title.outcome }}, DESCRIPTION:
${{ steps.description.outcome }} on the job or step and then in the run block
simply echo "$SOURCE_BRANCH" "$TITLE" "$DESCRIPTION" (no steps.* interpolation).
Apply the same change for the advisory-checks outputs (the corresponding step
names used there) so all usages use env variables instead of direct
steps.*.outcome shell interpolation.
In `@docs/pr-validation.md`:
- Around line 145-150: The jobs table still lists the removed composite
`pr-changelog` under the `advisory-checks` composites; update the documentation
to remove `pr-changelog` from the `advisory-checks` row so the composites column
matches current workflow (e.g., list only `pr-metadata`, `pr-size`, `pr-labels`
for `advisory-checks`) and ensure any surrounding text referencing
`pr-changelog` is also deleted or updated to reference existing composites.
- Around line 83-88: Remove the stale check_changelog input from the example
block so it matches the current workflow interface; locate the example
containing require_scope, min_description_length and enable_auto_labeler and
delete the check_changelog line (or replace it with the current valid input if
there is a new equivalent) so the example no longer references the removed
check_changelog parameter.
In `@src/validate/pr-checks-summary/action.yml`:
- Around line 4-32: The README still documents and uses the removed input
"changelog-result" causing a mismatch with action.yml; update the README to
remove all references to the "changelog-result" input (both the input table
entry and any usage/example blocks that pass or mention changelog-result) so it
matches the inputs declared in action.yml (e.g., source-branch-result,
title-result, size-result, description-result, label-result, metadata-result,
dry-run).
In `@src/validate/pr-metadata/action.yml`:
- Around line 1-39: The composite action currently calls
github.rest.issues.addAssignees (in the script block that references pr and
author) without honoring a dry-run mode; add a boolean input named dry-run to
the action inputs and then gate the write call by checking that input before
invoking github.rest.issues.addAssignees (or alternatively document/ensure the
workflow step uses if: !inputs.dry_run). Specifically, add an inputs: dry-run
entry in action.yml, expose it as an env var or process.env.DRY_RUN inside the
script, and wrap the call to github.rest.issues.addAssignees so it only runs
when dry-run is false.
---
Duplicate comments:
In @.github/workflows/pr-validation.yml:
- Around line 96-97: Replace the hard-coded feature branch ref used in composite
action refs with the stable develop ref: find occurrences of
"LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@refactor/modularize-pr-validation"
(and other composite refs that end with "@refactor/modularize-pr-validation")
and change them to use "@develop" instead; ensure you update every instance
flagged (including the repeats at the other occurrences) so no composite action
points to the soon-to-be-deleted feature branch.
In `@docs/pr-validation.md`:
- Around line 196-200: Update the three stale related-workflow links that use
the old "*-workflow.md" pattern—replace "./go-ci-workflow.md",
"./go-security-workflow.md", and "./pr-security-scan-workflow.md" with the
current doc filenames (follow the repo's new naming convention) while keeping
the link text ("Go CI", "Go Security", "PR Security Scan") intact so the
markdown under "Related Workflows" points to the updated documents.
- Line 26: Update the fenced code blocks that lack language specifiers by adding
an explicit language tag (e.g., ```text or ```yaml) to each block; specifically
modify the blocks containing "pr-validation.yml (reusable workflow)" and the
other two similar fenced blocks so they include the appropriate language
specifier (use text for plain text snippets or yaml for YAML content) to satisfy
MD040.
In `@src/validate/pr-description/action.yml`:
- Around line 45-47: The "Type of Change" missing branch currently pushes to
warnings (warnings.push('Missing \"Type of Change\" section.')) which lets PRs
pass; change that branch to push to errors instead (errors.push(...)) so a
missing "Type of Change" section triggers a failing validation; update any
related message text if needed and ensure downstream logic that checks the
errors array (errors, warnings) treats this as a blocking Tier 1 check.
In `@src/validate/pr-metadata/action.yml`:
- Line 19: Ensure access to pr.assignees is null-safe: replace direct use of
pr.assignees.length with a guard such as Array.isArray(pr.assignees) &&
pr.assignees.length > 0 or use optional chaining like (pr.assignees?.length ||
0) > 0 so malformed webhook payloads (undefined assignees) do not throw; update
the conditional that currently reads "if (pr.assignees.length > 0)" accordingly.
In `@src/validate/pr-size/action.yml`:
- Around line 71-82: The removal loop over allSizeLabels uses
github.rest.issues.removeLabel and currently swallows all errors; change the
error handling around the await github.rest.issues.removeLabel call so that you
catch errors, inspect error.status (or error.statusCode) and only ignore HTTP
404 (label not attached), while rethrowing or logging any other status (e.g.,
403, 5xx); update the try/catch around the loop or each remove call (referencing
allSizeLabels, currentLabels, github.rest.issues.removeLabel and context) to
implement this filtered-ignore behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7dc4b71e-845e-4621-91b5-cc28cdd22978
📒 Files selected for processing (7)
.github/workflows/pr-validation.ymldocs/pr-validation.mdsrc/validate/pr-checks-summary/action.ymlsrc/validate/pr-description/action.ymlsrc/validate/pr-metadata/action.ymlsrc/validate/pr-size/action.ymlsrc/validate/pr-source-branch/action.yml
- Remove stale check_changelog references from docs and examples - Remove pr-changelog from jobs table and pr-checks-summary README - Fix related-workflow links to current doc naming - Make missing "Type of Change" section an error, not a warning - Add null-safety for pr.assignees in pr-metadata - Add dry-run gate to pr-metadata auto-assign - Fix yamllint inline-comment spacing in pr-labels
bedatty
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
All comments addressed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/pr-validation.yml (2)
44-47:⚠️ Potential issue | 🟠 MajorWorkflow and composite defaults for description length are out of sync.
Line 118 always forwards
inputs.min_description_length, and the workflow input still defaults to50. That means callers ofpr-validation.ymlnever see the new30-character default fromsrc/validate/pr-description/action.yml; the reusable workflow and the composite now enforce different thresholds by default.Minimal fix
min_description_length: description: 'Minimum PR description length' type: number - default: 50 + default: 30Also applies to: 113-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 44 - 47, The workflow input min_description_length in pr-validation.yml is out of sync with the composite/action default (50 vs 30); update the default to match the composite by changing the input default from 50 to 30 (the input key is inputs.min_description_length) so callers of the reusable workflow inherit the same 30-character default as defined in src/validate/pr-description/action.yml, or alternatively stop hard-coding the default and forward the value transparently by passing inputs.min_description_length through without overriding it.
10-72:⚠️ Potential issue | 🟠 MajorThis interface change is still a breaking change for existing callers.
.github/workflows/self-pr-validation.yml:20-33still passescheck_changelog: false. After this lands, that caller will fail reusable-workflow input validation before any jobs start. Either keep a deprecated no-op input for one release or update the caller in the same PR.As per coding guidelines, `.github/workflows/*.yml`: "Flag any removed or renamed input/output as a breaking change for callers."Compatibility shim
+ check_changelog: + description: 'Deprecated. No-op; changelog validation was removed.' + type: boolean + default: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 10 - 72, The new workflow removed the check_changelog input which breaks callers (e.g., .github/workflows/self-pr-validation.yml) that still pass check_changelog: false; either restore a deprecated no-op input named check_changelog in the reusable workflow inputs (in pr-validation.yml) with the same type and a default of false so existing callers validate, or update the caller workflow (.github/workflows/self-pr-validation.yml) in the same PR to stop passing check_changelog; reference the input name check_changelog and the caller self-pr-validation.yml when making the change.
♻️ Duplicate comments (2)
.github/workflows/pr-validation.yml (2)
96-96:⚠️ Potential issue | 🟠 MajorReplace the feature-branch refs before merging.
These nested
uses:entries still point at@refactor/modularize-pr-validation. Once that branch is deleted, callers ofpr-validation.ymlfromdevelopwill fail at runtime because the reusable workflow resolves its internal composites/workflows against a ref that no longer exists.Based on learnings, workflows in this repository must reference shared composites/workflows via absolute refs such as `LerianStudio/github-actions-shared-workflows/...@develop`; feature-branch refs are temporary.Minimal fix
- uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@refactor/modularize-pr-validation + uses: LerianStudio/github-actions-shared-workflows/src/validate/pr-source-branch@developApply the same replacement to the other internal
uses:entries in this file.Also applies to: 106-106, 116-116, 164-164, 172-172, 182-182, 205-205, 220-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml at line 96, Replace all nested reusable-workflow and composite action refs that point to the feature branch suffix "@refactor/modularize-pr-validation" with the permanent branch ref (e.g., "@develop") so the internal "uses:" entries resolve after the feature branch is deleted; search for occurrences of "LerianStudio/github-actions-shared-workflows/...@refactor/modularize-pr-validation" (the nested uses lines such as the one referencing src/validate/pr-source-branch and any other internal "uses:" entries listed in the comment) and update each to use the stable ref (for example change "@refactor/modularize-pr-validation" → "@develop") consistently across the workflow.
207-224:⚠️ Potential issue | 🟠 MajorAdvisory-tier failures are still masked as
skipped/success.If
advisory-checksfails beforesteps.collectruns, Lines 210-212 degrade the missing outputs toskipped. Even when the job completes, Lines 222-224 ignoremetadata-result,size-result, andlabel-result, so Slack can still reportsuccesswhile Tier 2 surfaced failures.Minimal fix pattern
- size-result: ${{ needs.advisory-checks.outputs.size-result || 'skipped' }} + size-result: ${{ needs.advisory-checks.result == 'failure' && 'failure' || needs.advisory-checks.result == 'skipped' && 'skipped' || needs.advisory-checks.outputs.size-result || 'skipped' }} - status: ${{ (needs.blocking-checks.outputs.source-branch-result == 'failure' || needs.blocking-checks.outputs.title-result == 'failure' || needs.blocking-checks.outputs.description-result == 'failure') && 'failure' || 'success' }} + status: ${{ (needs.blocking-checks.outputs.source-branch-result == 'failure' || needs.blocking-checks.outputs.title-result == 'failure' || needs.blocking-checks.outputs.description-result == 'failure' || needs.advisory-checks.result == 'failure' || needs.advisory-checks.outputs.metadata-result == 'failure' || needs.advisory-checks.outputs.size-result == 'failure' || needs.advisory-checks.outputs.label-result == 'failure') && 'failure' || 'success' }}Apply the same job-result-aware fallback to
label-result,metadata-result, andfailed_jobs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-validation.yml around lines 207 - 224, The notify step currently treats advisory-checks outputs (label-result, size-result, metadata-result) as 'skipped' when the job failed before producing outputs, which can hide advisory-tier failures; update the expressions in the notify job (status and failed_jobs inputs) to use job-result-aware fallbacks like you already do for blocking-checks (e.g., check needs.advisory-checks.result or compare needs.advisory-checks.outputs.label-result == 'failure' instead of defaulting to 'skipped') so that label-result, size-result, and metadata-result are evaluated for 'failure' and included in the status calculation and concatenated into failed_jobs when they failed. Ensure you reference the same symbols used in the diff: needs.advisory-checks.outputs.label-result, needs.advisory-checks.outputs.size-result, needs.advisory-checks.outputs.metadata-result, and the notify job inputs status and failed_jobs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/pr-validation.md`:
- Line 16: Update the docs to reflect the new composite behavior: replace the
bullet "Metadata checks — Warns if no assignee or linked issues" and any related
architecture text to state that the PR metadata action (see
src/validate/pr-metadata/action.yml) will automatically assign the PR author
when no assignee exists and no longer inspects linked issues; adjust feature
list and architecture narrative accordingly so they no longer claim a
warnings-only check or issue-link inspection.
In `@src/validate/pr-description/action.yml`:
- Around line 19-20: The code parses MIN_LENGTH into minLength
(parseInt(process.env.MIN_LENGTH, 10)) but doesn't validate it, so when
minLength is NaN the subsequent check (e.g., comparing body.length to minLength)
silently allows short descriptions; fix this by validating minLength immediately
after parsing: if isNaN(minLength) or minLength <= 0, fail closed—use the action
runner's failure mechanism (throw an Error or call core.setFailed) with a clear
message referencing process.env.MIN_LENGTH and minLength so the action exits and
reports the invalid input before evaluating context.payload.pull_request.body.
---
Outside diff comments:
In @.github/workflows/pr-validation.yml:
- Around line 44-47: The workflow input min_description_length in
pr-validation.yml is out of sync with the composite/action default (50 vs 30);
update the default to match the composite by changing the input default from 50
to 30 (the input key is inputs.min_description_length) so callers of the
reusable workflow inherit the same 30-character default as defined in
src/validate/pr-description/action.yml, or alternatively stop hard-coding the
default and forward the value transparently by passing
inputs.min_description_length through without overriding it.
- Around line 10-72: The new workflow removed the check_changelog input which
breaks callers (e.g., .github/workflows/self-pr-validation.yml) that still pass
check_changelog: false; either restore a deprecated no-op input named
check_changelog in the reusable workflow inputs (in pr-validation.yml) with the
same type and a default of false so existing callers validate, or update the
caller workflow (.github/workflows/self-pr-validation.yml) in the same PR to
stop passing check_changelog; reference the input name check_changelog and the
caller self-pr-validation.yml when making the change.
---
Duplicate comments:
In @.github/workflows/pr-validation.yml:
- Line 96: Replace all nested reusable-workflow and composite action refs that
point to the feature branch suffix "@refactor/modularize-pr-validation" with the
permanent branch ref (e.g., "@develop") so the internal "uses:" entries resolve
after the feature branch is deleted; search for occurrences of
"LerianStudio/github-actions-shared-workflows/...@refactor/modularize-pr-validation"
(the nested uses lines such as the one referencing src/validate/pr-source-branch
and any other internal "uses:" entries listed in the comment) and update each to
use the stable ref (for example change "@refactor/modularize-pr-validation" →
"@develop") consistently across the workflow.
- Around line 207-224: The notify step currently treats advisory-checks outputs
(label-result, size-result, metadata-result) as 'skipped' when the job failed
before producing outputs, which can hide advisory-tier failures; update the
expressions in the notify job (status and failed_jobs inputs) to use
job-result-aware fallbacks like you already do for blocking-checks (e.g., check
needs.advisory-checks.result or compare
needs.advisory-checks.outputs.label-result == 'failure' instead of defaulting to
'skipped') so that label-result, size-result, and metadata-result are evaluated
for 'failure' and included in the status calculation and concatenated into
failed_jobs when they failed. Ensure you reference the same symbols used in the
diff: needs.advisory-checks.outputs.label-result,
needs.advisory-checks.outputs.size-result,
needs.advisory-checks.outputs.metadata-result, and the notify job inputs status
and failed_jobs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b6c427c-de86-437e-93fd-75b53f048b0a
📒 Files selected for processing (6)
.github/workflows/pr-validation.ymldocs/pr-validation.mdsrc/validate/pr-checks-summary/README.mdsrc/validate/pr-description/action.ymlsrc/validate/pr-labels/action.ymlsrc/validate/pr-metadata/action.yml
- Align min_description_length default to 30 (matches composite) - Remove stale check_changelog from self-pr-validation.yml - Update metadata feature description in docs - Validate min-length input against NaN in pr-description
bedatty
commented
Mar 25, 2026
bedatty
dismissed
coderabbitai[bot]’s stale review
All actionable comments addressed in commit 865936d. Branch ref replacement deferred to merge time. Advisory-tier failures are informational by design.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/pr-validation.md (1)
34-36:⚠️ Potential issue | 🟡 MinorUpdate the
pr-metadataarchitecture label.This still documents
pr-metadataas checking linked issues. The implementation now auto-assigns the PR author when no assignee exists and no longer inspects linked issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pr-validation.md` around lines 34 - 36, Update the pr-metadata architecture label to reflect current behavior: change the description for src/validate/pr-metadata to state that it auto-assigns the PR author when no assignee exists and no longer inspects linked issues, removing any mention of "linked issues" and instead documenting the auto-assignment behavior (reference: src/validate/pr-metadata).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/pr-validation.md`:
- Around line 121-123: The inputs table is out of sync: update the
`min_description_length` default from `50` to `30` in the table (the workflow
default is 30) and add a note that the length is measured after stripping
template boilerplate; ensure the row for `min_description_length` alongside
`require_scope` and `enable_auto_labeler` reflects this corrected default and
measurement behavior.
In `@src/validate/pr-description/action.yml`:
- Around line 19-20: The code dereferences context.payload.pull_request.body
directly which throws outside PR events; modify the entry logic that reads
context.payload.pull_request.body (and the MIN_LENGTH parse) to first check that
context.payload and context.payload.pull_request exist, and if not call
core.setFailed (or throw a clear Error) with an actionable message like "This
action must be run on pull_request events" so callers get a friendly failure;
keep the existing MIN_LENGTH parse but validate it (fallback to a sensible
default or fail with a clear message if NaN) and update the code paths that use
body and minLength (the variables assigned from
context.payload.pull_request.body and process.env.MIN_LENGTH) accordingly.
---
Duplicate comments:
In `@docs/pr-validation.md`:
- Around line 34-36: Update the pr-metadata architecture label to reflect
current behavior: change the description for src/validate/pr-metadata to state
that it auto-assigns the PR author when no assignee exists and no longer
inspects linked issues, removing any mention of "linked issues" and instead
documenting the auto-assignment behavior (reference: src/validate/pr-metadata).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f8beb15-5586-47f0-80ab-49a86147803b
📒 Files selected for processing (4)
.github/workflows/pr-validation.yml.github/workflows/self-pr-validation.ymldocs/pr-validation.mdsrc/validate/pr-description/action.yml
💤 Files with no reviewable changes (1)
- .github/workflows/self-pr-validation.yml
Comment on lines
+121
to
+123
| | `require_scope` | boolean | `false` | Require scope in PR title | | ||
| | `min_description_length` | number | `50` | Minimum PR description length | | ||
| | `enable_auto_labeler` | boolean | `true` | Enable automatic labeling | |
There was a problem hiding this comment.
The inputs table still advertises the old min_description_length default.
The workflow default is 30, not 50, and the length is measured after stripping template boilerplate. Leaving the old value here documents the wrong contract for callers.
Doc fix
-| `min_description_length` | number | `50` | Minimum PR description length |
+| `min_description_length` | number | `30` | Minimum PR description content length after stripping template boilerplate |As per coding guidelines, "Flag if inputs table is out of sync with the corresponding workflow changes in this PR."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `require_scope` | boolean | `false` | Require scope in PR title | | |
| | `min_description_length` | number | `50` | Minimum PR description length | | |
| | `enable_auto_labeler` | boolean | `true` | Enable automatic labeling | | |
| | `require_scope` | boolean | `false` | Require scope in PR title | | |
| | `min_description_length` | number | `30` | Minimum PR description content length after stripping template boilerplate | | |
| | `enable_auto_labeler` | boolean | `true` | Enable automatic labeling | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/pr-validation.md` around lines 121 - 123, The inputs table is out of
sync: update the `min_description_length` default from `50` to `30` in the table
(the workflow default is 30) and add a note that the length is measured after
stripping template boilerplate; ensure the row for `min_description_length`
alongside `require_scope` and `enable_auto_labeler` reflects this corrected
default and measurement behavior.
Comment on lines
+19
to
+20
| const body = context.payload.pull_request.body || ''; | ||
| const minLength = parseInt(process.env.MIN_LENGTH, 10); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/validate/pr-description/action.yml | head -30Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1431
🏁 Script executed:
ls -la src/validate/pr-description/Repository: LerianStudio/github-actions-shared-workflows
Length of output: 321
🏁 Script executed:
find src -name "action.yml" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -25 {}'Repository: LerianStudio/github-actions-shared-workflows
Length of output: 4585
🏁 Script executed:
cat -n src/validate/pr-description/README.mdRepository: LerianStudio/github-actions-shared-workflows
Length of output: 1392
🏁 Script executed:
grep -r "context.payload" src --include="*.yml" -B 2 -A 2 | head -50Repository: LerianStudio/github-actions-shared-workflows
Length of output: 1936
🏁 Script executed:
grep -r "pull_request" .github/workflows --include="*.yml" | grep "pr-description" | head -5Repository: LerianStudio/github-actions-shared-workflows
Length of output: 70
Guard the PR payload before dereferencing it.
context.payload.pull_request.body throws a TypeError when this composite runs outside a pull_request context. The README does not document context requirements, but callers may inadvertently trigger this action from non-PR workflows (e.g., workflow_run, schedule). Fail with an actionable message instead of surfacing a raw error.
Minimal hardening
- const body = context.payload.pull_request.body || '';
+ const pullRequest = context.payload.pull_request;
+ if (!pullRequest) {
+ core.setFailed('Validate PR Description must run on a pull_request event.');
+ return;
+ }
+
+ const body = pullRequest.body || '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/validate/pr-description/action.yml` around lines 19 - 20, The code
dereferences context.payload.pull_request.body directly which throws outside PR
events; modify the entry logic that reads context.payload.pull_request.body (and
the MIN_LENGTH parse) to first check that context.payload and
context.payload.pull_request exist, and if not call core.setFailed (or throw a
clear Error) with an actionable message like "This action must be run on
pull_request events" so callers get a friendly failure; keep the existing
MIN_LENGTH parse but validate it (fallback to a sensible default or fail with a
clear message if NaN) and update the code paths that use body and minLength (the
variables assigned from context.payload.pull_request.body and
process.env.MIN_LENGTH) accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Modularize the monolithic
pr-validation.ymlinto independent composite actions undersrc/validate/, then optimize the execution model with a 2-tier fail-fast architecture that reduces runner cost and provides faster feedback.Architecture: 2-tier fail-fast (9 jobs → 4)
Cost: 9 runners → 4, 3 checkouts → 1.
Composite improvements
pr-description## Description(strips HTML comments/boilerplate), checks at least one "Type of Change" checkbox is marked. Removedgithub-tokeninput.pr-metadatapr-sizesize/*label already exists. Removed noisy XL comment.pr-changelogpr-checks-summaryenforce_source_branchestrue— composite auto-skips when target != main.Security
All third-party actions pinned by commit SHA:
actions/github-script@v8,actions/checkout@v6,actions/labeler@v6,amannn/action-semantic-pull-request@v6Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)Breaking Changes
check_changeloginput removed — callers passing this input will get a warning but won't breakenforce_source_branchesdefault changed fromfalsetotrue— the composite auto-skips when the target branch is notmain, so this is safe for all existing callerspr-descriptionno longer acceptsgithub-token— callers passing it via the workflow are unaffected (the workflow handles this)min_description_lengthdefault lowered from50to30(now counts real content, not boilerplate)Testing
@developor the beta tagCaller repo / workflow run:
Related Issues
Summary by CodeRabbit