Merge PR #2651: fix(coding-agent): hide plugin skills from slash commands
This commit is contained in:
can1357
@@ -58,6 +58,10 @@
|
||||
|
||||
- Changed the `eval` `agent()` helper to accept `return_handle` (Python) / `returnHandle` (JS): instead of bare text it returns a DAG node dict `{ text, output, handle, id, agent }` whose `handle` is the spawned agent's recoverable `agent://<id>` URI, so a downstream `pipeline`/`parallel` stage can wire a large transcript by reference instead of re-inlining it. The default path is unchanged (bare text, or the parsed object under `schema`).
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed Claude marketplace plugin skills appearing as bare slash commands while keeping real plugin `commands/` entries available ([#2645](https://github.com/can1357/oh-my-pi/issues/2645)).
|
||||
|
||||
## [15.13.3] - 2026-06-15
|
||||
|
||||
### Added
|
||||
|
||||
@@ -124,43 +124,6 @@ async function loadSkills(ctx: LoadContext): Promise<LoadResult<Skill>> {
|
||||
}
|
||||
return { items, warnings };
|
||||
}
|
||||
async function loadSkillSlashCommands(ctx: LoadContext, root: ClaudePluginRoot): Promise<LoadResult<SlashCommand>> {
|
||||
const { dir: skillsDir, warning } = await resolvePluginDir(root, ["skills"], "skills");
|
||||
const warnings: string[] = warning ? [warning] : [];
|
||||
const skillsResult = await scanSkillsFromDir(ctx, {
|
||||
dir: skillsDir,
|
||||
providerId: PROVIDER_ID,
|
||||
level: root.scope,
|
||||
});
|
||||
warnings.push(...(skillsResult.warnings ?? []));
|
||||
|
||||
const commands = await Promise.all(
|
||||
skillsResult.items.map(async skill => {
|
||||
const content = await readFile(skill.path);
|
||||
if (content === null) {
|
||||
warnings.push(`Failed to read skill slash command: ${skill.path}`);
|
||||
return null;
|
||||
}
|
||||
// Slash command name MUST come from the skill directory basename, not
|
||||
// frontmatter `name`: `expandSlashCommand` splits the command at the first
|
||||
// whitespace, so a display name like "Understand Anything" would never match
|
||||
// `/understand`. The documented layout is `skills/<name>/SKILL.md` → `/<name>`.
|
||||
const command: SlashCommand = {
|
||||
name: path.basename(path.dirname(skill.path)),
|
||||
path: skill.path,
|
||||
content,
|
||||
level: skill.level,
|
||||
_source: skill._source,
|
||||
};
|
||||
return command;
|
||||
}),
|
||||
);
|
||||
|
||||
return {
|
||||
items: commands.filter((command): command is SlashCommand => command !== null),
|
||||
warnings,
|
||||
};
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Slash Commands
|
||||
@@ -189,16 +152,14 @@ async function loadSlashCommands(ctx: LoadContext): Promise<LoadResult<SlashComm
|
||||
};
|
||||
},
|
||||
});
|
||||
const skillCommandResult = await loadSkillSlashCommands(ctx, root);
|
||||
return { commandResult, skillCommandResult, warning };
|
||||
return { commandResult, warning };
|
||||
}),
|
||||
);
|
||||
|
||||
for (const { commandResult, skillCommandResult, warning } of results) {
|
||||
for (const { commandResult, warning } of results) {
|
||||
if (warning) warnings.push(warning);
|
||||
items.push(...commandResult.items, ...skillCommandResult.items);
|
||||
items.push(...commandResult.items);
|
||||
if (commandResult.warnings) warnings.push(...commandResult.warnings);
|
||||
if (skillCommandResult.warnings) warnings.push(...skillCommandResult.warnings);
|
||||
}
|
||||
|
||||
return { items, warnings };
|
||||
|
||||
@@ -9,7 +9,7 @@ import {
|
||||
listClaudePluginRoots,
|
||||
parseClaudePluginsRegistry,
|
||||
} from "@oh-my-pi/pi-coding-agent/discovery/helpers";
|
||||
import { expandSlashCommand, loadSlashCommands } from "@oh-my-pi/pi-coding-agent/extensibility/slash-commands";
|
||||
import { loadSlashCommands } from "@oh-my-pi/pi-coding-agent/extensibility/slash-commands";
|
||||
import { discoverAgents } from "@oh-my-pi/pi-coding-agent/task/discovery";
|
||||
import "@oh-my-pi/pi-coding-agent/discovery/claude-plugins";
|
||||
import type { Skill } from "@oh-my-pi/pi-coding-agent/capability/skill";
|
||||
@@ -356,9 +356,9 @@ describe("listClaudePluginRoots", () => {
|
||||
expect(found).toBeDefined();
|
||||
expect(found?.path).toContain(path.join(".claude", "skills", "manifest-skill", "SKILL.md"));
|
||||
});
|
||||
test("exposes plugin skills as bare slash commands", async () => {
|
||||
const pluginsDir = path.join(tempDir, ".omp", "plugins");
|
||||
const pluginPath = path.join(tempDir, ".omp", "plugins", "cache", "plugins", "understand-anything");
|
||||
test("keeps plugin skills out of slash commands while loading them as skills", async () => {
|
||||
const pluginsDir = path.join(tempDir, ".claude", "plugins");
|
||||
const pluginPath = path.join(tempDir, "plugins", "understand-anything");
|
||||
await fs.mkdir(pluginsDir, { recursive: true });
|
||||
await fs.mkdir(path.join(pluginPath, "skills", "understand"), { recursive: true });
|
||||
|
||||
@@ -384,45 +384,12 @@ describe("listClaudePluginRoots", () => {
|
||||
);
|
||||
|
||||
const commands = await loadSlashCommands({ cwd: tempDir });
|
||||
const found = commands.find(command => command.name === "understand");
|
||||
const skills = await loadCapability<Skill>("skills", { cwd: tempDir });
|
||||
|
||||
expect(found?.description).toBe("Build an understanding graph");
|
||||
expect(expandSlashCommand("/understand --language zh", commands)).toContain("Analyze the project.");
|
||||
});
|
||||
test("uses skill directory basename when frontmatter name contains spaces", async () => {
|
||||
const pluginsDir = path.join(tempDir, ".omp", "plugins");
|
||||
const pluginPath = path.join(tempDir, ".omp", "plugins", "cache", "plugins", "display-name-skill");
|
||||
await fs.mkdir(pluginsDir, { recursive: true });
|
||||
await fs.mkdir(path.join(pluginPath, "skills", "understand"), { recursive: true });
|
||||
|
||||
const registry = {
|
||||
version: 2,
|
||||
plugins: {
|
||||
"display-name-skill@display-name-skill": [
|
||||
{
|
||||
scope: "user",
|
||||
installPath: pluginPath,
|
||||
version: "1.0.0",
|
||||
installedAt: "2026-06-12T00:00:00Z",
|
||||
lastUpdated: "2026-06-12T00:00:00Z",
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
await fs.writeFile(path.join(pluginsDir, "installed_plugins.json"), JSON.stringify(registry));
|
||||
await fs.writeFile(
|
||||
path.join(pluginPath, "skills", "understand", "SKILL.md"),
|
||||
"---\nname: Understand Anything\ndescription: Build an understanding graph\n---\nAnalyze the project.\n",
|
||||
expect(commands.find(command => command.name === "understand")).toBeUndefined();
|
||||
expect(skills.all.find(skill => skill.name === "understand")?.frontmatter?.description).toBe(
|
||||
"Build an understanding graph",
|
||||
);
|
||||
|
||||
const commands = await loadSlashCommands({ cwd: tempDir });
|
||||
// Skill is registered by directory basename so `/understand` resolves,
|
||||
// even though the frontmatter `name` is the multi-word display label.
|
||||
const found = commands.find(command => command.name === "understand");
|
||||
expect(found?.description).toBe("Build an understanding graph");
|
||||
expect(commands.find(command => command.name === "Understand Anything")).toBeUndefined();
|
||||
expect(expandSlashCommand("/understand", commands)).toContain("Analyze the project.");
|
||||
});
|
||||
|
||||
test("reads slash commands directory from plugin manifest slash-commands field", async () => {
|
||||
|
||||
Reference in New Issue
Block a user