Skip to content

perf(drive): optimize drive +delete workflow#1909

Merged
wittam-01 merged 1 commit into
mainfrom
fix/drive-delete-async
Jul 16, 2026
Merged

perf(drive): optimize drive +delete workflow#1909
wittam-01 merged 1 commit into
mainfrom
fix/drive-delete-async

Conversation

@wittam-01

@wittam-01 wittam-01 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Optimize the drive +delete workflow to improve reliability for long-running Drive deletions without changing the command surface.

Changes

  • Optimize delete result handling across supported target types
  • Add follow-up guidance for delete operations that are still in progress
  • Add coverage for document, empty folder, and non-empty folder deletion scenarios
  • Update lark-drive guidance and coverage notes

Test Plan

  • Unit tests pass: go test ./shortcuts/drive -run 'TestDriveDelete|TestValidateDriveDelete' -count=1\n- [x] Dry-run E2E passes: go test ./tests/cli_e2e/drive -run TestDriveDeleteDryRunAsyncParams -count=1\n- [x] Live E2E passes: go test ./tests/cli_e2e/drive -run TestDrive_DeleteAsyncWorkflow -count=1 -v\n\n## Related Issues\n- None

Summary by CodeRabbit

  • New Features
    • drive +delete now always issues async deletes for files and folders, with reliable task status polling; timed-out operations can be resumed via drive +task_result.
  • Bug Fixes
    • Dry-run plans consistently include async=true and the required follow-up task-check step; fallback behavior is improved when no task_id is returned.
  • Documentation
    • Updated drive +delete and drive +task_result docs and wording to consistently describe Drive move/delete async polling and resume flows.
  • Tests
    • Expanded unit and CLI E2E coverage for async parameters, polling outcomes, and end-to-end delete workflows.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact labels Jul 16, 2026
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

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

Drive deletion now always uses asynchronous requests with task polling for files and folders. Outputs include task state and continuation data, while unit, E2E, documentation, and coverage updates validate the unified workflow.

Changes

Drive asynchronous deletion

Layer / File(s) Summary
Unify async delete execution
shortcuts/drive/drive_delete.go
Drive deletes send async=true, handle missing task_id responses, poll task status, and report completion or timeout continuation fields.
Validate async delete outcomes
shortcuts/drive/drive_delete_test.go
Tests cover request parameters, scopes, missing task IDs, success and failure states, timeout continuation, and resumed task results.
Align task polling terminology and docs
shortcuts/drive/drive_move_common.go, shortcuts/drive/drive_task_result.go, skills/lark-drive/references/*
Task polling messages, comments, and references describe Drive file/folder move and delete operations.
Exercise end-to-end async deletion
tests/cli_e2e/drive/*
E2E tests cover dry-run plans and live document, empty-folder, and nonempty-folder deletion workflows with task-result verification.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • larksuite/cli#415: Modifies the same Drive asynchronous deletion and task-polling behavior.

Suggested labels: size/M, domain/ccm

Suggested reviewers: fangshuyu-768

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant DriveDelete
  participant DriveAPI
  participant TaskResult
  CLI->>DriveDelete: drive +delete
  DriveDelete->>DriveAPI: Async DELETE
  DriveAPI-->>DriveDelete: task_id or immediate success
  DriveDelete->>DriveAPI: task_check polling
  DriveAPI-->>DriveDelete: status
  DriveDelete-->>CLI: ready or next_command
  CLI->>TaskResult: drive +task_result
  TaskResult->>DriveAPI: Query task_id
  DriveAPI-->>TaskResult: Final task status
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: optimizing the drive delete workflow.
Description check ✅ Passed The description follows the required template and includes summary, changes, test plan, and related issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/drive-delete-async

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.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/drive-delete-async -y -g

@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_delete_test.go`:
- Around line 146-157: Replace substring-based assertions with structured
contract checks throughout shortcuts/drive/drive_delete_test.go: at lines
146-157 decode the delete result and verify every changed field, including
status and type; at lines 160-183 assert the typed error category and
SubtypeInvalidResponse; at lines 203-220 define typed expected task-result
fields; at lines 285-289 decode stdout and compare the structured expectation;
and at lines 333-353 reset the buffer before resumption or decode each emitted
result separately.

In `@shortcuts/drive/drive_task_result.go`:
- Line 228: Update the task_check dry-run description in
shortcuts/drive/drive_task_result.go at lines 109 and 228 to remove the
remaining “folder” wording. Also update
skills/lark-drive/references/lark-drive-task-result.md at lines 70 to change
“Drive 移动删除任务” to “Drive 移动/删除任务”.
🪄 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: b3e38c8d-f71f-4af5-80a3-cc0a6d7d07ee

📥 Commits

Reviewing files that changed from the base of the PR and between 7f43b7e and 75ec9ea.

📒 Files selected for processing (10)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_task_result.go
  • skills/lark-drive/references/lark-drive-delete.md
  • skills/lark-drive/references/lark-drive-task-result.md
  • tests/cli_e2e/drive/coverage.md
  • tests/cli_e2e/drive/drive_delete_dryrun_test.go
  • tests/cli_e2e/drive/drive_delete_workflow_test.go
  • tests/cli_e2e/drive/helpers.go

Comment thread shortcuts/drive/drive_delete_test.go
Comment thread shortcuts/drive/drive_task_result.go Outdated
Comment thread skills/lark-drive/references/lark-drive-delete.md Outdated
Comment thread skills/lark-drive/references/lark-drive-delete.md Outdated
Comment thread skills/lark-drive/references/lark-drive-task-result.md Outdated
Comment thread skills/lark-drive/references/lark-drive-task-result.md Outdated
Comment thread skills/lark-drive/references/lark-drive-task-result.md Outdated
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.96970% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.96%. Comparing base (7f43b7e) to head (622fe81).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/drive/drive_task_result.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1909      +/-   ##
==========================================
+ Coverage   74.77%   74.96%   +0.18%     
==========================================
  Files         888      892       +4     
  Lines       92946    94058    +1112     
==========================================
+ Hits        69502    70506    +1004     
- Misses      18078    18139      +61     
- Partials     5366     5413      +47     

☔ View full report in Codecov by Harness.
📢 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.

Comment thread shortcuts/drive/drive_delete.go
Comment thread shortcuts/drive/drive_delete.go

@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_delete_test.go`:
- Around line 200-211: The deletion test’s stdout checks in the current test
function should structurally validate the JSON response instead of searching for
substrings. Decode stdout into the expected response structure, assert deleted,
file_token, and type fields directly, and verify task_id is absent for the
synchronous-success fallback while preserving the existing failure diagnostics.
🪄 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: 6944afa3-f402-445d-92f5-4b80a1812a78

📥 Commits

Reviewing files that changed from the base of the PR and between 0e3cffe and 8ff6723.

📒 Files selected for processing (3)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • skills/lark-drive/references/lark-drive-delete.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • skills/lark-drive/references/lark-drive-delete.md
  • shortcuts/drive/drive_delete.go

Comment thread shortcuts/drive/drive_delete_test.go
@wittam-01
wittam-01 force-pushed the fix/drive-delete-async branch from 8ff6723 to 4ed0185 Compare July 16, 2026 07:50
@wittam-01
wittam-01 force-pushed the fix/drive-delete-async branch from 4ed0185 to 622fe81 Compare July 16, 2026 07:52
@wittam-01
wittam-01 merged commit be1f362 into main Jul 16, 2026
32 checks passed
@wittam-01
wittam-01 deleted the fix/drive-delete-async branch July 16, 2026 08:21
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 16, 2026
3 tasks
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/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants