feat: add lark router and setup-lark-skills skills for opt-in skill isolation - #2076
Open
AKAZIK-py wants to merge 5 commits into
Open
feat: add lark router and setup-lark-skills skills for opt-in skill isolation#2076AKAZIK-py wants to merge 5 commits into
AKAZIK-py wants to merge 5 commits into
Conversation
github-actions
Bot
added
the
size/S
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Lark capability router for 27 ChangesLark skill management
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/setup-lark-skills/SKILL.md`:
- Around line 18-31: Update the skill discovery step in “定位已安装的 skill 文件” to
assign and export exactly one `$ROOT` from the candidate directories or fallback
search. Validate that discovery returns one match, and exit with a clear error
when no matches or multiple matches are found before any subsequent commands use
`$ROOT`.
- Around line 35-40: Update the activation detection and editing logic in the
SKILL.md processing loop to parse only each file’s YAML frontmatter block. Check
and modify disable-model-invocation within that delimited block, leaving
identical text in the document body untouched and preserving the existing
activation-state behavior.
🪄 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 Plus
Run ID: c81a8bc6-c387-4d57-bd24-fda6376fa118
📒 Files selected for processing (2)
skills/lark/SKILL.mdskills/setup-lark-skills/SKILL.md
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/setup-lark-skills/SKILL.md`:
- Around line 112-116: Update the awk logic in the is_disabled block to detect
any existing disable-model-invocation field within frontmatter and rewrite its
value to true, rather than inserting a duplicate. Track whether the field was
found, and preserve insertion immediately after name: only when no existing
field is present.
- Around line 118-126: Update the sleep/wake file-replacement workflows around
the awk transformations to preserve the original file’s permissions and ACLs
when creating the temporary file and atomically renaming it. Apply the
metadata-preserving workflow to both operations, ensure temporary files are
cleaned up, and report failures from awk or mv instead of silently continuing.
🪄 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 Plus
Run ID: f2d9ddf2-3879-4480-b426-f0b3b0ea1901
📒 Files selected for processing (1)
skills/setup-lark-skills/SKILL.md
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 `@skills/setup-lark-skills/SKILL.md`:
- Around line 114-119: Update the selected-skill normalization flow around
is_disabled so duplicate disable-model-invocation fields cannot bypass
rewriting. Either always run the existing idempotent awk rewrite for selected
skills, or tighten is_disabled to succeed only when the frontmatter contains
exactly one matching field with value true; preserve the rewrite’s behavior of
producing a single true field.
🪄 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 Plus
Run ID: 32f4c9b8-4352-4db3-b61e-ecd1ccf9354c
📒 Files selected for processing (1)
skills/setup-lark-skills/SKILL.md
…nd frontmatter scoping 1. Step 1 now assigns and exports exactly one $ROOT: candidates are collected, find(1) fallback only runs when none match, and discovery exits with a clear error on zero or multiple matches instead of letting later commands run against an unset/ambiguous ROOT. 2. State detection and sleep/wake edits now operate only within the YAML frontmatter block (first --- to second ---) via awk, so a disable-model-invocation line appearing in the document body (docs, code samples) is neither misread as state nor deleted by wake. Tolerates trailing whitespace; awk is identical on macOS/Linux so the platform-specific sed note is dropped. 3. Sleep command wraps the edit in a brace group so the idempotent re-run path (already disabled) skips awk+mv entirely — previously the 'A || B > tmp && mv tmp f' chaining ran mv with no tmp file, emitting a spurious error and a non-zero exit under set -e. Verified by an adversarial harness (28 checks): discovery 0/1/N-match and find fallback, frontmatter-vs-body scoping incl. markdown --- rules, insert position/idempotency, wake preserving body lines, and an end-to-end sleep+wake round-trip on the real repo skills that leaves files byte-identical.
…; preserve file metadata Round-2 CodeRabbit review on e02928f: 1. Sleep no longer inserts a duplicate disable-model-invocation line when frontmatter already carries the field with any value (e.g. false): the awk pass rewrites the first occurrence to 'true' in place and drops any further duplicates; it only inserts after name: when the field is entirely absent. Wake now removes the field regardless of its value, restoring the default (field-absent) state. 2. Sleep/wake replacements now go through 'cp -a' to clone the original file's metadata (mode/ACL/ownership/timestamps) onto the temp file before awk rewrites it and mv atomically replaces the original — previously the temp file got umask-default permissions, silently widening a private (e.g. 600) SKILL.md. Any failure in the cp/awk/mv chain now removes the temp file and exits non-zero with a clear error instead of leaving debris or reporting success. Adversarial harness extended to 36 checks (all passing): false-field rewrite in both field orders, multi-false collapse, pre-corrupted true+false duplicates (sleep stays a byte-identical no-op, wake clears all), mode-600 preservation across sleep+wake, and the missing-file failure contract (error message, exit 1, no temp residue).
…t normalized Round-3 CodeRabbit review: is_disabled succeeded on ANY true field, so a hand-corrupted frontmatter carrying both 'true' and 'false' lines was treated as disabled and skipped by sleep — while YAML parsers honoring the last value would keep the skill auto-invoked, silently defying the user's intent. is_disabled now requires exactly one disable-model-invocation field in frontmatter with value true. A duplicated/conflicting block fails the check, so the scan reports it as active and the idempotent sleep pass normalizes it to a single 'true' line (first occurrence rewritten, rest dropped). Clean single-true files still short-circuit to a byte-identical no-op, preserving the idempotency contract. Adversarial harness now at 38 checks: corrupted true+false is reported not-disabled, sleep normalizes it, sleep is byte-identical idempotent afterwards, and wake still clears every field line at once.
AKAZIK-py
force-pushed
the
feat/lark-router-setup-skills
branch
from
July 30, 2026 11:30
73a76eb to
19da9dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/setup-lark-skills/SKILL.md`:
- Around line 52-55: Update the skill enumeration loop in SKILL.md to enable
Bash nullglob before expanding the lark-*/SKILL.md pattern, then explicitly
handle an empty or partial matching set so literal wildcard paths are never
processed or reported as active. Preserve the existing is_disabled status output
for skills that are present.
- Around line 48-50: Update is_disabled and the related frontmatter
status/editing logic around the activation and sleep operations to normalize a
trailing carriage return before matching delimiter and field lines, while
preserving the original line content when printing or rewriting files. Ensure
CRLF frontmatter is scanned and updated correctly without changing line-ending
style or reporting success when no change occurred.
🪄 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 Plus
Run ID: 6032e9c8-a524-4c94-84ae-152dbc46fdda
📒 Files selected for processing (2)
skills/lark/SKILL.mdskills/setup-lark-skills/SKILL.md
…kill enumeration - awk matchers now strip a trailing CR before matching (^---$ failed on CRLF delimiter lines, silently leaving CRLF files unchanged); output always prints the original line, and inserted disable-model-invocation lines follow the file's own line-ending style - status scan enables nullglob and aborts with a clear error when no lark-*/SKILL.md exists, instead of scanning a literal '*' path and reporting a fake '*: active'
Author
|
Both open threads addressed in b56a540:
|
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
2 participants
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
Users running many agent harnesses (Claude Code, pi, Kimi Code, etc.) currently get all 27
lark-*skill descriptions (~11.8 KB) injected into every session's system prompt, whether or not the session involves Lark. This PR adds two infrastructure skills — a router (lark) and a configurator (setup-lark-skills) — that give users an opt-in path to isolate the skill family behind explicit/skill:invocation, without changing the default auto-injected behavior of any existing skill.Changes
skills/lark/SKILL.md(new): the authoritative routing index over all 27lark-*skills — a 27-row intent→skill table, a dedicated disambiguation section for meeting-related skills (the highest-collision boundary:lark-vc/lark-vc-agent/lark-minutes/lark-note/lark-workflow-meeting-summary), and multi-intent chaining guidance. Markeddisable-model-invocation: trueso the router itself is user-invoked.skills/setup-lark-skills/SKILL.md(new): a configurator that lets users switch low-frequencylark-*skills to dormant (insertdisable-model-invocation: true) or restore them, with idempotent, frontmatter-scanning commands (works regardless of where the harness installed the skills; includes install-path detection and Linux/macOSsednotes). Also markeddisable-model-invocation: true.disable-model-invocationis an optional Agent Skills frontmatter field — harnesses that don't recognize it ignore it (verified against pi, which honors it natively, and this repo's own non-strict YAML frontmatter parser).Test Plan
node scripts/skill-format-check/index.js skillspasses (29 skill dirs, no errors/warnings)lark-cli skills list(v1.0.71) and againstupstream/mainskills/directory — no driftgit diff --stat: 2 new files only)make unit-test— not run locally (no Go toolchain on this machine); change does not touch Go sources, CI should be unaffectedRelated Issues
Summary by CodeRabbit
/skill:lark, including direct/skill:lark-<name>shortcuts, an agent fallback when skills aren’t loaded, and clearer meeting-related disambiguation (with a rule to avoid using it for non-飞书 requests)./skill:setup-lark-skillsto batch-managelark-*skills’ “activated vs sleeping” behavior.disable-model-invocation, plus preset selection profiles and restart/usage guidance.