Skip to content

feat(drive): add +search shortcut with flat filter flags#658

Merged
fangshuyu-768 merged 1 commit intomainfrom
feat/drive-search
Apr 25, 2026
Merged

feat(drive): add +search shortcut with flat filter flags#658
fangshuyu-768 merged 1 commit intomainfrom
feat/drive-search

Conversation

@liujinkun2025
Copy link
Copy Markdown
Collaborator

@liujinkun2025 liujinkun2025 commented Apr 24, 2026

Summary

  • Adds a new drive +search shortcut that exposes the doc_wiki/search v2 API through flat per-field flags (--edited-since, --opened-since, --mine, --creator-ids, --doc-types, --folder-tokens, --space-ids, etc.) instead of a nested JSON filter, so natural-language agent prompts map 1:1 to CLI flags.
  • Handles the server's open_time 3-month cap automatically: when --opened-since / --opened-until span exceeds 90 days, the CLI narrows the current request to the most recent 90-day slice and emits a stderr notice listing every remaining slice's --opened-* values so the agent can walk older ranges deterministically. Spans beyond 365 days are rejected up front to bound runaway slicing.
  • Enriches a common opaque Lark error (99992351 — a referenced open_id falls outside the app's contact visibility) with a +search-specific hint that distinguishes API scope from contact visibility and points at --creator-ids / --sharer-ids as the likely source.
  • Misc flag ergonomics: --doc-types accepts mixed case and normalizes to upper case; --sort default is translated to the server enum DEFAULT_TYPE (all other sort values upper-case 1:1).
  • Deprecates docs +search in skill docs; routes all resource discovery (docs / wikis / sheets / bitable / folders) to drive +search, and includes a paginate-within-slice-before-switching playbook for agents.

Test plan

  • go test ./shortcuts/drive/... ./shortcuts/common/... ./internal/output/... passes
  • Dry-run: --opened-since 2m produces no notice; --opened-since 8m emits a 3-slice notice and the clamped request body covers exactly 90 days; --opened-since 2y fails validation before any API call
  • Live: lark-cli drive +search --as user --query '测试' --page-size 3 returns results
  • Live: lark-cli drive +search --as user --opened-since 1y --page-size 3 returns slice 1 results plus a 5-slice notice; feeding the slice 2 flag values back in returns a different window's results with no re-notice
  • --doc-types docx,SHEET,Bitable (mixed case) succeeds; the request carries uppercase values
  • --sort default reaches the server as DEFAULT_TYPE (the previous literal DEFAULT is rejected by the server enum)

Summary by CodeRabbit

  • New Features

    • Drive +search: unified cloud-object search with flat flags, pagination, dry-run, and “more available” hint.
  • Behavioral Improvements

    • Emits notices when time ranges are auto-sliced (90‑day cap) or hour-aligned; supports slice continuation and page-token guidance.
    • Enforces ID/type caps and validates inputs; surface-friendly error/hint messages for visibility-related errors.
  • Documentation

    • Updated Drive +search docs and deprecation notice for legacy docs +search.
  • Tests

    • Added extensive tests for clamping, slicing, validation, dry-run request shape, and rendering.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new drive +search shortcut that parses flat flags (creators, scope, time ranges, doc-types, chat/sharer IDs), enforces the API 90-day open_time cap by auto-slicing with notices, builds POST requests to /open-apis/search/v2/doc_wiki/search, normalizes times, and prints tabular results.

Changes

Cohort / File(s) Summary
Implementation
shortcuts/drive/drive_search.go
New DriveSearch shortcut: flag parsing, mutual-exclusion rules (e.g., --mine vs --creator-ids, --folder-tokens vs --space-ids), ID caps/format validation, --doc-types validation and uppercasing, page-size clamping, hour-snapping for edit/comment times, open_time clamping into 90-day slices with stderr continuation notices, builds POST body for /open-apis/search/v2/doc_wiki/search, supports DryRun, Execute calls API, normalizes numeric *_time*_time_iso recursively, renders table and "more available" hint, and maps/intercepts service error 99992351.
Tests & Registry
shortcuts/drive/drive_search_test.go, shortcuts/drive/shortcuts.go, shortcuts/drive/shortcuts_test.go
Comprehensive unit tests for clamping, parsing, validation, time-snapping, request-building, ISO enrichment, error enrichment, rendering; registers DriveSearch in Shortcuts() and updates tests expecting +search.
E2E Dry-run Tests
tests/cli_e2e/drive/drive_search_dryrun_test.go
Adds CLI e2e dry-run tests asserting generated request JSON shape, doc/wiki filter population, opened-window clamping into 90-day slices, validation rejections for >365-day --opened-since, invalid --sort, and invalid --doc-types.
Documentation
skills/lark-doc/SKILL.md, skills/lark-doc/references/lark-doc-search.md, skills/lark-drive/SKILL.md, skills/lark-drive/references/lark-drive-search.md
Promotes drive +search, deprecates docs +search, documents flat flag mappings, time-dimension rules (hour snapping vs open_time slicing), continuation semantics, output normalization, and permission/error notes (including 99992351).

Sequence Diagram

sequenceDiagram
    participant CLI as CLI Client
    participant Parser as Flag Parser
    participant Validator as Validator/Clamper
    participant Builder as Request Builder
    participant API as Search API
    participant Normalizer as Time Normalizer
    participant Output as Output Renderer

    CLI->>Parser: Parse flags (--query, --mine, --opened-*, --doc-types, etc.)
    Parser->>Validator: Validate IDs, doc-types, page-size, mutual exclusivity
    alt open_time span > 90 days
        Validator->>Validator: Clamp into 90-day slices\nemit stderr continuation notices
    end
    Validator->>Builder: Produce request spec (filters, slices, page-token)
    loop for each slice
        Builder->>API: POST /open-apis/search/v2/doc_wiki/search (body)
        API-->>Builder: Response (results, res_units, has_more)
    end
    Builder->>Normalizer: Add recursive *_time_iso fields
    Normalizer->>Output: Format table, strip highlights, show "more available" hint
    Output->>CLI: Print results and notices
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • fangshuyu-768

Poem

🐰 I twitch my whiskers at flags galore,
I slice ninety days and peek for more.
Query, creator, folder, sort—I'll roam,
Fetching Docs and Wikis to bring them home.
A rabbit's hop through time to find your chrome. 🥕📄

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: adding a new drive +search shortcut with flat filter flags, matching the main functionality introduced in the changeset.
Description check ✅ Passed The description covers all required template sections (Summary, Changes, Test Plan, Related Issues) with substantive detail about the feature, implementation approach, and specific test scenarios.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/drive-search

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels Apr 24, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
shortcuts/drive/drive_search.go (2)

505-511: Consider including a timezone indicator in hour-snap notices.

The snap notice renders the before/after timestamps with "2006-01-02 15:04:05" in the process-local timezone and no offset. Agents consuming stderr across machines/timezones can't tell whether 16:00:00 is UTC or local. Using time.RFC3339 (or appending the zone) keeps the notice unambiguous and matches the RFC3339 used in the --opened-* slice notice (line 426). Deferring is reasonable if you prefer consistency with other shortcut notice styles in this repo.

♻️ Suggested tweak
 func formatHourSnapNotice(key, side string, before, after int64) string {
 	return fmt.Sprintf("notice: %s has hour-level granularity server-side; %s %s → %s",
 		key, side,
-		time.Unix(before, 0).Format("2006-01-02 15:04:05"),
-		time.Unix(after, 0).Format("2006-01-02 15:04:05"),
+		time.Unix(before, 0).Format(time.RFC3339),
+		time.Unix(after, 0).Format(time.RFC3339),
 	)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 505 - 511, The hour-snap notice
produced by formatHourSnapNotice currently formats before/after with "2006-01-02
15:04:05" which omits timezone info; update formatHourSnapNotice to render the
timestamps with an explicit timezone (e.g. use time.RFC3339 or append the
zone/offset) so the notice is unambiguous and consistent with the RFC3339 style
used by the opened-* slice notices.

326-341: Silent fallback for non-positive --page-size is inconsistent with the non-numeric path.

--page-size abc rightly returns a validation error, but --page-size 0 or --page-size -5 silently becomes 15. Either both should error, or both should clamp, to avoid masking user typos (e.g., --page-size -20 probably means --page-size 20). Given the PR explicitly lists "non-positive auto-fallback to 15" as intended behavior, this is optional — but worth considering an stderr notice so the user sees what got substituted.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 326 - 341, The function
parseDriveSearchPageSize currently errors on non-numeric input but silently
falls back to 15 for n <= 0; make behavior consistent by returning a validation
error for non-positive values instead of silently substituting 15. Update
parseDriveSearchPageSize to use output.ErrValidation when n <= 0 (e.g.,
"--page-size must be a positive number, got %q") and remove the silent fallback
branch; keep the existing max clamp (n > 20 -> n = 20). This touches
parseDriveSearchPageSize and reuses output.ErrValidation to surface the error to
users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/core/types.go`:
- Around line 43-46: The default Open API host was pointed at a pre-prod
endpoint; change the Open constant back to the production URL
("https://open.feishu.cn") so unrecognized brands (via ParseBrand() falling back
to BrandFeishu) use production, remove the TEMP-PPE override comment, and add a
unit test that asserts ParseBrand() (or the default-brand resolution logic) maps
the default/unknown brand to BrandFeishu and that its Open host equals the
production URL to prevent regression.

In `@shortcuts/drive/drive_search.go`:
- Around line 610-625: The regex htmlTagRe in renderDriveSearchTable only strips
<h> tags but the server can also return <hb> / </hb>; update the pattern used to
strip highlights so it matches both single-letter and two-letter highlight tags
(e.g., change htmlTagRe from `</?h>` to a pattern that matches `<h>`, `</h>`,
`<hb>`, `</hb>`), and ensure you apply the same stripping to both
title_highlighted and summary_highlighted processing in renderDriveSearchTable
so no <hb> tags leak into the rendered table.

---

Nitpick comments:
In `@shortcuts/drive/drive_search.go`:
- Around line 505-511: The hour-snap notice produced by formatHourSnapNotice
currently formats before/after with "2006-01-02 15:04:05" which omits timezone
info; update formatHourSnapNotice to render the timestamps with an explicit
timezone (e.g. use time.RFC3339 or append the zone/offset) so the notice is
unambiguous and consistent with the RFC3339 style used by the opened-* slice
notices.
- Around line 326-341: The function parseDriveSearchPageSize currently errors on
non-numeric input but silently falls back to 15 for n <= 0; make behavior
consistent by returning a validation error for non-positive values instead of
silently substituting 15. Update parseDriveSearchPageSize to use
output.ErrValidation when n <= 0 (e.g., "--page-size must be a positive number,
got %q") and remove the silent fallback branch; keep the existing max clamp (n >
20 -> n = 20). This touches parseDriveSearchPageSize and reuses
output.ErrValidation to surface the error to users.
🪄 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: 9881e466-8f46-4bd2-90d9-07a8354768e6

📥 Commits

Reviewing files that changed from the base of the PR and between 2e7a11a and c568dcc.

📒 Files selected for processing (9)
  • internal/core/types.go
  • shortcuts/drive/drive_search.go
  • shortcuts/drive/drive_search_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-doc/SKILL.md
  • skills/lark-doc/references/lark-doc-search.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-search.md

Comment thread internal/core/types.go Outdated
Comment thread shortcuts/drive/drive_search.go Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 24, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d9209a1429e3aeb27ace4750990b2ba95cd21147

🧩 Skill update

npx skills add larksuite/cli#feat/drive-search -y -g

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
shortcuts/drive/drive_search.go (3)

326-341: parseDriveSearchPageSize silently maps 0 / negative to the default.

--page-size 0 or --page-size -1 are very likely typos / bugs rather than "please use default", yet they're silently coerced to 15 with no feedback. Help text documents the range as "1–20". Consider rejecting non-positive values with a validation error so a misconfigured agent gets a clear signal instead of an unrelated page size.

♻️ Proposed refactor
 	n, err := strconv.Atoi(raw)
 	if err != nil {
 		return 0, output.ErrValidation("--page-size must be a number, got %q", raw)
 	}
-	if n <= 0 {
-		return 15, nil
-	}
+	if n <= 0 {
+		return 0, output.ErrValidation("--page-size must be positive, got %d", n)
+	}
 	if n > 20 {
 		n = 20
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 326 - 341, The function
parseDriveSearchPageSize currently treats n <= 0 as a silent default; change it
to return a validation error for non-positive values instead. In
parseDriveSearchPageSize, when strconv.Atoi succeeds but n <= 0, return a
descriptive validation error (e.g., "--page-size must be between 1 and 20, got
%d") rather than defaulting to 15; keep the existing clamp for n > 20 (set to
20) and unchanged behavior for empty string defaulting to 15.

580-598: Overwriting Detail.Hint may drop a server-supplied hint.

When Detail.Code == 99992351, the server-supplied Hint (if any) is replaced outright by the CLI-specific message. If the server ever starts including actionable detail (e.g., the specific offending open_id), that context will be lost. Consider appending the CLI hint (e.g., prefix with server: …; cli: …) or at least preserving the original when non-empty.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 580 - 598, In
enrichDriveSearchError, instead of overwriting exitErr.Detail.Hint when
Detail.Code == driveSearchErrUserNotVisible, preserve any existing server hint
by appending/prepending the CLI hint (e.g., "cli: …") or combine them (e.g.,
"server: <original>; cli: <our message>"); update the construction of
detail.Hint so it checks exitErr.Detail.Hint and builds a combined string before
returning the new *output.ExitError (refer to exitErr, exitErr.Detail.Hint, and
driveSearchErrUserNotVisible).

524-570: Verify 1m = 30 days / 1y = 365 days is the intended contract.

driveSearchRelativeRe treats m as 30-day units and y as 365-day units. This matches the help text "7d, 1m, 1y" and comment, but readers/agents may interpret 1m as "1 calendar month" or 3m as "last quarter". The values pass through to boundary logic (e.g., 3m = 90d is exactly the slice boundary and therefore never triggers clamping, while 4m = 120d does), which is a sharp cliff worth documenting explicitly in the flag description or tips so agents don't craft edge-case queries unintentionally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 524 - 570, The code currently
treats "m" as 30 days and "y" as 365 days (see driveSearchRelativeRe and
parseTimeValue), so explicitly state that contract and the resulting cliff
behavior in the code and user-facing help: update the comment above
driveSearchRelativeRe/parseTimeValue to say "1m = 30 days, 1y = 365 days (e.g.
3m = 90d)" and update the flag/usage/help text where the search time filter is
documented to the same wording and note the boundary edge-case (e.g., 3m == 90d
exactly). If instead you want calendar-month or year semantics, change
parseTimeValue to compute months/years via time.AddDate and update tests
accordingly (referencing parseTimeValue and driveSearchRelativeRe).
skills/lark-doc/references/lark-doc-search.md (1)

22-218: Consider adding a pointer to drive +search flag equivalents in this file too.

Since this reference is now in maintenance mode, readers landing here from old links would benefit from a compact --filter JSON → flat-flag translation table (e.g., creator_ids--creator-ids, open_time--opened-since/--opened-until, folder_tokens--folder-tokens). This avoids the "deprecated but no migration guide" pitfall during the transition period. Non-blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-doc/references/lark-doc-search.md` around lines 22 - 218, Add a
brief pointer and compact mapping table in the lark-doc-search reference that
translates --filter JSON fields (e.g., creator_ids, doc_types, chat_ids,
sharer_ids, folder_tokens, space_ids, only_title, only_comment, open_time,
create_time) to equivalent flat CLI flags (suggested symbols: --creator-ids,
--doc-types, --chat-ids, --sharer-ids, --folder-tokens, --space-ids,
--only-title, --only-comment, --opened-since/--opened-until,
--created-since/--created-until, plus --page-size/--page-token/--format), and
add a short note near the top (around the --filter / 参数 section) pointing
readers to the drive +search flag equivalents and the migration guidance when
folder_tokens vs space_ids are used; keep it concise and non-blocking for
maintenance-mode docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/lark-doc/references/lark-doc-search.md`:
- Around line 4-8: The new standalone blockquote introduced before the existing
blockquote causes MD028; merge them into one blockquote by changing the lone
non-'>' line (the blank or plain line between the two quote blocks) into a
'>'-prefixed line so lines containing the new notice and the existing "前置条件:"
paragraph form a single contiguous blockquote; locate the two adjacent quote
fragments in lark-doc-search.md around the inserted notice and make the middle
line start with '>' to remove the MD028 violation.

---

Nitpick comments:
In `@shortcuts/drive/drive_search.go`:
- Around line 326-341: The function parseDriveSearchPageSize currently treats n
<= 0 as a silent default; change it to return a validation error for
non-positive values instead. In parseDriveSearchPageSize, when strconv.Atoi
succeeds but n <= 0, return a descriptive validation error (e.g., "--page-size
must be between 1 and 20, got %d") rather than defaulting to 15; keep the
existing clamp for n > 20 (set to 20) and unchanged behavior for empty string
defaulting to 15.
- Around line 580-598: In enrichDriveSearchError, instead of overwriting
exitErr.Detail.Hint when Detail.Code == driveSearchErrUserNotVisible, preserve
any existing server hint by appending/prepending the CLI hint (e.g., "cli: …")
or combine them (e.g., "server: <original>; cli: <our message>"); update the
construction of detail.Hint so it checks exitErr.Detail.Hint and builds a
combined string before returning the new *output.ExitError (refer to exitErr,
exitErr.Detail.Hint, and driveSearchErrUserNotVisible).
- Around line 524-570: The code currently treats "m" as 30 days and "y" as 365
days (see driveSearchRelativeRe and parseTimeValue), so explicitly state that
contract and the resulting cliff behavior in the code and user-facing help:
update the comment above driveSearchRelativeRe/parseTimeValue to say "1m = 30
days, 1y = 365 days (e.g. 3m = 90d)" and update the flag/usage/help text where
the search time filter is documented to the same wording and note the boundary
edge-case (e.g., 3m == 90d exactly). If instead you want calendar-month or year
semantics, change parseTimeValue to compute months/years via time.AddDate and
update tests accordingly (referencing parseTimeValue and driveSearchRelativeRe).

In `@skills/lark-doc/references/lark-doc-search.md`:
- Around line 22-218: Add a brief pointer and compact mapping table in the
lark-doc-search reference that translates --filter JSON fields (e.g.,
creator_ids, doc_types, chat_ids, sharer_ids, folder_tokens, space_ids,
only_title, only_comment, open_time, create_time) to equivalent flat CLI flags
(suggested symbols: --creator-ids, --doc-types, --chat-ids, --sharer-ids,
--folder-tokens, --space-ids, --only-title, --only-comment,
--opened-since/--opened-until, --created-since/--created-until, plus
--page-size/--page-token/--format), and add a short note near the top (around
the --filter / 参数 section) pointing readers to the drive +search flag
equivalents and the migration guidance when folder_tokens vs space_ids are used;
keep it concise and non-blocking for maintenance-mode docs.
🪄 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: ec89308b-bed2-4595-8f5e-d9824fcb6c38

📥 Commits

Reviewing files that changed from the base of the PR and between c568dcc and fb8c104.

📒 Files selected for processing (9)
  • internal/core/types.go
  • shortcuts/drive/drive_search.go
  • shortcuts/drive/drive_search_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-doc/SKILL.md
  • skills/lark-doc/references/lark-doc-search.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-search.md
✅ Files skipped from review due to trivial changes (2)
  • shortcuts/drive/shortcuts.go
  • internal/core/types.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-doc/SKILL.md
  • skills/lark-drive/references/lark-drive-search.md

Comment thread skills/lark-doc/references/lark-doc-search.md
Comment thread skills/lark-doc/SKILL.md Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 77.90974% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.95%. Comparing base (5d12931) to head (d9209a1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/drive/drive_search.go 77.85% 83 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #658      +/-   ##
==========================================
+ Coverage   62.78%   62.95%   +0.16%     
==========================================
  Files         434      436       +2     
  Lines       38112    38593     +481     
==========================================
+ Hits        23928    24295     +367     
- Misses      12047    12150     +103     
- Partials     2137     2148      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/drive/drive_search.go`:
- Around line 660-663: The current rawTitle assignment uses fmt.Sprintf("%v",
u["title_highlighted"]) which produces the literal "<nil>" when the key is
missing; change the extraction to the same guarded pattern used for typeStr:
check u["title_highlighted"] for existence and that it's a non-nil string, else
fall back to u["title"] or the empty string, then run
htmlTagRe.ReplaceAllString(...) and common.TruncateStr on that safe string;
update the variables rawTitle/title accordingly (references: rawTitle, title,
htmlTagRe, common.TruncateStr, and u["title_highlighted"]/u["title"]).

In `@skills/lark-drive/references/lark-drive-search.md`:
- Line 78: The doc string for `--page-size` is inaccurate:
`parseDriveSearchPageSize` (in drive_search.go) returns a validation error for
non-numeric input and only clamps non-positive numbers to the default 15, so
update the docs to reflect that non-numeric values cause a validation error and
that only zero/negative values fall back to 15 (and values >20 are clamped to
20). Locate the `--page-size` row in lark-drive-search.md and change the phrase
"非法/非正数自动回落 15" to explicitly state "非数值将返回校验错误;非正数(例如0或负数)回落到15;超过20将被 clamp
到20".
- Line 114: The docs list eight --sort values but the CLI flag uses the enum
variable driveSearchSortValues (in drive_search.go) which only accepts five
values: default, edit_time, edit_time_asc, open_time, create_time; update the
table in lark-drive-search.md to remove create_time_asc,
entity_create_time_desc, and entity_create_time_asc so the documented allowed
values exactly match driveSearchSortValues (or alternately add any missing
values to driveSearchSortValues if you intend to support them).
🪄 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: 99d97005-8b23-4c48-a5f4-07a32b89f494

📥 Commits

Reviewing files that changed from the base of the PR and between fb8c104 and c433df3.

📒 Files selected for processing (8)
  • shortcuts/drive/drive_search.go
  • shortcuts/drive/drive_search_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-doc/SKILL.md
  • skills/lark-doc/references/lark-doc-search.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-search.md
✅ Files skipped from review due to trivial changes (3)
  • shortcuts/drive/shortcuts_test.go
  • shortcuts/drive/shortcuts.go
  • skills/lark-doc/references/lark-doc-search.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/lark-doc/SKILL.md

Comment thread shortcuts/drive/drive_search.go Outdated
Comment thread skills/lark-drive/references/lark-drive-search.md Outdated
Comment thread skills/lark-drive/references/lark-drive-search.md Outdated
@liujinkun2025 liujinkun2025 force-pushed the feat/drive-search branch 2 times, most recently from 1ee107f to c2d1ac2 Compare April 25, 2026 03:51
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
shortcuts/drive/drive_search.go (2)

678-695: Minor: url and edit_time columns can still render the literal "<nil>" when result_meta keys are missing.

The earlier title fix (lines 660–666) deliberately scoped the <nil> guard to title only, and the test at line 870 explicitly notes the same %v pattern is still in place for url / update_time_iso. Since these end up in the pretty table, consider applying the same guarded-extraction pattern (typed assertion + fallback to empty string) here too:

♻️ Suggested guard
-		url := ""
-		editTime := ""
-		if resultMeta != nil {
-			url = fmt.Sprintf("%v", resultMeta["url"])
-			editTime = fmt.Sprintf("%v", resultMeta["update_time_iso"])
-		}
+		var url, editTime string
+		if resultMeta != nil {
+			if s, ok := resultMeta["url"].(string); ok {
+				url = s
+			}
+			if s, ok := resultMeta["update_time_iso"].(string); ok {
+				editTime = s
+			}
+		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 678 - 695, The url and
edit_time values are extracted from resultMeta using fmt.Sprintf("%v", ...)
which renders "<nil>" when keys are missing; update the extraction in the code
that sets url and editTime (before the rows = append(...) call) to perform typed
assertions with fallbacks (e.g. if v, ok := resultMeta["url"].(string); ok && v
!= "" { url = v } else { url = "" } and similarly for
resultMeta["update_time_iso"] into editTime) so the pretty table never shows
"<nil>".

729-742: Minor: non-deterministic output if a *_time and matching *_time_iso key coexist in the same map.

Map iteration order is random, so for an input like {"create_time": 1, "create_time_iso": "preserved"} the final value of out["create_time_iso"] depends on whether the _time_iso passthrough or the _time annotation runs last. The server doesn't currently emit *_time_iso so this is theoretical, but a small fix is to skip the annotation when the matching _iso key is already present in v:

♻️ Suggested guard
 			out[key] = addDriveSearchIsoTimeFieldsOne(item)
 			if strings.HasSuffix(key, "_time") {
+				if _, exists := v[key+"_iso"]; exists {
+					continue
+				}
 				if iso := unixToISO8601(item); iso != "" {
 					out[key+"_iso"] = iso
 				}
 			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_search.go` around lines 729 - 742, The map branch in
addDriveSearchIsoTimeFieldsOne iterates v and may overwrite a pre-existing
"<key>_iso" entry due to random map iteration; change the logic so when
processing a "<key>_time" entry you first check whether the original input map v
already contains "<key>_time_iso" and skip creating out["<key>_time_iso"] if it
does. Specifically, in the case map[string]interface{} block (variables v, key,
item) before calling unixToISO8601(item) for keys with suffix "_time", guard
against adding out[key+"_iso"] when v already has that key, preserving any
original "<...>_time_iso" value produced by the input.
skills/lark-drive/references/lark-drive-search.md (2)

86-86: Nit: clarify --mine open_id source.

The doc reads "从 config 里读 open_id,取不到直接报错", but the implementation actually pulls from runtime.UserOpenId() (line 116 of drive_search.go), which resolves through the auth/session layer rather than a literal config field. A small wording tweak avoids agents going to grep config.json when --mine errors:

📝 Suggested wording
-| `--mine` | `creator_ids = [当前用户 open_id]` | bool。一键"我创建的";从 config 里读 open_id,取不到直接报错 |
+| `--mine` | `creator_ids = [当前用户 open_id]` | bool。一键"我创建的";从当前登录用户身份(`runtime.UserOpenId()`)解析 open_id,取不到直接报错(提示运行 `lark-cli auth login`) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-drive/references/lark-drive-search.md` at line 86, Update the
`--mine` docs to state that the open_id is obtained via the auth/session layer
by calling runtime.UserOpenId() (used in drive_search.go) rather than read from
a static config field; revise the wording for the `--mine` row to reference
runtime.UserOpenId() and note that an error will be thrown if that resolver
cannot produce an open_id.

139-149: Optional: add a language tag to the bare fenced code blocks.

Static analysis flags MD040 (Fenced code blocks should have a language specified) at lines 139, 146 (this file), and 210. Since these blocks display CLI hints / stderr notices rather than code, text is the conventional choice and quiets the linter without changing rendering:

📝 Suggested fix
-```
+```text
 start: floor 到整点   16:23:45 → 16:00:00
 end:   ceil  到整点   16:23:45 → 17:00:00

@@
- +text
notice: my_edit_time has hour-level granularity server-side;
start 2026-04-22 16:23:00 → 2026-04-22 16:00:00
end 2026-04-22 16:28:00 → 2026-04-22 17:00:00

The same change applies to the multi-slice notice block at line 210.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-drive/references/lark-drive-search.md` around lines 139 - 149,
The fenced code blocks showing CLI hints/stderr notices in lark-drive-search.md
are missing language tags and trigger MD040; update each bare triple-backtick
block (the blocks containing "start: floor 到整点...", the "notice:
my_edit_time..." block, and the multi-slice notice block referenced around line
210) by changing ``` to ```text so the linter treats them as plain text and the
content/formatting remains unchanged.
🤖 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/drive/drive_search.go`:
- Around line 597-604: parseTimeValue currently treats any digit string length
>=10 as seconds, which silently misparses 13-digit epoch-millis into far-future
seconds; update parseTimeValue to mirror unixToISO8601's ms-vs-s heuristic: when
the parsed integer n is >= 1e12 or the digit length is >=13, divide n by 1000
(or otherwise normalize from milliseconds to seconds) before returning, or
alternatively return an explicit error for values that exceed a reasonable
seconds threshold; implement this normalization/rejection in parseTimeValue and
ensure downstream callers like clampOpenedTimeWindow receive normalized epoch
seconds.

---

Nitpick comments:
In `@shortcuts/drive/drive_search.go`:
- Around line 678-695: The url and edit_time values are extracted from
resultMeta using fmt.Sprintf("%v", ...) which renders "<nil>" when keys are
missing; update the extraction in the code that sets url and editTime (before
the rows = append(...) call) to perform typed assertions with fallbacks (e.g. if
v, ok := resultMeta["url"].(string); ok && v != "" { url = v } else { url = "" }
and similarly for resultMeta["update_time_iso"] into editTime) so the pretty
table never shows "<nil>".
- Around line 729-742: The map branch in addDriveSearchIsoTimeFieldsOne iterates
v and may overwrite a pre-existing "<key>_iso" entry due to random map
iteration; change the logic so when processing a "<key>_time" entry you first
check whether the original input map v already contains "<key>_time_iso" and
skip creating out["<key>_time_iso"] if it does. Specifically, in the case
map[string]interface{} block (variables v, key, item) before calling
unixToISO8601(item) for keys with suffix "_time", guard against adding
out[key+"_iso"] when v already has that key, preserving any original
"<...>_time_iso" value produced by the input.

In `@skills/lark-drive/references/lark-drive-search.md`:
- Line 86: Update the `--mine` docs to state that the open_id is obtained via
the auth/session layer by calling runtime.UserOpenId() (used in drive_search.go)
rather than read from a static config field; revise the wording for the `--mine`
row to reference runtime.UserOpenId() and note that an error will be thrown if
that resolver cannot produce an open_id.
- Around line 139-149: The fenced code blocks showing CLI hints/stderr notices
in lark-drive-search.md are missing language tags and trigger MD040; update each
bare triple-backtick block (the blocks containing "start: floor 到整点...", the
"notice: my_edit_time..." block, and the multi-slice notice block referenced
around line 210) by changing ``` to ```text so the linter treats them as plain
text and the content/formatting remains unchanged.
🪄 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: 01a96b7a-443f-462b-8070-57733b74ad0d

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee107f and c2d1ac2.

📒 Files selected for processing (9)
  • shortcuts/drive/drive_search.go
  • shortcuts/drive/drive_search_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-doc/SKILL.md
  • skills/lark-doc/references/lark-doc-search.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-search.md
  • tests/cli_e2e/drive/drive_search_dryrun_test.go
✅ Files skipped from review due to trivial changes (2)
  • shortcuts/drive/shortcuts.go
  • skills/lark-doc/references/lark-doc-search.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/drive/shortcuts_test.go

Comment thread shortcuts/drive/drive_search.go
@liujinkun2025 liujinkun2025 force-pushed the feat/drive-search branch 3 times, most recently from 9538808 to 286a48b Compare April 25, 2026 04:43
Expose doc_wiki/search v2 under the drive domain via explicit flags
(--query, --edited-since, --commented-since, --opened-since,
--created-since, --mine, --creator-ids, --doc-types, --folder-tokens,
--space-ids, ...) instead of a nested JSON filter, so natural-language
queries from AI agents map 1:1 to discrete flags.

Time handling:
- my_edit_time and my_comment_time are snapped to the hour (floor/ceil)
  with a stderr notice, since those fields are aggregated at hour
  granularity server-side. create_time passes through as-is.
- open_time has a server-side 3-month cap per request. When
  --opened-since / --opened-until span exceeds 90 days, the CLI narrows
  the request to the most recent 90-day slice and emits a stderr notice
  listing every remaining slice's --opened-* values so the agent can
  re-invoke for older ranges. Spans over 365 days are rejected up front
  to bound runaway slicing.

Flag ergonomics:
- --doc-types accepts mixed case; values are normalized to upper case
  before validation and before being sent to the server.
- --sort default is translated to the server enum DEFAULT_TYPE (every
  other sort value upper-cases 1:1).

Error hints:
- Lark code 99992351 (referenced open_id outside the app's contact
  visibility) is enriched with a +search-specific hint that
  distinguishes API scope from contact visibility and points at
  --creator-ids / --sharer-ids as the likely source.

Skill docs:
- new reference at skills/lark-drive/references/lark-drive-search.md,
  including the open_time slicing protocol and the paginate-within-
  slice-before-switching agent playbook.
- lark-drive/SKILL.md routes resource-discovery to drive +search.
- lark-doc/SKILL.md and lark-doc-search.md mark docs +search as
  deprecated and point users at drive +search.

Change-Id: I36d620045809b448446d4fdbdfa923b05794da19
@fangshuyu-768 fangshuyu-768 merged commit aa48d70 into main Apr 25, 2026
21 checks passed
@fangshuyu-768 fangshuyu-768 deleted the feat/drive-search branch April 25, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants