Skip to content

fix: guide drive import concurrency conflicts - #1751

Merged
wittam-01 merged 1 commit into
mainfrom
fix/drive-import-concurrency-guidance
Jul 6, 2026
Merged

fix: guide drive import concurrency conflicts#1751
wittam-01 merged 1 commit into
mainfrom
fix/drive-import-concurrency-guidance

Conversation

@wittam-01

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

Copy link
Copy Markdown
Collaborator

Summary

Improve drive +import guidance when import task failures include known same-location concurrency error codes, so agents can retry safely without looping indefinitely.

Changes

  • Detect 232140101, 232140100, and 233523001 in import task failure messages and expose the matched code on the typed API error.
  • Add retryable guidance that tells agents to serialize same-location batch imports, wait a few seconds, retry each failed item at most 3 times, then stop and report the conflict.
  • Update lark-drive skill docs and templates with the same batch import and bounded retry guidance.

Test Plan

  • Unit tests pass: go test ./shortcuts/drive
  • Manual local verification confirms the lark-cli drive +import flow works as expected: ran 20 concurrent real imports against the temporary Drive test folder; observed upload rate limits but no target concurrency codes, then cleaned up all created sheets.

Related Issues

  • None

Summary by CodeRabbit

  • Bug Fixes
    • Improved drive +import failure handling: concurrency-related backend errors are now recognized in the error message, labeled retryable, and include a clearer user hint for resolving conflicts.
  • Documentation
    • Updated drive import guidance to require serial execution when multiple imports target the same destination (same folder/root or --target-token).
    • Expanded conflict error-code guidance (232140101 / 232140100 / 233523001) with a serial retry approach for failed items (up to 3 attempts with delays) before stopping and reporting.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds concurrent-operation error code detection to the drive import failure path, marking matching errors retryable with a fixed hint. Unit tests cover the new error shaping, and drive import docs now require serial execution for same-target batch imports.

Changes

Concurrency Error Detection in Drive Import

Layer / File(s) Summary
Failure error builder with concurrency detection
shortcuts/drive/drive_import_common.go
Extracts failure error construction into driveImportFailureError, adds driveImportConcurrentOperationCodes and a hint constant, and adds driveImportConcurrentOperationCode/isASCIIDigit helpers to detect concurrency codes with digit-boundary checks, marking matches retryable.
Tests for concurrency error detection
shortcuts/drive/drive_import_common_test.go
Adds tests verifying concurrency-coded job errors yield retryable typed errors with the expected hint, and non-matching errors remain unchanged.

Serial Import Documentation Updates

Layer / File(s) Summary
Serial import rules and error code guidance
skill-template/domains/drive.md, skills/lark-drive/SKILL.md, skills/lark-drive/references/lark-drive-import.md
Adds rules requiring serial execution for batch imports to the same destination and documents handling of specific concurrency error codes with retry-and-report guidance.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

  • larksuite/cli#194: Introduced the pollDriveImportTask polling/error logic in drive_import_common.go that this PR directly extends with concurrency error detection.

Suggested reviewers: fangshuyu-768, liujinkun2025

🚥 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: guidance for drive import concurrency conflicts.
Description check ✅ Passed The description follows the template with Summary, Changes, Test Plan, and Related Issues, and the key details are filled in.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/drive-import-concurrency-guidance

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 added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.42%. Comparing base (c45ff56) to head (0d9916a).

Files with missing lines Patch % Lines
shortcuts/drive/drive_import_common.go 83.33% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1751   +/-   ##
=======================================
  Coverage   74.42%   74.42%           
=======================================
  Files         854      854           
  Lines       88457    88477   +20     
=======================================
+ Hits        65832    65852   +20     
+ Misses      17556    17555    -1     
- Partials     5069     5070    +1     

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

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/drive-import-concurrency-guidance -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.

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

458-480: 📐 Maintainability & Code Quality | 🔵 Trivial

Correct, but consider a regexp-based simplification for readability.

The manual substring scan with isASCIIDigit boundary checks is correct and covered by tests (verified against the "longer numeric code containing known code" test case), but it's more code than needed. A regexp with word-boundary anchors would express the same intent (find a code not embedded in a longer digit run) more concisely.
[optional_and_nitpick]

♻️ Optional simplification using regexp
-func driveImportConcurrentOperationCode(msg string) (int, bool) {
-	for _, code := range driveImportConcurrentOperationCodes {
-		codeText := strconv.Itoa(code)
-		for idx := strings.Index(msg, codeText); idx >= 0; {
-			end := idx + len(codeText)
-			if (idx == 0 || !isASCIIDigit(msg[idx-1])) && (end == len(msg) || !isASCIIDigit(msg[end])) {
-				return code, true
-			}
-
-			nextStart := idx + 1
-			next := strings.Index(msg[nextStart:], codeText)
-			if next < 0 {
-				break
-			}
-			idx = nextStart + next
-		}
-	}
-	return 0, false
-}
-
-func isASCIIDigit(ch byte) bool {
-	return ch >= '0' && ch <= '9'
-}
+var driveImportConcurrentOperationCodePattern = regexp.MustCompile(
+	`(?:^|\D)(` + strings.Join(driveImportConcurrentOperationCodeStrings(), "|") + `)(?:\D|$)`,
+)
+
+func driveImportConcurrentOperationCode(msg string) (int, bool) {
+	m := driveImportConcurrentOperationCodePattern.FindStringSubmatch(msg)
+	if m == nil {
+		return 0, false
+	}
+	code, _ := strconv.Atoi(m[1])
+	return code, true
+}
🤖 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 `@shortcuts/drive/drive_import_common.go` around lines 458 - 480, The current
driveImportConcurrentOperationCode helper correctly finds standalone error
codes, but it uses a manual substring scan with isASCIIDigit boundary checks
that can be simplified. Replace the nested strings.Index logic in
driveImportConcurrentOperationCode with a regexp-based match that looks for each
code only when it is not part of a longer digit sequence, while keeping the same
return behavior and preserving the existing test coverage for longer numeric
strings.
🤖 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.

Nitpick comments:
In `@shortcuts/drive/drive_import_common.go`:
- Around line 458-480: The current driveImportConcurrentOperationCode helper
correctly finds standalone error codes, but it uses a manual substring scan with
isASCIIDigit boundary checks that can be simplified. Replace the nested
strings.Index logic in driveImportConcurrentOperationCode with a regexp-based
match that looks for each code only when it is not part of a longer digit
sequence, while keeping the same return behavior and preserving the existing
test coverage for longer numeric strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d212c17b-8ded-4455-8e2a-f78568596987

📥 Commits

Reviewing files that changed from the base of the PR and between c45ff56 and 12737b5.

📒 Files selected for processing (5)
  • shortcuts/drive/drive_import_common.go
  • shortcuts/drive/drive_import_common_test.go
  • skill-template/domains/drive.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-import.md

@wittam-01
wittam-01 force-pushed the fix/drive-import-concurrency-guidance branch from 12737b5 to 0d9916a Compare July 5, 2026 05:39

@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 `@skills/lark-drive/SKILL.md`:
- Line 106: Update the retry guidance in the drive import conflict row so the
cap is clearly per failed item, not batch-wide. In the `SKILL.md` entry
describing `232140101` / `232140100` / `233523001`, rewrite the retry wording to
state that each failed item may be retried up to 3 times independently, with a
few seconds wait before each retry, while keeping the existing serial execution
guidance for concurrent imports to the same target.
🪄 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: 079cf034-086a-4108-87b8-f9ee89281e64

📥 Commits

Reviewing files that changed from the base of the PR and between 12737b5 and 0d9916a.

📒 Files selected for processing (5)
  • shortcuts/drive/drive_import_common.go
  • shortcuts/drive/drive_import_common_test.go
  • skill-template/domains/drive.md
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-import.md
✅ Files skipped from review due to trivial changes (2)
  • skill-template/domains/drive.md
  • skills/lark-drive/references/lark-drive-import.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • shortcuts/drive/drive_import_common_test.go
  • shortcuts/drive/drive_import_common.go

Comment thread skills/lark-drive/SKILL.md
@wittam-01
wittam-01 merged commit 1ba4f39 into main Jul 6, 2026
41 checks passed
@wittam-01
wittam-01 deleted the fix/drive-import-concurrency-guidance branch July 6, 2026 02:46
@lark-cli-release-bot lark-cli-release-bot Bot mentioned this pull request Jul 6, 2026
3 tasks
This was referenced Jul 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