Conversation
alt-glitch
added
type/bug
Reviewed by reviewer-e (AI automated review). Right routing semantics for Bot Mode: a same-install backend reachable over plain HTTP should never have its turns tunneled through an SSH window-primary just because that's where the user happens to be connected — restricting the "active primary wins" rule to the preferred class (local outright, else HTTP, else everything) while keeping SSH-only groups routing through SSH preserves the old guarantee exactly where it still applies. The hover-warm skip for SSH sourceScoped rows closes the connect-on-demand violation on the interaction side, and the tests cover the three decisive shapes: remote-beats-SSH-primary, SSH-only passthrough, and no-dial-on-hover. |
|
File-overlap interlock: #90198 now owns exact route identity at No fixed semantic dependency is required: whichever PR lands later should rebase and retain both suites. #90198's identity resolver must remain exact-ID/current-registry + ambiguity-safe; #89769's roster selection remains separate. |
…arch#89756) Same-install collapse no longer routes Bots through SSH just because the window primary is SSH. Hover also skips SSH warm so the roster stays connect-on-demand.
66970ac to
52b2eb2
Compare
|
Rebased onto current Same two semantics: same-install HTTP-over-SSH roster preference; SSH hover-warm skip.
|
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Review verdict: ready to merge at 52b2eb2c90. No code blockers found.
I rechecked the rebased delta against the current Bot Mode routing architecture and the adjacent merged work:
buildAgentRosternow selects fromlocal, else HTTP (remote/cloud), else SSH before applying the primary tie-break. That fixes the reported same-install_idSSH-primary case without changing SSH-only routing, same-kind registration-order behavior, or the post-collapse handle rules.- The SSH guard sits on the plugin's only
warmAgent/warmProfilepath. Active-source, union, and retained connect-on-demand SSH rows all remain source-qualified and carryconnectionKind: 'ssh', so neither opening Bot Mode nor hovering an SSH row can dial an isolated backend. Explicit open remains separate. - #92731's immutable owner routing and pure
prepareBotSourcecapability gate remain intact. #90198's exact current-registry identity resolver also remains intact; this PR changes roster preference, not route identity. - The focused witnesses cover remote-over-SSH-primary, SSH-only passthrough, absence of pane-wide prewarm, and absence of SSH hover warm. The rebased author run reports
connection-registry77 passing andprofile-prewarm7 passing.
main has advanced six commits since this rebase, but none touch the four changed files and GitHub still reports the PR mergeable. The three hosted workflows are currently action_required with zero jobs created, i.e. the fork-workflow approval gate rather than a test failure.
…remote backend Bot Mode rows pre-warm their backend on pointer entry. For a row that lives on an SSH registry source, "warm" meant openGatewayForAgent → main ensureRegistryBackend → bootstrapSshConnection, i.e. open the tunnel and run `hermes -p <bot> serve --isolated` on the remote box. Sweeping the pointer across a roster of specialists spawned one isolated serve per bot and the cascade knocked the window's own primary chat over (WebSocket 1012). The docs promise SSH sources stay connect-on-demand until you open an agent, so the guard lives in the single speculative-warm resolver (store/profile.ts::prewarmProfileBackend): a registry connection of kind 'ssh' is never dialed by a hover; explicit opens are unchanged and same-box Remote gateways still warm. Salvages the hover half of #89769 (@bblicke1). The other half — preferring a same-install Remote gateway over an SSH primary in the roster collapse — is not reachable on current main: SSH sources are enumerated without an install_id (c2c3058), so they never join a collapse group, and re-routing the active primary's rows would re-introduce the doubled-roster class (#88344) in the renderer merger. Left for a design call. Part of #89756
|
Thanks @bblicke1 — this landed on main through #113394 (fix(bot-mode): hovering an SSH Bot never dials the tunnel or spawns a remote backend (#897), merged as 3d9ddb7. Your commits/analysis were carried in with credit (see that PR's body and Co-authored-by trailers); the salvage rebased onto current main, widened to sibling surfaces where the review found gaps, and was live-verified in the real Electron app. Closing this one as superseded — thank you for the fix. |
|
Aweome! Glad I could help. Thank YOU @teknium1 for all your hardwork and dedication. |
5 participants
Problem
Bot Mode on a host registered as both SSH and a Remote gateway routed every Bot through SSH whenever SSH was the window primary. Opening/hovering Bots spawned
hermes -p <bot> serve --isolatedper profile and 1012'd the primary chat.Fixes #89756
Approach
install_idroster collapse now preferslocal, then Remote/cloud, then SSH. Window primary still wins among HTTP remotes, but SSH primary no longer beats a same-host Remote.warmAgents SSH rows (connect-on-demand). Explicit open is unchanged.Tests
64 + 6 passing.
Risk
Low. Roster identity only. Does not change SSH spawn/lock code or undo #88296. SSH-only setups still route through SSH.