Add plan-first issue review stage

[wwind123]

Summary

• add an opt-in agent-loop issue --plan-first planning gate before implementation
• add separate AGENT_PLAN_STATE parsing and plan review/revision prompts
• pass the approved plan into the normal issue implementation prompt and document the flag

Tests

• /tmp/coding-review-agent-loop/scratch/venv/bin/python -m pytest

Fixes #64

-- OpenAI Codex

[wwind123]

Implemented issue #64 and opened PR #65:

#65

Added agent-loop issue --plan-first, separate AGENT_PLAN_STATE parsing, planning/review/revision prompts, approved-plan handoff into implementation, docs, and regression coverage.

Tests passed:

/tmp/coding-review-agent-loop/scratch/venv/bin/python -m pytest

-- OpenAI Codex

[wwind123]

Plan-first issue review stage — PR #65 review

The implementation is clean, complete, and correct. Here is my analysis.

Correctness

Planning loop logic (_run_issue_plan_loop): The state machine is well-structured. The coder produces a plan with AGENT_PLAN_STATE: blocking; reviewers approve or block; if all approve, the plan is returned; if any block and max_rounds hasn't been reached, the coder revises; at max rounds a clear error is raised. The outer raise AgentLoopError("Reached max planning rounds …") after the for loop is correctly reachable when max_rounds=0 and acts as the appropriate fallback for that edge case.

Protocol separation: AGENT_PLAN_STATE uses a distinct regex (PLAN_STATE_RE) that is entirely separate from the existing STATE_RE / AGENT_STATE regex. There is no cross-contamination: plan-stage markers cannot be mistaken for implementation-stage markers and vice versa. Both parsers use last-match-wins, consistent with the existing convention.

Session ID continuity: The coder_session_id returned by _run_issue_plan_loop is passed as session_id to the implementation run_agent call, giving the coder full context of the approved plan when writing code. When plan_first=False, coder_session_id=None is passed — identical to the original code path.

No-reviewer edge case: With an empty reviewer list, blocking_reviews stays empty and the plan is immediately approved — consistent with how run_pr_loop behaves with no reviewers.

make_config compatibility: AgentLoopConfig.plan_first defaults to False, so the existing make_config(tmp_path, **overrides) helper transparently accepts plan_first=True without modification.

Security

No concerns introduced.

Test coverage

Five new tests are solid:

• test_parse_plan_state_accepts_separate_planning_marker — unit test, verifies isolation from AGENT_STATE
• test_issue_loop_plan_first_gets_plan_approval_before_implementation — verifies command ordering, prompt content (planning constraints, markers, approved plan handoff), and comment posting
• test_issue_loop_plan_first_revises_plan_until_all_reviewers_approve — verifies revision round with reviewer feedback injected
• test_issue_loop_plan_first_fails_after_max_planning_rounds — verifies hard stop with correct error text and no spurious PR comments
• test_issue_plan_first_cli_flag_is_configurable — verifies CLI argument wiring through to AgentLoopConfig

Same-PR follow-ups

None required.

Future follow-ups

1. The planning loop shares config.max_rounds with the implementation loop. For projects that want different limits for planning vs. implementation (e.g., allow more revision rounds in planning), a separate --plan-max-rounds option could be added.

2. When plan_first=True is used with the task subcommand (which has no --plan-first flag), the getattr(args, "plan_first", False) guard in config_from_args silently sets plan_first=False. If planning ever needs to be available for tasks, the CLI wiring will need a separate addition.

The feature is well-scoped, fully tested, and ready to merge.

-- Anthropic Claude

[wwind123]

Plan-first issue review stage — PR #65 review

The implementation is solid, well-integrated, and follows the existing orchestration patterns perfectly.

Correctness & Logic

• Planning Loop (_run_issue_plan_loop): The state machine correctly handles the coder producing a plan, multiple reviewers providing feedback, and the coder revising until approval or max_rounds is reached.
• Protocol Separation: Using AGENT_PLAN_STATE (distinct from AGENT_STATE) prevents any ambiguity between planning and implementation markers. The last-match-wins parsing is consistent with the rest of the project.
• Session Continuity: Passing the coder_session_id from the planning loop into the implementation stage ensures the coder has full context of the design discussion when starting to write code.
• Edge Cases:
  • No reviewers: Plan is immediately approved, matching run_pr_loop behavior.
  • Max rounds: Correctly raises AgentLoopError with clear messaging.
  • Empty output: Properly caught and reported for both coder and reviewers.

Documentation & CLI

• The opt-in --plan-first flag is correctly added to the issue subcommand and documented in both the README.md and docs/local_agent_loop.md.
• Help text is concise and accurate.

Test Coverage

• The five new tests in tests/test_agent_loop.py provide excellent coverage for:
  • Protocol parsing (parse_plan_state).
  • Success path (planning -> approval -> implementation).
  • Revision path (planning -> block -> revision -> approval -> implementation).
  • Failure path (max rounds reached).
  • CLI flag wiring.
• Verified by running python3 -m pytest tests/test_agent_loop.py (using the project's venv), where all 134 tests passed.

Security & Maintainability

• No new security risks introduced.
• Logic is modular and reuses existing abstractions (run_agent, AgentLoopConfig, reviewers).

This is a high-quality contribution that adds a valuable feature for complex issue handling.

-- Google Gemini

[wwind123]

Created approved-review future follow-up issues for PR #65:

• Follow up future review note: The planning loop shares config.max_rounds with the implementation loop. For projects th #66
• Follow up future review note: When plan_first=True is used with the task subcommand (which has no --plan-first fla #67

These were mentioned in approved reviews as future work and did not block merge readiness.

-- coding-review-agent-loop