fix(desktop): scope remote-profile session list reads and stop relabeling rows (#64999)
Per-profile remote overrides stripped ?profile= before reading the remote's /api/sessions and then stamped every row with the local connection-scope name. Against a multi-profile backend (one remote URL shared by several Desktop profiles) the unscoped read returned the backend's launch-profile sessions, relabeled as whichever local scope asked. fetchRemoteProfileSessions now names the profile scope on the wire (the managed-SSH remoteProfile alias when configured, else the Desktop profile name). A remote that rejects the scope (400/404 — the legacy single-launch-profile shape) falls back to the unscoped read exactly as before; auth/transport/5xx failures still propagate. The list splice no longer manufactures labels: the remote's own profile stamp is authoritative, and the legacy label only backfills rows an older remote returned unowned.
This commit is contained in:
Hermes Agent
brooklyn!
@@ -10,8 +10,11 @@ import {
|
||||
fetchRemoteProfileSessions,
|
||||
findRemoteOwnerProfileForSession,
|
||||
mergeProfileSessionWindow,
|
||||
pathWithRemoteOwnerScope,
|
||||
remoteProfileQueryScope,
|
||||
spliceRegistrySessionRows,
|
||||
tagRegistrySessionResponse
|
||||
tagRegistrySessionResponse,
|
||||
tagRemoteSessionRows
|
||||
} from './profile-session-routing'
|
||||
|
||||
test('remote sidebar slices all follow the selected profile', () => {
|
||||
@@ -138,9 +141,9 @@ test('remote session reads split oversized sidebar windows into API-safe pages',
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=100&offset=0&order=updated' },
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=100&offset=100&order=updated' },
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=50&offset=200&order=updated' }
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=100&offset=0&order=updated&profile=remote-work' },
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=100&offset=100&order=updated&profile=remote-work' },
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=50&offset=200&order=updated&profile=remote-work' }
|
||||
])
|
||||
assert.equal(result.sessions.length, 250)
|
||||
assert.equal(result.total, 250)
|
||||
@@ -182,7 +185,10 @@ test('remote paging preserves offsets and deduplicates pinned backfill rows', as
|
||||
}
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, ['/api/sessions?limit=100&offset=80', '/api/sessions?limit=50&offset=180'])
|
||||
assert.deepEqual(calls, [
|
||||
'/api/sessions?limit=100&offset=80&profile=remote-work',
|
||||
'/api/sessions?limit=50&offset=180&profile=remote-work'
|
||||
])
|
||||
assert.deepEqual(
|
||||
result.sessions.map(row => (row as { id: string }).id),
|
||||
[...rows.slice(80, 230).map(row => row.id), 'session-20']
|
||||
@@ -214,9 +220,9 @@ test('remote paging treats malformed totals as unknown instead of truncating the
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
'/api/sessions?limit=100&offset=0',
|
||||
'/api/sessions?limit=100&offset=100',
|
||||
'/api/sessions?limit=100&offset=200'
|
||||
'/api/sessions?limit=100&offset=0&profile=remote-work',
|
||||
'/api/sessions?limit=100&offset=100&profile=remote-work',
|
||||
'/api/sessions?limit=100&offset=200&profile=remote-work'
|
||||
])
|
||||
assert.equal(result.sessions.length, 250)
|
||||
assert.equal(result.total, 250)
|
||||
@@ -250,7 +256,9 @@ test('remote session reads keep small requests on one call', async () => {
|
||||
}
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, [{ profile: 'remote-work', path: '/api/sessions?limit=20&offset=0' }])
|
||||
assert.deepEqual(calls, [
|
||||
{ profile: 'remote-work', path: '/api/sessions?limit=20&offset=0&profile=remote-work' }
|
||||
])
|
||||
assert.equal(result, expected)
|
||||
})
|
||||
|
||||
@@ -486,3 +494,189 @@ test('remote owner lookup returns null when no remote lists the id or remotes fa
|
||||
|
||||
assert.equal(noRemotes, null)
|
||||
})
|
||||
|
||||
// #64999: two Desktop profile scopes can point at the SAME multi-profile
|
||||
// remote backend. The list read must name the requested profile scope so the
|
||||
// backend opens that profile's state.db — an unscoped read returns the
|
||||
// backend's launch-profile rows, and the old unconditional relabel stamped
|
||||
// them as whichever local scope happened to ask.
|
||||
test('remote session reads carry the profile scope against a multi-profile backend', async () => {
|
||||
const calls: Array<{ profile: string | null; path: string }> = []
|
||||
|
||||
await fetchRemoteProfileSessions(
|
||||
'wife',
|
||||
new URLSearchParams({ profile: 'wife', limit: '20', offset: '0' }),
|
||||
async (profile, path) => {
|
||||
calls.push({ profile, path })
|
||||
|
||||
return { sessions: [{ id: 's-1', profile: 'wife' }], total: 1, limit: 20, offset: 0 }
|
||||
}
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
{ profile: 'wife', path: '/api/sessions?limit=20&offset=0&profile=wife' }
|
||||
])
|
||||
})
|
||||
|
||||
// The same read for the OTHER scope sharing the backend names ITS scope — the
|
||||
// backend's launch profile (say `dad`) is never read for a `wife` request.
|
||||
test('two scopes sharing one multi-profile backend each read their own profile', async () => {
|
||||
const seenProfiles: string[] = []
|
||||
|
||||
for (const scope of ['wife', 'dad']) {
|
||||
const result = await fetchRemoteProfileSessions(
|
||||
scope,
|
||||
new URLSearchParams({ profile: scope, limit: '20', offset: '0' }),
|
||||
async (_profile, path) => {
|
||||
const url = new URL(path, 'http://desktop.test')
|
||||
const servedProfile = url.searchParams.get('profile') || 'launch-profile'
|
||||
seenProfiles.push(servedProfile)
|
||||
|
||||
return { sessions: [{ id: `s-${servedProfile}`, profile: servedProfile }], total: 1 }
|
||||
}
|
||||
)
|
||||
|
||||
assert.equal((result.sessions[0] as { profile: string }).profile, scope)
|
||||
}
|
||||
|
||||
assert.deepEqual(seenProfiles, ['wife', 'dad'])
|
||||
})
|
||||
|
||||
// A remote that rejects the scope (400/404: profile does not exist there) is
|
||||
// the legacy single-launch-profile shape — fall back to its own database.
|
||||
test('remote session reads fall back to the unscoped list when the remote rejects the profile scope', async () => {
|
||||
const calls: string[] = []
|
||||
|
||||
const result = await fetchRemoteProfileSessions(
|
||||
'wife',
|
||||
new URLSearchParams({ profile: 'wife', limit: '20', offset: '0' }),
|
||||
async (_profile, path) => {
|
||||
calls.push(path)
|
||||
|
||||
if (path.includes('profile=wife')) {
|
||||
const error: any = new Error('404: Profile \'wife\' does not exist.')
|
||||
error.statusCode = 404
|
||||
throw error
|
||||
}
|
||||
|
||||
return { sessions: [{ id: 's-1' }], total: 1 }
|
||||
}
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
'/api/sessions?limit=20&offset=0&profile=wife',
|
||||
'/api/sessions?limit=20&offset=0'
|
||||
])
|
||||
assert.equal((result.sessions[0] as { id: string }).id, 's-1')
|
||||
})
|
||||
|
||||
// Auth/transport/5xx failures are real errors — the unscoped fallback must
|
||||
// not swallow them.
|
||||
test('remote session reads propagate non-scope errors without the fallback', async () => {
|
||||
const calls: string[] = []
|
||||
|
||||
await assert.rejects(
|
||||
fetchRemoteProfileSessions(
|
||||
'wife',
|
||||
new URLSearchParams({ profile: 'wife', limit: '20', offset: '0' }),
|
||||
async (_profile, path) => {
|
||||
calls.push(path)
|
||||
const error: any = new Error('503: Service Unavailable')
|
||||
error.statusCode = 503
|
||||
throw error
|
||||
}
|
||||
),
|
||||
/503/
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, ['/api/sessions?limit=20&offset=0&profile=wife'])
|
||||
})
|
||||
|
||||
// A managed-SSH override can map the Desktop label to the remote's own
|
||||
// profile name (remoteProfile); the scope sent on the wire is the alias.
|
||||
test('remote session reads use the managed-SSH remoteProfile alias as the scope', async () => {
|
||||
const calls: string[] = []
|
||||
|
||||
await fetchRemoteProfileSessions(
|
||||
'mara',
|
||||
new URLSearchParams({ profile: 'mara', limit: '20', offset: '0' }),
|
||||
async (_profile, path) => {
|
||||
calls.push(path)
|
||||
|
||||
return { sessions: [], total: 0 }
|
||||
},
|
||||
{ remoteProfileAlias: 'dixie' }
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, ['/api/sessions?limit=20&offset=0&profile=dixie'])
|
||||
})
|
||||
|
||||
// Every concrete scope — `default` included — is named on the wire: a
|
||||
// multi-profile backend's launch profile is not necessarily `default`, so an
|
||||
// unscoped read could relaunch the bug for the default scope too.
|
||||
test('remote session reads for the default scope name it explicitly', async () => {
|
||||
const calls: string[] = []
|
||||
|
||||
await fetchRemoteProfileSessions(
|
||||
'default',
|
||||
new URLSearchParams({ profile: 'default', limit: '20', offset: '0' }),
|
||||
async (_profile, path) => {
|
||||
calls.push(path)
|
||||
|
||||
return { sessions: [], total: 0 }
|
||||
}
|
||||
)
|
||||
|
||||
assert.deepEqual(calls, ['/api/sessions?limit=20&offset=0&profile=default'])
|
||||
})
|
||||
|
||||
// The remote's own profile stamp is authoritative; the splice must only
|
||||
// backfill unowned rows instead of relabeling every one (#64999).
|
||||
test('remote list rows keep the remote profile stamp instead of the desktop scope label', () => {
|
||||
const rows: Array<Record<string, unknown>> = [
|
||||
{ id: 'w-1', profile: 'wife' },
|
||||
{ id: 'd-1', profile: 'dad' },
|
||||
{ id: 'legacy-1' },
|
||||
{ id: 'legacy-2', profile: '' }
|
||||
]
|
||||
|
||||
tagRemoteSessionRows(rows as unknown[], 'wife')
|
||||
|
||||
assert.equal(rows[0].profile, 'wife')
|
||||
assert.equal(rows[0].is_default_profile, false)
|
||||
assert.equal(rows[1].profile, 'dad') // NOT relabeled to wife
|
||||
assert.equal(rows[1].is_default_profile, undefined)
|
||||
assert.equal(rows[2].profile, 'wife') // unowned row backfilled
|
||||
assert.equal(rows[2].is_default_profile, false)
|
||||
assert.equal(rows[3].profile, 'wife')
|
||||
})
|
||||
|
||||
// A per-session read on a shared multi-profile backend must open the owning
|
||||
// profile's state.db — the same scope the list read sends.
|
||||
test('per-session remote reads carry the owner profile scope', () => {
|
||||
assert.equal(
|
||||
pathWithRemoteOwnerScope('/api/sessions/s-1?limit=50', 'wife'),
|
||||
'/api/sessions/s-1?limit=50&profile=wife'
|
||||
)
|
||||
assert.equal(
|
||||
pathWithRemoteOwnerScope('/api/sessions/s-1', 'wife'),
|
||||
'/api/sessions/s-1?profile=wife'
|
||||
)
|
||||
// Existing pagination params survive the scope.
|
||||
assert.equal(
|
||||
pathWithRemoteOwnerScope('/api/sessions/s-1/messages?limit=100&offset=200', 'wife'),
|
||||
'/api/sessions/s-1/messages?limit=100&offset=200&profile=wife'
|
||||
)
|
||||
// A legacy single-profile scope keeps the path bare.
|
||||
assert.equal(pathWithRemoteOwnerScope('/api/sessions/s-1', ''), '/api/sessions/s-1')
|
||||
})
|
||||
|
||||
// The scope helper itself: alias wins, every concrete scope is named.
|
||||
test('remoteProfileQueryScope resolves the wire scope for a profile override', () => {
|
||||
assert.equal(remoteProfileQueryScope('wife'), 'wife')
|
||||
assert.equal(remoteProfileQueryScope('mara', 'dixie'), 'dixie')
|
||||
assert.equal(remoteProfileQueryScope('mara', 'default'), 'mara')
|
||||
assert.equal(remoteProfileQueryScope('default'), 'default')
|
||||
assert.equal(remoteProfileQueryScope('default', 'dixie'), 'dixie')
|
||||
assert.equal(remoteProfileQueryScope(''), '')
|
||||
})
|
||||
|
||||
@@ -4,6 +4,21 @@ interface SessionListResponse {
|
||||
[key: string]: unknown
|
||||
}
|
||||
|
||||
/** HTTP status an error carries (the REST helpers stamp `err.statusCode`), NaN when none. */
|
||||
function httpStatusOf(error: unknown): number {
|
||||
return Number(error && typeof error === 'object' ? (error as { statusCode?: unknown }).statusCode : NaN)
|
||||
}
|
||||
|
||||
/** True when a remote rejected a profile-scoped read because it cannot serve
|
||||
* that scope: 400 (invalid profile param) or 404 (profile does not exist).
|
||||
* Auth (401/403), transport, and 5xx failures are real errors — retrying
|
||||
* those would just relabel the failure. */
|
||||
export function isRemoteProfileScopeError(error: unknown): boolean {
|
||||
const status = httpStatusOf(error)
|
||||
|
||||
return status === 400 || status === 404
|
||||
}
|
||||
|
||||
export interface ProfileSessionsResponse extends SessionListResponse {
|
||||
profile_totals: Record<string, number>
|
||||
}
|
||||
@@ -367,13 +382,120 @@ export function spliceRegistrySessionRows(
|
||||
return { added }
|
||||
}
|
||||
|
||||
/**
|
||||
* The remote-profile query scope for a per-profile override read: the remote
|
||||
* alias when the override is a managed SSH connection with a configured
|
||||
* `remoteProfile` (the remote knows THAT name, not the Desktop label), else
|
||||
* the Desktop profile name itself. Empty only for an empty profile — every
|
||||
* concrete scope, `default` included, is named on the wire so a multi-profile
|
||||
* backend opens that profile's state.db instead of its launch profile's.
|
||||
*/
|
||||
export function remoteProfileQueryScope(profile: string, remoteProfileAlias?: null | string): string {
|
||||
const configured = String(remoteProfileAlias || '').trim()
|
||||
|
||||
if (configured && configured !== 'default') {
|
||||
return configured
|
||||
}
|
||||
|
||||
return String(profile || '').trim()
|
||||
}
|
||||
|
||||
/** Options for {@link fetchRemoteProfileSessions}. */
|
||||
export interface RemoteProfileSessionsOptions {
|
||||
/** Managed-SSH `remoteProfile` mapping, when the remote knows the profile
|
||||
* under a different name than the Desktop label. */
|
||||
remoteProfileAlias?: null | string
|
||||
}
|
||||
|
||||
/**
|
||||
* #64999: stamp a remote session list's rows WITHOUT manufacturing labels.
|
||||
* The remote's own `profile` stamp is authoritative — overwriting it with the
|
||||
* Desktop connection-scope name relabeled rows from the backend's other
|
||||
* profiles (one remote URL shared by `wife` and `dad` scopes showed the launch
|
||||
* profile's sessions under both). The legacy label only backfills rows an
|
||||
* older remote returned unowned.
|
||||
*
|
||||
* Mutates and returns `rows` in place, mirroring the old splice behavior.
|
||||
*/
|
||||
export function tagRemoteSessionRows(rows: unknown[], scope: string): unknown[] {
|
||||
for (const row of rows) {
|
||||
if (!row || typeof row !== 'object') {
|
||||
continue
|
||||
}
|
||||
|
||||
const session = row as Record<string, unknown>
|
||||
const owned = typeof session.profile === 'string' && session.profile.trim() !== ''
|
||||
|
||||
if (!owned) {
|
||||
session.profile = scope
|
||||
}
|
||||
|
||||
if (session.profile === scope) {
|
||||
session.is_default_profile = false
|
||||
}
|
||||
}
|
||||
|
||||
return rows
|
||||
}
|
||||
|
||||
/**
|
||||
* #64999: a per-session read/mutation against a per-profile override must be
|
||||
* scoped for a multi-profile remote (an unscoped /api/sessions/{id} opens the
|
||||
* backend's launch-profile state.db, so a resume 4007s even though the row
|
||||
* exists — under its real owner). Returns the path with the owner scope
|
||||
* applied; a legacy single-profile scope (`''`) keeps the path bare.
|
||||
*/
|
||||
export function pathWithRemoteOwnerScope(path: string, scope: string): string {
|
||||
const scoped = String(scope || '').trim()
|
||||
|
||||
if (!scoped) {
|
||||
return path
|
||||
}
|
||||
|
||||
const url = new URL(path, 'http://hermes.local')
|
||||
url.searchParams.set('profile', scoped)
|
||||
|
||||
return `${url.pathname}${url.search}${url.hash}`
|
||||
}
|
||||
|
||||
export async function fetchRemoteProfileSessions(
|
||||
profile: string,
|
||||
searchParams: URLSearchParams,
|
||||
fetchJsonForProfile: FetchJsonForProfile
|
||||
fetchJsonForProfile: FetchJsonForProfile,
|
||||
options: RemoteProfileSessionsOptions = {}
|
||||
): Promise<SessionListResponse> {
|
||||
const params = new URLSearchParams(searchParams)
|
||||
params.delete('profile') // the remote serves its own database
|
||||
// #64999: a per-profile override can point at a MULTI-profile backend — one
|
||||
// `hermes serve` hosting several profiles — and an unscoped /api/sessions
|
||||
// reads whichever profile the backend process was launched under. Name the
|
||||
// scope so the rows provably belong to it; the remote's own stamps then
|
||||
// carry the authoritative identity (main.ts no longer relabels).
|
||||
const scope = remoteProfileQueryScope(profile, options.remoteProfileAlias)
|
||||
|
||||
const fetchPage = async (pageParams: URLSearchParams): Promise<SessionListResponse> => {
|
||||
pageParams.delete('profile')
|
||||
|
||||
if (scope) {
|
||||
pageParams.set('profile', scope)
|
||||
|
||||
try {
|
||||
return (await fetchJsonForProfile(profile, `/api/sessions?${pageParams}`)) as SessionListResponse
|
||||
} catch (error) {
|
||||
// A remote that rejects the scope (400 unknown profile / 404 profile
|
||||
// does not exist) proves it serves a single launch profile natively —
|
||||
// the legacy shape this code was written for. Fall back to the
|
||||
// remote's own database exactly as before; auth/transport/5xx errors
|
||||
// are real failures and propagate.
|
||||
if (!isRemoteProfileScopeError(error)) {
|
||||
throw error
|
||||
}
|
||||
|
||||
pageParams.delete('profile')
|
||||
}
|
||||
}
|
||||
|
||||
return (await fetchJsonForProfile(profile, `/api/sessions?${pageParams}`)) as SessionListResponse
|
||||
}
|
||||
|
||||
const requestedLimit = Number(params.get('limit'))
|
||||
const requestedOffset = Number(params.get('offset') || '0')
|
||||
@@ -385,7 +507,7 @@ export async function fetchRemoteProfileSessions(
|
||||
requestedOffset >= 0
|
||||
|
||||
if (!needsPaging) {
|
||||
return (await fetchJsonForProfile(profile, `/api/sessions?${params}`)) as SessionListResponse
|
||||
return fetchPage(params)
|
||||
}
|
||||
|
||||
const sessions: unknown[] = []
|
||||
@@ -402,7 +524,7 @@ export async function fetchRemoteProfileSessions(
|
||||
pageParams.set('limit', String(pageLimit))
|
||||
pageParams.set('offset', String(pageOffset))
|
||||
|
||||
const page = (await fetchJsonForProfile(profile, `/api/sessions?${pageParams}`)) as SessionListResponse
|
||||
const page = await fetchPage(pageParams)
|
||||
firstPage ??= page
|
||||
|
||||
const total = nonNegativeNumber(page.total)
|
||||
|
||||
Reference in New Issue
Block a user