From 2556729ba876faf60bdbf11e0598965a0f2c305a Mon Sep 17 00:00:00 2001 From: SSU-WEI HUANG Date: Sun, 2 Aug 2026 08:49:35 +0800 Subject: [PATCH] fix(codex): use app-server native skills (#1289) * fix(codex): use native skill catalog * fix(codex): refresh changed skill inventory * fix(codex): preserve skill fallback on discovery error * fix(web): refresh skills when completion opens --- cli/src/codex/appServerTypes.ts | 23 +++++ cli/src/codex/codexAppServerClient.ts | 9 ++ cli/src/codex/codexRemoteLauncher.test.ts | 100 ++++++++++++++++++++ cli/src/codex/codexRemoteLauncher.ts | 42 +++++++- cli/src/codex/utils/appServerConfig.test.ts | 31 ++++++ cli/src/codex/utils/appServerConfig.ts | 25 ++++- web/src/hooks/queries/useSkills.ts | 10 +- 7 files changed, 231 insertions(+), 9 deletions(-) diff --git a/cli/src/codex/appServerTypes.ts b/cli/src/codex/appServerTypes.ts index 100d9264..6791ca1f 100644 --- a/cli/src/codex/appServerTypes.ts +++ b/cli/src/codex/appServerTypes.ts @@ -62,6 +62,29 @@ export interface ModelListResponse { [key: string]: unknown; } +export interface SkillsListParams { + cwds: string[]; + forceReload?: boolean; +} + +export interface SkillMetadata { + name: string; + description: string; + path: string; + scope: string; + enabled: boolean; + [key: string]: unknown; +} + +export interface SkillsListResponse { + data?: Array<{ + cwd: string; + skills: SkillMetadata[]; + errors?: unknown[]; + }>; + [key: string]: unknown; +} + export interface CollaborationModeListItem { name?: string; mode?: 'plan' | 'default' | string | null; diff --git a/cli/src/codex/codexAppServerClient.ts b/cli/src/codex/codexAppServerClient.ts index e8ebd2e6..54411101 100644 --- a/cli/src/codex/codexAppServerClient.ts +++ b/cli/src/codex/codexAppServerClient.ts @@ -9,6 +9,8 @@ import type { InitializeResponse, ModelListParams, ModelListResponse, + SkillsListParams, + SkillsListResponse, ThreadStartParams, ThreadStartResponse, ThreadResumeParams, @@ -245,6 +247,13 @@ export class CodexAppServerClient extends JsonLineParser { return response as ModelListResponse; } + async listSkills(params: SkillsListParams): Promise { + const response = await this.sendRequest('skills/list', params, { + timeoutMs: 30_000 + }); + return response as SkillsListResponse; + } + async listCollaborationModes(): Promise { const response = await this.sendRequest('collaborationMode/list', {}, { timeoutMs: 30_000 diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index 3ee5c686..2dc6d1a0 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -13,6 +13,20 @@ const harness = vi.hoisted(() => ({ listCollaborationModeCalls: 0, collaborationModeResponse: { data: [{ mode: 'default' }, { mode: 'plan' }] } as unknown, failListCollaborationModes: false, + listSkillsCalls: [] as unknown[], + skillsListResponse: { + data: [{ + cwd: '/tmp/hapi-update', + skills: [{ + name: 'hapi', + description: 'Manage HAPI', + path: '/home/user/.agents/skills/hapi/SKILL.md', + scope: 'user', + enabled: true + }], + errors: [] + }] + } as unknown, startThreadIds: [] as string[], startThreadParams: [] as Array>, resumeThreadIds: [] as string[], @@ -100,6 +114,11 @@ vi.mock('./codexAppServerClient', () => { return harness.collaborationModeResponse; } + async listSkills(params: unknown): Promise { + harness.listSkillsCalls.push(params); + return harness.skillsListResponse; + } + async setExperimentalFeatureEnablement(params: unknown): Promise { harness.setFeatureEnablementCalls.push(params); if (harness.failSetFeatureEnablement) { @@ -1097,6 +1116,20 @@ describe('codexRemoteLauncher', () => { harness.listCollaborationModeCalls = 0; harness.collaborationModeResponse = { data: [{ mode: 'default' }, { mode: 'plan' }] }; harness.failListCollaborationModes = false; + harness.listSkillsCalls = []; + harness.skillsListResponse = { + data: [{ + cwd: '/tmp/hapi-update', + skills: [{ + name: 'hapi', + description: 'Manage HAPI', + path: '/home/user/.agents/skills/hapi/SKILL.md', + scope: 'user', + enabled: true + }], + errors: [] + }] + }; harness.startThreadIds = []; harness.startThreadParams = []; harness.resumeThreadIds = []; @@ -1192,6 +1225,73 @@ describe('codexRemoteLauncher', () => { expect(session.thinking).toBe(false); }); + it('uses the native skill catalog for completion and structured turn input', async () => { + const { session, rpcHandlers } = createSessionStub(['$hapi inspect']); + + await codexRemoteLauncher(session as never); + + expect(harness.listSkillsCalls).toEqual([{ + cwds: ['/tmp/hapi-update'], + forceReload: false + }]); + expect(Array.from(rpcHandlers.keys())).toContain('listSkills'); + expect(await rpcHandlers.get('listSkills')?.({})).toEqual({ + success: true, + skills: [{ name: 'hapi', description: 'Manage HAPI' }] + }); + expect(harness.startTurnParams[0]?.input).toEqual([ + { type: 'skill', name: 'hapi', path: '/home/user/.agents/skills/hapi/SKILL.md' }, + { type: 'text', text: ' inspect' } + ]); + }); + + it('keeps the filesystem skill handler when native discovery reports errors', async () => { + harness.skillsListResponse = { + data: [{ + cwd: '/tmp/hapi-update', + skills: [], + errors: ['failed to read skills'] + }] + }; + const { session, rpcHandlers } = createSessionStub(); + + await codexRemoteLauncher(session as never); + + expect(rpcHandlers.has('listSkills')).toBe(false); + }); + + it('reloads the native skill catalog after skills/changed', async () => { + const { session, rpcHandlers } = createSessionStub(); + + await codexRemoteLauncher(session as never); + harness.skillsListResponse = { + data: [{ + cwd: '/tmp/hapi-update', + skills: [{ + name: 'new-skill', + description: 'New skill', + path: '/tmp/new-skill/SKILL.md', + scope: 'repo', + enabled: true + }], + errors: [] + }] + }; + + harness.dispatchNotification?.('skills/changed', {}); + await vi.waitFor(() => { + expect(harness.listSkillsCalls.at(-1)).toEqual({ + cwds: ['/tmp/hapi-update'], + forceReload: true + }); + }); + + expect(await rpcHandlers.get('listSkills')?.({})).toEqual({ + success: true, + skills: [{ name: 'new-skill', description: 'New skill' }] + }); + }); + it('routes app-server MCP elicitation through the existing user-input transport', async () => { const { session, codexMessages, rpcHandlers, setPermissionMode } = createSessionStub(); diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index c39b208f..e877b202 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -17,10 +17,11 @@ import { AppServerEventConverter } from './utils/appServerEventConverter'; import { detectImageMimeType, registerGeneratedImage } from '@/modules/common/generatedImages'; import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter'; import { buildThreadStartParams, buildTurnStartParams } from './utils/appServerConfig'; -import type { ThreadGoal, ThreadGoalStatus } from './appServerTypes'; +import type { SkillMetadata, ThreadGoal, ThreadGoalStatus } from './appServerTypes'; import { shouldIgnoreTerminalEvent } from './utils/terminalEventGuard'; import { parseCodexSpecialCommand } from './codexSpecialCommands'; import { extractErrorInfo } from '@/utils/errorUtils'; +import { RPC_METHODS } from '@hapi/protocol/rpcMethods'; import { RemoteLauncherBase, type RemoteLauncherDisplayContext, @@ -3086,7 +3087,38 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } }); + let nativeSkills: SkillMetadata[] = []; + let nativeSkillsAvailable = false; + const refreshNativeSkills = async (forceReload: boolean): Promise => { + const response = await appServerClient.listSkills({ + cwds: [session.path], + forceReload + }); + const inventory = response.data?.find(entry => entry.cwd === session.path) + ?? response.data?.[0]; + if (!inventory || (inventory.skills.length === 0 && (inventory.errors?.length ?? 0) > 0)) { + throw new Error('skills/list returned no usable inventory'); + } + nativeSkills = inventory.skills.filter(skill => skill.enabled); + if (!nativeSkillsAvailable) { + nativeSkillsAvailable = true; + session.client.rpcHandlerManager.registerHandler(RPC_METHODS.ListSkills, async () => ({ + success: true, + skills: nativeSkills.map(skill => ({ + name: skill.name, + description: skill.description + })) + })); + } + }; + appServerClient.setNotificationHandler((method, params) => { + if (method === 'skills/changed') { + void refreshNativeSkills(true).catch((error) => { + logger.debug(`[Codex] failed to refresh skills: ${errorMessage(error)}`); + }); + return; + } const events = appServerEventConverter.handleNotification(method, params); for (const event of events) { const eventRecord = asRecord(event) ?? { type: undefined }; @@ -3165,6 +3197,13 @@ class CodexRemoteLauncher extends RemoteLauncherBase { experimentalApi: true } }); + + try { + await refreshNativeSkills(false); + } catch (error) { + logger.debug(`[Codex] skills/list failed: ${errorMessage(error)}; keeping filesystem fallback`); + } + let supportsTurnCollaborationMode = true; let supportsGoals = true; try { @@ -3695,6 +3734,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { cwd: session.path, mode, cliOverrides: session.codexCliOverrides, + skills: nativeSkills, overrides: suppressCollaborationMode ? { suppressCollaborationMode: true } : undefined diff --git a/cli/src/codex/utils/appServerConfig.test.ts b/cli/src/codex/utils/appServerConfig.test.ts index 8feba235..4ffd1ee6 100644 --- a/cli/src/codex/utils/appServerConfig.test.ts +++ b/cli/src/codex/utils/appServerConfig.test.ts @@ -564,6 +564,37 @@ describe('appServerConfig', () => { ]); }); + it('builds a structured leading skill input from the native catalog', () => { + expect(buildUserInputFromMessage('$hapi inspect @"README.md"', [{ + name: 'hapi', + path: '/home/user/.agents/skills/hapi/SKILL.md', + description: 'Manage HAPI', + scope: 'user', + enabled: true + }])).toEqual([ + { type: 'skill', name: 'hapi', path: '/home/user/.agents/skills/hapi/SKILL.md' }, + { type: 'text', text: ' inspect ' }, + { type: 'mention', name: 'README.md', path: 'README.md' } + ]); + }); + + it('keeps unknown and disabled skill references as text', () => { + const skills = [{ + name: 'disabled-skill', + path: '/skills/disabled/SKILL.md', + description: 'Disabled', + scope: 'user' as const, + enabled: false + }]; + + expect(buildUserInputFromMessage('$unknown run', skills)).toEqual([ + { type: 'text', text: '$unknown run' } + ]); + expect(buildUserInputFromMessage('$disabled-skill run', skills)).toEqual([ + { type: 'text', text: '$disabled-skill run' } + ]); + }); + it('builds mention inputs from quoted @file tokens with spaces', () => { expect(buildUserInputFromMessage('please inspect @"docs/My File.md" now')).toEqual([ { type: 'text', text: 'please inspect ' }, diff --git a/cli/src/codex/utils/appServerConfig.ts b/cli/src/codex/utils/appServerConfig.ts index cca94a61..69781063 100644 --- a/cli/src/codex/utils/appServerConfig.ts +++ b/cli/src/codex/utils/appServerConfig.ts @@ -6,6 +6,7 @@ import type { ApprovalPolicy, SandboxMode, SandboxPolicy, + SkillMetadata, ThreadStartParams, TurnStartParams, UserInput @@ -139,13 +140,26 @@ function mentionNameFromPath(path: string): string { return parts[parts.length - 1] ?? path; } -export function buildUserInputFromMessage(message: string): UserInput[] { +export function buildUserInputFromMessage( + message: string, + skills: readonly SkillMetadata[] = [] +): UserInput[] { const inputs: UserInput[] = []; + const skillMatch = /^\s*\$([^\s]+)(?=\s|$)/.exec(message); + const skill = skillMatch + ? skills.find(candidate => candidate.enabled && candidate.name === skillMatch[1]) + : undefined; + const inputMessage = skill && skillMatch + ? message.slice(skillMatch[0].length) + : message; + if (skill) { + inputs.push({ type: 'skill', name: skill.name, path: skill.path }); + } const mentionPattern = /(^|\s)@"((?:\\.|[^"\\])*)"/g; let lastIndex = 0; let match: RegExpExecArray | null; - while ((match = mentionPattern.exec(message)) !== null) { + while ((match = mentionPattern.exec(inputMessage)) !== null) { const prefix = match[1] ?? ''; const rawPath = match[2] ?? ''; const pathText = rawPath; @@ -153,7 +167,7 @@ export function buildUserInputFromMessage(message: string): UserInput[] { if (!path) continue; const atIndex = match.index + prefix.length; - const textBeforeMention = message.slice(lastIndex, atIndex); + const textBeforeMention = inputMessage.slice(lastIndex, atIndex); if (textBeforeMention) { inputs.push({ type: 'text', text: textBeforeMention }); } @@ -166,7 +180,7 @@ export function buildUserInputFromMessage(message: string): UserInput[] { lastIndex = mentionPattern.lastIndex - (rawPath.length - pathText.length); } - const remainder = message.slice(lastIndex); + const remainder = inputMessage.slice(lastIndex); if (remainder || inputs.length === 0) { inputs.push({ type: 'text', text: remainder }); } @@ -232,6 +246,7 @@ export function buildTurnStartParams(args: { cliOverrides?: CodexCliOverrides; baseInstructions?: string; developerInstructions?: string; + skills?: readonly SkillMetadata[]; overrides?: { approvalPolicy?: TurnStartParams['approvalPolicy']; sandboxPolicy?: TurnStartParams['sandboxPolicy']; @@ -242,7 +257,7 @@ export function buildTurnStartParams(args: { const params: TurnStartParams = { threadId: args.threadId, cwd: args.cwd, - input: buildUserInputFromMessage(args.message) + input: buildUserInputFromMessage(args.message, args.skills) }; const allowCliOverrides = args.mode?.permissionMode === 'default'; diff --git a/web/src/hooks/queries/useSkills.ts b/web/src/hooks/queries/useSkills.ts index 77c77bd4..f25d5fb1 100644 --- a/web/src/hooks/queries/useSkills.ts +++ b/web/src/hooks/queries/useSkills.ts @@ -61,6 +61,10 @@ export function useSkills( }, [query.data]) const getSuggestions = useCallback(async (queryText: string): Promise => { + const refreshed = queryText === '$' ? await query.refetch() : null + const currentSkills = refreshed?.data?.success + ? (refreshed.data.skills ?? []) + : skills const recent = getRecentSkills() const getRecency = (name: string) => recent[name] ?? 0 const searchTerm = queryText.startsWith('$') @@ -68,7 +72,7 @@ export function useSkills( : queryText.toLowerCase() if (!searchTerm) { - return [...skills] + return [...currentSkills] .sort((a, b) => getRecency(b.name) - getRecency(a.name) || a.name.localeCompare(b.name)) .map((skill) => ({ key: `$${skill.name}`, @@ -80,7 +84,7 @@ export function useSkills( } const maxDistance = Math.max(2, Math.floor(searchTerm.length / 2)) - return skills + return currentSkills .map(skill => { const name = skill.name.toLowerCase() let score: number @@ -102,7 +106,7 @@ export function useSkills( description: skill.description, source: 'builtin' })) - }, [skills]) + }, [query.refetch, skills]) return { skills,