fix: rebind self-hosted relays when their address changes on reinstall (7.0.0:1) - #14
Conversation
…ges (7.0.0:1) `resolveLocalRelayUri` already reads the server's address as a `const`, but `computeStartEnv` only called it in hands-off mode. Managed mode applies relays from the `sync-settings` oneshot, which runs after main has returned — a `const` read there doesn't bind to the service, so nothing was watching the address. Reinstalling the SimpleX Server changes its URI. The client kept the old one and messages stopped arriving. computeStartEnv now resolves relays in both modes. Hands-off still uses the result for `SMP_SERVERS`/`XFTP_SERVERS`; managed discards it and applies relays over the WS as before — the point is that the read happens inside main's body, where the `const` binds and a changed address re-runs main.
Reinstalling a self-hosted SimpleX Server regenerates its CA. An address created before that stays pinned to the old identity, and changing the relay selection doesn't re-home it — it only decides where new queues are created — so every command touching the address fails with a BROKER/unknownCAError until the address itself is reset. The Configure action surfaced that as a JSON dump, which names neither the cause nor the fix. assertNotCmdError now detects the shape and reports the relay host plus a pointer to Reset SimpleX Address. Every other refusal keeps the existing raw-response message.
helix-nine
left a comment
There was a problem hiding this comment.
Nice find on the stale-relay symptom, and the unknownCAError message is a real improvement — a raw JSON dump for that failure was useless and the new text names both the cause and the fix. The types it destructures (errorAgent → agentError.BROKER → brokerErr.NETWORK → networkError.unknownCAError, plus brokerAddress) all check out against @simplex-chat/types 0.7, tsc --noEmit is clean, and prettier is clean.
One blocker, and two things I'd like your read on before this goes in.
Blocker: the migrations.other key silently disables future 7.x migrations
other: { '>=7.0.0:0 && <8.0.0:0': { down: async ({ effects }) => {} } },VersionGraph.graph() doesn't only add the edge to the synthesized range vertex — it also walks findVertex(v => isExver(v.metadata) && v.metadata.satisfies(range)) and adds the same edge to every concrete version vertex inside the range. As soon as a 7.1.0:0 exists in the graph (which it will: 7.0.0:1 gets spun off to a historical file carrying this other entry, and the new version becomes current), that produces a no-op 7.0.0:1 → 7.1.0:0 edge. It's inserted before 7.1.0:0's real up edge, and shortestPath returns the first path it completes — so the real migration never runs.
Ran it against start-sdk 2.0.9, building the graph both ways and migrating 7.0.0:1 → 7.1.0:0 with a 7.1.0:0 whose up sets a flag:
other key |
down → 7.0.0:0 | down → 0.3.0:5 | future 7.1.0:0 up ran? |
|---|---|---|---|
'>=7.0.0:0 && <8.0.0:0' (this PR) |
reachable | blocked | no |
'7.0.0:0' |
reachable | blocked | no |
'=7.0.0:0' |
reachable | blocked | yes |
'>=7.0.0:0 && <7.0.0:1' |
reachable | blocked | yes |
Note the second row: a bare version key is not an exact match — VersionRange.parse('7.0.0:0') returns ^7.0.0:0, i.e. the same >=7.0.0:0 && <8.0.0:0 span. The anchor form is what you want:
other: { '=7.0.0:0': { down: async ({ effects }) => {} } },Same downgrade to 7.0.0:0, 0.3.0 downgrades still blocked, and no poisoned edge for whatever 7.x lands next. Everything else about this part is right — down: IMPOSSIBLE plus a narrow other is the correct mechanism here.
The stated cause doesn't survive a trace — what did you actually observe?
The change rests on this:
Managed mode applies relays from the
sync-settingsoneshot, which runs after main has returned — aconstread there doesn't bind to the service, so nothing was watching the address.
I went looking for where that binding is dropped and couldn't find it:
RpcListener'sstartbuilds main's effects withcallbacks.child('main'); that child is only removed onstop, soisInContextstays true for the whole run.SystemForStartOs.start()setseffects.constRetry = once(() => { if (effects.isInContext) effects.restart() })on that same object, before callingmain.CommandControllerinvokes a oneshot'sexec.fn(subcontainer, signal)— it passes no effects — so thefninmain.tsuses the closed-overeffectsfromsetupMain, andconfigureServers(effects, settings)hands it straight toresolveServerUrisbeforewithBotSessionis reached.Watchable.const()keys entirely offthis.effects.constRetryandisInContext, both of which are live at that point.
So by that reading the .const() in the oneshot was already registered against main's context and a changed address should already have triggered effects.restart(). Which means either I'm missing a layer, or the real cause is something else and this change fixes the symptom for a different reason.
There is one difference the move genuinely makes: in the oneshot the read sits behind await waitForBotReady(effects), inside a try whose catch only warns. If the WebSocket never answers, configureServers is never reached and no watch is registered at all. Hoisting it into main's body makes registration unconditional. That's a real win, just a narrower one than the comment claims.
Could you say what you saw — e.g. did the container restart at all when the server's address changed, and did the logs show .const() triggered? Worth pinning down, because the block comment asserting the mechanism will get copied into the next package that needs this.
The watch has no map, and this widens what restarts the bridge
resolveLocalRelayUri calls sdk.host.get(effects, { packageId, hostId }).const() with no selector, so it re-runs main on any change to SimpleX Server's whole host record — a LAN IP change, an added Tor address, a domain edit, not just the relay URI. Service-to-Service Networking § 2 is explicit that .const() belongs on a minimal mapped value for exactly this reason.
That's pre-existing, but it only affected hands-off mode, which is the minority path. This PR extends it to managed mode, which is the default — so the blast radius grows with the change. Since you're touching this code anyway, passing a map narrowing to the one interface's addressInfo would keep the restart tied to the thing you actually care about:
sdk.host.get(effects, { packageId, hostId }, host => /* just this interface's addressInfo */).const()Smaller notes
- The
catchincomputeStartEnvpreserves the prior failure semantics exactly — hands-off rethrows, managed swallows and retries in the oneshot. No regression, good. - Bumping to
7.0.0:1rather than re-releasing in place is right:7.0.0:0is published to community-prod, so it's permanent. - Comment budget: the 7-line block in
computeStartEnvand the 9-line JSDoc on a 7-linestaleRelayHostare both well over what the packaging guide asks for, and the block is where the unverified mechanism lives. One line stating the invariant, with the reasoning in the commit message, would serve better. - The error string is a single ~60-word sentence. Splitting the cause from the remedy would read better in a toast.
- CI green is on
9a36fa8only. The run on headffe6524came backskipped/0s, so theliveSync.tschange hasn't been built. Worth a re-run before merge.
Happy to take another pass once the other key is fixed.
Reinstalling the SimpleX Server regenerates its keys, and the CA fingerprint is part of the relay URI, so its address changes. The bridge kept the old one and messages stopped arriving, with no restart and nothing in the logs to say why. `resolveLocalRelayUri` read the address with `.const()` but passed no `map`, so the watch compared whole host records using the default deepEqual. Filled address objects carry enumerable lazy `nonLocal`/`public`/`bridge` getters that each return another object with the same getters, so the comparison recurses until the stack overflows. `watchGen` only invokes `eq` from the second value onward, and `const()`'s rejection path aborts the watch without calling `constRetry` — so the first address change killed the watcher silently: no restart, no log, no error surface. The URI is now resolved inside `map`, making the watched value a string. That avoids the comparison entirely and narrows reactivity to the address itself rather than every field of the dependency's host record. `computeStartEnv` resolves relays in both modes now and returns them alongside the env. Managed mode's only previous read was in the sync-settings oneshot, behind a wait for the bot socket inside a catch that only warns — so a socket that never answered left nothing watching the address, and a later change went unnoticed. Resolving in main's body registers the watch unconditionally. `configureServers` takes the resolved URIs rather than resolving them itself, so the oneshot and the Configure action both reuse a value their caller owns instead of reading the dependency's address a second time. Finally, a relay URI is accepted only once its userinfo looks like a CA fingerprint. The server publishes its binding before that fingerprint exists: mid-reinstall the address formats as `smp://undefined:null@host:5223`, which hands-off mode would have written into the container env as a relay that cannot work. Treating it as not-ready-yet also drops a wasted restart, since the malformed value no longer registers as a change. Verified on StartOS 0.4.0 with relays set to Local. Reinstalling the SimpleX Server now restarts the bridge twice — once when the dependency goes away, once when its new address resolves — and the profile comes back up on the new fingerprint with no manual step. Previously `/_servers` held the old address until the bridge was restarted by hand.
|
Thanks — all three points landed, and the third one turned out to be the whole bug rather than a nicety. Summary of where each stands. The
|
…retry 7.0.0:0 is released and versionGraph carries only `current`, so replacing its notes leaves anyone upgrading from 0.3.0 with no mention of the SimpleX Chat 7.0.0 jump or the in-place database upgrade. One entry now serves both the 0.3.0 and the 7.0.0:0 reader. The sync-settings oneshot no longer re-resolves the relays when the start-time resolve failed. That resolve registers the address watch before it throws, so main re-runs on its own once the dependency answers; the retry only added a second watch on the same host. Comment blocks trimmed to the facts that aren't recoverable from the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7d8cbc5 to
a33d83b
Compare
helix-nine
left a comment
There was a problem hiding this comment.
All three items from the last pass are addressed, and the mechanism question is now settled — including the part my previous trace got wrong. Approving; I've pushed the remaining small cleanups onto the branch rather than sending them back.
The cause holds up, and it's not where I looked
Last time I traced main's effects context and found it stays live through the oneshot, so I couldn't see how the .const() there failed to bind. That trace was right and the conclusion was wrong: the watch doesn't die from losing its context, it dies inside eq.
sdk.host.get(...) with no map leaves the default eq, which is deepEqual over the whole FilledHost. I ran the SDK's own deepEqual against a fillHost() result:
same-value THREW: RangeError Maximum call stack size exceeded
changed THREW: RangeError Maximum call stack size exceeded
self THREW: RangeError Maximum call stack size exceeded
filledAddress returns enumerable nonLocal/public/bridge getters, each returning another filled address carrying the same getters, and deepEqual has no identity short-circuit — so it recurses forever on any pair, including an object against itself. Watchable.watchGen only calls eq from the second value onward, which is why const() resolves normally and the failure waits for the first change. At that point gen.next() rejects into const()'s error handler, which does abort(); cleanup() and never calls constRetry. No restart, no log, and the rejection is handled so nothing surfaces.
That is exactly the symptom you reported, and resolving inside map is the right fix — getHost's own docs point at map for narrowing reactivity, and a string compares fine. It also means the managed-mode hoist into computeStartEnv is doing real work independent of the watch bug: registration is now unconditional rather than sitting behind waitForBotReady in a warn-only catch.
Worth knowing this isn't yours to carry: the deepEqual recursion is live on start-technologies master today, so every unmapped sdk.host.get(...).const() has a dead watch. swatcher and i2pd-startos both have it latent. I'm filing that upstream separately.
Same for RELAY_URI: the smp://undefined:null@host:5223 you're matching against comes from simplex-startos interpolating an unset fingerprint and a null password straight into username. Your guard is still worth having — a fingerprint-less relay URI is unusable however it's produced — but the source of the malformed value is our package and I'll fix it there.
Cleanups pushed to the branch
- Release notes.
7.0.0:0is published andversionGraphcarries onlycurrent, so overwriting its notes left anyone still on0.3.0:5with no mention of the SimpleX Chat 7.0.0 jump, the renumbering, or the in-place database upgrade. Rewritten as one entry that serves both readers, with your relay fix leading. - Dropped the oneshot's re-resolve.
servers ?? (await resolveServerUris(...))was redundant: the start-time resolve registers the address watch before it throws, so main re-runs on its own once the dependency answers. The retry only added a second watch on the same host. Nowif (servers) await configureServers(effects, servers). - Trimmed the three oversized comment blocks to the fact that isn't recoverable from the code, and split the cause from the remedy in the
unknownCAErrorstring.
=7.0.0:0 is correct, tsc --noEmit and prettier are clean on head.
One thing still unverified
Your note says verified with relays set to Local, but not which profile mode. Managed mode swallows a failed resolve; hands-off is the path where the new RELAY_URI guard makes main throw outright mid-reinstall. If you happen to have that configuration around, confirming it recovers would close the loop — not blocking, since hands-off was already throwing on an unresolvable relay before this PR.
Reinstalling the SimpleX Server regenerates its keys, and the CA fingerprint is part of the relay URI, so its address changes. The bridge kept the old one and messages stopped arriving, with no restart and nothing in the logs to say why.
resolveLocalRelayUriread the address with.const()but passed nomap, so the watch compared whole host records using the default deepEqual. Filled address objects carry enumerable lazynonLocal/public/bridgegetters that each return another object with the same getters, so the comparison recurses until the stack overflows.watchGenonly invokeseqfrom the second value onward, andconst()'s rejection path aborts the watch without callingconstRetry— so the first address change killed the watcher silently: no restart, no log, no error surface. That is why the read looked correct and the service simply never reacted.The URI is now resolved inside
map, making the watched value a string. That avoids the comparison entirely and narrows reactivity to the address itself rather than every field of the dependency's host record.computeStartEnvresolves relays in both modes now and returns them alongside the env. Managed mode's only previous read was in thesync-settingsoneshot, behind a wait for the bot socket inside a catch that only warns — so a socket that never answered left nothing watching the address, and a later change went unnoticed. Resolving in main's body registers the watch unconditionally.configureServerstakes the resolved URIs rather than resolving them itself, so the oneshot and the Configure action reuse a value their caller owns instead of reading the dependency's address a second time.A relay URI is also accepted only once its userinfo looks like a CA fingerprint. The server publishes its binding before that fingerprint exists: mid-reinstall the address formats as
smp://undefined:null@host:5223, which hands-off mode would have written into the container env as a relay that cannot work. Treating it as not-ready-yet also drops a wasted restart, since the malformed value no longer registers as a change.A second fix rides along, found while testing the first. Switching relays back to the presets on a bridge whose address was created against a since-reinstalled SimpleX Server would fail with a
BROKER/unknownCAError, reported as an 800-character JSON dump. Changing the relay selection does not re-home an address that already exists, so the queue stays pinned to the old server identity.assertNotCmdErrornow detects that shape and reports the relay host plus the remedy — Reset SimpleX Address — while every other refusal keeps the existing raw response.Verified on StartOS 0.4.0 with relays set to Local. A full uninstall/reinstall of the SimpleX Server now restarts the bridge twice — once when the dependency goes away, once when its new address resolves — and the profile comes back up on the new fingerprint with no manual step. Previously
/_serversheld the old address until the bridge was restarted by hand.