Skip to content

feat(ship): adopt gstack-based ship and add land-and-deploy - #640

Merged
baleen37 merged 2 commits into
mainfrom
feat/improve-ship
May 1, 2026
Merged

feat(ship): adopt gstack-based ship and add land-and-deploy#640
baleen37 merged 2 commits into
mainfrom
feat/improve-ship

Conversation

@baleen37

@baleen37 baleen37 commented May 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • /shipgstack 기반 readiness gate로 재작업 (이전 #639의 lightweight 버전 대체).
  • /land-and-deploy 스킬 신설 — PR 머지 → CI 대기 → 배포 추론·실행 → canary → revert escape.
  • A2(위임) 구조: gstack 본문/원칙 보존, 자체 수행 부분만 우리 스킬로 치환.
  • 보너스: /pickup 스킬 추가 (/handoff 짝).

Skills boundary

스킬 책임
/ship readiness 판정 + PR 생성까지
/land-and-deploy PR 머지 + 배포 + 검증

위임 매핑

  • 테스트 → /qa
  • E2E → /e2e
  • PR 생성 → /create-pr
  • 머지 + CI 대기 → /create-prwait-for-merge.sh
  • CI 실패 복구 → /pr-pass
  • VERSION/CHANGELOG → semantic-release (자동)

gstack에서 채택한 원칙

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 신규 검증 포함)
  • pre-commit hooks (yaml/json/EOF/trailing/BATS) PASS
  • markdownlint MD013/MD032 위반 — 레포 전체 패턴과 동일, 별도 PR 권장
  • 실제 /ship invocation으로 새 readiness flow 확인
  • 실제 /land-and-deploy invocation 시도 (bstack 자체엔 배포 인프라 없으므로 수동 검증 한계)

Design doc

docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.md

References

Summary by CodeRabbit

  • New Features

    • Introduced /land-and-deploy workflow for automated post-merge deployment verification and production health checks
    • Added /pickup skill for seamless session continuation and handoff retrieval
    • Enhanced /ship with comprehensive pre-landing verification gates and specialist-driven reviews
  • Documentation

    • Added detailed specification documents for multi-stage deployment workflows
    • Introduced specialized review checklists covering security, performance, testing, and maintainability assessment

Test User added 2 commits May 1, 2026 22:14
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.
@coderabbitai

coderabbitai Bot commented May 1, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR introduces architectural documentation and new skill specifications that formalize gstack's two-skill split: /ship handles pre-merge readiness evaluation and delegates actions, while /land-and-deploy manages post-merge deployment and health verification. New skills /pickup and updated /handoff enable context resumption across sessions. Specialist review checklists and process documentation define detailed review criteria and workflows.

Changes

Cohort / File(s) Summary
Architectural Specification
docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.md
New specification document formalizing the two-skill split, delegation mapping, core principles preservation, and known issues.
Ship Skill Rewrite
plugins/me/skills/ship/SKILL.md
Transforms /ship from a review gate into an automated, idempotent workflow with task tracking, scope-drift detection, specialist dispatch, fix-first pipeline, and delegated PR handoff via /create-pr.
Ship Specialist References
plugins/me/skills/ship/references/specialists/{api-contract, data-migration, maintainability, performance, red-team, security, testing}.md
Seven new specialist review checklists defining scope, JSON output format, and category-specific evaluation criteria for API contracts, migrations, maintainability, performance, adversarial analysis, security, and testing.
Ship Process References
plugins/me/skills/ship/references/{review-checklist, test-triage}.md
New standardized two-pass pre-landing checklist with security/safety checks and a triage guide for handling failing tests during verification.
New Skills
plugins/me/skills/land-and-deploy/SKILL.md, plugins/me/skills/pickup/SKILL.md
Two new skill specifications: /land-and-deploy handles post-merge CI gating, deployment waiting, canary verification, and revert escape hatches; /pickup enforces handoff retrieval, context validation, drift detection, and resume continuation.
Handoff Skill Update
plugins/me/skills/handoff/SKILL.md
Minor documentation update clarifying automatic context pickup via /pickup and alternate resume entrypoint behavior.
Test Updates
tests/me/me-specific.bats
New assertions verifying /ship skill constraints, reference documentation file existence, and /land-and-deploy skill frontmatter validation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 From ship to land, we choreograph the dance,
Each skill now knows its place, no overlap's chance,
New pickup springs up, remembering where we've been,
Specialists review with rigor keen,
Two-skill harmony—deploy with confidence! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(ship): adopt gstack-based ship and add land-and-deploy' is directly related to the PR's main objective of reworking /ship and introducing /land-and-deploy, as confirmed by the PR objectives and file summaries.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/improve-ship

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 7c04a53 and 9a333fb.

📒 Files selected for processing (15)
  • docs/superpowers/specs/2026-05-01-ship-land-and-deploy-design.md
  • plugins/me/skills/handoff/SKILL.md
  • plugins/me/skills/land-and-deploy/SKILL.md
  • plugins/me/skills/pickup/SKILL.md
  • plugins/me/skills/ship/SKILL.md
  • plugins/me/skills/ship/references/review-checklist.md
  • plugins/me/skills/ship/references/specialists/api-contract.md
  • plugins/me/skills/ship/references/specialists/data-migration.md
  • plugins/me/skills/ship/references/specialists/maintainability.md
  • plugins/me/skills/ship/references/specialists/performance.md
  • plugins/me/skills/ship/references/specialists/red-team.md
  • plugins/me/skills/ship/references/specialists/security.md
  • plugins/me/skills/ship/references/specialists/testing.md
  • plugins/me/skills/ship/references/test-triage.md
  • tests/me/me-specific.bats

## 출처

- <https://github.com/garrytan/gstack> `ship/SKILL.md`
- <https://github.com/garrytan/gstack> `land-and-deploy/SKILL.md>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
- <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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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.

Comment on lines +92 to +94
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
- **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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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.

Comment on lines +4 to +7
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Suggested change
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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).

@baleen37
baleen37 merged commit 8678a6e into main May 1, 2026
5 checks passed
@baleen37
baleen37 deleted the feat/improve-ship branch May 1, 2026 14:38
baleen-release-app Bot pushed a commit that referenced this pull request May 1, 2026
# [16.19.0](v16.18.0...v16.19.0) (2026-05-01)

### Features

* **ship:** adopt gstack-based ship and add land-and-deploy ([#640](#640)) ([8678a6e](8678a6e)), closes [#635](#635)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant