feat(ship): adopt gstack-based ship and add land-and-deploy - #640
Conversation
Split ship lifecycle into two skills along the PR-merge boundary: /ship gates readiness up to merge, /land-and-deploy owns merge through deploy. Preserve gstack core principles (Boil the Lake, idempotent re-run, adaptive gating, scope-drift detection, revert escape hatch) and delegate self-run pieces to /qa, /e2e, /create-pr, /pr-pass, semantic-release. Track progress via TaskCreate; avoid per-task confirm round-trips that sank the #635 launch task runner. Source: https://github.com/garrytan/gstack ship/ and land-and-deploy/
Reads the most recent handoff file from ~/.claude/handoff/, quotes the Resume Prompt verbatim, surfaces Failed Approaches, and warns on worktree/branch mismatch before continuing. Pair to /handoff so the next session can resume without the user pasting a path. Tested via RED-GREEN-REFACTOR with subagents under pressure scenarios: closed loopholes around auto-picking by topic match, summarizing the Resume Prompt, and nudging the user past worktree mismatch.
📝 WalkthroughWalkthroughThis PR introduces architectural documentation and new skill specifications that formalize gstack's two-skill split: Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.md`:
- Line 78: Fix the malformed source link by adding the missing closing backtick
for the `land-and-deploy/SKILL.md` fragment in the line that currently reads
"<https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md>"; ensure the
backticked path is properly closed (i.e., `land-and-deploy/SKILL.md`) so the
Markdown renders/clicks correctly.
In `@plugins/me/skills/handoff/SKILL.md`:
- Line 14: Update the sentence in SKILL.md that describes the /pickup behavior
to clarify that it does not automatically choose the most recent file; instead,
when multiple handoff files exist in ~/.claude/handoff/, the /pickup command
prompts the user to explicitly select which file to resume from (otherwise it
can read the single available file or a pasted path/content). Modify the
phrasing around “/pickup” and “most recent” to reflect this explicit selection
flow so readers know selection is prompted when multiple candidates exist.
In `@plugins/me/skills/land-and-deploy/SKILL.md`:
- Around line 92-94: The recovery step doesn't capture the CI run-id used in
subsequent commands; update the workflow/step that runs wait-for-merge.sh to
parse and store the run-id (e.g., capture the script/gh output into a variable)
and then use that stored run-id when calling gh run view --log-failed and when
invoking the Skill: me:pr-pass recovery action, ensuring the run-id is preserved
across the recovery flow (refer to wait-for-merge.sh output handling and the
Skill: me:pr-pass invocation).
In `@plugins/me/skills/ship/references/review-checklist.md`:
- Line 10: Update the incorrect specialist reference path string: replace the
"See `review/specialists/` for these." reference in the checklist entry that
begins "**Specialist categories (handled by parallel subagents, NOT this
checklist):**" with "See `references/specialists/` for these."; also scan the
same file for any other occurrences of `review/specialists/` and correct them to
`references/specialists/` so links point to the actual specialist checklists.
In `@plugins/me/skills/ship/references/specialists/red-team.md`:
- Line 3: Update the trigger sentence in red-team.md so it matches /ship’s
condition: replace the existing line "Scope: When diff > 200 lines OR security
specialist found CRITICAL findings." with wording that says CRITICAL findings
from any specialist (not just security) — e.g., "Scope: When diff > 200 lines OR
any specialist reports CRITICAL findings." Ensure the file's trigger text now
explicitly references "any specialist" so the red-team trigger behavior aligns
with /ship.
In `@plugins/me/skills/ship/references/specialists/testing.md`:
- Around line 4-7: The schema in
plugins/me/skills/ship/references/specialists/testing.md is inconsistent about
the "line" field (first shows it as required in the JSON example but the
following text marks it optional); choose a single contract (make "line"
optional or required), update the JSON schema example and the descriptive text
to match, and propagate the same normalization to the other specialist checklist
files in this PR (ensure the "line" key, examples, and any mention of
optional/required for "line" are consistent across files such as the
"specialist":"testing" example).
In `@plugins/me/skills/ship/SKILL.md`:
- Line 295: The document contains a control-flow contradiction between the
directive "3. **After fixes:** if any fixes were applied, the pre-flight
evidence is now stale. **STOP** and tell the user to re-run `/ship` to
re-verify." and a later post-fix re-test path; make the behavior single-source
by removing or replacing the post-fix re-test path so that whenever "After
fixes" applies the agent always stops and instructs the user to re-run `/ship`
to re-verify (update the text that references the post-fix re-test to instead
point to the stop-and-rerun flow and ensure both "Step 8" and "Step 9"
references consistently follow the new stop-and-rerun rule).
🪄 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: 40b4abb4-0751-4ccd-90f6-a33a6e314f70
📒 Files selected for processing (15)
docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.mdplugins/me/skills/handoff/SKILL.mdplugins/me/skills/land-and-deploy/SKILL.mdplugins/me/skills/pickup/SKILL.mdplugins/me/skills/ship/SKILL.mdplugins/me/skills/ship/references/review-checklist.mdplugins/me/skills/ship/references/specialists/api-contract.mdplugins/me/skills/ship/references/specialists/data-migration.mdplugins/me/skills/ship/references/specialists/maintainability.mdplugins/me/skills/ship/references/specialists/performance.mdplugins/me/skills/ship/references/specialists/red-team.mdplugins/me/skills/ship/references/specialists/security.mdplugins/me/skills/ship/references/specialists/testing.mdplugins/me/skills/ship/references/test-triage.mdtests/me/me-specific.bats
| ## 출처 | ||
|
|
||
| - <https://github.com/garrytan/gstack> `ship/SKILL.md` | ||
| - <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md> |
There was a problem hiding this comment.
Fix malformed source link formatting
Line 78 is missing the closing backtick before > in land-and-deploy/SKILL.md, which breaks rendering/clickability of the reference.
Suggested patch
-- <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md>
+- <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md> | |
| - <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.md` at line 78,
Fix the malformed source link by adding the missing closing backtick for the
`land-and-deploy/SKILL.md` fragment in the line that currently reads
"<https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md>"; ensure the
backticked path is properly closed (i.e., `land-and-deploy/SKILL.md`) so the
Markdown renders/clicks correctly.
| Save the current session's context to `~/.claude/handoff/YYYY-MM-DD-HHmm-<topic>.md` as a structured markdown file. Runs only when the user explicitly asks for it. | ||
|
|
||
| There is no resume logic. The next session picks up context when the user pastes the file path or contents — Claude reads the frontmatter and the Resume Prompt section and takes it from there. | ||
| There is no resume logic in this skill. The next session picks up context either by invoking `/pickup` (which reads the most recent file from `~/.claude/handoff/`) or by pasting the file path or contents directly — Claude reads the frontmatter and the Resume Prompt section and takes it from there. |
There was a problem hiding this comment.
Clarify /pickup selection behavior to match actual flow.
This line currently implies automatic "most recent" selection, but /pickup prompts for explicit selection when multiple candidates exist. Please align wording to avoid resume-flow confusion.
Suggested wording
-There is no resume logic in this skill. The next session picks up context either by invoking `/pickup` (which reads the most recent file from `~/.claude/handoff/`) or by pasting the file path or contents directly — Claude reads the frontmatter and the Resume Prompt section and takes it from there.
+There is no resume logic in this skill. The next session picks up context either by invoking `/pickup` (which lists handoff files from `~/.claude/handoff/`, then reads the selected file) or by pasting the file path or contents directly — Claude reads the frontmatter and the Resume Prompt section and takes it from there.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| There is no resume logic in this skill. The next session picks up context either by invoking `/pickup` (which reads the most recent file from `~/.claude/handoff/`) or by pasting the file path or contents directly — Claude reads the frontmatter and the Resume Prompt section and takes it from there. | |
| There is no resume logic in this skill. The next session picks up context either by invoking `/pickup` (which lists handoff files from `~/.claude/handoff/`, then reads the selected file) or by pasting the file path or contents directly — Claude reads the frontmatter and the Resume Prompt section and takes it from there. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/handoff/SKILL.md` at line 14, Update the sentence in
SKILL.md that describes the /pickup behavior to clarify that it does not
automatically choose the most recent file; instead, when multiple handoff files
exist in ~/.claude/handoff/, the /pickup command prompts the user to explicitly
select which file to resume from (otherwise it can read the single available
file or a pasted path/content). Modify the phrasing around “/pickup” and “most
recent” to reflect this explicit selection flow so readers know selection is
prompted when multiple candidates exist.
| If `wait-for-merge.sh` exits non-zero with a CI failure, run `gh run view <run-id> | ||
| --log-failed`, invoke `Skill: me:pr-pass` to repair, then re-enable auto-merge. | ||
| Permission denied on merge → STOP, ask the user (branch protection / maintainer |
There was a problem hiding this comment.
Capture CI run-id before invoking /pr-pass recovery
Line 92 uses <run-id> but Step 3 never stores it. On CI failure, this leaves the recovery branch without the required identifier.
Suggested patch
-"$S/wait-for-merge.sh" # run_in_background:true; 0=merged, 1=CI fail
+WAIT_LOG="$(mktemp)"
+if ! "$S/wait-for-merge.sh" 2> >(tee "$WAIT_LOG" >&2); then
+ RUN_ID="$(grep -oE 'run-id: [0-9]+' "$WAIT_LOG" | awk '{print $2}' | tail -1)"
+ if [ -n "$RUN_ID" ]; then
+ gh run view "$RUN_ID" --log-failed
+ fi
+ # invoke Skill: me:pr-pass, then re-enable auto-merge
+fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/land-and-deploy/SKILL.md` around lines 92 - 94, The
recovery step doesn't capture the CI run-id used in subsequent commands; update
the workflow/step that runs wait-for-merge.sh to parse and store the run-id
(e.g., capture the script/gh output into a variable) and then use that stored
run-id when calling gh run view --log-failed and when invoking the Skill:
me:pr-pass recovery action, ensuring the run-id is preserved across the recovery
flow (refer to wait-for-merge.sh output handling and the Skill: me:pr-pass
invocation).
| 2. **Choose a file** — strict rules: | ||
| - **0 candidates** → stop (above). | ||
| - **1 candidate** → use it. | ||
| - **2+ candidates** → use AskUserQuestion (load via ToolSearch if it's a deferred tool: `ToolSearch select:AskUserQuestion`; if unavailable, ask inline). Show top 5 with `<filename> — <first-line title>`. **Never auto-pick** by branch name match, topic match, or "most recent." The user picks. |
There was a problem hiding this comment.
Remove ToolSearch dependency or add it to allowed-tools
Lines 47 and 62 require ToolSearch, but this skill’s frontmatter does not allow it. That creates an execution path that the skill cannot legally follow.
Suggested patch (no ToolSearch dependency)
- - **2+ candidates** → use AskUserQuestion (load via ToolSearch if it's a deferred tool: `ToolSearch select:AskUserQuestion`; if unavailable, ask inline). Show top 5 with `<filename> — <first-line title>`. **Never auto-pick** by branch name match, topic match, or "most recent." The user picks.
+ - **2+ candidates** → use AskUserQuestion. Show top 5 with `<filename> — <first-line title>`. **Never auto-pick** by branch name match, topic match, or "most recent." The user picks.
...
- - **Stop and ask neutrally.** Use AskUserQuestion (load it via ToolSearch first if it appears as a deferred tool — `ToolSearch select:AskUserQuestion`; if it can't be loaded, ask inline as a single plain question). Question text must offer the three options and **nothing else** — no editorial like "the topic matches so we're probably fine here." Suggested phrasing:
+ - **Stop and ask neutrally.** Use AskUserQuestion as a single plain question. Question text must offer the three options and **nothing else** — no editorial like "the topic matches so we're probably fine here." Suggested phrasing:Also applies to: 62-62
| **Two-pass review:** | ||
| - **Pass 1 (CRITICAL):** Run SQL & Data Safety, Race Conditions, LLM Output Trust Boundary, Shell Injection, and Enum Completeness first. Highest severity. | ||
| - **Pass 2 (INFORMATIONAL):** Run remaining categories below. Lower severity but still actioned. | ||
| - **Specialist categories (handled by parallel subagents, NOT this checklist):** Test Gaps, Dead Code, Magic Numbers, Conditional Side Effects, Performance & Bundle Impact, Crypto & Entropy. See `review/specialists/` for these. |
There was a problem hiding this comment.
Fix incorrect specialist reference path
Line 10 references review/specialists/, but the specialist checklists in this skill live under references/specialists/. This can send reviewers/agents to a non-existent location and break execution flow.
Suggested patch
-- **Specialist categories (handled by parallel subagents, NOT this checklist):** Test Gaps, Dead Code, Magic Numbers, Conditional Side Effects, Performance & Bundle Impact, Crypto & Entropy. See `review/specialists/` for these.
+- **Specialist categories (handled by parallel subagents, NOT this checklist):** Test Gaps, Dead Code, Magic Numbers, Conditional Side Effects, Performance & Bundle Impact, Crypto & Entropy. See `references/specialists/` for these.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Specialist categories (handled by parallel subagents, NOT this checklist):** Test Gaps, Dead Code, Magic Numbers, Conditional Side Effects, Performance & Bundle Impact, Crypto & Entropy. See `review/specialists/` for these. | |
| - **Specialist categories (handled by parallel subagents, NOT this checklist):** Test Gaps, Dead Code, Magic Numbers, Conditional Side Effects, Performance & Bundle Impact, Crypto & Entropy. See `references/specialists/` for these. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/ship/references/review-checklist.md` at line 10, Update the
incorrect specialist reference path string: replace the "See
`review/specialists/` for these." reference in the checklist entry that begins
"**Specialist categories (handled by parallel subagents, NOT this checklist):**"
with "See `references/specialists/` for these."; also scan the same file for any
other occurrences of `review/specialists/` and correct them to
`references/specialists/` so links point to the actual specialist checklists.
| @@ -0,0 +1,45 @@ | |||
| # Red Team Review | |||
|
|
|||
| Scope: When diff > 200 lines OR security specialist found CRITICAL findings. Runs AFTER other specialists. | |||
There was a problem hiding this comment.
Align red-team trigger text with /ship’s actual condition.
This currently narrows the trigger to security-only CRITICAL findings, while /ship uses CRITICAL from any specialist.
Suggested wording
-Scope: When diff > 200 lines OR security specialist found CRITICAL findings. Runs AFTER other specialists.
+Scope: When diff > 200 lines OR any specialist found CRITICAL findings. Runs AFTER other specialists.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Scope: When diff > 200 lines OR security specialist found CRITICAL findings. Runs AFTER other specialists. | |
| Scope: When diff > 200 lines OR any specialist found CRITICAL findings. Runs AFTER other specialists. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/ship/references/specialists/red-team.md` at line 3, Update
the trigger sentence in red-team.md so it matches /ship’s condition: replace the
existing line "Scope: When diff > 200 lines OR security specialist found
CRITICAL findings." with wording that says CRITICAL findings from any specialist
(not just security) — e.g., "Scope: When diff > 200 lines OR any specialist
reports CRITICAL findings." Ensure the file's trigger text now explicitly
references "any specialist" so the red-team trigger behavior aligns with /ship.
| Output: JSON objects, one finding per line. Schema: | ||
| {"severity":"CRITICAL|INFORMATIONAL","confidence":N,"path":"file","line":N,"category":"testing","summary":"...","fix":"...","fingerprint":"path:line:testing","specialist":"testing"} | ||
| Optional: line, fix, fingerprint, evidence, test_stub. | ||
| If no findings: output `NO FINDINGS` and nothing else. |
There was a problem hiding this comment.
Make the finding contract internally consistent (line required vs optional).
The schema example marks line as required, but the next line says it’s optional. This ambiguity can cause inconsistent JSON outputs across specialist agents.
Suggested contract normalization
Output: JSON objects, one finding per line. Schema:
-{"severity":"CRITICAL|INFORMATIONAL","confidence":N,"path":"file","line":N,"category":"testing","summary":"...","fix":"...","fingerprint":"path:line:testing","specialist":"testing"}
-Optional: line, fix, fingerprint, evidence, test_stub.
+{"severity":"CRITICAL|INFORMATIONAL","confidence":N,"path":"file","category":"testing","summary":"...","specialist":"testing"}
+Optional: line, fix, fingerprint, evidence, test_stub.Apply the same normalization to the other new specialist checklist files in this PR to keep the contract uniform.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Output: JSON objects, one finding per line. Schema: | |
| {"severity":"CRITICAL|INFORMATIONAL","confidence":N,"path":"file","line":N,"category":"testing","summary":"...","fix":"...","fingerprint":"path:line:testing","specialist":"testing"} | |
| Optional: line, fix, fingerprint, evidence, test_stub. | |
| If no findings: output `NO FINDINGS` and nothing else. | |
| Output: JSON objects, one finding per line. Schema: | |
| {"severity":"CRITICAL|INFORMATIONAL","confidence":N,"path":"file","category":"testing","summary":"...","specialist":"testing"} | |
| Optional: line, fix, fingerprint, evidence, test_stub. | |
| If no findings: output `NO FINDINGS` and nothing else. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/ship/references/specialists/testing.md` around lines 4 - 7,
The schema in plugins/me/skills/ship/references/specialists/testing.md is
inconsistent about the "line" field (first shows it as required in the JSON
example but the following text marks it optional); choose a single contract
(make "line" optional or required), update the JSON schema example and the
descriptive text to match, and propagate the same normalization to the other
specialist checklist files in this PR (ensure the "line" key, examples, and any
mention of optional/required for "line" are consistent across files such as the
"specialist":"testing" example).
| - Each item: number, severity, problem, recommended fix, per-item options A) Fix B) Skip | ||
| - Overall recommendation | ||
| - 3 or fewer ASK items → individual `AskUserQuestion` calls allowed | ||
| 3. **After fixes:** if any fixes were applied, the pre-flight evidence is now stale. **STOP** and tell the user to re-run `/ship` to re-verify. If no fixes applied, continue. |
There was a problem hiding this comment.
Resolve Step 8/Step 9 control-flow contradiction
Line 295 mandates stopping whenever Step 8 applies fixes, but Line 307 defines a post-fix re-test path before handoff. These two instructions conflict and can produce inconsistent agent behavior.
Suggested patch (single-source behavior: always stop after fixes)
-1. If ANY code changed in Step 8 (auto-fixes), re-run the project test command. Stale output is not acceptable.
+1. This step runs only when Step 8 made no code changes. If Step 8 changed code, `/ship` must already have stopped and requested a re-run.Also applies to: 307-307
🧰 Tools
🪛 LanguageTool
[style] ~295-~295: Consider using the more polite verb “ask” (“tell” implies ordering/instructing someone).
Context: ...ght evidence is now stale. STOP and tell the user to re-run /ship to re-verify...
(TELL_ASK)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/me/skills/ship/SKILL.md` at line 295, The document contains a
control-flow contradiction between the directive "3. **After fixes:** if any
fixes were applied, the pre-flight evidence is now stale. **STOP** and tell the
user to re-run `/ship` to re-verify." and a later post-fix re-test path; make
the behavior single-source by removing or replacing the post-fix re-test path so
that whenever "After fixes" applies the agent always stops and instructs the
user to re-run `/ship` to re-verify (update the text that references the
post-fix re-test to instead point to the stop-and-rerun flow and ensure both
"Step 8" and "Step 9" references consistently follow the new stop-and-rerun
rule).
Summary
/ship을 gstack 기반 readiness gate로 재작업 (이전 #639의 lightweight 버전 대체)./land-and-deploy스킬 신설 — PR 머지 → CI 대기 → 배포 추론·실행 → canary → revert escape./pickup스킬 추가 (/handoff짝).Skills boundary
/ship/land-and-deploy위임 매핑
/qa/e2e/create-pr/create-pr의wait-for-merge.sh/pr-passgstack에서 채택한 원칙
Boil the Lake / Idempotent re-run / Non-blocking by default / Confidence Calibration / Adaptive Gating / Scope Drift Detection / Revert as escape hatch.
진행 단계는 TaskCreate로 추적하되 per-task confirm 왕복은 금지 (#635 함정 회피).
Migration
/land-and-deploy에 migration 단계 추가하지 않음 (gstack 원본 그대로). schema-data 안전성은/ship에서 평가.Test plan
bats tests/me/me-specific.bats— 10/10 PASS (ship references, land-and-deploy frontmatter 신규 검증 포함)/shipinvocation으로 새 readiness flow 확인/land-and-deployinvocation 시도 (bstack 자체엔 배포 인프라 없으므로 수동 검증 한계)Design doc
docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.mdReferences
Summary by CodeRabbit
New Features
/land-and-deployworkflow for automated post-merge deployment verification and production health checks/pickupskill for seamless session continuation and handoff retrieval/shipwith comprehensive pre-landing verification gates and specialist-driven reviewsDocumentation