diff --git a/roboco/services/gateway/choreographer/_impl.py b/roboco/services/gateway/choreographer/_impl.py index b87122d4..7bc61e52 100644 --- a/roboco/services/gateway/choreographer/_impl.py +++ b/roboco/services/gateway/choreographer/_impl.py @@ -5394,9 +5394,28 @@ class Choreographer: context_briefing=briefing, ).with_introspection(task=t, role=role_str) if after is None: + # The create_pr pre-side-effect already opened the cell→root PR + # BEFORE submit_for_review ran (its pr_created gate requires it — + # see lifecycle.py), so a None here means the task raced out of + # in_progress AFTER the PR was opened: an orphaned PR sits on + # GitHub whose task is not in awaiting_pr_review. Name the open + # PR in the remediate so the PM can reconcile it — create_pr is + # idempotent, so re-issuing once the task is back in_progress + # re-attaches to the same PR rather than opening a duplicate. + # Mirrors submit_root's F016 None-envelope remediate. return Envelope.invalid_state( - message="could not transition to awaiting_pm_review", - remediate="check task state — must be in_progress with PR ready", + message=( + "submit_up did not transition the task — the cell→root " + "PR was already opened or the task is no longer " + "in_progress." + ), + remediate=( + "the cell→root PR is already open (create_pr ran before " + "the transition). Re-fetch with evidence(task_id); if it " + "is awaiting_pr_review wait for the reviewer, otherwise " + "re-delegate the fixes and retry submit_up — create_pr " + "is idempotent and re-attaches to the existing PR." + ), context_briefing=briefing, ) return after diff --git a/tests/unit/gateway/test_submit_up_unchanged_pr_guard.py b/tests/unit/gateway/test_submit_up_unchanged_pr_guard.py index 3a189cad..4f55b9d5 100644 --- a/tests/unit/gateway/test_submit_up_unchanged_pr_guard.py +++ b/tests/unit/gateway/test_submit_up_unchanged_pr_guard.py @@ -142,3 +142,45 @@ async def test_submit_up_fail_open_when_no_prior_pr_fail_verdict() -> None: ) assert env.error is None, env.as_dict() assert env.status == "awaiting_pr_review" + + +# --------------------------------------------------------------------------- +# F122: when submit_for_review returns None (a concurrent state change moved +# the task out of in_progress AFTER the create_pr pre-side-effect already +# opened the cell→root PR), the invalid_state remediate must TELL the cell PM +# the PR is already open. The old remediate ('must be in_progress with PR +# ready') hid that the PR exists — so the agent could not tell an orphaned PR +# was sitting on GitHub. The orphan is inherent to the correct pre-side-effect +# ordering (submit_for_review's pr_created gate requires create_pr first, see +# lifecycle.py:1338-1343) and is recoverable via create_pr's idempotent re-issue +# — but only if the agent KNOWS the PR is open. Mirrors submit_root's F016 +# remediate (_impl.py:6305-6310). +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_submit_up_none_remediate_names_the_already_open_pr() -> None: + """F122: submit_for_review returns None (raced out of in_progress) AFTER + create_pr already opened the cell→root PR. The rejection remediate must + name the open PR and point the PM at re-fetching + reconciling, not the + misleading 'must be in_progress with PR ready' that hides the PR exists.""" + c, cell_pm_id, cell_task_id = _resubmit_cell(notes_structured=None) + # A concurrent transition (stale-heartbeat reaper unclaim, or a racing + # i_am_blocked) moved the task out of in_progress between the precondition + # gate and the runner's composed action → submit_for_review returns None. + c.task.submit_for_review.return_value = None + + env = await c.submit_up( + cell_pm_id, cell_task_id, notes="submitting the assembled cell scope" + ) + + assert env.error == "invalid_state", env.as_dict() + remediate = (env.remediate or "").lower() + # The create_pr pre-side-effect ran BEFORE the None transition, so the PR + # is open on GitHub — the remediate must say so. The old 'PR ready' hint + # hid this. + assert "pr" in remediate and "open" in remediate, env.as_dict() + # The misleading old hint is gone. + assert "pr ready" not in remediate, env.as_dict() + # And the PR really was opened (pre-side-effect ran before the None). + c.git.create_pr.assert_awaited_once()