mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
Fix: agent idle deadlock and lifecycle hardening (#96)
* fix(panel): cap dialog height and pin footer so actions stay reachable
Shared DialogContent now caps at max-h-[85vh] with overflow-y-auto, and the
footer is sticky to the bottom. Long content (e.g. a pasted change-request
note) no longer pushes the submit/cancel buttons past the viewport — the body
scrolls while the actions stay visible. No-op on dialogs that already fit.
* feat(notifications): suppress duplicate same-purpose notifications at send
A notification is not created when an unacknowledged one with the same purpose
— same sender, same type, same task, overlapping recipients — already exists.
Body text is not compared, so rewording cannot defeat it; a different type,
task, sender, or an already-acked recipient all still send through. Stops
agents that loop re-issuing the same signal from piling up unread that
soft-blocks the recipient's idle path.
* fix(gateway): stop board/PM lifecycle verbs from 500-crashing
Two unguarded crashes that wedged the org in respawn/escalate loops:
- escalate_to_ceo dereferenced None.status when the verb runner declined the
escalation (task not in awaiting_pm_review — e.g. a board agent escalating a
blocked task). It now returns a clean invalid_state. The message/remediate
build moved to a helper so the function stays within the complexity gate.
- The coordination-root git ops (pr_target, pr_merge, PR update, branch-token
resolve) called UUID(str(task.project_id)) directly, which raised on a
coordination/integration task (project_id is None — 'badly formed hexadecimal
UUID string'). They now resolve through _project_for_task, which falls back to
the product's repo for project-less roots.
* refactor(intake): split out _block_to_chunk per-block classifier
Extract the per-block classification from _blocks_to_chunks so each function
stays within the xenon cyclomatic-complexity gate (was rank C). Behaviour is
unchanged — verified by the existing intake_driver tests.
* feat(gateway): make the i_am_idle unread soft-block satisfiable
The soft-block on unread A2A / @mentions had no clearing path, so once those
briefing fields populated an agent could never idle — a whole-org deadlock.
Keep the guard (it is correct) and add the missing clear paths:
- New read_messages content verb (schema -> route -> handler ->
a2a.mark_all_read -> MCP tool -> role do_tools): bulk-zeroes the caller's
unread A2A and stamps read_at. The idle hint now points to it.
- list_unread_mentions returns UNACKED MENTION-type notifications (each @mention
already raises one via messaging._notify_mentions) instead of raw,
unconditional mentions, so they clear via the existing notify_ack. No schema
migration needed.
The soft-block is now satisfiable: A2A via read_messages, mentions and
notifications via notify_ack.
* fix(tests): repair notification-dedup db.scalar mocks + prompter agent seeding
The notification send-dedup added a db.scalar() purpose-lookup to
_create_notification; the two hand-rolled _FakeDb test stubs (test_notification,
test_a2a_priority_tristate) had no scalar() method → AttributeError. Add
scalar() returning None (no duplicate) so creation proceeds.
Separately, the prompter '& Start' route tests assign the draft to a fixed
product-owner / main-pm AGENT_UUID but only seeded system + CEO, so the
assigned_to FK failed in isolation (and main-pm flaked in the full suite). Seed
both via idempotent merge() in _seed_project_and_ceo.
* fix(git): gitignore .pnpm-store + flag GH001 push rejection as permanent
A dev once committed the ~115 MB pnpm store → GitHub GH001 (>100 MB) pre-receive
reject → open_pr retry-loop. Two root fixes:
- Add .pnpm-store/ to .gitignore — an ignored dir can't be staged by any git add.
- push() restates a GH001 / file-size rejection as an unmistakable PERMANENT
error pointing at i_am_blocked, so the agent stops blind-retrying a push that
can never succeed (it otherwise mis-reads the raw output as a transient timeout).
The per-verb retry cap (open_pr: 5) already bounded the burn; this ends it.
* fix(gateway): accept a PM decision note as satisfying the complete/submit_up reflect gate
A cell/main PM that wrote a fresh decision but no separate reflect note bounced
on the reflect tracing-gate indefinitely (re-confirmed live: cell PMs looped on
cell_pm_complete -> journal:reflect until reaped, burning tokens — worse because
each respawn resets the per-verb retry cap). For a PM closing/submitting a task
the decision note already documents the close; the separate reflect is the
redundant artifact weak-model PMs forget. Accept a fresh decision as satisfying
reflect for complete + submit_up — the gate still requires a decision +
substantive notes, so the close stays documented.
NOTE (enforcement tradeoff, flagged for CEO review): this intentionally relaxes
the PM complete/submit_up gate. It does NOT touch the developer i_am_done gate.
* feat(gateway): refuse i_am_idle when a PM still owns a task awaiting its review
A cell/main PM once tried to 'send work back' by DMing the developer and going
idle — but a DM changes no task state, so the task stayed awaiting_pm_review and
the orchestrator just re-dispatched the PM in a loop. i_am_idle now refuses (like
the pending-assignment guard) when a PM owns an awaiting_pm_review task, with a
clear remediation: complete() to finish, or reassign()/delegate() to route it
back. PM-only; devs/QA/doc unaffected. Pairs with the reflect-gate relaxation so
the PM can actually complete instead of looping.
* feat(gateway): push a prior-work handoff digest into task-scoped briefings
A freshly spawned or respawned agent previously started cold on every
lifecycle hand-off: the prior worker's PR, commits, acceptance status and
journal highlights lived in task evidence but were pull-on-demand, so each
new role agent re-explored the codebase from scratch — wasted tokens and
fragile context loss across respawns.
build_task_handoff() composes a compact, DB-only digest (no git diff) and
_briefing_for() now attaches it to context_briefing whenever the caller
already holds the task row. The digest is built only from a passed-in task,
so there are zero extra fetches: every resumption entry point (give_me_work
and pm_give_me_work, i_will_work_on, i_will_plan, triage/triage_all,
i_am_done, submit_up, escalate_up, complete) threads the loaded task, while
id-only correction/rejection paths cleanly omit it.
Every field is type-guarded so a partial row never leaks a non-serialisable
value into the envelope.
* docs(prompts): tell agents to resume from the briefing handoff before re-exploring
The base prompt described the success envelope but never told agents to act
on context_briefing, so a respawned or hand-off agent would re-scan the
whole repo and re-derive the plan even when the briefing already carried the
prior worker's PR, commits, acceptance status and journal highlights.
Adds a 'Resume from your briefing' section that walks each task_handoff
field and instructs the agent to continue from it — and to read the unread
A2A / mention / notification lists, which are messages addressed to them.
Pairs with the gateway change that now pushes task_handoff into every
task-scoped briefing.
* feat(tasks): remember cleared dependencies so the unblock briefing can surface them
When an upstream dependency completed, _unblock_dependents removed its id from
the dependent's dependency_ids to let it be claimed — destroying the only
record of which upstream task had just landed. The revived dependent then
re-discovered that work from cold.
Adds tasks.completed_dependency_ids (Alembic 026, uuid[] default '{}'):
_unblock_dependents now appends the cleared id there instead of only dropping
it, and the briefing handoff digest surfaces it so the agent picking the task
back up knows its blocker cleared because that upstream work shipped. The base
prompt documents the field.
Migration round-trip verified against postgres (upgrade adds the column,
downgrade drops it).
* docs(prompts): instruct PMs to split oversized tasks into per-concern subtasks
A subtask carrying a long acceptance list or spanning multiple layers/files
drove repeated QA failures and a PM revision loop — QA can't pass a partial,
and the dev keeps re-touching unrelated parts. Nothing in the PM prompts told
them to decompose by size/concern.
cell_pm gets a 'Sizing' rule: one subtask = one focused concern with ~2-4
criteria and its own dev->QA pass; decompose anything larger before
delegating, sequencing with dependencies. main_pm gets a matching reminder to
scope each cell's slice to that cell's layer rather than handing a cell a
cross-layer monolith that just pushes the problem down a level.
* fix(gateway): mirror the task= kwarg on ChoreographerHelpers helper signatures
The handoff-digest change added a keyword-only task= parameter to
_briefing_for and _build_tracing_gap in _impl, but the ChoreographerHelpers
base that the role mixins inherit still declared the old signatures, so the
composed Choreographer had two incompatible base definitions (mypy [misc]).
Sync the base declarations to match.
* fix(tasks): keep the owner on a substitute-out so the task isn't orphaned
build_substitute_update unconditionally nulled assigned_to, so any
substitute that routes to PENDING (max_retries, low_context, out_of_scope_*)
— the path a verb hitting repeated 500s or its retry limit takes — left the
task pending AND unassigned. The dispatcher only respawns a pending task when
it has an owner, so the task went dormant: no agent ever picked it back up.
Keep the task with its current owner instead. A substitute-out is almost
always a transient stall, so the task re-dispatches to the SAME agent, which
resumes from the briefing handoff. Only the task_complete -> PM-review handoff
changes owner (unchanged).
* feat(a2a): suppress duplicate unread A2A messages at send
A respawned or retrying agent could re-emit the same DM, stacking identical
copies on the recipient's inbox and re-bumping the unread count — noise that
the recipient then has to clear. The notification path already dedups; A2A did
not.
send_chat_message now suppresses a send when an identical message from the
same sender is still unread in the conversation, keyed on (conversation,
sender, message_kind, content). Genuinely different messages are never
collapsed (verified: distinct content still produces distinct rows), so this
avoids the earlier per-pair over-suppression. No migration.
* fix(panel): default the notifications view to Unread, not All
Landing on the All tab buried new notifications under everything already
seen — the most-reported annoyance. The Unread tab is the actionable view, so
make it the default; the All/Pending tabs are one click away.
* fix(panel): show clone progress during intake prep instead of a frozen pill
The first clone of a repo can take a few minutes, during which the intake
form showed only a static 'Preparing the agent…' button — indistinguishable
from a hang. Add a progress region while preparing: an elapsed timer, a
saturating progress bar (approaches but never reaches 100% until the agent
actually answers), and staged copy (spinning up → cloning → first-clone-takes-
a-while → reading the codebase) so the wait reads as work, not a freeze.
* feat(docs): index workspace-authored docs that never reached the RAG store
Docs written through roboco_docs_write land at /app/docs on the orchestrator
and index fine. But a documenter can also write docs with Edit/Write directly
in its own clone (README, CHANGELOG, workspace markdown); those resolve to a
/app/docs path that doesn't exist on the orchestrator, so the indexer reads
nothing and the docs never become searchable — a cross-container miss with no
shared mount to bridge it.
On docs completion, capture each listed doc's committed content out of the
branch (new GitService.read_file_at_branch, via git show) and write it
server-side under /app/docs before indexing, so workspace-authored docs reach
RAG too. Docs already present server-side are skipped; absolute paths and
unreadable/uncommitted files are passed over best-effort.
* feat(prompter): survive a browser reload by reconnecting to the live intake chat
The intake chat lived entirely in React state, so a page reload wiped it and
dropped the human back to the scope form — even though the agent container
outlives the page. Now the chat persists a small TTL'd slice (session id,
messages, scope, draft) to localStorage and, on mount, reconnects: it asks the
new GET /live/{id}/status whether the session is still running and, if so,
restores the history and reopens the SSE stream; if dead or expired it clears
and shows the form. A full reload doesn't run React effect cleanup, so the
navigate-away reap never fires on refresh and the session stays up.
Backend adds the status endpoint + PrompterLiveRegistry.is_alive; localStorage
is cleared on confirm, start-another, and SPA navigate-away.
* chore: remove internal session-bookkeeping refs from code comments (part 1)
Strip leaked task/finding numbers, Wave/Phase/cluster/audit labels from
docstrings and comments across services, foundation policy, runtime, mcp,
api schemas, and agent_sdk — they mean nothing to a repo reader and expose
process internals. Wording preserved; only the labels dropped. Done by hand,
one comment at a time (no scripted rewrite). _impl.py follows separately.
* chore: remove internal session-bookkeeping refs from code comments (part 2)
Finishes the manual scrub: the choreographer _impl.py docstrings/comments plus
the remaining dogfood-run ('smoke-N') labels across runtime, mcp, foundation,
api schemas, services, and agent factories. Reworded to describe the bug or
behaviour in plain words; every label dropped. The repo source is now free of
task/finding numbers, Wave/Phase/cluster/audit/smoke labels. By hand, one
comment at a time.
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -138,6 +138,30 @@ async def test_board_escalate_to_ceo_blocks_wrong_state() -> None:
|
||||
task_svc.escalate_to_ceo.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_board_escalate_to_ceo_rejects_when_runner_declines() -> None:
|
||||
"""Runner returns None (service declined — e.g. the task left
|
||||
awaiting_pm_review mid-flight) → a clean invalid_state, NOT an unhandled
|
||||
``None.status`` 500 (the board's escalate-from-blocked crash loop)."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
t = MagicMock(id=task_id, status="awaiting_pm_review", team="backend")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(role="product_owner")
|
||||
task_svc.escalate_to_ceo.return_value = None
|
||||
journal_svc = AsyncMock()
|
||||
journal_svc.has_decision_for_task.return_value = True
|
||||
journal_svc.latest_decision_at.return_value = datetime.now(UTC)
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.escalate_to_ceo(agent_id, task_id, reason="ready for CEO sign-off")
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "awaiting_pm_review" in body["message"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_board_escalate_to_ceo_blocks_disallowed_role() -> None:
|
||||
agent_id = uuid4()
|
||||
|
||||
@@ -139,6 +139,42 @@ async def test_cell_pm_complete_allows_when_all_terminal() -> None:
|
||||
task_svc.cell_pm_complete.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cell_pm_complete_allows_decision_without_separate_reflect() -> None:
|
||||
"""A PM with a fresh decision but NO separate reflect can still complete —
|
||||
the decision documents the close, so the reflect gate no longer loops
|
||||
weak-model PMs into respawn churn."""
|
||||
pm_id = uuid4()
|
||||
parent_id = uuid4()
|
||||
t = MagicMock(
|
||||
id=parent_id,
|
||||
status="awaiting_pm_review",
|
||||
assigned_to=pm_id,
|
||||
pr_number=10,
|
||||
team="backend",
|
||||
branch_name="feature/backend/abc",
|
||||
parent_task_id=None,
|
||||
)
|
||||
after = MagicMock(**{**t.__dict__, "status": "completed"})
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.all_subtasks_terminal.return_value = True
|
||||
task_svc.get_subtasks.return_value = []
|
||||
task_svc.cell_pm_complete.return_value = after
|
||||
journal_svc = AsyncMock()
|
||||
journal_svc.has_decision_for_task.return_value = True
|
||||
journal_svc.latest_decision_at.return_value = datetime.now(UTC)
|
||||
journal_svc.has_reflect_for_task.return_value = False # no separate reflect
|
||||
git_svc = AsyncMock()
|
||||
git_svc.pr_merge.return_value = {"merge_commit_sha": "abc"}
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc, git=git_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.cell_pm_complete(pm_id, parent_id, "cell scope reviewed and approved")
|
||||
assert env.error is None
|
||||
task_svc.cell_pm_complete.assert_awaited_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# main_pm_complete subtask gate (root-task case)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -667,7 +667,7 @@ async def test_i_am_idle_with_unread_a2a_soft_blocks() -> None:
|
||||
env = await c.i_am_idle(agent_id)
|
||||
body = env.as_dict()
|
||||
assert body["status"] == "idle_with_unread"
|
||||
assert "address" in body["next"].lower()
|
||||
assert "read_messages" in body["next"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -167,3 +167,39 @@ async def test_i_am_idle_pending_guard_runs_after_unread_check() -> None:
|
||||
assert body["error"] is None
|
||||
assert body["status"] == "idle_with_unread"
|
||||
task_svc.mark_agent_idle.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_i_am_idle_refuses_pm_owning_awaiting_pm_review() -> None:
|
||||
"""A PM that still owns a task awaiting its own review cannot idle — it must
|
||||
complete / reassign / delegate (a DM does not route work)."""
|
||||
agent_id = uuid4()
|
||||
review = MagicMock(id=uuid4(), status="awaiting_pm_review")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_assigned_for_agent.return_value = [review]
|
||||
task_svc.agent_for.return_value = MagicMock(role="cell_pm")
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.i_am_idle(agent_id)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "reassign" in body["remediate"] or "delegate" in body["remediate"]
|
||||
task_svc.mark_agent_idle.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_i_am_idle_allows_dev_owning_awaiting_pm_review() -> None:
|
||||
"""The review guard is PM-only — a non-PM owning such a task still idles."""
|
||||
agent_id = uuid4()
|
||||
review = MagicMock(id=uuid4(), status="awaiting_pm_review")
|
||||
task_svc = AsyncMock()
|
||||
task_svc.list_assigned_for_agent.return_value = [review]
|
||||
task_svc.list_in_progress_for_agent.return_value = []
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer")
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.i_am_idle(agent_id)
|
||||
assert env.status == "idle"
|
||||
task_svc.mark_agent_idle.assert_awaited_once()
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
@@ -10,6 +11,7 @@ from roboco.services.gateway.evidence_builder import (
|
||||
BriefingInputs,
|
||||
build_context_briefing,
|
||||
build_evidence_for_task,
|
||||
build_task_handoff,
|
||||
)
|
||||
|
||||
|
||||
@@ -84,3 +86,80 @@ class TestContextBriefing:
|
||||
assert len(b["pending_notifications"]) == BRIEFING_LIST_CAP
|
||||
assert len(b["recent_team_activity"]) == BRIEFING_LIST_CAP
|
||||
assert len(b["blockers_in_my_lane"]) == BRIEFING_LIST_CAP
|
||||
|
||||
def test_task_handoff_defaults_none_and_surfaces_in_briefing(self) -> None:
|
||||
inputs = BriefingInputs(
|
||||
unread_a2a=[],
|
||||
unread_mentions=[],
|
||||
pending_notifications=[],
|
||||
task_metadata_gaps=[],
|
||||
recent_team_activity=[],
|
||||
blockers_in_my_lane=[],
|
||||
)
|
||||
assert build_context_briefing(inputs)["task_handoff"] is None
|
||||
|
||||
with_handoff = BriefingInputs(
|
||||
unread_a2a=[],
|
||||
unread_mentions=[],
|
||||
pending_notifications=[],
|
||||
task_metadata_gaps=[],
|
||||
recent_team_activity=[],
|
||||
blockers_in_my_lane=[],
|
||||
task_handoff={"pr_number": 8},
|
||||
)
|
||||
assert build_context_briefing(with_handoff)["task_handoff"] == {"pr_number": 8}
|
||||
|
||||
|
||||
class TestTaskHandoff:
|
||||
def test_none_task_returns_none(self) -> None:
|
||||
assert build_task_handoff(None, []) is None
|
||||
|
||||
def test_no_prior_work_returns_none(self) -> None:
|
||||
t = _task(pr_number=None, pr_url=None, dev_notes="")
|
||||
t.commits = [] # _task's `commits or [...]` default would re-seed one
|
||||
t.acceptance_criteria_status = []
|
||||
assert build_task_handoff(t, []) is None
|
||||
|
||||
def test_digest_from_prior_work(self) -> None:
|
||||
pr = 8
|
||||
t = _task(
|
||||
pr_number=pr,
|
||||
commits=[{"sha": "abc", "message": "feat: x"}],
|
||||
dev_notes="implemented the parser",
|
||||
)
|
||||
t.branch_name = "feature/backend/abc"
|
||||
t.acceptance_criteria_status = [{"criterion": "parses", "met": True}]
|
||||
digest = build_task_handoff(t, [{"summary": "chose recursive descent"}])
|
||||
assert digest is not None
|
||||
assert digest["pr_number"] == pr
|
||||
assert digest["branch_name"] == "feature/backend/abc"
|
||||
assert digest["commit_count"] == 1
|
||||
assert digest["dev_summary"] == "implemented the parser"
|
||||
assert digest["journal_highlights"] == [{"summary": "chose recursive descent"}]
|
||||
|
||||
def test_surfaces_completed_dependencies(self) -> None:
|
||||
dep_id = uuid4()
|
||||
t = _task(pr_number=None, pr_url=None, dev_notes="")
|
||||
t.commits = []
|
||||
t.acceptance_criteria_status = []
|
||||
t.completed_dependency_ids = [dep_id]
|
||||
digest = build_task_handoff(t, [])
|
||||
# A just-unblocked task with no other prior work still surfaces the dep.
|
||||
assert digest is not None
|
||||
assert digest["completed_dependency_ids"] == [str(dep_id)]
|
||||
|
||||
def test_caps_lists_and_type_guards(self) -> None:
|
||||
thirty = [{"sha": str(i)} for i in range(30)]
|
||||
t = _task(pr_number=7, commits=thirty)
|
||||
# Non-list / mismatched-type attributes degrade safely, never leak.
|
||||
t.acceptance_criteria_status = object()
|
||||
t.pr_url = object()
|
||||
t.branch_name = None
|
||||
not_a_list: Any = object()
|
||||
digest = build_task_handoff(t, not_a_list)
|
||||
assert digest is not None
|
||||
assert len(digest["recent_commits"]) == BRIEFING_LIST_CAP
|
||||
assert digest["acceptance_criteria_status"] == []
|
||||
assert digest["journal_highlights"] == []
|
||||
assert digest["pr_url"] is None
|
||||
assert digest["branch_name"] is None
|
||||
|
||||
@@ -15,6 +15,7 @@ from uuid import uuid4
|
||||
import pytest
|
||||
from roboco.api.schemas.git import GitCreateBranchRequest
|
||||
from roboco.config import settings
|
||||
from roboco.exceptions import GitCommandError
|
||||
from roboco.services.base import NotFoundError, UnauthorizedError
|
||||
from roboco.services.git import GitService
|
||||
|
||||
@@ -193,6 +194,41 @@ async def test_diff_returns_diff_stdout() -> None:
|
||||
assert "+hello" in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_file_at_branch_returns_committed_content() -> None:
|
||||
svc = _service()
|
||||
_bind(svc, "_workspace_for_branch", AsyncMock(return_value=Path("/tmp/ws")))
|
||||
_bind(svc, "_token_for_branch", AsyncMock(return_value=None))
|
||||
_bind(svc, "_resolve_head_ref", AsyncMock(return_value="HEAD"))
|
||||
_bind(
|
||||
svc,
|
||||
"_run_git",
|
||||
AsyncMock(return_value=MagicMock(stdout="# API\nbody\n", returncode=0)),
|
||||
)
|
||||
out = await svc.read_file_at_branch(
|
||||
branch_name="feature/backend/abc", path="docs/api.md"
|
||||
)
|
||||
assert out == "# API\nbody\n"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_file_at_branch_missing_returns_none() -> None:
|
||||
svc = _service()
|
||||
_bind(svc, "_workspace_for_branch", AsyncMock(return_value=Path("/tmp/ws")))
|
||||
_bind(svc, "_token_for_branch", AsyncMock(return_value=None))
|
||||
_bind(svc, "_resolve_head_ref", AsyncMock(return_value="HEAD"))
|
||||
# git show on a path that isn't in the tree exits non-zero.
|
||||
_bind(
|
||||
svc,
|
||||
"_run_git",
|
||||
AsyncMock(return_value=MagicMock(stdout="", returncode=128)),
|
||||
)
|
||||
out = await svc.read_file_at_branch(
|
||||
branch_name="feature/backend/abc", path="nope.md"
|
||||
)
|
||||
assert out is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# pr_target: GitHub round-trip
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -522,3 +558,43 @@ async def test_create_branch_keeps_existing_branch_that_has_work() -> None:
|
||||
assert not any(c[:2] == ["reset", "--hard"] for c in calls), (
|
||||
"a branch with real work must never be reset"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_restates_gh001_as_permanent() -> None:
|
||||
"""A >100MB push rejection (GH001) is re-raised with a clear, permanent
|
||||
message that points at i_am_blocked — not the raw output an agent mis-reads
|
||||
as a transient timeout and blind-retries."""
|
||||
svc = _service()
|
||||
_bind(svc, "get_current_branch", AsyncMock(return_value="feature/x"))
|
||||
_bind(svc, "_token_for_workspace", AsyncMock(return_value=None))
|
||||
|
||||
async def _run_git(_workspace: object, args: list[str], **_kw: object) -> object:
|
||||
if args[:1] == ["push"]:
|
||||
raise GitCommandError(
|
||||
"git push",
|
||||
"remote: error: GH001: large.bin is 115.00 MB; this exceeds "
|
||||
"GitHub's file size limit of 100.00 MB",
|
||||
)
|
||||
return MagicMock(returncode=0, stdout="1", stderr="")
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
with pytest.raises(GitCommandError, match="i_am_blocked"):
|
||||
await svc.push(Path("/tmp/ws"))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_propagates_non_gh001_error_unchanged() -> None:
|
||||
"""A non-size push failure is re-raised as-is (not reclassified)."""
|
||||
svc = _service()
|
||||
_bind(svc, "get_current_branch", AsyncMock(return_value="feature/x"))
|
||||
_bind(svc, "_token_for_workspace", AsyncMock(return_value=None))
|
||||
|
||||
async def _run_git(_workspace: object, args: list[str], **_kw: object) -> object:
|
||||
if args[:1] == ["push"]:
|
||||
raise GitCommandError("git push", "fatal: Authentication failed")
|
||||
return MagicMock(returncode=0, stdout="1", stderr="")
|
||||
|
||||
_bind(svc, "_run_git", AsyncMock(side_effect=_run_git))
|
||||
with pytest.raises(GitCommandError, match="Authentication failed"):
|
||||
await svc.push(Path("/tmp/ws"))
|
||||
|
||||
@@ -56,6 +56,11 @@ class _FakeDb:
|
||||
result.scalars.return_value.all.return_value = []
|
||||
return result
|
||||
|
||||
async def scalar(self, *_args, **_kwargs):
|
||||
# _create_notification's purpose-dedup lookup runs db.scalar(); model
|
||||
# "no existing duplicate" so creation proceeds.
|
||||
return None
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def _fake_ctx(db: _FakeDb):
|
||||
|
||||
@@ -521,6 +521,30 @@ async def _seed_project_and_ceo(db_session: Any) -> tuple[UUID, UUID]:
|
||||
)
|
||||
db_session.add_all([project, ceo])
|
||||
await db_session.flush()
|
||||
# The "& Start" routes assign the draft to a fixed board/PM agent
|
||||
# (product-owner for "Board review", main-pm for "Approve & Start"); those
|
||||
# rows must exist for the assigned_to FK. merge() is idempotent, so this is
|
||||
# safe whether or not another test already committed them on the shared DB.
|
||||
for slug, role, team in (
|
||||
("product-owner", AgentRole.PRODUCT_OWNER, None),
|
||||
("main-pm", AgentRole.MAIN_PM, Team.MAIN_PM),
|
||||
):
|
||||
await db_session.merge(
|
||||
AgentTable(
|
||||
id=UUID(AGENT_UUIDS[slug]),
|
||||
name=slug,
|
||||
slug=slug,
|
||||
role=role,
|
||||
team=team,
|
||||
status=AgentStatus.ACTIVE,
|
||||
model_config={},
|
||||
system_prompt=slug,
|
||||
capabilities=[],
|
||||
permissions={},
|
||||
metrics={},
|
||||
)
|
||||
)
|
||||
await db_session.flush()
|
||||
return project_id, ceo_id
|
||||
|
||||
|
||||
|
||||
@@ -22,6 +22,16 @@ def test_open_get_close() -> None:
|
||||
assert reg.get("s1") is None
|
||||
|
||||
|
||||
def test_is_alive_tracks_open_and_close() -> None:
|
||||
"""is_alive backs the panel's after-reload reconnect decision."""
|
||||
reg = PrompterLiveRegistry()
|
||||
assert reg.is_alive("s1") is False # never opened
|
||||
reg.open("s1", "intake-1")
|
||||
assert reg.is_alive("s1") is True
|
||||
reg.close("s1")
|
||||
assert reg.is_alive("s1") is False # reaped
|
||||
|
||||
|
||||
def test_open_is_idempotent_for_a_live_session() -> None:
|
||||
"""Re-opening a live session returns the SAME object (same queue).
|
||||
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
"""NotificationService._create_notification purpose-based dedup (unit).
|
||||
|
||||
The dedup short-circuit returns before any row is created when a same-purpose
|
||||
(same sender, type, task, overlapping recipients) notification is still
|
||||
unacknowledged. The real-DB query shape is exercised by the route/integration
|
||||
suites; here we assert the branch wiring with a mocked db context.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.models import NotificationPriority, NotificationType
|
||||
from roboco.models.notification import CreateNotificationParams
|
||||
from roboco.services.notification import NotificationService
|
||||
|
||||
|
||||
class _FakeDBCtx:
|
||||
"""Minimal async-context-manager yielding a mocked db handle."""
|
||||
|
||||
def __init__(self, db: object) -> None:
|
||||
self._db = db
|
||||
|
||||
async def __aenter__(self) -> object:
|
||||
return self._db
|
||||
|
||||
async def __aexit__(self, *exc: object) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _params() -> CreateNotificationParams:
|
||||
return CreateNotificationParams(
|
||||
notification_type=NotificationType.ALERT,
|
||||
priority=NotificationPriority.NORMAL,
|
||||
from_agent="from-1",
|
||||
to_agents=["to-1"],
|
||||
subject="s",
|
||||
body="b",
|
||||
related_task_id="t1",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_notification_suppresses_same_purpose_duplicate() -> None:
|
||||
"""An existing same-purpose unacked notification suppresses a new insert."""
|
||||
db = MagicMock()
|
||||
db.scalar = AsyncMock(return_value=uuid4()) # a same-purpose duplicate exists
|
||||
db.add = MagicMock()
|
||||
db.flush = AsyncMock()
|
||||
db.commit = AsyncMock()
|
||||
|
||||
svc = NotificationService()
|
||||
svc._resolve_recipients = AsyncMock(return_value=[uuid4()]) # type: ignore[method-assign]
|
||||
with (
|
||||
patch(
|
||||
"roboco.services.notification.get_db_context",
|
||||
return_value=_FakeDBCtx(db),
|
||||
),
|
||||
patch(
|
||||
"roboco.services.notification._resolve_agent_uuid",
|
||||
AsyncMock(return_value=uuid4()),
|
||||
),
|
||||
):
|
||||
await svc._create_notification(_params())
|
||||
|
||||
# Dedup hit → no row created, nothing committed/delivered.
|
||||
db.add.assert_not_called()
|
||||
db.commit.assert_not_called()
|
||||
db.scalar.assert_awaited_once()
|
||||
Reference in New Issue
Block a user