FIX: Add /ship skill for branch, atomic commits, and draft PR workflow

[tahierhussain]

What

Adds a new manual-only Claude Code skill at .claude/skills/ship/ that
walks uncommitted (or already-committed) changes through a structured
flow: branch creation → atomic Conventional-Commits → push → draft PR.
The skill is composed of SKILL.md (the procedure) and
pr-body-template.md (the source-of-truth PR description template).

Why

Standardises how PRs get raised from this working directory so branch
names, commit grouping, and PR descriptions stay consistent across
sessions, without relying on memory or ad-hoc prompting. Manual-only
(disable-model-invocation: true) so it never auto-fires from
conversational pattern-matching — it only runs when explicitly
invoked via /ship.

How

Two new files under .claude/skills/ship/:

• SKILL.md defines six phases (discover/classify, plan branch name,
  plan commits with confirmation gate, execute, draft PR body with
  confirmation gate, raise PR), plus rules for branch prefixes (binary
  feat/ vs fix/), commit grouping, secrets scanning, and PR-section
  source mapping.
• pr-body-template.md is the canonical PR body template the skill
  reads on every run, so future structural changes to PR descriptions
  are made by editing the template alone.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

No runtime impact. The change is purely additive Claude Code tooling
under .claude/skills/ — no application code, build config, CI
workflow, or product surface is touched. The skill is gated behind
manual /ship invocation, so it cannot auto-trigger.

Database Migrations

None.

Env Config

None.

Relevant Docs

None.

Related Issues or PRs

None.

Dependencies Versions

None.

Notes on Testing

• Run /ship on a clean working tree on main and confirm it stops with a clear "no changes" message
• Run /ship with mixed feat + fix changes and confirm it asks whether to split before proceeding
• Run /ship with a staged file containing a fake sk-… token and confirm the secrets-scan hard-stop fires before any commit
• Run /ship end-to-end on a small change and confirm the draft PR is opened with the body sourced from pr-body-template.md (not .github/)
• Confirm a pre-commit hook failure surfaces the error and stops, with no --no-verify retry and no amend
• Confirm the skill never auto-invokes from conversational phrasing (disable-model-invocation: true honoured)

Screenshots

N/A — tooling-only change, no UI surface.

Checklist

I have read and understood the Contribution Guidelines.

[greptile-apps]

Greptile Summary

This PR adds a new /ship Claude Code skill under .claude/skills/ship/ that walks uncommitted (or already-committed) changes through branch creation → atomic Conventional Commits → push → draft PR. The revision addresses several issues from the prior review round (added AskUserQuestion to allowed-tools, sanitized the branch-name for /tmp paths, made --base dynamic, and fixed the secrets scan to cover staged files via git diff HEAD).

• One remaining security gap: the "already committed" edge-case at line 79 instructs the model to "skip Phases 1–4", which skips the secrets scan. Any commit made locally before /ship is invoked — containing AKIA*, sk-*, ghp_*, etc. — will be pushed without hitting the hard-stop gate, directly contradicting the guardrail at line 325 ("Never skip the secrets scan")."

Confidence Score: 4/5

Safe to merge after addressing the secrets-scan bypass in the already-committed fast-path.

One P1 security finding: the "skip Phases 1-4" shortcut in the already-committed edge case silently bypasses the secrets scan, contradicting an explicit guardrail. The rest of the skill is well-structured and resolves all issues from the prior review round. No application code is affected.

.claude/skills/ship/SKILL.md — specifically the edge-case block at lines 79-81.

Security Review

• Secrets scan bypass (.claude/skills/ship/SKILL.md line 79): the "already committed" fast-path instructs "skip Phases 1–4", which skips the secrets scan entirely. Committed-but-not-pushed changes containing API keys or tokens are pushed to origin without triggering the hard-stop gate, contradicting the guardrail at line 325.

Important Files Changed

Filename	Overview
.claude/skills/ship/SKILL.md	Adds the main ship skill procedure; addresses most issues from the previous review round, but the "already committed" fast-path at line 79 silently skips the secrets scan despite the guardrail at line 325 explicitly forbidding that.
.claude/skills/ship/pr-body-template.md	Canonical PR body template with placeholder - values for all required sections; clean and consistent with the section mapping in SKILL.md Section 3.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A(["/ship invoked"]) --> B{Changes already\ncommitted locally?}
    B -- Yes --> C["⚠️ Skip Phases 1–4\n(secrets scan bypassed)"]
    C --> G[Phase 5: Draft PR body\n+ confirmation gate]
    B -- No --> D[Phase 1: Discover\ngit status / git diff HEAD\ngit diff --cached]
    D --> E{Secrets found?}
    E -- Yes --> F[AskUserQuestion hard-stop:\nDrop/keep/cancel]
    F --> D2[Phase 2: Plan branch name]
    E -- No --> D2
    D2 --> D3[Phase 3: Plan commits\n+ confirmation gate]
    D3 --> D4[Phase 4: Execute\nbranch → commits → push]
    D4 --> G
    G --> H[Phase 6: gh auth check\nExisting PR check\nWrite /tmp body\ngh pr create --draft]
    H --> I([Draft PR URL returned\nTemp file cleaned up])

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: .claude/skills/ship/SKILL.md
Line: 79-81

Comment:
**"Already committed" path bypasses the secrets scan**

The edge-case branch at line 79 says "skip Phases 1–4", but Phase 1 is exactly where the secrets scan lives. Any commits that were made locally before `/ship` was invoked — and that contain `AKIA*`, `sk-*`, `ghp_*`, or similar tokens — are pushed to origin without ever hitting the hard-stop gate. This directly contradicts the guardrail at line 325 ("Never skip the secrets scan").

The fix is to still run the scan against `git diff <base>...HEAD` (already referenced for discovery in this path) before proceeding to Phase 5:

```markdown
- **No uncommitted changes (everything already committed locally)** → run the secrets
  scan against `git diff <base>...HEAD` first (same hard-stop rules apply); if clean,
  skip commit-planning; derive the branch name and PR description from existing commit
  messages and `git diff <base>...HEAD`; push existing commits in Phase 4; proceed to Phase 5
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "fix(claude): address /ship skill review ..." | Re-trigger Greptile}

[greptile-apps]

"Already committed" path bypasses the secrets scan

The edge-case branch at line 79 says "skip Phases 1–4", but Phase 1 is exactly where the secrets scan lives. Any commits that were made locally before /ship was invoked — and that contain AKIA*, sk-*, ghp_*, or similar tokens — are pushed to origin without ever hitting the hard-stop gate. This directly contradicts the guardrail at line 325 ("Never skip the secrets scan").

The fix is to still run the scan against git diff <base>...HEAD (already referenced for discovery in this path) before proceeding to Phase 5:

- **No uncommitted changes (everything already committed locally)** → run the secrets
  scan against `git diff <base>...HEAD` first (same hard-stop rules apply); if clean,
  skip commit-planning; derive the branch name and PR description from existing commit
  messages and `git diff <base>...HEAD`; push existing commits in Phase 4; proceed to Phase 5

Prompt To Fix With AI

This is a comment left during a code review.
Path: .claude/skills/ship/SKILL.md
Line: 79-81

Comment:
**"Already committed" path bypasses the secrets scan**

The edge-case branch at line 79 says "skip Phases 1–4", but Phase 1 is exactly where the secrets scan lives. Any commits that were made locally before `/ship` was invoked — and that contain `AKIA*`, `sk-*`, `ghp_*`, or similar tokens — are pushed to origin without ever hitting the hard-stop gate. This directly contradicts the guardrail at line 325 ("Never skip the secrets scan").

The fix is to still run the scan against `git diff <base>...HEAD` (already referenced for discovery in this path) before proceeding to Phase 5:

```markdown
- **No uncommitted changes (everything already committed locally)** → run the secrets
  scan against `git diff <base>...HEAD` first (same hard-stop rules apply); if clean,
  skip commit-planning; derive the branch name and PR description from existing commit
  messages and `git diff <base>...HEAD`; push existing commits in Phase 4; proceed to Phase 5
```

How can I resolve this? If you propose a fix, please make it concise.