feat(ship): add ship as a standalone plugin - #11
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 (1)
📝 WalkthroughWalkthroughThe change adds the ChangesShip plugin integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ShipSkill as ship skill
participant PipelineConfig as .ship/config.md
participant Verification as verification commands
participant GitHubCLI as GitHub CLI
ShipSkill->>PipelineConfig: resolve pipeline settings
PipelineConfig->>Verification: run configured checks
ShipSkill->>GitHubCLI: create and monitor pull request
GitHubCLI-->>ShipSkill: return review and release status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Detection alone finds only bin/scan-skills.sh, the one command the README names as a pre-commit gate. bin/sync-versions.sh is just as much a gate: it fails when a skill directory is claimed by no plugin entry, by more than one, or is claimed but missing. Record both so the next run does not rediscover a partial answer.
Ship carries the current branch from working tree to merged release. It resolves the project's own pipeline in stage 0 - verify command, base, branch and worktree conventions, release mechanism, post-merge command, and any PR-creation hook - from .ship/config.md, then the project's docs, then a declared aggregate task, then the pull-request CI job, then a composed fallback. Ambiguity produces one question before the tree is touched, and the answer is recorded in .ship/config.md as its own commit. Packaging: marketplace.json now defines two plugins that share the flat skills/ directory, each scoping itself with a "skills" array. fathom keeps execute, scaffold, and fathom-shared; ship takes skills/ship. The root plugin.json is removed because with source "./" one root manifest applies to every entry and its version silently overrides each entry's own, which `claude plugin validate` reports. sync-versions.sh therefore reads marketplace.json as the source of truth and additionally fails when a skill directory is claimed by no plugin, by more than one, or is claimed but missing - a scoped entry replaces the default scan, so an unlisted skill would otherwise install for nobody. npx skills add is unaffected: it walks skills/ to depth three regardless of which plugin owns what, so both install paths keep working.
cca880a to
3f510ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@README.md`:
- Line 115: Update the “Asks once, remembers” README description to remove the
claim that `.ship/config.md` never appears in the review diff, while preserving
that the file is recorded and committed separately on the shipping feature
branch.
In `@skills/ship/SKILL.md`:
- Around line 13-17: Remove the repository-local skill discovery and delegation
instructions from the opening workflow in SKILL.md. Replace them with a
declarative repository configuration approach that this skill validates under
the existing Stage 0 safety restrictions, without allowing local configuration
to expand authority or permissions.
- Line 231: Update the stage-0 pr-hook flow so the hook returns both the merged
pull request and merge SHA, while limiting its authority to review and merge.
After the hook completes, always continue through the existing release
monitoring and cleanup steps, using the returned values for those steps.
🪄 Autofix
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: 4fb5944d-b054-47eb-9325-83794c031960
📒 Files selected for processing (8)
.claude-plugin/marketplace.json.claude-plugin/plugin.json.gitignore.ship/config.md.skillspector-baseline.yamlREADME.mdbin/sync-versions.shskills/ship/SKILL.md
💤 Files with no reviewable changes (1)
- .claude-plugin/plugin.json
…-commit claim A pr-hook replaces stage 3's review and merge steps, including the one that records the merge commit SHA. The release watch follows the run whose head is that SHA, so a hook that merges without reporting one leaves the watch with no run to follow; require it back, and recover it from the pull request when it is missing. The README also claimed the .ship/config.md commit keeps the file out of the diff under review. Its own commit keeps it separable from the change, but the file is still part of the pull request and still reviewed.
…peline Precedence let a repository-local ship skill decide every stage, and the executable-command restrictions lived in stage 0. A local pipeline could therefore replace stage 0 and drop the credential, write-outside-repo, privilege-escalation, and network checks while still honouring the branch, force-push, history, and gate rules it was told it could not relax. Move those restrictions into Authority and boundary, where they apply to every command the run executes whoever resolved it, and say why they live there. Stage 0 now references the rule instead of owning it.
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 `@skills/ship/SKILL.md`:
- Around line 31-34: Update the command-boundary rule to narrowly permit Git and
forge operations required for the current branch and pull request, including
fetch, push, pull-request API, merge, and release-monitoring commands, while
retaining restrictions on credentials, privilege escalation, outside-repository
writes, and unrelated network access. In the executable-slot coverage around the
stage command rules, include release alongside the existing build, lint,
type-check, test, review, and tidy categories.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The immutable command shape added in the previous commit reads literally enough to forbid git fetch, git push, and the pull-request calls the skill itself makes, which would stop a run before it ever opened a review. Carve out those operations, scoped to the shipping branch and its own pull request, and grant nothing else: credentials, privilege escalation, writes outside the repository, and unrelated network access stay blocked. Also cover the release slot in stage 0's list of executable slots.
Summary
Adds
shipas a second plugin in this marketplace, alongsidefathom, and makes the packaging support both.The skill carries the current branch from working tree to merged release. What is new relative to the personal copy it replaces is stage 0: it resolves the project's own pipeline before touching anything - verify command, review command, base, branch and worktree conventions, release mechanism, post-merge command, and any PR-creation hook - from
.ship/config.md, then the project's docs, then a declared aggregate task, then the pull-request CI job, then a composed fallback. The first tier that answers wins. Ambiguity produces exactly one question before the tree is touched, and the answer is recorded in.ship/config.mdas its own commit so the next branch, clone, and teammate inherit it.Packaging
marketplace.jsonnow defines two plugins sharing the flatskills/directory, each scoped by its ownskillsarray:fathomexecute,scaffold,fathom-sharedshipship.claude-plugin/plugin.jsonis removed. Withsource: "./"a single root manifest applies to every entry and its version silently overrides each entry's own -claude plugin validatereports exactly that.bin/sync-versions.shtherefore treatsmarketplace.jsonas the version source of truth, and additionally fails when a skill directory is claimed by no plugin, by more than one, or is claimed but missing: a scoped entry replaces the defaultskills/scan, so an unlisted skill would silently install for nobody.npx skills add crod951/skillsis unaffected - it walksskills/to depth three regardless of plugin ownership.Verification
bin/scan-skills.sh- all 4 skills clean (shipcarries 3 documented AS3 suppressions for its project-local precedence check)bin/sync-versions.sh- clean; negative-tested by planting an unclaimed skill directory (correctly exits 1)claude plugin validate . --strict- passesship, and asked a headless session which skills it could see -SHIP=yes, EXECUTE=no, SCAFFOLD=no, FATHOMSHARED=no. Scoping works even though the plugin cache contains all four directories. Test marketplace and plugin removed afterwards.Review
Nine rounds of
coderabbit review --agentagainstmain, verify green in every round. Confirmed critical/major findings fixed, including several that were defects in text written earlier in the same session:base- preflight keyed its migration to "the default branch" while stage 0 resolves abasethat may differ, so a project shipping into a release branch would open a PR against itself. Now keyed tobasewith an explicitsettled != baseassertion.--match-head-commit.gh pr createwithout--title/--body- drops into an interactive prompt that hangs an unattended run.$expectedcompared but never defined in the push guard; refs now quoted and validated withgit check-ref-format --branch..ship/config.mdwrite followed symlinks out of the repository; path is now resolved against the repo root and refused if it escapes.baseinside the worktree it was about to remove.Dispositioned, not fixed
.claude/skills/ship/SKILL.mdkeeps its specialized pipeline. The injection concern is addressed instead by scoping authority: a local file decides every stage, command, and convention, but cannot relax the Authority and boundary section.--delete-branchunless the run created the branch" (raised twice) - deleting a merged branch is the intended outcome. The genuine half (it fails while a worktree holds the branch) was fixed: worktree runs leave both deletions to cleanup.originremote name, ignored files omitted bygit stash --include-untracked, intent-to-add entries dirtying the worktree, isolating skillspector report artifacts. Real but not defects in any run this repo will make.Stage 1 was exited by judgment rather than on a no-fix round: findings ran 8 → 9 → 8 → 3 → 6 → 3 → 6 → 14 on a document that grew 113 → 299 lines, with each added guard creating fresh surface. Verify stayed green throughout. CodeRabbit on this PR is the final bar.
Follow-ups
feat/stacked-reviewsbumps fathom to 2.2.0 in theplugin.jsonthis PR deletes. Merge that branch first, then set fathom's marketplace entry to2.2.0here./plugin install ship@crod951, delete the personal copy (~/.claude/skills/ship,~/.agents/skills/ship) or two skills namedshipwill both be live.Summary by CodeRabbit
New Features
shipplugin with an end-to-end workflow for reviewing, verifying, merging, and releasing changes.fathomandshipplugins or selecting individual agent skills.Documentation
Chores