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
This commit is contained in:
SSU-WEI HUANG
2026-08-02 08:49:35 +08:00
committed by GitHub
parent 82da539d32
commit 2556729ba8
7 changed files with 231 additions and 9 deletions
+23
View File
@@ -62,6 +62,29 @@ export interface ModelListResponse {
[key: string]: unknown; [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 { export interface CollaborationModeListItem {
name?: string; name?: string;
mode?: 'plan' | 'default' | string | null; mode?: 'plan' | 'default' | string | null;
+9
View File
@@ -9,6 +9,8 @@ import type {
InitializeResponse, InitializeResponse,
ModelListParams, ModelListParams,
ModelListResponse, ModelListResponse,
SkillsListParams,
SkillsListResponse,
ThreadStartParams, ThreadStartParams,
ThreadStartResponse, ThreadStartResponse,
ThreadResumeParams, ThreadResumeParams,
@@ -245,6 +247,13 @@ export class CodexAppServerClient extends JsonLineParser {
return response as ModelListResponse; return response as ModelListResponse;
} }
async listSkills(params: SkillsListParams): Promise<SkillsListResponse> {
const response = await this.sendRequest('skills/list', params, {
timeoutMs: 30_000
});
return response as SkillsListResponse;
}
async listCollaborationModes(): Promise<CollaborationModeListResponse> { async listCollaborationModes(): Promise<CollaborationModeListResponse> {
const response = await this.sendRequest('collaborationMode/list', {}, { const response = await this.sendRequest('collaborationMode/list', {}, {
timeoutMs: 30_000 timeoutMs: 30_000
+100
View File
@@ -13,6 +13,20 @@ const harness = vi.hoisted(() => ({
listCollaborationModeCalls: 0, listCollaborationModeCalls: 0,
collaborationModeResponse: { data: [{ mode: 'default' }, { mode: 'plan' }] } as unknown, collaborationModeResponse: { data: [{ mode: 'default' }, { mode: 'plan' }] } as unknown,
failListCollaborationModes: false, 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[], startThreadIds: [] as string[],
startThreadParams: [] as Array<Record<string, unknown>>, startThreadParams: [] as Array<Record<string, unknown>>,
resumeThreadIds: [] as string[], resumeThreadIds: [] as string[],
@@ -100,6 +114,11 @@ vi.mock('./codexAppServerClient', () => {
return harness.collaborationModeResponse; return harness.collaborationModeResponse;
} }
async listSkills(params: unknown): Promise<unknown> {
harness.listSkillsCalls.push(params);
return harness.skillsListResponse;
}
async setExperimentalFeatureEnablement(params: unknown): Promise<unknown> { async setExperimentalFeatureEnablement(params: unknown): Promise<unknown> {
harness.setFeatureEnablementCalls.push(params); harness.setFeatureEnablementCalls.push(params);
if (harness.failSetFeatureEnablement) { if (harness.failSetFeatureEnablement) {
@@ -1097,6 +1116,20 @@ describe('codexRemoteLauncher', () => {
harness.listCollaborationModeCalls = 0; harness.listCollaborationModeCalls = 0;
harness.collaborationModeResponse = { data: [{ mode: 'default' }, { mode: 'plan' }] }; harness.collaborationModeResponse = { data: [{ mode: 'default' }, { mode: 'plan' }] };
harness.failListCollaborationModes = false; 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.startThreadIds = [];
harness.startThreadParams = []; harness.startThreadParams = [];
harness.resumeThreadIds = []; harness.resumeThreadIds = [];
@@ -1192,6 +1225,73 @@ describe('codexRemoteLauncher', () => {
expect(session.thinking).toBe(false); 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 () => { it('routes app-server MCP elicitation through the existing user-input transport', async () => {
const { session, codexMessages, rpcHandlers, setPermissionMode } = createSessionStub(); const { session, codexMessages, rpcHandlers, setPermissionMode } = createSessionStub();
+41 -1
View File
@@ -17,10 +17,11 @@ import { AppServerEventConverter } from './utils/appServerEventConverter';
import { detectImageMimeType, registerGeneratedImage } from '@/modules/common/generatedImages'; import { detectImageMimeType, registerGeneratedImage } from '@/modules/common/generatedImages';
import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter'; import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter';
import { buildThreadStartParams, buildTurnStartParams } from './utils/appServerConfig'; 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 { shouldIgnoreTerminalEvent } from './utils/terminalEventGuard';
import { parseCodexSpecialCommand } from './codexSpecialCommands'; import { parseCodexSpecialCommand } from './codexSpecialCommands';
import { extractErrorInfo } from '@/utils/errorUtils'; import { extractErrorInfo } from '@/utils/errorUtils';
import { RPC_METHODS } from '@hapi/protocol/rpcMethods';
import { import {
RemoteLauncherBase, RemoteLauncherBase,
type RemoteLauncherDisplayContext, type RemoteLauncherDisplayContext,
@@ -3086,7 +3087,38 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
} }
}); });
let nativeSkills: SkillMetadata[] = [];
let nativeSkillsAvailable = false;
const refreshNativeSkills = async (forceReload: boolean): Promise<void> => {
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) => { 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); const events = appServerEventConverter.handleNotification(method, params);
for (const event of events) { for (const event of events) {
const eventRecord = asRecord(event) ?? { type: undefined }; const eventRecord = asRecord(event) ?? { type: undefined };
@@ -3165,6 +3197,13 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
experimentalApi: true experimentalApi: true
} }
}); });
try {
await refreshNativeSkills(false);
} catch (error) {
logger.debug(`[Codex] skills/list failed: ${errorMessage(error)}; keeping filesystem fallback`);
}
let supportsTurnCollaborationMode = true; let supportsTurnCollaborationMode = true;
let supportsGoals = true; let supportsGoals = true;
try { try {
@@ -3695,6 +3734,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase {
cwd: session.path, cwd: session.path,
mode, mode,
cliOverrides: session.codexCliOverrides, cliOverrides: session.codexCliOverrides,
skills: nativeSkills,
overrides: suppressCollaborationMode overrides: suppressCollaborationMode
? { suppressCollaborationMode: true } ? { suppressCollaborationMode: true }
: undefined : undefined
@@ -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', () => { it('builds mention inputs from quoted @file tokens with spaces', () => {
expect(buildUserInputFromMessage('please inspect @"docs/My File.md" now')).toEqual([ expect(buildUserInputFromMessage('please inspect @"docs/My File.md" now')).toEqual([
{ type: 'text', text: 'please inspect ' }, { type: 'text', text: 'please inspect ' },
+20 -5
View File
@@ -6,6 +6,7 @@ import type {
ApprovalPolicy, ApprovalPolicy,
SandboxMode, SandboxMode,
SandboxPolicy, SandboxPolicy,
SkillMetadata,
ThreadStartParams, ThreadStartParams,
TurnStartParams, TurnStartParams,
UserInput UserInput
@@ -139,13 +140,26 @@ function mentionNameFromPath(path: string): string {
return parts[parts.length - 1] ?? path; 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 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; const mentionPattern = /(^|\s)@"((?:\\.|[^"\\])*)"/g;
let lastIndex = 0; let lastIndex = 0;
let match: RegExpExecArray | null; let match: RegExpExecArray | null;
while ((match = mentionPattern.exec(message)) !== null) { while ((match = mentionPattern.exec(inputMessage)) !== null) {
const prefix = match[1] ?? ''; const prefix = match[1] ?? '';
const rawPath = match[2] ?? ''; const rawPath = match[2] ?? '';
const pathText = rawPath; const pathText = rawPath;
@@ -153,7 +167,7 @@ export function buildUserInputFromMessage(message: string): UserInput[] {
if (!path) continue; if (!path) continue;
const atIndex = match.index + prefix.length; const atIndex = match.index + prefix.length;
const textBeforeMention = message.slice(lastIndex, atIndex); const textBeforeMention = inputMessage.slice(lastIndex, atIndex);
if (textBeforeMention) { if (textBeforeMention) {
inputs.push({ type: 'text', text: textBeforeMention }); inputs.push({ type: 'text', text: textBeforeMention });
} }
@@ -166,7 +180,7 @@ export function buildUserInputFromMessage(message: string): UserInput[] {
lastIndex = mentionPattern.lastIndex - (rawPath.length - pathText.length); lastIndex = mentionPattern.lastIndex - (rawPath.length - pathText.length);
} }
const remainder = message.slice(lastIndex); const remainder = inputMessage.slice(lastIndex);
if (remainder || inputs.length === 0) { if (remainder || inputs.length === 0) {
inputs.push({ type: 'text', text: remainder }); inputs.push({ type: 'text', text: remainder });
} }
@@ -232,6 +246,7 @@ export function buildTurnStartParams(args: {
cliOverrides?: CodexCliOverrides; cliOverrides?: CodexCliOverrides;
baseInstructions?: string; baseInstructions?: string;
developerInstructions?: string; developerInstructions?: string;
skills?: readonly SkillMetadata[];
overrides?: { overrides?: {
approvalPolicy?: TurnStartParams['approvalPolicy']; approvalPolicy?: TurnStartParams['approvalPolicy'];
sandboxPolicy?: TurnStartParams['sandboxPolicy']; sandboxPolicy?: TurnStartParams['sandboxPolicy'];
@@ -242,7 +257,7 @@ export function buildTurnStartParams(args: {
const params: TurnStartParams = { const params: TurnStartParams = {
threadId: args.threadId, threadId: args.threadId,
cwd: args.cwd, cwd: args.cwd,
input: buildUserInputFromMessage(args.message) input: buildUserInputFromMessage(args.message, args.skills)
}; };
const allowCliOverrides = args.mode?.permissionMode === 'default'; const allowCliOverrides = args.mode?.permissionMode === 'default';
+7 -3
View File
@@ -61,6 +61,10 @@ export function useSkills(
}, [query.data]) }, [query.data])
const getSuggestions = useCallback(async (queryText: string): Promise<Suggestion[]> => { const getSuggestions = useCallback(async (queryText: string): Promise<Suggestion[]> => {
const refreshed = queryText === '$' ? await query.refetch() : null
const currentSkills = refreshed?.data?.success
? (refreshed.data.skills ?? [])
: skills
const recent = getRecentSkills() const recent = getRecentSkills()
const getRecency = (name: string) => recent[name] ?? 0 const getRecency = (name: string) => recent[name] ?? 0
const searchTerm = queryText.startsWith('$') const searchTerm = queryText.startsWith('$')
@@ -68,7 +72,7 @@ export function useSkills(
: queryText.toLowerCase() : queryText.toLowerCase()
if (!searchTerm) { if (!searchTerm) {
return [...skills] return [...currentSkills]
.sort((a, b) => getRecency(b.name) - getRecency(a.name) || a.name.localeCompare(b.name)) .sort((a, b) => getRecency(b.name) - getRecency(a.name) || a.name.localeCompare(b.name))
.map((skill) => ({ .map((skill) => ({
key: `$${skill.name}`, key: `$${skill.name}`,
@@ -80,7 +84,7 @@ export function useSkills(
} }
const maxDistance = Math.max(2, Math.floor(searchTerm.length / 2)) const maxDistance = Math.max(2, Math.floor(searchTerm.length / 2))
return skills return currentSkills
.map(skill => { .map(skill => {
const name = skill.name.toLowerCase() const name = skill.name.toLowerCase()
let score: number let score: number
@@ -102,7 +106,7 @@ export function useSkills(
description: skill.description, description: skill.description,
source: 'builtin' source: 'builtin'
})) }))
}, [skills]) }, [query.refetch, skills])
return { return {
skills, skills,