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.
This commit is contained in:
xiongyuanwen-byted
2026-05-25 14:15:48 +08:00
parent 868beaf004
commit 4be06c85f6
4 changed files with 124 additions and 49 deletions

View File

@@ -271,8 +271,8 @@ func TestExecute_BatchUpdate_Translated(t *testing.T) {
}
}
// TestExecute_WorkbookCreate covers the legacy POST + optional
// set_cell_range follow-up. Stubs both endpoints.
// TestExecute_WorkbookCreate covers the create POST + first-sheet lookup +
// set_cell_range follow-up. Stubs all three endpoints.
func TestExecute_WorkbookCreate(t *testing.T) {
t.Parallel()
create := &httpmock.Stub{
@@ -289,12 +289,15 @@ func TestExecute_WorkbookCreate(t *testing.T) {
},
},
}
// Initial fill first reads the workbook structure to resolve the default
// sheet's id (the create response doesn't echo it), then writes.
structure := toolOutputStub("shtcnBRAND", "read", `{"sheets":[{"sheet_id":"shtFirst","sheet_name":"Sheet1","index":0}]}`)
fill := toolOutputStub("shtcnBRAND", "write", `{"updated_cells":4}`)
out, err := runShortcutWithStubs(t, WorkbookCreate, []string{
"--title", "Sales",
"--headers", `["Name","Score"]`,
"--values", `[["alice",95]]`,
}, create, fill)
}, create, structure, fill)
if err != nil {
t.Fatalf("execute failed: %v\nout=%s", err, out)
}
@@ -306,15 +309,21 @@ func TestExecute_WorkbookCreate(t *testing.T) {
if data["initial_fill"] == nil {
t.Errorf("initial_fill missing in envelope")
}
// The fill must target the resolved first sheet, not an empty selector.
fillInput := decodeToolInput(t, decodeRawEnvelopeBody(t, fill.CapturedBody), "set_cell_range")
if fillInput["sheet_id"] != "shtFirst" {
t.Errorf("fill sheet_id = %v, want shtFirst (resolved from workbook structure)", fillInput["sheet_id"])
}
}
// TestExecute_DimMove covers the legacy v2 dimension_range call with
// CLI inclusive → API exclusive end-index conversion.
// TestExecute_DimMove covers the native v3 move_dimension call. CLI's
// 0-based inclusive --start/--end pass straight through to v3's
// source.{start_index,end_index} (also 0-based inclusive).
func TestExecute_DimMove(t *testing.T) {
t.Parallel()
move := &httpmock.Stub{
Method: "POST",
URL: "/open-apis/sheets/v2/spreadsheets/" + testToken + "/dimension_range",
URL: "/open-apis/sheets/v3/spreadsheets/" + testToken + "/sheets/" + testSheetID + "/move_dimension",
Body: map[string]interface{}{
"code": 0,
"msg": "success",
@@ -330,8 +339,11 @@ func TestExecute_DimMove(t *testing.T) {
}
body := decodeRawEnvelopeBody(t, move.CapturedBody)
src, _ := body["source"].(map[string]interface{})
if src["startIndex"].(float64) != 0 || src["endIndex"].(float64) != 3 {
t.Errorf("indices = (%v,%v), want (0,3)", src["startIndex"], src["endIndex"])
if src["start_index"].(float64) != 0 || src["end_index"].(float64) != 2 {
t.Errorf("indices = (%v,%v), want (0,2) — 0-based inclusive", src["start_index"], src["end_index"])
}
if body["destination_index"].(float64) != 10 {
t.Errorf("destination_index = %v, want 10", body["destination_index"])
}
}

View File

@@ -531,12 +531,17 @@ func columnIndexToLetter(idx int) string {
return string(out)
}
// ─── +dim-move (legacy OAPI, cli_status: cli-only) ───────────────────
// ─── +dim-move (native v3 move_dimension, cli_status: cli-only) ──────
//
// Moves a contiguous block of rows or columns to a new index in the same
// sheet via the legacy v2 endpoint (not the One-OpenAPI dispatcher).
// CLI's --start / --end are 0-based inclusive; the endpoint expects
// half-open [startIndex, endIndex).
// sheet via the native v3 move_dimension endpoint (not the One-OpenAPI
// dispatcher). CLI's --start / --end are 0-based inclusive; v3
// move_dimension's source.{start_index,end_index} are likewise 0-based
// inclusive, so they pass straight through. The earlier build POSTed a
// {source,destinationIndex} body to the v2 dimension_range endpoint, which
// is the add/update/delete surface and expects a `dimension` object —
// hence the server rejected it with "[9499] Missing required parameter:
// Dimension".
var DimMove = common.Shortcut{
Service: "sheets",
@@ -568,10 +573,9 @@ var DimMove = common.Shortcut{
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
token, _ := resolveSpreadsheetToken(runtime)
sheetID, sheetName, _ := resolveSheetSelector(runtime)
body := dimMoveBody(runtime, sheetSelectorPlaceholder(sheetID, sheetName))
return common.NewDryRunAPI().
POST(fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", token)).
Body(body).
POST(dimMovePath(token, sheetSelectorPlaceholder(sheetID, sheetName))).
Body(dimMoveBody(runtime)).
Set("spreadsheet_token", token)
},
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
@@ -583,8 +587,9 @@ var DimMove = common.Shortcut{
if err != nil {
return err
}
// Legacy v2 endpoint needs sheet_id. Resolve sheet_name client-side
// when needed (reuses lookupSheetIndex which fetches workbook structure).
// v3 move_dimension carries sheet_id in the path. Resolve
// sheet_name client-side when needed (reuses lookupSheetIndex
// which fetches workbook structure).
if sheetID == "" {
lookedID, _, err := lookupSheetIndex(ctx, runtime, token, "", sheetName)
if err != nil {
@@ -592,12 +597,7 @@ var DimMove = common.Shortcut{
}
sheetID = lookedID
}
body := dimMoveBody(runtime, sheetID)
data, err := runtime.CallAPI(
"POST",
fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)),
nil, body,
)
data, err := runtime.CallAPI("POST", dimMovePath(token, sheetID), nil, dimMoveBody(runtime))
if err != nil {
return err
}
@@ -606,18 +606,24 @@ var DimMove = common.Shortcut{
},
}
func dimMoveBody(runtime *common.RuntimeContext, sheetID string) map[string]interface{} {
// dimMovePath builds the native v3 move_dimension endpoint. sheet_id lives in
// the path (unlike the v2 dimension_range body that the earlier build used).
func dimMovePath(token, sheetID string) string {
return fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/move_dimension",
validate.EncodePathSegment(token), validate.EncodePathSegment(sheetID))
}
func dimMoveBody(runtime *common.RuntimeContext) map[string]interface{} {
dim := "ROWS"
if runtime.Str("dimension") == "column" {
dim = "COLUMNS"
}
return map[string]interface{}{
"source": map[string]interface{}{
"sheetId": sheetID,
"majorDimension": dim,
"startIndex": runtime.Int("start"),
"endIndex": runtime.Int("end") + 1, // CLI inclusive → API exclusive
"major_dimension": dim,
"start_index": runtime.Int("start"),
"end_index": runtime.Int("end"), // both CLI --end and v3 end_index are 0-based inclusive
},
"destinationIndex": runtime.Int("target"),
"destination_index": runtime.Int("target"),
}
}

View File

@@ -169,9 +169,10 @@ func TestDimRange_StartEndValidation(t *testing.T) {
}
}
// TestDimMove_DryRun verifies the legacy v2 dimension_range payload
// shape. CLI's 0-based inclusive (--start / --end) becomes the v2
// endpoint's half-open [startIndex, endIndex).
// TestDimMove_DryRun verifies the native v3 move_dimension payload shape.
// CLI's 0-based inclusive (--start / --end) maps straight through to v3's
// source.{start_index,end_index} (also 0-based inclusive), and sheet_id is
// carried in the path, not the body.
func TestDimMove_DryRun(t *testing.T) {
t.Parallel()
calls := parseDryRunAPI(t, DimMove, []string{
@@ -182,25 +183,23 @@ func TestDimMove_DryRun(t *testing.T) {
t.Fatalf("api calls = %d, want 1", len(calls))
}
c := calls[0].(map[string]interface{})
if !strings.Contains(c["url"].(string), "/sheets/v2/spreadsheets/") {
t.Errorf("url = %v, want sheets/v2 path", c["url"])
wantURL := "/sheets/v3/spreadsheets/" + testToken + "/sheets/" + testSheetID + "/move_dimension"
if !strings.Contains(c["url"].(string), wantURL) {
t.Errorf("url = %v, want suffix %v", c["url"], wantURL)
}
body, _ := c["body"].(map[string]interface{})
src, _ := body["source"].(map[string]interface{})
if src["sheetId"] != testSheetID {
t.Errorf("source.sheetId = %v", src["sheetId"])
if src["major_dimension"] != "ROWS" {
t.Errorf("source.major_dimension = %v, want ROWS", src["major_dimension"])
}
if src["majorDimension"] != "ROWS" {
t.Errorf("source.majorDimension = %v, want ROWS", src["majorDimension"])
if src["start_index"].(float64) != 0 {
t.Errorf("start_index = %v, want 0", src["start_index"])
}
if src["startIndex"].(float64) != 0 {
t.Errorf("startIndex = %v, want 0", src["startIndex"])
if src["end_index"].(float64) != 2 {
t.Errorf("end_index = %v, want 2 (0-based inclusive, passes straight through)", src["end_index"])
}
if src["endIndex"].(float64) != 3 {
t.Errorf("endIndex = %v, want 3 (CLI end+1 for half-open)", src["endIndex"])
}
if body["destinationIndex"].(float64) != 10 {
t.Errorf("destinationIndex = %v, want 10", body["destinationIndex"])
if body["destination_index"].(float64) != 10 {
t.Errorf("destination_index = %v, want 10", body["destination_index"])
}
}

View File

@@ -600,9 +600,11 @@ var WorkbookCreate = common.Shortcut{
Body(body)
if runtime.Str("headers") != "" || runtime.Str("values") != "" {
fill, _ := buildInitialFillInput(runtime)
fill["excel_id"] = "<new-token>"
fill["sheet_id"] = "<first-sheet-id>" // resolved from the workbook at execute time
wireBody, _ := buildToolBody("set_cell_range", fill)
dry.POST("/open-apis/sheet_ai/v2/spreadsheets/<new-token>/tools/invoke_write").
Desc("fill headers + data via set_cell_range").
Desc("fill headers + data via set_cell_range (sheet_id resolved after create)").
Body(wireBody)
}
return dry
@@ -633,6 +635,13 @@ var WorkbookCreate = common.Shortcut{
return err
}
fill["excel_id"] = token
// set_cell_range needs a concrete sheet selector; the create
// response doesn't echo the default sheet's id, so read it back.
firstSheetID, err := lookupFirstSheetID(ctx, runtime, token)
if err != nil {
return fmt.Errorf("spreadsheet %s created but resolving its first sheet for initial fill failed: %w", token, err)
}
fill["sheet_id"] = firstSheetID
fillOut, err := callTool(ctx, runtime, token, ToolKindWrite, "set_cell_range", fill)
if err != nil {
// Spreadsheet exists; surface the fill failure but keep the new
@@ -692,9 +701,11 @@ func buildInitialFillInput(runtime *common.RuntimeContext) (map[string]interface
endCol := columnIndexToLetter(maxCols - 1)
rangeStr := fmt.Sprintf("A1:%s%d", endCol, len(rows))
return map[string]interface{}{
"range": rangeStr,
"cells": rows,
"sheet_id": "", // filled in by caller if sheet_id known; otherwise server picks first sheet
"range": rangeStr,
"cells": rows,
// sheet_id is left for the caller to fill: Execute resolves the new
// workbook's first sheet via lookupFirstSheetID. The DryRun preview
// can't know it yet (the workbook doesn't exist), so it stays absent.
}, nil
}
@@ -940,3 +951,50 @@ func lookupSheetIndex(ctx context.Context, runtime *common.RuntimeContext, token
}
return "", 0, output.Errorf(output.ExitAPI, "not_found", fmt.Sprintf("sheet %q not found in workbook", target))
}
// lookupFirstSheetID returns the sheet_id of the sub-sheet at index 0 (the
// default sheet of a freshly created workbook). Used by +workbook-create to
// target the initial-fill set_cell_range write — set_cell_range rejects an
// empty sheet selector ("sheet_id or sheet_name is required"), and the v3
// create-spreadsheet response does not echo the default sheet's id.
func lookupFirstSheetID(ctx context.Context, runtime *common.RuntimeContext, token string) (string, error) {
out, err := callTool(ctx, runtime, token, ToolKindRead, "get_workbook_structure", map[string]interface{}{
"excel_id": token,
})
if err != nil {
return "", err
}
m, ok := out.(map[string]interface{})
if !ok {
return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned non-object output")
}
sheets, _ := m["sheets"].([]interface{})
bestID := ""
bestIdx := -1
for _, raw := range sheets {
sm, ok := raw.(map[string]interface{})
if !ok {
continue
}
id, _ := sm["sheet_id"].(string)
if id == "" {
continue
}
idx, ok := util.ToFloat64(sm["index"])
if !ok {
// No index field — fall back to first encountered sheet.
if bestID == "" {
bestID = id
}
continue
}
if bestIdx < 0 || int(idx) < bestIdx {
bestIdx = int(idx)
bestID = id
}
}
if bestID == "" {
return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned no sheets")
}
return bestID, nil
}