Skip to content

fix(drive): preserve parent token on nested overwrite#908

Merged
fangshuyu-768 merged 2 commits into
mainfrom
fix/drive-push-overwrite-parent
May 15, 2026
Merged

fix(drive): preserve parent token on nested overwrite#908
fangshuyu-768 merged 2 commits into
mainfrom
fix/drive-push-overwrite-parent

Conversation

@fangshuyu-768
Copy link
Copy Markdown
Collaborator

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

Summary

  • preserve the actual parent folder token when overwriting nested files with drive +push --if-exists=overwrite
  • report parent-folder resolution failures before upload attempts
  • add regression tests for the nested overwrite happy path and parent resolution failure

Testing

  • go test ./shortcuts/drive -run 'TestDrivePushOverwriteNestedFile(UsesParentFolderToken|ReportsParentEnsureFailure)'
  • go test ./shortcuts/drive

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Drive Push to correctly handle overwriting files in nested remote folders by ensuring proper parent-folder resolution per item.
    • Improved error handling and reporting when parent-folder resolution fails during overwrite operations, marking items as failed and continuing the run.
  • Tests

    • Added unit tests for nested-file overwrite and parent-folder resolution failure scenarios.
    • Added an end-to-end test validating nested overwrite behavior and subsequent sync status.

Review Change Stack

Ensure drive +push overwrite requests for nested files keep parent_node aligned with the actual remote parent folder and report parent resolution failures explicitly.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46244946-dde1-40cb-8df2-47616f19f764

📥 Commits

Reviewing files that changed from the base of the PR and between dddb007 and 00deebf.

📒 Files selected for processing (1)
  • tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go

📝 Walkthrough

Walkthrough

This PR ensures Drive push overwrites resolve and use the correct parent folder token per-relative-path, adds drivePushEnsureParentToken, records per-item parent-resolution failures, and adds unit and e2e tests covering success and failure paths for nested overwrites.

Changes

Drive overwrite with parent folder token resolution

Layer / File(s) Summary
Parent folder token resolution and overwrite logic
shortcuts/drive/drive_push.go
New helper drivePushEnsureParentToken computes and caches the parent folder token for a given relative path. The overwrite flow now ensures the per-path parent token before calling the upload; parent-token resolution failures are recorded as failed items and the upload is skipped for those items.
Unit tests for parent folder token handling
shortcuts/drive/drive_push_test.go
TestDrivePushOverwriteNestedFileUsesParentFolderToken verifies nested overwrites use the correct nested parent folder token. TestDrivePushOverwriteNestedFileReportsParentEnsureFailure verifies parent-folder resolution failures produce a failed item with the parent-creation error message.
End-to-end test for nested overwrite behavior
tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go
New sub-test exercises a parent-level drive +push --if-exists overwrite that overwrites sub/keep.txt under its real parent, asserts the original file_token is preserved for the overwrite, and verifies drive +status converges to a clean mirror with that unchanged entry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • larksuite/cli#709: Modifies the same DrivePush overwrite flow in drive_push.go and extends overwrite tests in drive_push_test.go.

Suggested labels

domain/ccm

Suggested reviewers

  • wittam-01
  • liangshuo-1

Poem

🐰 I hopped through folders, brave and spry,
Ensured each parent token, none passed by.
Nested files now find their rightful place,
Overwrites land gentle, tidy, and ace. 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 'fix(drive): preserve parent token on nested overwrite' is clear, specific, and directly matches the main change in the changeset: fixing parent token preservation during nested file overwrites in drive push operations.
Description check ✅ Passed The PR description covers the summary, testing approach, and key changes, though it doesn't follow the provided template structure exactly (missing explicit 'Changes' bullets and 'Related Issues' section).
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/drive-push-overwrite-parent

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/M Single-domain feat or fix with limited business impact labels May 15, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.97%. Comparing base (ed9eecf) to head (00deebf).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #908      +/-   ##
==========================================
+ Coverage   65.91%   65.97%   +0.05%     
==========================================
  Files         520      523       +3     
  Lines       49277    49598     +321     
==========================================
+ Hits        32483    32722     +239     
- Misses      14022    14085      +63     
- Partials     2772     2791      +19     

☔ 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
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/drive-push-overwrite-parent -y -g

Add a live drive +push workflow case for overwriting a nested remote file so the PR parent-token fix is exercised against the real backend and verified to converge via +status.
@fangshuyu-768 fangshuyu-768 merged commit 5778adf into main May 15, 2026
21 checks passed
@fangshuyu-768 fangshuyu-768 deleted the fix/drive-push-overwrite-parent branch May 15, 2026 10:33
@liangshuo-1 liangshuo-1 mentioned this pull request May 15, 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