mirror of
https://github.com/larksuite/cli.git
synced 2026-07-08 18:13:01 +08:00
feat/sidecar-remote-https
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
fangshuyu-768
|
5778adfefa |
fix(drive): preserve parent token on nested overwrite (#908)
* fix(drive): preserve parent token on nested overwrite Ensure drive +push overwrite requests for nested files keep parent_node aligned with the actual remote parent folder and report parent resolution failures explicitly. * test(drive): cover nested overwrite push workflow 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
|
f1aa7d8f42 | feat(drive): add modified-time smart sync mode (#859) | ||
|
fangshuyu-768
|
4ba39ef392 | fix(drive): handle duplicate remote sync paths (#803) | ||
|
fangshuyu-768
|
30ad38d4b6 |
feat(drive): add +pull shortcut for one-way Drive → local mirror (#696)
* feat(drive): add +pull shortcut to mirror a Drive folder onto local Adds `drive +pull`, a one-way Drive → local mirror command. It recursively lists --folder-token, downloads each type=file entry into --local-dir at the matching relative path, and optionally deletes local files absent from the remote (mirror semantics). Implementation notes: - Listing recurses through subfolders with the standard 200-page pagination loop. Online docs (docx, sheet, bitable, mindnote, slides) and shortcuts are skipped since there is no equivalent local binary to write back. Folder tree is reproduced under --local-dir, with parent directories auto-created by FileIO.Save. - Per-file --if-exists=overwrite (default) | skip controls how pre-existing local files are treated; the framework's enum guard rejects any other value. - --delete-local is the only destructive flag and is bound to --yes in Validate: --delete-local without --yes is rejected upfront so no listing or download even runs. --delete-local --yes performs downloads first, then walks --local-dir and removes regular files not present in the remote map. This matches the spec doc's "high-risk-write" intent for --delete-local without making the default pull path require confirmation. - --local-dir is funneled through validate.SafeLocalFlagPath so errors reference --local-dir instead of the framework default --file. FileIO().Stat then enforces existence and IsDir. - Scopes: drive:drive.metadata:readonly + drive:file:download. The broader drive:drive is disabled by enterprise policy in some tenants. - Listing helper (drivePullListRemote) is duplicated locally rather than reused from drive_status.go because that change is still in open PR #692; once it merges, both can be lifted into a shared drive package helper. TODO marker is left in the code. Tests cover six unit scenarios (happy-path with nested subfolder + docx skipping, --if-exists=skip, --delete-local rejection without --yes, --delete-local --yes deletes orphans, absolute-path rejection, bad enum) and four E2E dry-run scenarios (request shape, absolute path rejection, --delete-local --yes guard, missing required flag). * docs(skills): document drive +pull in lark-drive skill Adds references/lark-drive-pull.md covering parameters, output schema (summary + per-item action breakdown), the type=file scoping rule, the --if-exists policy matrix, and the --delete-local + --yes safety contract. Calls out the network-traffic caveat (pull is full-download, unlike +status which only fetches when both sides have the file) and the cwd boundary on --local-dir. Wires +pull into the Shortcuts table in SKILL.md. * fix(drive): walk +pull on canonical absolute root to close symlink/.. escape Same root cause as the +status fix: --local-dir was validated through SafeLocalFlagPath but the walk used the user-supplied raw string. SafeLocalFlagPath returns the original value (the canonical form is discarded), and SafeInputPath itself relies on filepath.Clean for normalization, which shrinks "link/.." to "." purely as string manipulation. The kernel then resolves "link/.." through the symlink target's parent at walk time, putting the traversal outside cwd. For +pull the bug is more dangerous than for +status because it travels through --delete-local --yes — a raw walk would let the delete pass land on files outside cwd. Fix: - In Execute, resolve --local-dir via validate.SafeInputPath to get a canonical absolute path, and resolve "." the same way for cwd. - Convert the resolved root back to a cwd-relative form (filepath.Rel) for download targets so FileIO.Save's existing SafeOutputPath check (which rejects absolute paths) still applies. - For --delete-local, walk the canonical absolute root, then delete via the absolute path. Both values come from the validated safeRoot, so kernel path resolution cannot redirect a delete to a file outside the canonical subtree. - drivePullWalkLocal now returns absolute paths instead of rel paths; the caller computes the rel_path via filepath.Rel against safeRoot for output / remote-set membership checks. Adds TestDrivePullDeleteLocalDoesNotEscapeViaSymlinkParentRef as a regression: it stages an "escape" sibling directory containing a sentinel file, adds a "link" symlink in cwd pointing into it, and runs +pull --delete-local --yes against an empty remote with --local-dir "link/..". The sentinel must survive (proving --delete did not escape) and the in-cwd file must be removed (proving the walk did run). * test(drive): pin walker / download behavior on +pull symlink corner cases Adds three regressions on top of the canonical-root walk fix: - TestDrivePullSkipsSymlinkInsideRoot: a child symlink inside the validated root pointing to a sibling temp dir. Under --delete-local --yes with an empty remote, the sentinel inside the target must survive (walker did not follow the child symlink) and the in-cwd file must be deleted (walker did run). - TestDrivePullSurvivesCircularSymlinkInsideRoot: a child symlink pointing at one of its ancestors. The walk must terminate so the test does not hang on the per-test timeout. - TestDrivePullDownloadDoesNotEscapeViaSymlinkParentRef: pins the download half of the fix. With --local-dir "link/.." the canonical root resolves to cwd, so the remote file must land in cwd, not inside the symlink target's parent. The preexisting sentinel inside the escape directory must remain untouched. * fix(drive): +pull --delete-local must not unlink local files shadowed by online docs CodeRabbit (PR #696) flagged that the --delete-local pass treated any local path missing from `remoteFiles` as orphaned, but `remoteFiles` only records type=file entries. If Drive held a docx/sheet/shortcut at the same rel_path as a local file, the local file would be unlinked even though Drive still owned that path. drivePullListRemote now returns two views: - files: rel_path -> file_token, type=file only (download/skip set) - allPaths: every entry's rel_path regardless of type The download loop continues to consume `files`; the --delete-local pass consults `allPaths`, so an online-doc shadow of a local filename keeps the local file safe. Also routes the local walk and the delete through the vfs abstraction (vfs.ReadDir + vfs.Remove) instead of filepath.WalkDir + os.Remove. This drops the //nolint:forbidigo justifications and lines up with how internal/keychain and internal/registry already do filesystem I/O. The recursive vfs.ReadDir walker preserves the same "do not follow child symlinks" semantics that filepath.WalkDir gave us, so the canonical-root escape protections in 240b772 stay intact. Adds TestDrivePullDeleteLocalPreservesLocalFileShadowedByOnlineDoc as a direct regression: Drive serves keep.txt (file) plus notes.docx (docx), local has both keep.txt and a hand-edited notes.docx; --delete-local --yes must download keep.txt, leave notes.docx untouched, and report deleted_local=0. * fix(drive): count +pull delete failures in summary.failed CodeRabbit (PR #696) flagged that both delete_failed branches in the --delete-local pass appended an item but left the `failed` counter at zero, so the JSON summary could legitimately report `"failed": 0` after a partially-failed mirror. Increment failed in both branches (the filepath.Rel error path and the vfs.Remove error path) so summary.failed reflects every item flagged delete_failed in items[]. Adds TestDrivePullDeleteLocalCountsFailureInSummary, which forces vfs.Remove to fail by chmod-ing the local dir 0o555 right before the run and restoring 0o755 in t.Cleanup so t.TempDir teardown still works. * fix(drive): swap +pull walk/remove back to filepath/os to satisfy depguard The previous fix-up commits used vfs.ReadDir + vfs.Remove inside the +pull shortcut, which depguard's "shortcuts-no-vfs" rule rejects: shortcuts cannot import internal/vfs directly. CI lint failed on the import line. Restore the same pattern used in drive_status.go and the prior +pull walker: - filepath.WalkDir to enumerate files under the canonical absolute root, gated by //nolint:forbidigo with a comment explaining why. - os.Remove for the actual delete, also gated by //nolint:forbidigo. The canonical-root safety still holds: validate.SafeInputPath bounds the walk root inside cwd before WalkDir runs, and WalkDir's default "do not follow child symlinks" policy is preserved. The two earlier fixes (drivePullListRemote returning allPaths so online-doc shadows do not look orphaned, and incrementing failed on delete_failed) stay in place. `go test ./shortcuts/drive/...` and `golangci-lint run --new-from-rev=origin/main` are both clean. * fix(drive): record remote folder rel_path in +pull allPaths Follow-up to 45fe4e3. The folder branch in drivePullListRemote merged descendant rel_paths into allPaths but never recorded the folder's own rel_path, so a local regular file with the same name as a remote folder still looked orphaned and got unlinked under --delete-local. Adds the missing allPaths[rel] for the folder case and a regression: TestDrivePullDeleteLocalPreservesLocalFileShadowedByRemoteFolder stages a Drive containing a folder named shadow alongside a downloadable file, with the local side holding a regular file named shadow; --delete-local --yes must download keep.txt and leave the shadow file untouched. * fix(drive): +pull pagination + dir/file conflict + skill doc symlink claim Codex review on PR #696 surfaced three issues; addressed in one go: 1. drivePullListRemote only honored next_page_token. The shared common.PaginationMeta helper accepts both page_token and next_page_token; switched +pull over so a backend reply using page_token no longer makes the lister stop at page 1 (which would silently drop later remote files from both download and --delete-local). 2. --if-exists=skip swallowed mirror conflicts. The skip/overwrite branch only checked Stat success, so a local directory shadowing a remote regular file was reported as action=skipped. Now Stat's IsDir() is checked first; the conflict surfaces as action=failed with a message naming the directory, under both --if-exists=skip and --if-exists=overwrite, and increments summary.failed. 3. Skill doc told callers to soft-link the target into cwd if they wanted to pull from outside cwd. That is wrong: SafeInputPath evaluates symlinks before the cwd check, so a symlink pointing out-of-tree is rejected. Replaced the bogus shortcut with the actually viable options (switch the agent working directory, physically move/copy the target, or skip the comparison). Two new regressions: - TestDrivePullSurfacesDirectoryFileMirrorConflict — table test over both policies asserting failed=1, no skipped, action=failed, plus the 'is a directory' hint in the error message. - TestDrivePullPaginationHandlesPageTokenField — first page returns page_token (not next_page_token) with has_more=true; asserts both pages are fetched and both files land on disk. * fix(drive): +pull exits non-zero on item failures; gate --delete-local Two PR-696 review fixes: - Item-level failures (download error, dir/file conflict, delete error) now surface as a structured partial_failure ExitError instead of a success envelope with summary.failed > 0. Exit code becomes non-zero and error.detail still carries the {summary, items[]} payload, so AI / script callers can detect the failure via the exit code without reaching into the JSON body. - A failed download pass now skips the --delete-local walk entirely. Previously +pull would continue removing local-only files even when the download phase had partially failed, leaving the mirror in a half-synced state (some Drive files missing locally AND some local-only files unlinked). Re-runs after fixing the download error recover cleanly. Skill doc / shortcut description / flag desc updated to call the operation a one-way file-level mirror, since --delete-local only unlinks regular files and does not prune empty local directories left behind by remote folder deletes (true directory-level mirroring is explicitly out of scope). Tests: existing dir/file-conflict and delete-failure cases now assert the partial_failure ExitError shape; new test covers the "download fails => --delete-local skipped" gating contract. * refactor(drive): consolidate folder-listing helpers into listRemoteFolder Closes the post-#692 / post-#709 TODO that lived in drive_pull.go (and the matching note in drive_push.go): both #692 and #709 are now on main, so the three near-identical recursive Drive folder listers can collapse into one. New shared helper in shortcuts/drive/list_remote.go: driveRemoteEntry { FileToken, Type, RelPath } listRemoteFolder(ctx, runtime, folderToken, relBase) -> map[rel]entry Returns one entry per Drive item (every type), keyed by rel_path. Subfolders are descended into and the folder's own entry is recorded so callers can reason about "this rel_path is occupied by a folder" without re-listing. Pagination via common.PaginationMeta is unchanged. Each shortcut now derives its own per-shortcut view from the unified listing: - drive_status.go: collapses to remoteFiles (Type=="file" -> token) for the content-hash diff. - drive_pull.go: derives remoteFiles (Type=="file") for the download set, plus remotePaths (every rel_path) as the --delete-local guard. - drive_push.go: derives remoteFiles (Type=="file") for upload / overwrite / orphan-delete, plus remoteFolders (Type=="folder") for the create_folder cache. drivePushRemoteEntry was a duplicate of driveRemoteEntry's first two fields and is dropped; the few call sites that read .FileToken keep working unchanged. Per-shortcut copies removed: - drive_status.go: listRemoteForStatus, joinRelStatus, driveStatusListPageSize/FileType/FolderType - drive_pull.go: drivePullListRemote, drivePullJoinRel, drivePullListPageSize/FileType/FolderType - drive_push.go: drivePushListRemote, drivePushJoinRel, drivePushListPageSize/FileType/FolderType, drivePushRemoteEntry drive_push_test.go's TestDrivePushHelpersRelPath is retargeted at the shared joinRelDrive; the docstrings on the same-name-conflict tests were tweaked to refer to "the remoteFiles view" instead of the just-removed drivePushListRemote. Net diff: +1 new file, -207 net lines across the four touched files. All existing unit + e2e dry-run tests pass without behavioral change; the rel_path / pagination / type-filter contracts each shortcut depends on are preserved by construction. |
||
|
fangshuyu-768
|
4d68e09537 |
feat(drive): add +push shortcut for one-way local → Drive mirror (#709)
* feat(drive): add +push shortcut for one-way local → Drive mirror
Mirrors a local directory onto a Drive folder: walks --local-dir,
recursively lists --folder-token, mirrors local subdirectory structure
(including empty dirs) onto Drive via create_folder, and for each
rel_path uploads new files, overwrites already-present files, or skips
them per --if-exists. With --delete-remote --yes, any Drive type=file
entry absent locally is removed; Lark native cloud docs (docx/sheet/
bitable/mindnote/slides) and shortcuts are never overwritten or deleted.
Overwrite hits POST /open-apis/drive/v1/files/upload_all with the
existing file_token in the form body and the response's `version` is
propagated to items[].version, mirroring the markdown +overwrite
contract. Files >20MB fall back to the 3-step
upload_prepare/upload_part/upload_finish path with a single shared fd
reused via io.NewSectionReader per block.
Output is a {summary, items[]} envelope; items[].action is one of
uploaded / overwritten / skipped / folder_created / deleted_remote /
failed / delete_failed.
--delete-remote is bound to --yes upfront in Validate, same pattern as
+pull's --delete-local: a stray flag never silently deletes anything.
Path safety reuses the canonical-root walk + SafeInputPath mechanics
from the sibling +status / +pull commands.
Scopes: drive:drive.metadata:readonly + drive:file:upload +
space:folder:create. space:document:delete is intentionally NOT in the
default set — the framework's pre-flight scope check would otherwise
block plain pushes and dry-runs for callers that haven't granted delete;
--delete-remote --yes relies on the runtime DELETE call to surface
missing_scope. The skill ref calls out the scope so users running
mirror sync can grant it upfront.
13 unit tests cover the upload/overwrite/skip/delete matrix, online-doc
protection, same-name conflict between local file and native cloud doc,
empty-directory mirroring, multipart, scope/path validation, and helper
correctness. 4 dry-run e2e tests pin the request shape.
* fix(drive +push): address review — failure semantics, default skip, scope pre-check, mirror wording
- Item-level failures now bump the exit code via output.ErrBare(ExitAPI)
while keeping the structured items[] envelope on stdout. The
--delete-remote phase is skipped entirely when any upload / overwrite /
folder step fails, so a partial upload never proceeds to delete remote
orphans (a half-synced state).
- Default --if-exists flipped from "overwrite" to the safer "skip": the
upload_all overwrite-version protocol field is still rolling out, so
the default no longer fails a first push against a pre-populated
folder. Callers must opt into "overwrite" explicitly.
- --delete-remote --yes now triggers a conditional space:document:delete
scope pre-check in Validate via the new RuntimeContext.EnsureScopes
helper, so a missing grant fails the run before any upload — instead
of after the upload phase, which would leave orphans uncleaned.
- Description, Tips and skill doc rewritten to call this a file-level
mirror (not a directory mirror): the command does not remove
remote-only directories or close gaps in directory structure that
exists only on Drive.
Tests:
- new TestDrivePushDefaultsToSkipForExistingRemote pins the new default
- new TestDrivePushSkipsDeleteAfterUploadFailure pins the half-sync
guard and the non-zero exit on item-level failure
- new TestDrivePushExitsZeroOnCleanRun pins the inverse
- existing tests that relied on the old overwrite default now opt in
explicitly with --if-exists=overwrite
- TestDrivePushOverwriteWithoutVersionFails updated to assert
*output.ExitError with Code=ExitAPI
- new TestDrive_PushDryRunAcceptsDeleteRemoteWithYes (e2e) symmetric to
the existing reject-without-yes test, pinning that EnsureScopes is a
silent no-op when the resolver has no scope metadata
* fix(drive +push): close remaining CodeRabbit comments
Three small follow-ups on the +push review thread that were still
open after the earlier failure-semantics / default-skip / scope
pre-check fix:
- drivePushUploadAll now extracts data.file_token before checking
larkCode, and surfaces the returned token on the partial-success
path (non-zero code + non-empty file_token). Without this, a backend
response where bytes already landed but code != 0 would force the
caller to fall back to entry.FileToken and silently lose the actual
Drive token, defeating the overwrite-error token-stability handling
in Execute.
- TestDrivePushOverwriteWithoutVersionFails switched from "tok_keep"
to "tok_keep_new" in the upload_all stub and now asserts that the
returned token (not entry.FileToken) lands in items[].file_token —
pins the contract that a regression to the fallback branch would
otherwise pass silently.
- New TestDrivePushOverwritePartialSuccessSurfacesReturnedToken pins
the new partial-success branch end-to-end.
- drive_push_dryrun_test.go: tightened the three Validate / cobra
rejections from `exit != 0` to exact codes — `exit == 2` for the
two Validate-stage rejections (--local-dir absolute,
--delete-remote without --yes), `exit == 1` for the cobra
required-flag check (--folder-token missing). Locks in failure
classification so a regression that misroutes the error layer
doesn't slip through.
|