Skip to content

fix: rebind self-hosted relays when their address changes on reinstall (7.0.0:1) - #14

Merged
MattDHill merged 4 commits into
Start9-Community:masterfrom
lundog:fix-server-rebinding
Aug 28, 2026
Merged

fix: rebind self-hosted relays when their address changes on reinstall (7.0.0:1)#14
MattDHill merged 4 commits into
Start9-Community:masterfrom
lundog:fix-server-rebinding

Conversation

@lundog

@lundog lundog commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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. 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.

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 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. assertNotCmdError now 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 /_servers held the old address until the bridge was restarted by hand.

…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.
@lundog
lundog marked this pull request as draft August 25, 2026 08:46
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.
@lundog
lundog marked this pull request as ready for review August 25, 2026 09:05

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (errorAgentagentError.BROKERbrokerErr.NETWORKnetworkError.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-settings oneshot, which runs after main has returned — a const read 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's start builds main's effects with callbacks.child('main'); that child is only removed on stop, so isInContext stays true for the whole run.
  • SystemForStartOs.start() sets effects.constRetry = once(() => { if (effects.isInContext) effects.restart() }) on that same object, before calling main.
  • CommandController invokes a oneshot's exec.fn(subcontainer, signal) — it passes no effects — so the fn in main.ts uses the closed-over effects from setupMain, and configureServers(effects, settings) hands it straight to resolveServerUris before withBotSession is reached.
  • Watchable.const() keys entirely off this.effects.constRetry and isInContext, 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 catch in computeStartEnv preserves the prior failure semantics exactly — hands-off rethrows, managed swallows and retries in the oneshot. No regression, good.
  • Bumping to 7.0.0:1 rather than re-releasing in place is right: 7.0.0:0 is published to community-prod, so it's permanent.
  • Comment budget: the 7-line block in computeStartEnv and the 9-line JSDoc on a 7-line staleRelayHost are 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 9a36fa8 only. The run on head ffe6524 came back skipped/0s, so the liveSync.ts change 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.
@lundog
lundog requested a review from helix-nine August 28, 2026 19:22
@lundog

lundog commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

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 other key — fixed

Changed to '=7.0.0:0'.

The stated cause — you were right, and the real cause is worse

Instrumented build on hardware, managed mode with Local relays:

[diag] main body (before computeStartEnv): constRetry=true isInContext=true
[diag] main body (after computeStartEnv): constRetry=true isInContext=true
[diag] sync-settings oneshot: constRetry=true isInContext=true

constRetry and isInContext are both live in the oneshot, exactly as your trace said. The read's location was never the problem, and my explanation was wrong. Then, on uninstalling the SimpleX Server:

[diag] xftp watch ended: RangeError: Maximum call stack size exceeded

That is deepEqual — the default eq for sdk.host.get. Filled address objects (filledAddress.js:139-186) carry enumerable lazy getters nonLocal, public, bridge, each returning a new object of the same shape with the same three getters. deepEqual walks Object.keys(x) and reads objects[0][key], invoking those getters and minting another level. It cannot terminate.

Reproduced standalone against two host records differing only in fingerprint:

a: [ 'smp://OLD_FINGERPRINT=@server.local:5223' ]
b: [ 'smp://NEW_FINGERPRINT=@server.local:5223' ]
deepEqual(a, b)             THREW: RangeError: Maximum call stack size exceeded
deepEqual(uris(a), uris(b)) = false

Two details make it match the field log. watchGen only calls eqFn from the second value onward, so the initial read always succeeds and the first change throws. And Watchable.const() attaches gen.next().then(onFulfilled, onRejected) where the rejection handler aborts and cleans up but never calls constRetry (Watchable.js:72-75) — so the crash is silent: no restart, no log, no error surface. It was only visible here because watch() let me catch and print it.

So both 7.0.0:0 and my first fix attempt passed no map, both compared whole host records, and both watchers died on the first change. The rebind change moved a read that was already doomed.

The missing map — this was the fix

resolveLocalRelayUri now resolves the URI inside the selector, so .const() compares a string | undefined:

sdk.host.get(effects, { packageId, hostId }, (host) => {
  const addressInfo = /* ...find the interface... */
  for (const tier of tiers) {
    const urls = tier.format('urlstring')
    if (urls.length) return urls[0]
  }
  return undefined
}).const()

That stops the crash and narrows reactivity to the address itself, which is what the docs section you linked asks for. Being pre-existing turned out to be the reason this was never noticed.

Worth an upstream issue against start-sdk: any package doing sdk.host.get(...).const() without a map has a silently dead watcher after the first change.

Smaller notes

  • staleRelayHost's JSDoc is down to three lines, and the error string is split into cause then remedy.
  • The block comment in computeStartEnv is rewritten — it asserted the mechanism that turned out to be wrong.

Confirmed on hardware

Full uninstall/reinstall cycle of the SimpleX Server, managed mode, Local relays. The bridge restarts twice — once when the dependency goes away, once when its new address resolves — and comes back on the new fingerprint with no manual step. Previously /_servers held the old address indefinitely.

The same run also caught something worth guarding. The server publishes its binding before its fingerprint exists, so mid-reinstall the address formats as:

smp://undefined:null@server.local:5223

Harmless in managed mode, but hands-off would have written that into SMP_SERVERS as a relay that cannot work. A relay URI is now accepted only once its userinfo looks like a CA fingerprint; anything else counts as not-ready-yet. That also removed a wasted restart, since the malformed value no longer registers as a change — three restarts down to two, both load-bearing.

…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>
@helix-nine
helix-nine force-pushed the fix-server-rebinding branch from 7d8cbc5 to a33d83b Compare August 28, 2026 19:45

@helix-nine helix-nine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:0 is published and versionGraph carries only current, so overwriting its notes left anyone still on 0.3.0:5 with 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. Now if (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 unknownCAError string.

=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.

@MattDHill
MattDHill merged commit d59c31f into Start9-Community:master Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants