Reject invalid issue implementation PR markers

[wwind123]

Fixes #570

• classify explicit PR references as absent, valid, or invalid and require positive IDs
• preserve blocking and clarification implementation outcomes before PR-dependent checks
• document the no-PR contract and cover it with parser/orchestrator regressions

[wwind123]

Implemented issue #570 and opened PR #572.

Tests: python3 -m pytest tests/test_skill_helpers.py::TestRunImplement::test_dry_run_runs_coder_and_prints_pr tests/test_skill_helpers.py::TestRunImplementByPhase::test_dry_run_runs_decomposition_and_implementation_preview tests/test_response_validation.py tests/test_orchestrator_issue.py tests/test_prompts.py -q (235 passed); python3 -m pytest -q (1555 passed).

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Blocking

The AGENT_PR: 0/negative/malformed rejection and the no-PR blocking/clarification terminal path are implemented correctly at both issue-implementation entry points, with human-requirements enforcement now correctly gated on the positive-PR classification (resolving the prior blocking findings) and validate_assigned_head_advanced correctly skipped for the no-PR terminal outcome. Regression tests cover AGENT_PR: 0 combined with blocking/clarification for both entry points, including a signed-human-requirements issue, and assert no gh pr view call occurs. Two small cleanup items from this PR's own refactor should be fixed before merge. GitHub checks are still pending (test: in_progress); not used as a blocking reason here.

Same-PR follow-ups

• protocol.py:22 - PR_RE is now dead code: parse_pr_number/classify_pr_reference were switched to _PR_MARKER_VALUE_RE, and nothing else references PR_RE anymore. Remove it so a future reader doesn't mistake it for the authoritative AGENT_PR regex.
• orchestrator.py:2323-2326 - When classify_pr_reference returns kind=="invalid" and no terminal blocking/clarify marker is present, the raised message reads 'did not include a valid invalid PR marker or PR URL', which is self-contradictory. Reword (e.g. 'the AGENT_PR marker/URL present was invalid' vs 'did not include a PR marker or PR URL') so the diagnostic stays clear, consistent with this PR's own goal of a clear diagnostic.

-- Anthropic Claude: claude-sonnet-5

[wwind123]

Coder follow-up

Implemented and pushed both requested same-PR cleanup fixes for reviewer follow-up.

Addressed items

• item-1: Same-PR follow-up from Anthropic Claude: claude-sonnet-5, round 1: protocol.py:22 - PR_RE is now dead code: parse_pr_number/classify_pr_reference were switche...
  • Resolution: Removed the unused PR_RE definition; _PR_MARKER_VALUE_RE remains the authoritative AGENT_PR marker parser.
• item-2: Same-PR follow-up from Anthropic Claude: claude-sonnet-5, round 1: orchestrator.py:2323-2326 - When classify_pr_reference returns kind=="invalid" and no terminal...
  • Resolution: Reworded the invalid-reference diagnostic to state that the present AGENT_PR marker or PR URL was invalid, and added a regression assertion.

Remaining items

• None.

Tests run

• python3 -m pytest tests/test_response_validation.py tests/test_orchestrator_issue.py -k 'parse_pr_number or classify_pr_reference or invalid_pr' (5 passed)
• python3 -m pytest -q (1555 passed)

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Approved

Both round-1 Same-PR follow-ups are resolved: the dead PR_RE regex was removed from protocol.py, and the self-contradictory 'valid invalid PR marker' error message in orchestrator.py was reworded to distinguish invalid-marker vs missing-marker cases, with the test updated to match. No new issues found; the AGENT_PR: 0/negative/malformed rejection and no-PR blocking/clarification handling remain correct and well-tested. GitHub checks are still pending (test: in_progress); not used as a blocking reason here.

Prior unresolved item dispositions

• [item-1] Same-PR follow-up from Anthropic Claude: claude-sonnet-5, round 1: protocol.py:22 - PR_RE is now dead code: parse_pr_number/classify_pr_reference were switche... -> resolved
• [item-2] Same-PR follow-up from Anthropic Claude: claude-sonnet-5, round 1: orchestrator.py:2323-2326 - When classify_pr_reference returns kind=="invalid" and no terminal... -> resolved

-- Anthropic Claude: claude-sonnet-5