fix(desktop): connect managed agents to the configured relay, not the normalized loopback identity - #4664
fix(desktop): connect managed agents to the configured relay, not the normalized loopback identity#4664spkaplan wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c071752d1c
ℹ️ 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".
| // joined on `localhost`, leaving it "online" but discovering 0 channels. | ||
| // `normalize_relay_url`'s own contract says connection code must retain the | ||
| // configured URL; this honors that. | ||
| let effective_relay_url = relay_url.to_string(); |
There was a problem hiding this comment.
Keep restart hashes on a consistent relay URL
For relay URLs whose configured form differs from the normalized runtime key (the fixed localhost case, [::1], default ports, etc.), this raw assignment also changes the value stamped into process.spawn_config_hash; later build_managed_agent_summary recomputes the hash with key.relay_url (the normalized value), so a freshly started agent is reported as needs_restart immediately even though no config changed. Split the connection URL from the hash input, or make the summary recompute with the same configured URL.
Useful? React with 👍 / 👎.
…start badge Buzz's relay is multi-tenant, keyed by the literal request Host, so localhost:3000 and 127.0.0.1:3000 are distinct communities with isolated channels/members. buzz-core::relay::normalize_relay_url folds all loopback spellings to 127.0.0.1 for runtime identity/dedup ONLY (its doc says connection code may retain the configured URL). But spawn_agent_child used that normalized URL as the child's actual BUZZ_RELAY_URL, so UI-managed agents connected to the empty 127.0.0.1 tenant — "online" but discovering 0 channels — while the human and their channels lived on localhost. Connect the child on the CONFIGURED relay at all spawn sites; keep the normalized form only as the dedup identity / key / receipts / logs. Corollary badge fix: needs_restart compares a SpawnConfigSnapshot stamped at spawn against one recomputed live, and the recompute reads the relay from the normalized runtime-map key. Stamping the snapshot with the now-configured effective_relay_url (localhost) while the recompute used the normalized key (127.0.0.1) made them mismatch forever — a permanent, un-clearable "Restart required" badge. Stamp SpawnConfigInputs.relay_url with the normalized runtime_key.relay_url so both sides compare the same spelling. cargo check --no-default-features clean; desktop file-size ratchet respected. Signed-off-by: Sam Kaplan <samuel.kaplan@salesforce.com>
c071752 to
8503e1f
Compare
|
🤖 Thank you for working on this. We reviewed the overlapping proposals and are consolidating the configured-vs-canonical relay URL fix in #4859 so review and implementation can happen in one place. We’re marking this PR as superseded and asking a maintainer or the author to close it. Please review #4859 and contribute any feedback, tests, or implementation details you think should be carried into the consolidated fix. |
|
salman asked us to consolidate on #4859. The restart-badge point - snapshot stays on the canonical spelling, connection does not - is in the follow-up. Thank you for pinning that so the badge cannot stick forever once the child dials |
Summary
Two coupled defects in Desktop-managed agents on loopback-addressed local communities:
localhost(the desktop default and the seeded local-dev community). They only worked when the community was addressed as127.0.0.1.Root cause (shared). The relay is multi-tenant, keyed by the literal request
Host, solocalhost:3000and127.0.0.1:3000are distinct communities with isolated channels/members.buzz_core::relay::normalize_relay_urlintentionally folds all loopback spellings to127.0.0.1for the runtime identity key only — its own doc says:But the spawn path reused that normalized
ManagedAgentRuntimeKey::relay_urlas the child's actualBUZZ_RELAY_URL, so an agent for a community joined onlocalhost:3000connected to the empty127.0.0.1:3000tenant:subscriberesolved 0 channels and the agent never woke.Fix
In
desktop/src-tauri/src/managed_agents/(runtime.rs,runtime_commands.rs,restore.rs):ManagedAgentRuntimeKeysolely as the dedup identity (receipts, pid files, logs). Honorsnormalize_relay_url's documented contract; no identity/dedup/receipts change.needs_restartcompares aSpawnConfigSnapshotstamped at spawn against one recomputed live (prospective_spawn_config_snapshot), and the recompute reads the relay from the normalized runtime-map key. Once the connection fix flips the spawn'seffective_relay_urlto the configured spelling, stamping the snapshot with it (localhost) while the recompute uses the normalized key (127.0.0.1) makes them mismatch forever. StampSpawnConfigInputs.relay_urlwith the normalizedruntime_key.relay_urlso both sides compare the same spelling.Validation
Reproduced and verified end-to-end on a local self-hosted stack (relay on
localhost:3000):relay=ws://127.0.0.1:3000 -> "discovered 0 channel(s)" -> "no channel subscriptions resolved -- agent will sit idle".relay=ws://localhost:3000 -> "discovered 1 channel(s)" -> "subscribed to channel <spike>", and two managed agents (a Codex harness and a cursor-agent harness) each replied to an owner@mentionin-channel. Restart badge clears on restart and stays clear.main; all pre-push hooks pass (branch-skew,desktop-checkincl. file-size ratchet,rust-tests,desktop-test,mobile-test,desktop-tauri-checks= clippy + Tauri crate tests).Test plan
localhost:<port>, attach it to a channel,@mentionit as owner — it should discover the channel and reply.127.0.0.1) identity (no dedup regression across loopback spellings).