From f69d13a1d1a5301c71ecc6fe86417380934c678a Mon Sep 17 00:00:00 2001 From: Renn F Date: Mon, 29 Jun 2026 00:14:18 +0200 Subject: [PATCH] [F128] require active claim on explicit-task content posts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _verify_explicit_task_ownership checked assigned_to, which is stale across a reap/handoff (persists until reassignment; active_claimant_id is cleared on release). A reaped agent could keep posting say/dm/note to its former task. Add the active-claimant check when assigned_to == caller; assigned_to=None keep its existing allow (read-side inspection between reassignments uses evidence, which has its own ownership path). Existing 'active owner' test mocks passed assigned_to=agent_id without active_claimant_id; production sets both together on claim, so the mocks were incomplete. Updated to set both — realistic, not a behavior change. --- roboco/services/gateway/content_actions.py | 5 + tests/unit/gateway/test_content_actions.py | 20 ++- .../gateway/test_content_actions_ownership.py | 118 +++++++++++++++++- tests/unit/gateway/test_dm_a2a_denied.py | 7 +- 4 files changed, 142 insertions(+), 8 deletions(-) diff --git a/roboco/services/gateway/content_actions.py b/roboco/services/gateway/content_actions.py index 675baf8a..59e52c5d 100644 --- a/roboco/services/gateway/content_actions.py +++ b/roboco/services/gateway/content_actions.py @@ -583,6 +583,11 @@ class ContentActions: if await self._board_may_co_review(agent_id, t): return None return _ownership_violation(task_id) + # assigned_to is stale across a reap/handoff (persists until + # reassignment; active_claimant_id is cleared on release). Require + # the active claim so a reaped agent can't keep posting. + if t.assigned_to == agent_id: + return await self._active_claim_violation(agent_id, t) return None async def note( diff --git a/tests/unit/gateway/test_content_actions.py b/tests/unit/gateway/test_content_actions.py index d408ac2d..439e701e 100644 --- a/tests/unit/gateway/test_content_actions.py +++ b/tests/unit/gateway/test_content_actions.py @@ -252,7 +252,10 @@ async def test_note_reflect_scope_succeeds() -> None: task_svc.get_active_task_for_agent.return_value = None # When task_id is explicit, ownership is verified — agent must be assignee. task_svc.get.return_value = MagicMock( - id=task_id, assigned_to=agent_id, status="in_progress" + id=task_id, + assigned_to=agent_id, + active_claimant_id=agent_id, + status="in_progress", ) journal_svc = AsyncMock() @@ -292,7 +295,10 @@ async def test_note_reflect_missing_fields_records_with_placeholder() -> None: task_svc = AsyncMock() task_svc.get_active_task_for_agent.return_value = None task_svc.get.return_value = MagicMock( - id=task_id, assigned_to=agent_id, status="in_progress" + id=task_id, + assigned_to=agent_id, + active_claimant_id=agent_id, + status="in_progress", ) journal_svc = AsyncMock() @@ -327,7 +333,10 @@ async def test_note_decision_thin_payload_records_not_rejected() -> None: task_svc = AsyncMock() task_svc.get_active_task_for_agent.return_value = None task_svc.get.return_value = MagicMock( - id=task_id, assigned_to=agent_id, status="in_progress" + id=task_id, + assigned_to=agent_id, + active_claimant_id=agent_id, + status="in_progress", ) journal_svc = AsyncMock() @@ -396,7 +405,10 @@ async def test_note_decision_scalar_list_fields_are_coerced() -> None: task_svc = AsyncMock() task_svc.get_active_task_for_agent.return_value = None task_svc.get.return_value = MagicMock( - id=task_id, assigned_to=agent_id, status="in_progress" + id=task_id, + assigned_to=agent_id, + active_claimant_id=agent_id, + status="in_progress", ) journal_svc = AsyncMock() diff --git a/tests/unit/gateway/test_content_actions_ownership.py b/tests/unit/gateway/test_content_actions_ownership.py index 73883c5c..f663f03c 100644 --- a/tests/unit/gateway/test_content_actions_ownership.py +++ b/tests/unit/gateway/test_content_actions_ownership.py @@ -132,7 +132,12 @@ async def test_note_with_task_id_allows_when_assignee() -> None: """note(task_id=X) where X is owned by caller succeeds.""" agent_id = uuid4() task_id = uuid4() - task_obj = MagicMock(id=task_id, status="in_progress", assigned_to=agent_id) + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=agent_id, + ) task_svc = AsyncMock() task_svc.get.return_value = task_obj task_svc.get_active_task_for_agent.return_value = None @@ -223,7 +228,12 @@ async def test_say_with_explicit_task_id_owned_by_caller_succeeds() -> None: """say(task_id=X) when caller owns X: allowed.""" agent_id = uuid4() task_id = uuid4() - task_obj = MagicMock(id=task_id, status="in_progress", assigned_to=agent_id) + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=agent_id, + ) task_svc = AsyncMock() task_svc.get.return_value = task_obj messaging_svc = AsyncMock() @@ -273,7 +283,12 @@ async def test_dm_with_task_id_blocks_when_not_assignee() -> None: async def test_dm_with_task_id_owned_by_caller_succeeds() -> None: agent_id = uuid4() task_id = uuid4() - task_obj = MagicMock(id=task_id, status="in_progress", assigned_to=agent_id) + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=agent_id, + ) task_svc = AsyncMock() task_svc.get.return_value = task_obj a2a_svc = AsyncMock() @@ -510,3 +525,100 @@ async def test_evidence_allows_dependency_inspection() -> None: env = await ca.evidence(agent_id=agent_id, task_id=dep_task_id) assert env.error is None task_svc.list_assigned_for_agent.assert_awaited() + + +# --------------------------------------------------------------------------- +# Reaped/handoff window: assigned_to persists, active_claimant_id is cleared +# on release. A reaped agent must not keep posting to its former task. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_note_reaped_assignee_cannot_post_to_former_task() -> None: + """A reaped agent (assigned_to=caller, active_claimant_id=None) is rejected + on note(task_id=X) — its claim was released, so it must not journal on the + task it no longer actively holds.""" + agent_id = uuid4() + task_id = uuid4() + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, # persists across the reap + active_claimant_id=None, # cleared on release — the reaped window + ) + task_svc = AsyncMock() + task_svc.get.return_value = task_obj + journal_svc = AsyncMock() + deps = _make_deps(task=task_svc, journal=journal_svc) + ca = ContentActions(deps) + + env = await ca.note( + agent_id=agent_id, + text="Posting after my claim was reaped", + scope="note", + task_id=task_id, + ) + body = env.as_dict() + assert body["error"] == "not_authorized", body + assert "active claim" in body["message"], body + journal_svc.write_entry.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_say_reaped_assignee_cannot_post_to_former_task() -> None: + """A reaped agent is rejected on say(task_id=X) too — same stale-window + divergence as note.""" + agent_id = uuid4() + task_id = uuid4() + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=None, + ) + task_svc = AsyncMock() + task_svc.get.return_value = task_obj + messaging_svc = AsyncMock() + deps = _make_deps(task=task_svc, messaging=messaging_svc) + ca = ContentActions(deps) + + env = await ca.say( + agent_id=agent_id, + channel="backend-cell", + text="Posting after my claim was reaped", + task_id=task_id, + ) + body = env.as_dict() + assert body["error"] == "not_authorized", body + assert "active claim" in body["message"], body + messaging_svc.post_to_channel.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_note_active_owner_can_still_post() -> None: + """No-regression: an active owner (assigned_to=caller AND + active_claimant_id=caller) still posts. The reaped-window rejection must + not weaken the legitimate active-owner path.""" + agent_id = uuid4() + task_id = uuid4() + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=agent_id, # active claim — the normal case + ) + task_svc = AsyncMock() + task_svc.get.return_value = task_obj + task_svc.get_active_task_for_agent.return_value = None + journal_svc = AsyncMock() + deps = _make_deps(task=task_svc, journal=journal_svc) + ca = ContentActions(deps) + + env = await ca.note( + agent_id=agent_id, + text="Working on my actively-claimed task", + scope="note", + task_id=task_id, + ) + assert env.error is None, env.as_dict() + journal_svc.write_entry.assert_awaited_once() diff --git a/tests/unit/gateway/test_dm_a2a_denied.py b/tests/unit/gateway/test_dm_a2a_denied.py index 17c147d9..176cf265 100644 --- a/tests/unit/gateway/test_dm_a2a_denied.py +++ b/tests/unit/gateway/test_dm_a2a_denied.py @@ -44,7 +44,12 @@ async def test_dm_a2a_denied_returns_envelope_not_authorized() -> None: """A2AAccessDeniedError is caught and returned as Envelope.not_authorized.""" agent_id = uuid4() task_id = uuid4() - task_obj = MagicMock(id=task_id, status="in_progress", assigned_to=agent_id) + task_obj = MagicMock( + id=task_id, + status="in_progress", + assigned_to=agent_id, + active_claimant_id=agent_id, + ) task_svc = AsyncMock() task_svc.agent_for.return_value = MagicMock(role="qa")