mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(a2a): CEO can DM the Auditor and PR reviewers (#623)
* feat(a2a): CEO can DM the Auditor and PR reviewers
A mid-flight PR reviewer or Auditor that's stuck was unreachable — the CEO
had no way to DM them. Both roles now carry dm/read_a2a, so the CEO can open
a 1:1 and they can reply in-thread through the existing CEO-reply path.
Scoped deliberately: the Auditor stays a silent observer to its peers — it
gains no peer-initiation surface (can_a2a_direct routes it through
_check_auditor_a2a, which refuses every initiation target; it can only reply
inside a CEO-opened DM). PR reviewers keep their owning-PM scope. Intake and
Secretary stay excluded — they have their own dedicated chat pages.
NO_COMMS_ROLES drops to {prompter, secretary}; the panel's EXCLUDE_NON_DM_ROLES
matches. KB/docs updated so the 'auditor/pr_reviewer have no dm' claim isn't
left stale.
* test(a2a): smoke guard checks _NO_COMMS_ROLES, not a hardcoded 'auditor'
The dm() runtime guard no longer names the auditor (it now carries dm to
reply to the CEO); it refuses the canonical _NO_COMMS_ROLES set. Assert on
that set so the smoke test tracks the guard, not a stale role name.
* chore(foundation): regenerate verb tables for auditor/pr_reviewer dm+read_a2a
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -99,23 +99,32 @@ def test_can_a2a_direct_to_ceo_message_explains_reply_only() -> None:
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"target_slug",
|
||||
["auditor", "pr-reviewer-1", "intake-1", "secretary-1"],
|
||||
["intake-1", "secretary-1"],
|
||||
)
|
||||
def test_can_a2a_direct_ceo_to_no_comms_role_denied(target_slug: str) -> None:
|
||||
"""The CEO's asymmetric reach still can't target a role with no dm/
|
||||
read_a2a on its manifest (auditor, pr_reviewer, prompter, secretary) —
|
||||
nothing on the other end could ever read or answer the DM. The panel's
|
||||
New-DM dialog already filters these client-side (EXCLUDE_NON_DM_ROLES);
|
||||
this is the server-side backstop for a direct API/A2A-service call."""
|
||||
read_a2a on its manifest (prompter, secretary — human-only, own chat
|
||||
pages) — nothing on the other end could ever read or answer the DM."""
|
||||
allowed, reason = can_a2a_direct("ceo", target_slug)
|
||||
assert allowed is False
|
||||
assert reason is not None
|
||||
assert "comms" in reason.lower()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("target_slug", ["auditor", "pr-reviewer-1"])
|
||||
def test_can_a2a_direct_ceo_to_auditor_or_pr_reviewer_allowed(target_slug: str) -> None:
|
||||
"""The CEO can now DM a mid-flight auditor or PR reviewer — both carry
|
||||
dm/read_a2a so they can read and reply in-thread, even though neither
|
||||
gains a peer-initiation surface (auditor stays silent via
|
||||
can_a2a_direct; the PR reviewer stays scoped to its owning PM)."""
|
||||
allowed, reason = can_a2a_direct("ceo", target_slug)
|
||||
assert allowed is True
|
||||
assert reason is None
|
||||
|
||||
|
||||
def test_can_a2a_direct_ceo_to_no_comms_role_reuses_canonical_set() -> None:
|
||||
"""The refusal set must be exactly foundation.policy.communications'
|
||||
NO_COMMS_ROLES — the same set services.gateway.content_actions uses to
|
||||
gate the dm() sender side — so the two never drift apart."""
|
||||
expected = {"auditor", "pr_reviewer", "prompter", "secretary"}
|
||||
expected = {"prompter", "secretary"}
|
||||
assert {role.value for role in NO_COMMS_ROLES} == expected
|
||||
|
||||
@@ -1,10 +1,15 @@
|
||||
"""Auditor is silent — runtime guard refuses dm().
|
||||
"""dm() sender-side no-comms guard: prompter/secretary refused; auditor and
|
||||
pr_reviewer now pass through (they carry dm/read_a2a so the CEO can DM a
|
||||
mid-flight one and it can reply in-thread).
|
||||
|
||||
Spec §5.5: the auditor is a silent observer. The spawn manifest already
|
||||
omits `dm` from the auditor's tool surface, but that is a convention-only
|
||||
defense. These tests pin a defense-in-depth runtime guard inside
|
||||
ContentActions.dm: if the caller's role is "auditor", the verb refuses with
|
||||
Envelope.not_authorized regardless of how the call arrived.
|
||||
Spec §5.5 originally made the auditor's silence absolute (no dm surface at
|
||||
all). It's now scoped: the auditor still never INITIATES peer A2A — that's
|
||||
enforced in agents_config.can_a2a_direct, not by ContentActions.dm's role
|
||||
gate — but the gate itself (``_NO_COMMS_ROLES``, derived from
|
||||
foundation.policy.communications.NO_COMMS_ROLES) no longer blocks it or
|
||||
pr_reviewer. These tests pin that the handler-level guard (defense-in-depth
|
||||
for any call that bypassed the manifest) matches the current NO_COMMS_ROLES
|
||||
set exactly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -43,30 +48,9 @@ def _make_deps(agent_role: str, **overrides: AsyncMock) -> ContentActionsDeps:
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auditor_dm_returns_not_authorized() -> None:
|
||||
"""Auditor role calling dm() is refused regardless of manifest."""
|
||||
auditor_id = uuid4()
|
||||
deps = _make_deps("auditor")
|
||||
actions = ContentActions(deps)
|
||||
|
||||
env = await actions.dm(
|
||||
agent_id=auditor_id,
|
||||
recipient=str(uuid4()),
|
||||
text="hi",
|
||||
task_id=uuid4(),
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "not_authorized"
|
||||
haystack = (body.get("message") or "") + " " + (body.get("remediate") or "")
|
||||
assert "silent" in haystack.lower() or "auditor" in haystack.lower()
|
||||
deps.a2a.send.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_developer_dm_passes_auditor_guard() -> None:
|
||||
"""dm() for a non-auditor role is not blocked by the new guard."""
|
||||
"""dm() for a non-auditor role is not blocked by the no-comms guard."""
|
||||
dev_id = uuid4()
|
||||
deps = _make_deps("developer")
|
||||
actions = ContentActions(deps)
|
||||
@@ -85,22 +69,23 @@ async def test_developer_dm_passes_auditor_guard() -> None:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The same no-comms invariant covers pr_reviewer / prompter / secretary
|
||||
# (CLAUDE.md): pr_reviewer "posts its change-request on the PR itself — no
|
||||
# say/dm"; prompter + secretary are "restricted to note + evidence — no
|
||||
# say/dm/notify". The auditor guard's own comment claims defence-in-depth for
|
||||
# "any call that bypassed the manifest" — that rationale must hold for these
|
||||
# three roles too, or the claimed defence-in-depth is only 1 of 4 silent roles.
|
||||
# The no-comms invariant now covers only prompter/secretary (CLAUDE.md):
|
||||
# they're restricted to note + evidence — human-only, own dedicated chat
|
||||
# pages. Auditor and pr_reviewer carry dm/read_a2a on their manifests (a CEO
|
||||
# can DM either and they can reply in-thread) so they must NOT hit this
|
||||
# guard — the auditor's silence toward PEERS is enforced separately, in
|
||||
# agents_config.can_a2a_direct, not here.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
_NO_COMMS_ROLES = ("pr_reviewer", "prompter", "secretary")
|
||||
_NO_COMMS_ROLES = ("prompter", "secretary")
|
||||
_DM_CAPABLE_ROLES = ("auditor", "pr_reviewer")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("role", _NO_COMMS_ROLES)
|
||||
async def test_no_comms_role_dm_returns_not_authorized(role: str) -> None:
|
||||
"""pr_reviewer / prompter / secretary may not dm() — handler-level guard.
|
||||
"""prompter / secretary may not dm() — handler-level guard.
|
||||
|
||||
Asserts the no-comms signal ("silent") in the message so the test fails for
|
||||
the right reason on RED: without the role guard, dm() with an unowned
|
||||
@@ -122,3 +107,28 @@ async def test_no_comms_role_dm_returns_not_authorized(role: str) -> None:
|
||||
haystack = (body.get("message") or "") + " " + (body.get("remediate") or "")
|
||||
assert "silent" in haystack.lower()
|
||||
deps.a2a.send.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("role", _DM_CAPABLE_ROLES)
|
||||
async def test_auditor_and_pr_reviewer_dm_pass_no_comms_guard(role: str) -> None:
|
||||
"""auditor / pr_reviewer are no longer refused by the no-comms guard.
|
||||
|
||||
Mirrors test_developer_dm_passes_auditor_guard: whatever else dm() does
|
||||
downstream (ownership checks on the fake task_id), it must not be the
|
||||
no-comms ("silent") rejection — that guard no longer names these roles.
|
||||
"""
|
||||
deps = _make_deps(role)
|
||||
actions = ContentActions(deps)
|
||||
|
||||
env = await actions.dm(
|
||||
agent_id=uuid4(),
|
||||
recipient=str(uuid4()),
|
||||
text="hi",
|
||||
task_id=uuid4(),
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
if body.get("error") == "not_authorized":
|
||||
haystack = (body.get("message") or "") + " " + (body.get("remediate") or "")
|
||||
assert "silent" not in haystack.lower()
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
"""Playbook content verbs — role grants + ContentActions RBAC.
|
||||
|
||||
Delivery roles DRAFT playbooks; only the Auditor CURATES (approve/reject/archive).
|
||||
The Auditor's no-say/no-dm restriction is preserved (these are KB curation
|
||||
actions, not agent comms).
|
||||
Curation is KB-curation, not agent comms, and stays separate from the
|
||||
Auditor's dm/read_a2a surface (CEO-reachable, reply-only — see
|
||||
agents_config.can_a2a_direct for the peer-initiation refusal).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -33,9 +34,12 @@ def test_auditor_curates_but_does_not_draft() -> None:
|
||||
for verb in _CURATE_VERBS:
|
||||
assert verb in do_tools
|
||||
assert "draft_playbook" not in do_tools
|
||||
# No-say/no-dm preserved.
|
||||
# No "say" tool exists; dm/read_a2a ARE present (CEO-reachable, reply-only
|
||||
# — the auditor still never initiates peer A2A, enforced in
|
||||
# agents_config.can_a2a_direct, not by omitting the tool here).
|
||||
assert "say" not in do_tools
|
||||
assert "dm" not in do_tools
|
||||
assert "dm" in do_tools
|
||||
assert "read_a2a" in do_tools
|
||||
|
||||
|
||||
def test_delivery_role_cannot_curate() -> None:
|
||||
|
||||
@@ -471,6 +471,23 @@ def test_can_a2a_direct_board_to_developer_denied() -> None:
|
||||
assert reason is not None
|
||||
|
||||
|
||||
def test_can_a2a_direct_auditor_to_product_owner_denied() -> None:
|
||||
"""The auditor is a silent observer of peers — it never INITIATES A2A,
|
||||
even to a fellow board member. It still replies inside a CEO-opened DM
|
||||
(a stateful path in A2AService, not gated by this matrix)."""
|
||||
allowed, reason = can_a2a_direct("auditor", "product-owner")
|
||||
assert allowed is False
|
||||
assert reason is not None
|
||||
assert "silent" in reason.lower()
|
||||
|
||||
|
||||
def test_can_a2a_direct_auditor_to_main_pm_denied() -> None:
|
||||
allowed, reason = can_a2a_direct("auditor", "main-pm")
|
||||
assert allowed is False
|
||||
assert reason is not None
|
||||
assert "silent" in reason.lower()
|
||||
|
||||
|
||||
def test_can_a2a_direct_pr_reviewer_to_main_pm_allowed() -> None:
|
||||
"""The root→master gate reviewer delivers pr_fail change-requests to the
|
||||
owning Main PM. Denying it silently strands the verdict (blind re-submit)."""
|
||||
@@ -533,10 +550,10 @@ def test_get_a2a_route_hint_unknown_from_agent_falls_through() -> None:
|
||||
# A2A_ALLOWED_PAIRS — the switchboard's static org-chart pair matrix
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_EXPECTED_PAIR_COUNT = 88
|
||||
_EXPECTED_PAIR_COUNT = 93
|
||||
_EXPECTED_GROUP_COUNTS = {
|
||||
"board": 3,
|
||||
"ceo": 18,
|
||||
"ceo": 23,
|
||||
"cell-backend": 15,
|
||||
"cell-frontend": 15,
|
||||
"cell-ux_ui": 15,
|
||||
@@ -574,22 +591,22 @@ def test_a2a_allowed_pairs_excludes_non_participants_keeps_ceo() -> None:
|
||||
|
||||
|
||||
def test_a2a_allowed_pairs_ceo_paired_with_every_dm_capable_agent() -> None:
|
||||
"""CEO → anyone with an agent-comms surface is allowed, so every non-CEO
|
||||
switchboard slug EXCEPT the no-comms roles (auditor, pr_reviewer — no
|
||||
dm/read_a2a on their manifest, so a CEO DM to them is a black hole)
|
||||
appears in exactly one ``ceo``-group pair."""
|
||||
"""CEO → anyone with an agent-comms surface is allowed. Every switchboard
|
||||
slug is now dm-capable (auditor and pr_reviewer carry dm/read_a2a so a
|
||||
mid-flight one can read + reply — prompter/secretary are the only
|
||||
no-comms roles left, and they're already excluded from the switchboard
|
||||
entirely, see test_a2a_allowed_pairs_excludes_non_participants_keeps_ceo),
|
||||
so every non-CEO slug appears in exactly one ``ceo``-group pair."""
|
||||
ceo_pairs = [p for p in A2A_ALLOWED_PAIRS if "ceo" in (p.agent_a, p.agent_b)]
|
||||
non_ceo_slugs = (
|
||||
{p.agent_a for p in A2A_ALLOWED_PAIRS} | {p.agent_b for p in A2A_ALLOWED_PAIRS}
|
||||
) - {"ceo"}
|
||||
dm_capable_slugs = {
|
||||
s for s in non_ceo_slugs if get_agent_role(s) not in ("auditor", "pr_reviewer")
|
||||
}
|
||||
assert all(p.group_key == "ceo" for p in ceo_pairs)
|
||||
assert len(ceo_pairs) == len(dm_capable_slugs)
|
||||
# And the no-comms roles are confirmed absent from any ceo-group pair.
|
||||
ceo_slugs = {p.agent_a for p in ceo_pairs} | {p.agent_b for p in ceo_pairs}
|
||||
assert ceo_slugs.isdisjoint(non_ceo_slugs - dm_capable_slugs)
|
||||
assert len(ceo_pairs) == len(non_ceo_slugs)
|
||||
ceo_slugs = ({p.agent_a for p in ceo_pairs} | {p.agent_b for p in ceo_pairs}) - {
|
||||
"ceo"
|
||||
}
|
||||
assert ceo_slugs == non_ceo_slugs
|
||||
|
||||
|
||||
def test_a2a_allowed_pairs_group_key_counts() -> None:
|
||||
|
||||
Reference in New Issue
Block a user