mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[bug] pr_review_claim: set active_claimant_id so the reviewer can post the review
pr_review_claim set assigned_to + claimed_by + heartbeat but never active_claimant_id (the single-claimant invariant _qa_or_doc_claim / _finalize_claim set). So after claim_pr_review the reviewer could read the PR but every note()/evidence() hit _active_claim_violation -> _not_active_claimant (not_authorized): the journal:learning entry post_pr_review's tracing gate requires could never be recorded, the reviewer deadlocked at post_pr_review (tracing_gap), burned tokens into the do-server breaker, idled -> task stuck paused (ccfb1bd7). Set active_claimant_id + claimed_at at claim (mirrors QA/doc); cleared by complete_review. TDD: extended test_pr_review_claim_and_complete to assert the invariant (set at claim, cleared at complete); 35 pr_review tests + ruff/mypy green.
This commit is contained in:
@@ -1386,8 +1386,18 @@ class TaskService(BaseService):
|
||||
task = await self.get(task_id)
|
||||
if task is None or task.status != TaskStatus.PENDING:
|
||||
return None
|
||||
now = datetime.now(UTC)
|
||||
task.assigned_to = cast("Any", reviewer_agent_id)
|
||||
task.claimed_by = cast("Any", reviewer_agent_id)
|
||||
task.claimed_at = now
|
||||
# Single-claimant invariant (mirrors _qa_or_doc_claim / _finalize_claim):
|
||||
# active_claimant_id is what _active_claim_violation checks for content
|
||||
# writes (note/evidence). Without it the reviewer can claim + read the PR
|
||||
# but every note() returns _not_active_claimant, so the journal:learning
|
||||
# entry post_pr_review's tracing gate requires can never be recorded ->
|
||||
# the reviewer deadlocks at post_pr_review (tracing_gap) and burns tokens
|
||||
# into the do-server breaker. Cleared by complete_review.
|
||||
task.active_claimant_id = cast("Any", reviewer_agent_id)
|
||||
self._validate_and_set_status(
|
||||
task, TaskStatus.CLAIMED, "pr_reviewer", audit_agent_id=reviewer_agent_id
|
||||
)
|
||||
|
||||
@@ -243,6 +243,16 @@ async def test_pr_review_claim_and_complete(db_session: AsyncSession) -> None:
|
||||
# claim to the reaper and, for a GROK reviewer, trips the idle-kill
|
||||
# watchdog before the review is posted (the wedge/respawn loop).
|
||||
assert claimed.last_heartbeat_at is not None
|
||||
# Single-claimant invariant (mirrors _qa_or_doc_claim / _finalize_claim):
|
||||
# active_claimant_id is what _active_claim_violation checks for content
|
||||
# writes (note/evidence). Without it the reviewer can claim + read the PR
|
||||
# but every note() returns _not_active_claimant -> the journal:learning
|
||||
# entry post_pr_review's tracing gate requires can never be recorded ->
|
||||
# the reviewer deadlocks at post_pr_review (tracing_gap) and burns tokens
|
||||
# into the do-server breaker. claimed_at is set for parity with QA/doc.
|
||||
assert claimed.active_claimant_id is not None
|
||||
assert UUID(str(claimed.active_claimant_id)) == reviewer_id
|
||||
assert claimed.claimed_at is not None
|
||||
|
||||
# Re-claiming a non-pending task is a no-op.
|
||||
assert await svc.pr_review_claim(reviewer_id, task_id) is None
|
||||
@@ -257,6 +267,8 @@ async def test_pr_review_claim_and_complete(db_session: AsyncSession) -> None:
|
||||
assert done.qa_notes is None
|
||||
assert (done.notes_structured or {}).get("pr_review", {}).get("verdict")
|
||||
assert done.claimed_by is None
|
||||
# Single-claimant lock cleared on completion (the review hand-off is done).
|
||||
assert done.active_claimant_id is None
|
||||
|
||||
# Re-completing a completed task is a no-op.
|
||||
assert await svc.complete_review(reviewer_id, task_id) is None
|
||||
|
||||
Reference in New Issue
Block a user