feat: add skills command to read embedded skill content#1277
Open
dc-bytedance wants to merge 2 commits into
Open
feat: add skills command to read embedded skill content#1277dc-bytedance wants to merge 2 commits into
dc-bytedance wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds a skill content reader, a new ChangesSkills command
Sequence DiagramsequenceDiagram
participant User
participant SkillCmd as skills command
participant Reader as skillcontent.Reader
participant FS as embedded fs.FS
User->>SkillCmd: list [skill[/path]]
SkillCmd->>SkillCmd: validate args (≤1)
SkillCmd->>Reader: List() or ListPath()
Reader->>FS: readdir, open SKILL.md
FS-->>Reader: content, entries
Reader->>Reader: parse YAML frontmatter
Reader-->>SkillCmd: []SkillInfo or []DirEntry
SkillCmd->>SkillCmd: emit JSON envelope
SkillCmd-->>User: JSON or error
User->>SkillCmd: read name[/path] [path]
SkillCmd->>SkillCmd: parseReadTarget()
SkillCmd->>Reader: ReadSkill() or ReadReference()
Reader->>FS: stat, open file
FS-->>Reader: bytes
Reader-->>SkillCmd: content, cleanPath
SkillCmd->>SkillCmd: append guidance (SKILL.md only)
SkillCmd->>SkillCmd: JSON or raw markdown
SkillCmd-->>User: output or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
the
size/L
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b0ac518d9a77b07d107ff1ee86fb7f39b26a139a🧩 Skill updatenpx skills add dc-bytedance/cli#feat/skill-content-read -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1277 +/- ##
==========================================
+ Coverage 70.33% 70.39% +0.06%
==========================================
Files 672 675 +3
Lines 65322 65542 +220
==========================================
+ Hits 45941 46139 +198
- Misses 15728 15740 +12
- Partials 3653 3663 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/skill/skill_test.go`:
- Around line 25-33: The run test helper does not isolate CLI config state, so
before calling cmdutil.TestFactory(t, ...) set the environment variable
LARKSUITE_CLI_CONFIG_DIR to a temp dir using
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()); update the run function
(around invocation of cmdutil.TestFactory and creation of NewCmdSkill) to call
t.Setenv(...) first to ensure each test gets an isolated config directory.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 699b2379-1c2e-460f-aae9-d334c1633dd0
📒 Files selected for processing (6)
cmd/build.gocmd/skill/skill.gocmd/skill/skill_test.gointernal/skillcontent/reader.gointernal/skillcontent/reader_test.goskills_embed.go
Add a top-level `lark-cli skills` meta command group that reads skill content compiled into the binary at build time via go:embed, so the content an AI agent reads always matches the running CLI version. It is an additional, version-consistent source and a fallback for when skills are not installed or are stale; it does not replace or thin the on-disk skills. Commands: - `skills list` lists every embedded skill with name, description, and the frontmatter metadata block. - `skills list <name>` / `skills list <name>/<sub>` list one directory layer (ls-style, no recursion); entries are skill-prefixed paths that can be fed straight back to `read`. - `skills read <name>` prints the skill's SKILL.md and appends a one-line tip steering the model to fetch reference files via this command. - `skills read <name> <path>` / `skills read <name>/<path>` print any file under the skill directory (no tip). - `--json` wraps output in an envelope; on the main SKILL.md it carries a separate `guidance` field instead of merging the tip into content. Implementation: - skills/ is embedded from the repo-root package main (go:embed cannot cross parent dirs) and injected into cmd/skill via a package variable, avoiding any change to cmd.Execute's signature. - internal/skillcontent is pure logic over an injected fs.FS; reference reads and directory listings share one path-traversal guard that rejects absolute paths, "..", and "..\\" escapes. - Errors are typed (validation -> exit 2, file_io -> exit 5); rejected paths never write anything to stdout. Risk is set per leaf subcommand (read); the group carries none, matching the config/service command groups. The commands need no auth (local embed reads only).
Set LARKSUITE_CLI_CONFIG_DIR to a per-test temp dir in the shared run() helper so the command tests never touch the real config dir, matching the repo-wide test convention. Addresses a CodeRabbit review comment.
SunPeiYang996
force-pushed
the
feat/skill-content-read
branch
from
8d67578 to
b0ac518
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
lark-cli ships one skill per business domain, but skills install through a separate channel and carry their own version, so after a CLI upgrade an on-disk skill can fall out of sync with the binary (the docs v1/v2 drift is a concrete case). This PR adds a
lark-cli skillscommand group that reads skill content compiled into the binary at build time viago:embed, giving AI agents a version-consistent source — and a fallback for when skills are not installed or are stale. It does not replace or thin the on-disk skills.Changes
skills_embed.go(package main)://go:embed skillsplus aninit()thatfs.Sub-roots the tree and injects it intocmd/skillvia a package variable, avoiding any change tocmd.Execute's signature.internal/skillcontent/reader.go: pure logic over an injectedfs.FS—List(skills with name/description/frontmatter metadata),ListPath(one directory layer, ls-style, no recursion),ReadSkill,ReadReference, and a sharedcleanSubPathtraversal guard.cmd/skill/skill.go: theskillscommand group withlist(catalog, or one-layer listing of<name>/<name>/<sub>) andread(<name>,<name> <path>, or<name>/<path>); reading the main SKILL.md appends a one-line guidance tip (a separateguidancefield under--json).cmd/build.go.cmd/skill/skill_test.go,internal/skillcontent/reader_test.go).Test Plan
make unit-testpassedlark-cli skills list,skills list lark-doc,skills list lark-doc/references,skills read lark-calendar [--json],skills read lark-doc references/lark-doc-fetch.md,skills read lark-doc/references/lark-doc-fetch.md; error paths (read no-such-skill,read lark-calendar ../../etc/passwd,read,list a b) all exit 2 with empty stdoutRelated Issues
N/A
Summary by CodeRabbit
New Features
skillsCLI group withskills listandskills read(supports--json).readoutput appends a one-line guidance tip when showing a skill's main content.Quality