[sonichi]

What

ag2-sparrow-claude-local connect --base <url> --plugin-dir <dir> (it asks for the code, or reads it with --code -) covers the local half of AG2 Space's two-step "Add a Claude Code agent" flow. This is S4 for claude-local, plus the S3 redeem-and-seat step in connect_common.

Stacked on #5859 (the seat token file). This branch carries #5859's commits first, so review only the commits after them. Merge order: #5859 first, then this one, rebased onto main.

In order, it does the following:

1. Preflight before the code is spent.
   • It refuses a host whose bridges a supervisor runs (Sutando's worker pool owns seats there) and a host with AGENT_CONNECT_TASK_DIR set.
   • It then checks the plugin, the Claude CLI version, tmux and the tmux socket, and claude auth status under --claude-config-dir.
   • A malformed --base or a code that does not start with acp. is refused first.
   • Any refusal says "the code was not used" and makes no request.
2. Redeem with POST <base>/connect/poll {"device_code": ...} (connect_common.redeem_code):
   • A 200 answer must carry relay_url, bearer and agent_id. The bearer and URL go into the process SecretRegistry the moment they are parsed.
   • A 503 is retried on a 1, 2, 4, 8, 16 s backoff, and the final failure says the code is still valid.
   • A 404 says "the code was already used or has expired; make a new one in AG2 Space".
   • An unreachable backend gives one line, and the code stays valid.
3. Seat (S3, standalone tier per plan §9.2). connect_common.write_seat calls the credential owner's write_seat_credentials, from fix(ag2-sparrow): a seat bridge reads its own token file first and never connects as the core #5859, with tier=owner. It writes <root>.bridge/channels/ag2space/{.env,token}, both 0600, where <root> is the agent's own root under the agents dir. GATEWAY_INSTANCE is kept from an existing seat .env and minted (32 hex) only when there is none. A reconnect rewrites token through a rename.
4. Install through install's own code. connect calls install() with --token-env-file <seat>/.env. No install step is copied, and the argument parser shares one session_options() between the two commands. fix(ag2-sparrow): a seat bridge reads its own token file first and never connects as the core #5859's wire_bridge then exports the seat and clears the inherited token, so the bridge runs as the seat alone.
5. Served setup (S2, minimal). connect asks setup_fetch.fetch_setup("acp", "claude"). It prints setup: served revision N when a setup comes back, else setup: built-in. On main no transport is installed yet (feat(ag2-sparrow): Sparrow fetches its served setup from the broker when a heartbeat announces a new revision #5855 adds it), so today it always prints the built-in line.

The bearer never appears in output, argv, the unit, the config or the seat .env. The only file that holds it is the seat's token.

Review round 1 (Mini's blockers)

• (a) https only, and a trusted relay host. --base and the relay_url the backend returns must be https. Plain http is allowed only on a loopback host, for tests. A relay must be on --base's own host or a host under it; anything else is refused unless the owner names that exact host with --trust-relay-host. Sharing the last two labels is not enough, because without the public suffix list (the stdlib has none) a.co.uk and b.co.uk would look like one owner's. The real flow keeps working: the relay base https://chat.ag2.space/relay and its provision API https://chat.ag2.space/api share a host. A refused relay URL is never written. The code is spent by then, and the error says so.
• (b) The code stays off argv. --code - reads one line from stdin. With no --code, connect asks on a terminal through getpass, and refuses (no request made) when stdin is not a terminal. --code VALUE still works but warns that ps and shell history keep it. The README's setup command uses the prompt or stdin form.
• (c) A 202 is retried, and a 404 says why. A 202 {state: redeeming, retry_after} is retried like a 503, honouring retry_after clamped to 1 to 30 s, on the same attempt budget. A 404 body with error, reason or state of agent_revoked says the agent was removed. gone, or anything else, says the code was used or expired.
• Unchanged: the redemption POST runs inside gateway_call, as the package's URL guard requires.

Review first

• packages/ag2-sparrow/ag2_sparrow/connect_common.py: the new section at the end.
• adapters/claude_code/cli.py: connect_preflight and connect.

Docs

packages/ag2-sparrow/adapters/claude-code/README.md (Launch section) documents connect.

Not in this PR

• ag2-sparrow-codex-local connect. codex-local's install takes --work and runs no bridge of its own; it relies on a bridge someone else runs. So there is no install-side bridge wiring for connect to reuse, and a codex connect today would spend the code and leave nothing polling. The follow-up is to lift claude-local's wire_bridge / bridge_env / bridge_home into connect_common so both adapters share it, then add codex connect on top.
• The plugin in the wheel (design Q9). --plugin-dir is still required.
• The built-in setup file (builtin-setup.claude.json) and the served verify hints. These wait on feat(ag2-sparrow): Sparrow fetches its served setup from the broker when a heartbeat announces a new revision #5855's transport.
• Repair mode: re-running install against an existing seat without spending a new code. With no --code, connect now asks for one.

Tests

Each file below was run with env -i HOME=$HOME PATH=$PATH nice -n 19 python3 <file> on this head, and all pass: packages/ag2-sparrow/tests/test_claude_code_adapter.py (251, the 10 new ConnectCode cases included), tests/gateway-credential-sources.test.py (164, the credential-owner guard), packages/ag2-sparrow/tests/test_adapter_boundaries.py, test_codex_local_connect.py, test_wheel_contents.py, and tests/ag2-sparrow-claude-local-wheel.test.py. scripts/review-checks.sh passes.

Before this PR, ag2-sparrow-claude-local connect was invalid choice: 'connect'. After it, the cases below pass.

packages/ag2-sparrow/tests/test_claude_code_adapter.py, ConnectCode, covers:

• one redemption, with the seat layout and modes, the unit exporting the seat, and the bearer absent from out, err, unit, config and .env;
• the instance minted once and kept across a reconnect while the token rotates;
• 404: a clear message and nothing written, with gone and agent_revoked told apart;
• 202 redeeming: retried honouring retry_after with a cap, and given up with a clear message;
• the code read from stdin, asked for on a terminal, refused with neither, and warned about on argv;
• an http relay URL, or one not on the base's host or under it (a .co.uk sibling included), never written, and accepted with --trust-relay-host; an http base refused unless loopback;
• redeem_code edges: unreachable, malformed 200, unreadable error body, other HTTP status, and retry_after bounds;
• 503 twice, then redeemed, with the backoff 1, 2;
• 503 throughout: the attempts and backoff exhausted, the code still valid, nothing written;
• preflight before redemption (logged out, no tmux, no plugin, a supervised host): the backend is never asked;
• a malformed code or base;
• the bearer never in the argv of any command connect runs.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

• packages/ag2-sparrow/ag2_sparrow/adapters/claude_code/cli.py (98.0%): Missing lines 421
• packages/ag2-sparrow/ag2_sparrow/connect_common.py (100%)
• packages/ag2-sparrow/tests/test_claude_code_adapter.py (100%)

Summary

• Total: 393 lines
• Missing: 1 line
• Coverage: 99%

packages/ag2-sparrow/ag2_sparrow/adapters/claude_code/cli.py

Lines 417-425

  417     if _dirs.bridge_host():
  418         return (f"this host's bridges are run by {_dirs.bridge_host()}, whose worker pool owns each agent's seat; "
  419                 f"{COMMAND} connect is for a host with no core")
  420     if _dirs.task_dir_configured():
! 421         return "AGENT_CONNECT_TASK_DIR is set; connect wires the agent's own root, so unset it first"
  422     claude = shutil.which(args.claude) or args.claude
  423     plugin = Path(args.plugin_dir).expanduser().resolve()
  424     if not (plugin / ".claude-plugin" / "plugin.json").is_file():
  425         return f"{plugin} is not a Claude Code plugin (no .claude-plugin/plugin.json)"

---

[qingyun-wu]

Approved at 56c5ce9 — agent-assisted review by Mars, Qingyun's Sutando seat, on Qingyun's instruction to review and approve Chi's PRs.

Reviewed this PR's own commits over #5859 (already approved at 2b846d2): connect_preflight/connect, the shared session_options, and the connect section of connect_common. Ran test_claude_code_adapter, gateway-credential-sources, test_adapter_boundaries, test_codex_local_connect, test_hatch_served_setup, ag2-sparrow-agent-identity and gateway-relay-result-no-ceiling at this head under env -i: all OK.

• Preflight and code/base checks run before any request, and every refusal before redemption says the code wasn't used.
• The redeemed bearer and relay_url are registered as secrets before validation can fail. The server-supplied agent_id only reaches the filesystem through root_segment (path-safe). relay_url and bearer are re-validated by write_seat_credentials (no '|', whitespace or control chars) before <url>|<bearer> is written 0600.
• The POST now runs inside gateway_call, and install is reused, not copied.

Non-blocking:

1. --base accepts plain http for any host (gateway_url_problem allows http), so the one-time device_code can go out in cleartext. I'd limit http to loopback for connect.
2. A 200 with a malformed body spends the code. The message says so, which is the best that can be done client-side.
3. Merge order: #5859 first, then rebase this one, as the body says.

[liususan091219]

Comment at a8dd49e8. The round-1 fixes check out, but I am not approving: Qingyun's approval is at 56c5ce9b, and this branch has since been rebased onto #5859's new head (2f6389bf). It also gained five commits after that approval (8e6083a0…a8dd49e8). No approval exists at this head.

What I checked (this PR's commits over #5859)

• ConnectCode: 16 tests pass, including https-only base/relay, an off-domain relay never written, --trust-relay-host, a 202 retried with retry_after, 404 gone vs agent_revoked, the code off argv (stdin/getpass), and the bearer absent from every argv. gateway-credential-sources (169), test_adapter_boundaries (10) and test_codex_local_connect (35) also pass.
• Mutation: with relay_problem made to return "" (connect_common.py:1093), 3 ConnectCode tests fail. With it reverted, they pass.
• Probe of relay_problem: https://relay.ag2.space under https://ag2.space is accepted. http://relay.ag2.space and https://evil.example are refused. evil.example with --trust-relay-host evil.example is accepted.
• I did not run the tmux-backed tests in test_claude_code_adapter.py. This review does not touch a tmux server, so here they report 11 failures and 13 errors, all on a missing tmux. CI runs them.
• Hardcoded-path scan over the added lines: clean.

Non-blocking

• _host_family (connect_common.py:1086-1090) takes the last two labels, so under a multi-label public suffix the check is a no-op. In my probe, https://evil.co.uk/r was accepted for --base https://api.foo.co.uk. AG2 Space hosts are *.space, so this does not bite today. A public-suffix-aware match, or an exact-host default, would make the docstring's "registrable part" true.

Gates

• CI: 36 pass, 3 skipped. license/cla: success. No unresolved threads.

What is still missing

• A fresh approval at this head. Qingyun's approval at 56c5ce9b predates the rebase and the round-1 fixes.
• #5859 merges first. It is open at 2f6389bf, and the body says to rebase this PR after it.
• The live witness: a real connect against AG2 Space with a fresh code, ending in a seated bridge that polls. It is named, not blocking.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Re-review at a8dd49e — agent-assisted review by Mars, Qingyun's Sutando seat. My approval at 56c5ce9 doesn't carry over to this head.

This PR's own commits are good (4d724cc → a8dd49e). test_claude_code_adapter, gateway-credential-sources and test_codex_local_connect pass under env -i.

• https is required, with plain http only on loopback. That resolves my earlier note about the device_code going out in cleartext.
• The code stays off argv (--code - or a prompt, and a warning on --code VALUE), and code/trust_relay_host are stripped before install.
• relay_problem runs before anything is written, so a refused relay URL never reaches disk.
• A 202 is retried honouring retry_after, clamped to 1–30 s with bool/NaN rejected, and a 404 tells agent_revoked apart from a used or expired code.

Changes requested only because of the stack. This branch contains #5859's 2f6389b (git merge-base --is-ancestor confirms it), which carries the core-launcher regression in my and Susan's #5859 reviews. If this PR merged first, that regression would land with it. Please rebase onto the fixed #5859 and I'll approve.

Non-blocking: _host_family takes the last two labels, so on a two-label public suffix (*.co.uk, *.com.au) any relay under the suffix would match. That doesn't matter for ag2.space, but it's worth a comment, or the PSL if a customer base ever lives under one.

[qingyun-wu]

Approve at 79912e4 — agent-assisted review by Mars, Qingyun's Sutando seat, on Qingyun's instruction to review and approve Chi's PRs. This replaces my changes-requested at a8dd49e.

• Stack: rebased onto 3f1808f (merge-base --is-ancestor confirms it), so it now carries the fixed #5859.
• My non-blocking note is addressed: _host_family is gone. _relay_host_ok accepts the relay only on the base host itself or a host under it (== base or endswith('.' + base), trailing dot stripped, lowercased), otherwise --trust-relay-host. That's stricter than a shared parent, and correct without a PSL. Tests cover the co.uk sibling, a host under a co.uk base, and the same host.
• Tests: test_claude_code_adapter, gateway-credential-sources, src/remote-gateway-bridge and test_codex_local_connect all pass at this head under env -i. The rest of the connect review stands as before.

Merge #5859 first. The CLA check hadn't posted on this head when I looked.

[sonichi]

@cla-assistant check

[liususan091219]

Approve at 79912e42. Qingyun approved at this head. I verified the one commit added since my comment at a8dd49e8. Before merge, the branch still needs a rebase onto main, and #5859 has to merge first.

What changed since a8dd49e8

• git range-diff shows the six connect commits unchanged across the rebase onto #5859's 3f1808fa. One commit is new: 79912e42 replaces _host_family with _relay_host_ok.
• The relay is now accepted only on the base's own host or a host under it. This resolves my non-blocking .co.uk note.

What I checked

• ConnectCode + RedeemEdges: 19 pass. gateway-credential-sources: 170 pass. test_codex_local_connect: 35 pass. test_adapter_boundaries: 10 pass. All run under env -i with a scratch HOME.
• Mutation 1. I put the old last-two-labels match back into _relay_host_ok (connect_common.py). The new relay test then fails 3 subtests: evil.co.uk, relay.example.co.uk under api.example.co.uk, and notexample.co.uk. With the mutation reverted, all tests pass.
• Probe of relay_problem against base https://chat.ag2.space/api:
  • Accepted: chat.ag2.space, CHAT.ag2.space. and x.chat.ag2.space.
  • Refused: evilchat.ag2.space, chat.ag2.space.evil.com, relay.ag2.space, http://chat.ag2.space, and the userinfo form chat.ag2.space@evil.com.
  • The real flow keeps working, because the bridge derives the provision API from the relay host itself.
• The stacked #5859 fix. My #5859 change request at 2f6389bf asked that only a seat read its token file private. 3f1808fa (contained here) does exactly that.
• Mutation 2. I dropped is_seat(...) from _read_token_file_text (remote_gateway_bridge.py). The new host-env test then fails for 0644 and symlink. With the mutation reverted, it passes.

Gates

• CI: 36 pass, 3 skipped.
• license/cla: success (1 context).
• No unresolved threads.

Still owed (named, not blocking this approval)

• Merge conflict with main. The branch is CONFLICTING. The conflict is in packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py, and #5859's head 3f1808fa hits the same one against main. The rebase will move the head, so approvals will need refreshing there.
• Merge order. #5859 merges first, then this PR. #5859 itself is still CHANGES_REQUESTED on GitHub, since my request there predates 3f1808fa.
• The live witness. A real connect against AG2 Space with a fresh code, ending in a seated bridge that polls.

Non-blocking

• A backend whose relay sits on a sibling host (say relay.ag2.space under api.ag2.space) now needs --trust-relay-host. That is the stated intent, but worth knowing if the hosts are ever split.

— Sutando (susans-MacBook-Pro)

[liususan091219]

Comment at 94ddf46b, not approving yet: no other approval exists at this head. Qingyun's approval and mine are both at 79912e42. The only change since then is 94ddf46b, a merge of main (56bd959b) into the branch, and I re-verified it below. The missing piece is a re-approval from another reviewer at this head; once that lands, this is ready to approve.

What changed since 79912e42

• One merge commit. git show --remerge-diff shows one conflict, in packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py at the package import block. The resolution is the union of both sides: main's setup_fetch import and this branch's read_private. Nothing else was hand-edited. Three other overlapping files (packages/ag2-sparrow/README.md, tests/test_hatch_served_setup.py, src/remote-gateway-bridge.test.py) auto-merged.
• read_private is still used by _read_token_file_text, so dropping it in the resolution would have been a NameError, not a silent change.

What I checked at 94ddf46b (env -i, scratch HOME)

• ConnectCode + RedeemEdges: 19 pass. test_codex_local_connect + test_adapter_boundaries + test_hatch_served_setup: 271 pass. test_task_lifecycle: 282 pass. gateway-credential-sources: 170 pass. src/remote-gateway-bridge.test.py: all green.
• Full test_claude_code_adapter: 24 failures, all LauncherTmux. The identical 24 fail at 79912e42 under the same env (no tmux on that PATH), so they are my instrument, not the merge.
• Mutation on the conflicted file. Dropping owner.is_seat(...) from _read_token_file_text makes test_a_host_reads_its_named_channel_env_as_before_and_only_a_seat_reads_it_private fail for 0644 and symlink. Reverted, all 170 pass.

Gates

• license/cla: success (1 context).
• CI: two red, one cause. python standalone tests (6/9) fails only in tests/skills/process-hygiene/reap-orphans.test.py ("the orphan was not a candidate"). That is the open flake #5733, and this diff does not touch process-hygiene. tsc + tests (clean install) fails only because it gathers that job's result. A re-run should clear it.
• No unresolved threads.

Still owed (named, not blocking)

• Merge order. #5859 is still open. Its head has moved to 158a257c (a main merge plus one test commit, 158a257c1, pinning the named token file over the channel .env and vault), and this branch does not contain it. After #5859 merges, this branch needs a rebase or merge onto main, and that moves the head again.
• The live witness. A real connect against AG2 Space with a fresh code, ending in a seated bridge that polls.

— Sutando (susans-MacBook-Pro)

[github-actions]

@cla-assistant check