Open
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
github-actions
bot
added
domain/base
|
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 two new Base shortcuts to generate record share links (single and batch up to 100), implements validation/dry-run/execute handlers, adds a string-slice flag type to runtime, updates the shortcuts catalog and test, and includes user-facing docs for both commands. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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 the current code and only fix it if needed.
Inline comments:
In `@shortcuts/base/record_ops.go`:
- Around line 132-154: validateRecordShareBatch currently checks the raw
runtime.StrSlice length which can include empty entries (e.g. "--record-ids ,")
that are later removed by deduplicateRecordIDs; change validateRecordShareBatch
to use the normalized, deduplicated slice (call deduplicateRecordIDs or extract
the normalization logic) and validate that the resulting slice is non-empty and
does not exceed maxShareBatchSize, returning the same FlagErrorf messages but
using the normalized length; keep deduplicateRecordIDs as the normalizer that
trims empty strings and deduplicates entries so both validation and payload
construction use the same canonical list.
🪄 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: a87ab232-1201-49ce-95c2-0d7e8b727b2d
📒 Files selected for processing (6)
shortcuts/base/base_shortcuts_test.goshortcuts/base/record_ops.goshortcuts/base/record_share_batch_create.goshortcuts/base/record_share_create.goshortcuts/base/shortcuts.goshortcuts/common/runner.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3514ee5f97b48adc0d9ad815fcaef847a6867ee2🧩 Skill updatenpx skills add larksuite/cli#feat/record-share-cli -y -g |
yballul-bytedance
force-pushed
the
feat/record-share-cli
branch
2 times, most recently
from
f47e987 to
e221e6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
shortcuts/base/record_ops.go (1)
132-154:⚠️ Potential issue | 🟠 MajorValidate the canonical
record-idslist, not the raw slice.
validateRecordShareBatchchecksruntime.StrSlice("record-ids"), butexecuteRecordShareBatchlater drops empty entries and deduplicates them. Inputs like--record-ids ,can still pass validation and then send an emptyrecord_idspayload, and CSV input likerec1, rec2will preserve the leading space on the second ID. Reuse the normalized list for validation and trim each element insidededuplicateRecordIDs.Proposed fix
func validateRecordShareBatch(runtime *common.RuntimeContext) error { - recordIDs := runtime.StrSlice("record-ids") + recordIDs := deduplicateRecordIDs(runtime) if len(recordIDs) == 0 { return common.FlagErrorf("--record-ids is required and must not be empty") } if len(recordIDs) > maxShareBatchSize { return common.FlagErrorf("--record-ids exceeds maximum limit of %d (got %d)", maxShareBatchSize, len(recordIDs)) @@ func deduplicateRecordIDs(runtime *common.RuntimeContext) []string { raw := runtime.StrSlice("record-ids") seen := make(map[string]bool, len(raw)) result := make([]string, 0, len(raw)) for _, id := range raw { - if id != "" && !seen[id] { + id = strings.TrimSpace(id) + if id != "" && !seen[id] { seen[id] = true result = append(result, id) } } return result }Also add
stringsto the import block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/record_ops.go` around lines 132 - 154, validateRecordShareBatch currently validates the raw runtime.StrSlice("record-ids") instead of the normalized list; change validateRecordShareBatch to call deduplicateRecordIDs (or otherwise normalize: trim each entry and drop empties) and validate the resulting canonical slice (check length >0 and <= maxShareBatchSize). Update deduplicateRecordIDs to trim spaces from each id (using strings.TrimSpace), ignore empty strings after trimming, and deduplicate as before. Add "strings" to the import block so TrimSpace is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lark-env.sh`:
- Around line 235-248: The trap for cleaning up tmp_js is currently installed
unconditionally (trap 'rm -f "$tmp_js"' EXIT HUP INT TERM) before checking
is_sourced, which alters the parent shell when this script is sourced; move the
trap installation so it is only set after the is_sourced check fails (i.e.,
inside the non-sourced execution path that proceeds to run "$wcmd" add ...), and
ensure any early-return path that applies when sourced does not rely on the trap
(clean tmp_js explicitly or avoid creating the temp file when sourced);
reference tmp_js, write_rule_js, trap, is_sourced and the block that runs
"$wcmd" add to locate where to adjust placement and cleanup logic.
- Around line 34-36: The fail() helper currently returns but callers don't
propagate its non-zero status; update the call sites (the case branches that
call fail, the callers around build_rules() and other helpers) to explicitly
propagate failures by checking return values (e.g., call helpers as "if !
build_rules ...; then return 1; fi" or append "|| return 1" after helper calls)
and avoid swallowing errors in command substitution—do not assign
rules="$(build_rules ...)" directly without first verifying success; also ensure
the top-level flow (main or caller) checks helper return codes or enable
script-wide errexit semantics so a helper failure aborts the sourced script.
In `@shortcuts/base/record_ops.go`:
- Around line 115-120: The dry-run payload in dryRunRecordShareBatch is using
the wrong key ("records") so it does not match the real request which sends
{"record_ids": ...}; update dryRunRecordShareBatch to build the body with the
"record_ids" key (using deduplicateRecordIDs(runtime)) and keep the rest of the
request unchanged so the output of the dry-run matches the actual POST to
/open-apis/base/v3/bases/:base_token/tables/:table_id/records/share_links/batch.
In `@skills/lark-base/references/lark-base-record-share-link-create.md`:
- Around line 28-30: The fenced code block containing the HTTP endpoint "POST
/open-apis/base/v3/bases/:base_token/tables/:table_id/records/:record_id/share_links"
is missing a language tag and triggers MD040; update that fenced block to
include the language identifier (e.g., add "http" after the opening backticks)
so the block becomes a language-specific code fence and satisfies markdownlint.
---
Duplicate comments:
In `@shortcuts/base/record_ops.go`:
- Around line 132-154: validateRecordShareBatch currently validates the raw
runtime.StrSlice("record-ids") instead of the normalized list; change
validateRecordShareBatch to call deduplicateRecordIDs (or otherwise normalize:
trim each entry and drop empties) and validate the resulting canonical slice
(check length >0 and <= maxShareBatchSize). Update deduplicateRecordIDs to trim
spaces from each id (using strings.TrimSpace), ignore empty strings after
trimming, and deduplicate as before. Add "strings" to the import block so
TrimSpace is available.
🪄 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: 96ab410a-bfe3-4b65-ae44-9f0392f54791
📒 Files selected for processing (10)
lark-env.shshortcuts/base/base_shortcuts_test.goshortcuts/base/record_ops.goshortcuts/base/record_share_link_batch_create.goshortcuts/base/record_share_link_create.goshortcuts/base/shortcuts.goshortcuts/common/runner.goskills/lark-base/references/lark-base-record-share-link-batch-create.mdskills/lark-base/references/lark-base-record-share-link-create.mdskills/lark-base/references/lark-base-record.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/base/shortcuts.go
- skills/lark-base/references/lark-base-record.md
- skills/lark-base/references/lark-base-record-share-link-batch-create.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/base/base_shortcuts_test.go
- shortcuts/base/record_share_link_create.go
skills/lark-base/references/lark-base-record-share-link-create.md
Outdated
Show resolved
Hide resolved
yballul-bytedance
force-pushed
the
feat/record-share-cli
branch
2 times, most recently
from
27d5c28 to
cd3b713
Compare
Change-Id: Ie78da99096cc1fc8a4671d8178176f4c587466ba
yballul-bytedance
force-pushed
the
feat/record-share-cli
branch
from
cd3b713 to
3514ee5
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.
Change-Id: Ie78da99096cc1fc8a4671d8178176f4c587466ba
Summary
Add +record-share-link-create and +record-share-link-batch-create base shortcuts for generating share links for single or batch records.
Changes
• Endpoint: POST /bases/:base_token/tables/:table_id/records/:record_id/share_links
• Flags: --base-token, --table-id, --record-id
• Returns the share link for the given record
• Endpoint: POST /bases/:base_token/tables/:table_id/records/share_links/batch
• Flags: --base-token, --table-id, --record-ids (string_slice type, comma-separated, max 100)
• Auto-deduplication + validation (empty input / over 100 records are rejected)
• Returns record_share_links map (key: record_id, value: share link)
• Leverages pflag's StringSlice, supporting both comma-separated and repeatable flag styles
• lark-base-record-share-link-create.md
• lark-base-record-share-link-batch-create.md (with request/response JSON examples)
• Update lark-base-record.md index page
Test Plan
• Unit tests pass (shortcuts/base, shortcuts/common)
• Manual local verification confirms the lark base +record-share-link-* commands work as expected
Related Issues
None
Summary by CodeRabbit
Release Notes
New Features
Documentation
Summary by CodeRabbit
New Features
Documentation