From 855e4aea5440537bda68fe222b58067d962f61cd Mon Sep 17 00:00:00 2001 From: Renn F Date: Wed, 24 Jun 2026 03:40:23 +0200 Subject: [PATCH] fix(gateway): guard the verb runner against a None task/agent The runner's atomic steps dereference task.id / agent.id with no None-check, so a verb invoked when the task or agent could not be resolved crashed with a cryptic "'NoneType' object has no attribute 'id'" (observed on i_will_plan for a task forced into an unexpected state out-of-band). Fail fast at run_intent's entry with an actionable INVALID_STATE error instead. --- CHANGELOG.md | 2 ++ .../gateway/choreographer/_verb_runner.py | 12 ++++++++++++ tests/unit/gateway/test_verb_runner.py | 19 +++++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e70c3737..6ad1bdf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - **The CEO and other human roles no longer get spammed with agent "learnings."** Whenever an agent recorded a learning, RoboCo broadcast it as a knowledge-share notification — and the recipient query swept in the human roles too (the CEO, plus the human-driven prompter and secretary). Agent knowledge-sharing is a signal for *agents*; in a human's inbox it is just noise. Those roles are now excluded from learning broadcasts. +- **A gateway verb on a vanished task/agent fails cleanly instead of crashing cryptically.** The verb runner's atomic steps dereference `task.id` / `agent.id` with no guard, so a verb invoked when the task or agent could not be resolved (e.g. a task forced into an unexpected state out-of-band) crashed with an opaque `'NoneType' object has no attribute 'id'`. The runner now fails fast with an actionable `INVALID_STATE` error that tells the agent to re-fetch and re-issue its claim verb. + ## [0.10.0] - 2026-06-23 ### Added diff --git a/roboco/services/gateway/choreographer/_verb_runner.py b/roboco/services/gateway/choreographer/_verb_runner.py index ddf541ad..528119d5 100644 --- a/roboco/services/gateway/choreographer/_verb_runner.py +++ b/roboco/services/gateway/choreographer/_verb_runner.py @@ -51,6 +51,18 @@ class VerbRunner: the underlying TaskService methods raise; the savepoint context rolls the DB back on raise. """ + # Fail loud + clean on a missing task/agent. The atomic handlers below + # dereference task.id / agent.id, so a None here would otherwise crash + # with a cryptic "'NoneType' object has no attribute 'id'" (observed when + # a task was forced into an unexpected state out-of-band) instead of an + # actionable error the agent can recover from. + if task is None or agent is None: + missing = "task" if task is None else "agent" + raise ValueError( + f"INVALID_STATE: cannot run '{intent_name}' — its {missing} " + "could not be resolved. Re-fetch with evidence(task_id) and " + "re-issue your claim verb." + ) intent = spec._INTENT_VERBS[intent_name] for side_effect_name in intent.pre_side_effects: await self._dispatch_side_effect(side_effect_name, task, agent) diff --git a/tests/unit/gateway/test_verb_runner.py b/tests/unit/gateway/test_verb_runner.py index 4d59cd46..fea3693d 100644 --- a/tests/unit/gateway/test_verb_runner.py +++ b/tests/unit/gateway/test_verb_runner.py @@ -53,6 +53,25 @@ async def test_runner_runs_composed_actions_in_order() -> None: assert final_task.status == "in_progress" +@pytest.mark.asyncio +async def test_runner_rejects_none_task_or_agent() -> None: + """A None task/agent fails loud with a clean error, not a NoneType crash. + + The atomic handlers dereference task.id / agent.id; without the guard a + missing one crashes with "'NoneType' object has no attribute 'id'" (observed + when a task was forced into an unexpected state out-of-band). + """ + runner = VerbRunner(task_service=AsyncMock(), git_service=AsyncMock()) + ctx = spec.Context(plan="p") + agent = MagicMock(id=uuid4(), role="cell_pm") + task = MagicMock(id=uuid4(), status="in_progress") + + with pytest.raises(ValueError, match="INVALID_STATE"): + await runner.run_intent("i_will_plan", None, agent, ctx) + with pytest.raises(ValueError, match="INVALID_STATE"): + await runner.run_intent("i_will_plan", task, None, ctx) + + @pytest.mark.asyncio async def test_runner_runs_side_effects_after_db_commit() -> None: """For open_pr: composes is empty; side_effects (push_branch, create_pr) run."""