feat(skills): add list command to show oo-managed skills#44
Conversation
Introduce `oo skills list` which scans the local Codex skills directory for child directories containing `.oo-metadata.json` and prints a summary with skill name, source package (or bundled marker), and recorded version. The `oo` skill is always listed first; remaining skills are sorted alphabetically. Includes unit tests for the listing/sorting logic, CLI integration tests for text and color output, and bilingual documentation. Signed-off-by: Kevin Cui <bh@bugs.cc>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR introduces a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ 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. Comment |
Why: GitHub Actions failed the PR because the skills list command\nused a chained terminal color helper shape that this codebase's\nTerminalFormatter type does not expose.\n\nWhat: Replace the chained hex-plus-bold calls in the command and\nits CLI color test with the supported nested formatter form.\n\nHow: Wrap the hex-colored text with the top-level bold formatter so\nthe output stays the same while remaining type-safe in CI.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/application/commands/skills/list.cli.test.ts (1)
25-27: Assert install setup success before continuing the test.At Line [25], the precondition step (
skills install) is not asserted. If setup fails, later assertions become noisy and less actionable.♻️ Suggested test hardening
- await sandbox.run(["skills", "install"], { + const installResult = await sandbox.run(["skills", "install"], { version: "9.9.9", }); + expect(installResult.exitCode).toBe(0); + expect(installResult.stderr).toBe("");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/application/commands/skills/list.cli.test.ts` around lines 25 - 27, The test calls sandbox.run(["skills", "install"], { version: "9.9.9" }) but does not assert the install succeeded; capture the result of sandbox.run and assert success (e.g., check result.exitCode === 0 or that stderr is empty / stdout contains a success message) before proceeding so failures in the setup step fail fast; update the test around the sandbox.run call to store its return value and add an assertion using the test framework's expect/assert helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/application/commands/skills/list.ts`:
- Line 145: Replace the invalid formatter chaining on colors.hex at the skill
rendering: colors.hex(managedSkillNameColor).bold(skill.name) is calling .bold()
on a TerminalFormatter function which doesn't exist; instead invoke the
formatter function with the value: use
colors.hex(managedSkillNameColor)(skill.name). Update the occurrence in the list
rendering (the line referencing managedSkillNameColor and skill.name) to call
the formatter rather than chaining .bold().
---
Nitpick comments:
In `@src/application/commands/skills/list.cli.test.ts`:
- Around line 25-27: The test calls sandbox.run(["skills", "install"], {
version: "9.9.9" }) but does not assert the install succeeded; capture the
result of sandbox.run and assert success (e.g., check result.exitCode === 0 or
that stderr is empty / stdout contains a success message) before proceeding so
failures in the setup step fail fast; update the test around the sandbox.run
call to store its return value and add an assertion using the test framework's
expect/assert helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 233fc322-cdd6-4ddb-981d-89b9cb94f8ab
📒 Files selected for processing (7)
docs/commands.mddocs/commands.zh-CN.mdsrc/application/commands/skills/index.tssrc/application/commands/skills/list.cli.test.tssrc/application/commands/skills/list.test.tssrc/application/commands/skills/list.tssrc/i18n/catalog.ts
Introduce
oo skills listwhich scans the local Codex skills directory for child directories containing.oo-metadata.jsonand prints a summary with skill name, source package (or bundled marker), and recorded version. Theooskill is always listed first; remaining skills are sorted alphabetically.Includes unit tests for the listing/sorting logic, CLI integration tests for text and color output, and bilingual documentation.