From d73e199d972abb033d2f5ec726197e4d6dc07d56 Mon Sep 17 00:00:00 2001 From: "huwenjiao.hwj" Date: Fri, 7 Aug 2026 17:26:47 +0800 Subject: [PATCH] fix(sheet): read back each sheet after --sheets table_put to catch silent data loss The --values branch already reads back its first non-empty cell after writing, to defend against the new-document initialization race where a write returns success but the data does not land. The --sheets branch called table_put and reported success with no read-back, so the same race would let the command exit successfully while one or more sheets silently lost their initial data. After table_put, the --sheets branch now: - re-fetches get_all_sheets to build a name -> sheetId map (table_put reuses the renamed default sheet and auto-creates the rest by name), and - for every spec that actually has content, reads back its first expected non-empty cell and fails if the read-back is empty or the sheet is missing. firstNonEmptySheetSpecCell mirrors firstNonEmptyValuesCell: it treats columns as the header row followed by data rows, honours start_cell, and returns hasContent=false for a name-only spec so a legitimately empty sheet is not misreported as data loss. Failures carry the nodeId and point at sheet table-put for recovery, matching the --values branch's error shape. Tests: - TestSheetCreateWithSheetsVerifiesEachSheetLanded covers an empty read-back (errors, naming the sheet + nodeId + table-put), a sheet missing from the post-write listing, and a name-only sheet that must not trigger a read-back. - TestFirstNonEmptySheetSpecCell covers header/data origins, an empty first header cell, a start_cell offset, and content-less specs. - Existing --sheets tests updated for the added get_all_sheets + per-sheet read-back calls. --- internal/helpers/sheet_create_with_data.go | 86 ++++++++++++++ .../helpers/sheet_create_with_data_test.go | 108 +++++++++++++++++- 2 files changed, 189 insertions(+), 5 deletions(-) diff --git a/internal/helpers/sheet_create_with_data.go b/internal/helpers/sheet_create_with_data.go index 42312fcd..9bf7b68c 100644 --- a/internal/helpers/sheet_create_with_data.go +++ b/internal/helpers/sheet_create_with_data.go @@ -142,6 +142,27 @@ func runCreateSheetWithData(cmd *cobra.Command, createArgs map[string]any, value }); err != nil { return fmt.Errorf("表格已创建 (nodeId=%s)，但写入初始数据失败: %w", nodeID, err) } + // 逐表回读校验：table_put 可能返回成功但在新建文档初始化竞态下数据未落盘， + // 与 --values 分支同源的问题。table_put 会按 name 复用/新建工作表，因此 + // 重取一次 name→sheetId 映射，再按每个 spec 的首个预期非空单元格回读。 + sheetIDByName, err := resolveSheetIDsByName(ctx, nodeID) + if err != nil { + return fmt.Errorf("表格已创建 (nodeId=%s)，数据已提交但无法回读校验（获取工作表列表失败）: %w；请自行确认各工作表数据是否落盘，必要时用 sheet table-put 补写", nodeID, err) + } + for _, spec := range sheetSpecs { + name, _ := spec["name"].(string) + probeCell, hasContent := firstNonEmptySheetSpecCell(spec) + if !hasContent { + continue // 只有 name、无 columns/data 的工作表本就应为空，不回读 + } + sid, ok := sheetIDByName[name] + if !ok { + return fmt.Errorf("表格已创建 (nodeId=%s)，但写入后未找到工作表 %q，其初始数据可能未落盘；请用 sheet table-put 对该文档补写", nodeID, name) + } + if err := verifyRangeNotEmpty(ctx, nodeID, sid, probeCell); err != nil { + return fmt.Errorf("表格已创建 (nodeId=%s)，但工作表 %q 的初始数据写入未生效: %w；请用 sheet table-put 对该文档补写", nodeID, name, err) + } + } } progress("初始数据写入完成。") @@ -749,6 +770,71 @@ func resolveFirstSheetID(ctx context.Context, nodeID string) (string, error) { return "", fmt.Errorf("工作表列表未返回 sheetId，响应: %s", text) } +// resolveSheetIDsByName 返回文档内 name→sheetId 映射，用于 table_put 之后按 +// 工作表名逐一回读校验（table_put 会按 name 复用/新建工作表）。 +func resolveSheetIDsByName(ctx context.Context, nodeID string) (map[string]string, error) { + text, err := callMCPToolReturnText(ctx, "get_all_sheets", map[string]any{"nodeId": nodeID}) + if err != nil { + return nil, err + } + data := unwrapSheetResult(text) + if data == nil { + return nil, fmt.Errorf("解析工作表列表失败，响应: %s", text) + } + sheets, _ := data["sheets"].([]any) + m := make(map[string]string, len(sheets)) + for _, s := range sheets { + sm, _ := s.(map[string]any) + name, _ := sm["name"].(string) + id, _ := sm["sheetId"].(string) + if name != "" && id != "" { + m[name] = id + } + } + return m, nil +} + +// sheetSpecGrid 把 table_put 的一个 sheet spec 还原成写入的二维逻辑网格： +// columns（若非空）作为表头行，其后接 data 行。用于定位首个预期非空单元格。 +func sheetSpecGrid(spec map[string]any) [][]any { + var grid [][]any + if cols, ok := spec["columns"].([]any); ok && len(cols) > 0 { + grid = append(grid, cols) + } + if data, ok := spec["data"].([]any); ok { + for _, r := range data { + if row, ok := r.([]any); ok { + grid = append(grid, row) + } else { + grid = append(grid, []any{r}) + } + } + } + return grid +} + +// firstNonEmptySheetSpecCell 返回该 sheet spec 首个预期非空单元格的 A1 地址。 +// 尊重 start_cell 偏移（默认 A1）。第二返回值为 false 表示该 spec 没有任何要写入 +// 的内容（例如只给了 name），这类工作表本就应为空，调用方跳过回读，避免把合法的 +// 空表误判为数据丢失。逻辑与 --values 分支的 firstNonEmptyValuesCell 一致：不能死盯 +// A1，首行/首列为空的合法数据不应被误报。 +func firstNonEmptySheetSpecCell(spec map[string]any) (string, bool) { + col0, row0 := 1, 1 + if sc, ok := spec["start_cell"].(string); ok && sc != "" { + if c, r, err := parseA1Cell(strings.ToUpper(sc)); err == nil { + col0, row0 = c, r + } + } + for i, row := range sheetSpecGrid(spec) { + for j, cell := range row { + if cellToString(cell) != "" { + return fmt.Sprintf("%s%d", sheetColumnLetterFromZeroBased(col0-1+j), row0+i), true + } + } + } + return "", false +} + // unwrapSheetResult 解析 MCP 响应 JSON，自动剥离外层 result 包装。 func unwrapSheetResult(text string) map[string]any { var data map[string]any diff --git a/internal/helpers/sheet_create_with_data_test.go b/internal/helpers/sheet_create_with_data_test.go index 0923f8fd..4c917ce7 100644 --- a/internal/helpers/sheet_create_with_data_test.go +++ b/internal/helpers/sheet_create_with_data_test.go @@ -295,6 +295,10 @@ func TestSheetCreateWithSheetsRenamesDefaultThenTablePut(t *testing.T) { {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, {text: `{"success":true}`}, // update_sheet 重命名默认表 {text: `{"success":true}`}, // table_put + // 回读校验阶段：先取 name→sheetId，再逐表回读 + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"一月"},{"sheetId":"SHEET_2","name":"二月"}]}`}, + {text: `{"csv":"[row=1] 项目\n"}`}, // 一月 回读非空 + {text: `{"csv":"[row=1] 项目\n"}`}, // 二月 回读非空 }} if err := runCreate(t, caller, map[string]string{ "name": "报表", @@ -302,12 +306,102 @@ func TestSheetCreateWithSheetsRenamesDefaultThenTablePut(t *testing.T) { }); err != nil { t.Fatalf("create with sheets: %v", err) } - if caller.tool != "table_put" { - t.Fatalf("last tool = %q, want table_put", caller.tool) + // create → 探活 → 定位默认表 → 重命名 → table_put → 取 name 映射 → 两表各回读一次 + if caller.calls != 8 { + t.Fatalf("calls = %d, want 8", caller.calls) } - // --sheets 路径不做 A1 回读校验，因此比 --values 少一次调用 - if caller.calls != 5 { - t.Fatalf("calls = %d, want 5", caller.calls) +} + +// table_put 返回成功但某工作表回读为空（新建文档初始化竞态），必须报数据未落盘， +// 而不是静默成功。错误须带 nodeId 与补写指引。 +func TestSheetCreateWithSheetsVerifiesEachSheetLanded(t *testing.T) { + // 二月回读为空 → 报错 + caller := &scriptedToolCaller{steps: []scriptedToolStep{ + {text: `{"nodeId":"NODE_1"}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"success":true}`}, + {text: `{"success":true}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"一月"},{"sheetId":"SHEET_2","name":"二月"}]}`}, + {text: `{"csv":"[row=1] 项目\n"}`}, // 一月 非空 + {text: `{"csv":""}`}, // 二月 回读为空 + }} + err := runCreate(t, caller, map[string]string{ + "name": "报表", + "sheets": `[{"name":"一月","columns":["项目"],"data":[["房租"]]},{"name":"二月","columns":["项目"],"data":[["房租"]]}]`, + }) + if err == nil { + t.Fatal("二月回读为空应报错，而非静默成功") + } + for _, want := range []string{"NODE_1", "二月", "写入未生效", "table-put"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("错误信息缺少 %q: %v", want, err) + } + } + + // table_put 后工作表列表里找不到某个 spec 名 → 同样报错 + missing := &scriptedToolCaller{steps: []scriptedToolStep{ + {text: `{"nodeId":"NODE_1"}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"success":true}`}, + {text: `{"success":true}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"一月"}]}`}, // 缺二月 + {text: `{"csv":"[row=1] 项目\n"}`}, // 一月 回读非空，随后二月查找失败 + }} + err = runCreate(t, missing, map[string]string{ + "name": "报表", + "sheets": `[{"name":"一月","columns":["项目"],"data":[["房租"]]},{"name":"二月","columns":["项目"],"data":[["房租"]]}]`, + }) + if err == nil || !strings.Contains(err.Error(), "二月") || !strings.Contains(err.Error(), "NODE_1") { + t.Fatalf("缺失工作表应带 nodeId 报错: %v", err) + } + + // 只有 name、无数据的工作表本就应为空，不回读、不报错 + empty := &scriptedToolCaller{steps: []scriptedToolStep{ + {text: `{"nodeId":"NODE_1"}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, + {text: `{"success":true}`}, + {text: `{"success":true}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"有数据"},{"sheetId":"SHEET_2","name":"空表"}]}`}, + {text: `{"csv":"[row=1] a\n"}`}, // 有数据 回读非空；空表不回读 + }} + if err := runCreate(t, empty, map[string]string{ + "name": "报表", + "sheets": `[{"name":"有数据","columns":["a"],"data":[["v"]]},{"name":"空表"}]`, + }); err != nil { + t.Fatalf("无数据工作表不应触发回读失败: %v", err) + } + if empty.calls != 7 { + t.Fatalf("calls = %d, want 7（空表不回读，只 6 步编排 + 有数据表 1 次回读）", empty.calls) + } +} + +func TestFirstNonEmptySheetSpecCell(t *testing.T) { + mustDecode := func(s string) map[string]any { + var m map[string]any + if err := json.Unmarshal([]byte(s), &m); err != nil { + t.Fatal(err) + } + return m + } + for _, tc := range []struct { + spec string + wantCell string + wantContent bool + }{ + {`{"name":"S","columns":["项目"],"data":[["房租"]]}`, "A1", true}, + {`{"name":"S","columns":["",""],"data":[["","x"]]}`, "B2", true}, // 首列表头空 + {`{"name":"S","data":[[1]]}`, "A1", true}, // 无表头，纯 data + {`{"name":"S","columns":["id"],"data":[[1]],"start_cell":"C3"}`, "C3", true}, // start_cell 偏移 + {`{"name":"S"}`, "", false}, // 只有 name + {`{"name":"S","columns":[],"data":[]}`, "", false}, // 空 + } { + gotCell, gotContent := firstNonEmptySheetSpecCell(mustDecode(tc.spec)) + if gotCell != tc.wantCell || gotContent != tc.wantContent { + t.Errorf("firstNonEmptySheetSpecCell(%s) = (%q,%v), want (%q,%v)", tc.spec, gotCell, gotContent, tc.wantCell, tc.wantContent) + } } } @@ -921,6 +1015,8 @@ func TestSheetCreateSheetsAppliesStylesBySheetName(t *testing.T) { {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, // resolveFirstSheetID {text: `{"success":true}`}, // update_sheet 重命名 {text: `{"success":true}`}, // table_put + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"一月"}]}`}, // resolveSheetIDsByName + {text: `{"csv":"[row=1] 项目\n"}`}, // 一月 回读非空 {text: `{"success":true}`}, // set_cell_range（样式） }} if err := runCreate(t, caller, map[string]string{ @@ -1166,6 +1262,8 @@ func TestSheetCreatePreservesLargeIntegerLiterals(t *testing.T) { {text: `{"sheets":[{"sheetId":"SHEET_1","name":"Sheet1"}]}`}, {text: `{"success":true}`}, {text: `{"success":true}`}, + {text: `{"sheets":[{"sheetId":"SHEET_1","name":"a"}]}`}, // resolveSheetIDsByName + {text: `{"csv":"[row=1] id\n"}`}, // a 回读非空 }}, map[string]string{ "name": "X", "sheets": `[{"name":"a","columns":["id"],"data":[[` + snowflake + `],[` + order + `]]}]`,