diff --git a/internal/helpers/aitable.go b/internal/helpers/aitable.go index 30f6e31a..7dc93a09 100644 --- a/internal/helpers/aitable.go +++ b/internal/helpers/aitable.go @@ -7051,8 +7051,8 @@ parentSectionId 为空串表示该节点在 Base 根目录下。 recordQueryCmd.Flags().Int("page-size", 0, "--limit 的别名(兼容 LLM 常见误用)") _ = recordQueryCmd.Flags().MarkHidden("page-size") recordQueryCmd.Flags().String("cursor", "", "分页游标,首次查询不传;cursor 为空表示已取完全部记录") - recordQueryCmd.Flags().Bool("all", false, "自动翻页获取全部记录。传入时自动循环直到无更多数据或达到 --page-limit 上限") - recordQueryCmd.Flags().Int("page-limit", 50, "自动翻页最大页数(仅 --all 时生效)。默认 50 页(5000 条),设为 0 表示无限制") + recordQueryCmd.Flags().Bool("all", false, "自动翻页获取完整记录集;达到 --page-limit 且仍有更多页时返回非零结构化错误,不把不完整结果作为成功输出") + recordQueryCmd.Flags().Int("page-limit", 50, "自动翻页最大页数(仅 --all 时生效)。默认 50 页(约 5000 条);设为 0 表示显式不限页数;超限时错误详情保留已取记录和续传 cursor") recordQueryCmd.Flags().String("view-id", "", "视图 ID(record query 不支持按视图过滤,此参数会被忽略并给出提示)") _ = recordQueryCmd.Flags().MarkHidden("view-id") recordCreateCmd.Flags().String("base-id", "", "Base ID,可通过 base list 或 base search 获取 (必填)") diff --git a/internal/helpers/aitable_record_query_e2e_test.go b/internal/helpers/aitable_record_query_e2e_test.go index 9c9c6634..ac0af18b 100644 --- a/internal/helpers/aitable_record_query_e2e_test.go +++ b/internal/helpers/aitable_record_query_e2e_test.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "reflect" + "strings" "testing" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" @@ -171,3 +172,21 @@ func TestCrossPlatformCoverageRecordQueryCLIInitialCursorE2E(t *testing.T) { t.Fatalf("resume args = %#v, want %#v", caller.calls[0].args, want) } } + +func TestCrossPlatformCoverageRecordQueryCLIPageLimitHelpE2E(t *testing.T) { + command := newAitableCommand() + command.SilenceErrors = true + command.SilenceUsage = true + out := &bytes.Buffer{} + command.SetOut(out) + command.SetErr(out) + command.SetArgs([]string{"record", "query", "--help"}) + if err := command.Execute(); err != nil { + t.Fatalf("record query help failed: %v", err) + } + for _, want := range []string{"返回非零结构化错误", "不完整结果", "错误详情保留已取记录和续传 cursor"} { + if !strings.Contains(out.String(), want) { + t.Fatalf("record query help missing %q:\n%s", want, out.String()) + } + } +} diff --git a/internal/helpers/helpers.go b/internal/helpers/helpers.go index a366c0b6..f96d2bc5 100644 --- a/internal/helpers/helpers.go +++ b/internal/helpers/helpers.go @@ -255,8 +255,8 @@ func parseMCPToolTextResult(serverID, toolName string, result *edition.ToolResul // acknowledgement. This low-level parser cannot know the business contract, // so preserve that representation. Data-returning callers must validate the // operation-specific shape (for example records/tables/valid) before they - // report success; write orchestrators use CallMCPWriteData, which treats an - // empty response as unknown unless an independent read-back proves the state. + // report success. Orchestrators whose contract requires a business result use + // CallMCPWriteDataStrict and may prove an unknown effect by independent read-back. return "", nil } diff --git a/internal/shortcut/aitable/attachment_composite.go b/internal/shortcut/aitable/attachment_composite.go index 3060f7f8..60fc8b9a 100644 --- a/internal/shortcut/aitable/attachment_composite.go +++ b/internal/shortcut/aitable/attachment_composite.go @@ -121,7 +121,7 @@ func executeAttachmentPut(rt *shortcut.RuntimeContext) error { result.Executed = false return rt.Output(result) } - prepareData, err := rt.CallMCPWriteData(serverMain, "prepare_attachment_upload", map[string]any{"baseId": baseID, "fileName": info.Name(), "size": info.Size(), "mimeType": mimeType}) + prepareData, err := rt.CallMCPWriteDataStrict(serverMain, "prepare_attachment_upload", map[string]any{"baseId": baseID, "fileName": info.Name(), "size": info.Size(), "mimeType": mimeType}) if err != nil { result.Status = "unknown" return compositeError(result, err, false) @@ -156,7 +156,7 @@ func executeAttachmentPut(rt *shortcut.RuntimeContext) error { } desired = append(desired, map[string]any{"fileToken": fileToken}) result.KnownEffects = append(result.KnownEffects, map[string]any{"tool": "HTTP PUT", "fileToken": fileToken, "fileName": info.Name(), "size": info.Size()}) - _, writeErr := rt.CallMCPWriteData(serverMain, "update_records", map[string]any{ + _, writeErr := rt.CallMCPWriteDataStrict(serverMain, "update_records", map[string]any{ "baseId": baseID, "tableId": tableID, "records": []any{map[string]any{"recordId": recordIDValue, "cells": map[string]any{fieldID: desired}}}, }) @@ -226,7 +226,7 @@ func executeAttachmentRemove(rt *shortcut.RuntimeContext) error { result.Executed = false return rt.Output(result) } - _, writeErr := rt.CallMCPWriteData(serverMain, "update_records", map[string]any{ + _, writeErr := rt.CallMCPWriteDataStrict(serverMain, "update_records", map[string]any{ "baseId": baseID, "tableId": tableID, "records": []any{map[string]any{"recordId": recordIDValue, "cells": map[string]any{fieldID: desired}}}, }) diff --git a/internal/shortcut/aitable/base_composite.go b/internal/shortcut/aitable/base_composite.go index 73971707..1addc982 100644 --- a/internal/shortcut/aitable/base_composite.go +++ b/internal/shortcut/aitable/base_composite.go @@ -191,7 +191,7 @@ func executeBaseBootstrap(rt *shortcut.RuntimeContext) error { if rt.Changed("template-id") { baseArgs["templateId"] = rt.Str("template-id") } - baseData, err := rt.CallMCPWriteData(serverMain, "create_base", baseArgs) + baseData, err := rt.CallMCPWriteDataStrict(serverMain, "create_base", baseArgs) if err != nil { result.Status = "unknown" result.FailedCount = len(tables) @@ -222,7 +222,7 @@ func executeBaseBootstrap(rt *shortcut.RuntimeContext) error { for index, spec := range tables { initialEnd := minInt(15, len(spec.Fields)) initialFields := spec.Fields[:initialEnd] - createData, createErr := rt.CallMCPWriteData(serverMain, "create_table", map[string]any{ + createData, createErr := rt.CallMCPWriteDataStrict(serverMain, "create_table", map[string]any{ "baseId": baseID, "tableName": spec.Name, "fields": initialFields, }) tableID := findStringByKeys(createData, "tableId", "sheetId") @@ -239,7 +239,7 @@ func executeBaseBootstrap(rt *shortcut.RuntimeContext) error { result.KnownEffects = append(result.KnownEffects, map[string]any{"tool": "create_table", "baseId": baseID, "tableId": tableID, "name": spec.Name}) for offset := initialEnd; offset < len(spec.Fields); offset += 15 { end := minInt(offset+15, len(spec.Fields)) - _, fieldErr := rt.CallMCPWriteData(serverMain, "create_fields", map[string]any{ + _, fieldErr := rt.CallMCPWriteDataStrict(serverMain, "create_fields", map[string]any{ "baseId": baseID, "tableId": tableID, "fields": spec.Fields[offset:end], }) if fieldErr != nil { diff --git a/internal/shortcut/aitable/record_batch.go b/internal/shortcut/aitable/record_batch.go index 9ad7f9ad..8c26c3b1 100644 --- a/internal/shortcut/aitable/record_batch.go +++ b/internal/shortcut/aitable/record_batch.go @@ -87,7 +87,7 @@ func executeRecordDeleteBatches(rt *shortcut.RuntimeContext) error { for offset := 0; offset < len(ids); offset += recordBatchSize { end := minInt(offset+recordBatchSize, len(ids)) batch := ids[offset:end] - writeData, writeErr := rt.CallMCPWriteData(serverMain, "delete_records", map[string]any{ + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, "delete_records", map[string]any{ "baseId": baseID, "tableId": tableID, "recordIds": batch, }) step := compositeStep{ @@ -192,7 +192,7 @@ func executeRecordBatches( wireRecords = append(wireRecords, record) } params := map[string]any{"baseId": baseID, "tableId": tableID, "records": wireRecords} - writeData, writeErr := rt.CallMCPWriteData(product, tool, params) + writeData, writeErr := rt.CallMCPWriteDataStrict(product, tool, params) step := compositeStep{ Index: len(result.CompletedSteps) + 1, Name: "write record batch", Tool: tool, Status: "completed", Offset: offset, Count: len(batch), Result: writeData, diff --git a/internal/shortcut/aitable/record_upsert_by_key.go b/internal/shortcut/aitable/record_upsert_by_key.go index 038afa45..b6c3efbc 100644 --- a/internal/shortcut/aitable/record_upsert_by_key.go +++ b/internal/shortcut/aitable/record_upsert_by_key.go @@ -129,7 +129,7 @@ func executeRecordUpsertByKey(rt *shortcut.RuntimeContext) error { return rt.Output(result) } - writeData, writeErr := rt.CallMCPWriteData(serverMain, tool, params) + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, tool, params) writeStep := compositeStep{Index: 1, Name: action + " record", Tool: tool, Status: "completed", Count: 1, Result: writeData} if writeErr != nil { writeStep.Status = "unknown" diff --git a/internal/shortcut/aitable/table_copy.go b/internal/shortcut/aitable/table_copy.go index 6da8c639..05df6c78 100644 --- a/internal/shortcut/aitable/table_copy.go +++ b/internal/shortcut/aitable/table_copy.go @@ -113,7 +113,7 @@ func executeTableCopy(rt *shortcut.RuntimeContext) error { } initialEnd := minInt(15, len(createFields)) - createData, err := rt.CallMCPWriteData(serverMain, "create_table", map[string]any{ + createData, err := rt.CallMCPWriteDataStrict(serverMain, "create_table", map[string]any{ "baseId": targetBase, "tableName": rt.Str("new-name"), "fields": createFields[:initialEnd], }) targetTable := findStringByKeys(createData, "tableId", "sheetId") @@ -129,7 +129,7 @@ func executeTableCopy(rt *shortcut.RuntimeContext) error { result.KnownEffects = append(result.KnownEffects, map[string]any{"tool": "create_table", "baseId": targetBase, "tableId": targetTable}) for offset := initialEnd; offset < len(createFields); offset += 15 { end := minInt(offset+15, len(createFields)) - _, fieldErr := rt.CallMCPWriteData(serverMain, "create_fields", map[string]any{"baseId": targetBase, "tableId": targetTable, "fields": createFields[offset:end]}) + _, fieldErr := rt.CallMCPWriteDataStrict(serverMain, "create_fields", map[string]any{"baseId": targetBase, "tableId": targetTable, "fields": createFields[offset:end]}) if fieldErr != nil { result.Warnings = append(result.Warnings, fmt.Sprintf("create_fields offset %d returned an error; final schema verification decides success: %v", offset, fieldErr)) } @@ -172,7 +172,7 @@ func executeTableCopy(rt *shortcut.RuntimeContext) error { batch = append(batch, record) wire = append(wire, record) } - writeData, writeErr := rt.CallMCPWriteData(serverMain, "create_records", map[string]any{"baseId": targetBase, "tableId": targetTable, "records": wire}) + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, "create_records", map[string]any{"baseId": targetBase, "tableId": targetTable, "records": wire}) createdIDs := createdRecordIDs(writeData) if len(createdIDs) == 0 { if returned, found := findRecords(writeData); found { diff --git a/internal/shortcut/aitable/view_preset.go b/internal/shortcut/aitable/view_preset.go index ef53e0df..6a16aebc 100644 --- a/internal/shortcut/aitable/view_preset.go +++ b/internal/shortcut/aitable/view_preset.go @@ -96,7 +96,7 @@ func executeViewPresetApply(rt *shortcut.RuntimeContext) error { result.Executed = false return rt.Output(result) } - writeData, writeErr := rt.CallMCPWriteData(serverMain, tool, params) + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, tool, params) if action == "create" { viewID = findStringByKeys(writeData, "viewId") } diff --git a/internal/shortcut/aitable/workflow_deploy.go b/internal/shortcut/aitable/workflow_deploy.go index d4f8f9c4..fc18390a 100644 --- a/internal/shortcut/aitable/workflow_deploy.go +++ b/internal/shortcut/aitable/workflow_deploy.go @@ -75,7 +75,7 @@ func executeWorkflowDeploy(rt *shortcut.RuntimeContext) error { result.Executed = false return rt.Output(result) } - writeData, writeErr := rt.CallMCPWriteData(serverMain, tool, params) + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, tool, params) if writeErr != nil { result.Status = "unknown" result.Checkpoint = map[string]any{"nextStep": "resolve workflow existence and definition before retrying", "workflowId": workflowID} @@ -105,7 +105,7 @@ func executeWorkflowDeploy(rt *shortcut.RuntimeContext) error { return compositeError(result, verifyErr, action == "update") } if rt.Bool("enable") { - enableData, enableErr := rt.CallMCPWriteData(serverHelper, "enable_workflow", map[string]any{"baseId": baseID, "workflowId": workflowID}) + enableData, enableErr := rt.CallMCPWriteDataStrict(serverHelper, "enable_workflow", map[string]any{"baseId": baseID, "workflowId": workflowID}) if enableErr != nil { result.Warnings = append(result.Warnings, "enable response error: "+enableErr.Error()) } diff --git a/internal/shortcut/runner.go b/internal/shortcut/runner.go index df1ed6bd..09810782 100644 --- a/internal/shortcut/runner.go +++ b/internal/shortcut/runner.go @@ -155,8 +155,19 @@ func (rt *RuntimeContext) CallMCPData(product, tool string, params map[string]an // CallMCPWriteData dispatches a write tool call and returns its parsed response. // Unlike CallMCPData, it refuses to run under --dry-run so smart shortcuts cannot -// accidentally perform writes while rendering a preview. +// accidentally perform writes while rendering a preview. For compatibility with +// existing write shortcuts, an empty text acknowledgement remains an empty map. func (rt *RuntimeContext) CallMCPWriteData(product, tool string, params map[string]any) (map[string]any, error) { + if rt.DryRun() { + return nil, dryRunWriteError(product, tool) + } + return rt.callMCPData(product, tool, params) +} + +// CallMCPWriteDataStrict dispatches a write tool call whose contract requires a +// non-empty JSON business result. An empty acknowledgement is reported as an +// unknown remote effect so callers can verify it independently before success. +func (rt *RuntimeContext) CallMCPWriteDataStrict(product, tool string, params map[string]any) (map[string]any, error) { if rt.DryRun() { return nil, dryRunWriteError(product, tool) } diff --git a/internal/shortcut/runner_coverage_test.go b/internal/shortcut/runner_coverage_test.go index 51f90346..b5d7d8dd 100644 --- a/internal/shortcut/runner_coverage_test.go +++ b/internal/shortcut/runner_coverage_test.go @@ -4,9 +4,11 @@ package shortcut import ( + "errors" "strings" "testing" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" "github.com/spf13/cobra" ) @@ -31,4 +33,18 @@ func TestCrossPlatformCoverageRuntimeWriteDataRemainingBranches(t *testing.T) { if caller.args == nil { t.Fatal("nil write parameters were not normalized") } + + caller.text = "" + legacy, err := rt.CallMCPWriteData("chat", "send_personal_message", nil) + if err != nil || legacy == nil || len(legacy) != 0 { + t.Fatalf("legacy empty acknowledgement = %#v, %v", legacy, err) + } + if _, err = rt.CallMCPWriteDataStrict("aitable", "update_records", nil); err == nil { + t.Fatal("strict empty acknowledgement was accepted") + } else { + var typed *apperrors.Error + if !errors.As(err, &typed) || typed.Reason != "empty_tool_response" || !typed.RetryableSet || typed.Retryable { + t.Fatalf("strict empty acknowledgement error = %#v", err) + } + } } diff --git a/internal/shortcut/shortcut_test.go b/internal/shortcut/shortcut_test.go index 7cd6cf81..b1a5139c 100644 --- a/internal/shortcut/shortcut_test.go +++ b/internal/shortcut/shortcut_test.go @@ -671,12 +671,17 @@ func TestCrossPlatformCoverageCallMCPWriteDataRejectsDryRun(t *testing.T) { } rt := &RuntimeContext{cmd: cmd, shortcut: Shortcut{Service: "calendar"}} - _, err := rt.CallMCPWriteData("calendar", "create_calendar_event", map[string]any{"summary": "x"}) - if err == nil { - t.Fatal("expected dry-run write guard error") - } - if !strings.Contains(err.Error(), "calendar/create_calendar_event") { - t.Fatalf("error = %q, want tool name", err.Error()) + for name, call := range map[string]func(string, string, map[string]any) (map[string]any, error){ + "legacy": rt.CallMCPWriteData, + "strict": rt.CallMCPWriteDataStrict, + } { + _, err := call("calendar", "create_calendar_event", map[string]any{"summary": "x"}) + if err == nil { + t.Fatalf("%s: expected dry-run write guard error", name) + } + if !strings.Contains(err.Error(), "calendar/create_calendar_event") { + t.Fatalf("%s: error = %q, want tool name", name, err.Error()) + } } }