Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixes/register-cli-skill-collector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **Skills**: register `cli-skill-collector` in the agent-skills catalog (types union, curated entry, CLI id list) — #6294 shipped the `skills/cli-skill-collector/` directory without the catalog registration, so it was unreachable via the API and Integration CI failed on the catalog-integrity test; counts aligned (44 API+CLI, 45 with config)
1 change: 1 addition & 0 deletions src/lib/agentSkills/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const CLI_SKILL_IDS: readonly string[] = [
"cli-eval",
"cli-plugins-skills",
"cli-setup",
"cli-skill-collector",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Adding "cli-skill-collector" increases the total number of CLI skills to 21. However, several places in the codebase still hardcode the CLI total to 20, which will lead to incorrect coverage metrics (e.g., coverage exceeding 100% when all skills are present) and inconsistent API/A2A responses.\n\nPlease update the following locations to use 21 instead of 20:\n1. src/lib/agentSkills/catalog.ts (line 153):\n typescript\n cli: { have: cliHave, total: 21 },\n \n2. src/lib/agentSkills/types.ts (line 71):\n typescript\n cli: { have: number; total: 21 };\n \n3. src/lib/a2a/skills/listCapabilities.ts (lines 17 and 65):\n typescript\n cli: { have: number; total: 21 };\n cli: { have: coverage.cli.have, total: 21 },\n \n4. Also, update the outdated comments/descriptions mentioning "20 CLI skills" (e.g., src/lib/agentSkills/catalog.ts line 49 and open-sse/mcp-server/tools/agentSkillTools.ts line 76).

] as const;

// ── Module-scope cache ──────────────────────────────────────────────────────
Expand Down
3 changes: 2 additions & 1 deletion src/lib/agentSkills/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export type SkillArea =
| "cli-batches"
| "cli-eval"
| "cli-plugins-skills"
| "cli-setup";
| "cli-setup"
| "cli-skill-collector";

export interface AgentSkill {
id: string; // canonical id (e.g. "omni-providers", "cli-serve")
Expand Down
9 changes: 9 additions & 0 deletions src/shared/constants/agentSkills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,15 @@ export const CURATED_SKILLS: CuratedSkillEntry[] = [
area: "cli-setup",
icon: "build",
},
{
id: "cli-skill-collector",
name: "CLI: Agent Skill Collector",
description:
"Detect installed CLI coding tools (Claude Code, Codex, Cursor, Copilot, Cline and more), search GitHub for matching agent skills, and install them to the detected tools via OmniRoute's built-in APIs.",
category: "cli",
area: "cli-setup",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In src/lib/agentSkills/types.ts, "cli-skill-collector" was added as a new unique SkillArea in the union type. However, here the area is set to "cli-setup". To maintain consistency with other CLI skills (where each skill has its own dedicated area matching its ID) and to utilize the newly added type, the area should be updated to "cli-skill-collector".

Suggested change
area: "cli-setup",
area: "cli-skill-collector",

icon: "extension",
},

// ── Config Skills ────────────────────────────────────────────────────────────

Expand Down
6 changes: 3 additions & 3 deletions tests/integration/agent-skills-content.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Integration tests for Agent Skills content integrity.
*
* Verifies:
* 1. All 43 skill IDs from catalog have skills/{id}/ folder with SKILL.md.
* 1. All 44 skill IDs from catalog have skills/{id}/ folder with SKILL.md.
* 2. Zero omniroute-* folders remain (post-prune: old omniroute-* skill dirs were removed).
* 3. 10 specific IDs have <!-- skill:custom-start --> ... <!-- skill:custom-end --> blocks:
* omni-mcp, omni-compression, cli-providers, cli-eval, omni-agents-a2a,
Expand Down Expand Up @@ -37,7 +37,7 @@ const CUSTOM_BLOCK_IDS = [

// ── §1: All 42 catalog IDs have skills/{id}/SKILL.md ─────────────────────────

test("all 43 catalog IDs have a skills/{id}/ directory", () => {
test("all 44 catalog IDs have a skills/{id}/ directory", () => {
const missing: string[] = [];
for (const id of ALL_IDS) {
const dirPath = path.join(SKILLS_DIR, id);
Expand All @@ -48,7 +48,7 @@ test("all 43 catalog IDs have a skills/{id}/ directory", () => {
assert.deepEqual(missing, [], `Missing skill directories: ${missing.join(", ")}`);
});

test("all 43 catalog IDs have a skills/{id}/SKILL.md file", () => {
test("all 44 catalog IDs have a skills/{id}/SKILL.md file", () => {
const missing: string[] = [];
for (const id of ALL_IDS) {
const skillPath = path.join(SKILLS_DIR, id, "SKILL.md");
Expand Down
14 changes: 7 additions & 7 deletions tests/integration/agent-skills-discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ test("every CLI skill ID has skills/<id>/SKILL.md on disk", () => {
assert.deepEqual(missing, [], `Missing CLI SKILL.md files: ${missing.join(", ")}`);
});

test("total skill count is exactly 43 (23 API + 20 CLI)", () => {
assert.equal(API_SKILL_IDS.length + CLI_SKILL_IDS.length, 43);
test("total skill count is exactly 44 (23 API + 21 CLI)", () => {
assert.equal(API_SKILL_IDS.length + CLI_SKILL_IDS.length, 44);
});

// ── §2: Frontmatter validation ────────────────────────────────────────────────
Expand Down Expand Up @@ -120,11 +120,11 @@ test("each SKILL.md body is at least 100 chars", () => {

// ── §3: MCP tool omniroute_agent_skills_list ─────────────────────────────────

test("MCP omniroute_agent_skills_list handler returns count 44 (43 + config)", async () => {
test("MCP omniroute_agent_skills_list handler returns count 45 (44 + config)", async () => {
const result = await agentSkillTools.omniroute_agent_skills_list.handler({});
assert.equal(result.count, 44, `Expected 44 but got ${result.count}`);
assert.equal(result.count, 45, `Expected 45 but got ${result.count}`);
assert.ok(Array.isArray(result.skills));
assert.equal(result.skills.length, 44);
assert.equal(result.skills.length, 45);
});

test("MCP omniroute_agent_skills_list result has all 42 IDs", async () => {
Expand Down Expand Up @@ -157,9 +157,9 @@ test("A2A list-capabilities artifact content contains 42 skill IDs as table rows
assert.ok(rows.length >= 42, `Expected at least 42 data rows but got ${rows.length}`);
});

test("A2A list-capabilities metadata.totalSkills === 44 (43 + config)", async () => {
test("A2A list-capabilities metadata.totalSkills === 45 (44 + config)", async () => {
const result = await executeListCapabilities(stubTask);
assert.equal(result.metadata.totalSkills, 44);
assert.equal(result.metadata.totalSkills, 45);
});

test("A2A list-capabilities artifact contains all 42 skill IDs", async () => {
Expand Down
12 changes: 6 additions & 6 deletions tests/unit/listCapabilities-a2a.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* Verifies:
* - Return shape matches §3.7 contract
* - Markdown table contains all 43 skill IDs
* - Markdown table contains all 44 skill IDs
* - Coverage bounds are within declared totals
* - metadata.source === "agent-skills-catalog"
* - metadata.generatedAt is an ISO datetime string
Expand Down Expand Up @@ -31,20 +31,20 @@ test("executeListCapabilities returns shape matching §3.7 contract", async () =
const { metadata } = result;
assert.ok(metadata, "metadata exists");
assert.equal(metadata.source, "agent-skills-catalog", "metadata.source matches");
assert.equal(metadata.totalSkills, 44, "metadata.totalSkills === 44 (43 + config)");
assert.equal(metadata.totalSkills, 45, "metadata.totalSkills === 45 (44 + config)");
assert.ok(metadata.coverage, "metadata.coverage exists");
assert.ok(metadata.coverage.api, "metadata.coverage.api exists");
assert.ok(metadata.coverage.cli, "metadata.coverage.cli exists");
assert.equal(metadata.coverage.api.total, 23, "api.total === 23");
assert.equal(metadata.coverage.cli.total, 20, "cli.total === 20");
});

test("executeListCapabilities markdown table contains all 43 API+CLI skill IDs", async () => {
test("executeListCapabilities markdown table contains all 44 API+CLI skill IDs", async () => {
const result = await executeListCapabilities(stubTask);
const content = result.artifacts[0].content;

const allIds = [...API_SKILL_IDS, ...CLI_SKILL_IDS] as string[];
assert.equal(allIds.length, 43, "API+CLI catalog declares 43 skill IDs");
assert.equal(allIds.length, 44, "API+CLI catalog declares 44 skill IDs");

for (const id of allIds) {
assert.ok(content.includes(id), `Markdown table missing skill ID: ${id}`);
Expand All @@ -58,11 +58,11 @@ test("metadata.coverage.api.have is within [0, 23]", async () => {
assert.ok(api.have <= 23, "api.have <= 23");
});

test("metadata.coverage.cli.have is within [0, 20]", async () => {
test("metadata.coverage.cli.have is within [0, 21]", async () => {
const result = await executeListCapabilities(stubTask);
const { cli } = result.metadata.coverage;
assert.ok(cli.have >= 0, "cli.have >= 0");
assert.ok(cli.have <= 20, "cli.have <= 20");
assert.ok(cli.have <= 21, "cli.have <= 21");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The assertion assert.ok(cli.have <= 21, "cli.have <= 21"); correctly reflects that there are now 21 CLI skills. However, the test on line 39 still asserts that metadata.coverage.cli.total is 20:\ntypescript\nassert.equal(metadata.coverage.cli.total, 20, "cli.total === 20");\n\nOnce the underlying implementation is updated to return 21 for the CLI total, please update the assertion on line 39 to:\ntypescript\nassert.equal(metadata.coverage.cli.total, 21, "cli.total === 21");\n

});

test("metadata.generatedAt is a valid ISO datetime", async () => {
Expand Down
Loading