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
78 changes: 78 additions & 0 deletions packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ const sdkMocks = vi.hoisted(() => {
sessions,
capabilities,
workspaceProviders,
workspaceSkills,
MockDaemonClient,
MockDaemonSessionClient,
workspaceMcpTools,
Expand Down Expand Up @@ -423,6 +424,54 @@ describe('DaemonSessionProvider', () => {
expect(connection).not.toHaveProperty('sessionId');
});

it('populates skill slash commands during deferred connect (before first prompt)', async () => {
sdkMocks.workspaceProviders.mockResolvedValueOnce({
v: 1,
workspaceCwd: '/mock-workspace',
initialized: true,
providers: [],
});
sdkMocks.workspaceSkills.mockResolvedValueOnce({
v: 1,
workspaceCwd: '/mock-workspace',
initialized: true,
skills: [
{
kind: 'skill',
status: 'ok',
name: 'review',
description: 'Review a GitHub pull request',
level: 'bundled',
modelInvocable: true,
},
],
});
let connection: DaemonConnectionState | undefined;

function Harness() {
connection = useDaemonConnection();
return null;
}

await renderWithProvider(<Harness />, {
autoConnect: true,
sessionId: undefined,
});

expect(
sdkMocks.MockDaemonSessionClient.createOrAttach,
).not.toHaveBeenCalled();
expect(connection?.status).toBe('connected');
expect(connection).not.toHaveProperty('sessionId');
expect(connection?.skills).toEqual(['review']);
expect(connection?.commands).toEqual([
expect.objectContaining({
name: 'review',
description: 'Review a GitHub pull request',
}),
]);
});

it('warns when deferred workspace providers fail', async () => {
const error = new Error('providers unavailable');
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
Expand Down Expand Up @@ -450,6 +499,35 @@ describe('DaemonSessionProvider', () => {
);
});

it('warns when deferred workspace skills fail', async () => {
const error = new Error('skills unavailable');
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
sdkMocks.workspaceSkills.mockRejectedValueOnce(error);
let connection: DaemonConnectionState | undefined;

function Harness() {
connection = useDaemonConnection();
return null;
}

await renderWithProvider(<Harness />, {
autoConnect: true,
sessionId: undefined,
});

// Skills failing must not block the deferred connect: providers still
// resolve and the connection reports connected, just without skill commands.
expect(connection).toMatchObject({
status: 'connected',
workspaceCwd: '/mock-workspace',
});
expect(connection).not.toHaveProperty('commands');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The "warns when deferred workspace skills fail" test asserts not.toHaveProperty('commands') but does not symmetrically assert not.toHaveProperty('skills'). Both properties derive from the same mapWorkspaceSkills(undefined) call — adding the missing assertion guards against a future regression that accidentally sets skills when the fetch fails.

Suggested change
expect(connection).not.toHaveProperty('commands');
expect(connection).not.toHaveProperty('commands');
expect(connection).not.toHaveProperty('skills');

— qwen3.7-max via Qwen Code /review

expect(warn).toHaveBeenCalledWith(
'[DaemonSessionProvider] workspaceSkills failed in deferred connect:',
error,
);
});

it('preserves a concurrently created session during deferred connect', async () => {
const providers = createDeferred<unknown>();
sdkMocks.workspaceProviders.mockReturnValueOnce(providers.promise);
Expand Down
38 changes: 33 additions & 5 deletions packages/webui/src/daemon/session/DaemonSessionProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
mapProviderStatus,
mapSessionContextModels,
mapSupportedCommands,
mapWorkspaceSkills,
updateConnectionFromDaemonEvent,
} from './mappers.js';
import {
Expand Down Expand Up @@ -421,20 +422,41 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {
!reconnectSessionId &&
!shouldCreateFreshSession
) {
const providerResult = await Promise.allSettled([
// Fetch skills alongside providers so skill-backed slash
// commands (e.g. /review) can autocomplete before the first
// prompt. Both are session-less workspace queries; the
// session-scoped supported-commands snapshot (which also carries
// custom/MCP/workflow commands) still lands once the first prompt
// creates a session.
const [providerResult, skillsResult] = await Promise.allSettled([
client.workspaceProviders(),
client.workspaceSkills(),
]);
if (providerResult[0].status === 'rejected') {
if (providerResult.status === 'rejected') {
console.warn(
'[DaemonSessionProvider] workspaceProviders failed in deferred connect:',
providerResult[0].reason,
providerResult.reason,
);
}
if (skillsResult.status === 'rejected') {
Comment thread
wenshao marked this conversation as resolved.
console.warn(
'[DaemonSessionProvider] workspaceSkills failed in deferred connect:',
skillsResult.reason,
);
}
const providers =
providerResult[0].status === 'fulfilled'
? providerResult[0].value
providerResult.status === 'fulfilled'
? providerResult.value
: undefined;
const providerModelStatus = mapProviderStatus(providers);
const {
commands: deferredSkillCommands,
skills: deferredSkills,
} = mapWorkspaceSkills(
skillsResult.status === 'fulfilled'
? skillsResult.value
: undefined,
);
setConnection((current) => ({
...current,
status: 'connected',
Expand All @@ -445,6 +467,12 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) {
contextWindow: providerModelStatus.contextWindow,
providers,
capabilities: caps,
...(deferredSkillCommands.length > 0
? { commands: deferredSkillCommands }
: {}),
...(deferredSkills.length > 0
? { skills: deferredSkills }
: {}),
}));
return;
}
Expand Down
71 changes: 69 additions & 2 deletions packages/webui/src/daemon/session/mappers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,15 @@
*/

import { describe, expect, it } from 'vitest';
import type { DaemonEvent } from '@qwen-code/sdk/daemon';
import { getReplayTokenCount, getReplayTokenUsage } from './mappers.js';
import type {
DaemonEvent,
DaemonWorkspaceSkillsStatus,
} from '@qwen-code/sdk/daemon';
import {
getReplayTokenCount,
getReplayTokenUsage,
mapWorkspaceSkills,
} from './mappers.js';

function usageEvent(
id: number,
Expand Down Expand Up @@ -142,3 +149,63 @@ describe('getReplayTokenCount', () => {
).toBe(500);
});
});

describe('mapWorkspaceSkills', () => {
it('returns empty commands and skills for undefined status', () => {
expect(mapWorkspaceSkills(undefined)).toEqual({ commands: [], skills: [] });
});

it('maps workspace skills into skill slash commands', () => {
const status: DaemonWorkspaceSkillsStatus = {
v: 1,
workspaceCwd: '/ws',
initialized: true,
skills: [
{
kind: 'skill',
status: 'ok',
name: 'review',
description: 'Review a GitHub pull request',
level: 'bundled',
modelInvocable: true,
argumentHint: '<pr-number>',
},
{
kind: 'skill',
status: 'ok',
name: 'deep-research',
description: '',
level: 'bundled',
modelInvocable: true,
},
],
};

const result = mapWorkspaceSkills(status);

expect(result.skills).toEqual(['review', 'deep-research']);
expect(result.commands).toEqual([
{
name: 'review',
description: 'Review a GitHub pull request',
argumentHint: '<pr-number>',
raw: {
name: 'review',
description: 'Review a GitHub pull request',
input: { hint: '<pr-number>' },
_meta: { source: 'skill' },
},
},
{
name: 'deep-research',
description: '',
raw: {
name: 'deep-research',
description: '',
input: null,
_meta: { source: 'skill' },
},
},
]);
});
});
38 changes: 38 additions & 0 deletions packages/webui/src/daemon/session/mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
DaemonSessionContextStatus,
DaemonSessionSupportedCommandsStatus,
DaemonWorkspaceProvidersStatus,
DaemonWorkspaceSkillsStatus,
} from '@qwen-code/sdk/daemon';
import type {
DaemonCommandInfo,
Expand Down Expand Up @@ -161,6 +162,43 @@ export function mapSupportedCommands(
};
}

/**
* Maps the session-less `/workspace/skills` status into slash-command entries.
*
* Session creation is deferred until the first prompt, so before any session
* exists the only way to populate skill-backed slash commands (e.g. `/review`)
* is this workspace-level status, which the daemon answers from `Config`'s
* SkillManager without a live session. The shape mirrors the skills portion of
* {@link mapSupportedCommands} so the deferred bootstrap and the post-attach
* snapshot stay consistent — except workspace status carries real descriptions
* and argument hints, which we surface here.
*/
export function mapWorkspaceSkills(
status: DaemonWorkspaceSkillsStatus | undefined,
): {
commands: DaemonCommandInfo[];
skills: string[];
} {
if (!status) return { commands: [], skills: [] };

const commands = status.skills.map((skill) => ({
name: skill.name,
Comment thread
wenshao marked this conversation as resolved.
description: skill.description || '',
...(skill.argumentHint ? { argumentHint: skill.argumentHint } : {}),
raw: {
name: skill.name,
description: skill.description || '',
input: skill.argumentHint ? { hint: skill.argumentHint } : null,
_meta: { source: 'skill' },
} satisfies DaemonAvailableCommand,
}));

return {
commands,
skills: status.skills.map((skill) => skill.name),
};
}

export function mergeCommands(
...groups: DaemonCommandInfo[][]
): DaemonCommandInfo[] {
Expand Down
Loading