feature: allow company agents to contact each other#694
Conversation
Phase one keeps the existing name-based A2A surface and only extends authorization/discovery for same-tenant company agents. The rule is centralized in permissions, then reused by prompt context, tool calls, and the OpenClaw gateway path so the first authorization pass exposes the same callable set that send-message accepts. Constraint: Phase one must be minimal and keep send_message_to_agent(agent_name=...) without target-id migration Rejected: Add directory tables or cache materialization | out of scope for phase one Rejected: Frontend communication-directory UI | user deferred UI work for now Confidence: high Scope-risk: moderate Directive: Do not broaden auto-contact beyond same-tenant access_mode=company agents without revisiting authorization semantics Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py backend/tests/test_agent_visibility.py Not-tested: Full browser UI flow because this commit intentionally has no frontend changes
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34fe01411e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if getattr(target_agent, "access_mode", None) != "company": | ||
| return False |
There was a problem hiding this comment.
When the source agent is private or custom but shares a tenant with a company agent, this helper still returns true because it only checks the target's access_mode. All new call sites use this result to add company agents to the source's relationship context/poll response and to bypass AgentAgentRelationship enforcement, so private/custom agents can auto-discover and message company agents without an explicit relationship even though the phase-one rule is described as company-agent auto-contact.
Useful? React with 👍 / 👎.
Capture the agreed Phase 1 split for roster permissions, query_roster, A2A ID sends, and prompt cleanup so code changes can land in small verified steps. Archive Phase 2/3 follow-ups separately from the immediate implementation scope. Constraint: docs/ is ignored locally, so these planning docs are force-added intentionally Constraint: Phase 1 should be implemented one small commit at a time Rejected: Start coding before committing the agreed plan | would mix design stabilization with implementation changes Confidence: high Scope-risk: narrow Directive: Keep later Phase 1 commits scoped to one phase slice each Tested: git diff --cached --stat Not-tested: runtime behavior; documentation-only commit
Introduce explicit use, manage, and roster visibility helpers so company/custom/private can follow the roster PRD without continuing to overload check_agent_access. Human use now treats company and custom as same-tenant usable while private stays creator-only; custom manage remains explicit manage grant plus creator/admin. Constraint: Keep current A2A auto-contact behavior unchanged in this slice Rejected: Rewrite send_message_to_agent in the same commit | would mix permission semantics with delivery changes Confidence: high Scope-risk: moderate Directive: Use evaluate_roster_agent_visibility for new A2A/toolized roster paths instead of AgentAgentRelationship authorization Tested: backend/.venv/bin/pytest backend/tests/test_agent_visibility.py Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py
Expose query_roster as an always-available agent tool and return structured roster results for visible digital employees and humans. The implementation uses Phase 1.1 visibility helpers, stable IDs, limit/offset paging without total counts, and conservative contact tool hints. Constraint: Keep existing send_message_to_agent and send_file_to_agent parameters unchanged in this slice Rejected: Build relevance ranking or department navigation now | V1 only needs stable search and pagination Confidence: high Scope-risk: moderate Directive: Do not use query_roster results as final send authorization; send tools must revalidate at delivery time Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py Tested: backend/.venv/bin/pytest backend/tests/test_agent_visibility.py Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py
Switch model-visible A2A send and file tools from agent_name to target_agent_id while preserving the existing delivery handlers, msg_type behavior, and string return style. Target resolution now uses roster visibility and current contactability instead of name matching plus relationship authorization. Constraint: Keep OpenClaw gateway's external target-by-name API unchanged Constraint: Do not JSON-ify A2A send results in this phase Rejected: Remove the old unused name resolver now | cleanup is intentionally deferred to keep this slice focused Confidence: high Scope-risk: moderate Directive: Call query_roster before A2A sends; do not reintroduce model-visible agent_name send parameters Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py Tested: backend/.venv/bin/pytest backend/tests/test_agent_visibility.py
Stop preloading digital employee relationships and company auto-contact agents into the system prompt. Human relationships remain as background under a narrower heading, while a short roster-first rule tells agents to use query_roster before A2A sends. Constraint: Human send tools are not ID-ified yet, so human relationship background remains temporarily Rejected: Delete relationship tables or UI now | Phase 1 only removes prompt dependency for digital employees Confidence: high Scope-risk: narrow Directive: Digital employee discovery belongs in query_roster, not prompt-time relationship expansion Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py Tested: backend/.venv/bin/pytest backend/tests/test_agent_visibility.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67a846d950
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| }, | ||
| "required": ["agent_name", "message", "msg_type"], | ||
| "required": ["target_agent_id", "message", "msg_type"], |
There was a problem hiding this comment.
Changing the hard-coded schema here is not enough for production agents: startup still calls seed_builtin_tools() from backend/app/services/tool_seeder.py, and that seeder still defines both A2A tools with required agent_name; get_agent_tools_for_llm() prefers the DB tool rows over these hard-coded definitions. In that normal DB-backed path the model will still be instructed to call send_message_to_agent/send_file_to_agent with agent_name, but the new runtime rejects calls that omit target_agent_id, so A2A messaging/file delivery fails after the first tool call instead of using the roster ID flow.
Useful? React with 👍 / 👎.
| if not target_agent_id or not message_text: | ||
| return "❌ Please provide target_agent_id and message content" |
There was a problem hiding this comment.
With this new required target_agent_id gate, existing direct callers that still pass agent_name now fail before any target lookup; backend/app/services/okr_daily_collection.py:172-180 still calls _send_message_to_agent() with {"agent_name": agent_member.name, ...} for tracked digital employees. When daily OKR collection runs for agent targets, every send returns this error and sent_agents stays at zero, so tracked agents stop receiving daily report requests.
Useful? React with 👍 / 👎.
| if visibility.can_contact and provider_type == "feishu" and (member.external_id or member.open_id): | ||
| contact_tools.append("send_feishu_message") |
There was a problem hiding this comment.
For a Feishu-synced org member, this marks the human as contactable and returns send_feishu_message solely from provider data, even when the source agent has no Feishu channel configured. I checked get_agent_tools_for_llm(): Feishu tools are only exposed when _agent_has_feishu(agent_id) is true, and a direct _send_feishu_message() call returns “This agent has no Feishu channel configured”; in those agents query_roster now reports a contact path the model cannot actually use, instead of returning the member as uncontactable or choosing an available platform route.
Useful? React with 👍 / 👎.
Phase 2 needs a concrete implementation boundary before code changes continue, because human messaging still mixes prompt relationships, names, platform users, and provider identities. This records the roster-first ID plan, compatibility boundary, commit sequence, and test plan in the existing agent roster planning document.\n\nConstraint: Code changes paused until the Phase 2 plan is written down.\nRejected: Continue directly in agent_tools.py | the user asked to stop code work and document the plan first.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check -- docs/agent-roster/phase2-3-future-archive.md\nNot-tested: Runtime behavior, documentation-only change
Phase 2 is ready for implementation, but the previous document mixed near-term human messaging work with later cleanup and productization. Splitting the documents makes the Phase 2 scope executable without carrying Phase 3 decisions into the code pass.\n\nConstraint: User wants to implement Phase 2 first and keep Phase 3 separate.\nRejected: Keep a combined Phase 2/3 archive | it blurs implementation scope for the next code changes.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check -- docs/agent-roster\nNot-tested: Runtime behavior, documentation-only change
Phase 2 now needs step-by-step review before implementation. The previous single document was too dense for confirming query roster changes, resolver design, send tool migration, and prompt/schema/test cleanup independently.\n\nConstraint: User asked to split the Phase 2 docs more finely before code changes.\nRejected: Keep one large Phase 2 document | it makes the implementation boundary harder to confirm.\nConfidence: high\nScope-risk: narrow\nDirective: Do not start Phase 2 code until the corresponding subdocument is confirmed.\nTested: git diff --check -- docs/agent-roster\nNot-tested: Runtime behavior, documentation-only change
Phase 2 should not teach the model a separate tool per third-party provider. This updates the plan so send_channel_message is the single third-party IM entrypoint, with provider-specific adapters behind it, while send_feishu_message remains only a legacy shortcut.\n\nConstraint: User chose a unified channel entrypoint with provider dispatch.\nRejected: Promote send_feishu_message as a new ID-based primary tool | it duplicates the channel abstraction and increases prompt/tool surface area.\nConfidence: high\nScope-risk: narrow\nDirective: Implement Phase 2 with send_platform_message for first-party users and send_channel_message for third-party channel users.\nTested: git diff --check -- docs/agent-roster\nNot-tested: Runtime behavior, documentation-only change
Phase 2 starts by making roster human records directly addressable before any messaging tool changes. The query path now treats target_member_id as a human exact lookup so normal broad roster search remains unchanged while send-tool preparation can re-check a specific OrgMember. Constraint: Do not change send tools or prompts in Phase 2.1. Rejected: Add resolver and send-tool IDs in the same commit | the user requested small confirmed steps. Confidence: high Scope-risk: narrow Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py Not-tested: Full backend test suite
Human messaging needs one shared place to turn roster identifiers into a checked OrgMember. This adds a resolver that applies tenant scoping, roster visibility, active-member checks, provider normalization, platform-user validation, and ambiguity errors without changing any send tool behavior yet. Constraint: Phase 2.2 is resolver-only; existing send tools remain on their old lookup paths until the next step. Rejected: Move send_platform_message and send_channel_message immediately onto the resolver | that belongs in the separately reviewable Phase 2.3 step. Confidence: high Scope-risk: narrow Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py backend/tests/test_roster_human_resolver.py Not-tested: Full backend test suite; live channel delivery
Human send tools now resolve recipients through the roster helper before dispatch. Channel sends accept stable member/provider identifiers, platform sends accept stable member/platform identifiers, and the legacy Feishu entry delegates to the unified channel path while the existing channel-specific delivery helpers remain in place. Constraint: Keep bottom-level channel delivery functions unchanged in this step. Rejected: Remove legacy Feishu implementation code now | the cleanup is reserved for Phase 3 after the new path is proven. Confidence: medium Scope-risk: moderate Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py backend/tests/test_roster_human_resolver.py backend/tests/test_human_send_tools.py backend/tests/test_a2a_msg_type.py::test_execute_tool_failure_writes_system_message Not-tested: Full backend test suite; live Feishu/DingTalk/WeCom/Slack/Teams/Wechat delivery
The model-facing contract now points human messaging through query_roster and stable roster IDs. Channel and platform schemas expose target_member_id, roster contact_tools recommend the unified channel sender, and prompt text no longer teaches name-based human sends as the primary route. Constraint: Keep send_feishu_message visible as a legacy fallback, but do not make it the preferred prompt path. Rejected: Remove human relationship background from prompts | it still provides useful context as long as it is not treated as the sending entry. Confidence: high Scope-risk: narrow Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py backend/tests/test_roster_human_resolver.py backend/tests/test_human_send_tools.py backend/tests/test_a2a_msg_type.py::test_execute_tool_failure_writes_system_message Not-tested: Full backend test suite; live model tool-choice behavior
Phase 2.4 should update model guidance, runtime schemas, seeded tool schemas, and tests without deleting the legacy human messaging paths. The old tool entrypoints and prompt removal work now has an explicit Phase 3 home so implementation can proceed in reviewable slices. Constraint: Phase 2 keeps legacy fallbacks available while moving model guidance to roster-first Rejected: Delete send_feishu_message and name-based fallbacks in Phase 2.4 | deletion belongs in Phase 3 after compatibility checks Confidence: high Scope-risk: narrow Directive: Keep Phase 2.4 limited to guidance/schema/test changes; do not delete legacy human messaging entrypoints before Phase 3 Tested: Reviewed doc diff Not-tested: No runtime tests; documentation-only change
Phase 2.4 moves the persisted builtin tool definitions and task-execution guidance onto the same roster-first path as the runtime schemas. Human outreach now points models at query_roster plus stable IDs while preserving legacy fallbacks for compatibility. Constraint: Phase 2.4 must not delete legacy human messaging entrypoints Rejected: Remove send_feishu_message from seeded tools | Phase 3 owns deletion after compatibility checks Confidence: high Scope-risk: narrow Directive: Keep seeded tool schemas aligned with AGENT_TOOLS when human messaging parameters change Tested: backend/.venv/bin/pytest backend/tests/test_human_send_tools.py backend/tests/test_query_roster_tool.py Tested: backend/.venv/bin/pytest backend/tests/test_a2a_msg_type.py
Strict verification surfaced pre-existing lint issues in files touched by Phase 2.4. Remove unused task executor imports and use the SQLAlchemy None comparator form so the touched surface passes ruff without changing runtime behavior. Constraint: Keep this separate from the Phase 2.4 behavior commit Confidence: high Scope-risk: narrow Tested: backend/.venv/bin/ruff check backend/app/services/tool_seeder.py backend/app/services/task_executor.py backend/tests/test_human_send_tools.py Tested: backend/.venv/bin/python -m py_compile backend/app/services/tool_seeder.py backend/app/services/task_executor.py backend/tests/test_human_send_tools.py Tested: backend/.venv/bin/pytest backend/tests/test_human_send_tools.py backend/tests/test_query_roster_tool.py backend/tests/test_a2a_msg_type.py Not-tested: Full backend suite still has unrelated existing failures listed in verification output
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 051630c448
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "description": "Send a message to a human colleague via their configured external channel (Feishu, DingTalk, WeCom, Slack, Teams, WeChat). Use query_roster first, then pass target_member_id. member_name and provider_user_id are legacy fallbacks only.", | ||
| "category": "communication", | ||
| "icon": "💬", | ||
| "is_default": True, |
There was a problem hiding this comment.
Adding send_channel_message as a default builtin communication tool bypasses the existing _agent_has_any_channel gate in get_agent_tools_for_llm(): the DB-backed loader includes default builtin tools and only filters the feishu category, so agents with no configured external channel will still be shown this tool. In that no-channel environment the model can follow the new roster instructions and call send_channel_message, only to fail at runtime with channel-specific configuration errors; keep this non-default/gated the same way _CHANNEL_MESSAGE_TOOL_NAMES is intended to work.
Useful? React with 👍 / 👎.
| if not target_agent_id or not message_text: | ||
| return "❌ Please provide target_agent_id and message content" |
There was a problem hiding this comment.
This new required target_agent_id check also breaks the OKR missing-objective outreach workflow: fresh evidence is the unchanged prompt in backend/app/api/okr.py:1873-1877, which still tells the OKR Agent to call send_message_to_agent(agent_name="...") even though that block already has agent_member.id. When that workflow contacts tracked agent members, the tool call is rejected here before any target resolution, so the OKR Agent cannot collect agent-owned OKRs unless the prompt is updated to pass target_agent_id or the legacy argument is accepted.
Useful? React with 👍 / 👎.
Phase 3 starts by removing old human-send affordances from the model-visible tool surface while preserving the internal compatibility functions and legacy DB records. The UI work is documented as Phase 4 so this commit stays limited to corresponding tool schemas and LLM tool assembly. Constraint: Phase 3.1 must precisely change the relevant tools only, not prompt text, Relationships UI, resolver behavior, send implementations, or DB tool enabled state. Rejected: Disable the send_feishu_message Tool row in seed data | that would widen a model-visibility cleanup into a DB tool-state migration. Confidence: high Scope-risk: narrow Directive: Keep legacy runtime handlers until old tasks and stored tool calls no longer need compatibility. Tested: backend/.venv/bin/pytest backend/tests/test_human_send_tools.py backend/tests/test_query_roster_tool.py backend/tests/test_a2a_msg_type.py Tested: backend/.venv/bin/ruff check backend/app/services/tool_seeder.py backend/tests/test_human_send_tools.py Tested: backend/.venv/bin/python -m py_compile backend/app/services/agent_tools.py backend/app/services/tool_seeder.py backend/tests/test_human_send_tools.py Tested: git diff --check Not-tested: Full backend ruff on agent_tools.py; existing unrelated file-level lint debt still reports errors.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c81ac2945c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if visibility.can_contact and provider_type in channel_provider_types and (member.external_id or member.open_id): | ||
| contact_tools.append("send_channel_message") |
There was a problem hiding this comment.
When a Feishu org member has only open_id populated, this condition still marks them contactable via send_channel_message, but the Feishu delivery path _send_feishu_message_to_member() only reads target_member.external_id and immediately returns that there is no linked Feishu user_id. In that open_id-only case, query_roster gives the model a target_member_id and contact tool that cannot deliver; either require external_id here for Feishu or send with receive_id_type="open_id" when only open_id is available.
Useful? React with 👍 / 👎.
| agent_result = await db.execute( | ||
| select(AgentModel) | ||
| .where(*agent_conditions) | ||
| .order_by(AgentModel.name.asc(), AgentModel.created_at.asc()) | ||
| .limit(fetch_size) | ||
| ) | ||
| for target_agent in agent_result.scalars().all(): | ||
| payload = _format_roster_agent(source, target_agent) | ||
| if payload and (include_uncontactable or payload["can_contact"]): |
There was a problem hiding this comment.
With the default include_uncontactable=false, this limits the database rows before _format_roster_agent() filters out stopped/error/expired targets. In a tenant where the first offset + limit + 1 name-sorted matching agents are unavailable, query_roster returns an empty or short page and has_more=false even though later running agents match, so agents can fail to discover valid colleagues. Filter contactable statuses before applying the SQL limit, or over-fetch until the requested page is filled after filtering.
Useful? React with 👍 / 👎.
Phase 3.2 closes the prompt side of the roster-first migration. Human relationship rows remain available only as collaboration notes, while both chat and task prompts now direct human contact through query_roster and ID-based send tools. Constraint: Keep the change to prompt text and prompt tests; do not alter resolver behavior, send implementations, relationship APIs, or UI. Rejected: Delete AgentRelationship data | too broad for this phase and still useful as context/notes. Rejected: Keep mentioning the legacy Feishu shortcut as fallback | the model already cannot see that tool and mentioning it reintroduces the wrong path. Confidence: high Scope-risk: narrow Directive: Future prompt edits must not reintroduce send_feishu_message, username, member_name, or Relationships as a human-send entry path. Tested: backend/.venv/bin/pytest backend/tests/test_agent_context.py backend/tests/test_a2a_msg_type.py backend/tests/test_human_send_tools.py backend/tests/test_query_roster_tool.py Tested: backend/.venv/bin/ruff check backend/app/services/agent_context.py backend/app/services/task_executor.py backend/tests/test_agent_context.py backend/tests/test_human_send_tools.py Tested: backend/.venv/bin/python -m py_compile backend/app/services/agent_context.py backend/app/services/task_executor.py backend/tests/test_agent_context.py backend/tests/test_a2a_msg_type.py Tested: git diff --check Not-tested: Full backend suite; prior unrelated infrastructure/DB fixtures have known failures.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 315a368641
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if status_value == "stopped": | ||
| return "agent_stopped" | ||
| if status_value == "error": | ||
| return "agent_error" |
There was a problem hiding this comment.
When a target agent is still creating (the model default and the create API status for native agents), this helper returns no unavailable reason, so query_roster advertises it as can_contact=true and _resolve_a2a_target_by_id() accepts sends to it. The gateway auto-contact path already limits company targets to running/idle, so the roster/A2A-ID path can delegate work to agents that are still provisioning instead of hiding them or returning an unavailable reason.
Useful? React with 👍 / 👎.
| if visibility.can_contact and member.user_id: | ||
| contact_tools.append("send_platform_message") |
There was a problem hiding this comment.
For an active org member with a stale or disabled linked user_id, this marks the roster entry contactable via send_platform_message, but the send resolver later loads UserModel and drops users from other tenants or with is_active=false, causing the send to fail with missing_platform_user. In tenants where platform accounts are deactivated while OrgMember rows remain active, query_roster gives the model a contact tool that cannot deliver; filter these entries here or include the same active-user check before adding the platform contact path.
Useful? React with 👍 / 👎.
The old Relationships tab still described and edited legacy relationship records, while the runtime path had already moved to roster IDs. This introduces a shared roster service, exposes it as a read-only HTTP API, and points the Agent detail tab at a read-only Directory UI while preserving the relationships API for compatibility. Constraint: Keep the existing relationships route key for link compatibility Constraint: query_roster tool behavior and limit validation must stay unchanged Rejected: Reuse relationships API as the UI data source | it would keep the wrong product semantics and diverge from query_roster Rejected: Delete legacy relationships UI/API | OKR and gateway compatibility paths still depend on it Confidence: high Scope-risk: moderate Directive: Do not reintroduce relationships data as the Directory source; use the roster service contract Tested: backend/.venv/bin/pytest backend/tests/test_query_roster_tool.py backend/tests/test_roster_human_resolver.py backend/tests/test_agent_roster_api.py Tested: npm run build Tested: git diff --check Not-tested: Browser click-through on the deployed server
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 213f1e8ca7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if access_mode == "company": | ||
| access_mode = _agent_access_mode(agent) | ||
| if access_mode in ("company", "custom"): |
There was a problem hiding this comment.
When a tenant has a custom agent, this now returns every active tenant user; however ensure_access_granted_platform_relationships() still runs for private/custom agents on create/update and when loading /relationships, and materializes every returned user as an AgentRelationship. Opening or creating one custom agent in a large tenant will therefore auto-add all platform users as relationship rows (the helper explicitly skips company agents to avoid that), bloating the relationship table and injecting a huge set of auto-added human notes into that agent's prompt.
Useful? React with 👍 / 👎.
The Phase 3 archive still described Phase 4 as management settings UI, while the dedicated Phase 4 documents now define the roster API, Directory UI, and legacy Relationships split. This updates the archive so the implementation sequence and current execution conclusion point at the actual Phase 4 rollout. Constraint: Keep this as documentation-only; code shipped in 213f1e8 Rejected: Fold this into the Phase 4 code commit | the doc edit pre-existed that code commit and should stay reviewable Confidence: high Scope-risk: narrow Tested: Documentation diff reviewed Not-tested: No runtime test needed for docs-only archive update
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad9a6c2d9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if visibility.can_contact and member.user_id: | ||
| contact_tools.append("send_platform_message") | ||
| channel_provider_types = {"feishu", "dingtalk", "wecom", "slack", "teams", "microsoft_teams", "wechat"} | ||
| if visibility.can_contact and provider_type in channel_provider_types and (member.external_id or member.open_id): |
There was a problem hiding this comment.
For Teams and WeChat roster members, this marks them contactable whenever the org member has an external/open ID, but the actual send paths require additional per-user inbound context: _send_teams_channel_message() needs a captured service_url plus an existing microsoft_teams chat session with external_conv_id, and _send_wechat_channel_message() needs a cached context_token and ignores open_id entirely. In tenants with synced Teams/WeChat users who have not messaged the bot first, query_roster will advertise send_channel_message and can_contact=true, but the follow-up send deterministically returns a “requires them to message the bot first”/missing user_id error instead of choosing an actually deliverable route.
Useful? React with 👍 / 👎.
Old agent and human contact calls could still succeed through names, usernames, provider user IDs, or stale DB tool schemas. That made model memory capable of bypassing the roster contract even after the UI/API had moved to stable target IDs. This change canonicalizes model-visible contact schemas, rejects legacy runtime arguments with query_roster guidance, and moves human file delivery to target_member_id for supported channels. Constraint: Existing user/model memory may still contain old tool calls, so legacy arguments must fail with actionable routing instructions instead of silently resolving. Rejected: Keep name/provider fallbacks for compatibility | they preserve the exact bypass that caused the A2A browser test failure. Confidence: high Scope-risk: moderate Directive: Do not reintroduce model-visible recipient names for contact tools; route contact actions through query_roster stable IDs. Tested: python -m py_compile backend/app/services/agent_tools.py backend/app/services/tool_seeder.py backend/app/services/agent_context.py Tested: backend/.venv/bin/pytest backend/tests/test_human_send_tools.py backend/tests/test_query_roster_tool.py backend/tests/test_a2a_msg_type.py -q
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 117fa0131c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .outerjoin(UserModel, OrgMember.user_id == UserModel.id) | ||
| .where(*human_conditions) | ||
| .order_by(OrgMember.name.asc(), OrgMember.synced_at.asc()) | ||
| .limit(fetch_size) |
There was a problem hiding this comment.
For member_type=human/all with the default include_uncontactable=false, this SQL limit is applied before format_roster_human() drops inactive members, stale platform users, and rows without a usable contact target. If the first offset + limit + 1 name-sorted matching OrgMember rows are uncontactable, later contactable humans are never fetched and has_more can be reported false, so the roster can appear empty even though valid recipients exist. Filter the contactable human cases before limiting, or over-fetch until the requested page is filled after formatting.
Useful? React with 👍 / 👎.
Directory is now the product and tool contract for contact discovery, so this replaces the model tool, HTTP route, frontend query path, and focused tests in one reviewable slice. Custom access is tightened to explicit authorization and the directory list path now uses bounded SQL pagination with supporting indexes.
Constraint: Feature has not shipped, so query_roster and /roster are replaced directly instead of kept as compatibility aliases.
Rejected: Keep query_roster as a hidden executable alias | the user explicitly requested no compatibility path for an unshipped feature.
Rejected: Reuse the legacy relationships editor as the Custom Directory maintenance surface | it still uses legacy relationship semantics and unpaginated candidate lists.
Confidence: medium
Scope-risk: moderate
Directive: Do not reintroduce query_roster or /agents/{agent_id}/roster without a new compatibility requirement.
Tested: python3 -m py_compile for modified backend modules and focused tests
Tested: npm run build in frontend
Not-tested: pytest; current local Python environment has no pytest/sqlalchemy installed
Custom agents need an explicit maintenance surface for the same data that Directory uses at runtime. This adds paginated Directory-owned endpoints for human and digital-employee entries, and surfaces them in the Directory tab for managers without reusing the legacy relationships editor. Constraint: Must keep the existing PRD data model: human entries use AgentPermission user scope, and agent entries use AgentAgentRelationship. Rejected: Reuse legacy relationships candidate endpoints | they are relationship-oriented and use fixed-size candidate lists that do not meet the Directory pagination requirement. Confidence: medium Scope-risk: moderate Directive: Keep Directory maintenance APIs under /directory/custom; legacy /relationships endpoints are not the source of truth for this UI. Tested: python3 -m py_compile backend/app/api/directory.py backend/tests/test_agent_directory_api.py Tested: npm run build in frontend Not-tested: pytest; current local Python environment has no pytest/sqlalchemy installed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62bede6f26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = await db.execute( | ||
| select(AgentAgentRelationship.id) | ||
| .where( | ||
| AgentAgentRelationship.agent_id == source_agent_id, | ||
| AgentAgentRelationship.target_agent_id == target_agent_id, |
There was a problem hiding this comment.
When the target is a custom agent, this helper treats any AgentAgentRelationship row as authorization, but the legacy relationship path also calls evaluate_agent_relationship_status() to revoke access when the relationship creator no longer manages both agents. In that stale-row scenario, query_directory and _resolve_a2a_target_by_id() still mark the custom target visible/contactable and allow sends, so permission revocations on either agent no longer take effect for custom-directory A2A contacts.
Useful? React with 👍 / 👎.
The product surface now names the former relationship roster as Directory, so direct links need a first-class /directory route instead of relying on the settings hash. The route still maps to the existing internal relationships tab to keep the change narrow and avoid reshaping tab state in this slice. Constraint: Existing tab state still uses the relationships key in translations and component branching. Rejected: Rename the internal tab key to directory | broader churn across translations and legacy settings hashes without product benefit for this fix Confidence: high Scope-risk: narrow Directive: Keep /agents/:id/directory as the canonical Directory URL while preserving /settings#relationships until the tab key is renamed deliberately. Tested: npm --prefix frontend run build
Directory cards were exposing platform/user/member wording at the same time, which made first-party Clawith users and third-party synced contacts hard to distinguish. The card now labels first-party humans as Clawith users, labels synced humans by provider, and hides the extra platform user id from the primary card surface. Constraint: Backend still returns both member_id and platform_user_id for tool execution and internal linking. Rejected: Rename backend fields in this slice | the confusion is UI copy and display hierarchy, not the API contract. Confidence: high Scope-risk: narrow Directive: Keep raw platform_user_id available in API responses, but do not surface it as a primary Directory card label unless a debugging/admin detail view needs it. Tested: npm --prefix frontend run build
Directory cards should explain where a contact comes from instead of exposing raw Clawith, member, or provider ids. The card now keeps source badges such as Clawith User, Feishu User, or Agent, and removes the visible id block entirely. Constraint: Raw ids remain in API responses for stable tool execution and React list keys. Rejected: Rename API fields in this slice | the user-facing issue is display noise, while backend ids are still required for operations. Confidence: high Scope-risk: narrow Directive: Do not reintroduce raw ids into the main Directory card; use a dedicated debug/admin detail surface if ids are needed. Tested: npm --prefix frontend run build
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71f749b34f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| select(Agent).where( | ||
| Agent.id == payload.target_agent_id, | ||
| Agent.tenant_id == agent.tenant_id, | ||
| Agent.id != agent.id, | ||
| Agent.access_mode != "private", |
There was a problem hiding this comment.
When payload.target_agent_id is another custom agent, this accepts it solely because it is non-private and in the same tenant. The inserted AgentAgentRelationship is later treated as the authorization for query_directory and _resolve_a2a_target_by_id, so a manager of one custom agent can grant that source access to a custom target they do not manage; the legacy relationship path avoided this by requiring the current user to manage both agents.
Useful? React with 👍 / 👎.
| }, | ||
| }, | ||
| "required": ["member_name", "message"], | ||
| "required": ["message"], |
There was a problem hiding this comment.
This canonical LLM schema only requires message, but _send_channel_message() immediately rejects calls that omit target_member_id. Because _canonicalize_llm_tool() replaces DB rows with this definition, channel-enabled agents can make schema-valid send_channel_message(message=...) calls that deterministically fail instead of being forced through the roster ID flow.
Useful? React with 👍 / 👎.
Chrome reported unlabeled form fields on the Directory route. The affected controls already had visual context, so this records stable form names and accessible labels without changing behavior. Constraint: Keep the fix limited to the current browser-reported controls Confidence: high Scope-risk: narrow Tested: npm run build Tested: git diff --check
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08184b3e4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| }, | ||
| "required": ["username", "message"], | ||
| "required": ["message"], |
There was a problem hiding this comment.
When an agent is shown send_platform_message, this canonical schema still allows a call with only message. Because _canonicalize_llm_tool() replaces DB rows with this definition, the model can make a schema-valid send_platform_message(message=...) call, but _send_platform_message() immediately rejects it unless target_member_id or platform_user_id is present, so platform roster contacts can fail instead of being forced through the query_directory ID flow.
Useful? React with 👍 / 👎.
A few runtime-facing descriptions still called Directory targets roster members. The code paths already use query_directory, so this aligns the remaining prompt and tool descriptions without touching tests or internal helper names. Constraint: User requested code changes only; test files were not modified Confidence: high Scope-risk: narrow Tested: git diff --check Tested: rg roster member backend/app frontend/src
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1b330a3cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| access_mode = _agent_access_mode(agent) | ||
| if _is_admin(user) and access_mode != "private": | ||
| return True | ||
|
|
||
| if access_mode == "custom": | ||
| result = await db.execute( |
There was a problem hiding this comment.
When a company-wide agent is configured with access_level="manage" (the create/update permissions API still stores this in company_access_level and a scope_type="company" permission), this helper now returns False for every non-creator, non-admin because only custom user permissions are checked after the admin branch. check_agent_access() then falls through to can_use_agent() and reports only use, so users who were granted company-wide manage access lose Settings/Directory/credential management despite the saved permission.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation