fix(webhook): auto-resume agent on initiator's no-mention reply#23
Merged
Conversation
Symptom: BA (castor) on QSALE-20 posted «Phase 1 — 3 questions» and exited done; initiator replied in the same sub-issue without an @-mention; nothing happened. Pipeline sat frozen until a second manual request_handoff was issued. Same shape repeated across SA / coder / tester elicitation rounds — the «answered → pipeline moves» mental model broke at every handoff. Root cause: webhook handler short-circuits when the comment carries no `<mention-component>` UUIDs. Plane rejects raw mentions in outbound agent comments (tower's _assert_no_mentions), so agents end handoff comments via `request_handoff(target_role='initiator', …)` — which auto-stamps the initiator mention at the top. The initiator's reply, by contrast, is bare text with no mention, so no spawn fires. Fix (option C from the bug report): when a comment arrives with no mentions AND the actor is the initiator, scan the issue's prior comments newest-first, find the latest agent-authored one whose body opens with an initiator-mention (the request_handoff auto-stamp), and re-spawn that agent. Same downstream spawn path as the mention-driven flow — duplicate detection, capacity check, and allowlist all kick in unchanged. Response now carries `auto_resumed: <member_uuid>` so the operator can see when the soft-trigger fires. False positives are bounded: we only act on initiator-authored, no-mention comments — the conversational-noise cost («thanks!» re-spawns the agent) is one redundant agent run that re-enters, sees no new actionable input, and exits idle. Idempotent. Strictly safer than the current «sit forever» behaviour. Six new tests in tests/test_webhook.py cover: positive resume, latest-of- several wins, no-prior-ping skip, no-initiator-mention skip, non-initiator commenter skip, unregistered-member skip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds an auto-resume fallback to the webhook handler. When an issue_comment event lacks explicit mention UUIDs but the actor is the workspace initiator, the handler probes recent comments to find the latest registered agent awaiting input and respawns that agent, recording the auto_resumed outcome in the response. ChangesAuto-resume agent fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_webhook.py (1)
591-729: ⚡ Quick winAdd two guardrail tests for auto-resume error/false-positive edges.
Current coverage is strong, but it still misses:
- transient
list_issue_comments(or probe-timeget_member) returning HTTP 503, andcomment_htmlcontaining initiator UUID text without a leading<mention-component ...>stamp.Suggested test additions
+def test_auto_resume_returns_503_on_transient_comment_lookup( + settings: Settings, workspace_config: WorkspaceConfig, initiator_uuid: UUID +) -> None: + from plane_conductor.exceptions import PlaneAPIError + + class FlakyPlane(StubPlane): + async def list_issue_comments(self, project_id: Any, issue_id: Any) -> list[dict[str, Any]]: + raise PlaneAPIError(503, "service unavailable") + + client = TestClient(_app(settings, workspace_config, FlakyPlane(), StubRunner())) + resp = _send(client, settings, workspace_config, _initiator_reply_body(initiator_uuid)) + assert resp.status_code == 503 + + +def test_auto_resume_skips_when_uuid_text_exists_without_mention_component( + settings: Settings, workspace_config: WorkspaceConfig, initiator_uuid: UUID +) -> None: + plane = StubPlane( + members={SARK: {"email": "sark@example.io"}}, + comments=[ + { + "actor": SARK, + "created_at": "2026-05-18T20:43:00Z", + "comment_html": f"<p>FYI initiator id is {initiator_uuid}</p>", + } + ], + ) + runner = StubRunner() + client = TestClient(_app(settings, workspace_config, plane, runner)) + resp = _send(client, settings, workspace_config, _initiator_reply_body(initiator_uuid)) + assert resp.status_code == 200 + assert resp.json()["spawned"] == [] + assert runner.calls == []As per coding guidelines,
src/plane_conductor/webhook.py: “Transient Plane API errors must return 503; 4xx must return 200 with a skipped[] entry.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_webhook.py` around lines 591 - 729, Add two guardrails: ensure transient Plane API errors surface as 503 (not treated as skipped) by propagating HTTP 503 from list_issue_comments and probe-time get_member calls (update the webhook handler that calls list_issue_comments/get_member to detect and return a 503 response when those client calls raise or return a 503); and tighten the auto-resume comment parsing so it only treats a comment as an initiator-mention handoff when comment_html contains the actual <mention-component ...> stamp (not merely the initiator UUID string) — update the logic that inspects comment_html in the auto-resume flow to require the mention-component tag before selecting an agent to spawn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/plane_conductor/webhook.py`:
- Around line 122-131: The try/except around Plane API calls (e.g., the
plane.list_issue_comments call in the auto-resume probe) currently swallows all
PlaneAPIError and returns None; change the except PlaneAPIError handling to
distinguish transient (5xx) vs client (4xx) errors by inspecting
exc.status_code: for status >= 500 log and propagate a 503 retry response (raise
or return an HTTP 503 path so caller retries), for 400-499 log and return the
existing None/skipped behavior; apply the same pattern to the other
PlaneAPIError catch sites (the other list_issue_comments/get_issue handlers
mentioned) so transient Plane failures result in 503 while 4xx still return
skipped/200.
- Around line 142-144: The current check uses `initiator_str in body.lower()`
which matches the UUID anywhere in the comment and yields false positives;
change it so the match requires the stamped mention token at the start of the
comment (allowing only leading whitespace/HTML wrappers). Locate the `body =
(comment.get("comment_html") or "")[:_INITIATOR_MENTION_PROBE_CHARS]` and the
subsequent `if initiator_str not in body.lower():` and replace the containment
test with a start-of-comment check (e.g., operate on
`comment.get("comment_html")` or its lstripped/lowercased form and use a
starts-with or anchored regex match) so only comments that begin with the
initiator mention token (referencing `initiator_str`) pass.
---
Nitpick comments:
In `@tests/test_webhook.py`:
- Around line 591-729: Add two guardrails: ensure transient Plane API errors
surface as 503 (not treated as skipped) by propagating HTTP 503 from
list_issue_comments and probe-time get_member calls (update the webhook handler
that calls list_issue_comments/get_member to detect and return a 503 response
when those client calls raise or return a 503); and tighten the auto-resume
comment parsing so it only treats a comment as an initiator-mention handoff when
comment_html contains the actual <mention-component ...> stamp (not merely the
initiator UUID string) — update the logic that inspects comment_html in the
auto-resume flow to require the mention-component tag before selecting an agent
to spawn.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d00e90c7-84c4-4735-9eae-b0b644caa738
📒 Files selected for processing (2)
src/plane_conductor/webhook.pytests/test_webhook.py
Two CodeRabbit findings from PR #23 review: 1. `_resolve_pending_agent_member` swallowed every PlaneAPIError on the list_issue_comments / get_member probe — including 5xx — silently returning None. Project contract is «4xx → skip, 5xx → re-raise so webhook returns 503 and Plane retries». Now matches the contract: exc.is_transient re-raises; the webhook handler catches it once at the call site and turns it into 503, same code path as the mention- driven get_member error block. 2. The «agent comment opens with initiator mention» check was a plain substring match on the raw UUID inside the first 500 chars of comment_html. False-positive territory: any prose mentioning the initiator's UUID («FYI initiator id is <uuid>») would auto-resume the wrong agent. Switched to extract_mention_uuids() and require the first parsed mention to equal initiator — exact match on the structured <mention-component entity_identifier="…"> tag that tower actually stamps via request_handoff. Two new guardrail tests pin both behaviours: - test_auto_resume_skips_when_uuid_appears_as_plain_text - test_auto_resume_returns_503_on_transient_comment_lookup Total auto-resume coverage now 8 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
BA (
@castor) on QSALE-20 posted «Phase 1 — 3 questions for initiator» and exiteddone. I replied in the same sub-issue without an explicit@castormention. Nothing happened. Pipeline sat frozen until I manually issued a secondrequest_handoff(target_role='business-analyst')~5 minutes later. Same shape on every elicitation / CHANGES / verification round across SA / coder / tester — the «I answered → pipeline moves» mental model broke at every handoff.Root cause
webhook.pyshort-circuits when the comment carries no<mention-component>UUIDs (L165-172 pre-change). Tower's_assert_no_mentionsrejects raw mentions in outbound agent comments, so agents end handoff comments viarequest_handoff(target_role='initiator', …)— which auto-stamps the initiator mention at the top. The initiator's reply, by contrast, is bare text with no mention, so no spawn fires.Fix
Option C from the original bug report — tower-side auto-resume. When a comment arrives with no mentions AND the actor is the initiator, scan the issue's prior comments newest-first, find the latest agent-authored one whose
comment_htmlopens with an initiator-mention (therequest_handoffauto-stamp), and re-spawn that agent. Same downstream spawn path as the mention-driven flow — duplicate detection, capacity check, and allowlist all kick in unchanged.Response now carries
auto_resumed: <member_uuid>so the operator can see when the soft-trigger fires.False-positive cost
We only act on initiator-authored, no-mention comments. The conversational-noise cost («thanks!» re-spawns the agent) is one redundant agent run that re-enters, sees no new actionable input, and exits idle. Idempotent. Strictly safer than the current «sit forever» behaviour.
Test plan
tests/test_webhook.py— positive resume, latest-of-several wins, no-prior-ping skip, no-initiator-mention skip, non-initiator commenter skip, unregistered-member skip217 passed, 7 skippedpre-commit runclean: ruff + ruff-format + mypy all passqsaleworkspace after merge: reproduce QSALE-20 sequence — BA exits awaiting input, initiator replies bare, agent must auto-respawn within 1-2 Plane webhook triesBumps version?
No. This is webhook-handler logic in
plane-conductor; no version-file change here.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes