fix(gateway): possibilities-matrix fast-path hardening + collision-context guards

The W7 fast path now rejects empty/trivial notes (its sole compensating
control for the skipped journal gates), pushes the branch before the
behind-base check, and pairs the local-gate fallback with the toolchain
guard; the WORK_ALREADY_DONE prompt no longer promises a fast path to
verifying tasks the gate routes elsewhere. build_collision_context now
degrades gracefully at all three call sites instead of breaking the
gate review, PM briefing, or collision-map route.
This commit is contained in:
Renn F
2026-07-15 08:25:06 +02:00
parent 3c1064e7d1
commit 85ac6422ff
8 changed files with 310 additions and 46 deletions
+14 -6
View File
@@ -1371,14 +1371,22 @@ async def get_task_collision_map(
raise HTTPException( raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND, detail="Task not found" status_code=status.HTTP_404_NOT_FOUND, detail="Task not found"
) )
siblings = (
await service.get_subtasks(UUID(str(task.parent_task_id)))
if task.parent_task_id
else []
)
# No actual files here — the panel shows the declared surface + sibling # No actual files here — the panel shows the declared surface + sibling
# overlap only; drift stays in the in-context evidence envelope. # overlap only; drift stays in the in-context evidence envelope.
ctx = build_collision_context(task=task, siblings=siblings) # Best-effort: a fetch/build failure degrades to no siblings rather than
# a 500 — the route still returns the task's own declared surface.
ctx: list[dict[str, Any]] | None = None
try:
siblings = (
await service.get_subtasks(UUID(str(task.parent_task_id)))
if task.parent_task_id
else []
)
ctx = build_collision_context(task=task, siblings=siblings)
except Exception as exc:
_logger.warning(
"collision_map_route_skip", task_id=str(task.id), error=str(exc)
)
return CollisionMapResponse( return CollisionMapResponse(
task_id=str(task.id), task_id=str(task.id),
parent_task_id=str(task.parent_task_id) if task.parent_task_id else None, parent_task_id=str(task.parent_task_id) if task.parent_task_id else None,
+1 -1
View File
@@ -14275,7 +14275,7 @@ If the fast path refuses (a gate it checks is not actually met), the
# just collapses the 3-5-turn re-derivation to a single i_am_done call. # just collapses the 3-5-turn re-derivation to a single i_am_done call.
if ( if (
settings.possibilities_matrix_enabled settings.possibilities_matrix_enabled
and status in ("claimed", "in_progress", "verifying") and status in ("claimed", "in_progress")
and task.get("pr_created") and task.get("pr_created")
and task.get("commits") and task.get("commits")
): ):
+53 -34
View File
@@ -1010,11 +1010,11 @@ class Choreographer:
return None return None
try: try:
siblings = await self.task.get_subtasks(parent_id) siblings = await self.task.get_subtasks(parent_id)
except Exception: # best-effort: omit on any fetch failure return build_collision_context(
task=t, siblings=siblings, actual_files=actual_files
)
except Exception: # best-effort: omit on any fetch/build failure
return None return None
return build_collision_context(
task=t, siblings=siblings, actual_files=actual_files
)
async def _with_collision_briefing( async def _with_collision_briefing(
self, briefing: dict[str, Any], full: bool, task: Any self, briefing: dict[str, Any], full: bool, task: Any
@@ -2853,6 +2853,49 @@ class Choreographer:
return None return None
return await self._i_am_done_fast_path(ctx) return await self._i_am_done_fast_path(ctx)
async def _fast_path_rejection(self, ctx: _IAmDoneContext) -> Any:
"""The fast path's ordered rejection cascade; None means proceed.
Order matters: notes first (cheap, no writes), then finding
resolution + guards, then the open-findings re-check, then the
quality verdict with its toolchain backstop.
"""
if soup := self._soup_reason(ctx.notes, "notes", 4):
return soup
await self._apply_resolved_findings(ctx)
guards = (
lambda: self._check_submit_qa_field_gates(
ctx.agent_id, ctx.task_id, ctx.task
),
# Push-first, mirroring the standard gate: _behind_base_gate reads
# origin, which must already reflect the pushed head.
lambda: self._ensure_branch_pushed(ctx),
lambda: self._behind_base_gate(ctx),
lambda: self._conventions_gate(ctx),
)
for guard in guards:
if rejection := await guard():
return rejection
# FINDINGS_ADDRESSED re-checked post-resolution: a resolved_findings entry
# in THIS call may have closed the open set; anything still open blocks.
if await self._open_finding_ids(ctx.task_id):
return Envelope.invalid_state(
message="fast path blocked — open findings remain on the ledger",
remediate=(
"name every open finding in resolved_findings "
"({finding_id, note}), or leave possibilities_matrix off "
"and run the standard i_am_done path"
),
context_briefing=ctx.briefing,
)
rejection, ran_local = await self._fast_path_quality_verdict(ctx)
# The local gate ran (no CI signal) without a toolchain that can
# execute it is a hollow pass — pair the two like the standard gate
# does. Skipped when CI-green already decided (no local run at all).
if rejection is None and ran_local:
rejection = await self._toolchain_broken_guard(ctx.agent_id, ctx.task)
return rejection
async def _i_am_done_fast_path(self, ctx: _IAmDoneContext) -> Envelope: async def _i_am_done_fast_path(self, ctx: _IAmDoneContext) -> Envelope:
"""Work-already-done fast path: slimmed gates + direct transition chain. """Work-already-done fast path: slimmed gates + direct transition chain.
@@ -2865,37 +2908,13 @@ class Choreographer:
non-negotiable gates the predicate already asserts, plus conventions non-negotiable gates the predicate already asserts, plus conventions
(leaf dev tasks skip ``awaiting_pr_review`` so the conventions check (leaf dev tasks skip ``awaiting_pr_review`` so the conventions check
must not also be skipped here it is tree-sitter, sub-second, 0 turns must not also be skipped here it is tree-sitter, sub-second, 0 turns
when green). when green). Since ``notes`` is the sole compensating control for the
skipped journal gates, it is enforced here directly (empty AND soup
unlike the standard path's soup-only check, which skips an empty value
because presence there is gated by the journal requirements this path
doesn't run).
""" """
await self._apply_resolved_findings(ctx) if rejection := await self._fast_path_rejection(ctx):
guards = (
lambda: self._check_submit_qa_field_gates(
ctx.agent_id, ctx.task_id, ctx.task
),
lambda: self._behind_base_gate(ctx),
lambda: self._ensure_branch_pushed(ctx),
lambda: self._conventions_gate(ctx),
)
for guard in guards:
if rejection := await guard():
return await self._reject_i_am_done(ctx, rejection)
# FINDINGS_ADDRESSED re-checked post-resolution: a resolved_findings entry
# in THIS call may have closed the open set; anything still open blocks.
if await self._open_finding_ids(ctx.task_id):
return await self._reject_i_am_done(
ctx,
Envelope.invalid_state(
message="fast path blocked — open findings remain on the ledger",
remediate=(
"name every open finding in resolved_findings "
"({finding_id, note}), or leave possibilities_matrix off "
"and run the standard i_am_done path"
),
context_briefing=ctx.briefing,
),
)
rejection, _ran_local = await self._fast_path_quality_verdict(ctx)
if rejection is not None:
return await self._reject_i_am_done(ctx, rejection) return await self._reject_i_am_done(ctx, rejection)
try: try:
if str(ctx.task.status) == "claimed": if str(ctx.task.status) == "claimed":
@@ -1213,6 +1213,9 @@ class PRGateMixin(_Base):
return None return None
try: try:
siblings = await self.task.get_subtasks(t.parent_task_id) siblings = await self.task.get_subtasks(t.parent_task_id)
return build_collision_context(
task=t, siblings=siblings, actual_files=files_changed or None
)
except Exception as exc: except Exception as exc:
logger.warning( logger.warning(
"gate_review_collision_context_skip", "gate_review_collision_context_skip",
@@ -1220,9 +1223,6 @@ class PRGateMixin(_Base):
error=str(exc), error=str(exc),
) )
return None return None
return build_collision_context(
task=t, siblings=siblings, actual_files=files_changed or None
)
async def _build_gate_review_evidence(self, t: Any) -> dict[str, Any]: async def _build_gate_review_evidence(self, t: Any) -> dict[str, Any]:
"""Inline evidence for claim_gate_review: the assembled diff + """Inline evidence for claim_gate_review: the assembled diff +
@@ -153,6 +153,42 @@ async def test_collision_map_shows_overlapping_sibling(collision_client: dict) -
assert sib_entry["undeclared"] == [] assert sib_entry["undeclared"] == []
@pytest.mark.asyncio
async def test_collision_map_degrades_on_builder_failure(
collision_client: dict, monkeypatch: pytest.MonkeyPatch
) -> None:
# build_collision_context is called outside the get_subtasks try/except at
# this site's call site; a raise there must still yield a 200 with no
# siblings, never a 500 — mirrors qa.py's claim_review evidence pattern.
client = collision_client["client"]
parent = _task(collision_client, title="parent", status=TaskStatus.PENDING)
await collision_client["db"].flush()
under = _task(
collision_client,
parent_task_id=parent.id,
title="under review",
intends_to_touch=["roboco/services/git.py"],
)
_task(
collision_client,
parent_task_id=parent.id,
title="colliding sibling",
intends_to_touch=["roboco/services/git.py"],
)
await collision_client["db"].flush()
def _boom(**_kw: Any) -> None:
raise RuntimeError("collision builder exploded")
monkeypatch.setattr("roboco.api.routes.tasks.build_collision_context", _boom)
response = await client.get(f"/api/tasks/{under.id}/collision-map", headers=_HDR)
assert response.status_code == HTTPStatus.OK
body = response.json()
assert body["siblings"] == []
assert body["intends_to_touch"] == ["roboco/services/git.py"]
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_collision_map_omits_non_overlapping_sibling( async def test_collision_map_omits_non_overlapping_sibling(
collision_client: dict, collision_client: dict,
@@ -4,13 +4,22 @@ Pins the truth table — no parent → None, no surfaced siblings → None,
file-overlap sibling shown, both-migration shown without file overlap, file-overlap sibling shown, both-migration shown without file overlap,
shared-only-without-overlap NOT shown, declared-vs-actual drift computed, shared-only-without-overlap NOT shown, declared-vs-actual drift computed,
and the caps respected. Pure (no DB, no IO): duck-typed task/sibling rows. and the caps respected. Pure (no DB, no IO): duck-typed task/sibling rows.
Also covers the two choreographer-level wrappers around it
(``_gate_collision_evidence`` / ``_collision_context_for``): a raise from the
builder itself not just the sibling fetch must still degrade to
``None``/omitted, never break the caller (``claim_gate_review`` evidence /
the planning briefing).
""" """
from __future__ import annotations from __future__ import annotations
from types import SimpleNamespace from types import SimpleNamespace
from unittest.mock import AsyncMock
from uuid import uuid4 from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
from roboco.services.gateway.choreographer.collision import ( from roboco.services.gateway.choreographer.collision import (
COLLISION_GLOB_CAP, COLLISION_GLOB_CAP,
COLLISION_SIBLING_CAP, COLLISION_SIBLING_CAP,
@@ -191,6 +200,80 @@ def test_sort_key_orders_by_priority_then_sequence() -> None:
assert [e["sequence"] for e in ctx] == [5, 1, 0] assert [e["sequence"] for e in ctx] == [5, 1, 0]
def _make_choreographer(*, task_service: AsyncMock) -> Choreographer:
return Choreographer(
ChoreographerDeps(
task=task_service,
work_session=AsyncMock(),
git=AsyncMock(),
a2a=AsyncMock(),
journal=AsyncMock(),
audit=AsyncMock(),
evidence_repo=AsyncMock(),
)
)
class TestGateCollisionEvidenceDegradesOnBuilderFailure:
"""``_gate_collision_evidence`` (claim_gate_review evidence)."""
@pytest.mark.asyncio
async def test_builder_raise_omits_block(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
t = _task()
task_service = AsyncMock()
task_service.get_subtasks.return_value = [_sib()]
c = _make_choreographer(task_service=task_service)
monkeypatch.setattr(
"roboco.services.gateway.choreographer.pr_gate.build_collision_context",
lambda **_kw: (_ for _ in ()).throw(RuntimeError("boom")),
)
result = await c._gate_collision_evidence(t, [])
assert result is None
@pytest.mark.asyncio
async def test_fetch_raise_still_omits_block(self) -> None:
t = _task()
task_service = AsyncMock()
task_service.get_subtasks.side_effect = RuntimeError("db down")
c = _make_choreographer(task_service=task_service)
result = await c._gate_collision_evidence(t, [])
assert result is None
class TestCollisionContextForDegradesOnBuilderFailure:
"""``_collision_context_for`` (the planning/claim briefing helper)."""
@pytest.mark.asyncio
async def test_builder_raise_returns_none(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
t = _task()
task_service = AsyncMock()
task_service.get_subtasks.return_value = [_sib()]
c = _make_choreographer(task_service=task_service)
monkeypatch.setattr(
"roboco.services.gateway.choreographer._impl.build_collision_context",
lambda **_kw: (_ for _ in ()).throw(RuntimeError("boom")),
)
assert await c._collision_context_for(t) is None
@pytest.mark.asyncio
async def test_fetch_raise_returns_none(self) -> None:
t = _task()
task_service = AsyncMock()
task_service.get_subtasks.side_effect = RuntimeError("db down")
c = _make_choreographer(task_service=task_service)
assert await c._collision_context_for(t) is None
if __name__ == "__main__": if __name__ == "__main__":
import pytest import pytest
@@ -256,6 +256,7 @@ class _FastPathMocks:
conventions: AsyncMock conventions: AsyncMock
open_findings: AsyncMock open_findings: AsyncMock
record_milestone: AsyncMock record_milestone: AsyncMock
toolchain: AsyncMock
def _stub_fast_path( def _stub_fast_path(
@@ -266,6 +267,7 @@ def _stub_fast_path(
ok = AsyncMock(return_value="OK") ok = AsyncMock(return_value="OK")
reject = AsyncMock(return_value="REJECT") reject = AsyncMock(return_value="REJECT")
record_milestone = AsyncMock(return_value=None) record_milestone = AsyncMock(return_value=None)
toolchain = AsyncMock(return_value=None)
monkeypatch.setattr(c, "_apply_resolved_findings", AsyncMock(return_value=None)) monkeypatch.setattr(c, "_apply_resolved_findings", AsyncMock(return_value=None))
monkeypatch.setattr(c, "_check_submit_qa_field_gates", AsyncMock(return_value=None)) monkeypatch.setattr(c, "_check_submit_qa_field_gates", AsyncMock(return_value=None))
monkeypatch.setattr(c, "_behind_base_gate", AsyncMock(return_value=None)) monkeypatch.setattr(c, "_behind_base_gate", AsyncMock(return_value=None))
@@ -277,6 +279,7 @@ def _stub_fast_path(
"_fast_path_quality_verdict", "_fast_path_quality_verdict",
AsyncMock(return_value=(quality_rejection, False)), AsyncMock(return_value=(quality_rejection, False)),
) )
monkeypatch.setattr(c, "_toolchain_broken_guard", toolchain)
monkeypatch.setattr(c, "_notify_qa", AsyncMock(return_value=None)) monkeypatch.setattr(c, "_notify_qa", AsyncMock(return_value=None))
monkeypatch.setattr(c, "_touch", AsyncMock(return_value=None)) monkeypatch.setattr(c, "_touch", AsyncMock(return_value=None))
monkeypatch.setattr(c, "_record_milestone_progress", record_milestone) monkeypatch.setattr(c, "_record_milestone_progress", record_milestone)
@@ -288,6 +291,7 @@ def _stub_fast_path(
conventions=conventions, conventions=conventions,
open_findings=open_findings, open_findings=open_findings,
record_milestone=record_milestone, record_milestone=record_milestone,
toolchain=toolchain,
) )
@@ -365,3 +369,113 @@ async def test_fast_path_ci_failure_rejects_before_transition(
await c._i_am_done_fast_path(_ctx()) await c._i_am_done_fast_path(_ctx())
stubs.reject.assert_awaited_once() stubs.reject.assert_awaited_once()
c.task.submit_qa.assert_not_awaited() c.task.submit_qa.assert_not_awaited()
# ---------------------------------------------------------------------------
# _i_am_done_fast_path: notes is the sole compensating control for the
# skipped journal gates — empty AND soup are rejected (unlike the standard
# path's soup-only check, which skips an empty value).
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_fast_path_empty_notes_rejected(monkeypatch: pytest.MonkeyPatch) -> None:
c = Choreographer(_deps())
stubs = _stub_fast_path(c, monkeypatch)
ctx = _ctx()
ctx.notes = ""
await c._i_am_done_fast_path(ctx)
stubs.reject.assert_awaited_once()
c.task.submit_verification.assert_not_awaited()
c.task.submit_qa.assert_not_awaited()
@pytest.mark.asyncio
async def test_fast_path_trivial_notes_rejected(
monkeypatch: pytest.MonkeyPatch,
) -> None:
c = Choreographer(_deps())
stubs = _stub_fast_path(c, monkeypatch)
ctx = _ctx()
ctx.notes = "wip"
await c._i_am_done_fast_path(ctx)
stubs.reject.assert_awaited_once()
c.task.submit_verification.assert_not_awaited()
c.task.submit_qa.assert_not_awaited()
@pytest.mark.asyncio
async def test_fast_path_real_notes_passes(monkeypatch: pytest.MonkeyPatch) -> None:
c = Choreographer(_deps())
stubs = _stub_fast_path(c, monkeypatch)
ctx = _ctx()
ctx.notes = "already implemented in a prior session"
await c._i_am_done_fast_path(ctx)
stubs.ok.assert_awaited_once()
c.task.submit_qa.assert_awaited_once()
# ---------------------------------------------------------------------------
# _i_am_done_fast_path: guard order — push before behind-base, mirroring the
# standard gate (_behind_base_gate reads origin, which must already reflect
# the pushed head).
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_fast_path_pushes_branch_before_behind_base_check(
monkeypatch: pytest.MonkeyPatch,
) -> None:
c = Choreographer(_deps())
_stub_fast_path(c, monkeypatch)
order: list[str] = []
async def _push(_ctx: Any) -> None:
order.append("push")
async def _behind(_ctx: Any) -> None:
order.append("behind")
monkeypatch.setattr(c, "_ensure_branch_pushed", _push)
monkeypatch.setattr(c, "_behind_base_gate", _behind)
await c._i_am_done_fast_path(_ctx())
assert order == ["push", "behind"]
# ---------------------------------------------------------------------------
# _i_am_done_fast_path: toolchain backstop pairs with the local quality gate
# fallback (no CI signal) — never with the CI-green branch.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_fast_path_toolchain_broken_blocks_on_local_fallback(
monkeypatch: pytest.MonkeyPatch,
) -> None:
c = Choreographer(_deps())
stubs = _stub_fast_path(c, monkeypatch)
monkeypatch.setattr(
c,
"_fast_path_quality_verdict",
AsyncMock(return_value=(None, True)), # local gate ran (no CI signal), passed
)
stubs.toolchain.return_value = Envelope.invalid_state(
message="toolchain broken", remediate="fix"
)
await c._i_am_done_fast_path(_ctx())
stubs.reject.assert_awaited_once()
c.task.submit_qa.assert_not_awaited()
@pytest.mark.asyncio
async def test_fast_path_toolchain_guard_skipped_on_ci_green(
monkeypatch: pytest.MonkeyPatch,
) -> None:
c = Choreographer(_deps())
stubs = _stub_fast_path(c, monkeypatch)
monkeypatch.setattr(
c, "_fast_path_quality_verdict", AsyncMock(return_value=(None, False))
)
await c._i_am_done_fast_path(_ctx())
stubs.ok.assert_awaited_once()
stubs.toolchain.assert_not_awaited()
@@ -80,12 +80,16 @@ async def test_armed_claimed_with_pr_and_commits_is_work_already_done(
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_armed_verifying_with_pr_and_commits_is_work_already_done( async def test_armed_verifying_with_pr_and_commits_is_not_work_already_done(
monkeypatch: pytest.MonkeyPatch, monkeypatch: pytest.MonkeyPatch,
) -> None: ) -> None:
# verifying is excluded from the proxy's trigger set: _i_am_done_pre_gate_dispatch
# routes an owned `verifying` task to the resume path before the fast path is
# ever reachable, so steering the prompt there would be a dead-end promise.
monkeypatch.setattr(settings, "possibilities_matrix_enabled", True) monkeypatch.setattr(settings, "possibilities_matrix_enabled", True)
prompt = await _orch()._build_dev_prompt(_task(status="verifying")) prompt = await _orch()._build_dev_prompt(_task(status="verifying"))
assert "WORK ALREADY DONE" in prompt assert "WORK ALREADY DONE" not in prompt
assert "WORKFLOW STATE: VERIFYING" in prompt
@pytest.mark.asyncio @pytest.mark.asyncio