Skip to content

feat(addressing): agents name a role, the platform resolves the person — reports and messages (abilityai/trinity-enterprise#606) - #3050

Merged
vybe merged 5 commits into
devfrom
feature/606-address-by-role
Sep 30, 2026
Merged

vybe merged 5 commits into
devfrom
feature/606-address-by-role

Conversation

@dolho

@dolho dolho commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes abilityai/trinity-enterprise#606. It's a cross-tracker issue, so it closes by hand at release.

What

The ent#606 ruling says every outbound object an agent produces for a human names a role, never a person: to: primary | approver | viewer | operator, which the platform resolves. #3028 built that for asks. This PR extends it to reports and messages through one shared rule, so an ask and a report addressed to the same role reach the same people.

services/role_addressing.resolve

Reports

  • ReportCreate.to:
to resolves to Result
operator, nobody, or the agent's owner operator-only (the owner reads the operator surface)
one other person the checked audience; not on the roster → 422 role_unreachable
several people 422 role_resolves_to_several (a report has one reader)
  • to together with audience_email → 422 addressing_conflict.
  • Neither → operator-only, unchanged.

Messages

  • SendMessageRequest takes exactly one of to (primary | approver | viewer) or recipient_email.
  • A role nobody fills, or several people fill, → named 422.
  • operator is not a message recipient. The operators' door is an ask_operator alert.

MCP

  • report and send_message accept to and echo the role, never the resolved person.

Compatibility

  • Agent-supplied audience_email / recipient_email still work for two releases. Each use is logged as deprecated once per agent and surface.

The provider side (people_for over role assignments) is abilityai/trinity-enterprise#721, stacked on the assignments PR.

Verification

  • tests/unit/test_ent606_role_addressing.py: 27 tests.
  • Mutations, each of which turns a test red when applied:
    • off: the several-people refusal (reports);
    • off: the several-people refusal (messages);
    • off: the no-primary → operator fallback;
    • off: the owner → operator-only rule.
  • Suites touching reports, messages, asks and the MCP/model guards: 1168 passed.
  • Full unit suite: 18637 passed and 15 failed. The 15 are the same set that fails on untouched dev (environment-dependent IPv4-mapped IPv6 SSRF cases).
  • tsc --noEmit on the MCP server is clean.
  • The enterprise-docs guard pattern finds nothing in the diff.

Docs:

  • architecture/backend.md (the service row);
  • feature-flows/agent-reports.md and proactive-messaging.md (an "Addressed by role" section in each);
  • requirements/security.md §26.11.

🤖 Generated with Claude Code

@vybe

vybe commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

merge-train FYI: your base #3028 was squash-merged to dev as a7de3b2. feature/ent611-native-ask was left in place so this PR wasn't auto-closed — retarget this PR to dev (and merge dev into it) when you're ready; the branch can be deleted after.

@dolho
dolho changed the base branch from feature/ent611-native-ask to dev September 29, 2026 10:29
…n — reports and messages (Abilityai/trinity-enterprise#606)

The ent#606 ruling (prds/permission-layers.md §5a): every outbound object
an agent produces for a human carries `to:` — primary | approver | viewer
| operator — and the platform resolves the person through the agent's
assignments. #611 (#3028) built it for asks; this extends it to reports
and messages through ONE shared rule.

- services/role_addressing.resolve: a provider's people_for answer first,
  else the core defaults (primary → owner, operator → nobody,
  approver/viewer refused). No primary assigned → operator. Refusals are
  named (invalid_to, role_unassigned). ask_service._address now delegates
  to it; asks are unchanged (#3028's tests pass as they were).
- Reports: ReportCreate.to.
  - operator, nobody, or the owner → operator-only (the owner reads the
    operator surface).
  - one other person → the checked audience; off the roster → 422
    role_unreachable.
  - several people → 422 role_resolves_to_several.
  - to + audience_email → 422 addressing_conflict.
  - Neither → operator-only, unchanged.
- Messages: exactly one of `to` (primary | approver | viewer) or
  recipient_email. A role nobody fills or several fill is a named 422;
  operator is not a message recipient (use an ask_operator alert).
- MCP report / send_message take `to` and echo the role, never the
  resolved person.
- Agent-supplied emails still work for two releases, logged as
  deprecated.

The provider side (people_for over role assignments) ships separately in
the private module.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dolho
dolho force-pushed the feature/606-address-by-role branch from c73d956 to c3c6327 Compare September 29, 2026 10:30

@obasilakis obasilakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran /review and /validate-pr on this PR. No blocking findings. Five minor items below.

Evidence

  • test_ent606_role_addressing.py: 27/27 pass. Adjacent suites (ent611 asks, ent364, ent428, ent365 report audience, #918 report endpoint, message access gate, proactive rate-limit/toggle, models guard): 431 pass.
  • Mutations: 7 of 9 go red. That covers the four in the PR body plus three more: role_unreachable removed, addressing_conflict removed, and provider lookup bypassed. The two survivors are items 3 and 4 below.
  • Security greps on the diff are clean: only @example.com addresses, and no IPs, host paths, env vars or mode changes.

Findings

  1. Private file path in public repo. services/role_addressing.py (module docstring) and docs/memory/requirements/security.md §26.11 cite prds/permission-layers.md §5a. That file is not in this repo, and nothing on dev referenced it before. The enterprise-docs guard pattern passes it, but CLAUDE.md bars references to private repos. Suggest citing ent#606 only.
  2. Resolved email can reach the agent on a failed role-addressed message. When to resolves to a person and delivery fails, the proactive service's error detail includes that person's email: proactive_message_service.py NotAuthorizedError (~L270) and the Telegram/Slack/WhatsApp RecipientNotFoundError messages (~L478, ~L592, ~L619). The router passes str(e) through as the 403/404 detail. The MCP send_message tool replaces 403/404 with generic text, so this is reachable only through a direct HTTP call with the agent's key. It still conflicts with the rule that an agent sees the role and never the person. Option: when request.to is set, return a role-only detail for 403/404.
  3. Dead branch in ask_service._address. The invalid_to mapping never runs, because create's validation at ask_service.py:507 rejects unknown roles earlier. A mutation disabling it leaves every test green. Drop it, or keep it and add a comment saying it is defensive.
  4. Deprecation log asymmetry. Reports log email addressing only when current_user.agent_name is set. Messages log it for every caller, and the message-side log has no test (the mutation removing it survives).
  5. No removal ticket for the deprecation. audience_email / recipient_email are said to keep working "for two releases". A follow-up issue for removing them would keep that from being lost.

Process notes

  • Fixes abilityai/trinity-enterprise#606 points at the other tracker, so the issue will not auto-promote or auto-close. It needs status-in-dev set by hand after merge and a manual close at release.
  • /cso --diff has not run. The pipeline table asks for it on P1 features.

…Abilityai/trinity-enterprise#606)

Review follow-ups on PR #3050:
- A role-addressed message that fails authorization (403) or delivery (404)
  now returns a role-only detail (role_not_opted_in / role_no_channel); the
  service's text names the resolved email, so it passes through only for
  email-addressed sends.
- The message-side email-addressing deprecation log fires only for agent
  keys, matching reports.
- Drop the unreachable invalid_to mapping in ask_service._address; its only
  caller passes a role already validated against ASK_ROLES.
- Cite trinity-enterprise#606 only, not a private design file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dolho

dolho commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @obasilakis — all five addressed in fbab7e4 (on top of a merge of current dev, 61f2764; clean, no migration/registry conflicts).

1. Private file path — prds/permission-layers.md §5a removed from services/role_addressing.py, docs/memory/requirements/security.md §26.11 and the test module docstring; they cite abilityai/trinity-enterprise#606 only. grep prds/ over the PR diff now shows only removed lines.

2. Resolved email on a failed role-addressed message — fixed at the router. When request.to is set, 403 → {"code": "role_not_opted_in", "role": ..., "message": ...} and 404 → {"code": "role_no_channel", ...}, naming the role only. Email-addressed sends keep str(e) byte-for-byte. New tests drive POST /api/agents/{name}/messages through a real TestClient with an agent-key principal over the real proactive service (db/audit/rate-limit stubbed):

  • role-addressed not-opted-in → 403, no @ / no resolved email in the body;
  • role-addressed no-channel for telegram, slack, web, auto → 404, role-only;
  • a service error whose text carries the email (the per-channel RecipientNotFoundError shape) → 404, role-only;
  • email-addressed → the existing 403 detail (with the email) and the existing 404 detail, unchanged.

Two notes. WhatsApp is not reachable here: the request model's channel Literal excludes it (422). Also, through the real service the per-channel RecipientNotFoundError texts are already swallowed by _send_message_inner's channel loop and collapse to "No delivery channel available for recipient". So the reachable leaks were the 403, plus any channel error text that lands in last_error. The router fix covers all of them regardless.

3. Dead invalid_to branch — dropped. ask_service._address has one caller (create, ~L354), and it passes norm["to"] from _validated_ask, which already rejects anything outside ASK_ROLES (identical to role_addressing.ROLES). There are no other callers in the backend, the enterprise submodule or the tests. The docstring now says so.

4. Deprecation log asymmetry — messages now log only when current_user.agent_name is set, the same condition reports use (the route takes current_user alongside AuthorizedAgentByName). Two new tests: an agent key using recipient_email logs once, and a human doesn't.

5. Removal ticket — filed #3087 (type-refactor, priority-p3). It covers what's deprecated, where the [role_addressing] log lines are (so operators can see remaining callers), and the removal criterion.

Tests (tests/.venv, from tests/):

  • test_ent606_role_addressing.py: 36 passed on -p randomly seeds 12345 and 99999.
  • With the adjacent suites (ent611 endings/native/native_pg, ent364, ent428, ent365, test_918 db/broadcast/endpoint, message access gate, proactive rate limits, proactive toggle guard, models centralized): 454 passed on seed 12345 and 454 passed on seed 99999.
  • No MCP files changed, so no npm test / tsc rerun.

Mutations (each reverted after):

  • router role-only detail reverted to str(e) → 6 red
  • message deprecation log removed → 1 red (agent-key test)
  • message log made unconditional → 1 red (human test)

Process

  • Because Fixes points at the enterprise tracker, status-in-dev needs setting by hand after merge, and the issue needs a manual close at release.
  • /cso --diff has not been run on this PR. It is still open for whoever lands it.

🤖 Generated with Claude Code

@vybe

vybe commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

merge-train: not on this train — validated READY (five review follow-ups verified; 407 passed), but it conflicts in src/backend/routers/messages.py with #3086, which is on this train. Rides the next train: merge dev once #3086 lands.

One question for you before then: with to=<role>, POST /api/agents/{name}/reports returns addressed_to as the resolved person's email (routers/reports.py:350-352). MCP strips it (reports.ts:52), but a direct REST call from the agent sees it — which contradicts test_ent606_role_addressing.py:13 and feature-flows/agent-reports.md:146 ("the agent never sees the person"), and no test asserts the response body. Also: the commit message of c3c632787 and the body still cite the private design file; edit the squash message at merge.

dolho and others added 2 commits September 30, 2026 10:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t's create response (ent#606)

merge-train mechanical fix: POST /api/agents/{name}/reports returned
addressed_to (the resolved person's email) to an agent-key caller. Route it
through _hide_audience like the read routes, and assert the response body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vybe

vybe commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

merge-train: this PR is on the train. I pushed one mechanical fix to your branch (fc6a2a1b4) for the open question from yesterday's pass.

  • routers/reports.py create response: it now returns Report(**_hide_audience(projected, current_user)), the same redaction the read routes use. Before this change, an agent key that published with to=primary got the resolved person's email back in addressed_to. That contradicts test_ent606_role_addressing.py:13 and requirements/security.md §26.11. A JWT human still gets the audience back.
  • Two new tests in test_ent606_role_addressing.py: they assert the response body for an mcp_scope="agent" caller (None) and for a human (the email). The agent test fails without the router change. 38/38 pass, and the report suites pass (559).
  • PR body: I removed the stale "Stacked on feat(operator-queue): agents raise asks natively — ask_operator, receipts, re-asks (abilityai/trinity-enterprise#611, PR B) #3028" line and the private design-file path. When squashing, please don't carry the c3c632787 message into the squash message, since it cites the same path.

Not blocking, but worth a follow-up: no TypeScript test runs send_message's exactly-one-of-to/recipient_email check or the role echo (tools/messages.ts:141-147). fbab7e492 also hasn't been re-reviewed since obasilakis's review.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: validated (validate-pr + review, mechanical fixes noted on the PR); merged individually — batch under 4, no train assembled.

@vybe
vybe merged commit 729423f into dev Sep 30, 2026
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants