github/larksuite-cli
1
0
Fork 0
mirror of https://github.com/larksuite/cli.git synced 2026-07-08 10:08:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(sheets): align +cond-format / +filter with server schema (#4 + #5)

Browse Source 
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).
This commit is contained in:
zhengzhijie
2026-05-21 16:22:15 +08:00
parent 8d0fefd9e0
commit 4e44e668f7
8 changed files with 128 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
shortcuts/sheets/batch_op_contract_test.go
View File

@@ -210,8 +210,8 @@ func TestBatchOp_BodyMatchesStandalone(t *testing.T) {
{
shortcut: "+cond-format-create",
sc: CondFormatCreate,
args: []string{"--sheet-id", "sh1", "--properties", `{"style":{}}`, "--rule-type", "duplicate", "--ranges", `["A1:A100"]`},
subInput: `{"sheet-id":"sh1","properties":{"style":{}},"rule-type":"duplicate","ranges":["A1:A100"]}`,
args: []string{"--sheet-id", "sh1", "--properties", `{"style":{}}`, "--rule-type", "duplicateValues", "--ranges", `["A1:A100"]`},
subInput: `{"sheet-id":"sh1","properties":{"style":{}},"rule-type":"duplicateValues","ranges":["A1:A100"]}`,
},
{
shortcut: "+filter-create",
@@ -516,6 +516,21 @@ func TestBatchOp_RejectsBadSubOpInput(t *testing.T) {
`{"sheet-id":"sh1","image-name":"x.png","image-token":"t","position-row":0,"position-col":"A","size-width":100,"size-height":50}`,
"--float-image-id is required",
},
// +filter-{update,delete} need sheet-id (not sheet-name) because
// server contract: filter_id === sheet_id, and we can't resolve
// sheet-name → sheet-id mid-batch.
{
"+filter-update with --sheet-name only (filter_id must equal sheet_id)",
"+filter-update",
`{"sheet-name":"Sheet1","range":"A1:F1000","properties":{"rules":[]}}`,
"+filter-update requires --sheet-id",
},
{
"+filter-delete with --sheet-name only (filter_id must equal sheet_id)",
"+filter-delete",
`{"sheet-name":"Sheet1"}`,
"+filter-delete requires --sheet-id",
},
}
for _, tc := range cases {

62
shortcuts/sheets/data/flag-defs.json
View File

@@ -773,14 +773,14 @@
"kind": "own",
"type": "int",
"required": "required",
"desc": "Start position (0-based, inclusive)"
"desc": "Start position (0-based)"
},
{
"name": "end",
"kind": "own",
"type": "int",
"required": "required",
"desc": "End position (0-based, inclusive)"
"desc": "End position (exclusive)"
},
{
"name": "dry-run",
@@ -838,14 +838,14 @@
"kind": "own",
"type": "int",
"required": "required",
"desc": "Start position (0-based, inclusive)"
"desc": "Start position (0-based)"
},
{
"name": "end",
"kind": "own",
"type": "int",
"required": "required",
"desc": "End position (0-based, inclusive)"
"desc": "End position (exclusive)"
},
{
"name": "dry-run",
@@ -961,14 +961,14 @@
"kind": "own",
"type": "int",
"required": "required",
"desc": "Start position (0-based, inclusive)"
"desc": "Start position (0-based)"
},
{
"name": "end",
"kind": "own",
"type": "int",
"required": "required",
"desc": "End position (0-based, inclusive)"
"desc": "End position (exclusive)"
},
{
"name": "depth",
@@ -1046,14 +1046,14 @@
"kind": "own",
"type": "int",
"required": "required",
"desc": "Start position (0-based, inclusive)"
"desc": "Start position (0-based)"
},
{
"name": "end",
"kind": "own",
"type": "int",
"required": "required",
"desc": "End position (0-based, inclusive)"
"desc": "End position (exclusive)"
},
{
"name": "depth",
@@ -3184,7 +3184,7 @@
"kind": "own",
"type": "string",
"required": "required",
"desc": "JSON: `{\"data_range\":\"Sheet1!A1:F1000\",\"rows\":[...],\"columns\":[...],\"values\":[...],\"filters\":[...],\"show_row_grand_total\":true,\"show_col_grand_total\":true}`",
"desc": "JSON: {\"rows\":[...],\"columns\":[...],\"values\":[...],\"filters\":[...],\"show_row_grand_total\":true,\"show_col_grand_total\":true} (data source goes through --source; do not put data_range here)",
"input": [
"file",
"stdin"
@@ -3436,19 +3436,19 @@
"required": "required",
"desc": "Conditional format rule type; takes precedence over the same-named field inside `--properties`",
"enum": [
"cellValue",
"formula",
"duplicate",
"unique",
"topBottom",
"aboveBelowAverage",
"duplicateValues",
"uniqueValues",
"cellIs",
"containsText",
"timePeriod",
"containsBlanks",
"notContainsBlanks",
"dataBar",
"colorScale",
"iconSet",
"textContains",
"dateOccurring",
"blankCell",
"errorCell"
"rank",
"aboveAverage",
"expression",
"iconSet"
]
},
{
@@ -3527,19 +3527,19 @@
"required": "required",
"desc": "Conditional format rule type; takes precedence over the same-named field inside `--properties`",
"enum": [
"cellValue",
"formula",
"duplicate",
"unique",
"topBottom",
"aboveBelowAverage",
"duplicateValues",
"uniqueValues",
"cellIs",
"containsText",
"timePeriod",
"containsBlanks",
"notContainsBlanks",
"dataBar",
"colorScale",
"iconSet",
"textContains",
"dateOccurring",
"blankCell",
"errorCell"
"rank",
"aboveAverage",
"expression",
"iconSet"
]
},
{

34
shortcuts/sheets/lark_sheet_object_crud.go
View File

@@ -306,18 +306,21 @@ var PivotDelete = newObjectDeleteShortcut(pivotSpec)
// conditional format — CLI surface uses --rule-id (short), wired to the
// tool's conditional_format_id on the wire. --rule-type and --ranges are
// hoisted out of properties (both required, set on every CRUD write).
//
// Wire shape matches manage_conditional_format_object.properties — the
// enum value lives at properties.rule_type (flat string, NOT nested under
// a `rule` object), and ranges is a sibling array. Earlier CLI builds
// wrote properties.rule.type which the server silently dropped — both
// path and enum vocabulary are now aligned with the server schema (see
// sheet-skill-spec/canonical-spec/tool-schemas/mcp-tools.json line
// 3305-3324).
var condFormatEnhance = func(rt flagView, input map[string]interface{}) {
props, _ := input["properties"].(map[string]interface{})
if props == nil {
return
}
if ruleType := strings.TrimSpace(rt.Str("rule-type")); ruleType != "" {
rule, _ := props["rule"].(map[string]interface{})
if rule == nil {
rule = map[string]interface{}{}
}
rule["type"] = ruleType
props["rule"] = rule
props["rule_type"] = ruleType
}
if rt.Str("ranges") != "" {
if arr, err := requireJSONArray(rt, "ranges"); err == nil {
@@ -648,6 +651,9 @@ func filterUpdateInput(runtime flagView, token, sheetID, sheetName string) (map[
if err := requireSheetSelector(sheetID, sheetName); err != nil {
return nil, err
}
if sheetID == "" {
return nil, common.FlagErrorf("+filter-update requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)")
}
if strings.TrimSpace(runtime.Str("range")) == "" {
return nil, common.FlagErrorf("--range is required")
}
@@ -660,6 +666,7 @@ func filterUpdateInput(runtime flagView, token, sheetID, sheetName string) (map[
input := map[string]interface{}{
"excel_id": token,
"operation": "update",
"filter_id": sheetID, // server contract: filter_id === sheet_id for sheet-scoped filters
"properties": props,
}
sheetSelectorForToolInput(input, sheetID, sheetName)
@@ -706,12 +713,23 @@ var FilterDelete = common.Shortcut{
}
// filterDeleteInput mirrors the standalone +filter-delete body for batch
// sub-op reuse. filter_id is implicit (sheet-scoped), so no extra id flag.
// sub-op reuse. Server contract: filter_id === sheet_id, and update/delete
// must populate filter_id (per manage_filter_object schema). The CLI has no
// separate --filter-id flag because the value is fully derived from sheet_id;
// only --sheet-id is accepted (not --sheet-name, since there's no mid-call
// network lookup to resolve it).
func filterDeleteInput(runtime flagView, token, sheetID, sheetName string) (map[string]interface{}, error) {
if err := requireSheetSelector(sheetID, sheetName); err != nil {
return nil, err
}
input := map[string]interface{}{"excel_id": token, "operation": "delete"}
if sheetID == "" {
return nil, common.FlagErrorf("+filter-delete requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)")
}
input := map[string]interface{}{
"excel_id": token,
"operation": "delete",
"filter_id": sheetID, // server contract: filter_id === sheet_id
}
sheetSelectorForToolInput(input, sheetID, sheetName)
return input, nil
}

45
shortcuts/sheets/lark_sheet_object_crud_test.go
View File

@@ -89,15 +89,19 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
"pivot_table_id": "ptA",
},
},
// cond-format — --rule-id rename + --rule-type / --ranges hoist
// cond-format — --rule-id rename + --rule-type / --ranges hoist.
// rule_type lives at properties.rule_type (flat string), not nested
// under a `rule` object; enum vocabulary matches server schema
// (cellIs / duplicateValues / ... — see mcp-tools.json
// manage_conditional_format_object.properties.rule_type).
{
name: "+cond-format-update id rename + rule-type/ranges",
sc: CondFormatUpdate,
args: []string{
"--url", testURL, "--sheet-id", testSheetID,
"--rule-id", "ruleA",
"--properties", `{"rule":{"operator":"greater_than","value":100}}`,
"--rule-type", "cellValue",
"--properties", `{"attrs":[{"operator":"greaterThan","value":"100"}],"style":{"back_color":"#FFD7D7"}}`,
"--rule-type", "cellIs",
"--ranges", `["A1:A100"]`,
},
toolName: "manage_conditional_format_object",
@@ -107,8 +111,10 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
"operation": "update",
"conditional_format_id": "ruleA",
"properties": map[string]interface{}{
"rule": map[string]interface{}{"operator": "greater_than", "value": float64(100), "type": "cellValue"},
"ranges": []interface{}{"A1:A100"},
"rule_type": "cellIs",
"attrs": []interface{}{map[string]interface{}{"operator": "greaterThan", "value": "100"}},
"style": map[string]interface{}{"back_color": "#FFD7D7"},
"ranges": []interface{}{"A1:A100"},
},
},
},
@@ -138,16 +144,43 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
},
},
{
name: "+filter-delete (no id flag, sheet-scoped)",
// +filter-delete has no separate --filter-id flag because the
// server contract sets filter_id === sheet_id; the translator
// auto-injects filter_id from --sheet-id. update/delete fail
// hard when only --sheet-name is given (no mid-call lookup).
name: "+filter-delete (sheet-scoped, auto-injects filter_id=sheet_id)",
sc: FilterDelete,
args: []string{"--url", testURL, "--sheet-id", testSheetID},
toolName: "manage_filter_object",
wantInput: map[string]interface{}{
"excel_id": testToken,
"sheet_id": testSheetID,
"filter_id": testSheetID,
"operation": "delete",
},
},
{
// +filter-update auto-injects filter_id from sheet_id, hoists
// --range out of properties, and merges properties.rules.
name: "+filter-update auto-injects filter_id, hoists --range",
sc: FilterUpdate,
args: []string{
"--url", testURL, "--sheet-id", testSheetID,
"--range", "A1:F1000",
"--properties", `{"rules":[{"col":"B"}]}`,
},
toolName: "manage_filter_object",
wantInput: map[string]interface{}{
"excel_id": testToken,
"sheet_id": testSheetID,
"filter_id": testSheetID,
"operation": "update",
"properties": map[string]interface{}{
"range": "A1:F1000",
"rules": []interface{}{map[string]interface{}{"col": "B"}},
},
},
},
// filter-view CRUD (cli-only via callTool)
{
name: "+filter-view-create",

4
skills/lark-sheets/references/lark-sheets-conditional-format.md
View File

@@ -96,7 +96,7 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--properties` | string + File + Stdin（复合 JSON） | required | 规则配置 JSON，含 `style`（命中样式，必填）和 `attrs?`（规则参数列表，因 `rule_type` 不同结构而异）/ `has_ref?`。`rule_type` 和 `ranges` 已拎为独立 flag |
| `--rule-type` | string | required | 条件格式规则类型；优先级高于 `--properties` 中同名字段（可选值：`cellValue` / `formula` / `duplicate` / `unique` / `topBottom` / `aboveBelowAverage` / `dataBar` / `colorScale` / `iconSet` / `textContains` / `dateOccurring` / `blankCell` / `errorCell`） |
| `--rule-type` | string | required | 条件格式规则类型；优先级高于 `--properties` 中同名字段（可选值：`duplicateValues` / `uniqueValues` / `cellIs` / `containsText` / `timePeriod` / `containsBlanks` / `notContainsBlanks` / `dataBar` / `colorScale` / `rank` / `aboveAverage` / `expression` / `iconSet`） |
| `--ranges` | string + File + Stdin（简单 JSON） | required | 应用条件格式的 A1 范围 JSON 数组（如 `["A1:A100","C2:C50"]`）；优先级高于 `--properties` 中同名字段 |
### `+cond-format-update`
@@ -107,7 +107,7 @@ _公共四件套 · 系统：`--dry-run`_
| --- | --- | --- | --- |
| `--rule-id` | string | required | 目标规则 id |
| `--properties` | string + File + Stdin（复合 JSON） | required | 规则配置 JSON，结构同 `+cond-format-create` 的 `--properties`；update 是整组覆盖式 |
| `--rule-type` | string | required | 条件格式规则类型；优先级高于 `--properties` 中同名字段（可选值：`cellValue` / `formula` / `duplicate` / `unique` / `topBottom` / `aboveBelowAverage` / `dataBar` / `colorScale` / `iconSet` / `textContains` / `dateOccurring` / `blankCell` / `errorCell`） |
| `--rule-type` | string | required | 条件格式规则类型；优先级高于 `--properties` 中同名字段（可选值：`duplicateValues` / `uniqueValues` / `cellIs` / `containsText` / `timePeriod` / `containsBlanks` / `notContainsBlanks` / `dataBar` / `colorScale` / `rank` / `aboveAverage` / `expression` / `iconSet`） |
| `--ranges` | string + File + Stdin（简单 JSON） | required | 应用条件格式的 A1 范围 JSON 数组（如 `["A1:A100","C2:C50"]`）；优先级高于 `--properties` 中同名字段 |
### `+cond-format-delete`

2
skills/lark-sheets/references/lark-sheets-pivot-table.md
View File

@@ -61,7 +61,7 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--properties` | string + File + Stdin（复合 JSON） | required | JSON：`{"data_range":"Sheet1!A1:F1000","rows":[...],"columns":[...],"values":[...],"filters":[...],"show_row_grand_total":true,"show_col_grand_total":true}` |
| `--properties` | string + File + Stdin（复合 JSON） | required | JSON：{"rows":[...],"columns":[...],"values":[...],"filters":[...],"show_row_grand_total":true,"show_col_grand_total":true}（数据源走 --source，不要再放进 properties.data_range） |
| `--target-sheet-id` | string | optional | 透视表落点子表 id；省略时自动新建子表（推荐） |
| `--target-position` | string | optional | 落点起始 cell（A1 格式，如 `A1`），默认 `A1` |
| `--source` | string | required | 透视表源数据区域（A1 表示法，格式 `SheetName!StartCell:EndCell`，如 `Sheet1!A1:D100`） |

16
skills/lark-sheets/references/lark-sheets-sheet-structure.md
View File

@@ -88,8 +88,8 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--dimension` | string | required | 维度方向（行或列）（可选值：`row` / `column`） |
| `--start` | int | required | 起始位置（0-based, inclusive） |
| `--end` | int | required | 结束位置（0-based, inclusive） |
| `--start` | int | required | 起始位置（0-based） |
| `--end` | int | required | 结束位置（exclusive） |
### `+dim-unhide`
@@ -98,8 +98,8 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--dimension` | string | required | 维度方向（行或列）（可选值：`row` / `column`） |
| `--start` | int | required | 起始位置（0-based, inclusive） |
| `--end` | int | required | 结束位置（0-based, inclusive） |
| `--start` | int | required | 起始位置（0-based） |
| `--end` | int | required | 结束位置（exclusive） |
### `+dim-freeze`
@@ -117,8 +117,8 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--dimension` | string | required | 维度方向（行或列）（可选值：`row` / `column`） |
| `--start` | int | required | 起始位置（0-based, inclusive） |
| `--end` | int | required | 结束位置（0-based, inclusive） |
| `--start` | int | required | 起始位置（0-based） |
| `--end` | int | required | 结束位置（exclusive） |
| `--depth` | int | optional | 嵌套层级（`+dim-group` 用），默认 1 |
| `--group-state` | string | optional | 分组初始展开状态（可选值：`expand` / `fold`） |
@@ -129,8 +129,8 @@ _公共四件套 · 系统：`--dry-run`_
| Flag | Type | 必填 | 说明 |
| --- | --- | --- | --- |
| `--dimension` | string | required | 维度方向（行或列）（可选值：`row` / `column`） |
| `--start` | int | required | 起始位置（0-based, inclusive） |
| `--end` | int | required | 结束位置（0-based, inclusive） |
| `--start` | int | required | 起始位置（0-based） |
| `--end` | int | required | 结束位置（exclusive） |
| `--depth` | int | optional | 嵌套层级（`+dim-group` 用），默认 1 |
### `+dim-move`

4
skills/lark-sheets/references/lark-sheets-workbook.md
View File

@@ -161,6 +161,10 @@ lark-cli sheets +sheet-create --url "https://example.feishu.cn/sheets/shtXXX" \
### `+sheet-move`
standalone 路径在缺 `--source-index` / 只给 `--sheet-name` 时会自动发起一次 `+workbook-info` 读把它们解出来。
> ⚠️ **在 `+batch-update` 内调用 `+sheet-move`**：必须同时显式传 `--sheet-id` 和 `--source-index`。batch 中途无法发起结构查询，所以 batch translator 强制要求两者都显式。
### `+sheet-copy`
### `+sheet-hide` / `+sheet-unhide`