Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# Lifecycle Skills Upstream Alignment Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or
> superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Strengthen the existing `test`, `review`, and `ship` skills with upstream-aligned test/review/launch practices
while preserving the current bstack lifecycle workflow.

**Architecture:** Keep the existing `plugins/me/skills/test`, `plugins/me/skills/review`, and `plugins/me/skills/ship`
skill entry points. Add clearer workflow gates and cross-links to existing local skills instead of adding duplicate
upstream skills.

**Tech Stack:** Markdown skill files with YAML frontmatter, bstack `me` plugin lifecycle documentation,
Bun/BATS/pre-commit verification.

---

## File Structure

- Modify: `plugins/me/skills/test/SKILL.md` — add stronger TDD, focused testing, browser/runtime verification, and
evidence rules.
- Modify: `plugins/me/skills/review/SKILL.md` — add severity criteria, simplification guidance, security escalation, and
review evidence rules.
- Modify: `plugins/me/skills/ship/SKILL.md` — add stronger production readiness gates for CI, observability, staged
rollout, rollback, and explicit NO-GO defaults.
- Modify: `plugins/me/README.md` — only if the skill descriptions need small wording updates after the three skill files
change.

### Task 1: Strengthen `/test`

**Files:**

- Modify: `plugins/me/skills/test/SKILL.md`

- [ ] **Step 1: Read the existing test skill**

Run: `sed -n '1,220p' plugins/me/skills/test/SKILL.md`
Expected: Existing `/test` workflow is visible and still uses YAML frontmatter with `disable-model-invocation: true`.

- [ ] **Step 2: Update testing principles**

Replace the short preference list with explicit principles covering failing tests first, observable outcomes, focused
commands, browser/runtime checks for UI, and exact evidence.

- [ ] **Step 3: Add workflow gates**

Ensure the workflow says bugs should be reproduced before fixing when feasible, implementation should be minimal,
focused tests run before broader suites, and final output includes commands and results.

- [ ] **Step 4: Link existing local skills**

Keep the skill local-first by linking `debugging-and-error-recovery`, `browse`, `qa`, `verify`, and `e2e` instead of
adding duplicate upstream skills.

- [ ] **Step 5: Verify markdown shape**

Run: `grep -n "^## \|^# /test\|debugging-and-error-recovery\|browse\|qa\|verify\|e2e" plugins/me/skills/test/SKILL.md`
Expected: The updated sections and local skill links are present.

### Task 2: Strengthen `/review`

**Files:**

- Modify: `plugins/me/skills/review/SKILL.md`

- [ ] **Step 1: Read the existing review skill**

Run: `sed -n '1,240p' plugins/me/skills/review/SKILL.md`
Expected: Existing `/review` workflow and final report format are visible.

- [ ] **Step 2: Add review severity rules**

Add clear definitions for blockers and non-blocking suggestions so review output does not mix correctness risks with
style preferences.

- [ ] **Step 3: Add simplification and security escalation**

Add rules to flag unnecessary abstraction, large speculative changes, secrets, auth/authz, injection, dependency/config
risks, and to use `security-auditor` for sensitive changes.

- [ ] **Step 4: Preserve subagent fan-out guidance**

Keep `code-reviewer`, `security-auditor`, and `test-engineer` guidance, but make their responsibilities more precise and
findings-only.

- [ ] **Step 5: Verify markdown shape**

Run: `grep -n "Severity\|Blocking\|Non-blocking\|security-auditor\|test-engineer\|code-reviewer"
plugins/me/skills/review/SKILL.md`
Expected: Severity rules, output format, and subagent references are present.

### Task 3: Strengthen `/ship`

**Files:**

- Modify: `plugins/me/skills/ship/SKILL.md`

- [ ] **Step 1: Read the existing ship skill**

Run: `sed -n '1,260p' plugins/me/skills/ship/SKILL.md`
Expected: Existing fan-out launch review and GO/NO-GO output format are visible.

- [ ] **Step 2: Add launch readiness gates**

Add explicit checks for CI status, migrations/config/env, feature flags, monitoring, rollback trigger/procedure, staged
rollout, documentation, and post-launch verification.

- [ ] **Step 3: Tighten GO/NO-GO defaults**

Ensure any Critical security finding, failing required test/build/check, missing rollback plan, or unverifiable
production risk defaults to NO-GO unless the user explicitly accepts the risk.

- [ ] **Step 4: Preserve parallel specialist fan-out**

Keep the requirement that `code-reviewer`, `security-auditor`, and `test-engineer` run in parallel for non-trivial
production-bound changes.

- [ ] **Step 5: Verify markdown shape**

Run: `grep -n "GO | NO-GO\|rollback\|monitoring\|staged\|CI\|security-auditor\|test-engineer"
plugins/me/skills/ship/SKILL.md`
Expected: Decision output, rollback, launch gates, and specialist references are present.

### Task 4: Documentation and repository verification

**Files:**

- Modify only if needed: `plugins/me/README.md`

- [ ] **Step 1: Check whether README descriptions still match**

Run: `grep -n "test\|review\|ship\|shipping-and-launch\|ci-cd-and-automation" plugins/me/README.md`
Expected: Existing lifecycle descriptions still accurately describe the updated skills.

- [ ] **Step 2: Update README only for factual mismatch**

If a description is stale, make the smallest wording-only edit. If descriptions remain accurate, do not edit README.

- [ ] **Step 3: Run focused markdown validation**

Run: `pre-commit run markdownlint --files plugins/me/skills/test/SKILL.md plugins/me/skills/review/SKILL.md
plugins/me/skills/ship/SKILL.md docs/superpowers/plans/2026-05-12-lifecycle-skills-upstream-alignment.md`
Expected: PASS, or only actionable markdown issues that should be fixed.

- [ ] **Step 4: Run project-required verification if available**

Run: `bats tests/`
Expected: PASS. If BATS is unavailable in the environment, report the exact command failure and do not claim full
verification.

- [ ] **Step 5: Check git diff**

Run: `git diff -- plugins/me/skills/test/SKILL.md plugins/me/skills/review/SKILL.md plugins/me/skills/ship/SKILL.md
plugins/me/README.md docs/superpowers/plans/2026-05-12-lifecycle-skills-upstream-alignment.md`
Expected: Diff only contains the planned lifecycle skill alignment changes.

## Self-Review

- Spec coverage: The plan covers `/test`, `/review`, `/ship`, local skill cross-links, optional README alignment, and
verification.
- Placeholder scan: No TBD/TODO/later placeholders remain.
- Type consistency: This plan edits Markdown skill files only; all referenced paths match existing project structure
except the new plan file itself.
42 changes: 30 additions & 12 deletions plugins/me/skills/review/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
---
name: review
description: Review code changes for correctness, readability, architecture, security, performance, and test coverage using subagents.
description: >-
Review code changes for correctness, readability, architecture, security, performance, and test coverage using
subagents.
disable-model-invocation: true
---

Expand All @@ -11,7 +13,8 @@ disable-model-invocation: true
Use `/review` when the user explicitly asks to review a change, PR, branch, diff, or implementation.
This is an explicit workflow skill: it should run only when the user invokes it.

Review in this priority order:
Review in this priority order. Correctness, security, and test coverage findings take precedence over style-only
concerns:

1. Correctness
2. Security and data safety
Expand All @@ -20,37 +23,52 @@ Review in this priority order:
5. Architecture fit
6. Performance

## Severity

- **Blocking:** A correctness bug, security risk, data loss risk, missing required test coverage, broken contract,
regression, or deploy/runtime failure that should be fixed before merge.
- **Non-blocking:** A suggestion, style issue, readability improvement, minor refactor, or optional follow-up that does
not make the change unsafe or incorrect.
- Treat unnecessary abstractions, speculative changes, and avoidable complexity as quality findings. Mark them blocking
only when they materially obscure correctness, increase risk, or make the change hard to verify.

## Workflow

1. Inspect the diff and changed files.
2. Understand the intended behavior before judging the code.
3. Check correctness issues and missing edge cases.
4. Check tests for meaningful coverage.
5. Check security-sensitive paths.
6. Check maintainability and fit with local conventions.
7. Separate blocking issues from suggestions.
8. Provide concise findings with file paths and reasoning.
5. Check security-sensitive paths. If the diff touches secrets, authentication, authorization, injection surfaces,
dependencies, or configuration, use `security-auditor`.
6. Check maintainability and fit with local conventions, including needless abstraction, speculative change, and
excessive complexity.
7. Separate blocking issues from non-blocking suggestions.
8. Provide concise findings with `file:line` evidence, impact, and an actionable recommendation.

## Subagent use

For non-trivial reviews, dispatch subagents in parallel:
For non-trivial reviews, dispatch relevant subagents in parallel. Include `security-auditor` when the diff touches
secrets, authentication, authorization, injection surfaces, dependencies, or configuration:

- `code-reviewer`: correctness, maintainability, architecture, readability, and local convention fit.
- `security-auditor`: authentication, authorization, secrets, injection, unsafe file or network access, dependency and configuration risks.
- `code-reviewer`: correctness, maintainability, architecture, readability, unnecessary abstraction, speculative change,
excessive complexity, and local convention fit.
- `security-auditor`: authentication, authorization, secrets, injection, unsafe file or network access, dependency and
configuration risks.
- `test-engineer`: test coverage, missing cases, flaky tests, and verification gaps.

Each subagent should receive:

- The user's review goal
- The relevant diff or changed file list
- Any test commands already run
- A request for findings only, not broad rewrites
- A request for findings only, not broad rewrites or implementation changes
- A request to cite `file:line` evidence and provide actionable recommendations

After subagents respond:

1. Deduplicate overlapping findings.
2. Prioritize blocking issues first.
3. Include only actionable recommendations.
2. Prioritize blocking issues first, especially correctness, security, and required test coverage gaps.
3. Include only actionable recommendations with `file:line` evidence.
4. Clearly mark non-blocking suggestions.

## Final report format
Expand Down
47 changes: 30 additions & 17 deletions plugins/me/skills/ship/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
---
name: ship
description: Run the pre-launch checklist via parallel fan-out to specialist personas, then synthesize a go/no-go decision with a rollback plan. Use when asked to "ship", "release", "deploy", or "is this ready to go live?".
description: >-
Run the pre-launch checklist via parallel fan-out to specialist personas, then synthesize a go/no-go decision with a
rollback plan. Use when asked to "ship", "release", "deploy", or "is this ready to go live?".
disable-model-invocation: true
---

Expand All @@ -13,12 +15,14 @@ then merges their reports into a single go/no-go decision with a rollback plan.
no shared state, no ordering — which is what makes parallel execution safe and useful here.

For the underlying pre-launch checklists, see the `shipping-and-launch` skill. That includes security, performance,
accessibility, feature flag lifecycle, staged rollout, monitoring, and rollback procedure.
accessibility, CI/check status, migrations/config/env readiness, feature flag lifecycle, staged rollout, monitoring and
alerts, documentation, post-launch verification, and rollback procedure.

## Phase A — Parallel fan-out

Spawn three subagents concurrently using the Agent tool. **Issue all three Agent tool calls in a single assistant turn**
so they execute in parallel — sequential calls defeat the purpose of this skill.
For non-trivial production-bound changes, spawn three subagents concurrently using the Agent tool. **Issue all three
Agent tool calls in a single assistant turn** so they execute in parallel — sequential calls defeat the purpose of this
skill.

In Claude Code, each call passes `subagent_type` matching the persona's `name` field:

Expand All @@ -36,8 +40,8 @@ Constraints (from Claude Code's subagent model):

- Subagents cannot spawn other subagents — do not let one persona delegate to another.
- Each subagent gets its own context window and returns only its report to this main session.
- If a persona is not installed in the current environment, fall back to running that persona's pass yourself in the main
context and label the section accordingly. Do not silently skip it.
- If a persona is not installed in the current environment, fall back to running that persona's pass yourself in the
main context and label the section accordingly. Do not silently skip it.

**Persona resolution.** This plugin includes default `code-reviewer`, `security-auditor`, and `test-engineer` personas.
If you've defined your own versions in `.claude/agents/` or `~/.claude/agents/`, those take precedence — `/ship` picks
Expand All @@ -48,15 +52,16 @@ priority table, so user-level definitions win by design.

Once all three reports are back, the main agent (not a sub-persona) synthesizes them:

1. **Code Quality** — Aggregate Critical/Important findings from `code-reviewer` and any failing tests, lint, or build
output. Resolve duplicates between reviewers.
1. **Code Quality** — Aggregate Critical/Important findings from `code-reviewer` and any failing tests, lint, build, or
CI/check output. Resolve duplicates between reviewers.
2. **Security** — Promote any Critical/High `security-auditor` findings to launch blockers. Cross-reference with
`code-reviewer`'s security axis.
3. **Performance** — Pull from `code-reviewer`'s performance axis; cross-check Core Web Vitals if applicable.
4. **Accessibility** — Verify keyboard nav, screen reader support, and contrast directly or with the accessibility
checklist.
5. **Infrastructure** — Env vars, migrations, monitoring, feature flags. Verify directly.
6. **Documentation** — README, ADRs, changelog. Verify directly.
5. **Infrastructure** — Verify CI/checks, migrations, config/env, feature flags, monitoring/alerts, staged rollout, and
rollback triggers/procedure directly.
6. **Documentation** — Verify README, ADRs, changelog, runbooks, and post-launch verification steps directly.

## Phase C — Decision and rollback

Expand All @@ -74,9 +79,14 @@ Produce a single output:
### Acknowledged risks (shipping anyway)
- [Risk + mitigation]

### Observability and rollout
- Monitoring/alerts: [dashboards, alerts, owners]
- Staged rollout: [flag/ramp plan and stop criteria]
- Post-launch verification: [checks to run after release]

### Rollback plan
- Trigger conditions: [what signals would prompt rollback]
- Rollback procedure: [exact steps]
- Trigger conditions: [metrics, alerts, logs, or user-impact signals that prompt rollback]
- Rollback procedure: [exact steps and owner]
- Recovery time objective: [target]

### Specialist reports (full)
Expand All @@ -89,9 +99,12 @@ Produce a single output:

1. The three Phase A personas run in parallel — never sequentially.
2. Personas do not call each other. The main agent merges in Phase B.
3. The rollback plan is mandatory before any GO decision.
4. If any persona returns a Critical finding, the default verdict is NO-GO unless the user explicitly accepts the risk.
5. **Skip the fan-out only if all of the following are true:** the change touches 2 files or fewer, the diff is under
3. The rollback plan is mandatory before any GO decision, including rollback trigger and procedure.
4. Default to NO-GO for any Critical security finding, failing required test/build/check/CI status, missing rollback
plan, or unverifiable production risk. Only override if the user explicitly accepts the risk.
Comment on lines 100 to +104

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 the parallelism rule contradiction.

Line 100 says “never sequentially,” but the fallback guidance allows sequential execution when Agent tooling is unavailable. This creates conflicting operator instructions. Clarify Rule 1 with an explicit exception for non-Agent harnesses so the procedure is unambiguous.

Suggested wording update
-1. The three Phase A personas run in parallel — never sequentially.
+1. In Claude Code (Agent tool available), run all three Phase A personas in parallel in a single assistant turn.
+   If the harness lacks an Agent tool, run persona passes sequentially and treat outputs as parallel inputs for Phase B synthesis.
🤖 Prompt for 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.

In `@plugins/me/skills/ship/SKILL.md` around lines 100 - 104, Update Rule 1 in
SKILL.md to state that the three "Phase A personas" must run in parallel under
the Agent harness, and add an explicit exception clause that if an Agent
tooling/harness is unavailable the operator MAY run persona steps sequentially
as a fallback — but only when the operator documents the deviation, the
increased risk, and obtains explicit user consent; also update any fallback
guidance text to reference this exception so there is no ambiguity between
"never sequentially" and the fallback path.

5. A GO decision must include observability coverage, monitoring/alert ownership, staged rollout or feature-flag plan,
and post-launch verification steps.
6. **Skip the fan-out only if all of the following are true:** the change touches 2 files or fewer, the diff is under
50 lines, and it does not touch auth, payments, data access, or config/env. Otherwise, default to fan-out. `/ship`
is designed for production-bound changes — when the blast radius is non-trivial, run the parallel review even if the
diff looks small.
is designed for production-bound changes — when the blast radius is non-trivial, run the parallel `code-reviewer`,
`security-auditor`, and `test-engineer` review even if the diff looks small.
Loading
Loading