Add drive +member-remove for removing one collaborator permission from Drive documents, files, folders, wiki nodes, and Miaoda apps. The shortcut validates resource and member contracts before issuing the high-risk DELETE request, preserves structured CLI errors, and documents identity and wiki permission behavior.
Key features:
- Resolve resource type from supported URLs or require it for bare tokens
- Accept Miaoda apps via /page/ URLs or explicit --type=apps
- Support user and bot identities with member-type-specific validation
- Require explicit confirmation and return stable removal metadata
- Reject unsupported slash-containing tokens and member IDs before API calls
- Add unit, dry-run E2E, live workflow, and skill documentation coverage
Extend +download and +preview to accept --url and --wiki-token in
addition to --file-token (mutually exclusive). Wiki nodes are resolved
to their underlying object; only file-backed nodes are supported, and
non-file documents (docx/sheet/bitable/slides) return a typed
validation error hinting to use +export.
Check Drive export permission before file and document media downloads, while preserving whiteboard behavior and adding actionable preview and rate-limit recovery hints.
Spec source: active@ddbecbafcf0d68cc115faf3fcf0214fb6edc30b284147d33e5c9934e7bad82b4
Wrap the Drive file-copy endpoint as drive +copy. Accept a document URL
(recommended) or bare token + --type for the source; the target takes a
folder token, a folder URL, or the my_space constant, which resolves the
caller's My Space root folder via the root-folder-meta endpoint (absent
from platform metadata, works for both user and bot). Repeatable --extra
key=value pairs are forwarded verbatim for special copy semantics (e.g.
target_type=docx to convert a legacy doc during copy). Source and folder
tokens are validated with validate.ResourceName before path
interpolation. Reject wiki URLs/tokens with a typed validation error
whose hint carries a wiki +node-copy command template using a fixed
<node-token> placeholder, because a Drive copy of a wiki-backed document
would land in Drive space instead of the wiki tree. In bot mode the CLI
auto-grants the current CLI user full_access on the new copy (same
behavior as +upload/+import), reporting the outcome in the
permission_grant output field without failing the copy.
Declare docs:document:copy (the narrowest scope in the endpoint's any-of
set) plus a conditional drive:drive.metadata:readonly for my_space
resolution. Cover the shortcut with unit tests, dry-run e2e and a
self-contained live workflow (upload -> copy -> download-verify ->
my_space copy -> cleanup), and register it in
tests/cli_e2e/drive/coverage.md. Route copy intents in the lark-drive
skill to the shortcut instead of the raw files copy service command.
* feat(drive): support Miaoda apps in permission shortcuts
Extend Drive permission shortcuts to accept Miaoda page URLs and the apps resource type while keeping each endpoint's accepted resource contract explicit.
Key features:
- Infer apps from /page/ URLs and accept explicit --type=apps in +apply-permission, +member-add, +member-list, and +permission-get-setting
- Decouple secure-label target parsing so expanding apply-permission does not widen secure-label support
- Align skill guidance and unit/dry-run coverage with the new resource type
* test(drive): cover apps permission target validation
Add focused coverage for Miaoda apps target handling across apply-permission and secure-label boundaries.
Exercise malformed page URLs, explicit apps bare tokens, typed validation errors, and command-level rejection so future resource-type changes cannot silently widen unsupported secure-label behavior.
* fix(drive): parse permission markers from URL paths
Keep drive +apply-permission resource inference aligned with URL component boundaries. Parse and validate URL inputs before extracting tokens so query strings and fragments cannot redirect permission requests to a different resource.
Key fixes:
- Match document and apps markers only against the parsed URL path
- Reject malformed URLs with a typed --token validation error
- Cover /page/ markers found only in query strings or fragments
* docs(skills): redact Miaoda page token example
Replace the concrete Miaoda page token with a representative pagcn placeholder. This keeps the token shape recognizable while avoiding exposure of a real resource identifier in the skill documentation.
* fix(drive): harden permission target resolution
Make Drive shortcut targets unambiguous before they reach read or write API paths. URL inputs now bind to a recognized root path and a single validated token segment, preventing encoded separators, dot segments, and type conflicts from silently changing the addressed resource.
Key fixes:
- Reject non-root URLs, dot/traversal tokens, and URL/type conflicts for secure-label and permission-apply writes
- Keep permission-setting URL parsing and pretty output reversible for every supported command-local resource kind
- Add unit and dry-run E2E regressions plus aligned permission-apply guidance
Add comment-domain shortcuts: +batch-query-comments, +resolve-comment,
+restore-comment, +add-reply, +list-replies, +update-reply, +delete-reply
and +react-reply, sharing one target resolver with per-endpoint file_type
sets.
Flatten the comment reference docs by dropping the comments-guide routing
layer and folding its cross-command knowledge into the command refs:
comment-card model, comment/reply/interaction counting and sorting rules
into lark-drive-list-comments.md; the --solved-status prerequisite into
lark-drive-restore-comment.md; the apps exception into
lark-drive-add-comment.md. Comment intents now route straight from the
drive SKILL.md Shortcuts table to each command ref.
Cover the new shortcuts with unit tests, dry-run e2e and live workflow
e2e behind LARK_DRIVE_MD_COMMENT_E2E=1, and register them in
tests/cli_e2e/drive/coverage.md.
* feat(drive): add +permission-get-setting shortcut
Add a Drive shortcut for reading public permission settings across supported documents, files, folders, and wiki nodes. Resolve URLs into typed resources, preserve permission_public output for machine consumers, and document the shortcut in the permission-governance workflow.
Key features:
- Infer resource type and token from supported Drive URLs while requiring --type for bare tokens
- Query the Drive v2 public permission endpoint with typed validation and user or bot identity
- Support folder permission inspection without recursing into child resources
- Add unit, dry-run E2E, live workflow, output, and skill guidance coverage
* fix(drive): harden permission get setting contract
Harden +permission-get-setting after review findings so callers receive only the documented permission payload and folder support is verified against the live workflow. This prevents malformed responses from being presented as permission settings and keeps the command guidance aligned with the shortcut contract.
Key fixes:
- Reject responses without data.permission_public instead of projecting arbitrary payload fields
- Render complete permission settings in pretty output and mark --token required
- Exercise a created Drive folder in the live workflow and add the command reference
- Correct folder resolution guidance while retaining the shortcut's documented URL forms
* feat/drive-folder-permission-get
* feat(drive): add +member-list shortcut
Add a Drive shortcut for listing collaborators on documents, files, folders, and wiki nodes. Resolve supported resource URLs into typed permission requests, preserve raw API data for machine consumers, and keep invalid flag combinations on typed validation paths.
Key features:
- Infer resource type and token from supported Drive URLs while requiring --type for bare tokens
- Validate optional member fields and wiki-only permission type filters
- Provide pretty output, skill guidance, unit coverage, and dry-run/live E2E workflows
- Read dry-run assertions from the standard data.api success envelope
* feat/drive-member-list
* fix: converge drive delete workflow test on terminal state
* fix: narrow drive delete tolerance to the verified transient
* test: lock the delete failure guard with a subprocess contract test
* test: lock task-result and retry-exhaustion failure boundaries
Skill references written before the typed-error refactor still taught retired envelope shapes. AI agents following them now read what the CLI actually emits:
- permission recovery reads error.missing_scopes instead of the upstream permission_violations detail
- confirmation gates use type=confirmation, subtype=confirmation_required, and flat risk/action fields
- drive duplicate-remote failures are typed validation envelopes (failed_precondition with params[]), not duplicate_remote_path with error.detail
- drive batch partial failures are ok:false results on stdout, not an error.type=partial_failure stderr envelope
- minutes edit-permission and word-replace misses branch on error.subtype, not retired error.type values
- slides replace failures are stderr typed envelopes only; no raw backend response is printed to stdout
- slides command outputs show the ok/identity/data success envelope instead of the raw {code,msg} OpenAPI wrapper
* fix: unify dry-run output contract
* fix: address dry-run review feedback
* fix(dryrun): tighten preview contract and unify data shape
- transcribe HTTP method verbatim in previews (HEAD/OPTIONS were
reported as GET); reject an empty method in api with a typed error
- unify the dry-run data payload across api/service/shortcut paths:
{api, context?: {app_id, user_open_id}}; drop data.as — the envelope
top-level identity is the single identity source
- mark pretty dry-run stdout with '# dry-run: request not sent' so logs
that drop stderr still show it was a preview
- extract the shared preview builder, collapse PrintDryRunWithFile's
loose params into FileUploadMeta, and fail loudly on nil previews
- revert description-marker identity parsing: stale prose must not
override corrected accessTokens (blocks legal user calls on
images.create); identity gating keys off accessTokens only
- pin the new contracts with tests: verbatim method, three-way context
parity, nil-preview error, empty-context omission, marker line
* docs(agents): add typed-data, faithful-transcription, and contract-test conventions
- typed struct at the boundary over map[string]interface{} threading;
distinct types where values could swap silently (internal/meta.Token)
- transcribe input verbatim in previews/transformations; reject
unhonorable flag combinations with typed errors instead of silently
substituting behavior
- contract tests must fail when the implementation is reverted
* test: migrate dry-run tests grown on main to the envelope format
main gained raw-format dry-run readers while the PR was in flight
(wiki drive export #1802, drive list comments #1845, slash commands,
sheets history, docs fetch, mail draft-send/triage, vc meeting events).
Migrate them to the envelope accessors (clie2e.DryRunGet / data-wrapped
decoders) and drop the now-redundant DryRunData extractions in files
unified on DryRunGet.
---------
Co-authored-by: guokexin.02 <264159873+Tantanz20020918@users.noreply.github.com>
* refactor: retire legacy error envelopes and enforce typed contract
Consolidate all command error reporting onto the typed errs.* contract, remove
the legacy error surface that predated it, and tighten the lint guards so the
contract holds across the whole repository going forward.
Every failure now reaches stderr as one envelope shape: a category, an
optional subtype, a human- and agent-readable message, and a recovery hint,
with invalid parameters listed under `params`. The legacy ExitError envelope,
its constructors, and the boundary bridge that promoted untyped config and
authorization errors are deleted, leaving a single path from error to wire.
Predicate commands keep their silent-exit behavior through a dedicated signal
that carries only an exit code.
Infrastructure paths that still emitted ad-hoc envelopes — flag parsing,
unknown commands and subcommands, plugin and policy guards, confirmation
prompts, and auth/config failures — now classify into the same taxonomy.
Business, API, auth, and config exit codes are preserved; the one behavioral
change is that Cobra usage failures (missing required flag, unknown command,
bad arguments) now emit the typed validation envelope and exit 2, matching the
explicit flag and subcommand guards, instead of Cobra's plain-text exit 1.
Enforcement is repo-wide rather than per-path:
- The errscontract guards run by default everywhere instead of through a
migration allowlist, so legacy envelopes cannot be reintroduced anywhere.
- errorlint runs across the whole repository: every error wrap must use %w and
every comparison must use errors.Is/errors.As, so interior wraps stay legal
but can no longer break the chain the typed boundary relies on.
- The errs-no-bare-wrap guard is keyed by structural prefix instead of an
explicit per-domain allowlist, so new shortcut domains are covered without
editing a list. It runs where forbidigo is enabled (the shortcut domains and
the auth/config/service command groups); repo-wide chain integrity for the
remaining command paths is carried by errorlint above.
* test: align cli_e2e success assertions to the ok envelope
The api and service success path now emits the {"ok":true} envelope, so the
cli_e2e workflow assertions that still expected the old {"code":0} shape via
AssertStdoutStatus(t, 0) fail once they run with live credentials. Switch those
workflow assertions to AssertStdoutStatus(t, true); the fake-payload helper test
in core_test.go keeps its code-shape assertion.
Drive-domain errors now leave the CLI as typed, machine-branchable
envelopes — a stable `type` plus `subtype` and named fields (param,
params, retryable, log_id, hint) — so scripts and AI agents can branch on
structure and act on a recovery hint instead of parsing prose.
Changes:
- Every error produced in the drive domain — validation, file I/O, and the
failures returned from its Lark API calls — is emitted as a typed errs.*
error; the exit code is derived from the error category. Drive's API calls
now go through a shared typed classifier, so failures carry subtype,
troubleshooter, a recovery hint, and the request's log_id whether the
server returns it in the response body or the x-tt-logid header; an
already-typed network/auth error is never downgraded into a generic API
error.
- Known API conditions (resource conflict, cross-tenant, cross-brand, ...)
carry a recovery hint keyed by their error class; a command can refine
that hint with command-specific guidance.
- Batch partial failures (+push / +pull / +sync, where some items succeed
and some fail) now report an honest ok:false multi-status result on
stdout — the summary and every per-item outcome stay machine-readable —
and exit non-zero, instead of a misleading ok:true success envelope.
- Duplicate rel_path conflicts report each colliding path as a structured
params entry (RFC 7807 invalid-params style).
- Static guards lock the drive path so legacy error construction — direct
envelopes or the auto-classifying API helpers — cannot be reintroduced,
making drive the template for the remaining domains.
Output changes worth noting for consumers:
- Error envelopes now carry typed type/subtype and named fields; exit
codes follow the error category (malformed or incomplete API responses
are reported as internal errors rather than generic API errors).
- Batch partial failures (+push / +pull / +sync) emit an ok:false result
envelope on stdout (summary + per-item items[]) and exit non-zero; the
per-item results stay on stdout rather than in a stderr error envelope.
Errors surfaced through shared cross-domain helpers (scope precheck, media
import upload, metadata lookup, save-path resolution) are not yet typed;
they migrate with the shared layer in a follow-up change.
Switch `drive +export --file-extension markdown` from the legacy V1
GET /open-apis/docs/v1/content API to the V2
POST /open-apis/docs_ai/v1/documents/{token}/fetch API for
higher-quality Lark-flavored Markdown output.
- Update DryRun and Execute paths to use V2 endpoint with JSON body
- Add docx:document:readonly scope for the new API
- Validate V2 response structure (fail fast on missing document/content)
- Encode token in URL path via validate.EncodePathSegment
- Update unit tests and add V2 response validation error path tests
- Add E2E dry-run test for markdown export path
- Update skill documentation
* feat(drive): add +inspect shortcut for document URL inspection with wiki unwrapping
Implements #662: `lark-cli drive +inspect --url <url>` inspects any
Lark/Feishu document URL to get its type, title, and canonical token,
with automatic wiki URL unwrapping via get_node API.
- Add ParseResourceURL (inverse of BuildResourceURL) in common
- Extract FetchDriveMetaTitle as public shared helper
- Add drive +inspect shortcut with wiki unwrapping support
- Add skill reference docs and update SKILL.md
- Dry-run E2E tests for docx URL, wiki URL, and bare token
* refactor: move host validation from ParseResourceURL to +inspect
ParseResourceURL is a general-purpose URL parser that should not
hardcode domain lists — future Lark domains would silently break.
Move isLarkHost/larkHostSuffixes to drive_inspect.go where host
validation is a business decision of the +inspect command.
Add E2E test for non-Lark host with Lark-like path.
* refactor: remove host validation from +inspect
Lark supports custom enterprise domains, so a hardcoded suffix list
can never be exhaustive and would falsely reject valid URLs.
Path-based matching in ParseResourceURL is sufficient; invalid URLs
will fail naturally at the API call stage.
Bidirectional sync between a local directory and a Drive folder with
diff detection (new_local, new_remote, modified, unchanged) and
conflict resolution strategies (--on-conflict: remote-wins, local-wins,
keep-both, ask).
Key behaviors:
- Type conflict detection: hard-fail when local file vs remote non-file
or local directory vs remote file
- Keep-both: rename local with __lark_<hash> suffix, then pull remote;
occupied map includes localDirs to prevent suffix collision
- Local-wins partial-success: prefer returned file_token on upload failure
- Empty directory mirroring: pre-create local dirs on Drive via
drivePushWalkLocal before scope preflight
- Structured errors throughout (output.Errorf / output.ErrWithHint)
Includes unit tests and E2E tests (dry-run + live workflow).
* 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.
* 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.
* 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.
* feat(drive): add +status shortcut for content-hash diff
Adds `drive +status`, a read-only diff primitive that walks --local-dir,
recursively lists --folder-token, and reports four buckets — new_local,
new_remote, modified, unchanged — by SHA-256 content hash.
Implementation notes:
- Drive's list/metas APIs do not expose a content hash, so files
present on both sides are downloaded via DoAPIStream and hashed in
memory (sha256 + io.Copy, no disk write). Files only on one side are
not fetched. The command stays Risk: "read".
- Only Drive entries with type=file participate. Online docs (docx,
sheet, bitable, mindnote, slides) and shortcuts are skipped — there
is no equivalent local binary to hash against.
- --local-dir is funneled through the framework's
validate.SafeLocalFlagPath helper so that absolute paths and any ..
that escapes cwd are rejected with --local-dir in the error message
(rather than the internal default --file). FileIO().Stat() then
enforces existence and the IsDir check.
- Local walk uses filepath.WalkDir behind a //nolint:forbidigo comment.
The runtime FileIO interface has no walker today and shortcuts can't
import internal/vfs; SafeInputPath has already bounded the walk root
inside cwd, so the bare walk is acceptable until a runtime-level
walker lands.
- Scopes: drive:drive.metadata:readonly (list folders) +
drive:file:download (fetch files for hashing). The broader
drive:drive scope is disabled by enterprise policy in some tenants;
this narrower pair was verified end-to-end.
Tests cover the four-bucket categorization with a nested subfolder and
docx/shortcut filtering, plus validation errors for missing local-dir,
non-directory local-dir, and absolute-path local-dir.
* docs(skills): document drive +status in lark-drive skill
Adds references/lark-drive-status.md covering parameters, output
schema, the type=file scoping rule, and the network-traffic caveat
(hash is streamed in memory, but bytes still cross the wire).
Notes that --local-dir is bounded to cwd by the CLI's path validation,
and that when a user wants to compare a directory outside cwd the
agent should ask the user to relocate or to switch the agent's working
directory rather than `cd`-ing on its own.
Wires +status into the Shortcuts table in SKILL.md.
* test(drive): cover --folder-token validation and add +status dry-run E2E
Addresses two CodeRabbit review comments on PR #692:
- Adds TestDriveStatusRejectsEmptyFolderToken and
TestDriveStatusRejectsMalformedFolderToken so the Validate-stage
required-check and the ResourceName format guard for --folder-token
are exercised, not just --local-dir.
- Adds tests/cli_e2e/drive/drive_status_dryrun_test.go which drives
the real binary in dry-run mode and asserts:
* the request shape (GET /open-apis/drive/v1/files with
folder_token in the dry-run envelope), plus the description text,
* --local-dir absolute paths are rejected by Validate (which still
runs under --dry-run) with --local-dir surfaced in the message,
* cobra's required-flag enforcement rejects a missing
--folder-token before any custom validation.
* fix(drive): walk +status on canonical absolute root to close symlink/.. escape
Reported in PR review: --local-dir was validated through
SafeLocalFlagPath, but the actual walk used the user-supplied raw
string. SafeLocalFlagPath returns the original value (it only checks
the path through SafeInputPath and discards the canonical form), and
SafeInputPath itself relies on filepath.Clean for path normalization.
filepath.Clean shrinks "link/.." to "." purely as string manipulation,
so the validator sees a path inside cwd. The kernel, however, resolves
"link/.." through the symlink target's parent — which is outside cwd
and is what filepath.WalkDir actually traverses.
Fix: in Execute, resolve --local-dir via validate.SafeInputPath to
get the canonical absolute path (this one fully evaluates symlinks
across the entire path), and walk that path. Each absolute walk hit
is converted to a cwd-relative form via filepath.Rel against
validate.SafeInputPath(".") so FileIO.Open's existing SafeInputPath
guard (which rejects absolute paths) still applies.
Adds TestDriveStatusDoesNotEscapeViaSymlinkParentRef as a regression:
it stages an "escape" sibling directory containing a sentinel file,
adds a "link" symlink in cwd pointing into the escape directory, and
runs +status with --local-dir "link/..". Without this fix, the raw
walk visits the sentinel and leaks it into new_local; with the fix,
the walk stays inside the canonical cwd.
A standalone repro confirms the underlying behavior: raw
filepath.WalkDir("link/..", ...) traversed dozens of unrelated files
in the kernel-resolved parent directory; walking the canonical root
visits only the legitimate cwd contents.
* test(drive): pin walker behavior on child / circular symlinks for +status
Adds two corner-case regressions to back up the canonical-root walk fix:
- TestDriveStatusSkipsSymlinkInsideRoot: a child symlink under
--local-dir that points to a sibling temp dir outside cwd. WalkDir's
default policy must report it as a non-regular entry so the callback
skips it, and the sentinel inside the target must not surface in
new_local. This pins the contract our caller relies on (walk
declines to follow child symlinks even when the canonical root
resolves cleanly).
- TestDriveStatusSurvivesCircularSymlinkInsideRoot: a child symlink
pointing back at one of its ancestors. The walk must terminate and
surface the legitimate sibling file; if WalkDir ever followed the
loop, the per-test timeout would catch it.
* fix(drive): close +status review gaps from Codex (pagination, doc, live E2E)
Three independent fixes flagged on PR #692:
1. Route the recursive Drive folder listing through common.PaginationMeta
instead of reading next_page_token directly. The shared helper accepts
both page_token and next_page_token, matching what okr/im already do
and keeping +status safe against a backend field rename. Adds
TestDriveStatusPaginatesRemoteListing, which serves a 2-page response
where page 1 advertises the cursor as next_page_token and page 2 as
page_token; either spelling alone would silently drop one page.
2. The skill doc previously suggested "or symlink the target into cwd"
as a workaround for cwd-relative --local-dir. SafeInputPath calls
filepath.EvalSymlinks before checking isUnderDir(canonicalCwd), so
any symlink whose final target sits outside cwd still gets rejected
as `unsafe file path`. Rewrite the section so agents stop steering
users into a path that always errors out.
3. Add tests/cli_e2e/drive/drive_status_workflow_test.go — the live
E2E that AGENTS.md requires for new shortcuts. Seeds a real Drive
folder with three uploaded files (unchanged.txt, modified.txt,
remote-only.txt), seeds a local tree with matching/diverging
content plus a local-only.txt, runs +status, and asserts each of
the four buckets contains exactly the file we expect with the
right file_token. Cleanup of every uploaded file plus the parent
folder is registered through the existing best-effort cleanup
helpers. Coverage table bumped: drive +status moves to ✓ and the
denominator goes from 28→29 to account for the new shortcut.
Codex also flagged the local-side filepath.WalkDir as a vfs-bypass.
Investigated: the depguard rule shortcuts-no-vfs explicitly forbids
shortcuts from importing internal/vfs (see commit c1b0bed on the
+pull branch where the same migration was rejected by CI). The
filepath.WalkDir + nolint:forbidigo pattern in walkLocalForStatus is
the lint-required convention until FileIO grows a walker, so leaving
it as-is.
zhaojiaxing-coding