Skip to content

fix(connections): room tools act through the connection serving the room's server - #5977

Merged
qingyun-wu merged 1 commit into
mainfrom
fix/room-server-connection
Oct 28, 2026
Merged

qingyun-wu merged 1 commit into
mainfrom
fix/room-server-connection

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Problem

After an install connects a second homeserver (for example after moving to a new one), room tools still act through the original connection. Asked to upload a file into a room on the new server, an agent's upload got 403 "not a member", and its room list showed only rooms on the old server. The same call with the old server's room ID worked.

Root cause

Nothing on the room-tool path looked at the room's server:

  • skills/agent-room-ops/_gateway.py gateway() took no room. It resolved env GATEWAY_TOKEN/REMOTE_TASK_TOKEN, then channels/$REMOTE_TASK_CHANNEL_DIR or ag2space/.env, then the vault key REMOTE_TASK_TOKEN. In the core session that chain always lands on the legacy, unsuffixed connection. Every room_ops verb (send, say, mention, read, fetch, react, join, members, grant, events emit) went through it.
  • scripts/channel-env.sh → src/channel_env_resolve.py resolve_channel_env(channels_dir, source) picks a file by layout only: .env first, then sorted *.env. With no room input, sourcing it gives the legacy token whatever room comes next.
  • skills/task-progress/scripts/notify.py _gateway_config(source) had the same env-first, channels/<source> chain.

The gateway bridges were already correct. Each lane posts task results itself, and the proactive drain only claims rooms on its own homeserver (delivery_destination.deliverable_here).

Fix

The choice now has one owner, src/connections.py:

  • connection_for_room(room_id) picks the row whose server (or agent_mxid server) is exactly the room ID's domain, so dev.example.org never matches example.org. A room ID with no domain goes to the one lane with a task open in that room, else the primary. No room means the primary. With fewer than two rows, no primary, or an unreadable record it returns None, and the caller keeps its existing single-connection chain.
  • connection_credential(row) takes the first token among the row's env var, its launch keys, its channel env file and its vault key, and only accepts one whose embedded gateway (if any) is the row's own host. Another server's token is never used.
  • It fails loudly with NoConnectionForRoom, naming the server, when no row serves the domain, the row is disconnected, or the row has no token for its own gateway.

The selection logic contains no server names.

Callers that now delegate:

  • _gateway.gateway(room_id) and every room_ops verb that has a room. A refusal makes room_ops print {"ok": false, "reason": ...} and exit 1.
  • notify.py _gateway_config(source, room).
  • channel-env.sh <source> --room <room-id>: prints the file for that room's connection, or nothing and exit 1 when that connection has no file here.
  • engine-conflict-resolve/deliver.py, which calls gateway(room_id).

docs/connections.md gains an "Acting in a room" section.

Tests

  • tests/connections-room-routing.test.py (new) is the resolver contract. It runs three connections at once (an old server, a new one, and that new one's dev. subdomain), plus an arbitrary example.org row. It checks: exact-domain routing; a subdomain never matching its parent; a case-insensitive domain; agent-mxid matching; no room giving the primary; a domainless room following the lane with a task open in it; an unknown server and a disconnected row refused by name; single-row and missing records returning None; a token for another gateway skipped; vault, launch-key and channel-file sources; a missing token refused.
  • tests/room-ops-room-connection.test.py (new) covers delegation. gateway() forwards the room and its answer outranks the env token. room_ops send and say reach the routed gateway with the routed bearer, a refusal exits 1, and notify delegates. Without core src/, both keep today's chain.
  • tests/channel-env-resolve.test.py adds --room cases in-process. Through the bash wrapper, a stub interpreter proves the room reaches the resolver, and bad --room usage exits 2.
  • Existing room_ops test stubs now accept the room argument.

Ran: connections, connections-cli, channel-env-, room-ops-, room_ops_, agent-room-ops-, test_room_ops, every notify test, engine-conflict-resolve, credential-contract-golden, hermetic-env-structural, requeue-prelude-rebuild, turn-ledger-stop-gate. All pass. Also ran ruff, review-checks.sh (prose-cap clean), lint-hermetic-bridge-tests and lint-claude-home-path. Diff coverage was 96% from a subset of those tests.

Same pattern, not in this PR

These still pick their gateway from the legacy chain with no room or server input. Each can delegate to connections.room_credential in a follow-up:

  • src/owner_room_access.py and src/channel_credentials.py: the pending-questions room database and the crons table fall back to the AG2 Space channel file.
  • skills/schedule-crons/ensure-cron-room.py, skills/bee-actions/scripts/bee_actions.py, and the health-check gateway credentials: env-only token chains.
  • skills/tutorial/scripts/getting_started.py, through channel_credentials.
  • room_ops rooms, history, agents and events pull/stream have no room, so they act through the primary only and do not list every server.
  • The bridge-injected context-first prompt (src/result_markers.py and its vendored copy) still names the lane's channel dir. room_ops now resolves by room itself, so that fallback is rarely needed.

🤖 Generated with Claude Code

…oom's server

On an install connected to several homeservers, room_ops, notify and
channel-env.sh always resolved the legacy unsuffixed credential, so an
upload to a room on another server got 403 "not a member" from the
wrong gateway.

connections.connection_for_room/room_credential is now the one owner of
that choice: the row whose server equals the room ID's domain exactly
(never a suffix match), the lane with a task open in a domainless room,
else the primary; no room is the primary. A domain no row serves, a
disconnected row, or a row with no token for its own gateway is refused
by name. Fewer than two rows keeps every caller's existing chain.

room_ops' gateway(room_id), notify's _gateway_config and
channel-env.sh --room delegate to it.

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

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • skills/agent-room-ops/_gateway.py (100%)
  • skills/agent-room-ops/events.py (100%)
  • skills/agent-room-ops/grant.py (100%)
  • skills/agent-room-ops/join.py (0.0%): Missing lines 34
  • skills/agent-room-ops/media.py (50.0%): Missing lines 69
  • skills/agent-room-ops/members.py (100%)
  • skills/agent-room-ops/mention.py (100%)
  • skills/agent-room-ops/react.py (0.0%): Missing lines 38
  • skills/agent-room-ops/read.py (100%)
  • skills/agent-room-ops/resolve.py (100%)
  • skills/agent-room-ops/room_ops.py (100%)
  • skills/agent-room-ops/say.py (100%)
  • skills/task-progress/scripts/notify.py (100%)
  • src/channel_env_resolve.py (100%)
  • src/connections.py (97.1%): Missing lines 1270-1271

Summary

  • Total: 159 lines
  • Missing: 5 lines
  • Coverage: 96%

skills/agent-room-ops/join.py

Lines 30-38

  30     gate = load_gate() if gate is None else gate
  31     if not gate_allows(agent_mxid, room_id, gate):
  32         return _result(False, room_id=room_id,
  33                        reason=f"client gate denied for {agent_mxid}")
! 34     base, headers = gateway(room_id)
  35     if not base:
  36         return _result(False, room_id=room_id, reason="no gateway configured")
  37     url = f"{base}/v1/rooms/{quote(room_id)}/join"
  38     try:

skills/agent-room-ops/media.py

Lines 65-73

  65         return _result(False, reason="no media ref given", room_id=room_id)
  66     gate = load_gate() if gate is None else gate
  67     if not gate_allows(agent_mxid, room_id, gate):
  68         return _result(False, ref=ref, room_id=room_id, reason=f"client gate denied for {agent_mxid}")
! 69     base, headers = gateway(room_id)
  70     if not base:
  71         return _result(False, ref=ref, room_id=room_id, reason="no gateway configured")
  72     from _gateway import urlencode
  73     q = {"ref": ref}

skills/agent-room-ops/react.py

Lines 34-42

  34     gate = load_gate() if gate is None else gate
  35     if not gate_allows(agent_mxid, room_id, gate):
  36         return _result(False, room_id=room_id, event_id=event_id, key=key,
  37                        reason=f"client gate denied for {agent_mxid}")
! 38     base, headers = gateway(room_id)
  39     if not base:
  40         return _result(False, room_id=room_id, event_id=event_id, key=key,
  41                        reason="no gateway configured")
  42     url = f"{base}/v1/rooms/{quote(room_id)}/{verb}"

src/connections.py

Lines 1266-1275

  1266         from channel_token import RELAY_TOKEN_VARS, token_from_env_file
  1267         from util_paths import claude_home_path
  1268         path = resolve_channel_env(claude_home_path("channels"), channel_dir or CHANNEL)
  1269         return [token_from_env_file(v, path) for v in RELAY_TOKEN_VARS] if path else []
! 1270     except Exception:  # noqa: BLE001 - no channel home or an unreadable file is simply no token
! 1271         return []
  1272 
  1273 
  1274 def connection_credential(row: Mapping[str, object], environ: Optional[Mapping[str, str]] = None, *,
  1275                           vault=None) -> "tuple[str, str]":

⚠️ Not measured

Changed, but absent from coverage.xml — outside [run] source in
.coveragerc, so diff-cover never examined these lines:

  • skills/agent-room-ops/test_room_ops.py
  • skills/engine-conflict-resolve/scripts/deliver.py

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.

1 participant