Skip to content

feat(drive): add quick mode to status diff#870

Merged
fangshuyu-768 merged 1 commit into
mainfrom
feat/drive-status-quick
May 14, 2026
Merged

feat(drive): add quick mode to status diff#870
fangshuyu-768 merged 1 commit into
mainfrom
feat/drive-status-quick

Conversation

@fangshuyu-768

@fangshuyu-768 fangshuyu-768 commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add a follow-up quick mode to drive +status on top of the modified-time smart sync PR. This keeps the default exact SHA-256 diff intact while adding a best-effort metadata-only path that avoids downloading remote bytes for files present on both sides.

Changes

  • add the --quick flag to drive +status and return detection=exact|quick in the output envelope
  • compare local mtime against Drive modified_time in quick mode, while keeping exact SHA-256 hashing as the default path
  • update shortcut docs and skill guidance to explain the best-effort semantics and performance trade-off
  • add unit coverage plus dry-run and live E2E coverage for the new quick mode
  • note the only outward-compatible surface change: successful drive +status output now includes data.detection (exact or quick), so strict JSON schema consumers should tolerate the extra field

Test Plan

  • Unit tests pass
  • Manual local verification confirms the command works as expected
  • go test ./shortcuts/drive -run TestDriveStatus
  • go test ./tests/cli_e2e/drive -run TestDrive_StatusDryRun|TestDrive_StatusWorkflow|TestDrive_StatusQuickWorkflow
  • make unit-test
  • go vet ./...
  • gofmt -l .
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main

Related Issues

Summary by CodeRabbit

  • New Features

    • Added --quick flag to drive +status for faster sync checks using modified-time comparison; runtime JSON now includes detection: "exact" or "quick".
  • Behavior

    • Default exact mode uses content-hash comparison and requires download permission; quick mode performs best-effort modified_time comparison and skips remote downloads; dry-run text reflects chosen mode.
  • Documentation

    • Docs updated with dual detection modes, examples, semantics, and performance notes.
  • Tests

    • New unit and e2e tests covering exact vs quick behavior, missing-scope handling, timestamp edge cases, and dry-run output.

Review Change Stack

@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 May 13, 2026
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

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 --quick flag to drive +status that compares local file mtimes to remote modified_time (best-effort) instead of SHA-256 hashes, conditionally relaxes download-scope validation, and includes a top-level "detection" field ("exact" or "quick") in output.

Changes

Quick-mode detection for drive +status command

Layer / File(s) Summary
Metadata types and quick comparison function
shortcuts/drive/drive_status.go
Introduce driveStatusLocalFile and driveStatusRemoteFile metadata structs; add exact and quick detection constants; implement driveStatusShouldTreatAsUnchangedQuick for modified-time equality checks.
Command definition and --quick flag
shortcuts/drive/drive_status.go
Update DriveStatus shortcut definition to add --quick boolean flag and adjust validation/dry-run behavior to reflect selected detection mode and conditional download scope enforcement.
Local file metadata collection
shortcuts/drive/drive_status.go
Refactor walkLocalForStatus to return metadata objects (PathToCwd and ModTime) instead of pre-computed SHA-256 hashes during directory traversal.
Main comparison and merge logic
shortcuts/drive/drive_status.go
Build local and remote metadata maps; merge paths; during classification, branch on --quick flag to use either modified-time comparison or exact hash-based comparison; update mergeStatusPaths signature for metadata maps.
Detection field and dry-run description
shortcuts/drive/drive_status.go
Add detection field to output; update DryRun method to generate appropriate description text for quick vs exact mode.
Unit tests for quick and exact modes
shortcuts/drive/drive_status_test.go
Strengthen existing exact-mode test with detection=exact assertion; add quick-mode tests using os.Chtimes to verify modified-time-based categorization and fallback behavior for invalid remote timestamps; add scope-related tests for download scope requirements.
E2E dry-run test for quick mode
tests/cli_e2e/drive/drive_status_dryrun_test.go
Add TestDrive_StatusDryRunQuick to verify dry-run request envelope and description text differences for quick mode.
E2E workflow tests and helper utilities
tests/cli_e2e/drive/drive_status_workflow_test.go
Extend existing workflow test with --quick assertions; add comprehensive TestDrive_StatusQuickWorkflow that seeds files with differing byte/mtime combinations and verifies both detection modes; introduce helpers for listing Drive files, parsing epoch timestamps at multiple resolutions, and asserting bucket contents.
Command documentation updates
skills/lark-drive/SKILL.md, skills/lark-drive/references/lark-drive-status.md
Document --quick flag, updated output schema with detection field, quick-mode comparison semantics (no remote downloads, conservative fallback for missing/invalid timestamps), CLI example, and performance characteristics.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • larksuite/cli#859: Both PRs add/extend Drive modified_time parsing/comparison logic and use it to decide timestamp-based bucket behavior.
  • larksuite/cli#692: Earlier work on SHA-256–based drive +status comparison that this PR extends with a quick modified-time mode.
  • larksuite/cli#803: Related changes to Drive listing and duplicate rel_path handling used by drive +status.

Suggested reviewers

  • wittam-01
  • zhaoleibd

Poem

🐰 I hopped through files both near and far,
I checked mod-times quick without hashing a bar,
If stamps agree I skip the download trick,
Else I crunch the bytes to be exact and slick,
Two modes, one rabbit—quick first, exact if thick!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.37% 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 summarizes the main change: adding a quick mode to the drive status diff command, which is the primary objective of this PR.
Description check ✅ Passed The description is comprehensive, covering all required template sections with detailed information about changes, test coverage, and related issues.
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-status-quick

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.

@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-status-quick branch from b64d8cf to de237d6 Compare May 14, 2026 02:40
@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-push-smart-if-exists branch 2 times, most recently from 3f4e468 to 82b41b4 Compare May 14, 2026 04:11
Base automatically changed from feat/drive-push-smart-if-exists to main May 14, 2026 06:10
@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-status-quick branch from de237d6 to 336eeaf Compare May 14, 2026 07:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shortcuts/drive/drive_status_test.go`:
- Around line 213-233: The test currently verifies buckets by substring matching
against the whole output string; change it to decode the stdout JSON into a Go
structure (e.g., unmarshal into map[string][]struct{RelPath string
`json:"rel_path"` FileToken string `json:"file_token"`}) and then, inside the
loop over checks, assert that the specific bucket key exists and contains an
entry whose rel_path equals c.path and whose file_token equals c.token (when
c.token != ""). Update the same pattern at the other occurrence referenced
(lines ~280-282) so each assertion checks membership per-bucket in the parsed
JSON rather than using strings.Contains on the entire output.

In `@skills/lark-drive/SKILL.md`:
- Line 232: The markdown table row for the `+status` skill contains an unescaped
pipe inside the inline code span (`detection=exact|quick`) which breaks table
parsing; update that cell in SKILL.md (the `+status` row) to escape the pipe
(e.g., `detection=exact\|quick`) or replace the inline span with an alternative
notation (e.g., `detection=exact`/`quick` or using `|`) so the table
renders correctly and MD056 is resolved.
🪄 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: d8ae35ee-1fb4-4919-8eca-cf88c02c7111

📥 Commits

Reviewing files that changed from the base of the PR and between 37459b6 and 336eeaf.

📒 Files selected for processing (6)
  • shortcuts/drive/drive_status.go
  • shortcuts/drive/drive_status_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-status.md
  • tests/cli_e2e/drive/drive_status_dryrun_test.go
  • tests/cli_e2e/drive/drive_status_workflow_test.go

Comment thread shortcuts/drive/drive_status_test.go
Comment thread skills/lark-drive/SKILL.md Outdated
@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.53846% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.91%. Comparing base (37459b6) to head (4bafcd1).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/common/types.go 76.92% 3 Missing and 3 partials ⚠️
shortcuts/drive/drive_status.go 83.78% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #870      +/-   ##
==========================================
+ Coverage   65.89%   65.91%   +0.01%     
==========================================
  Files         518      520       +2     
  Lines       48945    49269     +324     
==========================================
+ Hits        32254    32474     +220     
- Misses      13921    14021     +100     
- Partials     2770     2774       +4     

☔ 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.

@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

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

@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-status-quick branch from be00d3d to cdf5bde Compare May 14, 2026 07:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shortcuts/drive/drive_status.go`:
- Line 61: DriveStatus.Scopes currently lists only
"drive:drive.metadata:readonly" but the default command flow requires
"drive:file:download" (quick mode is opt-in), so update the declared scopes on
DriveStatus (and the similar set around lines 106-109) to include
"drive:file:download" as well, or alternatively change the default behavior to
make quick mode the default; specifically modify the DriveStatus.Scopes slice to
include both "drive:drive.metadata:readonly" and "drive:file:download" (or flip
the default mode flag so the static scopes match the new default) so runtime
auth won't fail after provisioning.
🪄 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: 5c569339-5e6d-4658-a7c4-9ba016cfe074

📥 Commits

Reviewing files that changed from the base of the PR and between 336eeaf and be00d3d.

📒 Files selected for processing (2)
  • shortcuts/drive/drive_status.go
  • shortcuts/drive/drive_status_test.go

Comment thread shortcuts/drive/drive_status.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
skills/lark-drive/SKILL.md (1)

232-232: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape the pipe character to fix table parsing.

The unescaped | in detection=exact|quick is being interpreted as a table column delimiter, breaking the markdown table structure and causing the MD056 linting warning.

🔧 Proposed fix
-| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact|quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径,越界路径 CLI 会直接拒绝;目标在 cwd 外时引导用户切换 agent 工作目录,不要私自 `cd` 绕过。 |
+| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection` (`exact` or `quick`). Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径,越界路径 CLI 会直接拒绝;目标在 cwd 外时引导用户切换 agent 工作目录,不要私自 `cd` 绕过。 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/lark-drive/SKILL.md` at line 232, The table cell containing
"detection=exact|quick" is breaking MD table parsing; update the `+status` table
entry in SKILL.md to escape the pipe inside that string (e.g., change
`detection=exact|quick` to `detection=exact\|quick` or use the HTML entity
`detection=exact|quick`) so the pipe is not treated as a column separator
and the MD056 lint warning is resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@skills/lark-drive/SKILL.md`:
- Line 232: The table cell containing "detection=exact|quick" is breaking MD
table parsing; update the `+status` table entry in SKILL.md to escape the pipe
inside that string (e.g., change `detection=exact|quick` to
`detection=exact\|quick` or use the HTML entity `detection=exact|quick`) so
the pipe is not treated as a column separator and the MD056 lint warning is
resolved.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 04dbcb39-a01e-425d-9bdf-3e3a669ee449

📥 Commits

Reviewing files that changed from the base of the PR and between be00d3d and cdf5bde.

📒 Files selected for processing (6)
  • shortcuts/drive/drive_status.go
  • shortcuts/drive/drive_status_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-status.md
  • tests/cli_e2e/drive/drive_status_dryrun_test.go
  • tests/cli_e2e/drive/drive_status_workflow_test.go
✅ Files skipped from review due to trivial changes (1)
  • skills/lark-drive/references/lark-drive-status.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • tests/cli_e2e/drive/drive_status_workflow_test.go
  • tests/cli_e2e/drive/drive_status_dryrun_test.go
  • shortcuts/drive/drive_status_test.go
  • shortcuts/drive/drive_status.go

@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-status-quick branch 3 times, most recently from efa9e31 to 97d89bc Compare May 14, 2026 10:09
@fangshuyu-768
fangshuyu-768 force-pushed the feat/drive-status-quick branch from 97d89bc to 4bafcd1 Compare May 14, 2026 12:12
@wittam-01
wittam-01 self-requested a review May 14, 2026 12:34
@fangshuyu-768
fangshuyu-768 merged commit 52e0129 into main May 14, 2026
21 checks passed
@fangshuyu-768
fangshuyu-768 deleted the feat/drive-status-quick branch May 14, 2026 12:37
tuxedomm pushed a commit to zhumiaoxin/cli that referenced this pull request Jun 6, 2026
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.

2 participants