Skip to content

Commit 6ac3561

Browse files
sjfsjf
authored
fix(codex): format skills command output (#87400)
1 parent 77fe36b commit 6ac3561

4 files changed

Lines changed: 260 additions & 7 deletions

File tree

‎extensions/codex/src/app-server/protocol.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,32 @@ export type CodexSkillsListParams = {
448448
forceReload?: boolean;
449449
};
450450

451+
export type CodexSkillScope = "user" | "repo" | "system" | "admin";
452+
453+
export type CodexSkillMetadata = {
454+
name: string;
455+
description: string;
456+
shortDescription?: string;
457+
interface?: JsonObject;
458+
dependencies?: JsonObject;
459+
path: string;
460+
scope: CodexSkillScope;
461+
enabled: boolean;
462+
};
463+
464+
export type CodexSkillErrorInfo = {
465+
path: string;
466+
message: string;
467+
};
468+
469+
export type CodexSkillsListEntry = {
470+
cwd: string;
471+
skills: CodexSkillMetadata[];
472+
errors: CodexSkillErrorInfo[];
473+
};
474+
451475
export type CodexSkillsListResponse = {
452-
data: JsonValue[];
453-
nextCursor?: string | null;
476+
data: CodexSkillsListEntry[];
454477
};
455478

456479
export type CodexHooksListParams = {

‎extensions/codex/src/command-formatters.ts‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export function formatCodexStatus(probes: CodexStatusProbes): string {
5757
lines.push(
5858
`Skills: ${
5959
probes.skills.ok
60-
? summarizeArrayLike(probes.skills.value)
60+
? summarizeCodexSkills(probes.skills.value)
6161
: formatCodexDisplayText(probes.skills.error)
6262
}`,
6363
);
@@ -199,6 +199,48 @@ export function formatList(response: JsonValue | undefined, label: string): stri
199199
].join("\n");
200200
}
201201

202+
export function formatSkills(response: JsonValue | undefined): string {
203+
const groups = isJsonObject(response) && Array.isArray(response.data) ? response.data : [];
204+
if (groups.length === 0) {
205+
return "Codex skills: none returned.";
206+
}
207+
const lines = ["Codex skills:"];
208+
let renderedSkills = 0;
209+
let loadErrors = 0;
210+
for (const group of groups) {
211+
const record = isJsonObject(group) ? group : {};
212+
if (Array.isArray(record.errors)) {
213+
loadErrors += record.errors.length;
214+
}
215+
const skills = Array.isArray(record.skills) ? record.skills : [];
216+
if (skills.length === 0) {
217+
continue;
218+
}
219+
for (const skill of skills) {
220+
if (isJsonObject(skill) && skill.enabled === false) {
221+
continue;
222+
}
223+
lines.push(`- ${formatCodexSkillEntry(skill)}`);
224+
renderedSkills += 1;
225+
}
226+
}
227+
if (renderedSkills === 0) {
228+
if (loadErrors > 0) {
229+
return `Codex skills: none returned (${loadErrors} load ${
230+
loadErrors === 1 ? "error" : "errors"
231+
}).`;
232+
}
233+
return "Codex skills: none returned.";
234+
}
235+
return lines.join("\n");
236+
}
237+
238+
function formatCodexSkillEntry(entry: JsonValue): string {
239+
const record = isJsonObject(entry) ? entry : {};
240+
const name = readString(record, "name") ?? "<unknown>";
241+
return `\`${formatCodexDisplayText(name)}\``;
242+
}
243+
202244
const CODEX_RESUME_SAFE_THREAD_ID_PATTERN = /^[A-Za-z0-9._:-]+$/;
203245

204246
function formatCodexResumeHint(threadId: string): string {
@@ -350,6 +392,36 @@ function summarizeArrayLike(value: JsonValue | undefined): string {
350392
return `${entries.length}`;
351393
}
352394

395+
function summarizeCodexSkills(value: JsonValue | undefined): string {
396+
const groups = isJsonObject(value) && Array.isArray(value.data) ? value.data : [];
397+
if (groups.length === 0) {
398+
return "none returned";
399+
}
400+
let enabledSkills = 0;
401+
let loadErrors = 0;
402+
for (const group of groups) {
403+
if (!isJsonObject(group)) {
404+
continue;
405+
}
406+
if (Array.isArray(group.errors)) {
407+
loadErrors += group.errors.length;
408+
}
409+
if (!Array.isArray(group.skills)) {
410+
continue;
411+
}
412+
enabledSkills += group.skills.filter(
413+
(skill) => !isJsonObject(skill) || skill.enabled !== false,
414+
).length;
415+
}
416+
if (enabledSkills > 0) {
417+
return `${enabledSkills}`;
418+
}
419+
if (loadErrors > 0) {
420+
return `none returned (${loadErrors} load ${loadErrors === 1 ? "error" : "errors"})`;
421+
}
422+
return "none returned";
423+
}
424+
353425
function formatCodexRateLimitSummary(value: JsonValue | undefined): string {
354426
const summary = summarizeCodexRateLimits(value);
355427
if (summary) {

‎extensions/codex/src/command-handlers.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
formatCodexStatus,
3131
formatList,
3232
formatModels,
33+
formatSkills,
3334
formatThreads,
3435
readString,
3536
} from "./command-formatters.js";
@@ -380,9 +381,8 @@ export async function handleCodexSubcommand(
380381
return { text: "Usage: /codex skills" };
381382
}
382383
return {
383-
text: formatList(
384+
text: formatSkills(
384385
await deps.codexControlRequest(options.pluginConfig, CODEX_CONTROL_METHODS.listSkills, {}),
385-
"Codex skills",
386386
),
387387
};
388388
}

‎extensions/codex/src/commands.test.ts‎

Lines changed: 160 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
1212
import { CODEX_CONTROL_METHODS } from "./app-server/capabilities.js";
1313
import type { CodexComputerUseStatus } from "./app-server/computer-use.js";
1414
import type { CodexAppServerStartOptions } from "./app-server/config.js";
15+
import type { JsonValue } from "./app-server/protocol.js";
1516
import {
1617
readRecentCodexRateLimits,
1718
resetCodexRateLimitCacheForTests,
@@ -912,6 +913,62 @@ describe("codex command", () => {
912913
expect(result.text).not.toContain("@here");
913914
});
914915

916+
it("summarizes Codex status skill groups by enabled nested skills", async () => {
917+
const deps = createDeps({
918+
readCodexStatusProbes: vi.fn(async () => ({
919+
models: { ok: true as const, value: { models: [] } },
920+
account: { ok: true as const, value: {} },
921+
limits: { ok: true as const, value: { rateLimits: null, rateLimitsByLimitId: null } },
922+
mcps: { ok: true as const, value: { data: [] } },
923+
skills: {
924+
ok: true as const,
925+
value: {
926+
data: [
927+
{
928+
cwd: "/repo-a",
929+
skills: [
930+
{
931+
name: "enabled-one",
932+
description: "",
933+
path: "/repo-a/.codex/skills/enabled-one/SKILL.md",
934+
scope: "repo" as const,
935+
enabled: true,
936+
},
937+
{
938+
name: "disabled-one",
939+
description: "",
940+
path: "/repo-a/.codex/skills/disabled-one/SKILL.md",
941+
scope: "repo" as const,
942+
enabled: false,
943+
},
944+
],
945+
errors: [],
946+
},
947+
{
948+
cwd: "/repo-b",
949+
skills: [
950+
{
951+
name: "enabled-two",
952+
description: "",
953+
path: "/repo-b/.codex/skills/enabled-two/SKILL.md",
954+
scope: "repo" as const,
955+
enabled: true,
956+
},
957+
],
958+
errors: [{ path: "/repo-b/bad/SKILL.md", message: "bad skill" }],
959+
},
960+
],
961+
},
962+
},
963+
})),
964+
});
965+
966+
const result = await handleCodexCommand(createContext("status"), { deps });
967+
968+
expect(result.text).toContain("Skills: 2");
969+
expect(result.text).not.toContain("Skills: 1");
970+
});
971+
915972
it("summarizes generated Codex rate-limit payloads", async () => {
916973
const limits = {
917974
ok: true as const,
@@ -3085,19 +3142,120 @@ describe("codex command", () => {
30853142
const codexControlRequest = vi
30863143
.fn()
30873144
.mockResolvedValueOnce({ data: [{ name: "<@U123> [mcp](https://evil)" }] })
3088-
.mockResolvedValueOnce({ data: [{ id: "skill_1 @here" }] });
3145+
.mockResolvedValueOnce({
3146+
data: [
3147+
{
3148+
cwd: "/repo",
3149+
skills: [
3150+
{
3151+
name: "skill_1 @here",
3152+
description: "",
3153+
path: "/repo/.codex/skills/skill_1/SKILL.md",
3154+
scope: "repo",
3155+
enabled: true,
3156+
},
3157+
],
3158+
errors: [],
3159+
},
3160+
],
3161+
});
30893162
const deps = createDeps({ codexControlRequest });
30903163

30913164
const mcp = await handleCodexCommand(createContext("mcp"), { deps });
30923165
const skills = await handleCodexCommand(createContext("skills"), { deps });
30933166

30943167
expect(mcp.text).toContain("&lt;\uff20U123&gt; \uff3bmcp\uff3d\uff08https://evil\uff09");
3095-
expect(skills.text).toContain("skill\uff3f1 \uff20here");
3168+
expect(skills.text).toContain("- `skill\uff3f1 \uff20here`");
30963169
expect(`${mcp.text}\n${skills.text}`).not.toContain("<@U123>");
30973170
expect(`${mcp.text}\n${skills.text}`).not.toContain("[mcp](https://evil)");
30983171
expect(`${mcp.text}\n${skills.text}`).not.toContain("@here");
30993172
});
31003173

3174+
it("formats every Codex skill as a code-styled bullet and tolerates malformed entries", async () => {
3175+
const malformedSkillEntries: JsonValue[] = [
3176+
null,
3177+
{ description: "missing name" },
3178+
{
3179+
name: "final-skill",
3180+
description: "Final skill",
3181+
path: "/repo-b/.codex/skills/final-skill/SKILL.md",
3182+
scope: "repo",
3183+
enabled: true,
3184+
},
3185+
];
3186+
const codexControlRequest = vi.fn(async () => ({
3187+
data: [
3188+
{
3189+
cwd: "/repo-a",
3190+
skills: Array.from({ length: 26 }, (_, index) => ({
3191+
name: `skill-${index + 1}`,
3192+
description: `Skill ${index + 1}`,
3193+
path: `/repo-a/.codex/skills/skill-${index + 1}/SKILL.md`,
3194+
scope: "repo",
3195+
enabled: true,
3196+
})).concat({
3197+
name: "disabled-skill",
3198+
description: "Disabled skill",
3199+
path: "/repo-a/.codex/skills/disabled-skill/SKILL.md",
3200+
scope: "repo",
3201+
enabled: false,
3202+
}),
3203+
errors: [{ path: "/repo-a/bad/SKILL.md", message: "bad skill" }],
3204+
},
3205+
{
3206+
cwd: "/repo-b",
3207+
skills: malformedSkillEntries,
3208+
errors: [],
3209+
},
3210+
"malformed group",
3211+
],
3212+
}));
3213+
const deps = createDeps({ codexControlRequest });
3214+
3215+
const result = await handleCodexCommand(createContext("skills"), { deps });
3216+
3217+
expect(result.text).toContain("- `skill-1`");
3218+
expect(result.text).toContain("- `skill-26`");
3219+
expect(result.text).toContain("- `&lt;unknown&gt;`");
3220+
expect(result.text).toContain("- `final-skill`");
3221+
expect(result.text).not.toContain("Workspace:");
3222+
expect(result.text).not.toContain("Error:");
3223+
expect(result.text).not.toContain("More skills available");
3224+
expect(result.text).not.toContain("Skill 1");
3225+
expect(result.text).not.toContain("/repo-a/.codex/skills");
3226+
expect(result.text).not.toContain("disabled-skill");
3227+
});
3228+
3229+
it("reports Codex skill load errors when no skills render", async () => {
3230+
const codexControlRequest = vi.fn(async () => ({
3231+
data: [
3232+
{
3233+
cwd: "/repo-a",
3234+
skills: [
3235+
{
3236+
name: "disabled-skill",
3237+
description: "Disabled skill",
3238+
path: "/repo-a/.codex/skills/disabled-skill/SKILL.md",
3239+
scope: "repo",
3240+
enabled: false,
3241+
},
3242+
],
3243+
errors: [
3244+
{ path: "/repo-a/bad/SKILL.md", message: "bad skill <@U123>" },
3245+
{ path: "/repo-a/other/SKILL.md", message: "other bad skill @here" },
3246+
],
3247+
},
3248+
],
3249+
}));
3250+
const deps = createDeps({ codexControlRequest });
3251+
3252+
const result = await handleCodexCommand(createContext("skills"), { deps });
3253+
3254+
expect(result.text).toBe("Codex skills: none returned (2 load errors).");
3255+
expect(result.text).not.toContain("<@U123>");
3256+
expect(result.text).not.toContain("@here");
3257+
});
3258+
31013259
it("returns sanitized command failures instead of leaking app-server errors", async () => {
31023260
const sessionFile = path.join(tempDir, "session.jsonl");
31033261
await fs.writeFile(

0 commit comments

Comments
 (0)