feat: add drive import, export, move, task result shortcuts#193
feat: add drive import, export, move, task result shortcuts#193liujinkun2025 merged 8 commits intofeat/drive_devfrom
Conversation
* ci: make pkg.pr.new comment flow fork-safe * ci: harden trusted comment workflow inputs * ci: skip comment steps when payload artifact is missing * ci: use artifact PR number when workflow_run pull_requests is empty * ci: allow PR comment workflow to write pull requests --------- Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
* fix: Fix the issue where the URL returned by the "lark-cli auth login --no-wait" command contains \u0026 * style: fix indentation and whitespace in error handling code * fix(auth): handle JSON encoding errors in login output * docs(cmd/auth): add comment for authLoginRun function
* add pull request template * fix: use safe related issue placeholder in PR template --------- Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
…139) * feat(mail): auto-resolve local image paths in draft body HTML (#81) Allow <img src="./local/path.png" /> in set_body/set_reply_body HTML. Local file paths are automatically resolved into inline MIME parts with generated CIDs, eliminating the need to manually pair add_inline with set_body. Removing or replacing an <img> tag in the body automatically cleans up or replaces the corresponding MIME inline part. - Add postProcessInlineImages to unify resolve, validate, and orphan cleanup into a single post-processing step - Extract loadAndAttachInline shared helper to deduplicate addInline and resolveLocalImgSrc logic - Cache resolved paths so the same file is only attached once - Use whitelist URI scheme detection instead of blacklist - Remove dead validateInlineCIDAfterApply and validateOrphanedInlineCIDAfterApply functions Closes #81 * fix(mail): harden inline image CID handling 1. Fix imgSrcRegexp to skip attribute names like data-src/x-src that contain "src" as a suffix — only match the real src attribute. 2. Sanitize cidFromFileName to replace whitespace with hyphens, producing RFC-safe CID tokens (e.g. "my logo.png" → "my-logo"). 3. Add CID validation in newInlinePart to reject spaces, tabs, angle brackets, and parentheses — fail fast instead of silently producing broken inline images in the sent email. * refactor(mail): use UUID for auto-generated inline CIDs Replace filename-derived CID generation (cidFromFileName + uniqueCID) with UUID-based generation. UUIDs contain only [0-9a-f-] characters, eliminating all RFC compliance risks from special characters, Unicode, or filename collisions. Same-file deduplication via pathToCID cache is preserved — multiple <img> tags referencing the same file still share one MIME part and one CID. * fix(mail): avoid panic in generateCID by using uuid.NewRandom uuid.New() calls Must(NewRandom()) which panics if the random source fails. Replace with uuid.NewRandom() and propagate the error through resolveLocalImgSrc, so the CLI returns a clear error instead of crashing in extreme environments. * fix(mail): restore quote block hint in set_reply_body template description The auto-resolve PR accidentally dropped "the quote block is re-appended automatically" from the set_reply_body shape description. Restore it alongside the new local-path support note. * fix(mail): add orphan invariant comment and expand regex test coverage - Add comment in postProcessInlineImages explaining that partially attached inline parts on error are cleaned up by the next Apply. - Add regex test cases: single-quoted src, multiple spaces before src, and newline before src. * fix(mail): use consistent inline predicate and safer HTML part lookup 1. removeOrphanedInlineParts: change condition from ContentDisposition=="inline" && ContentID!="" to isInlinePart(child) && ContentID!="", matching the predicate used elsewhere — parts with only a ContentID (no Content-Disposition) are now correctly cleaned up. 2. postProcessInlineImages: use findPrimaryBodyPart instead of findPart(snapshot.Body, PrimaryHTMLPartID) to avoid stale PartID after ops restructure the MIME tree. * fix(mail): revert orphan cleanup to ContentDisposition check to protect HTML body The previous change (d3d1982) broadened the orphan cleanup predicate to isInlinePart(), which treats any part with a ContentID as inline. This deletes the primary HTML body when it carries a Content-ID header (valid in multipart/related), even on metadata-only edits like set_subject. Revert to the original ContentDisposition=="inline" && ContentID!="" condition — only parts explicitly marked as inline attachments are candidates for orphan removal. Add regression test covering multipart/related with a Content-ID-bearing HTML body.
* docs: add concise AGENTS development guide * docs: align AGENTS with toolchain and CI license checks * docs: remove toolchain prerequisite section --------- Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
…heck (#148) * feat(ci): add PR size label pipeline * chore(ci): make PR label sync non-blocking * feat(ci): add dry-run mode for PR label sync * feat(ci): add PR label dry-run samples * test(ci): update PR label samples with real historical merged PRs Replaced synthetic or open PR samples with actual merged/closed PRs from the repository to provide a more accurate reflection of the size label categorization. Added 4 samples each for sizes S, M, and L covering docs, fixes, ci, and features. * feat(ci): add high-level area tags for PRs Based on user feedback, fine-grained domain labels (like `domain/base`) are too detailed for the early stages. This change adds support for applying `area/*` tags to indicate which important top-level modules a PR touches. Currently tracked areas: - `area/shortcuts` - `area/skills` - `area/cmd` Minor modules like docs, ci, and tests are intentionally excluded to keep tags focused on critical architectural components. * refactor(ci): extract pr-label-sync logic to a dedicated directory To avoid polluting the root `scripts/` directory, moved `sync_pr_labels.js` and `sync_pr_labels.samples.json` into a new `scripts/sync-pr-labels/` folder. Added a dedicated README to document its usage and behavior. Updated `.github/workflows/pr-labels.yml` to reflect the new path. * refactor(ci): rename pr label script directory for simplicity Renamed `scripts/sync-pr-labels/` to `scripts/pr-labels/` to keep directory names concise. Updated internal references and GitHub workflow files to point to the new path. * ci: add GitHub Actions workflow to check skill format * test(ci): update sample json to include expected_areas Added `expected_areas` lists to each sample in `samples.json` to reflect the newly added `area/*` high-level module tagging logic. Allows testing to accurately check both `size/*` and `area/*` outputs. * refactor(scripts): move skill format check to isolated directory and add README * test(scripts): add positive and negative tests for skill format check * fix(scripts): revert skill changes and downgrade version/metadata checks to warnings * fix(scripts): completely remove version check and skip lark-shared * refactor(ci): improve pr-labels script readability and maintainability - Reorganized code into logical sections with clear comments - Encapsulated GitHub API interactions into a reusable `GitHubClient` class - Extracted and centralized classification logic into a pure `evaluateRules` function - Replaced magic numbers with named constants (`THRESHOLD_L`, `THRESHOLD_XL`) - Fixed `ROOT` path resolution logic - Simplified conditional statements and control flow * ci: fix setup-node version in pr-labels workflow * tmp * refactor(ci): replace generic area labels with business-specific ones - Add PATH_TO_AREA_MAP to map shortcuts/skills paths to business areas (im, vc, ccm, base, mail, calendar, task, contact) - Replace importantAreas with businessAreas throughout the codebase - Remove area/shortcuts, area/skills, area/cmd generic labels - Now generates specific labels like area/im, area/vc, area/ccm, etc. - Update samples.json expected_areas to match new behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): address PR review feedback for label scripts and workflows - Add `edited` event to PR labels workflow to trigger on title changes - Add security warning comment in pr-labels.yml workflow - Update pr-labels README with latest business area labels - Exclude `skills/lark-*` paths from low risk doc classification - Handle renamed files properly in PR path classification - Fix YAML frontmatter extraction to handle CRLF line endings - Use precise regex for YAML key validation instead of substring match - Fix exit code checking logic in skill-format-check test script - Translate Chinese comments in skill-format-check to English * fix(skill-format-check): address CodeRabbit review feedback - Fix frontmatter closing delimiter detection to strictly match '---' using regex, preventing invalid closing tags like '----' from passing. - Improve test fixture reliability by failing tests immediately if fixture preparation fails, avoiding false positives. * fix: address review comments from PR 148 - ci: warn when PR label sync fails in job summary - test(skill-format-check): capture validator output for negative tests - fix(skill-format-check): catch errors when reading SKILL.md to avoid hard crashes * fix: add error handling for directory enumeration in skill-format-check - refactor: use `fs.readdirSync` with `{ withFileTypes: true }` to avoid extra stat calls - fix: catch and report errors gracefully during skills directory enumeration instead of crashing * docs(skill-format-check): clarify `metadata` requirement in README test(pr-labels): add edge case samples for skills paths, CCM multi-paths, and renames * test(pr-labels): add real PR edge case samples - use PR #134 to test skill path behaviors - use PR #57 to test multi-path CCM resolution - use PR #11 to test track renames cross domains * refactor(ci): migrate pr labels from area to domain prefix - Replaced `area/` prefix with `domain/` for PR labeling to align with existing GitHub labels - Renamed internal constants and variables from `area` to `domain` (e.g. `PATH_TO_AREA_MAP` to `PATH_TO_DOMAIN_MAP`) - Updated `samples.json` test data to use new `domain/` format and `expected_domains` key - Added `scripts/pr-labels/test.js` runner script for continuous validation of labeling logic against PR samples - Corrected expected size label for PR #134 test sample * test: use execFileSync instead of execSync in pr-labels test script * fix: resolve target path against process.cwd() instead of __dirname in skill-format-check * docs: correct label prefix in PR label workflow README - Updated README.md to reflect the new `domain/` label prefix instead of `area/` * fix(ci): fix dry-run console output formatting and enforce auth in tests - Removed duplicate domain array interpolation in printDryRunResult - Added process.env.GITHUB_TOKEN guard in test.js to prevent ambiguous failures from API rate limits * fix(ci): ensure PR labels can be applied reliably - Added `issues: write` permission to pr-labels workflow, which is strictly required by the GitHub REST API to modify labels on pull requests - Reordered script execution in `index.js` to apply/remove labels on the PR *before* attempting to sync repository-level label definitions (colors/descriptions). The definition sync is now a trailing best-effort step with error catching so transient repo-level API failures don't abort the critical path. * fix(ci): fix edge cases in pr-label index script - Added missing `skills/lark-task/` to `PATH_TO_DOMAIN_MAP` to properly detect task domain modifications - Updated GitHub REST API error checking in `syncLabelDefinition` to reliably match `error.status === 422` rather than loosely checking substring - Moved token presence check in `main()` to happen before `resolveContext` to avoid triggering unauthenticated 401 API limits when GITHUB_TOKEN is omitted locally * test(ci): clean up PR label test samples - Removed duplicate PR entries (#11 and #57) to reduce redundant API calls during testing - Renamed sample test cases to correctly reflect their expected labels (e.g. `size-l-skill-format-check` -> `size-m-skill-format-check`) * fix(ci): bootstrap new labels before applying to PRs - Prior changes correctly made full label sync best-effort, but broke the flow for brand new domains - GitHub API returns a 422 error if you attempt to attach a label to an Issue/PR that does not exist in the repository - Added a targeted bootstrap loop to create/sync specifically the labels in `toAdd` before attempting `client.addLabels()` - Left the remaining global label synchronization as a best-effort trailing action * test(ci): automate PR label regression testing - Added a dedicated GitHub Actions workflow (`pr-labels-test.yml`) to automatically run `test.js` against `samples.json` whenever the labeling logic is updated - Documented local testing instructions in `scripts/pr-labels/README.md` --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…ironments (#173) * refactor(keychain): improve error handling and consistency across platforms - Change platformGet to return error instead of empty string - Add proper error wrapping for keychain operations - Make master key creation conditional in getMasterKey - Improve error messages and handling for keychain access - Update dependent code to handle new error returns * docs(keychain): improve function documentation and error message Add detailed doc comments for all platform-specific keychain functions to clarify their purpose and behavior. Also enhance the error hint message to include a suggestion for reconfiguring the CLI when keychain access fails. * refactor(keychain): reorder operations in platformGet for better error handling Check for file existence before attempting to read and get master key * fix(keychain): improve error handling and consistency across platforms. * fix(keychain): handle corrupted master key case * fix(keychain): handle I/O errors when reading master key
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
* feat: add TestGenerateShortcutsJSON for registry shortcut export Add a test that exports all shortcuts as JSON when SHORTCUTS_OUTPUT env var is set, enabling the registry repo to extract shortcut metadata without depending on a dump-shortcuts CLI command.
Greptile SummaryThis PR refactors the mail draft inline-image workflow and hardens the keychain layer, but the PR title and description ("add drive import, export, move, task result shortcuts") do not match the actual changes — no Drive shortcut code appears in the diff. The changes are entirely in the mail/draft pipeline and infrastructure. Key changes:
Issues found:
Confidence Score: 4/5Mostly safe to merge; one P1 regression in keychain/default.go should be addressed before shipping The inline-image automation and keychain error-handling improvements are well-implemented and thoroughly tested. One P1 issue exists: defaultKeychain.Get silently returns an empty AppSecret when the keychain entry is missing, which is a user-facing regression. The two P2 issues (regex in comments, partial-failure orphan window) are minor edge cases. internal/keychain/default.go — not-found guard removal is the primary concern; shortcuts/mail/draft/patch.go — imgSrcRegexp context-blindness and partial-failure handling are secondary Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Apply
participant applyOp
participant postProcessInlineImages
participant resolveLocalImgSrc
participant loadAndAttachInline
participant removeOrphanedInlineParts
Caller->>Apply: Apply(snapshot, patch)
Apply->>applyOp: for each op (set_body, add_inline, …)
applyOp-->>Apply: snapshot mutated
Apply->>postProcessInlineImages: postProcessInlineImages(snapshot)
postProcessInlineImages->>postProcessInlineImages: findPrimaryBodyPart → htmlPart
postProcessInlineImages->>resolveLocalImgSrc: resolveLocalImgSrc(snapshot, origHTML)
loop for each local img src
resolveLocalImgSrc->>loadAndAttachInline: loadAndAttachInline(snapshot, src, uuid-cid, …)
loadAndAttachInline-->>resolveLocalImgSrc: container (MIME inline part appended)
resolveLocalImgSrc-->>resolveLocalImgSrc: replace src=path → src=cid:uuid
end
resolveLocalImgSrc-->>postProcessInlineImages: modified HTML
postProcessInlineImages->>postProcessInlineImages: extractCIDRefs → validate all cid refs have MIME parts
postProcessInlineImages->>removeOrphanedInlineParts: removeOrphanedInlineParts(root, refSet)
removeOrphanedInlineParts-->>postProcessInlineImages: orphaned parts removed
postProcessInlineImages-->>Apply: nil
Apply->>Apply: refreshSnapshot(snapshot)
Apply-->>Caller: nil or error
Reviews (1): Last reviewed commit: "feat: add TestGenerateShortcutsJSON and ..." | Re-trigger Greptile |
|
|
||
| func (d *defaultKeychain) Get(service, account string) (string, error) { | ||
| val := Get(service, account) | ||
| if val == "" { | ||
| return "", fmt.Errorf("keychain entry not found: %s/%s", service, account) | ||
| } | ||
| return val, nil | ||
| return Get(service, account) |
There was a problem hiding this comment.
Silent empty secret when keychain entry is missing
defaultKeychain.Get previously returned an explicit error when no entry existed ("keychain entry not found: %s/%s"). With this change it now passes through keychain.Get(...) directly, which returns ("", nil) for a missing entry (see platformGet on all platforms — ErrNotExist/ErrNotFound is mapped to ("", nil)).
The consequence: ResolveSecretInput in secret_resolve.go calls kc.Get for source: "keychain" entries and returns its result verbatim:
case "keychain":
return kc.Get(keychain.LarkCliService, s.Ref.ID)When the entry is missing, this returns ("", nil). RequireConfig treats that as success and builds a CliConfig with an empty AppSecret. The user then gets a cryptic API authentication error rather than a clear "keychain entry not found" message.
Consider restoring the not-found guard here (or inside ResolveSecretInput):
func (d *defaultKeychain) Get(service, account string) (string, error) {
val, err := Get(service, account)
if err != nil {
return "", err
}
if val == "" {
return "", fmt.Errorf("keychain entry not found: %s/%s", service, account)
}
return val, nil
}| // imgSrcRegexp matches <img ... src="value" ...> and captures the src value. | ||
| // It handles both single and double quotes. | ||
| var imgSrcRegexp = regexp.MustCompile(`(?i)<img\s(?:[^>]*?\s)?src\s*=\s*["']([^"']+)["']`) |
There was a problem hiding this comment.
imgSrcRegexp matches <img> tags inside HTML comments and script/style blocks
The regex (?i)<img\s(?:[^>]*?\s)?src\s*=\s*["']([^"']+)["'] is applied to the raw HTML string without any awareness of parsing context. An <img> tag inside an HTML comment, <script>, or <style> block will be matched and resolveLocalImgSrc will attempt to load the referenced path as a local file.
For example, an author commenting out an old inline image reference:
<!-- old: <img src="./draft-logo.png" /> -->would trigger a file-load attempt for ./draft-logo.png. If the file doesn't exist this produces a confusing "inline image './draft-logo.png': ..." error on set_body; if it does exist, an unexpected MIME inline part is silently attached.
Consider documenting this known limitation in the function comment, or applying a pre-filter pass that blanks out <!-- ... --> / <script>...</script> / <style>...</style> sections before running the regex.
| // resolveLocalImgSrc scans HTML for <img src="local/path"> references, | ||
| // creates MIME inline parts for each local file, and returns the HTML | ||
| // with those src attributes replaced by cid: URIs. | ||
| func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) { | ||
| matches := imgSrcRegexp.FindAllStringSubmatchIndex(html, -1) | ||
| if len(matches) == 0 { | ||
| return html, nil | ||
| } | ||
|
|
||
| var container *Part | ||
| // Cache resolved paths so the same file is only attached once. | ||
| pathToCID := make(map[string]string) | ||
|
|
||
| // Iterate in reverse so that index offsets remain valid after replacement. | ||
| for i := len(matches) - 1; i >= 0; i-- { | ||
| srcStart, srcEnd := matches[i][2], matches[i][3] | ||
| src := html[srcStart:srcEnd] | ||
| if !isLocalFileSrc(src) { | ||
| continue | ||
| } | ||
| cids[strings.ToLower(part.ContentID)] = true | ||
|
|
||
| resolvedPath, err := validate.SafeInputPath(src) | ||
| if err != nil { | ||
| return "", fmt.Errorf("inline image %q: %w", src, err) | ||
| } | ||
|
|
||
| cid, ok := pathToCID[resolvedPath] | ||
| if !ok { | ||
| fileName := filepath.Base(src) | ||
| cid, err = generateCID() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| pathToCID[resolvedPath] = cid | ||
|
|
||
| container, err = loadAndAttachInline(snapshot, src, cid, fileName, container) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| } | ||
|
|
||
| html = html[:srcStart] + "cid:" + cid + html[srcEnd:] | ||
| } | ||
| for _, ref := range refs { | ||
| if !cids[strings.ToLower(ref)] { | ||
| return fmt.Errorf("html body references missing inline cid %q", ref) | ||
|
|
||
| return html, nil |
There was a problem hiding this comment.
Partial-failure window leaves orphaned inline parts in the snapshot
The comment at lines 991–993 acknowledges this, but it's worth calling out explicitly: when resolveLocalImgSrc fails mid-loop (e.g., the second of three local images can't be read), inline parts already attached by earlier iterations are appended to the snapshot's MIME tree but the HTML body is never updated (the function returns an error before the html != origHTML check). The draft is now in an inconsistent state with orphaned MIME parts.
removeOrphanedInlineParts will clean these up on the next successful Apply, but there is no next Apply if the caller surfaces the error to the user and they abandon the operation. In that scenario the saved draft carries invisible attachment baggage.
A simple mitigation is to roll back snapshot mutations on error, or at minimum promote the existing comment to a // TODO with a tracking note.
Change-Id: I7e164a39f7635c2147ca010e657e17303a4cd03f feat: add drive import, export, move, task result shortcuts
Change-Id: I7e164a39f7635c2147ca010e657e17303a4cd03f feat: add drive import, export, move, task result shortcuts
Change-Id: I7e164a39f7635c2147ca010e657e17303a4cd03f feat: add drive import, export, move, task result shortcuts
Change-Id: I7e164a39f7635c2147ca010e657e17303a4cd03f feat: add drive import, export, move, task result shortcuts
Change-Id: I7e164a39f7635c2147ca010e657e17303a4cd03f feat: add drive import, export, move, task result shortcuts
Summary
This PR adds a new set of Drive shortcuts to support document import, export,
move, and async task result queries in
lark-cli. It also updates the relatedskill documentation and registry metadata so the new capabilities are easier to
discover and use.
Changes
drive +importto upload local files and import them asdocx,sheet, orbitabledrive +exportto exportdoc,docx,sheet, andbitablefiles to local output with bounded pollingdrive +moveto move files and folders, including async polling for folder moves and root-folder fallbackdrive +task_resultto query async task status for import, export, and folder move scenariosTest Plan
go test ./shortcuts/drive/...Related Issues