From 436b362b5a62b756d1af037350e652ccaa6cb03d Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Thu, 14 May 2026 20:42:55 +0800 Subject: [PATCH 1/3] feat(drive): add sync filtering controls Support --ext, --include, --exclude, and .larkignore across drive +push, +pull, and +status so users can scope sync operations safely without deleting files outside the filtered view. --- go.mod | 1 + go.sum | 2 + shortcuts/drive/drive_pull.go | 16 + shortcuts/drive/drive_pull_test.go | 54 +++ shortcuts/drive/drive_push.go | 15 +- shortcuts/drive/drive_push_test.go | 70 ++++ shortcuts/drive/drive_status.go | 13 + shortcuts/drive/drive_status_test.go | 54 +++ shortcuts/drive/filter.go | 361 ++++++++++++++++++ shortcuts/drive/filter_test.go | 65 ++++ tests/cli_e2e/drive/drive_pull_dryrun_test.go | 29 ++ tests/cli_e2e/drive/drive_push_dryrun_test.go | 29 ++ .../cli_e2e/drive/drive_status_dryrun_test.go | 31 +- 13 files changed, 738 insertions(+), 2 deletions(-) create mode 100644 shortcuts/drive/filter.go create mode 100644 shortcuts/drive/filter_test.go diff --git a/go.mod b/go.mod index 770cdf5898..92b03f7741 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect + github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect github.com/catppuccin/go v0.3.0 // indirect github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/bubbletea v1.3.6 // indirect diff --git a/go.sum b/go.sum index 451a3591d3..264a495717 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiE github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3vj1nolY= github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E= +github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I= +github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY= github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc= github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws= diff --git a/shortcuts/drive/drive_pull.go b/shortcuts/drive/drive_pull.go index 04fb4509f7..7154f9a9eb 100644 --- a/shortcuts/drive/drive_pull.go +++ b/shortcuts/drive/drive_pull.go @@ -72,6 +72,9 @@ var DrivePull = common.Shortcut{ Flags: []common.Flag{ {Name: "local-dir", Desc: "local root directory (relative to cwd)", Required: true}, {Name: "folder-token", Desc: "source Drive folder token", Required: true}, + {Name: "ext", Type: "string_slice", Desc: "only include files with these extensions (e.g. md,mdx)"}, + {Name: "include", Type: "string_slice", Desc: "include only files whose rel_path matches these glob patterns"}, + {Name: "exclude", Type: "string_slice", Desc: "exclude files whose rel_path matches these glob patterns"}, {Name: "if-exists", Desc: "policy when a local file already exists (skip = never touch existing files; smart = skip when local mtime is already up to date; overwrite = always replace)", Default: drivePullIfExistsOverwrite, Enum: []string{drivePullIfExistsOverwrite, drivePullIfExistsSmart, drivePullIfExistsSkip}}, {Name: "on-duplicate-remote", Desc: "policy when multiple remote Drive entries map to the same rel_path", Default: driveDuplicateRemoteFail, Enum: []string{driveDuplicateRemoteFail, driveDuplicateRemoteRename, driveDuplicateRemoteNewest, driveDuplicateRemoteOldest}}, {Name: "delete-local", Type: "bool", Desc: "delete local regular files absent from Drive (file-level mirror; empty directories are NOT pruned); requires --yes"}, @@ -83,6 +86,7 @@ var DrivePull = common.Shortcut{ "For repeat syncs, --if-exists=smart is the recommended best-effort incremental mode: it compares local mtime with Drive modified_time and skips downloads when the local copy is already up to date.", "Duplicate remote rel_path conflicts fail by default. Use --on-duplicate-remote=rename to download duplicate files with stable hashed suffixes.", "--delete-local requires --yes; without --yes the command is rejected upfront so a stray flag never deletes anything.", + "Filter precedence is CLI flags (--ext/--include/--exclude) > .larkignore > built-in excludes such as .git/ and .lark-sync/.", }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { localDir := strings.TrimSpace(runtime.Str("local-dir")) @@ -109,6 +113,9 @@ var DrivePull = common.Shortcut{ if runtime.Bool("delete-local") && !runtime.Bool("yes") { return output.ErrValidation("--delete-local requires --yes (high-risk: deletes local files absent from Drive)") } + if _, err := buildDriveSyncFilter(runtime, localDir); err != nil { + return err + } return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { @@ -129,6 +136,10 @@ var DrivePull = common.Shortcut{ duplicateRemote = driveDuplicateRemoteFail } deleteLocal := runtime.Bool("delete-local") + filter, err := buildDriveSyncFilter(runtime, localDir) + if err != nil { + return err + } // Resolve --local-dir to its canonical absolute path before we // touch the filesystem. SafeInputPath fully evaluates symlinks @@ -162,6 +173,7 @@ var DrivePull = common.Shortcut{ if err != nil { return err } + entries = filterDriveRemoteEntries(entries, filter) if duplicates := blockingRemotePathConflicts(entries, duplicateRemote); len(duplicates) > 0 { return duplicateRemotePathError(duplicates) } @@ -248,6 +260,10 @@ var DrivePull = common.Shortcut{ if err != nil { return err } + localAbsPaths, err = filterDrivePullLocalAbsPaths(safeRoot, localAbsPaths, filter) + if err != nil { + return err + } for _, absPath := range localAbsPaths { rel, relErr := filepath.Rel(safeRoot, absPath) if relErr != nil { diff --git a/shortcuts/drive/drive_pull_test.go b/shortcuts/drive/drive_pull_test.go index c47018481a..dd1ec94458 100644 --- a/shortcuts/drive/drive_pull_test.go +++ b/shortcuts/drive/drive_pull_test.go @@ -106,6 +106,60 @@ func TestDrivePullDownloadsAndCreatesParents(t *testing.T) { mustReadFile(t, filepath.Join("local", "sub", "b.txt"), "BBB") } +func TestDrivePullFiltersRemoteFilesAndScopesDeleteLocalToFilteredView(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "keep.md"), []byte("old"), 0o644); err != nil { + t.Fatalf("WriteFile keep.md: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "skip.txt"), []byte("stay"), 0o644); err != nil { + t.Fatalf("WriteFile skip.txt: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"files": []interface{}{ + map[string]interface{}{"token": "tok_keep", "name": "keep.md", "type": "file"}, + map[string]interface{}{"token": "tok_skip", "name": "skip.txt", "type": "file"}, + }, "has_more": false}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_keep/download", + Status: 200, + Body: []byte("new-md"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DrivePull, []string{ + "+pull", + "--local-dir", "local", + "--folder-token", "folder_root", + "--ext", "md", + "--delete-local", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + mustReadFile(t, filepath.Join("local", "keep.md"), "new-md") + mustReadFile(t, filepath.Join("local", "skip.txt"), "stay") + out := stdout.String() + if strings.Contains(out, "skip.txt") { + t.Fatalf("filtered-out file should not appear in stdout: %s", out) + } +} + // TestDrivePullSkipsExistingWhenSkipPolicy verifies --if-exists=skip leaves // existing local files untouched and counts them under summary.skipped. func TestDrivePullSkipsExistingWhenSkipPolicy(t *testing.T) { diff --git a/shortcuts/drive/drive_push.go b/shortcuts/drive/drive_push.go index bc790653c9..8344c18c16 100644 --- a/shortcuts/drive/drive_push.go +++ b/shortcuts/drive/drive_push.go @@ -93,6 +93,9 @@ var DrivePush = common.Shortcut{ Flags: []common.Flag{ {Name: "local-dir", Desc: "local root directory (relative to cwd)", Required: true}, {Name: "folder-token", Desc: "target Drive folder token", Required: true}, + {Name: "ext", Type: "string_slice", Desc: "only include files with these extensions (e.g. md,mdx)"}, + {Name: "include", Type: "string_slice", Desc: "include only files whose rel_path matches these glob patterns"}, + {Name: "exclude", Type: "string_slice", Desc: "exclude files whose rel_path matches these glob patterns"}, {Name: "if-exists", Desc: "policy when a Drive file already exists at the same rel_path (skip = never touch existing remote files; smart = skip when remote modified_time already matches or is newer, otherwise fall through to overwrite semantics; overwrite = always replace)", Default: drivePushIfExistsSkip, Enum: []string{drivePushIfExistsOverwrite, drivePushIfExistsSmart, drivePushIfExistsSkip}}, {Name: "on-duplicate-remote", Desc: "policy when multiple remote Drive entries map to the same rel_path", Default: driveDuplicateRemoteFail, Enum: []string{driveDuplicateRemoteFail, driveDuplicateRemoteNewest, driveDuplicateRemoteOldest}}, {Name: "delete-remote", Type: "bool", Desc: "delete Drive files absent locally (file-level mirror; remote-only directories are not removed); requires --yes"}, @@ -107,6 +110,7 @@ var DrivePush = common.Shortcut{ "--delete-remote requires --yes; without --yes the command is rejected upfront so a stray flag never deletes anything.", "--delete-remote --yes also requires the space:document:delete scope. Validate runs a dynamic pre-flight check when the flag is on, so a missing grant fails the run before any upload — preventing a half-synced state where files were uploaded but the cleanup pass cannot delete.", "Item-level failures (upload, overwrite, folder, delete) bump summary.failed and the run exits non-zero. If any upload or folder step fails, the --delete-remote phase is skipped entirely so a partial upload never triggers remote deletion.", + "Filter precedence is CLI flags (--ext/--include/--exclude) > .larkignore > built-in excludes such as .git/ and .lark-sync/.", }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { localDir := strings.TrimSpace(runtime.Str("local-dir")) @@ -150,6 +154,9 @@ var DrivePush = common.Shortcut{ return err } } + if _, err := buildDriveSyncFilter(runtime, localDir); err != nil { + return err + } return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { @@ -174,6 +181,10 @@ var DrivePush = common.Shortcut{ duplicateRemote = driveDuplicateRemoteFail } deleteRemote := runtime.Bool("delete-remote") + filter, err := buildDriveSyncFilter(runtime, localDir) + if err != nil { + return err + } // Resolve --local-dir to its canonical absolute path before walking. // SafeInputPath fully evaluates symlinks across the entire path, @@ -193,16 +204,18 @@ var DrivePush = common.Shortcut{ } fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir) - localFiles, localDirs, err := drivePushWalkLocal(safeRoot, cwdCanonical) + localFiles, walkedDirs, err := drivePushWalkLocal(safeRoot, cwdCanonical) if err != nil { return err } + localFiles, localDirs := filterDrivePushLocalView(localFiles, walkedDirs, filter) fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) entries, err := listRemoteFolderEntries(ctx, runtime, folderToken, "") if err != nil { return err } + entries = filterDriveRemoteEntries(entries, filter) if duplicates := blockingRemotePathConflicts(entries, duplicateRemote); len(duplicates) > 0 { return duplicateRemotePathError(duplicates) } diff --git a/shortcuts/drive/drive_push_test.go b/shortcuts/drive/drive_push_test.go index ec71e4bfa2..b26a7dac32 100644 --- a/shortcuts/drive/drive_push_test.go +++ b/shortcuts/drive/drive_push_test.go @@ -134,6 +134,76 @@ func TestDrivePushUploadsAndCreatesParents(t *testing.T) { } } +func TestDrivePushFiltersLocalFilesAndScopesDeleteRemoteToFilteredView(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll(filepath.Join("local", "docs"), 0o755); err != nil { + t.Fatalf("MkdirAll docs: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "docs", "keep.md"), []byte("KEEP"), 0o644); err != nil { + t.Fatalf("WriteFile keep.md: %v", err) + } + if err := os.WriteFile(filepath.Join("local", "skip.txt"), []byte("SKIP"), 0o644); err != nil { + t.Fatalf("WriteFile skip.txt: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"files": []interface{}{ + map[string]interface{}{"token": "tok_keep_remote", "name": "docs", "type": "folder"}, + map[string]interface{}{"token": "tok_skip_remote", "name": "skip.txt", "type": "file"}, + }, "has_more": false}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=tok_keep_remote", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"files": []interface{}{ + map[string]interface{}{"token": "tok_keep_existing", "name": "keep.md", "type": "file"}, + }, "has_more": false}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/files/upload_all", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"file_token": "tok_uploaded", "version": "v2"}, + }, + }) + + err := mountAndRunDrive(t, DrivePush, []string{ + "+push", + "--local-dir", "local", + "--folder-token", "folder_root", + "--ext", "md", + "--if-exists", "overwrite", + "--delete-remote", + "--yes", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + out := stdout.String() + if !strings.Contains(out, `"uploaded": 1`) { + t.Fatalf("expected uploaded=1, got: %s", out) + } + if strings.Contains(out, "skip.txt") { + t.Fatalf("filtered-out file should not appear in stdout: %s", out) + } + if strings.Contains(out, `"deleted_remote": 1`) { + t.Fatalf("filtered-out remote file must not be deleted: %s", out) + } +} + // TestDrivePushOverwritesWhenIfExistsOverwrite verifies that a local file // whose rel_path already maps to a type=file on Drive is sent through // upload_all with the existing file_token in the form body, and that the diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go index b3e9470c04..f30be0b5b3 100644 --- a/shortcuts/drive/drive_status.go +++ b/shortcuts/drive/drive_status.go @@ -65,11 +65,15 @@ var DriveStatus = common.Shortcut{ {Name: "local-dir", Desc: "local root directory (relative to cwd)", Required: true}, {Name: "folder-token", Desc: "Drive folder token", Required: true}, {Name: "quick", Type: "bool", Desc: "compare modified_time only and skip remote downloads for files present on both sides"}, + {Name: "ext", Type: "string_slice", Desc: "only include files with these extensions (e.g. md,mdx)"}, + {Name: "include", Type: "string_slice", Desc: "include only files whose rel_path matches these glob patterns"}, + {Name: "exclude", Type: "string_slice", Desc: "exclude files whose rel_path matches these glob patterns"}, }, Tips: []string{ "Only entries with type=file are compared; online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped.", "Default detection=exact downloads files present on both sides and SHA-256 hashes them in memory; expect noticeable I/O on large folders.", "Pass --quick for the recommended fast preflight mode: it compares local mtime with Drive modified_time, skips remote downloads, and reports detection=quick as a best-effort diff.", + "Filter precedence is CLI flags (--ext/--include/--exclude) > .larkignore > built-in excludes such as .git/ and .lark-sync/.", }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { localDir := strings.TrimSpace(runtime.Str("local-dir")) @@ -109,6 +113,9 @@ var DriveStatus = common.Shortcut{ return err } } + if _, err := buildDriveSyncFilter(runtime, localDir); err != nil { + return err + } return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { @@ -128,6 +135,10 @@ var DriveStatus = common.Shortcut{ if runtime.Bool("quick") { detection = driveStatusDetectionQuick } + filter, err := buildDriveSyncFilter(runtime, localDir) + if err != nil { + return err + } // Resolve --local-dir to its canonical absolute path before walking. // SafeInputPath fully evaluates symlinks across the entire path, @@ -156,12 +167,14 @@ var DriveStatus = common.Shortcut{ if err != nil { return err } + localFiles = filterDriveStatusLocalFiles(localFiles, filter) fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) entries, err := listRemoteFolderEntries(ctx, runtime, folderToken, "") if err != nil { return err } + entries = filterDriveRemoteEntries(entries, filter) if duplicates := duplicateRemoteFilePaths(entries); len(duplicates) > 0 { return duplicateRemotePathError(duplicates) } diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index 89c1e42feb..6730d32b5c 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -250,6 +250,60 @@ func TestDriveStatusQuickCategorizesByModifiedTimeWithoutDownloads(t *testing.T) reg.Verify(t) } +func TestDriveStatusFiltersLocalAndRemoteByExt(t *testing.T) { + f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + + tmpDir := t.TempDir() + withDriveWorkingDir(t, tmpDir) + if err := os.MkdirAll("local", 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile("local/a.md", []byte("aaa"), 0o644); err != nil { + t.Fatalf("WriteFile a.md: %v", err) + } + if err := os.WriteFile("local/b.txt", []byte("bbb"), 0o644); err != nil { + t.Fatalf("WriteFile b.txt: %v", err) + } + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "folder_token=folder_root", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "data": map[string]interface{}{"files": []interface{}{ + map[string]interface{}{"token": "tok_a", "name": "a.md", "type": "file"}, + map[string]interface{}{"token": "tok_c", "name": "c.txt", "type": "file"}, + }, "has_more": false}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v1/files/tok_a/download", + Status: 200, + Body: []byte("aaa"), + Headers: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }) + + err := mountAndRunDrive(t, DriveStatus, []string{ + "+status", + "--local-dir", "local", + "--folder-token", "folder_root", + "--ext", "md", + "--as", "bot", + }, f, stdout) + if err != nil { + t.Fatalf("unexpected error: %v\nstdout: %s", err, stdout.String()) + } + + out := stdout.String() + if !strings.Contains(out, `"a.md"`) { + t.Fatalf("expected a.md in status output: %s", out) + } + if strings.Contains(out, `"b.txt"`) || strings.Contains(out, `"c.txt"`) { + t.Fatalf("filtered-out txt files should not appear in output: %s", out) + } +} + // TestDriveStatusQuickMarksUntrustedTimestampAsModified locks in the // conservative fallback for malformed remote modified_time values. func TestDriveStatusQuickMarksUntrustedTimestampAsModified(t *testing.T) { diff --git a/shortcuts/drive/filter.go b/shortcuts/drive/filter.go new file mode 100644 index 0000000000..fab2a8c636 --- /dev/null +++ b/shortcuts/drive/filter.go @@ -0,0 +1,361 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "bufio" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/bmatcuk/doublestar/v4" + + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/shortcuts/common" +) + +const driveIgnoreFileName = ".larkignore" + +var driveSyncBuiltinExcludePatterns = []string{ + ".lark-sync/**", + ".git/**", + ".DS_Store", + "Thumbs.db", + "**/*~", + "**/*.swp", + "**/*.tmp", + "**/*.temp", + "node_modules/**", + "dist/**", + "build/**", + "coverage/**", +} + +type driveSyncFilter struct { + exts map[string]struct{} + includes []string + excludes []string + ignoreRules []string + builtinExcludes []string +} + +type driveFilterDecision struct { + Included bool + Reason string +} + +func buildDriveSyncFilter(runtime *common.RuntimeContext, localDir string) (*driveSyncFilter, error) { + ignoreRules, err := readDriveIgnoreRules(runtime, localDir) + if err != nil { + return nil, err + } + includes, err := normalizeDrivePatterns(runtime.StrSlice("include"), "--include") + if err != nil { + return nil, err + } + excludes, err := normalizeDrivePatterns(runtime.StrSlice("exclude"), "--exclude") + if err != nil { + return nil, err + } + builtinExcludes, err := normalizeDrivePatterns(driveSyncBuiltinExcludePatterns, "built-in excludes") + if err != nil { + return nil, err + } + exts := make(map[string]struct{}) + for _, ext := range runtime.StrSlice("ext") { + ext = strings.TrimSpace(strings.ToLower(ext)) + if ext == "" { + continue + } + ext = strings.TrimPrefix(ext, ".") + if ext == "" { + return nil, output.ErrValidation("--ext contains an empty extension") + } + exts[ext] = struct{}{} + } + return &driveSyncFilter{ + exts: exts, + includes: includes, + excludes: excludes, + ignoreRules: ignoreRules, + builtinExcludes: builtinExcludes, + }, nil +} + +func readDriveIgnoreRules(runtime *common.RuntimeContext, localDir string) ([]string, error) { + ignorePath := filepath.Join(localDir, driveIgnoreFileName) + if _, err := runtime.FileIO().Stat(ignorePath); err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, common.WrapInputStatError(err) + } + f, err := runtime.FileIO().Open(ignorePath) + if err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, common.WrapInputStatError(err) + } + defer f.Close() + rules, err := parseDriveIgnore(f) + if err != nil { + return nil, output.ErrValidation("%s: %s", ignorePath, err) + } + return rules, nil +} + +func parseDriveIgnore(r io.Reader) ([]string, error) { + var rules []string + scanner := bufio.NewScanner(r) + for lineNo := 1; scanner.Scan(); lineNo++ { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + normalized, err := normalizeDrivePattern(line) + if err != nil { + return nil, fmt.Errorf("line %d invalid pattern %q: %w", lineNo, line, err) + } + rules = append(rules, normalized) + } + if err := scanner.Err(); err != nil { + return nil, err + } + return rules, nil +} + +func normalizeDrivePatterns(patterns []string, source string) ([]string, error) { + out := make([]string, 0, len(patterns)) + for _, pattern := range patterns { + normalized, err := normalizeDrivePattern(pattern) + if err != nil { + return nil, output.ErrValidation("%s contains invalid glob %q: %s", source, pattern, err) + } + if normalized == "" { + continue + } + out = append(out, normalized) + } + return out, nil +} + +func normalizeDrivePattern(pattern string) (string, error) { + pattern = strings.TrimSpace(pattern) + if pattern == "" { + return "", nil + } + pattern = filepath.ToSlash(pattern) + pattern = strings.TrimPrefix(pattern, "./") + pattern = strings.TrimPrefix(pattern, "/") + if strings.HasSuffix(pattern, "/") { + pattern = strings.TrimSuffix(pattern, "/") + "/**" + } + if _, err := doublestar.Match(pattern, ""); err != nil { + return "", err + } + return pattern, nil +} + +func (f *driveSyncFilter) MatchFile(rel string) driveFilterDecision { + rel = filepath.ToSlash(rel) + if rel == "" || rel == "." { + return driveFilterDecision{Included: false, Reason: "default"} + } + if matchedAny(rel, f.excludes) { + return driveFilterDecision{Included: false, Reason: "flag_exclude"} + } + if len(f.includes) > 0 && !matchedAny(rel, f.includes) { + return driveFilterDecision{Included: false, Reason: "flag_include_miss"} + } + if len(f.exts) > 0 { + ext := strings.TrimPrefix(strings.ToLower(path.Ext(rel)), ".") + if _, ok := f.exts[ext]; !ok { + return driveFilterDecision{Included: false, Reason: "flag_ext_miss"} + } + } + if matchedAny(rel, f.includes) { + return driveFilterDecision{Included: true, Reason: "flag_include"} + } + if matchedAny(rel, f.ignoreRules) { + return driveFilterDecision{Included: false, Reason: "ignore_file"} + } + if matchedAny(rel, f.builtinExcludes) { + return driveFilterDecision{Included: false, Reason: "builtin"} + } + return driveFilterDecision{Included: true, Reason: "default"} +} + +func (f *driveSyncFilter) MatchDir(rel string) driveFilterDecision { + rel = filepath.ToSlash(rel) + if rel == "" || rel == "." { + return driveFilterDecision{Included: true, Reason: "default"} + } + if matchedAny(rel, f.excludes) { + return driveFilterDecision{Included: false, Reason: "flag_exclude"} + } + if len(f.includes) > 0 && matchedAny(rel, f.includes) { + return driveFilterDecision{Included: true, Reason: "flag_include"} + } + if matchedAny(rel, f.ignoreRules) { + return driveFilterDecision{Included: false, Reason: "ignore_file"} + } + if matchedAny(rel, f.builtinExcludes) { + return driveFilterDecision{Included: false, Reason: "builtin"} + } + if len(f.includes) > 0 { + prefix := rel + "/" + for _, pattern := range f.includes { + base := strings.TrimSuffix(pattern, "/**") + if strings.HasPrefix(pattern, prefix) || strings.HasPrefix(base, prefix) { + return driveFilterDecision{Included: true, Reason: "flag_include_ancestor"} + } + } + return driveFilterDecision{Included: false, Reason: "flag_include_miss"} + } + return driveFilterDecision{Included: true, Reason: "default"} +} + +func matchedAny(rel string, patterns []string) bool { + for _, pattern := range patterns { + if driveMatchPattern(pattern, rel) { + return true + } + } + return false +} + +func driveMatchPattern(pattern, rel string) bool { + if pattern == "" || rel == "" { + return false + } + if strings.HasSuffix(pattern, "/**") { + base := strings.TrimSuffix(pattern, "/**") + if rel == base { + return true + } + } + if ok, err := doublestar.Match(pattern, rel); err == nil && ok { + return true + } + if !strings.Contains(pattern, "/") { + if ok, err := doublestar.Match("**/"+pattern, rel); err == nil && ok { + return true + } + } + return false +} + +func filterDriveRemoteEntries(entries []driveRemoteEntry, filter *driveSyncFilter) []driveRemoteEntry { + if filter == nil { + return entries + } + out := make([]driveRemoteEntry, 0, len(entries)) + for _, entry := range entries { + decision := filter.MatchFile(entry.RelPath) + if entry.Type == driveTypeFolder { + decision = filter.MatchDir(entry.RelPath) + } + if decision.Included { + out = append(out, entry) + } + } + return out +} + +func filterDrivePushLocalView(files map[string]drivePushLocalFile, dirs []string, filter *driveSyncFilter) (map[string]drivePushLocalFile, []string) { + if filter == nil { + dirsSet := make(map[string]struct{}) + for rel := range files { + for parent := drivePushParentRel(rel); parent != ""; parent = drivePushParentRel(parent) { + dirsSet[parent] = struct{}{} + } + } + for _, dir := range dirs { + dirsSet[dir] = struct{}{} + } + return files, sortedDriveDirs(dirsSet) + } + filtered := make(map[string]drivePushLocalFile, len(files)) + dirsSet := make(map[string]struct{}) + for rel, file := range files { + if !filter.MatchFile(rel).Included { + continue + } + filtered[rel] = file + for parent := drivePushParentRel(rel); parent != ""; parent = drivePushParentRel(parent) { + dirsSet[parent] = struct{}{} + } + } + for _, dir := range dirs { + if filter.MatchDir(dir).Included { + dirsSet[dir] = struct{}{} + } + } + return filtered, sortedDriveDirs(dirsSet) +} + +func sortedDriveDirs(dirsSet map[string]struct{}) []string { + dirs := make([]string, 0, len(dirsSet)) + for d := range dirsSet { + dirs = append(dirs, d) + } + sort.Slice(dirs, func(i, j int) bool { + di, dj := strings.Count(dirs[i], "/"), strings.Count(dirs[j], "/") + if di != dj { + return di < dj + } + return dirs[i] < dirs[j] + }) + return dirs +} + +func filterDriveStatusLocalHashes(files map[string]string, filter *driveSyncFilter) map[string]string { + if filter == nil { + return files + } + filtered := make(map[string]string, len(files)) + for rel, hash := range files { + if filter.MatchFile(rel).Included { + filtered[rel] = hash + } + } + return filtered +} + +func filterDriveStatusLocalFiles(files map[string]driveStatusLocalFile, filter *driveSyncFilter) map[string]driveStatusLocalFile { + if filter == nil { + return files + } + filtered := make(map[string]driveStatusLocalFile, len(files)) + for rel, file := range files { + if filter.MatchFile(rel).Included { + filtered[rel] = file + } + } + return filtered +} + +func filterDrivePullLocalAbsPaths(root string, absPaths []string, filter *driveSyncFilter) ([]string, error) { + if filter == nil { + return absPaths, nil + } + filtered := make([]string, 0, len(absPaths)) + for _, absPath := range absPaths { + rel, err := filepath.Rel(root, absPath) + if err != nil { + return nil, output.Errorf(output.ExitInternal, "io", "rel %s: %s", absPath, err) + } + if filter.MatchFile(filepath.ToSlash(rel)).Included { + filtered = append(filtered, absPath) + } + } + return filtered, nil +} diff --git a/shortcuts/drive/filter_test.go b/shortcuts/drive/filter_test.go new file mode 100644 index 0000000000..d58dab65b2 --- /dev/null +++ b/shortcuts/drive/filter_test.go @@ -0,0 +1,65 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "strings" + "testing" +) + +func TestParseDriveIgnore(t *testing.T) { + rules, err := parseDriveIgnore(strings.NewReader("# comment\n\nnode_modules/\n*.log\n docs/** \n")) + if err != nil { + t.Fatalf("parseDriveIgnore: %v", err) + } + want := []string{"node_modules/**", "*.log", "docs/**"} + if len(rules) != len(want) { + t.Fatalf("len(rules) = %d, want %d (%v)", len(rules), len(want), rules) + } + for i := range want { + if rules[i] != want[i] { + t.Fatalf("rules[%d] = %q, want %q", i, rules[i], want[i]) + } + } +} + +func TestDriveSyncFilterPrecedence(t *testing.T) { + filter := &driveSyncFilter{ + exts: map[string]struct{}{"md": {}}, + includes: []string{"docs/**"}, + excludes: []string{"docs/private/**"}, + ignoreRules: []string{"docs/**", "tmp/**"}, + builtinExcludes: []string{".git/**"}, + } + + tests := []struct { + name string + rel string + want bool + }{ + {name: "include overrides ignore", rel: "docs/readme.md", want: true}, + {name: "exclude beats include", rel: "docs/private/secret.md", want: false}, + {name: "ext filters non-matching file", rel: "docs/readme.txt", want: false}, + {name: "builtin excluded when not included", rel: ".git/config.md", want: false}, + {name: "include miss excluded", rel: "other/readme.md", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.MatchFile(tt.rel).Included + if got != tt.want { + t.Fatalf("MatchFile(%q) = %v, want %v", tt.rel, got, tt.want) + } + }) + } +} + +func TestDriveSyncFilterMatchDirIncludeAncestor(t *testing.T) { + filter := &driveSyncFilter{includes: []string{"docs/**/*.md"}} + if !filter.MatchDir("docs").Included { + t.Fatal("docs dir should be included as ancestor of include pattern") + } + if filter.MatchDir("vendor").Included { + t.Fatal("vendor dir should not be included when include patterns only target docs") + } +} diff --git a/tests/cli_e2e/drive/drive_pull_dryrun_test.go b/tests/cli_e2e/drive/drive_pull_dryrun_test.go index b637e0b66f..154c1fe3c6 100644 --- a/tests/cli_e2e/drive/drive_pull_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_pull_dryrun_test.go @@ -67,6 +67,35 @@ func TestDrive_PullDryRun(t *testing.T) { } } +func TestDrive_PullDryRunAcceptsFilterFlags(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "local"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workDir, "local", ".larkignore"), []byte("*.tmp\n"), 0o644)) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+pull", + "--local-dir", "local", + "--folder-token", "fldcnE2E001", + "--ext", "md", + "--include", "docs/**", + "--exclude", "docs/private/**", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) +} + // TestDrive_PullDryRunRejectsAbsoluteLocalDir confirms the path validator // runs in the real binary's Validate stage and surfaces a structured error // referencing --local-dir. diff --git a/tests/cli_e2e/drive/drive_push_dryrun_test.go b/tests/cli_e2e/drive/drive_push_dryrun_test.go index 7533b7999f..7f7f1d6483 100644 --- a/tests/cli_e2e/drive/drive_push_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_push_dryrun_test.go @@ -70,6 +70,35 @@ func TestDrive_PushDryRun(t *testing.T) { } } +func TestDrive_PushDryRunAcceptsFilterFlags(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "local"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workDir, "local", ".larkignore"), []byte("*.tmp\n"), 0o644)) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+push", + "--local-dir", "local", + "--folder-token", "fldcnE2E001", + "--ext", "md,mdx", + "--include", "docs/**", + "--exclude", "docs/private/**", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) +} + // TestDrive_PushDryRunRejectsAbsoluteLocalDir confirms the path validator // runs in the real binary's Validate stage and surfaces a structured error // referencing --local-dir. diff --git a/tests/cli_e2e/drive/drive_status_dryrun_test.go b/tests/cli_e2e/drive/drive_status_dryrun_test.go index 434dcfa715..5c7e959622 100644 --- a/tests/cli_e2e/drive/drive_status_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_status_dryrun_test.go @@ -83,7 +83,6 @@ func TestDrive_StatusDryRunQuick(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) t.Cleanup(cancel) - result, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{ "drive", "+status", @@ -114,6 +113,36 @@ func TestDrive_StatusDryRunQuick(t *testing.T) { } } +func TestDrive_StatusDryRunAcceptsFilterFlags(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + workDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "local"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workDir, "local", ".larkignore"), []byte("*.tmp\n"), 0o644)) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "drive", "+status", + "--local-dir", "local", + "--folder-token", "fldcnE2E001", + "--ext", "md", + "--include", "docs/**", + "--exclude", "docs/private/**", + "--dry-run", + }, + WorkDir: workDir, + DefaultAs: "user", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) +} + // TestDrive_StatusDryRunRejectsAbsoluteLocalDir confirms that the // --local-dir path validator runs in the real binary's Validate stage and // surfaces a structured error referencing --local-dir (not the framework From 39795747b0498f73325bc482aa91359fd5681a18 Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Mon, 18 May 2026 15:12:35 +0800 Subject: [PATCH 2/3] fix(drive): address PR #892 review issues - Fix MatchDir ancestor logic for wildcard include patterns by extracting concrete directory prefix (e.g. "docs/" from "docs/**/*.md") so intermediate dirs like "docs/sub" are correctly included - Add docstrings to all filter.go functions/types to meet 80% docstring coverage CI threshold - Strengthen E2E dry-run filter tests with envelope assertions (method, URL, folder_token) instead of exit-code-only checks - Add reg.Verify(t) and remote token assertion to TestDriveStatusFiltersLocalAndRemoteByExt - Add nested ancestor include test case (MatchDir("docs/sub") for pattern "docs/**/*.md") - Restore accidentally deleted blank line in TestDrive_StatusDryRunQuick - Remove unused filterDriveStatusLocalHashes dead code - Run go mod tidy to promote doublestar to direct dependency --- go.mod | 2 +- shortcuts/drive/drive_status_test.go | 4 + shortcuts/drive/filter.go | 88 +++++++++++++++---- shortcuts/drive/filter_test.go | 3 + tests/cli_e2e/drive/drive_pull_dryrun_test.go | 11 +++ tests/cli_e2e/drive/drive_push_dryrun_test.go | 11 +++ .../cli_e2e/drive/drive_status_dryrun_test.go | 12 +++ 7 files changed, 115 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 92b03f7741..60fe08ec14 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.23.0 require ( github.com/Microsoft/go-winio v0.6.2 + github.com/bmatcuk/doublestar/v4 v4.6.1 github.com/charmbracelet/huh v1.0.0 github.com/charmbracelet/lipgloss v1.1.0 github.com/gofrs/flock v0.8.1 @@ -26,7 +27,6 @@ require ( require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect - github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect github.com/catppuccin/go v0.3.0 // indirect github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/bubbletea v1.3.6 // indirect diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index 6730d32b5c..1cb3a3a84b 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -302,6 +302,10 @@ func TestDriveStatusFiltersLocalAndRemoteByExt(t *testing.T) { if strings.Contains(out, `"b.txt"`) || strings.Contains(out, `"c.txt"`) { t.Fatalf("filtered-out txt files should not appear in output: %s", out) } + if !strings.Contains(out, `"file_token": "tok_a"`) { + t.Fatalf("expected tok_a file_token in output (remote stub must be exercised): %s", out) + } + reg.Verify(t) } // TestDriveStatusQuickMarksUntrustedTimestampAsModified locks in the diff --git a/shortcuts/drive/filter.go b/shortcuts/drive/filter.go index fab2a8c636..696369df2a 100644 --- a/shortcuts/drive/filter.go +++ b/shortcuts/drive/filter.go @@ -21,8 +21,11 @@ import ( "github.com/larksuite/cli/shortcuts/common" ) +// driveIgnoreFileName is the name of the ignore file read from the sync root. const driveIgnoreFileName = ".larkignore" +// driveSyncBuiltinExcludePatterns lists glob patterns that are always excluded +// from drive sync unless overridden by --include. var driveSyncBuiltinExcludePatterns = []string{ ".lark-sync/**", ".git/**", @@ -38,6 +41,8 @@ var driveSyncBuiltinExcludePatterns = []string{ "coverage/**", } +// driveSyncFilter applies filtering rules to drive sync file paths. +// Precedence: CLI flags (--exclude > --include > --ext) > .larkignore > built-in excludes. type driveSyncFilter struct { exts map[string]struct{} includes []string @@ -46,11 +51,14 @@ type driveSyncFilter struct { builtinExcludes []string } +// driveFilterDecision reports whether a path is included and why. type driveFilterDecision struct { Included bool Reason string } +// buildDriveSyncFilter constructs a driveSyncFilter from CLI flags and the +// .larkignore file in localDir. Returns a validation error for invalid patterns. func buildDriveSyncFilter(runtime *common.RuntimeContext, localDir string) (*driveSyncFilter, error) { ignoreRules, err := readDriveIgnoreRules(runtime, localDir) if err != nil { @@ -89,6 +97,8 @@ func buildDriveSyncFilter(runtime *common.RuntimeContext, localDir string) (*dri }, nil } +// readDriveIgnoreRules reads and parses .larkignore from localDir. +// Returns nil rules (no error) when the file does not exist. func readDriveIgnoreRules(runtime *common.RuntimeContext, localDir string) ([]string, error) { ignorePath := filepath.Join(localDir, driveIgnoreFileName) if _, err := runtime.FileIO().Stat(ignorePath); err != nil { @@ -112,6 +122,8 @@ func readDriveIgnoreRules(runtime *common.RuntimeContext, localDir string) ([]st return rules, nil } +// parseDriveIgnore parses ignore rules from r. Blank lines and lines +// starting with "#" are skipped. Each rule is normalized via normalizeDrivePattern. func parseDriveIgnore(r io.Reader) ([]string, error) { var rules []string scanner := bufio.NewScanner(r) @@ -132,6 +144,8 @@ func parseDriveIgnore(r io.Reader) ([]string, error) { return rules, nil } +// normalizeDrivePatterns validates and normalizes a slice of glob patterns. +// source is used in error messages to identify the origin of a bad pattern. func normalizeDrivePatterns(patterns []string, source string) ([]string, error) { out := make([]string, 0, len(patterns)) for _, pattern := range patterns { @@ -147,6 +161,9 @@ func normalizeDrivePatterns(patterns []string, source string) ([]string, error) return out, nil } +// normalizeDrivePattern normalizes a single glob pattern: trims whitespace, +// converts to forward slashes, strips leading "./" and "/", appends "/**" to +// trailing slashes, and validates the pattern with doublestar. func normalizeDrivePattern(pattern string) (string, error) { pattern = strings.TrimSpace(pattern) if pattern == "" { @@ -164,6 +181,8 @@ func normalizeDrivePattern(pattern string) (string, error) { return pattern, nil } +// MatchFile returns the filter decision for a file at the given relative path. +// The precedence order is: --exclude > --include miss > --ext miss > --include > .larkignore > built-in excludes > default allow. func (f *driveSyncFilter) MatchFile(rel string) driveFilterDecision { rel = filepath.ToSlash(rel) if rel == "" || rel == "." { @@ -193,6 +212,9 @@ func (f *driveSyncFilter) MatchFile(rel string) driveFilterDecision { return driveFilterDecision{Included: true, Reason: "default"} } +// MatchDir returns the filter decision for a directory at the given relative path. +// Directories are included if they are ancestors of an include pattern so that +// directory traversal can reach the matching files beneath them. func (f *driveSyncFilter) MatchDir(rel string) driveFilterDecision { rel = filepath.ToSlash(rel) if rel == "" || rel == "." { @@ -213,8 +235,23 @@ func (f *driveSyncFilter) MatchDir(rel string) driveFilterDecision { if len(f.includes) > 0 { prefix := rel + "/" for _, pattern := range f.includes { - base := strings.TrimSuffix(pattern, "/**") - if strings.HasPrefix(pattern, prefix) || strings.HasPrefix(base, prefix) { + // Direct prefix match: pattern starts with "docs/sub/". + if strings.HasPrefix(pattern, prefix) { + return driveFilterDecision{Included: true, Reason: "flag_include_ancestor"} + } + // Extract the leading non-wildcard directory prefix from the + // pattern. For "docs/**/*.md" the concrete prefix is "docs/", + // so "docs" and "docs/sub" are ancestors. For "src/lib/*.go" + // the concrete prefix is "src/lib/". + concretePrefix := driveConcreteDirPrefix(pattern) + if concretePrefix == "" { + continue + } + // rel is an ancestor if its path falls within the concrete + // prefix (e.g. "docs" is a prefix of "docs/") or the + // concrete prefix falls within rel (e.g. "docs/" is a + // prefix of "docs/sub/"). + if strings.HasPrefix(prefix, concretePrefix) || strings.HasPrefix(concretePrefix, prefix) { return driveFilterDecision{Included: true, Reason: "flag_include_ancestor"} } } @@ -223,6 +260,28 @@ func (f *driveSyncFilter) MatchDir(rel string) driveFilterDecision { return driveFilterDecision{Included: true, Reason: "default"} } +// driveConcreteDirPrefix returns the leading non-wildcard directory portion of +// a glob pattern, with a trailing "/". For example: +// - "docs/**/*.md" → "docs/" +// - "src/lib/*.go" → "src/lib/" +// - "docs/**" → "docs/" +// - "*.log" → "" (no concrete directory prefix) +func driveConcreteDirPrefix(pattern string) string { + segments := strings.Split(pattern, "/") + var concrete []string + for _, seg := range segments { + if strings.ContainsAny(seg, "*?[") { + break + } + concrete = append(concrete, seg) + } + if len(concrete) == 0 { + return "" + } + return strings.Join(concrete, "/") + "/" +} + +// matchedAny reports whether rel matches any of the given glob patterns. func matchedAny(rel string, patterns []string) bool { for _, pattern := range patterns { if driveMatchPattern(pattern, rel) { @@ -232,6 +291,9 @@ func matchedAny(rel string, patterns []string) bool { return false } +// driveMatchPattern matches a single glob pattern against rel. +// It handles "/**" suffix matching, doublestar glob matching, and +// bare-name patterns (no "/") which are matched at any depth via "**/" prefix. func driveMatchPattern(pattern, rel string) bool { if pattern == "" || rel == "" { return false @@ -253,6 +315,8 @@ func driveMatchPattern(pattern, rel string) bool { return false } +// filterDriveRemoteEntries filters remote Drive entries using the filter. +// Folders are matched with MatchDir; files are matched with MatchFile. func filterDriveRemoteEntries(entries []driveRemoteEntry, filter *driveSyncFilter) []driveRemoteEntry { if filter == nil { return entries @@ -270,6 +334,9 @@ func filterDriveRemoteEntries(entries []driveRemoteEntry, filter *driveSyncFilte return out } +// filterDrivePushLocalView filters local files and directories for the push +// flow. It returns the filtered file map and a sorted list of directories +// (both parent dirs of kept files and explicitly included dirs). func filterDrivePushLocalView(files map[string]drivePushLocalFile, dirs []string, filter *driveSyncFilter) (map[string]drivePushLocalFile, []string) { if filter == nil { dirsSet := make(map[string]struct{}) @@ -302,6 +369,7 @@ func filterDrivePushLocalView(files map[string]drivePushLocalFile, dirs []string return filtered, sortedDriveDirs(dirsSet) } +// sortedDriveDirs returns the directory set sorted by depth then name. func sortedDriveDirs(dirsSet map[string]struct{}) []string { dirs := make([]string, 0, len(dirsSet)) for d := range dirsSet { @@ -317,19 +385,7 @@ func sortedDriveDirs(dirsSet map[string]struct{}) []string { return dirs } -func filterDriveStatusLocalHashes(files map[string]string, filter *driveSyncFilter) map[string]string { - if filter == nil { - return files - } - filtered := make(map[string]string, len(files)) - for rel, hash := range files { - if filter.MatchFile(rel).Included { - filtered[rel] = hash - } - } - return filtered -} - +// filterDriveStatusLocalFiles filters the local file info map by the filter. func filterDriveStatusLocalFiles(files map[string]driveStatusLocalFile, filter *driveSyncFilter) map[string]driveStatusLocalFile { if filter == nil { return files @@ -343,6 +399,8 @@ func filterDriveStatusLocalFiles(files map[string]driveStatusLocalFile, filter * return filtered } +// filterDrivePullLocalAbsPaths filters absolute local paths for the pull +// --delete-local flow, keeping only paths whose relative form passes the filter. func filterDrivePullLocalAbsPaths(root string, absPaths []string, filter *driveSyncFilter) ([]string, error) { if filter == nil { return absPaths, nil diff --git a/shortcuts/drive/filter_test.go b/shortcuts/drive/filter_test.go index d58dab65b2..9841e2c0fb 100644 --- a/shortcuts/drive/filter_test.go +++ b/shortcuts/drive/filter_test.go @@ -59,6 +59,9 @@ func TestDriveSyncFilterMatchDirIncludeAncestor(t *testing.T) { if !filter.MatchDir("docs").Included { t.Fatal("docs dir should be included as ancestor of include pattern") } + if !filter.MatchDir("docs/sub").Included { + t.Fatal("docs/sub dir should be included as ancestor of include pattern docs/**/*.md") + } if filter.MatchDir("vendor").Included { t.Fatal("vendor dir should not be included when include patterns only target docs") } diff --git a/tests/cli_e2e/drive/drive_pull_dryrun_test.go b/tests/cli_e2e/drive/drive_pull_dryrun_test.go index 154c1fe3c6..791642788b 100644 --- a/tests/cli_e2e/drive/drive_pull_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_pull_dryrun_test.go @@ -94,6 +94,17 @@ func TestDrive_PullDryRunAcceptsFilterFlags(t *testing.T) { }) require.NoError(t, err) result.AssertExitCode(t, 0) + + out := result.Stdout + if got := gjson.Get(out, "api.0.method").String(); got != "GET" { + t.Fatalf("method = %q, want GET\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "api.0.url").String(); got != "/open-apis/drive/v1/files" { + t.Fatalf("url = %q, want /open-apis/drive/v1/files\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "folder_token").String(); got != "fldcnE2E001" { + t.Fatalf("folder_token = %q, want fldcnE2E001\nstdout:\n%s", got, out) + } } // TestDrive_PullDryRunRejectsAbsoluteLocalDir confirms the path validator diff --git a/tests/cli_e2e/drive/drive_push_dryrun_test.go b/tests/cli_e2e/drive/drive_push_dryrun_test.go index 7f7f1d6483..c4bd04254d 100644 --- a/tests/cli_e2e/drive/drive_push_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_push_dryrun_test.go @@ -97,6 +97,17 @@ func TestDrive_PushDryRunAcceptsFilterFlags(t *testing.T) { }) require.NoError(t, err) result.AssertExitCode(t, 0) + + out := result.Stdout + if got := gjson.Get(out, "api.0.method").String(); got != "GET" { + t.Fatalf("method = %q, want GET\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "api.0.url").String(); got != "/open-apis/drive/v1/files" { + t.Fatalf("url = %q, want /open-apis/drive/v1/files\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "folder_token").String(); got != "fldcnE2E001" { + t.Fatalf("folder_token = %q, want fldcnE2E001\nstdout:\n%s", got, out) + } } // TestDrive_PushDryRunRejectsAbsoluteLocalDir confirms the path validator diff --git a/tests/cli_e2e/drive/drive_status_dryrun_test.go b/tests/cli_e2e/drive/drive_status_dryrun_test.go index 5c7e959622..c6df84deb2 100644 --- a/tests/cli_e2e/drive/drive_status_dryrun_test.go +++ b/tests/cli_e2e/drive/drive_status_dryrun_test.go @@ -83,6 +83,7 @@ func TestDrive_StatusDryRunQuick(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) t.Cleanup(cancel) + result, err := clie2e.RunCmd(ctx, clie2e.Request{ Args: []string{ "drive", "+status", @@ -141,6 +142,17 @@ func TestDrive_StatusDryRunAcceptsFilterFlags(t *testing.T) { }) require.NoError(t, err) result.AssertExitCode(t, 0) + + out := result.Stdout + if got := gjson.Get(out, "api.0.method").String(); got != "GET" { + t.Fatalf("method = %q, want GET\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "api.0.url").String(); got != "/open-apis/drive/v1/files" { + t.Fatalf("url = %q, want /open-apis/drive/v1/files\nstdout:\n%s", got, out) + } + if got := gjson.Get(out, "folder_token").String(); got != "fldcnE2E001" { + t.Fatalf("folder_token = %q, want fldcnE2E001\nstdout:\n%s", got, out) + } } // TestDrive_StatusDryRunRejectsAbsoluteLocalDir confirms that the From 8f3c69869c7175a280cc6582805a572075564ae3 Mon Sep 17 00:00:00 2001 From: fangshuyu Date: Mon, 18 May 2026 16:34:01 +0800 Subject: [PATCH 3/3] test(drive): add comprehensive filter unit tests for CI coverage Cover normalizeDrivePattern, normalizeDrivePatterns, parseDriveIgnore error paths, MatchFile ignore/builtin/default branches, MatchDir exclude/ignore/builtin/default branches, driveConcreteDirPrefix, driveMatchPattern, filterDriveRemoteEntries, filterDrivePushLocalView, sortedDriveDirs, filterDriveStatusLocalFiles, filterDrivePullLocalAbsPaths. --- shortcuts/drive/filter_test.go | 374 ++++++++++++++++++++++++++++++++- 1 file changed, 364 insertions(+), 10 deletions(-) diff --git a/shortcuts/drive/filter_test.go b/shortcuts/drive/filter_test.go index 9841e2c0fb..37ecd8aa2d 100644 --- a/shortcuts/drive/filter_test.go +++ b/shortcuts/drive/filter_test.go @@ -9,19 +9,98 @@ import ( ) func TestParseDriveIgnore(t *testing.T) { - rules, err := parseDriveIgnore(strings.NewReader("# comment\n\nnode_modules/\n*.log\n docs/** \n")) - if err != nil { - t.Fatalf("parseDriveIgnore: %v", err) + t.Run("valid rules", func(t *testing.T) { + rules, err := parseDriveIgnore(strings.NewReader("# comment\n\nnode_modules/\n*.log\n docs/** \n")) + if err != nil { + t.Fatalf("parseDriveIgnore: %v", err) + } + want := []string{"node_modules/**", "*.log", "docs/**"} + if len(rules) != len(want) { + t.Fatalf("len(rules) = %d, want %d (%v)", len(rules), len(want), rules) + } + for i := range want { + if rules[i] != want[i] { + t.Fatalf("rules[%d] = %q, want %q", i, rules[i], want[i]) + } + } + }) + t.Run("invalid pattern returns error", func(t *testing.T) { + _, err := parseDriveIgnore(strings.NewReader("[invalid\n")) + if err == nil { + t.Fatal("expected error for invalid glob pattern") + } + }) + t.Run("empty input returns nil", func(t *testing.T) { + rules, err := parseDriveIgnore(strings.NewReader("")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(rules) != 0 { + t.Fatalf("expected nil rules, got %v", rules) + } + }) +} + +func TestNormalizeDrivePattern(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"", ""}, + {" ", ""}, + {"*.log", "*.log"}, + {"docs/**", "docs/**"}, + {"./src/", "src/**"}, + {"/root.txt", "root.txt"}, + {"trailing/", "trailing/**"}, } - want := []string{"node_modules/**", "*.log", "docs/**"} - if len(rules) != len(want) { - t.Fatalf("len(rules) = %d, want %d (%v)", len(rules), len(want), rules) + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + got, err := normalizeDrivePattern(tt.input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("normalizeDrivePattern(%q) = %q, want %q", tt.input, got, tt.want) + } + }) } - for i := range want { - if rules[i] != want[i] { - t.Fatalf("rules[%d] = %q, want %q", i, rules[i], want[i]) + t.Run("invalid glob returns error", func(t *testing.T) { + _, err := normalizeDrivePattern("[invalid") + if err == nil { + t.Fatal("expected error for invalid glob") } - } + }) +} + +func TestNormalizeDrivePatterns(t *testing.T) { + t.Run("invalid pattern returns validation error", func(t *testing.T) { + _, err := normalizeDrivePatterns([]string{"[bad"}, "--include") + if err == nil { + t.Fatal("expected error for invalid pattern") + } + if !strings.Contains(err.Error(), "--include") { + t.Fatalf("error should reference --include, got: %v", err) + } + }) + t.Run("empty patterns returns empty slice", func(t *testing.T) { + out, err := normalizeDrivePatterns([]string{}, "test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(out) != 0 { + t.Fatalf("expected empty, got %v", out) + } + }) + t.Run("whitespace-only entries are skipped", func(t *testing.T) { + out, err := normalizeDrivePatterns([]string{" ", "*.log"}, "test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(out) != 1 || out[0] != "*.log" { + t.Fatalf("expected [*.log], got %v", out) + } + }) } func TestDriveSyncFilterPrecedence(t *testing.T) { @@ -54,6 +133,83 @@ func TestDriveSyncFilterPrecedence(t *testing.T) { } } +func TestDriveSyncFilterMatchFileIgnoreAndBuiltin(t *testing.T) { + t.Run("ignore file excludes when no include", func(t *testing.T) { + filter := &driveSyncFilter{ + ignoreRules: []string{"tmp/**"}, + } + if filter.MatchFile("tmp/cache.dat").Included { + t.Fatal("file matching .larkignore should be excluded") + } + }) + t.Run("builtin excludes when no include", func(t *testing.T) { + filter := &driveSyncFilter{ + builtinExcludes: []string{".git/**"}, + } + if filter.MatchFile(".git/config").Included { + t.Fatal("file matching builtin excludes should be excluded") + } + }) + t.Run("default allow when no rules match", func(t *testing.T) { + filter := &driveSyncFilter{} + if !filter.MatchFile("readme.md").Included { + t.Fatal("file with no matching rules should be included by default") + } + }) + t.Run("empty rel excluded", func(t *testing.T) { + filter := &driveSyncFilter{} + if filter.MatchFile("").Included { + t.Fatal("empty rel should be excluded") + } + if filter.MatchFile(".").Included { + t.Fatal("dot rel should be excluded") + } + }) +} + +func TestDriveSyncFilterMatchDir(t *testing.T) { + t.Run("root dir always included", func(t *testing.T) { + filter := &driveSyncFilter{includes: []string{"docs/**"}} + if !filter.MatchDir("").Included { + t.Fatal("empty dir should be included") + } + if !filter.MatchDir(".").Included { + t.Fatal("dot dir should be included") + } + }) + t.Run("exclude overrides all", func(t *testing.T) { + filter := &driveSyncFilter{ + excludes: []string{"vendor/**"}, + includes: []string{"vendor/**"}, + } + if filter.MatchDir("vendor").Included { + t.Fatal("excluded dir should not be included even with matching include") + } + }) + t.Run("ignore file excludes dir", func(t *testing.T) { + filter := &driveSyncFilter{ + ignoreRules: []string{"tmp/**"}, + } + if filter.MatchDir("tmp").Included { + t.Fatal("dir matching .larkignore should be excluded") + } + }) + t.Run("builtin excludes dir", func(t *testing.T) { + filter := &driveSyncFilter{ + builtinExcludes: []string{".git/**"}, + } + if filter.MatchDir(".git").Included { + t.Fatal("dir matching builtin excludes should be excluded") + } + }) + t.Run("default allow when no rules", func(t *testing.T) { + filter := &driveSyncFilter{} + if !filter.MatchDir("anydir").Included { + t.Fatal("dir with no rules should be included by default") + } + }) +} + func TestDriveSyncFilterMatchDirIncludeAncestor(t *testing.T) { filter := &driveSyncFilter{includes: []string{"docs/**/*.md"}} if !filter.MatchDir("docs").Included { @@ -66,3 +222,201 @@ func TestDriveSyncFilterMatchDirIncludeAncestor(t *testing.T) { t.Fatal("vendor dir should not be included when include patterns only target docs") } } + +func TestDriveConcreteDirPrefix(t *testing.T) { + tests := []struct { + pattern string + want string + }{ + {"docs/**/*.md", "docs/"}, + {"src/lib/*.go", "src/lib/"}, + {"docs/**", "docs/"}, + {"*.log", ""}, + {"**/*.tmp", ""}, + {"a/b/c/*.txt", "a/b/c/"}, + } + for _, tt := range tests { + t.Run(tt.pattern, func(t *testing.T) { + got := driveConcreteDirPrefix(tt.pattern) + if got != tt.want { + t.Fatalf("driveConcreteDirPrefix(%q) = %q, want %q", tt.pattern, got, tt.want) + } + }) + } +} + +func TestDriveMatchPattern(t *testing.T) { + t.Run("empty inputs return false", func(t *testing.T) { + if driveMatchPattern("", "file.txt") { + t.Fatal("empty pattern should not match") + } + if driveMatchPattern("*.txt", "") { + t.Fatal("empty rel should not match") + } + }) + t.Run("glob star suffix exact match", func(t *testing.T) { + if !driveMatchPattern("docs/**", "docs") { + t.Fatal("docs/** should match docs exactly") + } + }) + t.Run("bare name matches at any depth", func(t *testing.T) { + if !driveMatchPattern("*.log", "error.log") { + t.Fatal("*.log should match error.log at root") + } + if !driveMatchPattern("*.log", "sub/error.log") { + t.Fatal("*.log should match error.log in subdirectory") + } + }) + t.Run("full glob match", func(t *testing.T) { + if !driveMatchPattern("docs/**/*.md", "docs/sub/readme.md") { + t.Fatal("docs/**/*.md should match docs/sub/readme.md") + } + }) +} + +func TestFilterDriveRemoteEntries(t *testing.T) { + t.Run("nil filter returns all", func(t *testing.T) { + entries := []driveRemoteEntry{ + {RelPath: "a.txt", Type: driveTypeFile}, + {RelPath: "b.md", Type: driveTypeFile}, + } + got := filterDriveRemoteEntries(entries, nil) + if len(got) != 2 { + t.Fatalf("nil filter should return all entries, got %d", len(got)) + } + }) + t.Run("filters files and dirs", func(t *testing.T) { + filter := &driveSyncFilter{exts: map[string]struct{}{"md": {}}, includes: []string{"docs/**"}} + entries := []driveRemoteEntry{ + {RelPath: "a.txt", Type: driveTypeFile}, + {RelPath: "docs/b.md", Type: driveTypeFile}, + {RelPath: "other", Type: driveTypeFolder}, + } + got := filterDriveRemoteEntries(entries, filter) + if len(got) != 1 { + t.Fatalf("expected 1 entry, got %d", len(got)) + } + if got[0].RelPath != "docs/b.md" { + t.Fatalf("expected docs/b.md, got %s", got[0].RelPath) + } + }) +} + +func TestFilterDrivePushLocalView(t *testing.T) { + t.Run("nil filter returns all with parent dirs", func(t *testing.T) { + files := map[string]drivePushLocalFile{ + "sub/a.txt": {}, + "b.txt": {}, + } + dirs := []string{"other"} + filtered, filteredDirs := filterDrivePushLocalView(files, dirs, nil) + if len(filtered) != 2 { + t.Fatalf("expected 2 files, got %d", len(filtered)) + } + // "sub" should appear as parent dir of "sub/a.txt", plus "other" + foundSub := false + foundOther := false + for _, d := range filteredDirs { + if d == "sub" { + foundSub = true + } + if d == "other" { + foundOther = true + } + } + if !foundSub { + t.Fatal("expected 'sub' in dirs") + } + if !foundOther { + t.Fatal("expected 'other' in dirs") + } + }) + t.Run("filter removes non-matching files and dirs", func(t *testing.T) { + files := map[string]drivePushLocalFile{ + "a.txt": {}, + "b.md": {}, + } + dirs := []string{"sub"} + filter := &driveSyncFilter{exts: map[string]struct{}{"md": {}}, includes: []string{"docs/**"}} + filtered, filteredDirs := filterDrivePushLocalView(files, dirs, filter) + if len(filtered) != 0 { + t.Fatalf("expected 0 files (no docs/ prefix), got %d", len(filtered)) + } + // "sub" dir should be excluded (include only targets docs/) + for _, d := range filteredDirs { + if d == "sub" { + t.Fatal("sub dir should be filtered out") + } + } + }) +} + +func TestSortedDriveDirs(t *testing.T) { + dirs := sortedDriveDirs(map[string]struct{}{ + "a/b": {}, + "a": {}, + "x/y/z": {}, + "x": {}, + }) + // Should be sorted by depth then name + expected := []string{"a", "x", "a/b", "x/y/z"} + if len(dirs) != len(expected) { + t.Fatalf("expected %d dirs, got %d: %v", len(expected), len(dirs), dirs) + } + for i, e := range expected { + if dirs[i] != e { + t.Fatalf("dirs[%d] = %q, want %q", i, dirs[i], e) + } + } +} + +func TestFilterDriveStatusLocalFiles(t *testing.T) { + t.Run("nil filter returns all", func(t *testing.T) { + files := map[string]driveStatusLocalFile{ + "a.txt": {}, + "b.md": {}, + } + got := filterDriveStatusLocalFiles(files, nil) + if len(got) != 2 { + t.Fatalf("nil filter should return all, got %d", len(got)) + } + }) + t.Run("filter by ext", func(t *testing.T) { + files := map[string]driveStatusLocalFile{ + "a.txt": {}, + "b.md": {}, + } + filter := &driveSyncFilter{exts: map[string]struct{}{"md": {}}} + got := filterDriveStatusLocalFiles(files, filter) + if len(got) != 1 { + t.Fatalf("expected 1 file, got %d", len(got)) + } + if _, ok := got["b.md"]; !ok { + t.Fatal("expected b.md in filtered files") + } + }) +} + +func TestFilterDrivePullLocalAbsPaths(t *testing.T) { + t.Run("nil filter returns all", func(t *testing.T) { + paths := []string{"/root/a.txt", "/root/b.md"} + got, err := filterDrivePullLocalAbsPaths("/root", paths, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 2 { + t.Fatalf("nil filter should return all, got %d", len(got)) + } + }) + t.Run("filter by ext", func(t *testing.T) { + paths := []string{"/root/a.txt", "/root/b.md"} + filter := &driveSyncFilter{exts: map[string]struct{}{"md": {}}} + got, err := filterDrivePullLocalAbsPaths("/root", paths, filter) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 1 || got[0] != "/root/b.md" { + t.Fatalf("expected [/root/b.md], got %v", got) + } + }) +}