* refactor: retire legacy error envelopes and enforce typed contract
Consolidate all command error reporting onto the typed errs.* contract, remove
the legacy error surface that predated it, and tighten the lint guards so the
contract holds across the whole repository going forward.
Every failure now reaches stderr as one envelope shape: a category, an
optional subtype, a human- and agent-readable message, and a recovery hint,
with invalid parameters listed under `params`. The legacy ExitError envelope,
its constructors, and the boundary bridge that promoted untyped config and
authorization errors are deleted, leaving a single path from error to wire.
Predicate commands keep their silent-exit behavior through a dedicated signal
that carries only an exit code.
Infrastructure paths that still emitted ad-hoc envelopes — flag parsing,
unknown commands and subcommands, plugin and policy guards, confirmation
prompts, and auth/config failures — now classify into the same taxonomy.
Business, API, auth, and config exit codes are preserved; the one behavioral
change is that Cobra usage failures (missing required flag, unknown command,
bad arguments) now emit the typed validation envelope and exit 2, matching the
explicit flag and subcommand guards, instead of Cobra's plain-text exit 1.
Enforcement is repo-wide rather than per-path:
- The errscontract guards run by default everywhere instead of through a
migration allowlist, so legacy envelopes cannot be reintroduced anywhere.
- errorlint runs across the whole repository: every error wrap must use %w and
every comparison must use errors.Is/errors.As, so interior wraps stay legal
but can no longer break the chain the typed boundary relies on.
- The errs-no-bare-wrap guard is keyed by structural prefix instead of an
explicit per-domain allowlist, so new shortcut domains are covered without
editing a list. It runs where forbidigo is enabled (the shortcut domains and
the auth/config/service command groups); repo-wide chain integrity for the
remaining command paths is carried by errorlint above.
* test: align cli_e2e success assertions to the ok envelope
The api and service success path now emits the {"ok":true} envelope, so the
cli_e2e workflow assertions that still expected the old {"code":0} shape via
AssertStdoutStatus(t, 0) fail once they run with live credentials. Switch those
workflow assertions to AssertStdoutStatus(t, true); the fake-payload helper test
in core_test.go keeps its code-shape assertion.
* refactor(sheets): rebuild lark-sheets on sheet-skill-spec canonical + One-OpenAPI
Restart lark-sheets as a spec-driven downstream. Skill content (SKILL.md
and 16 references covering 13 operations skills + 3 workflow skills,
including the standalone filter-view skill) is mirrored from the
sheet-skill-spec canonical-spec; do not hand-edit, change upstream and
rerun npm run sync:consumers.
Drop the 11 legacy shortcut sources (spreadsheet / sheet management,
cell ops, dropdown, filter-view, float image, etc.) and 10 associated
tests. Wire up the new sheet_ai/v2 One-OpenAPI single entry that
dispatches by tool_name with JSON-string input/output, and land the
first canonical shortcut +workbook-info as a template that exercises
the public token XOR pair, Risk tiering, and zero-side-effect DryRun.
sheet_ai_api.go provides callTool / invokeToolDryRun and bypasses
runtime.CallAPI's silent swallowing of non-envelope responses so
gateway and business errors from the new endpoint surface precisely.
The remaining 55 shortcuts will be designed and landed separately,
canonical skill by canonical skill.
* feat(sheets): implement lark_sheet_workbook shortcuts (B1)
Land the 8 modify_workbook_structure shortcuts that round out the
lark_sheet_workbook canonical skill alongside the existing +workbook-info:
+sheet-create / +sheet-delete / +sheet-rename / +sheet-move / +sheet-copy
/ +sheet-hide / +sheet-unhide / +sheet-set-tab-color. All eight call
modify_workbook_structure via the One-OpenAPI invoke_write endpoint,
dispatched by the `operation` enum.
Helpers in helpers.go grow publicSheetFlags() / resolveSheetSelector() /
sheetSelectorForToolInput() / sheetSelectorPlaceholder() so future
sheet-level shortcuts share the public --sheet-id / --sheet-name XOR
treatment. +sheet-create intentionally drops the sheet selector pair since
create has no existing-sheet anchor (matches the spec fix in
tool-shortcut-map.json).
+sheet-delete is the first high-risk-write shortcut in the canonical
package; the framework requires --yes (exit code 10 otherwise).
+sheet-move's tool requires source_index in addition to target_index. The
CLI accepts an optional --source-index override and falls back to a
single get_workbook_structure read to derive it (and to resolve sheet_id
from --sheet-name). DryRun stays network-free by rendering <resolve>
placeholders for any field that would need that read.
* feat(sheets): implement lark_sheet_sheet_structure shortcuts (B2)
Add 8 shortcuts under the lark_sheet_sheet_structure canonical skill:
+sheet-info (get_sheet_structure) plus +dim-insert / +dim-delete /
+dim-hide / +dim-unhide / +dim-freeze / +dim-group / +dim-ungroup
(modify_sheet_structure, dispatched by operation enum).
Two reusable conversion helpers cover the impedance mismatch between
the CLI surface and the tool input:
- dimRange / dimPosition translate the CLI's 0-based exclusive-end
range into the tool's 1-based A1 notation. row 5..8 becomes
position "6" + count 3 (insert) or range "6:8" (range ops); column
26..29 becomes "AA:AC".
- infoTypeFromInclude maps the fine-grained --include vocabulary
(row_heights / col_widths / merges / hidden_rows / hidden_cols /
groups / frozen) to the coarse info_type enum the tool accepts;
mixed categories collapse to "all".
+dim-delete is high-risk-write (irreversible row/column removal).
+dim-freeze --count 0 auto-dispatches to operation=unfreeze. +dim-group
accepts --depth for forward-compat with a future server-side nested
group endpoint but does not pass it through today.
* feat(sheets): implement read_data / search_replace / write_cells shortcuts (B3)
Land 11 shortcuts across three canonical skills:
- lark_sheet_read_data (3): +cells-get / +csv-get / +dropdown-get
- lark_sheet_search_replace (2): +cells-search / +cells-replace
- lark_sheet_write_cells (6): +cells-set / +cells-set-style / +csv-put
/ +dropdown-set / +dropdown-update / +dropdown-delete
+dropdown-get reads the data_validation field via get_cell_ranges with
the range carrying its own sheet prefix (no --sheet-id needed). The
fine-grained --include vocabulary (value / formula / style / comment /
data_validation) maps to the tool's coarse include_styles bool plus
value_render_option enum. +csv-get's --include-row-prefix=false strips
the [row=N] prefix client-side because the tool only emits the
annotated form.
+cells-search / +cells-replace flatten the tool's options sub-object
into four independent flags (--match-case / --match-entire-cell /
--regex / --include-formulas) per the flat-flag rule, then repack them on the way
in.
+cells-set takes a raw --data JSON body whose `cells` array must match
the --range dimensions. +cells-set-style fans a single --style block
out to every cell in the range via a new fillCellsMatrix helper; the
range parser (rangeDimensions / splitCellRef / letterToColumnIndex)
only accepts rectangular A1:B2 forms — whole-column / whole-row need
sheet totals and are deferred.
+dropdown-set fans the validation block out to one range; +dropdown-
update / +dropdown-delete iterate sheet-prefixed --ranges and call
set_cell_range sequentially (partial failure leaves earlier ranges
already mutated; the Tip calls this out). +dropdown-delete is
high-risk-write and requires --yes.
+cells-set-image stays deferred to the cli-only batch (needs the
shared local-file upload helper alongside +workbook-create / +dim-move
/ +workbook-export).
* refactor(sheets): move +dropdown-update / +dropdown-delete to lark_sheet_batch_update
Follow-up to B3 after the spec re-mapped these two shortcuts to the
batch_update tool (atomic multi-range CRUD) instead of fan-out via
set_cell_range. Drop their Go implementations + helper validateDropdownRanges
+ splitSheetPrefixedRange from lark_sheet_write_cells.go and remove the
registrations from Shortcuts(); the shortcuts will reappear under
lark_sheet_batch_update during B7.
Also pull in the re-rendered reference docs:
- skills/lark-sheets/references/lark-sheets-write-cells.md
- skills/lark-sheets/references/lark-sheets-batch-update.md
* feat(sheets): implement lark_sheet_range_operations shortcuts (B4)
Land 8 shortcuts across four canonical tools:
- clear_cell_range → +cells-clear (high-risk-write)
- merge_cells → +cells-merge / +cells-unmerge
- resize_range → +dim-resize
- transform_range → +range-move / +range-copy / +range-fill / +range-sort
Three CLI↔tool vocabulary bridges live in this file:
- +cells-clear: --scope content normalizes to the tool's clear_type
"contents" (singular/plural spec mismatch is absorbed in the CLI).
- +dim-resize: --size <px> wraps as resize_{height,width}:{value:N};
--reset wraps as {reset:true}. The two flags are mutually exclusive
and at least one is required.
- +range-fill: CLI's five-valued --series-type collapses to the tool's
binary fill_type — `copy` → "copyCells", anything else → "fillSeries"
(the actual series progression is inferred server-side from the
seed cells in --source-range).
- +range-copy: --paste-type {values, formulas, formats} maps to the
tool's {value_only, formula_only, format_only}; "all" omits the
field entirely so the server applies its default.
+cells-clear is the second high-risk-write shortcut in the package;
the framework enforces --yes with exit code 10 as usual.
* feat(sheets): implement object-list shortcuts (B5)
Land 7 read shortcuts, one per object skill — chart / pivot table /
conditional format / filter / filter view / sparkline / float image. All
share the same shape (public sheet selector + optional <obj>-id filter)
so they're declared via newObjectListShortcut + an objectListSpec.
Notes:
- +cond-format-list exposes --rule-id, which is renamed to
conditional_format_id on the wire (the tool's full field name).
- +sparkline-list exposes --group-id (the higher-level handle); the
tool also accepts sparkline_id, intentionally not surfaced.
- +filter-list takes no id filter — at most one sheet-level filter
per sheet, so the listing is already unique.
- +filter-view-list is `cli_status: cli-only` but get_filter_view_objects
is in mcp-tools.json and dispatches through the same One-OpenAPI
endpoint; no special path required.
* feat(sheets): implement object CRUD shortcuts (B6)
Land 21 shortcuts — three (create / update / delete) per object skill —
backed by the manage_<obj>_object tools dispatched on the operation
enum. Five standard objects (chart / cond-format / sparkline /
float-image / filter-view) share an objectCRUDSpec factory; pivot and
filter are special-cased.
Shared wire contract:
excel_id + sheet_id|sheet_name + operation + [<obj>_id] + [properties]
CLI --data is passed through as the tool's `properties` field as-is, so
callers shape it per each object's spec doc.
Special cases:
- pivot adds optional --target-sheet-id / --target-position on create
(siblings of properties, not inside it).
- cond-format exposes --rule-id (short CLI name) wired to the tool's
conditional_format_id on the wire.
- sparkline uses --group-id (higher-level object handle) instead of
sparkline_id.
- filter has no separate id flag — at most one filter per sheet, so
filter_id is implicit. +filter-create promotes --range to a first-
class flag (instead of burying it inside --data).
- filter-view CRUD are `cli_status: cli-only` but
manage_filter_view_object is in mcp-tools.json, so they go through
callTool / One-OpenAPI alongside everything else.
All delete shortcuts are high-risk-write and require --yes.
* feat(sheets): implement lark_sheet_batch_update shortcuts (B7)
Land 4 shortcuts that all funnel through the batch_update tool's atomic
operations array:
- +batch-update raw passthrough; --data carries the full
{ operations: [{tool, params}, ...] } payload
plus optional continue_on_error. high-risk-write
since the caller may stuff anything inside.
- +cells-batch-set-style --data is [{ranges, style}, ...]; CLI flattens
each (entry × range) pair into a set_cell_range
op with a fan-out cells matrix carrying
cell_styles + border_styles.
- +dropdown-update --ranges + --options (+ --colors / --multiple /
--highlight) — installs/replaces one dropdown
across many ranges, each becoming a separate
set_cell_range op with data_validation in cells.
- +dropdown-delete --ranges — clears data_validation across many
ranges (high-risk-write).
Default is strict transaction: if any sub-tool fails the whole batch rolls
back. +batch-update exposes --continue-on-error to flip the policy; the
three fan-out shortcuts leave it strict (they're meant to be all-or-nothing).
Reinstates validateDropdownRanges + splitSheetPrefixedRange that were
removed during B3 → B7 relocation.
* feat(sheets): implement cli-only shortcuts (B8) — 70/70 complete
Land the four cli-only shortcuts that can't route through the One-OpenAPI
dispatcher (their backing capabilities aren't in mcp-tools.json):
- +workbook-create POST /open-apis/sheets/v3/spreadsheets
+ optional set_cell_range follow-up that zips
--headers and --data into the first sheet starting
at A1.
- +workbook-export POST /open-apis/drive/v1/export_tasks (type=sheet)
→ poll /export_tasks/:ticket up to ~30s
→ optional GET /export_tasks/file/:file_token/download.
CSV mode requires --sheet-id (single sheet export).
- +dim-move POST /open-apis/sheets/v2/spreadsheets/:token
/dimension_range
CLI is 0-indexed inclusive (--start / --end); the v2
endpoint expects half-open [startIndex, endIndex)
so the body uses endIndex = --end + 1. --sheet-name
is resolved client-side to sheet_id via
lookupSheetIndex when needed.
- +cells-set-image common.UploadDriveMediaAll
(parent_type=sheet_image, parent_node=token)
then callTool set_cell_range with cells carrying
rich_text: [{type:"embed-image", attachment_token, attachment_name}].
--range must be exactly one cell.
All four use runtime.CallAPI / DoAPI directly; only +cells-set-image
combines a legacy upload with the new One-OpenAPI for the second step
(set_cell_range is in mcp-tools.json so callTool is the right path).
This closes the migration: 70 shortcuts × 17 canonical skills × matching
the sheet-skill-spec v0.5.0 tool-shortcut-map.
* test(sheets): cover all 70 shortcuts with dry-run + execute-path tests
Twelve _test.go files alongside the implementation, mirroring the legacy
package's coverage style:
- testhelpers_test.go shared rig: TestFactory + Mount + dry-run
capture + JSON-input decode + envelope helpers.
- lark_sheet_*_test.go one test file per implementation file (9
files), table-driven dry-run cases per shortcut
plus targeted validation guards.
- execute_paths_test.go end-to-end execute paths via httpmock stubs.
Covers callTool unwrap, JSON-string output
decoding, two-step lookup (+sheet-move),
batch_update fan-out, dropdown atomic writes,
and the legacy OAPI shortcuts (+workbook-create,
+dim-move) including CLI inclusive → API
half-open index conversion.
Test coverage on the sheets package is 60.5 % of statements with -race
clean, meeting the dev manual's ≥ 60 % patch-coverage gate.
* refactor(sheets): inline cli-only shortcuts into their canonical skill files
Two naming cleanups:
- lark_sheet_cli_only.go is gone. The four shortcuts it grouped
(+workbook-create / +workbook-export / +dim-move / +cells-set-image)
were bundled by their implementation pattern (legacy OAPI direct
calls) rather than by canonical skill. The whole sheets package IS
the CLI implementation, so "cli only" wasn't a meaningful grouping
at the Go layer. Each shortcut now lives next to its skill peers:
+workbook-create / +workbook-export → lark_sheet_workbook.go
+dim-move → lark_sheet_sheet_structure.go
+cells-set-image → lark_sheet_write_cells.go
Per-skill shortcut counts now match tool-shortcut-map.json exactly
(workbook: 11, sheet_structure: 9, write_cells: 5). Helpers
(buildInitialFillInput, pollExportTask, downloadExportFile,
dimMoveBody) move with their shortcuts; nothing else in the package
referenced them.
- testhelpers_test.go → helpers_test.go. The _test.go suffix already
conveys "test"; the leading "test" was redundant. Matches the
helpers.go naming convention.
Behavior unchanged. go test -race -cover stays at 60.5 %.
* refactor(sheets): sync shortcut flags with sheet-skill-spec v0.5.0
Upstream hoisted a batch of high-frequency scalar fields out of --data
into independent flags and renamed several composite-JSON flags to
match their semantic content. CLI catches up.
Renames (drop-in, same payload semantics):
- +cells-replace --replace → --replacement
- +cells-set --data → --cells
- +workbook-create --data → --values
- +batch-update --data → --operations (now a bare array;
still accepts the envelope form for
back-compat with continue_on_error)
Flat-flag hoists out of --style / --data:
- +cells-set-style / +cells-batch-set-style
--style JSON drops; replaced by 11 flat style flags
(--background-color / --font-color / --font-size / --font-style /
--font-weight / --font-line / --horizontal-alignment /
--vertical-alignment / --word-wrap / --number-format) plus
--border-styles for the one field that's still nested. Both
shortcuts share styleFlatFlags() + buildCellStyleFromFlags().
- +cells-batch-set-style also drops the [{ranges, style}] array shape
in favor of one --ranges + the same flat style flags applied to
all of them.
Object CRUD --data → --properties everywhere (chart / pivot / cond-format
/ filter / filter-view / sparkline / float-image). Per-skill scalar
hoists merged into properties via an enhanceCreate/UpdateInput callback:
- +pivot-create adds --source (required), --range
(and continues to expose --target-sheet-id /
--target-position at top level)
- +cond-format-{create,update}
adds --rule-type (enum) + --ranges (JSON array);
merged into properties.rule.type and
properties.ranges respectively
- +filter-view-{create,update}
adds --view-name and --range; both override
their properties.* counterparts
- +filter-update adds first-class --range (was buried in --data)
Float-image is fully hoisted — no --properties flag at all. Ten flat
flags (--image-name / --image-token | --image-uri / --position-row /
--position-col / --size-width / --size-height / --offset-row /
--offset-col / --z-index) compose the properties block. Implemented as
its own factory (newFloatImageWriteShortcut) since it diverges from the
shared CRUD spec.
Tests track every flag renamed and add explicit cases for the new flag
combos. go test -race -cover stays at 60.3 %.
* refactor(sheets): align batch_update + cells-set with synced reference docs
Sync to upstream reference doc updates for 9 skills:
- batch_update sub-ops: rewrite wire fields tool/params -> tool_name/input
in CellsBatchSetStyle and DropdownUpdate/Delete fan-out (the actual
server contract per Schemas section); update --operations flag desc
and tests.
- +cells-set --cells: accept bare 2D matrix [[{cell},...],...] instead
of envelope {"cells":[[...]]}; spec example shows bare-array form.
- sparkline createDataDesc enum: win_loss -> winLoss (camelCase).
All other doc changes (float-image flat flags, cond-format
--rule-type/--ranges, pivot create-only --source/--range, filter /
filter-view extra flags, chart --properties) were already aligned in
commit ce33315.
* fix(sheets): repair cells-set-image rich_text embed payload
The server rejected set_cell_range calls from +cells-set-image with three
distinct errors: missing "text" property, missing image_width/image_height,
and unknown attachment_token field. Realign the rich_text element to the
embed-image schema (text/image_token/image_width/image_height) and decode
PNG/JPEG/GIF dimensions from the local file before the write.
* refactor(sheets)!: split +dim-resize into +rows-resize and +cols-resize
Sync to upstream spec change that splits the legacy +dim-resize shortcut
into +rows-resize and +cols-resize. Reasoning is that row vs column
resize has divergent semantics (only rows support auto-fit) and the
shared --dimension flag was hiding that.
Behavior changes (BREAKING):
- +dim-resize is removed; use +rows-resize or +cols-resize.
- --dimension and --reset flags are gone.
- --type enum replaces --size/--reset:
pixel (requires --size)
standard (reset to sheet default; no --size)
auto (auto-fit row height; +rows-resize only)
- --end is now inclusive (was exclusive). Old "--start 0 --end 5"
(5 rows) becomes "--start 0 --end 4".
- Wire payload for resize_height / resize_width changes from
{value: N} | {reset: true} to {type: "pixel", value: N} |
{type: "standard"} | {type: "auto"}.
Tests cover both shortcuts across pixel / standard / auto and the
new guard surface (--type pixel needs --size; standard/auto reject
--size; +cols-resize rejects --type auto; --end < --start).
Also pulls in synced reference docs for 5 skills (batch-update,
core-operations, range-operations, sheet-structure, visual-standards)
that update prose mentions of +dim-resize.
* feat(sheets): add --print-schema runtime introspection for composite JSON flags
Composite JSON flags (--cells / --properties / --operations /
--border-styles / --sort-keys / --options) carry non-trivial structured
payloads. Reference docs cover top-level fields but agents writing
those flags often need the full JSON Schema to build a valid payload.
This adds a system-level introspection contract so any shortcut whose
flags are tracked upstream can serve its schemas locally:
lark-cli sheets <shortcut> --print-schema --flag-name <name>
lark-cli sheets <shortcut> --print-schema # list flags
The schema data is embedded at build time from a synced artifact
(shortcuts/sheets/data/flag-schemas.json). Upstream is the source of
truth — never hand-edit the JSON; update the source Base table and
rerun the sheet-skill-spec sync.
Framework changes (shortcuts/common):
- types.go: Shortcut gains an opt-in PrintFlagSchema hook
(flagName -> bytes/error). When non-nil the framework auto-injects
--print-schema / --flag-name and short-circuits Validate/Execute.
- runner.go: register the two system flags when PrintFlagSchema is
set; intercept in runShortcut before identity/scope/config so
pure-local lookups don't trigger auth or network. Install a
PreRunE that relaxes cobra's required-flag gate when
--print-schema is set, since asking for a schema shouldn't need
unrelated required flags.
Sheets surface (shortcuts/sheets):
- flag_schema.go (new): go:embed data/flag-schemas.json; expose
printFlagSchemaFor(command) closure. When flagName is empty it
emits a JSON listing of introspectable flags for discovery;
otherwise it returns the schema subtree as pretty JSON.
- flag_schema_test.go (new): cover embed parsing, listing /
by-name lookup, unknown-flag error path, registration via
Shortcuts(), and the full system-flag short-circuit through
cobra (required flags relaxed, schema printed on stdout).
- shortcuts.go: Shortcuts() now wraps shortcutList() and attaches
PrintFlagSchema to every command present in flag-schemas.json,
so shortcuts opt in by being listed upstream — no per-shortcut
boilerplate.
- data/flag-schemas.json (new, synced from sheet-skill-spec):
19 entries, schema_version "2". Generated upstream from the Lark
Base source-of-truth (see sheet-skill-spec
scripts/fetch_cli_flag_schema_map.mjs); ships only per-flag
subtrees (not the full mcp-tools.json) to keep tool internals
out of the open-source repo.
Skill docs (skills/lark-sheets):
- SKILL.md: system-flag table gains --print-schema / --flag-name and
an "Agent 使用提示" note steering agents to prefer --print-schema
over guessing JSON shape from the cheatsheet.
- references/*.md: regenerated by upstream sync (Schemas-section
boilerplate updated, plus accumulated upstream prose refinements).
* docs(sheets): remove sandbox references and normalize tool names to CLI shortcuts
Replace export_sheet_to_sandbox / import_sandbox_to_sheet / doubao_code_interpreter
with local-script + batch csv-get/csv-put workflows; unify legacy MCP tool names
(set_cell_range, get_range_as_csv, etc.) to CLI shortcut format (+cells-set, +csv-get).
* feat(sheets): add flag-descriptions.en.json and wire applyFlagDescs into Shortcuts()
Embed data/flag-descriptions.en.json (synced from upstream spec) and
apply it at shortcut assembly time so every Flag.Desc is sourced from
the canonical JSON rather than hardcoded Go strings. Existing hardcoded
Desc values serve as fallback for flags not yet in the JSON.
Also sync reference doc updates from upstream.
* feat(shortcuts): support int64 and float64 flag types
Flag.Type previously could not express non-integer numbers. Add int64
and float64 cases to flag registration plus Int64/Float64 runtime
accessors.
* refactor(sheets): build shortcut flags generically from flag-defs.json
Replace flag-descriptions.en.json with the richer flag-defs.json (full
flag definitions: type / default / enum / input / hidden / required /
kind) synced from sheet-skill-spec. Add flagsFor(command) to materialize
each shortcut's []common.Flag straight from the JSON, skipping
system-kind flags the framework injects.
Migrate every sheets shortcut (including the CRUD/list/dim/merge/
visibility factories) to Flags: flagsFor("+command"), dropping all
hand-written flag literals plus the now-dead publicTokenFlags /
publicSheetFlags / styleFlatFlags helpers and enum vars. A coverage test
locks the Go-flags-match-JSON contract.
Align Go with the new spec where they diverged: +cells-get --ranges →
--range, font-size int → float64, +filter-view-create --range now
required, +sheet-create row/col-count defaults 200/20.
* docs(sheets): sync +batch-update CLI override schema (shortcut/input form)
Pulled from sheet-skill-spec:
- skills/lark-sheets/references/lark-sheets-batch-update.md: --operations
now documents the {shortcut, input} form; tool_name references gone
- shortcuts/sheets/data/flag-schemas.json: --operations resolves to the
CLI-side array<{shortcut(enum), input}> schema, sourced from spec's
canonical-spec/tool-schemas/cli-schemas.json (cli: prefix). +dropdown
--options also drilled one level deeper
NOTE: the binary still raw-passes --operations to MCP batch_update which
expects {tool_name, input}. A follow-up will add a shortcut→tool_name
translation layer (with per-shortcut operation field) before the docs
become actionable.
* feat(sheets): translate +batch-update sub-ops {shortcut,input} → MCP shape
Users now hand +batch-update --operations a CLI-shape array
([{shortcut, input}, ...]) and the binary translates each sub-op to the
underlying MCP batch_update shape ({tool_name, input(+operation)}) via
a new dispatch table in shortcuts/sheets/batch_op_dispatch.go.
Dispatch table covers 50 batchable write shortcuts. Excluded by design:
- all read ops
- fan-out wrappers (+batch-update self, +cells-batch-set-style,
+dropdown-update, +dropdown-delete) — nesting these = nested batch
- +dim-move — single shortcut uses legacy v2 /dimension_range endpoint,
not MCP, can't be batched
- +cells-set-image — multi-step image upload, not atomic-batch friendly
- +workbook-create — new workbook, not batch-on-existing semantics
Translator also rejects sub-ops that hand-fill input.operation (implied
by shortcut name) or input.excel_id / spreadsheet_token / url (set
once at +batch-update top level).
+dim-freeze always injects operation=freeze; the count==0 unfreeze
path of the single shortcut is intentionally not supported in batch —
callers should use the single shortcut for unfreeze.
Tests cover: end-to-end translation, --continue-on-error propagation,
13 rejection cases (banned shortcuts, malformed shapes, reserved keys).
Sync'd from sheet-skill-spec: skills/lark-sheets/references/
lark-sheets-batch-update.md + shortcuts/sheets/data/flag-schemas.json
pick up the corrected enum (+cells-set-style / +dropdown-set added,
+dim-move removed).
* fix(sheets): make +batch-update sub-ops reuse standalone flag→body translators
Sub-ops previously near-passed-through their input, so any shortcut whose
standalone translator renames fields broke inside a batch: +range-copy lost
range/destination_range (transform_range errored "range missing") and
+rows-resize lost range/resize_height ("No resize operation specified").
Introduce a flagView interface (satisfied by *common.RuntimeContext) and a
map-backed mapFlagView, then route every batchable sub-op through the SAME
*Input builder the standalone shortcut uses. mapFlagView seeds flag-defs.json
defaults for value reads while keeping Changed() user-driven, so a sub-op body
is byte-identical to the standalone body — locked by a batch-vs-standalone
contract test over all ~40 batchable shortcuts.
Also fix single-row/column resize: start==end now formats as "23:23" / "C:C"
(resize_range rejects a bare "23"); dimRangeFull keeps both sides while
dimRange's collapse stays for modify_sheet_structure consumers.
* fix(sheets): align +cells-get/+csv-get range flags with synced spec
sheet-skill-spec now declares +cells-get --range as a single string
(was string_array) and +csv-get --range as required. Match the
flag→body translators:
- +cells-get wraps the single --range into the tool's `ranges` array
and validates with Str() instead of StrArray(), which silently
returned nil against the now-String flag and broke the command.
- +csv-get gains a trim-based required-range guard.
Update read-data dry-run tests to single-range form and add a guard
test for the empty --range path.
* fix(sheets): push +batch-update sub-op validation down into xxxInput builders
Sub-ops that omit --sheet-id (or any other required flag) used to slip
past CLI validation — Validate ran only against the standalone shortcut
path, and batchOpDispatch's translators built bodies from whatever
flagView returned, so a structurally broken sub-op surfaced as an opaque
server "sheet undefined not found" after a network round-trip.
Push each batchable shortcut's check trio down into its xxxInput builder:
1. resolveSpreadsheetToken — stays in Validate (batch already does it
once at the top level; sub-ops don't repeat).
2. requireSheetSelector(sheetID, sheetName) — new helper; flagView-
agnostic XOR + control-char check, called at the top of every
xxxInput.
3. shortcut-specific required / range / enum checks (--dimension,
--range, --start <= --end, --type pixel needs --size,
--float-image-id, image-token XOR image-uri, ...) — moved out of
Validate into the builder body.
All ~30 batchable xxxInput builders now return (map, error). Standalone
Validate shrinks to validateViaInput(xxxInput); DryRun / Execute
propagate the error. batch_op_dispatch entries drop the noErrTranslate
wrapper and pass the builder directly — its error bubbles up wrapped
with "operations[N] (+shortcut):" context.
Tests:
- TestBatchOp_ErrorEquivalence (7 cases): XOR / logical-constraint
errors fire identically from standalone and batch sub-op paths.
- TestBatchOp_RejectsBadSubOpInput (8 cases): cobra-required flags that
standalone catches via MarkFlagRequired now also get rejected CLI-side
on the batch path (where cobra is not in the loop).
- TestBatchOp_BodyMatchesStandalone (~40 cases) and
TestBatchOp_DispatchCoversReportedBugs continue to pass — bodies stay
byte-identical.
- BOE smoke (spreadsheet ICFwstkUGheyfptGWS2bB7RgcDf, sheet 51991c):
+batch-update with a sub-op missing --sheet-id now returns
"operations[0] (+dim-insert): specify at least one of --sheet-id or
--sheet-name" before any network call.
sheetMoveBatchInput (xiongyuanwen's batch-only explicit-source-index
requirement) is preserved — it's an orthogonal batch-specific constraint
not affected by this push-down.
* fix(sheets): align +cond-format / +filter with server schema (#4 + #5)
Two latent bugs in the object_crud translator surfaced during BOE smoke
testing of +batch-update. Both are schema-alignment fixes against
manage_conditional_format_object / manage_filter_object as declared in
sheet-skill-spec/canonical-spec/tool-schemas/mcp-tools.json.
#4 +cond-format: rule_type path + enum vocabulary
---------------------------------------------------
condFormatEnhance used to write the user's --rule-type value into
`properties.rule.type` (nested under a `rule` object). The server
schema actually puts it at flat `properties.rule_type` and silently
drops the nested form — so every conditional-format create/update
secretly built the wrong document.
Worse, the CLI enum exposed via flag-defs.json was its own invented
vocabulary (cellValue / formula / duplicate / unique / topBottom /
aboveBelowAverage / dataBar / colorScale / iconSet / textContains /
dateOccurring / blankCell / errorCell) — none of those values were
the strings the server accepts.
Fix:
- condFormatEnhance now writes `properties.rule_type = <value>`
directly (no nested `rule` object).
- Synced flag-defs.json + lark-sheets-conditional-format.md enum
vocabulary from base to match the server: duplicateValues,
uniqueValues, cellIs, containsText, timePeriod, containsBlanks,
notContainsBlanks, dataBar, colorScale, rank, aboveAverage,
expression, iconSet.
- ⚠️ Breaking: scripts passing the old CLI-invented enum values
(e.g. --rule-type cellValue) now get a cobra "invalid value …
allowed: …" error listing the new vocabulary. No alias layer.
- TestObjectCRUDShortcuts_DryRun's +cond-format-update case updated
to assert the flat properties.rule_type shape + new enum.
#5 +filter-{update,delete}: auto-inject filter_id = sheet_id
-------------------------------------------------------------
manage_filter_object's contract is "filter_id === sheet_id" for the
sheet-scoped filter (per per-tool description in mcp-tools.json),
and update / delete operations MUST carry filter_id. Standalone
filterUpdateInput / filterDeleteInput never set it, so the server
rejected with "filter_id is required for update/delete operation"
on every call — both standalone AND inside +batch-update.
Fix:
- filterUpdateInput / filterDeleteInput now set
input["filter_id"] = sheetID.
- Because filter_id must equal sheet_id (not sheet_name), update /
delete reject when only --sheet-name is given — there's no
network lookup available inside the builder. The friendly error
points at +workbook-info for resolving sheet-name → sheet-id.
- create still omits filter_id (server requires that — id is
server-allocated on creation).
- New tests:
* TestObjectCRUDShortcuts_DryRun gains a +filter-update happy-path
case asserting filter_id is auto-injected + --range hoisting.
* +filter-delete case updated to assert filter_id presence.
* TestBatchOp_RejectsBadSubOpInput gains two cases asserting both
+filter-update and +filter-delete reject --sheet-name-only with
the friendly error.
Docs (#2 + #3 + #8) synced from sheet-skill-spec
-------------------------------------------------
Companion doc fixes that landed via npm run generate:cli + sync:cli
in sheet-skill-spec; included here because the regenerated flag-defs
and references markdown are byte-tracked in this repo:
- #2: lark-sheets-sheet-structure.md — +dim-{hide,unhide,group,
ungroup} --start/--end desc changed from "(0-based, inclusive)" to
"(0-based)" / "(exclusive)" to match the half-open range semantics
the code has always implemented (requireDimRange: end > start;
dimRange uses end - 1 for column end letters).
- #3: lark-sheets-workbook.md — +sheet-move section gains a note
about the batch-internal requirement to pass --sheet-id AND
--source-index explicitly (sheetMoveBatchInput's constraint).
- #8: lark-sheets-pivot-table.md — +pivot-create --properties
example drops the stale data_range field (the actual server
schema uses --source as a hoisted flag; properties only carries
rows / columns / values / filters / show_*_grand_total).
* feat(sheets): add +cells-batch-clear fan-out over batch_update
Clear content/formats across many sheet-prefixed ranges in a single atomic
batch_update (one clear_cell_range op per range), mirroring the existing
+cells-batch-set-style / +dropdown-{update,delete} fan-out wrappers. The
--scope to clear_type normalization is shared with standalone +cells-clear
(normalizeClearType) so the two stay in lockstep.
high-risk-write (requires --yes); rejected as a batch sub-op like the other
fan-out wrappers. flag-defs/flag-schemas and skill docs updated to match.
* docs(sheets): sync stdin guidance and sparkline reference
- skills/lark-shared/SKILL.md: drop the generic "prefer stdin" section
- skills/lark-sheets/SKILL.md: add expanded stdin guidance (use stdin over @file abs paths; don't cd or write into the project dir)
- skills/lark-sheets/references/lark-sheets-sparkline.md: document the group_id / sparkline_id two-tier model with worked examples
* fix(sheets): require sparkline_id on +sparkline-update items (#6)
manage_sparkline_object uses two layers of IDs: --group-id picks the
sparkline group, and properties.sparklines[i].sparkline_id picks each
item inside the group. The server contract requires sparkline_id on
every update item (server maps each entry back to an existing
sparkline by this id). Agents that called +sparkline-update without
the per-item ids hit an opaque server-side rejection that didn't
mention sparkline_id at all, then got stuck in a try-fail-list-retry
loop.
Pre-check CLI-side in objectUpdateInput via a new validateUpdateInput
hook on objectCRUDSpec. sparklineSpec wires validateSparklineUpdateItems,
which walks properties.sparklines[] and rejects with a message that
points at +sparkline-list:
+sparkline-update properties.sparklines[N] missing sparkline_id
(run `+sparkline-list --group-id <id>` first to read sparkline_id
for each item, then echo each id back on the corresponding update
entry)
Scope is update-only. config-only updates (properties.config without
sparklines) stay legal — the validator skips when sparklines is
absent. Delete is not pre-checked: objectDeleteInput doesn't pass
properties through, so the partial-delete branch can't be reached
today (separate follow-up).
Tests:
- TestObjectCRUDShortcuts_DryRun: positive case for update with
sparkline_id present.
- TestSparklineUpdate_MissingSparklineID: standalone path — error
contains both "missing sparkline_id" and "+sparkline-list".
- TestBatchOp_RejectsBadSubOpInput: batch sub-op missing sparkline_id
rejected with the same friendly error.
Docs synced from sheet-skill-spec (canonical change committed there):
skills/lark-sheets/references/lark-sheets-sparkline.md documents the
two-layer id model, the three "+sparkline-list first" cases, and both
delete modes.
* docs(sheets): sync lark-sheets skill from spec (audit 20260521)
Pull latest spec from sheet-skill-spec (PR ee/sheet-skill-spec!6 + earlier
develop commits) into skills/lark-sheets/ and shortcuts/sheets/data/.
Audit findings now reflected in CLI docs:
- A2 +cond-format-create example: --rule-type duplicate → duplicateValues
- A3 +cond-format-create Validate: cellValue/formula → cellIs/expression
- A5 +csv-put examples: --range → --start-cell; drop redundant --allow-overwrite
- A7 +sparkline-create: Validate / Examples aligned with real schema
(config/sparklines), executable JSON example added
- B13 cross-doc dead links: lark_sheet_*/cli-shortcuts.md → lark-sheets-*.md
- C2 +csv-put: `=` literal warning next to Examples
- CC5 +rows-resize/+cols-resize --type auto: single point of truth in
range-operations reference
flag-defs.json description / required sync (from base):
- A4 +float-image-update: image-name/position-*/size-* required → optional
(patch mode)
- A8 +dim-move --start/--end description cleanup
- B3 +pivot-create --properties: data_range → source (real field name)
Also picks up the +cells-batch-clear shortcut doc (introduced in spec
develop). Go-side implementation for that shortcut is intentionally not
in this PR — docs-only preview; runtime dispatch will land in a follow-up.
`go test ./shortcuts/sheets/...` passes.
* feat(sheets): add +cells-set --copy-to-range and sync skill spec
Sync lark-sheets skill references and flag schemas from upstream
sheet-skill-spec, and wire the newly-specced --copy-to-range flag into
+cells-set: it passes copy_to_range to the set_cell_range tool so a
template block written via --cells fans out across a larger range with
auto-shifted formula refs.
* docs(sheets): sync lark-sheets skill spec (chart/pivot wire mappings, --end semantics)
Sync skill references and flag-defs descriptions from upstream
sheet-skill-spec: clarify +chart-create properties structure
(snapshot.data), +pivot-create --target-position / --range wire-field
mappings, add a cross-command --end endpoint-semantics table
(insert/delete/hide/group exclusive vs move/resize inclusive), note
--group-state default, and rename reference identifiers to lark-sheets-*.
Description-only refinement; the existing CLI implementation already
matches the clarified wire mappings and --end semantics.
* fix(sheets): make --max-chars the single read cap for +cells-get / +csv-get
Drop --cell-limit (+cells-get) and --max-rows (+csv-get) from the CLI surface
and pin the underlying tool's cell_limit / max_rows to a very large sentinel so
the tool's own defaults never truncate before --max-chars. --max-chars stays the
only knob (default 200000, unchanged).
- lark_sheet_read_data.go: add unboundedReadLimit (1e9); cellsGetInput pins
cell_limit, csvGetInput pins max_rows; --max-chars still passed through
- data/flag-defs.json: synced from spec (drops the two flags)
- tests: spot-check moved to --max-chars; dry-run wantInput asserts cell_limit /
max_rows are pinned high
Mirrors sheet-skill-spec (Base flag records removed).
go build ./... + go test ./shortcuts/sheets/ green.
* docs(sheets): sync lark-sheets read docs — --max-chars as single read cap
Sync skills/lark-sheets references from spec: drop --cell-limit / --max-rows
guidance; 大表分批读 switches to --range row windows + --max-chars auto cap + has_more.
Mirrors sheet-skill-spec 58e7456 and handler change 2befc49.
* docs(sheets): sync lark-sheets skill spec from upstream
Refine reference docs and flag-defs descriptions from upstream
sheet-skill-spec (--depth wording for +dim-group / +dim-ungroup,
plus assorted reference clarifications). Description-only; no CLI
behavior or flag surface change.
* docs(sheets): sync chart properties schema (position/size required)
Regenerate flag-schemas.json from upstream sheet-skill-spec: the chart
properties schema now marks position and size as required, and the chart
reference doc reflects the same. flag-schemas.json is print-schema-only
(no client-side validation), so this is a generated-artifact + doc sync
with no CLI behavior change.
* docs(sheets): sync lark-sheets skill spec from upstream
Refine reference docs and flag-defs descriptions from upstream
sheet-skill-spec: clarify +workbook-export sheet flag scope, +filter-*
--properties optionality (omitted => empty filter on --range; rules must
be non-empty when provided), float-image reference_id wording, and
assorted reference cleanups. Description-only; existing CLI behavior
(filter passthrough, properties optional) already matches.
* docs(sheets): sync lark-sheets skill spec from upstream
Trim and refine reference docs from upstream sheet-skill-spec
(condense core-operations workflow, tidy write-cells / range-operations /
float-image / SKILL guidance). Description-only; no flag or CLI behavior
change.
* docs(sheets): sync lark-sheets skill spec from upstream
Refine reference docs from upstream sheet-skill-spec (core-operations,
formula-translation, visual-standards, SKILL guidance). Description-only;
no flag or CLI behavior change.
* fix(sheets): correct +workbook-create initial fill and +dim-move endpoint
+workbook-create: the v3 create response does not echo the default sheet's id, so the initial-fill set_cell_range was sent with an empty sheet_id and rejected ("sheet_id or sheet_name is required"). Resolve the workbook's first sheet via get_workbook_structure before filling.
+dim-move: the move request was POSTed to the v2 dimension_range endpoint (the add/update/delete surface, which requires a `dimension` object) and rejected with "[9499] Missing required parameter: Dimension". Switch to the native v3 move_dimension endpoint (sheet_id in path; snake_case source.{major_dimension,start_index,end_index} + destination_index). CLI --end and v3 end_index are both 0-based inclusive, so they pass through unchanged.
* fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema
Five separate E2E failures in shortcuts/sheets/ that all trace back to a
CLI ↔ server contract mismatch. Each is independently scoped; bundling
them because they share the test-report citation and the same one-line
fix shape in most cases.
buildInitialFillInput sent {"sheet_id": ""} on the secondary
set_cell_range call after creating the workbook. The empty value was a
holdover from "...otherwise server picks first sheet" — but
set_cell_range rejects an empty selector with
"sheet_id or sheet_name is required" rather than falling back to the
default sheet.
Use sheet_name "Sheet1" instead. POST /sheets/v3/spreadsheets always
creates that sheet on workbook creation, and set_cell_range accepts
sheet_name as an equivalent selector — saves an extra
get_workbook_structure round-trip just to learn the auto-generated id.
buildDropdownValidation emitted four fields that don't exist in the
canonical set_cell_range.data_validation schema:
- "values" (options list) → renamed to "items"
- "multiple_values" → renamed to "support_multiple_values"
- "colors" (per-option color) → removed (not in schema; flag also
removed from data/flag-defs.json
for +dropdown-set / -update)
- "highlight_options" → removed (not in schema; flag also
removed)
The canonical schema lives at sheet-skill-spec/canonical-spec/tool-
schemas/mcp-tools.json (set_cell_range tool, data_validation property);
the colors / highlight knobs were CLI inventions the server never
accepted, so removing the flags is correct (renaming would leave the
flags broken). Skill reference docs (write-cells.md, batch-update.md)
synced.
validateDropdownOptionsColors lost its colors check; renamed to
validateDropdownOptions to reflect the narrower contract.
dropdownGetInput sent "Sheet1!C2:C6" verbatim as a ranges[] entry.
get_cell_ranges expects sheet_id / sheet_name as separate fields and
ranges entries without the sheet prefix; the server bounced with
"sheet not found, sheetId:" (empty).
Use the existing splitSheetPrefixedRange helper (declared in
lark_sheet_batch_update.go) to break "Sheet1!C2:C6" into ("Sheet1",
"C2:C6"), then thread the sheet name through sheetSelectorForToolInput
exactly like +cells-get does.
The shortcut was POSTing to /sheets/v2/spreadsheets/{token}/dimension_
range, which is the v2 insert-dimension endpoint and requires a top-
level {"dimension": {...}} body. Move uses a separate endpoint:
POST /sheets/v2/spreadsheets/{token}/move_dimension
body: { "source": {...}, "destination_index": N }
(camelCase "destinationIndex" → snake_case "destination_index" to
match the v2 contract.) Both DryRun and Execute updated, plus the
TestDimMove_DryRun and TestExecute_DimMove assertions.
transform_range.sort_conditions[i] requires both `column` (string) and
`ascending` (bool); rangeSortInput passed the --sort-keys array through
to the server unvalidated, so missing fields surfaced as opaque
"required property X missing" errors with no per-item context.
Walk the parsed array client-side, reject with item-pointing messages.
Test fixtures and a contract-test fixture switched from the historical
{col, order} vocabulary (which the server has never accepted) to the
correct {column, ascending}.
Server-schema citations and test-report case mapping in this branch's
plan file.
* revert(sheets): drop direct flag-defs.json edits — generated from spec
data/flag-defs.json is regenerated from the upstream sheet-skill-spec
canonical-spec; editing it here gets clobbered on the next sync. The
schema realignment for +dropdown-set / -update --colors / --highlight
removal needs to land on the base table first, then flow back through
sheet-skill-spec → larksuite-cli sync, not via a direct CLI-side edit.
Restore the previous flag entries verbatim. The Go-side change in
buildDropdownValidation still drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until the upstream removal lands;
- after upstream removal + sync, both the flag declarations and the
Go-side handling will be in sync.
Functional fixes (#1 workbook-create, #3 dropdown-get, #4 dim-move,
#5 range-sort) and dropdown wire-shape rename (#2) are unaffected.
* revert(sheets): drop direct edits to skills/lark-sheets/references/
These md files are sync targets generated from sheet-skill-spec; editing
them here gets clobbered on the next sync, same as data/flag-defs.json.
The --colors / --highlight row removals belong on the upstream base
table → canonical-spec sync, not here.
Restore the previous --colors / --highlight rows in both
lark-sheets-write-cells.md (+dropdown-set) and lark-sheets-batch-update.md
(+dropdown-update). The Go-side change in buildDropdownValidation still
drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until upstream removes the flag;
- after upstream removal + sync, both flag declarations, ref docs, and
Go-side handling will be in sync.
Functional fixes (#1 workbook-create, #3 dropdown-get, #4 dim-move,
#5 range-sort) and dropdown wire-shape rename (#2) are unaffected.
* docs(sheets): sync from sheet-skill-spec — remove dropdown --colors / --highlight
Upstream sheet-skill-spec base table deleted the --colors and --highlight
flags on +dropdown-set / +dropdown-update (the corresponding wire fields
data_validation.colors / .highlight_options were never accepted by the
server schema; see prior fix in this branch). Re-running the sync from
canonical-spec brings the CLI flag-defs and skill reference docs back in
line with the Go-side handling that already drops these fields.
Generated by `npm run sync:cli` in sheet-skill-spec @ ac7acef.
* fix(sheets): restore +dropdown --colors / --highlight, map to canonical fields
Reverses the --colors / --highlight removal from 7932ab2 (item #2 of the
batch-1 schema-alignment commit). That commit dropped both flags after the
test report flagged data_validation.colors / highlight_options as "unexpected
property" — at the time the canonical set_cell_range.data_validation schema
listed only help_text / items / operator / range / support_multiple_values /
type / values, so the flags had no server-side target and the removal was
correct.
Since then, set_cell_range.data_validation has gained two fields explicitly
modelling the dropdown highlight UI (mcp-tools.json in sheet-skill-spec
2026-05-22 base sync):
enable_highlight (bool) — show pill backgrounds
highlight_colors (string[]) — hex pill colors, length must match items
So the flags are back, but rewired:
--colors -> data_validation.highlight_colors (was: colors)
--highlight -> data_validation.enable_highlight (was: highlight_options)
--options -> items and --multiple -> support_multiple_values renames from
7932ab2 are kept.
Changes:
- buildDropdownValidation: re-add --colors / --highlight handling against
the new field names; --colors length check stays inline (so dropdownSetInput
Validate path catches it via validateViaInput, no separate guard needed).
- validateDropdownOptions -> validateDropdownOptionsColors: restore the
Validate-time --colors length check on +dropdown-update / +dropdown-delete
(called from lark_sheet_batch_update.go).
- TestDropdownSet_CellsShape: extend to assert highlight_colors /
enable_highlight emitted; assert legacy `colors` / `highlight_options`
absent.
- TestDropdownSet_ColorsLengthMismatch: new — covers the early Validate
error path.
- TestDropdownUpdate_BatchPayload: extend to cover dropdownBatchInput
propagation of --colors / --highlight through batch_update.
- skills/lark-sheets/references/lark-sheets-{write-cells,batch-update}.md,
shortcuts/sheets/data/flag-defs.json, flag-schemas.json: synced from
sheet-skill-spec generate output (MR !7).
* chore(sheets): re-sync from spec + loosen --colors length check
Catches up to sheet-skill-spec's 2026-05-25 base sync (MR !7) after
rebasing onto upstream feat/lark-sheets-refactor (12 new upstream commits
including the lark-sheets skill refactor + tools-schema migration).
Spec changes flowing in:
- highlight_colors description loosened: length may be **shorter than**
--options (server cycles remaining slots through a built-in 10-color
palette); previously the tool errored on any length mismatch.
- shortcuts/sheets/data/flag-schemas.json: mass re-mirror — generator now
emits `type` before `properties` and adds explicit `additionalProperties:
false` on object schemas (cosmetic, no behavior change).
- skills/lark-sheets/references/lark-sheets-{batch-update,chart,write-cells}.md:
--options gains the type='list' tag; data_validation inline field-count
goes 7 → 9 (catches up the highlight schema in the summary); chart
position / size marked optional per upstream.
Go-side adjustment:
- buildDropdownValidation / validateDropdownOptionsColors: change the
--colors length check from strict-equal to "must not exceed --options"
to match the relaxed schema.
- TestDropdownSet_ColorsLengthMismatch -> TestDropdownSet_ColorsLongerThanOptions
(now hits the overflow path with 3 colors vs 2 options).
- New TestDropdownSet_ColorsShorterAccepted: 2 colors vs 4 options is
legal and forwarded as-is.
* docs(sheets): sync dropdown --colors/--highlight clarification from spec
Mirrors sheet-skill-spec MR !7 changes:
- skills/lark-sheets/references/lark-sheets-write-cells.md: new "Dropdown
配色" section explaining how --colors (→ data_validation.highlight_colors)
and --highlight (→ data_validation.enable_highlight) compose — length
rule (shorter ok, longer rejected), --highlight gating, palette
fallback behavior, minimal +dropdown-set example.
- skills/lark-sheets/references/lark-sheets-batch-update.md: one-line
pointer to the write_cells section for +dropdown-update / -delete
(same rules).
- shortcuts/sheets/data/flag-defs.json: --colors / --highlight `desc`
fields gain the long-form server-field / length-rule descriptions
used by `--help`.
No Go-side change — earlier commit 538eb2e already loosened the
buildDropdownValidation length check to "must not exceed"; this PR step
just makes the docs / `--help` text catch up.
* feat(sheets): +dropdown-set/-update --source-range for listFromRange mode
Previously +dropdown-set / +dropdown-update only emitted
data_validation.type=list — agents wanting listFromRange (dropdown options
sourced from existing cells, kept in sync with that range) had to drop down
to +cells-set and hand-build a data_validation map. The flag now exposes it
natively as --source-range, paired with --options under XOR.
CLI changes:
- shortcuts/sheets/lark_sheet_write_cells.go:
* new dropdownTypeAndItems(runtime) — central XOR resolver: rejects 0 or
2 of {--options, --source-range}, returns (sourceSize, partial dv with
type+items|range filled in). Source size = options length for list
mode, rangeDimensions(--source-range) cell count for listFromRange.
* buildDropdownValidation rewritten to call the resolver, then layer
--colors / --multiple / --highlight on top — semantics unchanged
for callers, just two modes instead of one.
* validateDropdownOptions / -Colors renamed to validateDropdownSourceOrOptions
so the XOR + length check fires at +dropdown-update Validate time too.
* --colors length error message generalized: "must not exceed dropdown
source size (N)" (covers both modes).
- shortcuts/sheets/lark_sheet_batch_update.go: rename call site.
- shortcuts/sheets/lark_sheet_write_cells_test.go: 4 new tests —
ListFromRange (happy path: range + items absent + colors + highlight all
emit), ListFromRange_ColorsLongerThanCells (overflow against T1:T3 cell
count), XorBothSet, XorNeitherSet. Updated the existing
ColorsLongerThanOptions assertion to match the new "source size" wording.
Spec-driven changes (synced via npm run sync:cli from sheet-skill-spec
MR !7 2c298b6):
- shortcuts/sheets/data/flag-defs.json: --options Required flips to xor on
+dropdown-set/-update; new --source-range row gains long-form description
pointing at server data_validation.range + the XOR semantics.
- skills/lark-sheets/references/lark-sheets-write-cells.md: "Dropdown 配色"
section reorganized into "Dropdown 选项 + 配色" — XOR comparison table
(list vs listFromRange), shared config flag table (--highlight /
--colors), explicit length rule covering both modes, side-by-side
minimal examples, server-range-normalization gotcha callout.
- skills/lark-sheets/references/lark-sheets-batch-update.md pointer updated
to mention both modes + that +dropdown-delete is unaffected.
PPE smoke (ppe_lark_cli_sheet) on UFJxszjrZhZ1LVtc9FdcICSbn6b C column:
- +cells-set C1 → "性别" (bold + centered): updated_cells_count=1
- +dropdown-set --range C2:C21 --source-range "Sheet1!T1:T3" --colors
'["#cce8ff","#ffd6e7","#e6e6e6"]' --highlight: updated_cells_count=20
- read-back: data_validation.type=listFromRange + range=$T$1:$T$3 (server
normalizes the prefix away on storage; highlight_colors /
enable_highlight not echoed by get_cell_ranges, see byted-sheet read
projection TODO).
- error-path replay (both XOR violations + colors > source-size) all
rejected at Validate stage with the expected messages.
* docs(sheets): sync agent-voice rewrite of Dropdown 选项+配色 from spec
Mirrors sheet-skill-spec MR !7 60df610 — narrative now describes how the
flags interact (XOR, colors length rule, highlight gating, sheet-prefix
read-back gotcha) without exposing the underlying data_validation field
names or server-side normalization details that agents don't act on.
No Go-side change, no shortcut behavior change.
* chore(sheets): restore --colors in parseJSONFlag docstring example list
The earlier commit 49104ec swapped --colors out of parseJSONFlag's "Used
by" example list when it deleted the flag (item #2 there removed --colors
/ --highlight from +dropdown-set/-update). Subsequent commits 8672d8e /
538eb2e / fb90c8b reinstated --colors (and added --source-range) but did
not roll back this docstring tweak — leaving an orphan reference to
--properties where --colors used to be.
This restores the example list to its pre-49104ec form so the docstring
matches what the helper actually services on this branch's HEAD.
Pure docstring change — function behavior unaffected, no test movement.
* fix(sheets): post-rebase test fixups after dropping superseded fix#1
Two test fallouts from rebasing onto upstream 4be06c8 (which independently
re-fixed +workbook-create and +dim-move with a more thorough approach):
- shortcuts/sheets/lark_sheet_workbook_test.go: our PR's earlier
TestWorkbookCreate_DryRun "with headers and data → 2-step plan" subtest
asserted the expedient sheet_name="Sheet1" / no-sheet_id wire body that
matched our dropped fix#1 implementation. Upstream's fix#1 resolves
the workbook's first sheet via get_workbook_structure and fills with
the real sheet_id instead. Reset this file to upstream's version — our
superseded assertions disappear, upstream's tests cover the new wire
shape.
- shortcuts/sheets/execute_paths_test.go: TestExecute_RangeSort fixture
still used the legacy {col, order} sort-key shape because the rebase
resolution picked the upstream version of this file wholesale (it
contained other unrelated changes). Re-apply just the one fixture
update to {column, ascending} so fix#5's CLI-side rejection logic
exercises a valid input — server-side sort_conditions has required
fields `column` (string) and `ascending` (bool); the historical
{col, order} vocabulary was never accepted.
go build ./... + go test ./shortcuts/sheets/... -count=1 both green.
* feat(sheets): +dropdown --highlight tri-state via Changed() for opt-out
The server-side default for data_validation.enable_highlight flipped from
false to true (aligning with the UI behavior). With the previous code path
if runtime.Bool("highlight") { dv["enable_highlight"] = true }
omitting --highlight and passing --highlight=false both produced the same
"enable_highlight key absent" body, leaving CLI users with no way to opt
out of the (now-default) highlighting.
Switch to runtime.Changed() so the translator can distinguish all three
input shapes:
- omitted -> no enable_highlight key (server applies default=true)
- --highlight=true -> enable_highlight: true (explicit no-op vs default)
- --highlight=false -> enable_highlight: false (the only opt-out path)
flagView already exposes Changed() and mapFlagView (the +batch-update
sub-op adapter) implements it via raw-key presence — same pattern other
translators use for "Changed-only" branching (e.g. omit target_index
unless --index was set), so no interface surface change is needed.
Test coverage:
- TestDropdownSet_HighlightTriState pins all four shapes (omit / presence
form / explicit true / explicit false) and asserts the enable_highlight
key's presence/value
- TestBatchOp_BodyMatchesStandalone adds a --highlight=false sub-op case
so the batch sub-op path produces a body byte-identical to the
standalone +dropdown-set --highlight=false body
* chore(sheets): sync +dropdown flag desc + write-cells narrative from spec
Mirror sheet-skill-spec generated/ into shortcuts/sheets/data/ and
skills/lark-sheets/ for the +dropdown-set / +dropdown-update path. No
hand edits in this repo.
The +dropdown flag desc and the Dropdown 配色 narrative now match the
server-side enable_highlight default flip (true) and the tri-state
--highlight semantics introduced in the sibling commit:
* --highlight desc: 不传 = 开（按内置 10 色色板循环上色），
--highlight=false 关闭得到纯白下拉
* --colors desc: 单独传即生效（高亮默认开），--highlight=false 时忽略
* write-cells reference: 三种意图三条线（默认色板 / 指定颜色 /
纯白下拉）+ 新增 --highlight=false 示例
Source upstream: sheet-skill-spec MR !8.
* fix(sheets): validate +cells-set-image --image path in Validate
The unsafe-path check only ran at Execute (via FileIO.Stat), so --dry-run
printed a misleading success preview for an absolute / out-of-cwd --image
path that a real run would then reject. Move the path-safety check into
Validate (validate.SafeLocalFlagPath), so --dry-run and Execute fail
identically and both name the real --image flag. File existence stays
deferred to Execute, so legitimate relative paths still preview cleanly.
Add TestCellsSetImage_DryRunRejectsUnsafePath.
* feat(sheets): support local --image in +float-image-create
+float-image-create now accepts a local file via --image (XOR with
--image-token / --image-uri): the CLI uploads it as a sheet_image and
embeds the returned file_token, removing the previous "upload elsewhere
to get a token first" workaround. Path safety is checked in Validate,
--dry-run previews the extra upload step, and +batch-update rejects
--image (no upload phase). +float-image-update is unchanged (it does not
register --image).
Also syncs the lark-sheets skill docs/flag-defs from sheet-skill-spec:
the new --image flag, partial-merge / border-per-side / bare sheet-prefix
clarifications, and refreshed dropdown --colors/--highlight descriptions
(already pending in the source Base table).
* fix(sheets): +dropdown-get accepts --sheet-id/--sheet-name + bare --range
Align +dropdown-get with its get_cell_ranges siblings (+cells-get / +csv-get):
sheet selection is now via --sheet-id / --sheet-name (XOR) and --range is a
bare A1 reference. The previous shape required the sheet prefix inside --range
(e.g. "Sheet1!A2:A100") and was the odd one out among the read-data wrappers;
callers pasting the sheet-id form straight from the URL hit a misleading
"sheet not found, sheetId: , sheetName: <id>" error because the prefix was
unconditionally treated as sheet_name.
Flag schema + skill reference regenerated from the upstream Lark Base
Shortcut-flags table.
* fix(sheets): drop Sheet1! prefix from +cells-get / +csv-get / +csv-put flag examples
Server tools-schema.json for get_cell_ranges, get_range_as_csv and set_range_from_csv
does not accept a sheet prefix on --range / --start-cell; the sheet is selected via
--sheet-id / --sheet-name. +csv-put --start-cell also now states it must be a single
cell (no range notation).
Synced from spec repo.
* feat: 把环境变量提交上去
* fix(sheets): clarify batch --ranges prefix must be sheet display name
E2E test cases repeatedly trip on this:
$ lark-cli sheets +cells-batch-set-style \
--ranges '["7f8fba!A2:B3","7f8fba!C2:D3"]' --font-color '#3366FF' ...
→ tool "batch_update" failed: [900015206]
sheet "7f8fba" not found. Available sheets: [{id: "7f8fba", name: "Sheet1"}]
Callers paste the hex sheet-id (e.g. "7f8fba") from a spreadsheet URL /
+sheet-create response straight into the --ranges sheet prefix. The four
batch shortcuts (+cells-batch-set-style / +cells-batch-clear /
+dropdown-update / +dropdown-delete) fan each range out into a
batch_update sub-op (set_cell_range / clear_cell_range) and pass the
prefix through as sheet_name; the server only matches sheet_name
literally, so the lookup fails.
The set_cell_range tool schema is explicit: sheet_id is the
reference_id and "must be correct or it errors"; sheet_name is the
display name. CLI can't disambiguate purely from the literal because
users can rename sheets to anything (including six-char hex strings).
Cleanest fix is at the source: each batch shortcut's --ranges flag
description now states explicitly that the prefix must be the sheet
display name and that the sheet reference_id is rejected, so agents
reading the reference don't try the id form in the first place.
No Go changes; these files are regenerated from the upstream Lark Base
Shortcut-flags table via the sheet-skill-spec sync chain.
* docs(sheets): sync lark-sheets skill docs from upstream spec
- SKILL.md: clarify --url only resolves /sheets/ and /spreadsheets/ links; /wiki/ links must be resolved via wiki +node-get first (confirm obj_type=sheet, use obj_token)
- formula-translation: document IMPORTRANGE cross-workbook limits (max 5-level nesting, 100 refs per sheet)
- write-cells: document rich_text cells for hyperlinks, @mentions and @docs
* feat: 同步 tools-schema.json 改动
* fix(sheets): warn when +dropdown source-range exceeds 2000 cells with highlight on
byted-sheet's ListFromRangeValidation.checkOptionsValid() sets
isOptionError=true when shouldHighlightValidData is on and the source
range exceeds LIST_WITH_COLOR_MAX_COUNT (2000 cells) — the highlight +
large source combo is unsupported. CLI previously had no signal for
this, so users only learned by seeing the dropdown render as
option-error in the workbook.
Add a Validate-phase stderr warning in +dropdown-set and +dropdown-update
when --source-range covers >2000 cells unless --highlight=false. Soft
warning, never blocks the request. Inline --options is not subject to
this limit — server enforces no count or per-item length cap on inline
lists, so no warning fires there.
* docs(sheets): sync lark-sheets skill from spec — dropdown flag descs reflect server reality
Pulls sheet-skill-spec canonical-spec → generated → consumers chain for
dropdown flag desc corrections committed upstream (Shortcut-flags base
table rows for +dropdown-set / +dropdown-update --options and
--source-range).
Aligns flag descs with byted-sheet behavior:
- --options: dropped fabricated "≤500 items, each ≤100 chars, no commas"
promise. byted-sheet ListOfItemValidation enforces none of these.
- --source-range: appended note about the only real cap —
LIST_WITH_COLOR_MAX_COUNT=2000 when --highlight is on (server flags the
dropdown as option-error beyond that; CLI warns at Validate time per
bb7ccae).
Also picks up an unrelated upstream tools-schema.json drift (chart float
block schema + data_validation.items description tweak) that surfaced
via npm run check:tool-schemas; bundling keeps the spec sync gate green.
* revert(sheets): drop tools-schema drift mirror from previous spec sync
930c9c7 顺带 sync 了 spec 的 tools-schema bundling — 跟那条 commit 一起
误带进来 chart float block required 和 data_validation.items 描述微调，
这两处其实是上游 sheet-ai-skills 还在 pending 的 revert。
配套 sheet-skill-spec 的 revert commit (a3aa9f2 on
fix/dropdown-flag-desc-real-limits / !11)，重跑 sync:consumers 拉回
正确的 generated mirror：
- shortcuts/sheets/data/flag-schemas.json（chart 部分）
- skills/lark-sheets/references/lark-sheets-{chart,batch-update,write-cells}.md（rendered schema 段）
dropdown 文案改动（flag-defs.json 4 处 desc + dropdown 段的 reference
渲染）不在本 commit 范围，保持 930c9c7 的状态。
* docs(sheets): sync lark-sheets skill from spec — +filter-view-update --properties desc
去掉 +filter-view-update --properties 描述里"pass at least one of
--properties.rules / --range / --view-name"的误导承诺。--properties
实际是硬必填（MarkFlagRequired），且 update 走 PUT 整组覆盖语义。
* fix(sheets): align +cells-search/+cells-replace option keys with server schema
The CLI emitted `options.regex` and `options.include_formulas`, but the
server-side `search_data` / `replace_data` tool schemas declare and
consume `use_regex` and `match_formulas`. Result: passing `--regex` or
`--include-formulas` always failed with `unexpected property ... is not
defined in schema`.
Keep the user-facing flag names (`--regex`, `--include-formulas`) — only
the JSON keys sent to the server change. Updates the dry-run test that
locked the wrong contract.
* docs(sheets): sync float-image reference from spec — fix non-runnable examples
Two examples in skills/lark-sheets/references/lark-sheets-float-image.md
didn't actually run against PPE; sync brings them in line with CLI behavior:
- +float-image-create local-path example missed --image-name (CLI rejects
with `required flag(s) "image-name" not set` even when path basename
already has the filename). Add `--image-name "logo.png"` + inline note.
- +float-image-update "only change position" example missed image source
(CLI rejects with `one of --image, --image-token, or --image-uri is
required`). Expand to two steps: list with --jq pulls the current
image_token, then update re-passes --image-token to satisfy the guard.
- Leading warning realigned: image source is mandatory on every update
call; "keep original image" still requires passing the token explicitly.
Upstream change: sheet-skill-spec MR fix/float-image-reference-examples.
* feat: 同步 tools-schema.json 改动
* fix(sheets): allow +float-image-update to omit the image source
The image source (--image-token / --image-uri) is the only optional part
of an update: omit all of them to keep the current image. image_name,
position and size stay required — the manage_float_image tool rejects an
update without them, and +float-image-list does not return image_name to
backfill. Previously the shortcut forced an image source even when only
position/size changed, so those updates were rejected CLI-side before any
API call (reported as a Fail case in the sheets e2e rerun).
- floatImageProperties: gate the image-source requirement on create only;
keep image_name/position/size required on both; emit image_uri only when set
- sync flag-defs.json + lark-sheets-float-image.md from sheet-skill-spec
(image-name/position/size now required on +float-image-update)
- tests: cover the image-source-optional dry-run; the single-required checks
move to the +batch-update sub-op path (cobra owns the standalone path)
* docs(sheets): sync lark-sheets skill from spec
Mirror the canonical-spec reference fixes into the consumer skill:
- search_replace output contract: `matches[]` with `address` (+ `has_more`/`next_offset`)
- workbook sheet fields: `sheet_name`/`is_hidden`/`*_count`, no `frozen_*`
- `+range-fill` example uses a non-overlapping target (A3:A100)
- drop the unimplemented `envelope.meta.verification` auto-readback claim; advise
manual list/get verification instead
* fix(sheets): allow +pivot-create to omit both sheet selectors
manage_pivot_table_object treats sheet_id / sheet_name as the placement
target — when both are absent, handleCreate() auto-creates a new sub-sheet
to host the pivot table. The CLI's flag schema didn't reflect this:
- Exposed a third flag --target-sheet-id that mapped to the same wire
field as --sheet-id, leaving the caller unsure which one to use
- --sheet-id / --sheet-name had "XOR with the other" descriptions that
read like "operation context", so callers (especially LLM tool callers)
felt obligated to set one — frequently the source sheet — which
silently disabled the backend's auto-create guardrail and dropped the
pivot at A1, overlapping the source data
Wire change (synced from sheet-skill-spec): drop the duplicate
--target-sheet-id flag; rewrite --sheet-id / --sheet-name descriptions
to make the placement-target semantics explicit and call out that
omitting both is the recommended path.
Implementation change (this PR): add an at-most-one sheet-selector
helper and let object create-shortcuts opt into it.
- helpers.go: new optionalSheetSelector (both empty allowed; both set
still rejected; control-char validation unchanged). requireSheetSelector
is untouched — every existing caller keeps the exactly-one contract.
- lark_sheet_object_crud.go: objectCRUDSpec gains
allowEmptySheetSelectorOnCreate; objectCreateInput dispatches to
optionalSheetSelector when it's set. Only pivotSpec opts in;
chart / cond-format / sparkline / filter-view / float-image keep
the existing require semantics. DryRun and Execute switch to direct
flag extraction (same pattern Validate already used) so the XOR
check happens in exactly one place (the builder).
- pivotSpec: drop the enhanceCreateInput branch that read the now-removed
--target-sheet-id flag.
- Tests: TestPivotCreate_SheetSelectorSemantics covers both-empty /
both-set / single-set; TestObjectCreate_RequiresSheetSelector
regresses chart / cond-format / sparkline / filter-view to lock the
scope of the relaxation.
* docs(sheets): clarify filter/filter-view rules update is whole-set PUT
Synced from upstream tools-schema. The rules field on manage_filter_object and manage_filter_view_object now documents update as whole-set PUT semantics: submitted rules become the complete rule set, all existing columns' rules are cleared first, columns not listed lose their old rules (no merge), and [] clears everything. Description-only change, no structural/field change.
* refactor(sheets): switch dim-* / rows-cols-resize to A1-string range schema
The 9 row/column-region shortcuts used to share two int flags --start /
--end with inconsistent end semantics across commands — +dim-insert /
-delete / -hide / -unhide / -group / -ungroup treated --end as exclusive,
while +dim-move / +rows-resize / +cols-resize treated it as inclusive.
The skill reference even called this out as "the highest-frequency
off-by-one source", patched in docs rather than at the surface. Three
underlying tool schemas (position+count, A1 range string, 0-based int
pair) were all flattened onto the same --start/--end pair, which forced
a different normaliser per command and pushed mental math (count =
end - start) onto every caller.
Schema (sourced from base, regenerated via sheet-skill-spec, mirrored
into shortcuts/sheets/data/ and skills/lark-sheets/):
+dim-insert --position + --count
rows: "3"; columns: "C". --count rows/columns
inserted *before* --position.
+dim-delete / -hide / -unhide / -group / -ungroup
--range
+rows-resize / +cols-resize --range
A1 closed range. Rows: "3:7" or "5". Columns: "C:F" or "C".
Mixing letters and digits in one range is rejected.
+dim-move --source-range + --target
--target must match --source-range's dimension (both row or both
column). The move places the source block *before* --target.
Wire-shape preserved: modify_sheet_structure still receives `position`
+ `count` (insert) or a `range` A1 string (other dim-* ops); v3
move_dimension still receives 0-based inclusive ints (CLI parses the
A1 strings into them); resize_range still receives a two-sided A1
range (single-element form is expanded to "N:N" before send).
This is a flag-surface break (--start / --end / --dimension flags
removed from these 9 shortcuts); --dimension stays only on +dim-freeze
since it has no range to derive from.
Code: A1 parser added (parseA1Range / parseA1Position /
letterToColumnIndex reused from write_cells); dimRange / dimRangeFull /
dimPosition deleted; dim-move switches to source-range + target parsing;
resize gains a same-dimension guard so +rows-resize rejects "A:C" with
a clear "+rows-resize expects row numbers" message.
Tests: TestSheetStructureShortcuts_DryRun / TestDimMove_DryRun /
TestDimMove_Column / TestDimMove_MismatchedDimension /
TestDimRange_Validation / TestParseA1Range / TestResize_TypeAndSizeGuards
/ TestRangeOperationsShortcuts_DryRun all rewritten against the new
schema. Batch contract trio (BodyMatchesStandalone /
ErrorEquivalence / RejectsBadSubOpInput) and
TestBatchOp_DispatchCoversReportedBugs likewise. Full
`go test ./shortcuts/sheets/` passes.
* docs(sheets): sync +pivot-create placement reference from spec
Companion sync from sheet-skill-spec — the canonical reference rewrites
+pivot-create's "5 placement-related flags" rundown into a clearer
"4 placement-related flags" form (--target-sheet-id was already removed
in #1130, this updates the prose accordingly), and clarifies that
--sheet-id / --sheet-name on +pivot-create are the *placement* sheet
(not the source-data sheet), with omit-both as the strongly-recommended
default.
Also picks up a base-side --target-position description tweak that
dropped the now-stale "与 --target-sheet-id 配套" reference.
No CLI surface change.
* docs(sheets): sync +pivot-create summarize_by lowercase enum values from spec
* docs(sheets): wrap sheet names in single quotes in A1 examples
Synced from spec. Affects 3 reference md (pivot-table / batch-update /
write-cells) and 2 generated flag-data JSONs.
A1 examples like `Sheet1!A1:D100` now read `'Sheet1'!A1:D100` so models
default to single-quoted sheet names. Excel A1 notation requires single
quotes for sheet names containing hyphens / spaces / non-ASCII chars;
always-quoting is also valid for plain names, so this is the safer default
to teach.
Affected flags:
- +pivot-create --source
- +dropdown-update --ranges / --source-range
- +dropdown-delete --ranges
- +dropdown-set --source-range
- +cells-batch-set-style --ranges
- +cells-batch-clear --ranges
* docs(sheets): wrap A1 sheet names in handwritten examples + bash histexpand guide
Synced from spec. Affects 4 reference md (chart / pivot-table / sparkline /
write-cells) and SKILL.md.
In addition to wrapping sheet names in single quotes in all remaining
handwritten examples (covers chart refs.value / nameRef, sparkline source,
write-cells --source-range, pivot-create narrative), SKILL.md gains a new
"Shell quoting for A1 references with !" section.
The new section addresses bash history expansion: in interactive bash
(e.g., ShellExec sandbox), unescaped `!Word` after `"..."` triggers
`bash: !A1: event not found`, dropping the command before lark-cli sees
it. The section gives 4 quoting strategies (shell single-quote outer,
`set +H` prefix, mixed quoting, sheet-rename fallback) and an anti-pattern
list.
Affected files:
- skills/lark-sheets/SKILL.md (new section)
- skills/lark-sheets/references/lark-sheets-chart.md
- skills/lark-sheets/references/lark-sheets-pivot-table.md
- skills/lark-sheets/references/lark-sheets-sparkline.md
- skills/lark-sheets/references/lark-sheets-write-cells.md
* docs(sheets): drop bash histexpand section, fix write-cells table escape
Sync from spec, refining the bash-quoting deep-dive added in 0f695b6:
- Drop the `## Shell 调用注意事项` section in SKILL.md and the inline
`⚠️ bash 引号` callouts in lark-sheets-pivot-table.md and
lark-sheets-write-cells.md. The 4-scenario quoting table + anti-pattern
list turned out too verbose for the SKILL intro; single-quoted examples
in the references are themselves enough nudge.
- lark-sheets-write-cells.md L146: fix the table cell escape from the
malformed `'''Sheet1''!T1:T3'` (consecutive `''` are no-op empty
strings) to `''\''Sheet1'\''!T1:T3'`, matching the bash example at
L191 verbatim.
Net: 1 insertion, 40 deletions across 3 files.
* feat(sheets): rename +pivot-create sheet selector → --target-sheet-{id,name}
+pivot-create's placement selector (where the pivot table lands) is no
longer the generic --sheet-id / --sheet-name; it is now
--target-sheet-id / --target-sheet-name. The new names mark this as the
*output* sheet, distinct from the *data-source* sheet (which lives
inside --source as `'Sheet'!Range`). The other +pivot-{list,update,delete}
shortcuts keep --sheet-id / --sheet-name (their semantics are
"sheet that hosts the existing pivot", same as every other shortcut).
Motivation: an LLM agent reading the previous CLI surface saw +pivot-create
expose --sheet-id and assumed (as it had to) that it pointed at the data
source, like every other shortcut. The new flag name makes the intent
unambiguous at the call site, without relying on the agent having read
the narrative caveat in the reference doc.
Background: evaluation case U046 spent multiple rounds tripping on this
exact confusion before working around it with +sheet-rename.
Implementation:
- objectCRUDSpec gains createSheetIDFlag / createSheetNameFlag (with
default-fallback accessors sheetIDFlagOnCreate / sheetNameFlagOnCreate);
newObjectCreateShortcut + objectCreateInput consult the spec instead of
hard-coded "sheet-id" / "sheet-name". pivotSpec sets target-sheet-*;
every other create spec inherits the defaults.
- optionalSheetSelector (only used by pivot create) takes the two flag
names as parameters so its mutex / control-char errors quote the names
the user actually typed (--target-sheet-id, not --sheet-id).
- batch_op_dispatch: introduce sheetSelectorFlagsForSubOp(shortcut) →
(idFlag, nameFlag) returning target-sheet-* for "+pivot-create" and
the defaults otherwise; translateBatchOp uses it so +pivot-create
sub-ops in +batch-update accept the same renamed input keys.
- Tests:
- lark_sheet_object_crud_test.go: pivot-create cases switch args and
expected error wording to target-sheet-*; extra assertion that the
mutex error quotes the renamed flag (regression guard against
flag-name drift between code and error message).
- batch_op_contract_test.go: +pivot-create sub-op test uses
target-sheet-id / target-sheet-name input keys; the body-vs-standalone
contract loop reads the selector via sheetSelectorFlagsForSubOp so
every other shortcut keeps using sheet-id / sheet-name.
Synced reference docs (skills/lark-sheets/{SKILL.md,
references/lark-sheets-pivot-table.md}) mirror the spec's new flag names,
narrative, 3-placement-strategy block, and SKILL.md exception bullet that
explains why +pivot-create's badge says 无 sheet 定位 yet still has
placement selectors (just under different names).
flag-defs.json synced from spec picks up the renamed flags + kind=own.
All sheets-package tests pass.
* docs(sheets): strip migration-history language from pivot reference / SKILL
Synced from spec. Removes "renamed from / no longer called / not
--sheet-id" style migration-history language that snuck into the
previous sync. Reference and SKILL now describe the current flag names
directly without referencing the old names.
* docs(sheets): require +workbook-info before guessing sheet name
Synced from spec. SKILL.md adds a new rule under the sheet-locator
section: unless the user has explicitly named a sheet, the agent must
call +workbook-info first to fetch sheets[].sheet_id / sheets[].title
rather than guessing the default `Sheet1`. The Chinese-language tables
this CLI is typically used against rarely use that literal name —
"数据" / "Sheet" (no digit) / "工作表 1" / business-named sheets are
far more common — so guessing wastes a round-trip before the agent
ends up calling +workbook-info anyway.
The 统一调用范式 example also switches its `--sheet-name "Sheet1"`
placeholder to `<真实表名>` to remove the inadvertent suggestion that
`Sheet1` is a sensible default.
* docs(sheets): tell agent to `set +H` for A1 references containing `!`
Synced from spec. The sheet-locator section now warns: when a flag value
contains `!` (--source / --range / --ranges with a cross-sheet prefix),
run `set +H` at the start of the bash session to disable history
expansion — otherwise interactive bash (e.g. inside an agent's shell
sandbox) lexes "Sheet1!A1" as a history reference and fails with
`event not found` before lark-cli ever sees the argument.
When the sheet name itself contains hyphens / spaces / non-ASCII
characters, the A1 reference also needs single quotes around the sheet
name per A1 notation, e.g. --source "'Sales-2025'!A1:D100".
Also flips the previous `--range` example to `--range 'Sheet1!A1:B2'`
(shell single-quote) for consistency.
* feat(sheets): add schema-driven JSON flag validation
Validate composite JSON flags (--properties, --cells, --options,
--border-styles, --sort-keys) against the embedded flag-schemas.json
on every standalone and +batch-update sub-op invocation, replacing
ad-hoc per-shortcut guards.
Supports the JSON Schema subset actually used upstream: type / enum
/ oneOf / required / properties / items / nullable / minimum /
maximum / minItems / maxItems / additionalProperties (true | false
| <schema>). Enum errors quote the failing value, truncate beyond 8
entries, and surface case-only "did you mean" hints (SUM -> sum).
Coverage: 18 / 19 (shortcut, flag) pairs. +batch-update --operations
stays validator-skipped; its translator already does richer per
sub-op checks. mapFlagView.Command() routes batch sub-ops through
the same (command, flag) -> schema pipeline as standalone.
loadFlagSchemas() is now sync.Once-guarded so parallel first access
from t.Parallel test sets and concurrent shortcut invocations is
race-free.
Removes superseded hand-written guards:
- +pivot-create validateCreateInput / validatePivotCreateProps
- +range-sort sort-keys per-item shape check
Test fixtures updated to be schema-conformant (chart position/size,
pivot summarize_by lowercase, cells 2D-array shape).
* feat(sheets): add --rows-json output flag to +csv-get
+csv-get --rows-json returns structured rows ({row_number, values:{col→cell}})
instead of the CSV string, so callers can address cells by row_number / column
letter without parsing [row=N] or RFC-4180 CSV. Same read, alternate output
shape — a flag on +csv-get (default stays CSV), not a separate shortcut, since
the two differ only in representation.
- CsvGet.Execute: --rows-json reshapes the response via assembleRowsJSON
(parses annotated_csv into per-row records keyed by column letter; every
logical row emitted; embedded newlines parsed into cell values)
- surfaces the under-read hint structurally as data_not_fully_read
- flag-defs.json + read-data reference synced from spec
* feat(cli): agent-friendly errors, proxy silencing, +csv-put --range
Agent-experience fixes distilled from analyzing 50 real sheets
trajectories, where the top failures were hallucinated command/flag
names, proxy warnings corrupting JSON on stdout, and --range carried
over from +csv-get to +csv-put.
- did-you-mean: unify the duplicated Levenshtein into a shared
internal/suggest package and wire its prefix-weighted ranker into
unknown-subcommand and unknown-flag errors; flag-parse errors now
return a structured envelope with suggestions plus the full valid list,
so agents recover from semantic typos (e.g. --query vs --find).
- proxy: suppress the one-time proxy warning in non-interactive
(agent/CI/piped) runs so a 2>&1-merged stderr line cannot corrupt
stdout JSON; interactive sessions still warn.
- sheets +csv-put: accept --range as an alias for --start-cell (parity
with +csv-get / +cells-set) and echo the computed writes_range in
dry-run and the success envelope, so agents see the paste footprint
before it overwrites neighbours.
- docs(sheets): add an intent->command cheat-sheet to SKILL.md, a
runtime-prerequisites section, and document the --range alias and
writes_range behaviour.
* feat(sheets): close P0-4 pivot gaps — enum case, clear→pivot-delete hint, placement warning
Last open P0 from the 50-trajectory analysis — the two pivot black holes:
upper-cased summarize_by, and pivots built over the source sheet that hit
#REF! and then couldn't be removed.
- enum case tolerance: validateAgainstSchema rewrites a case-only enum
mismatch to the canonical (lower-case) spelling in place ("SUM" -> "sum")
before the request is sent, killing the whole class instead of only
hinting at it. Covers every nested enum (values[], calculated_fields[]);
genuinely unknown values still fail with the existing did-you-mean message.
- +cells-clear / +cells-batch-clear: when the backend reports "can not find
embedded block" (the range overlaps a pivot/chart), annotate the error
with the real fix — clearing cells can't delete an embedded object; remove
it with +pivot-delete / +chart-delete (id via +pivot-list / +chart-list).
Applied to both shortcuts, a Tips line, and the cells-clear reference.
- +pivot-create: a --help Tips block making "omit --target-* -> backend
auto-creates a sub-sheet, zero overwrite" the can't-miss default, plus a
placement_warning (dry-run + execute output) when an explicit target sheet
is set with no offset — definite when the target name matches the source
sheet, conditional otherwise. Local-only, advisory, never blocks the call.
The placement_warning is structured output, not a stderr line, so it
survives non-interactive proxy-warning silencing and isn't swallowed by 2>&1.
* feat(sheets): strip UTF-8 BOM from stdin/@file flag input
resolveInputFlags now strips a leading UTF-8 BOM from content read via stdin
or @file, so it cannot corrupt the first CSV cell or break JSON parsing of
payloads like --operations / --cells downstream.
Also pulls the synced lark-sheets skill docs from sheet-skill-spec and drops
scheme-number tags from two test comments.
* fix(sheets): drop dead --value-render-option flag from +csv-get
+csv-get wraps get_range_as_csv, which has no value_render_option support
(absent from its input type, executor, and published tool schema — it always
returns formatted display text via getText()). The CLI passed the flag through
as a silent no-op: callers asking for raw_value/formula got formatted values.
Remove the flag from flag-defs, drop the value_render_option passthrough in
csvGetInput, and clean the stale SKILL references. The real value_render_option
capability is unchanged on +cells-get (get_cell_ranges) via --include formula.
* chore: rename ppe x-tt-env lane to ppe_moa_canvas
* docs(sheets): sync skill description from spec (cloud-drive alias, lark-drive search, doubao routing)
* feat(sheets): restore pre-refactor shortcuts under backward/ for compatibility
The lark-sheets refactor renamed every shortcut (verb-noun → noun-verb,
e.g. +create-sheet → +sheet-create) and dropped the old commands. External
callers and the tests/cli_e2e/sheets suite still drive the legacy command
names (+create, +read, +write, +create-sheet, ...), which broke.
Re-add the pre-refactor implementations verbatim from main as an isolated
shortcuts/sheets/backward package (package rename only) and register
backward.Shortcuts() alongside sheets.Shortcuts(). Both sets mount under the
`sheets` service; their command names are fully disjoint (38 new vs 42 old,
zero overlap), so old and new commands coexist without collision.
* fix(sheets): resolve 30 golangci-lint v2.1.6 issues — copyloopvar, nilerr, unused
Removed 25 Go 1.22+ loop variable copies (copyloopvar) from test files where
tc := tc / tt := tt / c := c are no longer needed. Fixed 4 nilerr false
positives in flag_schema_validate.go by making intentional error discards
explicit (schema validation failures skip silently — best-effort guard).
Dropped unused batchOpDispatchKeys helper in batch_op_dispatch.go.
* feat(sheets): flag pre-refactor backward aliases via _notice and --help grouping
Nudge users whose lark-sheets skill predates the refactor to migrate off
the pre-refactor aliases (+read, +write, ...), without requiring anyone
to read --help.
- internal/deprecation: process-level pending Notice slot (mirrors
internal/skillscheck), surfaced in the JSON "_notice" envelope under a
"deprecated_command" key.
- internal/cmdutil: shared DeprecatedGroupID cobra group + helper so both
--help rendering and the unknown-subcommand path classify aliases the
same way.
- shortcuts/register.go: applySheetsCompatGroups splits the aliases into a
dedicated "update your skill" help group with "(-> +new)" pointers;
wrapSheetsBackwardDeprecation records the notice from Validate/Execute so
direct callers that never read --help still get flagged.
- cmd/root.go: extract composePendingNotice (now unit-testable) and split
availableSubcommandNames into current vs deprecated buckets while still
ranking unknown-subcommand suggestions across both.
* chore: drop hardcoded ppe lane routing from base security headers
The x-tt-env/x-use-ppe headers forced every request onto the
ppe_moa_canvas pre-release lane; they were only meant for exercising the
sheets refactor against the staging backend. Remove them so the CLI
routes to production by default.
* chore(sheets): promote lark-sheets skill to 2.0.0
Drop the -draft suffix now that the refactored sheets skill is ready to
ship.
* fix(sheets): correct +dropdown-get sheet-locator doc, finalize skill to 2.0.0
+dropdown-get requires a mandatory sheet selector — its Validate calls
resolveSheetSelector — so drop it from the "no sheet locator" exception
list in SKILL.md. It was wrongly grouped with +dropdown-update/+dropdown-delete,
which take only --ranges. +dropdown-get's own per-shortcut badge (公共四件套)
was already correct. Also finalize the skill version 2.0.0-draft -> 2.0.0.
* fix(sheets): enforce required-flag contract in batch sub-ops
Batch sub-ops reuse each shortcut's shared *Input builder through mapFlagView,
which seeds flag-defs defaults — so any required check that lives OUTSIDE the
builder (cobra MarkFlagsOneRequired, or a shortcut's own Validate) is silently
bypassed and the default value wins. Two gaps surfaced in PR review:
- +csv-put: with neither --start-cell nor --range set, start-cell's "A1"
default won and the paste silently anchored at A1. Require an explicit anchor
(guard on Changed, mirroring the standalone MarkFlagsOneRequired).
- +sheet-move: --index (plus >=0 bounds for index / source-index) was not
enforced in the batch path; a missing --index silently moved the sheet to the
front. Mirror SheetMove.Validate.
Also from the same review:
- +batch-update: an explicit --continue-on-error=false now wins over an
--operations envelope's continue_on_error:true (guard on Changed, not value).
- validateDropdownRanges rejects malformed sheet!range ("!A1", "Sheet1!",
"Sheet1!bad") at Validate instead of deferring to the server.
Tests added/updated for each path; full sheets suite green.
* fix(cli): surface skill in deprecated_command notice
deprecation.Notice carries Skill, but the _notice.deprecated_command payload
dropped it, forcing callers to parse `message` to learn which skill to update.
Emit `skill` when set, alongside the existing `replacement`.
* fix(sheets): harden batch type-checking and +workbook-create edge cases
From the branch code-review doc (3 findings):
- +batch-update sub-ops: `operations` is skipped by parse-time schema
validation and mapFlagView coerces a type-mismatched scalar to its zero
value, so "index":"abc" or "multiple":"true" silently became 0 / false and
wrote to the wrong place. translateBatchOp now runs validateRawTypes, which
checks each sub-op scalar against its flag-defs type and rejects mismatches.
- +workbook-create with empty arrays: buildInitialFillInput returned (nil,nil)
for empty rows while the caller wrote fill["excel_id"] unconditionally, so
--values '[]' panicked on a nil map and --headers '[]' produced an illegal
"A1:1" range. It now also returns nil when no cells survive (maxCols==0
guard) and Execute/DryRun skip the fill when fill==nil.
- +workbook-create partial failure: after the spreadsheet was created, a
first-sheet lookup or fill failure returned a bare fmt.Errorf, losing the new
token. It now returns a structured partial_success error carrying
spreadsheet_token in the detail so callers can retry or clean up.
Tests added for each path; sheets suite green.
* fix(cli): structured errors for unknown flags, print-schema, deprecated aliases
From the branch code-review doc (3 findings):
- pure-group UnknownFlags: installUnknownSubcommandGuard whitelists unknown
flags so a mistyped subcommand still reaches the suggestion path, but a lone
unknown flag before any subcommand (`sheets --badflag`) was swallowed and the
group fell through to help + exit 0. unknownSubcommandRunE now recovers the
swallowed tokens (from os.Args captured at Execute entry) and fails with a
structured unknown_flag error; a misplaced but known flag (e.g. --format)
still prints help.
- deprecated-alias notice: a backward-compat alias that fails a cobra-level
required flag short-circuits before RunE, so the Validate/Execute-wrapped
deprecation notice was dropped. Added Shortcut.OnInvoke, fired from PreRunE
(ahead of ValidateRequiredFlags); and the root legacy error fallback now
routes through the structured envelope when a deprecation is pending so the
migration hint survives. Non-deprecated errors keep the plain output.
- --print-schema: runShortcut returned the bare error from PrintFlagSchema. It
is now wrapped as a structured output.ExitError (type print_schema_error) so
agent introspection can parse the failure.
Tests added for each path; cmd + sheets suites green.
* fix(sheets): resolve --sheet-name via title + keep bare sheet selectors verbatim
Two review findings on the backward-compat layer:
- lookupSheetIndex matched only sm["sheet_name"], but get_workbook_structure
surfaces the sub-sheet display name as "title". Every --sheet-name path that
relies on the lookup (e.g. +sheet-move) failed to resolve. Fall back to
"title" when "sheet_name" is absent so either field resolves.
- +read / +write / +append fell back to --sheet-id when --range was omitted,
then routed that bare sheet id through the range normalizer. A sheet id that
looks A1-ish (letters+digits, e.g. "shtABC123") got mangled into
"shtABC123!shtABC123:shtABC123". Split the sheet-only path from the
range-normalization path: read/append pass the selector through verbatim,
write builds the rect from the selector's A1.
Regression tests added for both paths; sheets suite green.
* fix(sheets): silence nilerr/copyloopvar lint in batch type-check additions
- flag_view.go: annotate the fail-open return in validateRawTypes with
//nolint:nilerr (matches the repo convention for intentional fail-open).
- execute_paths_test.go: drop the redundant tc := tc copy (Go 1.22+ scopes
the loop var per iteration).
* test(sheets): data-driven required-flag parity contract for batch sub-ops
Adds TestBatchOp_RequiredFlagParity, the systematic standalone-vs-batch parity
check the branch review asked for. Data-driven over batchOpDispatch + flag-defs,
it asserts that for every batchable shortcut a +batch-update sub-op which
satisfies the sheet locator but omits the shortcut's business-required flags
fails in translateBatchOp, never silently defaulting.
This generalizes the hand-picked TestBatchOp_ErrorEquivalence / GuardsBeyondCobra
cases to the full 50-command surface and auto-covers shortcuts added later, so a
future refactor that moves a required check out of the shared *Input builder
(the failure mode behind the csv-put / sheet-move gaps) is caught here. 45
sub-tests run; locator-only commands (+sheet-delete / +sheet-hide / ...) have no
business-required flag to omit and are skipped. A missing-locator error is also
rejected so a bad fixture can't mask a real gap.
* refactor(sheets): drop unused int64 flag-type plumbing
No sheets flag-def declares an int64 type and RuntimeContext.Int64 had
zero callers, so remove the premature support: the RuntimeContext.Int64
helper, the registerShortcutFlagsWithContext int64 branch, the flagView
Int64 method + mapFlagView impl, and the typedDefault/validateRawTypes
int64 cases. float64 (consumed by --font-size) is kept.
* test(sheets): drop redundant copyloopvar copy in required-flag parity test
Go 1.22+ scopes the loop var per iteration, so `cmd, business := cmd, business`
in TestBatchOp_RequiredFlagParity is a no-op that trips the repo's copyloopvar
linter (same cleanup as 2132472). Behavior unchanged; 45 sub-tests still pass.
* revert(cli): drop non-interactive proxy-warning silencing
WarnIfProxied's interactivity gate is a generic CLI/agent-UX change
unrelated to the sheets refactor / backward-compat scope of this branch.
Split out to a dedicated PR; restore WarnIfProxied to its single-arg form
here (warn.go, warn_test.go, factory_default.go callers).
* docs(sheets): correct +workbook-info output field and batch +sheet-move index requirement
Sync from spec: +workbook-info returns sheet display name as 'title'
(sheet_name only as legacy fallback), and +sheet-move inside +batch-update
also requires --index, not just --sheet-id/--source-index.
* fix(sheets): reject non-integer numbers for batch int flags
validateRawTypes treated int and float64 identically (both only required a
JSON number), but mapFlagView.Int() truncates float64 via int(t), so a batch
sub-op accepted 1.9 for an int flag (e.g. --index) and silently floored it to
1. Standalone cobra rejects non-integer input for int flags at parse time;
enforce the same in the batch path with a math.Trunc check so batch/standalone
parity holds and positional fields can't land on a floored value.
* fix(cli): align flag-before-subcommand unknown_flag detail schema
The flag-before-subcommand recovery path emitted a Type: unknown_flag whose
detail only carried unknown_flags + command_path, diverging from
flagDidYouMean's unknown_flag detail (unknown, command_path, suggestions,
valid_flags). A consumer keyed on Type then saw two shapes for one Type.
Emit the same keys from both paths: add unknown (the offending flag; joined
when multiple), plus empty suggestions/valid_flags — the subcommand isn't
resolved at this point, so there is no meaningful flag universe to suggest
from, and the group's own flags would mislead. unknown_flags is retained as
the authoritative multi-flag field. Test locks the shared schema.
* perf(sheets): compile flag specs to Go to drop startup JSON parse
Every lark-cli invocation (sheets or not) unmarshaled data/flag-defs.json
(122KB) and data/flag-schemas.json (256KB) during package init, before
main(): flag-defs via the shortcut package vars (flagsFor runs at init),
flag-schemas via shortcuts.init() -> Shortcuts() -> commandsWithFlagSchema().
On a 0.5-core sandbox this cold-start cost lands on every command.
Compile both specs to Go at build time instead of parsing at runtime:
- flag-defs.json -> flag_defs_gen.go: flagDefs is a compiled map literal;
loadFlagDefs() returns it directly (no embed, no Unmarshal).
~3.3ms/4110 allocs -> ~0.57ms/539 allocs at sheets package init.
- flag-schemas.json -> flag_schemas_gen.go: only the command-name set
(commandsWithSchema) is compiled in; registration and the validate
fast-path gate on it without touching the 256KB blob. The blob stays
embedded and is unmarshaled lazily only on --print-schema or when
validating a command that has a schema. Removes the 256KB parse from
init entirely.
data/*.json remain the canonical source; *_gen.go are committed, derived
artifacts regenerated with `go generate ./shortcuts/sheets/...`
(shortcuts/sheets/internal/gen). *_gen_test.go guard source/generated drift.
No behavior change: flag rendering, required/enum/default, --print-schema,
and composite-flag schema validation verified unchanged; ./shortcuts/...
tests pass.
* ci(sheets): exempt internal/gen generators from forbidigo
The shortcuts/sheets/internal/gen code generator is a standalone
`package main` run via go:generate, not shortcut runtime code, so the
forbidigo bans on log.Fatal / os.ReadFile / fmt.Printf do not apply.
Making it "compliant" is impossible anyway: a structured error return
needs os.Exit (also banned), and the vfs alternative is blocked by
depguard shortcuts-no-vfs. Exempt shortcut internal/gen paths, matching
the existing _test.go and internal/vfs forbidigo exemptions.
* fix(cli): fail structured on flags before a missing subcommand
A pure group invoked with flags but no subcommand (e.g. `im --format=json`,
`sheets --format json`) silently fell through to help + exit 0, so an agent
could mistake a malformed call for success. The unknown-subcommand guard's
FParseErrWhitelist swallows the flags and leaves RunE with empty args; it now
recovers the raw flag tokens and fails structured:
- unknown flag(s) -> unknown_flag (unchanged)
- valid flag, no subcmd -> missing_subcommand (new, exit 2)
- bare group -> help, exit 0 (unchanged)
Because the group RunE is hook-wrapped, returning a real error also makes
plugin observers record the call as failed instead of ok (the lifecycle Err
is no longer flipped to nil).
Hardening from the same review:
- document the cobra error-text contract unknownFlagName relies on, in
both cmd/root.go and go.mod, so an i18n/reword is caught on upgrade.
- guard the reserved --print-schema/--flag-name registration with a Lookup
so a shortcut declaring same-named flags can't panic pflag.
Tests cover the new missing_subcommand path and the reserved-flag collision.
* fix(cli): don't flag group-valid globals as a missing subcommand
9f8dfa72 made a pure group invoked with flags but no subcommand fail with
missing_subcommand, keying on "any flag defined in the tree". That also matches
inherited global flags (--profile, ...), so `lark-cli --profile p im` and
`lark-cli im --profile p` errored with a misleading "flag --profile belongs to
a subcommand" instead of printing the group's help — a regression, since a bare
group carrying a global flag should print help.
Only treat a flag as missing_subcommand when it is valid on a subcommand but
not on the group itself or inherited (subcommandOnlyFlagTokens). A bare group
carrying only group-valid/global flags falls through to help; flags that
genuinely belong to an omitted subcommand (`im --format json`) still fail
structured, and unknown flags (`im --badflag`) still report unknown_flag.
Test covers a global flag on a bare group resolving to help.
---------
Co-authored-by: zhengzhijie <zhengzhijie.j@bytedance.com>
Follow-up to #1223. The hand-written FLAGS block in `lark-cli --help`
restated leaf-command flags at the root level — flags that are not
registered on the root command (they error "unknown flag" there). Even
trimmed to an illustrative example list, it duplicated information Cobra's
per-command `--help` already renders authoritatively, and any static list
in root help drifts from the real per-command flag sets over time.
Drop the section entirely: Cobra's per-command `Flags:` output is the
single source of truth. `USAGE:`/`EXAMPLES:` still show flags in context,
and the `Flags:` block at the bottom of root help lists the actual root
flags. Also removes the now-obsolete TestRootLong_FlagsSectionPointsToCommandHelp.
The hand-written FLAGS block in `lark-cli --help` listed --params, --data,
--as, --format, --page-all, --page-size, --page-limit, --page-delay, -o,
--jq, -q and --dry-run as if they were global flags. None are registered
on the root command — they all error "unknown flag" at the top level and
exist only on leaf commands (api, service). The block also contradicted
the Cobra-generated "Flags:" section rendered directly below it, which
shows only -h/--help, --profile, -v/--version.
Replace it with a short illustrative example list (common flags first) and
a pointer to `lark-cli <command> --help` for the full per-command set.
Root help stays a discovery signpost without claiming the flags are global
or restating defaults/descriptions that drift from the real flag sets.
Change-Id: Ia1cab889dd70b6b49a61dac468dedfd7fe39043f
Every failure on the authentication, authorization, and configuration
path now surfaces as a typed structured error instead of an ad-hoc
envelope. Users and scripts that consume CLI output get:
- a fixed nine-category taxonomy on the wire, each mapped to a
stable shell exit code (authentication/authorization/config = 3,
network = 4, internal = 5, policy = 6, confirmation = 10)
- identity-aware detail fields (missing_scopes, requested_scopes,
granted_scopes, console_url, log_id, retryable, hint) carried
uniformly on the envelope
- a single canonical policy envelope at exit 6; the legacy
auth_error carve-out is retired
- per-subtype canonical message + hint that preserves Lark's
diagnostic phrasing and routes recovery to the right actor:
app developer (app_scope_not_applied), user (missing_scope,
token_scope_insufficient, user_unauthorized), or tenant admin
(app_unavailable, app_disabled)
- wrong app credentials classify as config/invalid_client whether
surfaced by the Open API endpoint (99991543) or the tenant
access-token mint endpoint (10003 / 10014), instead of
collapsing to a transport error or api/unknown
- local shortcut scope preflight emits the same
authorization/missing_scope envelope (identity + deterministic
missing-scope set) used by the post-call permission path, so AI
consumers read the same structured shape from precheck and from
server-returned permission denial
- streaming download/upload failures keep the same network subtype
split (timeout / TLS / DNS / transport) as the non-stream path
instead of collapsing every cause to a generic transport failure
- console_url is carried only on the bot-perspective
app_scope_not_applied envelope (where the recovery action is
"developer applies the scope at the developer console"); the
user-perspective missing_scope envelope drops the field, since
the only actionable user recovery is `lark-cli auth login --scope`
and pointing an end user at a console they cannot modify is
misleading
- bind workflows (Hermes / OpenClaw / lark-channel) flatten dynamic
Type tags to wire 'config' with the original module name kept
as a metric label
All 10 typed errors are cause-bearing, nil-safe on .Error() and
.Unwrap(), and defensively clone slice setter inputs. Four lint
rules (CheckNilSafeError / CheckBuilderImmutable / CheckUnwrapSymmetry
/ CheckBuildAPIErrorArms) lock these invariants on migrated paths.
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(extension): introduce Plugin / Hook framework with command pruning
Add a single public extension contract under extension/platform: integrators
implement the Plugin interface and register Observers, Wrappers, Lifecycle
handlers, and pruning Rules through the Registrar in one Install call.
Command pruning:
- Rule (Allow / Deny / MaxRisk / Identities) with doublestar globs
- 4-axis AND evaluation, parent-group aggregation, unknown-risk allow
- Sources: Plugin.Restrict (single-rule) and ~/.lark-cli/policy.yml
- Plugin path is fail-closed (envelope on rule error / multiple Restrict);
yaml path is fail-open (warning, CLI continues)
- strict-mode stubs now also write the denial annotation so the hook
layer's denial guard physically isolates Wrap chains on them
- HOME path never leaked through policy_source label
Hook framework:
- Observer (panic-safe, Before/After), Wrapper (middleware, may short-circuit
via AbortError), Lifecycle (Startup + Shutdown only)
- Recover guards every plugin entry point: Capabilities(), Install(),
Wrapper factory composition AND inner Handler, Lifecycle handlers
- namespacedWrap copies AbortError so a plugin's package-level sentinel
is never mutated across concurrent invocations
- Selector unknown-risk uniform: ByExactRisk / ByWrite / ByReadOnly never
match unannotated commands; safety-side hooks opt in via
ByWrite().Or(ByUnknownRisk())
Bootstrap orchestration (cmd/build.go + cmd/policy.go):
- InstallAll uses a staging Registrar + atomic commit
- FailClosed plugin install / Plugin.Restrict conflict / Startup handler
failure each install a structured envelope guard at every dispatch path
- walkGuard neutralises every cobra bypass we know of (PersistentPreRunE
first-wins, ValidateArgs, ParseFlags, legacyArgs, __complete /
__completeNoDesc, non-runnable groups, required-arg subcommands)
- cmd/root.go::Execute calls hook.Emit(Shutdown, runErr) after
rootCmd.Execute; isCompletionCommand skips both __complete and
__completeNoDesc so Tab completion never triggers Shutdown handlers
Capabilities consistency:
- Restricts=true must declare FailurePolicy=FailClosed
- RequiredCLIVersion (semver constraint) is validated against build.Version;
a malformed constraint is treated as untrusted-config and aborts
unconditionally, regardless of FailurePolicy (DEV builds included)
JSON envelope contract:
- error.type closed enum: pruning / strict_mode / hook / plugin_install /
plugin_conflict / plugin_lifecycle
- reason_code closed enums per type, all referenced by structured tests
Bootstrap surfaces (new user commands):
- lark-cli config policy show -- JSON view of the active Rule + source
- lark-cli config policy validate -- parse + schema + glob check, no apply
Coverage:
- extension/platform: every public type has a unit test
- internal/{pruning,hook,platformhost,policydecision,cmdmeta}: full coverage
of denial guard isolation, AbortError sentinel safety, observer panic
safety, lifecycle error/panic typing, staging atomic rollback
- cmd/plugin_integration_test.go: end-to-end through buildInternal with
synthetic and real command trees
- cmd/install_guard_test.go: walkGuard covers auth / config / __complete /
__completeNoDesc / non-runnable parents
* fix(pruning): deny stub must override Args + PersistentPreRunE
The pruning denyStub and the strict-mode stub previously only swapped
RunE plus Hidden + DisableFlagParsing. Cobra's dispatch order means
several pre-RunE gates can fire BEFORE the stub's RunE ever runs:
1. Args validator: shortcut commands often declare cobra.NoArgs.
With DisableFlagParsing=true the user's `--doc xxx --mode append`
looks like positional args, so ValidateArgs surfaces a usage
error instead of the pruning / strict_mode envelope. Observer
hooks also miss the dispatch entirely.
2. Parent PersistentPreRunE: cmd/auth/auth.go declares a
PersistentPreRunE that returns external_provider when env
credentials are set. Cobra's "first PersistentPreRunE wins
walking up from the leaf" then short-circuits with
external_provider instead of the leaf's denial envelope.
Both stubs now also set:
- Args = cobra.ArbitraryArgs (bypass gate 1)
- PersistentPreRunE = no-op leaf hook (bypass gate 2)
- PreRunE / PreRun / PersistentPreRun = nil (defensive)
Effect: dispatch reaches the wrapped RunE, observers fire, the real
pruning / strict_mode envelope is emitted regardless of credential
provider or flag count.
Adds regression tests covering both gates on both stub paths.
* fix(config): policy subcommand bypasses parent's credential check
cmd/config/config.go::NewCmdConfig declares a PersistentPreRunE that
calls f.RequireBuiltinCredentialProvider; with env credentials set,
it returns external_provider for every config subcommand.
`config policy show` and `config policy validate` are READ-ONLY
diagnostic commands -- they inspect or parse the user-layer rule
without touching credentials. They MUST work regardless of which
credential provider is active, otherwise users on env-credential
deployments cannot debug their policy.
Same shape as the codex C11/C13 fix: install a no-op leaf-level
PersistentPreRunE on the `policy` group so cobra's "first walking up
from leaf" rule picks ours over the config parent's.
Regression caught by divergent e2e (F1-F6 all returned external_provider
before this fix; all pass after). Adds a unit test pinning the
PersistentPreRunE override.
* feat(shortcuts): tag service groups with cmdmeta.Domain
RegisterShortcutsWithContext now calls cmdmeta.SetDomain on each
service-level cobra.Command (im, docs, drive, calendar, ...) so the
business-domain axis is actually populated on every shortcut leaf via
parent-chain inheritance.
Before this change, platform.ByDomain("docs") never matched any
command: the domain annotation was unset across the entire shortcut
tree, so the selector's d != "" guard always failed and risk-style
selectors silently degraded to no-op.
The SetDomain call is placed AFTER the create-or-reuse branch so it
fires whether the service command was freshly created here or had
already been added by cmd/service/service.go's OpenAPI auto-
registration (which runs first and creates im, drive, calendar, etc.).
Without this placement only pure-shortcut services like docs would
have been tagged.
Adds a regression test asserting:
- service-group cobra.Command carries the cmdmeta.domain annotation
- leaf shortcuts inherit the domain via parent-chain walk
* feat(diagnostic): add unconditionally allowed command paths for introspection
* feat(plugins): add diagnostic command to inspect installed plugins and their contributions
* fix(cli): surface unknown_subcommand error instead of silent help fallback
When a user passed an unknown subcommand or shortcut (e.g. `lark-cli drive
+bogus`), cobra returned `flag.ErrHelp` for the non-runnable group command,
printed the parent help, and exited 0. AI agents couldn't distinguish a
typo from an intentional help request.
Install a tree-wide guard that attaches a RunE to every group command
without its own Run/RunE. The RunE forwards no-args invocations to help
(preserving prior behavior) and emits a structured unknown_subcommand
ExitError (exit 2) listing available subcommands when args are present.
* refactor(envelope): rename error.type pruning/strict_mode to command_denied
The envelope's `type` field was leaking implementation terms ("pruning",
"strict_mode") that describe enforcement mechanism rather than the user-
facing semantic. It also duplicated `detail.layer`, and forced consumers
to branch on two values for the same conceptual error ("a command was
denied by policy").
Collapse both into a single semantic type "command_denied". The
enforcement layer ("pruning" / "strict_mode") is preserved in
`detail.layer` so debugging and per-layer diagnostics still work.
* feat(platform): fail closed on unannotated/invalid risk when a Rule is active
The pruning engine used to treat any command without a risk annotation as
ALLOW even when a Rule with MaxRisk was set, and would silently skip the
MaxRisk comparison whenever the command's risk string was outside the
closed taxonomy. Both gaps let an unannotated or typo'd write command
slip past an "agent read-only" pruning rule.
Engine now denies before any other axis when a Rule is registered:
- reason_code "risk_not_annotated" for commands with no risk
- reason_code "risk_invalid" for commands whose risk is outside
the read | write | high-risk-write
taxonomy (e.g. typo "wrtie")
Main-flow is preserved: a nil Rule still returns Allowed=true
unconditionally, so a CLI with no pruning plugin behaves identically to
before. ByUnknownRisk() is removed from the public surface since the
Unknown state is no longer reachable through risk-based selectors when
any Rule is active; safety-side widening composition is no longer needed.
* chore(config): hide diagnostic policy/plugins commands from --help
`config policy show`, `config policy validate`, and `config plugins show`
are local-introspection-only commands kept behind the pruning
diagnostic whitelist so operators can always inspect why a command was
denied. They do not need to surface in `--help` for AI agents and were
contributing to help noise.
Hide the `policy` and `plugins` parent groups and both `show` /
`validate` leaves. Commands remain callable by exact name and continue
to bypass user-layer pruning via diagnosticPaths.
* style: gofmt
* fix(platform): nil Selector honours None contract; reject multi-doc policy yaml
- selector.go: And/Or/Not now treat nil Selector as None() per godoc,
preventing runtime panic when composed selectors are invoked.
- schema.go: Parse rejects multi-document YAML input so a stray '---'
separator can't silently drop trailing policy constraints.
* chore: go mod tidy
* feat(extension/platform): plugin SDK with policy engine, hooks, and Builder
Introduces extension/platform — the in-process plugin SDK external
Go forks of lark-cli use to extend or restrict the command surface.
Plugins compile in via blank import; there is no dynamic loading
and no RPC isolation.
Public SDK (extension/platform):
- Plugin interface (Name / Version / Capabilities / Install).
- Registrar verbs: Observe, Wrap, On, Restrict.
- Hook types: Observer (side-effect, panic-safe, fires Before/After
RunE), Wrapper (middleware, may short-circuit via AbortError),
LifecycleHandler (Startup / Shutdown), Selector with nil-safe
And/Or/Not composition.
- Risk / Identity are defined string types with closed taxonomies;
ParseRisk / ParseIdentity convert raw strings with the
absent-vs-invalid distinction the engine relies on.
- Builder ergonomic constructor (NewPlugin().Observer().Wrap()
...MustBuild()) that enforces name/hookName grammar, hookName
uniqueness, and the Restrict ↔ FailClosed pairing regardless of
call order.
- Invocation is a read-only interface; the framework's concrete
invocation type lives in internal/hook so plugins cannot
fabricate denial / strict-mode / identity state. Args() returns
a defensive copy on every call so hook mutation cannot leak
into the original RunE.
- CommandDeniedError + AbortError carry structured fields for the
closed `command_denied` / `hook` envelope contract.
- ResetForTesting gated behind //go:build testing.
- README + godoc examples (Observer / Wrapper / Restrict) + two
runnable example forks (audit-observer, readonly-policy).
Host (internal/platform, internal/hook, internal/cmdpolicy):
- InstallAll: staged plugin registration with atomic commit, panic
isolation, FailOpen / FailClosed semantics, RequiredCLIVersion
semver check, single-Restrict invariant, duplicate-plugin-name
detection.
- hook.Install wraps every runnable cmd.RunE with:
Before observers (panic-safe) → denial guard → composed Wrap
chain → original RunE → After observers (always fire, even on
err). Denied commands physically bypass the Wrap chain so a
plugin Wrapper cannot suppress or rewrite a denial; observers
still see the attempt for audit.
- Recover shim around plugin Wrappers converts panics (including
the factory call) into a structured `hook` envelope with
reason_code=panic; namespacing shim attributes AbortError to
the namespaced hook name.
- cmdpolicy (renamed from internal/pruning) is the user-layer
command policy engine: walks the cobra tree, evaluates each
runnable command against a Rule's four-axis filter (Allow /
Deny / MaxRisk / Identities), produces parent-group aggregate
denials, and installs denyStubs. Rule.AllowUnannotated opts out
of the unannotated-deny gate for gradual adoption; risk_invalid
typos always deny with an edit-distance "did you mean"
suggestion.
- Strict-mode stub in cmd/prune.go composes the shared
detail.* / wrapped CommandDeniedError shape via cmdpolicy
helpers (BuildDenialError / CommandDeniedFromDenial /
DenialDetailMap), so command_denied envelopes from strict-mode
and user-layer policy carry the same closed-enum fields
(detail.layer / reason_code / policy_source). The historical
short Message + independent Hint are preserved unchanged.
- cmdpolicy/yaml: structural parsing of ~/.lark-cli/policy.yml
with KnownFields strict mode, including allow_unannotated.
- `config policy show` / `config policy validate` and the plugin
inventory diagnostic surface the resolved Rule (allow,
deny, max_risk, identities, allow_unannotated) and the hook
contributions per plugin.
Envelope contract (docs/extension/reason-codes.md):
- error.type is a closed set: command_denied, hook, plugin_install,
plugin_conflict, plugin_lifecycle.
- reason_code is a closed enum per error.type, dispatched on by
external agents and CI integrations.
- detail.layer = "policy" | "strict_mode" attributes the rejection.
Build / CI:
- Makefile unit-test / vet / coverage and ci.yml fast-gate +
unit-test + coverage now pass -tags testing so register_testing.go
is visible; ./extension/... is in the package list so the SDK's
own tests actually run.
- fmt-check and examples-build Makefile targets.
- bmatcuk/doublestar/v4 added as a direct dependency for `**` glob
matching in Rule.Allow / Rule.Deny.
Author-facing material:
- docs/extension/ (quickstart, plugin-author-guide, reason-codes)
is provided in the working tree but kept out of git tracking
per repo convention (.gitignore covers docs/).
Change-Id: I3b8ecc2923bd54c2dff19e5dce8a0855a6f9e703
* feat(extension/platform): plugin SDK with policy engine, hooks, and Builder
Introduces extension/platform — the in-process plugin SDK external
Go forks of lark-cli use to extend or restrict the command surface.
Plugins compile in via blank import; there is no dynamic loading
and no RPC isolation.
Public SDK (extension/platform):
- Plugin interface (Name / Version / Capabilities / Install).
- Registrar verbs: Observe, Wrap, On, Restrict.
- Hook types: Observer (side-effect, panic-safe, fires Before/After
RunE), Wrapper (middleware, may short-circuit via AbortError),
LifecycleHandler (Startup / Shutdown), Selector with nil-safe
And/Or/Not composition.
- Risk / Identity are defined string types with closed taxonomies;
ParseRisk / ParseIdentity convert raw strings with the
absent-vs-invalid distinction the engine relies on.
- Builder ergonomic constructor (NewPlugin().Observer().Wrap()
...MustBuild()) that enforces name/hookName grammar, hookName
uniqueness, and the Restrict ↔ FailClosed pairing regardless of
call order.
- Invocation is a read-only interface; the framework's concrete
invocation type lives in internal/hook so plugins cannot
fabricate denial / strict-mode / identity state. Args() returns
a defensive copy on every call so hook mutation cannot leak
into the original RunE.
- CommandDeniedError + AbortError carry structured fields for the
closed `command_denied` / `hook` envelope contract.
- ResetForTesting gated behind //go:build testing.
- README + godoc examples (Observer / Wrapper / Restrict) + two
runnable example forks (audit-observer, readonly-policy).
Host (internal/platform, internal/hook, internal/cmdpolicy):
- InstallAll: staged plugin registration with atomic commit, panic
isolation, FailOpen / FailClosed semantics, RequiredCLIVersion
semver check, single-Restrict invariant, duplicate-plugin-name
detection.
- hook.Install wraps every runnable cmd.RunE with:
Before observers (panic-safe) → denial guard → composed Wrap
chain → original RunE → After observers (always fire, even on
err). Denied commands physically bypass the Wrap chain so a
plugin Wrapper cannot suppress or rewrite a denial; observers
still see the attempt for audit.
- Recover shim around plugin Wrappers converts panics (including
the factory call) into a structured `hook` envelope with
reason_code=panic; namespacing shim attributes AbortError to
the namespaced hook name.
- cmdpolicy (renamed from internal/pruning) is the user-layer
command policy engine: walks the cobra tree, evaluates each
runnable command against a Rule's four-axis filter (Allow /
Deny / MaxRisk / Identities), produces parent-group aggregate
denials, and installs denyStubs. Rule.AllowUnannotated opts out
of the unannotated-deny gate for gradual adoption; risk_invalid
typos always deny with an edit-distance "did you mean"
suggestion.
- Strict-mode stub in cmd/prune.go composes the shared
detail.* / wrapped CommandDeniedError shape via cmdpolicy
helpers (BuildDenialError / CommandDeniedFromDenial /
DenialDetailMap), so command_denied envelopes from strict-mode
and user-layer policy carry the same closed-enum fields
(detail.layer / reason_code / policy_source). The historical
short Message + independent Hint are preserved unchanged.
- cmdpolicy/yaml: structural parsing of ~/.lark-cli/policy.yml
with KnownFields strict mode, including allow_unannotated.
- `config policy show` / `config policy validate` and the plugin
inventory diagnostic surface the resolved Rule (allow,
deny, max_risk, identities, allow_unannotated) and the hook
contributions per plugin.
Envelope contract (docs/extension/reason-codes.md):
- error.type is a closed set: command_denied, hook, plugin_install,
plugin_conflict, plugin_lifecycle.
- reason_code is a closed enum per error.type, dispatched on by
external agents and CI integrations.
- detail.layer = "policy" | "strict_mode" attributes the rejection.
Build / CI:
- Makefile unit-test / vet / coverage and ci.yml fast-gate +
unit-test + coverage now pass -tags testing so register_testing.go
is visible; ./extension/... is in the package list so the SDK's
own tests actually run.
- fmt-check and examples-build Makefile targets.
- bmatcuk/doublestar/v4 added as a direct dependency for `**` glob
matching in Rule.Allow / Rule.Deny.
Author-facing material:
- docs/extension/ (quickstart, plugin-author-guide, reason-codes)
is provided in the working tree but kept out of git tracking
per repo convention (.gitignore covers docs/).
Change-Id: I3b8ecc2923bd54c2dff19e5dce8a0855a6f9e703
* refactor(policy): remove validate command and update diagnostics
* fix(extension/platform): address PR review must-fix items
- cmdpolicy: skip AnnotationPureGroup commands in EvaluateAll,
aggregateParents, and hasRunnableDescendant so user-layer policy
no longer blocks `<group> --help` after the unknown-subcommand
guard attaches RunE to every parent
- cmd/root: tag guarded parent groups with AnnotationPureGroup
- extension/platform: drop `//go:build testing` from register_testing.go
so `go test ./...` works without an extra build tag
- extension/platform/README: inline reason_code reference, fix plugin
lifecycle diagram order (init/Register precede RegisteredPlugins)
- cmd/platform_bootstrap: route userPolicyPath through
core.GetBaseConfigDir so LARKSUITE_CLI_CONFIG_DIR is honoured
- cmdpolicy: add RedactHomeDir helper, fold base config dir and
$HOME prefixes for config policy show + resolver errors
- internal/platform: reject unrecognised FailurePolicy values with
invalid_capability instead of silently fail-open
- cmd/config: surface diagnostic policy/plugins commands in
`config --help` Long text
- CHANGELOG: document command_denied error.type rename and
unknown_subcommand exit-2 behavior change
* fix(extension/platform): address CodeRabbit review comments + CI gofmt
- hook/install: propagate wrapper-injected ctx to invokeOriginal so
RunE/Run see context values added by upstream Wrappers
- hook/testing: SetStderrForTesting returns a restore func; tests now
defer it via t.Cleanup to avoid cross-test sink leakage
- cmdpolicy/active: deep-copy ActivePolicy.Rule on SetActive/GetActive
so callers can't mutate the stored global through shared slices
- platform/inventory: deep-copy Inventory + nested Plugins / HookEntry
/ RuleView slices on SetActiveInventory / GetActiveInventory
- platform/staging: Restrict clones the plugin-supplied Rule before
retaining it so the plugin can't mutate it after Install returns
- platform/version: reject RequiredCLIVersion with more than three
numeric components instead of silently truncating 1.2.3.4 to 1.2.3
- cmd/platform_bootstrap: clear cmdpolicy.SetActive on yaml resolver
error so config policy show doesn't surface a stale rule
- cmd/platform_bootstrap_test: tmpHome pins LARKSUITE_CLI_CONFIG_DIR
so host env can't bleed into the policy test fixtures
- cmdpolicy/apply: installDenyStub returns bool; Apply count no longer
over-reports when strict-mode short-circuits the install
- cmdpolicy/engine: aggregateParents now returns the runnable hybrid's
own denial status when all children are placeholder branches
- cmdpolicy/resolver_test: use t.TempDir()-rooted missing path instead
of hardcoded /nonexistent for hermetic missing-file assertion
- cmd/config/plugins: empty-inventory branch emits total: 0 so the
JSON schema stays stable across populated/empty cases
- cmd/platform_guards_test: select leaf by RunE != nil (not Runnable)
so the test doesn't nil-deref on Run-only commands
- gofmt run on previously committed cmdpolicy/path*.go (CI fast-gate)
* fix(cmdpolicy): replace filepath.Abs with filepath.Clean for lint policy
The depguard / forbidigo rule blocks filepath.Abs in internal/ on the
grounds that it accesses the filesystem (Getwd) directly. Switch
RedactHomeDir + foldPrefix to operate on filepath.Clean strings; real
callers pass already-absolute paths (resolver builds yamlPath via
filepath.Join on the absolute config root), so the redaction outcome
is unchanged for production inputs. Relative inputs fall through to
the unchanged branch — filepath.Rel rejects the mixed-absoluteness
case with an error, which the foldPrefix helper already treats as
"not a hit".
* refactor(cmdpolicy): pure Resolve + drop path redaction & verbose comments
- Resolve becomes a pure function; I/O moves to LoadYAMLPolicy so
precedence selection can be unit-tested without vfs mocks
- ActivePolicy drops YAMLPath; config policy show JSON loses yaml_path
and yaml_shadowed (and the TOCTOU stat that surfaced them)
- RedactHomeDir and path_test.go removed: the home-dir folding was only
earning its keep through the now-deleted yaml_path field
- cmd/build.go bootstrap block trimmed from 71 to 39 lines by cutting
PR-rationale comments; one note kept for the fail-CLOSED-vs-fail-OPEN
business rule
- cmd/config/config.go: parent Long no longer hard-codes hidden command
hints, matching their Hidden:true intent
Change-Id: Icfbb818ce3ef523c63286bfbed34c49be08ed6a2
* refactor(platform): drop StrictMode/Identity from Invocation interface
These two accessors were documented in the public SDK as "After observers
always see ok=true" but the framework never plumbed values to them, so they
always returned ("", false). Zero internal/example/test callers; a plugin
author trusting the doc would silently get wrong behaviour.
Identity is also fundamentally unsuited for Before observers (per-command
identity resolves inside RunE via f.AuthFor, after Before fires). StrictMode
is a global value better placed on a Framework/Environment interface than
per-Invocation. Removing is non-breaking now (no callers); adding later is
non-breaking too.
Change-Id: Ice200543e9bca3bda759ad98a6e34a56df69e915
* fix(prune): preserve original metadata on strict-mode denial stubs
strictModeStubFrom built a fresh *cobra.Command from scratch, dropping
the original command's annotations (risk_level, lark:supportedIdentities,
cmdmeta.domain) and help text. cobraCommandView is a live proxy walking
parent annotations, so after the Remove+Add replacement, audit observers
firing on a strict-mode-denied command saw Cmd().Risk()=("",false) and
Cmd().Identities()=nil -- breaking the first-class use case for
audit/compliance plugins.
Copy child.Annotations into the stub (stamping the denial annotations on
top) and propagate Short/Long for help-text parity with
cmdpolicy/apply.go::installDenyStub, which preserves these by virtue of
mutating in place.
Regression test asserts risk_level / supportedIdentities / Short / Long
all survive replacement, alongside the denial annotations.
Change-Id: I19810a34575996344b63e839066888c154d69335
* chore(platform): align docs with implementation; fold home in yaml warnings
Followup cleanup to the previous three refactor commits, addressing review
fallout where public docs / examples / contract notes still pointed at
deleted symbols or unimplemented designs:
- cmd/build.go: Build() docstring now mentions the plugin install + Startup
emit side effects; Shutdown only fires on Execute path
- extension/platform/doc.go, lifecycle.go, invocation.go: drop references
to the deleted StrictMode/Identity methods, restore minimal Godoc on
Cmd/Args/Started
- extension/platform/view.go, cmd/platform_bootstrap.go,
internal/hook/install.go: rewrite "snapshot before pruning" promise to
match the actual contract (live view + strict-mode stub metadata
preservation)
- cmd/platform_guards_test.go: stubInvocation drops the two old methods
- cmd/platform_bootstrap.go: redactHome() last-mile folds $HOME -> ~ in
warnPolicyError so an os.PathError carrying the absolute policy path
does not leak the user's home dir to stderr / agent / CI logs
- examples/readonly-policy/README.md: drop yaml_path from the sample
`config policy show` envelope (the field was removed in 52cbb92)
Change-Id: I2874cc2cf9225dfa44a9c07b2449149181b387cb
* chore(build): drop vestigial -tags testing from Makefile and CI
The `testing` build tag was introduced in 461e3c6 to gate
extension/platform/register_testing.go (ResetForTesting); PR review
0efee93 then dropped the //go:build testing directive from that file
so downstream `go test ./...` would work without the tag, but never
cleaned the matching tag references out of Makefile and ci.yml.
The result: 8 places passing -tags testing for a tag that nothing in
the repo actually gates, plus a Makefile comment that confidently
claims a gate exists. Net behaviour is identical to omitting the flag;
the only effect is misleading developers into believing there is a
test-only surface separation.
Drop the flag from vet / unit-test / lint / coverage / deadcode (head
+ base worktree) and remove the misleading comment. ResetForTesting's
public-API exposure was the conscious trade-off taken in 0efee93 and
is left untouched.
Change-Id: If0cd78c87d4aec2a2533419fe75b01aae6b165fd
* feat(cmdpolicy): enrich denial Reason with attempted value + rule constraint
The envelope reason for command_denied previously told the caller WHAT
axis failed but not the concrete values on each side, so an AI agent
reading the envelope could not tell which command identity / risk /
path was attempted vs. which the rule permits. The natural temptation
was then to recommend modifying the rule -- exactly the wrong nudge,
since policy exists to prevent the agent from rewriting its own limits.
Each Reason now carries both the attempted value and the rule's
constraint:
identity_mismatch:
"command supports identities [user]; rule allows [bot]"
domain_not_allowed:
"command path \"drive/+upload\" not in allow list [docs/** contact/**]"
command_denylisted:
"command path \"docs/+delete-doc\" matched deny pattern \"docs/+delete-*\""
risk_too_high / write_not_allowed:
"command risk \"high-risk-write\" exceeds rule max_risk \"write\""
risk_not_annotated:
"command has no risk_level annotation; rule denies unannotated commands"
(drops the prescriptive "set allow_unannotated=true" hint -- that
belongs in docs, not in the engine's denial path)
Adds firstMatch() helper so command_denylisted can name the specific
glob that fired; matchesAny() now wraps firstMatch.
Regression test pins the substring contract per reason_code so future
"comment cleanup" cannot silently strip the values out again.
Change-Id: I17c7cc9411f58e3e43ade5e1ce875f3b7fe3e5ea
* fix(cmdpolicy): gofmt engine_test.go
CI fast-gate flagged the test added in 2eb0c2b as unformatted. Local
make unit-test had it cached; should have run `make vet` (which runs
gofmt-equivalent check via fmt-check) before pushing. Trivial 3-line
indent fix.
Change-Id: I42297ae59f607b97b32e976c9ec1c9ec4ab7de21
* feat(cmd): annotate risk_level on all hand-written cobra commands
Without this, any non-empty user-layer policy.yml (default
allow_unannotated=false) denies these commands with reason_code
risk_not_annotated -- bricking auth login, config init, profile use
etc. on first contact with a policy.
cmdpolicy/engine evaluation now resolves to the intended axis (deny
list / allow list / max_risk / identities) instead of failing closed
on the unannotated gate. Policy authors can write `max_risk: write`
or `allow: [auth/** config/** ...]` to express real intent.
Classification:
read auth status/check/list/scopes, config show /
policy show / plugins show, doctor, completion,
schema, profile list, event list/status/schema/
consume
write auth login/logout, config init/bind/remove/
default-as/strict-mode, profile add/remove/
rename/use, event stop/_bus, api (raw transit)
high-risk-write update (replaces the CLI binary; failure can
leave the install broken)
Notes:
- api standalone is conservatively `write`; per-call risk is unknown
at parse time (raw transit), so static gating only enforces the
write-class minimum.
- event _bus is the hidden IPC daemon forked by consume; standalone
invocation by users is not expected, but the annotation keeps
policy evaluation consistent with the other event subcommands.
- The two diagnostic-allowlisted commands (config policy show /
plugins show) still bypass the engine via diagnosticPaths; the
read annotation is for consistency with surrounding leaves.
---------
Co-authored-by: liangshuo-1 <266696938+liangshuo-1@users.noreply.github.com>
The previous default (atomic.Bool zero-value = enabled) meant any
*cobra.Command built without first calling configureFlagCompletions
leaked into cobra's package-global flagCompletionFunctions map. Bench
runs (scripts/bench_build) showed hundreds of KB and thousands of
objects retained per Build call.
Flip the semantics so the zero-value matches the safe default:
- Rename internal var to flagCompletionsEnabled (zero = disabled).
- Rename public API to SetFlagCompletionsEnabled / FlagCompletionsEnabled.
- Update call sites in cmd/root.go and scripts/bench_build/main.go.
- Add cmd.TestBuild_DefaultNoCompletionLeak: asserts that, with no
setter call at all, repeated cmd.Build invocations stay under 50 KB
and 500 objects per build (observed: ~0.7 KB, 3 objs/build). This
closes the gap that let the wrong default ship — every previous
test explicitly Set the switch before exercising it.
Change-Id: Ifefb04af5fd45eea9676a344a64ad071b6a4cd1a
* fix: skip flag-completion registration outside completion path
Cobra keeps completion callbacks in a package-global map keyed by
*pflag.Flag with no removal path, so registrations made during Build()
outlive the command itself. Route all seven call sites through
cmdutil.RegisterFlagCompletion and enable registration only when the
invocation actually serves a __complete request.
Measured over 30 dropped Builds: ~202 KB / 2180 retained objects per
Build before, ~0 after.
Change-Id: I734d598a4c91a92c33b02e0f292f640cc0e224c6
MarkRaw previously skipped both enrichPermissionError and
WriteErrorEnvelope, causing api command to exit 1 with no output
on API errors. Now MarkRaw only skips enrichPermissionError while
WriteErrorEnvelope always runs, ensuring stderr error envelope is
always written.
- Simplify apiRun MarkRaw logic (remove unnecessary IsJSONContentType check)
- Update existing tests to match new behavior
- Add 5 e2e tests covering api/service/shortcut error output
liangshuo-1