mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(sessions): always stamp owning profile so cross-profile open works toward default
Session rows served without ?profile= carried no profile field, so in multi-profile desktops the default profile's sessions circulated unowned: resolveStoredSession cached them profile-less, resolveSessionProfile returned undefined, and session.resume targeted whichever gateway was active -- opening a default-profile session from a non-default window failed with "session can't be found" while the reverse direction worked (#67603 family). Server: GET /api/sessions/{id} and GET /api/sessions now stamp profile/ is_default_profile unconditionally -- the serving profile is always known (_cron_default_profile() when the request is unscoped). Renderer: resolveStoredSession treats a profile-less $sessions cache hit as unresolved when >1 profile exists (falls through to the stamped by-id ladder) and back-fills the active profile on bare by-id hits from older backends, so unowned rows are never re-cached. Verified via CDP against a live 4-profile renderer: bare by-id GET returned hasProfileField:false and the stale cache rows matched; with the fix both lookups return the owning profile and the resume routes to the right backend.
This commit is contained in:
parent
015718066a
commit
a8ec50f681
4 changed files with 188 additions and 6 deletions
|
|
@ -0,0 +1,109 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type * as HermesModule from '@/hermes'
|
||||
import { getSession } from '@/hermes'
|
||||
import { $activeGatewayProfile, $profiles } from '@/store/profile'
|
||||
import { $sessions } from '@/store/session'
|
||||
import type { SessionInfo } from '@/types/hermes'
|
||||
|
||||
import { resolveSessionProfile, resolveStoredSession } from './utils'
|
||||
|
||||
vi.mock('@/hermes', async importActual => ({
|
||||
...(await importActual<typeof HermesModule>()),
|
||||
getSession: vi.fn()
|
||||
}))
|
||||
|
||||
const mockGetSession = vi.mocked(getSession)
|
||||
|
||||
const session = (over: Partial<SessionInfo>): SessionInfo => over as SessionInfo
|
||||
|
||||
const profiles = (...names: string[]) => names.map(name => ({ name }) as never)
|
||||
|
||||
describe('resolveStoredSession profile ownership', () => {
|
||||
beforeEach(() => {
|
||||
$sessions.set([])
|
||||
$profiles.set(profiles('default', 'meta'))
|
||||
$activeGatewayProfile.set('meta')
|
||||
mockGetSession.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
$sessions.set([])
|
||||
$profiles.set([])
|
||||
$activeGatewayProfile.set('default')
|
||||
})
|
||||
|
||||
it('returns a cached row that carries an owning profile', async () => {
|
||||
$sessions.set([session({ id: 's1', profile: 'default' })])
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.profile).toBe('default')
|
||||
expect(mockGetSession).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('treats a profile-less cache hit as unresolved when multiple profiles exist', async () => {
|
||||
$sessions.set([session({ id: 's1' })])
|
||||
mockGetSession.mockRejectedValueOnce(new Error('404: Session not found'))
|
||||
mockGetSession.mockResolvedValueOnce(session({ id: 's1', profile: 'default' }))
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.profile).toBe('default')
|
||||
// rung 2 (bare) then rung 3 (stamped cross-profile probe)
|
||||
expect(mockGetSession).toHaveBeenNthCalledWith(1, 's1')
|
||||
expect(mockGetSession).toHaveBeenNthCalledWith(2, 's1', 'default')
|
||||
})
|
||||
|
||||
it('accepts a profile-less cache hit for single-profile users', async () => {
|
||||
$profiles.set(profiles('default'))
|
||||
$sessions.set([session({ id: 's1' })])
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.id).toBe('s1')
|
||||
expect(mockGetSession).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('stamps the active profile on a bare by-id hit from an older backend', async () => {
|
||||
mockGetSession.mockResolvedValueOnce(session({ id: 's1' }))
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.profile).toBe('meta')
|
||||
// the upserted cache row is owned too, so the next hit short-circuits
|
||||
expect($sessions.get().find(s => s.id === 's1')?.profile).toBe('meta')
|
||||
})
|
||||
|
||||
it('probed desktop profile overrides a remote backend answering as its own "default"', async () => {
|
||||
// Per-profile remote override: Electron strips the desktop alias before
|
||||
// forwarding, so the standalone backend stamps its backend-local root.
|
||||
mockGetSession.mockRejectedValueOnce(new Error('404: Session not found'))
|
||||
mockGetSession.mockResolvedValueOnce(session({ id: 's1', profile: 'default' }))
|
||||
$activeGatewayProfile.set('default')
|
||||
$profiles.set(profiles('default', 'meta'))
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.profile).toBe('meta')
|
||||
expect($sessions.get().find(s => s.id === 's1')?.profile).toBe('meta')
|
||||
})
|
||||
|
||||
it('stamps the probed profile on a scoped hit from an older backend that omits it', async () => {
|
||||
mockGetSession.mockRejectedValueOnce(new Error('404: Session not found'))
|
||||
mockGetSession.mockResolvedValueOnce(session({ id: 's1' }))
|
||||
|
||||
const resolved = await resolveStoredSession('s1')
|
||||
|
||||
expect(resolved?.profile).toBe('default')
|
||||
// the cached row is owned too — no unowned row is ever re-cached
|
||||
expect($sessions.get().find(s => s.id === 's1')?.profile).toBe('default')
|
||||
})
|
||||
|
||||
it('resolveSessionProfile routes a default-profile session from a non-default gateway', async () => {
|
||||
mockGetSession.mockRejectedValueOnce(new Error('404: Session not found'))
|
||||
mockGetSession.mockResolvedValueOnce(session({ id: 's1', profile: 'default' }))
|
||||
|
||||
await expect(resolveSessionProfile('s1')).resolves.toBe('default')
|
||||
})
|
||||
})
|
||||
|
|
@ -587,7 +587,13 @@ function upsertResolvedSession(session: SessionInfo, storedSessionId: string) {
|
|||
export async function resolveStoredSession(storedSessionId: string): Promise<SessionInfo | undefined> {
|
||||
const cached = $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId))
|
||||
|
||||
if (cached) {
|
||||
// A row with no owning profile can't route a resume when more than one
|
||||
// profile exists — a resume without a profile lands on whichever gateway is
|
||||
// active (#67603 family, cross-profile open asymmetry). Treat such a hit as
|
||||
// unresolved and fall through to the by-id lookups, which stamp ownership.
|
||||
const multiProfile = $profiles.get().length > 1
|
||||
|
||||
if (cached && (cached.profile?.trim() || !multiProfile)) {
|
||||
return cached
|
||||
}
|
||||
|
||||
|
|
@ -597,6 +603,12 @@ export async function resolveStoredSession(storedSessionId: string): Promise<Ses
|
|||
try {
|
||||
const session = await getSession(storedSessionId)
|
||||
|
||||
// Older backends omit `profile` on unscoped GETs; the serving backend is
|
||||
// the active gateway's, so back-fill that rather than caching an unowned
|
||||
// row. A present stamp is preserved: in app-global remote mode a bare hit
|
||||
// can legitimately carry another profile's row (see the branch tests).
|
||||
session.profile ||= normalizeProfileKey($activeGatewayProfile.get())
|
||||
|
||||
upsertResolvedSession(session, storedSessionId)
|
||||
|
||||
return session
|
||||
|
|
@ -618,6 +630,12 @@ export async function resolveStoredSession(storedSessionId: string): Promise<Ses
|
|||
try {
|
||||
const session = await getSession(storedSessionId, profile)
|
||||
|
||||
// Same ownership contract: the DESKTOP profile we explicitly probed is
|
||||
// authoritative, whatever the scoped backend stamped (older backends
|
||||
// omit the field; a per-profile remote override strips the alias before
|
||||
// forwarding, so that backend answers as its own "default").
|
||||
session.profile = profile
|
||||
|
||||
upsertResolvedSession(session, storedSessionId)
|
||||
|
||||
return session
|
||||
|
|
|
|||
|
|
@ -4986,14 +4986,17 @@ def get_sessions(
|
|||
exclude_children=True,
|
||||
)
|
||||
now = time.time()
|
||||
# Same ownership contract as get_session_detail: rows are stamped
|
||||
# with the serving profile even when the request wasn't explicitly
|
||||
# scoped, so default-profile rows never circulate unowned.
|
||||
row_profile = profile_name or _cron_default_profile()
|
||||
for s in sessions:
|
||||
s["is_active"] = (
|
||||
s.get("ended_at") is None
|
||||
and (now - s.get("last_active", s.get("started_at", 0))) < 300
|
||||
)
|
||||
if profile_name:
|
||||
s["profile"] = profile_name
|
||||
s["is_default_profile"] = profile_name == "default"
|
||||
s["profile"] = row_profile
|
||||
s["is_default_profile"] = row_profile == "default"
|
||||
# SQLite stores the flag as 0/1; expose a real JSON boolean.
|
||||
s["archived"] = bool(s.get("archived"))
|
||||
if not full:
|
||||
|
|
@ -11986,8 +11989,16 @@ async def get_session_detail(session_id: str, profile: Optional[str] = None):
|
|||
session = db.get_session(sid) if sid else None
|
||||
if not session:
|
||||
raise HTTPException(status_code=404, detail="Session not found")
|
||||
if profile:
|
||||
session["profile"] = _cron_profile_home(profile)[0]
|
||||
# Always stamp the owning profile — the serving profile is known even
|
||||
# when the request carries no ``?profile=`` (it's this process's own
|
||||
# profile). Stamping only on explicit ``?profile=`` left rows for the
|
||||
# default/primary profile systematically unowned, so multi-profile
|
||||
# clients resolved them to whichever gateway happened to be active
|
||||
# (cross-profile open asymmetry, #67603 family).
|
||||
session["profile"] = (
|
||||
_cron_profile_home(profile)[0] if profile else _cron_default_profile()
|
||||
)
|
||||
session["is_default_profile"] = session["profile"] == "default"
|
||||
return session
|
||||
finally:
|
||||
db.close()
|
||||
|
|
|
|||
|
|
@ -1858,6 +1858,50 @@ class TestWebServerEndpoints:
|
|||
row = next(s for s in rows if s["id"] == "session-no-cwd")
|
||||
assert row["cwd"] is None
|
||||
|
||||
def test_session_detail_stamps_profile_without_query_param(self, monkeypatch):
|
||||
"""GET /api/sessions/{id} must carry the owning profile even when the
|
||||
request has no ?profile= (bug: default-profile rows circulated unowned,
|
||||
so multi-profile desktop clients resumed them on the wrong gateway).
|
||||
"""
|
||||
from hermes_state import SessionDB
|
||||
|
||||
import hermes_cli.web_server as web_server
|
||||
|
||||
monkeypatch.setattr(web_server, "_cron_default_profile", lambda: "default")
|
||||
|
||||
db = SessionDB()
|
||||
try:
|
||||
db.create_session(session_id="detail-stamp", source="cli")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
resp = self.client.get("/api/sessions/detail-stamp")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["profile"] == "default"
|
||||
assert data["is_default_profile"] is True
|
||||
|
||||
def test_session_list_stamps_profile_without_query_param(self, monkeypatch):
|
||||
"""GET /api/sessions rows must carry the serving profile even when the
|
||||
request is unscoped (sibling of the detail-endpoint stamp gap)."""
|
||||
from hermes_state import SessionDB
|
||||
|
||||
import hermes_cli.web_server as web_server
|
||||
|
||||
monkeypatch.setattr(web_server, "_cron_default_profile", lambda: "default")
|
||||
|
||||
db = SessionDB()
|
||||
try:
|
||||
db.create_session(session_id="list-stamp", source="cli")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
resp = self.client.get("/api/sessions?limit=20&offset=0")
|
||||
assert resp.status_code == 200
|
||||
row = next(s for s in resp.json()["sessions"] if s["id"] == "list-stamp")
|
||||
assert row["profile"] == "default"
|
||||
assert row["is_default_profile"] is True
|
||||
|
||||
def test_get_sessions_forwards_min_messages(self, monkeypatch):
|
||||
"""The ?min_messages= filter must reach SessionDB.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue