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
198 changes: 198 additions & 0 deletions packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Harness />, {
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(<Harness />, {
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<void> | 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(<Harness />, {
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<void> | 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<void>();
const releaseOldEvent = createDeferred<void>();
Expand Down
12 changes: 10 additions & 2 deletions packages/webui/src/daemon/session/DaemonSessionProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment thread
wenshao marked this conversation as resolved.
Resolved
currentModel: sessionCurrentModel ?? current.currentModel,
currentMode: currentMode ?? current.currentMode,
Expand Down
30 changes: 30 additions & 0 deletions packages/webui/src/daemon/session/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Comment thread
wenshao marked this conversation as resolved.
Resolved
});

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');
Expand Down
13 changes: 11 additions & 2 deletions packages/webui/src/daemon/session/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
wenshao marked this conversation as resolved.
Resolved
}
return {
...next,
Expand Down
70 changes: 70 additions & 0 deletions packages/webui/src/daemon/session/mappers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,38 @@ import {
getReplayTokenCount,
getReplayTokenUsage,
mapWorkspaceSkills,
updateConnectionFromDaemonEvent,
} from './mappers.js';
import type { DaemonConnectionState } from './types.js';

function availableCommandsEvent(
availableCommands: Array<Record<string, unknown>>,
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,
Expand Down Expand Up @@ -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([]);
});
});
7 changes: 6 additions & 1 deletion packages/webui/src/daemon/session/mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
}
Expand Down
Loading