docs(lark-shared): split SKILL.md into a slim core plus references - #2226
Open
leave330 wants to merge 11 commits into
Open
docs(lark-shared): split SKILL.md into a slim core plus references#2226leave330 wants to merge 11 commits into
leave330 wants to merge 11 commits into
Conversation
github-actions
Bot
added
the
size/L
|
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:
📝 WalkthroughWalkthrough
Changes共享 CLI 使用规则
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Agent
participant LarkCLI
participant User
Agent->>LarkCLI: 执行 auth login --no-wait --json
LarkCLI-->>Agent: 返回 verification_url 和授权信息
Agent->>User: 展示 URL 和二维码
User->>LarkCLI: 完成浏览器授权
Agent->>LarkCLI: 后续轮次执行 --device-code
LarkCLI-->>Agent: 返回登录结果
🚥 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.githubusercontent.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/lark-shared/references/lark-shared-auth-split-flow.md`:
- Line 39: Update the split-flow guidance around the prohibition on caching
verification_url or device_code to allow retaining the active device_code for
the current continuation. Require discarding it after completion or expiry, and
prohibit only stale or cross-flow reuse while preserving the required
authorization options when starting a new flow.
In `@skills/lark-shared/references/lark-shared-identity-and-permissions.md`:
- Line 63: Update the auth login rule near the documented scope requirement to
allow --recommend alongside --domain and --scope. Preserve the existing
incremental authorization and agent follow-up guidance.
🪄 Autofix
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: 0c2580fa-6f1d-4574-b01d-daa9f51514aa
📒 Files selected for processing (7)
skills/lark-shared/SKILL.mdskills/lark-shared/references/lark-shared-auth-split-flow.mdskills/lark-shared/references/lark-shared-config-init.mdskills/lark-shared/references/lark-shared-high-risk-approval.mdskills/lark-shared/references/lark-shared-identity-and-permissions.mdskills/lark-shared/references/lark-shared-output-contract.mdskills/lark-shared/references/lark-shared-update-notice.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@e34c504acf1068855f3c77c01406d3087be17292🧩 Skill updatenpx skills add larksuite/cli#docs/lark-shared-slim -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2226 +/- ##
=======================================
Coverage 76.46% 76.47%
=======================================
Files 1013 1013
Lines 112034 112079 +45
=======================================
+ Hits 85665 85707 +42
- Misses 19857 19859 +2
- Partials 6512 6513 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lark-shared is force-loaded by nearly every lark-* skill (26 skills, 190 references), so its body is paid on almost every task. Split the 211-line monolith into a slim always-loaded core plus references/ that are read only when relevant. This is deliberately a move, not a rewrite. Existing wording is carried over as-is wherever possible so the change stays easy to regression-test. The core keeps the rules whose violation is silent or has no self-announcing trigger: identity semantics, the ok == true success contract, non-blocking auth split-flow, QR forwarding, write/delete intent confirmation, the exit-10 approval gate, path handling and the no-secrets rule. Mechanics, field paths and step-by-step flows move to references/. New references, all carrying over existing text: - lark-shared-identity-and-permissions.md - lark-shared-auth-split-flow.md - lark-shared-high-risk-approval.md - lark-shared-output-contract.md - lark-shared-update-notice.md - lark-shared-config-init.md The body now also routes to references/lark-wiki-token-routing.md, which already existed but was unreachable from the skill body. Wording changes are confined to lark-shared-high-risk-approval.md, where the exit-10 guidance was generalized to match the implementation: the recovery flag is read from hint rather than assumed to be --yes, and --dry-run is suggested only where the target command supports it.
Regroup the always-loaded body so the safety rules read as one block: the no-secrets rule leads, write/delete intent confirmation and --dry-run preview follow, and the exit-10 approval gate and path handling close it out. Trim wording that restated consequences already covered by the linked reference, and align a few reference passages with the command surface (`config init` without the flag, envelope phrasing).
Replace the loose "other scenarios" link list with an explicit trigger index: each row names the conditions that oblige the agent to open that reference before taking the next step, and the table covers all six references rather than the three the list happened to mention. Trigger conditions beat prose pointers here because the failure mode is an agent that never opens the reference at all — a link it reads as optional is a link it skips. Emphasis markers on individual rules are dropped in favour of plain arrows, since the obligation now lives in the index.
Name what the three `_notice` keys mean for the agent rather than listing the keys themselves, so the trigger reads as a condition to recognize instead of a field spec to memorize.
leave330
force-pushed
the
docs/lark-shared-slim
branch
from
August 10, 2026 08:20
3dacd40 to
af7441a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Restate the identity trigger in the terms an agent actually encounters ("who am
I", "get the fields of the current identity") rather than in diagnostic
vocabulary, and gloss high-risk-write so the row is readable without already
knowing the term.
Use MUST for the read obligation, matching how the rest of the skill set marks
non-negotiable steps.
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/lark-shared/SKILL.md`:
- Line 46: Update the trigger text in SKILL.md to use the documented
`missing_scopes` field name instead of `missing_scope`, while preserving the
existing identity-and-permissions reference mapping and other trigger terms.
🪄 Autofix
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: 8f8270a2-6b62-4e8a-955f-31f6fe9b236c
📒 Files selected for processing (1)
skills/lark-shared/SKILL.md
The split-flow steps require carrying device_code from the first turn into the second, so a flat ban on caching it contradicted the procedure it accompanied. Scope the ban to reuse across flows, which is the case that actually matters. Also match the trigger table to the field name the error envelope carries (missing_scopes), keeping it consistent with the reference it routes to.
…scope flags Bring back the note that a bot reading user resources returns an empty success rather than an error. That failure is silent, so it belongs in the body: an agent that never learns it reads the empty result as "the user has no data". Also list --recommend alongside --scope and --domain as a way to specify the authorization range, matching what auth login actually accepts.
2 tasks
…sions Auth split-flow and identity/permission recovery are consulted in the same situation — an agent about to obtain or repair authorization — so a separate file bought a second hop without buying separation. Merge the split-flow steps into identity-and-permissions as its agent-initiated-auth section, and route its trigger row there. The non-blocking rule leaves the resident body with this merge; the trigger index already forces the read before any auth login is initiated, which is the moment the rule matters. Also reword the _notice.skills gloss to "out of sync" — the mismatch is bidirectional, not only the skills lagging the CLI.
Resolve the SKILL.md conflict in favor of the restructured body, and relocate the two sections #2288 added to the old monolith into lark-shared-identity-and-permissions.md verbatim, at the same relative positions they held upstream: identity continuity across command workflows (after the identity-selection principles) and the missing-scope vs resource-ACL recovery table (inside the permission-failure section). No upstream sentence is dropped.
Omitting --as does not keep the current identity — it hands the choice back to the profile default, and nothing errors when that happens. That silent switch is exactly the class of rule the resident body exists for, so state the kernel in rule 2; the mechanics and examples stay in the identity-and-permissions reference brought in from main.
Weaker models transcribe the visible link text when deciding what to read, and short aliases made them reconstruct the real filename — a step that misspells. Display the exact filename everywhere so copying the visible text yields a resolvable path.
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-sharedcarries the cross-cutting rules every otherlark-*skill depends on, and nearly all of them force-load it — 26 skills reference it from 190 places. That means its whole body is paid on almost every task. At 211 lines / ~3.6k tokens it was well past the 2k-token guidance for a skill body, and most of that weight was mechanics an agent only needs occasionally.This restructures the monolith into a slim always-loaded core plus
references/that are read on demand. The body drops to 48 lines / ~1.3k tokens (-64%), and the mechanics stay one hop away.The body keeps two kinds of content. Resident rules cover what fails silently if unknown: identity semantics (including that a bot reading user resources returns an empty success rather than an error), the
ok == truesuccess contract, QR forwarding for authorization URLs, write/delete intent confirmation, and the exit-10 approval gate. A trigger index then routes to the references: each row names the observable conditions — a field in an error envelope, an exit code, a command about to be run — under which the agent must read the corresponding reference before acting, so loading is driven by explicit triggers rather than a judgment call.Changes
skills/lark-shared/SKILL.md— restructured into resident rules, security rules, and a trigger-indexed reference table. 211 → 48 lines.lark-shared-identity-and-permissions.md— identity model, grant layers, permission recovery, the auth quick reference, and the agent-initiated (split-flow) authorization stepslark-shared-high-risk-approval.md— exit-10 envelope, field positions, risk predictionlark-shared-output-contract.md— success/error envelope shapeslark-shared-update-notice.md—_noticehandlinglark-shared-config-init.md— first-run configurationhintrather than assumed to be--yes;--dry-runis suggested only where the target command supports it;--recommendis listed as a third way to scopeauth login; and the credential-reuse ban is scoped to reuse across flows, so it no longer contradicts the split-flow steps it accompanies.version1.0.0 → 1.1.0.nameanddescriptionare unchanged, so skill triggering is unaffected.Test Plan
make build/make quality-gate/make unit-test— all pass. quality-gate is the meaningful automated gate here: it harvestslark-cli ...examples from the changed markdown and validates them against the command index../lark-cli skills read lark-shared— serves the rebuilt embedded tree and reportsversion: 1.1.0, confirming the change is live through the skills embed; each reference is readable through the built binary.references/*.mdtarget linked from the body resolves; no dead links.Not covered: there is no automated assertion that a given guardrail stays in the body, so a future slimming pass could weaken one without anything failing. A lightweight keyword assertion would close that gap and is worth adding separately.
Also unchanged on purpose: the path rule still states that path arguments accept cwd-relative paths only. That is not accurate for every command — some accept absolute paths — and is tracked as a follow-up.
Related Issues
None.