mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): working exits for wedged agents + declare_coverage roll-up unblock (#341)
A live task burned 5+ hours because every exit was locked. unclaim now works from verifying and needs_revision (service guard + lifecycle edge); the circuit breaker and the i_am_done push-failure remediate name the working chain ending in unclaim(); sync_branch(stash=true) clears the DIRTY_WORKSPACE dead-end (pop-conflict preserves the stash); blocking a task QA already owns now says to idle instead of listing states; the orchestrator auto-block logs real errors and skips states where blocking is meaningless instead of force-blocking them. declare_coverage (cell/main PM) retroactively stamps parent-AC refs on a child that implements them -- closing the roll-up deadlock where the declaring child was cancelled and its re-delegated replacement completed the work uncredited. Cancelling a ref-declaring child now warns and surfaces the orphaned criteria. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
Renzo F
@@ -236,6 +236,7 @@ def test_check_returns_envelope_at_or_above_limit() -> None:
|
||||
assert "i_am_done" in result["message"]
|
||||
assert result["remediate"] is not None
|
||||
assert "i_am_blocked" in result["remediate"]
|
||||
assert "unclaim" in result["remediate"]
|
||||
|
||||
|
||||
def test_check_returns_none_for_unlimited_retry_verbs() -> None:
|
||||
@@ -511,6 +512,7 @@ def test_slow_drip_never_trips_window_but_trips_absolute_cap() -> None:
|
||||
assert result["error"] == "circuit_open"
|
||||
assert "absolute cap" in result["message"]
|
||||
assert "i_am_blocked" in result["remediate"]
|
||||
assert "unclaim" in result["remediate"]
|
||||
|
||||
|
||||
def test_absolute_check_returns_none_below_cap() -> None:
|
||||
|
||||
@@ -1087,3 +1087,236 @@ async def test_cell_pm_complete_survives_parent_advance_failure() -> None:
|
||||
assert body.get("error") is None, body
|
||||
assert body.get("warning") is not None
|
||||
assert "advance" in body["warning"].lower()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# declare_coverage
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _declare_coverage_deps(
|
||||
*,
|
||||
parent_id: Any,
|
||||
child_id: Any,
|
||||
parent_kwargs: dict[str, Any],
|
||||
child_kwargs: dict[str, Any],
|
||||
agent_kwargs: dict[str, Any],
|
||||
) -> tuple[AsyncMock, MagicMock, MagicMock]:
|
||||
"""Wire a task_svc AsyncMock whose .get resolves parent_id/child_id, plus
|
||||
the parent + child MagicMocks (declare_coverage loads both by id)."""
|
||||
parent = MagicMock(id=parent_id, **parent_kwargs)
|
||||
child = MagicMock(id=child_id, parent_task_id=parent_id, **child_kwargs)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.side_effect = lambda tid: parent if tid == parent_id else child
|
||||
task_svc.agent_for.return_value = MagicMock(**agent_kwargs)
|
||||
return task_svc, parent, child
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_task_not_found() -> None:
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = None
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(uuid4(), uuid4(), ["id-a"])
|
||||
assert env.error == "not_found"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_no_parent_returns_invalid_state() -> None:
|
||||
pm_id = uuid4()
|
||||
child_id = uuid4()
|
||||
child = MagicMock(id=child_id, parent_task_id=None, team="backend")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = child
|
||||
task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend")
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error == "invalid_state"
|
||||
task_svc.add_parent_ac_refs.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_non_pm_rejected() -> None:
|
||||
pm_id = uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, _child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={"assigned_to": pm_id},
|
||||
child_kwargs={"team": "backend"},
|
||||
agent_kwargs={"role": "developer", "team": "backend"},
|
||||
)
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error == "not_authorized"
|
||||
task_svc.add_parent_ac_refs.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_rejects_pm_off_team_without_parent_ownership() -> None:
|
||||
pm_id, other_pm_id = uuid4(), uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, _child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={"assigned_to": other_pm_id},
|
||||
child_kwargs={"team": "frontend"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error == "not_authorized"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_allows_pm_on_child_team_without_ownership() -> None:
|
||||
"""The minimum authorization bar: a PM on the child's own team may
|
||||
declare coverage even without owning the parent coordination task."""
|
||||
pm_id, other_pm_id = uuid4(), uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={
|
||||
"assigned_to": other_pm_id,
|
||||
"acceptance_criteria": ["crit a"],
|
||||
"acceptance_criteria_ids": ["id-a"],
|
||||
},
|
||||
child_kwargs={"team": "backend", "status": "completed"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=[])
|
||||
task_svc.add_parent_ac_refs.return_value = child
|
||||
task_svc.uncovered_parent_acceptance_criteria.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error is None, env.as_dict()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_unknown_criterion_rejected_lists_parent_acs() -> None:
|
||||
pm_id = uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, _child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={
|
||||
"assigned_to": pm_id,
|
||||
"acceptance_criteria": ["crit a", "crit b"],
|
||||
"acceptance_criteria_ids": ["id-a", "id-b"],
|
||||
},
|
||||
child_kwargs={"team": "backend"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=["bogus"])
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["bogus"])
|
||||
assert env.error == "invalid_state"
|
||||
assert env.remediate is not None
|
||||
assert "crit a" in env.remediate and "crit b" in env.remediate
|
||||
task_svc.add_parent_ac_refs.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_happy_path_stamps_refs_and_returns_remaining() -> None:
|
||||
pm_id = uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={
|
||||
"assigned_to": pm_id,
|
||||
"acceptance_criteria": ["crit a", "crit b"],
|
||||
"acceptance_criteria_ids": ["id-a", "id-b"],
|
||||
},
|
||||
child_kwargs={"team": "backend", "status": "completed"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=[])
|
||||
task_svc.add_parent_ac_refs.return_value = child
|
||||
task_svc.uncovered_parent_acceptance_criteria.return_value = ["crit b"]
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error is None, env.as_dict()
|
||||
task_svc.add_parent_ac_refs.assert_awaited_once_with(
|
||||
child_id, ["id-a"], declared_by=pm_id
|
||||
)
|
||||
assert env.evidence == {"remaining_uncovered_parent_acs": ["crit b"]}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_idempotent_redeclare() -> None:
|
||||
pm_id = uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={
|
||||
"assigned_to": pm_id,
|
||||
"acceptance_criteria": ["crit a"],
|
||||
"acceptance_criteria_ids": ["id-a"],
|
||||
},
|
||||
child_kwargs={"team": "backend", "status": "completed"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=[])
|
||||
task_svc.add_parent_ac_refs.return_value = child
|
||||
task_svc.uncovered_parent_acceptance_criteria.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
first = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
count_after_first = task_svc.add_parent_ac_refs.await_count
|
||||
second = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
|
||||
assert first.error is None, first.as_dict()
|
||||
assert second.error is None, second.as_dict()
|
||||
assert task_svc.add_parent_ac_refs.await_count == count_after_first + 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_declare_coverage_then_submit_up_gate_passes() -> None:
|
||||
"""declare_coverage followed by the roll-up gate — the production
|
||||
deadlock's end-to-end fix: once uncovered_parent_acceptance_criteria
|
||||
empties, _parent_acs_covered_envelope no longer blocks submit_up."""
|
||||
pm_id = uuid4()
|
||||
parent_id, child_id = uuid4(), uuid4()
|
||||
task_svc, _parent, child = _declare_coverage_deps(
|
||||
parent_id=parent_id,
|
||||
child_id=child_id,
|
||||
parent_kwargs={
|
||||
"assigned_to": pm_id,
|
||||
"acceptance_criteria": ["crit a"],
|
||||
"acceptance_criteria_ids": ["id-a"],
|
||||
},
|
||||
child_kwargs={"team": "backend", "status": "completed"},
|
||||
agent_kwargs={"role": "cell_pm", "team": "backend"},
|
||||
)
|
||||
task_svc.unknown_ac_refs = MagicMock(return_value=[])
|
||||
task_svc.add_parent_ac_refs.return_value = child
|
||||
task_svc.uncovered_parent_acceptance_criteria.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.declare_coverage(pm_id, child_id, ["id-a"])
|
||||
assert env.error is None, env.as_dict()
|
||||
assert env.evidence == {"remaining_uncovered_parent_acs": []}
|
||||
|
||||
gate_env = await c._parent_acs_covered_envelope(
|
||||
pm_id, parent_id, context_phrase="bubbling up"
|
||||
)
|
||||
assert gate_env is None
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
"""``i_am_blocked`` must never demand paperwork — a non-empty ``reason`` is
|
||||
the only requirement (blocker_type / what_needed stay optional), so a wedged
|
||||
agent can bail with one sentence. Also pins the awaiting_qa bail message: a
|
||||
dev whose task already moved to QA review is not blocked, it's done — the
|
||||
rejection must say so and point at i_am_idle(), not list allowed states like
|
||||
a wall.
|
||||
|
||||
Mirrors the fake-dependency shape of test_i_am_blocked_no_escalation_target.py.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
|
||||
|
||||
def _make_evidence_repo() -> AsyncMock:
|
||||
repo = AsyncMock()
|
||||
for method in (
|
||||
"list_unread_a2a",
|
||||
"list_unread_mentions",
|
||||
"list_pending_notifications",
|
||||
"task_metadata_gaps",
|
||||
"recent_team_activity",
|
||||
"blockers_in_lane",
|
||||
"journal_highlights_for_task",
|
||||
"similar_memory",
|
||||
):
|
||||
getattr(repo, method).return_value = []
|
||||
return repo
|
||||
|
||||
|
||||
def _make_task(agent_id: object, task_id: object, status: str) -> MagicMock:
|
||||
return MagicMock(
|
||||
id=task_id,
|
||||
status=status,
|
||||
assigned_to=agent_id,
|
||||
pre_block_state=None,
|
||||
task_type="code",
|
||||
team="backend",
|
||||
dependency_ids=[],
|
||||
acceptance_criteria=[],
|
||||
quick_context=None,
|
||||
notes_structured=None,
|
||||
)
|
||||
|
||||
|
||||
def _make_task_svc(agent_id: object, task: object) -> AsyncMock:
|
||||
task_svc = AsyncMock()
|
||||
task_svc.session = MagicMock()
|
||||
task_svc.session.begin_nested = MagicMock(
|
||||
return_value=MagicMock(
|
||||
__aenter__=AsyncMock(return_value=None),
|
||||
__aexit__=AsyncMock(return_value=False),
|
||||
)
|
||||
)
|
||||
task_svc.get.return_value = task
|
||||
task_svc.agent_for.return_value = MagicMock(
|
||||
id=agent_id,
|
||||
role="developer",
|
||||
team="backend",
|
||||
slug="be-dev-1",
|
||||
)
|
||||
return task_svc
|
||||
|
||||
|
||||
def _make_deps(task_svc: AsyncMock) -> ChoreographerDeps:
|
||||
return ChoreographerDeps(
|
||||
task=task_svc,
|
||||
work_session=AsyncMock(),
|
||||
git=AsyncMock(),
|
||||
a2a=AsyncMock(),
|
||||
journal=AsyncMock(),
|
||||
audit=AsyncMock(),
|
||||
evidence_repo=_make_evidence_repo(),
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_i_am_blocked_succeeds_with_only_reason() -> None:
|
||||
"""No blocker_type/what_needed supplied — a bare reason is enough."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task = _make_task(agent_id, task_id, "in_progress")
|
||||
task_svc = _make_task_svc(agent_id, task)
|
||||
blocked_task = _make_task(agent_id, task_id, "blocked")
|
||||
task_svc.escalate.return_value = blocked_task
|
||||
deps = _make_deps(task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.i_am_blocked(agent_id, task_id, "wedged, cannot push, need help")
|
||||
|
||||
assert env.error is None, env.as_dict()
|
||||
assert env.status == "blocked"
|
||||
task_svc.escalate.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_i_am_blocked_from_awaiting_qa_names_i_am_idle() -> None:
|
||||
"""A task that already moved to QA is not this dev's to block anymore —
|
||||
the rejection must say the truth (done, QA owns it) and point at
|
||||
i_am_idle(), not a generic 'find a task in [...]' dead end."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task = _make_task(agent_id, task_id, "awaiting_qa")
|
||||
task_svc = _make_task_svc(agent_id, task)
|
||||
deps = _make_deps(task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.i_am_blocked(agent_id, task_id, "stuck on this task")
|
||||
|
||||
assert env.error == "invalid_state", env.as_dict()
|
||||
assert "awaiting_qa" in (env.message or "")
|
||||
remediate = env.remediate or ""
|
||||
assert "i_am_idle" in remediate
|
||||
assert "QA" in remediate
|
||||
task_svc.escalate.assert_not_awaited()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__, "-q"])
|
||||
@@ -88,7 +88,7 @@ async def test_sync_branch_rebases_and_returns_evidence() -> None:
|
||||
env = await c.sync_branch(aid, tid)
|
||||
|
||||
git_svc.sync_task_branch.assert_awaited_once_with(
|
||||
t, base_branch=_BASE, actor_agent_id=aid
|
||||
t, base_branch=_BASE, actor_agent_id=aid, stash=False
|
||||
)
|
||||
assert env.error is None
|
||||
assert env.evidence is not None
|
||||
@@ -227,6 +227,118 @@ async def test_sync_branch_git_failure_steers_to_i_am_blocked() -> None:
|
||||
assert "i_am_blocked" in (env.remediate or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_branch_passes_stash_flag_through() -> None:
|
||||
"""stash=True on the verb forwards to GitService.sync_task_branch."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
t = _task(tid=tid, aid=aid)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||
git_svc = AsyncMock()
|
||||
git_svc.sync_task_branch.return_value = {"status": "rebased", "unique_commits": 1}
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
with patch(
|
||||
"roboco.services.gateway.choreographer._impl.resolve_parent_branch",
|
||||
new=AsyncMock(return_value=_BASE),
|
||||
):
|
||||
env = await c.sync_branch(aid, tid, stash=True)
|
||||
|
||||
git_svc.sync_task_branch.assert_awaited_once_with(
|
||||
t, base_branch=_BASE, actor_agent_id=aid, stash=True
|
||||
)
|
||||
assert env.error is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_branch_dirty_workspace_failure_steers_to_stash_or_commit() -> None:
|
||||
"""A DIRTY_WORKSPACE failure gets a specific, actionable remediate — not
|
||||
the generic i_am_blocked escalation."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
t = _task(tid=tid, aid=aid)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||
git_svc = AsyncMock()
|
||||
git_svc.sync_task_branch.side_effect = RuntimeError(
|
||||
"DIRTY_WORKSPACE: Cannot rebase with uncommitted changes."
|
||||
)
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
with patch(
|
||||
"roboco.services.gateway.choreographer._impl.resolve_parent_branch",
|
||||
new=AsyncMock(return_value=_BASE),
|
||||
):
|
||||
env = await c.sync_branch(aid, tid)
|
||||
|
||||
assert env.error == "invalid_state"
|
||||
assert "stash=True" in (env.remediate or "")
|
||||
assert "commit(" in (env.remediate or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_branch_conflicts_with_stash_preserved_notes_it_in_next() -> None:
|
||||
"""A conflict with stash_preserved=True tells the dev their stash is safe."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
t = _task(tid=tid, aid=aid)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||
git_svc = AsyncMock()
|
||||
git_svc.sync_task_branch.return_value = {
|
||||
"status": "conflicts",
|
||||
"files": ["src/a.py"],
|
||||
"stash_preserved": True,
|
||||
}
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
with patch(
|
||||
"roboco.services.gateway.choreographer._impl.resolve_parent_branch",
|
||||
new=AsyncMock(return_value=_BASE),
|
||||
):
|
||||
env = await c.sync_branch(aid, tid, stash=True)
|
||||
|
||||
assert env.error is None
|
||||
assert "stash" in (env.next or "").lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_branch_stash_pop_conflict_notes_preserved_stash() -> None:
|
||||
"""A clean rebase whose stash pop conflicted must not read as a plain
|
||||
success — the dev still has manual work to finish."""
|
||||
aid = uuid4()
|
||||
tid = uuid4()
|
||||
t = _task(tid=tid, aid=aid)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||
git_svc = AsyncMock()
|
||||
git_svc.sync_task_branch.return_value = {
|
||||
"status": "rebased",
|
||||
"unique_commits": 2,
|
||||
"stash_pop_conflict": True,
|
||||
}
|
||||
deps = _make_deps(task=task_svc, git=git_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
with patch(
|
||||
"roboco.services.gateway.choreographer._impl.resolve_parent_branch",
|
||||
new=AsyncMock(return_value=_BASE),
|
||||
):
|
||||
env = await c.sync_branch(aid, tid, stash=True)
|
||||
|
||||
assert env.error is None
|
||||
assert "conflict" in (env.next or "").lower()
|
||||
assert "preserved" in (env.next or "").lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_branch_rejection_writes_audit_row() -> None:
|
||||
"""Every rejection envelope must call audit.log_event (Task 6 contract)."""
|
||||
|
||||
@@ -270,7 +270,7 @@ def test_i_am_done_notes_defaults_to_empty(flow_module: types.ModuleType) -> Non
|
||||
|
||||
|
||||
def test_sync_branch_posts_to_dev_path(flow_module: types.ModuleType) -> None:
|
||||
"""sync_branch forwards task_id to /api/v1/flow/developer/sync_branch."""
|
||||
"""sync_branch forwards task_id + stash to /api/v1/flow/developer/sync_branch."""
|
||||
fake_client = _make_fake_client({"status": "ok"})
|
||||
|
||||
with patch("httpx.Client", return_value=fake_client):
|
||||
@@ -279,7 +279,18 @@ def test_sync_branch_posts_to_dev_path(flow_module: types.ModuleType) -> None:
|
||||
assert result == {"status": "ok"}
|
||||
args, kwargs = fake_client.post.call_args
|
||||
assert "/api/v1/flow/developer/sync_branch" in args[0]
|
||||
assert kwargs["json"] == {"task_id": "task-abc"}
|
||||
assert kwargs["json"] == {"task_id": "task-abc", "stash": False}
|
||||
|
||||
|
||||
def test_sync_branch_forwards_stash_true(flow_module: types.ModuleType) -> None:
|
||||
"""sync_branch(stash=True) forwards the flag through the body."""
|
||||
fake_client = _make_fake_client({"status": "ok"})
|
||||
|
||||
with patch("httpx.Client", return_value=fake_client):
|
||||
flow_module.sync_branch("task-abc", stash=True)
|
||||
|
||||
_, kwargs = fake_client.post.call_args
|
||||
assert kwargs["json"] == {"task_id": "task-abc", "stash": True}
|
||||
|
||||
|
||||
def test_i_am_blocked_sends_reason(flow_module: types.ModuleType) -> None:
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
"""`_auto_block_task` must be state-aware and never log an empty error.
|
||||
|
||||
Live incident: the orchestrator logged `{"error": "", "event": "Failed to
|
||||
auto-block task"}` for a task whose owning container had died mid-
|
||||
awaiting_qa — an empty error string with no diagnostic value, from a PATCH
|
||||
attempting to force a task QA already owns back to "blocked". This pins:
|
||||
|
||||
- a task already past dev control (awaiting_qa, terminal, ...) is skipped
|
||||
with an info log, no PATCH attempted
|
||||
- a still-blockable task (pending) proceeds to the PATCH as before
|
||||
- a PATCH failure logs a non-empty error even for exception types whose
|
||||
str() is empty (e.g. a bare TimeoutError)
|
||||
- a failed status pre-check does not swallow the block attempt itself
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from roboco.runtime.orchestrator import AgentOrchestrator
|
||||
|
||||
|
||||
def _make_orch() -> AgentOrchestrator:
|
||||
return AgentOrchestrator.__new__(AgentOrchestrator)
|
||||
|
||||
|
||||
def _resp(status_code_ok: bool, payload: dict[str, Any]) -> MagicMock:
|
||||
r = MagicMock()
|
||||
r.is_success = status_code_ok
|
||||
r.json.return_value = payload
|
||||
return r
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_block_skips_task_already_in_awaiting_qa() -> None:
|
||||
orch = _make_orch()
|
||||
client: Any = AsyncMock()
|
||||
client.get.return_value = _resp(True, {"status": "awaiting_qa"})
|
||||
|
||||
await orch._auto_block_task(client, "tid-1", "container died")
|
||||
|
||||
client.patch.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_block_skips_completed_task() -> None:
|
||||
orch = _make_orch()
|
||||
client: Any = AsyncMock()
|
||||
client.get.return_value = _resp(True, {"status": "completed"})
|
||||
|
||||
await orch._auto_block_task(client, "tid-2", "stale readiness check")
|
||||
|
||||
client.patch.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_block_proceeds_for_pending_task() -> None:
|
||||
"""The main existing use case (stuck pending tasks) must be unaffected."""
|
||||
orch = _make_orch()
|
||||
client: Any = AsyncMock()
|
||||
client.get.return_value = _resp(True, {"status": "pending"})
|
||||
|
||||
await orch._auto_block_task(client, "tid-3", "needs a project_id")
|
||||
|
||||
client.patch.assert_awaited_once()
|
||||
args, kwargs = client.patch.await_args
|
||||
assert "tid-3" in args[0]
|
||||
assert kwargs["json"]["status"] == "blocked"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_block_proceeds_when_status_precheck_fails() -> None:
|
||||
"""A GET failure must not swallow the block attempt — fall through."""
|
||||
orch = _make_orch()
|
||||
client: Any = AsyncMock()
|
||||
client.get.side_effect = RuntimeError("network down")
|
||||
|
||||
await orch._auto_block_task(client, "tid-4", "some reason")
|
||||
|
||||
client.patch.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_block_logs_nonempty_error_for_blank_exception() -> None:
|
||||
"""str(TimeoutError()) is '' — the log must still carry a real message."""
|
||||
assert str(TimeoutError()) == "" # the exact gotcha this guards against
|
||||
orch = _make_orch()
|
||||
client: Any = AsyncMock()
|
||||
client.get.return_value = _resp(True, {"status": "pending"})
|
||||
client.patch.side_effect = TimeoutError()
|
||||
|
||||
with patch("roboco.runtime.orchestrator.logger") as mock_logger:
|
||||
await orch._auto_block_task(client, "tid-5", "some reason")
|
||||
|
||||
mock_logger.error.assert_called_once()
|
||||
_, kwargs = mock_logger.error.call_args
|
||||
assert kwargs["error"], "error field must never be blank"
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__, "-q"])
|
||||
@@ -1069,6 +1069,123 @@ async def test_rebase_onto_base_proceeds_on_clean_tree() -> None:
|
||||
assert ["rebase", "origin/master"] in calls
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# rebase_onto_base — stash=True auto-stash/pop (the dirty-workspace exit)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rebase_onto_base_stash_true_auto_stashes_and_pops() -> None:
|
||||
"""stash=True: a dirty tree is stashed (not refused), rebased, popped back."""
|
||||
svc = _service()
|
||||
calls: list[list[str]] = []
|
||||
|
||||
async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock:
|
||||
calls.append(args)
|
||||
res = MagicMock()
|
||||
res.returncode = 0
|
||||
if args[:2] == ["status", "--porcelain"]:
|
||||
res.stdout = " M dirty.py\n"
|
||||
elif args[:2] == ["rev-list", "--count"]:
|
||||
res.stdout = "1"
|
||||
else:
|
||||
res.stdout = ""
|
||||
return res
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
|
||||
result = await svc.rebase_onto_base(
|
||||
Path("/tmp/ws"),
|
||||
head_branch="feature/backend/h",
|
||||
base_branch="master",
|
||||
git_token="t",
|
||||
stash=True,
|
||||
)
|
||||
|
||||
assert result == {"status": "rebased", "unique_commits": 1}
|
||||
push_args = ["stash", "push", "-u", "-m", "sync_branch autostash"]
|
||||
pop_args = ["stash", "pop"]
|
||||
assert push_args in calls
|
||||
assert pop_args in calls
|
||||
# Stash push runs before the rebase, pop runs after.
|
||||
assert calls.index(push_args) < calls.index(["rebase", "origin/master"])
|
||||
assert calls.index(pop_args) > calls.index(["rebase", "origin/master"])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rebase_onto_base_stash_pop_conflict_preserves_stash() -> None:
|
||||
"""A conflicted pop is flagged, never auto-resolved — stash stays intact."""
|
||||
svc = _service()
|
||||
|
||||
async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock:
|
||||
res = MagicMock()
|
||||
res.returncode = 0
|
||||
if args[:2] == ["status", "--porcelain"]:
|
||||
res.stdout = " M dirty.py\n"
|
||||
elif args[:2] == ["rev-list", "--count"]:
|
||||
res.stdout = "1"
|
||||
elif args == ["stash", "pop"]:
|
||||
res.returncode = 1 # pop conflicted — stash is NOT dropped by git
|
||||
else:
|
||||
res.stdout = ""
|
||||
return res
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
|
||||
result = await svc.rebase_onto_base(
|
||||
Path("/tmp/ws"),
|
||||
head_branch="feature/backend/h",
|
||||
base_branch="master",
|
||||
git_token="t",
|
||||
stash=True,
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"status": "rebased",
|
||||
"unique_commits": 1,
|
||||
"stash_pop_conflict": True,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rebase_onto_base_stash_true_rebase_conflict_skips_pop() -> None:
|
||||
"""A rebase conflict aborts before ever attempting the pop — no double
|
||||
conflict; the stash is reported preserved for the caller to surface."""
|
||||
svc = _service()
|
||||
calls: list[list[str]] = []
|
||||
|
||||
async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock:
|
||||
calls.append(args)
|
||||
res = MagicMock()
|
||||
res.returncode = 0
|
||||
if args[:2] == ["status", "--porcelain"]:
|
||||
res.stdout = " M dirty.py\n"
|
||||
elif args == ["rebase", "origin/master"]:
|
||||
res.returncode = 1
|
||||
elif args[:2] == ["diff", "--name-only"]:
|
||||
res.stdout = "src/a.py\n"
|
||||
else:
|
||||
res.stdout = ""
|
||||
return res
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
|
||||
result = await svc.rebase_onto_base(
|
||||
Path("/tmp/ws"),
|
||||
head_branch="feature/backend/h",
|
||||
base_branch="master",
|
||||
git_token="t",
|
||||
stash=True,
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"status": "conflicts",
|
||||
"files": ["src/a.py"],
|
||||
"stash_preserved": True,
|
||||
}
|
||||
assert ["stash", "pop"] not in calls
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _link_commit_to_task — flush; the runner commits (no out-of-band commit)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -1094,6 +1094,18 @@ async def test_uncovered_parent_acs_recognizes_text_declared_coverage() -> None:
|
||||
assert await svc.uncovered_parent_acceptance_criteria(parent.id) == []
|
||||
|
||||
|
||||
def test_unknown_ac_refs_flags_refs_not_on_parent() -> None:
|
||||
# declare_coverage's validation primitive: accepts a parent criterion by
|
||||
# id OR exact text; anything else is unknown and must be rejected with
|
||||
# the parent's real AC list in the remediate.
|
||||
parent = _build_task(
|
||||
acceptance_criteria=["crit a", "crit b"],
|
||||
acceptance_criteria_ids=["id-a", "id-b"],
|
||||
)
|
||||
assert TaskService.unknown_ac_refs(parent, ["id-a", "crit b", "bogus"]) == ["bogus"]
|
||||
assert TaskService.unknown_ac_refs(parent, ["id-a", "crit b"]) == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_parent_ac_coverage_normalizes_text_refs() -> None:
|
||||
# A text-declared coverage ref from a COMPLETED child surfaces as
|
||||
|
||||
Reference in New Issue
Block a user