mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-13 09:01:54 +00:00
* fix(desktop): rebind sessions after websocket reconnect * docs(desktop): explain the reconnect-resume guard in use-route-resume The reconnect fix turns on two subtle conditions with no inline rationale: `seenGatewayStateRef` suppresses a spurious "became open" on the first effect run (so a session mounting with the gateway already open doesn't double-resume), and the `gatewayBecameOpen ||` arm forces a re-resume even when the route looks `alreadyActive` because the cached runtime id can be stale after the gateway rebinds/reaps the session. Comment both so the next reader doesn't "simplify" them back into the original bug. No behavior change. --------- Co-authored-by: Josh Dow <josh.dow@prepad.io>
This commit is contained in:
parent
46fedef07f
commit
8d71c38919
5 changed files with 126 additions and 3 deletions
|
|
@ -187,4 +187,72 @@ describe('useRouteResume', () => {
|
|||
expect(resumeSession).toHaveBeenCalledTimes(1)
|
||||
expect(resumeSession).toHaveBeenCalledWith('session-2', true)
|
||||
})
|
||||
|
||||
it('resumes the selected route again when the gateway reconnects', () => {
|
||||
const resumeSession = vi.fn(async () => undefined)
|
||||
const startFreshSessionDraft = vi.fn()
|
||||
const activeSessionIdRef: MutableRefObject<null | string> = { current: 'runtime-1' }
|
||||
const creatingSessionRef = { current: false }
|
||||
const runtimeIdByStoredSessionIdRef = { current: new Map([['session-1', 'runtime-1']]) }
|
||||
const selectedStoredSessionIdRef: MutableRefObject<null | string> = { current: 'session-1' }
|
||||
|
||||
const { rerender } = render(
|
||||
<RouteResumeHarness
|
||||
activeSessionId="runtime-1"
|
||||
activeSessionIdRef={activeSessionIdRef}
|
||||
creatingSessionRef={creatingSessionRef}
|
||||
currentView="chat"
|
||||
freshDraftReady={false}
|
||||
gatewayState="open"
|
||||
locationPathname="/session-1"
|
||||
resumeSession={resumeSession}
|
||||
routedSessionId="session-1"
|
||||
runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef}
|
||||
selectedStoredSessionId="session-1"
|
||||
selectedStoredSessionIdRef={selectedStoredSessionIdRef}
|
||||
startFreshSessionDraft={startFreshSessionDraft}
|
||||
/>
|
||||
)
|
||||
|
||||
expect(resumeSession).not.toHaveBeenCalled()
|
||||
|
||||
rerender(
|
||||
<RouteResumeHarness
|
||||
activeSessionId="runtime-1"
|
||||
activeSessionIdRef={activeSessionIdRef}
|
||||
creatingSessionRef={creatingSessionRef}
|
||||
currentView="chat"
|
||||
freshDraftReady={false}
|
||||
gatewayState="closed"
|
||||
locationPathname="/session-1"
|
||||
resumeSession={resumeSession}
|
||||
routedSessionId="session-1"
|
||||
runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef}
|
||||
selectedStoredSessionId="session-1"
|
||||
selectedStoredSessionIdRef={selectedStoredSessionIdRef}
|
||||
startFreshSessionDraft={startFreshSessionDraft}
|
||||
/>
|
||||
)
|
||||
|
||||
rerender(
|
||||
<RouteResumeHarness
|
||||
activeSessionId="runtime-1"
|
||||
activeSessionIdRef={activeSessionIdRef}
|
||||
creatingSessionRef={creatingSessionRef}
|
||||
currentView="chat"
|
||||
freshDraftReady={false}
|
||||
gatewayState="open"
|
||||
locationPathname="/session-1"
|
||||
resumeSession={resumeSession}
|
||||
routedSessionId="session-1"
|
||||
runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef}
|
||||
selectedStoredSessionId="session-1"
|
||||
selectedStoredSessionIdRef={selectedStoredSessionIdRef}
|
||||
startFreshSessionDraft={startFreshSessionDraft}
|
||||
/>
|
||||
)
|
||||
|
||||
expect(resumeSession).toHaveBeenCalledTimes(1)
|
||||
expect(resumeSession).toHaveBeenCalledWith('session-1', true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -56,13 +56,19 @@ export function useRouteResume({
|
|||
startFreshSessionDraft
|
||||
}: RouteResumeOptions) {
|
||||
const lastPathnameRef = useRef<string | null>(null)
|
||||
const seenGatewayStateRef = useRef(false)
|
||||
const wasGatewayOpenRef = useRef(false)
|
||||
|
||||
useEffect(() => {
|
||||
const gatewayOpen = gatewayState === 'open'
|
||||
const pathnameChanged = lastPathnameRef.current !== locationPathname
|
||||
const gatewayBecameOpen = !wasGatewayOpenRef.current && gatewayOpen
|
||||
// Fire only on a genuine closed->open transition (a reconnect). seenGatewayStateRef
|
||||
// stays false until the first effect run, so a session that mounts with the gateway
|
||||
// already open is not mistaken for "became open" and does not double-resume with the
|
||||
// pathname-driven initial resume below.
|
||||
const gatewayBecameOpen = seenGatewayStateRef.current && !wasGatewayOpenRef.current && gatewayOpen
|
||||
lastPathnameRef.current = locationPathname
|
||||
seenGatewayStateRef.current = true
|
||||
wasGatewayOpenRef.current = gatewayOpen
|
||||
|
||||
if (currentView !== 'chat' || !gatewayOpen) {
|
||||
|
|
@ -99,7 +105,11 @@ export function useRouteResume({
|
|||
// before the pathname updates from /:sid -> /.
|
||||
const shouldResume = pathnameChanged || gatewayBecameOpen || stuckOnRoutedSession
|
||||
|
||||
if (!alreadyActive && shouldResume && !creatingSessionRef.current) {
|
||||
// On a reconnect (gatewayBecameOpen) re-resume even when the route looks
|
||||
// `alreadyActive`: the cached runtime id can be stale once the gateway
|
||||
// rebinds/reaps the session on its side, and trusting it strands Desktop on
|
||||
// a dead id ("session not found"). Otherwise keep skipping when already active.
|
||||
if ((gatewayBecameOpen || !alreadyActive) && shouldResume && !creatingSessionRef.current) {
|
||||
void resumeSession(routedSessionId, true)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1477,6 +1477,7 @@ AUTHOR_MAP = {
|
|||
# v0.15.0 additions
|
||||
"glen@workmanfirearms.com": "sgtworkman",
|
||||
"jorge.fuenmayort@gmail.com": "jfuenmayor",
|
||||
"josh.dow@prepad.io": "joshuadow", # PR #43004 salvage (desktop WS session rebind)
|
||||
"mordred@inaugust.com": "emonty",
|
||||
"rodrigoeq@hotmail.com": "rodrigoeqnit",
|
||||
"soliva.johnpaul@icloud.com": "jonpol01",
|
||||
|
|
|
|||
|
|
@ -613,6 +613,44 @@ def test_session_resume_live_payload_uses_current_history_with_ancestors(server,
|
|||
]
|
||||
|
||||
|
||||
def test_session_activate_rebinds_orphaned_ws_session_to_current_transport(server, monkeypatch):
|
||||
"""Reconnect + activate must reattach a parked live session before orphan reap."""
|
||||
|
||||
class _Transport:
|
||||
def write(self, _obj):
|
||||
return True
|
||||
|
||||
sid = "runtime01"
|
||||
old_transport = server._stdio_transport
|
||||
new_transport = _Transport()
|
||||
server._sessions[sid] = {
|
||||
"agent": types.SimpleNamespace(model="test/model"),
|
||||
"created_at": 123.0,
|
||||
"history": [],
|
||||
"history_lock": threading.RLock(),
|
||||
"last_active": 123.0,
|
||||
"running": False,
|
||||
"session_key": "20260409_010101_abc123",
|
||||
"transport": old_transport,
|
||||
}
|
||||
monkeypatch.setattr(server, "current_transport", lambda: new_transport)
|
||||
monkeypatch.setattr(server, "_get_db", lambda: None)
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
"_session_info",
|
||||
lambda _agent, _session=None: {"model": "test/model"},
|
||||
)
|
||||
|
||||
resp = server.handle_request(
|
||||
{"id": "activate", "method": "session.activate", "params": {"session_id": sid}}
|
||||
)
|
||||
|
||||
assert "error" not in resp
|
||||
assert resp["result"]["session_id"] == sid
|
||||
assert server._sessions[sid]["transport"] is new_transport
|
||||
assert not server._ws_session_is_orphaned(server._sessions[sid])
|
||||
|
||||
|
||||
def test_session_branch_persists_branched_from_marker(server, monkeypatch):
|
||||
"""TUI /branch must persist a _branched_from marker so the branch stays
|
||||
visible in /resume and /sessions.
|
||||
|
|
|
|||
|
|
@ -3844,10 +3844,16 @@ def _(rid, params: dict) -> dict:
|
|||
session, err = _sess_nowait({"session_id": sid}, rid)
|
||||
if err:
|
||||
return err
|
||||
assert session is not None
|
||||
|
||||
return _ok(
|
||||
rid,
|
||||
_live_session_payload(sid, session, touch=True),
|
||||
_live_session_payload(
|
||||
sid,
|
||||
session,
|
||||
touch=True,
|
||||
transport=current_transport() or _stdio_transport,
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue