feat: add markdown shortcuts and skill docs#704
Merged
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:
📝 WalkthroughWalkthroughAdds a new Markdown domain with CLI shortcuts to create, fetch, and overwrite Drive-native Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (lark-cli)" bgcolor rgba(200,200,255,0.5)
participant FS as "Local FS / stdin" bgcolor rgba(200,255,200,0.5)
participant Drive as "Drive API" bgcolor rgba(255,200,200,0.5)
Note over CLI,Drive: Create flow (choose single vs multipart)
CLI->>FS: read bytes (from --content or --file)
CLI->>CLI: decide single-part vs multipart (size)
alt single-part
CLI->>Drive: POST /files/upload_all (form-data)
Drive-->>CLI: {file_token, version/data_version}
else multipart
CLI->>Drive: POST /files/upload_prepare
Drive-->>CLI: {upload_id, part_size, parts}
loop upload parts
CLI->>Drive: POST /files/upload_part
Drive-->>CLI: 200 OK
end
CLI->>Drive: POST /files/upload_finish
Drive-->>CLI: {file_token, version/data_version}
end
Note over CLI,Drive: Fetch flow
CLI->>Drive: GET /files/{file_token}/download
Drive-->>CLI: stream (Content-Disposition, bytes)
CLI->>FS: write to output path or stdout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
shortcuts/register_markdown_test.go (1)
13-24: Broaden the markdown registration regression test.This only checks
+create, so a partial registration bug in+fetchor+overwritewould still slip through. Please assert the full markdown shortcut bundle here.Suggested coverage expansion
func TestRegisterShortcutsMountsMarkdownCommands(t *testing.T) { program := &cobra.Command{Use: "root"} RegisterShortcuts(program, &cmdutil.Factory{}) - markdownCmd, _, err := program.Find([]string{"markdown", "+create"}) - if err != nil { - t.Fatalf("find markdown create shortcut: %v", err) - } - if markdownCmd == nil || markdownCmd.Name() != "+create" { - t.Fatalf("markdown create shortcut not mounted: %#v", markdownCmd) - } + for _, path := range [][]string{ + {"markdown", "+create"}, + {"markdown", "+fetch"}, + {"markdown", "+overwrite"}, + } { + cmd, _, err := program.Find(path) + if err != nil { + t.Fatalf("find markdown shortcut %v: %v", path, err) + } + if cmd == nil || cmd.Name() != path[1] { + t.Fatalf("markdown shortcut not mounted: %#v", cmd) + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/register_markdown_test.go` around lines 13 - 24, The test TestRegisterShortcutsMountsMarkdownCommands currently only asserts the "+create" shortcut; expand it to assert the full markdown shortcut bundle by using RegisterShortcuts(program, &cmdutil.Factory{}) and then locating and validating each expected shortcut (e.g., find ["markdown", "+create"], ["markdown", "+fetch"], and ["markdown", "+overwrite"]) with program.Find; for each, assert err is nil, the returned command is non-nil and command.Name() matches the expected shortcut string, and optionally assert the parent "markdown" command exists to ensure the bundle is fully mounted.
🤖 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/markdown/helpers.go`:
- Around line 370-375: The prettyPrintMarkdownWrite function only checks the
"version" key and omits the fallback "data_version", so output can miss the
version; update prettyPrintMarkdownWrite to read version using common.GetString
for "version" and, if empty, fall back to common.GetString for "data_version",
then print the resulting non-empty value (e.g., use the existing fmt.Fprintf
call when the resolved version is not empty); reference the function
prettyPrintMarkdownWrite and the keys "version" and "data_version" when applying
the change.
- Around line 57-64: The code treats spec.Content != "" as the presence check so
an explicit empty --content is misclassified as file mode; add an explicit
presence flag (e.g., spec.ContentSet bool or change spec.Content to *string and
treat non-nil as set) when parsing flags, then replace all checks that currently
use spec.Content != "" (referenced at the mode-detection sites and validation
around requireName, and functions that call validateMarkdownFileName) with the
presence check (spec.ContentSet or spec.Content != nil) and use spec.Content
(possibly empty) only when reading content; ensure the flag parsing code sets
the new presence flag so --content "" is handled as “content mode.”
In `@shortcuts/markdown/markdown_fetch.go`:
- Around line 39-48: The current pre-check using validate.SafeOutputPath +
runtime.FileIO().Stat is racy; instead enforce no-overwrite atomically when
opening/writing the file. Remove or relax the Stat-based existence check and
change the write path that creates the file (the runtime.FileIO() open/save call
used later—e.g., the Save/Open routine that writes outputPath) to use an
exclusive-create flow (or open with O_CREATE|O_EXCL) when
runtime.Bool("overwrite") is false, and only fall back to truncating/replacing
when overwrite is true; keep validate.SafeOutputPath for path validation but
move the existence guard into the file creation call to avoid race conditions.
In `@tests/cli_e2e/markdown/markdown_workflow_test.go`:
- Around line 17-25: The test TestMarkdownLifecycleWorkflow should isolate the
CLI config by creating a temporary config dir (use t.TempDir() or
ioutil.TempDir) and setting the LARKSUITE_CLI_CONFIG_DIR environment variable to
that path before invoking clie2e.SkipWithoutUserToken or running any CLI
operations; save the original env value, call
os.Setenv("LARKSUITE_CLI_CONFIG_DIR", tmpDir), and register a t.Cleanup to
restore the original env (or unset if absent) so the binary cannot read or
mutate the developer’s real CLI state during the create/fetch/overwrite/delete
flows.
---
Nitpick comments:
In `@shortcuts/register_markdown_test.go`:
- Around line 13-24: The test TestRegisterShortcutsMountsMarkdownCommands
currently only asserts the "+create" shortcut; expand it to assert the full
markdown shortcut bundle by using RegisterShortcuts(program, &cmdutil.Factory{})
and then locating and validating each expected shortcut (e.g., find ["markdown",
"+create"], ["markdown", "+fetch"], and ["markdown", "+overwrite"]) with
program.Find; for each, assert err is nil, the returned command is non-nil and
command.Name() matches the expected shortcut string, and optionally assert the
parent "markdown" command exists to ensure the bundle is fully mounted.
🪄 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: 855b674d-b595-40e8-aa41-f08a6cff7157
📒 Files selected for processing (17)
README.mdREADME.zh.mdshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
a318ea6 to
f883501
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cli_e2e/markdown/markdown_dryrun_test.go (1)
96-120: Consider using a temp file instead of relying on repository root structure.The test navigates to the repository root via
WorkDir: "../../.."to readREADME.md. This creates a fragile dependency on the test file location and the existence ofREADME.mdat the repo root.A more robust approach (as demonstrated in
TestMarkdownCreateDryRun_FileShowsConcreteSize) is to create a temp file with known content:♻️ Suggested refactor
func TestMarkdownOverwriteDryRun_ContentFile(t *testing.T) { setMarkdownDryRunConfigEnv(t) + dir := t.TempDir() + content := "# overwrite test\n" + require.NoError(t, os.WriteFile(dir+"/input.md", []byte(content), 0o644)) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) t.Cleanup(cancel) result, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{ "markdown", "+overwrite", "--file-token", "boxcnMarkdownDryRun", - "--content", "@README.md", + "--content", "@input.md", "--dry-run", }, DefaultAs: "bot", - WorkDir: "../../..", + WorkDir: dir, }) require.NoError(t, err) result.AssertExitCode(t, 0) output := strings.TrimSpace(result.Stdout) assert.Contains(t, output, "/open-apis/drive/v1/metas/batch_query") assert.Contains(t, output, "/open-apis/drive/v1/files/upload_all") assert.Contains(t, output, `"file_token": "boxcnMarkdownDryRun"`) - assert.Contains(t, output, `"size":`) + assert.Contains(t, output, `"size": 18`) }
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a051a06-7611-423d-b4be-8f7514ac92d4
📒 Files selected for processing (17)
README.mdREADME.zh.mdshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
✅ Files skipped from review due to trivial changes (4)
- skills/lark-markdown/references/lark-markdown-fetch.md
- skills/lark-markdown/references/lark-markdown-create.md
- skills/lark-markdown/references/lark-markdown-overwrite.md
- skills/lark-markdown/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/register.go
- shortcuts/markdown/shortcuts.go
- README.md
- skills/lark-drive/SKILL.md
- tests/cli_e2e/markdown/markdown_workflow_test.go
- shortcuts/markdown/markdown_overwrite.go
- shortcuts/markdown/markdown_fetch.go
- shortcuts/markdown/helpers.go
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
f883501 to
4958e0e
Compare
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/markdown/helpers.go`:
- Around line 359-403: The multipart upload can silently truncate because
uploadMarkdownMultipartParts uses session.BlockNum to drive the loop but doesn't
verify that the planned blocks cover the whole payload; add a validation to
ensure the plan matches payload size (e.g., check
int64(session.BlockNum)*int64(session.BlockSize) >= int64(len(payload)) before
the loop) or, if you prefer a post-check, after the for loop verify remaining ==
0 and return output.ErrValidation describing inconsistent block_num if not;
update uploadMarkdownMultipartParts to return an error when
session.BlockNum/BlockSize are too small instead of succeeding silently.
🪄 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: db496486-7d2a-494d-9d84-87bf904df428
📒 Files selected for processing (19)
README.mdREADME.zh.mdinternal/core/types.goshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
✅ Files skipped from review due to trivial changes (8)
- skills/lark-drive/references/lark-drive-upload.md
- skills/lark-drive/SKILL.md
- shortcuts/markdown/shortcuts.go
- skills/lark-markdown/SKILL.md
- skills/lark-markdown/references/lark-markdown-create.md
- skills/lark-markdown/references/lark-markdown-fetch.md
- skills/lark-markdown/references/lark-markdown-overwrite.md
- shortcuts/markdown/markdown_create.go
🚧 Files skipped from review as they are similar to previous changes (6)
- shortcuts/register_markdown_test.go
- README.md
- shortcuts/markdown/markdown_fetch.go
- tests/cli_e2e/markdown/markdown_workflow_test.go
- shortcuts/markdown/markdown_overwrite.go
- tests/cli_e2e/markdown/markdown_dryrun_test.go
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
4958e0e to
fdba0b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
shortcuts/markdown/markdown_fetch.go (1)
92-105:⚠️ Potential issue | 🟠 MajorMake
--overwrite=falseenforcement atomic.Between Line 95 and Line 99, another process can create
outputPathafter theStatcheck but before the write starts, so a fetch that was supposed to refuse overwrites can still clobber data. Please move the no-overwrite guarantee into the create/write path instead of enforcing it with a pre-check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/markdown/markdown_fetch.go` around lines 92 - 105, The current pre-check using runtime.FileIO().Stat allows a race where another process creates outputPath after the check; remove that pre-check and enforce no-overwrite atomically by attempting the write/create with exclusive semantics and treating "file exists" errors as the validation error. Concretely, stop relying on runtime.FileIO().Stat and instead call runtime.FileIO().Save (or open/create the target file via the FileIO implementation) using exclusive create flags (O_CREATE|O_EXCL) or equivalent, and if the save/open fails with an EEXIST-alike error return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath); keep existing error wrapping via common.WrapSaveErrorByCategory for other IO errors and preserve use of markdownFetchOutputIsDirectory and resp.Header resp.Body handling.
🤖 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/markdown/helpers.go`:
- Around line 359-409: In uploadMarkdownMultipartParts validate the multipart
plan before reading/uploading: compute the expected block count from payload
length and session.BlockSize (e.g., expectedBlocks =
ceil(len(payload)/session.BlockSize)) and compare it to session.BlockNum; if
they differ return an API/validation error immediately (with a clear message
including session.BlockNum and expectedBlocks) so you never start the upload
loop and thus avoid ReadFull reading past EOF or partial uploads; reference
symbols: uploadMarkdownMultipartParts, session.BlockNum, session.BlockSize,
payload, remaining.
In `@shortcuts/markdown/markdown_overwrite.go`:
- Around line 71-85: When computing spec.FileName in the overwrite flow (the
block that calls fetchMarkdownFileName and assigns spec.FileName), re-validate
the fetched remote name before accepting it: call
validateMarkdownFileName(remoteName) (or the existing filename validation
helper) on the trimmed remoteName returned by fetchMarkdownFileName(runtime,
fileToken) and only use it if it passes; if validation fails, either fall back
to fileToken + ".md" (and assign that to spec.FileName) or return an error per
the shortcut’s existing semantics. Update the logic around
fetchMarkdownFileName, fileName assignment, and spec.FileName so no unvalidated
metadata.title can bypass validateMarkdownFileName.
In `@shortcuts/markdown/markdown_test.go`:
- Around line 552-665: Tests for markdown fetch only cover fresh destinations;
add new unit tests to exercise the "--overwrite" and "output file already
exists" failure paths so regressions are caught. Specifically, extend or add
tests similar to TestMarkdownFetchSavesFile (and the directory variants) that:
1) create an existing target file with different contents, run
mountAndRunMarkdown(MarkdownFetch, ... "--output", "copy.md") without
"--overwrite" and assert the command fails with the expected "output file
already exists" error/state; and 2) repeat with the same existing file but pass
"--overwrite" and assert the file is replaced with the downloaded content; use
the same httpmock stub (box_md_fetch) and inspect stdout/envelope or error
return values as the other tests do.
---
Duplicate comments:
In `@shortcuts/markdown/markdown_fetch.go`:
- Around line 92-105: The current pre-check using runtime.FileIO().Stat allows a
race where another process creates outputPath after the check; remove that
pre-check and enforce no-overwrite atomically by attempting the write/create
with exclusive semantics and treating "file exists" errors as the validation
error. Concretely, stop relying on runtime.FileIO().Stat and instead call
runtime.FileIO().Save (or open/create the target file via the FileIO
implementation) using exclusive create flags (O_CREATE|O_EXCL) or equivalent,
and if the save/open fails with an EEXIST-alike error return
output.ErrValidation("output file already exists: %s (use --overwrite to
replace)", outputPath); keep existing error wrapping via
common.WrapSaveErrorByCategory for other IO errors and preserve use of
markdownFetchOutputIsDirectory and resp.Header resp.Body handling.
🪄 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: 83fdd1db-d4ef-4db1-baac-84241dc1b99e
📒 Files selected for processing (19)
README.mdREADME.zh.mdinternal/core/types.goshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
✅ Files skipped from review due to trivial changes (7)
- skills/lark-drive/references/lark-drive-upload.md
- shortcuts/register.go
- skills/lark-markdown/references/lark-markdown-fetch.md
- skills/lark-drive/SKILL.md
- skills/lark-markdown/references/lark-markdown-create.md
- skills/lark-markdown/references/lark-markdown-overwrite.md
- skills/lark-markdown/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/markdown/shortcuts.go
- shortcuts/register_markdown_test.go
- internal/core/types.go
- shortcuts/markdown/markdown_create.go
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
fdba0b9 to
9f7a374
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/markdown/markdown_fetch.go (1)
95-103:⚠️ Potential issue | 🟠 MajorMake no-overwrite enforcement atomic with the write path.
At Line 95-Line 103, the
Statcheck is racy. Another process can create/replace the target between check andSave, so--overwrite=falseis not reliably enforced.Please move the no-overwrite guarantee into an atomic create/write path (e.g., exclusive-create semantics in FileIO/Save layer) instead of pre-checking existence only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/markdown/markdown_fetch.go` around lines 95 - 103, The current existence check using runtime.FileIO().Stat + runtime.Bool("overwrite") is racy; move the no-overwrite enforcement into the atomic write path by changing the Save call to support exclusive-create semantics (or use an atomic create/write operation in the FileIO implementation) instead of pre-checking. Update the call site that uses runtime.FileIO().Save(outputPath, fileio.SaveOptions{...}, resp.Body) to pass an option (e.g., SaveOptions.ExclusiveCreate or a flag derived from runtime.Bool("overwrite")) and implement exclusive/open-with-O_EXCL behavior inside the FileIO.Save implementation (or perform write-to-temp-then-rename with an atomic replace when overwrite is true), ensuring Save returns a clear error when the destination exists and overwrite is false.shortcuts/markdown/markdown_overwrite.go (1)
75-85:⚠️ Potential issue | 🟠 MajorRe-validate remote title before using it as overwrite
file_name.At Line 76-Line 81,
fetchMarkdownFileNameoutput is trusted verbatim. A non-.mdremote title can bypass this shortcut’s Markdown filename constraint when--nameis omitted.Suggested fix
if fileName == "" { remoteName, err := fetchMarkdownFileName(runtime, fileToken) if err != nil { return err } - fileName = strings.TrimSpace(remoteName) + remoteName = strings.TrimSpace(remoteName) + if remoteName != "" { + if err := validateMarkdownFileName(remoteName, "--name"); err == nil { + fileName = remoteName + } + } } if fileName == "" { fileName = fileToken + ".md" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/markdown/markdown_overwrite.go` around lines 75 - 85, The code trusts fetchMarkdownFileName(runtime, fileToken) output as a valid Markdown filename; re-validate remoteName after TrimSpace by ensuring it ends with ".md" (case-insensitive) and, if not, append ".md" before assigning to fileName/spec.FileName; keep the existing fallback to fileToken+".md" when remoteName is empty and preserve the existing trimming behavior—update the logic around fetchMarkdownFileName, fileName, and spec.FileName accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/markdown/markdown_fetch.go`:
- Around line 95-103: The current existence check using runtime.FileIO().Stat +
runtime.Bool("overwrite") is racy; move the no-overwrite enforcement into the
atomic write path by changing the Save call to support exclusive-create
semantics (or use an atomic create/write operation in the FileIO implementation)
instead of pre-checking. Update the call site that uses
runtime.FileIO().Save(outputPath, fileio.SaveOptions{...}, resp.Body) to pass an
option (e.g., SaveOptions.ExclusiveCreate or a flag derived from
runtime.Bool("overwrite")) and implement exclusive/open-with-O_EXCL behavior
inside the FileIO.Save implementation (or perform write-to-temp-then-rename with
an atomic replace when overwrite is true), ensuring Save returns a clear error
when the destination exists and overwrite is false.
In `@shortcuts/markdown/markdown_overwrite.go`:
- Around line 75-85: The code trusts fetchMarkdownFileName(runtime, fileToken)
output as a valid Markdown filename; re-validate remoteName after TrimSpace by
ensuring it ends with ".md" (case-insensitive) and, if not, append ".md" before
assigning to fileName/spec.FileName; keep the existing fallback to
fileToken+".md" when remoteName is empty and preserve the existing trimming
behavior—update the logic around fetchMarkdownFileName, fileName, and
spec.FileName accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39032090-c558-4022-9998-d4d15acd26d8
📒 Files selected for processing (19)
README.mdREADME.zh.mdinternal/core/types.goshortcuts/markdown/helpers.goshortcuts/markdown/markdown_create.goshortcuts/markdown/markdown_fetch.goshortcuts/markdown/markdown_overwrite.goshortcuts/markdown/markdown_test.goshortcuts/markdown/shortcuts.goshortcuts/register.goshortcuts/register_markdown_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-upload.mdskills/lark-markdown/SKILL.mdskills/lark-markdown/references/lark-markdown-create.mdskills/lark-markdown/references/lark-markdown-fetch.mdskills/lark-markdown/references/lark-markdown-overwrite.mdtests/cli_e2e/markdown/markdown_dryrun_test.gotests/cli_e2e/markdown/markdown_workflow_test.go
✅ Files skipped from review due to trivial changes (6)
- shortcuts/markdown/shortcuts.go
- internal/core/types.go
- skills/lark-markdown/references/lark-markdown-create.md
- shortcuts/register.go
- skills/lark-markdown/references/lark-markdown-overwrite.md
- skills/lark-drive/references/lark-drive-upload.md
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/register_markdown_test.go
- README.md
- skills/lark-markdown/references/lark-markdown-fetch.md
- skills/lark-markdown/SKILL.md
- skills/lark-drive/SKILL.md
- shortcuts/markdown/markdown_create.go
- shortcuts/markdown/markdown_test.go
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
9f7a374 to
14be4ab
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@059d7495ea70f2dfcd0cf51c68fd0af4dce83b23🧩 Skill updatenpx skills add larksuite/cli#feat/markdown-shortcuts -y -g |
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
14be4ab to
4d189f9
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #704 +/- ##
==========================================
+ Coverage 64.14% 64.34% +0.20%
==========================================
Files 504 511 +7
Lines 44285 45035 +750
==========================================
+ Hits 28406 28980 +574
- Misses 13411 13527 +116
- Partials 2468 2528 +60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
2 times, most recently
from
fbd7957 to
4cc079b
Compare
Collaborator
fangshuyu-768
left a comment
fangshuyu-768
left a comment
There was a problem hiding this comment.
One more functional gap I found outside the touched files: lark-cli auth login now accepts --domain markdown in the flag path, but the interactive selector still won't show markdown. getShortcutOnlyDomainNames() is still hard-coded to base/contact/docs, and service_descriptions.json also has no markdown entry yet, so this new shortcut-only domain is missing from the TUI authorization flow and has no localized title/description metadata. Please wire those two places up as part of introducing the new domain.
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
3 times, most recently
from
473e8e1 to
a143275
Compare
github-actions
Bot
added
size/XL
Change-Id: Iced88525deb10b014b755ec68bd9a8ae6a935143
wittam-01
force-pushed
the
feat/markdown-shortcuts
branch
from
a143275 to
059d749
Compare
github-actions
Bot
added
size/L
fangshuyu-768
approved these changes
Apr 30, 2026
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
Add a new markdown shortcut domain for Drive-native .md files, covering create, fetch, and overwrite flows with structured outputs for AI agents. This PR also adds the corresponding lark-markdown skill docs and updates Drive routing guidance so raw Markdown file operations are clearly separated from drive +import --type docx.
Changes
Test Plan
Related Issues
Summary by CodeRabbit
New Features
Documentation
Tests