feat: incremental reviews, timestamped dirs, worktree discovery

[dean0x]

Summary

Implements incremental code reviews with timestamped directories and automatic git worktree discovery for parallel review processing.

Changes

• Incremental Reviews: Track last reviewed HEAD SHA in .last-review-head; second review only diffs from last commit
• Timestamped Directories: Reports organized in .docs/reviews/{branch-slug}/{YYYY-MM-DD_HHMM}/ subdirectories
• Worktree Discovery: Auto-detect all reviewable branches in git worktrees and process in parallel
• Agent Updates: Reviewer, Resolver, and Git agents updated to support new structure
• Skills: New pipeline-orchestration, review-orchestration, resolve-orchestration skills

Breaking Changes

None. Review directory structure enhanced; existing reviews remain accessible.

Testing

• Multi-branch review workflows tested with worktree discovery
• Incremental review diffs verified against last HEAD
• Timestamped directory creation and naming conventions validated
• New orchestration skills integration tested

Related Issues

None at this time.

[dean0x]

Code Review Summary: Incremental Reviews & Worktree Discovery

Status: CHANGES_REQUESTED
Confidence: 80%+ for blocking issues

This comprehensive feature adds incremental review capabilities and multi-worktree auto-discovery across 23 files. The design is well-conceived — incremental diffs significantly improve performance and the timestamped directory structure enables clean separation of review batches. However, the implementation introduces DRY violations and several regression risks that must be addressed before merge.

---

Overall Recommendation

Block Merge until HIGH-severity blocking issues are resolved. The feature is architecturally sound but has concrete blocking regressions (synthesizer glob mismatch, DIFF_COMMAND inconsistency, path traversal, input validation gaps) that require fixes. Most fixes are 1-2 line edits.

Reviewer consensus: 8 reviewers flagged HIGH-severity blocking issues. All other findings (MEDIUM/LOW) are consistency, documentation, or test gaps that improve quality but are not runtime blockers.

---

Blocking Issues (HIGH Confidence ≥80%)

Critical Path Regression: Synthesizer Glob Mismatch (Regression, HIGH, 92% confidence)

• Impact: BLOCKING — Risk of data corruption
• Location: shared/agents/synthesizer.md:139
• Problem: Output filename changed from review-summary.${TIMESTAMP}.md → review-summary.md, but self-exclusion pattern is still review-summary.*.md (glob requiring at least 1 char). On incremental reviews, synthesizer will re-ingest its own previous output.
• Fix: Change line 139 from exclude review-summary.*.md to exclude review-summary.md and resolution-summary.md
• Estimated effort: 1 line

Security: Path Traversal via --path Parameter (Security, HIGH, 82% confidence)

• Impact: BLOCKING — Defense-in-depth input validation
• Location: plugins/devflow-code-review/commands/code-review.md:15, plugins/devflow-resolve/commands/resolve.md:15
• Problem: --path is used directly in git -C {worktree_path} and .docs/reviews/{worktree_path}/... without validation. No check that path is a legitimate worktree or within repo boundaries.
• Fix: Add validation step: verify path appears in git worktree list output before use.
• Estimated effort: 3-4 lines of instruction

Regression: Unsanitized SHA from .last-review-head (Security, HIGH, 80% confidence)

• Impact: BLOCKING — Shell injection risk
• Location: plugins/devflow-code-review/commands/code-review.md:59-63
• Problem: SHA read from .last-review-head is used directly in git diff {sha}...HEAD without format validation (e.g., could contain shell metacharacters if file corrupted).
• Fix: Add SHA format validation before use (must match ^[0-9a-f]{7,40}$).
• Estimated effort: 2-3 lines of instruction

Regression: DIFF_COMMAND Inconsistency (Regression, HIGH, 88% confidence)

• Impact: BLOCKING — Incorrect diffs in multi-worktree mode
• Location: plugins/devflow-code-review/commands/code-review.md:120 vs code-review-teams.md:125
• Problem: Non-teams variant passes DIFF_COMMAND: git diff {DIFF_RANGE} (no -C prefix), while teams variant correctly includes git {-C worktree_path} diff {DIFF_RANGE}. In multi-worktree mode, reviewers will run diff in wrong directory.
• Fix: Align both variants: DIFF_COMMAND: git {-C worktree_path} diff {DIFF_RANGE} or add explicit note to prefix with -C if WORKTREE_PATH provided.
• Estimated effort: 1 line

Test: Missing Preamble Drift Detection (Tests, HIGH, 88% confidence)

• Impact: BLOCKING — Test failure in CI/CD
• Location: tests/ambient.test.ts:250-298
• Problem: Existing preamble drift detection test was designed to catch exactly this kind of addition but does not include the new MULTI_WORKTREE keyword. If test runs and fails, CI will block.
• Fix: Add assertion to existing test: expect(shellPreamble).toContain('MULTI_WORKTREE');
• Estimated effort: 1 line

API Performance: Unbounded PR Comment Fetch (Performance, HIGH, 85% confidence)

• Impact: BLOCKING — Performance degradation
• Location: shared/agents/git.md:190 (comment-pr process)
• Problem: Dedup logic fetches existing PR comments before each new comment creation, resulting in O(n*m) API calls. For 20 inline comments, creates 20+ extra API round-trips.
• Fix: Restructure to fetch comments once before loop, build lookup set, then skip based on cached dedup set.
• Estimated effort: 4-5 lines of instruction restructuring

---

Blocking Consistency Issues (HIGH Confidence ≥80%)

Missing Backwards Compatibility Section (Consistency, HIGH, 90% confidence)

• Location: plugins/devflow-resolve/commands/resolve-teams.md
• Problem: Base variant has "## Backwards Compatibility" section but teams variant doesn't. All four command files should have it (both code-review variants and both resolve variants have it).
• Fix: Copy section from resolve.md:249-253 to resolve-teams.md.
• Estimated effort: 4-5 lines

Git Command Syntax Ambiguity (Consistency, HIGH, 92% confidence)

• Location: plugins/devflow-code-review/commands/code-review-teams.md:125,140,155,174
• Problem: Reviewer prompts use git {-C worktree_path} diff with inconsistent brace syntax that mixes placeholder semantics. Ambiguous whether entire -C flag is conditional or just the path.
• Fix: Use explicit conditional notation: git -C {worktree_path} diff {DIFF_RANGE} (prefix with -C if WORKTREE_PATH provided) or adopt base variant's approach with separate WORKTREE_PATH parameter.
• Estimated effort: 2-3 lines

---

DRY Violations (Architecture, HIGH Confidence 85-90%)

These are not runtime blockers but architectural debt that compounds over time. All three reviewers flagged the same duplication pattern (mirrors existing PF-005 pitfall).

Worktree Support Boilerplate Across 9 Agents (88-90% confidence)

• Files: 9 agent files (coder, git, resolver, reviewer, scrutinizer, shepherd, simplifier, synthesizer, validator)
• Problem: Identical 6-8 line "Worktree Support (Optional)" section copied to each file. When logic changes (e.g., new path resolution rule), all 9 must be updated in lockstep.
• Recommended approach: Extract to shared reference document (docs/reference/worktree-patterns.md or skill) and reference from each agent with single line.

Worktree Discovery Logic Duplicated Across 4 Commands (88-90% confidence)

• Files: code-review.md, code-review-teams.md, resolve.md, resolve-teams.md (all Step 0a)
• Problem: 7-step discovery algorithm (branch filtering, protected branch exclusion, rebase/merge detection, dedup) appears identically in all 4 files. Change to algorithm or protected branch list requires 4 synchronized edits.
• Recommended approach: Extract to shared skill (shared/skills/worktree-discovery/SKILL.md) and reference from each command.

Incremental Detection Logic Duplicated Between Variants (85% confidence)

• Files: code-review.md and code-review-teams.md (both Step 0c)
• Problem: Incremental detection (check .last-review-head, verify SHA reachable, set DIFF_RANGE, handle timestamp collisions) is complex enough (5+ steps with branching) to warrant extraction.
• Recommended approach: Extract to reference document or skill.

---

Should-Fix Issues (MEDIUM Confidence 80-85%)

These are not blocking but should be fixed before merge to prevent future divergence.

Missing Skimmer Worktree Support (Consistency, MEDIUM, 80% confidence)

• Location: shared/agents/skimmer.md
• Problem: All 9 other agents received Worktree Support section, but skimmer (used by plan-orchestration which now passes WORKTREE_PATH) is missing it.
• Fix: Add standard Worktree Support section (3-4 lines).

Missing WORKTREE_PATH Documentation in Git Agent Ops Table (Documentation, MEDIUM, 80% confidence)

• Location: shared/agents/git.md:30-36
• Problem: Operations table doesn't clarify which operations support WORKTREE_PATH. Reader has to infer from context.
• Fix: Add brief note: "Only review/resolve operations support WORKTREE_PATH. Task setup and release operations always use cwd."

Missing Timestamp Validation (Security, MEDIUM, 83% confidence)

• Location: plugins/devflow-resolve/commands/resolve.md:14
• Problem: --review {timestamp} parameter used to construct paths without format validation. Crafted input like ../../etc could traverse directories.
• Fix: Add format validation: must match YYYY-MM-DD_HHMM pattern.

Redundant Backwards Compatibility Bullet (Documentation, MEDIUM, 82% confidence)

• Location: plugins/devflow-resolve/commands/resolve.md:252-253
• Problem: Two bullets describe same fallback behavior ("Legacy flat layout" and "No timestamped directories").
• Fix: Consolidate into single bullet.

Edge Case Table Divergence (Consistency, MEDIUM, 88% confidence)

• Location: code-review.md:207-221 vs code-review-teams.md:330-344
• Problem: Base variant has "Many worktrees (5+)" edge case; teams variant doesn't.
• Fix: Add missing row to teams variant or document intentional difference.

---

Test Gaps

Severity: HIGH (no tests added for significant behavioral changes)
Files: 0 new test files, but 1 existing test needs update

Required: Update Preamble Drift Detection Test

• Add expect(shellPreamble).toContain('MULTI_WORKTREE'); to existing test
• Add other missing classification checks: RESOLVE:, PIPELINE:, REVIEW depth:

Recommended: Add get_review_path() Shell Tests

• The new timestamped directory contract in docs-framework/references/patterns.md:203-213 has no test coverage
• Consider adding shell helper tests for path generation logic

---

Impact Summary

Category	CRITICAL	HIGH	MEDIUM	LOW
Blocking	0	7	5	0
DRY Violations	0	3	0	0
Should Fix	0	0	5	0
Test Gaps	0	1	2	2

Total action items: 23 fixes (7 blocking, 5 should-fix, 3 DRY refactors, 2 tests)
Estimated effort to fix: 45-60 minutes (mostly 1-3 line edits)

---

What's Working Well ✓

• Feature design: Incremental reviews + timestamped directories are clever and well-structured
• Backwards compatibility: Fallback to flat layout is clean and doesn't break existing projects
• Worktree auto-discovery: 7-step filtering algorithm is thorough (handles rebase/merge, protected branches, dedup)
• Edge case handling: Comprehensive table covering multi-worktree, incremental, and legacy scenarios
• Ambient mode integration: New MULTI_WORKTREE classification fits cleanly into existing intent routing
• Documentation coverage: 23 files updated in lockstep (commands, agents, skills, hooks, references)

---

Merge Readiness Checklist

• Fix 7 HIGH blocking issues (synthesizer glob, DIFF_COMMAND, path validation, SHA validation, git syntax, backwards compat, preamble test)
• Add Skimmer Worktree Support section
• Consolidate DRY violations (worktree discovery to skill, support section to reference doc)
• Fix edge case table divergence
• Add timestamp validation
• Update preamble drift detection test
• Run full test suite: npm test
• Verify command execution: /code-review --help and /resolve --help

After fixes, recommendation will be APPROVED.

---

Report generated by Architecture, Complexity, Consistency, Documentation, Performance, Regression, Security, and Tests reviewers.