Repository navigation
fix(connections): a relay that lists the owner's whole fleet names the token's agent by whoami - #6029
Conversation
…e token's agent by whoami preview_connection verified an identity only when GET /v1/agents listed one agent. AG2 Space relays list every agent of the token's owner, so a relay token for one of several agents never verified: agent_mxid stayed null and a later start refused with no_identity. The gateway's GET /v1/whoami names the bearer's own agent from its registry; it now settles a multi-agent listing, taken only for an agent of the resolver-registered server. Rows imported from the environment (the legacy row included) never learned an identity. agent-api now runs learn_missing_identities at start and every minute, so GET /api/v1/connections names each row's agent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Requesting changes at 7615de0 (agent-assisted, Blue for Chi's seat). The identity check itself is sound:
_fetch_agentsand_fetch_whoamiuse the same pinned, vetted address through_fetch_json(TLS hostname check, no redirects);- the registered-server comparison is an exact lowercased match;
- an unregistered gateway or a rejected token never reaches whoami;
- each row's token goes only to its own embedded URL (
connection_credential'sown()).
tests/connections.test.py passes locally (295 OK). What I'd change:
- A whoami id that isn't in a non-empty listing is still accepted (
preview_connection, thenext(..., {"id": mine})fallback). Probe: whoami naming@ghost:<registered>against a fleet of@a/@breturnsverified=True. The owner-scoped listing is an independent cross-check, so when it's non-empty and doesn't contain whoami's id, leave the row unverified (keep the fallback only for an empty listing). - The learner has no backoff, and it writes every pass. For a row that can never verify (an unregistered gateway, or a fleet gateway without whoami),
run_identity_learnerdoes DNS +/v1/agents+/v1/whoami+ a Keychain read every 60 s, forever.learn_identity→_transactionalso rewritesconnections.jsonunder its lock each time, even when nothing was verified. Write only whenseen["verified"], and back off per row (for example, double up to an hour). - The same policy is now in two places.
_learn_missing_identity(launch path) and the newlearn_missing_identitiesshare their skip rules but resolve credentials differently. Have the launch path call one per-row helper. - Hardening:
_whoami_agent→server_of_mxidaccepts an id like@evil\nX=1:<registered>\n(re.match, loose localpart). Today it only lands in a process env var, but whoami is a new input, so usere.fullmatchwith a strict localpart. - Small:
run_identity_learnerswallows exceptions silently; one log line (no token) would help.test_a_row_without_a_token_or_a_disabled_one_is_left_alonenever exercises a disabled or parked row. Worth adding tests for 1, and for "the learner leaves the record untouched when nothing verifies".
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
…rner backs off and writes only on a verified preview - A whoami id that a non-empty agent listing does not name leaves the row unverified; the bare-id fallback stays only for an empty listing. - The whoami id is matched whole against a strict Matrix id (lowercase localpart grammar). - One per-row helper (_learn_row_identity) holds the skip rules for both the launch path and the background learner; each passes its own token resolution. - learn_identity runs only for a verified preview, so a pass that verifies nothing leaves connections.json untouched. - run_identity_learner backs a row that did not learn off, doubling from the interval up to an hour, and logs a failed pass by exception type. - Tests: whoami vs listing, strict id, disabled/parked rows, record untouched, backoff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in 8d9fd0d (fast-forward on top of 7615de0, no force-push):
One existing test changed.
|
sonichi
left a comment
There was a problem hiding this comment.
Approving at 8d9fd0d (agent-assisted, Blue for Chi's seat). All my items are resolved:
- A whoami id that a non-empty listing doesn't name now leaves the row unverified; the bare-id fallback is kept only for an empty listing.
_learn_row_identitywrites only on a verified preview, and the learner backs off per row (doubling, capped at an hour).- One per-row helper is shared by the launch path and the learner.
- The whoami id is
fullmatched against a strict mxid: I probed@evil\nX=1:x.org\n(rejected), a trailing newline (rejected), uppercase localpart (rejected) and a port (accepted). - A failed pass is logged by exception type, with no token.
tests/connections.test.py passes locally.
|
@cla-assistant check |
sonichi
left a comment
There was a problem hiding this comment.
Re-approving at cb33e45 (agent-assisted, Blue for Chi's seat). The push is a merge of main (#5929, #6018, #6028, #6030–#6032, #6034, #6035). The PR's own diff against main is still the three files I approved (+239/−13), and tests/connections.test.py passes locally at this head, now with #6030/#6031/#6032's connection changes merged in.
liususan091219
left a comment
There was a problem hiding this comment.
The code meets the approval bar at cb33e45; the one missing piece is a green CI run, so this is a comment, not an approval yet.
Evidence:
- Expert approval at this exact head from Chi's seat, after his five-item change request at 7615de0 was resolved (whoami must agree with a non-empty listing; learner writes only on a verified preview and backs off per row; one shared per-row helper; strict
fullmatchon the whoami mxid; failures logged without the token). - Verification exists: the PR body shows a read-only probe of real relay tokens — parent
verified=False agent_mxid=Nonefor both lanes, branchverified=True— and 8 new behavioural cases intests/connections.test.pythat fail at the parent. - The latest push is only a main merge; the PR diff is still the same three files (+239/−13). No hardcoded paths; license/cla success; no unresolved threads.
- CI is red at this head:
create-worker-roster-preflight.test.py(timed out reaping descendants) andwatch-tasks-stream-malformed-roster-row.test.py, plus the clean-install aggregate. All are worker-pool / watcher tests outside this diff, and the same roster-preflight failure hit #6033 at the same time, so they look like flakes. A rerun to green is what is missing.
Not blocking, named: a live round trip is still owed — the learner thread only starts on the next agent-api start, and nobody has restarted a core on this branch yet.
— Sutando (susans-MacBook-Pro)
What changed, and why?
preview_connectioncounted an identity as verified only when the relay'sGET /v1/agentslisted exactly one agent. An AG2 Space relay lists every agent of the token's owner for an agent relay token. So a token for an owner with more than one agent never verified:agent_mxidstayednull, the connections UI warned "Couldn't verify the identity behind this code", and a laterconnections.py startrefused withno_identity.The gateway already has
GET /v1/whoami, which returns the agent its registry resolves the bearer to, never a client claim. This PR:preview_connectionasksGET /v1/whoamion the same pinned, vetted address. The answer is accepted only when the agent's server is the resolver-registered server for the gateway host, so an unregistered gateway still can't establish an identity. Display name, avatar and owner come from the listing entry with that id.fetchbut nowhoamiasks no whoami, so existing tests and callers keep their behaviour.GET /api/v1/connectionsshowedagent_mxid: nullfor the ag2.space row an external core runs. Newlearn_missing_identitieslearns the identity onto every enabled row that names no agent, using that row's own credential (connection_credential). agent-api runs it at start and every 60 s (run_identity_learner) in a daemon thread.What should reviewers look at first?
preview_connection/_whoami_agent/learn_missing_identitiesinsrc/connections.py, then the thread start insrc/agent-api.py.What user behavior or bug does this prove?
The desktop app, with a core started outside it, could not tell which agent the core runs on ag2.space. So s.space sign-in fell through to "Choose your first agent" instead of moving that agent, and a pasted relay code was added unverified.
Before/after (read-only probe of this Mac's real relay tokens, agent ids redacted):
Feature/documentation consistency
N/A. No user-visible surface changes: the connections API shape is the same, and rows now carry an identity they were missing.
Tests
python3 tests/connections.test.py: 295 tests OK, 8 of them new inOwnerScopedListing:/v1/agentsthen/v1/whoamion the same pinned address;preview_connection() got an unexpected keyword argument 'whoami').python3 tests/agent-api-connections.test.py: PASS.tests/connections-cli.test.py,tests/runtime-api-connections-view.test.py,tests/connections-no-cloud-trust.test.py,tests/connections-room-routing.test.py: OK.ruff check: clean.Live path: the evidence is the read-only probe above. I did not restart a live core with this branch, on purpose (the owner asked that the running core not be touched). The learner thread starts with the next agent-api start.
Related
This is one of four independent engine fixes found while connecting a desktop-app owner's external core to s.space. The others are bridge reaping, promote keeping a connected bridge, and skip_backlog. Each one merges on its own.
🤖 Generated with Claude Code