Skip to content

feat(skills): add search command for published skills#33

Merged
BlackHole1 merged 2 commits into
mainfrom
add-skills-search
Mar 28, 2026
Merged

feat(skills): add search command for published skills#33
BlackHole1 merged 2 commits into
mainfrom
add-skills-search

Conversation

@BlackHole1
Copy link
Copy Markdown
Member

Introduce oo skills search <text> (aliased as find) to query the skills search API with free-form text. Supports --keywords for comma-separated keyword filtering and --format=json/--json for structured output. Text output renders title, description, and source package reference per skill entry. Each invocation requests at most 5 results.

Introduce `oo skills search <text>` (aliased as `find`) to query the
skills search API with free-form text. Supports `--keywords` for
comma-separated keyword filtering and `--format=json`/`--json` for
structured output. Text output renders title, description, and source
package reference per skill entry. Each invocation requests at most 5
results.

Signed-off-by: Kevin Cui <bh@bugs.cc>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 28, 2026

Caution

Review failed

Pull request was closed or merged during review

Summary by CodeRabbit

  • New Features

    • Added oo skills search <text> (alias oo skills find <text>) to search published skills by free-form text
    • --keywords accepts comma-separated keywords; --format=json/--json prints JSON array
    • Results limited to at most 5 items; text view shows name, optional description, and source package
  • Documentation

    • Added English and Chinese docs for the new command and options
  • Tests

    • End-to-end and unit tests added to validate CLI behavior, output formats, and argument validation

Walkthrough

Adds a new CLI subcommand skills search <text> (alias find) that queries a skills-search API with required text, optional --keywords, and --format=json. Implements input validation, keyword normalization, authenticated requests with repeated keywords query params and size=5, response JSON parsing and Zod validation, and dual output modes (JSON array or formatted text with title/description/package). Adds i18n entries, docs (EN/zh-CN), wiring into the skills command, and comprehensive unit/CLI tests including request assertions and error handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • oomol-lab/oo-cli PR 1 — Adds and populates the skills command children (install/uninstall); directly related to adding skillsSearchCommand to the skills command.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title follows the required format <type>(<scope>): <subject> with feat as type and skills as scope, and clearly describes the main change: adding a search command for published skills.
Description check ✅ Passed The pull request description is directly related to the changeset, providing clear details about the new oo skills search command, its alias, supported options, output formats, and result limits.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/application/commands/skills/search.ts (2)

15-30: Consider consolidating duplicate schemas.

skillSearchJsonResponseSchema and skillSearchResponseSchema are identical. Both schemas parse the same structure with identical field definitions. This duplication may cause maintenance burden if the schema needs to change.

♻️ Proposed consolidation
-const skillSearchJsonResponseSchema = z.object({
-    data: z.array(skillSearchItemSchema).optional().default([]),
-}).passthrough();
-
-const skillSearchResponseSchema = z.object({
-    data: z.array(skillSearchItemSchema).optional().default([]),
-}).passthrough();
+const skillSearchResponseSchema = z.object({
+    data: z.array(skillSearchItemSchema).optional().default([]),
+}).passthrough();

-type SkillSearchJsonResponse = z.output<typeof skillSearchJsonResponseSchema>;
-type SkillSearchResponse = z.output<typeof skillSearchResponseSchema>;
+type SkillSearchResponse = z.output<typeof skillSearchResponseSchema>;

Then use SkillSearchResponse for both json and text in parseSkillsSearchResponse.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/application/commands/skills/search.ts` around lines 15 - 30,
skillSearchJsonResponseSchema and skillSearchResponseSchema are duplicates;
consolidate by keeping a single schema (e.g., skillSearchResponseSchema or
rename to SkillSearchResponseSchema) built from skillSearchItemSchema and use
that single schema everywhere (including where parseSkillsSearchResponse expects
json and text parsing). Update any references/imports/exports and related type
aliases so parseSkillsSearchResponse uses the consolidated schema for both
`json` and `text` paths, and remove the now-unneeded duplicate declaration to
avoid divergence.

64-68: mapInputError may not cover all validation failure scenarios.

The mapInputError function only produces an "invalid format" error, but the inputSchema validates both text (required string) and format (enum). If text is somehow invalid (e.g., passed as a non-string in programmatic usage), the error message would incorrectly reference "format".

However, since missingArgumentBehavior: "showHelp" handles the missing text case and Commander handles argument type coercion, this is likely a non-issue in practice.

Also applies to: 96-102

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/application/commands/skills/search.ts` around lines 64 - 68, The
mapInputError currently always returns an "invalid format" message while
inputSchema validates multiple fields (text, format, keywords); update
mapInputError (used where inputSchema is parsed) to inspect the ZodError issues
and return field-specific messages (e.g., "invalid text" when issue.path
includes "text", "invalid format" when path includes "format") or a generic
"invalid input" with the list of issue messages; reference the ZodError from
parsing inputSchema and adjust mapInputError to switch on issue.path/issue.code
so validation failures for text are reported correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/application/commands/skills/search.ts`:
- Around line 15-30: skillSearchJsonResponseSchema and skillSearchResponseSchema
are duplicates; consolidate by keeping a single schema (e.g.,
skillSearchResponseSchema or rename to SkillSearchResponseSchema) built from
skillSearchItemSchema and use that single schema everywhere (including where
parseSkillsSearchResponse expects json and text parsing). Update any
references/imports/exports and related type aliases so parseSkillsSearchResponse
uses the consolidated schema for both `json` and `text` paths, and remove the
now-unneeded duplicate declaration to avoid divergence.
- Around line 64-68: The mapInputError currently always returns an "invalid
format" message while inputSchema validates multiple fields (text, format,
keywords); update mapInputError (used where inputSchema is parsed) to inspect
the ZodError issues and return field-specific messages (e.g., "invalid text"
when issue.path includes "text", "invalid format" when path includes "format")
or a generic "invalid input" with the list of issue messages; reference the
ZodError from parsing inputSchema and adjust mapInputError to switch on
issue.path/issue.code so validation failures for text are reported correctly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7423168-29df-4bfe-acb5-a112d637dce2

📥 Commits

Reviewing files that changed from the base of the PR and between 029e6c0 and 8a09995.

⛔ Files ignored due to path filters (1)
  • src/application/commands/skills/__snapshots__/search.cli.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (7)
  • docs/commands.md
  • docs/commands.zh-CN.md
  • src/application/commands/skills/index.ts
  • src/application/commands/skills/search.cli.test.ts
  • src/application/commands/skills/search.test.ts
  • src/application/commands/skills/search.ts
  • src/i18n/catalog.ts

Apply field-specific terminal colors to skills search text output:
title in green (#59F78D) and package label in purple (#CAA8FA).
Color support is determined by the stdout writer capabilities.

Signed-off-by: Kevin Cui <bh@bugs.cc>
@BlackHole1 BlackHole1 merged commit b71de55 into main Mar 28, 2026
1 of 2 checks passed
@BlackHole1 BlackHole1 deleted the add-skills-search branch March 28, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant