Files
roboco/tests/unit/test_agents_config.py
T
06682f33c6 Fix: agent workflow hardening (#70)
* fix(gateway): push the branch before QA handoff so reviewers see the latest commits

The commit content tool commits locally without pushing; only open_pr pushed
the branch. On the first submission that was fine, but a fix committed while
addressing needs_revision never reached origin (open_pr is skipped once the PR
exists), so QA — which reviews the remote PR branch — re-reviewed the stale
remote and re-failed the task on every cycle, a loop that never converged.

i_am_done now pushes the task branch (idempotent; a no-op when nothing is
unpushed) as part of the shared submit gate, covering both the normal and
resume-from-verifying paths. A push failure blocks the handoff with a clear
remediation rather than parking the task in awaiting_qa with commits that exist
only in the developer's local workspace.

* fix(orchestrator): don't reap a stale claim while the agent's container is alive

The stale-claim reaper released any claimed/in_progress task whose
last_heartbeat_at exceeded the TTL. The heartbeat only updates on certain
gateway calls, so a developer deep in a long edit/test cycle outran the TTL and
had its claim reaped mid-work — churning the task and risking a double spawn
against the still-running container.

The reaper now skips a task whose assignee still holds a live (ACTIVE) agent
instance, trusting container liveness — the ground truth — over the heartbeat
proxy. The check is defensive on missing fields so a heartbeat-only caller (and
the reaper's existing unit tests) behave exactly as before.

* fix(gateway): refuse to unblock a task while a dependency is unfinished

A PM unblock on a dependency-gated task moved it straight to in_progress,
overriding the dependency — letting a dependent proceed without its upstream's
work (e.g. a frontend task built before its UX design lands). A dependency
block is meant to clear on its own via _unblock_dependents the moment the
upstream reaches a terminal state.

unblock now refuses while any dependency is still non-terminal, returning a
clear remediation that the block resolves automatically. Manual unblock remains
available for genuine, non-dependency blockers.

* fix(gateway): release a dependency-blocked claim to pending instead of looping

A task that reached claimed/in_progress with an unfinished dependency was left
in that state when the claim guard rejected, so the orchestrator's respawn loop
kept reviving its assignee — which could make no progress — burning work for
nothing.

The claim guard now releases such a task back to pending. claimed -> blocked is
not a legal transition, so pending — held by the dispatch dependency filter — is
the lifecycle-correct resting state: the respawn loop ignores pending tasks, and
_unblock_dependents re-dispatches it once the upstream reaches a terminal state.
release_dependency_blocked_claim shares a _force_unclaim_to_pending core with
unclaim_for_reaper so both record a truthful work-session abandon reason.

* feat(security): warn at startup in header-trust mode + document the auth posture

When ROBOCO_AGENT_AUTH_REQUIRED is not enabled the API accepts the X-Agent-Id /
X-Agent-Role headers without a signed token, so any client that can reach it may
act as any role (including 'ceo'). The API now logs a clear warning at startup
in this mode, and the README gains a Security section documenting the auth
posture and how to harden it. Acceptable only on a trusted private network — do
not expose the API to untrusted networks.

* fix(workspace): scope the refresh fetch to current + default branch

ensure_workspace's healthy short-circuit ran an all-refs 'git fetch origin' to
keep every origin/<branch> ref current. On a monorepo with many accumulated
feature/* branches that exceeds the refresh timeout, the fetch silently fails,
and the workspace keeps a stale base — so an agent builds on an out-of-date
branch.

The refresh now fetches only the workspace's current branch and the repo's
default branch (resolved via origin/HEAD), with --no-tags --prune: it transfers
near-nothing and can't time out. Readers need their own branch and the default;
the integration branch is refreshed at branch-creation time.

* fix(git): refresh a dependency-blocked task's branch off the current integration tip

A cross-cell dependent (e.g. a frontend task waiting on the UX design) was
branched off a base captured before its upstream merged into the integration
branch, and the branch was never re-synced — so the agent built on a stale
snapshot with none of the upstream's work.

Two changes close the gap:
- release_dependency_blocked_claim now clears branch_name, so the re-claim
  (after the dependency clears) re-runs branch creation.
- create_branch, when the branch is already on disk with no commits of its own,
  resets it onto the freshly-pulled base — the dependent now builds on the
  current integration tip. A branch carrying real commits is left untouched, so
  no work is discarded; the cell->leaf cascade carries the upstream down to the
  dev branch automatically.

* refactor(gateway): drop the sibling-sequence claim guard

Sibling sequence no longer gates a claim. Cross-cell ordering is
enforced by task dependencies — a cell task that depends on another is
held until its upstream reaches a terminal state, a stronger,
status-aware gate than the sequence-number check. That check was
dormant in practice anyway: every fan-out child carries sequence 0, on
which the guard short-circuited. `sequence` stays a sibling-ordering /
dispatch-priority field (list_pending ordering and the panel).

Removes sibling_sequence_guard and its _earlier_blocking_sibling
helper, the now-unused skip_sequence parameter threaded through the
claim verbs, and the sibling fetch that fed it.

* feat(gateway): sort a cross-cell dependent after its upstream

When the frontend cell task is wired to depend on its UX/UI sibling, set
its sequence to the upstream's sequence + 1 so it sorts after the design
it waits on — list_pending ordering and the panel now show UX ahead of
the implementation it gates, in either delegation order.

Adds TaskService.set_sequence (the sibling-ordering field is a service
write; it carries no claim-gating semantics — dependencies gate claims).

* feat(gateway): make the backend cell depend on UX too

UX/UI design defines the screens and API contracts both implementation
cells build against, so the backend cell — not just the frontend — waits
on the UX/UI cell task in a product fan-out and sorts after it. Wires in
either delegation order: a backend task delegated after UX gets the
dependency directly; a UX task delegated after a still-pending backend
sibling retro-wires it.

Mirrors the existing frontend wiring (_depend_backend_on_ux and
_depend_pending_backends_on_ux). Backend is held by the same dependency
gate, so it costs no extra dispatch churn.

* fix(websocket): forward notification acks instead of logging them incomplete

The bridge handler serves both notification.sent and notification.acked,
but acked events carry `agent_id` (the acking agent) rather than
`recipient_id`, so every acknowledgement tripped the missing-field guard
and logged "Incomplete notification event" instead of reaching the panel.
Accept either field as the recipient.

* feat(api): hint the full UUID when a truncated task id fails validation

Agents copy the 8-character task prefix the system shows them (the commit
prefix, task summaries) and send it as task_id, which fails UUID
validation with an opaque "invalid length" 422 and wastes a call. The
request-validation handler now detects a task_id UUID error and attaches
a `remediate` hint telling the agent to retry with the full 36-character
UUID from its task envelope.

* fix(audit): record the blocked transition when a task is escalated

Escalation sets a task to blocked by writing task.status directly, which
bypassed the validated transition helper and so never emitted a
task.blocked audit row — the lifecycle moved but the Auditor saw nothing.
Extract the audit emit from the central transition helper into
_emit_status_transition_audit and call it from the escalate path,
capturing the prior status and outgoing owner before reassignment so the
row is attributed correctly.

* fix(docs): stop doubling the docs path so design specs index into RAG

The documenter sometimes hands a doc path already rooted at docs/, and
joining it onto DOCS_BASE_PATH (/app/docs) produced /app/docs/docs/...,
so the file was never found and the spec never indexed — the frontend
cell could not retrieve the UX design over RAG. Normalize the path
before joining: trust an absolute path, otherwise strip a single
redundant leading docs/ segment.

* feat(security): let the control panel authenticate in secure mode

With ROBOCO_AGENT_AUTH_REQUIRED=true every request must carry a valid
HMAC token, which locked the human control panel out — it sends role
headers but no token. nginx, the only trusted hop between the browser
and the API, now injects the CEO token on /api and /ws, so the browser
never holds the signing secret. The injected value is just the existing
per-agent token issued for the CEO identity (issue_panel_token), so the
token-verification path is unchanged. An empty value (dev/header-trust
mode) renders to no header.

`make panel-token` prints the value; set it as ROBOCO_PANEL_AGENT_TOKEN
in .env before enabling secure mode. .env.example and the README
Security section document the flow.

* chore(compose): consolidate the two compose files into one

docker-compose.yml and docker-compose.yaml had diverged: .yml — the file
Docker actually uses — carried ROBOCO_PUBLIC_BASE_URL but was missing the
/app/manifests bind-mount, while .yaml had the manifests mount but not
the base URL. Merge the union into docker-compose.yml and delete the
duplicate so there is one source of truth and no "multiple config files"
warning.

This activates the manifests mount in the deployed file: without it the
orchestrator writes per-agent tool manifests to its ephemeral container
fs, they never reach the host for the daemon to bind-mount, and agents
fall back to all-verbs registration. Drop the stale .yaml reference from
the config.py docstring, the labeler, and the CI path filters.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-06-05 16:35:22 +02:00

422 lines
13 KiB
Python

"""agents_config coverage — pure-function role/team resolution."""
from __future__ import annotations
from typing import TYPE_CHECKING
from roboco.agents_config import (
can_a2a_direct,
can_assign_tasks,
can_cancel_tasks,
can_create_tasks,
can_send_notifications,
get_a2a_route_hint,
get_agent_cell,
get_agent_role,
get_agent_skills,
get_agent_team,
get_cell_members,
get_escalation_target,
get_pm_for_agent,
get_pm_for_team,
is_board_member,
is_ceo,
is_management,
is_pm,
issue_agent_token,
issue_panel_token,
verify_agent_token,
)
from roboco.seeds.initial_data import CEO_AGENT_ID
if TYPE_CHECKING:
import pytest
# ---------------------------------------------------------------------------
# get_agent_role / get_agent_team / get_agent_cell
# ---------------------------------------------------------------------------
def test_get_agent_role_known() -> None:
assert get_agent_role("be-dev-1") == "developer"
def test_get_agent_role_main_pm() -> None:
assert get_agent_role("main-pm") == "main_pm"
def test_get_agent_role_unknown() -> None:
assert get_agent_role("ghost-agent") == "unknown"
def test_get_agent_team_known() -> None:
assert get_agent_team("be-dev-1") == "backend"
def test_get_agent_team_for_main_pm_returns_value() -> None:
"""main-pm has team 'main_pm' or None depending on config."""
result = get_agent_team("main-pm")
assert result is None or isinstance(result, str)
def test_get_agent_team_for_unknown() -> None:
assert get_agent_team("ghost-agent") is None
def test_get_agent_cell_alias_for_team() -> None:
assert get_agent_cell("be-dev-1") == "backend"
# ---------------------------------------------------------------------------
# Role predicates
# ---------------------------------------------------------------------------
def test_is_pm_for_cell_pm() -> None:
assert is_pm("be-pm") is True
def test_is_pm_for_main_pm() -> None:
assert is_pm("main-pm") is True
def test_is_pm_for_developer() -> None:
assert is_pm("be-dev-1") is False
def test_is_management_for_main_pm() -> None:
assert is_management("main-pm") is True
def test_is_management_for_developer() -> None:
assert is_management("be-dev-1") is False
def test_is_ceo_for_ceo() -> None:
assert is_ceo("ceo") is True
def test_is_ceo_for_developer() -> None:
assert is_ceo("be-dev-1") is False
def test_is_board_member() -> None:
# Whatever the actual board members are, the function should return bool.
assert isinstance(is_board_member("auditor"), bool)
# ---------------------------------------------------------------------------
# Permission predicates
# ---------------------------------------------------------------------------
def test_can_send_notifications_main_pm() -> None:
assert can_send_notifications("main-pm") is True
def test_can_send_notifications_developer() -> None:
assert can_send_notifications("be-dev-1") is False
def test_can_create_tasks_main_pm() -> None:
assert can_create_tasks("main-pm") is True
def test_can_create_tasks_developer() -> None:
assert can_create_tasks("be-dev-1") is False
def test_can_assign_tasks_main_pm() -> None:
assert can_assign_tasks("main-pm") is True
def test_can_assign_tasks_developer() -> None:
assert can_assign_tasks("be-dev-1") is False
def test_can_cancel_tasks_pm() -> None:
assert can_cancel_tasks("main-pm") is True
def test_can_cancel_tasks_ceo() -> None:
"""CEO cannot cancel — they observe only."""
assert can_cancel_tasks("ceo") is False
def test_can_cancel_tasks_auditor() -> None:
assert can_cancel_tasks("auditor") is False
# ---------------------------------------------------------------------------
# Escalation + PM resolution
# ---------------------------------------------------------------------------
def test_get_escalation_target() -> None:
target = get_escalation_target("be-dev-1")
assert target is None or isinstance(target, str)
def test_get_pm_for_team_known() -> None:
assert get_pm_for_team("backend") == "be-pm"
def test_get_pm_for_team_unknown() -> None:
assert get_pm_for_team("mars") is None
def test_get_pm_for_agent_cell_pm_returns_main_pm() -> None:
assert get_pm_for_agent("be-pm") == "main-pm"
def test_get_pm_for_agent_developer() -> None:
pm = get_pm_for_agent("be-dev-1")
# Cell members' PM is their cell PM.
assert pm == "be-pm" or pm is None
# ---------------------------------------------------------------------------
# A2A policy
# ---------------------------------------------------------------------------
def test_can_a2a_direct_to_ceo_denied() -> None:
allowed, reason = can_a2a_direct("be-dev-1", "ceo")
assert allowed is False
assert reason is not None
def test_can_a2a_direct_within_cell() -> None:
allowed, _ = can_a2a_direct("be-dev-1", "be-qa")
assert isinstance(allowed, bool)
def test_get_a2a_route_hint_returns_string() -> None:
hint = get_a2a_route_hint("be-dev-1", "fe-dev-1")
assert isinstance(hint, str)
def test_get_a2a_route_hint_for_ceo() -> None:
hint = get_a2a_route_hint("be-dev-1", "ceo")
assert "CEO" in hint
# ---------------------------------------------------------------------------
# get_cell_members
# ---------------------------------------------------------------------------
def test_get_cell_members_backend() -> None:
members = get_cell_members("backend")
assert isinstance(members, list)
def test_get_cell_members_unknown() -> None:
assert get_cell_members("mars") == []
# ---------------------------------------------------------------------------
# get_agent_skills
# ---------------------------------------------------------------------------
def test_get_agent_skills_returns_list() -> None:
skills = get_agent_skills("be-dev-1")
assert isinstance(skills, list)
def test_get_agent_skills_unknown_agent() -> None:
skills = get_agent_skills("ghost-agent")
assert isinstance(skills, list)
# ---------------------------------------------------------------------------
# Token issuance + verification (lines 45-46, 67-72, 83-89)
# ---------------------------------------------------------------------------
def test_issue_agent_token_returns_unsigned_when_secret_missing(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.delenv("ROBOCO_AGENT_AUTH_SECRET", raising=False)
assert issue_agent_token("be-dev-1", "developer", "backend") == "UNSIGNED"
_SHA256_HEX_LEN = 64
def test_issue_agent_token_signs_when_secret_present(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "test-secret")
tok = issue_agent_token("be-dev-1", "developer", "backend")
assert tok != "UNSIGNED"
# 64 hex chars from sha256
assert len(tok) == _SHA256_HEX_LEN
def test_verify_agent_token_round_trips(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "rt-secret")
tok = issue_agent_token("be-dev-1", "developer", "backend")
assert verify_agent_token(tok, "be-dev-1", "developer", "backend") is True
def test_verify_agent_token_rejects_when_secret_missing(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.delenv("ROBOCO_AGENT_AUTH_SECRET", raising=False)
assert verify_agent_token("anything", "be-dev-1", "developer", "backend") is False
def test_verify_agent_token_rejects_unsigned_sentinel(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "any-secret")
assert verify_agent_token("UNSIGNED", "be-dev-1", "developer", "backend") is False
def test_verify_agent_token_rejects_empty_token(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "any-secret")
assert verify_agent_token("", "be-dev-1", "developer", "backend") is False
def test_verify_agent_token_rejects_mismatched_signature(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "real-secret")
tok = issue_agent_token("be-dev-1", "developer", "backend")
# Verify with different role → mismatch.
assert verify_agent_token(tok, "be-dev-1", "qa", "backend") is False
# ---------------------------------------------------------------------------
# issue_panel_token — the panel's CEO credential for secure mode
# ---------------------------------------------------------------------------
def test_issue_panel_token_verifies_under_panel_headers(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The panel token must verify under the EXACT headers the panel sends:
X-Agent-Id = CEO uuid, X-Agent-Role = ceo, and NO team (empty)."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "panel-secret")
tok = issue_panel_token()
assert verify_agent_token(tok, CEO_AGENT_ID, "ceo", "") is True
def test_issue_panel_token_unsigned_without_secret(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.delenv("ROBOCO_AGENT_AUTH_SECRET", raising=False)
assert issue_panel_token() == "UNSIGNED"
def test_panel_token_does_not_grant_other_roles_or_identities(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""The panel token is bound to the CEO identity — it cannot be replayed to
claim a different role or agent id."""
monkeypatch.setenv("ROBOCO_AGENT_AUTH_SECRET", "panel-secret")
tok = issue_panel_token()
assert verify_agent_token(tok, CEO_AGENT_ID, "developer", "") is False
assert verify_agent_token(tok, "be-dev-1", "ceo", "") is False
# ---------------------------------------------------------------------------
# get_pm_for_agent main_pm escalation (line 360)
# ---------------------------------------------------------------------------
def test_get_pm_for_agent_main_pm_returns_product_owner() -> None:
assert get_pm_for_agent("main-pm") == "product-owner"
# ---------------------------------------------------------------------------
# A2A check branches: cell PM rejects board (line 682), cell-member without
# team falls through to management route (line 704), board successful path
# (732), main_pm dispatch (744), cell-member outbound to no-team (771-772).
# ---------------------------------------------------------------------------
def test_can_a2a_direct_cell_pm_to_board_denied() -> None:
allowed, reason = can_a2a_direct("be-pm", "product-owner")
assert allowed is False
assert reason is not None
assert "main-pm" in reason
def test_can_a2a_direct_cell_member_to_unknown_management() -> None:
"""Cell dev → CEO routes through CEO branch, not management."""
# Dev → main-pm: management → falls into cell_member branch's else case.
allowed, reason = can_a2a_direct("be-dev-1", "main-pm")
# Either path works; just ensure structured output.
assert isinstance(allowed, bool)
assert reason is None or isinstance(reason, str)
def test_can_a2a_direct_main_pm_to_developer_denied() -> None:
allowed, reason = can_a2a_direct("main-pm", "be-dev-1")
# main-pm to a developer routed via cell PM.
assert allowed is False
assert reason is not None
def test_can_a2a_direct_main_pm_to_cell_pm_allowed() -> None:
allowed, _ = can_a2a_direct("main-pm", "be-pm")
assert allowed is True
def test_can_a2a_direct_board_to_main_pm_allowed() -> None:
allowed, _ = can_a2a_direct("product-owner", "main-pm")
assert allowed is True
def test_can_a2a_direct_board_to_developer_denied() -> None:
allowed, reason = can_a2a_direct("product-owner", "be-dev-1")
assert allowed is False
assert reason is not None
def test_get_a2a_route_hint_cell_member_to_management() -> None:
"""Non-CEO cell-member → management agent (no team) routes via cell PM."""
hint = get_a2a_route_hint("be-dev-1", "main-pm")
# main-pm has team None → falls through to management branch (771-772).
assert "be-pm" in hint or "main-pm" in hint or "Use" in hint
def test_can_a2a_direct_cell_member_to_unknown_returns_management_hint() -> None:
"""Cell-member → agent with unresolvable team hits management branch (704)."""
allowed, reason = can_a2a_direct("be-dev-1", "ghost-agent")
assert allowed is False
assert reason is not None
assert "be-pm" in reason or "main-pm" in reason
def test_can_a2a_direct_unknown_from_agent_falls_through() -> None:
"""An agent with no team hits the final fallback (line 750)."""
allowed, reason = can_a2a_direct("ghost", "be-dev-1")
assert allowed is False
assert reason is not None
assert "not permitted" in reason
def test_get_a2a_route_hint_cell_member_to_unknown_routes_via_cell_pm() -> None:
"""Cell-member → unknown agent (no team) hits lines 770-772."""
hint = get_a2a_route_hint("be-dev-1", "ghost-agent")
assert "be-pm" in hint
assert "main-pm" in hint
def test_get_a2a_route_hint_unknown_from_agent_falls_through() -> None:
"""from_agent with no team falls through to escalate fallback (line 774)."""
hint = get_a2a_route_hint("ghost", "be-dev-1")
assert "escalate" in hint.lower()