feat: add initial GitHub Action wrapping skill-linter#1
Merged
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
action.yml (1)
150-150: 💤 Low valueStderr output may be lost in SARIF mode.
The output redirection
> "$SL_SARIF_OUTPUT"on line 150 only captures stdout. If the linter emits diagnostic messages or errors to stderr, they will be lost or interleaved with the GitHub Actions log rather than preserved. Consider whether stderr should be redirected to the SARIF file (with>&), logged separately, or allowed to flow to the Actions console.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@action.yml` at line 150, The current command npx "skill-linter@${SL_VERSION}" check "${args[@]}" > "$SL_SARIF_OUTPUT" only captures stdout so stderr diagnostics are lost; update the invocation in the action to capture stderr as well (e.g., redirect both streams into the SARIF file with > "$SL_SARIF_OUTPUT" 2>&1 or use bash shorthand &> "$SL_SARIF_OUTPUT"), or alternatively send stderr to a separate log file if you want SARIF to remain stdout-only—modify the line with the npx "skill-linter@${SL_VERSION}" check "${args[@]}" invocation accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@action.yml`:
- Around line 75-78: The outputs contract promises an absolute SARIF path but
the action currently emits the raw SL_SARIF_OUTPUT (a relative
"skill-linter-results.sarif"); change the place where the output is written (the
step that echoes to GITHUB_OUTPUT / sets steps.lint.outputs.sarif-file) to
resolve SL_SARIF_OUTPUT to an absolute path (e.g., via realpath/$(pwd)/join or
equivalent in the runner shell) and then write that resolved path to
GITHUB_OUTPUT so steps.lint.outputs.sarif-file always contains an absolute path.
- Around line 149-154: The SARIF file is created via output redirection even if
the linter fails; modify the block that runs npx "skill-linter@${SL_VERSION}"
check when SL_FORMAT == "sarif" so you capture the command exit status (e.g.,
run the check without direct redirection, store its exit code from `$?`), only
write to SL_SARIF_OUTPUT and append "sarif-file=${SL_SARIF_OUTPUT}" to
GITHUB_OUTPUT if the exit status indicates success, and preserve/propagate the
original exit code (so set -e behavior and failure semantics remain intact);
update the conditional around SL_FORMAT/SL_SARIF_OUTPUT and the invocation of
npx accordingly.
In `@README.md`:
- Around line 130-133: The SARIF upload step currently uses a hardcoded path
("results.sarif") which will fail if the linter didn’t produce a file; change
the upload step (github/codeql-action/upload-sarif@v3) to guard on the linter
output and consume that output instead by adding a conditional (if:
steps.lint.outputs.sarif-file != '') and passing the SARIF path from
steps.lint.outputs.sarif-file to the sarif_file input; update the step to
reference the output symbol steps.lint.outputs.sarif-file rather than
"results.sarif" so the uploader only runs when a SARIF file exists.
---
Nitpick comments:
In `@action.yml`:
- Line 150: The current command npx "skill-linter@${SL_VERSION}" check
"${args[@]}" > "$SL_SARIF_OUTPUT" only captures stdout so stderr diagnostics are
lost; update the invocation in the action to capture stderr as well (e.g.,
redirect both streams into the SARIF file with > "$SL_SARIF_OUTPUT" 2>&1 or use
bash shorthand &> "$SL_SARIF_OUTPUT"), or alternatively send stderr to a
separate log file if you want SARIF to remain stdout-only—modify the line with
the npx "skill-linter@${SL_VERSION}" check "${args[@]}" invocation accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
adrianriobo
force-pushed
the
feat/initial-action
branch
from
4b8e725 to
8f282bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
README.md (1)
27-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard SARIF upload in the short example too.
This snippet should mirror the complete example so upload only runs when
sarif-fileis actually set.📝 Suggested README snippet update
- uses: github/codeql-action/upload-sarif@v3 + if: steps.lint.outputs.sarif-file != '' with: sarif_file: ${{ steps.lint.outputs.sarif-file }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 27 - 30, The short README example should guard the SARIF upload so it only runs when the sarif-file exists: update the upload step that uses "github/codeql-action/upload-sarif@v3" and the sarif_file input (currently set to "${{ steps.lint.outputs.sarif-file }}") to include a conditional like "if: ${{ steps.lint.outputs.sarif-file }}" (or "if: ${{ steps.lint.outputs['sarif-file'] != '' }}") so the upload step is skipped when the sarif-file output is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test-action.yml:
- Line 17: Replace mutable action tags (e.g., uses: actions/checkout@v4 and the
other occurrences using `@v3/`@v4) with the corresponding full commit SHAs to pin
versions; locate each usage string (e.g., "actions/checkout@v4") in the
workflow, visit the action's GitHub repository to find the latest trusted commit
SHA for the desired version, and update the uses line to
"actions/checkout@<full-commit-sha>" (and similarly for the other two actions)
so all third-party actions are pinned to immutable SHAs.
- Line 17: Update both checkout steps that use actions/checkout@v4 to disable
credential persistence by adding persist-credentials: false to their step
configuration; locate the steps referencing "uses: actions/checkout@v4" and
insert a top-level key persist-credentials: false so the GitHub token is not
written to git config for subsequent steps.
- Around line 45-48: Guard the "Upload SARIF to GitHub Security tab" step (uses:
github/codeql-action/upload-sarif@v3) so it only runs for non-fork PRs or non-PR
workflows: add an if condition checking the event context (e.g., if:
github.event.pull_request == null ||
github.event.pull_request.head.repo.full_name == github.repository) and keep
sarif_file: ${{ steps.lint.outputs.sarif-file }}; this prevents the upload step
from running on forked pull requests and avoids "Resource not accessible by
integration" errors.
In `@action.yml`:
- Around line 134-146: When parsing SL_RULE and SL_CATEGORY into arrays (using
IFS=',' read -ra rules/cats and looping over r/c), skip tokens that are empty or
only whitespace before appending to args to avoid adding --rule "" or --category
""; trim or remove spaces (you already use ${r// /} / ${c// /}) then check that
the resulting token is non-empty (e.g., if [[ -n "$token" ]]) before executing
args+=(--rule "$token") or args+=(--category "$token"); apply this guard in both
the SL_RULE loop (variables rules and r) and the SL_CATEGORY loop (variables
cats and c).
---
Duplicate comments:
In `@README.md`:
- Around line 27-30: The short README example should guard the SARIF upload so
it only runs when the sarif-file exists: update the upload step that uses
"github/codeql-action/upload-sarif@v3" and the sarif_file input (currently set
to "${{ steps.lint.outputs.sarif-file }}") to include a conditional like "if:
${{ steps.lint.outputs.sarif-file }}" (or "if: ${{
steps.lint.outputs['sarif-file'] != '' }}") so the upload step is skipped when
the sarif-file output is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cb6798c-9f51-41a4-aaf0-09ee74981d34
📒 Files selected for processing (4)
.github/workflows/test-action.ymlREADME.mdaction.ymltests/fixtures/hello-world/SKILL.md
adrianriobo
force-pushed
the
feat/initial-action
branch
from
7a8de10 to
5b430e3
Compare
Composite action (action.yml) wrapping npx skill-linter check: - All CLI flags exposed as inputs via env-var injection (no shell injection risk) - SARIF mode suspends set -e, captures exit code, resolves absolute path via realpath, sets sarif-file output only for exit 0/1 (valid SARIF), propagates original exit code - upload-sarif input triggers an internal upload step (if: always()) guarded against fork PRs; callers only need format: sarif, upload-sarif: true and security-events: write - All third-party uses: pinned to immutable commit SHAs - Empty-token guard in rule/category loops prevents blank flags on double commas CI workflow (.github/workflows/test-action.yml) self-tests the action on every PR and push to main using uses: ./ against a hello-world skill fixture (tests/fixtures/hello-world/SKILL.md) — two jobs: github annotations format and SARIF format with auto upload. Checkout steps pinned and use persist-credentials: false. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
adrianriobo
force-pushed
the
feat/initial-action
branch
from
b1f3688 to
9a9abad
Compare
ktdreyer
reviewed
May 20, 2026
| with: | ||
| path: ./skills | ||
| deep: true | ||
| anthropic-api-key: ${{ secrets.ANTHROPIC_API_KEY }} |
There was a problem hiding this comment.
Our team will probably never test this, and I don't think it's secure to hard-code static keys in GH secrets anyway. What if we just removed ANTHROPIC_API_KEY support and only supported Vertex?
Contributor
Author
There was a problem hiding this comment.
This is coming from skill linter itself... I can remove it but the thing is we will not use but the idea is this action can be used by anyone...so it is up to them use it or not
Contributor
Author
There was a problem hiding this comment.
Eventually we will trigger action creations based in skill-linter project so both should be in sync...
There was a problem hiding this comment.
We discussed this in Slack. I'm ok with merging this today.
Contributor
Author
There was a problem hiding this comment.
ok, lets go with only testable code in our end 👌 we will merge this to have something initially and I will create a follow up to remove un testable code
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.
Summary
action.ymlas a composite GitHub Action wrappingnpx skill-linter checkformat,fix,deep,strict,rule,category, etc.) as action inputssarif-file)Test plan
format: githuband verify PR annotations appearformat: sarifand verify thesarif-fileoutput is set and the file is valid SARIFstrict: truecauses exit 1 on warningsdeep: truewith a validanthropic-api-keyruleandcategoryinputs expand to repeated flags correctly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests