fix(desktop): dial the requested relay URL when spawning an agent harness

[ahmetkca]

Fixes #3033

Summary

spawn_agent_child hands the managed-agent harness a canonicalized identity key as the address to dial, so an agent connects to a host the operator never configured. On a relay using the shipped default RELAY_URL, the harness is rejected at row-zero host binding and agents never start.

The fix passes the relay URL as requested, in spawn_agent_child and its three call sites, in the relay access probe, and in reconciliation's start_pair call.

Note on history. This PR originally fixed the same bug in buzz-core::tenant::normalize_host by folding loopback host spellings. Review showed that approach conflicts with a deliberate, documented, tested property of the relay, so it was reverted in favour of fixing the caller. See "Why not fix this in the relay" below. The first two commits are that approach and its revert; the net change is the last two commits.

Impact

With the shipped .env.example (RELAY_URL=ws://localhost:3000), the relay seeds communities.host = localhost:3000. That is the documented behavior, per the existing test relay_url_authority_keeps_explicit_nondefault_port, whose comment reads "The default dev seed: startup, bind_deployment_community, and buzz-admin must all derive localhost:3000".

The desktop app connects to that community without trouble, because it dials the URL the operator entered. The agent harness does not. ManagedAgentRuntimeKey::new canonicalizes the relay URL when building the runtime key, and spawn_agent_child passed that canonical value to the child as BUZZ_RELAY_URL. The harness therefore dialed ws://127.0.0.1:3000, which maps to no community, and binding rejected it:

WARN buzz_acp::relay: initial relay connect failed with terminal error:
     WebSocket error: HTTP error: 404 Not Found
Error: relay connect error: WebSocket error: HTTP error: 404 Not Found

The 404 is deliberately generic so callers cannot probe which hosts are mapped. That is correct for an unmapped host, but it means the failure carries no diagnostic signal, and the desktop app surfaces no error at all: the agent simply never starts.

There is no community URL that satisfies both surfaces:

Community URL entered	Desktop socket	Agent harness (canonicalized)
ws://localhost:3000	connects	dials 127.0.0.1:3000, receives 404
ws://127.0.0.1:3000	receives 404	would connect

The only workaround today is to move RELAY_URL away from the shipped default.

Reproduction

Verified against a relay started from the shipped compose configuration with RELAY_URL=ws://localhost:3021, on a pristine database. The relay seeded communities.host = localhost:3021.

Pointing the real harness at that relay, varying only the spelling of the loopback host:

$ buzz-acp --relay-url ws://localhost:3021 --agent-command claude-agent-acp
WARN buzz_acp::relay: initial relay connect failed with terminal error:
     Auth failed: restricted: not a relay member

$ buzz-acp --relay-url ws://127.0.0.1:3021 --agent-command claude-agent-acp
WARN buzz_acp::relay: initial relay connect failed with terminal error:
     WebSocket error: HTTP error: 404 Not Found

The localhost run completes the WebSocket upgrade and reaches NIP-42 authentication, failing there for an unrelated and expected reason (that key was not a member of the throwaway relay). The 127.0.0.1 run never reaches authentication: it is rejected at host binding, before auth exists. buzz-acp itself does no canonicalization; its raw upgrade request carries Host: localhost:3021 with the port intact.

Root cause

ManagedAgentRuntimeKey is documented as "Canonical identity of one managed-agent harness on one relay", and runtime_id() hashes relay_url into a stable path suffix. Canonicalizing there is correct: it keeps one pair's on-disk identity stable no matter how the operator spelled the host.

The bug is that the same value was then used as a network address. An identity key only has to be stable. A dial address has to be correct against a server that keys tenants on the literal Host header.

All three callers passed &key.relay_url:

• runtime.rs (ensure_pair_runtime)
• runtime_commands.rs (start_pair)
• restore.rs (startup restore)

Each already had the requested URL in scope, so no signature changes were needed.

The fix

spawn_agent_child now dials the relay_url it was passed, and each caller passes the requested URL instead of the canonical key. The key is still built by canonicalizing that same input, so runtime_id() and every existing runtime directory are byte-identical to before.

The pin described by the existing call-site comment is preserved: the child may still connect to exactly one relay, chosen explicitly by the caller. It is now the relay the operator actually configured.

Two further sites needed the same treatment, one layer above spawn_agent_child:

• probe_agent_relay_access derived its HTTP base from key.relay_url; it now uses the requested_relay_url parameter it already had.
• Reconciliation's successful-probe branch called start_pair with key.relay_url while requested was in scope and already used for the status row; it now passes requested.

The spawn config fingerprint deliberately keeps the canonical URL. needs_restart recomputes spawn_config_hash from key.relay_url, so feeding the requested spelling on the spawn side would make the two disagree for any folded host and report needs_restart permanently. Dial the request, fingerprint the identity. This matches upstream for the fingerprint; only the dial changes.

Why not fix this in the relay

The first version of this PR made tenant::normalize_host fold loopback spellings so that localhost and 127.0.0.1 resolved to one community. That is reverted, for two reasons found in review.

It contradicts a deliberate property. crates/buzz-auth/src/nip98.rs documents, under "No loopback aliasing", that localhost, ::1 and 127.0.0.1 are distinct hosts and that collapsing them would let an event signed for one pass against another. loopback_aliases_are_distinct_hosts enforces it. Since nip98_expected_url builds the expected u from tenant.host(), folding made the relay expect 127.0.0.1 while clients such as buzz-cli (default http://localhost:3000) sign localhost, breaking authenticated bridge calls: /query, /events, /count, invites, and git.

It carried far more risk than the defect warranted. Changing the community lookup key required a migration rewriting communities.host, because lower('localhost:3000') does not conflict with lower('127.0.0.1:3000') and startup would otherwise insert a second community, leaving existing channels, members, and events attached to the old id. That exposes every existing deployment to a data-shaped upgrade risk to fix a defect that only affects launching agents.

Fixing the caller touches no tenancy code, needs no migration, cannot affect a deployment that never launches an agent, and leaves the documented no-aliasing property intact.

Testing

New test runtime_key_relay_url_is_canonical_identity_not_a_dial_address in runtime_types.rs pins the fact that makes this bug possible: every loopback spelling of a relay URL canonicalizes to ws://127.0.0.1:3000 in the key, and all spellings share one runtime_id(). It documents in place why that field must not be used as a dial address.

Gate	Result
cargo fmt (workspace and Tauri crate)	clean
cargo clippy --manifest-path desktop/src-tauri/Cargo.toml --all-targets -- -D warnings	clean
cargo test --manifest-path desktop/src-tauri/Cargo.toml	1637 passed, 0 failed, 14 ignored
cargo test -p buzz-core -p buzz-conformance	all passed
cargo test -p buzz-db --lib	83 passed, 1 pre-existing failure

The buzz-db failure is replica_fence::tests::fence_starts_closed_and_opens_on_advance. This branch does not touch crates/buzz-db (git diff upstream/main -- crates/buzz-db is empty), and the same test fails in a clean upstream/main worktree in this environment, so it is unrelated to this change.

Scope and limitations

Three restart paths are deliberately not fixed here. Auditing every use of a canonical key relay_url in the desktop crate found three more places that reach a dial or probe:

• desktop/src-tauri/src/commands/global_agent_config.rs:348 (restart after a global config change)
• desktop/src-tauri/src/commands/agent_discovery.rs:501 (restart after installing an ACP runtime)
• desktop/src/features/settings/ui/ActiveAgentCommunitiesSettingsCard.tsx:39 (Settings Start/Restart, reaching startManagedAgentRuntime in managedAgentRuntimeHooks.ts:193)

The first two dial through commands/agents.rs:352. In all three the requested URL is genuinely unavailable: these flows begin by stopping the running pairs and working from the returned ManagedAgentRuntimeKey values, so no requested URL exists in the call chain, and it cannot be recovered from the key because localhost, 127.0.0.1 and [::1] normalize to one key. Fixing them requires the requested URL to be retained at pair creation or rejoined against the configured community list, which is a design decision left to the maintainers.

So: agents started normally, by reconciliation, or restored at launch now connect correctly. Agents restarted through those three flows still will not, on a localhost-configured deployment.

A related pre-existing issue, not touched. Because spawn_config_hash receives the canonical URL on both the spawn and the needs_restart side, switching a workspace between localhost and 127.0.0.1 does not change the hash, so a child can keep running against the previous community. upstream/main already hashes canonical on both sides, so this predates this change. Noted so the canonical fingerprint does not read as an oversight; it shares a root cause with the three paths above.

Other notes

• Reproduced using the shipped .env.example relay configuration rather than by invoking just dev directly, because port 3000 was occupied in my environment. The relay configuration under test is the one just dev uses.
• Integration tests requiring Postgres and Redis were not run.
• The pre-existing buzz-db failure noted above was not investigated beyond confirming it reproduces without this change.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2e65d956c

ℹ️ 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".

[ahmetkca]

Both findings were valid. Fixed in the amended commit.

P1, migrate existing loopback rows. Confirmed and addressed. ensure_configured_community uses ON CONFLICT (lower(host)) DO UPDATE against idx_communities_host, and lower('localhost:3000') does not conflict with lower('127.0.0.1:3000'), so a second community would indeed be inserted with a fresh UUID while the existing data stayed on the old id.

Added migrations/0025_fold_loopback_community_hosts.sql, which rewrites existing loopback hosts to the canonical key. It fails loudly rather than guessing when two rows would fold onto the same key, naming both hosts in the error, on the grounds that silently choosing a winner would strand one community's data.

Exercised against Postgres 17 in three scenarios: the happy path (localhost:3000, [::1]:9000, and 127.5.5.5 all fold, a non-loopback host is left alone), a collision (aborts with both hosts named, rows untouched), and idempotence (canonical hosts are a no-op, and applying it twice is safe).

I took the migration route over a compatibility lookup because a fallback keeps two live keys for one deployment indefinitely, which is the ambiguity this change exists to remove.

P2, reject malformed bracketed hosts before folding. Also confirmed. Compiling the helper in isolation reproduced it exactly:

[::1]            -> ("::1", None)
[::1]:3000       -> ("::1", Some("3000"))
[::1]evil        -> ("::1", None)      <- should not parse
[::1]xyz:3000    -> ("::1", None)      <- should not parse

split_host_port now returns None for an unterminated bracket, trailing junk after the closing bracket, or an empty or non-numeric port, so those inputs pass through untouched and fail closed at resolution instead of folding onto the loopback community. Covered by normalize_host_does_not_fold_malformed_authority, which pins [::1]evil, [::1]xyz:3000, [::1, [::1]:, [::1]:port, localhost:abc, and 127.0.0.1:.

Gates after the changes: cargo fmt clean, clippy -D warnings clean, buzz-core 233 passed, buzz-conformance 6 passed, and buzz-relay --lib still shows no new failures against a main baseline.

[ahmetkca]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91bd81cfa3

ℹ️ 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".

[ahmetkca]

Changed approach based on the review. Both new findings were valid, and the first one showed the original fix was in the wrong place.

On "Keep NIP-98 URLs aligned with loopback folding": confirmed, and it is decisive. crates/buzz-auth/src/nip98.rs documents "No loopback aliasing" and loopback_aliases_are_distinct_hosts enforces it, because NIP-98 clients sign the exact URL they call. Since nip98_expected_url builds the expected u from tenant.host(), folding loopback in normalize_host made the relay expect 127.0.0.1 while buzz-cli (default http://localhost:3000) signs localhost. That would have broken /query, /events, /count, invites, and git for anyone on the default relay URL.

I could have extended the fold into verify_nip98_event, but that means deleting a security test written on purpose. The right conclusion is that the relay's strictness is not the bug.

On "Update the embedded migration manifest test": also confirmed. crates/buzz-db/src/migration.rs:563 pins assert_eq!(migrations.len(), 24). It is moot now, since the migration is gone, but it did reveal that I had not run cargo test -p buzz-db before claiming the gates were green. Fixed in this round: buzz-db is included below.

What changed. The relay-side fold and migration 0025 are fully reverted, and the fix now lives where the defect actually is. ManagedAgentRuntimeKey is an identity, canonicalized so runtime_id() stays stable, and spawn_agent_child was reusing it as a dial address. It now dials the URL as requested, and the three callers pass the requested URL rather than &key.relay_url. All three already had it in scope, so no signatures changed and runtime_id() is byte-identical to before.

Net effect versus the original approach: no tenancy code touched, no migration, no auth impact, and no effect at all on a deployment that never launches an agent. Diff is 4 desktop files, +55/-6.

The branch keeps the history rather than rewriting it, so the abandoned approach and its revert are both visible: 91bd81cf (approach A), 2cd48c50 (revert, with reasoning), 21d4a02f (the fix).

Gates: fmt clean; clippy -D warnings clean on the Tauri crate; desktop tests 1637 passed / 0 failed; buzz-core and buzz-conformance pass. buzz-db has one failure, replica_fence::tests::fence_starts_closed_and_opens_on_advance, which reproduces on a clean upstream/main worktree and which this branch does not touch (git diff upstream/main -- crates/buzz-db is empty).

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[ahmetkca]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21d4a02f96

ℹ️ 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".

[ahmetkca]

Pushed a3e4923a, which addresses the reconciliation finding. I also want to be explicit about what this PR does and does not cover, because I found more affected paths than the review flagged and I would rather declare them than quietly half-fix them.

What the last commit changes

Two things the previous commit missed:

• probe_agent_relay_access built its HTTP base from key.relay_url. It now uses requested_relay_url, which was already a parameter.
• The successful-probe branch called start_pair with key.relay_url, even though requested was in scope and already used two lines later for the status row. It now passes requested.

It also restores the spawn config fingerprint to the canonical pair URL. My earlier commit redefined effective_relay_url to the requested spelling, and that binding was doing two unrelated jobs: the address to dial, and the input to spawn_config_hash. Since needs_restart recomputes that hash from key.relay_url, the two sides disagreed for any host the key folds and would have reported needs_restart permanently. Dial the request, fingerprint the identity. The fingerprint now matches upstream behaviour again and only the dial differs.

What this PR does not fix

Auditing every use of a canonical key relay_url in the desktop crate turned up three more places that reach a dial or probe, and I have deliberately left all three alone:

• desktop/src-tauri/src/commands/global_agent_config.rs:348 (restart after a global config change)
• desktop/src-tauri/src/commands/agent_discovery.rs:501 (restart after installing an ACP runtime)
• desktop/src/features/settings/ui/ActiveAgentCommunitiesSettingsCard.tsx:39 (Settings Start/Restart, which reaches startManagedAgentRuntime in managedAgentRuntimeHooks.ts:193)

The first two both dial through commands/agents.rs:352.

These are not an oversight. In each case the requested URL is genuinely unavailable: those flows begin by stopping the currently running pairs and working from the returned ManagedAgentRuntimeKey values, so no requested URL exists anywhere in the call chain. It also cannot be recovered from the key, because localhost, 127.0.0.1 and [::1] all normalize to one key.

Fixing them needs the requested URL either retained when a pair is created, or rejoined against the configured community list. That is a design decision about where that value should live, and it did not feel like mine to make in a first contribution. Happy to follow up in this PR or a separate one, whichever you prefer, and happy to take direction on which approach you want.

So concretely: agents started normally, by reconciliation, or restored at launch now connect correctly. Agents restarted through those three flows still will not, on a deployment configured with localhost.

One pre-existing issue, for context

While reviewing the fingerprint change I noticed that because spawn_config_hash takes the canonical URL on both sides, switching a workspace between localhost and 127.0.0.1 does not change the hash, so needs_restart stays false and a child can keep running against the previous community. I checked upstream/main and both sides already hash canonical there, so this predates my change and I have not touched it. Mentioning it only so the choice of canonical in the fingerprint does not look like an oversight. It has the same root cause as the three paths above.

Verification

• cargo fmt clean
• cargo clippy --manifest-path desktop/src-tauri/Cargo.toml --all-targets -- -D warnings clean
• cargo test --manifest-path desktop/src-tauri/Cargo.toml: 1637 passed, 0 failed, 14 ignored
• cargo test -p buzz-core -p buzz-conformance: all passed
• cargo test -p buzz-db --lib: 83 passed, 1 failure in replica_fence::tests::fence_starts_closed_and_opens_on_advance. This branch does not touch crates/buzz-db and that test also fails in a clean upstream/main worktree here, so it looks unrelated and environment specific. I did not investigate further.

Integration tests needing Postgres and Redis were not run.

[dophsquare]

Review — fix(desktop): dial the requested relay URL when spawning an agent harness

Verdict: correct and merge-ready on review. ✅ CI green, MERGEABLE. Reviewed at commit a3e4923.

The dial-vs-fingerprint split is coherent and consistently applied:

• spawn_agent_child(relay_url: &str, …) now dials relay_url.to_string() (the requested spelling) but re-derives runtime_key = ManagedAgentRuntimeKey::new(pubkey, relay_url) internally for keying/log-path, and hashes runtime_key.relay_url (canonical) for spawn_config_hash. So the child connects to the literal Host while the identity/needs-restart fingerprint stays stable.
• Verified every call site passes the requested URL, not key.relay_url: start_pair (287), start_managed_agent_process, restore.rs (bound from effective_agent_relay_url, 289 → spawn 320), reconcile_managed_agent_runtimes (passes requested.clone()), and probe_agent_relay_access (probes &requested_relay_url, 411). No stragglers.
• The bug is real: the relay resolves a community from the literal request Host and fail-closes with a 404, so a ws://localhost:PORT deployment rejected canonicalized ws://127.0.0.1:PORT. The fix addresses it at all spawn + probe paths.
• New unit test asserts the canonicalization-is-identity invariant across localhost / 127.0.0.1 / [::1].

Action items

• Review approval — no code changes required.

---

Independent review by Fizz 🐝 — read the diff and traced call paths against the checked-out branch.

[edumagiceco]

Independent repro of this bug in local self-hosted dev, plus a data point on the spawn_config_hash trap this PR calls out.

To be clear about what I tested: I did not run this branch. I hit the bug independently, root-caused it to the same place, and applied a narrower local change on main (only spawn_agent_child, reading the workspace relay via relay_ws_url_with_override instead of runtime_key.relay_url). The measurements below are from that variant, so they confirm the diagnosis and the fix direction — not this specific diff.

Repro

just dev with the desktop on ws://localhost:3000. Every managed-agent harness spawned with ws://127.0.0.1:3000, so discover_channels queried a community that had no channels:

buzz-acp starting: relay=ws://127.0.0.1:3000 ... respond_to=owner-only
INFO  buzz_acp: discovered 0 channel(s)
WARN  buzz_acp: no channel subscriptions resolved — agent will sit idle

@agent mentions never produced a reply. The relay had grown a community per host spelling, confirming the split:

community_id	host	contents
7f15eb93…	localhost:3000	all channels + the desktop
3093c812…	127.0.0.1:3000	empty — what the harness queried

Membership itself was fine: the latest kind-39002 event for #general carried the agent pubkey in a p tag the whole time.

After the change

Same relay, same agent, harness restarted:

buzz-acp starting: relay=ws://localhost:3000 ... respond_to=owner-only
INFO  buzz_acp: discovered 2 channel(s)

No idle warning, and an @agent mention got a reply — 60s on one run, 105s on another.

The spawn_config_hash note is not hypothetical

Worth underlining for reviewers, since it is easy to miss when writing a minimal version of this fix. My first attempt changed only the dialed URL and left spawn_config_hash fed from effective_relay_url. build_managed_agent_summary recomputes that hash from key.relay_url, so the two disagree for every loopback host and needs_restart would have been stuck on permanently. Reading this PR is what caught it — after switching the hash input to the canonical runtime_key.relay_url, the badge stayed clear.

So "dial the request, fingerprint the identity" holds up: the two concerns really are separate, and a fix that only moves the dialed URL trades one bug for another.

Not covered by my variant

probe_agent_relay_access and reconcile_managed_agent_runtimes still used the canonical URL in my version — this PR handling both is the more complete fix.

Environment: macOS 26.5.2 (arm64), local relay from just dev, Postgres/Redis/MinIO via the repo's compose stack.

Related issues: #3505, #4147. Also overlaps #3484.