mirror of
https://github.com/larksuite/cli.git
synced 2026-09-14 18:42:53 +08:00
codex/opencreate-async-e2e
557 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
5fa9a6a241 | feat: opt document creation into async promotion | ||
|
|
3af7e858bb | feat: poll async document creation tasks | ||
|
|
d12b39cf46 |
feat(vfs): allow absolute paths under a built-in path policy (#2580)
* feat(vfs): allow absolute paths under a built-in path allowlist Path flags only accepted paths relative to the working directory, so an agent passing a full path (typically under /tmp) failed on its first call and had to retry with a relative one. Absolute paths are now accepted when they resolve inside a built-in allowlist: the working directory, /tmp, and ~/files. A built-in denylist covers system and credential locations and wins over the allowlist, including over the working directory. Both lists are compiled in and read no environment variable, flag, or config file, so the effective policy is fixed by the binary; upgrading is all it takes for the new behavior to apply. Containment is decided by file identity (device and inode) alongside the resolved name, because a single directory has many spellings: APFS folds U+017F onto "s", so ".sshh" spelled with it opens ~/.ssh, and NTFS and APFS both compare case-insensitively. Reads are hardened where the policy applies: O_NOFOLLOW pins the final component, O_NONBLOCK keeps a FIFO from blocking before it can be refused, and the opened descriptor is matched against the inspected object, rejected when it is not a regular file, and rejected when it carries extra hard links. The relaxed local-input tier used by apps upload keeps its own contract (symlinks are legitimate arguments there) and gains the denylist check instead. Two behaviors are deliberate rather than incidental. Working inside a denylisted directory now refuses even relative paths, since the denylist is unconditional. Running as root leaves only the working directory and /tmp, because the home directory is then /root, itself a deny root. Existing tests asserted the old "every absolute path is refused" baseline; they now assert the allowlist. Traversal fixtures escape to the filesystem root, which stays outside every allowed root on Linux, where the temp directory that hosts t.TempDir() is /tmp itself. * fix(vfs): close two paths around the built-in denylist A "~/..." argument had two readings: validation expanded it to the home directory, while a caller that keeps the original string — SafeLocalFlagPath returns it verbatim — opens whatever "~" names in the working directory. A symlink there carried reads past the denylist, confirmed by reading /etc/passwd through it. Every interpretation of an argument is now checked, so the shorthand still reaches ~/files while the literal entry cannot escape. With no LARKSUITE_CLI_CONFIG_DIR and no reachable home directory, core.GetBaseConfigDir keeps credentials in a bare ".lark-cli" resolved against the working directory, which is an allow root. That fallback is now mirrored as a deny root, so containers whose home lookup fails do not expose their stored tokens. * fix(vfs): enforce hard-link checks across readers * fix(vfs): stop an output hard link from rewriting a file outside the allowlist A hard link has no target for name resolution to follow, so a link inside an allowed root looked like an allowed destination while sharing its inode with a file outside every root. A caller that truncated the approved name in place rewrote that outside file: `auth qrcode --output <link>` reported success and replaced a 43-byte JSON file outside the allowlist with its PNG. Output validation now refuses an existing target that carries more than one name, which covers callers that write directly, and auth qrcode commits through a temp file and a rename, which replaces the directory entry and leaves the other names alone. Writers already going through FileIO.Save were never affected, since that path has always committed by rename. * fix(vfs): give the hard-link refusal a workable recovery hint The message told the caller to copy the file into an allowed directory, which answers a question they did not ask: the file that triggers this is normally already inside one, with every one of its names there too. It now states what the check actually cannot do — enumerate the other names a file is reachable by — and offers the step that works, which is to copy the file and use the copy. * test(vfs): pick the denylist fixture for the platform under test Two tests reached for "/etc/passwd" as a denylisted absolute path. That path is not absolute on Windows, so one test met the foreign-path rejection instead of the denylist it was asserting, and the other saw the path joined to the working directory and no rejection at all. Both now ask for a deny root that exists on the platform running them — the credential directories under the account home qualify everywhere — which keeps the denylist covered on Windows rather than skipping it there. Verified on Windows 10.0.19045 by running the package's test binary from this branch and from main: main passed, this branch failed these two, and both pass after the change. The other packages this branch touches were compared the same way and their Windows results are identical on both sides. * fix(vfs): state the hard-link check as the condition it tests The check read as "bail out unless the target can be inspected", which nilerr reads as an error swallowed on the way out. It now names the case it acts on — an existing regular file with more than one name — and the comment carries what the early return used to imply: a target that cannot be inspected has no link count to judge, and the write layer reports the real failure with proper typing. * docs(vfs): scope the policy's environment claim to what holds The header promised that neither list accepts runtime input and that no caller controlling the environment can widen them. Two inputs contradict that: LARKSUITE_CLI_CONFIG_DIR contributes a deny root, and where the account database cannot name the running uid, $HOME decides where ~/files points — reproduced in a container running as an unregistered uid, which wrote into a directory the environment chose. The comments now state the preference and its boundary rather than a guarantee, and record what the boundary costs: a directory named "files" under the named path, with the home directory itself still outside the allowlist and every candidate home still carrying the credential deny roots. The trustedHome note also said the pure-Go lookup falls back to $HOME silently; it does so only when $USER is set as well, and returns an error otherwise, which drops the ~/files root instead of moving it. No behavior change. * fix(auth): keep the mode of a QR output file that already exists Committing the QR write by rename fixed a hard link from rewriting a file outside the allowlist, but it also changed what happens to the target's mode. A rename installs the temp file's inode, mode included, where the previous in-place write left the existing file's mode untouched. Overwriting a target the caller had restricted to 0600 therefore published it as 0644. The mode now comes from the file already at the path; only a path with nothing at it takes the default. Verified against main, which preserved 0600 here, and covered by a test that fails when the fixed mode is restored. * test(sheets): move the csv file-alias tests onto the new path baseline Merging main brought #2559's tests for the --file → --csv alias, written against the policy this branch replaces. Two of them fail on it, both because the verdict they describe moved rather than disappeared. The out-of-tree case used /tmp, which the allowlist now accepts, so the value came back as a missing file instead of an out-of-tree one; it now names a path no allow root can contain. The directory case is refused when the descriptor is inspected, before a read is attempted, so the message reads "not a regular file". What the caller sees of both — the flag named, the cause kept, stdin offered — is unchanged. That message listed the kinds it refuses and omitted directories, which is how it reached a directory test reading as a mismatch. It now names them. * fix(im): let the path policy judge a download target `+messages-resources-download` refused an absolute --output before the shared policy saw it, so the flag stayed relative-only after the policy learned to accept full paths. It is the command behind 99% of a reported 1,189 download path errors in one week, where 97.2% of first calls passed an absolute path and every later success had switched to a relative one. The shape checks are gone. Both call sites already hand the result to ResolveSavePath, which applies the allowlist, the denylist and symlink resolution, so refusing a shape here decided nothing the policy would not decide better — an absolute path is now answered by where it points rather than by how it is written. The file-key checks stay, and they are what the batch caller relies on: it embeds the key in the path, and a key carrying a separator is refused as a malformed key, so a traversal cannot be built from one. Verified against a real tenant: /tmp and ~/files now save, while ~/.ssh, /etc and a path outside every root are still refused. * test(im): pin the download output contract the policy now decides The dry-run suite listed an absolute path among the values --output must refuse. That held while the command rejected the shape itself; now that the built-in policy decides, /tmp is an allowed root and the path is accepted, so the case asserted a rule that no longer exists. It is replaced by the two halves of the real contract: an absolute path inside an allowed root reaches the request, and a path that resolves outside every root — a parent escape from this working directory, or a denylisted directory — is still turned down as a validation error naming --output. * fix(vfs): hold a relative path to the working directory Accepting /tmp as an allow root gave a relative path somewhere new to go. A process whose working directory sits under /tmp — CI runners, containers and agent sandboxes commonly arrange that — could climb out with "../" and still satisfy the allowlist, because the sibling it landed in was also under /tmp. /tmp is world-writable, so that sibling can belong to another user or another session, and the write side commits by rename, which replaces an existing target unconditionally. The previous policy refused this: it required every resolved path to stay under the working directory. Naming a full path and climbing out of the working directory are different acts and no longer share one verdict. An absolute path is judged by the allowlist, which is what this branch set out to allow; a relative one has to resolve inside the working directory, whatever wider root contains it. The home denylist grows at the same time and for the same reason: the working directory is an allow root and running from the home directory is ordinary, so a credential store there is reachable by a relative name unless the list covers it. It now names the common ones — netrc, git and shell credentials, kube, docker, azure, gh, gcloud, the language package registries — and the shell histories, which carry pasted keys as reliably as a credential file. --------- |
||
|
|
c6c040c2c5 |
refactor(sheets)!: remove legacy sheets command surface (#2572)
* refactor(sheets)!: remove legacy sheets command surface
The `shortcuts/sheets/backward` package kept 42 pre-refactor command names
(`+create`, `+read`, `+write`, `+create-sheet`, `+media-upload`, ...) alive
alongside the refactored ones. Monitoring puts their combined share below 5%,
so they are dropped along with the machinery that carried them.
Removed with the package:
- the `sheetsAliasReplacement` map and `wrapSheetsBackwardDeprecation`, which
tagged each alias with a `_notice` deprecation envelope on execution;
- the deprecated cobra group and the custom `sheets --help` usage template that
existed only to hide it. `applySheetsCompatGroups` becomes
`applySheetsCommandGroups`: it still groups the `+`-shortcuts so the OpenAPI
metaapi subcommands keep filing under cobra's stock "Additional Commands".
The deleted package owned no shared logic. Its `parent_type` mapping for image
uploads was, by its own header, a deliberate mirror of the canonical one in
`shortcuts/sheets/helpers.go`; `common.IsLocalOfficeToken` and the drive upload
helpers are untouched and keep their other callers.
E2E tests still drove the removed commands and are ported to the refactored
surface: `+workbook-create` / `+workbook-info` / `+cells-set` / `+cells-get` /
`+cells-search` / `+sheet-*`. The sub-sheet dry-run assertions had to be
rewritten rather than renamed, because the old commands posted to
`sheets/v2/sheets_batch_update` while the new ones invoke
`modify_workbook_structure` over `sheet_ai/v2`. `+update-sheet` fanned out to
`+sheet-rename` + `+sheet-hide` + `+dim-freeze`. Every migrated command was
verified against a live workbook, which is where the assertions come from:
rename and hide answer with a bare revision counter, so their effect is read
back from `+workbook-info` (`sheet_name`, `is_hidden`).
Also updated, since these referenced the removed surface:
- three `skills/lark-drive` reference docs that instructed agents to run
`sheets +read` / `sheets +find`; these ship embedded in the binary, so the
instructions would have produced unknown-subcommand errors;
- `skill-template/domains/sheets.md`, deleted: every sheets command it named
was removed and the cell payload shape it taught
(`{"type":"formula","text":...}`) is rejected by `+cells-set`;
- stale comments naming `backward.uploadSheetMediaFile` and
`backward/helpers.go`.
`Shortcut.OnInvoke` and `internal/deprecation` now have no producers. Both are
generic framework plumbing wired into the `_notice` envelope in `cmd/root.go`,
so they are left in place; the `OnInvoke` doc comment no longer claims a caller.
BREAKING CHANGE: removes the 42 pre-refactor sheets commands (`+create`,
`+read`, `+write`, `+append`, `+find`, `+set-style`, `+create-sheet`,
`+update-sheet`, `+add-dimension`, `+set-dropdown`, `+media-upload`,
`+create-filter-view`, ...). They now fail with `unknown subcommand` and carry
no deprecation notice, so a caller still on the old names gets no migration
pointer at runtime.
Replacements for all 42, plus the differences that are not simple renames — the
cell payload vocabulary (`{"type":"formula","text":...}` is now rejected),
response field paths, and `+update-sheet` / `+update-dimension` fanning out to
several commands — are documented in
skills/lark-sheets/references/lark-sheets-legacy-command-migration.md, reachable
at runtime via:
lark-cli skills read lark-sheets references/lark-sheets-legacy-command-migration.md
* test(sheets): close the assertion gaps found in review
- The append subtest asserted only the ok envelope, so a +cells-set that
reported success without persisting would pass; the later +cells-search
covers row 2 only. Read A4:C4 back and assert the row landed. Verified
non-vacuous against a live sheet: an unwritten row returns cells carrying
no value, so the read-back fails if the write does not persist.
- Cover the omitted-title +sheet-copy path. The comment on the empty-title
suite states an omitted --title means "let the server name the copy", but
nothing exercised it; the new case pins that new_name is absent from the
payload rather than sent empty.
- Assert tool_name in the shared dry-run loop instead of only in the create
case, so copy / delete / rename / move cannot pass by selecting a different
tool on the same /tools/invoke_write endpoint.
* docs(sheets): fix the migration guide examples found in review
- `+table-put --sheets` requires the `{"sheets":[…]}` envelope; the `+append`
row showed a bare array, which the flag rejects outright ("top level must be
the object {\"sheets\":[…]}, got a bare JSON array"). Verified both forms
against a dry-run before and after.
- Tag the diagnostic fence as `text` (markdownlint MD040).
|
||
|
|
1d6e7731d3 |
feat: add shortcut for +list-attendees (#2591)
feat: optimize +freebusy shortcut fix: hint timezone feat: operate recurrence event |
||
|
|
ea17864b52 |
docs(sheets): clarify dropdown values and default colors (#2582)
* docs(sheets): clarify multi-select dropdown values * docs(sheets): clarify dropdown values and default colors * docs(sheets): address dropdown review feedback * docs(sheets): document dropdown color readback |
||
|
|
8a7fa53355 |
fix(shortcuts): remove non-actionable stderr progress (#2532)
* fix(shortcuts): remove non-actionable stderr progress Keep successful shortcut output machine-readable by removing lifecycle, retry, and completion progress from docs, wiki, drive, and shared multipart flows. Preserve actionable warnings, fallbacks, and interactive prompts, and update stderr regression assertions. * test(wiki): distinguish warnings from progress |
||
|
|
baf9640bec |
Feat/okr comment (#2558)
* feat: OKR comments * fix: CR issue * fix: skill text & content field validation |
||
|
|
835b52cc88 |
feat(sheets): cut the top command-error clusters from the 08-18..24 eval batch (#2559)
* feat(sheets): cut the top command-error clusters from the 08-18..24 eval batch
A trace analysis over 14,818 `lark-cli sheets` calls attributed 2,036 command
errors to 57 (subcommand, flag) groups, with the top 6 covering 76%. Four of
them are ours to fix; each is addressed at the layer that produced it.
--styles vocabulary (291 cases, the only group whose retry also failed):
- Prescribe the border family as a whole. foldBorderFamilyAliases already
absorbs border / borders / border_<side> / border_<attr>; what still
reached the error path was the Lark OpenAPI's border_type (FULL_BORDER,
OUTER_BORDER) and CSS's border_width — real vocabularies with no
equivalent here, and border_type was the single top field in the group.
Neither maps unambiguously onto a per-side style/weight/color triple, so
they get one shared answer, not a silent alias.
- Prescribe the OpenAPI's nested {range, style:{...}} envelope, plus
bg_color / fill_color / text_color.
- Match prescriptions on the key's letters alone, so border_type,
borderType and border-type are one mistake, not three.
- Collapse repeated issues in the --styles / --writes folds. One wrong
field name in a payload styling N cells produced N identical issues, each
re-listing the full supported vocabulary: the fold meant to save round
trips was burying its own answer. The defect is now stated once and the
other locations are named.
--sheets payload (419 cases):
- Accept dtypes / formats as a positional array. The same pandas habit that
produces `columns` and `data` produces df.dtypes.tolist(), and the
payloads were otherwise correct. Only a 1:1 match with `columns` is
accepted; a length mismatch is rejected rather than guessed.
+csv-put --file (35 cases):
- Read the value as a path. --file is aliased onto --csv because agents
reach for it, but the names promise different things — rewriting only the
name left the path to be written into the sheet as literal text, which the
file-path guard then rejected, with an error naming a flag the caller
never typed. The read goes through the same cmdutil.ReadInputFile as
@file, so the relative-path policy is unchanged and stdin stays the
out-of-tree route. A value naming nothing readable still falls through to
the guard, so --file holding literal CSV keeps working.
--help (88 cases of "required flag(s) ... not set"):
- Mark required flags in the sheets help. MarkFlagRequired only sets a
completion annotation cobra never renders, so a required flag read exactly
like an optional one. Sourced from flag-defs, since +chart-create and
+csv-put deliberately clear that annotation after mounting; a flag cobra
has put in a one-required group is left unmarked, because neither member
of such a pair is individually required.
The remaining big group (absolute paths passed to --file / @file, 636 cases)
is deliberately untouched: the cwd-relative policy is a protocol decision, and
that thread is being followed up separately.
* fix(sheets): correct --file alias provenance and mark its value resolved
Two defects in the +csv-put --file rule from the previous commit, both found
in review:
- The alias record was written from the flag-name normalizer, which pflag also
runs for Lookup and Set — including once with the canonical name right after
the rewrite, and again on every later lookup. `--file a.csv --csv ./b.csv`
therefore still counted as "supplied by --file", and an explicit --csv path
was silently read as a file instead of meeting its guard. The spelling is now
staged by the normalizer and committed by the flag's Value, which runs once
per real occurrence, so the last occurrence wins in either order.
- The rewritten value was not marked as read from a source, so a file whose
contents are themselves path-shaped ("report.csv") failed csvPutInput's shape
check — a valid CSV rejected as a caller who forgot the @. Marking it makes
--file behave exactly like --csv @<path> all the way down, which also lets
the guard skip it on its own rather than through a special case in Validate.
RuntimeContext gains an exported MarkInputResolved for the second half: the bit
already existed for @file / stdin, and a domain that resolves a source itself
needs to set it. No other domain calls it, so nothing else changes.
Also assert the typed contract (Param, Cause) rather than message text alone in
the style-prescription corpus and the collapsed-issue fold, per the repo's
error-test guideline.
* fix(sheets): answer every unreadable --file path under --file, and stage alias provenance only while parsing
Both from self-review of the branch.
An unreadable path passed as --file fell through to the --csv guard, which
answered naming a flag the caller never typed — and for a file that exists but
cannot be opened, prescribed "pass the same path with an @ prefix", which routes
through this very reader and fails identically. Only one case may fall through
now: a value that names nothing AND is not path-shaped, i.e. literal CSV text,
which --file accepted before this rule existed. A path-shaped value naming
nothing, an unreadable file, and a directory each answer under --file.
The alias spelling was staged with no Parsed() guard (the first commit had one;
flagalias.Bind still does). chainFlagAliases looks its aliases up while
installing and pflag normalizes on Lookup, so composing PostMount twice — which
installAliasProvenance explicitly anticipates — replayed "file" through the
already-installed normalizer at mount time, and the next real --csv occurrence
committed it: an explicit --csv path would then be read from disk instead of
meeting its guard. Verified the new regression test fails without the guard.
* fix(sheets): reset alias staging on remount, and assert cause on every --file read error
Second review pass, both valid.
FlagSet.Parsed() stays true once parsing has started, so the guard added in the
last commit only covers a remount that happens BEFORE the first parse. A remount
afterwards — its own alias lookups running through the normalizer the first pass
installed — could still leave a spelling staged for the next occurrence to
commit, which would read an explicit --csv path from disk. Re-running the
install now resets staging, closing the window from the other side. Verified the
new parse/remount/reparse test fails without the reset.
The unreadable-file and directory branches preserve the read error as Cause;
their tests now assert it, matching the sibling that already did.
|
||
|
|
fe8ce4675b |
docs(lark-doc): retain draft workspaces after creation (#2574)
* docs(lark-doc): retain draft workspaces after creation * test(lark-doc): check cleanup phrases in both references |
||
|
|
a257fcbaf9 |
feat(base): support ranking dashboard blocks (#2528)
* feat(base): support ranking dashboard blocks * fix(base): align ranking validation with strict schema * fix(base): scope ranking validation to dashboards |
||
|
|
2d44a5e045 | feat(docs): route local Word media uploads to office mount point (#2568) | ||
|
|
1181dafc76 |
feat(im): support rich-text message attachment zone in send/reply/mge… (#2515)
* feat(im): support rich-text message attachment zone in send/reply/mget/edit Support the post message attachment zone (top-level files array) end to end: - +messages-send / +messages-reply: repeatable --attachment file_key flags merged into the post content's files array (deduplicated). - +messages-mget: render attachment-zone files/folders as <file>/<folder> tags in content, extract file keys for --download-resources. - +messages-edit: new shortcut (PUT /open-apis/im/v1/messages/:id) with --set-attachments / --clear-attachments; body-only edits preserve the attachment zone by default. - Attachment flags are mutually exclusive with --content carrying a files array (declare the zone via one or the other, not both). - bot-only identity, matching server behavior (user token rejected). - Fixes from review: attachments no longer bypass content mutual-exclusion validation (P1); merge dedups by key. - Docs (SKILL.md, references, affordance) and unit tests updated. * fix(im): address design-review findings (auto-infer post, dedup set, doc routing) - --attachment/--set-attachments/--clear-attachments now infer msg_type=post automatically; only an explicit incompatible --msg-type conflicts. --text is rejected with attachments (text is a standalone message, not a post body) with a hint to use --markdown or --content. - --set-attachments deduplicates repeated keys (docs promised this; the replace helper now enforces it). - Shortcut Description no longer leaks the HTTP path or the raw server error phrase; it describes the command semantically. - affordance/im.md +messages-edit now routes WHEN: interactive cards go to messages.patch, corrected messages go to +messages-send, and attachment tri-state tips are listed. - mget doc no longer claims --format json exposes raw wire fields (the output is the rendered content); download eligibility clarified. |
||
|
|
603d13b7eb |
fix(sheets): suppress multipart stderr noise and tighten e2e boundary test (#2550)
Add a Quiet flag to DriveMediaMultipartUploadConfig so callers whose success contract forbids non-empty stderr (the sheets shortcuts) can suppress the chunk-plan and per-block progress lines the multipart path writes unconditionally on success. Both uploadSheetImage and the deprecated uploadSheetMediaFile pass Quiet: true. Also simplify isLocallyOpenedOfficeToken to two HasPrefix calls instead of a loop over an inline slice, so the two-prefix invariant stays in one expression rather than drifting from common. Finally, fix the e2e "at the ceiling stays single-part" test case: the small.png fixture was 9 bytes, not the 20 MB the name implied. Truncate it to singlePartCeiling so the boundary is actually pinned. |
||
|
|
62be9cf20e |
feat(sheets): add +cond-format-result-get and --include conditional_format (#2502)
* feat(sheets): add +cond-format-result-get shortcut and --conditional-format flag - lark_sheet_read_data.go: add CondFormatResultGet shortcut with include_conditional_format_style hardcoded to true - cellsGetInput(): add --conditional-format flag mapping - shortcuts.go: register CondFormatResultGet alongside existing cond-format shortcuts - lark_sheet_read_data_test.go: add dry-run test cases covering new shortcut and --conditional-format - flag-defs.json / flag_defs_gen.go: sync from sheet-skill-spec * fix(sheets): fold conditional format into include flag * refactor(sheets): isolate conditional format result output * fix(sheets): satisfy nested slice lint |
||
|
|
2f8d816512 |
fix(sheets): make image-upload previews match what Execute sends (#2537)
* fix(sheets): repair the build after the local-office detection move main does not compile: shortcuts/sheets/lark_sheet_workbook.go references isOfficeSpreadsheet and officePrefixes, which no longer exist in the package. Neither PR was wrong on its own. #2531 (merged 10:39) moved local-office token detection into common.IsLocalOfficeToken and deleted the sheets-local copies; #2533 (merged 12:48) added errLocalOfficeExportUnsupported, which calls them. #2533's branch predated the move, so its CI was green against a base that still had the symbols, and merging it left main broken. Repoints both references at the moved API. Behaviour is unchanged: common.IsLocalOfficeToken is the same predicate #2531 moved, and the prefix pair is the exported form of the same two constants. #2533's own coverage — TestWorkbookExport_LocalOfficeTokenRejected across the local_office_ prefix, the fake_office_ prefix, and an interleaved OFL0X token, plus the wiki-node and dry-run cases — passes unchanged, which is what pins the equivalence. * fix(sheets): derive dry-run image parent_type from the ref kind, not the token A `/wiki/` URL reaches a DryRun hook as the wiki node_token: resolving it to the backing spreadsheet needs the get_node call a preview must not make. Both image-upload previews fed that node_token straight to sheetMediaParentType, so the parent_type they showed was derived from a token that is not the one Execute uploads against. A node_token shaped like an imported office token previewed office_sheet_file for a spreadsheet that will upload as sheet_image. sheetsDryRunParentType decides from the ref's kind instead. A wiki ref is native by construction, not by default: resolveWikiNodeToSpreadsheetToken rejects any node whose obj_type is not "sheet", and a spreadsheet backed by an imported office file sits in drive as a "file" node, so it never survives that gate to reach an upload. Execute is unaffected either way — it derives from the resolved token. This mirrors slidesDryRunParentType, which the slides domain already applies for the same reason. The hooks now hold the parsed ref rather than re-deriving the token from it, so the kind is visible where the preview is built. Also records, at uploadSheetMediaFile, that office_sheet_file survives the multipart path. Slides caps image uploads at 20 MB because upload_prepare rejects its parent types outright, which raised the question for sheets, whose deprecated +media-upload has no such cap. Verified against the live API: upload_prepare accepts both sheet_image and office_sheet_file, and a 20.6 MB file uploaded with office_sheet_file completes prepare -> 6 x upload_part -> upload_finish and returns a file_token a float image then accepts. Tests: sheetsDryRunParentType over both ref kinds, including wiki refs carrying office-shaped and office-prefixed node tokens; dry-run coverage through the public flags for +cells-set-image and +float-image-create, each checked against the identical token as a /wiki/ URL and as a raw spreadsheet token so the two rows differ only in kind; the same pair added to the e2e dry-run lane. All four fail against the previous behaviour. * fix(sheets): send oversized images through the chunked upload, not upload_all uploadSheetImage always used the single-part endpoint, so an image past the 20 MB ceiling failed with a bare 1061002 "upload media failed: params error" naming neither the size nor the limit. The capability was already in the domain: the deprecated sheets +media-upload has dispatched by size since it was written (backward.uploadSheetMediaFile), which left the same image succeeding through the old shortcut and failing through +cells-set-image and +float-image-create, the ones meant to replace it. uploadSheetImage now picks the endpoint by size the way doc and the deprecated shortcut already do. The parent_type is unchanged and still comes from sheetMediaParentType, so the office/native split rides along either branch. The preview follows the same branch. appendSheetImageUploadDryRun renders one upload_all under the ceiling and the upload_prepare / upload_part / upload_finish trio above it, and both image-write hooks now build their upload step through it rather than each spelling out an upload_all. A preview that promised a single-part upload for a file the CLI will send in chunks is a preview of a different request. Verified against the live API with a 20.6 MB PNG: +cells-set-image and +float-image-create both complete, the cell reads back holding the uploaded image_token at the file's real 3000x2400 dimensions, and the dry-run shows the three chunked steps Execute hits. The same file failed with 1061002 before. Tests: the chunked branch at exactly one byte past the ceiling, asserted through upload_prepare's parent_type with upload_all deliberately left unstubbed so a regression fails loudly; the preview's step list on both sides of the boundary, as a unit test and in the e2e dry-run lane. All fail against the previous behaviour. |
||
|
|
decc9549b5 |
refactor(sheets): keep the success path off stderr (#2533)
* feat(sheets): reject local-office tokens in +workbook-export
A locally opened Office file (a local_office_ / fake_office_ token, or an
interleaved OFL0X one) names a file the Lark client is showing, not a cloud
document, so the drive export task can only fail on the backend -- and it
fails late, after the create and poll round trips, with an opaque message.
Refuse it up front with a typed failed_precondition that says the workbook is
already a file on disk, and points at +workbook-import for callers who want a
cloud spreadsheet they can export later. The check runs in Validate (so
--dry-run is covered too) and again after the wiki hop in Execute, where the
real spreadsheet token is first known.
* refactor(sheets): report success-path advisories in the result, not on stderr
Every sheets shortcut that had something to say on a successful run said it on
stderr: ignored sub-op locators, emulated dimension semantics, the deprecated
--dimension/--count and +cells-batch-set-style spellings, the dropdown
option-error steer, and the upload/export stage lines in the compatibility
layer. PowerShell's native-command handling and most agent harnesses read
non-empty stderr as failure, so a working call reported itself as an error --
and the facts a caller actually needed sat outside the JSON they parse.
Pure stage text ("Writing image", "Waiting for export task") is deleted: it
duplicates what the result already proves. Everything decision-relevant moves
into the payload:
- data.warnings ignored locators, colliding freezes, the dropdown
option-error steer (also shown in --dry-run now)
- data.effective_operation +dim-insert's anchor shift under --inherit-style
before, and the whole (rows, cols) state a freeze
leaves behind
- data.deprecation +cells-batch-set-style and +dim-freeze's legacy
flag pair, under a key of its own rather than
mixed into warnings
- data.upload how +media-upload sent the file
Clean calls keep their exact previous payload shape: every field above is
added only when it has something to report.
Scope is shortcuts/sheets/** on purpose. The remaining success-path stderr in
this domain comes from shared code (the drive export/import core behind
+workbook-export / +workbook-import, the multipart media helper, the auto-grant
helper), which other domains share; cleaning those up belongs to their own
change. The one sheets-owned exception is +workbook-import's extension
correction, which has no slot in the import core's output envelope -- it is
documented at the call site and allowlisted in the guard test.
Tests pin the contract (a successful run leaves stderr empty) and each new
field, plus a source scan that stops new direct ErrOut writes from appearing.
* docs(sheets): point the dropdown option-error warning at data.warnings
The --source-range flag help still told callers the option-error steer arrives
on stderr; it now rides in the result. Mirrors the same edit in the upstream
spec (canonical-spec/spec-tables/flags.json), so the next sync is a no-op.
* fix(sheets): keep export identifiers in +export output, tighten the stderr guard
Review follow-ups on the success-path stderr change:
- +export --output-path lost file_token: on the download branch the token
reached the caller only through the deleted "Export complete: file_token=…"
stderr line, and the payload carried just saved_path and size_bytes. Both
file_token and ticket now ride in the download result, so a caller can
re-download or resume without re-running the export.
- The stderr guard allowlisted a whole file, hiding any future write in it.
It now matches one exact statement in one file and asserts that write still
exists, so both a new write and a stale exception fail the test.
- The contract comments claimed more than the tests prove. They now state
that only sheets-OWNED code is silent, name the three commands whose noise
comes from shared implementations (+workbook-export, +workbook-import,
+media-upload over 20MB), and a new test pins that the shared export core
does still write -- failing, by design, once that core is cleaned up.
* fix(drive): keep the export and import cores off stderr on success
+workbook-export and +workbook-import delegate to drive.RunExport /
drive.RunImport, so the sheets success-path contract could not hold while
those cores narrated every step: task creation, each poll attempt, completion,
"still in progress", and the import's media upload. Callers that read
non-empty stderr as failure saw a finished export report itself as an error.
The stage text is deleted -- ticket, ready, status, file_token, token and
next_command are all already in the payload. What the narration alone carried
moves into the result:
- poll attempts / transient_failures / last_error, added only
when a poll actually had to be retried, so a caller can
tell a clean run from one that limped to the finish
- warnings markdown export falling back to the token as file name
after a failed title lookup
- input_corrections a caller-supplied record of inputs the CLI rewrote
before the request ran; sheets +workbook-import uses it
for a mislabeled .xls that is really an .xlsx, which was
its last stderr write
drive +export / +import get the same treatment, since they share these cores.
Clean runs keep their exact previous payload shape.
With this, the sheets stderr guard needs no allowlist, and the contract test
covers both workbook commands end to end. Two shared paths a sheets caller can
still reach stay noisy and are named in the contract comment: multipart media
upload over 20MB, and the bot-identity auto-grant warning.
* test(sheets): cover the annotation shapes and both guard call sites
Review follow-ups, all test-side except one comment:
- +dim-insert's effective_operation had no test: a regression could drop the
emulated-anchor block and still keep stderr empty. Now asserted field by
field, plus the negative case (--inherit-style after rewrites nothing, so it
must not gain the block).
- The local-office guard's second call site had no test. A /wiki/ URL only
reveals its backing token after get_node runs in Execute, so that branch is
now covered, asserting both the typed rejection and that no export task was
created.
- annotateSheetsResult's three payload shapes are pinned: object annotated in
place, array/scalar preserved under `result`, and an empty tool result left
without an invented `result: null`. The doc comment now spells out that last
case instead of lumping it in with non-object output.
- The export poll summary test asserted transient_failures but not attempts,
so a wrong or missing count would have passed.
* fix: preserve recovery state on failure paths and TTY liveness during polls
Review round 2. Removing the success-path narration also removed information
from paths that fail after remote work has started, and removed the only
liveness signal an interactive user had:
- drive +import / sheets +workbook-import: once the import task exists, the
ticket is the only handle back to it. A poll failure returned bare, so the
ticket -- previously visible through the polling line -- was lost. It now
rides on the typed error together with the +task_result command.
- sheets +export --output-path: a download or save failure happens after the
artifact is ready, so the error now carries ticket, file_token and the
+export-download command; re-running the whole export is not the recovery.
A poll timeout carries the ticket for the same reason.
- sheets +batch-update / +batch-chart-*: batch_update is fail-fast without
rollback, so the ignored-locator and colliding-freeze advisories matter most
exactly when the call fails part-way -- they decide the safe retry set. They
are now attached to the typed error's hint as well as the success payload.
- Bounded polls and the import upload are wrapped in RuntimeContext.StartSpinner,
which is gated on StderrIsTerminal and is a strict no-op for pipes, CI and
captured output. A human terminal gets liveness back; a machine caller's
stderr stays empty (the contract tests, which capture stderr, still pass).
+workbook-export's rejection of Office tokens also stopped assuming the caller
holds the file: a local_office_ / fake_office_ prefix means the workbook is
already on their disk, but an interleaved OFL0X token is a file stored in Lark
that may never have been downloaded, so that class is now pointed at
drive +download (then +workbook-import if they want a Lark spreadsheet).
Each behaviour above has a regression test; httpmock's CapturedBodies doc
comment is corrected, since it is appended on every match, not only for
Reusable stubs.
|
||
|
|
0f60fbfbdd |
fix(slides): relax office token length check from 28 to >=25 (#2531)
* fix(slides): relax office token length check from 28 to >=25 The interleaved "OFL0X" product/region marker is read at fixed positions (1-based 5/10/15/20/25), so a token only has to be long enough to hold it. Pinning the total length to exactly 28 silently reclassified every other length as native. 28 is already stale: per #2509 the local-office format is "OFL0X + 21 random + 1 office type enum" = 27 characters, and sheets relaxed the identical guard to >= 25 in that PR. Slides was missed, so an imported office deck at the current length uploaded with parent_type "slide_file" instead of "office_slide_file". The marker positions are unchanged. Relaxing the length is only safe because of them: a false positive is the dangerous direction, since the drive backend does not validate that parent_node actually names an office file, so a misclassified native deck uploads successfully and only surfaces later as an image that will not render. The interleaved native token cases (same length, different marker) are what keep the floor honest. Tests: replaced two mislabelled rows with five verified ones covering 27, 29, 25, 24 characters and a 28-character token carrying the ppt office type enum. #2509's own labels were off by one or two characters and it never covered the 25/24 boundary; this does. * refactor(common): extract local-office token detection into common The office token shape existed in three identical copies: shortcuts/sheets/helpers.go, shortcuts/sheets/backward, and shortcuts/slides. Every copy is somewhere a format change has to be found again, and that is not hypothetical — #2509 had to apply the same 28-to->=25 relaxation twice inside sheets, and missed slides entirely. Moved the shape to common.IsLocalOfficeToken. It belongs there because recognising a local-office document is a drive-level property, not a per-domain one: an imported office file is an imported office file whether it backs a spreadsheet or a deck. What genuinely differs per domain is the parent_type the answer selects — office_sheet_file vs office_slide_file — so those mappings stay with each domain. The name deliberately matches the vocabulary #2509 already used ("local-office format"). Its doc comment calls out that "local office" is the whole category and not the LocalOfficeTokenPrefix case, since the two now share a word stem while the predicate also accepts FakeOfficeTokenPrefix and the interleaved marker. Only slides is rewired here. The two sheets copies are left alone on purpose to keep this reviewable as a pure no-op for them; they can follow separately. The marker offsets are now an array whose length is tied to the marker string, so adding a character to one without the other stops compiling, and TestOfficeTokenMinLenMatchesMarkerOffsets pins the length floor to one past the last offset rather than letting the two merely agree by coincidence. Behaviour is unchanged, verified by diffing dry-run parent_type between the pre-refactor and post-refactor binaries across 13 tokens covering both prefixes, the 24/25 boundary, 27/28/29 characters, interleaved native pptcn/shtcn markers, a leading-but-misaligned OFL0X, and an off-by-one offset: 13/13 identical. * refactor(sheets): route local-office detection through common Deletes the last two copies of the token shape, both byte-identical to the one now in common: shortcuts/sheets/helpers.go and shortcuts/sheets/backward/lark_sheets_float_images.go. sheetMediaParentType keeps owning the sheets half of the decision — which parent_type the answer selects — and only the shape moves. Equivalence was not assumed from reading. A throwaway fuzz test compared isOfficeSpreadsheet against common.IsLocalOfficeToken in both packages over an alphabet biased toward the characters that can actually disagree (OFL0X plus the native product markers), every single-byte mutation of a known office token at all 28 positions, and 400k fixed-seed random tokens of length 0-33. Zero disagreements in either package. Dry-run parent_type was then diffed binary-to-binary against origin/main across 11 tokens covering the 24/25 boundary, 27/28/29 characters, interleaved native shtcn/pptcn markers, a leading-but-misaligned OFL0X and an off-by-one offset: sheets identical on all 11. Two comment fixes that the extraction made unavoidable: The const-block doc in both files still described "a 28-character token". That was already wrong on main — #2509 relaxed the guard to >= 25 and left the comment behind — and the shape is no longer described here at all now, so both defer to common.IsLocalOfficeToken. Four rows in TestSheetMediaParentType were mislabelled: "25 char, at boundary" held a 27-character token and the three "new 27-char" rows held 28-character ones, so the floor those labels claimed to cover was never tested. Relabelled by measured length, and the real 25/24 boundary added. |
||
|
|
93817909cb |
feat(base): add field extension shortcuts (#2463)
Co-authored-by: yballul-bytedance <273011618+yballul-bytedance@users.noreply.github.com> Co-authored-by: TRAE CLI <traecli@bytedance.com> |
||
|
|
84f9414311 | feat(base): streamline record workflows (#2529) | ||
|
|
1c4f7588dd |
feat(calendar): add +join-event shortcut and share token support (#2508)
* feat(calendar): add +join-event shortcut for joining via share token Add a share-token-only join path so callers cannot forge a plaintext event id, wire it into Shortcuts(), and document the flow in the lark-calendar skill. * docs(calendar): document sharing events via share_info link - Route "share event to person/group" intent to calendar events share_info then lark-im, clarifying the share link is not an applink * fix(im): preserve calendar share token in shortcuts --------- Co-authored-by: 张哲伟 <zhangzhewei@bytedance.com> |
||
|
|
971e639622 |
feat(sheets): relax local-office token length check from 28 to >=25 (#2509)
The local-office token format is changing from 28 to 27 characters per the new rule (OFL0X + 21 random + 1 office type enum). Relax the guard from to so 27-char tokens can reach the OFL0X interleaved marker check. All legacy detection paths (fake_office_ prefix, local_office_ prefix, OFL0X marker) are preserved unchanged. |
||
|
|
d0158ab289 |
fix(docs): recover PowerShell-dequoted presentation JSON (#2501)
* fix(docs): recover PowerShell-dequoted presentation JSON * fix(docs): recover quoted-key shell JSON |
||
|
|
0f9553385c |
feat(im): add chat AppLink output (#2491)
Add chat AppLink fields to IM chat outputs and update lark-im guidance to prefer CLI-provided links. |
||
|
|
083f0f4719 | feat(drive): support appid in member-remove (#2499) | ||
|
|
6952d3aa7f |
feat(extension): add business command extension v1 (#2308)
* feat(shortcuts): import typed shortcut framework from |
||
|
|
5f35c72bd3 |
feat(sheets): combine chart workflows and special chart types (#2374)
* feat(sheets): support partial chart snapshot schemas * feat(sheets): add semantic chart shortcuts * feat(sheets): improve semantic chart workflows * fix(sheets): prefer semantic chart shortcuts * fix(sheets): normalize chart range and flag inputs * fix(sheets): normalize irregular chart ranges * feat(sheets): add chart data update shortcut * feat(sheets): harden chart update workflows * feat(sheets): improve chart creation dimension handling * feat(sheets): add dedicated chart batch shortcuts * fix(sheets): allow chart color theme patches * fix(sheets): persist chart color theme updates * feat(sheets): simplify batch chart operations * fix(sheets): preserve batch scope and cross-sheet chart ranges * feat(sheets): support bubble waterfall and pareto charts * fix(sheets): sync special chart tool schema * feat(sheets): add semantic bubble chart indexes * docs(sheets): sync combined chart workflow guidance * fix(sheets): align combined chart artifacts * feat(sheets): add x-axis number interpretation flag * docs(sheets): validate chart axis semantics * fix(sheets): allow recursive chart update patches * test(sheets): isolate chart create schema check * feat(sheets): refine semantic chart creation * docs(sheets): sync semantic chart guidance * docs(sheets): remove unrelated label position guidance * fix(sheets): support all chart data label combinations * fix(sheets): support numeric x-axis bounds * feat(sheets): support chart y-axis bounds * feat(sheets): add last-point chart label flag * fix(sheets): preserve disabled waterfall stacking * fix(sheets): nest last-point chart label property * fix(sheets): resolve lint and dead-code CI failures - lark_sheet_chart.go: drop redundant chartConfigUpdateInput / chartDataUpdateInput calls in Execute whose result is immediately overwritten by the *FromSnapshot variant (ineffassign); the snapshot variants already re-run the same validation internally. - lark_sheet_chart_test.go: remove Go 1.22+ redundant loop-variable copies (copyloopvar). - batch_op_dispatch.go / lark_sheet_batch_update.go: remove unreachable allowedBatchShortcuts and batchUpdateInput; callers use the lower-level allowedShortcuts and buildBatchUpdatePlan directly. * fix(sheets): validate chart config updates * fix(sheets): sync skill specs and chart schema validation * fix(sheets): resolve chart review follow-ups * fix(sheets): restore the two-color contract * fix(sheets): require at least two chart colors * docs(sheets): expose advanced chart shortcut flags * fix(sheets): address chart review feedback * fix(sheets): surface ignored batch locators * chore(sheets): bump skill version to 3.1.6 * fix(sheets): surface batch warnings consistently * test(sheets): satisfy copyloopvar lint * docs(sheets): sync skill from spec * fix(sheets): harden chart batch updates * fix(sheets): tighten chart update validation * fix(sheets): canonicalize chart ranges and batch targets |
||
|
|
faa2f8d3e0 | fix(docs): continue media downloads on permission scope errors (#2498) | ||
|
|
f09414f9d5 | fix(wiki): keep node-get stderr machine-readable (#2449) | ||
|
|
aec659c461 | feat: words replace and minutes fix (#2490) | ||
|
|
e668fff669 | fix(drive): continue downloads on permission scope errors (#2494) | ||
|
|
1d24a39659 |
fix(slides): strip stale <note> id in +update-slide to avoid backend crash (#2475)
* fix(slides): strip stale <note> id in +update-slide to avoid backend crash
A +update-slide carrying a <note id="..."> that is not the page's current
note block makes RewriteSlideBySXSD reject the whole page with
"block is not NoteBlock". This happens when the XML is copied from another
page, or written over a page that was re-created (add-slide reassigns ids,
so the note block's id no longer matches).
Drop only the <note> id before sending. The backend then targets the page's
own note block and the write succeeds. Every visible element keeps its id,
so it is updated in place rather than rebuilt — text layout is preserved and
there is no risk to svg-internal id references.
* fix(slides): strip single-quoted and spaced note id too
The note-id strip only matched id="...". A single-quoted or spaced form
(id='...', id = "...") slipped through. Both are valid XML, and the backend
accepts single-quoted markup — verified on ppe: an all-single-quote page
updates fine, and a single-quoted stale note id reproduces the exact
"block is not NoteBlock" crash this strip is meant to prevent, while the
double-quoted equivalent is stripped and succeeds.
Widen the regex to `\s+id\s*=\s*("[^"]*"|'[^']*')` so any quote style and
whitespace around '=' are covered. Still a targeted edit on the <note> tag,
not a re-serialization, so the caller's bytes are otherwise preserved.
Add regression cases: single quotes, whitespace around '=', single-quote
attribute order, and a single-quoted visible-element id left untouched.
Addresses CodeRabbit review on #2475.
* test(slides): assert the note id attribute is removed, not just a value
The strip tests checked that a specific id value ("blw") disappeared, which
would also pass if the implementation swapped the id for another value.
Assert on the <note> opening tag carrying no id attribute at all (any quote
style / spacing) via a noteTagHasID helper, so the removal itself is verified.
Addresses CodeRabbit review on #2475.
* fix(slides): locate the <note> tag with the XML tokenizer before stripping id
The raw regex parsed XML as plain text, so it could miss or mis-edit valid
input: an id after an attribute whose value contains '>', and note-like text
inside comments or CDATA. It also had no notion of where the note sat in the
tree.
Walk the document with encoding/xml to find the start tag of the <note> that
is a direct child of the root <slide>, then delete the id attribute by editing
only that tag's bytes. Nothing is re-serialized, so quote style, attribute
order, whitespace, and every other element (notably inline <svg> namespaces,
whose round-tripping is a known source of "embed missing inner svg") survive
untouched — the same byte-preservation contract ensureXMLRootID keeps.
This covers the cases the regex could not: '>' in an attribute value, and
comment/CDATA text that merely looks like a <note>; and it scopes the edit to
the slide-level note only. Regression cases added for each.
Addresses CodeRabbit review on #2475.
* test(slides): assert everything but the note id survives byte-for-byte
Existing tests spot-checked that individual elements survived. Add exact-equality
cases asserting the output equals the input with only the one note id removed —
proving nothing else moves: inline svg subtrees, CDATA, a '>'-bearing attribute,
quote style, attribute order, and whitespace all stay verbatim.
Addresses CodeRabbit review on #2475.
* style: gofmt slides_update_slide.go
* test(slides): cover numeric char refs — InputOffset must not drift the note span
|
||
|
|
35bd5ecfcd |
feat(vc): add agent meeting control shortcuts (#2466)
* feat(vc): add agent calendar meeting actions * feat(vc): add meeting screenshot shortcut for visual context * feat(vc): add meeting countdown commands and events * fix(skills): avoid screenshot recall from meeting description * fix(vc): omit screenshot log ID on success --------- Co-authored-by: renaocheng <renaocheng@bytedance.com> Co-authored-by: shike.11 <shike.11@bytedance.com> |
||
|
|
0679884761 | fix(base): hide dashboard auto analysis setting (#2465) | ||
|
|
7874dc144b | feat(slides): add media download shortcut (#2446) | ||
|
|
8ebdc3f193 |
feat(slides): un-deprecate +replace-pages shortcut (#2470)
Remove deprecation markers from +replace-pages: the command is now a supported shortcut again, not a deprecated compatibility shim. Delete the deprecation-note constant and the "deprecated" output field from dry-run, validate-only, and real-run envelopes, update the command description and comments, and remove the deprecation-specific test. |
||
|
|
e0e90a4e1b |
fix(apps): classify the online DDL ban and the file storage quota failure (#2460)
* fix(apps): classify the online DDL/DCL ban on +db-execute
Running DDL against the online branch of a multi-env app came back as
api/server_error with exit 1, hinted "fix the SQL and re-run", and carried a
statement position that did not exist. All three point the caller the wrong way:
- server_error means "upstream 5xx, retryable"; this is a product rule and no
number of retries changes it;
- the SQL is fine — the target environment is what has to change;
- "(at statement 1 of 1)" is fabricated. The server pre-validates the whole
batch and returns a single ERROR sentinel, so a 5-statement request with the
DDL in position 4 still rendered as "1 of 1". The CLI does not split the SQL,
so it cannot know the real count and cannot detect the mismatch generally —
only codes known to be batch-level rejections can drop the suffix.
Give code 4000001 its own arm: validation/failed_precondition (exit 2, "change
the environment, do not retry"), a hint pointing at dev plus +db-env-migrate, and
no statement position. Every other code keeps its current classification, wording
and position suffix; a test pins that.
4000001 is a dedicated server-side code (ErrOnlineEnvForbidDDLDCL, client-error
band), raised only by the pre-validation pass when env==online on a multi-env
workspace. Syntax errors and PG errors use different codes, so keying on it is
safe. Matching on the numeric value also covers the "k_dl_4000001" wire form,
since codeString already strips that prefix — both forms are tested.
"No statements were applied" is stated rather than inferred here: the validator
walks every statement and rejects the batch on the first DDL, so nothing lands.
The default arm would have inferred the opposite for a DDL in a later position
("Earlier statements were committed"), which is wrong for this code.
* fix(apps): classify tenant file storage quota exceeded
+file-upload against a tenant whose file storage is full returned api/unknown with
no hint at all, so a caller could not tell "the quota is full, stop" from "the
upstream had a bad minute, retry" — and had no next step either.
Register 400000055 as api/quota_exceeded. That is a dedicated server-side code
(ErrTenantStorageQuotaExceeded, client-error band) raised only on the upload path,
and the subtype already carries "retrying will not help", so the framework's
existing quota wording is enough and no domain-specific hint is added.
Left in CategoryAPI (exit 1) rather than Validation (exit 2): a full quota is not
something a different argument fixes, and exit 2 would imply it is.
The test asserts the hint is non-empty on purpose. The wording comes from the
shared APIHint table, so if quota_exceeded is ever dropped from there this fails
and says the code now needs its own wording, instead of silently shipping an
empty hint.
|
||
|
|
1f53f6e2f5 |
refactor(slides): assert dry-run parent_type instead of deriving it from a placeholder (#2461)
* refactor(slides): assert dry-run parent_type instead of deriving it from a placeholder A wiki --presentation cannot be resolved during a dry-run: the real presentation token only exists after a get_node call a preview must not make, so parent_node shows a "<resolved_slides_token>" placeholder. appendSlidesUploadDryRun derived parent_type from parent_node, which sent that placeholder through the office-token check. The value it produced was correct. A placeholder matches no office token shape, so it fell through to slide_file, and slide_file is right here: a wiki ref that reaches an upload is native by construction, because resolvePresentationID rejects any wiki node whose obj_type is not "slides" and an imported office deck sits in drive as a "file" node. It was correct by accident, though, which left the preview hostage to the placeholder's spelling and to every rule later added to isOfficePresentation. Pass parent_type in explicitly instead, so slidesDryRunParentType states the wiki case as a decision with its reason recorded, and the placeholder is never classified. No behaviour change: dry-run output is byte-identical for native tokens, imported office tokens, legacy office prefixes, slides URLs, wiki URLs, and +create, across +media-upload / +add-slide / +update-slide. Tests pin the contract the refactor protects, including a wiki ref whose node token is itself office-shaped -- a wiki node token and the deck token it points at are different tokens in different namespaces, so classifying ref.Token would be wrong for a wiki ref even though it is right for every other kind. That is the regression this makes impossible. * test(slides): use the fixture hostname for the wiki dry-run probes domaincontract rejects "bytedance.larkoffice.com": it is in neither allowlist, and a real tenant host does not belong in a fixture. Use example.feishu.cn, already in fixture-domains.txt and the hostname the rest of the slides wiki tests use. The URL is only a parse fixture -- nothing resolves it -- so only the hostname changes. |
||
|
|
b624948e48 |
Support repeated mail compose flags (#2271)
* feat: support repeated mail compose flags * fix(mail): address review feedback for repeatable inline flags Change-Type: ci-fix * test(mail): cover inline validation and upload assertions Change-Type: ci-fix * test(mail): assert inline validation category Change-Type: ci-fix * fix(mail): preserve inline compatibility cases * test(mail): strengthen inline compatibility coverage * docs(mail): prefer one repeatable flag form * docs(mail): keep skill references unchanged * docs(mail): drop skill reference edits * docs(mail): document repeatable mail flags consistently * docs(mail): standardize quoted flag examples * docs(mail): keep inline flag constraints in help * fix(mail): validate template inline cids * fix(mail): preserve recipient names and validate template cids * fix(mail): support repeated recipient parsing Normalize repeated recipient values through ParseMailboxList for every flag occurrence so legacy comma lists still split, quoted display-name commas stay intact, and Unicode display names remain raw before final header rendering. Local check: gofmt -l shortcuts/mail/helpers.go shortcuts/mail/mail_repeatable_flags_test.go * fix(mail): scope template inline update validation --------- Co-authored-by: bubbmon233 <272202079+bubbmon233@users.noreply.github.com> |
||
|
|
56ad837c3d | feat: event organizer transfer bot to user (#2448) | ||
|
|
52f970f23e | refactor(shortcuts): remove MCP text location paths (#2439) | ||
|
|
fbd1aa49cd | feat: add IM read status shortcuts (#2318) | ||
|
|
b343e67639 |
feat(slides): use office_slide_file parent_type for imported office presentations (#2441)
Image uploads to a presentation hard-coded parent_type=slide_file at every
entry point. Imported "office" presentations carry either a legacy synthetic
token prefix ("fake_office_" / "local_office_") or a 28-character token whose
interleaved product/region marker is "OFL0X", and for those the drive backend
requires parent_type=office_slide_file. This mirrors the office_sheet_file rule
the sheets domain already applies: the token shapes are identical, because an
imported office file is an imported office file whether it backs a spreadsheet
or a deck.
Funnel the selection through one slides-domain helper so the rule lives in a
single place and every image-upload path stays consistent with its own dry-run
preview. As in sheets, the rule stays inside the domain rather than leaking
into common.UploadDriveMediaAllTyped, which mail/doc/drive/base/calendar share.
- Replace the slidesMediaParentType const with slidesMediaParentType(token),
backed by isOfficePresentation(token); keep the native and office values as
named constants.
- Route both parent_type call sites through it: uploadSlidesMedia (the Execute
path shared by +media-upload and the <img src="@path"> placeholder pipeline
behind +create / +add-slide / +update-slide) and appendSlidesUploadDryRun.
- Known gap, documented at the helper: when --presentation is a wiki URL the
dry-run only has a "<resolved_slides_token>" placeholder, since the real
token needs a get_node call the preview must not make, so such a preview
shows slide_file regardless. Execute is unaffected -- it resolves first.
The negative half of the mapping is what the tests weight most heavily. The
backend does not validate parent_node against parent_type, so a native deck
misread as office still uploads successfully and only surfaces later as an
image that will not render, far from its cause; the marker check is therefore
pinned at its exact length and offsets rather than a looser "contains OFL0X".
Tests:
- shortcuts/slides/slides_media_parent_type_test.go: 14-case pure-function
table (off-by-one length, prefix appearing mid-string, wiki placeholder),
a real-multipart Execute assertion across four token shapes, and the
+add-slide / +update-slide placeholder dry-run previews.
- tests/cli_e2e/slides/slides_image_upload_dryrun_test.go: five cases through
the built binary, covering every surface a local file can enter through.
- Verified non-vacuous: short-circuiting the office branch fails all three
package tests plus the e2e lane.
Evidence note: office_slide_file is confirmed accepted by upload_all, and the
symmetry with office_sheet_file is exact, but this has not been exercised
against a real imported-pptx presentation to confirm slide_file fails there.
|
||
|
|
79b8647196 |
feat(base): add template discovery and form question field reuse (#2340)
Co-authored-by: yballul-bytedance <273011618+yballul-bytedance@users.noreply.github.com> Co-authored-by: TRAE CLI <noreply@bytedance.com> |
||
|
|
42060154dd | feat(base): support button workflow bindings (#2437) | ||
|
|
e525beb8d6 |
feat(skills): unify meeting related skills (#2387)
* feat(skills): unify meeting guidance * fix(meeting): restore domain boundary guidance * docs(meeting): remove agent rollout qualification guidance * docs(meeting): front-load skill routing description * docs(meeting): refine identity and command guidance * docs(meeting): clarify identity and pagination guidance * docs(meeting): fix minutes todo detail command * fix(meeting): clarify artifact query routing * fix(meeting): improve live meeting skill recall * fix(qualitygate): generate valid minute token placeholders * fix(meeting): address unified skill review findings * docs(meeting): add minutes permission guidance * docs(lark-meeting): 更新SKILL.md并新增会议问答引导脚本 1. 优化SKILL.md表格排版与快速行动章节内容,新增批量获取当日会议脚本的使用说明 2. 新增meeting_qa_bootstrap.py脚本,实现一站式采集当日进行中、已结束会议及未来日程,生成可直接执行的命令引导 * docs(calendar): clarify today's meeting lookup * revert(meeting): remove meeting Q&A bootstrap guidance * fix(skills): register lark-meeting suite keywords --------- Co-authored-by: maozhixiang <maozhixiang@bytedance.com> |
||
|
|
da371dc242 |
feat(base): add dashboard and form share shortcuts (#2282)
- preserve explicit false values and validate partial share updates - add dry-run and deployment-gated live E2E coverage - document share routing in the bundled Base skill |
||
|
|
f28a418019 |
feat(base): add --position and statistics number_format to dashboard-block create/update (#2118)
* feat(base): add --position and statistics number_format to dashboard-block create/update
Add an optional top-level --position flag ({x,y,w,h} JSON, parsed but not
coordinate-validated, passed through as a sibling of name/type/data_config) and
optional statistics data_config.number_format ({formatName,precision}) with
light enum + 0-9 integer validation. Both are backward compatible. Body
assembly is unified in a shared buildDashboardBlockBody helper so DryRun and
Execute stay isomorphic. Adds toIntStrict for strict precision parsing, focused
helper/execute/dry-run tests, an E2E dry-run test, and syncs the lark-base
dashboard + data-config skill references.
Co-authored-by: TRAE CLI <noreply@bytedance.com>
* fix(base): validate number_format on update path and add symmetry tests
The dashboard-block-update command parsed data_config but never ran the
statistics number_format check, so an illegal formatName/precision slipped
through locally while create rejected it — violating the SSOT + backend-design
§4.5 promise of CLI-side interception on BOTH paths. Update has no --type flag
(block type is immutable) and intentionally skips strong type validation, so it
now reuses the shared validateNumberFormat sub-validator that
validateBlockDataConfig delegates to, keeping create/update symmetric without
demanding table_name/series on a number_format-only update.
Also: add tests for the --no-validate bypass on create+update, a combined
update carrying position + number_format + name, and extend the DryRun/Execute
body isomorphism assertion to the update path. Clarify the --position flag Desc
that coordinate bounds are advisory (not validated locally or server-side) and
sync the lark-base SKILL.md routing table for --position / number_format.
Co-authored-by: TRAE CLI <noreply@bytedance.com>
* fix(base): align dashboard block validation paths
Validate dashboard block JSON consistently across dry-run and execute paths, enforce statistics number_format boundaries, and add layout precision workflow coverage and documentation.
* fix(base): resolve dashboard layout doc contradictions and harden isomorphism test
Follow-up to the --position / number_format feature, addressing review findings.
Docs (SSOT contradictions):
- SKILL.md:135 and lark-base-dashboard.md still told agents that dashboard
shortcuts cannot set x/y/w/h and to offer auto-layout instead, which would
have left --position unreachable through the skill. Both statements are now
scoped to +dashboard-arrange, which genuinely cannot take coordinates.
- number_format was documented as supporting sub-field merge on update. That
contradicts the update Tips and lark-base-dashboard.md's own data_config
rule ("每个传入的字段内部是全量替换"). Documented as whole-key replacement
and made the update Tip example carry formatName back.
- Trimmed both reference sections: dropped the duplicated field table, the
restated validation blockquote, the standalone bash example and the 4-column
comparison table; kept the enum table and the two load-bearing gotchas.
Reformatted the example to the file's multi-line JSON style, and generalized
the 场景 3 --position argument to '{...}' like its neighbours.
Tests:
- The isomorphism check called buildDashboardBlockBody twice with the same
arguments, so it could never fail. Replaced with an end-to-end comparison of
the --dry-run preview body against the body captured from Execute; verified
it fails under single-path fault injection.
- The live workflow now updates to values distinct from the create call and
asserts them on read-back, instead of asserting substrings that the created
state already satisfied. Dropped the position read-back assertion: this
iteration does not contract get to echo coordinates.
- Filled in the two missing --no-validate cells (create data-config, update
position).
Cleanup:
- Deleted the inline DryRun closures; both commands now point at the
dryRunDashboardBlock* functions, matching the DryRun: dryRunX convention used
across the package and removing the second body-assembly site.
- Rewrote the update comment that referenced review-round codenames and an
external design doc section to be self-contained.
* fix(base): keep dashboard dry-run previews free of empty identifiers
Wiring the block create/update commands to the shared dryRunDashboardBlock*
functions routed them through dryRunDashboardBase, which Set all three
identifiers unconditionally. A create preview has no block_id yet, so it began
advertising "block_id": "" — an argument that reads as failed to resolve.
Skip empty values in the shared helper rather than special-casing create, which
also clears the same pre-existing noise from the +dashboard-arrange preview.
Pinned with a test asserting a create preview carries base_token and
dashboard_id and no block_id.
* fix(base): require complete --position objects and close the arrange/position gap
Round-2 review follow-up. Three findings, all one-liners in effect, that
compounded into a real failure mode: an agent told to "move this chart to the
right half" could send a partial position, have it accepted, and silently
resize the block to nothing — with no coordinate read-back to diagnose it.
- --position now requires all four of x/y/w/h. The server fills missing
coordinates with zero rather than leaving them alone, so a partial object is
a resize disguised as a move. Only the object's shape is checked; coordinate
VALUES stay unvalidated (out-of-range, negative and overlapping still pass
through) as documented. The check is semantic, so --no-validate skips it
while the JSON parse still runs — the same split the rest of this command
pair already uses. Rejected the alternative of validating ranges too: that
would contradict the documented dws-aligned pass-through contract.
- +dashboard-arrange's Tips now point at --position. The cross-reference was
one-directional: create/update told agents about arrange, but arrange — the
command an agent reaches for first when asked to "fix the layout" — never
mentioned that exact placement had become possible.
- Documented that coordinates are write-only this iteration. The reference doc
offered "replicate an existing dashboard's layout" as a use case while the
PR itself scopes out coordinate read-back, sending agents to look for x/y/w/h
that get/list do not return.
Also from the same review:
- The dry-run builders no longer discard buildDashboardBlockBody's error. It is
unreachable while Validate parses the same flags first, but returning nil
makes the runner fail loudly instead of previewing a body with a field
silently missing.
- Added precision cases that run through the real command. The existing
table-driven ones decode with UseNumber and hit toIntStrict's json.Number
branch, which production never takes — parseJSONObject uses a plain
json.Unmarshal, so precision always arrives as float64.
- coverage.md now says which four commands rest solely on the credential-gated
live test that has not been executed yet.
- Marked the number_format fallback claim as unverified against the backend.
* fix(base): close the position guard's null hole and the contract drift it left behind
Round-3 review follow-up. Two of these were introduced by the previous
follow-up commit, not by the original feature.
- The --position completeness guard only asked whether the key was present,
and a JSON null key IS present. `{"x":6,"y":null,"w":null,"h":null}` sailed
through the very check meant to stop it — the exact scenario the guard's own
comment describes. Each coordinate must now actually decode as a number, so
null, strings, objects and bools are rejected alongside missing keys. This is
still a shape check: out-of-range, negative and fractional values keep
passing through as documented. The package's neighbours (`cfg["text"].(string)`,
`table_name`) already validate required fields with a type assertion; this
was the one place that did not. Mutation-verified: reverting the assertion
turns the explicit-nulls case red.
- coverage.md claimed `+dashboard-block-get` "reads back position" while the
test it cites deliberately stopped asserting coordinates — a line the
previous commit invalidated and did not update. It now says number_format
only. The `+dashboard-block-update` row also claimed dry-run coverage for
number_format that only the unexecuted live test provides.
- dashboard-block-data-config.md still said the update path does no local
validation, which commit bb7d8fbc made false in this same PR. An agent
reading it would not expect exit 2 and might reach for --no-validate, which
now also disables the position guard.
Also from that review:
- --no-validate's flag Desc only mentioned data_config; it silently covers the
--position check too. Said so, in both commands.
- Four places stated unverified backend behaviour as fact — including a claim
that the server zeroes missing coordinates, which was the guard's entire
premise, and a "backend defaults to digital" line 23 lines above a blockquote
saying that very fallback was unverified. All reworded to what is actually
known; the guard's rationale is now stated in terms of the request we send.
- E2E dry-run assertions were whole-output substring matches (`"w": 6` could
match anywhere); switched to clie2e.DryRunGet path assertions like the
sibling suites, which also lets them prove position is a top-level sibling
rather than nested in data_config.
- Documented that formatName is case-sensitive, unlike rollup which is
normalized — same object, two conventions, worth saying out loud.
- The --position canonical rewrite's comment claimed it kept Validate/DryRun/
Execute consistent; they re-parse anyway. Its real job is folding @file input
inline so the two paths cannot read a changed file. Comment now says that.
- Named buildDashboardBlockBody's bool at the call sites; covered all three
branches of the identifier skip, not just block_id.
* fix(base): stop dry-run previews leaking route templates; finish the unverified-claim sweep
Round-4 review follow-up. Both findings trace back to earlier follow-up commits
rather than the original feature, and both are the same failure shape: fixing
the instance instead of the class.
- 68bdaccd made dryRunDashboardBase skip empty identifiers, but Set() doubles as
the substitution source for :param placeholders in the URL. Skipping a
declared-but-empty identifier therefore printed the raw route template —
`.../blocks/:block_id` — while also removing `"block_id": ""`, the one signal
that told the caller their argument was empty. An agent whose `$BLOCK_ID` did
not expand would see a preview that looks like the CLI failed to substitute,
with nothing pointing at the real cause. The condition is now whether the
command declares the flag, which is what the comment claimed all along: create
genuinely has no block-id, and that is the case worth omitting.
Not fixed here: a declared-but-empty required identifier still reaches the
wire as a request to the collection endpoint (`baseV3Path` drops empty
segments). That predates this PR and spans the whole base package — worth its
own change rather than guarding two commands and leaving nine inconsistent.
- The isomorphism test only compared bodies, so a preview could target a
different endpoint than Execute and still pass. It now compares method and URL
as well, and rejects any leftover ":" placeholder — that is the mechanism that
would have caught the above.
- 82f72540's message claimed all four unverified backend statements had been
reworded; five survived, three of them in `--help`, where the --position Desc
said server-side acceptance was unverified two lines above a Tip asserting
overlaps are not server-checked. All five now match the wording already used
in lark-base-dashboard.md, and the PR body Summary no longer contradicts its
own Known limitations.
The rejected-alternative for the first item: guarding empty required identifiers
in Validate would be the root-cause fix, but applying it to the two commands
this PR owns while nine sibling dashboard commands keep the old behaviour trades
one inconsistency for another.
* fix(base): stabilize dashboard block validation inputs
* docs(base): clarify precise dashboard layout workflow
* docs(base): align dashboard live coverage status
* docs(base): soften absolute dashboard layout phrasing in skill
Replace "run exactly once / stop" wording for +dashboard-arrange and
--position with intent-based guidance (prefer whole-dashboard arrange,
generally no need to re-read position) so the skill routes agents away
from per-block churn and useless retries without forbidding legitimate
user-driven follow-up adjustments.
Co-authored-by: TRAE CLI <traecli@bytedance.com>
* docs(base): clarify dashboard layout guidance
* docs(skills): move dashboard layout guidance to reference
* docs(base): verify dashboard number format defaults
---------
Co-authored-by: wanglei.75 <wanglei.75@bytedance.com>
Co-authored-by: TRAE CLI <noreply@bytedance.com>
Co-authored-by: TRAE CLI <traecli@bytedance.com>
|
||
|
|
ca35f60616 |
fix(apps): make cache-clear ask first, and make apps failures classifiable (#2415)
* docs(skills): require explicit confirmation before apps +cache-clear
Asked to clear an app's online cache, an agent read `Risk: high-risk-write` from
--help and then supplied `--yes` itself on the first call, wiping production
cache without ever hitting the confirmation gate.
The CLI gate is fine: no --yes -> exit 10 confirmation_required, and --dry-run ->
exit 0 without triggering it. The wording was not. It only forbade appending
`--yes` *after* an exit-10, and said "已明确授权可直接带 --yes" without defining
authorization — so "clear my cache" read as authorization.
- `+cache-clear` gets a CAUTION block: never self-supply `--yes` on the first
call; without confirmation, either --dry-run or ask, then stop and wait. exit
10 is not a signal to retry with --yes.
- Add a zero-ambiguity table separating a *request* to clear ("clear the online
cache") from a *confirmation* ("我确认清 dev"), so blocking the accidental wipe
does not also kill the cases that were already correct: an explicit
confirmation still goes straight to `--yes`, and a request with no environment
named still has to ask instead of picking one.
- Note that online needs a confirmation phrase even when named explicitly.
`+cache-delete` gains the response field an agent has to read
(`deleted_key_count`): 0 means the key never existed, not "deleted
successfully", plus the get -> delete -> get chain needed to prove a delete took
effect — a single miss afterwards cannot tell the two apart.
SKILL.md: add +cache-clear to 禁止预授权判定底线, the one list a pre-authorized
run cannot skip; a reference-level rule alone would be bypassed there. The
routing table is left alone — no other row annotates risk, including
+file-delete, +role-delete and +member-remove.
* fix(apps): stop attaching request-shaped hints to precondition failures
`+db-execute` against a tenant that never activated Miaoda returns code 221800
"miaoda UAT not activated" with the hint "verify table/column names with
`+db-table-get` ... target the dev database with --environment dev". Neither step
can help: the failure is tenant-level, so a caller following the hint loops over
table lookups and env retries that fail identically.
Two causes. 221800 was unregistered, so it degraded to api/unknown — nothing in
the envelope distinguished "your tenant is not activated, stop" from "your SQL
was wrong, fix it and retry". And withAppsHint filled the caller's hint whenever
the server sent none, without looking at what failed: the hints are
command-scoped ("verify --app-id", "verify table/column names", "list releases"),
so every one of them describes the request, and the request is exactly what
failed_precondition says was fine.
Register 221800 as validation/failed_precondition (same shape as 400002465 "app
has no database yet") and gate the hint fallback on the subtype.
Blast radius is two codes, since that is all the subtype covers here:
- 221800 — now withheld; message and code still carry the meaning.
- 400002655 "no running container" — only when it reaches a non-observability
command; the observability pair rewrites it first, and "verify --app-id" was
never the fix for an undeployed app.
400002465 / 500002759 are intercepted by the isAppNoDatabaseError branch above
the gate, and 400002479 is served by withDBSyncHint, which does not delegate
here. The other 78 call sites take the original path for every input.
Gate on the one subtype, not on Category: this package asserts on purpose that an
authentication failure on +role-list (99991663) keeps the app-access hint and a
503 on credential issuance keeps the developer-access hint. Those hints are broad
enough to survive a caller-standing failure; only the precondition class is
misdescribed by construction. A test pins that, so widening the gate to Category
fails loudly instead of silently dropping those hints.
The gate is asserted on the real classification path (BuildAPIError -> the code
table -> withAppsHint), not only on a hand-built Problem. Constructing
SubtypeFailedPrecondition directly feeds the gate the input it wants and passes
whether or not 221800 is registered, so the registration itself has to be part of
what the test covers.
No recovery hint for 221800 — the activation path is a product procedure, and
guessing one is what made this failure misleading in the first place.
* fix(apps): classify file-storage and app-level failures
Five Spark business codes reached the CLI unregistered, so every one of them came
out as api/unknown with exit 1: a caller could not tell "your app id is wrong"
from "you lack permission" from "the upstream is having a bad minute", and the
exit code offered no way to branch either.
400002484 app not found -> validation/invalid_argument exit 2
400002467 no admin/developer perm -> authorization/permission_denied exit 3
500002761 ditto, pre-4xx renumber -> same
400000034 file not found/no access -> api/not_found exit 1
500000034 ditto, pre-4xx renumber -> same
400002467 is not file-specific: db commands (+db-table-list, +db-table-get,
+db-quota-get, +db-changelog-list) return it for an app the caller cannot access,
so registering it fixes both domains at once.
400002484 covers a well-formed id that does not exist AND a malformed one
("notanappid", "app_1" return it too), so the argument itself is the failure ->
invalid_argument, whose exit 2 separates "you passed the wrong id" from an
upstream fault. Environments that have not picked it up answer with 400002465
instead, conflating it with "app has no database yet"; the CLI cannot tell those
apart on the old code, so nothing here keys on that.
Both the current and the pre-4xx number are registered for each file failure.
The domain is moving its client-class errors from the 5xxxxxxxx band into
4xxxxxxxx, rolled out per environment, so both are live at once and dropping the
old one would silently return the un-migrated half to api/unknown — the same trap
that made the no-database recovery flow disappear when the server renumbered it
(see appNoDatabaseCode). The new number is not derivable from the old either:
500002761 became 400002467, tail digits included.
No hints added: permission_denied already has framework recovery wording, and a
domain-specific one would have to invent a remedy.
|
||
|
|
755daa4de3 | feat: add minutes transcript degradation logic (#2404) |