diff --git a/docs/superpowers/plans/2026-05-12-lifecycle-skills-upstream-alignment.md b/docs/superpowers/plans/2026-05-12-lifecycle-skills-upstream-alignment.md new file mode 100644 index 00000000..6ca6a6bb --- /dev/null +++ b/docs/superpowers/plans/2026-05-12-lifecycle-skills-upstream-alignment.md @@ -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. diff --git a/plugins/me/skills/review/SKILL.md b/plugins/me/skills/review/SKILL.md index 408a394d..8afe8667 100644 --- a/plugins/me/skills/review/SKILL.md +++ b/plugins/me/skills/review/SKILL.md @@ -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 --- @@ -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 @@ -20,23 +23,37 @@ 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: @@ -44,13 +61,14 @@ 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 diff --git a/plugins/me/skills/ship/SKILL.md b/plugins/me/skills/ship/SKILL.md index 66f052af..f39b98fb 100644 --- a/plugins/me/skills/ship/SKILL.md +++ b/plugins/me/skills/ship/SKILL.md @@ -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 --- @@ -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: @@ -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 @@ -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 @@ -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) @@ -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. +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. diff --git a/plugins/me/skills/test/SKILL.md b/plugins/me/skills/test/SKILL.md index 164211f0..9f40586b 100644 --- a/plugins/me/skills/test/SKILL.md +++ b/plugins/me/skills/test/SKILL.md @@ -13,10 +13,11 @@ This is an explicit workflow skill: it should run only when the user invokes it. `/test` favors: +- Creating a failing test or repro before feature/bugfix work when feasible - Proving behavior with the smallest meaningful test -- Reproducing bugs before fixing them when feasible -- Testing observable outcomes over implementation details -- Focused checks before broad suites +- Testing observable user/system outcomes over implementation details +- Running the focused test command before the smallest relevant broader suite +- Verifying UI, browser, and runtime changes in the real runtime - Evidence over confidence ## Test discovery @@ -30,16 +31,18 @@ Before adding tests: ## Workflow -1. State the behavior under test. +1. State the observable behavior under test from the user/system point of view. 2. Find the closest existing test pattern. -3. Add or update a focused test. -4. For bugs, confirm the test fails before the fix when feasible. +3. For features or bug fixes, add a failing focused test or repro first when feasible. +4. Confirm the focused test fails for the expected reason before changing implementation. 5. Implement or adjust only what is needed for the test to pass. -6. Run the focused test again. -7. Run the smallest relevant broader suite. -8. Summarize exact commands and results. +6. Run the focused test command again. +7. Run the smallest relevant broader suite after the focused check passes. +8. For UI, browser, or runtime changes, verify the real runtime with `browse`, `e2e`, or related verification. +9. Summarize exact commands, results, and remaining gaps. -Use `debugging-and-error-recovery` for unexpected failures, `browse` for browser runtime behavior, and `qa` or `verify` for final evidence when appropriate. +Use `debugging-and-error-recovery` for unexpected failures, `browse` for browser runtime behavior, `e2e` for end-to-end +coverage, and `qa` or `verify` for final evidence when appropriate. ## Subagent use @@ -48,12 +51,14 @@ Use the `test-engineer` subagent for non-trivial testing work. Ask `test-engineer` to inspect: - Missing happy-path, edge-case, and error-path coverage -- Overly coupled assertions +- Whether a failing test or repro came before implementation when feasible +- Whether assertions prove observable behavior rather than implementation details - Flaky setup or timing assumptions -- Whether tests prove the requested behavior -- Whether broader verification is needed +- Whether focused and broader commands are appropriately scoped +- Whether runtime verification is needed -For security-sensitive behavior, also use `security-auditor`. For large behavior changes, use `code-reviewer` after tests pass. +For security-sensitive behavior, also use `security-auditor`. For large behavior changes, use `code-reviewer` after +tests pass. ## Final response @@ -61,6 +66,7 @@ Report: - Behavior verified - Tests added or changed -- Commands run -- Pass/fail results -- Remaining gaps +- Commands run, including focused command first and broader suite if run +- Pass/fail results for each command +- Runtime verification used, such as `browse`, `qa`, `verify`, or `e2e` when applicable +- Remaining gaps or unverified areas