Skip to content

[Feature]: /speckit.review — comments-only mode, batch-reject, post-merge verification #2054

@SteeZyT33

Description

@SteeZyT33

Problem Statement

During a real-world spec execution (75 tasks, PR #130 with 38 external reviewer comments), /speckit.review was invoked three times:

  1. Self-review: Spec compliance + code quality + security passes → all PASS
  2. Copilot comments: 22 comments from copilot-pull-request-reviewer → 8 ADDRESSED, 14 REJECTED
  3. CodeRabbit comments: 16 comments from coderabbitai[bot] → 2 ADDRESSED, 14 REJECTED (1 overlap with Copilot)

Each invocation re-ran all three review passes even when the user only wanted to process new PR comments. 71% of external comments were rejected, many following obvious patterns (vendor code, salvaged reference material). A post-merge linter silently reverted several reviewed changes, which the skill did not detect.

Proposed Solution

1. --comments-only mode

Skip spec compliance / code quality / security passes entirely. Jump straight to the Comment Response Protocol for new PR comments.

/speckit.review --comments-only

This addresses the common pattern: self-review passes once, then external reviewers leave comments that need responses without re-reviewing the entire implementation.

2. Batch-reject for path patterns

When 3+ rejections share a path pattern, offer batch rejection:

Detected pattern: 6 comments target .specify/scripts/* (vendor code)
Batch reject all with: "REJECTED — vendor code managed upstream by spec-kit"? [y/N]

Configurable via an exclusion paths list in the spec feature directory:

<!-- review-exclusions.md -->
- .specify/scripts/* — vendor code, upstream responsibility
- docs/research/kanban-exploration/* — salvaged reference material, not production
- specs/legacy/* — historical specs, already executed

3. Post-merge verification

After merge, diff main against the last reviewed commit. Flag any silent reversions:

Post-merge verification:
  REVERTED: .github/workflows/ci.yml:3 — permissions moved back to workflow-level
  REVERTED: .claude-plugin/plugin.json:3 — version changed from "0.1.0" to "5.1.1"
  REVERTED: scripts/query-index.py:166 — nosec comment removed

This catches linters, auto-fixers, or post-merge hooks that silently undo reviewed changes.

4. Conversation thread resolution guidance

Add to the PR lifecycle section:

Merge readiness: If branch protection requires conversation resolution, all review threads must be resolved before merge. Use gh api graphql with resolveReviewThread mutation to batch-resolve threads after responding to all comments.

During Spec 1, 34 unresolved threads blocked the merge and required a GraphQL batch operation to resolve.

5. CI debug structure for new integrations

Replace the vague "fix and push again" loop with structured guidance for new CI tool integration:

For new CI tools (first-time scanner integration):
1. Enable one scanner at a time
2. Run locally first to identify findings
3. Commit triage per-scanner (nosec, config, exclusions)
4. Push and verify one scanner passes before adding the next
5. Expect 3-5 rounds for a multi-scanner setup

6. Reviewer profile awareness

Track what each external reviewer focuses on to pre-categorize comments:

Reviewer Focus Areas Common False Positives
copilot-pull-request-reviewer Error handling, input validation, type safety Flags vendor code, reference material
coderabbitai[bot] Architecture, documentation, naming Flags salvaged historical content

Alternatives Considered

  • Separate /speckit.respond command: Considered but adds command proliferation — --comments-only on existing command is simpler
  • Auto-reject all vendor paths: Too aggressive — some vendor comments may be valid (e.g., security issues in vendor code you ship)
  • Skip post-merge verification: Tempting but the linter reversion problem is real and silent — better to catch it

Component

Spec templates (BDD, Testing Strategy, etc.)

AI Agent

All agents

Use Cases

  1. Iterative PR review: Self-review passes once, then Copilot comments arrive, then CodeRabbit comments arrive. Each pass should only process new comments, not re-run the full review.
  2. Vendor code noise: Projects using spec-kit have .specify/scripts/ (vendor) that external reviewers always flag. Batch-reject eliminates repetitive work.
  3. Post-merge integrity: Linters, auto-formatters, or CI bots that run after merge can silently revert reviewed changes. Post-merge verification catches this before it becomes a bug.
  4. Branch protection with conversation resolution: Common enterprise GitHub setup that blocks merges until all threads are resolved. The skill should guide users through this.

Acceptance Criteria

  • --comments-only flag skips review passes and processes only new PR comments
  • Batch-reject detection when 3+ comments share a path pattern
  • Optional review-exclusions.md (or equivalent) for declaring known exclusion paths
  • Post-merge verification step that diffs merged code against last reviewed commit
  • Conversation thread resolution mentioned in PR lifecycle documentation
  • Structured CI debugging guidance for new scanner integrations
  • review.md clearly separates self-review findings from external comment responses

Additional Context

Field report from perf-lab Spec 1 (002-cleanup-cicd-release): 38 external comments (22 Copilot + 16 CodeRabbit), 27 rejected (71%). Security scanner integration required 5+ CI rounds. Post-merge linter reverted workflow permissions, plugin version, and nosec comments on main.

Source file: templates/commands/review.md

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions