fix: harden codex copied skill rewriting (#285)
This commit is contained in:
@@ -85,6 +85,24 @@ When converting copied `SKILL.md` content for Codex:
|
|||||||
- References to non-entrypoint skills should use the exact skill name, not a normalized alias
|
- References to non-entrypoint skills should use the exact skill name, not a normalized alias
|
||||||
- Actual Claude commands that are converted to Codex prompts can continue using `/prompts:...`
|
- Actual Claude commands that are converted to Codex prompts can continue using `/prompts:...`
|
||||||
|
|
||||||
|
### Regression hardening
|
||||||
|
|
||||||
|
When rewriting copied `SKILL.md` files, only known workflow and command references should be rewritten.
|
||||||
|
|
||||||
|
Do not rewrite arbitrary slash-shaped text such as:
|
||||||
|
|
||||||
|
- application routes like `/users` or `/settings`
|
||||||
|
- API path segments like `/state` or `/ops`
|
||||||
|
- URLs such as `https://www.proofeditor.ai/...`
|
||||||
|
|
||||||
|
Unknown slash references should remain unchanged in copied skill content. Otherwise Codex installs silently corrupt unrelated skills while trying to canonicalize workflow handoffs.
|
||||||
|
|
||||||
|
Personal skills loaded from `~/.claude/skills` also need tolerant metadata parsing:
|
||||||
|
|
||||||
|
- malformed YAML frontmatter should not cause the entire skill to disappear
|
||||||
|
- keep the directory name as the stable skill name
|
||||||
|
- treat frontmatter metadata as best-effort only
|
||||||
|
|
||||||
## Future Entry Points
|
## Future Entry Points
|
||||||
|
|
||||||
Do not hard-code an allowlist of workflow names in the converter.
|
Do not hard-code an allowlist of workflow names in the converter.
|
||||||
|
|||||||
@@ -37,12 +37,17 @@ async function loadPersonalSkills(skillsDir: string): Promise<ClaudeSkill[]> {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
await fs.access(skillPath)
|
await fs.access(skillPath)
|
||||||
const raw = await fs.readFile(skillPath, "utf8")
|
|
||||||
const { data } = parseFrontmatter(raw)
|
|
||||||
// Resolve symlink to get the actual source directory
|
// Resolve symlink to get the actual source directory
|
||||||
const sourceDir = entry.isSymbolicLink()
|
const sourceDir = entry.isSymbolicLink()
|
||||||
? await fs.realpath(entryPath)
|
? await fs.realpath(entryPath)
|
||||||
: entryPath
|
: entryPath
|
||||||
|
let data: Record<string, unknown> = {}
|
||||||
|
try {
|
||||||
|
const raw = await fs.readFile(skillPath, "utf8")
|
||||||
|
data = parseFrontmatter(raw).data
|
||||||
|
} catch {
|
||||||
|
// Keep syncing the skill even if frontmatter is malformed.
|
||||||
|
}
|
||||||
skills.push({
|
skills.push({
|
||||||
name: entry.name,
|
name: entry.name,
|
||||||
description: data.description as string | undefined,
|
description: data.description as string | undefined,
|
||||||
|
|||||||
@@ -66,7 +66,12 @@ async function copyCodexSkillDir(
|
|||||||
|
|
||||||
if (entry.name === "SKILL.md") {
|
if (entry.name === "SKILL.md") {
|
||||||
const content = await readText(sourcePath)
|
const content = await readText(sourcePath)
|
||||||
await writeText(targetPath, transformContentForCodex(content, invocationTargets))
|
await writeText(
|
||||||
|
targetPath,
|
||||||
|
transformContentForCodex(content, invocationTargets, {
|
||||||
|
unknownSlashBehavior: "preserve",
|
||||||
|
}),
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,10 @@ export type CodexInvocationTargets = {
|
|||||||
skillTargets: Record<string, string>
|
skillTargets: Record<string, string>
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type CodexTransformOptions = {
|
||||||
|
unknownSlashBehavior?: "prompt" | "preserve"
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Transform Claude Code content to Codex-compatible content.
|
* Transform Claude Code content to Codex-compatible content.
|
||||||
*
|
*
|
||||||
@@ -18,10 +22,12 @@ export type CodexInvocationTargets = {
|
|||||||
export function transformContentForCodex(
|
export function transformContentForCodex(
|
||||||
body: string,
|
body: string,
|
||||||
targets?: CodexInvocationTargets,
|
targets?: CodexInvocationTargets,
|
||||||
|
options: CodexTransformOptions = {},
|
||||||
): string {
|
): string {
|
||||||
let result = body
|
let result = body
|
||||||
const promptTargets = targets?.promptTargets ?? {}
|
const promptTargets = targets?.promptTargets ?? {}
|
||||||
const skillTargets = targets?.skillTargets ?? {}
|
const skillTargets = targets?.skillTargets ?? {}
|
||||||
|
const unknownSlashBehavior = options.unknownSlashBehavior ?? "prompt"
|
||||||
|
|
||||||
const taskPattern = /^(\s*-?\s*)Task\s+([a-z][a-z0-9:-]*)\(([^)]+)\)/gm
|
const taskPattern = /^(\s*-?\s*)Task\s+([a-z][a-z0-9:-]*)\(([^)]+)\)/gm
|
||||||
result = result.replace(taskPattern, (_match, prefix: string, agentName: string, args: string) => {
|
result = result.replace(taskPattern, (_match, prefix: string, agentName: string, args: string) => {
|
||||||
@@ -45,6 +51,9 @@ export function transformContentForCodex(
|
|||||||
if (skillTargets[normalizedName]) {
|
if (skillTargets[normalizedName]) {
|
||||||
return `the ${skillTargets[normalizedName]} skill`
|
return `the ${skillTargets[normalizedName]} skill`
|
||||||
}
|
}
|
||||||
|
if (unknownSlashBehavior === "preserve") {
|
||||||
|
return match
|
||||||
|
}
|
||||||
return `/prompts:${normalizedName}`
|
return `/prompts:${normalizedName}`
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -61,4 +61,22 @@ describe("loadClaudeHome", () => {
|
|||||||
expect(config.skills[0]?.description).toBe("Reviewer skill")
|
expect(config.skills[0]?.description).toBe("Reviewer skill")
|
||||||
expect(config.skills[0]?.argumentHint).toBe("[topic]")
|
expect(config.skills[0]?.argumentHint).toBe("[topic]")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("keeps personal skills when frontmatter is malformed", async () => {
|
||||||
|
const tempHome = await fs.mkdtemp(path.join(os.tmpdir(), "claude-home-skill-yaml-"))
|
||||||
|
const skillDir = path.join(tempHome, "skills", "reviewer")
|
||||||
|
|
||||||
|
await fs.mkdir(skillDir, { recursive: true })
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(skillDir, "SKILL.md"),
|
||||||
|
"---\nname: ce:plan\nfoo: [unterminated\n---\nReview things.\n",
|
||||||
|
)
|
||||||
|
|
||||||
|
const config = await loadClaudeHome(tempHome)
|
||||||
|
|
||||||
|
expect(config.skills).toHaveLength(1)
|
||||||
|
expect(config.skills[0]?.name).toBe("reviewer")
|
||||||
|
expect(config.skills[0]?.description).toBeUndefined()
|
||||||
|
expect(config.skills[0]?.argumentHint).toBeUndefined()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -206,4 +206,57 @@ Also run bare agents:
|
|||||||
expect(installedSkill).toContain("Use the $best-practices-researcher skill to: topic")
|
expect(installedSkill).toContain("Use the $best-practices-researcher skill to: topic")
|
||||||
expect(installedSkill).not.toContain("Task best-practices-researcher")
|
expect(installedSkill).not.toContain("Task best-practices-researcher")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("preserves unknown slash text in copied SKILL.md files", async () => {
|
||||||
|
const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-skill-preserve-"))
|
||||||
|
const sourceSkillDir = path.join(tempRoot, "source-skill")
|
||||||
|
await fs.mkdir(sourceSkillDir, { recursive: true })
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(sourceSkillDir, "SKILL.md"),
|
||||||
|
`---
|
||||||
|
name: proof
|
||||||
|
description: Proof skill
|
||||||
|
---
|
||||||
|
|
||||||
|
Route examples:
|
||||||
|
- /users
|
||||||
|
- /settings
|
||||||
|
|
||||||
|
API examples:
|
||||||
|
- https://www.proofeditor.ai/api/agent/{slug}/state
|
||||||
|
- https://www.proofeditor.ai/share/markdown
|
||||||
|
|
||||||
|
Workflow handoff:
|
||||||
|
- /ce:plan
|
||||||
|
`,
|
||||||
|
)
|
||||||
|
|
||||||
|
const bundle: CodexBundle = {
|
||||||
|
prompts: [],
|
||||||
|
skillDirs: [{ name: "proof", sourceDir: sourceSkillDir }],
|
||||||
|
generatedSkills: [],
|
||||||
|
invocationTargets: {
|
||||||
|
promptTargets: {
|
||||||
|
"ce-plan": "ce-plan",
|
||||||
|
},
|
||||||
|
skillTargets: {},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
await writeCodexBundle(tempRoot, bundle)
|
||||||
|
|
||||||
|
const installedSkill = await fs.readFile(
|
||||||
|
path.join(tempRoot, ".codex", "skills", "proof", "SKILL.md"),
|
||||||
|
"utf8",
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(installedSkill).toContain("/users")
|
||||||
|
expect(installedSkill).toContain("/settings")
|
||||||
|
expect(installedSkill).toContain("https://www.proofeditor.ai/api/agent/{slug}/state")
|
||||||
|
expect(installedSkill).toContain("https://www.proofeditor.ai/share/markdown")
|
||||||
|
expect(installedSkill).toContain("/prompts:ce-plan")
|
||||||
|
expect(installedSkill).not.toContain("/prompts:users")
|
||||||
|
expect(installedSkill).not.toContain("/prompts:settings")
|
||||||
|
expect(installedSkill).not.toContain("https://prompts:www.proofeditor.ai")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user