Skip to content

feat: add drive folder delete shortcut with async task polling#415

Merged
fangshuyu-768 merged 1 commit intomainfrom
feat/file_delete_shortcut
Apr 11, 2026
Merged

feat: add drive folder delete shortcut with async task polling#415
fangshuyu-768 merged 1 commit intomainfrom
feat/file_delete_shortcut

Conversation

@liujinkun2025
Copy link
Copy Markdown
Collaborator

@liujinkun2025 liujinkun2025 commented Apr 11, 2026

Summary

Add a new lark-cli drive +delete shortcut for deleting Drive files and folders. Folder deletes are handled as async operations with bounded polling and a resumable +task_result follow-up command so AI agents and humans get a clear, actionable
result shape.

Changes

  • Add drive +delete shortcut with support for Drive files and folders
  • Return structured output for sync deletes and async folder delete tasks
  • Reuse task_check follow-up flow and fix shared status parsing so "fail" is treated as a terminal failure state
  • Add unit tests for validation, dry-run output, sync delete success, async folder success, timeout, and failure cases
  • Register the new shortcut in Drive shortcut listings and update skill docs/reference docs

Test Plan

  • Unit tests pass
  • Manual local verification confirms the lark xxx command works as expected

Manual verification:

  • Ran go test ./shortcuts/drive
  • Ran make unit-test
  • Verified a real lark-cli drive +delete --as user --file-token <folder_token> --type folder --yes flow
  • Verified follow-up lark-cli drive +task_result --scenario task_check --task-id <task_id> returned status=success

Related Issues

  • None

Summary by CodeRabbit

  • New Features

    • Added a drive +delete shortcut to delete files and folders, with confirmation gating, type validation (wiki unsupported), and masked-token output. Folder deletes use limited async polling and can return a continuation command.
  • Bug Fixes

    • Improved task-status handling and polling: treats "fail" as terminal, refines retry/timeout behavior, and preserves clearer error/reporting on polling failures.
  • Documentation

    • Added detailed delete command guide with examples, parameters, async behavior, and continuation usage.
  • Tests

    • Added end-to-end and unit tests covering delete flows, confirmation, folder polling outcomes, task-result handling, and polling helpers.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 11, 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 shortcut command drive +delete with validation, dry-run planning, and execution (including async folder delete polling); updates drive task-check polling logic; adds tests, test helpers, and documentation; and registers the new shortcut.

Changes

Cohort / File(s) Summary
New shortcut implementation
shortcuts/drive/drive_delete.go
Introduces exported DriveDelete shortcut: validates flags (--file-token, --type), normalizes/allowlists type, DryRun declares DELETE (and folder task_check), Execute issues DELETE, handles async folder task polling and structured output/continuation command.
Delete tests
shortcuts/drive/drive_delete_test.go
End-to-end tests: spec validation (wiki rejected), DryRun plan assertions, confirmation gating, file delete success, and folder task outcomes (success, timeout, fail, task-check error) with controlled polling.
Task-check polling logic
shortcuts/drive/drive_move_common.go, shortcuts/drive/drive_task_result_test.go
Renamed polling globals, broadened failure detection to treat "fail" as failed, adjusted poll return semantics and messages, changed driveTaskCheckResultCommand signature (adds as param), and added test for "fail" handling.
Polling test helpers / mutex
shortcuts/drive/drive_io_test.go
Adds driveTaskCheckPollMu and withSingleDriveTaskCheckPoll(t) to safely override poll attempts/interval in tests.
Registry & shortcut tests
shortcuts/drive/shortcuts.go, shortcuts/drive/shortcuts_test.go
Registers DriveDelete in Shortcuts() and updates test expectations to include +delete.
Docs / references
skills/lark-drive/SKILL.md, skills/lark-drive/references/lark-drive-delete.md
Documents new +delete shortcut, parameters, sync vs async behavior for folders, continuation command, limits, and permission notes.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI (drive +delete)
  participant API as Drive API
  participant Poll as TaskCheck Poller

  CLI->>API: DELETE /open-apis/drive/v1/files/:file_token?type=...
  alt non-folder
    API-->>CLI: 200 OK (deleted)
    CLI->>CLI: output { deleted: true, file_token, type }
  else folder (async)
    API-->>CLI: 200 OK (task_id)
    CLI->>Poll: start polling GET /open-apis/drive/v1/files/task_check?task_id=...
    Poll->>API: GET /open-apis/drive/v1/files/task_check?task_id=...
    alt status=success
      API-->>Poll: status=success
      Poll-->>CLI: output { ready: true, deleted: true, task_id, status }
    else status=fail
      API-->>Poll: status=fail
      Poll-->>CLI: output { ready: false, failed: true, task_id, status }
    else no terminal status (timeout)
      Poll-->>CLI: output { ready: false, timed_out: true, next_command }
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • fangshuyu-768

Poem

"I'm a rabbit with a tiny quill,
I hopped to tidy Drive's long hill.
Files vanish quick, folders wake slow,
I poll and wait as task checks go.
Hop on, run +delete — watch me thrill!" 🐇

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main feature being added: a drive folder delete shortcut with async task polling, which is the primary objective of the changeset.
Description check ✅ Passed The description covers the required sections: Summary explains the motivation, Changes lists the main modifications, Test Plan documents verification steps with checkmarks, and Related Issues is included.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/file_delete_shortcut

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 11, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 11, 2026

Greptile Summary

Adds a new drive +delete shortcut for synchronously deleting Drive files and asynchronously deleting folders, with bounded polling and a resumable +task_result follow-up. The PR also generalises the shared pollDriveTaskCheck infrastructure: it fixes Failed() to accept both "fail" and "failed", adds a seenStatus guard so an all-error poll loop returns the last error instead of a silent non-result, and threads --as through the generated resume command.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style/test-coverage suggestions with no blocking correctness issues.

The new delete shortcut correctly reuses the shared polling infrastructure, the Failed() fix and seenStatus guard are both correct, and test coverage is solid across all meaningful outcomes. Only P2 concerns remain: a missing comment on the mutex pattern in the test helper, and a missing failed terminal-status sub-case in the move test table.

shortcuts/drive/drive_io_test.go (mutex comment), shortcuts/drive/drive_move_common_test.go (missing failed sub-case)

Important Files Changed

Filename Overview
shortcuts/drive/drive_delete.go New shortcut for file/folder deletion; reuses pollDriveTaskCheck correctly, validates type and token, handles async folder deletes with timeout/resume pattern. No issues found.
shortcuts/drive/drive_move_common.go Renamed poll variables, generalised pollDriveTaskCheck, fixed Failed() to handle both "fail" and "failed", added seenStatus guard so all-error loops return the error rather than silently succeeding; all changes correct.
shortcuts/drive/drive_delete_test.go Good coverage: validates wiki rejection, dry-run params, confirmation gate, sync success, and all async folder outcomes (success/timeout/fail/API error).
shortcuts/drive/drive_io_test.go Adds withSingleDriveTaskCheckPoll with a mutex to guard global variable mutation; pattern is safe for sequential subtests but would deadlock if t.Parallel() were ever added to a consuming subtest — see inline comment.
shortcuts/drive/drive_move_common_test.go Refactored into table-driven test and adopted withSingleDriveTaskCheckPoll; missing a "failed" terminal-status sub-case for the move path.
shortcuts/drive/drive_move.go Single-line fix passes runtime.As() into driveTaskCheckResultCommand so the resume command includes --as; correct.
shortcuts/drive/drive_task_result_test.go New test correctly verifies that status="fail" is surfaced as failed=true/ready=false from the +task_result command.
shortcuts/drive/shortcuts.go DriveDelete correctly registered in the shortcuts list.
skills/lark-drive/references/lark-drive-delete.md New reference doc; accurately describes sync/async behaviour, timeout handling, status values, and permission requirements.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["lark-cli drive +delete\n--file-token X --type T --yes"] --> B{type == folder?}
    B -- No --> C["DELETE /drive/v1/files/:token\n?type=file|docx|…"]
    C --> D["Output: deleted=true, file_token, type"]
    B -- Yes --> E["DELETE /drive/v1/files/:token\n?type=folder"]
    E --> F{task_id in response?}
    F -- No --> G["Error: no task_id"]
    F -- Yes --> H["pollDriveTaskCheck\n(up to 30 × 2s)"]
    H --> I{status?}
    I -- success --> J["Output: deleted=true,\nready=true, task_id, status"]
    I -- fail/failed --> K["Error: folder task failed"]
    I -- pending/process --> L{attempts exhausted?}
    L -- No --> H
    L -- Yes --> M["Output: ready=false,\ntimed_out=true, next_command"]
    M --> N["lark-cli drive +task_result\n--scenario task_check --task-id X --as user|bot"]
    N --> O["GET /drive/v1/files/task_check\n?task_id=X"]
    O --> P["Output: status, ready, failed"]
Loading

Reviews (5): Last reviewed commit: "feat: add drive folder delete shortcut w..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 11, 2026

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/file_delete_shortcut -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 (1)
shortcuts/drive/drive_delete.go (1)

41-64: Reuse the shared task_check status type/parser here.

driveDeleteTaskCheckStatus and getDriveDeleteTaskCheckStatus now mirror driveTaskCheckStatus in shortcuts/drive/drive_move_common.go, so the next task-state tweak has to be made in two places. Keeping the delete-specific log/error strings in the polling flow is enough; the status shape/parsing can stay shared.

Also applies to: 181-191

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

In `@shortcuts/drive/drive_delete.go` around lines 41 - 64, The delete-specific
type driveDeleteTaskCheckStatus duplicates driveTaskCheckStatus; remove this
duplicate and use the shared driveTaskCheckStatus from drive_move_common.go
instead. Replace references/usages of driveDeleteTaskCheckStatus and
getDriveDeleteTaskCheckStatus to return or accept driveTaskCheckStatus, leaving
only delete-specific polling/error message strings in the delete flow; ensure
Ready(), Failed(), and StatusLabel() behavior is provided by the shared
driveTaskCheckStatus so parsing logic is centralized.
🤖 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_delete.go`:
- Around line 202-217: The polling loop calls getDriveDeleteTaskCheckStatus
repeatedly but drops the last error, causing Execute to report a timeout when
every poll failed; add a lastErr variable alongside lastStatus, set lastErr =
err whenever getDriveDeleteTaskCheckStatus returns an error, and after the loop
return that lastErr (wrapped appropriately) when no successful status was ever
observed (i.e., lastStatus is nil and lastErr != nil) instead of returning nil
and a timed-out result; update the function that returns (currently returning
lastStatus, false, nil) to return the stored error so backend/auth errors
surface.

---

Nitpick comments:
In `@shortcuts/drive/drive_delete.go`:
- Around line 41-64: The delete-specific type driveDeleteTaskCheckStatus
duplicates driveTaskCheckStatus; remove this duplicate and use the shared
driveTaskCheckStatus from drive_move_common.go instead. Replace
references/usages of driveDeleteTaskCheckStatus and
getDriveDeleteTaskCheckStatus to return or accept driveTaskCheckStatus, leaving
only delete-specific polling/error message strings in the delete flow; ensure
Ready(), Failed(), and StatusLabel() behavior is provided by the shared
driveTaskCheckStatus so parsing logic is centralized.
🪄 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: 11b84cc7-673f-4030-9676-734cb2056ec0

📥 Commits

Reviewing files that changed from the base of the PR and between 368ec7e and 7582966.

📒 Files selected for processing (8)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md

@liujinkun2025 liujinkun2025 force-pushed the feat/file_delete_shortcut branch from 7582966 to f1fdadf Compare April 11, 2026 07:13
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.

♻️ Duplicate comments (1)
shortcuts/drive/drive_delete.go (1)

102-123: ⚠️ Potential issue | 🟠 Major

Avoid mapping repeated polling failures to timed_out=true.

At Line 102, this path assumes err == nil && ready == false means “still running”. But the current shared helper can return that shape even when every poll attempt failed (backend/auth/network), which makes real failures look like resumable timeouts.

Suggested fix (in shared helper)
--- a/shortcuts/drive/drive_move_common.go
+++ b/shortcuts/drive/drive_move_common.go
@@
 func pollDriveTaskCheck(runtime *common.RuntimeContext, taskID string) (driveTaskCheckStatus, bool, error) {
 	lastStatus := driveTaskCheckStatus{TaskID: taskID}
+	var (
+		seenStatus bool
+		lastErr    error
+	)
 	for attempt := 1; attempt <= driveTaskCheckPollAttempts; attempt++ {
 		if attempt > 1 {
 			time.Sleep(driveTaskCheckPollInterval)
 		}
 
 		status, err := getDriveTaskCheckStatus(runtime, taskID)
 		if err != nil {
+			lastErr = err
 			fmt.Fprintf(runtime.IO().ErrOut, "Error polling task %s: %s\n", taskID, err)
 			continue
 		}
+		seenStatus = true
 		lastStatus = status
@@
 		if status.Failed() {
 			return status, false, output.Errorf(output.ExitAPI, "api_error", "folder task failed")
 		}
 	}
+	if !seenStatus && lastErr != nil {
+		return driveTaskCheckStatus{}, false, lastErr
+	}
 	return lastStatus, false, nil
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_delete.go` around lines 102 - 123, The current code
treats any (err==nil, ready==false) result from pollDriveTaskCheck as a running
task, but pollDriveTaskCheck can return that shape when all poll attempts
failed; fix by changing pollDriveTaskCheck to return an additional boolean
(e.g., pollFailed or allAttemptsFailed) or a distinct sentinel error so callers
can distinguish "still running" vs "polling failed", then update this handler
(which calls pollDriveTaskCheck and uses driveTaskCheckResultCommand and
runtime.IO().ErrOut) to: if pollFailed is true, do not set timed_out=true or
suggest the resume command—instead set out["poll_failed"]=true (or
out["error"]=true) and write an error message to runtime.IO().ErrOut describing
the polling failure; only when pollFailed==false and ready==false emit
next_command and timed_out=true as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@shortcuts/drive/drive_delete.go`:
- Around line 102-123: The current code treats any (err==nil, ready==false)
result from pollDriveTaskCheck as a running task, but pollDriveTaskCheck can
return that shape when all poll attempts failed; fix by changing
pollDriveTaskCheck to return an additional boolean (e.g., pollFailed or
allAttemptsFailed) or a distinct sentinel error so callers can distinguish
"still running" vs "polling failed", then update this handler (which calls
pollDriveTaskCheck and uses driveTaskCheckResultCommand and runtime.IO().ErrOut)
to: if pollFailed is true, do not set timed_out=true or suggest the resume
command—instead set out["poll_failed"]=true (or out["error"]=true) and write an
error message to runtime.IO().ErrOut describing the polling failure; only when
pollFailed==false and ready==false emit next_command and timed_out=true as
before.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 949dd857-1273-4d3f-a613-12d3533b0d19

📥 Commits

Reviewing files that changed from the base of the PR and between 7582966 and f1fdadf.

📒 Files selected for processing (9)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_move_common_test.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md
✅ Files skipped from review due to trivial changes (3)
  • skills/lark-drive/SKILL.md
  • shortcuts/drive/shortcuts.go
  • skills/lark-drive/references/lark-drive-delete.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • shortcuts/drive/shortcuts_test.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_move_common.go

@liujinkun2025 liujinkun2025 force-pushed the feat/file_delete_shortcut branch from f1fdadf to 875ea79 Compare April 11, 2026 07:39
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.

🧹 Nitpick comments (1)
shortcuts/drive/drive_io_test.go (1)

40-47: Guard global poll overrides to avoid cross-test interference.

withSingleDriveTaskCheckPoll mutates package-level globals. If two tests call this helper concurrently, poll settings can interleave and cause flaky behavior.

Suggested hardening
 import (
 	"bytes"
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 	"testing"
@@
 )
 
+var driveTaskCheckPollMu sync.Mutex
+
 func withSingleDriveTaskCheckPoll(t *testing.T) {
 	t.Helper()
+	driveTaskCheckPollMu.Lock()
 
 	prevAttempts, prevInterval := driveTaskCheckPollAttempts, driveTaskCheckPollInterval
 	driveTaskCheckPollAttempts, driveTaskCheckPollInterval = 1, 0
 	t.Cleanup(func() {
 		driveTaskCheckPollAttempts, driveTaskCheckPollInterval = prevAttempts, prevInterval
+		driveTaskCheckPollMu.Unlock()
 	})
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/drive/drive_io_test.go` around lines 40 - 47, The helper
withSingleDriveTaskCheckPoll mutates package-level globals
driveTaskCheckPollAttempts and driveTaskCheckPollInterval and can race when
tests run concurrently; protect those mutations by introducing a package-level
sync.Mutex (e.g., driveTaskPollMu) and lock it at the start of
withSingleDriveTaskCheckPoll and release it in the t.Cleanup closure after
restoring prev values so concurrent tests cannot interleave changes to the
globals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/drive/drive_io_test.go`:
- Around line 40-47: The helper withSingleDriveTaskCheckPoll mutates
package-level globals driveTaskCheckPollAttempts and driveTaskCheckPollInterval
and can race when tests run concurrently; protect those mutations by introducing
a package-level sync.Mutex (e.g., driveTaskPollMu) and lock it at the start of
withSingleDriveTaskCheckPoll and release it in the t.Cleanup closure after
restoring prev values so concurrent tests cannot interleave changes to the
globals.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08012e07-8460-4427-a6ac-ec91329b3a19

📥 Commits

Reviewing files that changed from the base of the PR and between f1fdadf and 875ea79.

📒 Files selected for processing (10)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_io_test.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_move_common_test.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md
✅ Files skipped from review due to trivial changes (3)
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/drive_move_common_test.go

@liujinkun2025 liujinkun2025 force-pushed the feat/file_delete_shortcut branch from 875ea79 to a64fe7c Compare April 11, 2026 07: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.

🧹 Nitpick comments (2)
shortcuts/drive/drive_delete_test.go (1)

128-215: Add a regression case for the all-error polling path.

This table covers success, pending, and terminal failure statuses, but it never exercises pollDriveTaskCheck’s !seenStatus && lastErr != nil branch in shortcuts/drive/drive_move_common.go. A single-attempt task_check API/transport error case here would pin the false-timeout fix from this PR.

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

In `@shortcuts/drive/drive_delete_test.go` around lines 128 - 215, Add a
regression case to TestDriveDeleteFolderTaskCheckOutcomes that simulates a
single-attempt transport/API error for the task_check call so
pollDriveTaskCheck's branch for "!seenStatus && lastErr != nil" is exercised: in
the test table add a case (e.g., name "task_check error") whose reg.Register for
URL "/open-apis/drive/v1/files/task_check" returns an HTTP error (non-200 status
or httpmock transport error) instead of a normal body, and assert the command
returns an error containing a short identifying message (e.g., "task_check" or
"transport error") so the test fails when pollDriveTaskCheck in
shortcuts/drive/drive_move_common.go doesn't handle the single-attempt error
path correctly.
shortcuts/drive/drive_delete.go (1)

42-43: Make the resume command self-contained.

DriveDelete supports both user and bot, but next_command only preserves task_id. Threading the selected --as through driveTaskCheckResultCommand would make the follow-up reproducible instead of depending on whatever auth default is active later.

Also applies to: 117-121

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

In `@shortcuts/drive/drive_delete.go` around lines 42 - 43, The follow-up command
built in DriveDelete is not capturing the chosen auth ("--as" / auth type), so
replaying the next_command only preserves task_id and may use a different
default auth later; update the DriveDelete flow to include the selected auth
type in next_command (e.g., add an auth field like "as" or "authType" alongside
task_id) and thread that through to driveTaskCheckResultCommand so it reads and
uses that auth value when reconstructing/executing the follow-up; ensure both
places that build next_command (the DriveDelete response and any other builder
referenced by driveTaskCheckResultCommand) include and consume this auth
parameter so the follow-up is reproducible for both "user" and "bot".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/drive/drive_delete_test.go`:
- Around line 128-215: Add a regression case to
TestDriveDeleteFolderTaskCheckOutcomes that simulates a single-attempt
transport/API error for the task_check call so pollDriveTaskCheck's branch for
"!seenStatus && lastErr != nil" is exercised: in the test table add a case
(e.g., name "task_check error") whose reg.Register for URL
"/open-apis/drive/v1/files/task_check" returns an HTTP error (non-200 status or
httpmock transport error) instead of a normal body, and assert the command
returns an error containing a short identifying message (e.g., "task_check" or
"transport error") so the test fails when pollDriveTaskCheck in
shortcuts/drive/drive_move_common.go doesn't handle the single-attempt error
path correctly.

In `@shortcuts/drive/drive_delete.go`:
- Around line 42-43: The follow-up command built in DriveDelete is not capturing
the chosen auth ("--as" / auth type), so replaying the next_command only
preserves task_id and may use a different default auth later; update the
DriveDelete flow to include the selected auth type in next_command (e.g., add an
auth field like "as" or "authType" alongside task_id) and thread that through to
driveTaskCheckResultCommand so it reads and uses that auth value when
reconstructing/executing the follow-up; ensure both places that build
next_command (the DriveDelete response and any other builder referenced by
driveTaskCheckResultCommand) include and consume this auth parameter so the
follow-up is reproducible for both "user" and "bot".

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: db57902c-7adb-4d1c-b9d7-68d6b2ee2f10

📥 Commits

Reviewing files that changed from the base of the PR and between 875ea79 and a64fe7c.

📒 Files selected for processing (10)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_io_test.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_move_common_test.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md
✅ Files skipped from review due to trivial changes (3)
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/references/lark-drive-delete.md
  • shortcuts/drive/drive_move_common_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • skills/lark-drive/SKILL.md
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/drive_io_test.go
  • shortcuts/drive/shortcuts.go

Change-Id: Ifb34f67296b800501a1b4960e02d5fed3382b84a
@liujinkun2025 liujinkun2025 force-pushed the feat/file_delete_shortcut branch from a64fe7c to 8236288 Compare April 11, 2026 08:12
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

🤖 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_delete.go`:
- Around line 16-26: driveDeleteAllowedTypes contains "shortcut" while
driveMoveAllowedTypes does not, causing an inconsistency; verify the Drive API
behavior for moving shortcuts and then update the code: if the API allows moving
shortcuts, add "shortcut": true to driveMoveAllowedTypes (symbol:
driveMoveAllowedTypes) so both allowlists match; if the API disallows moving
shortcuts, add a clear inline comment or documentation near
driveMoveAllowedTypes explaining that shortcuts cannot be moved and why
(referencing driveDeleteAllowedTypes and driveMoveAllowedTypes).
🪄 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: 01f3d620-0068-478e-aa7e-b77e364a7016

📥 Commits

Reviewing files that changed from the base of the PR and between a64fe7c and 8236288.

📒 Files selected for processing (11)
  • shortcuts/drive/drive_delete.go
  • shortcuts/drive/drive_delete_test.go
  • shortcuts/drive/drive_io_test.go
  • shortcuts/drive/drive_move.go
  • shortcuts/drive/drive_move_common.go
  • shortcuts/drive/drive_move_common_test.go
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-delete.md
✅ Files skipped from review due to trivial changes (5)
  • skills/lark-drive/SKILL.md
  • shortcuts/drive/shortcuts_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/drive_io_test.go
  • skills/lark-drive/references/lark-drive-delete.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • shortcuts/drive/drive_task_result_test.go
  • shortcuts/drive/drive_move_common_test.go

@fangshuyu-768 fangshuyu-768 merged commit bd5a33c into main Apr 11, 2026
17 checks passed
@fangshuyu-768 fangshuyu-768 deleted the feat/file_delete_shortcut branch April 11, 2026 08:47
chenjinxiong03-bit pushed a commit to chenjinxiong03-bit/cli that referenced this pull request Apr 12, 2026
chenjinxiong03-bit pushed a commit to chenjinxiong03-bit/cli that referenced this pull request Apr 13, 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