From 7094c3c17122af0ff8fac260eb37ecbd2fac56c9 Mon Sep 17 00:00:00 2001
From: Renzo F <45401804+rennf93@users.noreply.github.com>
Date: Wed, 24 Jun 2026 03:56:36 +0200
Subject: [PATCH] fix(run-hardening): workspace branch-collision + verb-runner
 None guard (#251)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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.

* fix(git): reset the dev workspace before a fresh-claim branch checkout

A developer's persistent per-dev clone is shared across tasks, so a finished or
abandoned prior task can leave it dirty and on a sibling branch. create_branch's
checkouts then fail on the dirty tree — and because this git work runs as a
side-effect AFTER the claim's DB transition commits, the task is left marked
assigned while the workspace stays on the wrong branch, so the dev's next commit
is rejected BRANCH_MISMATCH (stalling then blocking the task).

reset --hard the tree before the base/feature checkouts. This runs only on a
fresh claim (resume short-circuits in _dev_reentry), so discarded changes are
abandoned cruft from a finished task — never commits (reset --hard keeps HEAD),
never the gitignored .venv. The branch-preservation test invariant is refined to
its real intent: a work-carrying branch must never be RE-POINTED
(reset --hard <base>); a bare tree-clean reset is allowed.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
---
 CHANGELOG.md                                  |  4 ++++
 .../gateway/choreographer/_verb_runner.py     | 12 ++++++++++++
 roboco/services/git.py                        | 13 +++++++++++++
 tests/unit/gateway/test_verb_runner.py        | 19 +++++++++++++++++++
 tests/unit/services/test_git.py               | 11 +++++++++--
 5 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e70c3737..c0a69af4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,10 @@ 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.
+
+- **A dev claiming a new task no longer gets stuck on `BRANCH_MISMATCH`.** Each developer has one persistent clone shared across all their tasks, so a finished or abandoned prior task could leave the clone dirty and sitting on a sibling task's branch. The claim's git work (creating/checking out the new task's branch) runs as a side-effect *after* the claim's DB transition commits — so when the checkout failed on that dirty tree, the task was already marked assigned while the workspace stayed on the wrong branch, and the dev's next commit was rejected with `BRANCH_MISMATCH` (stalling, then blocking, the task). The claim now does a `git reset --hard` to clean the tree before the checkouts. It runs only on a fresh claim (resume short-circuits earlier), so the discarded changes are abandoned cruft from a finished task — never committed work, and never the gitignored `.venv`.
+
 ## [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/roboco/services/git.py b/roboco/services/git.py
index df3e0fad..680852f6 100644
--- a/roboco/services/git.py
+++ b/roboco/services/git.py
@@ -894,6 +894,19 @@ class GitService(BaseService):
             timeout=_network_git_timeout(),
         )
 
+        # The dev workspace is one persistent clone shared across this dev's
+        # tasks, so a finished/abandoned prior task can leave it dirty and on a
+        # sibling branch. Without a clean tree the base + feature checkouts below
+        # fail; and because this git work is a side-effect that runs AFTER the
+        # claim's DB transition has committed, a failed checkout leaves the
+        # workspace on the wrong branch while the task is already marked
+        # assigned — so the dev's next commit is rejected with BRANCH_MISMATCH.
+        # This runs only on a FRESH claim (resume short-circuits in _dev_reentry
+        # before reaching here), so any uncommitted changes are abandoned cruft
+        # from a finished task — safe to discard. `reset --hard` clears tracked
+        # changes; the gitignored .venv (and other ignored files) are untouched.
+        await self._run_git(workspace, ["reset", "--hard"], check=False)
+
         base_branch = await self._checkout_base_with_fallback(
             workspace, base_branch, default_branch, task_id
         )
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."""
diff --git a/tests/unit/services/test_git.py b/tests/unit/services/test_git.py
index dfbf4007..ef8aac6e 100644
--- a/tests/unit/services/test_git.py
+++ b/tests/unit/services/test_git.py
@@ -629,8 +629,15 @@ async def test_create_branch_keeps_existing_branch_that_has_work() -> None:
     calls = await _run_create_branch_with_existing_branch(
         svc, "feature/frontend/abc12345--def67890", unique_commits="3"
     )
-    assert not any(c[:2] == ["reset", "--hard"] for c in calls), (
-        "a branch with real work must never be reset"
+    # The fresh-claim tree-clean (a BARE `reset --hard`) is expected — it discards
+    # only uncommitted cruft from a prior task in the shared clone, never commits.
+    assert ["reset", "--hard"] in calls
+    # But the RE-POINT reset (`reset --hard <base>`, which throws commits away)
+    # must NEVER fire for a branch carrying its own work.
+    # `c[2:]` truthy == there is a ref arg after "reset --hard" → it re-points.
+    repoint_resets = [c for c in calls if c[:2] == ["reset", "--hard"] and c[2:]]
+    assert not repoint_resets, (
+        "a branch with real work must never be re-pointed onto base"
     )