fix(codex): omit disabled skills from slash commands (#2759)
* fix(codex): omit disabled skills from slash commands Codex skills/list returns disabled skills with enabled:false. Filter them out in loadSkills so Paseo matches Codex CLI/TUI and no longer surfaces stale slash entries for skills the user disabled. Fixes #2617 * fix(codex): preserve empty skill discovery --------- Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Mohamed Boudra
parent
3d420720c5
commit
0906d1d7bd
@@ -263,9 +263,22 @@ function capturedThreadStartConfig(records: CapturedFakeCodexRecord[]): unknown
|
|||||||
return params?.config;
|
return params?.config;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function listCommandsFromFakeCodex(skills: unknown[]): Promise<AgentSlashCommand[]> {
|
async function listCommandsFromFakeCodex(
|
||||||
|
skills: unknown[],
|
||||||
|
filesystemSkills: Array<{ name: string; description: string }> = [],
|
||||||
|
): Promise<AgentSlashCommand[]> {
|
||||||
const tempDir = await mkdtemp(path.join(tmpdir(), "codex-command-list-"));
|
const tempDir = await mkdtemp(path.join(tmpdir(), "codex-command-list-"));
|
||||||
|
const projectCwd = path.join(tempDir, "project");
|
||||||
const fakeCodexPath = path.join(tempDir, "fake-codex.cjs");
|
const fakeCodexPath = path.join(tempDir, "fake-codex.cjs");
|
||||||
|
mkdirSync(projectCwd, { recursive: true });
|
||||||
|
for (const skill of filesystemSkills) {
|
||||||
|
const skillDir = path.join(projectCwd, ".codex", "skills", skill.name);
|
||||||
|
mkdirSync(skillDir, { recursive: true });
|
||||||
|
writeFileSync(
|
||||||
|
path.join(skillDir, "SKILL.md"),
|
||||||
|
`---\nname: ${skill.name}\ndescription: ${skill.description}\n---\n`,
|
||||||
|
);
|
||||||
|
}
|
||||||
writeFileSync(
|
writeFileSync(
|
||||||
fakeCodexPath,
|
fakeCodexPath,
|
||||||
`
|
`
|
||||||
@@ -276,7 +289,7 @@ function resultFor(method, params) {
|
|||||||
if (method === "collaborationMode/list") return { data: [] };
|
if (method === "collaborationMode/list") return { data: [] };
|
||||||
if (method === "skills/list") {
|
if (method === "skills/list") {
|
||||||
const cwds = params && params.cwds;
|
const cwds = params && params.cwds;
|
||||||
const projectCwd = "/tmp/codex-question-test";
|
const projectCwd = ${JSON.stringify(projectCwd)};
|
||||||
if (!Array.isArray(cwds) || cwds.length !== 1 || cwds[0] !== projectCwd) {
|
if (!Array.isArray(cwds) || cwds.length !== 1 || cwds[0] !== projectCwd) {
|
||||||
return { data: [] };
|
return { data: [] };
|
||||||
}
|
}
|
||||||
@@ -316,7 +329,7 @@ process.stdin.on("data", (chunk) => {
|
|||||||
const client = new CodexAppServerAgentClient(createTestLogger(), {
|
const client = new CodexAppServerAgentClient(createTestLogger(), {
|
||||||
command: { mode: "replace", argv: [process.execPath, fakeCodexPath] },
|
command: { mode: "replace", argv: [process.execPath, fakeCodexPath] },
|
||||||
});
|
});
|
||||||
const session = await client.createSession(createConfig());
|
const session = await client.createSession(createConfig({ cwd: projectCwd }));
|
||||||
try {
|
try {
|
||||||
return await session.listCommands();
|
return await session.listCommands();
|
||||||
} finally {
|
} finally {
|
||||||
@@ -1686,6 +1699,53 @@ describe("Codex app-server provider", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("omits disabled Codex skills from slash commands", async () => {
|
||||||
|
const commands = await listCommandsFromFakeCodex([
|
||||||
|
{
|
||||||
|
name: "enabled-skill",
|
||||||
|
description: "An enabled skill.",
|
||||||
|
path: "/tmp/skills/enabled-skill/SKILL.md",
|
||||||
|
enabled: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "disabled-skill",
|
||||||
|
description: "A disabled skill.",
|
||||||
|
path: "/tmp/skills/disabled-skill/SKILL.md",
|
||||||
|
enabled: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "legacy-skill",
|
||||||
|
description: "Skill without enabled field (older Codex).",
|
||||||
|
path: "/tmp/skills/legacy-skill/SKILL.md",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const skillCommands = commands.filter((command) => command.kind === "skill");
|
||||||
|
expect(skillCommands.map((command) => command.name).sort()).toEqual([
|
||||||
|
"enabled-skill",
|
||||||
|
"legacy-skill",
|
||||||
|
]);
|
||||||
|
expect(skillCommands.find((command) => command.name === "disabled-skill")).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("does not rediscover disabled Codex skills through filesystem fallback", async () => {
|
||||||
|
const commands = await listCommandsFromFakeCodex(
|
||||||
|
[
|
||||||
|
{
|
||||||
|
name: "disabled-skill",
|
||||||
|
description: "A disabled skill.",
|
||||||
|
path: "/tmp/skills/disabled-skill/SKILL.md",
|
||||||
|
enabled: false,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
[{ name: "disabled-skill", description: "A disabled skill." }],
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(commands).not.toContainEqual(
|
||||||
|
expect.objectContaining({ name: "disabled-skill", kind: "skill" }),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
test("maps image prompt blocks to Codex localImage input", async () => {
|
test("maps image prompt blocks to Codex localImage input", async () => {
|
||||||
const input = await codexAppServerTurnInputFromPrompt(
|
const input = await codexAppServerTurnInputFromPrompt(
|
||||||
[
|
[
|
||||||
|
|||||||
@@ -3230,7 +3230,7 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
settings: Record<string, unknown>;
|
settings: Record<string, unknown>;
|
||||||
name: string;
|
name: string;
|
||||||
} | null = null;
|
} | null = null;
|
||||||
private cachedSkills: Array<{ name: string; description: string; path: string }> = [];
|
private cachedSkills: Array<{ name: string; description: string; path: string }> | null = null;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
config: AgentSessionConfig,
|
config: AgentSessionConfig,
|
||||||
@@ -3462,6 +3462,10 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
const skillRecord = toObjectRecord(skill);
|
const skillRecord = toObjectRecord(skill);
|
||||||
if (typeof skillRecord?.name !== "string" || typeof skillRecord?.path !== "string")
|
if (typeof skillRecord?.name !== "string" || typeof skillRecord?.path !== "string")
|
||||||
continue;
|
continue;
|
||||||
|
// Codex skills/list returns disabled skills with enabled:false; omit them from
|
||||||
|
// slash-command surfaces so Paseo matches Codex CLI/TUI behavior.
|
||||||
|
// Missing enabled (older binaries) is treated as enabled.
|
||||||
|
if (skillRecord.enabled === false) continue;
|
||||||
if (!skillsByName.has(skillRecord.name)) {
|
if (!skillsByName.has(skillRecord.name)) {
|
||||||
skillsByName.set(skillRecord.name, {
|
skillsByName.set(skillRecord.name, {
|
||||||
name: skillRecord.name,
|
name: skillRecord.name,
|
||||||
@@ -3483,7 +3487,7 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
},
|
},
|
||||||
"provider.codex.metadata.skills_failed",
|
"provider.codex.metadata.skills_failed",
|
||||||
);
|
);
|
||||||
this.cachedSkills = [];
|
this.cachedSkills = null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3806,7 +3810,7 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
} else {
|
} else {
|
||||||
await this.loadSkills();
|
await this.loadSkills();
|
||||||
}
|
}
|
||||||
const skill = this.cachedSkills.find((entry) => entry.name === commandName);
|
const skill = this.cachedSkills?.find((entry) => entry.name === commandName);
|
||||||
if (skill) {
|
if (skill) {
|
||||||
const trimmedArgs = args?.trim() ?? "";
|
const trimmedArgs = args?.trim() ?? "";
|
||||||
const text = trimmedArgs ? `$${skill.name} ${trimmedArgs}` : `$${skill.name}`;
|
const text = trimmedArgs ? `$${skill.name} ${trimmedArgs}` : `$${skill.name}`;
|
||||||
@@ -4511,14 +4515,14 @@ export class CodexAppServerAgentSession implements AgentSession {
|
|||||||
} else {
|
} else {
|
||||||
await this.loadSkills();
|
await this.loadSkills();
|
||||||
}
|
}
|
||||||
const appServerSkills = this.cachedSkills.map((skill) => ({
|
const appServerSkills = (this.cachedSkills ?? []).map((skill) => ({
|
||||||
name: skill.name,
|
name: skill.name,
|
||||||
description: skill.description,
|
description: skill.description,
|
||||||
argumentHint: "",
|
argumentHint: "",
|
||||||
kind: "skill" as const,
|
kind: "skill" as const,
|
||||||
}));
|
}));
|
||||||
const fallbackSkills =
|
const fallbackSkills =
|
||||||
appServerSkills.length === 0
|
this.cachedSkills === null
|
||||||
? await listCodexSkills(this.config.cwd, this.deps.workspaceGitService)
|
? await listCodexSkills(this.config.cwd, this.deps.workspaceGitService)
|
||||||
: [];
|
: [];
|
||||||
const builtin: AgentSlashCommand[] = [
|
const builtin: AgentSlashCommand[] = [
|
||||||
|
|||||||
Reference in New Issue
Block a user