From 6dbc7ed82b7c2d15e08a9ad39939acd270ea2f6a Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 19 Aug 2026 10:47:24 +0800 Subject: [PATCH 1/5] fix: harden shortcut functional workflows --- internal/helpers/aitable.go | 360 +++++++++++++++++- .../helpers/aitable_command_edges_test.go | 109 ++++++ internal/helpers/aitable_core_test.go | 6 +- .../helpers/aitable_workflow_write_test.go | 63 ++- internal/helpers/drive_folder_contract.go | 4 +- .../helpers/drive_folder_contract_test.go | 23 ++ internal/helpers/drive_push.go | 40 +- .../drive_push_sync_final_coverage_test.go | 23 ++ internal/helpers/drive_sync.go | 118 +++++- .../helpers/drive_sync_confirmation_test.go | 84 +++- internal/helpers/wiki.go | 7 +- internal/helpers/wiki_coverage_more_test.go | 27 +- internal/shortcut/aitable/aitable.go | 18 +- .../aitable/base_composite_e2e_test.go | 81 ++++ internal/shortcut/aitable/base_copy.go | 143 +++++++ internal/shortcut/aitable/primary_doc.go | 133 +++++++ internal/shortcut/aitable/record_batch.go | 84 +++- .../aitable/record_coverage_gap_test.go | 63 +++ .../aitable/record_upsert_by_key_e2e_test.go | 142 +++++-- .../aitable/remaining_coverage_gap_test.go | 5 + internal/shortcut/aitable/view_preset.go | 122 +++++- .../view_workflow_composite_e2e_test.go | 53 +++ internal/shortcut/aitabletarget/resolver.go | 3 + .../shortcut/aitabletarget/resolver_test.go | 18 + internal/shortcut/contact/contact.go | 9 +- .../contact/search_mobile_coverage_test.go | 33 ++ internal/shortcut/minutesdata/workflow.go | 3 +- .../shortcut/minutesdata/workflow_test.go | 20 + .../shortcut/todo/page_size_coverage_test.go | 51 +++ internal/shortcut/wiki/common.go | 24 ++ internal/shortcut/wiki/nodes.go | 2 +- .../shortcut/wiki/workflow_coverage_test.go | 23 ++ 32 files changed, 1783 insertions(+), 111 deletions(-) create mode 100644 internal/shortcut/aitable/base_copy.go create mode 100644 internal/shortcut/aitable/primary_doc.go create mode 100644 internal/shortcut/contact/search_mobile_coverage_test.go create mode 100644 internal/shortcut/todo/page_size_coverage_test.go diff --git a/internal/helpers/aitable.go b/internal/helpers/aitable.go index f981d7cd..d31b1f7e 100644 --- a/internal/helpers/aitable.go +++ b/internal/helpers/aitable.go @@ -7,6 +7,7 @@ import ( "io" "math" "os" + "reflect" "strconv" "strings" "time" @@ -126,6 +127,83 @@ func resolveWorkflowDSL(cmd *cobra.Command) (map[string]any, error) { return dsl, nil } +// executeAitableWorkflowPublish executes a publish exactly once, then requires +// the reviewed valid/flowId envelope before rendering any success output. +// create_workflow is non-idempotent, so transport uncertainty is never retried. +func executeAitableWorkflowPublish(toolName string, args map[string]any) error { + if deps.Caller.DryRun() { + return callMCPToolOnServer("aitable", toolName, args) + } + raw, err := callMCPToolReturnTextOnServer(context.Background(), "aitable", toolName, args) + if err != nil { + return err + } + var envelope map[string]any + if err := json.Unmarshal([]byte(raw), &envelope); err != nil || envelope == nil { + return fmt.Errorf("%s response is not a JSON object", toolName) + } + valid, validFound, flowID, err := strictAitableWorkflowPublishResult(envelope) + if err != nil { + return fmt.Errorf("%s response validation failed: %w", toolName, err) + } + if !validFound { + return fmt.Errorf("%s response is missing valid", toolName) + } + if !valid { + return fmt.Errorf("%s returned valid=false; inspect issues and correct the workflow DSL", toolName) + } + if flowID == "" { + return fmt.Errorf("%s response is missing a non-empty flowId", toolName) + } + return RenderLegacyMCPText(toolName, raw) +} + +func strictAitableWorkflowPublishResult(envelope map[string]any) (valid bool, validFound bool, flowID string, err error) { + var visit func(map[string]any) error + visit = func(object map[string]any) error { + if raw, exists := object["valid"]; exists { + value, ok := raw.(bool) + if !ok { + return fmt.Errorf("valid must be boolean, got %T", raw) + } + if validFound && valid != value { + return fmt.Errorf("conflicting valid values") + } + valid, validFound = value, true + } + for _, key := range []string{"flowId", "workflowId"} { + raw, exists := object[key] + if !exists { + continue + } + value, ok := raw.(string) + value = strings.TrimSpace(value) + if !ok || value == "" { + return fmt.Errorf("%s must be a non-empty string", key) + } + if flowID != "" && flowID != value { + return fmt.Errorf("conflicting workflow IDs") + } + flowID = value + } + for _, key := range []string{"data", "result", "response"} { + if nested, ok := object[key].(map[string]any); ok { + if err := visit(nested); err != nil { + return err + } + } + } + return nil + } + if envelope == nil { + return false, false, "", fmt.Errorf("empty response") + } + if err := visit(envelope); err != nil { + return false, false, "", err + } + return valid, validFound, flowID, nil +} + func validateWorkflowRunFlags(cmd *cobra.Command, _ []string) error { tableID, _ := cmd.Flags().GetString("table-id") tableID = strings.TrimSpace(tableID) @@ -1184,6 +1262,282 @@ func runAitableViewUpdateArray(cmd *cobra.Command, blockKey string) error { return callUpdateViewWithBlock(baseID, tableID, viewID, blockKey, cfgMap[blockKey], nil) } +func runAitableViewUpdateFilter(cmd *cobra.Command) error { + baseID, tableID, viewID, _, err := viewUpdateCommonPreflight(cmd, "filter", nil, false) + if err != nil { + return err + } + jsonStr, _ := cmd.Flags().GetString("json") + if jsonStr == "" { + return fmt.Errorf("必须指定 --json 传入 filter JSON 数组") + } + var parsed any + if err := json.Unmarshal([]byte(jsonStr), &parsed); err != nil { + return fmt.Errorf("--json 解析失败: %v", err) + } + cfgMap := map[string]any{"filter": parsed} + if err := normalizeViewConfigBlock(cfgMap); err != nil { + return err + } + filter, _ := cfgMap["filter"].([]any) + fieldTypes, err := loadAitableFieldTypes(context.Background(), baseID, tableID) + if err != nil { + return err + } + if err := validateAitableViewFilter(filter, fieldTypes); err != nil { + return err + } + toolArgs := map[string]any{ + "baseId": baseID, "tableId": tableID, "viewId": viewID, + "config": map[string]any{"filter": filter}, + } + if deps.Caller.DryRun() { + return deps.Out.PrintJSON(map[string]any{ + "dry_run": true, "executed": false, "tool": "update_view", "arguments": toolArgs, + }) + } + if _, err := callMCPToolReturnTextOnServer(context.Background(), "aitable", "update_view", toolArgs); err != nil { + return err + } + var actual any + var readBackErr error + for attempt := 0; attempt < aitableViewFilterReadbackAttempts; attempt++ { + if attempt > 0 { + backoff := time.Duration(1<<(attempt-1)) * time.Second + if backoff > 8*time.Second { + backoff = 8 * time.Second + } + aitableViewFilterReadbackSleep(backoff) + } + view, _, err := getViewRaw(context.Background(), baseID, tableID, viewID) + if err != nil { + readBackErr = err + continue + } + actualViewID, _ := view["viewId"].(string) + if actualViewID != viewID { + readBackErr = fmt.Errorf("update_view read-back returned viewId %q, want %q", actualViewID, viewID) + continue + } + actual = walkViewPath(view, "filter") + if persistedViewFilterMatches(actual, filter) { + readBackErr = nil + break + } + readBackErr = fmt.Errorf("update_view filter read-back mismatch: got %s, want %s", compactJSON(actual), compactJSON(filter)) + } + if readBackErr != nil { + return &CLIError{Code: CodeMCPToolError, Message: readBackErr.Error(), Suggestion: "重新读取 view get filter，确认服务端支持所用字段类型和操作符后再试"} + } + return deps.Out.PrintJSON(map[string]any{ + "success": true, + "data": map[string]any{"baseId": baseID, "tableId": tableID, "viewId": viewID, "filter": filter, "verified": true}, + }) +} + +const aitableViewFilterReadbackAttempts = 6 + +var aitableViewFilterReadbackSleep = time.Sleep + +func persistedViewFilterMatches(actual any, expected []any) bool { + if reflect.DeepEqual(actual, expected) { + return true + } + root, ok := actual.(map[string]any) + if !ok || root["operator"] != "and" { + return false + } + operands, ok := root["operands"].([]any) + return ok && reflect.DeepEqual(operands, expected) +} + +func loadAitableFieldTypes(ctx context.Context, baseID, tableID string) (map[string]string, error) { + raw, err := callMCPReadToolReturnTextOnServer(ctx, "aitable", "get_fields", map[string]any{"baseId": baseID, "tableId": tableID}) + if err != nil { + return nil, err + } + var payload any + if err := json.Unmarshal([]byte(raw), &payload); err != nil { + return nil, fmt.Errorf("get_fields response is not valid JSON: %v", err) + } + fields, ok := findAitableObjectList(payload, "fields", "fieldList") + if !ok { + return nil, fmt.Errorf("get_fields response is missing the fields collection") + } + types := make(map[string]string, len(fields)) + for index, field := range fields { + fieldID, _ := field["fieldId"].(string) + if strings.TrimSpace(fieldID) == "" { + fieldID, _ = field["id"].(string) + } + fieldType, _ := field["type"].(string) + if strings.TrimSpace(fieldType) == "" { + fieldType, _ = field["fieldType"].(string) + } + if strings.TrimSpace(fieldID) == "" || strings.TrimSpace(fieldType) == "" { + return nil, fmt.Errorf("get_fields field %d is missing fieldId or type", index) + } + types[strings.TrimSpace(fieldID)] = strings.TrimSpace(fieldType) + } + return types, nil +} + +func findAitableObjectList(value any, names ...string) ([]map[string]any, bool) { + wanted := make(map[string]bool, len(names)) + for _, name := range names { + wanted[strings.ToLower(name)] = true + } + var walk func(any) ([]map[string]any, bool) + walk = func(current any) ([]map[string]any, bool) { + switch typed := current.(type) { + case map[string]any: + for key, child := range typed { + if !wanted[strings.ToLower(key)] { + continue + } + items, ok := child.([]any) + if !ok { + return nil, false + } + out := make([]map[string]any, 0, len(items)) + for _, item := range items { + object, ok := item.(map[string]any) + if !ok { + return nil, false + } + out = append(out, object) + } + return out, true + } + for _, child := range typed { + if found, ok := walk(child); ok { + return found, true + } + } + case []any: + for _, child := range typed { + if found, ok := walk(child); ok { + return found, true + } + } + } + return nil, false + } + return walk(value) +} + +func validateAitableViewFilter(filter []any, fieldTypes map[string]string) error { + for index, raw := range filter { + condition, ok := raw.(map[string]any) + if !ok { + return fmt.Errorf("filter[%d] must be an object", index) + } + if err := validateAitableViewFilterCondition(condition, fieldTypes); err != nil { + return fmt.Errorf("filter[%d]: %w", index, err) + } + } + return nil +} + +func validateAitableViewFilterCondition(condition map[string]any, fieldTypes map[string]string) error { + operator, _ := condition["operator"].(string) + operator = strings.TrimSpace(operator) + if operator == "" || !validFilterOperators[operator] { + return fmt.Errorf("unsupported operator %q", operator) + } + operands, ok := condition["operands"].([]any) + if !ok { + return fmt.Errorf("operator %s requires an operands array", operator) + } + if operator == "and" || operator == "or" { + return fmt.Errorf("logical operator %s is not supported by the persisted view protocol; pass a flat array of leaf conditions (combined as AND)", operator) + } + wantOperands := 2 + if operator == "exist" || operator == "un_exist" { + wantOperands = 1 + } + if len(operands) != wantOperands { + return fmt.Errorf("operator %s requires %d operands", operator, wantOperands) + } + fieldID, ok := operands[0].(string) + fieldID = strings.TrimSpace(fieldID) + if !ok || fieldID == "" { + return fmt.Errorf("operator %s requires a fieldId as its first operand", operator) + } + fieldType, exists := fieldTypes[fieldID] + if !exists { + return fmt.Errorf("filter references unknown fieldId %q", fieldID) + } + if isAitableDateFieldType(fieldType) && !isAitableDateFilterOperator(operator) { + return fmt.Errorf("operator %s is invalid for %s field %s; use date_eq/before/after/not_before/not_after/exist/un_exist", operator, fieldType, fieldID) + } + if operator == "any_of" || operator == "all_of" || operator == "none_of" { + if !strings.EqualFold(fieldType, "multipleSelect") && !strings.EqualFold(fieldType, "multiSelect") { + return fmt.Errorf("operator %s requires a multipleSelect field, got %s", operator, fieldType) + } + if operator == "any_of" { + if values, ok := operands[1].([]any); ok { + if _, err := expandAitableMultiSelectAnyOf(fieldID, values); err != nil { + return err + } + return fmt.Errorf("multipleSelect any_of with multiple values is not supported by the persisted view protocol; use one scalar option or separate views") + } + } + if err := validateAitableMultiSelectFilterValue(operands[1]); err != nil { + return err + } + } + return nil +} + +func expandAitableMultiSelectAnyOf(fieldID string, values []any) ([]any, error) { + if len(values) == 0 { + return nil, fmt.Errorf("multipleSelect any_of array must not be empty") + } + expanded := make([]any, 0, len(values)) + for index, value := range values { + text, ok := value.(string) + text = strings.TrimSpace(text) + if !ok || text == "" { + return nil, fmt.Errorf("multipleSelect any_of value %d must be a non-empty option-name string", index) + } + expanded = append(expanded, map[string]any{ + "operator": "eq", "operands": []any{fieldID, text}, + }) + } + return expanded, nil +} + +func isAitableDateFieldType(fieldType string) bool { + return strings.EqualFold(fieldType, "date") || + strings.EqualFold(fieldType, "createdTime") || + strings.EqualFold(fieldType, "lastModifiedTime") +} + +func isAitableDateFilterOperator(operator string) bool { + switch operator { + case "date_eq", "before", "after", "not_before", "not_after", "exist", "un_exist": + return true + default: + return false + } +} + +func validateAitableMultiSelectFilterValue(value any) error { + if typed, ok := value.(string); ok && strings.TrimSpace(typed) != "" { + return nil + } + return fmt.Errorf("multipleSelect filter value must be one option-name string; the persisted view protocol does not support a multi-value OR expression") +} + +func compactJSON(value any) string { + raw, err := json.Marshal(value) + if err != nil { + return fmt.Sprintf("%#v", value) + } + return string(raw) +} + func newAitableCommand() *cobra.Command { // Product-level Agent routing Decl (migrated from selection/aitable.json // products.aitable). Catalog assembly stamps provenance contract_final. @@ -4076,7 +4430,7 @@ colorConfigs (JSON 数组) / officialHoliday (bool)。`, 若传对象会自动 wrap 为数组；其他非法格式拒绝。`, Example: ` dws aitable view update filter --view-id VIEW_ID --json '[{"operator":"and","operands":[{"operator":"eq","operands":["fldX","value"]}]}]'`, RunE: func(cmd *cobra.Command, args []string) error { - return runAitableViewUpdateArray(cmd, "filter") + return runAitableViewUpdateFilter(cmd) }, } DeclareLeafMetadata(viewUpdateFilterCmd, LeafSpec{ @@ -5282,7 +5636,7 @@ valid=false 仍表示 DSL 校验或发布未通过，必须读取 issues 修正 } // create_workflow is non-idempotent. Bypass the retry wrapper to // prevent an uncertain first response from creating a duplicate. - return callMCPToolOnServer("aitable", "create_workflow", toolArgs) + return executeAitableWorkflowPublish("create_workflow", toolArgs) }, } DeclareLeafMetadata(workflowCreateCmd, LeafSpec{ @@ -5376,7 +5730,7 @@ valid=false 仍表示 DSL 校验或发布未通过，必须读取 issues 修正 if locale, _ := cmd.Flags().GetString("locale"); strings.TrimSpace(locale) != "" { toolArgs["locale"] = locale } - return callAitableTool("update_workflow", toolArgs) + return executeAitableWorkflowPublish("update_workflow", toolArgs) }, } DeclareLeafMetadata(workflowUpdateCmd, LeafSpec{ diff --git a/internal/helpers/aitable_command_edges_test.go b/internal/helpers/aitable_command_edges_test.go index e821023c..262b0f09 100644 --- a/internal/helpers/aitable_command_edges_test.go +++ b/internal/helpers/aitable_command_edges_test.go @@ -361,3 +361,112 @@ func TestCrossPlatformCoverageAitableDeleteCancellationEdges(t *testing.T) { _ = runAitableCoverageCommand(t, &aitableCommandCoverageCaller{}, args...) } } + +func TestCrossPlatformCoverageAitableViewFilterValidationAndReadBack(t *testing.T) { + testseam.Swap(t, &aitableViewFilterReadbackSleep, func(time.Duration) {}) + oldArgs := os.Args + t.Cleanup(func() { os.Args = oldArgs }) + os.Args = []string{"dws", "aitable"} + filter := `[{"operator":"eq","operands":["fldA","x"]},{"operator":"any_of","operands":["fldMulti","A"]}]` + fields := `{"data":{"fields":[{"fieldId":"fldA","type":"text"},{"fieldId":"fldB","type":"text"},{"fieldId":"fldMulti","type":"multipleSelect"}]}}` + readBack := `{"data":{"views":[{"viewId":"view","viewType":"Grid","filter":[{"operator":"eq","operands":["fldA","x"]},{"operator":"any_of","operands":["fldMulti","A"]}]}]}}` + + t.Run("flat leaf filters are written then exactly verified", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields, `{"success":true}`, readBack}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+filter) + if err != nil || len(caller.calls) != 3 || caller.calls[0].tool != "get_fields" || caller.calls[1].tool != "update_view" || caller.calls[2].tool != "get_views" { + t.Fatalf("verified view filter = err:%v calls:%#v", err, caller.calls) + } + config, _ := caller.calls[1].args["config"].(map[string]any) + if _, ok := config["filter"].([]any); !ok { + t.Fatalf("update_view filter encoding = %#v", caller.calls[1].args) + } + }) + + t.Run("logical groups fail closed before write", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", `--json=[{"operator":"or","operands":[{"operator":"eq","operands":["fldA","x"]},{"operator":"eq","operands":["fldB","y"]}]}]`) + if err == nil || !strings.Contains(err.Error(), "persisted view protocol") || len(caller.calls) != 1 || caller.calls[0].tool != "get_fields" { + t.Fatalf("logical filter group fail-closed = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("unknown field is rejected before write", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", `--json=[{"operator":"eq","operands":["missing","x"]}]`) + if err == nil || !strings.Contains(err.Error(), "unknown fieldId") || len(caller.calls) != 1 { + t.Fatalf("unknown filter field = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("eventual persisted and wrapper is retried then verified", func(t *testing.T) { + input := `[{"operator":"any_of","operands":["fldMulti","A"]}]` + stale := `{"data":{"views":[{"viewId":"view","viewType":"Grid","filter":{"operator":"and","operands":[]}}]}}` + terminal := `{"data":{"views":[{"viewId":"view","viewType":"Grid","filter":{"operator":"and","operands":[{"operator":"any_of","operands":["fldMulti","A"]}]}}]}}` + caller := &aitableTestCaller{responses: []string{fields, `{"success":true}`, stale, terminal}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+input) + if err != nil || len(caller.calls) != 4 || caller.calls[2].tool != "get_views" || caller.calls[3].tool != "get_views" { + t.Fatalf("eventual wrapped filter = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("multi-select operator rejects text field", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", `--json=[{"operator":"any_of","operands":["fldA",["A"]]}]`) + if err == nil || !strings.Contains(err.Error(), "requires a multipleSelect field") || len(caller.calls) != 1 { + t.Fatalf("wrong filter field type = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("multi-select array fails closed before write", func(t *testing.T) { + input := `[{"operator":"any_of","operands":["fldMulti",["A","B"]]}]` + caller := &aitableTestCaller{responses: []string{fields}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+input) + if err == nil || !strings.Contains(err.Error(), "persisted view protocol") || len(caller.calls) != 1 || caller.calls[0].tool != "get_fields" { + t.Fatalf("multi-select array fail-closed = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("multi-select invalid array value fails before write", func(t *testing.T) { + input := `[{"operator":"any_of","operands":["fldMulti",["A",""]]}]` + caller := &aitableTestCaller{responses: []string{fields}} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+input) + if err == nil || !strings.Contains(err.Error(), "non-empty option-name") || len(caller.calls) != 1 { + t.Fatalf("invalid multi-select array = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("date and system-time fields require date operators", func(t *testing.T) { + fieldTypes := map[string]string{"date": "date", "created": "createdTime", "modified": "lastModifiedTime"} + for fieldID := range fieldTypes { + valid := []any{map[string]any{"operator": "date_eq", "operands": []any{fieldID, "2026-08-18"}}} + if err := validateAitableViewFilter(valid, fieldTypes); err != nil { + t.Fatalf("date_eq for %s: %v", fieldID, err) + } + invalid := []any{map[string]any{"operator": "eq", "operands": []any{fieldID, "2026-08-18"}}} + if err := validateAitableViewFilter(invalid, fieldTypes); err == nil || !strings.Contains(err.Error(), "invalid for") { + t.Fatalf("eq for %s = %v, want date-operator error", fieldID, err) + } + } + }) + + t.Run("dry-run validates but performs no write or read-back", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields}, dryRun: true} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+filter, "--dry-run") + if err != nil || len(caller.calls) != 1 || caller.calls[0].tool != "get_fields" { + t.Fatalf("view filter dry-run = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("read-back mismatch is not success", func(t *testing.T) { + responses := []string{fields, `{"success":true}`} + for range aitableViewFilterReadbackAttempts { + responses = append(responses, `{"data":{"views":[{"viewId":"view","viewType":"Grid","filter":[]}]}}`) + } + caller := &aitableTestCaller{responses: responses} + err := runAitableCoverageCommand(t, caller, "view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json="+filter) + if err == nil || !strings.Contains(err.Error(), "read-back mismatch") || len(caller.calls) != 2+aitableViewFilterReadbackAttempts { + t.Fatalf("mismatched filter readback = err:%v calls:%#v", err, caller.calls) + } + }) +} diff --git a/internal/helpers/aitable_core_test.go b/internal/helpers/aitable_core_test.go index 66f97454..774008b8 100644 --- a/internal/helpers/aitable_core_test.go +++ b/internal/helpers/aitable_core_test.go @@ -24,6 +24,7 @@ type aitableTestCaller struct { responses []string errors []error calls []aitableTestCall + dryRun bool } func (c *aitableTestCaller) CallTool(_ context.Context, server, tool string, args map[string]any) (*edition.ToolResult, error) { @@ -38,8 +39,11 @@ func (c *aitableTestCaller) CallTool(_ context.Context, server, tool string, arg } return textToolResult(response), nil } +func (c *aitableTestCaller) CallReadTool(ctx context.Context, server, tool string, args map[string]any) (*edition.ToolResult, error) { + return c.CallTool(ctx, server, tool, args) +} func (*aitableTestCaller) Format() string { return "json" } -func (*aitableTestCaller) DryRun() bool { return false } +func (c *aitableTestCaller) DryRun() bool { return c.dryRun } func (*aitableTestCaller) Fields() string { return "" } func (*aitableTestCaller) JQ() string { return "" } diff --git a/internal/helpers/aitable_workflow_write_test.go b/internal/helpers/aitable_workflow_write_test.go index 1feff897..db95925a 100644 --- a/internal/helpers/aitable_workflow_write_test.go +++ b/internal/helpers/aitable_workflow_write_test.go @@ -23,27 +23,38 @@ type aitableWorkflowCall struct { } type aitableWorkflowCaller struct { - calls []aitableWorkflowCall + calls []aitableWorkflowCall + response string + dryRun bool } func (c *aitableWorkflowCaller) CallTool(_ context.Context, productID, toolName string, args map[string]any) (*edition.ToolResult, error) { c.calls = append(c.calls, aitableWorkflowCall{productID: productID, toolName: toolName, args: args}) + response := c.response + if response == "" { + response = `{"status":"success","data":{"valid":true,"flowId":"flow-test","issues":[]}}` + } return &edition.ToolResult{Content: []edition.ContentBlock{{ Type: "text", - Text: `{"status":"success","data":{"valid":true,"flowId":"flow-test","issues":[]}}`, + Text: response, }}}, nil } func (*aitableWorkflowCaller) Format() string { return "json" } -func (*aitableWorkflowCaller) DryRun() bool { return false } +func (c *aitableWorkflowCaller) DryRun() bool { return c.dryRun } func (*aitableWorkflowCaller) Fields() string { return "" } func (*aitableWorkflowCaller) JQ() string { return "" } func runAitableWorkflowCommand(t *testing.T, stdin io.Reader, args ...string) (*aitableWorkflowCaller, error) { + t.Helper() + caller := &aitableWorkflowCaller{} + return caller, runAitableWorkflowCommandWithCaller(t, caller, stdin, args...) +} + +func runAitableWorkflowCommandWithCaller(t *testing.T, caller *aitableWorkflowCaller, stdin io.Reader, args ...string) error { t.Helper() testseam.Protect(t, &os.Args) - caller := &aitableWorkflowCaller{} InitDepsForTest(t, caller) deps.Out.w = io.Discard os.Args = append([]string{"dws", "aitable", "workflow"}, args...) @@ -51,6 +62,7 @@ func runAitableWorkflowCommand(t *testing.T, stdin io.Reader, args ...string) (* cmd := newAitableCommand() cmd.PersistentFlags().String("format", "json", "output format") cmd.PersistentFlags().Bool("yes", false, "skip confirmation") + cmd.PersistentFlags().Bool("dry-run", false, "preview only") cmd.SilenceErrors = true cmd.SilenceUsage = true cmd.SetArgs(append([]string{"workflow"}, args...)) @@ -58,7 +70,7 @@ func runAitableWorkflowCommand(t *testing.T, stdin io.Reader, args ...string) (* stdin = strings.NewReader("") } cmd.SetIn(stdin) - return caller, cmd.Execute() + return cmd.Execute() } func TestAitableWorkflowCreateMapsDSLWithoutRetry(t *testing.T) { @@ -92,6 +104,47 @@ func TestAitableWorkflowCreateMapsDSLWithoutRetry(t *testing.T) { } } +func TestCrossPlatformCoverageAitableWorkflowPublishRejectsFalseSuccess(t *testing.T) { + tests := []struct { + name string + response string + want string + }{ + {name: "valid false", response: `{"status":"success","data":{"valid":false,"issues":[{"message":"bad dsl"}]}}`, want: "valid=false"}, + {name: "missing valid", response: `{"status":"success","data":{"flowId":"flow-test"}}`, want: "missing valid"}, + {name: "missing flow id", response: `{"status":"success","data":{"valid":true}}`, want: "missing a non-empty flowId"}, + {name: "wrong valid type", response: `{"status":"success","data":{"valid":"true","flowId":"flow-test"}}`, want: "valid must be boolean"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + caller := &aitableWorkflowCaller{response: tc.response} + err := runAitableWorkflowCommandWithCaller(t, caller, nil, + "create", "--base-id", "base", "--dsl", `{"version":"workflow-dsl/v1","name":"test"}`) + if err == nil || !strings.Contains(err.Error(), tc.want) || len(caller.calls) != 1 { + t.Fatalf("workflow false success = err:%v calls:%#v", err, caller.calls) + } + }) + } + + t.Run("update uses the same strict contract", func(t *testing.T) { + caller := &aitableWorkflowCaller{response: `{"status":"success","data":{"valid":false}}`} + err := runAitableWorkflowCommandWithCaller(t, caller, nil, + "update", "--base-id", "base", "--workflow-id", "flow", "--dsl", `{"version":"workflow-dsl/v1","name":"test"}`) + if err == nil || !strings.Contains(err.Error(), "valid=false") || len(caller.calls) != 1 || caller.calls[0].toolName != "update_workflow" { + t.Fatalf("workflow update false success = err:%v calls:%#v", err, caller.calls) + } + }) + + t.Run("dry-run does not call publish tool", func(t *testing.T) { + caller := &aitableWorkflowCaller{dryRun: true} + err := runAitableWorkflowCommandWithCaller(t, caller, nil, + "create", "--base-id", "base", "--dsl", `{"version":"workflow-dsl/v1","name":"test"}`, "--dry-run") + if err != nil || len(caller.calls) != 0 { + t.Fatalf("workflow create dry-run = err:%v calls:%#v", err, caller.calls) + } + }) +} + func TestAitableWorkflowEditExampleMapsEmptyArguments(t *testing.T) { caller, err := runAitableWorkflowCommand(t, nil, "edit-example") if err != nil { diff --git a/internal/helpers/drive_folder_contract.go b/internal/helpers/drive_folder_contract.go index 624db528..57d90d72 100644 --- a/internal/helpers/drive_folder_contract.go +++ b/internal/helpers/drive_folder_contract.go @@ -108,7 +108,7 @@ func driveFolderPushResultSpec() *contract.ResultSpec { "preview_kind":{"type":"string","description":"预览类型","enum":["plan"]}, "operation":{"type":"string","description":"预览的命令"}, "if_exists":{"type":"string","description":"远端同名文件处理策略"}, - "plan":{"type":"object","description":"预览的推送计划","properties":{"summary":{"type":"object","description":"计划动作计数","properties":{"uploaded":{"type":"integer","description":"计划上传或覆盖的文件数"},"skipped":{"type":"integer","description":"计划跳过的文件数"},"failed":{"type":"integer","description":"预检失败的条目数"},"aborted":{"type":"boolean","description":"计划是否会中止"}},"required":["uploaded","skipped","failed","aborted"],"additionalProperties":false},"items":{"type":"array","description":"计划中的逐条目动作","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件或目录路径"},"action":{"type":"string","description":"计划动作"},"size_bytes":{"type":"integer","description":"文件字节数"},"error":{"type":"string","description":"预检失败原因"}},"required":["rel_path","action"],"additionalProperties":false}}},"required":["summary","items"],"additionalProperties":false} + "plan":{"type":"object","description":"预览的推送计划","properties":{"summary":{"type":"object","description":"计划动作计数；兼容执行计数 uploaded/skipped 在预览中保持为零","properties":{"uploaded":{"type":"integer","description":"兼容字段；预览未执行上传，固定为零"},"skipped":{"type":"integer","description":"兼容字段；预览未执行跳过，固定为零"},"failed":{"type":"integer","description":"预检失败的条目数"},"aborted":{"type":"boolean","description":"计划是否会中止"},"planned_uploads":{"type":"integer","description":"计划上传或覆盖的文件数"},"planned_skips":{"type":"integer","description":"计划按策略跳过的文件数"},"planned_folders":{"type":"integer","description":"计划创建的远端目录数"}},"required":["uploaded","skipped","failed","aborted"],"additionalProperties":false},"items":{"type":"array","description":"计划中的逐条目动作","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件或目录路径"},"action":{"type":"string","description":"未执行的 planned_* 计划动作或已确认的预检失败"},"size_bytes":{"type":"integer","description":"文件字节数"},"error":{"type":"string","description":"预检失败原因"}},"required":["rel_path","action"],"additionalProperties":false}}},"required":["summary","items"],"additionalProperties":false} }, "additionalProperties":false }`), @@ -134,7 +134,7 @@ func driveFolderSyncResultSpec() *contract.ResultSpec { "executed":{"type":"boolean","description":"是否执行了写操作","const":false}, "preview_kind":{"type":"string","description":"预览类型","enum":["plan"]}, "operation":{"type":"string","description":"预览的命令","const":"drive sync"}, - "plan":{"type":"object","description":"预览的同步计划","properties":{"detection":{"type":"string","description":"计划使用的差异检测模式","enum":["exact","quick"]},"diff":{"type":"object","description":"计划执行前的双端差异","properties":{"new_local":{"type":"array","description":"仅本地存在的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"new_remote":{"type":"array","description":"仅钉盘存在的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"modified":{"type":"array","description":"双端都存在但内容或时间不同的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"unchanged":{"type":"array","description":"双端一致的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"unknown":{"type":"array","description":"exact 模式下因缺少可靠远端哈希而无法判定的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}}},"required":["new_local","new_remote","modified","unchanged","unknown"],"additionalProperties":false},"summary":{"type":"object","description":"计划动作计数","properties":{"pulled":{"type":"integer","description":"计划拉取的条目数"},"pushed":{"type":"integer","description":"计划推送的条目数"},"skipped":{"type":"integer","description":"计划跳过的条目数"},"failed":{"type":"integer","description":"预检失败的条目数"}},"required":["pulled","pushed","skipped","failed"],"additionalProperties":false},"items":{"type":"array","description":"计划中的逐条目动作","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件或目录路径"},"action":{"type":"string","description":"计划动作"},"direction":{"type":"string","description":"同步方向"},"error":{"type":"string","description":"失败或跳过原因"}},"required":["rel_path","action"],"additionalProperties":false}}},"required":["detection","diff","summary","items"],"additionalProperties":false} + "plan":{"type":"object","description":"预览的同步计划","properties":{"detection":{"type":"string","description":"计划使用的差异检测模式","enum":["exact","quick"]},"diff":{"type":"object","description":"计划执行前的双端差异","properties":{"new_local":{"type":"array","description":"仅本地存在的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"new_remote":{"type":"array","description":"仅钉盘存在的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"modified":{"type":"array","description":"双端都存在但内容或时间不同的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"unchanged":{"type":"array","description":"双端一致的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}},"unknown":{"type":"array","description":"exact 模式下因缺少可靠远端哈希而无法判定的文件","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件路径"}},"required":["rel_path"],"additionalProperties":false}}},"required":["new_local","new_remote","modified","unchanged","unknown"],"additionalProperties":false},"summary":{"type":"object","description":"计划动作计数；兼容执行计数 pulled/pushed/skipped 在预览中保持为零","properties":{"pulled":{"type":"integer","description":"兼容字段；预览未执行拉取，固定为零"},"pushed":{"type":"integer","description":"兼容字段；预览未执行推送，固定为零"},"skipped":{"type":"integer","description":"兼容字段；预览未执行跳过，固定为零"},"failed":{"type":"integer","description":"预检失败的条目数"},"planned_pulls":{"type":"integer","description":"计划拉取的文件数"},"planned_pushes":{"type":"integer","description":"计划上传或覆盖的文件数"},"planned_skips":{"type":"integer","description":"计划按策略跳过的文件数"},"planned_folders":{"type":"integer","description":"计划创建的远端目录数"}},"required":["pulled","pushed","skipped","failed"],"additionalProperties":false},"items":{"type":"array","description":"计划中的逐条目动作","items":{"type":"object","properties":{"rel_path":{"type":"string","description":"相对文件或目录路径"},"action":{"type":"string","description":"未执行的 planned_* 计划动作、待决策动作或预检失败"},"direction":{"type":"string","description":"同步方向"},"error":{"type":"string","description":"失败或跳过原因"}},"required":["rel_path","action"],"additionalProperties":false}}},"required":["detection","diff","summary","items"],"additionalProperties":false} }, "oneOf":[ {"required":["detection","diff","summary","items"]}, diff --git a/internal/helpers/drive_folder_contract_test.go b/internal/helpers/drive_folder_contract_test.go index 6495708d..3898cb81 100644 --- a/internal/helpers/drive_folder_contract_test.go +++ b/internal/helpers/drive_folder_contract_test.go @@ -93,10 +93,26 @@ func TestCrossPlatformCoverageDriveFolderContractsPublishResultAndDryRun(t *test if name == "sync" { assertDriveSyncResultSchema(t, final.Result.DataSchema) } + if name == "push" { + assertDrivePushResultSchema(t, final.Result.DataSchema) + } }) } } +func assertDrivePushResultSchema(t *testing.T, raw json.RawMessage) { + t.Helper() + properties := resultSchemaProperties(t, raw) + plan := schemaObjectProperty(t, properties, "plan") + summary := schemaObjectProperty(t, schemaProperties(t, plan), "summary") + summaryProperties := schemaProperties(t, summary) + for _, name := range []string{"planned_uploads", "planned_skips", "planned_folders"} { + if _, ok := summaryProperties[name]; !ok { + t.Fatalf("push plan summary schema is missing %s", name) + } + } +} + func resultSchemaProperties(t *testing.T, raw json.RawMessage) map[string]any { t.Helper() var schema map[string]any @@ -133,6 +149,13 @@ func assertDriveSyncResultSchema(t *testing.T, raw json.RawMessage) { planProperties := schemaProperties(t, plan) planDiff := schemaObjectProperty(t, planProperties, "diff") assertSchemaRequired(t, planDiff, "new_local", "new_remote", "modified", "unchanged", "unknown") + planSummary := schemaObjectProperty(t, planProperties, "summary") + planSummaryProperties := schemaProperties(t, planSummary) + for _, name := range []string{"planned_pulls", "planned_pushes", "planned_skips", "planned_folders"} { + if _, ok := planSummaryProperties[name]; !ok { + t.Fatalf("sync plan summary schema is missing %s", name) + } + } var schema map[string]any if err := json.Unmarshal(raw, &schema); err != nil { diff --git a/internal/helpers/drive_push.go b/internal/helpers/drive_push.go index e5e64e25..31431440 100644 --- a/internal/helpers/drive_push.go +++ b/internal/helpers/drive_push.go @@ -39,6 +39,11 @@ const ( pushActionSkipped = "skipped" // 按 --if-exists 跳过 pushActionFolderCreated = "folder_created" // 新建远端目录（不计入 uploaded） pushActionFailed = "failed" + + pushActionPlannedUpload = "planned_upload" + pushActionPlannedOverwrite = "planned_overwrite" + pushActionPlannedSkip = "planned_skip" + pushActionPlannedFolderCreate = "planned_folder_create" ) // localPushFile 描述一个待推送的本地常规文件。 @@ -79,10 +84,13 @@ type drivePushItem struct { // drivePushSummary 是各动作的计数汇总。uploaded 同时统计新建与覆盖。 type drivePushSummary struct { - Uploaded int `json:"uploaded"` - Skipped int `json:"skipped"` - Failed int `json:"failed"` - Aborted bool `json:"aborted"` + Uploaded int `json:"uploaded"` + Skipped int `json:"skipped"` + Failed int `json:"failed"` + Aborted bool `json:"aborted"` + PlannedUploads int `json:"planned_uploads,omitempty"` + PlannedSkips int `json:"planned_skips,omitempty"` + PlannedFolders int `json:"planned_folders,omitempty"` } type drivePushResult struct { @@ -380,7 +388,8 @@ func printDrivePushDryRunWithPreflight(ifExists string, remoteFiles map[string]* continue } plannedFolders[dir] = "dry-run-planned-folder" - plan.Items = append(plan.Items, drivePushItem{RelPath: dir, Action: pushActionFolderCreated}) + plan.Summary.PlannedFolders++ + plan.Items = append(plan.Items, drivePushItem{RelPath: dir, Action: pushActionPlannedFolderCreate}) } for _, lf := range localFiles { size := lf.Size @@ -390,25 +399,25 @@ func printDrivePushDryRunWithPreflight(ifExists string, remoteFiles map[string]* plan.Items = append(plan.Items, drivePushItem{RelPath: lf.RelPath, Action: pushActionFailed, SizeBytes: &size, Error: "父目录未能创建"}) continue } - action := pushActionUploaded + action := pushActionPlannedUpload if rf, exists := remoteFiles[lf.RelPath]; exists { switch ifExists { case ifExistsSkip: - action = pushActionSkipped + action = pushActionPlannedSkip case ifExistsSmart: if rf.ModifiedTimeValid && rf.ModifiedTime >= lf.ModTimeMillis { - action = pushActionSkipped + action = pushActionPlannedSkip } else { - action = pushActionOverwritten + action = pushActionPlannedOverwrite } case ifExistsOverwrite: - action = pushActionOverwritten + action = pushActionPlannedOverwrite } } - if action == pushActionSkipped { - plan.Summary.Skipped++ + if action == pushActionPlannedSkip { + plan.Summary.PlannedSkips++ } else { - plan.Summary.Uploaded++ + plan.Summary.PlannedUploads++ } plan.Items = append(plan.Items, drivePushItem{RelPath: lf.RelPath, Action: action, SizeBytes: &size}) } @@ -811,7 +820,10 @@ func defaultPushPutOpenedFile(ctx context.Context, url string, headers map[strin if _, err := file.Seek(0, io.SeekStart); err != nil { return fmt.Errorf("failed to seek upload file: %w", err) } - req, err := http.NewRequestWithContext(ctx, http.MethodPut, url, file) + // net/http owns and closes Request.Body after RoundTrip. The *os.File belongs + // to pushUploadFilePinned, which must stat it after PUT before commit_upload; + // wrap it in a no-op closer so HTTP completion cannot close that shared handle. + req, err := http.NewRequestWithContext(ctx, http.MethodPut, url, io.NopCloser(file)) if err != nil { return fmt.Errorf("failed to create upload request: %w", err) } diff --git a/internal/helpers/drive_push_sync_final_coverage_test.go b/internal/helpers/drive_push_sync_final_coverage_test.go index 715e4362..199b4261 100644 --- a/internal/helpers/drive_push_sync_final_coverage_test.go +++ b/internal/helpers/drive_push_sync_final_coverage_test.go @@ -421,6 +421,29 @@ func TestCrossPlatformCoverageDrivePushFinalDefaultOpenedPUTAndCommit(t *testing } } +func TestCrossPlatformCoverageDrivePushOpenedPUTKeepsCallerOwnedHandle(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + _, _ = io.Copy(io.Discard, request.Body) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + path := filepath.Join(t.TempDir(), "payload") + mustWrite(t, path, "payload") + file, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + if err := defaultPushPutOpenedFile(context.Background(), server.URL, nil, file, 7); err != nil { + t.Fatalf("PUT failed: %v", err) + } + if _, err := file.Stat(); err != nil { + t.Fatalf("HTTP PUT closed its caller-owned file handle: %v", err) + } +} + func TestCrossPlatformCoverageDriveSyncFinalPreflightAndKeepBothErrors(t *testing.T) { t.Run("local target preflight lstat error and directory", func(t *testing.T) { _, root, _ := openDriveFinalCoverageRoot(t) diff --git a/internal/helpers/drive_sync.go b/internal/helpers/drive_sync.go index dad77166..2bf8b8eb 100644 --- a/internal/helpers/drive_sync.go +++ b/internal/helpers/drive_sync.go @@ -65,6 +65,14 @@ const ( syncActionRenamedLocal = "renamed_local" syncActionSkipped = "skipped" syncActionFailed = "failed" + + syncActionPlannedDownload = "planned_download" + syncActionPlannedUpload = "planned_upload" + syncActionPlannedOverwrite = "planned_overwrite" + syncActionPlannedFolderCreate = "planned_folder_create" + syncActionPlannedRenameLocal = "planned_rename_local" + syncActionPlannedSkip = "planned_skip" + syncActionDecisionRequired = "decision_required" ) // driveSyncItem 是输出 items[] 中每条操作的明细。 @@ -77,10 +85,14 @@ type driveSyncItem struct { // driveSyncSummary 是各方向的计数汇总。 type driveSyncSummary struct { - Pulled int `json:"pulled"` - Pushed int `json:"pushed"` - Skipped int `json:"skipped"` - Failed int `json:"failed"` + Pulled int `json:"pulled"` + Pushed int `json:"pushed"` + Skipped int `json:"skipped"` + Failed int `json:"failed"` + PlannedPulls int `json:"planned_pulls,omitempty"` + PlannedPushes int `json:"planned_pushes,omitempty"` + PlannedSkips int `json:"planned_skips,omitempty"` + PlannedFolders int `json:"planned_folders,omitempty"` } // driveSyncDiff 是本次同步前算出的五类差异（与 status 同源）。 @@ -267,6 +279,10 @@ func runDriveSync(cmd *cobra.Command, _ []string) error { // --dry-run：只算差异、不执行任何同步动作。 if deps.Caller.DryRun() { + if len(preflight) == 0 && res.Summary.Failed == 0 { + appendDriveSyncDryRunPlan(&res, localDirs, localByRel, remoteFiles, remoteFolders, + newLocal, newRemote, modified, unknown, onConflict) + } return deps.Out.PrintJSON(driveSyncDryRunResult{ DryRun: true, Executed: false, PreviewKind: "plan", Operation: "drive sync", Plan: res, }) @@ -394,6 +410,100 @@ func runDriveSync(cmd *cobra.Command, _ []string) error { return nil } +func appendDriveSyncDryRunPlan( + res *driveSyncResult, + localDirs []string, + localByRel map[string]localPushFile, + remoteFiles map[string]*remoteFile, + remoteFolders map[string]string, + newLocal, newRemote, modified, unknown []string, + onConflict string, +) { + for _, rel := range unknown { + res.Summary.PlannedSkips++ + res.Items = append(res.Items, driveSyncItem{ + RelPath: rel, Action: syncActionPlannedSkip, Direction: syncDirectionConflict, + Error: "远端无可靠 md5，内容无法核对，执行时会跳过（可改用 --quick 按 modified_time 比对）", + }) + } + + plannedFolders := make(map[string]string, len(remoteFolders)+len(localDirs)) + for rel, fileID := range remoteFolders { + plannedFolders[rel] = fileID + } + for _, dir := range localDirs { + if _, ok := plannedFolders[dir]; ok { + continue + } + parentRel, _ := splitRel(dir) + if plannedFolders[parentRel] == "" { + res.Summary.Failed++ + res.Items = append(res.Items, driveSyncItem{RelPath: dir, Action: syncActionFailed, Direction: syncDirectionPush, Error: "父目录未能创建"}) + continue + } + plannedFolders[dir] = "dry-run-planned-folder" + res.Summary.PlannedFolders++ + res.Items = append(res.Items, driveSyncItem{RelPath: dir, Action: syncActionPlannedFolderCreate, Direction: syncDirectionPush}) + } + + for _, rel := range newRemote { + res.Summary.PlannedPulls++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedDownload, Direction: syncDirectionPull}) + } + for _, rel := range newLocal { + parentRel, _ := splitRel(rel) + if plannedFolders[parentRel] == "" { + res.Summary.Failed++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionFailed, Direction: syncDirectionPush, Error: "父目录未能创建"}) + continue + } + res.Summary.PlannedPushes++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedUpload, Direction: syncDirectionPush}) + } + + occupied := make(map[string]bool, len(localByRel)+len(localDirs)+len(remoteFiles)+len(remoteFolders)) + for rel := range localByRel { + occupied[rel] = true + } + for _, rel := range localDirs { + occupied[rel] = true + } + for rel := range remoteFiles { + occupied[rel] = true + } + for rel := range remoteFolders { + if rel != "" { + occupied[rel] = true + } + } + for _, rel := range modified { + switch onConflict { + case syncConflictRemoteWins: + res.Summary.PlannedPulls++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedDownload, Direction: syncDirectionPull}) + case syncConflictLocalWins: + res.Summary.PlannedPushes++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedOverwrite, Direction: syncDirectionConflict}) + case syncConflictKeepBoth: + candidate := driveSyncSuffixedRel(rel, remoteFiles[rel].FileID, occupied) + occupied[candidate] = true + res.Summary.PlannedPulls++ + res.Items = append(res.Items, + driveSyncItem{RelPath: candidate, Action: syncActionPlannedRenameLocal, Direction: syncDirectionConflict}, + driveSyncItem{RelPath: rel, Action: syncActionPlannedDownload, Direction: syncDirectionPull}, + ) + case syncConflictAsk: + res.Items = append(res.Items, driveSyncItem{ + RelPath: rel, Action: syncActionDecisionRequired, Direction: syncDirectionConflict, + Error: "执行时需要交互选择 remote-wins、local-wins、keep-both 或 skip", + }) + default: + res.Summary.PlannedSkips++ + res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedSkip, Direction: syncDirectionConflict}) + } + } +} + // syncPathBlockedByTypeConflict 判断 rel 本身或任一祖先是否是文件/目录类型冲突根。 // 祖先检查保证冲突目录下的本地、远端后代不会绕过根路径的 fail-closed 结论。 func syncPathBlockedByTypeConflict(rel string, conflicts map[string]string) bool { diff --git a/internal/helpers/drive_sync_confirmation_test.go b/internal/helpers/drive_sync_confirmation_test.go index 8c22e83b..dec9633f 100644 --- a/internal/helpers/drive_sync_confirmation_test.go +++ b/internal/helpers/drive_sync_confirmation_test.go @@ -3,6 +3,7 @@ package helpers import ( "bytes" "context" + "encoding/json" "os" "path/filepath" "strings" @@ -189,6 +190,83 @@ func TestCrossPlatformCoverageDrivePush_dryRunPlansWithoutRemoteWrites(t *testin } } +func TestCrossPlatformCoverageDrivePushDryRunPublishesPlannedNotExecutedActions(t *testing.T) { + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "local.txt"), "local") + caller := syncCaller(nil) + caller.dryRun = true + + var out bytes.Buffer + testseam.Swap(t, &deps, &Deps{Caller: caller, Out: &Formatter{w: &out}}) + testseam.Swap(t, &os.Args, []string{"dws", "drive", "push"}) + cmd := findDriveSubcommand(t, "push") + mustSetFlags(t, cmd, map[string]string{"local-folder": dir, "remote-folder": "ROOT"}) + if err := cmd.RunE(cmd, nil); err != nil { + t.Fatalf("dry-run push: %v", err) + } + + var payload map[string]any + if err := json.Unmarshal(out.Bytes(), &payload); err != nil { + t.Fatalf("decode dry-run push: %v\n%s", err, out.String()) + } + plan := payload["plan"].(map[string]any) + summary := plan["summary"].(map[string]any) + if summary["uploaded"] != float64(0) || summary["planned_uploads"] != float64(1) { + t.Fatalf("dry-run summary reports execution instead of a plan: %#v", summary) + } + items := plan["items"].([]any) + if got := items[0].(map[string]any)["action"]; got != "planned_upload" { + t.Fatalf("dry-run action = %v, want planned_upload", got) + } +} + +func TestCrossPlatformCoverageDriveSyncDryRunReturnsRealActionPlan(t *testing.T) { + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "local.txt"), "local") + mustWrite(t, filepath.Join(dir, "shared.txt"), "shared") + if err := os.Mkdir(filepath.Join(dir, "empty-folder"), 0o755); err != nil { + t.Fatal(err) + } + caller := syncCaller(map[string]string{ + "ROOT": `{"result":{"items":[{"name":"remote.txt","type":"file","fileId":"R","modifyTime":2},{"name":"shared.txt","type":"file","fileId":"S","modifyTime":2}],"nextToken":""}}`, + }) + caller.dryRun = true + + var out bytes.Buffer + testseam.Swap(t, &deps, &Deps{Caller: caller, Out: &Formatter{w: &out}}) + testseam.Swap(t, &os.Args, []string{"dws", "drive", "sync"}) + cmd := findDriveSubcommand(t, "sync") + mustSetFlags(t, cmd, map[string]string{"local-folder": dir, "remote-folder": "ROOT"}) + if err := cmd.RunE(cmd, nil); err != nil { + t.Fatalf("dry-run sync: %v", err) + } + + var payload driveSyncDryRunResult + if err := json.Unmarshal(out.Bytes(), &payload); err != nil { + t.Fatalf("decode dry-run sync: %v\n%s", err, out.String()) + } + if payload.Executed || payload.Plan.Summary.Pulled != 0 || payload.Plan.Summary.Pushed != 0 || payload.Plan.Summary.Skipped != 0 { + t.Fatalf("dry-run plan summary = %+v, executed=%v", payload.Plan.Summary, payload.Executed) + } + if payload.Plan.Summary.PlannedPulls != 1 || payload.Plan.Summary.PlannedPushes != 1 || + payload.Plan.Summary.PlannedSkips != 1 || payload.Plan.Summary.PlannedFolders != 1 { + t.Fatalf("dry-run planned summary = %+v", payload.Plan.Summary) + } + actions := map[string]int{} + for _, item := range payload.Plan.Items { + actions[item.Action]++ + } + if len(payload.Plan.Items) != 4 || actions["planned_download"] != 1 || actions["planned_upload"] != 1 || + actions["planned_skip"] != 1 || actions["planned_folder_create"] != 1 { + t.Fatalf("dry-run action plan = %#v", payload.Plan.Items) + } + for _, tool := range []string{"download_file", "get_upload_info", "commit_upload", "create_folder"} { + if calls := caller.callsFor(tool); len(calls) != 0 { + t.Fatalf("dry-run called %s: %#v", tool, calls) + } + } +} + func TestCrossPlatformCoverageDrivePull_dryRunPlanBranches(t *testing.T) { root := t.TempDir() mustWrite(t, filepath.Join(root, "skip.txt"), "skip") @@ -242,7 +320,7 @@ func TestCrossPlatformCoverageDrivePush_dryRunPlanBranches(t *testing.T) { []string{"existing", "missing/child"}, files("orphan/a.txt", "skip.txt", "new.txt")); err != nil { t.Fatalf("skip plan: %v", err) } - if text := out.String(); !strings.Contains(text, pushActionFailed) || !strings.Contains(text, pushActionSkipped) || !strings.Contains(text, pushActionUploaded) { + if text := out.String(); !strings.Contains(text, pushActionFailed) || !strings.Contains(text, pushActionPlannedSkip) || !strings.Contains(text, pushActionPlannedUpload) { t.Fatalf("skip plan did not cover failed/skipped/uploaded: %s", text) } out.Reset() @@ -250,14 +328,14 @@ func TestCrossPlatformCoverageDrivePush_dryRunPlanBranches(t *testing.T) { files("smart-skip.txt", "smart-overwrite.txt")); err != nil { t.Fatalf("smart plan: %v", err) } - if text := out.String(); !strings.Contains(text, pushActionSkipped) || !strings.Contains(text, pushActionOverwritten) { + if text := out.String(); !strings.Contains(text, pushActionPlannedSkip) || !strings.Contains(text, pushActionPlannedOverwrite) { t.Fatalf("smart plan did not cover skip/overwrite: %s", text) } out.Reset() if err := printDrivePushDryRun(ifExistsOverwrite, remoteFiles, remoteFolders, nil, files("overwrite.txt")); err != nil { t.Fatalf("overwrite plan: %v", err) } - if !strings.Contains(out.String(), pushActionOverwritten) { + if !strings.Contains(out.String(), pushActionPlannedOverwrite) { t.Fatalf("overwrite plan must overwrite existing file: %s", out.String()) } } diff --git a/internal/helpers/wiki.go b/internal/helpers/wiki.go index 3c8dc9cf..8d3bac3e 100644 --- a/internal/helpers/wiki.go +++ b/internal/helpers/wiki.go @@ -10,6 +10,7 @@ import ( "time" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/corecmd/contract" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/spf13/cobra" ) @@ -557,13 +558,17 @@ func newWikiCommand() *cobra.Command { if err := validateRequiredFlags(cmd, "role"); err != nil { return err } + role := normalizePermissionRole(mustGetFlag(cmd, "role")) + if role == "OWNER" { + return apperrors.NewValidation("OWNER 角色不可通过 wiki member add 添加") + } userIds, err := collectUserIDs(cmd) if err != nil { return err } return callMCPTool("add_member", map[string]any{ "workspaceId": workspaceID, - "roleId": normalizePermissionRole(mustGetFlag(cmd, "role")), + "roleId": role, "userIds": userIds, }) }, diff --git a/internal/helpers/wiki_coverage_more_test.go b/internal/helpers/wiki_coverage_more_test.go index 9ebd7561..446008f2 100644 --- a/internal/helpers/wiki_coverage_more_test.go +++ b/internal/helpers/wiki_coverage_more_test.go @@ -1,11 +1,13 @@ package helpers import ( + stderrors "errors" "io" "os" "strings" "testing" + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/spf13/cobra" ) @@ -106,10 +108,17 @@ func TestCrossPlatformCoverageProxySubCommandCoverage(t *testing.T) { } func executeWikiEdge(t *testing.T, args ...string) error { + t.Helper() + _, err := executeWikiEdgeWithCaller(t, args...) + return err +} + +func executeWikiEdgeWithCaller(t *testing.T, args ...string) (*scriptedToolCaller, error) { t.Helper() oldDeps := deps oldArgs := os.Args - InitDeps(&scriptedToolCaller{}) + caller := &scriptedToolCaller{} + InitDeps(caller) deps.Out.w = io.Discard deps.Out.errW = io.Discard t.Cleanup(func() { @@ -125,7 +134,7 @@ func executeWikiEdge(t *testing.T, args ...string) error { root.SetIn(os.Stdin) root.SetArgs(args) os.Args = append([]string{"dws", "wiki"}, args...) - return root.Execute() + return caller, root.Execute() } func TestCrossPlatformCoverageWikiRoutingAndValidationEdges(t *testing.T) { @@ -154,6 +163,20 @@ func TestCrossPlatformCoverageWikiRoutingAndValidationEdges(t *testing.T) { } } +func TestCrossPlatformCoverageWikiMemberAddRejectsOwner(t *testing.T) { + caller, err := executeWikiEdgeWithCaller(t, "member", "add", "--workspace", "space", "--users", "u1", "--role", "OWNER") + if err == nil || !strings.Contains(strings.ToUpper(err.Error()), "OWNER") { + t.Fatalf("member add OWNER error = %v, want local OWNER rejection", err) + } + var appErr *apperrors.Error + if !stderrors.As(err, &appErr) || appErr.Category != apperrors.CategoryValidation { + t.Fatalf("member add OWNER error = %#v, want validation category", err) + } + if caller.calls != 0 { + t.Fatalf("member add OWNER made %d remote calls, want 0", caller.calls) + } +} + func TestCrossPlatformCoverageWikiDeleteCancellationEdges(t *testing.T) { oldStdin := os.Stdin t.Cleanup(func() { os.Stdin = oldStdin }) diff --git a/internal/shortcut/aitable/aitable.go b/internal/shortcut/aitable/aitable.go index ac7443da..c5c20534 100644 --- a/internal/shortcut/aitable/aitable.go +++ b/internal/shortcut/aitable/aitable.go @@ -403,11 +403,7 @@ var BaseCopy = shortcut.Shortcut{ }, Tips: []string{`dws aitable +base-copy --base-id BASE_ID --target-folder-id FOLDER_ID`}, Execute: func(rt *shortcut.RuntimeContext) error { - return rt.CallMCP("copy_base", map[string]any{ - "baseId": rt.Str("base-id"), - "targetFolderId": rt.Str("target-folder-id"), - "onlyCopyMeta": rt.Bool("only-struct"), - }) + return executeBaseCopy(rt) }, } @@ -660,7 +656,7 @@ var RecordQuery = shortcut.Shortcut{ Command: "+record-query", Product: serverMain, Description: "查询表格记录（按 ID / 条件 / 关键词，并支持字段投影和分页）", - Intent: "读取表格行数据；可按 recordId 精确取、按条件筛选、按关键词搜索，并用 fieldIds 只返回用户要求的字段以避免无关数据和 token 消耗。", + Intent: "读取表格行数据；可按 recordId 精确取、按条件筛选、按关键词搜索，并用 fieldIds 只返回用户要求的字段以避免无关数据和 token 消耗；服务端未返回的计算字段不会由 CLI 本地补算。", Risk: shortcut.RiskRead, Safety: contract.SafetySpec{ Effect: "read", Risk: "low", @@ -682,7 +678,7 @@ var RecordQuery = shortcut.Shortcut{ }, Selection: contract.SelectionSpec{ AgentSummary: "查询表格记录（按 ID / 条件 / 关键词，并支持字段投影和分页）", - UseWhen: []string{"读取表格行数据；可按 recordId 精确取、按条件筛选、按关键词搜索，并用 fieldIds 只返回用户要求的字段以避免无关数据和 token 消耗。"}, + UseWhen: []string{"读取表格行数据；可按 recordId 精确取、按条件筛选、按关键词搜索，并用 fieldIds 只返回用户要求的字段以避免无关数据和 token 消耗；服务端未返回的计算字段不会由 CLI 本地补算。"}, AvoidWhen: []string{"需要该 Shortcut 未公开的底层参数、原始响应或不同执行语义时，改用对应原子命令"}, Examples: []string{ "dws aitable +record-query --base-id B --table-id T --query \"关键词\" --limit 50", @@ -975,11 +971,7 @@ var RecordPrimaryDocGet = shortcut.Shortcut{ }, Tips: []string{`dws aitable +record-primary-doc-get --base-id B --table-id T --record-id R`}, Execute: func(rt *shortcut.RuntimeContext) error { - return rt.CallMCP("get_primary_doc", map[string]any{ - "baseId": rt.Str("base-id"), - "tableId": rt.Str("table-id"), - "recordId": rt.Str("record-id"), - }) + return executeRecordPrimaryDocGet(rt) }, } @@ -2078,7 +2070,7 @@ func workflowListProject(data map[string]any) ([]map[string]any, error) { return nil, fmt.Errorf("list_workflows response item %d must be an object, got %T", index, item) } row := map[string]any{} - if v, ok := workflowListFirst(m, "workflowId", "workflow_id", "id"); ok { + if v, ok := workflowListFirst(m, "workflowId", "flowId", "workflow_id", "id"); ok { row["workflowId"] = v } if v, ok := workflowListFirst(m, "name", "workflowName", "title"); ok { diff --git a/internal/shortcut/aitable/base_composite_e2e_test.go b/internal/shortcut/aitable/base_composite_e2e_test.go index 20c6965d..0bd20ea5 100644 --- a/internal/shortcut/aitable/base_composite_e2e_test.go +++ b/internal/shortcut/aitable/base_composite_e2e_test.go @@ -79,6 +79,87 @@ func TestCrossPlatformCoverageBaseSchemaSnapshotE2E(t *testing.T) { }) } +func TestCrossPlatformCoverageBaseCopyRequiresNewIDAndExactReadBackE2E(t *testing.T) { + t.Run("verified copy", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"data":{"fileId":"folder","type":"FOLDER"}}`}, + {text: `{"data":{"newBaseId":"new-base"}}`}, + {text: `{"data":{"baseId":"new-base","tables":[]}}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + if err != nil || !strings.Contains(out, `"newBaseId": "new-base"`) || len(caller.calls) != 3 || caller.calls[0].product != "drive" || caller.calls[0].tool != "get_file_info" || caller.calls[2].tool != "get_base" { + t.Fatalf("base copy = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("missing new ID is unknown without read-back", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"data":{"fileId":"folder","type":"folder"}}`}, + {text: `{"success":true}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || len(caller.calls) != 2 || !errors.As(err, &typed) || typed.Reason != "aitable_composite_unknown" || typed.Retryable { + t.Fatalf("missing newBaseId = output:%q err:%#v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("explicit downstream target rejection is stable and not retryable", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"data":{"fileId":"folder","type":"folder"}}`}, + {err: errors.New("Invalid target folder ID")}, + }} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || len(caller.calls) != 2 || !errors.As(err, &typed) || typed.Reason != "target_not_supported" || typed.Retryable { + t.Fatalf("rejected target = output:%q err:%#v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("rejects a non-folder target before write", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"data":{"fileId":"file-1","type":"FILE"}}`}}} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "file-1", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || len(caller.calls) != 1 || !errors.As(err, &typed) || typed.Reason != "target_wrong_type" || typed.Retryable || typed.ExecutionStarted == nil || *typed.ExecutionStarted { + t.Fatalf("wrong target type = output:%q err:%#v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("rejects target metadata without a supported type", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"data":{"fileId":"folder"}}`}}} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || len(caller.calls) != 1 || !errors.As(err, &typed) || typed.Reason != "target_not_supported" || typed.Retryable { + t.Fatalf("unsupported target metadata = output:%q err:%#v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("rejects URL target before transport", func(t *testing.T) { + caller := &upsertByKeyCaller{} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "https://example.test/folder", "--yes") + if err == nil || out != "" || len(caller.calls) != 0 { + t.Fatalf("URL target = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("rejects pure numeric dentryId before transport", func(t *testing.T) { + caller := &upsertByKeyCaller{} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "123456789", "--yes") + if err == nil || out != "" || len(caller.calls) != 0 { + t.Fatalf("numeric target = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("nodeId-only metadata cannot prove dentryUuid", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"data":{"nodeId":"folder","type":"FOLDER"}}`}}} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || len(caller.calls) != 1 || !errors.As(err, &typed) || typed.Reason != "target_not_supported" || typed.Retryable { + t.Fatalf("nodeId-only metadata = output:%q err:%#v calls:%#v", out, err, caller.calls) + } + }) +} + func bootstrapFields(count int) []any { fields := make([]any, 0, count) for index := 0; index < count; index++ { diff --git a/internal/shortcut/aitable/base_copy.go b/internal/shortcut/aitable/base_copy.go new file mode 100644 index 00000000..c37f3a1e --- /dev/null +++ b/internal/shortcut/aitable/base_copy.go @@ -0,0 +1,143 @@ +// Copyright 2026 Alibaba Group +// SPDX-License-Identifier: Apache-2.0 + +package aitable + +import ( + "fmt" + "strings" + "unicode" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" +) + +func executeBaseCopy(rt *shortcut.RuntimeContext) error { + baseID := strings.TrimSpace(rt.Str("base-id")) + targetFolderID := strings.TrimSpace(rt.Str("target-folder-id")) + if !validCompositeOpaqueID(baseID) { + return apperrors.NewValidation("--base-id 不是合法的 Base ID") + } + if !validCompositeOpaqueID(targetFolderID) || pureNumericID(targetFolderID) { + return apperrors.NewValidation("--target-folder-id 必须是文件夹 dentryUuid，不能是 URL、路径或空值") + } + params := map[string]any{ + "baseId": baseID, + "targetFolderId": targetFolderID, + "onlyCopyMeta": rt.Bool("only-struct"), + } + result := newCompositeResult("base_copy") + result.Resolved = map[string]any{"sourceBaseId": baseID, "targetFolderId": targetFolderID} + result.Plan = []compositeStep{ + {Index: 1, Name: "verify target folder", Tool: "drive/get_file_info", Status: "planned", Arguments: map[string]any{"fileId": targetFolderID}}, + {Index: 2, Name: "copy base", Tool: "copy_base", Status: "planned", Arguments: params}, + } + if rt.DryRun() { + result.Status = "planned" + result.Executed = false + return rt.Output(result) + } + + targetData, err := rt.CallMCPData("drive", "get_file_info", map[string]any{"fileId": targetFolderID}) + if err != nil { + return err + } + actualTargetID := findStringByKeys(targetData, "fileId", "dentryUuid") + if actualTargetID == "" || actualTargetID != targetFolderID { + return apperrors.NewAPI("drive/get_file_info did not prove the exact target dentryUuid", + apperrors.WithOperation("drive/get_file_info"), apperrors.WithReason("target_not_supported"), + apperrors.WithFailureStage("target_resolution"), apperrors.WithExecutionStarted(false), apperrors.WithRetryable(false)) + } + targetType := findStringByKeys(targetData, "type", "dentryType", "fileType") + if targetType == "" { + return apperrors.NewAPI("drive/get_file_info response is missing the target node type", + apperrors.WithOperation("drive/get_file_info"), apperrors.WithReason("target_not_supported"), + apperrors.WithFailureStage("response_validation"), apperrors.WithExecutionStarted(false), apperrors.WithRetryable(false)) + } + if !strings.EqualFold(targetType, "folder") { + return apperrors.NewAPI(fmt.Sprintf("target %s is %s, not a folder", targetFolderID, targetType), + apperrors.WithOperation("drive/get_file_info"), apperrors.WithReason("target_wrong_type"), + apperrors.WithFailureStage("target_resolution"), apperrors.WithExecutionStarted(false), apperrors.WithRetryable(false)) + } + result.CompletedSteps = append(result.CompletedSteps, compositeStep{Index: 1, Name: "verify target folder", Tool: "drive/get_file_info", Status: "completed", Result: map[string]any{"fileId": actualTargetID, "type": targetType}}) + + writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, "copy_base", params) + newBaseID := findStringByKeys(writeData, "newBaseId") + if newBaseID == "" && copyBaseRejectedTarget(writeErr) { + return apperrors.NewAPI("copy_base rejected a target that Drive proved is the exact folder dentryUuid", + apperrors.WithOperation("aitable/copy_base"), apperrors.WithOrigin("mcp"), + apperrors.WithReason("target_not_supported"), apperrors.WithFailureStage("copy_base"), + apperrors.WithExecutionStarted(true), apperrors.WithRetryable(false), apperrors.WithCause(writeErr), + apperrors.WithHint("the current copy_base service does not accept this Drive folder contract; retrying or substituting spaceId/nodeId/dentryId is unsafe")) + } + if newBaseID == "" || !validCompositeOpaqueID(newBaseID) { + cause := fmt.Errorf("copy_base response is missing a valid newBaseId") + if writeErr != nil { + cause = fmt.Errorf("copy_base response error: %w; newBaseId is unavailable", writeErr) + } + result.Status = "unknown" + return compositeError(result, cause, false) + } + result.KnownEffects = append(result.KnownEffects, map[string]any{"tool": "copy_base", "newBaseId": newBaseID}) + + readBack, verifyErr := rt.CallMCPData(serverMain, "get_base", map[string]any{"baseId": newBaseID}) + if verifyErr == nil { + actualID := findStringByKeys(readBack, "baseId") + if actualID == "" { + verifyErr = fmt.Errorf("get_base read-back is missing baseId") + } else if actualID != newBaseID { + verifyErr = fmt.Errorf("get_base read-back identity mismatch: got %q, want %q", actualID, newBaseID) + } + } + if verifyErr != nil { + result.Status = "partial_success" + result.Verification = map[string]any{"status": "failed", "newBaseId": newBaseID, "error": verifyErr.Error()} + if writeErr != nil { + result.Warnings = append(result.Warnings, "copy response error: "+writeErr.Error()) + } + return compositeError(result, verifyErr, false) + } + + result.CompletedCount = 2 + result.CompletedSteps = append(result.CompletedSteps, compositeStep{Index: 2, Name: "copy base", Tool: "copy_base", Status: "completed", Result: map[string]any{"newBaseId": newBaseID}}) + result.Verification = map[string]any{"status": "verified", "newBaseId": newBaseID} + result.Result = map[string]any{"newBaseId": newBaseID, "base": readBack} + if writeErr != nil { + result.Status = "recovered" + result.Warnings = append(result.Warnings, "copy response was an error, but the new Base was proven by exact read-back") + } + return rt.Output(result) +} + +func copyBaseRejectedTarget(err error) bool { + if err == nil { + return false + } + message := strings.ToLower(err.Error()) + return strings.Contains(message, "invalid target folder") || + strings.Contains(message, "invalid targetfolder") +} + +func validCompositeOpaqueID(value string) bool { + if value == "" || len(value) > 512 || strings.ContainsAny(value, "/?#") { + return false + } + for _, r := range value { + if unicode.IsSpace(r) || unicode.IsControl(r) { + return false + } + } + return true +} + +func pureNumericID(value string) bool { + if value == "" { + return false + } + for _, r := range value { + if r < '0' || r > '9' { + return false + } + } + return true +} diff --git a/internal/shortcut/aitable/primary_doc.go b/internal/shortcut/aitable/primary_doc.go new file mode 100644 index 00000000..0e2f8eab --- /dev/null +++ b/internal/shortcut/aitable/primary_doc.go @@ -0,0 +1,133 @@ +// Copyright 2026 Alibaba Group +// SPDX-License-Identifier: Apache-2.0 + +package aitable + +import ( + "fmt" + "strings" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" +) + +func executeRecordPrimaryDocGet(rt *shortcut.RuntimeContext) error { + baseID, tableID, requestedRecordID := rt.Str("base-id"), rt.Str("table-id"), rt.Str("record-id") + fieldsData, err := rt.CallMCPData(serverMain, "get_fields", map[string]any{"baseId": baseID, "tableId": tableID}) + if err != nil { + return err + } + fields, found := findNamedObjectList(fieldsData, "fields", "fieldList") + if !found { + return apperrors.NewAPI("get_fields response is missing the fields collection", + apperrors.WithOperation("aitable/get_fields"), apperrors.WithReason("target_invalid_response"), + apperrors.WithFailureStage("response_validation"), apperrors.WithExecutionStarted(false)) + } + primaryFieldIDs := make([]string, 0, 1) + for index, field := range fields { + fieldType := stringValue(field, "type", "fieldType") + if !strings.EqualFold(fieldType, "primaryDoc") { + continue + } + fieldID := stringValue(field, "fieldId", "id") + if fieldID == "" { + return apperrors.NewAPI(fmt.Sprintf("primaryDoc field %d is missing fieldId", index), + apperrors.WithOperation("aitable/get_fields"), apperrors.WithReason("target_invalid_response"), + apperrors.WithFailureStage("response_validation"), apperrors.WithExecutionStarted(false)) + } + primaryFieldIDs = append(primaryFieldIDs, fieldID) + } + + recordParams := map[string]any{ + "baseId": baseID, "tableId": tableID, "recordIds": []string{requestedRecordID}, + } + if len(primaryFieldIDs) > 0 { + recordParams["fieldIds"] = primaryFieldIDs + } + window, err := queryRecordWindow(rt, recordParams, 1) + if err != nil { + return err + } + records := window.Records + if len(records) == 0 { + return apperrors.NewAPI(fmt.Sprintf("record %s was not found", requestedRecordID), + apperrors.WithOperation("aitable/query_records"), apperrors.WithReason("RESOURCE_NOT_FOUND"), + apperrors.WithFailureStage("target_resolution"), apperrors.WithExecutionStarted(false)) + } + if len(records) != 1 || recordID(records[0]) != requestedRecordID { + return apperrors.NewAPI("record preflight did not return the exact requested record", + apperrors.WithOperation("aitable/query_records"), apperrors.WithReason("target_invalid_response"), + apperrors.WithFailureStage("response_validation"), apperrors.WithExecutionStarted(false)) + } + if len(primaryFieldIDs) == 0 { + return rt.Output(map[string]any{ + "status": "no_primary_doc_field", "exists": false, "recordId": requestedRecordID, "nodeId": nil, + }) + } + data, err := rt.CallMCPData(serverHelper, "get_primary_doc", map[string]any{ + "baseId": baseID, "tableId": tableID, "recordId": requestedRecordID, + }) + if err != nil { + if knownPrimaryDocUnassociatedError(err) { + return rt.Output(map[string]any{ + "status": "unassociated", "exists": false, "recordId": requestedRecordID, "fieldIds": primaryFieldIDs, "nodeId": nil, + }) + } + return err + } + nodeID := findStringByKeys(data, "nodeId", "dentryUuid") + if nodeID == "" { + if knownPrimaryDocUnassociatedData(data) { + return rt.Output(map[string]any{ + "status": "unassociated", "exists": false, "recordId": requestedRecordID, "fieldIds": primaryFieldIDs, "nodeId": nil, + }) + } + return apperrors.NewAPI("get_primary_doc response is missing nodeId", + apperrors.WithOperation("aitable-helper/get_primary_doc"), apperrors.WithReason("target_invalid_response"), + apperrors.WithFailureStage("response_validation"), apperrors.WithExecutionStarted(false)) + } + return rt.Output(map[string]any{ + "status": "associated", "exists": true, "recordId": requestedRecordID, "fieldIds": primaryFieldIDs, "nodeId": nodeID, + }) +} + +func knownPrimaryDocUnassociatedError(err error) bool { + message := strings.ToLower(strings.TrimSpace(err.Error())) + return strings.Contains(message, "no record") || strings.Contains(message, "unassociated") +} + +func knownPrimaryDocUnassociatedData(value any) bool { + var walk func(any) bool + walk = func(current any) bool { + switch typed := current.(type) { + case map[string]any: + if exists, ok := typed["exists"].(bool); ok && !exists { + return true + } + for _, key := range []string{"nodeId", "dentryUuid"} { + if nodeID, exists := typed[key]; exists && (nodeID == nil || strings.TrimSpace(fmt.Sprint(nodeID)) == "") { + return true + } + } + if status, ok := typed["status"].(string); ok { + status = strings.ToLower(strings.TrimSpace(status)) + if status == "unassociated" || status == "no_record" { + return true + } + } + for _, child := range typed { + if walk(child) { + return true + } + } + case []any: + for _, child := range typed { + if walk(child) { + return true + } + } + } + return false + } + return walk(value) +} diff --git a/internal/shortcut/aitable/record_batch.go b/internal/shortcut/aitable/record_batch.go index 63f7d92f..7ff35ebd 100644 --- a/internal/shortcut/aitable/record_batch.go +++ b/internal/shortcut/aitable/record_batch.go @@ -84,9 +84,43 @@ func executeRecordDeleteBatches(rt *shortcut.RuntimeContext) error { return rt.Output(result) } - for offset := 0; offset < len(ids); offset += recordBatchSize { - end := minInt(offset+recordBatchSize, len(ids)) - batch := ids[offset:end] + existingIDs, err := queryExistingRecordIDs(rt, baseID, tableID, ids) + if err != nil { + return err + } + existingSet := make(map[string]bool, len(existingIDs)) + for _, id := range existingIDs { + existingSet[id] = true + } + missingIDs := make([]string, 0, len(ids)-len(existingIDs)) + for _, id := range ids { + if !existingSet[id] { + missingIDs = append(missingIDs, id) + } + } + result.Resolved = map[string]any{ + "existingRecordIds": existingIDs, + "missingRecordIds": missingIDs, + } + result.Plan = nil + for offset := 0; offset < len(existingIDs); offset += recordBatchSize { + end := minInt(offset+recordBatchSize, len(existingIDs)) + result.Plan = append(result.Plan, compositeStep{ + Index: len(result.Plan) + 1, Name: "delete existing record batch", Tool: "delete_records", + Status: "planned", Offset: offset, Count: end - offset, + }) + } + if len(existingIDs) == 0 { + result.Status = "unchanged" + result.Executed = false + result.Verification = map[string]any{"status": "verified_absent_before_write", "missingCount": len(missingIDs)} + result.Result = map[string]any{"deletedCount": 0, "missingCount": len(missingIDs), "batchCount": 0} + return rt.Output(result) + } + + for offset := 0; offset < len(existingIDs); offset += recordBatchSize { + end := minInt(offset+recordBatchSize, len(existingIDs)) + batch := existingIDs[offset:end] writeData, writeErr := rt.CallMCPWriteDataStrict(serverMain, "delete_records", map[string]any{ "baseId": baseID, "tableId": tableID, "recordIds": batch, }) @@ -109,7 +143,7 @@ func executeRecordDeleteBatches(rt *shortcut.RuntimeContext) error { step.Status = "unknown" result.CompletedSteps = append(result.CompletedSteps, step) result.CompletedCount = offset - result.FailedCount = len(ids) - offset + result.FailedCount = len(existingIDs) - offset result.Status = "unknown" if offset > 0 { result.Status = "partial_success" @@ -130,11 +164,49 @@ func executeRecordDeleteBatches(rt *shortcut.RuntimeContext) error { result.CompletedCount = end result.KnownEffects = append(result.KnownEffects, map[string]any{"tool": "delete_records", "offset": offset, "recordIds": batch}) } - result.Verification = map[string]any{"status": "verified_absent", "verifiedCount": len(ids)} - result.Result = map[string]any{"deletedCount": len(ids), "batchCount": len(result.CompletedSteps)} + result.Verification = map[string]any{ + "status": "verified_absent", "verifiedCount": len(existingIDs), + "missingBeforeWriteCount": len(missingIDs), + } + result.Result = map[string]any{ + "deletedCount": len(existingIDs), "missingCount": len(missingIDs), + "batchCount": len(result.CompletedSteps), + } return rt.Output(result) } +func queryExistingRecordIDs(rt *shortcut.RuntimeContext, baseID, tableID string, ids []string) ([]string, error) { + records, err := queryDeletedRecordsByIDs(rt, baseID, tableID, ids) + if err != nil { + return nil, err + } + wanted := make(map[string]bool, len(ids)) + for _, id := range ids { + wanted[id] = true + } + present := make(map[string]bool, len(records)) + for index, record := range records { + id := recordID(record) + if id == "" { + return nil, fmt.Errorf("delete preflight record %d is missing recordId", index) + } + if !wanted[id] { + return nil, fmt.Errorf("delete preflight returned unexpected recordId %q", id) + } + if present[id] { + return nil, fmt.Errorf("delete preflight returned duplicate recordId %q", id) + } + present[id] = true + } + existing := make([]string, 0, len(present)) + for _, id := range ids { + if present[id] { + existing = append(existing, id) + } + } + return existing, nil +} + func parseRecordIDs(values []string) ([]string, error) { seen := map[string]bool{} ids := make([]string, 0, len(values)) diff --git a/internal/shortcut/aitable/record_coverage_gap_test.go b/internal/shortcut/aitable/record_coverage_gap_test.go index 00d89975..ed41a74d 100644 --- a/internal/shortcut/aitable/record_coverage_gap_test.go +++ b/internal/shortcut/aitable/record_coverage_gap_test.go @@ -9,6 +9,8 @@ import ( "strconv" "strings" "testing" + + apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" ) func TestCrossPlatformCoverageRecordObjectAndIDValidation(t *testing.T) { @@ -90,6 +92,67 @@ func TestCrossPlatformCoverageRecordDeleteDryRunAndDualFailureE2E(t *testing.T) } } +func TestCrossPlatformCoverageRecordPrimaryDocPreflightAndNormalizationE2E(t *testing.T) { + t.Run("record not found is classified", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"fields":[{"fieldId":"primary","type":"primaryDoc"}]}`}, + {text: `{"records":[]}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "missing") + var typed *apperrors.Error + if err == nil || out != "" || !errors.As(err, &typed) || typed.Reason != "RESOURCE_NOT_FOUND" { + t.Fatalf("missing primary-doc record = output:%q err:%#v", out, err) + } + }) + + t.Run("table without primary doc field is normalized", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"fields":[{"fieldId":"text","type":"text"}]}`}, + {text: `{"records":[{"recordId":"r1","cells":{"text":"x"}}]}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "r1") + if err != nil || !strings.Contains(out, `"status": "no_primary_doc_field"`) || !strings.Contains(out, `"exists": false`) || len(caller.calls) != 2 { + t.Fatalf("no primary field = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("empty primary doc cell is unassociated", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"fields":[{"fieldId":"primary","type":"primaryDoc"}]}`}, + {text: `{"records":[{"recordId":"r1","cells":{}}]}`}, + {text: `{"data":{"nodeId":null}}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "r1") + if err != nil || !strings.Contains(out, `"status": "unassociated"`) || !strings.Contains(out, `"exists": false`) || len(caller.calls) != 3 || caller.calls[2].tool != "get_primary_doc" { + t.Fatalf("unassociated primary doc = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("known helper no-record error is unassociated", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"fields":[{"fieldId":"primary","type":"primaryDoc"}]}`}, + {text: `{"records":[{"recordId":"r1","cells":{}}]}`}, + {err: errors.New("no record")}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "r1") + if err != nil || !strings.Contains(out, `"status": "unassociated"`) || !strings.Contains(out, `"exists": false`) || len(caller.calls) != 3 { + t.Fatalf("no-record primary doc = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("associated doc is resolved through helper", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"fields":[{"fieldId":"primary","type":"primaryDoc"}]}`}, + {text: `{"records":[{"recordId":"r1","cells":{"primary":{"associated":true}}}]}`}, + {text: `{"data":{"nodeId":"node-1"}}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "r1") + if err != nil || !strings.Contains(out, `"nodeId": "node-1"`) || !strings.Contains(out, `"exists": true`) || len(caller.calls) != 3 || caller.calls[2].tool != "get_primary_doc" { + t.Fatalf("associated primary doc = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) +} + func TestCrossPlatformCoverageRecordBatchVerificationEdgesE2E(t *testing.T) { record := []map[string]any{{"recordId": "r1", "cells": map[string]any{"f": "new"}}} diff --git a/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go b/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go index 2304cf6c..653a80b5 100644 --- a/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go +++ b/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go @@ -612,11 +612,18 @@ func TestCrossPlatformCoverageRecordQueryFailureAndContinuationBranches(t *testi }) t.Run("delete readback follows active continuation to absence", func(t *testing.T) { - caller := &upsertByKeyCaller{callFn: func(call int, _, tool string, args map[string]any) (string, error) { + written := false + verificationPage := 0 + caller := &upsertByKeyCaller{callFn: func(_ int, _, tool string, args map[string]any) (string, error) { if tool != "query_records" { + written = true return `{"deletedCount":1}`, nil } - if call == 1 { + if !written { + return `{"success":true,"data":{"records":[{"recordId":"r1"}],"hasMore":false}}`, nil + } + verificationPage++ + if verificationPage == 1 { if _, exists := args["cursor"]; exists { t.Fatalf("first deletion readback unexpectedly has cursor: %#v", args) } @@ -628,23 +635,30 @@ func TestCrossPlatformCoverageRecordQueryFailureAndContinuationBranches(t *testi return `{"success":true,"data":{"records":[],"hasMore":false}}`, nil }} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) - if err != nil || out == "" || len(caller.calls) != 3 { + if err != nil || out == "" || len(caller.calls) != 4 { t.Fatalf("delete continued absence output=%q error=%v calls=%#v", out, err, caller.calls) } }) t.Run("delete readback follows active continuation to remaining record", func(t *testing.T) { - caller := &upsertByKeyCaller{callFn: func(call int, _, tool string, _ map[string]any) (string, error) { + written := false + verificationPage := 0 + caller := &upsertByKeyCaller{callFn: func(_ int, _, tool string, _ map[string]any) (string, error) { if tool != "query_records" { + written = true return `{"deletedCount":1}`, nil } - if call == 1 { + if !written { + return `{"success":true,"data":{"records":[{"recordId":"r1"}],"hasMore":false}}`, nil + } + verificationPage++ + if verificationPage == 1 { return `{"success":true,"data":{"records":[],"hasMore":true,"nextCursor":"c1"}}`, nil } return `{"success":true,"data":{"records":[{"recordId":"r1"}],"hasMore":false}}`, nil }} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) - if err == nil || out != "" || len(caller.calls) != 3 { + if err == nil || out != "" || len(caller.calls) != 4 { t.Fatalf("delete continued remaining output=%q error=%v calls=%#v", out, err, caller.calls) } var typed *apperrors.Error @@ -654,14 +668,21 @@ func TestCrossPlatformCoverageRecordQueryFailureAndContinuationBranches(t *testi }) t.Run("delete readback fails closed on empty continuation stall", func(t *testing.T) { - caller := &upsertByKeyCaller{callFn: func(call int, _, tool string, _ map[string]any) (string, error) { + written := false + verificationPage := 0 + caller := &upsertByKeyCaller{callFn: func(_ int, _, tool string, _ map[string]any) (string, error) { if tool != "query_records" { + written = true return `{"deletedCount":1}`, nil } - return fmt.Sprintf(`{"success":true,"data":{"records":[],"hasMore":true,"nextCursor":"c%d"}}`, call), nil + if !written { + return `{"success":true,"data":{"records":[{"recordId":"r1"}],"hasMore":false}}`, nil + } + verificationPage++ + return fmt.Sprintf(`{"success":true,"data":{"records":[],"hasMore":true,"nextCursor":"c%d"}}`, verificationPage), nil }} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) - if err == nil || out != "" || len(caller.calls) != recordQueryMaxConsecutiveEmptyPages+1 { + if err == nil || out != "" || len(caller.calls) != recordQueryMaxConsecutiveEmptyPages+2 { t.Fatalf("delete stalled continuation output=%q error=%v calls=%#v", out, err, caller.calls) } var typed *apperrors.Error @@ -678,21 +699,26 @@ func TestCrossPlatformCoverageRecordWriteReadbackUsesStableServicePagesE2E(t *te records := updateFixtureRecords(0, size, "完成") caller := &upsertByKeyCaller{} queryCalls := 0 + writeSeen := false caller.callFn = func(_ int, _, tool string, args map[string]any) (string, error) { if tool != "query_records" { + writeSeen = true return `{"success":true}`, nil } queryCalls++ if operation == "delete" { ids := args["recordIds"].([]string) - empty := make([]map[string]any, len(ids)) - response := pagedRecordQueryResponse(t, empty, args) + requested := make([]map[string]any, 0, len(ids)) + if !writeSeen { + for _, id := range ids { + requested = append(requested, map[string]any{"recordId": id}) + } + } + response := pagedRecordQueryResponse(t, requested, args) var payload map[string]any if err := json.Unmarshal([]byte(response), &payload); err != nil { t.Fatal(err) } - page := payload["data"].(map[string]any) - page["records"] = []any{} raw, _ := json.Marshal(payload) return string(raw), nil } @@ -725,6 +751,9 @@ func TestCrossPlatformCoverageRecordWriteReadbackUsesStableServicePagesE2E(t *te t.Fatalf("%s size %d false negative: output=%q err=%v", operation, size, out, err) } wantQueries := (size + recordQueryServicePageSize - 1) / recordQueryServicePageSize + if operation == "delete" { + wantQueries *= 2 + } if queryCalls != wantQueries { t.Fatalf("%s size %d query calls=%d, want %d", operation, size, queryCalls, wantQueries) } @@ -860,15 +889,20 @@ func recordIDFixtures(count int) []string { func TestCrossPlatformCoverageRecordDeleteAutoChunksAndProvesAbsenceE2E(t *testing.T) { ids := recordIDFixtures(101) - caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ - {text: `{"deletedCount":100}`}, - {text: `{"data":{"records":[]}}`}, - {text: `{"data":{"records":[]}}`}, - {text: `{"data":{"records":[]}}`}, - {text: `{"data":{"records":[]}}`}, - {text: `{"data":{"records":[]}}`}, - {text: `{"deletedCount":1}`}, - {text: `{"data":{"records":[]}}`}, + writes := 0 + caller := &upsertByKeyCaller{callFn: func(_ int, _, tool string, args map[string]any) (string, error) { + if tool == "delete_records" { + writes++ + return fmt.Sprintf(`{"deletedCount":%d}`, len(args["recordIds"].([]string))), nil + } + requested := args["recordIds"].([]string) + records := make([]map[string]any, 0, len(requested)) + if writes == 0 { + for _, id := range requested { + records = append(records, map[string]any{"recordId": id}) + } + } + return recordListJSON(t, records), nil }} out, err := runRecordDeleteCLI(t, caller, ids) if err != nil { @@ -879,14 +913,43 @@ func TestCrossPlatformCoverageRecordDeleteAutoChunksAndProvesAbsenceE2E(t *testi t.Fatalf("delete output missing %s: %s", want, out) } } - if len(caller.calls) != 8 || caller.calls[0].tool != "delete_records" || caller.calls[1].tool != "query_records" || caller.calls[6].tool != "delete_records" { + if len(caller.calls) != 14 || caller.calls[0].tool != "query_records" || caller.calls[6].tool != "delete_records" || caller.calls[12].tool != "delete_records" { t.Fatalf("delete call sequence = %#v", caller.calls) } } +func TestCrossPlatformCoverageRecordDeleteDoesNotCountMissingIDsE2E(t *testing.T) { + t.Run("all requested IDs were already absent", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"success":true,"data":{"records":[],"hasMore":false}}`}}} + out, err := runRecordDeleteCLI(t, caller, []string{"missing-1", "missing-2"}) + if err != nil || len(caller.calls) != 1 || caller.calls[0].tool != "query_records" || + !strings.Contains(out, `"deletedCount": 0`) || !strings.Contains(out, `"missingCount": 2`) { + t.Fatalf("already absent delete = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("only records proven present are counted", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"success":true,"data":{"records":[{"recordId":"r1"}],"hasMore":false}}`}, + {text: `{"deletedCount":1}`}, + {text: `{"success":true,"data":{"records":[],"hasMore":false}}`}, + }} + out, err := runRecordDeleteCLI(t, caller, []string{"missing", "r1"}) + if err != nil || !strings.Contains(out, `"deletedCount": 1`) || !strings.Contains(out, `"missingCount": 1`) { + t.Fatalf("mixed delete = output:%q err:%v", out, err) + } + deleted := caller.calls[1].args["recordIds"].([]string) + if !slices.Equal(deleted, []string{"r1"}) { + t.Fatalf("delete target IDs = %#v", deleted) + } + }) +} + func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t *testing.T) { t.Run("legal empty collection proves deletion", func(t *testing.T) { - caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: ""}, {text: `{"records":[]}`}}} + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"records":[{"recordId":"r1"}]}`}, {text: ""}, {text: `{"records":[]}`}, + }} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) if err != nil || !strings.Contains(out, `"status": "recovered"`) { t.Fatalf("delete recovered = output:%q err:%v", out, err) @@ -895,6 +958,7 @@ func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t t.Run("explicit service success with empty data proves deletion", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"records":[{"recordId":"r1"}]}`}, {text: `{"deletedCount":1}`}, {text: `{"success":true,"status":"success","error":{},"data":{}}`}, }} @@ -905,7 +969,9 @@ func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t }) t.Run("missing records contract is unknown", func(t *testing.T) { - caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"deletedCount":1}`}, {text: `{"data":{}}`}}} + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"records":[{"recordId":"r1"}]}`}, {text: `{"deletedCount":1}`}, {text: `{"data":{}}`}, + }} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) if err == nil || out != "" { t.Fatalf("delete missing records = output:%q err:%v", out, err) @@ -918,6 +984,7 @@ func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t t.Run("remaining record is not success", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"records":[{"recordId":"r1"}]}`}, {text: `{"deletedCount":0}`}, {text: `{"records":[{"recordId":"r1","cells":{"fld":"still here"}}]}`}, }} @@ -930,15 +997,22 @@ func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t func TestCrossPlatformCoverageRecordDeletePartialCheckpointE2E(t *testing.T) { ids := recordIDFixtures(101) - caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ - {text: `{"deletedCount":100}`}, - {text: `{"records":[]}`}, - {text: `{"records":[]}`}, - {text: `{"records":[]}`}, - {text: `{"records":[]}`}, - {text: `{"records":[]}`}, - {text: `{"deletedCount":0}`}, - {text: `{"records":[{"recordId":"r100","cells":{}}]}`}, + writes := 0 + caller := &upsertByKeyCaller{callFn: func(_ int, _, tool string, args map[string]any) (string, error) { + if tool == "delete_records" { + writes++ + return `{"deletedCount":0}`, nil + } + requested := args["recordIds"].([]string) + records := make([]map[string]any, 0, len(requested)) + if writes == 0 { + for _, id := range requested { + records = append(records, map[string]any{"recordId": id}) + } + } else if writes == 2 { + records = append(records, map[string]any{"recordId": "r100", "cells": map[string]any{}}) + } + return recordListJSON(t, records), nil }} out, err := runRecordDeleteCLI(t, caller, ids) if err == nil || out != "" { diff --git a/internal/shortcut/aitable/remaining_coverage_gap_test.go b/internal/shortcut/aitable/remaining_coverage_gap_test.go index 2b834f68..b50d5588 100644 --- a/internal/shortcut/aitable/remaining_coverage_gap_test.go +++ b/internal/shortcut/aitable/remaining_coverage_gap_test.go @@ -77,6 +77,10 @@ func TestCrossPlatformCoverageAITableProjectionValidAndMissingIdentityShapes(t * if err != nil || len(workflows) != 1 { t.Fatalf("workflow projection = %#v, %v", workflows, err) } + workflows, err = workflowListProject(map[string]any{"list": []any{map[string]any{"flowId": "flow", "name": "real service shape"}}}) + if err != nil || len(workflows) != 1 || workflows[0]["workflowId"] != "flow" { + t.Fatalf("workflow flowId projection = %#v, %v", workflows, err) + } } func TestCrossPlatformCoverageCompositeAndReviewedContractFallbacks(t *testing.T) { @@ -183,6 +187,7 @@ func TestCrossPlatformCoverageViewPresetValidationAndDryRunE2E(t *testing.T) { } func TestCrossPlatformCoverageViewPresetReadBackFailuresAndUpdateE2E(t *testing.T) { + disableViewPresetSleep(t) preflight := `{"views":[{"viewId":"v","viewName":"X","viewType":"Grid","config":{"f":0}}]}` cases := []struct { name string diff --git a/internal/shortcut/aitable/view_preset.go b/internal/shortcut/aitable/view_preset.go index 6a16aebc..66ea741f 100644 --- a/internal/shortcut/aitable/view_preset.go +++ b/internal/shortcut/aitable/view_preset.go @@ -7,12 +7,17 @@ import ( "fmt" "reflect" "strings" + "time" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/corecmd/contract" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" ) +const viewPresetReadbackAttempts = 8 + +var viewPresetSleep = time.Sleep + var ViewPresetApply = shortcut.Shortcut{ Service: "aitable", Command: "+view-preset-apply", @@ -100,23 +105,39 @@ func executeViewPresetApply(rt *shortcut.RuntimeContext) error { if action == "create" { viewID = findStringByKeys(writeData, "viewId") } - readBack, verifyErr := rt.CallMCPData(serverMain, "get_views", map[string]any{"baseId": baseID, "tableId": tableID}) - verifiedViews, found := findNamedObjectList(readBack, "views", "viewList") - if verifyErr == nil && !found { - verifyErr = fmt.Errorf("get_views read-back is missing the views collection") - } - verifiedMatches := viewsByExactName(verifiedViews, name) - if verifyErr == nil && len(verifiedMatches) != 1 { - verifyErr = fmt.Errorf("view read-back matched %d exact-name views, want 1", len(verifiedMatches)) - } - if verifyErr == nil { - actualID := stringValue(verifiedMatches[0], "viewId", "id") - if actualID == "" || (viewID != "" && actualID != viewID) { - verifyErr = fmt.Errorf("view read-back identity mismatch: got %q, response %q", actualID, viewID) - } else if !presetViewMatches(verifiedMatches[0], viewType, config) { - verifyErr = fmt.Errorf("view read-back does not contain the declared type/config") - } else { - viewID = actualID + var verifiedMatches []map[string]any + var verifyErr error + for attempt := 0; attempt < viewPresetReadbackAttempts; attempt++ { + if attempt > 0 { + backoff := time.Duration(1<<(attempt-1)) * time.Second + if backoff > 12*time.Second { + backoff = 12 * time.Second + } + viewPresetSleep(backoff) + } + readBack, readErr := rt.CallMCPData(serverMain, "get_views", map[string]any{"baseId": baseID, "tableId": tableID}) + verifyErr = readErr + verifiedViews, found := findNamedObjectList(readBack, "views", "viewList") + if verifyErr == nil && !found { + verifyErr = fmt.Errorf("get_views read-back is missing the views collection") + } + verifiedMatches = viewsByExactName(verifiedViews, name) + if verifyErr == nil && len(verifiedMatches) != 1 { + verifyErr = fmt.Errorf("view read-back matched %d exact-name views, want 1", len(verifiedMatches)) + if len(verifiedMatches) > 1 { + break + } + } + if verifyErr == nil { + actualID := stringValue(verifiedMatches[0], "viewId", "id") + if actualID == "" || (viewID != "" && actualID != viewID) { + verifyErr = fmt.Errorf("view read-back identity mismatch: got %q, response %q", actualID, viewID) + } else if !presetViewMatches(verifiedMatches[0], viewType, config) { + verifyErr = fmt.Errorf("view read-back does not contain the declared type/config") + } else { + viewID = actualID + break + } } } if verifyErr != nil { @@ -153,8 +174,71 @@ func presetViewMatches(view map[string]any, viewType string, config map[string]a if actualType != "" && actualType != viewType { return false } - actualConfig, ok := view["config"].(map[string]any) - return ok && mapContains(actualConfig, config) + actualConfig := make(map[string]any, len(config)) + if nested, ok := view["config"].(map[string]any); ok { + for key, value := range nested { + actualConfig[key] = value + } + } + for key := range config { + if value, ok := view[key]; ok { + actualConfig[key] = value + } + } + if _, wanted := config["visibleFieldIds"]; wanted { + if visible, ok := projectedVisibleFieldIDs(view); ok { + actualConfig["visibleFieldIds"] = visible + } + } + return mapContains(actualConfig, config) +} + +// get_views projects visible fields as columns plus hiddenFields instead of +// echoing create_view's config.visibleFieldIds input. Different deployments +// return hiddenFields either as a fieldId-keyed object or a parallel array. +func projectedVisibleFieldIDs(view map[string]any) ([]any, bool) { + columns, columnsOK := view["columns"].([]any) + custom, customOK := view["custom"].(map[string]any) + if !columnsOK || !customOK { + return nil, false + } + if hidden, ok := custom["hiddenFields"].(map[string]any); ok { + visible := make([]any, 0, len(columns)) + for _, column := range columns { + fieldID, fieldOK := column.(string) + if !fieldOK { + return nil, false + } + isHidden, exists := hidden[fieldID] + if !exists { + return nil, false + } + hiddenFlag, hiddenTypeOK := isHidden.(bool) + if !hiddenTypeOK { + return nil, false + } + if !hiddenFlag { + visible = append(visible, fieldID) + } + } + return visible, true + } + hidden, hiddenOK := custom["hiddenFields"].([]any) + if !hiddenOK || len(columns) != len(hidden) { + return nil, false + } + visible := make([]any, 0, len(columns)) + for index, column := range columns { + fieldID, fieldOK := column.(string) + isHidden, hiddenTypeOK := hidden[index].(bool) + if !fieldOK || !hiddenTypeOK { + return nil, false + } + if !isHidden { + visible = append(visible, fieldID) + } + } + return visible, true } func mapContains(actual, expected map[string]any) bool { diff --git a/internal/shortcut/aitable/view_workflow_composite_e2e_test.go b/internal/shortcut/aitable/view_workflow_composite_e2e_test.go index ee437148..450964cd 100644 --- a/internal/shortcut/aitable/view_workflow_composite_e2e_test.go +++ b/internal/shortcut/aitable/view_workflow_composite_e2e_test.go @@ -7,13 +7,21 @@ import ( "errors" "strings" "testing" + "time" apperrors "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/errors" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" ) const workflowDSLFixture = `{"version":"workflow-dsl/v1","name":"提醒","nodes":[]}` +func disableViewPresetSleep(t *testing.T) { + t.Helper() + testseam.Swap(t, &viewPresetSleep, func(time.Duration) {}) +} + func TestCrossPlatformCoverageViewPresetCreateUpdateAndVerificationE2E(t *testing.T) { + disableViewPresetSleep(t) t.Run("create and verify exact config", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ {text: `{"views":[]}`}, @@ -30,6 +38,32 @@ func TestCrossPlatformCoverageViewPresetCreateUpdateAndVerificationE2E(t *testin } }) + t.Run("projected columns and hidden flags verify visible field config", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"views":[]}`}, + {text: `{"data":{"viewId":"v1"}}`}, + {text: `{"views":[{"viewId":"v1","viewName":"投影","viewType":"Grid","columns":["f1","f2","f3"],"custom":{"hiddenFields":[false,true,false]}}]}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", + "--base-id", "base", "--table-id", "table", "--name", "投影", "--view-type", "Grid", "--config", `{"visibleFieldIds":["f1","f3"]}`, "--yes") + if err != nil || !strings.Contains(out, `"status": "verified"`) { + t.Fatalf("projected view preset = output:%q err:%v", out, err) + } + }) + + t.Run("projected columns and keyed hidden flags verify visible field config", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"views":[]}`}, + {text: `{"data":{"viewId":"v1"}}`}, + {text: `{"views":[{"viewId":"v1","viewName":"投影对象","viewType":"Grid","columns":["f1","f2","f3"],"custom":{"hiddenFields":{"f1":true,"f2":false,"f3":false}}}]}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", + "--base-id", "base", "--table-id", "table", "--name", "投影对象", "--view-type", "Grid", "--config", `{"visibleFieldIds":["f2","f3"]}`, "--yes") + if err != nil || !strings.Contains(out, `"status": "verified"`) { + t.Fatalf("keyed projected view preset = output:%q err:%v", out, err) + } + }) + t.Run("unchanged preset makes no write", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"views":[{"viewId":"v1","viewName":"待处理","viewType":"Grid","config":{"visibleFieldIds":["f1"]}}]}`}}} out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", @@ -52,7 +86,26 @@ func TestCrossPlatformCoverageViewPresetCreateUpdateAndVerificationE2E(t *testin }) } +func TestCrossPlatformCoverageViewPresetRetriesUntilTerminalStateE2E(t *testing.T) { + disableViewPresetSleep(t) + steps := []upsertByKeyStep{ + {text: `{"views":[]}`}, + {text: `{"data":{"viewId":"v1"}}`}, + } + for attempt := 0; attempt < viewPresetReadbackAttempts-1; attempt++ { + steps = append(steps, upsertByKeyStep{text: `{"views":[]}`}) + } + steps = append(steps, upsertByKeyStep{text: `{"views":[{"viewId":"v1","viewName":"X","viewType":"Grid","config":{"visibleFieldIds":["f1"]}}]}`}) + caller := &upsertByKeyCaller{steps: steps} + out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", + "--base-id", "base", "--table-id", "table", "--name", "X", "--view-type", "Grid", "--config", `{"visibleFieldIds":["f1"]}`, "--yes") + if err != nil || !strings.Contains(out, `"status": "verified"`) || len(caller.calls) != viewPresetReadbackAttempts+2 { + t.Fatalf("eventual view preset = output:%q err:%v calls:%#v", out, err, caller.calls) + } +} + func TestCrossPlatformCoverageViewPresetAmbiguousOrMismatchedIsNotSuccessE2E(t *testing.T) { + disableViewPresetSleep(t) t.Run("duplicate name stops before write", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"views":[{"viewId":"v1","viewName":"X"},{"viewId":"v2","viewName":"X"}]}`}}} out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", diff --git a/internal/shortcut/aitabletarget/resolver.go b/internal/shortcut/aitabletarget/resolver.go index 25baf098..882d4e3f 100644 --- a/internal/shortcut/aitabletarget/resolver.go +++ b/internal/shortcut/aitabletarget/resolver.go @@ -219,6 +219,9 @@ func ResolveBaseName(reader Reader, name string, allowFuzzy bool) (Resolution, e all = append(all, Candidate{ID: id, Name: candidateName}) } next, hasMore, hasMoreKnown := pagination(data) + if hasMoreKnown && !hasMore { + return selectCandidate("base", query, dedupe(all), allowFuzzy) + } if next == "" { if hasMoreKnown && hasMore { return Resolution{}, incomplete("base", query, all, "search_bases 声明有后续页但没有 cursor") diff --git a/internal/shortcut/aitabletarget/resolver_test.go b/internal/shortcut/aitabletarget/resolver_test.go index 8a011a2a..68c989b5 100644 --- a/internal/shortcut/aitabletarget/resolver_test.go +++ b/internal/shortcut/aitabletarget/resolver_test.go @@ -125,6 +125,24 @@ func TestCrossPlatformCoverageResolveBaseNameExactPaginationE2E(t *testing.T) { } } +func TestCrossPlatformCoverageResolveBaseStopsOnAuthoritativeTerminalCursor(t *testing.T) { + reader := &resolverReader{steps: []resolverStep{ + {data: map[string]any{"bases": []any{}, "hasMore": true, "nextCursor": "next"}}, + {data: map[string]any{ + "bases": []any{map[string]any{"baseId": "b1", "baseName": "项目管理"}}, + "hasMore": false, + "nextCursor": "next", + }}, + }} + got, err := ResolveBaseName(reader, "项目管理", false) + if err != nil || got.Selected.ID != "b1" { + t.Fatalf("terminal cursor resolution = %#v, %v", got, err) + } + if len(reader.calls) != 2 { + t.Fatalf("terminal hasMore=false made %d calls, want 2", len(reader.calls)) + } +} + func TestCrossPlatformCoverageResolveNameFuzzyIsExplicitAndAmbiguityFails(t *testing.T) { data := map[string]any{"bases": []any{ map[string]any{"baseId": "b1", "baseName": "项目管理归档"}, diff --git a/internal/shortcut/contact/contact.go b/internal/shortcut/contact/contact.go index df7ad96e..d84b2819 100644 --- a/internal/shortcut/contact/contact.go +++ b/internal/shortcut/contact/contact.go @@ -162,8 +162,13 @@ var SearchUser = shortcut.Shortcut{ // the clean output projection applied to every list command. // Field names are probed defensively across candidate keys. func searchUserProject(data map[string]any) []map[string]any { - raw, ok := data["result"].([]any) - if !ok { + var raw []any + switch result := data["result"].(type) { + case []any: + raw = result + case map[string]any: + raw = []any{result} + default: return []map[string]any{} } out := make([]map[string]any, 0, len(raw)) diff --git a/internal/shortcut/contact/search_mobile_coverage_test.go b/internal/shortcut/contact/search_mobile_coverage_test.go new file mode 100644 index 00000000..fcadbb2a --- /dev/null +++ b/internal/shortcut/contact/search_mobile_coverage_test.go @@ -0,0 +1,33 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 + +package contact + +import "testing" + +func TestCrossPlatformCoverageSearchMobileProjectsObjectEnvelope(t *testing.T) { + users := searchUserProject(map[string]any{ + "success": true, + "result": map[string]any{ + "name": "Alice", + "userId": "u1", + "flowerName": "A", + "openDingTalkId": "open-1", + "title": "Engineer", + "unrelatedPrivate": "drop-me", + }, + }) + if len(users) != 1 || users[0]["userId"] != "u1" || users[0]["name"] != "Alice" { + t.Fatalf("object envelope projection = %#v, want one user", users) + } + if _, exists := users[0]["unrelatedPrivate"]; exists { + t.Fatalf("projection leaked unrelated field: %#v", users[0]) + } +} + +func TestCrossPlatformCoverageSearchMobilePreservesEmptyObjectSemantics(t *testing.T) { + users := searchUserProject(map[string]any{"success": true, "result": map[string]any{}}) + if len(users) != 0 { + t.Fatalf("empty result projected users: %#v", users) + } +} diff --git a/internal/shortcut/minutesdata/workflow.go b/internal/shortcut/minutesdata/workflow.go index 4ffc9a83..236f77b7 100644 --- a/internal/shortcut/minutesdata/workflow.go +++ b/internal/shortcut/minutesdata/workflow.go @@ -33,7 +33,8 @@ func RequireWriteAcknowledgement(operation string, data map[string]any) error { // RequirePermissionMutationAcknowledgement validates the target-level payload // returned by add/remove_member_permission. The backend's top-level // success=true is not sufficient: historically it also accompanied a -// resultMap that merely echoed nonexistent task UUIDs. +// resultMap that merely echoed nonexistent task UUIDs, and it does not prove +// that every requested task/member pair was changed. func RequirePermissionMutationAcknowledgement(operation string, taskUUIDs, memberUIDs []string, data map[string]any) error { if err := validateEnvelope(data); err != nil { return err diff --git a/internal/shortcut/minutesdata/workflow_test.go b/internal/shortcut/minutesdata/workflow_test.go index b0bccb58..e566b645 100644 --- a/internal/shortcut/minutesdata/workflow_test.go +++ b/internal/shortcut/minutesdata/workflow_test.go @@ -26,6 +26,26 @@ func TestCrossPlatformCoverageMinutesWorkflowShapesRejectUnknownE2E(t *testing.T } } +func TestCrossPlatformCoverageMinutesPermissionMutationRequiresExactResultMap(t *testing.T) { + for _, data := range []map[string]any{ + {"success": true, "result": map[string]any{}}, + {"success": true, "result": map[string]any{"resultMap": map[string]any{}}}, + {"success": true, "result": map[string]any{"resultMap": map[string]any{"u1": []any{}}}}, + {"success": true, "result": map[string]any{"resultMap": map[string]any{"other": []any{"m1"}}}}, + } { + if err := RequirePermissionMutationAcknowledgement("unshare", []string{"u1"}, []string{"m1"}, data); err == nil { + t.Fatalf("incomplete permission acknowledgement accepted: %#v", data) + } + } + valid := map[string]any{ + "success": true, + "result": map[string]any{"resultMap": map[string]any{"u1": []any{"m1"}}}, + } + if err := RequirePermissionMutationAcknowledgement("unshare", []string{"u1"}, []string{"m1"}, valid); err != nil { + t.Fatalf("valid permission acknowledgement rejected: %v", err) + } +} + func TestCrossPlatformCoverageMinutesWorkflowObservedShapesE2E(t *testing.T) { record, err := RecordResult("pause", "u1", map[string]any{"success": true, "result": map[string]any{"cmd": "pause", "uuid": "u1"}}) if err != nil || record["cmd"] != "pause" { diff --git a/internal/shortcut/todo/page_size_coverage_test.go b/internal/shortcut/todo/page_size_coverage_test.go new file mode 100644 index 00000000..c2deb5ba --- /dev/null +++ b/internal/shortcut/todo/page_size_coverage_test.go @@ -0,0 +1,51 @@ +// Copyright 2026 Alibaba Group +// Licensed under the Apache License, Version 2.0 + +package todo + +import ( + "context" + "io" + "strings" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/helpers" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/pkg/edition" + "github.com/spf13/cobra" +) + +type todoPageSizeCaller struct { + calls int +} + +func (c *todoPageSizeCaller) CallTool(context.Context, string, string, map[string]any) (*edition.ToolResult, error) { + c.calls++ + return &edition.ToolResult{Content: []edition.ContentBlock{{Type: "text", Text: `{"success":true,"result":{"todoCards":[]}}`}}}, nil +} + +func (*todoPageSizeCaller) Format() string { return "json" } +func (*todoPageSizeCaller) DryRun() bool { return false } +func (*todoPageSizeCaller) Fields() string { return "" } +func (*todoPageSizeCaller) JQ() string { return "" } + +func TestCrossPlatformCoverageGetMyTasksRejectsUnsupportedPageSize(t *testing.T) { + caller := &todoPageSizeCaller{} + helpers.InitDepsForTest(t, caller) + root := &cobra.Command{Use: "dws", SilenceErrors: true, SilenceUsage: true} + root.PersistentFlags().Bool("yes", false, "") + root.PersistentFlags().Bool("dry-run", false, "") + root.PersistentFlags().String("format", "json", "") + root.SetOut(io.Discard) + root.SetErr(io.Discard) + root.AddCommand(shortcut.Commands()...) + root.SetArgs([]string{"todo", "+get-my-tasks", "--size", "21"}) + + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "--size") { + t.Fatalf("size 21 error = %v, want local --size validation", err) + } + if caller.calls != 0 { + t.Fatalf("size 21 made %d remote calls, want 0", caller.calls) + } +} diff --git a/internal/shortcut/wiki/common.go b/internal/shortcut/wiki/common.go index ab57746d..94603853 100644 --- a/internal/shortcut/wiki/common.go +++ b/internal/shortcut/wiki/common.go @@ -154,6 +154,30 @@ func projectWikiRows(items []any, aliases map[string][]string) []map[string]any return rows } +func projectWikiRowsPreservingSource(items []any, aliases map[string][]string) []map[string]any { + rows := make([]map[string]any, 0, len(items)) + for _, item := range items { + source := item.(map[string]any) + row := make(map[string]any, len(source)+len(aliases)) + for key, value := range source { + row[key] = value + } + for canonical, candidates := range aliases { + if value, ok := row[canonical]; ok && value != nil { + continue + } + for _, candidate := range candidates { + if value, ok := source[candidate]; ok && value != nil { + row[canonical] = value + break + } + } + } + rows = append(rows, row) + } + return rows +} + func addWikiPagination(out, page map[string]any) { for _, pair := range [][2]string{{"nextCursor", "nextCursor"}, {"nextToken", "nextCursor"}, {"nextPageToken", "nextCursor"}, {"pageToken", "nextCursor"}, {"hasMore", "hasMore"}, {"truncated", "truncated"}, {"totalCount", "totalCount"}, {"autoPageComplete", "autoPageComplete"}, {"autoPageStopReason", "autoPageStopReason"}, {"pagesFetched", "pagesFetched"}} { if value, ok := page[pair[0]]; ok && value != nil { diff --git a/internal/shortcut/wiki/nodes.go b/internal/shortcut/wiki/nodes.go index aa893c87..7011e00a 100644 --- a/internal/shortcut/wiki/nodes.go +++ b/internal/shortcut/wiki/nodes.go @@ -237,7 +237,7 @@ var FeedList = readShortcut("+feed-list", "严格分页列出知识库动态", " if err != nil { return err } - feeds := projectWikiRows(items, map[string][]string{"id": {"id", "feedId"}, "type": {"type", "feedType", "action"}, "time": {"time", "createTime"}, "name": {"name", "title", "fileName"}, "nodeId": {"nodeId", "fileId"}}) + feeds := projectWikiRowsPreservingSource(items, map[string][]string{"id": {"id", "feedId"}, "type": {"type", "feedType", "action"}, "time": {"time", "createTime"}, "name": {"name", "title", "fileName"}, "nodeId": {"nodeId", "fileId"}}) out := map[string]any{"count": len(feeds), "feeds": feeds} addWikiPagination(out, page) return rt.Output(out) diff --git a/internal/shortcut/wiki/workflow_coverage_test.go b/internal/shortcut/wiki/workflow_coverage_test.go index 457ceec9..b66490cc 100644 --- a/internal/shortcut/wiki/workflow_coverage_test.go +++ b/internal/shortcut/wiki/workflow_coverage_test.go @@ -294,6 +294,29 @@ func TestCrossPlatformCoverageWikiReadWorkflows(t *testing.T) { }) } +func TestCrossPlatformCoverageWikiFeedListPreservesBusinessPayload(t *testing.T) { + caller := &wikiCoverageCaller{responses: map[string][]string{ + "wiki/list_workspace_feeds": {`{"success":true,"feeds":[{"type":1,"time":1750067400000,"content":{"doc":{"name":"Important","docKey":"dk1"}},"users":[{"nick":"Alice","userId":"u1"}],"futureField":{"value":"keep"}}],"hasMore":false}`}, + }} + out, err := runWikiCoverageCLI(t, caller, "+feed-list", "--workspace", "w") + if err != nil { + t.Fatal(err) + } + feeds := out["feeds"].([]any) + feed := feeds[0].(map[string]any) + content, contentOK := feed["content"].(map[string]any) + users, usersOK := feed["users"].([]any) + if !contentOK || content["doc"].(map[string]any)["name"] != "Important" { + t.Fatalf("content was lost or changed: %#v", feed) + } + if !usersOK || users[0].(map[string]any)["nick"] != "Alice" { + t.Fatalf("users were lost or changed: %#v", feed) + } + if _, ok := feed["futureField"]; !ok { + t.Fatalf("unknown feed business fields must be preserved: %#v", feed) + } +} + func TestCrossPlatformCoverageWikiWriteWorkflows(t *testing.T) { t.Run("confirmation gates precede every remote call", func(t *testing.T) { for _, tc := range []struct { From ae1edefee6cf5514ed48509b1eb204d5bee89a05 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 19 Aug 2026 12:17:08 +0800 Subject: [PATCH 2/5] test: cover shortcut hardening branches --- .../1050-shortcut-functional-hardening.md | 5 + .../helpers/aitable_command_edges_test.go | 107 ++++++++++++++++++ .../helpers/aitable_workflow_write_test.go | 22 ++++ .../helpers/drive_sync_confirmation_test.go | 30 +++++ .../aitable/base_composite_e2e_test.go | 67 +++++++++++ internal/shortcut/aitable/base_copy.go | 7 -- .../aitable/record_coverage_gap_test.go | 57 ++++++++++ .../aitable/remaining_coverage_gap_test.go | 34 ++++++ 8 files changed, 322 insertions(+), 7 deletions(-) create mode 100644 .changes/1050-shortcut-functional-hardening.md diff --git a/.changes/1050-shortcut-functional-hardening.md b/.changes/1050-shortcut-functional-hardening.md new file mode 100644 index 00000000..1d28f673 --- /dev/null +++ b/.changes/1050-shortcut-functional-hardening.md @@ -0,0 +1,5 @@ +--- +category: Fixed +--- + +- **Shortcut functional workflows** (#1050) — fixes truthful Drive push/sync previews, strict AITable write verification and deletion accounting, lossless Wiki feeds, and false-success handling across task, Contact, Minutes, and Wiki operations. diff --git a/internal/helpers/aitable_command_edges_test.go b/internal/helpers/aitable_command_edges_test.go index 262b0f09..367b133b 100644 --- a/internal/helpers/aitable_command_edges_test.go +++ b/internal/helpers/aitable_command_edges_test.go @@ -470,3 +470,110 @@ func TestCrossPlatformCoverageAitableViewFilterValidationAndReadBack(t *testing. } }) } + +func TestCrossPlatformCoverageAitableViewFilterFailureAndShapeEdges(t *testing.T) { + testseam.Swap(t, &aitableViewFilterReadbackSleep, func(time.Duration) {}) + testseam.Protect(t, &os.Args) + os.Args = []string{"dws", "aitable"} + fields := `{"data":{"fields":[{"fieldId":"fldA","type":"text"}]}}` + filter := `[{"operator":"eq","operands":["fldA","x"]}]` + args := []string{"view", "update", "filter", "--base-id=b", "--table-id=t", "--view-id=view", "--json=" + filter} + + t.Run("update transport error", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{fields}, errors: []error{nil, context.Canceled}} + if err := runAitableCoverageCommand(t, caller, args...); err == nil || !strings.Contains(err.Error(), context.Canceled.Error()) || len(caller.calls) != 2 { + t.Fatalf("update error = %v, calls=%#v", err, caller.calls) + } + }) + + t.Run("readback transport errors exhaust", func(t *testing.T) { + errs := []error{nil, nil} + for range aitableViewFilterReadbackAttempts { + errs = append(errs, context.DeadlineExceeded) + } + caller := &aitableTestCaller{responses: []string{fields, `{"success":true}`}, errors: errs} + if err := runAitableCoverageCommand(t, caller, args...); err == nil || len(caller.calls) != 2+aitableViewFilterReadbackAttempts { + t.Fatalf("readback errors = %v, calls=%d", err, len(caller.calls)) + } + }) + + t.Run("readback wrong identity exhausts", func(t *testing.T) { + responses := []string{fields, `{"success":true}`} + for range aitableViewFilterReadbackAttempts { + responses = append(responses, `{"data":{"views":[{"viewId":"other","filter":[]}]}}`) + } + caller := &aitableTestCaller{responses: responses} + if err := runAitableCoverageCommand(t, caller, args...); err == nil || !strings.Contains(err.Error(), "returned viewId") { + t.Fatalf("wrong readback identity = %v", err) + } + }) + + loadCases := []struct { + name string + response string + callErr error + wantError string + }{ + {name: "transport", callErr: context.Canceled, wantError: "context canceled"}, + {name: "invalid json", response: `{`, wantError: "not valid JSON"}, + {name: "missing collection", response: `{}`, wantError: "missing the fields collection"}, + {name: "missing identity", response: `{"fields":[{"type":"text"}]}`, wantError: "missing fieldId or type"}, + } + for _, tc := range loadCases { + t.Run("load fields "+tc.name, func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{tc.response}, errors: []error{tc.callErr}} + installAitableDeps(t, caller) + if _, err := loadAitableFieldTypes(context.Background(), "b", "t"); err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Fatalf("load fields error = %v, want %q", err, tc.wantError) + } + }) + } + t.Run("load legacy field keys", func(t *testing.T) { + caller := &aitableTestCaller{responses: []string{`{"fieldList":[{"id":"legacy","fieldType":"text"}]}`}} + installAitableDeps(t, caller) + got, err := loadAitableFieldTypes(context.Background(), "b", "t") + if err != nil || got["legacy"] != "text" { + t.Fatalf("legacy field keys = %#v, %v", got, err) + } + }) + + if _, ok := findAitableObjectList(map[string]any{"fields": "bad"}, "fields"); ok { + t.Fatal("scalar fields collection must fail") + } + if _, ok := findAitableObjectList(map[string]any{"fields": []any{"bad"}}, "fields"); ok { + t.Fatal("scalar field item must fail") + } + if got, ok := findAitableObjectList([]any{"skip", map[string]any{"nested": map[string]any{"fields": []any{map[string]any{"fieldId": "f"}}}}}, "fields"); !ok || len(got) != 1 { + t.Fatalf("recursive fields = %#v, %v", got, ok) + } + + invalidFilters := []struct { + filter []any + want string + }{ + {filter: []any{"bad"}, want: "must be an object"}, + {filter: []any{map[string]any{"operator": "bogus", "operands": []any{}}}, want: "unsupported operator"}, + {filter: []any{map[string]any{"operator": "eq", "operands": "bad"}}, want: "requires an operands array"}, + {filter: []any{map[string]any{"operator": "and", "operands": []any{}}}, want: "logical operator"}, + {filter: []any{map[string]any{"operator": "exist", "operands": []any{"f", "extra"}}}, want: "requires 1 operands"}, + {filter: []any{map[string]any{"operator": "eq", "operands": []any{1, "x"}}}, want: "requires a fieldId"}, + {filter: []any{map[string]any{"operator": "any_of", "operands": []any{"multi", 1}}}, want: "one option-name string"}, + } + for _, tc := range invalidFilters { + if err := validateAitableViewFilter(tc.filter, map[string]string{"f": "text", "multi": "multipleSelect"}); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("validate filter %#v = %v, want %q", tc.filter, err, tc.want) + } + } + if _, err := expandAitableMultiSelectAnyOf("multi", nil); err == nil { + t.Fatal("empty any_of array must fail") + } + if _, err := expandAitableMultiSelectAnyOf("multi", []any{"ok", 1}); err == nil { + t.Fatal("non-string any_of option must fail") + } + if got := compactJSON(make(chan int)); !strings.HasPrefix(got, "(chan int)") { + t.Fatalf("compactJSON fallback = %q", got) + } + if persistedViewFilterMatches("bad", nil) || persistedViewFilterMatches(map[string]any{"operator": "or"}, nil) { + t.Fatal("invalid persisted wrapper must not match") + } +} diff --git a/internal/helpers/aitable_workflow_write_test.go b/internal/helpers/aitable_workflow_write_test.go index db95925a..562e1a08 100644 --- a/internal/helpers/aitable_workflow_write_test.go +++ b/internal/helpers/aitable_workflow_write_test.go @@ -25,11 +25,15 @@ type aitableWorkflowCall struct { type aitableWorkflowCaller struct { calls []aitableWorkflowCall response string + err error dryRun bool } func (c *aitableWorkflowCaller) CallTool(_ context.Context, productID, toolName string, args map[string]any) (*edition.ToolResult, error) { c.calls = append(c.calls, aitableWorkflowCall{productID: productID, toolName: toolName, args: args}) + if c.err != nil { + return nil, c.err + } response := c.response if response == "" { response = `{"status":"success","data":{"valid":true,"flowId":"flow-test","issues":[]}}` @@ -114,6 +118,11 @@ func TestCrossPlatformCoverageAitableWorkflowPublishRejectsFalseSuccess(t *testi {name: "missing valid", response: `{"status":"success","data":{"flowId":"flow-test"}}`, want: "missing valid"}, {name: "missing flow id", response: `{"status":"success","data":{"valid":true}}`, want: "missing a non-empty flowId"}, {name: "wrong valid type", response: `{"status":"success","data":{"valid":"true","flowId":"flow-test"}}`, want: "valid must be boolean"}, + {name: "malformed response", response: `{`, want: "not a JSON object"}, + {name: "null response", response: `null`, want: "not a JSON object"}, + {name: "conflicting valid", response: `{"valid":true,"data":{"valid":false,"flowId":"flow-test"}}`, want: "conflicting valid values"}, + {name: "invalid flow id", response: `{"valid":true,"flowId":1}`, want: "flowId must be a non-empty string"}, + {name: "conflicting workflow ids", response: `{"valid":true,"flowId":"one","data":{"workflowId":"two"}}`, want: "conflicting workflow IDs"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -143,6 +152,19 @@ func TestCrossPlatformCoverageAitableWorkflowPublishRejectsFalseSuccess(t *testi t.Fatalf("workflow create dry-run = err:%v calls:%#v", err, caller.calls) } }) + + t.Run("transport error", func(t *testing.T) { + caller := &aitableWorkflowCaller{err: context.Canceled} + err := runAitableWorkflowCommandWithCaller(t, caller, nil, + "create", "--base-id", "base", "--dsl", `{"version":"workflow-dsl/v1","name":"test"}`) + if err == nil || !strings.Contains(err.Error(), context.Canceled.Error()) || len(caller.calls) != 1 { + t.Fatalf("workflow transport error = err:%v calls:%#v", err, caller.calls) + } + }) + + if _, _, _, err := strictAitableWorkflowPublishResult(nil); err == nil || !strings.Contains(err.Error(), "empty response") { + t.Fatalf("nil workflow envelope = %v", err) + } } func TestAitableWorkflowEditExampleMapsEmptyArguments(t *testing.T) { diff --git a/internal/helpers/drive_sync_confirmation_test.go b/internal/helpers/drive_sync_confirmation_test.go index dec9633f..70a303d7 100644 --- a/internal/helpers/drive_sync_confirmation_test.go +++ b/internal/helpers/drive_sync_confirmation_test.go @@ -267,6 +267,36 @@ func TestCrossPlatformCoverageDriveSyncDryRunReturnsRealActionPlan(t *testing.T) } } +func TestCrossPlatformCoverageDriveSyncDryRunPlanBranchMatrix(t *testing.T) { + remoteFiles := map[string]*remoteFile{ + "conflict.txt": {FileID: "remote-file"}, + } + remoteFolders := map[string]string{"": "root", "existing": "existing-id"} + localDirs := []string{"existing", "new", "missing/child"} + localFiles := map[string]localPushFile{"conflict.txt": {}, "new/file.txt": {}} + + for _, policy := range []string{syncConflictRemoteWins, syncConflictLocalWins, syncConflictKeepBoth, syncConflictAsk, syncConflictSkip} { + t.Run(policy, func(t *testing.T) { + res := &driveSyncResult{} + appendDriveSyncDryRunPlan(res, localDirs, localFiles, remoteFiles, remoteFolders, + []string{"new/file.txt", "missing/file.txt"}, []string{"remote.txt"}, []string{"conflict.txt"}, []string{"unknown.txt"}, policy) + if res.Summary.PlannedSkips == 0 || res.Summary.PlannedFolders == 0 || res.Summary.PlannedPulls == 0 || res.Summary.Failed != 2 { + t.Fatalf("dry-run plan summary for %s = %#v; items=%#v", policy, res.Summary, res.Items) + } + switch policy { + case syncConflictRemoteWins, syncConflictKeepBoth: + if res.Summary.PlannedPulls < 2 { + t.Fatalf("%s planned pulls = %d", policy, res.Summary.PlannedPulls) + } + case syncConflictLocalWins: + if res.Summary.PlannedPushes < 2 { + t.Fatalf("local wins planned pushes = %d", res.Summary.PlannedPushes) + } + } + }) + } +} + func TestCrossPlatformCoverageDrivePull_dryRunPlanBranches(t *testing.T) { root := t.TempDir() mustWrite(t, filepath.Join(root, "skip.txt"), "skip") diff --git a/internal/shortcut/aitable/base_composite_e2e_test.go b/internal/shortcut/aitable/base_composite_e2e_test.go index 0bd20ea5..9df47ae6 100644 --- a/internal/shortcut/aitable/base_composite_e2e_test.go +++ b/internal/shortcut/aitable/base_composite_e2e_test.go @@ -160,6 +160,73 @@ func TestCrossPlatformCoverageBaseCopyRequiresNewIDAndExactReadBackE2E(t *testin }) } +func TestCrossPlatformCoverageBaseCopyFailureAndDryRunEdgesE2E(t *testing.T) { + t.Run("invalid source", func(t *testing.T) { + out, err := runAITableCompositeCLI(t, &upsertByKeyCaller{}, "+base-copy", "--base-id", "bad/id", "--target-folder-id", "folder", "--yes") + if err == nil || out != "" { + t.Fatalf("invalid source = output:%q err:%v", out, err) + } + }) + + t.Run("dry run", func(t *testing.T) { + caller := &upsertByKeyCaller{dryRun: true} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--dry-run") + if err != nil || len(caller.calls) != 0 || !strings.Contains(out, `"status": "planned"`) { + t.Fatalf("dry run = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("target lookup error", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{err: context.Canceled}}} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + if err == nil || !strings.Contains(err.Error(), context.Canceled.Error()) || out != "" || len(caller.calls) != 1 { + t.Fatalf("target error = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + + t.Run("generic copy error is unknown", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"data":{"fileId":"folder","type":"folder"}}`}, + {err: errors.New("transport failed")}, + }} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + if err == nil || out != "" || len(caller.calls) != 2 { + t.Fatalf("copy error = output:%q err:%v", out, err) + } + }) + + readBackCases := []struct { + name string + step upsertByKeyStep + }{ + {name: "transport", step: upsertByKeyStep{err: context.DeadlineExceeded}}, + {name: "missing id", step: upsertByKeyStep{text: `{}`}}, + {name: "wrong id", step: upsertByKeyStep{text: `{"baseId":"other"}`}}, + } + for _, tc := range readBackCases { + t.Run("readback "+tc.name, func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"data":{"fileId":"folder","type":"folder"}}`}, + {text: `{"newBaseId":"new-base"}`}, + tc.step, + }} + out, err := runAITableCompositeCLI(t, caller, "+base-copy", "--base-id", "source", "--target-folder-id", "folder", "--yes") + if err == nil || out != "" || len(caller.calls) != 3 { + t.Fatalf("readback %s = output:%q err:%v", tc.name, out, err) + } + }) + } + + for _, value := range []string{"", "white space", "control\x00"} { + if validCompositeOpaqueID(value) { + t.Errorf("validCompositeOpaqueID(%q) = true", value) + } + } + if pureNumericID("") || pureNumericID("123a") || !pureNumericID("123") { + t.Fatal("pureNumericID edge mismatch") + } +} + func bootstrapFields(count int) []any { fields := make([]any, 0, count) for index := 0; index < count; index++ { diff --git a/internal/shortcut/aitable/base_copy.go b/internal/shortcut/aitable/base_copy.go index c37f3a1e..bc569a68 100644 --- a/internal/shortcut/aitable/base_copy.go +++ b/internal/shortcut/aitable/base_copy.go @@ -92,9 +92,6 @@ func executeBaseCopy(rt *shortcut.RuntimeContext) error { if verifyErr != nil { result.Status = "partial_success" result.Verification = map[string]any{"status": "failed", "newBaseId": newBaseID, "error": verifyErr.Error()} - if writeErr != nil { - result.Warnings = append(result.Warnings, "copy response error: "+writeErr.Error()) - } return compositeError(result, verifyErr, false) } @@ -102,10 +99,6 @@ func executeBaseCopy(rt *shortcut.RuntimeContext) error { result.CompletedSteps = append(result.CompletedSteps, compositeStep{Index: 2, Name: "copy base", Tool: "copy_base", Status: "completed", Result: map[string]any{"newBaseId": newBaseID}}) result.Verification = map[string]any{"status": "verified", "newBaseId": newBaseID} result.Result = map[string]any{"newBaseId": newBaseID, "base": readBack} - if writeErr != nil { - result.Status = "recovered" - result.Warnings = append(result.Warnings, "copy response was an error, but the new Base was proven by exact read-back") - } return rt.Output(result) } diff --git a/internal/shortcut/aitable/record_coverage_gap_test.go b/internal/shortcut/aitable/record_coverage_gap_test.go index ed41a74d..64a128c0 100644 --- a/internal/shortcut/aitable/record_coverage_gap_test.go +++ b/internal/shortcut/aitable/record_coverage_gap_test.go @@ -4,6 +4,7 @@ package aitable import ( + "context" "errors" "fmt" "strconv" @@ -153,6 +154,62 @@ func TestCrossPlatformCoverageRecordPrimaryDocPreflightAndNormalizationE2E(t *te }) } +func TestCrossPlatformCoverageRecordPrimaryDocFailureAndShapeEdgesE2E(t *testing.T) { + cases := []struct { + name string + steps []upsertByKeyStep + }{ + {name: "fields transport", steps: []upsertByKeyStep{{err: context.Canceled}}}, + {name: "missing fields collection", steps: []upsertByKeyStep{{text: `{}`}}}, + {name: "primary field missing id", steps: []upsertByKeyStep{{text: `{"fields":[{"type":"primaryDoc"}]}`}}}, + {name: "record query error", steps: []upsertByKeyStep{{text: `{"fields":[]}`}, {err: context.DeadlineExceeded}}}, + {name: "wrong record identity", steps: []upsertByKeyStep{{text: `{"fields":[]}`}, {text: `{"records":[{"recordId":"other"}]}`}}}, + {name: "helper unknown error", steps: []upsertByKeyStep{{text: `{"fields":[{"fieldId":"p","type":"primaryDoc"}]}`}, {text: `{"records":[{"recordId":"r1"}]}`}, {err: errors.New("permission denied")}}}, + {name: "helper missing node", steps: []upsertByKeyStep{{text: `{"fields":[{"fieldId":"p","type":"primaryDoc"}]}`}, {text: `{"records":[{"recordId":"r1"}]}`}, {text: `{"data":{"message":"ok"}}`}}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, err := runAITableCompositeCLI(t, &upsertByKeyCaller{steps: tc.steps}, "+record-primary-doc-get", "--base-id", "base", "--table-id", "table", "--record-id", "r1") + if err == nil || out != "" { + t.Fatalf("primary doc failure = output:%q err:%v", out, err) + } + }) + } + + for _, value := range []any{ + map[string]any{"exists": false}, + map[string]any{"dentryUuid": ""}, + map[string]any{"status": "NO_RECORD"}, + map[string]any{"nested": []any{map[string]any{"status": "unassociated"}}}, + } { + if !knownPrimaryDocUnassociatedData(value) { + t.Errorf("known unassociated shape not recognized: %#v", value) + } + } + if knownPrimaryDocUnassociatedData([]any{"unrelated"}) || knownPrimaryDocUnassociatedData(nil) { + t.Fatal("unrelated shapes must not be unassociated") + } +} + +func TestCrossPlatformCoverageRecordDeleteRejectsMalformedPreflightRecordsE2E(t *testing.T) { + for _, tc := range []struct { + name string + records string + }{ + {name: "missing id", records: `[{"cells":{}}]`}, + {name: "unexpected id", records: `[{"recordId":"other"}]`}, + {name: "duplicate id", records: `[{"recordId":"r1"},{"recordId":"r1"}]`}, + } { + t.Run(tc.name, func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"records":` + tc.records + `}`}}} + out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) + if err == nil || out != "" || len(caller.calls) != 1 { + t.Fatalf("delete malformed preflight = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + } +} + func TestCrossPlatformCoverageRecordBatchVerificationEdgesE2E(t *testing.T) { record := []map[string]any{{"recordId": "r1", "cells": map[string]any{"f": "new"}}} diff --git a/internal/shortcut/aitable/remaining_coverage_gap_test.go b/internal/shortcut/aitable/remaining_coverage_gap_test.go index b50d5588..3f95b40e 100644 --- a/internal/shortcut/aitable/remaining_coverage_gap_test.go +++ b/internal/shortcut/aitable/remaining_coverage_gap_test.go @@ -242,6 +242,40 @@ func TestCrossPlatformCoverageViewPresetPureMatching(t *testing.T) { } } +func TestCrossPlatformCoverageViewPresetProjectionShapeEdges(t *testing.T) { + if !presetViewMatches(map[string]any{"viewType": "Grid", "f": 1}, "Grid", map[string]any{"f": 1}) { + t.Fatal("top-level projected config must match") + } + invalid := []map[string]any{ + {"columns": "bad", "custom": map[string]any{}}, + {"columns": []any{"f"}, "custom": map[string]any{"hiddenFields": map[string]any{"f": "bad"}}}, + {"columns": []any{1}, "custom": map[string]any{"hiddenFields": map[string]any{"1": false}}}, + {"columns": []any{"f"}, "custom": map[string]any{"hiddenFields": map[string]any{}}}, + {"columns": []any{"f"}, "custom": map[string]any{"hiddenFields": []any{}}}, + {"columns": []any{1}, "custom": map[string]any{"hiddenFields": []any{false}}}, + {"columns": []any{"f"}, "custom": map[string]any{"hiddenFields": []any{"bad"}}}, + } + for _, view := range invalid { + if got, ok := projectedVisibleFieldIDs(view); ok || got != nil { + t.Errorf("invalid projection %#v = %#v, %v", view, got, ok) + } + } +} + +func TestCrossPlatformCoverageViewPresetReadBackDuplicateStopsE2E(t *testing.T) { + disableViewPresetSleep(t) + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"views":[]}`}, + {text: `{"viewId":"v1"}`}, + {text: `{"views":[{"viewId":"v1","viewName":"X"},{"viewId":"v2","viewName":"X"}]}`}, + }} + out, err := runAITableCompositeCLI(t, caller, "+view-preset-apply", + "--base-id", "base", "--table-id", "table", "--name", "X", "--view-type", "Grid", "--config", `{"f":1}`, "--yes") + if err == nil || out != "" || len(caller.calls) != 3 { + t.Fatalf("duplicate readback = output:%q err:%v calls:%#v", out, err, caller.calls) + } +} + func TestCrossPlatformCoverageWorkflowDeployValidationAndFailureStagesE2E(t *testing.T) { cases := []struct { name string From 0ed05a2c5d6b171079d62d906de622dc8b563ba7 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 19 Aug 2026 14:44:04 +0800 Subject: [PATCH 3/5] fix: address shortcut review edge cases --- internal/helpers/aitable.go | 61 +++++++++------- .../helpers/aitable_command_edges_test.go | 13 +++- internal/helpers/drive_sync.go | 56 +++++++------- .../helpers/drive_sync_confirmation_test.go | 7 +- internal/shortcut/aitable/primary_doc.go | 73 +++++++++++-------- .../aitable/record_coverage_gap_test.go | 16 +++- 6 files changed, 138 insertions(+), 88 deletions(-) diff --git a/internal/helpers/aitable.go b/internal/helpers/aitable.go index d31b1f7e..2f72dcd0 100644 --- a/internal/helpers/aitable.go +++ b/internal/helpers/aitable.go @@ -8,6 +8,7 @@ import ( "math" "os" "reflect" + "sort" "strconv" "strings" "time" @@ -1383,34 +1384,46 @@ func loadAitableFieldTypes(ctx context.Context, baseID, tableID string) (map[str } func findAitableObjectList(value any, names ...string) ([]map[string]any, bool) { - wanted := make(map[string]bool, len(names)) + wanted := make([]string, 0, len(names)) + seen := make(map[string]bool, len(names)) for _, name := range names { - wanted[strings.ToLower(name)] = true + name = strings.ToLower(name) + if !seen[name] { + wanted = append(wanted, name) + seen[name] = true + } } var walk func(any) ([]map[string]any, bool) walk = func(current any) ([]map[string]any, bool) { switch typed := current.(type) { case map[string]any: - for key, child := range typed { - if !wanted[strings.ToLower(key)] { - continue - } - items, ok := child.([]any) - if !ok { - return nil, false - } - out := make([]map[string]any, 0, len(items)) - for _, item := range items { - object, ok := item.(map[string]any) + keys := make([]string, 0, len(typed)) + for key := range typed { + keys = append(keys, key) + } + sort.Strings(keys) + for _, name := range wanted { + for _, key := range keys { + if !strings.EqualFold(key, name) { + continue + } + items, ok := typed[key].([]any) if !ok { return nil, false } - out = append(out, object) + out := make([]map[string]any, 0, len(items)) + for _, item := range items { + object, ok := item.(map[string]any) + if !ok { + return nil, false + } + out = append(out, object) + } + return out, true } - return out, true } - for _, child := range typed { - if found, ok := walk(child); ok { + for _, key := range keys { + if found, ok := walk(typed[key]); ok { return found, true } } @@ -1477,7 +1490,7 @@ func validateAitableViewFilterCondition(condition map[string]any, fieldTypes map } if operator == "any_of" { if values, ok := operands[1].([]any); ok { - if _, err := expandAitableMultiSelectAnyOf(fieldID, values); err != nil { + if err := validateAitableMultiSelectOptionNames(values); err != nil { return err } return fmt.Errorf("multipleSelect any_of with multiple values is not supported by the persisted view protocol; use one scalar option or separate views") @@ -1490,22 +1503,18 @@ func validateAitableViewFilterCondition(condition map[string]any, fieldTypes map return nil } -func expandAitableMultiSelectAnyOf(fieldID string, values []any) ([]any, error) { +func validateAitableMultiSelectOptionNames(values []any) error { if len(values) == 0 { - return nil, fmt.Errorf("multipleSelect any_of array must not be empty") + return fmt.Errorf("multipleSelect any_of array must not be empty") } - expanded := make([]any, 0, len(values)) for index, value := range values { text, ok := value.(string) text = strings.TrimSpace(text) if !ok || text == "" { - return nil, fmt.Errorf("multipleSelect any_of value %d must be a non-empty option-name string", index) + return fmt.Errorf("multipleSelect any_of value %d must be a non-empty option-name string", index) } - expanded = append(expanded, map[string]any{ - "operator": "eq", "operands": []any{fieldID, text}, - }) } - return expanded, nil + return nil } func isAitableDateFieldType(fieldType string) bool { diff --git a/internal/helpers/aitable_command_edges_test.go b/internal/helpers/aitable_command_edges_test.go index 367b133b..738059ea 100644 --- a/internal/helpers/aitable_command_edges_test.go +++ b/internal/helpers/aitable_command_edges_test.go @@ -546,6 +546,12 @@ func TestCrossPlatformCoverageAitableViewFilterFailureAndShapeEdges(t *testing.T if got, ok := findAitableObjectList([]any{"skip", map[string]any{"nested": map[string]any{"fields": []any{map[string]any{"fieldId": "f"}}}}}, "fields"); !ok || len(got) != 1 { t.Fatalf("recursive fields = %#v, %v", got, ok) } + if got, ok := findAitableObjectList(map[string]any{ + "fieldList": []any{map[string]any{"fieldId": "legacy"}}, + "fields": []any{map[string]any{"fieldId": "canonical"}}, + }, "fields", "fieldList"); !ok || len(got) != 1 || got[0]["fieldId"] != "canonical" { + t.Fatalf("declared collection priority = %#v, %v", got, ok) + } invalidFilters := []struct { filter []any @@ -564,12 +570,15 @@ func TestCrossPlatformCoverageAitableViewFilterFailureAndShapeEdges(t *testing.T t.Errorf("validate filter %#v = %v, want %q", tc.filter, err, tc.want) } } - if _, err := expandAitableMultiSelectAnyOf("multi", nil); err == nil { + if err := validateAitableMultiSelectOptionNames(nil); err == nil { t.Fatal("empty any_of array must fail") } - if _, err := expandAitableMultiSelectAnyOf("multi", []any{"ok", 1}); err == nil { + if err := validateAitableMultiSelectOptionNames([]any{"ok", 1}); err == nil { t.Fatal("non-string any_of option must fail") } + if err := validateAitableMultiSelectOptionNames([]any{"first", " second "}); err != nil { + t.Fatalf("valid any_of option names = %v", err) + } if got := compactJSON(make(chan int)); !strings.HasPrefix(got, "(chan int)") { t.Fatalf("compactJSON fallback = %q", got) } diff --git a/internal/helpers/drive_sync.go b/internal/helpers/drive_sync.go index 2bf8b8eb..b7dbad34 100644 --- a/internal/helpers/drive_sync.go +++ b/internal/helpers/drive_sync.go @@ -280,8 +280,10 @@ func runDriveSync(cmd *cobra.Command, _ []string) error { // --dry-run：只算差异、不执行任何同步动作。 if deps.Caller.DryRun() { if len(preflight) == 0 && res.Summary.Failed == 0 { - appendDriveSyncDryRunPlan(&res, localDirs, localByRel, remoteFiles, remoteFolders, - newLocal, newRemote, modified, unknown, onConflict) + appendDriveSyncDryRunPlan(&res, driveSyncDryRunPlanInput{ + LocalDirs: localDirs, LocalByRel: localByRel, RemoteFiles: remoteFiles, RemoteFolders: remoteFolders, + NewLocal: newLocal, NewRemote: newRemote, Modified: modified, Unknown: unknown, OnConflict: onConflict, + }) } return deps.Out.PrintJSON(driveSyncDryRunResult{ DryRun: true, Executed: false, PreviewKind: "plan", Operation: "drive sync", Plan: res, @@ -410,16 +412,20 @@ func runDriveSync(cmd *cobra.Command, _ []string) error { return nil } -func appendDriveSyncDryRunPlan( - res *driveSyncResult, - localDirs []string, - localByRel map[string]localPushFile, - remoteFiles map[string]*remoteFile, - remoteFolders map[string]string, - newLocal, newRemote, modified, unknown []string, - onConflict string, -) { - for _, rel := range unknown { +type driveSyncDryRunPlanInput struct { + LocalDirs []string + LocalByRel map[string]localPushFile + RemoteFiles map[string]*remoteFile + RemoteFolders map[string]string + NewLocal []string + NewRemote []string + Modified []string + Unknown []string + OnConflict string +} + +func appendDriveSyncDryRunPlan(res *driveSyncResult, input driveSyncDryRunPlanInput) { + for _, rel := range input.Unknown { res.Summary.PlannedSkips++ res.Items = append(res.Items, driveSyncItem{ RelPath: rel, Action: syncActionPlannedSkip, Direction: syncDirectionConflict, @@ -427,11 +433,11 @@ func appendDriveSyncDryRunPlan( }) } - plannedFolders := make(map[string]string, len(remoteFolders)+len(localDirs)) - for rel, fileID := range remoteFolders { + plannedFolders := make(map[string]string, len(input.RemoteFolders)+len(input.LocalDirs)) + for rel, fileID := range input.RemoteFolders { plannedFolders[rel] = fileID } - for _, dir := range localDirs { + for _, dir := range input.LocalDirs { if _, ok := plannedFolders[dir]; ok { continue } @@ -446,11 +452,11 @@ func appendDriveSyncDryRunPlan( res.Items = append(res.Items, driveSyncItem{RelPath: dir, Action: syncActionPlannedFolderCreate, Direction: syncDirectionPush}) } - for _, rel := range newRemote { + for _, rel := range input.NewRemote { res.Summary.PlannedPulls++ res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedDownload, Direction: syncDirectionPull}) } - for _, rel := range newLocal { + for _, rel := range input.NewLocal { parentRel, _ := splitRel(rel) if plannedFolders[parentRel] == "" { res.Summary.Failed++ @@ -461,23 +467,23 @@ func appendDriveSyncDryRunPlan( res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedUpload, Direction: syncDirectionPush}) } - occupied := make(map[string]bool, len(localByRel)+len(localDirs)+len(remoteFiles)+len(remoteFolders)) - for rel := range localByRel { + occupied := make(map[string]bool, len(input.LocalByRel)+len(input.LocalDirs)+len(input.RemoteFiles)+len(input.RemoteFolders)) + for rel := range input.LocalByRel { occupied[rel] = true } - for _, rel := range localDirs { + for _, rel := range input.LocalDirs { occupied[rel] = true } - for rel := range remoteFiles { + for rel := range input.RemoteFiles { occupied[rel] = true } - for rel := range remoteFolders { + for rel := range input.RemoteFolders { if rel != "" { occupied[rel] = true } } - for _, rel := range modified { - switch onConflict { + for _, rel := range input.Modified { + switch input.OnConflict { case syncConflictRemoteWins: res.Summary.PlannedPulls++ res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedDownload, Direction: syncDirectionPull}) @@ -485,7 +491,7 @@ func appendDriveSyncDryRunPlan( res.Summary.PlannedPushes++ res.Items = append(res.Items, driveSyncItem{RelPath: rel, Action: syncActionPlannedOverwrite, Direction: syncDirectionConflict}) case syncConflictKeepBoth: - candidate := driveSyncSuffixedRel(rel, remoteFiles[rel].FileID, occupied) + candidate := driveSyncSuffixedRel(rel, input.RemoteFiles[rel].FileID, occupied) occupied[candidate] = true res.Summary.PlannedPulls++ res.Items = append(res.Items, diff --git a/internal/helpers/drive_sync_confirmation_test.go b/internal/helpers/drive_sync_confirmation_test.go index 70a303d7..0c8974d0 100644 --- a/internal/helpers/drive_sync_confirmation_test.go +++ b/internal/helpers/drive_sync_confirmation_test.go @@ -278,8 +278,11 @@ func TestCrossPlatformCoverageDriveSyncDryRunPlanBranchMatrix(t *testing.T) { for _, policy := range []string{syncConflictRemoteWins, syncConflictLocalWins, syncConflictKeepBoth, syncConflictAsk, syncConflictSkip} { t.Run(policy, func(t *testing.T) { res := &driveSyncResult{} - appendDriveSyncDryRunPlan(res, localDirs, localFiles, remoteFiles, remoteFolders, - []string{"new/file.txt", "missing/file.txt"}, []string{"remote.txt"}, []string{"conflict.txt"}, []string{"unknown.txt"}, policy) + appendDriveSyncDryRunPlan(res, driveSyncDryRunPlanInput{ + LocalDirs: localDirs, LocalByRel: localFiles, RemoteFiles: remoteFiles, RemoteFolders: remoteFolders, + NewLocal: []string{"new/file.txt", "missing/file.txt"}, NewRemote: []string{"remote.txt"}, + Modified: []string{"conflict.txt"}, Unknown: []string{"unknown.txt"}, OnConflict: policy, + }) if res.Summary.PlannedSkips == 0 || res.Summary.PlannedFolders == 0 || res.Summary.PlannedPulls == 0 || res.Summary.Failed != 2 { t.Fatalf("dry-run plan summary for %s = %#v; items=%#v", policy, res.Summary, res.Items) } diff --git a/internal/shortcut/aitable/primary_doc.go b/internal/shortcut/aitable/primary_doc.go index 0e2f8eab..52d2a662 100644 --- a/internal/shortcut/aitable/primary_doc.go +++ b/internal/shortcut/aitable/primary_doc.go @@ -75,7 +75,7 @@ func executeRecordPrimaryDocGet(rt *shortcut.RuntimeContext) error { } return err } - nodeID := findStringByKeys(data, "nodeId", "dentryUuid") + nodeID := primaryDocNodeID(data) if nodeID == "" { if knownPrimaryDocUnassociatedData(data) { return rt.Output(map[string]any{ @@ -96,38 +96,47 @@ func knownPrimaryDocUnassociatedError(err error) bool { return strings.Contains(message, "no record") || strings.Contains(message, "unassociated") } -func knownPrimaryDocUnassociatedData(value any) bool { - var walk func(any) bool - walk = func(current any) bool { - switch typed := current.(type) { - case map[string]any: - if exists, ok := typed["exists"].(bool); ok && !exists { - return true - } - for _, key := range []string{"nodeId", "dentryUuid"} { - if nodeID, exists := typed[key]; exists && (nodeID == nil || strings.TrimSpace(fmt.Sprint(nodeID)) == "") { - return true - } - } - if status, ok := typed["status"].(string); ok { - status = strings.ToLower(strings.TrimSpace(status)) - if status == "unassociated" || status == "no_record" { - return true - } - } - for _, child := range typed { - if walk(child) { - return true - } - } - case []any: - for _, child := range typed { - if walk(child) { - return true - } - } +func primaryDocNodeID(value any) string { + object, ok := value.(map[string]any) + if !ok { + return "" + } + for _, key := range []string{"nodeId", "dentryUuid"} { + if nodeID, ok := object[key].(string); ok && strings.TrimSpace(nodeID) != "" { + return strings.TrimSpace(nodeID) } + } + for _, envelopeKey := range []string{"data", "result", "response"} { + if nodeID := primaryDocNodeID(object[envelopeKey]); nodeID != "" { + return nodeID + } + } + return "" +} + +func knownPrimaryDocUnassociatedData(value any) bool { + object, ok := value.(map[string]any) + if !ok { return false } - return walk(value) + if exists, ok := object["exists"].(bool); ok && !exists { + return true + } + for _, key := range []string{"nodeId", "dentryUuid"} { + if nodeID, exists := object[key]; exists && (nodeID == nil || strings.TrimSpace(fmt.Sprint(nodeID)) == "") { + return true + } + } + if status, ok := object["status"].(string); ok { + status = strings.ToLower(strings.TrimSpace(status)) + if status == "unassociated" || status == "no_record" { + return true + } + } + for _, envelopeKey := range []string{"data", "result", "response"} { + if child, ok := object[envelopeKey].(map[string]any); ok && knownPrimaryDocUnassociatedData(child) { + return true + } + } + return false } diff --git a/internal/shortcut/aitable/record_coverage_gap_test.go b/internal/shortcut/aitable/record_coverage_gap_test.go index 64a128c0..253e2e90 100644 --- a/internal/shortcut/aitable/record_coverage_gap_test.go +++ b/internal/shortcut/aitable/record_coverage_gap_test.go @@ -180,7 +180,7 @@ func TestCrossPlatformCoverageRecordPrimaryDocFailureAndShapeEdgesE2E(t *testing map[string]any{"exists": false}, map[string]any{"dentryUuid": ""}, map[string]any{"status": "NO_RECORD"}, - map[string]any{"nested": []any{map[string]any{"status": "unassociated"}}}, + map[string]any{"data": map[string]any{"result": map[string]any{"status": "unassociated"}}}, } { if !knownPrimaryDocUnassociatedData(value) { t.Errorf("known unassociated shape not recognized: %#v", value) @@ -189,6 +189,20 @@ func TestCrossPlatformCoverageRecordPrimaryDocFailureAndShapeEdgesE2E(t *testing if knownPrimaryDocUnassociatedData([]any{"unrelated"}) || knownPrimaryDocUnassociatedData(nil) { t.Fatal("unrelated shapes must not be unassociated") } + for _, value := range []any{ + map[string]any{"metadata": map[string]any{"exists": false}}, + map[string]any{"items": []any{map[string]any{"nodeId": nil}}}, + } { + if knownPrimaryDocUnassociatedData(value) { + t.Errorf("unrelated nested marker must not be unassociated: %#v", value) + } + } + if got := primaryDocNodeID(map[string]any{"metadata": map[string]any{"nodeId": "wrong"}}); got != "" { + t.Fatalf("unrelated nested nodeId = %q, want empty", got) + } + if got := primaryDocNodeID(map[string]any{"data": map[string]any{"response": map[string]any{"dentryUuid": " node-1 "}}}); got != "node-1" { + t.Fatalf("known envelope nodeId = %q, want node-1", got) + } } func TestCrossPlatformCoverageRecordDeleteRejectsMalformedPreflightRecordsE2E(t *testing.T) { From d10446bea7e2044383975b8a3f0a9ae86d53a8a3 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 19 Aug 2026 15:50:03 +0800 Subject: [PATCH 4/5] ci: reuse preinstalled archive tooling --- .github/workflows/ci.yml | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc437165..4b5384b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -611,7 +611,13 @@ jobs: - name: Install archive tooling if: ${{ matrix.shard == 'release-scripts' && steps.select.outputs.affected == 'true' }} - run: sudo apt-get update && sudo apt-get install -y zip unzip + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi - name: Test shard with Race Detection if: ${{ steps.select.outputs.affected == 'true' }} @@ -751,7 +757,13 @@ jobs: go-version-file: go.mod - name: Install archive tooling - run: sudo apt-get update && sudo apt-get install -y zip unzip + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi - name: Test release scripts shell: bash @@ -1129,7 +1141,13 @@ jobs: go-version-file: go.mod - name: Install archive tooling - run: sudo apt-get update && sudo apt-get install -y zip unzip + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi - name: Build run: make build @@ -1180,7 +1198,13 @@ jobs: go-version-file: go.mod - name: Install archive tooling - run: sudo apt-get update && sudo apt-get install -y zip unzip + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi - name: Run policy and shortcut coverage run: | @@ -1261,7 +1285,13 @@ jobs: - name: Install archive tooling if: needs.lint.outputs.full_suite == 'true' && steps.baseline-cache.outputs.cache-hit != 'true' - run: sudo apt-get update && sudo apt-get install -y zip unzip + run: | + if command -v zip >/dev/null && command -v unzip >/dev/null; then + echo "zip and unzip are already available" + else + sudo apt-get update + sudo apt-get install -y zip unzip + fi - name: Run baseline unit tests with coverage if: steps.baseline-cache.outputs.cache-hit != 'true' From 17741851f5cd7e0e25e77697075cd1b2ed78f386 Mon Sep 17 00:00:00 2001 From: Dennis Date: Wed, 19 Aug 2026 16:20:04 +0800 Subject: [PATCH 5/5] test: satisfy native shortcut coverage gate --- internal/helpers/aitable_workflow_write_test.go | 2 +- .../shortcut/contact/search_mobile_coverage_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/helpers/aitable_workflow_write_test.go b/internal/helpers/aitable_workflow_write_test.go index 562e1a08..3709277c 100644 --- a/internal/helpers/aitable_workflow_write_test.go +++ b/internal/helpers/aitable_workflow_write_test.go @@ -77,7 +77,7 @@ func runAitableWorkflowCommandWithCaller(t *testing.T, caller *aitableWorkflowCa return cmd.Execute() } -func TestAitableWorkflowCreateMapsDSLWithoutRetry(t *testing.T) { +func TestCrossPlatformCoverageAitableWorkflowCreateMapsDSLWithoutRetry(t *testing.T) { wantDSL := map[string]any{ "version": "workflow-dsl/v1", "name": "create test", diff --git a/internal/shortcut/contact/search_mobile_coverage_test.go b/internal/shortcut/contact/search_mobile_coverage_test.go index fcadbb2a..06e392d5 100644 --- a/internal/shortcut/contact/search_mobile_coverage_test.go +++ b/internal/shortcut/contact/search_mobile_coverage_test.go @@ -31,3 +31,15 @@ func TestCrossPlatformCoverageSearchMobilePreservesEmptyObjectSemantics(t *testi t.Fatalf("empty result projected users: %#v", users) } } + +func TestCrossPlatformCoverageSearchMobileProjectsArrayAndMissingResult(t *testing.T) { + users := searchUserProject(map[string]any{ + "result": []any{map[string]any{"name": "Alice", "userId": "u1"}}, + }) + if len(users) != 1 || users[0]["userId"] != "u1" || users[0]["name"] != "Alice" { + t.Fatalf("array envelope projection = %#v, want one user", users) + } + if users := searchUserProject(map[string]any{"success": true}); len(users) != 0 { + t.Fatalf("missing result projected users: %#v", users) + } +}