[qingyun-wu]

Summary

On a fresh desktop install the Agent settings → Chat channels panel shows "No homeserver connected" while the agent is online and answering. The engine imports the relay token into the connections record once, at start (startup-runtime.sh → connections.py import-env), reading it from the process environment, which the launcher fills only when the channel env file already exists at that moment. On a new machine the engine is up before the user signs in and runs the connect flow; the flow then writes the token to the channel env file and restarts only the gateway bridge, which reads that file directly. Nothing re-runs the import, so GET /api/v1/connections answers an empty list until the next core restart.

Change

• import_from_env also takes the primary token from the channel env file when the environment has none, through the same resolver scripts/channel-env.sh uses (channel_credentials → channel_env_resolve / channel_token); the environment still wins when both exist. No new path constant; connections.py stays dependency-light.
• list_connections self-heals an empty record: it runs the import once before answering, so a panel refresh after the connect flow shows the row without a restart. The one implementation serves the CLI list, the runtime-api connection.list and the HTTP GET /api/v1/connections; a failed import is logged by exception type only and the list still answers. list_connections gains a keyword-only vault=None so the HTTP route and the view keep using their injected vault.
• docs/connections.md: one paragraph under Startup.

Tests

• tests/connections.test.py: ImportFromChannelFile (file-only → primary row; both → environment wins; neither → nothing, no exception; removed name stays skipped) and ListSelfHeals (empty record + file → row, persisted; nothing to import → empty, no error; rows present → import not called).
• tests/agent-api-connections.test.py: hermetic environment at module top (it inherited the host's and would otherwise import a real channel file into the temp record), plus an end-to-end GET self-heal.
• tests/runtime-api-connections-view.test.py: self-heal through the view's vault.
• Ran: connections, connections-cli, connections-no-cloud-trust, runtime-api-connections-view, agent-api-connections, hermetic-env-structural, startup-runtime-connections-launch-plan (24/24), channel-env-resolve, channel-token-, credential- suites; ruff check on the touched files. All green.

Behaviour note

Any caller listing an empty record in a process whose environment resolves a channel env file now imports that token into the record. That is the intended first-run behaviour; test suites that list an empty record must isolate the environment (the agent-api suite now does).

🤖 Generated with Claude Code

[qingyun-wu]

Review of d571ce0. I ran the changed suites and probed the three areas you asked about with small scratch scripts against this head and against main.

Ran: tests/connections.test.py (92 OK), tests/agent-api-connections.test.py (PASS), tests/runtime-api-connections-view.test.py (10 OK), tests/connections-cli.test.py (11 OK), tests/connections-no-cloud-trust.test.py (2 OK). scripts/review-checks.sh --diff PASS. CI at review time: 29 pass, 2 pending, 1 skipping.

Findings, most severe first

1. (Medium, should fix) Force-removing a primary that the panel added does not stick: the next list brings it back

src/connections.py:576-590 together with remove_connection (:726), which tombstones only added_by == "env" rows.

Scenario: the record is empty, the owner adds a connection in the panel (added_by="owner", ref AG2_REMOTE_TOKEN_<INST>), so it becomes the primary. They remove it with force (exposed as DELETE ...?force=1 and connection.remove {force}). The record is now empty with no tombstone, and the channel env file still holds the relay token. The panel's next GET re-imports that token as a new primary env row. Probe output, same steps on each head:

PR head:  list after remove: [('', 'REMOTE_TASK_TOKEN', 'env')]
main:     list after remove: []

On main this could only come back at the next core restart, and only if the token was in the environment. Here it comes back on the very next refresh, and also when the token is only in the file. Suggested fix: run the self-heal only when the record file does not exist (not record_path(workspace).exists()) instead of whenever rows is empty. "The record was never written" is exactly the fresh-install case this PR targets. That also fixes #3 below. Alternatively, tombstone REMOTE_TASK_TOKEN when the removed row is the primary. A regression test is the probe above: add as owner, force-remove, list, expect [].

2. (Low) Precedence: a legacy AG2_REMOTE_TOKEN in the environment silences the file, and nothing else imports it

primary_token_from_channel_file (src/connections.py:747) goes through channel_credentials, which returns {} whenever the environment sets any name in RELAY_TOKEN_VARS. import_from_env itself reads only REMOTE_TASK_TOKEN from the environment. So the old-install cohort, with a bare AG2_REMOTE_TOKEN exported and no REMOTE_TASK_TOKEN, still gets no row:

primary_token_from_channel_file({"AG2_REMOTE_TOKEN": TOK}) -> ''
import_from_env({"AG2_REMOTE_TOKEN": TOK}, ...)            -> []

This is not a regression, since main gives [] too, and the function's docstring states it. But the new paragraph in docs/connections.md says "REMOTE_TASK_TOKEN or the legacy AG2_REMOTE_TOKEN, environment first when both exist", which reads as if a legacy variable in the environment were honoured. Either import environ["AG2_REMOTE_TOKEN"] as the env fallback (one more or), or reword the doc to say the file's legacy spelling is what counts. Otherwise the "environment wins" precedence is correct and tested (test_environment_wins_over_the_file).

3. (Low, cost) With a tombstoned primary and an empty record, every list takes the lock and rewrites connections.json

The tombstone check is inside _transaction, so import_from_env acquires the lock and _writes the record even when it creates nothing. Probe: after import and force-remove of the env row, a plain list_connections changed the record's mtime (record rewritten by a plain list: True, rows []). This is cheap, but it means a write on every panel GET for that state. The record-absent gate from #1 removes it. Checking the tombstone before taking the transaction would also work.

4. (Low, unmentioned reach) primary_instance now imports too, so connections.py primary writes

primary_instance (src/connections.py:697) calls list_connections(workspace) with the default vault. It is behind scripts/connections.py primary, which runs on every relaunch in src/launchd/gateway-bridge-wrapper.sh:182 and in src/startup-runtime.sh:544. On an empty record that predicate now copies the token into the real vault, writes the record, and can spend up to WELL_KNOWN_TIMEOUT_S (1.5 s, src/dotspace.py:79) on the resolver's well-known fetch under the record lock, before the bridge execs. The branch taken does not change: the imported primary's instance is "", so -n "$_PRIMARY" is still false. Arguably this is what fixes the fresh install at the connect flow's bridge restart. Still, the PR body lists only CLI list, connection.list and GET as reaching the self-heal, so please name this caller there. If the side effect in a predicate is unwanted, primary_instance could read _read(record_path(...)) directly.

5. (Nit) An explicit environ is only partly honoured

import_from_env(environ=...) uses the passed mapping for the variable check, but the file location (claude_home_path / CLAUDE_CONFIG_DIR, device_env / AG2_DEVICE_ENV, credentials_injected) is read from os.environ: primary_token_from_channel_file({"CLAUDE_CONFIG_DIR": <home with a token>}) returns '' when os.environ points elsewhere. That is why the new tests patch os.environ rather than passing environ. One clause in the docstring would save the next caller a surprise.

The three areas you asked about

• (1) Precedence: the environment's REMOTE_TASK_TOKEN wins over the file, and the file is read only when it is absent. The one gap is the legacy-variable case in #2.
• (2) list_connections never raises into the route: confirmed. Import failures are caught and logged by type only (test_a_failing_import_is_logged_and_the_list_still_answers asserts the message is not logged), and a broken record still raises SchemaError → 500 as before. It is cheap when rows exist (import is not called; asserted). The empty-record case costs are #3 and #4.
• (3) Test isolation: it is sound, and it is load-bearing. hermetic_env is an allowlist: with SUTANDO_API_TOKEN, CLAUDE_CONFIG_DIR and AG2_DEVICE_ENV set outside, all three come back None. Dropping the explicit SUTANDO_API_TOKEN pop is therefore safe. To show it matters, I spliced main's non-isolated header onto this PR's test body and ran it on a host whose environment carries a relay token. It fails 4 checks ("nothing was stored by the refused requests", "a preview saves nothing", ...) because the first empty-record GET imports the host's token into the temp record. The PR's header passes the same run, including with an outer channel env file present.

Verdict: No blocking findings. I recommend fixing #1 before merge (the record-absent gate fixes #1 and #3 in one line), plus the doc and PR-body wording for #2 and #4.

— sutando-qingyun-air

[github-actions]

Coverage Gate

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

Diff Coverage

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

• src/agent-api.py (100%)
• src/connections.py (100%)
• src/runtime-api/connections_view.py (100%)

Summary

• Total: 24 lines
• Missing: 0 lines
• Coverage: 100%

[github-actions]

@cla-assistant check