Skip to content

Fix is_clarification_request to defer to final state marker over embedded AGENT_CLARIFY#280

Closed
wwind123 wants to merge 1 commit into
mainfrom
fix-278-clarify-marker-precedence
Closed

Fix is_clarification_request to defer to final state marker over embedded AGENT_CLARIFY#280
wwind123 wants to merge 1 commit into
mainfrom
fix-278-clarify-marker-precedence

Conversation

@wwind123

@wwind123 wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • is_clarification_request() previously returned True for any response containing <!-- AGENT_CLARIFY --> anywhere, including examples in prose or code blocks.
  • A plan ending with <!-- AGENT_PLAN_STATE: blocking --> but containing a literal AGENT_CLARIFY example in its body would incorrectly abort the loop as a clarification request.
  • Fix: when a valid AGENT_STATE, AGENT_PLAN_STATE, or AGENT_PR marker appears after the last AGENT_CLARIFY in the text, the state marker takes precedence and the response is not treated as a clarification.

Fixes #278

Test plan

  • test_is_clarification_request_state_marker_after_clarify_wins: plan/PR/coder body with embedded AGENT_CLARIFY example followed by a valid footer → not clarification.
  • test_is_clarification_request_clarify_as_final_marker_is_still_detected: AGENT_CLARIFY as the final marker → still a clarification; AGENT_STATE in body then AGENT_CLARIFY at end → clarification wins.
  • test_is_clarification_request_code_block_example_does_not_trigger: AGENT_CLARIFY in a fenced code block + AGENT_PLAN_STATE footer → not clarification.
  • All 710 existing tests continue to pass.

🤖 Generated with Claude Code

…en it is the final marker

A valid AGENT_STATE, AGENT_PLAN_STATE, or AGENT_PR marker appearing after the last
AGENT_CLARIFY now takes precedence, preventing embedded protocol examples in plan
prose or code blocks from triggering spurious clarification handling.

Fixes #278

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wwind123

wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

Fix: AGENT_CLARIFY no longer overrides a valid final state marker

Root cause

is_clarification_request() in protocol.py called CLARIFY_RE.search(text), which matched any occurrence of <!-- AGENT_CLARIFY --> anywhere in the response — including literal examples in plan prose or code blocks describing the protocol.

Fix

is_clarification_request() now checks whether a valid AGENT_STATE, AGENT_PLAN_STATE, or AGENT_PR marker appears after the last AGENT_CLARIFY match. If one does, that final state marker takes precedence and the response is not treated as a clarification request.

def is_clarification_request(text: str) -> bool:
    matches = list(CLARIFY_RE.finditer(text))
    if not matches:
        return False
    last_clarify_pos = matches[-1].end()
    remaining = text[last_clarify_pos:]
    if STATE_RE.search(remaining) or PLAN_STATE_RE.search(remaining) or PR_RE.search(remaining):
        return False
    return True

Acceptance criteria coverage

  • Plan with embedded AGENT_CLARIFY example + ending AGENT_PLAN_STATE: blocking → handled as blocking
  • Coder/PR response with embedded AGENT_CLARIFY example + ending AGENT_STATE: blocking or AGENT_PR: Nnot treated as clarification ✓
  • Response whose final marker is AGENT_CLARIFY → still treated as clarification
  • AGENT_STATE in body then AGENT_CLARIFY at end → clarification wins ✓

All 710 existing tests pass; 6 new tests added covering issue #278 shapes.

-- Anthropic Claude

@wwind123

wwind123 commented Jun 8, 2026

Copy link
Copy Markdown
Owner Author

Accidental duplicate of PR #279

@wwind123 wwind123 closed this Jun 8, 2026
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.

Treat clarification markers as active only when they are final response markers

1 participant