refactor: make skill execution host-neutral - #28
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThis PR moves Git, release, and shipping runners into skill-local scripts with compatibility wrappers, updates shipping review publication contracts, and generalizes skill authoring, evaluation, runner selection, validation, and documentation for host-neutral execution. ChangesPortable runner implementations
Shipping PR review workflow
Host-neutral skill authoring
Repository validation and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
🧹 Nitpick comments (1)
plugins/skill-set/skills/creating-skills/reference/patterns.md (1)
107-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
The fenced code block is missing a language specifier, which triggers a markdown lint warning. Consider adding
textto properly format the block.♻️ Proposed refactor
For complex operations where mistakes are costly, have the active agent create a verifiable intermediate plan before executing. -``` +```text analyze input → create plan file → validate plan → execute → verify output</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@plugins/skill-set/skills/creating-skills/reference/patterns.mdaround lines
107 - 111, Specify the fenced code block’s language as text in the pattern
example, while preserving the existing workflow content unchanged.</details> <!-- cr-comment:v1:13f93304f71c119128cc7940 --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@plugins/skill-set/scripts/run-evals:
- Around line 216-224: Update the baseline runner setup around
baseline_pr_runner and baseline_command so the archived baseline path always
supplies SKILL_SET_PR_RUNNER with an absolute executable runner path, including
when both existing runner locations are unavailable. Preserve the current
preference order for plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr
and plugins/skill-set/bin/skill-set-pr, and ensure the fallback is valid for the
shipping-pr fixture.In
@plugins/skill-set/skills/creating-skills/reference/checklist.md:
- Around line 60-62: Restore explicit model examples at both affected sites: in
plugins/skill-set/skills/creating-skills/reference/checklist.md lines 60-62,
append Claude Haiku, Claude Sonnet, and Claude Opus examples to the smallest,
primary balanced, and strongest model checklist items respectively; make the
same corresponding additions in
plugins/skill-set/skills/creating-skills/reference/testing.md lines 82-84 to the
capability tier definitions.In
@plugins/skill-set/skills/managing-git-workflow/SKILL.md:
- Around line 4-11: Restore a matching Bash or Skill permission for the
dynamically resolved skill-set runner in each affected file:
plugins/skill-set/skills/managing-git-workflow/SKILL.md lines 4-11,
plugins/skill-set/skills/bumping-version/SKILL.md lines 4-14,
plugins/skill-set/commands/git/commit.md line 4,
plugins/skill-set/commands/git/pr.md line 4, and
plugins/skill-set/commands/pr/fix.md line 3. Ensure the allowance covers the
documented skill-set-git, skill-set-release, and skill-set-pr runner invocations
without changing their execution instructions.In
@plugins/skill-set/tests/test-authoring-skills.sh:
- Around line 31-33: Update the Claude CLI detection in the host-neutrality
check around the grep command to match boundary-delimited inline references,
including bullet-prefixed text and commands embedded in documentation, while
still detecting “claude plugin” and “claude --version” forms. Preserve the
existing failure message and validation scope.In
@plugins/skill-set/tests/test-portable-runner-layout.sh:
- Around line 43-54: The jq assertions in the portable runner layout test lack
diagnostic failure handling. Update the assertions for input-prepare,
skill-set-pr usage, and skill-set-release usage to append explicit failure
handlers that call fail with a concise, assertion-specific message while
preserving the existing validation conditions.
Nitpick comments:
In@plugins/skill-set/skills/creating-skills/reference/patterns.md:
- Around line 107-111: Specify the fenced code block’s language as text in the
pattern example, while preserving the existing workflow content unchanged.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `c08eeddd-1c51-41ac-beda-22f9211838b6` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 19d66fb009c5e41864902051f2a8c6413ef6f08f and e2d4ea0ad0d5cfb4ce0d4e56556e2a93600b6f81. </details> <details> <summary>📒 Files selected for processing (67)</summary> * `.github/workflows/validate-skill-set.yml` * `AGENTS.md` * `CHANGELOG.md` * `README.md` * `plugins/skill-set/agents/ci-failure-resolver.md` * `plugins/skill-set/agents/merge-conflict-resolver.md` * `plugins/skill-set/agents/pr-review-feedback.md` * `plugins/skill-set/agents/resolving-pr-blockers.md` * `plugins/skill-set/bin/skill-set-git` * `plugins/skill-set/bin/skill-set-pr` * `plugins/skill-set/bin/skill-set-release` * `plugins/skill-set/commands/git/commit.md` * `plugins/skill-set/commands/git/pr.md` * `plugins/skill-set/commands/git/push.md` * `plugins/skill-set/commands/pr/fix.md` * `plugins/skill-set/commands/pr/ship.md` * `plugins/skill-set/evals/bumping-version/minor-bump-plugin-json/case.yaml` * `plugins/skill-set/evals/bumping-version/patch-bump-package-json/case.yaml` * `plugins/skill-set/evals/creating-skills/existing-with-creator/fixtures/scaffold.sh` * `plugins/skill-set/evals/creating-skills/existing-with-creator/prompt.md` * `plugins/skill-set/evals/creating-skills/new-with-creator/fixtures/scaffold.sh` * `plugins/skill-set/evals/creating-skills/new-with-creator/prompt.md` * `plugins/skill-set/evals/creating-skills/trigger-positive-01/case.yaml` * `plugins/skill-set/evals/managing-git-workflow/commit-with-conventional-style/case.yaml` * `plugins/skill-set/evals/managing-git-workflow/pr-title-and-body-generation/case.yaml` * `plugins/skill-set/evals/shipping-pr/state-machine-happy-path/case.yaml` * `plugins/skill-set/evals/shipping-pr/state-machine-happy-path/fixtures/mock-gh` * `plugins/skill-set/evals/shipping-pr/state-machine-happy-path/fixtures/run-shipping-eval` * `plugins/skill-set/evals/shipping-pr/state-machine-happy-path/fixtures/scaffold.sh` * `plugins/skill-set/evals/shipping-pr/state-machine-happy-path/prompt.md` * `plugins/skill-set/evals/trigger-cases.json` * `plugins/skill-set/scripts/generate-inventory` * `plugins/skill-set/scripts/run-evals` * `plugins/skill-set/scripts/summarize-evals` * `plugins/skill-set/scripts/validate-references` * `plugins/skill-set/skills/bumping-version/SKILL.md` * `plugins/skill-set/skills/bumping-version/scripts/skill-set-release` * `plugins/skill-set/skills/creating-skills/SKILL.md` * `plugins/skill-set/skills/creating-skills/reference/checklist.md` * `plugins/skill-set/skills/creating-skills/reference/evaluation.md` * `plugins/skill-set/skills/creating-skills/reference/patterns.md` * `plugins/skill-set/skills/creating-skills/reference/structure.md` * `plugins/skill-set/skills/creating-skills/reference/testing.md` * `plugins/skill-set/skills/creating-skills/reference/troubleshooting.md` * `plugins/skill-set/skills/managing-git-workflow/SKILL.md` * `plugins/skill-set/skills/managing-git-workflow/reference/commit.md` * `plugins/skill-set/skills/managing-git-workflow/reference/pr.md` * `plugins/skill-set/skills/managing-git-workflow/reference/push.md` * `plugins/skill-set/skills/managing-git-workflow/scripts/skill-set-git` * `plugins/skill-set/skills/shipping-pr/SKILL.md` * `plugins/skill-set/skills/shipping-pr/reference/blocker-resolution.md` * `plugins/skill-set/skills/shipping-pr/reference/polling.md` * `plugins/skill-set/skills/shipping-pr/reference/troubleshooting.md` * `plugins/skill-set/skills/shipping-pr/scripts/skill-set-pr` * `plugins/skill-set/tests/fixtures/git-runner/bin/gh` * `plugins/skill-set/tests/fixtures/mock-gh-pr` * `plugins/skill-set/tests/test-authoring-skills.sh` * `plugins/skill-set/tests/test-eval-layout.sh` * `plugins/skill-set/tests/test-eval-summary.sh` * `plugins/skill-set/tests/test-git-runner.sh` * `plugins/skill-set/tests/test-host-neutral-skill-runtime.sh` * `plugins/skill-set/tests/test-managing-evals.sh` * `plugins/skill-set/tests/test-portable-runner-layout.sh` * `plugins/skill-set/tests/test-pr-runner.sh` * `plugins/skill-set/tests/test-release-runner.sh` * `plugins/skill-set/tests/test-shipping-pr-authorization.sh` * `plugins/skill-set/tests/test-shipping-pr-happy-path-eval.sh` </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * plugins/skill-set/commands/git/push.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/skill-set/tests/fixtures/mock-claude-eval`:
- Around line 32-37: Change the validation failure branch in the mock evaluation
fixture so an invalid SKILL_SET_PR_RUNNER exits with a non-reserved failure
status such as 1 instead of 2. Preserve the existing absolute-path and
executable checks and the diagnostic message.
🪄 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
Run ID: f6b5392c-3581-407b-a7d9-ee7f0ac0262b
📒 Files selected for processing (4)
plugins/skill-set/scripts/run-evalsplugins/skill-set/tests/fixtures/mock-claude-evalplugins/skill-set/tests/test-authoring-skills.shplugins/skill-set/tests/test-run-evals.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/skill-set/tests/test-authoring-skills.sh
- plugins/skill-set/scripts/run-evals
Owner
Author
|
@coderabbitai resolve Restored named model tiers as documentation-only examples and added explicit diagnostics to the three portable runner JSON assertions. Kept the host-neutral permission design because safely matching an arbitrary installed runner path would require a host-specific path or a broader shell allowance. Focused checks and the full test suite passed. |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedComments resolved. Approval is disabled; enable |
This was referenced Jul 22, 2026
This was referenced Jul 31, 2026
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
1 participant
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
bin/compatibility wrappers.CLAUDE_PLUGIN_ROOT, and Claude-specific discovery requirements from portable skill execution and evaluation paths.Test Plan
plugins/skill-set/tests/run.shbash -n plugins/skill-set/bin/skill-set-pr plugins/skill-set/skills/shipping-pr/scripts/skill-set-prgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation