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
* feat(mail): add +share-to-chat shortcut to share emails as IM cards
Two-step API (create share token → send card) wrapped in a single
shortcut. Supports message-id/thread-id, five receive-id-type variants
(chat_id, open_id, user_id, union_id, email), and dry-run mode.
Change-Id: Ic7b8c01c0d25fef262f35be92555f1fd019bd679
Co-Authored-By: AI
* fix(mail): regenerate SKILL.md from skill-template instead of manual edit
Add missing safety rule 8 (draft link rule) to skill-template/domains/mail.md
so it survives regeneration. SKILL.md is now produced by `make gen-skills`
in the registry repo rather than hand-edited.
Change-Id: I9cf3605deae8b6de2042e40819fedc304967e78e
Co-Authored-By: AI
* fix(mail): add docstrings and use real validation path in tests
- Add Go doc comments to exported symbols for docstring coverage
- Rewrite tests to exercise MailShareToChat.Validate via RuntimeContext
instead of duplicating validation logic
- Replace hand-rolled containsStr with strings.Contains
- Add httpmock stubs for execute and error path tests
Change-Id: Ic781494f61e9e844224185844bce7b0c48e8e200
Co-Authored-By: AI
* test(mail): add dry-run E2E test for +share-to-chat
Validate request shape (method, URL, mailbox path) under --dry-run
with fake credentials. Covers message-id, thread-id, and custom
mailbox variants.
Change-Id: Iae87bf141cbe4f312d3e9b1fca4ba175052c5c35
Co-Authored-By: AI
* fix(mail): include request body and params in dry-run output
DryRun now mirrors Execute: the share-token POST shows message_id or
thread_id, and the send POST shows receive_id_type and receive_id.
E2E test updated to assert these fields. Also fix strconv.Itoa usage.
Change-Id: I00f8770fd5a12b7354986c5e5077f97cfe5d6653
* style(mail): gofmt dry-run test file
Change-Id: I47dc6a9a47252dcfb7853737f88dfdaef65a0ae7
* test(mail): assert exact API call count in dry-run test
Change-Id: I9f4a1a183b55d03f5248eb4adddfddb08037ca95
Give each AI Agent (OpenClaw, Hermes) its own lark-cli workspace so
its Feishu calls don't overwrite the developer's local config or
collide with other Agents.
lark-cli config bind [--source openclaw|hermes] [--app-id <id>]
[--identity bot-only|user-default] [--force]
Key capabilities:
- Source auto-detected from OPENCLAW_* / HERMES_* env signals; config
written to ~/.lark-cli/<agent>/, isolated per Agent.
- Two identity presets: 'bot-only' (flag-mode default) and
'user-default'. Flag mode rejects silent bot→user escalation
without --force; TUI prompts are exempt.
- Agent-friendly stdout JSON with 'identity' + 'message' for
next-step branching.
- 'config show' and 'doctor' expose the bound 'workspace'.
- OpenClaw SecretRef resolution: plain / ${VAR} / file:+JSON Pointer
/ exec:.
Wrap the POST /drive/v1/permissions/:token/members/apply endpoint as a
user-only shortcut. --token accepts either a bare token or a document
URL, with type auto-inferred from the URL path (/docx/, /sheets/,
/base/, /bitable/, /file/, /wiki/, /doc/, /mindnote/, /minutes/,
/slides/); an explicit --type always wins. --perm is limited to view or
edit; full_access is rejected client-side to match the spec.
Classifier gains two domain-specific hints for the endpoint's newly
documented error codes: 1063006 (per-user-per-document quota of 5/day
reached) and 1063007 (document does not accept apply requests — covers
disallow-external-apply, already-has-access, and unsupported-type).
test(drive): add dry-run E2E for +apply-permission
Invoke the real CLI binary via clie2e.RunCmd under --dry-run and
parse the rendered request JSON with gjson to lock in method, URL
path (including the token segment), type query parameter (auto-inferred
for docx / sheet / slides URLs, taken from explicit --type for bare
tokens), perm body field, and remark presence/omission. A separate
test asserts --perm full_access is rejected by the enum validator
before reaching the server. Fake LARKSUITE_CLI_APP_ID / APP_SECRET /
BRAND are enough because dry-run short-circuits before any API call.
Update drive coverage.md to add a row and refresh metrics.
test(drive): isolate E2E dry-run subprocess from local CLI config
Set LARKSUITE_CLI_CONFIG_DIR to t.TempDir() in both +apply-permission
dry-run tests so the subprocess can't read a developer's real
credentials/profile instead of the fake env vars the tests inject.
test(drive): add E2E case that exercises URL inference override
Previous "bare token with explicit type wins over inference" row used a
bare token, which has no URL-derived type to override. Replace it with
a /docx/ URL + --type wiki combo that actually forces the explicit flag
to win over URL inference, and add a separate bare-token row to keep
the simpler path covered. Refresh coverage.md wording to match.
* feat(doc): add pre-write semantic warnings to docs +update
Two static checks run before the MCP update-doc call:
1. replace_* + blank-line markdown: replace_range / replace_all only
swap text inside an existing block — a \n\n in the payload will
render as literal text, not a paragraph break. Hint to use
delete_range + insert_before instead.
2. Combined bold+italic emphases (***text***, **_text_**, _**text**_)
cannot round-trip through Lark and are silently downgraded to a
single emphasis. Hint to split into two separate emphases.
Both warnings go to stderr and never block the update — they inform,
not gate. Adds table-driven tests for each check plus an aggregation
test, and wires the checks into Execute right before CallMCPTool.
Closes the first batch of items from the docs +update pitfalls
review (Cases 1 and 5).
* fix(doc): exclude code regions and escaped markers from docs +update checks (#578)
* fix(doc): exclude code regions and escaped markers from docs +update checks
Addresses the three review comments on #569: the blank-line paragraph
check and the bold+italic emphasis check both operate on the raw
markdown string, so fenced code blocks / inline code spans / literal
escaped markers produce false-positive warnings on content users
expect to pass through verbatim.
Changes:
- Add proseHasBlankLine(): fence-aware detector that returns true only
when a blank line sits outside of ```...``` or ~~~...~~~ regions.
Replaces the raw strings.Contains("\n\n") check in
checkDocsUpdateReplaceMultilineMarkdown.
- Add stripMarkdownCodeRegions(): blanks out fenced code lines and
masks inline code spans (via scanInlineCodeSpans from markdown_fix.go)
with equal-length whitespace so byte offsets outside the stripped
regions are preserved.
- Add stripEscapedEmphasisMarkers(): removes "\*" and "\_" so literal
sequences like "\***text***" — which CommonMark renders as a literal
asterisk plus bold — don't match the combined bold+italic regex.
- Wire both helpers into checkDocsUpdateBoldItalic(): the regex now runs
on stripEscapedEmphasisMarkers(stripMarkdownCodeRegions(markdown)),
so code samples and escaped markers are sanitized away before
detection.
Shared fence-parsing helpers (codeFenceOpenMarker, isCodeFenceClose,
leadingRun) are kept local to this file to avoid touching files outside
the scope of the reviewed PR. If a future change wants to reuse them
across the doc package, they can be promoted then.
Tests:
- TestCheckDocsUpdateReplaceMultilineMarkdown: add 4 negative/positive
cases — blank line inside backtick and tilde fences (no flag), blank
line in prose while fence also has blanks (flag wins), fenced code
with no blank lines (no flag).
- TestCheckDocsUpdateBoldItalic: add 9 cases — ***text*** / **_text_** /
_**text**_ inside fenced code (backtick and tilde), inside inline
code spans, and escaped \***text*** / \*\*_text_\*\* (none flagged);
plus two positive cases to verify the strip doesn't over-sanitize
(real emphasis in prose still fires when inline/fenced code is nearby).
* fix(doc): close CommonMark gaps and add three more combined-emphasis shapes
Self-review of the first commit turned up three issues:
- isCodeFenceClose was strict on exact marker length. Per CommonMark
§4.5, a closing fence must be at least as long as the opener, not
exactly the same length. A 3-backtick open legitimately closed by a
4-backtick closer (used to embed triple-backticks inside the code
sample) was left open-ended, causing the rest of the document to be
treated as code and both checks to silently skip it.
- Both fence helpers accepted any amount of leading whitespace because
they ran on strings.TrimSpace(line). CommonMark allows 0..3 leading
spaces before a fence marker; 4+ spaces (or any tab in leading
position, which expands to 4 columns) makes the line indented code
block content, not a fence open/close. Indented fence-like lines now
correctly remain prose and blank lines around them are detected.
- The bold/italic check only covered three of the six documented
combined-emphasis shapes. Added ___text___, __*text*__, and
*__text__* so parity with the asterisk variants is complete. The
regex set is now table-driven (combinedEmphasisPatterns) to make
adding future shapes a one-line change.
Implementation changes:
- New fenceIndentOK(line) helper: returns (body, true) for 0..3 leading
spaces with no tabs, else (_, false). Used by both codeFenceOpenMarker
and isCodeFenceClose.
- isCodeFenceClose now counts the fence-char run and accepts any run
length >= len(marker), with trailing whitespace only.
- checkDocsUpdateBoldItalic replaced three named var regexes with a
table of six {shape, re} entries and a single early-exit loop.
- Updated docsUpdateWarnings top docstring to list all six shapes.
- Noted the known limitation of stripEscapedEmphasisMarkers around
doubled backslash escapes ("\\***text***"), which is a false negative
we accept in exchange for keeping this a simple string replace.
Test additions (docs_update_check_test.go):
- Fence close: longer-marker close correctly ends fence; real prose
blank after a longer-close fence is still detected.
- Indentation: 4-space indented fence-like line is not a fence open,
so a surrounding blank line still flags; tab-indented variant same;
3-space indented fence is still a real fence.
- New shapes: ___text___ positive + all three negative-guards (fenced
code, inline code, escaped); __*text*__ and *__text__* positive +
fenced/inline negative-guards; plus two composition tests to ensure
the strip does not over-sanitize across the six-regex alternative set.
All 53 sub-tests in this file pass; go vet and gofmt are clean.
---------
Co-authored-by: fangshuyu-768 <shuyufang768@outlook.com>
* fix(doc): address CodeRabbit review on docs +update warnings (#581)
Two CodeRabbit nits from #569:
1. Unit test hint assertion only checked for `delete_range` in the
remediation message; the companion `insert_before` half of the
guidance could regress undetected. Broaden the assertion to require
both tokens so a future edit that drops half the remediation
produces an immediate test failure.
2. No E2E coverage proved the dry-run contract in the PR description
("Not emitted in dry-run mode — kept quiet during planning"). The
helper itself is unit-tested, but nothing caught a regression where
a later refactor wired docsUpdateWarnings into the DryRun path.
Add tests/cli_e2e/docs/docs_update_dryrun_test.go:
TestDocs_UpdateDryRunSuppressesSemanticWarnings invokes
`docs +update --dry-run --mode=replace_range --markdown "***x***\n\nb"`
— an input crafted to trip BOTH pre-write warnings — and asserts
neither the "warning:" prefix, the blank-line message, nor the
combined-emphasis message appears on stdout or stderr.
Note: the file needs -f to add because .gitignore has a bare
`docs/` rule that accidentally matches tests/cli_e2e/docs/. The
existing tracked files under that directory predate the rule; new
additions have to be force-added until the ignore pattern is
narrowed. Not worth rewriting .gitignore for one file.
Verified manually that the new E2E fails cleanly when warnings are
injected into DryRun and passes again after reverting — the test has
real regression-detection power, not just a sticker.
Co-authored-by: fangshuyu-768 <shuyufang768@outlook.com>
* fix(api): add stdin and single-quote support for --params/--data on Windows (#64)
Windows PowerShell 5.x mangles JSON double-quotes when passing arguments
to native executables, causing --params and --data to fail with
"invalid JSON format". This commit adds two mitigations at the framework
level:
- stdin piping: `echo '{"k":"v"}' | lark-cli --params -` bypasses
shell argument parsing entirely and works on all platforms/shells.
- single-quote stripping: cmd.exe passes literal single quotes which
are now transparently removed before JSON parsing.
Implementation:
- New `cmdutil.ResolveInput(raw, stdin)` handles `-` (stdin), strip
surrounding `'...'`, and plain passthrough.
- `ParseJSONMap` and `ParseOptionalBody` now accept an `io.Reader` and
delegate to `ResolveInput` before JSON unmarshalling.
- `cmd/api` and `cmd/service` pass `IOStreams.In` and guard against
simultaneous stdin usage by --params and --data.
- Empty stdin is rejected with a clear error message.
Closes#64
Change-Id: If21e735d0aed5c6a2d6674c1e6c898186fca3aba
* test: add stdin e2e regression coverage
Change-Id: I4e00bf1c6b6f3259f503e3414cae10fa4b34ba75
* feat: cli e2e test framework and demo
* feat: add cli-e2e-testcase-writer skill and task case
* feat: add cli e2e config and fix test resource prefix
