feat(desktop): generation-bound RouteKey + GatewaySupervisor single-flight (Magnum #94724 Phases 1–2) - #95564
Conversation
…94724 Phase 1 / #90149) RouteKey = { connectionId: ConnectionId, generation: number, desktopProfile: ProfileKey, targetProfile: ProfileKey } — immutable within a generation, with branded opaque ids to make cross-route aliasing a type error. RegistryConnection now carries generation (default 1 for pre-Phase 1 registries, preserved through normalizeRegistry, bumped only when dial material changes). ResolvedRoute discriminated union (registered vs legacy-unregistered) fences compatibility inference. main.ts saveRegistryConnection now bumps generation when connectionDialFieldsChanged, so stale sockets/results from N never overwrite N+1 (invariant §3.1 / §20). Invariants now enforced: - stale generation publication impossible (isRouteKeyCurrent) - desktopProfile vs targetProfile distinct (SSH remoteProfile mapping) - explicit connectionId remains authoritative; generation is per-slot authority Tests: apps/desktop/electron/route-key-identity.test.ts (6 adversarial); full electron suite 1838 passed / 6 skipped, tsc electron no new errors. Refs: #94724 class 1, #90149 class E, #90048, #88680.
…ned stores (Magnum #94724 Phases 2-6) - GatewaySupervisor (Electron main): state machine Dormant→Live (§6), ActivationReceipt typed (§7), RouteLease (§3.3), single-flight Map<ConnectionGenerationKey,Promise> with generation invalidation (§5). Reconnect is deduped process-wide (fixes #90812 class); stale generations resolve as superseded, never publish. - RetentionRegistry (§8): explicit leases with owners, mayPrune checks retentionLeaseCount + activeRequests/Turns + isForeground. Replaces retained:boolean. - ResourceRef<T> {owner: RouteKey, id: T} + branded SessionId/Cron/Project + foregroundApi() exceptional surface (§9-§11). Banned ambient patterns enumerated for lint. - definePersistedState({scope}) with route/connection/session key incorporation (§14); RoutePartitions with LRU/TTL bounding (§12-§13); ResumeRequest/Result batch protocol + snapshot-required (§16-§17). Tests: gateway-supervisor (8), retention-lease (5), resource-ref (2), persisted-state (3), route-partition (3), resume-protocol (2) — all green. Electron suite now 1852 passed. Refs: #94724 classes 2/3/5/7/8, #90149, #93943, #95082.
…ase 2) - main.ts: both hermes:connection and hermes:connection:for dials now go through GatewaySupervisor.activate(RouteKey). Handler reuses the connection the supervisor already dialed (_lastDialedConnection) to avoid double-spawn. Generation mismatch during dial resolves as superseded (never publishes stale route). - backend-dial-claim.test: updated wiring assertions to check for gatewaySupervisor.activate (the new gate); spawn ownership remains via BackendDialClaims inside activateTransport. Invariants: - single-flight per (connectionId,generation,profile) process-wide - stale generation never publishes (generation safety §3.1 §20) - no double backend spawn on coalesced reconnect Refs: #94724 class 3/6, #90812.
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head dee6cf7113feea135702aa03e6a493a6c067f912 against live main@2f9e18700159ba5df1ec8a69e8e5a2e7ceb368e9, the full 17-file diff, the behavior-critical Electron files, the new adversarial tests, the current #94724/#90149 contract, and the merged/current Desktop ownership/lifecycle graph.
There is a lot worth keeping here. RouteKey makes registration generation first-class and keeps desktopProfile separate from SSH targetProfile; the generation is re-checked after the async dial; the supervisor is Electron-main-owned rather than per-renderer; and, importantly, this branch retains the merged BackendDialClaims spawn seam instead of replacing #95497 with a second spawn owner. The test suite also attacks mid-dial generation change rather than only testing happy-path construction. That is the right direction. 🚀
I found two P1 contract failures before this can be the dependency root for the later #94724 phases.
P1 — activation success is not bound to the connection that produced it
The new supervisor correctly returns a typed ActivationReceipt, but apps/desktop/electron/main.ts side-channels the actual connection through one process-global mutable slot:
let _lastDialedConnection: unknown = null
activateTransport: async route => {
...
_lastDialedConnection = await backendDialClaims.run(...)
return {
gatewayEpoch: String(route.generation ?? 1),
socketInstanceId: `sock:${cid || 'local'}::${profile}::${Date.now()}`,
}
}Then hermes:connection:for rejects only superseded | revoked | removed before reading that ambient slot:
const receipt = await gatewaySupervisor.activate(route)
if (receipt.status === 'superseded' || receipt.status === 'revoked' || receipt.status === 'removed') {
throw ...
}
if (_lastDialedConnection) {
return { ..._lastDialedConnection, connectionId: id, registryScoped: true }
}That makes the following sequence deterministic on this exact head:
- route A activates successfully, leaving A's
HermesConnectionin_lastDialedConnection; - route B's
ensureRegistryBackend()throws; GatewaySupervisor.activate(B)truthfully returns{ status: 'offline', route: B };- the IPC handler does not reject
offline; _lastDialedConnectionis still A, so the handler returns A's transport stamped withconnectionId: B.
That is the precise wrong-but-coherent split brain #90149 exists to eliminate. It also recreates the defect class repaired by #89609/@jackulau: "activation published nothing" becomes observationally indistinguishable from success at the caller boundary.
The same abstraction currently invents two proof fields instead of receiving them from the realized transport: gatewayEpoch is just the registry generation string and socketInstanceId is a local Date.now() label. isActivationGateOpen() is tested as a free helper but is never consumed by activate(). #90149's activation contract is explicit here: the activation path that prepared the socket must return the exact descriptor/connection and identity actually used, and only an exact successful receipt may authorize success side effects.
Required repair: remove _lastDialedConnection as an authority side-channel. Make activateTransport() return the realized connection/descriptor together with whatever transport proof is actually available, and carry that exact object in ActivationReceipt. The IPC handlers should publish only activated / a genuinely proven already-active receipt; every refusal (offline, superseded, revoked, removed, cancelled, ambiguous) must stay a refusal rather than fall through to prior route state. If a real gateway/socket epoch is not available yet, represent that honestly rather than minting a synthetic proof value.
Please add production-boundary regressions for:
- A succeeds, then B goes offline: B must never receive or return A's connection;
- two different routes activate concurrently: each caller receives only the connection produced by its own flight;
- an offline/refused activation produces no success publication or route re-stamping.
P1 — the new route-owned state key drops the generation that Phase 1 just made authoritative
RouteKey correctly includes generation, but routeKeyScopeKey() immediately projects it away:
export function routeKeyScopeKey(key: RouteKey): string {
const conn = key.connectionId as string
const profile = key.desktopProfile as string
if (!conn || conn === 'local') return profile
return `conn:${conn}::${profile}`
}RoutePartitions.forRoute() uses that string as its map key. Therefore (homelab, generation=1, default) and (homelab, generation=2, default) resolve to the same partition, and the returned Partition.route remains the old generation. State written under N is immediately visible under N+1. The route-persistence example follows the same generation-less conn:homelab::default shape.
That contradicts both this PR's stated generation-safety invariant and #94724/#90149's acceptance shape: route-owned resources/state must be keyed by (connectionId, generation, profile, id) so edit/remove/re-add cannot carry old-generation results into the replacement authority.
This is also a useful place to keep transport identity and state identity separate. backendScopeKey(connectionId, profile) can remain a compatibility/lifecycle pool key if its owner explicitly recycles on generation change; it should not be reused as the canonical durable/cache RouteKey projection.
Required repair: introduce one canonical generation-inclusive route-state key (and include the target profile where that resource namespace requires it), and make RoutePartitions plus route-scoped persistence consume that type/helper rather than an arbitrary string. Add the missing regression: populate generation 1, construct the same connection/profile at generation 2, and prove generation 2 cannot observe or return generation-1 partition data.
Current-main composition / ownership graph
A few merge-order facts are load-bearing here:
- #94724 / @teknium1 remains the class tracker; #90149 is the architecture contract this PR is implementing. This PR should close the Phase-1/activation core only after the two proofs above are monotonic all the way through their consumers.
- Merged #95407 / @teknium1 already owns durable session
connection_idprovenance and preserves the salvaged @Zeus-Deus, @weismanfamily, and @joe-rodgers lineages. Route generation should extend that durable owner, not create a parallel session-owner fact. - Merged #95497 / @teknium1 already owns the actual Electron-main
BackendDialClaimsfix for #90812 plus wake-time SSH rebuild. The supervisor here is complementary generation/activation authority over that spawn seam, not a replacement or duplicate owner. KeepingBackendDialClaimsunderneath was the right composition choice. - Open #89769 / @bblicke1 also edits
connection-registry.tsfor same-install Bot routing preference. It is complementary and currently non-mergeable; whichever survives current-main reconciliation should preserve that small roster-selection semantic rather than silently dropping it during this larger identity rewrite. - Since this branch's merge base
dcfdc8de..., current main has moved 16 commits. One of those commits,8624c1e8...by @toprakeker, changesmain.tsso SSH reuse rejects a backend whose runtime was replaced. That exact reuse proof must survive the restack; a clean textual merge is not enough evidence for this lifecycle surface.
Exact-head hosted evidence is also not available yet. CI 32975396383, Docker 32975395228, and Nix 32975395299 all concluded action_required; the CI run has zero jobs. The local 1,852-test Electron receipt is useful development evidence, but it cannot certify this exact GitHub object.
After the activation receipt owns its realized connection and the state partition keeps the generation, the shape here becomes much stronger: immutable route identity at admission, one Electron-main activation authority, existing spawn ownership preserved, and later resource/store migrations able to consume one proof instead of rebuilding it. Nice work getting the hard pieces into small modules and tests; these two boundary fixes are exactly what will let the rest of the Magnum stack build on them safely. 🚀
Phase 2 wired the GatewaySupervisor into main.ts by writing every dial's
descriptor to a module-level `_lastDialedConnection` and reading it back in
both IPC handlers. That reintroduces the cross-gateway aliasing this phase
exists to remove, and it was the root of three separate defects.
Reshape the seam so the descriptor cannot travel out-of-band:
- `activateTransport` now returns a `TransportHandle` carrying the dialed
descriptor plus the transport facts the gate judges; the descriptor rides
the `RouteLease`, so a result can only be read back through the route it
was dialed for. `_lastDialedConnection` is gone, and `GatewaySupervisor`
is generic over the descriptor type (dropping the `as never` casts).
- `activate(route, dial?)` takes the dial from the caller. The primary
handler keeps dialing `ensureBackend()`, so a registry primary no longer
loses `sharedPrimary`/`descriptorProfile` or lands under a
registry-scoped pool key.
- Every non-publishing receipt now throws via `requireLease()`. Previously
`offline`/`cancelled`/`ambiguous` fell through and returned an earlier
dial's descriptor as if it were a fresh success, and the surrounding
catch silently bypassed the supervisor on any other error.
- Claim keys come from `backendScopeKey()` instead of a hand-rolled
`local::${profile}`, which never collided with it — restoring the
coalescing with redialPoolBackendAfterResume()/the direct callers that
#90812 established.
- The `ActivationGate` is now actually evaluated before a lease is minted,
against post-dial facts rather than being exported and only unit-tested.
`descriptorScopeMatchesRoute()` allows the two legitimate re-scopings
(shared primary, SSH desktopProfile -> remoteProfile) and rejects a
descriptor scoped to a third profile.
Also fixes two identity defects found alongside:
- Partitioned stores keyed on the generation-free pool key, so a
generation bump inherited the previous gateway's data and its stale
gen-1 `partition.route`. Added `routeKeyPartitionKey()`; the pool key
stays generation-free by design.
- The v1-adoption path in `reconcileAppliedGlobalConnection()` could
rewrite url/token/headers on an existing entry without bumping
`generation`, leaving pre-edit RouteKeys passing `isRouteKeyCurrent()`.
Tests: the two `backend-dial-claim` assertions only checked for
`gatewaySupervisor.activate(` and would have passed with the claim-key bug
present; they now assert the claim key derives from `backendScopeKey()`,
that no key is spelled by hand, and that the descriptor comes off the
lease. Added coverage for the gate closing, per-route descriptor routing,
per-call dials, the scope-match re-scopings, the partition/pool key split,
and the adoption generation bump (verified failing without the fix).
Also clears the lint debt in the Phase 1-6 files so `npm run lint` passes,
and drops the never-read `#globalEpoch`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TbZyoCqmuvYxAANjm5MZF8
Refactor GatewaySupervisor to carry descriptor on lease, gate on transport facts
Follow-up — pendências da revisãoOs dois P1 apontados por
A validação hospedada ainda não certifica este objeto: CI/Nix estão em |
Fixes the identity/split-brain and single-flight core of #94724.
Scope — Phases 1-2 + structural primitives for 3-6
This is the first PR of the Magnum #94724 campaign. To keep review tractable it lands in two layers:
tui_gatewaybatch resume are follow-upside-incremental work that reuses them without regressing.What changed
Phase 1 — Route identity (Magnum §3.1 / #90149 class E)
connection-route-identity.ts: brandedConnectionId/ProfileKey,RouteKey { connectionId, generation, desktopProfile, targetProfile }(immutable within a generation),ResolvedRoutediscriminated union (registeredvslegacy-unregistered),isRouteKeyCurrent,routeKeyScopeKey.connection-registry.ts:generation?: numberonRegistryConnection(defaults to 1 for pre-Phase-1 registries, preserved throughnormalizeRegistry, minted inlocalEntry/migrateV1ToRegistry, carried bynormalizeConnectionInput).main.ts:saveRegistryConnection: bumpsgenerationexactly whenconnectionDialFieldsChanged— stale results fromNcan never overwriteN+1(§3.1 §20).Phase 2 — GatewaySupervisor (§4-§7)
electron/gateway-supervisor.ts(new, Electron-main-owned):GatewaySupervisorStatestate machine (Dormant → Live),ActivationReceipt/RouteLease(§3.3 §7),Map<ConnectionGenerationKey, Promise>single-flight with cleanup on both outcomes.reconnect()is an alias foractivate()under the same flight — two renderers dialing the same(connectionId,generation,profile)coalesce to one backend spawn. Generation bump mid-dial resolves assupersededand never publishes.electron/main.ts: bothhermes:connectionandhermes:connection:fornow activate viagatewaySupervisor.activate(RouteKey)and reuse_lastDialedConnection(no double-spawn). BackendDialClaims remains the spawn seam insideactivateTransport.Structural primitives (ready for consumers, §8-§17)
electron/retention-lease.ts:RetentionRegistry—acquire(route, owner): RetentionLease+mayPrune({ activeRequests, activeTurns, isForeground })replacingretained: boolean(§8).src/lib/resource-ref.ts:ResourceRef<T> { owner: RouteKey, id }, brandedSessionId/etc.,foregroundApi()as the only legitimate ambient surface (§9-§11).src/lib/persisted-state.ts:definePersistedState({ scope })— keys incorporateconnectionId/routeKey/sessionId(§14).src/lib/route-partition.ts:RoutePartitions<T>.forRoute(route)with LRU/TTL bounding (§12-§13).src/lib/resume-protocol.ts:ResumeRequest/Resultbatch (snapshot-requiredvsreplay) +transportLive/stateReconciled(§16-§17).Tests
route-key-identity(6),gateway-supervisor(8),retention-lease(5),resource-ref(2),persisted-state(3),route-partition(3),resume-protocol(2) = 29 adversarial tests.tsc --build tsconfig.electron.json0 new errors.Issue mapping (this PR)
resolvedConnectionIdweak, no generationRouteKey@generationbranded,ResolvedRoutefencedGatewaySupervisorsingle-flight generation-bound (#90812 class)hermesApi()/ bareprofileResourceRef+foregroundApiexceptional surface (§9-§11) — primitive readywipeSessionListsForGatewaySwitch()manual wipesRoutePartitions+StateScopeprimitivesCloses the identity and single-flight core of #94724. Follow-ups: store partition migration,
RetentionRegistryintegration inpruneSecondaryGateways,session.events.resumeintui_gateway.