Every failure on the authentication, authorization, and configuration
path now surfaces as a typed structured error instead of an ad-hoc
envelope. Users and scripts that consume CLI output get:
- a fixed nine-category taxonomy on the wire, each mapped to a
stable shell exit code (authentication/authorization/config = 3,
network = 4, internal = 5, policy = 6, confirmation = 10)
- identity-aware detail fields (missing_scopes, requested_scopes,
granted_scopes, console_url, log_id, retryable, hint) carried
uniformly on the envelope
- a single canonical policy envelope at exit 6; the legacy
auth_error carve-out is retired
- per-subtype canonical message + hint that preserves Lark's
diagnostic phrasing and routes recovery to the right actor:
app developer (app_scope_not_applied), user (missing_scope,
token_scope_insufficient, user_unauthorized), or tenant admin
(app_unavailable, app_disabled)
- wrong app credentials classify as config/invalid_client whether
surfaced by the Open API endpoint (99991543) or the tenant
access-token mint endpoint (10003 / 10014), instead of
collapsing to a transport error or api/unknown
- local shortcut scope preflight emits the same
authorization/missing_scope envelope (identity + deterministic
missing-scope set) used by the post-call permission path, so AI
consumers read the same structured shape from precheck and from
server-returned permission denial
- streaming download/upload failures keep the same network subtype
split (timeout / TLS / DNS / transport) as the non-stream path
instead of collapsing every cause to a generic transport failure
- console_url is carried only on the bot-perspective
app_scope_not_applied envelope (where the recovery action is
"developer applies the scope at the developer console"); the
user-perspective missing_scope envelope drops the field, since
the only actionable user recovery is `lark-cli auth login --scope`
and pointing an end user at a console they cannot modify is
misleading
- bind workflows (Hermes / OpenClaw / lark-channel) flatten dynamic
Type tags to wire 'config' with the original module name kept
as a metric label
All 10 typed errors are cause-bearing, nil-safe on .Error() and
.Unwrap(), and defensively clone slice setter inputs. Four lint
rules (CheckNilSafeError / CheckBuilderImmutable / CheckUnwrapSymmetry
/ CheckBuildAPIErrorArms) lock these invariants on migrated paths.
Add `lark-cli mail +draft-send` shortcut that takes one or more existing
draft IDs and sends each via POST /drafts/:draft_id/send sequentially.
Per-draft failures are isolated and aggregated into a structured output;
fatal failures (auth, permission, network, mailbox quota) abort the
entire batch immediately while recoverable failures honor --stop-on-error.
Also extend internal/output with six mail-send-specific errno constants
(LarkErrMailboxNotFound=4013, LarkErrMailSendQuota{User,UserExt,TenantExt},
LarkErrMailQuota, LarkErrTenantStorageLimit) consumed by isFatalSendErr.
Risk is "high-risk-write" so the framework's --yes gate applies; the
shortcut declares only the minimal mail:user_mailbox.message:send scope
to avoid asking users for permissions it does not need.
Introduce a typed error contract framework for lark-cli so in-process
Go callers can branch via errors.As(&errs.XxxError{}) and shell scripts,
AI agents, and protocol adapters can branch on stable JSON type/subtype
fields instead of regex-parsing free-form messages.
Adds:
- Canonical taxonomy under errs/ (9 categories + typed Error structs
embedding a shared Problem, RFC 7807-aligned)
- Centralized Lark code metadata + identity-aware BuildAPIError dispatch
- Typed JSON envelope writer alongside the legacy envelope writer
- MCP / OAuth (RFC 6750 Bearer) projection adapters
- Five CI lint guards preventing ad-hoc taxonomy drift
Backward compatibility: legacy *output.ExitError producers (ErrAPI,
ErrWithHint, Errorf, ErrBare) and business shortcuts that use them
continue to render the legacy envelope unchanged. SecurityPolicyError
wire format and exit code are preserved via a carve-out; taxonomy
migration is deferred to PR 2. Domain-specific business migration is
staged across PR 3+.
Framework-direct paths now return typed *errs.*Error: ErrAuth /
ErrValidation / ErrNetwork emit category literals on the wire
(authentication / validation / network), *core.ConfigError is promoted
at the cmd/root boundary with exit code aligned from 2 to 3, and Lark
API permission denials classified by BuildAPIError exit 3.
At the SDK boundary, WrapDoAPIError preserves any already-classified
error (legacy *output.ExitError or typed *errs.*) so output.ErrAuth
from missing credentials surfaces with the auth category and exit 3
intact instead of being downgraded to a network error. Policy responses
classified by BuildAPIError (codes 21000 / 21001) extract challenge_url
and the canonical hint from the response body, matching what the
auth transport already surfaces at the HTTP layer; non-https
challenge URLs are dropped.
First PR in the feat/error-contract-* series.
* feat(apps): replace +html-publish cwd hard-reject with credential-file scan
The previous --path == "." block was a coarse heuristic: it caught the
common foot-gun of publishing a repo root, but also rejected legitimate
clean cwds, and let a ./dist with a forgotten .env ship the secret
through anyway (the sensitive-paths scanner was advisory and never ran
on the Execute path).
Move the gate from path shape to path content:
- Validate now walks --path candidates and rejects publishes that
include well-known credential files (.env / .env.* / .npmrc / .netrc
/ .git-credentials / .aws/credentials / .gcloud/credentials* /
.docker/config.json / .kube/config). Living in Validate (not DryRun)
means dry-run returns non-zero on hit too, so the dry-run preview
matches Execute.
- Narrow the credential pattern set. .git/, SSH private keys, *.pem
and *.key are out of scope -- they're not env-token files and the
false-positive rate (public certs, docs about key formats) is high.
- Add --allow-sensitive as the escape hatch for legitimate cases
(e.g. a docs site shipping .env.example on purpose). DryRun surfaces
the waived list in sensitive_waived so the caller can relay it.
- Drop the cwd defense-in-depth in runHTMLPublish. A clean cwd is now
a valid publish target.
The lark-apps skill and the html-publish reference are updated to
describe the new gate, the override flag, and the patterns now
explicitly out of scope.
* feat(apps): drop .gcloud/* from credential-file scan
The .gcloud/credentials pattern matched a non-existent path: gcloud's
actual config dir is ~/.config/gcloud/ (XDG-based), and the real
credential files there are credentials.db / access_tokens.db /
application_default_credentials.json -- none of which would land under
a .gcloud/ segment in a publish payload.
Drop the rule rather than fix it: the realistic gcloud foot-gun would
require recognizing the .config/gcloud/* tree by file basename, which
is a broader change than the targeted env/cred scan in this PR. The
remaining 7 patterns (.env / .env.* / .npmrc / .netrc /
.git-credentials / .aws/credentials / .docker/config.json /
.kube/config) cover the common Node/Python/CLI-tooling foot-guns.
* fix(apps): close credential-scan bypass when --path is the parent dir itself
isSensitiveRelPath anchors cloud-SDK matchers on adjacent parent/file
segments (.aws/credentials, .docker/config.json, .kube/config), but
walker strips that parent via filepath.Rel when --path is the conventional
parent dir (e.g. ./.aws), yielding a bare RelPath="credentials" that
slipped through silently. Same bypass for the single-file form
--path ./.aws/credentials (walker sets RelPath = Base(rootPath)).
Wrap the scan in isSensitiveCandidate: keep the fast RelPath scan, and
on miss fall back to filepath.Abs(AbsPath) so the parent segment is
visible again. isSensitiveRelPath itself is unchanged; existing tests
still pin its pure-function contract.
* fix(apps): drop filepath.Abs from sensitive scan to satisfy forbidigo lint
The previous fix called filepath.Abs(c.AbsPath) — banned by the repo's
forbidigo rule because shortcuts must not reach into the filesystem for
path resolution.
Reframe the same fix without fs access: re-prepend the root's basename
(or, for the single-file form, the parent dir's basename of rootPath)
to RelPath and re-scan only the parent-anchored credential pairs
(.aws/credentials, .docker/config.json, .kube/config). Leaf matchers
(.env / .npmrc / ...) stay scoped to RelPath — incidentally closing a
latent false-positive where --path /home/alice/.env/dist would have
flagged every file under it just because .env appeared in the
absolute path.
When creating wiki nodes under the same parent concurrently, the API
returns error code 131009 (lock contention) ~5-15% of the time. This
adds automatic retry with exponential backoff (250ms, 500ms; max 2
retries) so callers no longer need to implement retry logic themselves.
- Retry loop in runWikiNodeCreate: only retries on code 131009, respects
context cancellation, prints progress to stderr
- wrapWikiNodeCreateRetryError preserves Err/Raw/Detail.Code in ExitError
- 6 unit tests covering retry success, exhaustion, non-contention error,
single-retry success, context cancellation, no-retry on success
- 8 dry-run E2E tests for wiki +node-create request shape and validation
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.
Introduce three new wiki shortcuts that wrap the corresponding raw APIs
with structured flags, formatted output, my_library alias handling, and
unified envelope shape, replacing the bare `lark-cli wiki spaces list`
/ `wiki nodes list` / `wiki nodes copy` flows for the common cases.
Shortcuts
- wiki +space-list (read, scopes: wiki:space:retrieve):
lists wiki spaces. Default fetches a single page; --page-all walks
every page capped by --page-limit (default 10, 0 = unlimited).
Supports --page-size / --page-token / --format json|pretty|table|csv|ndjson.
Output: {spaces, has_more, page_token} + Meta.Count. Pretty mode
distinguishes "no spaces" from "empty page with has_more" and hints
the caller to resume.
- wiki +node-list (read, scopes: wiki:node:retrieve):
lists nodes in a space or under a parent. Same pagination + format
story as +space-list. Accepts the my_library alias for --space-id
with --as user (resolved via a shared resolveMyLibrarySpaceID helper
extracted from +node-create); rejects my_library upfront for --as bot.
- wiki +node-copy (high-risk-write, scopes: wiki:node:copy):
copies a node into a target space or parent. --target-space-id and
--target-parent-node-token are mutually exclusive. Risk is marked
high-risk-write to match the upstream API's danger: true flag, so the
framework requires --yes. Source is preserved; subtree is copied.
Both list shortcuts pick the narrowest scope the upstream API accepts.
The framework's preflight (internal/auth/scope.go MissingScopes) does
exact-string scope matching, so declaring the broader wiki:wiki:readonly
form would wrongly reject tokens that carry only the per-API scope —
which the API itself accepts — and emit a misleading missing-scope hint.
Shared changes
- shortcuts/wiki/wiki_node_create.go: factor out resolveMyLibrarySpaceID
so +node-list and +node-create share one my_library resolution path.
- shortcuts/wiki/shortcuts.go: register the three new shortcuts.
- skills/lark-wiki/SKILL.md and references/lark-wiki-{space,node-list,
node-copy}.md: documentation for the new shortcuts.
Tooling
- scripts/check-doc-tokens.sh + Makefile gitleaks target:
pre-commit check that scans skill reference docs for realistic-looking
Lark token values without the _EXAMPLE_TOKEN placeholder convention,
preventing gitleaks false positives.
- .gitleaks.toml: allowlist tuning.
- .gitignore: ignore .tmp/.
Tests
- shortcuts/wiki/wiki_list_copy_test.go: unit tests covering registry
membership, declared-narrow-scope pinning, flag validation (page-size
range, page-limit >= 0, target flag exclusivity, my_library + bot
rejection), auto-pagination merging, --page-limit truncation
surfacing next cursor, --page-token single-page mode, empty-slice
serialisation, has_more hint pretty rendering, my_library user-path
resolution, +node-copy copy-to-space / copy-to-parent + body shape,
pretty rendering, and the high-risk-write --yes gate.
- tests/cli_e2e/wiki/wiki_shortcut_workflow_test.go: live end-to-end
workflow exercising the shortcut layer against a real tenant.
Reuses an existing my_library node as a host so the test never adds
to the top-layer quota; the copy is placed under the same host node.
- tests/cli_e2e/wiki/coverage.md: shortcut coverage entries added.
Minor cleanups
- skills/lark-doc/references/lark-doc-search.md and
skills/lark-minutes/references/lark-minutes-search.md: replace
realistic-looking example ou_ tokens with _EXAMPLE_ placeholders so
scripts/check-doc-tokens.sh passes.
Change-Id: I9efb0557f477d369d7f26a09c1e154d4ab15b253
Co-authored-by: liujinkun <liujinkun@bytedance.com>
Add IM flag shortcut commands to lark-cli, enabling users to create, list, and cancel bookmarks on messages and threads via +flag-create, +flag-list, and +flag-cancel.
Change-Id: I8f87f0eadf83fb59b024a3b9fe67b23d363abe0a
AI agents running inside OpenClaw / Hermes were routinely creating a parallel
app via `config init --new` instead of binding to the agent's existing app,
because every "not configured" hint and several deny errors hard-coded
`config init` regardless of workspace. Once bound, the same agents could
silently grant themselves user identity (impersonation) without the user
ever seeing a risk message in chat.
Changes:
- Introduce `core.NotConfiguredError` / `NoActiveProfileError` /
`reconfigureHint` helpers that branch on `CurrentWorkspace()`. In agent
workspaces they point at `lark-cli config bind --help` (a help page, not
a ready-to-run command) so AI must read the binding workflow and confirm
identity preset with the user before acting. In local terminals they
preserve the previous `config init --new` guidance.
- Migrate every `config init` hint that should be workspace-aware:
RequireConfigForProfile, default credential provider, credential provider
fallback, secret-resolve mismatch, config show, strict-mode entry-point
errors, default-as, profile use/rename/remove, auth list, doctor's
config_file check (which now also wraps the OS-level "no such file"
noise into the user-shaped "not configured" message).
- Refuse `config init` when run inside an OpenClaw / Hermes workspace by
default; add `--force-init` for the rare case the user genuinely wants
a parallel app. Without this guard, hint fixes were undone the moment
AI ignored them.
- Rewrite the strict-mode deny errors in cmd/auth/login.go, cmd/prune.go,
and internal/cmdutil/factory.go. The previous "AI agents are strictly
prohibited from modifying this setting" terminated AI reasoning while
providing no real gate. New errors point at `config strict-mode --help`
with the legitimate confirmation flow and explicitly note that switching
does NOT require re-bind. Integration test envelopes updated.
- Tighten `config bind --help` and `config strict-mode --help` to encode
the user-confirmation discipline directly: identity preset semantics
(bot-only vs user-default), "DO NOT switch without explicit user
confirmation", and a cross-reference clarifying that `config bind` is
for changing the underlying app while `config strict-mode` is the
policy-only switch (resolves an ambiguity an audit run found).
- Surface user-identity (impersonation) risk at every config write that
newly grants it, by reusing the canonical IdentityEscalationMessage
string from bind_messages.go:
- `noticeUserDefaultRisk` fires on flag-mode bind landing on
user-default, including the first-time case `warnIdentityEscalation`
misses (it requires a previous bot lock).
- `setStrictMode` warns when transitioning bot → user or bot → off
(newly permits user identity); stays quiet on narrowing changes
and on off → user (off already permitted user).
- Add tests: notconfigured_test.go (workspace branches),
init_guard_test.go (refuse + --force-init bypass), bind_warning_test.go
(user-default warning fires; bot-only does not), strict_mode_warning_test.go
(5 transitions covering both warn and no-warn paths).
Two follow-ups intentionally deferred: the keychain master-key hint at
internal/keychain/keychain.go:42 still suggests `config init` because the
keychain package can't import core (would be circular); fixing requires
either parameterizing the hint via callback or extracting workspace into
its own package. The lark-shared skill doc still tells AI to run
`config init` for first-time setup; updating the skill is in scope for
a follow-up PR.
Change-Id: I02273e044d9e061d211ceaa4f3ed5a3fb28325b3
* 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.
Support minutes +upload to generate a minute from an uploaded media file token.
Change-Id: I59c0719a39541134e395a23262aea7f387105715
Co-authored-by: calendar-assistant <calendar-assistant@users.noreply.github.com>
`im chats link` is registered as a regular service method (no
`risk: high-risk-write` annotation), so the framework does not register
the `--yes` flag on it. Setting `Yes: true` on the e2e Request makes
the runner append `--yes`, which cobra rejects with `unknown flag:
--yes` before the request is ever issued — the rest of the assertions
then fall through with empty stdout.
The flag was added in #633 alongside the risk-tiering rollout that
covered other workflows that genuinely flipped to high-risk-write.
For chats link the API call (creating a chat share link with a
configurable validity period) is not destructive and was never
re-classified, so the line is just leftover from that pass. Drop it
to restore the e2e green; if we ever decide to gate share-link
creation behind confirmation we can re-add it together with the
metadata flip.
Change-Id: Ieb094407a7f0fa18cd130a9d80c7146274b5ecc7
* feat(risk): implement confirmation for high-risk write operations
* feat(risk): streamline confirmation for high-risk write operations
* feat(risk): document approval protocol for high-risk write operations
* feat(risk): refine confirmation protocol for high-risk write operations
* feat(risk): remove redundant variable declaration in risk test
* feat(risk): add 'Yes' flag to various test cases for confirmation
* fix(e2e/wiki): pass obj_type when deleting wiki nodes in cleanup
The wiki node DELETE endpoint now rejects requests without obj_type
(API error 99992402: "obj_type is required"), causing TestWiki_NodeWorkflow
cleanup to fail on every run. Forward the obj_type from the create/copy
response into the delete query params so cleanup succeeds.
* fix(e2e/wiki): delete cleanup wiki nodes via drive v1 endpoint
The wiki v2 DELETE /spaces/{space_id}/nodes/{node_token} endpoint is
undocumented and rejects requests with `obj_type is required` even when
obj_type is forwarded as a query parameter (see actions run #25005966144).
Switch cleanup to the documented path: delete the underlying drive file
via DELETE /drive/v1/files/{obj_token}?type=<obj_type>, which removes the
backing document and the wiki node in one call.
Change-Id: Ieb93b1f92ea758d8b80bcfdd4f20b2be8f35a0bd
* fix(e2e/wiki): pass obj_type to wiki delete in body, not query
Previous attempts:
- query (?obj_type=docx) → API still rejects with 99992402 obj_type
required (the wiki delete-node endpoint reads it from the body, not
the query string).
- drive v1 fallback → bot identity does not have drive write scope and
returns 1061004 forbidden, so we cannot reuse drive's delete API for
the cleanup helpers.
Expose doc_wiki/search v2 under the drive domain via explicit flags
(--query, --edited-since, --commented-since, --opened-since,
--created-since, --mine, --creator-ids, --doc-types, --folder-tokens,
--space-ids, ...) instead of a nested JSON filter, so natural-language
queries from AI agents map 1:1 to discrete flags.
Time handling:
- my_edit_time and my_comment_time are snapped to the hour (floor/ceil)
with a stderr notice, since those fields are aggregated at hour
granularity server-side. create_time passes through as-is.
- open_time has a server-side 3-month cap per request. When
--opened-since / --opened-until span exceeds 90 days, the CLI narrows
the request to the most recent 90-day slice and emits a stderr notice
listing every remaining slice's --opened-* values so the agent can
re-invoke for older ranges. Spans over 365 days are rejected up front
to bound runaway slicing.
Flag ergonomics:
- --doc-types accepts mixed case; values are normalized to upper case
before validation and before being sent to the server.
- --sort default is translated to the server enum DEFAULT_TYPE (every
other sort value upper-cases 1:1).
Error hints:
- Lark code 99992351 (referenced open_id outside the app's contact
visibility) is enriched with a +search-specific hint that
distinguishes API scope from contact visibility and points at
--creator-ids / --sharer-ids as the likely source.
Skill docs:
- new reference at skills/lark-drive/references/lark-drive-search.md,
including the open_time slicing protocol and the paginate-within-
slice-before-switching agent playbook.
- lark-drive/SKILL.md routes resource-discovery to drive +search.
- lark-doc/SKILL.md and lark-doc-search.md mark docs +search as
deprecated and point users at drive +search.
Change-Id: I36d620045809b448446d4fdbdfa923b05794da19
