Skip to content

Add plan-first issue review stage#76

Merged
wwind123 merged 1 commit into
mainfrom
fix-issue-64-plan-first
May 18, 2026
Merged

Add plan-first issue review stage#76
wwind123 merged 1 commit into
mainfrom
fix-issue-64-plan-first

Conversation

@wwind123
Copy link
Copy Markdown
Owner

Summary

  • add optional agent-loop issue --plan-first planning/review loop with AGENT_PLAN_STATE markers
  • support --implement-after-approval to continue into the existing PR flow with the approved plan
  • post plan-stage comments and consensus summary back to the issue
  • document the new issue workflow and protocol markers

Tests

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

Fixes #64

@wwind123
Copy link
Copy Markdown
Owner Author

Implemented and opened PR #76 against main:

#76

Tests passed: /tmp/coding-review-agent-loop/scratch/test-venv/bin/python -m pytest

-- OpenAI Codex

@wwind123
Copy link
Copy Markdown
Owner Author

Plan-first review — PR #76

The implementation is well-structured and correct. The new --plan-first flow is a clean addition that doesn't disturb the existing issue/PR/task loops.

Protocol and orchestration

parse_plan_state (protocol.py) mirrors parse_agent_state exactly — it extracts the final AGENT_PLAN_STATE marker, raises on absence, and returns the lowercased value. The orchestrator correctly validates the coder's initial plan and each revision for marker presence without checking whether the value is "blocking" or "approved" (since the coder always emits "blocking" as a handoff signal; only reviewers can emit "approved"). This is consistent with the design and documented in the prompts.

post_issue_comment in github.py

The function correctly mirrors post_pr_comment: dry_run path passes --body inline, normal path writes to a temp file and passes --body-file. The finally: os.unlink guard is in place. No issues.

Prompt design

All four new prompt builders (build_issue_plan_prompt, build_plan_review_prompt, build_plan_revision_prompt, build_issue_implementation_prompt) are clear and internally consistent. The explicit prohibition on file edits, branch pushes, and PR creation during planning is stated in each planning-stage prompt. The coder is never offered an "approved" option — only "blocking" to hand off for review, or AGENT_CLARIFY for ambiguous issues. Reviewers then use the standard "approved"/"blocking" dichotomy.

Session continuity

coder_session_id is threaded through the plan → revision → implementation path. Per-reviewer session IDs are tracked in reviewer_session_ids as in run_pr_loop. Correct.

Test coverage

Three new tests validate the three main scenarios:

  • test_issue_loop_plan_first_stops_after_approved_plan — approval in round 1, stop without implementing. Comment count (3) traced correctly: plan, reviewer approval, consensus summary.
  • test_issue_loop_plan_first_revises_until_all_reviewers_approve — one blocking round then approval. Comment count (5) correct. Verifies revision prompt includes blocking review text.
  • test_issue_loop_plan_first_can_implement_after_approval — approval then implementation → PR review loop. Comment count (5) correct: 3 issue comments + 2 PR comments.

The gh issue comment stub in FakeRunner captures both --body-file and --body paths, consistent with how the PR comment stub works.

Alignment with issue refinement

The owner's refinement comment describes the exact CLI shape this PR implements: --plan-first (stops after approval) and --plan-first --implement-after-approval (continues to implementation). The broader discuss mode with multiple outcomes (do-not-implement, needs-human, split) is a natural next step but is explicitly out of scope here.

Future follow-ups

  • The broader discuss mode proposed in the issue refinement comment — multiple outcome types (implement, do-not-implement, needs-human, split) posted as a signed consensus comment — would generalize the planning infrastructure already laid down here.

-- Anthropic Claude

@wwind123
Copy link
Copy Markdown
Owner Author

PR #76 implements the "plan-first" review stage for issues, as requested in issue #64. The implementation is comprehensive and follows the existing patterns in the codebase.

Key Changes:

  • CLI Flags: Added --plan-first and --implement-after-approval to the issue command.
  • Protocol: Introduced AGENT_PLAN_STATE: approved|blocking markers to manage the planning lifecycle separately from the implementation/PR lifecycle.
  • Orchestration:
    • Added _run_plan_first_loop which handles the multi-round planning process.
    • Ensures all configured reviewers approve the plan in the same round before proceeding.
    • Supports stopping after plan approval or continuing to implementation.
    • Maintains agent session state across planning and implementation phases.
  • Prompts: Added specific prompts for plan generation, plan review, plan revision, and implementation based on an approved plan.
  • GitHub Integration: Added post_issue_comment to facilitate agent discussion directly on the issue.
  • Documentation: Updated README.md and docs/local_agent_loop.md with instructions for the new mode.
  • Testing: Added thorough tests covering approval, revision, and implementation flows.

Future follow-ups:

  • Interactive Clarification: Extending the interactive clarification flow to issue mode (planning and implementation).
  • Auto-enablement: Automatically triggering plan-first based on issue labels (e.g., architecture, large).
  • Discussion Mode: Implementing a dedicated discuss command for broader triage that doesn't necessarily lead to implementation.

The PR is well-structured, follows project conventions, and includes robust tests.

-- Google Gemini

@wwind123
Copy link
Copy Markdown
Owner Author

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

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

-- coding-review-agent-loop

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.

Add optional plan-first review stage for issue mode

1 participant