Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions apps/desktop/src/app/session/hooks/default-new-session.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ vi.mock('@/store/gateway', async original => ({
retainGatewayForAgent: vi.fn(async () => () => undefined)
}))

// Routed session.create dials are user gestures (send / "New session"), so the
// hook tags them foreground (#105104); the two undefineds are timeout/signal.
const FOREGROUND_CREATE_DIAL = [undefined, undefined, { spawnPriority: 'foreground' }] as const

function mountActions() {
const ref = <T,>(current: T) => ({ current })
const requestGateway = vi.fn(async () => ({ session_id: 'ambient', stored_session_id: 'ambient-stored' }) as never)
Expand Down Expand Up @@ -193,7 +197,8 @@ describe('generic new session default routing', () => {
'peer-host',
'peer-agent',
'session.create',
expect.objectContaining({ profile: 'peer-agent' })
expect.objectContaining({ profile: 'peer-agent' }),
...FOREGROUND_CREATE_DIAL
)
})

Expand All @@ -216,7 +221,8 @@ describe('generic new session default routing', () => {
connectionId,
profile,
'session.create',
expect.objectContaining({ profile })
expect.objectContaining({ profile }),
...FOREGROUND_CREATE_DIAL
)
expect(getSessionOwnerHint('created-stored')).toEqual({ connectionId, profile })
}
Expand All @@ -239,7 +245,8 @@ describe('generic new session default routing', () => {
expected.connectionId,
expected.profile,
'session.create',
expect.objectContaining({ profile: expected.profile })
expect.objectContaining({ profile: expected.profile }),
...FOREGROUND_CREATE_DIAL
)
expect(getSessionOwnerHint('created-stored')).toEqual(expected)
expect($sessions.get().find(row => row.id === 'created-stored')).toMatchObject({
Expand Down
10 changes: 8 additions & 2 deletions apps/desktop/src/app/session/hooks/use-session-actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { createClientSessionState } from '@/lib/chat-runtime'
import { $clarifyRequests, clearClarifyRequest, setClarifyRequest } from '@/store/clarify'
import { clearSessionDraft, stashSessionDraft, takeSessionDraft } from '@/store/composer'
import { requestGatewayForAgent, requestGatewayForProfile } from '@/store/gateway'
import { requestGatewayForAgent, requestGatewayForProfile, retainGatewayForAgent } from '@/store/gateway'
import { $pinnedSessionIds } from '@/store/layout'
import {
$activeGatewayProfile,
Expand Down Expand Up @@ -893,8 +893,14 @@ describe('createBackendSessionForSend profile routing', () => {
'source-a',
'default',
'session.create',
expect.objectContaining({ profile: 'backend-default', source: 'desktop' })
expect.objectContaining({ profile: 'backend-default', source: 'desktop' }),
undefined,
undefined,
// #105104 / #105390: first send on a fresh chat is a user gesture; the
// create dial must not queue behind background roster hydration.
{ spawnPriority: 'foreground' }
)
expect(retainGatewayForAgent).toHaveBeenCalledWith('source-a', 'default', { spawnPriority: 'foreground' })
expect(ambientRequest).not.toHaveBeenCalledWith('session.create', expect.anything())
})

Expand Down
27 changes: 22 additions & 5 deletions apps/desktop/src/app/session/hooks/use-session-actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,15 @@ export function useSessionActions({
// foreground hold below takes over from that point until the created
// chat is selected. Between the two, nothing may close the socket
// that just minted the runtime.
//
// 'foreground' spawn priority (#102281 primitive): this is the user
// hitting send on a fresh chat, so a cold spawn must not queue behind
// background roster hydration on a saturated pool. The retain is the
// first dial, so it carries the tag as well as the create RPC.
const releaseCreateLease = capturedRoute
? await retainGatewayForAgent(capturedRoute.connectionId, capturedRoute.profile)
? await retainGatewayForAgent(capturedRoute.connectionId, capturedRoute.profile, {
spawnPriority: 'foreground'
})
: () => undefined

let created: SessionCreateResponse
Expand All @@ -622,7 +629,10 @@ export function useSessionActions({
capturedRoute.connectionId,
capturedRoute.profile,
'session.create',
params
params,
undefined,
undefined,
{ spawnPriority: 'foreground' }
)
: await requestGateway<SessionCreateResponse>('session.create', params)

Expand Down Expand Up @@ -843,9 +853,13 @@ export function useSessionActions({

// Same lease chain as createBackendSessionForSend: owner socket held
// across the create, then the foreground hold carries it until the
// tile is mounted ($sessionTiles names the owner from then on).
// tile is mounted ($sessionTiles names the owner from then on). Same
// 'foreground' spawn priority too: "New session" / tab-strip "+" is a
// direct user click, not background hydration.
const releaseCreateLease = capturedRoute
? await retainGatewayForAgent(capturedRoute.connectionId, capturedRoute.profile)
? await retainGatewayForAgent(capturedRoute.connectionId, capturedRoute.profile, {
spawnPriority: 'foreground'
})
: () => undefined

let created: SessionCreateResponse
Expand All @@ -857,7 +871,10 @@ export function useSessionActions({
capturedRoute.connectionId,
capturedRoute.profile,
'session.create',
params
params,
undefined,
undefined,
{ spawnPriority: 'foreground' }
)
: await requestGateway<SessionCreateResponse>('session.create', params)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ vi.mock('./shared', () => ({ getPluginCtx: () => null }))

/** Route every RPC through one table, recording what was asked. */
function respondWith(handler: (method: string, params: Record<string, unknown>) => unknown) {
const calls: Array<{ method: string; params: Record<string, unknown> }> = []
const calls: Array<{ method: string; options?: { spawnPriority?: string }; params: Record<string, unknown> }> = []

requestForBotMock.mockImplementation(async (_bot: unknown, method: string, params: Record<string, unknown>) => {
calls.push({ method, params: structuredClone(params ?? {}) })
requestForBotMock.mockImplementation(
async (_bot: unknown, method: string, params: Record<string, unknown>, options?: { spawnPriority?: string }) => {
calls.push({ method, options, params: structuredClone(params ?? {}) })

return handler(method, params)
})
return handler(method, params)
}
)

return calls
}
Expand Down Expand Up @@ -134,6 +136,9 @@ describe('the registry row wins, always', () => {
include_hidden: true,
title: 'Bot Chat'
})
// #105104: the click's first RPC is the one that cold-spawns the bot's
// backend; it must dial foreground or it queues behind roster hydration.
expect(list?.options).toEqual({ spawnPriority: 'foreground' })
})

it('opens the lineage tip of a compression-rotated registry row', async () => {
Expand Down Expand Up @@ -197,6 +202,10 @@ describe('no registry row → create', () => {
hidden: true,
title: 'Bot Chat'
})
// Same click gesture as the lookup: the create dials foreground too, while
// the follow-up title write stays untagged (the socket is already open).
expect(calls.find(call => call.method === 'session.create')?.options).toEqual({ spawnPriority: 'foreground' })
expect(calls.find(call => call.method === 'session.title')?.options).toBeUndefined()
// The eager title write persists the row; no user-attributed intro.
expect(calls.find(call => call.method === 'session.title')?.params).toMatchObject({ session_id: 'rt-1' })
expect(calls.find(call => call.method === 'prompt.submit')).toBeUndefined()
Expand Down
61 changes: 39 additions & 22 deletions apps/desktop/src/plugins/hermes-bots/canonical-chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,22 @@ async function findExistingCanonicalChat(owner: RosterRow | string): Promise<Can
let res: { sessions?: CanonicalChatRow[] }

try {
res = await requestForBot<{ sessions?: CanonicalChatRow[] }>(bot, 'session.list', {
profile: backendTargetProfile(route, name),
title: CANONICAL_CHAT_TITLE,
limit: PROFILE_SESSION_LIST_LIMIT,
include_hidden: true
})
// Every caller is a user gesture (roster click, Create Bot), and this is
// the FIRST RPC of the gesture — the one that cold-spawns the bot's
// backend on a local pool. Dial foreground so the click is not queued
// behind background roster hydration on a saturated pool (#105104: roster
// click, zero backend activity, "try again" toast).
res = await requestForBot<{ sessions?: CanonicalChatRow[] }>(
bot,
'session.list',
{
profile: backendTargetProfile(route, name),
title: CANONICAL_CHAT_TITLE,
limit: PROFILE_SESSION_LIST_LIMIT,
include_hidden: true
},
{ spawnPriority: 'foreground' }
)
} catch (error) {
// Plugin tests and host bridges can return Error-like values from another
// JS realm, where `instanceof Error` is false. Preserve the provider/RPC
Expand Down Expand Up @@ -390,22 +400,29 @@ export function createCanonicalChat(
return existing.id
}

const res = await requestForBot<{ session_id?: string; stored_session_id?: string }>(bot, 'session.create', {
profile: backendTargetProfile(route, name),
title: CANONICAL_CHAT_TITLE,
// Always born hidden from the global sidebar — Bot Mode sessions are
// plugin-owned. Core applies this via the generic `hidden` flag
// (deferred as pending_hidden until the row exists); older gateways
// ignore the unknown param and it stays visible.
hidden: true,
// Explicit contract (PR #97008): this session's runtime always follows
// the member profile's CURRENT config. Resume must NOT restore the
// stored model/provider pin from an old row — that left bot DMs stuck
// on a stale/dead provider after a profile switch. Older gateways
// ignore the unknown param; the server's exact-title backfill then
// covers the legacy path.
follow_profile_config: true
})
// Same click gesture as the foreground lookup above: a first-ever open
// has no row to find and mints one, still on the user's dial.
const res = await requestForBot<{ session_id?: string; stored_session_id?: string }>(
bot,
'session.create',
{
profile: backendTargetProfile(route, name),
title: CANONICAL_CHAT_TITLE,
// Always born hidden from the global sidebar — Bot Mode sessions are
// plugin-owned. Core applies this via the generic `hidden` flag
// (deferred as pending_hidden until the row exists); older gateways
// ignore the unknown param and it stays visible.
hidden: true,
// Explicit contract (PR #97008): this session's runtime always follows
// the member profile's CURRENT config. Resume must NOT restore the
// stored model/provider pin from an old row — that left bot DMs stuck
// on a stale/dead provider after a profile switch. Older gateways
// ignore the unknown param; the server's exact-title backfill then
// covers the legacy path.
follow_profile_config: true
},
{ spawnPriority: 'foreground' }
)

const sid = res?.stored_session_id
const runtime = res?.session_id
Expand Down
26 changes: 26 additions & 0 deletions apps/desktop/src/plugins/hermes-bots/routing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,32 @@ describe('requestForBot rides the bot’s own source', () => {
})
})

it('passes a foreground spawnPriority through to host.requestProfile and keeps untagged calls at three args', async () => {
// #105104: the Bot Chat open is a user click. Its RPC must reach the SDK
// with the dial tag; passive roster warming (no options) must keep the
// exact call shape older shells expect.
hostMock.requestProfile.mockResolvedValue({})
const bot = { connectionId: 'local', name: 'ops', sourceScoped: true } as RosterRow

await requestForBot(bot, 'session.list', {}, { spawnPriority: 'foreground' })
await requestForBot(bot, 'profiles.list', {})

expect(hostMock.requestProfile).toHaveBeenNthCalledWith(
1,
expect.objectContaining({ connectionId: 'local', profile: 'ops' }),
'session.list',
{},
undefined,
{ spawnPriority: 'foreground' }
)
expect(hostMock.requestProfile).toHaveBeenNthCalledWith(
2,
expect.objectContaining({ connectionId: 'local', profile: 'ops' }),
'profiles.list',
{}
)
})

it('fails closed rather than falling back to the ambient request', async () => {
// A scoped row whose shell predates requestProfile must NOT silently
// execute against whichever gateway happens to be active.
Expand Down
18 changes: 16 additions & 2 deletions apps/desktop/src/plugins/hermes-bots/routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,18 @@ export function botBackendProfileScope(route: null | ProfileRoute | undefined, f

/** Gateway RPC on the bot's OWN source. Source-scoped rows always use the
* explicit descriptor, including a registered local source. */
export interface BotRequestOptions {
/** 'foreground' for an explicit user gesture (roster click, Create Bot) so
* a cold backend spawn takes the pool's reserved slot; leave unset for
* passive roster warming. Only source-scoped routes can carry it. */
spawnPriority?: 'background' | 'foreground'
}

export async function requestForBot<T = unknown>(
bot: Partial<RosterRow> | null | undefined,
method: string,
params: Record<string, unknown> = {}
params: Record<string, unknown> = {},
options?: BotRequestOptions
): Promise<T> {
const route = botConnectionRoute(bot)

Expand All @@ -210,7 +218,13 @@ export async function requestForBot<T = unknown>(
}

try {
return await host.requestProfile(route, method, scopedBotParams(route, method, params))
const routedParams = scopedBotParams(route, method, params)

// Keep the three-argument shape when no options were given so older
// desktop shells (and the arity-pinning tests) see the same call.
return await (options?.spawnPriority
? host.requestProfile(route, method, routedParams, undefined, { spawnPriority: options.spawnPriority })
: host.requestProfile(route, method, routedParams))
} catch (error) {
// React 19 formats query errors with `(error.name || '').trim()`. IPC /
// JSON-RPC rejections are often plain objects whose `name` is a number,
Expand Down
Loading
Loading