fix(aitable): scope strict write response checks
This commit is contained in:
@@ -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 获取 (必填)")
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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}}},
|
||||
})
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user