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
106 changes: 106 additions & 0 deletions apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@
submitText: (...args: Parameters<typeof actions.submitText>) =>
act(async () => actions.submitText(...args)) as Promise<boolean>
})
}, [

Check warning on line 193 in apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

React Hook useEffect has a missing dependency: 'actions'. Either include it or remove the dependency array
actions.cancelRun,
actions.editMessage,
actions.reloadFromMessage,
Expand Down Expand Up @@ -317,6 +317,112 @@
})
}

describe('usePromptActions slash session targeting', () => {
const STORED_SESSION_ID = 'stored-db-xyz789'
const RECOVERED_SESSION_ID = 'rt-recovered-456'

afterEach(() => {
cleanup()
vi.restoreAllMocks()
})

it('runs /goal status against the ROUTED stored session instead of minting a new one', async () => {
// Teknium's report: start a goal in the desktop app, then `/goal status`
// says there is no goal. `/goal` state lives per-session in SessionDB
// (`goal:<session_id>`), and slash.ts used to resolve its target with a
// bare `hint || activeRef || createSession()`. With the runtime binding
// momentarily absent (profile swap / reconnect / orphan-reap / timeout) it
// minted a NEW session, so the status query asked a session that never had
// a goal. submit.ts already resumes the routed chat here; both pipelines
// must resolve identically.
const calls: { method: string; params?: Record<string, unknown> }[] = []
const activeSessionIdRef: MutableRefObject<string | null> = { current: null }
const selectedStoredSessionIdRef: MutableRefObject<string | null> = { current: null }
let boundRuntimeId: null | string = null

const createBackendSessionForSend = vi.fn(async () => 'rt-brand-new-WRONG')

const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
calls.push({ method, params })

if (method === 'session.resume') {
boundRuntimeId = RECOVERED_SESSION_ID
selectedStoredSessionIdRef.current = STORED_SESSION_ID
activeSessionIdRef.current = RECOVERED_SESSION_ID

return { session_id: RECOVERED_SESSION_ID } as never
}

if (method === 'slash.exec') {
return { output: '⊙ Goal (active, 1/20 turns): build a rocket' } as never
}

return {} as never
})

let handle: HarnessHandle | null = null
await actRender(
<Harness
activeSessionId={null}
activeSessionIdRef={activeSessionIdRef}
createBackendSessionForSend={createBackendSessionForSend}
getRoutedStoredSessionId={() => STORED_SESSION_ID}
getRuntimeIdForStoredSession={() => boundRuntimeId}
onReady={h => (handle = h)}
refreshSessions={async () => undefined}
requestGateway={requestGateway}
selectedStoredSessionIdRef={selectedStoredSessionIdRef}
storedSessionId={null}
/>
)

await handle!.submitText('/goal status')

// Never fork the conversation to answer a question about it.
expect(createBackendSessionForSend).not.toHaveBeenCalled()
expect(calls.map(c => c.method)).toEqual(['session.resume', 'slash.exec'])
expect(calls[0]?.params).toMatchObject({ session_id: STORED_SESSION_ID })
// The command lands on the recovered runtime that owns the goal.
expect(calls[1]?.params).toEqual({ command: 'goal status', session_id: RECOVERED_SESSION_ID })
})

it('does not fork the chat when the routed session cannot be rebound', async () => {
const calls: string[] = []
const createBackendSessionForSend = vi.fn(async () => 'rt-brand-new-WRONG')

const requestGateway = vi.fn(async (method: string) => {
calls.push(method)

if (method === 'session.resume') {
throw new Error('4007 session not found')
}

return {} as never
})

let handle: HarnessHandle | null = null
await actRender(
<Harness
activeSessionId={null}
activeSessionIdRef={{ current: null }}
createBackendSessionForSend={createBackendSessionForSend}
getRoutedStoredSessionId={() => STORED_SESSION_ID}
getRuntimeIdForStoredSession={() => null}
onReady={h => (handle = h)}
refreshSessions={async () => undefined}
requestGateway={requestGateway}
selectedStoredSessionIdRef={{ current: null }}
storedSessionId={null}
/>
)

await handle!.submitText('/goal status')

expect(createBackendSessionForSend).not.toHaveBeenCalled()
expect(calls).not.toContain('slash.exec')
})
})

describe('usePromptActions /compress', () => {
beforeEach(() => {
setSessions(() => [sessionInfo()])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,8 @@ export function usePromptActions({
busyRef,
copy,
createBackendSessionForSend,
getRoutedStoredSessionId,
getRuntimeIdForStoredSession,
handleSkinCommand,
handoffSession,
openMemoryGraph,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { describe, expect, it, vi } from 'vitest'

import { resolveTargetSessionId } from './resolve-target-session'

vi.mock('../use-session-actions/utils', () => ({
resolveSessionProfile: vi.fn(async () => 'work')
}))

const RECOVERED = 'rt-recovered'
const STORED = 'stored-goal-session'

function deps(overrides: Partial<Parameters<typeof resolveTargetSessionId>[0]> = {}) {
return {
activeRuntimeId: null,
createSession: vi.fn(async () => 'rt-brand-new'),
getRuntimeIdForStoredSession: () => null,
requestGateway: vi.fn(async () => ({ session_id: RECOVERED })) as never,
routedStoredSessionId: null,
selectedStoredSessionId: null,
...overrides
}
}

describe('resolveTargetSessionId', () => {
it('resumes the routed stored session instead of minting a new one when the runtime binding is gone', async () => {
// The exact condition behind "/goal status says No active goal": the
// runtime binding was cleared (profile swap / reconnect / orphan-reap) but
// the durable route still names the chat carrying the goal.
const createSession = vi.fn(async () => 'rt-brand-new-WRONG')
const requestGateway = vi.fn(async () => ({ session_id: RECOVERED }))

const resolved = await resolveTargetSessionId(
deps({ createSession, requestGateway: requestGateway as never, routedStoredSessionId: STORED })
)

expect(resolved).toBe(RECOVERED)
expect(createSession).not.toHaveBeenCalled()
expect(requestGateway).toHaveBeenCalledWith('session.resume', {
session_id: STORED,
source: 'desktop',
profile: 'work'
})
})

it('reuses the live runtime id when the cache confirms it owns the targeted stored session', async () => {
const createSession = vi.fn(async () => 'rt-brand-new-WRONG')
const requestGateway = vi.fn(async () => ({ session_id: 'rt-resume-WRONG' }))

const resolved = await resolveTargetSessionId(
deps({
activeRuntimeId: RECOVERED,
createSession,
getRuntimeIdForStoredSession: () => RECOVERED,
requestGateway: requestGateway as never,
selectedStoredSessionId: STORED
})
)

expect(resolved).toBe(RECOVERED)
expect(createSession).not.toHaveBeenCalled()
expect(requestGateway).not.toHaveBeenCalled()
})

it('rejects a stale runtime id the route does not bind to the targeted session', async () => {
// A stale ref left over from the previous profile must not capture the
// command — that runs it against another profile's session. The durable
// route outranks the ref here.
const resolved = await resolveTargetSessionId(
deps({
activeRuntimeId: 'rt-wrong-profile',
getRuntimeIdForStoredSession: () => null,
routedStoredSessionId: STORED,
selectedStoredSessionId: STORED
})
)

expect(resolved).toBe(RECOVERED)
})

it('honors an explicit runtime id above everything else', async () => {
const createSession = vi.fn(async () => 'rt-brand-new-WRONG')
const requestGateway = vi.fn(async () => ({ session_id: 'rt-resume-WRONG' }))

const resolved = await resolveTargetSessionId(
deps({
createSession,
explicitRuntimeId: 'rt-explicit',
requestGateway: requestGateway as never,
routedStoredSessionId: STORED
})
)

expect(resolved).toBe('rt-explicit')
expect(createSession).not.toHaveBeenCalled()
expect(requestGateway).not.toHaveBeenCalled()
})

it('creates a session only for a genuine new-chat draft', async () => {
const createSession = vi.fn(async () => 'rt-brand-new')

const resolved = await resolveTargetSessionId(deps({ createSession }))

expect(resolved).toBe('rt-brand-new')
expect(createSession).toHaveBeenCalledTimes(1)
})

it('reuses the live runtime for a new-chat draft without creating a second session', async () => {
const createSession = vi.fn(async () => 'rt-brand-new-WRONG')

const resolved = await resolveTargetSessionId(deps({ activeRuntimeId: 'rt-live', createSession }))

expect(resolved).toBe('rt-live')
expect(createSession).not.toHaveBeenCalled()
})

it('returns null rather than forking when a targeted durable session cannot be rebound', async () => {
const createSession = vi.fn(async () => 'rt-brand-new-WRONG')

const requestGateway = vi.fn(async () => {
throw new Error('4007 session not found')
})

const resolved = await resolveTargetSessionId(
deps({ createSession, requestGateway: requestGateway as never, routedStoredSessionId: STORED })
)

expect(resolved).toBeNull()
expect(createSession).not.toHaveBeenCalled()
})

it('prefers the routed stored session over a differing selected one', async () => {
const requestGateway = vi.fn(async () => ({ session_id: RECOVERED }))

await resolveTargetSessionId(
deps({
requestGateway: requestGateway as never,
routedStoredSessionId: STORED,
selectedStoredSessionId: 'stored-stale-selection'
})
)

expect(requestGateway).toHaveBeenCalledWith('session.resume', expect.objectContaining({ session_id: STORED }))
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { resolveSessionProfile } from '../use-session-actions/utils'

import type { GatewayRequest } from './utils'

/**
* Resolve the runtime session a submit or slash command must target.
*
* Both prompt pipelines need this answer and they must agree: `submit.ts`
* sends the user's text into it, and `slash.ts` runs backend commands against
* it. Per-session backend state — `/goal` (persisted in SessionDB `state_meta`
* under `goal:<session_id>`), usage, status, yolo — is keyed by that id, so a
* pipeline that resolves it differently reads and writes a DIFFERENT session's
* state than the one the user is looking at.
*
* That is the bug this exists to prevent. `slash.ts` used to resolve with a
* bare `hint || activeRef || createSession()`, so whenever the runtime binding
* was momentarily absent — profile swap, reconnect, orphan-reap, request
* timeout — a slash command silently MINTED A NEW SESSION and ran against it.
* `/goal <text>` then set a goal on the chat, and a following `/goal status`
* reported "No active goal" because it was asking a session that had just been
* created. Same hole for every exec/rpc command (`/usage`, `/status`,
* `/tools`, …), not just `/goal`.
*
* The ladder (highest-trust rung first), mirroring `submit.ts`:
*
* 1. An explicit runtime id from the caller (queue drain / tile) — always
* authoritative.
* 2. The live runtime ref. A durable ROUTE outranks it: when the URL names a
* conversation whose runtime binding the cache does not confirm, the ref
* is stale or cross-wired (often from the previous profile) and must not
* capture the command. With no route in play the ref is authoritative.
* 3. `session.resume` on the routed (else selected) stored session,
* re-registered on the profile that OWNS it — never whichever profile
* happens to be live, which forks the conversation into the wrong
* state.db (#67603).
* 4. Only a genuine new-chat draft — no durable session in play at all —
* creates a session.
*
* Returns null when a durable conversation was targeted but its runtime could
* not be rebound. Callers surface that instead of silently retargeting: a
* command that runs against the wrong session is worse than one that reports
* it could not run.
*/
export interface ResolveTargetSessionDeps {
activeRuntimeId: null | string
createSession: () => Promise<null | string>
explicitRuntimeId?: null | string
getRuntimeIdForStoredSession: (storedSessionId: string) => null | string
requestGateway: GatewayRequest
routedStoredSessionId: null | string
selectedStoredSessionId: null | string
}

export async function resolveTargetSessionId(deps: ResolveTargetSessionDeps): Promise<null | string> {
const {
activeRuntimeId,
createSession,
explicitRuntimeId,
getRuntimeIdForStoredSession,
requestGateway,
routedStoredSessionId,
selectedStoredSessionId
} = deps

// 1. An explicit target always wins — the caller knows which session it means.
if (explicitRuntimeId) {
return explicitRuntimeId
}

// A route whose runtime binding is incomplete or cross-wired outranks the
// live ref: a profile swap / reconnect can leave the previous profile's
// runtime active while the URL still names the conversation on screen.
// Matches submit.ts's `routedSessionNeedsResume`.
const routedNeedsResume = Boolean(
routedStoredSessionId &&
(selectedStoredSessionId !== routedStoredSessionId ||
!activeRuntimeId ||
activeRuntimeId !== getRuntimeIdForStoredSession(routedStoredSessionId))
)

// 2. Trust the live runtime unless the durable route disagrees with it.
if (activeRuntimeId && !routedNeedsResume) {
return activeRuntimeId
}

// 3. Rebind the durable conversation on its owning profile. The route wins
// over a stale selection; otherwise continue whatever is selected.
const storedTarget = routedNeedsResume ? routedStoredSessionId : (selectedStoredSessionId ?? routedStoredSessionId)

if (storedTarget) {
try {
const profile = await resolveSessionProfile(storedTarget)

const resumed = await requestGateway<{ session_id?: string }>('session.resume', {
session_id: storedTarget,
source: 'desktop',
...(profile ? { profile } : {})
})

return resumed?.session_id || null
} catch {
// A targeted durable conversation whose runtime cannot be rebound must
// NOT fall through to createSession() — that is precisely the fork this
// resolver exists to prevent (#55578 class).
return null
}
}

// 4. A genuine new-chat draft: nothing durable is in play.
return activeRuntimeId || (await createSession())
}
Loading
Loading