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.
This commit is contained in:
zhengzhijie
2026-05-22 11:37:35 +08:00
parent 4be06c85f6
commit 556b2292c7
14 changed files with 113 additions and 87 deletions

View File

@@ -132,8 +132,8 @@ func TestBatchOp_BodyMatchesStandalone(t *testing.T) {
{
shortcut: "+range-sort",
sc: RangeSort,
args: []string{"--sheet-id", "sh1", "--range", "A1:D10", "--sort-keys", `[{"col":"B","order":"asc"}]`, "--has-header"},
subInput: `{"sheet-id":"sh1","range":"A1:D10","sort-keys":[{"col":"B","order":"asc"}],"has-header":true}`,
args: []string{"--sheet-id", "sh1", "--range", "A1:D10", "--sort-keys", `[{"column":"B","ascending":true}]`, "--has-header"},
subInput: `{"sheet-id":"sh1","range":"A1:D10","sort-keys":[{"column":"B","ascending":true}],"has-header":true}`,
},
{
shortcut: "+sheet-create",

View File

@@ -1874,17 +1874,6 @@
"stdin"
]
},
{
"name": "colors",
"kind": "own",
"type": "string",
"required": "optional",
"desc": "RGB hex color array (e.g. `[\"#1FB6C1\",\"#F006C2\"]`); length must equal `--options`",
"input": [
"file",
"stdin"
]
},
{
"name": "multiple",
"kind": "own",
@@ -1892,13 +1881,6 @@
"required": "optional",
"desc": "Enable multi-select; default `false`"
},
{
"name": "highlight",
"kind": "own",
"type": "bool",
"required": "optional",
"desc": "Color-highlight options; default `false`"
},
{
"name": "dry-run",
"kind": "system",
@@ -2802,17 +2784,6 @@
"stdin"
]
},
{
"name": "colors",
"kind": "own",
"type": "string",
"required": "optional",
"desc": "Color array (same length as `--options`)",
"input": [
"file",
"stdin"
]
},
{
"name": "multiple",
"kind": "own",
@@ -2820,13 +2791,6 @@
"required": "optional",
"desc": "Enable multi-select"
},
{
"name": "highlight",
"kind": "own",
"type": "bool",
"required": "optional",
"desc": "Color-highlight options"
},
{
"name": "dry-run",
"kind": "system",

View File

@@ -156,7 +156,7 @@ func sheetSelectorPlaceholder(sheetID, sheetName string) string {
// parseJSONFlag parses a JSON string from a flag value. Returns nil when the
// flag is empty (caller decides if that's acceptable). Used by --data /
// --style / --options / --ranges / --colors and friends.
// --style / --options / --ranges / --properties and friends.
func parseJSONFlag(runtime flagView, name string) (interface{}, error) {
raw := strings.TrimSpace(runtime.Str(name))
if raw == "" {

View File

@@ -332,7 +332,7 @@ var DropdownUpdate = common.Shortcut{
if _, err := validateDropdownRanges(runtime); err != nil {
return err
}
if _, err := validateDropdownOptionsColors(runtime); err != nil {
if _, err := validateDropdownOptions(runtime); err != nil {
return err
}
return nil

View File

@@ -215,8 +215,12 @@ func TestDropdownUpdate_BatchPayload(t *testing.T) {
if dv == nil || dv["type"] != "list" {
t.Errorf("op[%d] missing data_validation list: %#v", i, cell)
}
if dv["multiple_values"] != true {
t.Errorf("op[%d] multiple_values = %v, want true", i, dv["multiple_values"])
items, _ := dv["items"].([]interface{})
if len(items) != 3 {
t.Errorf("op[%d] data_validation.items length = %d, want 3", i, len(items))
}
if dv["support_multiple_values"] != true {
t.Errorf("op[%d] support_multiple_values = %v, want true", i, dv["support_multiple_values"])
}
}
}

View File

@@ -598,6 +598,23 @@ func rangeSortInput(runtime flagView, token, sheetID, sheetName string) (map[str
if err != nil {
return nil, err
}
// transform_range.sort_conditions[i] requires both `column` (string)
// and `ascending` (bool); the server's own validation surfaces a
// terse "required property X is missing" with no per-item context.
// Pre-check here so the user sees which entry is malformed.
for i, raw := range keys {
item, ok := raw.(map[string]interface{})
if !ok {
return nil, common.FlagErrorf("--sort-keys[%d] must be an object {column, ascending}; got %T", i, raw)
}
col, _ := item["column"].(string)
if strings.TrimSpace(col) == "" {
return nil, common.FlagErrorf("--sort-keys[%d] missing required string field `column` (the column letter to sort by, e.g. \"C\")", i)
}
if _, ok := item["ascending"].(bool); !ok {
return nil, common.FlagErrorf("--sort-keys[%d] missing required bool field `ascending`", i)
}
}
input := map[string]interface{}{
"excel_id": token,
"operation": "sort",

View File

@@ -185,7 +185,7 @@ func TestRangeOperationsShortcuts_DryRun(t *testing.T) {
{
name: "+range-sort multi-key with header",
sc: RangeSort,
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--range", "A1:E100", "--has-header", "--sort-keys", `[{"col":"B","order":"asc"},{"col":"D","order":"desc"}]`},
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--range", "A1:E100", "--has-header", "--sort-keys", `[{"column":"B","ascending":true},{"column":"D","ascending":false}]`},
toolName: "transform_range",
wantInput: map[string]interface{}{
"excel_id": testToken,
@@ -194,8 +194,8 @@ func TestRangeOperationsShortcuts_DryRun(t *testing.T) {
"range": "A1:E100",
"has_header": true,
"sort_conditions": []interface{}{
map[string]interface{}{"col": "B", "order": "asc"},
map[string]interface{}{"col": "D", "order": "desc"},
map[string]interface{}{"column": "B", "ascending": true},
map[string]interface{}{"column": "D", "ascending": false},
},
},
},
@@ -211,6 +211,41 @@ func TestRangeOperationsShortcuts_DryRun(t *testing.T) {
}
}
// TestRangeSort_RejectsMalformedKeys verifies the pre-check that each
// --sort-keys entry has both `column` (string) and `ascending` (bool);
// previously the CLI passed any JSON through and the server bounced
// with a terse "required property X missing" that didn't name the bad
// entry.
func TestRangeSort_RejectsMalformedKeys(t *testing.T) {
t.Parallel()
cases := []struct {
name string
keys string
want string
}{
{"missing column", `[{"ascending":true}]`, "missing required string field `column`"},
{"missing ascending", `[{"column":"B"}]`, "missing required bool field `ascending`"},
{"old vocab col/order", `[{"col":"B","order":"asc"}]`, "missing required string field `column`"},
{"non-object item", `["B"]`, "must be an object"},
}
for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
stdout, stderr, err := runShortcutCapturingErr(t, RangeSort, []string{
"--url", testURL, "--sheet-id", testSheetID,
"--range", "A1:E10", "--sort-keys", c.keys, "--dry-run",
})
if err == nil {
t.Fatalf("expected validation error; stdout=%s stderr=%s", stdout, stderr)
}
if !strings.Contains(stdout+stderr+err.Error(), c.want) {
t.Errorf("want substring %q in error; got stdout=%s stderr=%s err=%v", c.want, stdout, stderr, err)
}
})
}
}
func TestResize_TypeAndSizeGuards(t *testing.T) {
t.Parallel()
cases := []struct {

View File

@@ -215,8 +215,12 @@ func stripRowPrefixFromCsvOutput(out interface{}) interface{} {
}
// DropdownGet wraps get_cell_ranges scoped to data_validation: read the
// dropdown configuration on a range. The range carries its own sheet prefix
// (e.g. "sheet1!A2:A100"), so no separate --sheet-id / --sheet-name is needed.
// dropdown configuration on a range. The CLI accepts the range in the
// sheet-prefixed form (e.g. "sheet1!A2:A100") for convenience; the
// prefix is split client-side into sheet_name + bare A1 because the
// get_cell_ranges tool wants sheet selector and ranges as separate
// fields (ranges with the "sheet!" prefix gets the empty-sheet_id
// rejection from the server).
var DropdownGet = common.Shortcut{
Service: "sheets",
Command: "+dropdown-get",
@@ -257,10 +261,15 @@ var DropdownGet = common.Shortcut{
}
func dropdownGetInput(runtime *common.RuntimeContext, token string) map[string]interface{} {
return map[string]interface{}{
// Validate already enforced the "Sheet!range" prefix, so the
// split error path can't be reached here in practice.
sheetName, bareRange, _ := splitSheetPrefixedRange(strings.TrimSpace(runtime.Str("range")))
input := map[string]interface{}{
"excel_id": token,
"ranges": []string{strings.TrimSpace(runtime.Str("range"))},
"ranges": []string{bareRange},
"include_styles": false,
"value_render_option": "formatted_value",
}
sheetSelectorForToolInput(input, "", sheetName)
return input
}

View File

@@ -54,7 +54,8 @@ func TestReadDataShortcuts_DryRun(t *testing.T) {
toolName: "get_cell_ranges",
wantInput: map[string]interface{}{
"excel_id": testToken,
"ranges": []interface{}{"sheet1!A2:A100"},
"sheet_name": "sheet1",
"ranges": []interface{}{"A2:A100"},
"include_styles": false,
"value_render_option": "formatted_value",
},

View File

@@ -317,6 +317,15 @@ func TestWorkbookCreate_DryRun(t *testing.T) {
if input["range"] != "A1:B3" {
t.Errorf("fill range = %v, want A1:B3 (1 header + 2 data rows × 2 cols)", input["range"])
}
// New workbook → fill targets the default sheet by name (no
// extra get_workbook_structure call is needed to learn the
// auto-generated sheet_id).
if input["sheet_name"] != "Sheet1" {
t.Errorf("fill sheet_name = %v, want Sheet1", input["sheet_name"])
}
if _, hasID := input["sheet_id"]; hasID {
t.Errorf("fill sheet_id should be omitted (server rejects empty); got %v", input["sheet_id"])
}
})
}

View File

@@ -331,52 +331,36 @@ func dropdownSetInput(runtime flagView, token, sheetID, sheetName string) (map[s
// ─── shared dropdown helpers ──────────────────────────────────────────
// buildDropdownValidation packs --options / --colors / --multiple / --highlight
// into the data_validation block expected by set_cell_range.
// buildDropdownValidation packs --options / --multiple into the
// data_validation block expected by set_cell_range. Field names match
// the canonical schema: items (not values) for the option list, and
// support_multiple_values (not multiple_values) for multi-select.
// Earlier CLI builds also emitted `colors` and `highlight_options`,
// neither of which exists in the server schema for set_cell_range; both
// were rejected as "unexpected property". The --colors / --highlight
// flags have been removed; only --options + --multiple remain.
func buildDropdownValidation(runtime flagView) (map[string]interface{}, error) {
options, err := requireJSONArray(runtime, "options")
if err != nil {
return nil, err
}
dv := map[string]interface{}{
"type": "list",
"values": options,
}
if runtime.Str("colors") != "" {
colors, err := requireJSONArray(runtime, "colors")
if err != nil {
return nil, err
}
if len(colors) != len(options) {
return nil, common.FlagErrorf("--colors length (%d) must equal --options length (%d)", len(colors), len(options))
}
dv["colors"] = colors
"type": "list",
"items": options,
}
if runtime.Bool("multiple") {
dv["multiple_values"] = true
}
if runtime.Bool("highlight") {
dv["highlight_options"] = true
dv["support_multiple_values"] = true
}
return dv, nil
}
// validateDropdownOptionsColors validates --options is a JSON array and that
// --colors (when set) has matching length. Used by +dropdown-update Validate.
func validateDropdownOptionsColors(runtime flagView) (int, error) {
// validateDropdownOptions checks that --options parses as a JSON array
// and returns its length so callers can size their cells matrix.
func validateDropdownOptions(runtime flagView) (int, error) {
options, err := requireJSONArray(runtime, "options")
if err != nil {
return 0, err
}
if runtime.Str("colors") != "" {
colors, err := requireJSONArray(runtime, "colors")
if err != nil {
return 0, err
}
if len(colors) != len(options) {
return 0, common.FlagErrorf("--colors length (%d) must equal --options length (%d)", len(colors), len(options))
}
}
return len(options), nil
}

View File

@@ -95,7 +95,7 @@ func TestWriteCellsShortcuts_DryRun(t *testing.T) {
"--url", testURL, "--sheet-id", testSheetID,
"--range", "A2:A4",
"--options", `["a","b"]`,
"--multiple", "--highlight",
"--multiple",
},
toolName: "set_cell_range",
wantInput: map[string]interface{}{
@@ -143,8 +143,15 @@ func TestDropdownSet_CellsShape(t *testing.T) {
if dv["type"] != "list" {
t.Errorf("row %d data_validation.type = %v, want list", i, dv["type"])
}
if dv["multiple_values"] != true {
t.Errorf("row %d data_validation.multiple_values = %v, want true", i, dv["multiple_values"])
items, _ := dv["items"].([]interface{})
if len(items) != 2 || items[0] != "a" || items[1] != "b" {
t.Errorf("row %d data_validation.items = %#v, want [\"a\",\"b\"]", i, dv["items"])
}
if dv["support_multiple_values"] != true {
t.Errorf("row %d data_validation.support_multiple_values = %v, want true", i, dv["support_multiple_values"])
}
if _, hasLegacy := dv["multiple_values"]; hasLegacy {
t.Errorf("row %d data_validation should not emit legacy `multiple_values`", i)
}
}
}

View File

@@ -70,9 +70,7 @@ _公共URL/token无 sheet 定位) · 系统:`--dry-run`_
| --- | --- | --- | --- |
| `--ranges` | string + File + Stdin简单 JSON | required | 目标范围 JSON 数组(如 `["sheet1!A2:A100"]`),每项必须带 sheet 前缀 |
| `--options` | string + File + Stdin复合 JSON | required | 选项 JSON 数组(如 `["opt1","opt2"]` |
| `--colors` | string + File + Stdin简单 JSON | optional | 颜色数组(与 `--options` 等长) |
| `--multiple` | bool | optional | 启用多选 |
| `--highlight` | bool | optional | 选项配色 |
### `+dropdown-delete`

View File

@@ -199,9 +199,7 @@ _公共四件套 · 系统:`--dry-run`_
| --- | --- | --- | --- |
| `--range` | string | required | 目标范围A1 格式,如 `A2:A100` |
| `--options` | string + File + Stdin复合 JSON | required | 选项 JSON 数组 `["opt1","opt2"]`;最多 500 项,每项 ≤100 字符,不含逗号 |
| `--colors` | string + File + Stdin简单 JSON | optional | RGB hex 颜色数组(如 `["#1FB6C1","#F006C2"]`),长度必须与 `--options` 一致 |
| `--multiple` | bool | optional | 启用多选;默认 `false` |
| `--highlight` | bool | optional | 选项配色显示;默认 `false` |
### `+csv-put`