Merge pull request #1130 from zhengzhijiej-tech/fix/pivot-create-schema-alignment

fix(sheets): allow +pivot-create to omit both sheet selectors

shortcuts/sheets/data/flag-defs.json

@@ -3244,14 +3244,14 @@
         "kind": "public",
         "type": "string",
         "required": "xor",
-        "desc": "Sheet reference_id (XOR with `--sheet-name`)"
+        "desc": "Reference_id of the sub-sheet where the pivot table is located / will be created to (mutually exclusive with --sheet-name; takes priority when both given; when both omitted, a new sub-sheet is auto-created to host the pivot — recommended)"
       },
       {
         "name": "sheet-name",
         "kind": "public",
         "type": "string",
         "required": "xor",
-        "desc": "Sheet name (XOR with `--sheet-id`)"
+        "desc": "Name of the sub-sheet where the pivot table is located / will be created to (mutually exclusive with --sheet-id; when both omitted, a new sub-sheet is auto-created to host the pivot — recommended)"
       },
       {
         "name": "properties",
@@ -3264,13 +3264,6 @@
           "stdin"
         ]
       },
-      {
-        "name": "target-sheet-id",
-        "kind": "own",
-        "type": "string",
-        "required": "optional",
-        "desc": "Destination sub-sheet id for the pivot table; auto-creates a new sub-sheet when omitted (recommended)"
-      },
       {
         "name": "target-position",
         "kind": "own",

shortcuts/sheets/helpers.go

@@ -133,6 +133,33 @@ func requireSheetSelector(sheetID, sheetName string) error {
 	return nil
 }
 
+// optionalSheetSelector is the "at most one" counterpart of
+// requireSheetSelector: both empty is acceptable (the backend tool then
+// decides what to do — e.g. manage_pivot_table_object auto-creates a new
+// sub-sheet to host the pivot), and both set is rejected. Control-char
+// validation still applies whenever a value is provided.
+//
+// Used by shortcuts whose backend tool treats sheet_id/sheet_name as the
+// placement target rather than the operation context (currently only
+// +pivot-create). Other shortcuts continue to use requireSheetSelector.
+func optionalSheetSelector(sheetID, sheetName string) error {
+	sheetID = strings.TrimSpace(sheetID)
+	sheetName = strings.TrimSpace(sheetName)
+	if sheetID != "" && sheetName != "" {
+		return common.FlagErrorf("--sheet-id and --sheet-name are mutually exclusive")
+	}
+	if sheetID != "" {
+		if err := validate.RejectControlChars(sheetID, "sheet-id"); err != nil {
+			return common.FlagErrorf("%v", err)
+		}
+	} else if sheetName != "" {
+		if err := validate.RejectControlChars(sheetName, "sheet-name"); err != nil {
+			return common.FlagErrorf("%v", err)
+		}
+	}
+	return nil
+}
+
 // sheetSelectorForToolInput packs --sheet-id / --sheet-name into the tool
 // input map, omitting empty fields. Use after resolveSheetSelector returns.
 func sheetSelectorForToolInput(input map[string]interface{}, sheetID, sheetName string) {

shortcuts/sheets/lark_sheet_object_crud.go

@@ -27,10 +27,11 @@ import (
 // the surface narrow even though everything funnels through one tool).
 //
 // Five of the seven objects share the factory below (newObjectCRUDShortcuts).
-// pivot adds optional --target-sheet-id / --target-position on create,
-// declared with extraCreateFlags. filter is special-cased further down
-// (no separate id flag — filter_id is implicit per sheet — and --range is
-// a first-class create flag, not buried in --data).
+// pivot opts into allowEmptySheetSelectorOnCreate=true so the backend can
+// auto-create a placement sub-sheet when neither --sheet-id nor --sheet-name
+// is given; it also exposes optional --target-position on create. filter is
+// special-cased further down (no separate id flag — filter_id is implicit
+// per sheet — and --range is a first-class create flag, not buried in --data).
 
 // objectCRUDSpec describes a 3-shortcut create/update/delete cluster.
 // idFlag / idField empty → no per-object id flag (only filter uses that
@@ -54,6 +55,13 @@ type objectCRUDSpec struct {
 	// +sparkline-list instead of letting the caller hit an opaque
 	// server-side rejection).
 	validateUpdateInput func(input map[string]interface{}) error
+	// allowEmptySheetSelectorOnCreate, when true, makes the *create*
+	// shortcut accept empty --sheet-id / --sheet-name (backend then picks
+	// the placement target — e.g. manage_pivot_table_object auto-creates
+	// a sub-sheet to host the pivot). Both flags being set is still
+	// rejected. Update/delete continue to require an explicit selector.
+	// Today only pivotSpec opts in.
+	allowEmptySheetSelectorOnCreate bool
 }
 
 func newObjectCreateShortcut(spec objectCRUDSpec) common.Shortcut {
@@ -79,7 +87,8 @@ func newObjectCreateShortcut(spec objectCRUDSpec) common.Shortcut {
 		},
 		DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
 			token, _ := resolveSpreadsheetToken(runtime)
-			sheetID, sheetName, _ := resolveSheetSelector(runtime)
+			sheetID := strings.TrimSpace(runtime.Str("sheet-id"))
+			sheetName := strings.TrimSpace(runtime.Str("sheet-name"))
 			input, _ := objectCreateInput(runtime, token, sheetID, sheetName, spec)
 			return invokeToolDryRun(token, ToolKindWrite, spec.toolName, input)
 		},
@@ -88,10 +97,8 @@ func newObjectCreateShortcut(spec objectCRUDSpec) common.Shortcut {
 			if err != nil {
 				return err
 			}
-			sheetID, sheetName, err := resolveSheetSelector(runtime)
-			if err != nil {
-				return err
-			}
+			sheetID := strings.TrimSpace(runtime.Str("sheet-id"))
+			sheetName := strings.TrimSpace(runtime.Str("sheet-name"))
 			input, err := objectCreateInput(runtime, token, sheetID, sheetName, spec)
 			if err != nil {
 				return err
@@ -107,7 +114,13 @@ func newObjectCreateShortcut(spec objectCRUDSpec) common.Shortcut {
 }
 
 func objectCreateInput(runtime flagView, token, sheetID, sheetName string, spec objectCRUDSpec) (map[string]interface{}, error) {
-	if err := requireSheetSelector(sheetID, sheetName); err != nil {
+	var err error
+	if spec.allowEmptySheetSelectorOnCreate {
+		err = optionalSheetSelector(sheetID, sheetName)
+	} else {
+		err = requireSheetSelector(sheetID, sheetName)
+	}
+	if err != nil {
 		return nil, err
 	}
 	props, err := requireJSONObject(runtime, "properties")
@@ -288,17 +301,18 @@ var ChartCreate = newObjectCreateShortcut(chartSpec)
 var ChartUpdate = newObjectUpdateShortcut(chartSpec)
 var ChartDelete = newObjectDeleteShortcut(chartSpec)
 
-// pivot — create exposes --target-sheet-id / --target-position (top-level
-// of the tool input) plus --source / --range hoisted from properties.
+// pivot — create exposes --target-position (top-level of the tool input)
+// plus --source / --range hoisted from properties. --sheet-id / --sheet-name
+// are the placement target (where the pivot table lands); the backend
+// auto-creates a new sub-sheet when both are omitted, so create opts into
+// allowEmptySheetSelectorOnCreate.
 var pivotSpec = objectCRUDSpec{
-	commandPrefix: "+pivot",
-	toolName:      "manage_pivot_table_object",
-	idFlag:        "pivot-table-id",
-	idField:       "pivot_table_id",
+	commandPrefix:                   "+pivot",
+	toolName:                        "manage_pivot_table_object",
+	idFlag:                          "pivot-table-id",
+	idField:                         "pivot_table_id",
+	allowEmptySheetSelectorOnCreate: true,
 	enhanceCreateInput: func(rt flagView, input map[string]interface{}) {
-		if v := strings.TrimSpace(rt.Str("target-sheet-id")); v != "" {
-			input["target_sheet_id"] = v
-		}
 		if v := strings.TrimSpace(rt.Str("target-position")); v != "" && v != "A1" {
 			input["target_position"] = v
 		}

shortcuts/sheets/lark_sheet_object_crud_test.go

@@ -51,16 +51,20 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
 				"properties": map[string]interface{}{"type": "bar"},
 			},
 		},
-		// pivot — has extra create flags incl. required --source
+		// pivot — has extra create flags incl. required --source.
+		// --sheet-id is the placement target (where the pivot lands);
+		// pivotSpec.allowEmptySheetSelectorOnCreate lets both --sheet-id
+		// and --sheet-name be omitted so the backend auto-creates a
+		// sub-sheet — covered separately in the +pivot-create empty-
+		// selector / mutex tests below.
 		{
-			name: "+pivot-create with target / source / range flags",
+			name: "+pivot-create with placement / source / range flags",
 			sc:   PivotCreate,
 			args: []string{
 				"--url", testURL, "--sheet-id", testSheetID,
 				"--properties", `{"rows":[{"field":"A"}]}`,
 				"--source", "Sheet1!A1:F1000",
 				"--range", "F1",
-				"--target-sheet-id", "sh2",
 				"--target-position", "B5",
 			},
 			toolName: "manage_pivot_table_object",
@@ -68,7 +72,6 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
 				"excel_id":        testToken,
 				"sheet_id":        testSheetID,
 				"operation":       "create",
-				"target_sheet_id": "sh2",
 				"target_position": "B5",
 				"properties": map[string]interface{}{
 					"rows":   []interface{}{map[string]interface{}{"field": "A"}},
@@ -77,6 +80,26 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
 				},
 			},
 		},
+		// +pivot-create accepts both sheet selectors empty — backend
+		// auto-creates a placement sub-sheet.
+		{
+			name: "+pivot-create empty --sheet-id / --sheet-name omits sheet from input",
+			sc:   PivotCreate,
+			args: []string{
+				"--url", testURL,
+				"--properties", `{"rows":[{"field":"A"}]}`,
+				"--source", "Sheet1!A1:F1000",
+			},
+			toolName: "manage_pivot_table_object",
+			wantInput: map[string]interface{}{
+				"excel_id":  testToken,
+				"operation": "create",
+				"properties": map[string]interface{}{
+					"rows":   []interface{}{map[string]interface{}{"field": "A"}},
+					"source": "Sheet1!A1:F1000",
+				},
+			},
+		},
 		{
 			name:     "+pivot-delete",
 			sc:       PivotDelete,
@@ -325,6 +348,98 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) {
 	}
 }
 
+// TestPivotCreate_SheetSelectorSemantics locks in the "at most one"
+// semantics for +pivot-create (and only +pivot-create): both --sheet-id
+// and --sheet-name may be omitted (backend auto-creates a placement
+// sub-sheet), but passing both is rejected.
+//
+// Companion regression — TestObjectCreate_RequiresSheetSelector below —
+// confirms every other *-create still rejects empty selector.
+func TestPivotCreate_SheetSelectorSemantics(t *testing.T) {
+	t.Parallel()
+
+	t.Run("both empty is accepted", func(t *testing.T) {
+		t.Parallel()
+		body := parseDryRunBody(t, PivotCreate, []string{
+			"--url", testURL,
+			"--properties", `{"rows":[{"field":"A"}]}`,
+			"--source", "Sheet1!A1:F1000",
+		})
+		input := decodeToolInput(t, body, "manage_pivot_table_object")
+		if _, ok := input["sheet_id"]; ok {
+			t.Errorf("expected no sheet_id in input; got %v", input["sheet_id"])
+		}
+		if _, ok := input["sheet_name"]; ok {
+			t.Errorf("expected no sheet_name in input; got %v", input["sheet_name"])
+		}
+	})
+
+	t.Run("both set is rejected", func(t *testing.T) {
+		t.Parallel()
+		_, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{
+			"--url", testURL,
+			"--sheet-id", testSheetID,
+			"--sheet-name", "Sheet1",
+			"--properties", `{"rows":[{"field":"A"}]}`,
+			"--source", "Sheet1!A1:F1000",
+		})
+		if err == nil {
+			t.Fatalf("expected CLI to reject both --sheet-id and --sheet-name set; stderr=%s", stderr)
+		}
+		combined := stderr + err.Error()
+		if !strings.Contains(combined, "mutually exclusive") {
+			t.Errorf("expected error to say 'mutually exclusive'; got=%s|%v", stderr, err)
+		}
+	})
+
+	t.Run("only sheet-id is accepted", func(t *testing.T) {
+		t.Parallel()
+		body := parseDryRunBody(t, PivotCreate, []string{
+			"--url", testURL,
+			"--sheet-id", testSheetID,
+			"--properties", `{"rows":[{"field":"A"}]}`,
+			"--source", "Sheet1!A1:F1000",
+		})
+		input := decodeToolInput(t, body, "manage_pivot_table_object")
+		if got, _ := input["sheet_id"].(string); got != testSheetID {
+			t.Errorf("sheet_id = %q, want %q", got, testSheetID)
+		}
+	})
+}
+
+// TestObjectCreate_RequiresSheetSelector regresses the non-pivot create
+// shortcuts: pivot-create is the only one whose spec sets
+// allowEmptySheetSelectorOnCreate=true. Every other *-create must still
+// reject empty --sheet-id / --sheet-name (this is the guardrail that
+// keeps the change minimally scoped).
+func TestObjectCreate_RequiresSheetSelector(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name string
+		sc   common.Shortcut
+		args []string // omit sheet selector flags on purpose
+	}{
+		{"chart", ChartCreate, []string{"--url", testURL, "--properties", `{"type":"line"}`}},
+		{"cond-format", CondFormatCreate, []string{"--url", testURL, "--properties", `{"attrs":[]}`, "--rule-type", "cellIs", "--ranges", `["A1:A10"]`}},
+		{"sparkline", SparklineCreate, []string{"--url", testURL, "--properties", `{"sparklines":[]}`}},
+		{"filter-view", FilterViewCreate, []string{"--url", testURL, "--properties", `{}`, "--range", "A1:F10"}},
+	}
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			_, stderr, err := runShortcutCapturingErr(t, tt.sc, tt.args)
+			if err == nil {
+				t.Fatalf("expected CLI to reject empty sheet selector for +%s-create; stderr=%s", tt.name, stderr)
+			}
+			combined := stderr + err.Error()
+			if !strings.Contains(combined, "specify at least one of --sheet-id or --sheet-name") {
+				t.Errorf("expected 'specify at least one of --sheet-id or --sheet-name'; got=%s|%v", stderr, err)
+			}
+		})
+	}
+}
+
 // TestSparklineUpdate_MissingSparklineID confirms the standalone-path
 // pre-check fires: +sparkline-update with properties.sparklines[] but no
 // per-item sparkline_id must fail CLI-side with a pointer to

skills/lark-sheets/references/lark-sheets-pivot-table.md

@@ -62,7 +62,6 @@ _公共四件套 · 系统：`--dry-run`_
 | Flag | Type | 必填 | 说明 |
 | --- | --- | --- | --- |
 | `--properties` | string + File + Stdin（复合 JSON） | required | JSON：{"rows":[...],"columns":[...],"values":[...],"filters":[...],"show_row_grand_total":true,"show_col_grand_total":true}（数据源走 --source，不要再放进 properties.source） |
-| `--target-sheet-id` | string | optional | 透视表落点子表 id；省略时自动新建子表（推荐） |
 | `--target-position` | string | optional | 透视表落点子表内的起始 cell（A1 格式，如 `A1`），与 `--target-sheet-id` 配套、映射到顶层 `target_position`，默认 `A1`（值为 A1 时不下发）。它与 `--range` 都表达落点但落在不同 wire 字段，避免两者同时给冲突值 |
 | `--source` | string | required | 透视表源数据区域（A1 表示法，格式 `SheetName!StartCell:EndCell`，如 `Sheet1!A1:D100`） |
 | `--range` | string | optional | 透视表左上角放置位置（A1 单值，如 `F1`，仅 create 生效），映射到 `properties.range`；省略时放在落点子表（默认新建子表）的左上角。它与 `--target-position` 都表达落点但落在不同 wire 字段，避免两者同时给冲突值 |