Conversation
The relay reads only RELAY_URL (config.rs); BUZZ_RELAY_URL is the agent-side connection target used by buzz-acp, docs/remote-agents.md, and .env.example. Most relay-facing variables are BUZZ_-prefixed, so an operator setting BUZZ_RELAY_URL for the relay too is a reasonable mistake: relay_url then silently falls back to ws://localhost:3000, the deployment community is provisioned bound to that host, and every real request 404s with no signal beyond the startup log line for relay_url. Warn once at startup when this mismatch is detected, following the existing inert_env_vars pattern: a pure helper takes an injected env lookup, covered by four unit tests over the input space plus two tests that drive the real Config::from_env() path with captured tracing output (one per direction: warns, stays quiet). Also fixes the startup error at main.rs that names BUZZ_RELAY_URL when the value it prints (config.relay_url) actually comes from RELAY_URL, sending an operator debugging a failed boot to the wrong variable. Fixes block#6764 Signed-off-by: Yyunozor <yyunozor@icloud.com>
🔐 Codex Security Review
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The relay reads only
RELAY_URL(crates/buzz-relay/src/config.rs).BUZZ_RELAY_URLis the agent-side connection target — used bybuzz-acp,docs/remote-agents.md, and.env.example— and most relay variables areBUZZ_-prefixed, soBUZZ_RELAY_URLis a name an operator can reasonablyexpect the relay itself to read too. When it's set instead of
RELAY_URL,relay_urlsilently falls back tows://localhost:3000, the deploymentcommunity is provisioned bound to that host, and every real request 404s
with
no community is configured for this host— the only signal is astartup log line easy to miss among healthy-looking ones (as reported in
#6764).
This adds a startup warning for that specific mismatch:
BUZZ_RELAY_URLsetand
RELAY_URLnot set. It follows the existinginert_env_varspattern inthe same file — a pure helper takes an injected env lookup so the predicate
tests don't touch process env.
A second, related bug: when
BUZZ_REQUIRE_RELAY_MEMBERSHIP=trueand thehost can't be derived, the fatal error in
main.rsnamesBUZZ_RELAY_URL,but the value it prints (
config.relay_url) actually comes fromRELAY_URL. An operator debugging that crash is sent to the wrongvariable — the error now names
RELAY_URL, the variable whose value itprints. One-line fix, same file.
Does not add
BUZZ_RELAY_URLas an accepted alias (option 2 in the issue)— that changes runtime behavior and is left as a maintainer-directed
follow-up.
Known limitation: this covers only the process-env misconfiguration
reported in the issue. It does not cover a compose file or other setup
where both variables end up set to different hosts.
Related issue
Fixes #6764. Credit to @morven-ai for the diagnosis (traced it to
router.rs's tenant-host resolution) and for suggesting this exactwarning as the first of three options. They also offered to send a PR for it;
happy to close this one if they would rather send theirs.
Searched for a competing PR: none references #6764 (0 comments, 0
assignees, one unrelated cross-reference). Closest open PR: #7793 also adds
a log-capture helper to the
config.rstests; if it lands first, I'llrebase onto its helper.
Testing
All three pass clean (82
configtests, 0 failed, 2 ignored — Postgres-only).--test-threads=1is needed for a pre-existing, unrelated race: some testhelpers elsewhere in the crate call
Config::from_env()without holdingENV_MUTEX, so they can observe a concurrently-running writer testmid-mutation. Reproduced live on this branch (parallel run, same filter,
5 tries): 1 run failed with 7 unrelated assertion panics
(
api::admin,api::gifs,config::tests::valid_relay_owner_pubkey_...),4 runs passed clean. PR #6265 (open, unmerged) targets this same gap
crate-wide.
Two of the new tests drive the real
Config::from_env()startup path(not just the pure helper), with tracing captured the same way
config_with_admin_env_capturing_logsdoes. Note:config::testsis in noCI test selector today (the Justfile selects
nip_fi_config::tests::but notconfig::tests::), so these tests run locally only.Also ran the actual binary (
cargo build --release -p buzz-relay, noDocker services, no private key configured — so this exercises the config
warning itself, not the full boot):
The warning fires exactly once during config load, before the process
exits at key load (no key configured in this minimal run, so it never
reaches the
Config loadedline). WithRELAY_URLset instead, the samerun produces no such warning. A full end-to-end run against a live local
relay (
just bootstrap && just setup) is still owed as a humanconfirmation step before merge.