diff --git a/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx b/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx index 03fb2ce109..c9a041dfe4 100644 --- a/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx +++ b/packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx @@ -4355,6 +4355,204 @@ describe('DaemonSessionProvider', () => { ).not.toHaveBeenCalled(); }); + it('keeps workspace skill slash commands after clearing so /review still autocompletes', async () => { + const firstSession = createMockSession({ + sessionId: 'session-a', + supportedCommands: vi.fn(async () => ({ + v: 1 as const, + sessionId: 'session-a', + availableCommands: [], + availableSkills: ['review'], + })), + }); + sdkMocks.sessions.push(firstSession); + let actions: DaemonSessionActions | undefined; + let connection: DaemonConnectionState | undefined; + + function Harness() { + actions = useDaemonActions(); + connection = useDaemonConnection(); + return null; + } + + await renderWithProvider(, { + autoConnect: true, + sessionId: 'session-a', + }); + await act(async () => { + await flushPromises(); + }); + expect(connection?.commands?.map((command) => command.name)).toContain( + 'review', + ); + expect(connection?.skills).toContain('review'); + + await act(async () => { + await actions?.clearSession(); + }); + for (let i = 0; i < 10 && connection?.sessionId !== undefined; i++) { + await act(async () => { + await wait(5); + await flushPromises(); + }); + } + + // Clearing returns to the deferred pre-first-prompt state (no sessionId), + // but the workspace-scoped skill command must survive so '/rev' + Tab keeps + // completing '/review' in the new session before its first prompt creates a + // session (matches the initial deferred-connect guarantee from #6153). + expect(connection).not.toHaveProperty('sessionId'); + expect(connection?.commands?.map((command) => command.name)).toContain( + 'review', + ); + expect(connection?.skills).toContain('review'); + // The session-scoped raw snapshots are still dropped so the next session + // refetches instead of reusing stale metadata. + expect(connection).not.toHaveProperty('supportedCommands'); + expect(connection).not.toHaveProperty('context'); + }); + + it('drops preserved commands when the next session reports an empty list', async () => { + const firstSession = createMockSession({ + sessionId: 'session-a', + supportedCommands: vi.fn(async () => ({ + v: 1 as const, + sessionId: 'session-a', + availableCommands: [ + { name: 'cmd-a', description: 'Command A', input: null }, + ], + availableSkills: ['review'], + })), + }); + const emptySession = createMockSession({ + sessionId: 'session-b', + supportedCommands: vi.fn(async () => ({ + v: 1 as const, + sessionId: 'session-b', + availableCommands: [], + availableSkills: [], + })), + }); + // session-a loads on mount; the detached session created after the clear + // pops session-b next. + sdkMocks.sessions.push(firstSession, emptySession); + let actions: DaemonSessionActions | undefined; + let connection: DaemonConnectionState | undefined; + + function Harness() { + actions = useDaemonActions(); + connection = useDaemonConnection(); + return null; + } + + await renderWithProvider(, { + autoConnect: true, + sessionId: 'session-a', + }); + await act(async () => { + await flushPromises(); + }); + expect(connection?.commands?.map((command) => command.name)).toContain( + 'cmd-a', + ); + + await act(async () => { + await actions?.clearSession(); + }); + // The preserved list keeps autocompleting through the clear (commands is + // still present here)... + expect(connection?.commands?.map((command) => command.name)).toContain( + 'cmd-a', + ); + + // ...but once the next session's supported-commands fetch comes back empty, + // that fulfilled snapshot is authoritative — the stale commands must not + // survive it. + const providerActions = requireActions(actions); + await act(async () => { + await providerActions.createSession(); + }); + let attach: Promise | undefined; + act(() => { + attach = providerActions.attachSession(); + }); + await act(async () => { + await flushPromises(); + }); + await attach; + + expect(connection).toMatchObject({ sessionId: 'session-b' }); + expect(connection?.commands).toEqual([]); + expect(connection?.skills).toEqual([]); + }); + + it('keeps preserved commands when the next supported-commands fetch fails', async () => { + const firstSession = createMockSession({ + sessionId: 'session-a', + supportedCommands: vi.fn(async () => ({ + v: 1 as const, + sessionId: 'session-a', + availableCommands: [ + { name: 'cmd-a', description: 'Command A', input: null }, + ], + availableSkills: ['review'], + })), + }); + const failingSession = createMockSession({ + sessionId: 'session-b', + supportedCommands: vi.fn(async () => { + throw new Error('supported-commands unavailable'); + }), + }); + sdkMocks.sessions.push(firstSession, failingSession); + let actions: DaemonSessionActions | undefined; + let connection: DaemonConnectionState | undefined; + + function Harness() { + actions = useDaemonActions(); + connection = useDaemonConnection(); + return null; + } + + await renderWithProvider(, { + autoConnect: true, + sessionId: 'session-a', + }); + await act(async () => { + await flushPromises(); + }); + expect(connection?.commands?.map((command) => command.name)).toContain( + 'cmd-a', + ); + + await act(async () => { + await actions?.clearSession(); + }); + + // A skipped or failed fetch is not authoritative — unlike a fulfilled empty + // snapshot it must not clobber the preserved list. supportedCommands() + // rejecting here leaves supportedCommands === undefined, so the commands + // survive rather than being wiped. + const providerActions = requireActions(actions); + await act(async () => { + await providerActions.createSession(); + }); + let attach: Promise | undefined; + act(() => { + attach = providerActions.attachSession(); + }); + await act(async () => { + await flushPromises(); + }); + await attach; + + expect(connection).toMatchObject({ sessionId: 'session-b' }); + expect(connection?.commands?.map((command) => command.name)).toContain( + 'cmd-a', + ); + expect(connection?.skills).toContain('review'); + }); + it('ignores streamed events from a session after it is cleared', async () => { const streamStarted = createDeferred(); const releaseOldEvent = createDeferred(); diff --git a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx index ab974faa96..32050c2a00 100644 --- a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx +++ b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx @@ -840,8 +840,16 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { ? { clientId: activeSession.clientId } : {}), workspaceCwd: activeSession.workspaceCwd, - commands: commands.length > 0 ? commands : current.commands, - skills: skills.length > 0 ? skills : current.skills, + // A fulfilled supported-commands fetch is authoritative even when + // it returns an empty list: fall back to the preserved + // `current.commands` only when the fetch was skipped or failed + // (supportedCommands === undefined). Keying on length instead + // would let a genuinely-empty snapshot leave a stale command list + // in place (see getConnectionAfterSessionClear, which now + // preserves commands across a clear). + commands: + supportedCommands !== undefined ? commands : current.commands, + skills: supportedCommands !== undefined ? skills : current.skills, models: sessionModels.length > 0 ? sessionModels : current.models, currentModel: sessionCurrentModel ?? current.currentModel, currentMode: currentMode ?? current.currentMode, diff --git a/packages/webui/src/daemon/session/actions.test.ts b/packages/webui/src/daemon/session/actions.test.ts index 02c86ad911..01c090b6b4 100644 --- a/packages/webui/src/daemon/session/actions.test.ts +++ b/packages/webui/src/daemon/session/actions.test.ts @@ -43,6 +43,36 @@ describe('getConnectionAfterSessionClear', () => { expect(next).not.toHaveProperty('clientId'); expect(next).not.toHaveProperty('displayName'); expect(next).not.toHaveProperty('tokenCount'); + expect(next).not.toHaveProperty('supportedCommands'); + expect(next).not.toHaveProperty('context'); + // Workspace-scoped slash commands and skills survive a clear so skill-backed + // commands (e.g. /review) keep autocompleting in the fresh deferred session + // before its first prompt creates a session (mirrors #6153 / #6066). + expect(next.commands).toEqual([commandInfo('old-command')]); + expect(next.skills).toEqual(['old-skill']); + }); + + it('handles commands and skills being undefined before clear', () => { + // Optional fields: clearing before the first available_commands_update + // (open the app, immediately start a new chat) leaves them absent. The + // delete calls are harmless no-ops and nothing is fabricated. + const next = getConnectionAfterSessionClear( + { + status: 'disconnected', + workspaceCwd: '/workspace', + sessionId: 'session-a', + clientId: 'client-a', + supportedCommands: supportedCommandsStatus('session-a'), + context: contextStatus('session-a'), + } as DaemonConnectionState, + 'session-a', + ); + + expect(next).toMatchObject({ + status: 'connected', + workspaceCwd: '/workspace', + }); + expect(next).not.toHaveProperty('sessionId'); expect(next).not.toHaveProperty('commands'); expect(next).not.toHaveProperty('skills'); expect(next).not.toHaveProperty('supportedCommands'); diff --git a/packages/webui/src/daemon/session/actions.ts b/packages/webui/src/daemon/session/actions.ts index aa7de39477..efa47f2299 100644 --- a/packages/webui/src/daemon/session/actions.ts +++ b/packages/webui/src/daemon/session/actions.ts @@ -81,10 +81,19 @@ export function getConnectionAfterSessionClear( delete next.displayName; delete next.tokenUsage; delete next.tokenCount; - delete next.commands; - delete next.skills; + // Drop the session-scoped raw snapshots (both carry the cleared + // sessionId), which also makes the effect's canReuseSessionMetadata + // check refetch fresh data for the next session. delete next.supportedCommands; delete next.context; + // Keep `commands`/`skills`: they are workspace-scoped (skills, custom, + // MCP-prompt and workflow slash commands all live at the workspace/config + // level, not the session), so they stay valid after the session is + // cleared. Clearing starts a fresh deferred session that is not created + // until the first prompt (#6066); preserving these keeps skill-backed + // slash commands like /review autocompleting in that window — the same + // guarantee #6153 added for the initial deferred connect. The next + // session's available_commands_update refreshes them once it lands. } return { ...next, diff --git a/packages/webui/src/daemon/session/mappers.test.ts b/packages/webui/src/daemon/session/mappers.test.ts index ef76b292be..9e1ca914a1 100644 --- a/packages/webui/src/daemon/session/mappers.test.ts +++ b/packages/webui/src/daemon/session/mappers.test.ts @@ -13,7 +13,38 @@ import { getReplayTokenCount, getReplayTokenUsage, mapWorkspaceSkills, + updateConnectionFromDaemonEvent, } from './mappers.js'; +import type { DaemonConnectionState } from './types.js'; + +function availableCommandsEvent( + availableCommands: Array>, + availableSkills: string[], +): DaemonEvent { + return { + id: 1, + v: 1, + type: 'session_update', + data: { + update: { + sessionUpdate: 'available_commands_update', + availableCommands, + availableSkills, + }, + }, + } as DaemonEvent; +} + +function applyEvent( + current: DaemonConnectionState, + event: DaemonEvent, +): DaemonConnectionState { + let next = current; + updateConnectionFromDaemonEvent(event, (update) => { + next = typeof update === 'function' ? update(next) : update; + }); + return next; +} function usageEvent( id: number, @@ -217,3 +248,42 @@ describe('mapWorkspaceSkills', () => { ]); }); }); + +describe('updateConnectionFromDaemonEvent', () => { + it('replaces commands and skills from an available_commands_update', () => { + const next = applyEvent( + { status: 'connected', workspaceCwd: '/workspace' }, + availableCommandsEvent( + [{ name: 'review', description: 'Review a PR', input: null }], + ['review'], + ), + ); + + expect(next.commands?.map((command) => command.name)).toEqual(['review']); + expect(next.skills).toEqual(['review']); + }); + + it('clears stale commands when the update reports an empty list', () => { + // The daemon snapshot is authoritative: a list that shrank to empty must + // not leave the previous commands autocompleting. Keying on length would + // preserve the stale entries. + const next = applyEvent( + { + status: 'connected', + workspaceCwd: '/workspace', + commands: [ + { + name: 'review', + description: '', + raw: { name: 'review', description: '', input: null }, + }, + ], + skills: ['review'], + }, + availableCommandsEvent([], []), + ); + + expect(next.commands).toEqual([]); + expect(next.skills).toEqual([]); + }); +}); diff --git a/packages/webui/src/daemon/session/mappers.ts b/packages/webui/src/daemon/session/mappers.ts index 3d9a4df44e..53b21dd0fd 100644 --- a/packages/webui/src/daemon/session/mappers.ts +++ b/packages/webui/src/daemon/session/mappers.ts @@ -242,9 +242,14 @@ export function updateConnectionFromDaemonEvent( } if (getString(update, 'sessionUpdate') === 'available_commands_update') { const { commands, skills } = mapAvailableCommandsUpdate(update); + // An available_commands_update is the daemon's authoritative snapshot of + // the current slash commands, so assign it directly (matching `skills`) + // rather than keeping the previous list when it is empty — otherwise a + // command list that shrank to empty would leave stale entries + // autocompleting. setConnection((current) => ({ ...current, - commands: commands.length > 0 ? commands : current.commands, + commands, skills, })); }