From 344ef091965ff76c2411359b20033fbdb42b0f36 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 25 Jul 2026 15:07:05 -0700 Subject: [PATCH] fix(desktop): slash commands target the user's chat, not a new session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/goal status` reported "No active goal" for a goal that was live: the desktop's slash pipeline resolved its target session differently than the submit pipeline, so the command ran against a different session than the chat on screen. `/goal` state is persisted per-session in SessionDB (`state_meta` key `goal:`). slash.ts resolved with a bare `hint || activeRef || createBackendSessionForSend()`, so whenever the runtime binding was momentarily absent — profile swap, reconnect, orphan-reap, request timeout — it silently MINTED A NEW SESSION and ran there. submit.ts already handles this case by resuming the routed stored session on its owning profile (#55578, #67603). Extract that ladder into one shared resolver both pipelines use, per the "one resolver owns each policy" rule in apps/desktop/AGENTS.md. This fixes the whole class, not just `/goal`: every exec/rpc slash command (`/usage`, `/status`, `/tools`, …) had the same hole. A targeted durable conversation whose runtime cannot be rebound now returns null instead of forking the chat — reporting that a command could not run beats running it against the wrong session. --- .../hooks/use-prompt-actions/index.test.tsx | 106 +++++++++++++ .../session/hooks/use-prompt-actions/index.ts | 2 + .../resolve-target-session.test.ts | 144 ++++++++++++++++++ .../resolve-target-session.ts | 111 ++++++++++++++ .../session/hooks/use-prompt-actions/slash.ts | 26 +++- 5 files changed, 388 insertions(+), 1 deletion(-) create mode 100644 apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.test.ts create mode 100644 apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.ts diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index 0eb73bdd98e82..c9a5e5db9db1b 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -317,6 +317,112 @@ function renderedSeedTexts(seeds: Record[]): string[] { }) } +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:`), 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 }[] = [] + const activeSessionIdRef: MutableRefObject = { current: null } + const selectedStoredSessionIdRef: MutableRefObject = { 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) => { + 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( + 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( + 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()]) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 74e93379b590a..b755fbc884d56 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -498,6 +498,8 @@ export function usePromptActions({ busyRef, copy, createBackendSessionForSend, + getRoutedStoredSessionId, + getRuntimeIdForStoredSession, handleSkinCommand, handoffSession, openMemoryGraph, diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.test.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.test.ts new file mode 100644 index 0000000000000..fe1b20d236db6 --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.test.ts @@ -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[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 })) + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.ts new file mode 100644 index 0000000000000..62c9acb90624f --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/resolve-target-session.ts @@ -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:`), 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 ` 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 + explicitRuntimeId?: null | string + getRuntimeIdForStoredSession: (storedSessionId: string) => null | string + requestGateway: GatewayRequest + routedStoredSessionId: null | string + selectedStoredSessionId: null | string +} + +export async function resolveTargetSessionId(deps: ResolveTargetSessionDeps): Promise { + 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()) +} diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts index 0edbea218b227..39e2fd668595a 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts @@ -40,6 +40,7 @@ import type { SlashExecResponse } from '../../../types' +import { resolveTargetSessionId } from './resolve-target-session' import { type GatewayRequest, isSessionIdCandidate, @@ -75,6 +76,8 @@ interface SlashCommandDeps { busyRef: MutableRefObject copy: Translations['desktop'] createBackendSessionForSend: (preview?: string | null) => Promise + getRoutedStoredSessionId: () => null | string + getRuntimeIdForStoredSession: (storedSessionId: string) => null | string handleSkinCommand: (arg: string) => string handoffSession: ( platform: string, @@ -103,6 +106,8 @@ export function useSlashCommand(deps: SlashCommandDeps) { busyRef, copy, createBackendSessionForSend, + getRoutedStoredSessionId, + getRuntimeIdForStoredSession, handleSkinCommand, handoffSession, openMemoryGraph, @@ -119,8 +124,25 @@ export function useSlashCommand(deps: SlashCommandDeps) { return useCallback( async (rawCommand: string, options?: { sessionId?: string; recordInput?: boolean }) => { + // Resolve the session this command targets through the SHARED ladder that + // submit.ts uses. A slash command runs backend commands against a runtime + // session, and per-session state (`/goal`, `/usage`, `/status`) is keyed by + // that id — so resolving it differently than submit would run the command + // against a different session than the user's chat. The old bare + // `hint || activeRef || createSession()` did exactly that: with the runtime + // binding momentarily absent (profile swap, reconnect, orphan-reap, + // timeout) it minted a NEW session, so `/goal status` reported "No active + // goal" for a goal that was live on the real chat. const ensureSessionId = async (sessionHint?: string) => - sessionHint || activeSessionIdRef.current || (await createBackendSessionForSend()) + resolveTargetSessionId({ + activeRuntimeId: activeSessionIdRef.current, + createSession: () => createBackendSessionForSend(), + explicitRuntimeId: sessionHint, + getRuntimeIdForStoredSession, + requestGateway, + routedStoredSessionId: getRoutedStoredSessionId(), + selectedStoredSessionId: selectedStoredSessionIdRef.current + }) // Resolve the target session plus a writer for inline slash output, or // notify + return null when none can be created. Folds the ensure / bail / @@ -868,6 +890,8 @@ export function useSlashCommand(deps: SlashCommandDeps) { busyRef, copy, createBackendSessionForSend, + getRoutedStoredSessionId, + getRuntimeIdForStoredSession, handleSkinCommand, handoffSession, openMemoryGraph,