Skip to content

feat: add Step 8.5 PRE-PR CODE REVIEW to maestro#16

Open
vs-praveen-chittem wants to merge 1 commit intomainfrom
feat/step-8-5-pre-pr-review
Open

feat: add Step 8.5 PRE-PR CODE REVIEW to maestro#16
vs-praveen-chittem wants to merge 1 commit intomainfrom
feat/step-8-5-pre-pr-review

Conversation

@vs-praveen-chittem
Copy link
Copy Markdown
Collaborator

Summary

  • Inserts new Step 8.5 PRE-PR CODE REVIEW between Step 8 VERIFY and Step 9 FINISH
  • Step 8.5 dispatches code-reviewer (always), security-reviewer (when the diff touches auth/input/DB/LLM/secrets), and any installed language-specific reviewers in parallel against the local diff
  • CRITICAL + HIGH findings block Step 9; MEDIUM fixed where practical; LOW deferred to Step 10
  • Trivial config/docs changes skip the step (CLASSIFY skip table updated). Bug fixes still run it.
  • Bumps version 1.7.1 to 1.7.2 and adds the pre-pr-review keyword

Why

Maestro's Step 10 only fires AFTER the PR is open and runs against the merged-base diff. Nothing previously enforced a local diff review BEFORE push. The user's CLAUDE.md rule "code review BEFORE PR creation" was relying on memory rather than the workflow itself. Step 8.5 makes it explicit and saves a full claude-review bot cycle on every PR.

Why a separate step (not folded into Step 8 or Step 10)

Step 8 VERIFY enforces correctness (tests, types, lint). Step 10 REVIEW is post-PR external review. Step 8.5 owns the local pre-push window where a focused diff review prevents predictable bot rework. Single-purpose steps stay readable.

Test plan

  • Read skills/maestro/SKILL.md end to end and confirm flow diagram, skip table, and Step 8.5 section all render correctly in markdown
  • Confirm 8.5 digit alignment under 8 in the flow diagram
  • Confirm no broken cross-references to "Step 9", "Step 10", "Step 6" elsewhere in the file
  • Confirm plugin.json is valid JSON and version is 1.7.2
  • Manually walk through one task using the new flow and confirm Step 8.5 makes sense in practice

Trade-offs acknowledged

Adds one parallel agent dispatch on every non-trivial task before commit. That latency is the cost of catching issues locally instead of burning a claude-review bot cycle. Trivial changes are unaffected via the skip rule.

Closes the gap between local correctness gates (Step 8 VERIFY) and the
external PR review polling loop (Step 10). Runs code-reviewer (and
security-reviewer when the diff touches sensitive code, plus any
language-specific reviewers) in parallel against the local diff before
the PR is opened, blocking on CRITICAL and HIGH findings.

Skip rule: trivial config/docs changes bypass the step. Bug fixes do
not bypass it.

Bumps plugin to 1.7.2 and adds the pre-pr-review keyword.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants