Skip to content

Commit 2977143

Browse files
tahierhussainclaude
andcommitted
fix(claude): address /ship skill review feedback
- Drop unused Edit from allowed-tools. - Document the Co-Authored-By trailer in commit-format conventions. - Tighten the AWS secret-key scan to assignment + 40-char value so bare references in .env.example don't trip the gate. - Reorder Phase 6 so gh auth status runs before any temp-file write, add an existing-PR check that stops with the URL surfaced (no silent gh pr create failure on re-runs), make the title prefix derive from classification instead of hard-coded FEAT, and switch cleanup to rm -f. - Require every '-' placeholder from the PR-body template to be replaced with prose or a literal TODO before raising. - Update Section 9 to describe the existing-PR stop behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f19c21b commit 2977143

1 file changed

Lines changed: 66 additions & 19 deletions

File tree

.claude/skills/ship/SKILL.md

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
name: ship
33
description: Create a branch, atomic commits, push, and open a draft PR using the project template
44
disable-model-invocation: true
5-
allowed-tools: Bash, Read, Edit, AskUserQuestion
5+
allowed-tools: Bash, Read, AskUserQuestion
66
---
77

88
# /ship
@@ -28,6 +28,12 @@ just the files touched. E.g., `feat/google-oauth-signin`, not `feat/update-auth-
2828
**Commit format:** Conventional Commits — `<type>(<scope>): <subject>`.
2929
Types used: `feat`, `fix`, `refactor`, `chore`, `docs`, `test`.
3030

31+
**Commit trailer:** Every commit message ends with the trailer
32+
`Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>`,
33+
separated from the body by a blank line, per the global protocol. Pass the
34+
full message — subject, blank line, body, blank line, trailer — through a
35+
HEREDOC.
36+
3137
**Commit grouping rules:**
3238
- One logical concern per commit
3339
- Refactors separate from new behavior
@@ -47,7 +53,18 @@ Types used: `feat`, `fix`, `refactor`, `chore`, `docs`, `test`.
4753

4854
### Phase 1 — Discover and classify
4955

50-
Run `git status` and `git diff` to inventory uncommitted changes.
56+
Inventory **all** pending changes — both staged and unstaged — so nothing
57+
sneaks past the scan or the plan:
58+
- `git status` for the file list
59+
- `git diff HEAD` for the full content diff (covers staged + unstaged)
60+
- `git diff --cached --name-only` to identify files that were already in the
61+
index before `/ship` was invoked
62+
63+
Any files already staged before invocation must be either explicitly included
64+
in the commit plan or unstaged before Phase 4 — otherwise they will be silently
65+
swept into the first `git commit`. If pre-staged files exist that don't fit
66+
the plan, surface the list and ask the user whether to include or unstage them
67+
before proceeding.
5168

5269
Classify the work as **feature** or **non-feature**:
5370
- *Feature signals:* new files (routes, components, endpoints), new exports, additions
@@ -66,13 +83,23 @@ Classify the work as **feature** or **non-feature**:
6683
- On `main`/`master`/`develop` with **no changes at all** → stop with a clear message
6784
- Diff spans clearly unrelated areas → stop and ask whether to split
6885

69-
**Secrets scan (hard stop):** grep the diff for high-entropy strings and known key
70-
prefixes — `sk-`, `AKIA`, `ghp_`, `gho_`, `xoxb-`, `xoxp-`, `-----BEGIN ` (private
71-
keys), AWS-style `aws_secret_access_key`, etc. On hit:
86+
**Secrets scan (hard stop):** run the grep against `git diff HEAD` (so both
87+
staged and unstaged content are inspected) for high-entropy strings and known
88+
key prefixes — `sk-`, `AKIA`, `ghp_`, `gho_`, `xoxb-`, `xoxp-`, `-----BEGIN `
89+
(private keys), and an AWS secret-key assignment regex
90+
`aws_secret_access_key\s*[=:]\s*['"]?[A-Za-z0-9/+=]{40}['"]?` (case-insensitive
91+
— anchors on assignment + a 40-char value so bare references in `.env.example`
92+
don't trip the gate). On hit:
7293
1. Print the matched line and the file it came from
73-
2. Use AskUserQuestion to offer:
74-
- **Unstage the file** from this ship (`git restore --staged <path>`)
75-
- **Skip the file from this PR** (leave staged but exclude from commits)
94+
2. Use AskUserQuestion to offer (both options must call
95+
`git restore --staged <path>` first — leaving the file staged means it
96+
*will* be included in the next commit, regardless of which paths you
97+
`git add` afterwards):
98+
- **Drop from this PR, keep in working tree**
99+
`git restore --staged <path>` (file remains on disk for a future ship)
100+
- **Drop from this PR and remove from working tree**
101+
`git restore --staged <path>` then `rm <path>` (or `git rm <path>` if
102+
it was tracked) so the secret is no longer present anywhere
76103
- **Cancel ship entirely**
77104
3. Do not proceed past Phase 1 until the user picks one
78105

@@ -106,7 +133,8 @@ After confirmation:
106133
2. For each planned commit:
107134
- Stage the relevant files: `git add <path>...` (use `-p` only when a single file
108135
legitimately needs to be split across commits)
109-
- Commit with a Conventional Commits message via HEREDOC
136+
- Commit with a Conventional Commits message via HEREDOC. The HEREDOC
137+
body must include the `Co-Authored-By` trailer documented in Section 1.
110138
3. `git push -u origin <branch-name>` (skip if already pushed and up to date)
111139

112140
Hard rules:
@@ -126,30 +154,47 @@ Fill each section per the source-of-truth mapping in Section 3.
126154
Use **descriptive, full-sentence prose** — telegraph bullets are not acceptable.
127155
Use `TODO` as a marker for any section that can't be filled confidently.
128156

157+
Every `-` placeholder from the template must be replaced — either with a
158+
filled, prose answer or with a literal `TODO`. A bare `-` is never an
159+
acceptable final value in the rendered body.
160+
129161
**Render the full PR body and show it to the user. Wait for confirmation** before raising. Phrase the prompt as a natural question (e.g., "Ready to open the draft PR?"), not as "Confirmation Gate."
130162

131163
### Phase 6 — Raise the PR
132164

133-
1. Sanitize the branch name (the `feat/`/`fix/` prefix contains a `/`, which
165+
1. Check `gh auth status`. If it fails, instruct the user to run
166+
`gh auth login` and stop. Do not retry silently — and do not proceed to
167+
any of the steps below, so no temp file is left behind.
168+
2. Check whether a PR for this branch already exists:
169+
```bash
170+
EXISTING_PR=$(gh pr view --json url --jq '.url' 2>/dev/null || true)
171+
```
172+
If `EXISTING_PR` is non-empty, stop with a clear message: print the
173+
existing PR URL and tell the user that `/ship` does not currently update
174+
existing PRs (see Section 9). Suggest they push further commits manually
175+
with `git push` and edit the PR description in the GitHub UI. Do not
176+
write the temp file. Do not invoke `gh pr create`.
177+
3. Sanitize the branch name (the `feat/`/`fix/` prefix contains a `/`, which
134178
would create a non-existent subdirectory under `/tmp`):
135179
```bash
136180
BRANCH_SAFE=$(echo "<branch-name>" | tr '/' '-')
137181
# e.g. feat/google-oauth-signin → feat-google-oauth-signin
138182
```
139-
2. Write the rendered body to `/tmp/pr-body-$BRANCH_SAFE.md`
140-
3. Run:
183+
4. Write the rendered body to `/tmp/pr-body-$BRANCH_SAFE.md`
184+
5. Run:
141185
```bash
142-
# Title prefix: "FEAT: <summary>" for features, "FIX: <summary>" otherwise
186+
# Pick the prefix from the Phase 1 classification:
187+
# feature → TITLE_PREFIX="FEAT"
188+
# non-feature → TITLE_PREFIX="FIX"
189+
TITLE_PREFIX=<FEAT|FIX> # set per classification
143190
BASE=$(gh repo view --json defaultBranchRef --jq '.defaultBranchRef.name')
144191
gh pr create --draft \
145-
--title "FEAT: <summary>" \
192+
--title "${TITLE_PREFIX}: <imperative summary>" \
146193
--body-file "/tmp/pr-body-$BRANCH_SAFE.md" \
147194
--base "$BASE"
148195
```
149-
4. If `gh` is not authenticated (`gh auth status` fails), instruct the user to run
150-
`gh auth login` and stop. Do not retry silently
151-
5. Capture stdout, extract and print the PR URL
152-
6. Clean up the temp file (`rm "/tmp/pr-body-$BRANCH_SAFE.md"`)
196+
6. Capture stdout, extract and print the PR URL
197+
7. Clean up the temp file (`rm -f "/tmp/pr-body-$BRANCH_SAFE.md"`)
153198

154199
---
155200

@@ -289,7 +334,9 @@ prompts at each gate, and the final summary after the PR is opened.
289334
## 9. Out of scope (v1)
290335

291336
Explicitly not handled — keep the skill focused:
292-
- Updating an existing PR (force-push to same branch + edit body)
337+
- Updating an existing PR. If `/ship` detects a PR already exists for the
338+
current branch, it stops and surfaces the URL — pushing further commits
339+
and editing the description must be done manually.
293340
- Pre-push lint/test enforcement
294341
- Auto-merging or marking ready-for-review
295342
- Cross-repo or fork-based PR flows

0 commit comments

Comments
 (0)