fix: normalize skill names and avoid subagent tool conflicts in pi

src/commands/plugin-path.ts

@@ -2,6 +2,7 @@ import { defineCommand } from "citty"
 import { promises as fs } from "fs"
 import os from "os"
 import path from "path"
+import { assertNoSymlinkAncestors, assertNoSymlinkTarget, assertPathWithinRoot, assertSafePathComponent, ensureManagedDir } from "../utils/files"
 
 export default defineCommand({
   meta: {
@@ -21,7 +22,7 @@ export default defineCommand({
     },
   },
   async run({ args }) {
-    const pluginName = String(args.plugin)
+    const pluginName = assertSafePathComponent(String(args.plugin), "plugin name")
     const branch = String(args.branch)
 
     // Reversible encoding: / -> ~ (safe because ~ is illegal in git branch names per
@@ -32,10 +33,13 @@ export default defineCommand({
       .replace(/[^a-zA-Z0-9._~-]/g, (ch) => `%${ch.charCodeAt(0).toString(16).padStart(2, "0")}`)
     const dirName = `${pluginName}-${sanitized}`
     const cacheRoot = path.join(os.homedir(), ".cache", "compound-engineering", "branches")
-    await fs.mkdir(cacheRoot, { recursive: true })
+    await ensureManagedDir(cacheRoot)
     const targetDir = path.join(cacheRoot, dirName)
     const source = resolveGitHubSource()
 
+    await assertNoSymlinkAncestors(targetDir)
+    await assertNoSymlinkTarget(targetDir)
+
     if (await dirExists(targetDir)) {
       console.error(`Updating existing checkout at ${targetDir}`)
       await fetchAndCheckout(targetDir, branch)
@@ -45,6 +49,9 @@ export default defineCommand({
     }
 
     const pluginPath = path.join(targetDir, "plugins", pluginName)
+    assertPathWithinRoot(pluginPath, path.join(targetDir, "plugins"), "plugin path")
+    await assertNoSymlinkAncestors(pluginPath)
+    await assertNoSymlinkTarget(pluginPath)
     if (!(await dirExists(pluginPath))) {
       throw new Error(`Plugin directory not found: ${pluginPath}`)
     }
@@ -57,7 +64,10 @@ export default defineCommand({
 
 async function dirExists(p: string): Promise<boolean> {
   try {
-    const stat = await fs.stat(p)
+    const stat = await fs.lstat(p)
+    if (stat.isSymbolicLink()) {
+      throw new Error(`Refusing to use symlinked cache checkout ${p}`)
+    }
     return stat.isDirectory()
   } catch {
     return false

src/converters/claude-to-pi.ts

@@ -1,4 +1,7 @@
+import path from "path"
 import { formatFrontmatter } from "../utils/frontmatter"
+import { appendCompatibilityNoteIfNeeded, normalizePiSkillName, transformPiBodyContent, uniquePiSkillName, type PiNameMaps } from "../utils/pi-skills"
+import { isSafePiManagedName } from "../utils/pi-managed"
 import type { ClaudeAgent, ClaudeCommand, ClaudeMcpServer, ClaudePlugin } from "../types/claude"
 import type {
   PiBundle,
@@ -9,22 +12,83 @@ import type {
 import type { ClaudeToOpenCodeOptions } from "./claude-to-opencode"
 import { PI_COMPAT_EXTENSION_SOURCE } from "../templates/pi/compat-extension"
 
-export type ClaudeToPiOptions = ClaudeToOpenCodeOptions
+export type ClaudeToPiOptions = ClaudeToOpenCodeOptions & {
+  extraNameMaps?: PiNameMaps
+  preserveUnknownQualifiedRefs?: boolean
+  rejectUnknownQualifiedTaskRefs?: boolean
+  rejectUnresolvedFirstPartyQualifiedRefs?: boolean
+}
 
 const PI_DESCRIPTION_MAX_LENGTH = 1024
 
 export function convertClaudeToPi(
   plugin: ClaudePlugin,
-  _options: ClaudeToPiOptions,
+  options: ClaudeToPiOptions,
 ): PiBundle {
   const promptNames = new Set<string>()
-  const usedSkillNames = new Set<string>(plugin.skills.map((skill) => normalizeName(skill.name)))
+  const usedSkillNames = new Set<string>()
+
+  const sortedSkills = [...plugin.skills].sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)
+  const sortedAgents = [...plugin.agents].sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)
+
+  assertNoConfiguredSharedTargetConflicts(options.extraNameMaps?.skills, options.extraNameMaps?.agents)
+  reserveConfiguredPiTargetNames(sortedSkills.map((skill) => skill.name), options.extraNameMaps?.skills, usedSkillNames)
+  reserveConfiguredPiTargetNames(sortedAgents.map((agent) => agent.name), options.extraNameMaps?.agents, usedSkillNames)
+  reserveConfiguredPiTargetNames(
+    [...plugin.commands].filter((command) => !command.disableModelInvocation).map((command) => command.name),
+    options.extraNameMaps?.prompts,
+    promptNames,
+  )
+
+  const skillDirs = sortedSkills.map((skill) => ({
+    name: resolvePiTargetName(skill.name, options.extraNameMaps?.skills, usedSkillNames),
+    sourceDir: skill.sourceDir,
+    sourceName: skill.name,
+  }))
+
+  const agentNames = sortedAgents.map((agent) =>
+    resolvePiTargetName(agent.name, options.extraNameMaps?.agents, usedSkillNames),
+  )
+
+  const agentMap: Record<string, string> = {}
+
+  const skillMap: Record<string, string> = {}
+  sortedSkills.forEach((skill, i) => {
+    const emitted = skillDirs[i].name
+    skillMap[skill.name] = emitted
+    addQualifiedAlias(skillMap, plugin.manifest.name, skill.name, emitted)
+  })
 
-  const prompts = plugin.commands
+  const convertibleCommands = [...plugin.commands]
     .filter((command) => !command.disableModelInvocation)
-    .map((command) => convertPrompt(command, promptNames))
+    .sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)
+  const promptTargetNames = convertibleCommands.map((command) =>
+    resolvePiTargetName(command.name, options.extraNameMaps?.prompts, promptNames),
+  )
+
+  const promptMap: Record<string, string> = {}
+  convertibleCommands.forEach((command, i) => {
+    const emitted = promptTargetNames[i]
+    promptMap[command.name] = emitted
+    addQualifiedAlias(promptMap, plugin.manifest.name, command.name, emitted)
+  })
+
+  sortedAgents.forEach((agent, i) => {
+    const emitted = agentNames[i]
+    agentMap[agent.name] = emitted
+    addQualifiedAlias(agentMap, plugin.manifest.name, agent.name, emitted)
+    const qualifiedAgentAlias = buildQualifiedAgentAlias(plugin.root, plugin.manifest.name, agent)
+    if (qualifiedAgentAlias) {
+      agentMap[qualifiedAgentAlias] = emitted
+    }
+  })
+
+  const nameMaps: PiNameMaps = { agents: agentMap, skills: skillMap, prompts: promptMap }
+  const transformMaps = mergeNameMaps(nameMaps, options.extraNameMaps)
+
+  const prompts = convertibleCommands.map((command, i) => convertPrompt(command, promptTargetNames[i], transformMaps, options))
 
-  const generatedSkills = plugin.agents.map((agent) => convertAgent(agent, usedSkillNames))
+  const generatedSkills = sortedAgents.map((agent, i) => convertAgent(agent, agentNames[i], transformMaps, options))
 
   const extensions = [
     {
@@ -34,35 +98,59 @@ export function convertClaudeToPi(
   ]
 
   return {
+    pluginName: plugin.manifest.name,
     prompts,
-    skillDirs: plugin.skills.map((skill) => ({
-      name: skill.name,
-      sourceDir: skill.sourceDir,
-    })),
+    skillDirs,
     generatedSkills,
     extensions,
     mcporterConfig: plugin.mcpServers ? convertMcpToMcporter(plugin.mcpServers) : undefined,
+    nameMaps,
   }
 }
 
-function convertPrompt(command: ClaudeCommand, usedNames: Set<string>) {
-  const name = uniqueName(normalizeName(command.name), usedNames)
+function addQualifiedAlias(map: Record<string, string>, pluginName: string | undefined, sourceName: string, emitted: string) {
+  if (!pluginName || !sourceName) return
+  map[`${pluginName}:${sourceName}`] = emitted
+}
+
+function buildQualifiedAgentAlias(root: string, pluginName: string | undefined, agent: ClaudeAgent): string | undefined {
+  if (!pluginName) return undefined
+
+  const agentsRoot = path.join(root, "agents")
+  const relative = path.relative(agentsRoot, agent.sourcePath)
+  if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) {
+    return undefined
+  }
+
+  const withoutExt = relative.replace(/\.md$/i, "")
+  const segments = withoutExt.split(path.sep).filter(Boolean)
+  if (segments.length <= 1) {
+    return `${pluginName}:${agent.name}`
+  }
+
+  return [pluginName, ...segments.slice(0, -1), agent.name].join(":")
+}
+
+function convertPrompt(command: ClaudeCommand, name: string, nameMaps: PiNameMaps, options: ClaudeToPiOptions) {
   const frontmatter: Record<string, unknown> = {
     description: command.description,
     "argument-hint": command.argumentHint,
   }
 
-  let body = transformContentForPi(command.body)
-  body = appendCompatibilityNoteIfNeeded(body)
+  const body = appendCompatibilityNoteIfNeeded(transformPiBodyContent(command.body, nameMaps, {
+    preserveUnknownQualifiedRefs: options.preserveUnknownQualifiedRefs,
+    rejectUnknownQualifiedTaskRefs: options.rejectUnknownQualifiedTaskRefs,
+    rejectUnresolvedFirstPartyQualifiedRefs: options.rejectUnresolvedFirstPartyQualifiedRefs,
+  }))
 
   return {
     name,
     content: formatFrontmatter(frontmatter, body.trim()),
+    sourceName: command.name,
   }
 }
 
-function convertAgent(agent: ClaudeAgent, usedNames: Set<string>): PiGeneratedSkill {
-  const name = uniqueName(normalizeName(agent.name), usedNames)
+function convertAgent(agent: ClaudeAgent, name: string, nameMaps: PiNameMaps, options: ClaudeToPiOptions): PiGeneratedSkill {
   const description = sanitizeDescription(
     agent.description ?? `Converted from Claude agent ${agent.name}`,
   )
@@ -77,77 +165,24 @@ function convertAgent(agent: ClaudeAgent, usedNames: Set<string>): PiGeneratedSk
     sections.push(`## Capabilities\n${agent.capabilities.map((capability) => `- ${capability}`).join("\n")}`)
   }
 
-  const body = [
+  const body = transformPiBodyContent([
     ...sections,
     agent.body.trim().length > 0
       ? agent.body.trim()
       : `Instructions converted from the ${agent.name} agent.`,
-  ].join("\n\n")
+  ].join("\n\n"), nameMaps, {
+    preserveUnknownQualifiedRefs: options.preserveUnknownQualifiedRefs,
+    rejectUnknownQualifiedTaskRefs: options.rejectUnknownQualifiedTaskRefs,
+    rejectUnresolvedFirstPartyQualifiedRefs: options.rejectUnresolvedFirstPartyQualifiedRefs,
+  })
 
   return {
     name,
     content: formatFrontmatter(frontmatter, body),
+    sourceName: agent.name,
   }
 }
 
-export function transformContentForPi(body: string): string {
-  let result = body
-
-  // Task repo-research-analyst(feature_description) or Task compound-engineering:research:repo-research-analyst(args)
-  // -> Run subagent with agent="repo-research-analyst" and task="feature_description"
-  const taskPattern = /^(\s*-?\s*)Task\s+([a-z][a-z0-9:-]*)\(([^)]*)\)/gm
-  result = result.replace(taskPattern, (_match, prefix: string, agentName: string, args: string) => {
-    const finalSegment = agentName.includes(":") ? agentName.split(":").pop()! : agentName
-    const skillName = normalizeName(finalSegment)
-    const trimmedArgs = args.trim().replace(/\s+/g, " ")
-    return trimmedArgs
-      ? `${prefix}Run subagent with agent=\"${skillName}\" and task=\"${trimmedArgs}\".`
-      : `${prefix}Run subagent with agent=\"${skillName}\".`
-  })
-
-  // Claude-specific tool references
-  result = result.replace(/\bAskUserQuestion\b/g, "ask_user_question")
-  result = result.replace(/\bTodoWrite\b/g, "file-based todos (todos/ + /skill:todo-create)")
-  result = result.replace(/\bTodoRead\b/g, "file-based todos (todos/ + /skill:todo-create)")
-
-  // /command-name or /workflows:command-name -> /workflows-command-name
-  const slashCommandPattern = /(?<![:\w])\/([a-z][a-z0-9_:-]*?)(?=[\s,."')\]}`]|$)/gi
-  result = result.replace(slashCommandPattern, (match, commandName: string) => {
-    if (commandName.includes("/")) return match
-    if (["dev", "tmp", "etc", "usr", "var", "bin", "home"].includes(commandName)) {
-      return match
-    }
-
-    if (commandName.startsWith("skill:")) {
-      const skillName = commandName.slice("skill:".length)
-      return `/skill:${normalizeName(skillName)}`
-    }
-
-    const withoutPrefix = commandName.startsWith("prompts:")
-      ? commandName.slice("prompts:".length)
-      : commandName
-
-    return `/${normalizeName(withoutPrefix)}`
-  })
-
-  return result
-}
-
-function appendCompatibilityNoteIfNeeded(body: string): string {
-  if (!/\bmcp\b/i.test(body)) return body
-
-  const note = [
-    "",
-    "## Pi + MCPorter note",
-    "For MCP access in Pi, use MCPorter via the generated tools:",
-    "- `mcporter_list` to inspect available MCP tools",
-    "- `mcporter_call` to invoke a tool",
-    "",
-  ].join("\n")
-
-  return body + note
-}
-
 function convertMcpToMcporter(servers: Record<string, ClaudeMcpServer>): PiMcporterConfig {
   const mcpServers: Record<string, PiMcporterServer> = {}
 
@@ -173,36 +208,69 @@ function convertMcpToMcporter(servers: Record<string, ClaudeMcpServer>): PiMcpor
   return { mcpServers }
 }
 
-function normalizeName(value: string): string {
-  const trimmed = value.trim()
-  if (!trimmed) return "item"
-  const normalized = trimmed
-    .toLowerCase()
-    .replace(/[\\/]+/g, "-")
-    .replace(/[:\s]+/g, "-")
-    .replace(/[^a-z0-9_-]+/g, "-")
-    .replace(/-+/g, "-")
-    .replace(/^-+|-+$/g, "")
-  return normalized || "item"
-}
-
 function sanitizeDescription(value: string, maxLength = PI_DESCRIPTION_MAX_LENGTH): string {
   const normalized = value.replace(/\s+/g, " ").trim()
   if (normalized.length <= maxLength) return normalized
   const ellipsis = "..."
   return normalized.slice(0, Math.max(0, maxLength - ellipsis.length)).trimEnd() + ellipsis
 }
 
-function uniqueName(base: string, used: Set<string>): string {
-  if (!used.has(base)) {
-    used.add(base)
-    return base
+function mergeNameMaps(primary: PiNameMaps, secondary?: PiNameMaps): PiNameMaps {
+  return {
+    agents: { ...(secondary?.agents ?? {}), ...(primary.agents ?? {}) },
+    skills: { ...(secondary?.skills ?? {}), ...(primary.skills ?? {}) },
+    prompts: { ...(secondary?.prompts ?? {}), ...(primary.prompts ?? {}) },
+  }
+}
+
+function resolvePiTargetName(sourceName: string, configuredMap: Record<string, string> | undefined, usedNames: Set<string>): string {
+  const configured = configuredMap?.[sourceName]
+  if (configured && isSafePiManagedName(configured)) {
+    usedNames.add(configured)
+    return configured
   }
-  let index = 2
-  while (used.has(`${base}-${index}`)) {
-    index += 1
+
+  return uniquePiSkillName(normalizePiSkillName(sourceName), usedNames)
+}
+
+function reserveConfiguredPiTargetNames(
+  sourceNames: string[],
+  configuredMap: Record<string, string> | undefined,
+  usedNames: Set<string>,
+) {
+  const reservedBySource = new Map<string, string>()
+
+  for (const sourceName of sourceNames) {
+    const configured = configuredMap?.[sourceName]
+    if (!configured || !isSafePiManagedName(configured)) continue
+
+    const existingSource = reservedBySource.get(configured)
+    if (existingSource && existingSource !== sourceName) {
+      throw new Error(`Configured Pi target name collision for ${sourceName}: ${configured}`)
+    }
+
+    reservedBySource.set(configured, sourceName)
+    usedNames.add(configured)
+  }
+}
+
+function assertNoConfiguredSharedTargetConflicts(
+  skillMap: Record<string, string> | undefined,
+  agentMap: Record<string, string> | undefined,
+) {
+  const reserved = new Map<string, string>()
+
+  for (const [sourceName, configured] of Object.entries(skillMap ?? {})) {
+    if (!isSafePiManagedName(configured)) continue
+    reserved.set(configured, sourceName)
+  }
+
+  for (const [sourceName, configured] of Object.entries(agentMap ?? {})) {
+    if (!isSafePiManagedName(configured)) continue
+    const existing = reserved.get(configured)
+    if (existing && existing !== sourceName) {
+      throw new Error(`Configured Pi target name collision for ${sourceName}: ${configured}`)
+    }
+    reserved.set(configured, sourceName)
   }
-  const name = `${base}-${index}`
-  used.add(name)
-  return name
 }