Skip to content

llm: Add plan-implement-review pipeline command - #7121

Open
SaintPatrck wants to merge 4 commits into
mainfrom
llm/plan-implement-review-command
Open

llm: Add plan-implement-review pipeline command#7121
SaintPatrck wants to merge 4 commits into
mainfrom
llm/plan-implement-review-command

Conversation

@SaintPatrck

Copy link
Copy Markdown
Contributor

Objective

Add a plan-implement-review command that orchestrates an end-to-end Android development pipeline — requirements analysis, architecture planning, implementation, and multi-agent code review — driven by a Claude Code agent team.

Pipeline

  • Plan (once): Requirements → Architecture → Work Breakdown → QA Handoff
  • Implement + Review (per phase): implement → 4-reviewer gauntlet → up-to-3-round fix cycle
  • Reviewer roles map to Bitwarden marketplace agents (product-analyst, architect, security-engineer, code-reviewer, software-engineer).

Notes

  • Requires CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1.
  • Targets the implicit agent-team model (Claude Code v2.1.178+): teammates are spawned via the Agent tool and team setup/teardown is automatic — no TeamCreate/TeamDelete.

@github-actions github-actions Bot added the t:llm Change Type - LLM related change (e.g. CLAUDE.md files) label Jun 30, 2026
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.31%. Comparing base (4d6c8c4) to head (6cc0878).
⚠️ Report is 89 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7121      +/-   ##
==========================================
+ Coverage   85.86%   86.31%   +0.44%     
==========================================
  Files         926      921       -5     
  Lines       65940    66443     +503     
  Branches     9376     9794     +418     
==========================================
+ Hits        56617    57347     +730     
+ Misses       6098     5592     -506     
- Partials     3225     3504     +279     
Flag Coverage Δ
app-data 17.69% <ø> (+0.48%) ⬆️
app-ui-auth-tools 18.61% <ø> (-0.23%) ⬇️
app-ui-platform 16.57% <ø> (+0.04%) ⬆️
app-ui-vault 27.87% <ø> (-0.47%) ⬇️
authenticator 6.09% <ø> (-0.08%) ⬇️
lib-core-network-bridge 4.12% <ø> (+0.06%) ⬆️
lib-data-ui 1.20% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SaintPatrck
SaintPatrck marked this pull request as ready for review August 7, 2026 17:55
@SaintPatrck
SaintPatrck requested a review from a team as a code owner August 7, 2026 17:55
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds a single new slash command, .claude/commands/plan-implement-review.md (304 lines), which orchestrates an agent-team pipeline covering requirements analysis, architecture planning, phased implementation, and a four-reviewer gauntlet with a bounded fix cycle. No application code, dependencies, or settings files are touched, so there is no runtime or supply-chain risk. Frontmatter (description, argument-hint) matches the conventions of the existing plan-android-work, work-on-android, and review-android commands, and the referenced agent identifiers resolve against the installed marketplace plugins. Two documentation-consistency items were found; neither blocks merge.

Code Review Details
  • ♻️ : Consolidated review summary path is not phase/round-scoped, so it is overwritten each cycle while reviewer output files are correctly scoped — Step 7 then references artifacts that no longer exist
    • .claude/commands/plan-implement-review.md:188
  • ♻️ : New command is not registered in the .claude/CLAUDE.md command table
    • .claude/commands/plan-implement-review.md:3

Also noted, not raised inline: Step 7 suggests Skill(bitwarden-delivery-tools:creating-pull-request) but bitwarden-delivery-tools is absent from the Prerequisites table.

Comment thread .claude/commands/plan-implement-review.md Outdated
Comment thread .claude/commands/plan-implement-review.md
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude Config Validation — PR #7121

Scope: .claude/CLAUDE.md, .claude/commands/plan-implement-review.md
Verdict: ✅ No critical issues. No secrets, no dangerous auto-approvals, valid frontmatter. 4 major and 8 minor items below — all quality/correctness, none blocking on security grounds.

Section Status
1. Plugin validation (plugin-validator) ⏭️ Skipped — no plugin directories changed
2. Skill review (skill-reviewer) ⏭️ Skipped — no SKILL.md files changed
3. Config & security review (reviewing-claude-config) ✅ Ran — command + CLAUDE.md checklists

Note on the checkout: the working tree at review time had .claude/commands/plan-implement-review.md deleted and the CLAUDE.md row reverted (unstaged). Review was performed against the PR head commit 6cc0878ca via git show, so line numbers match the committed file.


Security scan (all changed files) — PASS

  • ✅ No hardcoded API keys, tokens, passwords, or private keys. Regex sweep for sk-*, ghp_, AKIA*, -----BEGIN, bearer, api_key returned only prose matches ("master passwords", "Password Manager").
  • ✅ No settings.local.json committed; .claude/settings.json is unchanged by this PR.
  • ✅ No permission grants or auto-approvals added. .claude/settings.json has no permissions block at all.
  • ✅ Only shell command introduced is echo $CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS (plan-implement-review.md:70) — read-only, safe.
  • ✅ All file writes confined to .claude/outputs/plans/ and .claude/outputs/reviews/ — no writes outside the repo, no ~/.ssh, /etc, or wildcard paths.

Structure validation — PASS

  • plan-implement-review.md:1-4 — YAML frontmatter well-formed, description and argument-hint present, matching the convention of plan-android-work.md, review-android.md, and work-on-android.md.
  • $ARGUMENTS interpolation used correctly; empty-input and --confirm-only cases handled explicitly (:16-18).
  • ✅ Registered in the CLAUDE.md command table (CLAUDE.md:81) — the command is discoverable.
  • ✅ 310 lines — within reasonable size for a self-contained orchestration command.

Major (should fix)

1. No branch is created before the implementer commits — autonomous runs can commit to main

File: .claude/commands/plan-implement-review.md:130-140

Step 3a instructs the implementer to "commit when done" and Step 3a.5 says to "Verify that new commits exist on the branch (check git log)" — but no step anywhere establishes which branch. Branch creation is only ever mentioned as optional and only for subsequent phases (:48, :166). If the user invokes this from main in autonomous mode (the default), the pipeline commits implementation work directly to the default branch across up to N phases × 3 fix rounds.

Fix: add an explicit branch step to Step 1 (Initialize), before any teammate spawns:

3. **Check the current branch** via `git rev-parse --abbrev-ref HEAD`. If it is `main`,
   create a feature branch from the derived slug (e.g. `git switch -c {slug}`) and tell
   the user. Never allow the implementer to commit to `main`.

2. Review files are overwritten each round, but Step 6c requires cross-round history

File: .claude/commands/plan-implement-review.md:259 (with :267)

Step 6b tells reviewers to "Write updated findings to the same output file (overwriting the previous round)", and Step 5 (:194) scopes the summary filename per phase, not per round. Step 6c then requires the lead to escalate with "what was attempted across all rounds, and which findings remain disputed" — but rounds 1 and 2 have been overwritten on disk by then, so that escalation can only be reconstructed from conversation context, which may have been compacted after 3 review rounds × 4 reviewers.

Fix: suffix the round as well as the phase, e.g. {slug}-REVIEW-CODE-P{N}-R{M}.md and {slug}-REVIEW-SUMMARY-P{N}-R{M}.md, and have Step 7 list the final round per phase while retaining the earlier ones.

3. Autonomous (uncontrolled) mode is the default for a pipeline that writes and commits code

File: .claude/commands/plan-implement-review.md:17

--confirm is opt-in, so the default behaviour is: plan → implement → commit → review → fix → commit → next phase, with no user gate until all phases finish. In a security-sensitive repository, defaulting a multi-phase autonomous code-writing loop to no-approval is a meaningful risk, and it compounds finding #1.

Fix (pick one): invert the flag so gated is the default and --autonomous opts out; or, at minimum, keep autonomous mode but add a mandatory single confirmation after the phase plan is presented (Step 2, :123 already presents the plan — make the answer a hard gate in both modes).

4. Contradictory description of what drives progression in autonomous mode

File: .claude/commands/plan-implement-review.md:17 vs :302 (and :8, :115, :185)

  • :8 — "let teammates self-organize around the task list. The task dependency chain drives execution order"
  • :17 — autonomous mode = "the task dependency chain drives progression automatically"
  • :115 — standing teammates "must NOT self-activate based on task dependencies"
  • :302 — "The team lead manually drives the phase loop"

The document states both that dependencies drive progression and that they must not. The intended split (dependencies drive the four planning tasks; the lead drives the implement+review loop) is inferable but never stated in one place — and the doc itself marks reviewer self-activation as CRITICAL to avoid (:153), so ambiguity here has a real failure mode.

Fix: rewrite :17 to "autonomous mode (the team lead advances phases without asking for approval)" and add one line under Pipeline Structure: "Task dependencies order the planning phase only; the team lead explicitly dispatches every implement and review step."


Minor (consider fixing)

# File:line Issue Suggested fix
5 plan-implement-review.md:81 "Create 4 planning tasks and 5 standing teammates" undercounts — the step also spawns 4 planning teammates, for 9 concurrent agents. Reword to "4 planning tasks, 4 planning teammates, and 5 standing teammates (9 agents total)" so the cost is visible upfront.
6 plan-implement-review.md:25-28 The four per-reviewer output paths are defined without the -P{N} phase suffix; the suffix convention is only documented for the summary (:29) and again inline at :149. Apply the same "(append the phase number if multiple phases)" note to all five review paths in the Input Parsing list.
7 plan-implement-review.md:71 "Confirm each required plugin from the Prerequisites table is installed" gives no mechanism for confirming. State how — e.g. "check the plugin list via /plugin, or treat a failed Agent spawn as 'not installed' per Step 4.5."
8 plan-implement-review.md:120 "Shut down planning teammates" names no mechanism, while Step 7 (:284) specifies SendMessage with shutdown_request. Use the same explicit wording in both places.
9 plan-implement-review.md:64 "The bitwarden-tech-lead agent is provided by the bitwarden-tech-lead marketplace plugin" duplicates the table row at :59. Delete the line.
10 plan-implement-review.md:85-90 The planning-task table does not name the tool used to create tasks, whereas Steps 3a/3b/6a explicitly say TaskCreate. Add "Create these via TaskCreate with the blockedBy values shown."
11 plan-implement-review.md:21-29 The command writes 9 artifacts under .claude/outputs/, which is not gitignored (no .gitignore entry for it anywhere in the repo). Generated plans and reviews will show up as untracked files in every subsequent git status and can be committed accidentally by the implementer. Pre-existing (plan-android-work.md:106 also writes there) but this PR multiplies it. Add .claude/outputs/ to .gitignore in this PR.
12 CLAUDE.md:61-64 The "Quick start" blurb still describes only the tech-lead → software-engineer → /review-android path and doesn't mention the new pipeline command, even though it is the most complete of the three workflows. Add one clause pointing at /plan-implement-review as the all-in-one alternative.

Unverifiable in this environment (informational)

The command declares seven required marketplace plugins (plan-implement-review.md:54-62) and five agent types (:98-113). Only claude-config-validator is present in the local plugin cache, so these references could not be verified:

  • bitwarden-product-analyst:product-analyst, bitwarden-security-engineer:bitwarden-security-engineer, bitwarden-code-review:bitwarden-code-reviewer — note these three use three different naming shapes (plugin:short-name, plugin:plugin, plugin:plugin-er); worth double-checking each against bitwarden/ai-plugins before merge, since a typo here fails at spawn time.
  • bitwarden-tech-lead:bitwarden-tech-lead and bitwarden-software-engineer:bitwarden-software-engineer — ✅ consistent with the identifiers already used in CLAUDE.md:61-63.
  • Skills referenced from bitwarden-delivery-tools (committing-changes, labeling-changes, architecting-solutions, creating-pull-request at :62/:294).

Related: .claude/settings.json registers the bitwarden-marketplace source but has no enabledPlugins entry, so none of the six required plugins are enabled by repo config. Every user running /plan-implement-review will hit the Step 1 hard stop until they install all six by hand. The command handles this gracefully, but adding enabledPlugins to .claude/settings.json would make it work out of the box.


Summary

Severity Count
Critical (must fix) 0
Major (should fix) 4
Minor (consider) 8

Errors (must fix before merge): none.
Warnings (should fix): findings 1–4 — branch safety before autonomous commits is the one worth blocking on if this pipeline will be run against main.

Both changed files pass all security and structural checks: valid frontmatter, no secrets, no permission or auto-approval changes, no broken intra-repo references, correctly registered in the command table, and consistent with the existing .claude/commands/ conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t:llm Change Type - LLM related change (e.g. CLAUDE.md files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant