feat(ci): add YAML and GitHub Actions lint analysis for PRs#148
Open
feat(ci): add YAML and GitHub Actions lint analysis for PRs#148
Conversation
Add self-pr-analysis workflow that runs yamllint and actionlint on pull requests to validate YAML syntax and GitHub Actions workflow correctness.
WalkthroughAdds Dependabot lint group and a new lint label; introduces YAML/actionlint/markdown-link-check configs; many new composite lint actions and their READMEs; a changed-files detector; a PR lint reporter; and a self PR‑validation workflow wiring these components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.yamllint.yml:
- Around line 4-5: The yamllint configuration uses a glob "yaml-files: -
\"*.yml\"" that only matches root-level YAML files; change the pattern to a
recursive glob by replacing the value "*.yml" with "**/*.yml" so
README/workflows and composite actions (e.g., files under .github/workflows/ and
src/) are included in linting; update the "yaml-files" entry in .yamllint.yml
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64c7aa2f-68e7-4587-b8fc-09ebd2080306
📒 Files selected for processing (4)
.github/dependabot.yml.github/labels.yml.github/workflows/self-pr-analysis.yml.yamllint.yml
Filter both linters to only check files modified in the PR instead of scanning the entire repository. Falls back to full scan on workflow_dispatch.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/self-pr-analysis.yml:
- Around line 30-36: The PR diff may include deleted/renamed paths, so filter
the list from gh pr diff --name-only to only those that exist in the checked-out
workspace before assigning to the files output; update both places where you
build the files variable (the branches that set files=$(gh pr diff --name-only
...) and the later block around lines referencing the same pattern) to pipe the
name list through a check for file existence (e.g., test -e or a small loop) and
only include existing filenames, then write that filtered list to GITHUB_OUTPUT
as the files variable.
- Around line 18-72: The self-entrypoint currently contains the linting business
logic inside jobs yamllint and actionlint (including the "Get changed YAML
files" and "Get changed workflow files" steps); extract that logic into a
reusable workflow file (e.g., create or update
.github/workflows/self-pr-analysis-reusable.yml) that defines the yamllint and
actionlint jobs and their steps, and then simplify this self-pr-analysis.yml to
a thin wrapper that uses workflow_call to call the local reusable workflow
(./.github/workflows/<reusable-name>.yml), keeping only the minimal
inputs/outputs and the uses: './.github/workflows/<reusable-name>.yml'
invocation; ensure job names (yamllint, actionlint) and step IDs (changed)
remain consistent so callers continue to work.
- Around line 30-32: The grep in the changed-file collectors (the command
assigning files=$(gh pr diff ... | grep -E '\.yml$' | tr '\n' ' ' | sed 's/
$//') and the similar collector later) can return non-zero when there are no
matches and cause the step to fail; change the pipeline to tolerate no matches
by making grep non-fatal (e.g., use grep -E '\.yml$' || true or grep -E '\.yml$'
|| : directly in the pipeline) so the subsequent tr/sed still run and yield an
empty string, ensuring the job cleanly skips when there are no matching changed
files.
- Line 1: The self-pr-analysis.yml contains business logic (yamllint/actionlint)
and must be replaced by a call to a reusable workflow: extract the existing jobs
into a new reusable workflow file named pr-analysis.yml that defines
workflow_call inputs and contains the yamllint and actionlint jobs (preserving
job names and any required secrets/permissions), then change
self-pr-analysis.yml to be a thin wrapper that invokes the reusable workflow
using uses: ./path/to/pr-analysis.yml (passing through any needed
inputs/secrets) and remove all direct job definitions from self-pr-analysis.yml
so it only delegates to pr-analysis.yml.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2cdcd2a5-4629-461e-b5ca-7b186ff7844e
📒 Files selected for processing (1)
.github/workflows/self-pr-analysis.yml
Add actionlint config with blacksmith-4vcpu-ubuntu-2404 as a known self-hosted runner label to avoid false-positive runner-label errors.
…elf-pr-validation - Pinned Actions Check: fails on uses: @main/@master (skips LerianStudio) - Markdown Link Check: validates links in changed .md files - Spelling Check: typos-cli across the codebase - self-pr-validation: thin entrypoint calling pr-validation.yml with source branch enforcement for PRs to main - Add .github/markdown-link-check-config.json - Update dependabot groups for new actions
Consolidate all PR checks into a single self-pr-validation workflow: PR validation, YAML lint, actionlint, pinned actions, markdown link check, and spelling. Remove self-pr-analysis.yml.
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/self-pr-validation.yml:
- Around line 42-49: The file discovery only matches ".yml" because the grep
pattern used when building the files variable (the command with grep -E
'\.yml$') omits ".yaml" files; update that grep invocation to match both
extensions (e.g., use a regex like '\.(yml|yaml)$' or '\.ya?ml$') so the files
variable includes both .yml and .yaml files when constructing the list for the
pull_request branch.
- Around line 71-78: The workflow only matches .yml files; update the file
selection so it includes both .yml and .yaml and follows the pinned-actions
pattern: when constructing the PR diff list change the grep pattern used to
build files to match both extensions (use the same '\.ya?ml$' style regex
referenced in the diff) and change the default branch case to include both
extensions (e.g., set files to match both .yml and .yaml instead of only
".github/workflows/*.yml"); keep the rest of the logic that writes to the
GITHUB_OUTPUT variable and the use of
github.event_name/github.event.pull_request.number unchanged.
- Around line 113-133: The run step uses bash-specific process substitution (<
<(grep -n ...)) inside the block that defines the loop (while IFS= read -r line;
do ... done < <(grep -n ...)), so make the step explicitly use bash by adding
shell: bash to that run step in the workflow; this ensures the
process-substitution and the loop/IFS/read logic work reliably across runners
and avoids sh/bash default drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a4804b6-6de9-4910-9981-2cf56ae4238e
📒 Files selected for processing (1)
.github/workflows/self-pr-validation.yml
Create 5 composite actions under src/lint/: - yamllint: YAML syntax validation - actionlint: GitHub Actions workflow validation - pinned-actions: ensures uses: refs are pinned - markdown-link-check: validates links in .md files - typos: spelling check via typos-cli Refactor self-pr-validation to use composites directly with a shared changed-files detection job for all lint checks.
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/self-pr-validation.yml:
- Around line 61-64: Combine the four echo lines that append to the same output
file into a single grouped redirection to reduce repeated opens; update the
block that currently echoes "yaml_files=${yaml_files}",
"workflow_files=${workflow_files}", "action_files=${action_files}", and
"markdown_files=${markdown_files}" so they are written in one grouped redirect
(using { ... } >> "$GITHUB_OUTPUT") instead of four separate echo >>
"$GITHUB_OUTPUT" calls.
In `@src/lint/pinned-actions/action.yml`:
- Line 41: The current grep check (done < <(grep -n 'uses:.*@\(main\|master\)'
"$file" 2>/dev/null || true)) intentionally only flags `@main/`@master; either
tighten the check or document the allowed exceptions: to enforce SHA-only
pinning, expand the grep to also match semantic version tags (e.g. change the
pattern to include v<semver> like
'uses:.*@\(main\|master\|v[0-9]\+\(\.[0-9]\+\)*\)' or a more robust semver
regex), or update the action description to explicitly state that semantic
version tags (e.g. `@v3`, `@v3.1.1`) are acceptable exceptions while disallowing
branch pins.
In `@src/lint/typos/action.yml`:
- Line 14: The workflow step currently pins the GitHub Action using a mutable
tag ("uses: crate-ci/typos@v1.44.0"); replace that tag with the immutable commit
SHA for the corresponding release (i.e., update the "uses:
crate-ci/typos@v1.44.0" entry to "uses: crate-ci/typos@<COMMIT_SHA>") so the
action is pinned to a specific commit and the PR validation "Unpinned action
found" error is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71047d0a-3685-4b55-aad2-09f653333905
📒 Files selected for processing (11)
.github/workflows/self-pr-validation.ymlsrc/lint/actionlint/README.mdsrc/lint/actionlint/action.ymlsrc/lint/markdown-link-check/README.mdsrc/lint/markdown-link-check/action.ymlsrc/lint/pinned-actions/README.mdsrc/lint/pinned-actions/action.ymlsrc/lint/typos/README.mdsrc/lint/typos/action.ymlsrc/lint/yamllint/README.mdsrc/lint/yamllint/action.yml
…tion Create src/config/changed-workflows composite that categorizes changed files by type (YAML, workflows, actions, markdown) for downstream lint jobs. Refactor self-pr-validation to use it.
|
This PR is very large (23 files, 1353 lines changed). Consider breaking it into smaller PRs for easier review. |
|
This PR is very large (24 files, 1412 lines changed). Consider breaking it into smaller PRs for easier review. |
|
This PR is very large (24 files, 1412 lines changed). Consider breaking it into smaller PRs for easier review. |
|
This PR is very large (24 files, 1412 lines changed). Consider breaking it into smaller PRs for easier review. |
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/lint/shellcheck/action.yml`:
- Around line 9-12: The action currently only increments violations when level
== 'error' and ignores non-JSON failures; update the result-handling logic so
that (1) you map severity strings to a priority order (e.g., {style:0, info:1,
warning:2, error:3}), read the configured input severity, and when iterating
parsed shellcheck findings increment the violations counter for any finding
whose level priority is >= configured minimum (use the existing level variable
from parsed items); and (2) handle non-JSON runs by detecting an empty
result.stdout and treating that as a failure: increment violations, include
result.stderr (or stdout) text in the annotations/logs, and fail the action
accordingly. Ensure you update references to the existing violations variable
and use result.stdout/result.stderr parsing locations so behavior is consistent
for both JSON and non-JSON outputs.
- Around line 89-105: The code always forces bash by prefixing scripts with
'#!/usr/bin/env bash' and passing '--shell=bash' to shellcheck; change both to
use the actual shell variable (shell) from the step so steps with 'shell: sh'
are handled correctly: set the shebang using the shell value (e.g.,
'#!/usr/bin/env ' + shell) when building script and replace the hardcoded
'--shell=bash' in the subprocess.run argument with f'--shell={shell}'
(references: the variables shell, step, script and the subprocess.run call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1839affe-91ad-4887-a748-c18a314202ff
📒 Files selected for processing (6)
.github/workflows/self-pr-validation.ymlsrc/lint/readme-check/README.mdsrc/lint/readme-check/action.ymlsrc/lint/shellcheck/README.mdsrc/lint/shellcheck/action.ymlsrc/notify/pr-lint-reporter/action.yml
|
This PR is very large (24 files, 1412 lines changed). Consider breaking it into smaller PRs for easier review. |
|
This PR is very large (24 files, 1416 lines changed). Consider breaking it into smaller PRs for easier review. |
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/lint/shellcheck/action.yml`:
- Around line 57-59: The regex in replace_gha_exprs fails for GHA expressions
with nested braces because r'\$\{\{[^}]*\}\}' stops at the first '}', so update
the pattern used in the re.sub inside replace_gha_exprs to use a non-greedy
match (e.g., change the inner token from [^}]* to a non-greedy .*? ) so
expressions like ${{ format('{0}', x) }} are fully matched and replaced; keep
the replacement string '${GHA_PLACEHOLDER}' and ensure the re.sub call uses the
new pattern.
- Around line 121-123: ShellCheck line numbers are offset by one because the
temporary file includes a prepended shebang; adjust the reported line before
printing annotations by decrementing the retrieved line number by 1 (but clamp
to a minimum of 1) so the annotation reflects the original run: block; apply
this change where line is computed (the variable line used later in the print
call) so the printed annotation (using filepath, step_name, code, msg, ann)
reports the corrected line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3540787f-2419-409a-844a-667b902cb4ce
📒 Files selected for processing (1)
src/lint/shellcheck/action.yml
|
This PR is very large (24 files, 1416 lines changed). Consider breaking it into smaller PRs for easier review. |
|
This PR is very large (26 files, 1592 lines changed). Consider breaking it into smaller PRs for easier review. |
…bab-case and reserved prefix checks
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/lint/composite-schema/action.yml`:
- Around line 73-91: The inputs loop is silently skipping malformed scalar
entries and will crash on a null description due to .strip(); update the logic
around inputs = data.get('inputs') and the for name, spec in inputs.items():
block so that non-dict specs are treated as violations (print an error and
increment violations rather than continue), guard description access by using
(spec.get('description') or '').strip() when computing has_desc, and ensure the
final branch that prints the required=false/no-default error also increments
violations; refer to the variables/locations: inputs, spec, has_desc, required,
has_default, violations and the for-name loop to make these changes.
In `@src/lint/composite-schema/README.md`:
- Around line 22-43: Add a "Usage as reusable workflow" section to
src/lint/composite-schema/README.md alongside the existing "Usage as composite
step" example: include a complete GitHub Actions workflow YAML that demonstrates
calling the composite as a reusable workflow (uses:
LerianStudio/github-actions-shared-workflows/src/lint/composite-schema@develop),
include the required permissions block, and explicitly show "secrets: inherit"
in the caller workflow so the README meets the composite-action template
guidelines.
In `@src/notify/pr-lint-reporter/action.yml`:
- Around line 81-129: The JS literals in the checks array (entries like jobName
'YAML Lint', 'Action Lint', 'Pinned Actions', 'Markdown Link Check', 'Spelling
Check', 'Shell Check', 'README Check', 'Composite Schema') directly interpolate
inputs.* into single-quoted strings which can break on apostrophes/backslashes;
update each files: and entireRepo: interpolation to use a safely escaped JSON
form (e.g., toJSON(inputs['yamllint-files']) or env-mapped values) so strings
are properly escaped before .trim().split(...).filter(Boolean) is called and
ensure the same pattern is applied for all '*-files' inputs referenced in the
checks array.
In `@src/notify/pr-lint-reporter/README.md`:
- Around line 32-62: The README's "Usage as composite step" example only
supplies the result inputs (yamllint-result, actionlint-result,
shellcheck-result, readme-result, composite-schema-result, github-token) which
yields empty file lists and misleading "Files Scanned" output; update that
section to a full composite-step YAML that also passes the corresponding file
list inputs (e.g., yamllint-files, actionlint-files, shellcheck-files,
readme-files, composite-schema-files or whatever file-list input names the
action defines) and any other required inputs, and add the missing "Usage as
reusable workflow" section with a complete workflow YAML that calls
LerianStudio/github-actions-shared-workflows/src/notify/pr-lint-reporter@develop
and includes secrets: inherit and proper permissions (pull-requests: write,
issues: write, checks: read) so the reusable example is complete and shows real
file lists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc49c857-c79a-4b4b-814e-e8e0bea5a0e9
📒 Files selected for processing (5)
.github/workflows/self-pr-validation.ymlsrc/lint/composite-schema/README.mdsrc/lint/composite-schema/action.ymlsrc/notify/pr-lint-reporter/README.mdsrc/notify/pr-lint-reporter/action.yml
|
This PR is very large (26 files, 1640 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/lint/composite-schema/README.md (1)
36-57:⚠️ Potential issue | 🟡 MinorMissing "Usage as reusable workflow" section.
The README documents composite step usage but omits the reusable workflow form required by repo conventions.
As per coding guidelines,
README.md in composite actions must include... usage as reusable workflow (full YAML withsecrets: inherit).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lint/composite-schema/README.md` around lines 36 - 57, Add a "Usage as reusable workflow" section to src/lint/composite-schema/README.md showing the reusable-workflow YAML pattern required by repo conventions: include the workflow invocation with uses: LerianStudio/.../lint/composite-schema@develop (or the correct ref), the jobs calling the reusable workflow, and include the required permissions and secrets: inherit entry; ensure the section mirrors the composite step example but in reusable-workflow form and explicitly shows "secrets: inherit".src/lint/composite-schema/action.yml (1)
95-98:⚠️ Potential issue | 🟠 MajorMalformed input specs are silently skipped instead of flagged as violations.
If
inputsin a composite action is a list (not a mapping), line 96 will raiseAttributeErroron.items(). Non-dict spec entries are also silently ignored rather than reported.Proposed fix
# ── Input conventions ── inputs = data.get('inputs') or {} + if not isinstance(inputs, dict): + err(filepath, '"inputs" must be a mapping, not a scalar or list.') + continue + for name, spec in inputs.items(): if not isinstance(spec, dict): - continue + err(filepath, f'Input "{name}" definition must be a mapping.') + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lint/composite-schema/action.yml` around lines 95 - 98, The loop over inputs blindly calls data.get('inputs') and then .items(), which will raise AttributeError if inputs is a list and currently silently skips non-dict specs; update the logic to first validate that inputs is a mapping (e.g., isinstance(inputs, dict)) and emit a violation if it is not, and when iterating over inputs ensure each spec is validated (emit a violation for any non-dict spec entry instead of continue) so malformed input specs are reported rather than ignored; refer to the inputs variable and the for name, spec in inputs.items() loop when making these changes.
🤖 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/self-pr-validation.yml:
- Around line 12-32: The validation job is being invoked for manual
workflow_dispatch runs which lack pull_request context; modify the job named
"validation" to guard it so it only runs for pull_request events (i.e., add an
if that checks github.event_name == 'pull_request') before it calls the reusable
workflow .github/workflows/pr-validation.yml so the PR-specific context fields
are defined.
---
Duplicate comments:
In `@src/lint/composite-schema/action.yml`:
- Around line 95-98: The loop over inputs blindly calls data.get('inputs') and
then .items(), which will raise AttributeError if inputs is a list and currently
silently skips non-dict specs; update the logic to first validate that inputs is
a mapping (e.g., isinstance(inputs, dict)) and emit a violation if it is not,
and when iterating over inputs ensure each spec is validated (emit a violation
for any non-dict spec entry instead of continue) so malformed input specs are
reported rather than ignored; refer to the inputs variable and the for name,
spec in inputs.items() loop when making these changes.
In `@src/lint/composite-schema/README.md`:
- Around line 36-57: Add a "Usage as reusable workflow" section to
src/lint/composite-schema/README.md showing the reusable-workflow YAML pattern
required by repo conventions: include the workflow invocation with uses:
LerianStudio/.../lint/composite-schema@develop (or the correct ref), the jobs
calling the reusable workflow, and include the required permissions and secrets:
inherit entry; ensure the section mirrors the composite step example but in
reusable-workflow form and explicitly shows "secrets: inherit".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aa270591-302c-4958-99f8-59446190b34e
📒 Files selected for processing (4)
.github/workflows/self-pr-validation.ymlsrc/config/changed-workflows/action.ymlsrc/lint/composite-schema/README.mdsrc/lint/composite-schema/action.yml
…inputs, escape JS file paths with toJSON
|
This PR is very large (26 files, 1668 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/self-pr-validation.yml:
- Around line 183-197: The workflow permissions block for the reporter job is
missing the actions scope so the reporter step (named "Post Lint Report" which
invokes src/notify/pr-lint-reporter and calls actions.listJobsForWorkflowRun())
gets a token that causes 403s; add actions: read to the permissions list
alongside contents: read, pull-requests: write, issues: write, checks: read so
the reporter can call actions.listJobsForWorkflowRun successfully.
In `@src/lint/composite-schema/action.yml`:
- Around line 69-74: The validator currently silently skips or will later crash
on malformed action.yml entries; instead, when the top-level `data` is not a
dict or when `runs` is missing or `runs.get('using') != 'composite'`, treat this
as a validation failure (emit an annotation/error) rather than `continue`;
similarly validate that `runs` is a mapping and that `runs.get('steps')` is a
list of mappings before accessing `len(steps)` or iterating — update the checks
surrounding the `data`, `runs`, and `steps` variables (and the analogous block
at the later occurrence around lines 88-92) to raise/record a clear validation
error when types/values are wrong so malformed manifests fail with a helpful
message.
- Around line 81-85: The current checks use str(data.get('name')).strip() and
str(data.get('description')).strip(), which treat null/None as the string
"None"; update the validations to require a non-empty string: ensure
data.get('name') is an instance of str and that data.get('name').strip() is not
empty (call err(filepath, 'Missing top-level "name" field.') otherwise), and do
the same for data.get('description') (calling err(filepath, 'Missing top-level
"description" field.') when missing). Use the existing err(...) function and the
data.get(...) calls to locate where to change the checks.
In `@src/notify/pr-lint-reporter/action.yml`:
- Around line 5-7: Update the github-token input description to include the
missing actions:read permission: the github-token input (named "github-token")
currently documents pull-requests:write, issues:write and checks:read but must
also mention actions:read because the code calls
github.rest.actions.listJobsForWorkflowRun() when reporting failure details;
change the description text for github-token to list actions:read alongside the
other scopes so callers know to provide that permission.
In `@src/notify/pr-lint-reporter/README.md`:
- Around line 38-63: The example job's needs array is missing the dependent job
"changed-files" while the step POST Lint Report references
needs.changed-files.outputs.*, so update the job's needs declaration (the needs
array used in the YAML snippet) to include "changed-files" alongside yamllint,
actionlint, pinned-actions, markdown-link-check, typos, shellcheck,
readme-check, and composite-schema so that the outputs referenced
(needs.changed-files.outputs.yaml_files, workflow_files, action_files,
markdown_files, all_files, composite_files) are available at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 37850669-c962-4e8c-a564-a9bce863df30
📒 Files selected for processing (4)
.github/workflows/self-pr-validation.ymlsrc/lint/composite-schema/action.ymlsrc/notify/pr-lint-reporter/README.mdsrc/notify/pr-lint-reporter/action.yml
…guards, fix README example
|
This PR is very large (26 files, 1678 lines changed). Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lint/composite-schema/action.yml (1)
73-78:⚠️ Potential issue | 🟠 MajorFail non-composite manifests instead of skipping them.
runs.using != 'composite'currently falls throughcontinue, so a wrong action type undersrc/**/action.ymlpasses this check entirely.As per coding guidelines, `src/**/action.yml`: `This is a composite action in a shared CI/CD library.`Suggested fix
runs = data.get('runs') if not isinstance(runs, dict): err(filepath, '"runs" must be a mapping.') continue if runs.get('using') != 'composite': - continue + err(filepath, 'Composite actions must declare "runs.using: composite".') + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lint/composite-schema/action.yml` around lines 73 - 78, The code currently skips manifests whose runs.using is not "composite"; change this to report an error instead: in the block that reads runs = data.get('runs') and checks isinstance(runs, dict), replace the continue when runs.get('using') != 'composite' with a call to err(...) (include filepath and a clear message like '"runs.using" must be "composite"') and then continue; update the check around runs.get('using') in the same scope (refer to the runs variable and the err function) so non-composite manifests fail the validation rather than being silently ignored.
🤖 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/lint/composite-schema/action.yml`:
- Around line 103-112: The code currently normalizes malformed "inputs" via
inputs = data.get('inputs') or {}, which lets non-mapping values slip through
and causes double-counting because err(filepath, ...) already increments
violations; change the logic in the validation block that sets and checks inputs
so you (1) retrieve inputs with data.get('inputs') without falling back to {}
and (2) if inputs is None skip the loop, but if inputs exists and is not a dict
call err(filepath, '"inputs" must be a mapping.') and continue without
iterating; also remove the extra violations += 1 where err(...) already
increments violations (or ensure only err increments) so the scalar-spec branch
for for name, spec in inputs.items() does not double-count.
In `@src/notify/pr-lint-reporter/README.md`:
- Around line 38-39: The example workflow currently uses "if: always() &&
github.event_name == 'pull_request'" which lets the job run even when the
required changed-files job failed; update the job's if condition to also require
the changed-files job succeeded (e.g., include needs.changed-files.result ==
'success') so that this job is gated by the outcome of the changed-files
check—modify the if expression where the "if: always() && github.event_name ==
'pull_request'" line appears (the same location that mirrors the gating used in
.github/workflows/self-pr-validation.yml for the lint-report job).
---
Duplicate comments:
In `@src/lint/composite-schema/action.yml`:
- Around line 73-78: The code currently skips manifests whose runs.using is not
"composite"; change this to report an error instead: in the block that reads
runs = data.get('runs') and checks isinstance(runs, dict), replace the continue
when runs.get('using') != 'composite' with a call to err(...) (include filepath
and a clear message like '"runs.using" must be "composite"') and then continue;
update the check around runs.get('using') in the same scope (refer to the runs
variable and the err function) so non-composite manifests fail the validation
rather than being silently ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d0bb763-9155-4213-8815-fb5e2243324b
📒 Files selected for processing (4)
.github/workflows/self-pr-validation.ymlsrc/lint/composite-schema/action.ymlsrc/notify/pr-lint-reporter/README.mdsrc/notify/pr-lint-reporter/action.yml
Comment on lines
+103
to
+112
| inputs = data.get('inputs') or {} | ||
| if not isinstance(inputs, dict): | ||
| err(filepath, '"inputs" must be a mapping.') | ||
| continue | ||
|
|
||
| for name, spec in inputs.items(): | ||
| if not isinstance(spec, dict): | ||
| err(filepath, f'Input "{name}" definition must be a mapping.') | ||
| violations += 1 | ||
| continue |
There was a problem hiding this comment.
Don't normalize malformed inputs to {} or double-count them.
data.get('inputs') or {} lets inputs: [] / '' pass as a valid empty mapping, and the scalar-spec branch inflates the final total because err() already increments violations.
Suggested fix
- inputs = data.get('inputs') or {}
+ if 'inputs' not in data:
+ inputs = {}
+ else:
+ inputs = data.get('inputs')
if not isinstance(inputs, dict):
err(filepath, '"inputs" must be a mapping.')
continue
for name, spec in inputs.items():
if not isinstance(spec, dict):
err(filepath, f'Input "{name}" definition must be a mapping.')
- violations += 1
continue
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lint/composite-schema/action.yml` around lines 103 - 112, The code
currently normalizes malformed "inputs" via inputs = data.get('inputs') or {},
which lets non-mapping values slip through and causes double-counting because
err(filepath, ...) already increments violations; change the logic in the
validation block that sets and checks inputs so you (1) retrieve inputs with
data.get('inputs') without falling back to {} and (2) if inputs is None skip the
loop, but if inputs exists and is not a dict call err(filepath, '"inputs" must
be a mapping.') and continue without iterating; also remove the extra violations
+= 1 where err(...) already increments violations (or ensure only err
increments) so the scalar-spec branch for for name, spec in inputs.items() does
not double-count.
Comment on lines
+38
to
+39
| needs: [changed-files, yamllint, actionlint, pinned-actions, markdown-link-check, typos, shellcheck, readme-check, composite-schema] | ||
| if: always() && github.event_name == 'pull_request' |
There was a problem hiding this comment.
Gate the example on successful file detection.
This snippet still runs on always() alone. If changed-files fails, copied workflows will recreate the misleading “skipped / no changes” report that the real workflow already fixed.
Suggested fix
- if: always() && github.event_name == 'pull_request'
+ if: always() && github.event_name == 'pull_request' && needs.changed-files.result == 'success'
📝 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
| needs: [changed-files, yamllint, actionlint, pinned-actions, markdown-link-check, typos, shellcheck, readme-check, composite-schema] | |
| if: always() && github.event_name == 'pull_request' | |
| needs: [changed-files, yamllint, actionlint, pinned-actions, markdown-link-check, typos, shellcheck, readme-check, composite-schema] | |
| if: always() && github.event_name == 'pull_request' && needs.changed-files.result == 'success' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/notify/pr-lint-reporter/README.md` around lines 38 - 39, The example
workflow currently uses "if: always() && github.event_name == 'pull_request'"
which lets the job run even when the required changed-files job failed; update
the job's if condition to also require the changed-files job succeeded (e.g.,
include needs.changed-files.result == 'success') so that this job is gated by
the outcome of the changed-files check—modify the if expression where the "if:
always() && github.event_name == 'pull_request'" line appears (the same location
that mirrors the gating used in .github/workflows/self-pr-validation.yml for the
lint-report job).
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.
GitHub Actions Shared Workflows
Description
Add a
self-pr-analysisworkflow that runs yamllint and actionlint on every pull request targetingdevelopormain. This ensures YAML syntax correctness and GitHub Actions workflow validity before merge..ymlfiles against.yamllint.ymlconfig (relaxed truthy foron:, 200-char line limit, flexible indentation)Also adds:
.yamllint.ymlconfig at repo rootlintinggroup in Dependabot foribiqlik/action-yamllintandraven-actions/actionlintlintlabel in.github/labels.ymlType of Change
ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)Breaking Changes
None.
Testing
@developor the beta tagCaller repo / workflow run: N/A — self-CI workflow, will validate on this PR's own checks.
Related Issues
N/A
Summary by CodeRabbit
Chores
New Features
Documentation