feat(drive): Strengthen lark-drive high-risk write operations and read-only recognition boundaries.#1801
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR updates lark-drive documentation to clarify ChangesLark Drive skill documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6e4dfbac07121dd53dd9f22fea650aadaf972791🧩 Skill updatenpx skills add yballul-bytedance/cli#auto-research-sync/01KWW7ZHEWMRH9JSBMDAMSNJZ4/mr-1001-d0eab7a0 -y -g |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-drive/references/lark-drive-delete.md`:
- Around line 10-21: Clarify the deletion gate in lark-drive-delete.md so a raw
URL is not treated as a concrete delete target; update the guidance around the
delete prerequisites to say that `drive +delete` must use the resolved
`file-token` and `type` from prior inspection, and that URLs/tokens are only for
identifying or confirming candidates until resolved. Keep the existing
distinction in the deletion policy text between candidate discovery and
execution, and adjust the wording near the “目标明确” and `drive +delete --yes`
checks to reference the resolved target pair rather than the URL itself.
In `@skills/lark-drive/references/lark-drive-files-list.md`:
- Around line 44-72: The sort examples in the references are misleading because
they look like complete ordered listings, but `lark-cli drive files list` only
returns one page unless `page_size` and `page_token` are handled. Update the
examples under the time-sorting section to either explicitly say they are
first-page-only or add the pagination flow used by `folder_token`/`page_token`
guidance, and keep the sort examples consistent with the `order_by`/`direction`
rules in the document.
In `@skills/lark-drive/SKILL.md`:
- Around line 24-25: The inspect gate wording is too broad and still implies
URL-shape parsing can determine canonical type/token/title; tighten it in
SKILL.md so lightweight URL checks are only used to decide whether to invoke
lark-cli drive +inspect, not to infer the real resource identity. Make it
explicit that canonical type, real token, and title resolution must come from
drive +inspect for wiki URLs or when disambiguation is needed, and keep the rule
that inspect is only for identification/disambiguation and not a universal
prerequisite.
🪄 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: 5930543c-bfcf-451d-98a2-f47417336c15
📒 Files selected for processing (5)
skills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-delete.mdskills/lark-drive/references/lark-drive-files-list.mdskills/lark-drive/references/lark-drive-inspect.mdskills/lark-drive/references/lark-drive-permission-guide.md
3ae6fa7 to
8692fa0
Compare
8692fa0 to
f51db4a
Compare
f51db4a to
ed1d19c
Compare
ed1d19c to
c6d1007
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
skills/lark-drive/references/lark-drive-delete.md (1)
10-21: 🗄️ Data Integrity & Integration | 🟠 MajorUse the resolved delete target, not the raw URL/token.
drive +deleteshould gate on the resolvedfile-token/typepair; treating a raw URL/token as a concrete delete target keeps the inspect artifact in the execution path. Please rewrite the prerequisite table and the “目标明确” definition accordingly. Based on the PR objective that deletion requires a concrete target plus explicit confirmation in the current turn.🤖 Prompt for 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. In `@skills/lark-drive/references/lark-drive-delete.md` around lines 10 - 21, The delete prerequisite wording in lark-drive-delete.md still treats a raw URL/token as a concrete target; update the “目标明确” definition and the prerequisite table so `drive +delete --yes` is only allowed when the resolved `file-token/type` pair is available and the user has explicitly confirmed that exact target in the current turn. Keep the guidance around `drive +search` and `drive +inspect` for unresolved candidates, and make the distinction between candidate selection and executable delete target unambiguous.skills/lark-drive/SKILL.md (1)
24-25: 🎯 Functional Correctness | 🟠 MajorKeep URL-shape checks out of canonical identity resolution.
轻量判断still reads as if the URL itself can determine the real type/token/title beforedrive +inspect. Please limit the URL check to deciding whetherdrive +inspectis needed; the canonical identity should come fromdrive +inspectonly. Based on the PR objective that URL type, real token, and title detection belong todrive +inspect.🤖 Prompt for 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. In `@skills/lark-drive/SKILL.md` around lines 24 - 25, Adjust the guidance in SKILL.md so the initial URL check in the drive workflow is only a gate for whether lark-cli drive +inspect --url '<url>' should be used, not part of canonical identity resolution. Update the wording around the URL-shape check and drive +inspect so that real type, token, and title detection are attributed solely to +inspect, and keep the existing symbols drive +inspect, inspect, and the high-risk write-operation rules aligned with that separation.
🤖 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.
Duplicate comments:
In `@skills/lark-drive/references/lark-drive-delete.md`:
- Around line 10-21: The delete prerequisite wording in lark-drive-delete.md
still treats a raw URL/token as a concrete target; update the “目标明确” definition
and the prerequisite table so `drive +delete --yes` is only allowed when the
resolved `file-token/type` pair is available and the user has explicitly
confirmed that exact target in the current turn. Keep the guidance around `drive
+search` and `drive +inspect` for unresolved candidates, and make the
distinction between candidate selection and executable delete target
unambiguous.
In `@skills/lark-drive/SKILL.md`:
- Around line 24-25: Adjust the guidance in SKILL.md so the initial URL check in
the drive workflow is only a gate for whether lark-cli drive +inspect --url
'<url>' should be used, not part of canonical identity resolution. Update the
wording around the URL-shape check and drive +inspect so that real type, token,
and title detection are attributed solely to +inspect, and keep the existing
symbols drive +inspect, inspect, and the high-risk write-operation rules aligned
with that separation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63c186a7-8083-4f34-a9d0-fa89416d77aa
📒 Files selected for processing (5)
skills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-delete.mdskills/lark-drive/references/lark-drive-files-list.mdskills/lark-drive/references/lark-drive-inspect.mdskills/lark-drive/references/lark-drive-permission-guide.md
✅ Files skipped from review due to trivial changes (3)
- skills/lark-drive/references/lark-drive-inspect.md
- skills/lark-drive/references/lark-drive-permission-guide.md
- skills/lark-drive/references/lark-drive-files-list.md
c6d1007 to
24365cb
Compare
|
One broader consistency concern: the new guardrails are valuable, but the quick-decision rules and the detailed references currently give agents different signals in a couple of places.
Because agents usually read the quick-decision section first, these broader rules can accidentally loosen or contradict the stricter reference-level rules. I think the safest model is to make the quick-decision section act only as routing guidance, and leave canonical identity resolution plus executable write prerequisites to the detailed references. |
24365cb to
9b0a8ec
Compare
…d-only recognition boundaries.
9b0a8ec to
6e4dfba
Compare
Summary
Strengthen the lark-drive skill’s guardrails for high-risk write operations, including deletion, public permission changes, owner transfer, version deletion/rollback, and bulk move/overwrite/
sync. Distinguish “target state” from “specific target plus execution confirmation.” Also define URL type/token recognition as a read-only responsibility of drive +inspect, and add common usage
guidance for files list sorting, file.statistics, and file.view_records typed flags to reduce Drive command misuse and unauthorized execution.
Changes
skills/lark-drive/SKILL.md
Expanded the description to include scenarios for identifying link type, real token, and title.
Added two quick-decision rules:
ones,” “open it to everyone,” or “overwrite/move these” as execution confirmation.
Changed the files list section from “must” to “read the reference before use and pass parameters according to the template,” avoiding overly rigid binding.
Clarified that file.statistics get and file.view_records list should use typed flags such as --file-token, --file-type, --page-size, and --viewer-id-type, instead of rewriting them as
--params JSON.
skills/lark-drive/references/lark-drive-delete.md
Clarified the meaning of “specific target”: a concrete URL/token, or a resource list confirmed item-by-item or in batch by the user. Candidates found through heuristic search are pending-
confirmation targets.
Added a “pre-deletion gate” table requiring both a specific target and execution confirmation. If either is missing, list candidates and stop to wait for confirmation.
skills/lark-drive/references/lark-drive-files-list.md
skills/lark-drive/references/lark-drive-inspect.md
Expanded the scenario table: use drive +inspect when asking for resource type, canonical token, title, canonical URL, or when the path looks like docx / Base / Sheet / Slides / Drive folder
but the task is only identification.
Clarified that identifying link type/token/title is a Drive inspect task. Do not replace it with body reading, Base table commands, or manual URL path extraction.
skills/lark-drive/references/lark-drive-permission-guide.md
Added a “pre-public-permission-change gate” table: --yes is allowed only when all three are present: specific target, user-selected link_share_entity level, and current-turn execution
confirmation.
Clarified that phrases like “open it up,” “share it with everyone,” or “let everyone view it” only express the target state, not the specific public scope. List available levels first, then
stop and wait for the user’s choice.
Test Plan
reference files applied cleanly.
and drive +inspect, confirming that agents reliably hit the new gates and typed-flag guidance.
Related Issues
Auto research task: 01KWW7ZHEWMRH9JSBMDAMSNJZ4
Summary by CodeRabbit
Summary
drive +inspectusage with typical scenarios and reinforced “don’t infer from page content/Base/manual path truncation” rules.drive files listsorting/pagination and tightened--params/typed--format jsonexamples for related native commands.