Add test plan execution as a skill or pr-review subagent

[LeanAndMean]

Summary

Add the ability to automatically execute the test plan checklist in a PR body, either as a standalone skill (e.g., /mach10:pr-test-plan) or as an additional subagent in the existing /mach10:pr-review command.

Motivation

During PR review for merckgroup/aidd_lab_codebase#649, we manually ran through the test plan items in the PR body (pytest, integration tests, Docker validation, etc.) and checked them off after verifying each one. This is a repeatable workflow that could be automated:

1. Parse the PR body for a ## Test plan section with checkbox items
2. Execute each item (run tests, start services, validate configs, etc.)
3. Check off items that pass, report failures
4. Post a summary comment on the PR

Proposed Behavior

Option A: Standalone skill (/mach10:pr-test-plan <pr-number>)

• Reads the PR body, extracts test plan checkboxes
• Attempts to execute each item based on natural language interpretation
• Updates the PR body to check off passing items
• Posts a comment summarizing results (pass/fail per item, errors encountered)

Option B: Subagent within /mach10:pr-review

• After the existing review agents complete, a test-plan-executor agent runs
• Same parsing and execution logic as Option A
• Results included in the review output

Considerations

• Some test plan items are automatable (run pytest, validate docker-compose, check a page loads) while others require manual verification (visual UI checks, production deployment). The agent should distinguish between these and skip items it cannot verify.
• The agent should have access to Bash, the test database, and the application for integration-style checks.
• Checking off items in the PR body requires a gh pr edit --body call -- this is a mutating action and should probably require user confirmation or be opt-in.
• Consider whether to update checkboxes in-place or just post a comment with results (less invasive).

Example Workflow

User: /mach10:pr-test-plan 649

Agent:
  1. Parses PR body, finds 7 test plan items
  2. Runs pytest -> pass (5388 passed)
  3. Starts listener_main.py with test dir -> pass (starts and shuts down cleanly)
  4. Drops CSV files, checks DB for import records -> pass
  5. Re-scans same files, confirms skip -> pass
  6. Points listener at missing dir, confirms no crash -> pass
  7. Hits history page endpoint, checks for UI elements -> pass
  8. Validates docker-compose config -> pass
  9. Checks off all items in PR body
  10. Posts summary comment

[LeanAndMean]

Independent assessment: holistic placement

Considering this proposal against the full mach10 workflow, I recommend a three-touch-point shape rather than picking just Option A or Option B.

Resolve the A vs. B choice toward A (standalone command)

Option B (subagent inside /mach10:pr-review) cuts against the codebase's separation of concerns:

• pr-review is read-only today (no Edit/Write in allowed-tools). Folding in execution + PR-body mutation breaks that boundary.
• pr-review runs on opus for deep diff analysis. Test execution is mostly mechanical Bash and does not need opus.
• pr-review finds issues in the diff. Test plan execution verifies author claims. Different concerns.

Option A (standalone /mach10:pr-test-plan) slots in between pr-review-fix and pr-pre-merge the same way doc-review does: fresh session, checkout branch, fan-out per-item agents, gate mutations through AskUserQuestion.

Improve how the test plan is written in the PR (upstream root cause)

The biggest reliability lever is the test plan itself. Today the template at pr-create.md:120-122 is just - [ ] <bulleted checklist of how to verify the changes>. Items end up as free-form prose like "Verify the login flow works" -- which is hard for both human reviewers and any executor.

Improving the writing guidance in pr-create benefits the human PR review and makes the executor reliable. Recommended additions to the template instruction:

• Be specific. "Run `pytest tests/auth/test_login.py`" beats "Run the auth tests."
• State pass/fail criteria. "Hit `/api/users/me` and confirm 200 + `email` field present" beats "Verify the endpoint works."
• Make commands executable when known. Backtick-wrap shell commands so they can be lifted directly. "Run `docker compose up && curl localhost:8080/healthz`" beats "Spin up the stack and check the health endpoint."
• Tag expected execution mode. Optional [auto] / [manual] tags so the executor (and reviewers) immediately see which items require a human (e.g., visual checks, production deploys) vs. which can be run.
• One concern per item. "Run X" + "Verify Y" should be two checkboxes, not one.
• Order setup before action before teardown when the items have dependencies.

Build the standalone executor on top of that improved format

A new /mach10:pr-test-plan <pr-number> command:

• Checkout the PR branch.
• Parse ## Test plan checkboxes from the body.
• Classify each item using the new tags (or LLM heuristic for legacy plans without tags).
• For each [auto] item: present to the user via AskUserQuestion with Run / Skip / Modify-then-run, then execute.
• Skip [manual] items with a one-line "manual: " note.
• Post results as a <!-- mach10-test-plan --> comment. Do not mutate the PR body -- preserves the author's original plan as a stable artifact and follows existing marker conventions.

Prevent duplication with the existing pr-pre-merge Step 5d

pr-pre-merge already auto-detects and runs the project test suite. PR-body test plans typically include the suite plus PR-specific extras. Without coordination, the suite runs twice or it is unclear which command "owns" testing.

Light cross-reference in Step 5d: read any prior <!-- mach10-test-plan --> comment and surface its summary; skip duplicate suite runs if it already covered the suite.

Recommended placement summary

Touch point	Change
commands/pr-create.md (test plan template + writing guidance)	Strengthen the template instruction with specificity, pass/fail criteria, executable phrasing, optional [auto]/[manual] tags, single-concern items, and ordering. Helps human reviewers regardless of whether the executor is run.
New /mach10:pr-test-plan <pr-number>	Fresh-session command. Parse checkboxes, classify, run autos with per-item gating, post <!-- mach10-test-plan --> results comment. No PR-body mutation.
commands/pr-pre-merge.md Step 5d	Detect prior <!-- mach10-test-plan --> comment; surface results, skip duplicate runs.

Optional fourth touch: mention pr-test-plan in pr-review Step 8 as a "next step" alongside pr-review-fix and pr-pre-merge so the new command is discoverable.

Open questions that should be settled in planning

1. PR-body checkbox mutation: yes or no? Recommendation: no -- post a results comment with a marker. Preserves the author's original plan and follows existing <!-- mach10-* --> conventions.
2. Failure policy? Precedent at pr-pre-merge.md Step 5d is "report, do not block." Same here unless we want this command to act as a hard gate.
3. Idempotency on re-run? Detect prior marker comment and offer to re-run only failing items.
4. Stack-agnostic command discovery? Reuse the CONTRIBUTING.md -> DEVELOPMENT.md -> .github/CONTRIBUTING.md lookup chain plus the auto-detection heuristics from pr-pre-merge.md Step 5d.
5. Naming collision with pr-review-toolkit:pr-test-analyzer? That existing agent reviews test coverage (static analysis); this command executes test plans (dynamic). Worth distinguishing in docs.
6. Per-item user gating model? Follow doc-review's pattern: one AskUserQuestion per item with Run / Skip / Modify-then-run.
7. Backwards compatibility for legacy plans without tags? Heuristic classification with low confidence threshold (skip rather than guess).

Security note

Executing shell commands inferred from a PR body introduces command-injection risk on PRs from external contributors. Per-item user approval (no bulk-run mode) is the safe default and aligns with mach10's existing safety conventions.

Assessment Notes

User framing during the assessment session: explicitly asked for holistic placement across the workflow, expecting more than one touch point ("custom command and as part of an existing command"). User also flagged that improving the writing of the acceptance test list in PR bodies should be in scope -- this elevated the pr-create.md touchpoint from a small structural tweak (just [auto]/[manual] tags) to a substantive writing-quality improvement (specificity, pass/fail criteria, single-concern items, ordering). That change benefits human reviewers as a first-order outcome and makes the executor reliable as a second-order consequence. The standalone-command preference (Option A over Option B) was driven by the existing pr-review read-only convention, which Option B would have broken.

[LeanAndMean]

Implementation Plan for Issue #136

Overview

Adds /mach10:pr-test-plan <pr-number> [context] as a new slash command that works through the PR's ## Test plan checklist as a TODO list -- verifying each item autonomously, attempting minor in-flight fixes for failures, delegating to /feature-dev:feature-dev for non-trivial failures, updating PR body checkboxes for passed items, and posting a marker comment with HEAD SHA, summary, and per-item details. Two integration touchpoints (pr-pre-merge.md Step 5d marker coordination and pr-create.md test plan template strengthening) plus sync-doc updates and a plugin version bump round out the work.

This is a 2-stage plan. Stage 1 lands the core command and version bump. Stage 2 lands the integration touchpoints and sync documentation.

Architecture Choice

Minimal command structure + body mutation + feature-dev delegation for non-trivial fixes.

Selected after considering:

• A pragmatic-balance variant with a 9-step command and a fenced YAML results block in the marker comment (for hypothetical future tooling that parses results programmatically). Rejected as premature abstraction; the marker comment's human-readable summary plus HEAD SHA is sufficient for pr-pre-merge coordination.
• A read-only variant that posts findings without code changes. Rejected because the user's expectation (validated against actual Claude behavior in non-mach10 sessions) is that minor fixes should happen in flight.

Key design decisions:

1. Sequential autonomous execution -- Claude works through items one at a time, no upfront selection prompts.
2. No mid-flight user prompts -- the only AskUserQuestion is the re-run prompt when a prior marker comment is detected.
3. Body checkbox mutation -- passed items get checked in the PR body via gh pr edit --body, so GitHub UI users see real-time status. Failed and unchecked-blocked items stay unchecked.
4. Auto-fix-then-escalate -- Claude attempts minor in-flight fixes (typos, imports, formatting). If the fix is non-trivial, the item is flagged for batched delegation to /feature-dev:feature-dev after the autonomous pass.
5. Marker comment with HEAD SHA -- <!-- mach10-test-plan --> first line, recorded HEAD SHA second line, then summary and per-item details. pr-pre-merge Step 5d reads this for staleness detection (compares recorded SHA to current HEAD).

Stages

Stage 1: Create pr-test-plan command and bump plugin version

Files to create:

• commands/pr-test-plan.md (new file)

Files to modify:

• .claude-plugin/plugin.json (bump version 1.29.0 -> 1.30.0)

commands/pr-test-plan.md specification:

YAML frontmatter:

• description: "Execute the PR test plan checklist, fix what can be fixed, and post results"
• argument-hint: <pr-number> [context]
• allowed-tools: Bash, Read, Grep, Glob, Edit, Write, Skill, AskUserQuestion, TaskCreate, TaskUpdate
• model: opus

Step structure (9 steps):

• Step 0: Parse input and create task list -- Standard Step 0 bootstrap per CLAUDE.md. Parse <pr-number> (required) and optional [context] free-text. The context is honored by downstream steps where applicable (e.g., "only items 3 and 5", "skip docker tests", "items 4 and 6 verified manually"). Create 9 fixed-step tasks; dynamic per-item sub-tasks added in Step 2.

• Step 1: Pre-flight and checkout -- Reuse the dirty-tree pre-check pattern from commands/issue-implement.md (Step 2): git status --porcelain, refuse if non-empty. Then gh pr checkout <pr> and git pull (matching doc-review.md Step 1 wording verbatim). Capture git rev-parse HEAD for later use in the marker comment.

• Step 2: Read PR body, parse test plan, detect prior marker, create item sub-tasks

  • gh pr view <pr-number> for body, gh pr view <pr-number> --comments for comments (two-call pattern with the synced parenthetical).
  • Locate the ## Test plan section (case-insensitive heading match; section ends at next same-or-higher heading or end of body).
  • Parse checkbox items using the regex ^\s*[-*+]\s+\[[ xX]\]\s+(.+)$ (matching the Strategy B convention from commands/issue-plan.md).
  • Handle degenerate cases gracefully: missing section, empty section, or section containing the literal placeholder <bulleted checklist...> -- clear error message, leave step in_progress.
  • Honor [context] filtering if it scopes the run (e.g., "only items 3 and 5").
  • Search comments from end-to-start for <!-- mach10-test-plan -->. If found, parse the HEAD SHA from line 2 and use AskUserQuestion: Re-run all / Re-run only previously-failed-or-blocked / Cancel. Cancel exits cleanly without posting; the other paths flow into Step 3 with the appropriate item-set.
  • Create one task per item to be processed: subject "Step 3.M: Verify <item text>" (truncate text to ~80 chars), all pending. Sub-tasks are created sequentially, stored by index alongside item metadata.

• Step 3: Verify items autonomously and attempt minor fixes -- For each item, mark its sub-task in progress; analyze the item:

  • If text contains backticked commands, extract and run via Bash. Default timeout 120000 ms; escalate to 600000 ms for items whose text matches \b(full|all|integration|suite|e2e|end.to.end)\b.
  • If text describes a procedural check, interpret and execute using best judgment.
  • If item requires human perception (visual UI, browser rendering, production deploy, subjective evaluation), record as BLOCKED and continue.
  • On verification failure, attempt a minor in-flight fix using best judgment (typos, missing imports, formatting, lint warnings, simple type errors). Re-verify.
    • If the fix works, record PASS (auto-fixed) with brief description.
    • If the fix is non-trivial, flag for Step 4 delegation; record placeholder NEEDS_FEATURE_DEV with failure context.
  • Mark sub-task complete after each item (regardless of outcome -- completion reflects that the item was processed, not that it passed).

• Step 4: Delegate non-trivial failures to feature-dev -- Skip if no items were flagged NEEDS_FEATURE_DEV. Otherwise, invoke /feature-dev:feature-dev once with all flagged items bundled as context (each item: text, failure output, context summary). Apply the CLAUDE.md "delegation double completion" plus "delegation phase tracking" conventions:

  • Pre-delegation marker: "Mark Step 4 complete when all sub-tasks of the delegation below are completed."
  • Skill invocation includes the standard task-tracking instruction so feature-dev creates sub-tasks for its 7-phase plan as Step 4.M: <phase action>.
  • Post-delegation marker: "Mark Step 4 complete. Do not stop here -- continue to Step 5."

• Step 5: Re-verify feature-dev'd items -- For each item that went through Step 4, re-run its verification. If it now passes, record PASS (feature-dev). If it still fails, record FAIL (feature-dev attempted) with the residual failure output.

• Step 6: Commit and push in-flight fixes -- Check git status --porcelain for any uncommitted changes from Step 3's minor fixes (feature-dev's changes from Step 4 may already be committed by it; verify via git log). If there are uncommitted changes, stage them by name (no git add -A), commit with message "Auto-fix from pr-test-plan: " and push. If no uncommitted changes and no new commits, skip.

• Step 7: Update PR body checkboxes -- Re-read the current PR body via gh pr view <pr>, apply checkbox updates (only items with status PASS, PASS (auto-fixed), or PASS (feature-dev) get [x]; failed and blocked items stay [ ]). Push the updated body via gh pr edit <pr> --body "$(cat <<'INNER_EOF' ... INNER_EOF)". Use a HEREDOC to avoid escaping issues. If the body has been edited externally since Step 2 (detect via re-read comparison), warn the user but proceed (mach10 is single-user-per-PR in practice).

• Step 8: Post marker comment -- Capture the post-fix HEAD SHA (git rev-parse HEAD). Compose the marker comment with this structure (very first line is the HTML marker; second line is the SHA):

  • Line 1: <!-- mach10-test-plan -->
  • Line 2: HEAD: <40-char SHA>
  • Line 3: Run at: <ISO-8601 timestamp>
  • Then: ## Test plan results heading, summary line ("N items: P passed, F failed, B blocked"), and a Markdown table with columns Item / Status / Notes (statuses: PASS, PASS (auto-fixed), PASS (feature-dev), FAIL, FAIL (feature-dev attempted), BLOCKED).
  • Footer: *Test plan executed by Claude Opus 4.7*

  Post via gh pr comment <pr> --body "$(cat <<'INNER_EOF' ... INNER_EOF)". Retrieve the comment URL via gh pr view <pr> --json comments --jq '.comments[-1].url'.

• Step 9: Final report and next-step suggestion -- CLI-only: summary table, comment URL, and Next: /clear then /mach10:pr-pre-merge <pr-number>.

.claude-plugin/plugin.json change: version field 1.29.0 -> 1.30.0.

Test approach: mach10 has no runtime tests (pure plugin definition). The new command file is validated organically when pr-review is run on the implementing PR (which invokes plugin-dev:plugin-validator via pr-review-toolkit). Manual validation: invoke the new command on a test PR with a varied test plan (auto-passable items, minor-fixable items, non-trivial-fixable items, blocked items) and verify the full flow end-to-end.

Why Stage 1 is sized for one CLI session: ~250 lines of careful prose in a single file, following established patterns from doc-review.md and pr-review-fix.md. The plugin.json edit is a one-line change. No exploration is needed during implementation -- the spec above is complete.

Stage 2: Integration touchpoints and sync documentation

Files to modify:

1. commands/pr-create.md -- replace the Body block at lines 116-127 with a strengthened ## Test plan template. Insert an HTML comment guidance block (invisible in rendered GitHub view, visible in raw view) describing: specificity, pass/fail criteria, executable backticked commands, single-concern items, automated-before-manual ordering. Include 3 example bullets demonstrating the conventions. Do NOT introduce [auto]/[manual] tags. Keep all surrounding content (Closing keywords rule, "plain words" reminder) intact.

2. commands/pr-pre-merge.md -- rewrite Step 5d (lines 168-179):

   • Fetch PR comments: gh pr view <pr-number> --json comments --jq '.comments[].body'. Note this is the second gh pr view call (Step 4 already fetched the body), so it is subject to the --comments two-call sync.
   • Search comments end-to-start for <!-- mach10-test-plan -->.
   • If marker found: extract HEAD SHA from line 2 (format: HEAD: <40-char hex>). Compare to git rev-parse HEAD.
     • SHA matches: surface the prior summary line in Step 5 output, skip running the suite.
     • SHA differs (stale): use AskUserQuestion: Re-run test plan / Accept stale results / Run suite directly. The "Re-run test plan" path stops the checklist and instructs the user to run /clear then /mach10:pr-test-plan <pr> first.
   • If no marker: existing auto-detect prose unchanged.
     Also update Step 7 Tests bullet (line 206) to include the new statuses: [ ] Tests: [from pr-test-plan: N/T passed (commit <short-SHA>) / stale results accepted / user re-running pr-test-plan / suite passing / N failures noted]

3. commands/pr-review.md -- Step 8 (lines 315-329): add one closing note after the existing two conditional bullets: "Optionally, if the PR description includes a ## Test plan checklist, suggest running /clear then /mach10:pr-test-plan <pr-number> to execute it before pre-merge."

4. CLAUDE.md -- 5 edits:

   • Line 7 Project Overview: "14 slash commands" -> "15 slash commands"
   • Line 27 PR workflow list: insert pr-test-plan between doc-review and pr-pre-merge
   • Line 69 --comments two-call sync: extend file list from 6 to 8 (add commands/pr-test-plan.md Step 2 and commands/pr-pre-merge.md Step 5d). Update count and the doc-review.md PR-target note to also cover pr-test-plan.md.
   • Line 71 Step 0 bootstrap sync: extend file list from 7 to 8 (add commands/pr-test-plan.md) and update the count "8 locations" -> "9 locations".
   • Insert a new "Test-plan marker sync" entry after the Decision-comment marker sync (around current line 67): documents <!-- mach10-test-plan --> emitter (commands/pr-test-plan.md Step 8) and consumer (commands/pr-pre-merge.md Step 5d), and the HEAD SHA companion line on line 2 of the marker comment.

5. README.md -- 4 edits:

   • Line 15: "14 slash commands" -> "15 slash commands"
   • PR lifecycle command table (~line 256): insert new row for /mach10:pr-test-plan <pr> [context] between doc-review and pr-pre-merge.
   • Phase 7 commands line (~line 186): add /mach10:pr-test-plan <pr> (optional) before pr-pre-merge. Update Phase 7 prose to describe the test plan execution step.
   • Quick reference (~lines 210-214): renumber Sessions 13-14 to 14-15 and insert a new Session 13 line /mach10:pr-test-plan 108 (optional).

Test approach: N/A (sync-doc edits, validated by cross-reference reading during the edit). The plugin-dev:plugin-validator agent (invoked via pr-review-toolkit) will check the cross-references when the implementing PR is reviewed.

Why Stage 2 is sized for one CLI session: Each edit is small and bookkeeping-style. The implementer reads each cross-reference once, applies the matching edit, and verifies consistency before moving on. No exploration needed.

Stage Dependencies

Stage 1 must complete before Stage 2:

• The CLAUDE.md sync entries in Stage 2 reference the exact file paths and step numbers established in Stage 1.
• The pr-pre-merge.md Step 5d marker-detection logic references the marker comment format defined in Stage 1's Step 8.
• The pr-create.md template rewrite mentions pr-test-plan as the consumer of the new template guidance.

Decision Log

Clarifying Questions (Step 3)

• Q: How aggressive should the rewrite of the ## Test plan template instruction in pr-create.md be?
  Answered: Structured rewrite with example bullets, expressed as an HTML comment guidance block (invisible in rendered view, visible in raw view) plus 3 example bullets demonstrating writing-quality conventions. No [auto]/[manual] tags.

• Q: What per-item user-gating model should /mach10:pr-test-plan use?
  Answered: No mid-flight user prompts. Claude works through items as a TODO checklist using best judgment, attempts minor in-flight fixes for failures, delegates non-trivial failures to /feature-dev:feature-dev, and only prompts the user once -- on re-runs, when a prior marker comment is detected. Constrained the design away from upfront selection / classification phases / external-contributor gating, which were rejected as unnecessary friction given mach10 user demographics.

• Q: How deeply should commands/pr-pre-merge.md Step 5d coordinate with the new marker comment?
  Answered: Detect the marker, parse the HEAD SHA, compare to current HEAD; surface results when fresh, prompt for re-run/accept-stale/skip when stale.

• Q: Should the optional 4th touchpoint (mention pr-test-plan in pr-review.md Step 8) be in scope?
  Answered: Yes, add it in Stage 2 as a one-line closing note after the existing two conditional bullets.

• Q: Should pr-test-plan mutate the PR body to check off completed items, or only post a marker comment?
  Answered: Both. Body checkboxes for GitHub UI friendliness (passed items get [x]); marker comment for HEAD SHA staleness signal and detailed status. Earlier draft had marker-only per the issue's prior assessment recommendation; reweighed in favor of UI friendliness since the "preserve original plan" benefit is already provided by git history of body edits.

• Q: What should pr-test-plan do for failures it cannot trivially fix?
  Answered: Delegate to /feature-dev:feature-dev (batched: one delegation for all non-trivial failures), then re-verify after feature-dev returns. This brings pr-test-plan into the same family as pr-review-fix and pr-ci-fix. Adds Edit, Write, Skill to allowed-tools.

• Q: Does the [context] argument generalize to all mach10 commands?
  Answered: Yes, separately tracked as issue Standardize optional [context] argument across all mach10 commands #137 (Standardize optional [context] argument across all mach10 commands). Out of scope for this issue; pr-test-plan will land conformant with the principle by virtue of including <pr-number> [context] in its argument-hint.

Architecture Choice (Step 4)

Selected: Minimal command structure (9 steps) with body mutation, feature-dev delegation, and HEAD-SHA-tracking marker comment.

Two alternatives were considered:

• Pragmatic-balance with YAML results block in marker comment -- adds a fenced YAML block to the marker comment for hypothetical future tooling. Rejected as premature abstraction; no current consumer for machine-parseable results, and the human-readable summary plus HEAD SHA already serves pr-pre-merge coordination.
• Read-only marker-only design (carrying forward the issue's prior assessment recommendation) -- post a comment without code changes or body mutation. Rejected after user feedback that Claude should auto-fix minor failures in flight (matching observed Claude behavior in non-mach10 sessions) and that GitHub UI users benefit from real-time checkbox progress.

The selected approach maximizes reuse: the dirty-tree pre-check from issue-implement.md, the checkout pattern from doc-review.md, the checkbox regex from issue-plan.md, the marker-first-line convention from pr-review.md, the HEREDOC comment-posting from pr-review.md, and the feature-dev delegation pattern from pr-review-fix.md/pr-ci-fix.md.

---

This comment is the implementation plan; future CLI sessions will use it as input via /mach10:issue-implement 136 1 for Stage 1 and /mach10:issue-implement 136 2 for Stage 2.

[LeanAndMean]

Implementation Plan for Issue #136 (Revised)

Overview

Replaces the original "new `/mach10:pr-test-plan` command" approach with a `test-plan-executor` subagent invoked from `/mach10:pr-review`'s existing fan-out, in parallel with the other review agents. Test plan execution results become part of the review comment (``); failures appear as `F` findings that flow naturally into `/mach10:pr-review-fix` via the existing identifier convention. No new slash command, no new marker, no PR-body mutation, no auto-fix or auto-push pipeline.

This is a 1-stage plan.

Architecture Choice

Subagent inside `pr-review` (originally Option B in the issue), preserving the read-only review boundary by having the executor report findings rather than fix code.

The original plan introduced a new `/mach10:pr-test-plan` command with a 9-step workflow including auto-fix, auto-commit, auto-push, PR body checkbox mutation, and a new `` marker with HEAD SHA companion line. Plan review surfaced two Critical findings (command injection from PR body, push race after feature-dev's own push) and 11 Important findings -- almost all rooted in the auto-fix pipeline and body mutation.

Reframing the work as a subagent inside `pr-review`:

• Preserves `pr-review`'s read-only boundary -- the executor reports outcomes; it does not fix code, commit, push, or mutate the PR body.
• Reuses the existing `F`/`S` finding-identifier convention. Test failures become `F` findings; `pr-review-fix` already handles `F` findings via feature-dev delegation, so no new fix workflow is needed.
• Eliminates the new marker, the SHA companion line, the new sync entries, the slash-command count update, the ASCII workflow diagram update, and the quick-reference renumbering.

The original Option B objections (read-only boundary, opus model overhead, different concerns) are resolved: (1) read-only is preserved by design; (2) model selection is per-subagent frontmatter (`model: inherit` or override); (3) "verifying the PR works" via test execution and "finding issues in the diff" via code review are both kinds of "review" -- unifying them is more consistent than splitting.

Key design decisions:

1. Read-only executor. The subagent runs items, classifies outcomes (PASS / FAIL / BLOCKED), and returns findings. No `Edit`/`Write`/git-mutating tools.
2. Findings flow through existing identifier convention. FAIL items become `F` (Critical/Important); BLOCKED items become `S` notes or are surfaced in a dedicated "Test plan results" section without an identifier.
3. Parallel fan-out. Invoked alongside `pr-review-toolkit:code-reviewer` and other review agents in the existing Skill delegation. Authorized via a new "always include" bullet in `pr-review.md` Step 2.
4. No new marker. `pr-pre-merge` Step 5d reads the existing `` comment for prior test results. Staleness is detected by comparing the comment's `createdAt` against the latest commit timestamp.
5. Internal-trust model with destructive-pattern filter. The executor runs backticked commands extracted from the PR body. Per-item `AskUserQuestion` is not feasible from within a subagent. The safety floor is a small refused-pattern allowlist (`rm -rf` outside repo, `sudo`, `curl|sh`, env-rooted destructive ops); refused items become `BLOCKED` findings with a "filtered for safety: " note.

Stage 1 (only stage): Subagent + integration touchpoints + sync updates

Files to create:

• `agents/test-plan-executor.md` (new)

Files to modify:

• `commands/pr-review.md` -- Step 2 (Skill delegation): authorize and require invocation of `mach10:test-plan-executor` via a new "always include" instruction bullet alongside the existing five at lines 67-71. Step 3 (review comment): include a "## Test plan results" section in the review comment, with a per-item table (Item / Status / Notes) and FAIL items also appearing in the standard findings list under their `F` identifiers. If no `## Test plan` section exists in the PR body, surface a single `S` finding ("PR description does not contain a test plan section") instead of running the executor.
• `commands/pr-create.md` -- strengthen the test plan template (lines 116-127). Replace the placeholder `- [ ] ` (line 121) with an HTML comment guidance block (invisible in rendered view, visible in raw view) plus 3 example bullets demonstrating: specificity, observable pass/fail criteria, backticked executable commands when known, single-concern items, and order (setup -> action -> teardown). No `[auto]`/`[manual]` tags. Do not change the "Closing keywords rule" (line 129) or "plain words" reminder (line 134).
• `commands/pr-pre-merge.md` -- Step 5d (lines 168-179): read the most recent `` comment via `gh pr view --json comments --jq '.comments[].body'`. If the comment contains a "## Test plan results" section AND the comment's `createdAt` is newer than the current branch's last commit timestamp (`git log -1 --format=%cI`), surface the prior summary in the checklist and skip running the auto-detected suite. If stale (commit newer than review), use `AskUserQuestion`: Re-run pr-review (re-runs the executor as part of full review) / Accept stale results / Run auto-detected suite directly. If no marker found, the existing auto-detect prose is unchanged. Update Step 7 Tests bullet (line 206) to surface the new statuses.
• `CLAUDE.md` -- three small updates: (a) add the new subagent to any agent listing if one is maintained; verify it's not; (b) confirm the No-background-agent sync at line 73 covers the new agent -- `pr-review.md` Step 2's Skill pass-through already instructs the Skill not to use background agents, and the test-plan-executor is invoked through that Skill, so no new spawn site is added; (c) verify the Finding-identifier sync at the existing entry covers test plan failures -- the convention already handles this, no edit needed. No new sync entries are introduced. No "14 -> 15 slash command" updates.
• `README.md` -- one or two small updates: (a) add a brief mention in the `pr-review` row of the PR lifecycle command table that test plan execution is now part of the review; (b) note the disambiguation from the existing `pr-review-toolkit:pr-test-analyzer` agent (which performs static test-coverage analysis; the new executor performs dynamic test plan execution).
• `.claude-plugin/plugin.json` -- bump version 1.29.0 -> 1.30.0.

`agents/test-plan-executor.md` specification:

YAML frontmatter:

• `name`: `test-plan-executor`
• `description`: when to use the agent (within `pr-review` only), with `` blocks showing trigger scenarios (PR with a `## Test plan` section to verify).
• `model`: `inherit` (defers to caller's model; sufficient for mostly-mechanical execution).
• `color`: distinct (e.g., `green`).

Body sections:

• Role: Execute the `## Test plan` checklist from a PR body, classify each item, and return findings to the calling review session. Read-only -- does not edit code, commit, push, or mutate the PR body.
• Inputs: PR number; the parent step provides the parsed item list (so the agent does not re-fetch). The agent operates on the already-checked-out PR branch.
• Per-item process:
  1. Classify item type:
     • Backticked shell commands present -> extract for execution.
     • Procedural check described in prose -> interpret with best judgment.
     • Requires human perception (visual UI, browser rendering, production deploy, subjective evaluation) -> mark `BLOCKED` with reason.
  2. Apply destructive-pattern filter to extracted commands. If the command matches any refused pattern, mark `BLOCKED` with note "filtered for safety: ".
  3. Execute via Bash. Default timeout 120000ms; bump to 600000ms when item text matches `\b(full|all|integration|suite|e2e|end.to.end)\b`.
  4. Classify outcome: `PASS` (exit 0), `FAIL` (non-zero), `BLOCKED` (filter or perception).
  5. Capture last 30 lines of combined output for FAIL items.
• Returns: structured findings list. FAIL items: severity Important by default (Critical if the failure is a hard build/test-suite failure rather than a single test). Each finding includes the item text, the command run (if any), exit code, and truncated output.
• Tools needed: `Bash, Read, Grep, Glob` only. No `Edit`, no `Write`, no `Skill`.
• Heading parsing: case-insensitive match for `## Test plan` (`## Test Plan`, `## test plan` accepted; subheading variants like `### Test plan` also accepted but lower priority -- first match wins). Section ends at next same-or-higher heading or end of body. Items match `^\s*[-*+]\s+\[[ xX]\]\s+(.+)$`.
• Degenerate cases: missing section (return a single "no test plan section" note); empty section (return "test plan section is empty"); multiple sections (use first); items containing the literal placeholder `` (return "test plan still uses template placeholder").

Destructive-pattern filter (initial allowlist of refused patterns):

Conservative list, refusal is informative not silent:

• `\brm\s+-rf?\s+(/|$HOME|~|\*|\$\{?HOME\}?)`
• `\bsudo\b`
• `\b(curl|wget)\b[^|]\|\s(sh|bash)`
• `\b(eval|source)\s+<\s*\(`
• `>\s*(/dev/sd[a-z]|/dev/disk)`

Refused commands become `BLOCKED` findings with the matched pattern in the note.

`commands/pr-review.md` Step 2 Skill delegation update:

Add a sixth "always include" instruction bullet after the existing five (lines 67-71):

Always include this instruction in the Skill invocation: "If the PR description contains a `## Test plan` section, also invoke the `mach10:test-plan-executor` agent in parallel with the other review agents. The executor runs each test plan item, classifies outcomes (PASS / FAIL / BLOCKED), and returns findings. FAIL items must be merged into the standard Critical/Important findings list under sequential `F` identifiers; BLOCKED items appear in a dedicated `## Test plan results` section in the review comment. If no `## Test plan` section exists, emit a single `S` finding ('PR description does not contain a test plan section') and do not invoke the executor."

Stage Dependencies: N/A -- single stage.

Test approach:

Manual validation (no runtime tests in mach10):

1. Identify two existing PRs whose test plans cover varied cases and document them in the implementation PR description as the validation fixtures.
2. Invoke `/mach10:pr-review` on each fixture PR and verify:
   • PASS items appear in the "Test plan results" section without F-identifiers.
   • FAIL items appear both in the "Test plan results" section AND in the standard Critical/Important findings list with sequential F-identifiers.
   • BLOCKED items appear only in the "Test plan results" section with reasons.
   • Synthetic test of the destructive-pattern filter: temporarily add a malicious-looking item to a fixture PR body (`` - [ ] `rm -rf /` ``) and confirm it is BLOCKED with the matched pattern note, not executed.
3. Test "no test plan section" handling on a PR whose description lacks the section.
4. Test pr-pre-merge Step 5d coordination: invoke `pr-pre-merge` on a PR after a fresh `pr-review` run (verify it skips the suite); add a commit, invoke `pr-pre-merge` again (verify it offers the stale-results AskUserQuestion).
5. Test pr-review-fix flow on a PR with a FAIL F-finding: confirm `/mach10:pr-review-fix F` delegates to feature-dev for the test failure as it would for any other F-finding.

Why one stage: the redesign is meaningfully smaller than the original two-stage plan. The agent file is ~80 lines, the pr-review.md edit is one new instruction bullet, the pr-create.md template change is a small block, the pr-pre-merge.md Step 5d edit is moderate, and the CLAUDE.md/README.md/plugin.json edits are bookkeeping. All edits are tightly coupled -- separating them risks shipping a half-working state where the executor runs but pr-pre-merge can't read the results.

Decision Log

Redesign Rationale

The original plan was rejected during plan review (this session) after surfacing two Critical findings (command injection from PR body shell execution, push race after feature-dev's own push) and 11 Important findings -- most rooted in the auto-fix / auto-push / body-mutation pipeline. Rather than patching the original design's failure modes one-by-one, the user reframed the work as a subagent inside `/mach10:pr-review`. This collapses Critical and Important findings related to autonomous code mutation (autonomous shell execution, push race, undefined auto-fix boundary, body atomicity, SHA staleness, bundled feature-dev delegation) because the executor neither fixes nor pushes -- it reports findings and lets `pr-review-fix` handle remediation through the existing flow.

Open Questions Resolved

• Per-item user gating: not feasible from within a subagent. Replaced with a destructive-pattern filter as the safety floor.
• Re-run cadence: every `pr-review` re-run runs the executor. No skip-flag in this issue; future enhancement if it becomes painful.
• Missing test plan section: surfaced as an `S` finding ("PR description does not contain a test plan section") rather than silently passing.
• `[context]` argument: not applicable -- no new command, no new argument.
• Marker / command rename: not applicable -- no new marker, no new command. Existing `` carries the test results.
• Scope distinction from `pr-review-toolkit:pr-test-analyzer`: that existing agent performs static test-coverage analysis; the new `mach10:test-plan-executor` performs dynamic test plan execution. Disambiguated in the README PR-lifecycle table.

Findings collapsed by the redesign (from plan review)

F1 (command injection): replaced by destructive-pattern filter and the implicit gate of running through `pr-review` rather than a standalone command.
F2 (push race), F3 (auto-fix scope), F4 (body atomicity), F5 (SHA staleness), F6 (`[context]` parsing), F7 (HEREDOC misattribution), F8 (`--comments` two-call misapplied), F9 (task-tracking instruction sync missed), F10 (Contributing-guide lookup), F12 (bundled feature-dev): all not applicable in the redesign.
S1 (read-only redesign), S2 (comment-only result artifact), S3 (per-item gating): incorporated by design.
S4 (defer `[context]`), S5 (commit message format), S6 (cancel comment), S7 (re-run UX), S11-S14 (README/CLAUDE updates), S16 (machine-readable companion line), S17 (HEREDOC delimiter): not applicable.
S8 (degenerate cases): incorporated -- heading variants, multiple sections, empty section, placeholder text, missing section all enumerated.
S9 (manual-validation fixture): incorporated -- fixture PRs to be named in the implementation PR description.
S10 (pr-pre-merge behavioral validation): incorporated -- both fresh and stale cases documented in test approach.

Findings still in scope and addressed in this plan

• Test plan template strengthening in `pr-create.md` (preserved from original plan).
• `pr-pre-merge` Step 5d coordination (simpler now: timestamp-based staleness, no SHA comparison).
• Destructive-pattern safety filter (new commitment introduced by this redesign).
• `pr-test-analyzer` naming-collision disambiguation in README (S14, incorporated).

---

This comment is the implementation plan; future CLI sessions will use it as input via `/mach10:issue-implement 136 1`.

[LeanAndMean]

Stage 1 implementation complete

Implemented the revised plan as a single stage in commit 2a197e3 on feature/issue-136-pr-test-plan-execution.

Files changed (7):

• agents/test-plan-executor.md (new) -- read-only subagent: classifies test plan items as PASS / FAIL / BLOCKED, returns findings for the calling review to merge into F-identifier flow.
• commands/pr-review.md -- Step 2 adds a 6th "always include" Skill bullet to invoke the executor when a ## Test plan section exists; Step 3 owns the canonical ## Test plan results table format (positioned after Strengths, before model attribution).
• commands/pr-create.md -- test plan template now has an HTML guidance comment plus 3 example bullets demonstrating specificity, backticked commands, single-concern items, and setup -> action -> teardown ordering.
• commands/pr-pre-merge.md -- Step 5d coordinates with the <!-- mach10-review --> marker. Fresh review (per epoch-second timestamp comparison): surface prior summary, skip suite. Stale: prompt with three options (re-run pr-review / accept stale / run suite directly). No marker: existing auto-detect prose. Step 7 Tests bullet surfaces the new statuses.
• README.md -- pr-review row mentions test plan execution; new note disambiguates the executor from pr-review-toolkit:pr-test-analyzer (dynamic execution vs static coverage analysis).
• CLAUDE.md -- added a Test-plan-results section sync entry (departure from the plan, but captures a load-bearing cross-file string).
• .claude-plugin/plugin.json -- 1.29.0 -> 1.30.0.

Notable decisions:

• Added the CLAUDE.md sync entry after a code reviewer flagged the ## Test plan results heading as load-bearing across two files. The plan said no CLAUDE.md edits; this is a small, defensible departure.
• Used Format A for the pr-create template (visible example bullets + HTML guidance) per session input.
• Did NOT add a --comments two-call sync entry for the new pr-pre-merge fetch -- that sync is specific to the --comments flag, and the new fetch uses --json comments (a different mechanism).
• Kept the 3-option staleness prompt rather than collapsing to 2 -- the branches serve genuinely different scenarios.

Phase 6 review fixes applied (all 7):

1. Heading regex broadened from ^## to ^#{2,} so ### Test plan actually matches.
2. Staleness comparison switched to Unix epoch seconds (git log --format=%ct vs date -u +%s) to avoid string-compare bugs across timezone offsets.
3. rm filter broadened to \brm\b[^|;&\n]*\s(target) to catch rm -fr, rm -rfv, etc.
4. Removed redundant read-only restatement in the agent body opening paragraph.
5. Slimmed Step 2 Skill bullet so Step 3 owns the canonical table format.
6. Added the CLAUDE.md sync entry mentioned above.
7. Renamed the staleness option label to "Re-run pr-review (stops the checklist)" to make session-stopping behavior explicit.

Validation:

mach10 has no runtime tests. The plan's manual-validation step (run /mach10:pr-review against fixture PRs with varied test plans, including a synthetic destructive-pattern entry) should happen before merge.