Close the last legacy/typed capability gaps so migration never downgrades:
- []string fields: registerLeaf/bindLeaf gain a Slice case. Default maps to
cobra StringSlice (comma-separated, repeatable); `split:"none"` opts into
StringArray (repeatable, no comma split). Non-[]string slices and split on
non-slice fields error at Mount time. bucketLeafValue handles slices in
OneOf/group too.
- per-flag hidden: `hidden:"true"` tag → fieldSpec.Hidden → MarkHidden;
typed help skips hidden flags (parity with legacy Flag.Hidden).
- @file/stdin discoverability: typed help now appends a (supports @file /
- for stdin) hint for input-tagged flags, matching legacy.
Adds binder_slice_hidden_test.go (11 cases).
Bring TypedShortcut[T] to parity with legacy common.Shortcut on two flag
capabilities that were silently missing on the typed path:
- @file / stdin: declare `input:"file,stdin"` on an Args field; the binder
resolves @path (file) and - (stdin) before binding, recursing into OneOf
buckets and groups. resolveInputForFlag is extracted from the legacy
resolveInputFlags so both paths behave identically.
- enum: registerLeaf registers shell completion and typed help renders the
candidate list, matching legacy. enum/input tags on non-string fields now
error at Mount time instead of being silently skipped at runtime.
Legacy behavior unchanged. Adds binder_input_enum_test.go (11 cases).
UserOpenIDList ([]string) had zero production callers and cannot be bound
as a typed flag field: the binder only handles string/bool/int scalars, so
a []string field panics at bind time (reflect.Value.Convert: string cannot
convert to []string). Remove the type, its ParseUserOpenIDList helper and
the test instead of special-casing slice binding for a type nobody uses.
Maybe[T] solved a real problem (distinguishing "user did not provide this
flag" from "user provided the type's zero value, e.g. --notify=false or
--limit 0"), but it added a framework-specific wrapper that every migrator
had to learn. Two independent dogfood migrations both flagged it as the
guide's biggest cognitive bump.
This change retires Maybe[T] and reuses the existing OneOf pointer
convention at the top level: *T now means "optional with tri-state
semantics — nil iff the user did not set the flag". The rule is the same
whether the *T sits inside a OneOf bucket (Chat *ChatID — variant not
selected) or at the top level of an Args struct (Notify *bool — flag not
given), so users learn one concept instead of two.
Code:
- binder.go: bindLeaf now gates pointer allocation behind
cmd.Flags().Changed(name); previously a *T top-level pointer was always
allocated, defeating the "nil = not given" semantic. fieldSpec loses the
IsMaybe field; parseFieldSpec drops Maybe-shape detection; registerFlags
/ bindFlags drop their IsMaybe branches; the bindMaybe helper is deleted.
- protocol.go: Maybe[T] type removed.
- protocol_test.go: TestMaybe_UnsetVsZero removed.
- binder_coverage_test.go: bindMaybe tests replaced with two
TestBindLeaf_Ptr* tests covering the new "*T top-level pointer" contract
— nil when absent, non-nil with the user's value (including the tri-state
case --notify=false) when explicitly set.
Verified: go build ./..., go test ./shortcuts/common/..., gofmt clean, and
`go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./...` vs origin/main
shows zero new dead code.
Three fixes surfaced by a dogfood migration of an existing legacy shortcut
to the TypedShortcut framework.
1. Required flags rendered under OPTIONAL
The typed help renderer had no notion of required vs optional and lumped
every top-level leaf into a single OPTIONAL section. Add renderRequiredSection
that lists fields whose `required` tag is set under a REQUIRED: header; the
updated renderOptionalSection skips those so each leaf appears in exactly
one section.
2. Default values silently dropped
Fields tagged `default:"x"` showed no `(default "x")` suffix, unlike cobra's
legacy default help. Add a formatLeafLine helper that appends `(default "x")`
whenever fieldSpec.DefaultValue is non-empty; the REQUIRED, OPTIONAL, and
CHOOSE ONE bucket renderers all reuse it for consistent information density.
3. TypedShortcut.Mount was missing
Legacy common.Shortcut exposes BOTH .Mount(parent, factory) and
.MountWithContext. The framework dropped .Mount in 8d8acb82 to satisfy
deadcode, but a dogfood migration revealed the asymmetry forces every
migrating shortcut's tests to also switch to MountWithContext. Restore the
3-line Mount convenience method (delegates to MountWithContext with a
background context) and add a unit test exercising it directly — the test
doubles as API documentation and keeps deadcode happy.
Tests added:
- TestTypedShortcut_Mount: verifies the legacy-shaped Mount(parent, factory)
call still wires the subcommand
- TestTypedHelp_RequiredSectionAndDefaults: asserts REQUIRED comes before
OPTIONAL, --limit lands in REQUIRED not OPTIONAL, `(default "20")` appears
for --page-size, and a plain --verbose carries no default suffix
Verified locally: go build ./..., go test ./shortcuts/common/..., gofmt -l,
and `go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./...` vs
origin/main — all pass with zero new dead code.
CI fast-gate's `gofmt -l .` flagged an alignment issue in contentBucket:
gofmt aligns struct field tags to the longest *named* tag-bearing field
in the group; my hand-written extra spaces in `Text *string \`flag:"ct"\``
got normalised to a single space.
Behaviour-neutral; tests still pass.
The previous checkOneOf required group / bucket variants inside a OneOf
to mark exactly one inner field with `oneof_trigger:"true"`, so the
framework could identify "the flag that signals the variant was selected".
This forces the Args writer to think about an implementation detail and
produces a misleading shortcut_oneof_missing when a user supplies only a
companion field (e.g. --video-cover without --video) — the user actually
attempted that variant, but the framework couldn't see it.
Replace the explicit trigger with implicit detection: any inner flag of a
group / bucket variant that is Changed counts as attempting that variant.
The follow-up checkGroup catches the partial-fill case and surfaces a
shortcut_group_incomplete pointing at the missing flag, which is strictly
more useful than the prior oneof_missing.
Code changes:
- binder.go: drop fieldSpec.OneOfTrig, drop parseFieldSpec's oneof_trigger
tag handling, rewrite checkOneOf with the inferred-attempt logic, delete
the isTrigger helper (now inlined and simplified).
- typed_help.go: renderFlagsInBucket flattens all inner flags to the
parent's indent (no more "trigger vs companion" visual distinction,
matching the framework's new semantics).
- binder_test.go: add three behavioural tests covering the new contract:
(1) companion alone surfaces group_incomplete (not oneof_missing),
(2) full group passes,
(3) simple variant + group companion both attempted yields oneof_multiple.
Verified: go build ./..., go test ./shortcuts/common/...,
`go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./...` against HEAD
vs origin/main yields zero new dead code.
Net: -1 tag, -1 field on fieldSpec, -1 helper function, simpler writer
ergonomics, more accurate error messages.
bindFlags and bindBuckets only populated top-level leaves and OneOf
buckets; a top-level group sub-struct (a regular nested struct without
OneOf() marker) had its inner flags registered and group-completeness
checked, but its field values were never written back into the Args
struct — Execute would read empty values from args.Group.X.
Add bindGroups as the top-level counterpart to bindBuckets for IsGroup
fields, and invoke it from mountTyped's Validate closure right after
bindBuckets. Behavior mirrors bindBuckets / bindBucketInner:
- Value-type top-level group: always populated; inner fields receive
cobra flag values (including defaults).
- Pointer-type top-level group: allocated iff at least one inner flag
was Changed, so a nil group still signals "user did not engage this
group" while a non-nil group means "the user opted into it".
Tests cover four cases: value group with explicit flags, value group
with defaults applied, pointer group allocated when an inner flag is
set, pointer group left nil when nothing is set.
Verified: go build ./..., go test ./shortcuts/common/..., and
`go run golang.org/x/tools/cmd/deadcode@v0.31.0 -test ./...` against
HEAD vs origin/main yields zero new dead code.
This closes the previously-documented limitation that group sub-structs
had to be nested inside an OneOf bucket to actually bind values.
After reverting the im pilot in ccf654d3 the TypedShortcut framework
has no production caller, so the CI deadcode check flagged five
methods as newly unreachable. Resolve without breaking the framework:
- Remove TypedShortcut.Mount — a convenience wrapper over
MountWithContext with zero callers (production OR test); not part
of any interface contract.
- Add focused unit tests for the four ShortcutDescriptor methods
required by the Mountable contract — GetAuthTypes plus
ScopesForIdentity / ConditionalScopesForIdentity /
DeclaredScopesForIdentity, with table-driven coverage of the
user / bot / fallback / dedupe branches.
Locally verified: go run golang.org/x/tools/cmd/deadcode@v0.31.0
-test ./... against HEAD vs origin/main yields an empty diff.
The im +messages-send TypedShortcut pilot was exploratory only; revert it
while retaining the TypedShortcut framework for future migrations.
- shortcuts/im: restored to pre-pilot state (im_messages_send.go back to
legacy common.Shortcut; deleted protocol.go, protocol_test.go,
im_messages_send_test.go; shortcuts.go drops TypedShortcuts()).
- shortcuts/register.go: removed addTyped(im.TypedShortcuts()) wiring and
the now-unused addTyped helper; legacy addLegacy + Mountable dispatch
retained.
- shortcuts/common, argstype, errs subtypes, cmd/auth adapters: kept.
- framework doc comments: replaced examples referencing the removed pilot
types (MessageTarget/MessageContent/VideoContent/RawContent) with neutral
descriptions; noted no typed shortcut is registered today.
Framework now has no production caller. Verified: go build ./... and
go test ./shortcuts/... ./errs/... ./cmd/auth/... ./cmd/ all pass.
Add strongly-typed shortcut protocol that coexists with the legacy
common.Shortcut. The new common.TypedShortcut[T] is a generic outer
wrapper backed by a reflect-driven binder; both legacy and typed
shortcuts satisfy a new common.Mountable / common.ShortcutDescriptor
interface pair so register.go can dispatch either through the same
pipeline.
Framework (shortcuts/common):
- protocol.go — Mountable / ShortcutDescriptor / OneOfMarker /
Validatable / Normalizable[T] / ArgsValidator / Maybe[T] /
HelpExample
- binder.go — reflect Args walk, intra-Args flag-tag uniqueness
panic, cobra flag registration, bindFlags + bindMaybe, runNormalize
(via MethodByName dispatch — Normalizable[T] can't be type-asserted
through a non-generic interface), runValidateValue, runFrameworkRules
for required / enum / OneOf / group
- typed_shortcut.go — TypedShortcut[T] struct, 8 descriptor methods,
mountTyped adapter that synthesizes a legacy Shortcut shell and
reuses runShortcut verbatim (identity / scopes / @file / stdin / jq
/ dry-run / high-risk gate)
- typed_help.go — sectioned --help (CHOOSE ONE / OPTIONAL / EXAMPLES)
with cmdutil.GetRisk/GetTips passthrough so typed shortcuts keep the
Risk: and Tips: blocks
- runner.go — typedArgs lifecycle slot on RuntimeContext
- types.go — 5 GetX accessors on *Shortcut so legacy shortcuts
satisfy ShortcutDescriptor alongside the existing pointer-receiver
scope methods
Typed primitives (shortcuts/common/argstype):
- ChatID / UserOpenID / UserOpenIDList — prefix-validated identifiers
- SafePath — cwd-relative, rejects absolute paths and ".." segments
- MediaInput — tri-state (URL bypass / img_xxx-file_xxx key bypass /
SafePath delegation)
- SpreadsheetRef — Normalize extracts shtcn token from feishu URLs
Error contract (errs):
- 3 new Subtype constants: shortcut_oneof_missing /
shortcut_oneof_multiple / shortcut_group_incomplete. Per-field
failures (required / enum / typed primitive format) reuse the
existing SubtypeInvalidArgument so no new error type is introduced.
Registry refactor (shortcuts + cmd):
- AllShortcuts() now returns []common.ShortcutDescriptor; legacy
shortcuts are boxed as *Shortcut (pointer required for the
pointer-receiver scope methods), typed shortcuts boxed as Mountable
- Register dispatches via the Mountable interface
- cmd/auth/login, cmd/auth/login_interactive, cmd/error_auth_hint,
cmd/diagnose_scope_test, shortcuts/register_test (shortcuts.json
generator) updated to read through GetService / GetCommand /
GetAuthTypes / GetDescription / DeclaredScopesForIdentity
- shortcutSupportsIdentity helpers accept ShortcutDescriptor
Pilot (shortcuts/im):
- protocol.go — MessageTarget / MessageContent (with seven content
variants) / VideoContent (paired video + cover) / RawContent
(--content with explicit msg-type, validates JSON in
ValidateValue)
- im_messages_send.go — migrated to TypedShortcut[*ImMessagesSendArgs].
Inline Validate closure is replaced by framework-derived checks
(OneOf target, OneOf content, VideoContent group, typed-primitive
formats, RawContent JSON). Helpers (resolveMediaContent,
wrapMarkdownAsPostForDryRun, normalizeAtMentions, etc.) reused
verbatim; only the field-access pattern changes from
runtime.Str("x") to args.X.
- shortcuts.go — new TypedShortcuts() exporter; ImMessagesSend
removed from the legacy Shortcuts() slice so it is not
double-mounted
- register.go wires addTyped(im.TypedShortcuts()) into init
Known follow-ups:
- runFrameworkRules and bindFlags do not recurse into OneOf bucket /
group sub-structs; im messages-send compensates with a local
bindMessagesSendArgs + validateVideoGroup. Generalizing the binder
to recurse will let future migrations drop the local shim.
- common.ValidateChatID / common.ValidateUserID become redundant
once all legacy shortcuts that call them migrate; can be retired
with the last legacy caller.
Refs: docs/superpowers/specs/2026-05-26-shortcut-protocol-design.md
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.
* fix(apps): read app object from data.app for +create and +update
The Miaoda OpenAPI returns the application object nested under
data.app for both POST /apps and PATCH /apps/{appId}. The CLI text
helper was reading common.GetString(data, "app_id"), which yields an
empty string against the wire format -- so `lark-cli apps +create
--format pretty` printed `created: ` with no ID.
Navigate the new nested path via GetString(data, "app", "app_id") for
both create and update. Update unit-test mocks to wrap the response
under `app`. Refresh the lark-apps skill references (example response
shape + jq paths) so agents reading them follow the right path.
Wire format is passed through to the user's JSON envelope untouched
-- no unwrapping in CLI. Consumers reading the response should use
.data.app.app_id.
The GET /apps list endpoint is unchanged: per the design doc its
items[] are flat objects, no wrapper.
* docs(apps): add required --app-type HTML to scenario 2 snippet
The "用户没有 app_id" snippet in lark-apps-html-publish.md was missing
the required --app-type flag, so copy-pasting it triggered Validate
("--app-type is required") and left $APP empty -- the following
+html-publish then failed with --app-id "". Bring the snippet in line
with every other apps +create example in the skill.
* docs(apps): simplify auth-recovery rule to error.type == missing_scope
Every apps shortcut declares Scopes, so the precheck path in
shortcuts/common/runner.go:825 is always the one that fires on scope
violations and the envelope's error.type is the stable discriminator.
Drop the keyword-sniffing of error.hint, the chain explanation, and the
bot caveat — they all reduce to one boolean: error.type == "missing_scope"
→ run `lark-cli auth login --domain apps`.
Also collapse the corresponding bullet in 快速决策 to point at this rule.
* fix(common): escape special chars in multipart form filenames
MultipartWriter.CreateFormFile concatenated the fieldname and filename
into the Content-Disposition header without escaping, so a filename
containing a double-quote, backslash, CR, or LF produced a malformed
header. For example, uploading `report "draft" v2.pdf` via
`task +upload-attachment` made the server see `filename="report "`
(truncated at the first internal quote) and drop the rest.
Drop the custom override and let CreateFormFile be promoted from the
embedded *multipart.Writer, which applies the stdlib's quoteEscaper
(backslash and double-quote get a backslash prefix; CR and LF get
percent-encoded). The Content-Type ("application/octet-stream") and
the wrapper API are unchanged, so the existing `task +upload-attachment`
call site is unaffected -- filenames with special characters just now
round-trip correctly.
Add helpers_test.go covering plain, quoted, backslashed, mixed, and
unicode filenames. The test asserts both the on-wire encoding and a
round-trip through mime.ParseMediaType (bypassing Part.FileName, whose
filepath.Base is platform-dependent for backslash on Windows).
* test(common): cover CR/LF/CRLF in multipart filename escaping
Per code-review feedback, extend the helpers_test.go cases table with
CR, LF, and CRLF filenames so the test exercises both legs of the
stdlib's quoteEscaper:
- backslash and double-quote use backslash escaping (quoted-pair);
these round-trip exactly through mime.ParseMediaType.
- CR and LF use percent encoding to prevent header injection; the
MIME parser does not decode percent escapes, so the read-side
filename param contains literal "%0D"/"%0A".
The cases table grows a wantParsed column so each case can declare its
expected post-parse value (same as filename for backslash-escaped chars,
percent-encoded for CR/LF).
* refactor(common): polish doc comments and regroup test cases
Two follow-up tweaks suggested by a re-read of the PR:
- helpers.go: stop naming the stdlib's internal `quoteEscaper` in the
doc comment. Describe the observable behaviour ("escapes special
characters") instead, so the comment stays valid if the stdlib ever
renames or reimplements its escaping.
- helpers_test.go: rename the vague `with both` case to
`backslash and quote`; split the table-driven cases into three
visually-separated groups (happy path / backslash escaping /
percent encoding) so it is obvious why two cases have a different
wantParsed than filename.
No behaviour change; tests still pass 8/8.
* test(common): drop CR/LF filename cases that depend on Go 1.24+ stdlib
CI runs against the toolchain pinned in go.mod (1.23.0), whose
multipart/Writer.quoteEscaper escapes only backslash and double-quote.
Percent-encoding of CR and LF was added to the stdlib later, so the
three CR / LF / CRLF cases I added on review feedback fail on CI: the
literal CR/LF lands in the Content-Disposition header and the parser
reports `malformed MIME header: missing colon`.
Drop those three cases. The fix in the prior commits still covers the
real-world bug — backslash and double-quote in filenames — which is
what the original `report "draft".pdf` example demonstrates. CR or LF
in a filename is essentially never legal on any supported OS, so
leaving that edge case to a future stdlib upgrade keeps the test
stable across toolchains.
Also dropped the now-unused wantParsed column from the cases table:
with only round-trippable characters left, mime.ParseMediaType returns
the original filename byte-for-byte, so a single tc.filename comparison
suffices.
---------
Co-authored-by: Wang-Yeah623 <Wang-Yeah623@users.noreply.github.com>
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
Per issue #1049 (third point), wiki +node-get used --token while sibling
commands (+node-delete / +node-copy / +move) use --node-token. The
inconsistency forced humans and AI agents to remember which adjacent
command takes which flag.
Make --node-token the canonical flag and keep --token as a hidden,
deprecated alias so existing scripts continue to work. pflag's
MarkDeprecated prints "Flag --token has been deprecated, use --node-token
instead" to stderr on use, guiding callers to migrate. Conflict between
the two with different values is rejected upfront.
Skills docs (lark-wiki, lark-base) updated to prefer --node-token.
Change-Id: I3415a98f079613c0b1a0b989cf54a09cbb8986fb
Wiki write-path operations (most commonly `wiki +node-create` against the
same parent) surface code 131009 "lock contention" under concurrent calls.
Currently this falls through to the generic "api_error" classification,
giving users no hint that it is transient and safe to retry.
Mirror the existing `LarkErrDriveResourceContention` (1061045) treatment:
add a named constant, classify as "conflict", and emit a hint that points
the caller toward exponential backoff or serializing sibling-node writes.
Refs: #1012
In buildFanoutResponse, when every fanout query fails AND the first failure
has no Lark API code (i.e. transport, parse, panic, or context-cancel),
the returned ExitError was carrying an empty Hint. This is the only
output.ErrWithHint call in shortcuts/ that ships an empty hint.
AGENTS.md states: "every error message you write will be parsed by an AI
to decide its next action. Make errors structured, actionable, and
specific." An empty hint gives the agent nothing to do.
Populate the hint with the actionable next step for this branch — retry,
and if it persists, narrow --queries to a single term to isolate the
failing input. The companion test exercises the no-code path and asserts
the hint is non-empty and mentions "retry".
Co-authored-by: Wang-Yeah623 <Wang-Yeah623@users.noreply.github.com>
Replace 8 bare fmt.Errorf calls with output.ErrValidation across 3 files
so validation errors consistently return structured JSON (type: validation,
exit 2) matching the rest of the codebase.
Affected functions: validateExpectedFlag (sheets), validateSendTime,
validateComposeInlineAndAttachments, validateEventFlags (mail),
validateSignatureWithPlainText (mail)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
When AutoGrantCurrentUserDrivePermission encounters lark code 99991672/99991679,
extract permission_violations from the underlying ExitError and surface
lark_code, required_scope, and console_url on the result map. Override the
generic fallback hint with one pointing at the developer console — the
concrete next step a user can take.
Refactor extractRequiredScopes / SelectRecommendedScope wrapping / console URL
construction out of cmd/root.go into internal/registry/scope_hint.go so both
the top-level enrichPermissionError path and the best-effort sub-call path in
shortcuts/common share one implementation.
Change-Id: Ida63ed160d1167b7961b6faac5c2cf9b7f971c65
- description: switch from trigger-word enumeration to a general
principle (any HTML artifact intended to be independently accessible
falls under this skill; defer the deploy-vs-demo decision to the
skill body)
- surface apps +access-scope-get in prerequisites list and Shortcuts
table so agents can find the read side of access-scope
- add "writing HTML hard constraints" section: index.html is the
required entry filename, --path cannot equal cwd (both are CLI-side
hard rejects that previously only lived in the html-publish ref)
* feat(sidecar): support multi-client identity isolation in server-demo
When multiple CLI sandbox environments share a single sidecar instance,
user tokens (UAT) were not isolated -- the last user to log in would
overwrite previous users' tokens, causing identity cross-contamination.
This change introduces per-client HMAC key isolation:
- Each client gets a unique client-*.key file for data-plane HMAC signing,
allowing the sidecar to identify request origin.
- A new auth_bridge.go handles management endpoints (login/poll/status)
with explicit client-to-feishuOpenId binding.
- User token resolution is strictly bound to the matched client -- no
fallback to other users' tokens when a client has no mapping.
- The shared proxy.key is reused across restarts instead of regenerated,
fixing a race condition when multiple sidecar instances start together.
Wire protocol (sidecar package) is unchanged; existing single-client
deployments are fully backward compatible.
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
* fix(sidecar): address review feedback on filesystem and safety
- Replace os.ReadFile/WriteFile/ReadDir with vfs.* equivalents for test
mockability, consistent with project coding guidelines.
- Limit auth bridge request body to 64KB to prevent memory exhaustion.
- Log errors in saveUserMap instead of silently discarding them.
- Reject client keys that collide with the shared proxy key.
- Reject duplicate client keys instead of silently overwriting.
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
* refactor(sidecar): remove workspace-specific naming and backward compat
- parseClientID: only accept "client_id" field, remove legacy fallback
- loadClientKeys: scan all *.key (excluding proxy.key), no prefix required
- Remove legacy file migration logic in newAuthBridge
- Update flag description to reflect generic key scanning
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
* refactor(sidecar): extract multi-tenant demo and add unit tests
Address review feedback from sang-neo03:
1. Extract multi-client code into sidecar/server-multi-tenant-demo/,
keeping server-demo as the minimal single-tenant reference.
2. Add unit tests for the isolation guarantee:
- loadClientKeys: shared-key collision and duplicate keyHex are skipped
- verifyWithClientKeys: correct client matched, unknown key rejected
- loadUserMap/saveUserMap: round-trip persistence across restart
3. Cross-link READMEs between server-demo and server-multi-tenant-demo.
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
* docs(sidecar): rewrite multi-tenant demo README with problem statement and client guide
- Explain the multi-app credential isolation problem (app_secret must
not be exposed to client environments)
- Document typical deployment topology with multiple sidecar instances
- Add complete client setup guide: env vars, multi-app switching, login
flow, and end-to-end workflow example
- Document design decisions and management endpoint details
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
* fix(sidecar): address CodeRabbit review feedback on tests and docs
- Make TestProxyHandler_AcceptsAllowedAuthHeaders fully offline by using
httptest.NewTLSServer instead of depending on open.feishu.cn
- Isolate TestRun_RejectsSelfProxy config state with t.Setenv and temp dirs
- Check os.MkdirAll error in test fixture setup
- Add language identifiers to fenced code blocks (MD040)
- Validate user-supplied CLI paths with validate.SafeInputPath
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
---------
Signed-off-by: Gao Yang <grany@yeah.net> (topwin.tech)
- Bump version to 1.0.38
- Update CHANGELOG.md with the apps brand gating change since v1.0.37
- Backfill the [v1.0.38] link reference at the bottom of CHANGELOG.md
Change-Id: I6fd0d1243e2219a1eaa1fae5fae4ff6d8de361da
* feat(apps): gate apps domain off on Lark brand
The Miaoda apps OpenAPI is Feishu-only. On Lark brand:
- shortcut subtree is registered + hidden, RunE returns a structured
brand-restriction error so users see a clear message instead of
cobra's generic "unknown command"
- auth login `--domain apps` is treated as unknown; `--domain all`
skips apps; help text omits it
- scope collection skips apps shortcuts so spark:* scopes are never
requested
The leaf-stub pattern mirrors internal/cmdpolicy/apply.go::installDenyStub
(DisableFlagParsing + ArbitraryArgs + leaf-level PersistentPreRunE
override) so cobra can't short-circuit the stub with a missing-flag or
parent-PreRunE detour.
Change-Id: I5817e87ae6fedabdb5faf05d0d32ea988f7effc9
- +member-add: wrap POST /spaces/{id}/members; --member-type / --member-role
enums, optional --need-notification query (omitted entirely when the flag
is unset, instead of forcing need_notification=false), my_library
resolution under --as user, flattened single-member output
- +member-remove: wrap DELETE /spaces/{id}/members/{member_id}; surfaces the
required member_type + member_role body the API expects, my_library
resolution, fallback to echoing the caller's inputs when the API omits
the member echo
- +member-list: wrap GET /spaces/{id}/members; reuses the +space-list /
+node-list pagination contract (single page by default, --page-all walks
every page capped by --page-limit, --page-token resumes a cursor)
- All three reject bot identity + my_library upfront with a clear hint and
declare the narrowest scope the API accepts (wiki:member:create /
wiki:member:update / wiki:member:retrieve) so tokens carrying only the
narrow scope are not false-rejected by the exact-string preflight
- skill docs: reference pages for the three new shortcuts + SKILL.md
shortcuts table; switch the membership flow guidance from raw
`wiki members create` to the new +member-add path
Change-Id: I158a86aa7f00bb7cecc7a4e99346f3fb151b3c09
When a resource is created with bot identity, the CLI attempts to
auto-grant full_access to the current user. If the user open_id is
missing or the grant API call fails, the result was only written to
the JSON permission_grant field and easily overlooked.
Changes:
- Add stderr warnings when auto-grant is skipped or fails
- Add 'hint' field to permission_grant JSON output with failure reason
and actionable next step (e.g. auth login, check scope, retry)
- Add end-to-end skipped/failed tests across all affected shortcuts
(doc, drive, sheets, slides, wiki, markdown, base)
Closes#963
strings.Fields("") returns an empty slice, causing --scope "" to bypass
validation and return ok: true. Replace the false-positive success path
with an ErrValidation error so callers correctly detect the invalid input.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>