From fe2f3ca92f92cda08453da4b7bf05e9f5430c4a2 Mon Sep 17 00:00:00 2001 From: Dennis Date: Fri, 14 Aug 2026 11:57:41 +0800 Subject: [PATCH] fix: harden aitable pagination and minutes unshare --- internal/app/param_alias_e2e_test.go | 2 + internal/shortcut/aitable/aitable.go | 2 +- internal/shortcut/aitable/record_batch.go | 45 +++-- internal/shortcut/aitable/record_query_all.go | 127 +++++++++++++- .../aitable/record_upsert_by_key_e2e_test.go | 164 +++++++++++++++++- internal/shortcut/minutes/workflows.go | 17 +- .../minutes/workflows_coverage_test.go | 21 ++- .../minutesdata/coverage_completion_test.go | 16 ++ internal/shortcut/minutesdata/workflow.go | 55 ++++++ 9 files changed, 432 insertions(+), 17 deletions(-) diff --git a/internal/app/param_alias_e2e_test.go b/internal/app/param_alias_e2e_test.go index 4717d2b1..71298e27 100644 --- a/internal/app/param_alias_e2e_test.go +++ b/internal/app/param_alias_e2e_test.go @@ -52,6 +52,8 @@ func (c *paramAliasCaptureCaller) paramAliasResponseForTool(tool string) string switch tool { case "list_calendar_events": return `{"result":{"events":[]}}` + case "query_records": + return `{"success":true,"status":"success","error":{},"data":{}}` case "search_mail_users": return `{"users":[{"name":"Fixture User","email":"fixture@example.com","id":"fixture-user"}]}` case "search_dept_by_keyword": diff --git a/internal/shortcut/aitable/aitable.go b/internal/shortcut/aitable/aitable.go index 0077dda9..2d1a9d64 100644 --- a/internal/shortcut/aitable/aitable.go +++ b/internal/shortcut/aitable/aitable.go @@ -733,7 +733,7 @@ var RecordQuery = shortcut.Shortcut{ if rt.Changed("cursor") { params["cursor"] = rt.Str("cursor") } - return rt.CallMCP("query_records", params) + return executeRecordQuery(rt, params) }, } diff --git a/internal/shortcut/aitable/record_batch.go b/internal/shortcut/aitable/record_batch.go index 8c26c3b1..05294ad3 100644 --- a/internal/shortcut/aitable/record_batch.go +++ b/internal/shortcut/aitable/record_batch.go @@ -98,7 +98,7 @@ func executeRecordDeleteBatches(rt *shortcut.RuntimeContext) error { step.Status = "unknown" step.Error = writeErr.Error() } - remaining, verifyErr := queryRecordsByIDs(rt, baseID, tableID, batch) + remaining, verifyErr := queryDeletedRecordsByIDs(rt, baseID, tableID, batch) if verifyErr == nil && len(remaining) > 0 { verifyErr = fmt.Errorf("read-back still contains deleted record IDs: %s", strings.Join(recordIDs(remaining), ",")) } @@ -303,20 +303,43 @@ func verifyUpsertBatch(rt *shortcut.RuntimeContext, baseID, tableID string, batc } func queryRecordsByIDs(rt *shortcut.RuntimeContext, baseID, tableID string, ids []string) ([]map[string]any, error) { - data, err := rt.CallMCPData(serverMain, "query_records", map[string]any{ - "baseId": baseID, "tableId": tableID, "recordIds": ids, "limit": len(ids), - }) + window, err := queryRecordWindow(rt, map[string]any{ + "baseId": baseID, "tableId": tableID, "recordIds": ids, + }, len(ids)) if err != nil { return nil, err } - records, found := findRecords(data) - if !found { - return nil, fmt.Errorf("query_records read-back is missing records") + // Exact-ID verification below compares every requested ID with the returned + // records. The service can publish a continuation even after all requested + // IDs are present, so hasMore is not evidence that this bounded read-back is + // incomplete. + return window.Records, nil +} + +func queryDeletedRecordsByIDs(rt *shortcut.RuntimeContext, baseID, tableID string, ids []string) ([]map[string]any, error) { + remaining := make([]map[string]any, 0) + for offset := 0; offset < len(ids); offset += recordQueryServicePageSize { + end := minInt(offset+recordQueryServicePageSize, len(ids)) + chunk := ids[offset:end] + data, err := rt.CallMCPData(serverMain, "query_records", map[string]any{ + "baseId": baseID, "tableId": tableID, "recordIds": chunk, "limit": len(chunk), + }) + if err != nil { + return nil, err + } + records, found := findRecords(data) + if !found { + if explicitEmptyRecordQuery(data) { + continue + } + return nil, fmt.Errorf("query_records deletion read-back chunk %d is missing records", offset/recordQueryServicePageSize+1) + } + if responseHasMore(data) { + return nil, fmt.Errorf("query_records deletion read-back chunk %d is incomplete", offset/recordQueryServicePageSize+1) + } + remaining = append(remaining, records...) } - if responseHasMore(data) { - return nil, fmt.Errorf("query_records read-back is incomplete") - } - return records, nil + return remaining, nil } func recordIDs(records []map[string]any) []string { diff --git a/internal/shortcut/aitable/record_query_all.go b/internal/shortcut/aitable/record_query_all.go index 0606abba..fa2c2368 100644 --- a/internal/shortcut/aitable/record_query_all.go +++ b/internal/shortcut/aitable/record_query_all.go @@ -10,13 +10,138 @@ import ( "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/shortcut" ) +// query_records is backed by a service pager whose stable typed-envelope +// boundary is 20 records. Asking that layer to aggregate more than one page can +// drop the envelope fields used by the record projector, turning a successful +// read into an empty collection. Keep every remote request on one service page +// and aggregate only after each page has passed the CLI's shape validation. +const recordQueryServicePageSize = 20 + +type recordQueryWindow struct { + Records []map[string]any + HasMore bool + NextCursor string + Pages int +} + +func queryRecordWindow(rt *shortcut.RuntimeContext, params map[string]any, limit int) (recordQueryWindow, error) { + if limit <= 0 { + return recordQueryWindow{}, fmt.Errorf("record query limit must be positive, got %d", limit) + } + request := cloneAnyMap(params) + cursor, _ := request["cursor"].(string) + cursor = strings.TrimSpace(cursor) + seen := map[string]bool{} + if cursor != "" { + seen[cursor] = true + } + window := recordQueryWindow{Records: make([]map[string]any, 0, limit)} + + for len(window.Records) < limit { + pageSize := minInt(recordQueryServicePageSize, limit-len(window.Records)) + request["limit"] = pageSize + if cursor == "" { + delete(request, "cursor") + } else { + request["cursor"] = cursor + } + data, err := rt.CallMCPData(serverMain, "query_records", request) + if err != nil { + return recordQueryWindow{}, err + } + records, found := findRecords(data) + if !found { + if explicitEmptyRecordQuery(data) { + window.Pages++ + window.HasMore = false + window.NextCursor = "" + return window, nil + } + return recordQueryWindow{}, fmt.Errorf("query_records page %d is missing the records collection", window.Pages+1) + } + window.Records = append(window.Records, records...) + window.Pages++ + + if !responseHasMore(data) { + window.HasMore = false + window.NextCursor = "" + return window, nil + } + next := responseCursor(data) + if next == "" { + return recordQueryWindow{}, fmt.Errorf("query_records page %d reports more data but no next cursor", window.Pages) + } + if seen[next] { + return recordQueryWindow{}, fmt.Errorf("query_records cursor cycle detected at %q", next) + } + seen[next] = true + cursor = next + window.HasMore = true + window.NextCursor = next + } + + return window, nil +} + +// explicitEmptyRecordQuery recognizes the service's reviewed zero-match wire +// shape. It is deliberately stricter than "records is absent": only a complete +// success envelope with an empty data object is accepted as an empty set. +func explicitEmptyRecordQuery(data map[string]any) bool { + if data == nil || data["success"] != true || data["status"] != "success" { + return false + } + payload, ok := data["data"].(map[string]any) + if !ok || len(payload) != 0 { + return false + } + if rawError, exists := data["error"]; exists && rawError != nil { + errorObject, ok := rawError.(map[string]any) + if !ok || len(errorObject) != 0 { + return false + } + } + return true +} + +func executeRecordQuery(rt *shortcut.RuntimeContext, params map[string]any) error { + limit := 100 + if rt.Changed("limit") { + limit = rt.Int("limit") + } + if limit < 1 || limit > recordBatchSize { + return fmt.Errorf("--limit must be in [1,%d], got %d", recordBatchSize, limit) + } + window, err := queryRecordWindow(rt, params, limit) + if err != nil { + return err + } + records := make([]any, 0, len(window.Records)) + for _, record := range window.Records { + records = append(records, record) + } + data := map[string]any{ + "records": records, + "hasMore": window.HasMore, + "page": window.Pages, + "size": len(window.Records), + } + if window.NextCursor != "" { + data["nextCursor"] = window.NextCursor + } + return rt.Output(map[string]any{ + "success": true, + "status": "success", + "data": data, + }) +} + func queryAllRecords(rt *shortcut.RuntimeContext, params map[string]any, maxRecords int) ([]map[string]any, error) { all := make([]map[string]any, 0) cursor := "" seen := map[string]bool{} for page := 0; ; page++ { request := cloneAnyMap(params) - request["limit"] = recordBatchSize + request["limit"] = recordQueryServicePageSize if cursor != "" { request["cursor"] = cursor } 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 64bf759a..4744fbe1 100644 --- a/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go +++ b/internal/shortcut/aitable/record_upsert_by_key_e2e_test.go @@ -265,6 +265,149 @@ func recordListJSON(t *testing.T, records []map[string]any) string { return string(raw) } +func pagedRecordQueryResponse(t *testing.T, records []map[string]any, args map[string]any) string { + t.Helper() + limit, ok := args["limit"].(int) + if !ok || limit < 1 || limit > recordQueryServicePageSize { + t.Fatalf("query_records limit = %#v, want 1..%d", args["limit"], recordQueryServicePageSize) + } + offset := 0 + if cursor, _ := args["cursor"].(string); cursor != "" { + if _, err := fmt.Sscanf(cursor, "offset-%d", &offset); err != nil { + t.Fatalf("invalid test cursor %q: %v", cursor, err) + } + } + end := minInt(offset+limit, len(records)) + pageRecords := records[offset:end] + data := map[string]any{"records": pageRecords, "hasMore": end < len(records)} + if end < len(records) { + data["nextCursor"] = fmt.Sprintf("offset-%d", end) + } + raw, err := json.Marshal(map[string]any{ + "success": true, + "hasMore": end < len(records), + "page": offset/recordQueryServicePageSize + 1, + "size": limit, + "data": data, + }) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +func runRecordQueryShortcutCLI(t *testing.T, caller *upsertByKeyCaller, limit int) (map[string]any, error) { + t.Helper() + 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.AddCommand(shortcut.Commands()...) + stdout := &bytes.Buffer{} + root.SetOut(stdout) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"aitable", "+record-query", "--base-id", "base", "--table-id", "table", "--limit", fmt.Sprint(limit)}) + err := root.Execute() + if stdout.Len() == 0 { + return nil, err + } + var payload map[string]any + if decodeErr := json.Unmarshal(stdout.Bytes(), &payload); decodeErr != nil { + t.Fatalf("decode record query output %q: %v", stdout.String(), decodeErr) + } + return payload, err +} + +func TestCrossPlatformCoverageRecordQueryServicePageBoundariesE2E(t *testing.T) { + for _, size := range []int{20, 21, 22, 100} { + t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) { + records := updateFixtureRecords(0, size, "可见") + caller := &upsertByKeyCaller{} + caller.callFn = func(_ int, _, tool string, args map[string]any) (string, error) { + if tool != "query_records" { + return "", fmt.Errorf("unexpected tool %s", tool) + } + return pagedRecordQueryResponse(t, records, args), nil + } + payload, err := runRecordQueryShortcutCLI(t, caller, size) + if err != nil { + t.Fatalf("record query size %d: %v", size, err) + } + data, ok := payload["data"].(map[string]any) + if !ok || payload["success"] != true || data["hasMore"] != false || len(data["records"].([]any)) != size { + t.Fatalf("record query size %d payload = %#v", size, payload) + } + wantCalls := (size + recordQueryServicePageSize - 1) / recordQueryServicePageSize + if len(caller.calls) != wantCalls || data["page"] != float64(wantCalls) || data["size"] != float64(size) { + t.Fatalf("record query size %d calls=%d payload=%#v, want calls=%d", size, len(caller.calls), payload, wantCalls) + } + }) + } +} + +func TestCrossPlatformCoverageRecordWriteReadbackUsesStableServicePagesE2E(t *testing.T) { + for _, operation := range []string{"update", "upsert", "delete"} { + for _, size := range []int{21, 22, 100} { + t.Run(fmt.Sprintf("%s_%d", operation, size), func(t *testing.T) { + records := updateFixtureRecords(0, size, "完成") + caller := &upsertByKeyCaller{} + queryCalls := 0 + caller.callFn = func(_ int, _, tool string, args map[string]any) (string, error) { + if tool != "query_records" { + return `{"success":true}`, nil + } + queryCalls++ + if operation == "delete" { + ids := args["recordIds"].([]string) + empty := make([]map[string]any, len(ids)) + response := pagedRecordQueryResponse(t, empty, 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 + } + response := pagedRecordQueryResponse(t, records, args) + var payload map[string]any + if err := json.Unmarshal([]byte(response), &payload); err != nil { + t.Fatal(err) + } + page := payload["data"].(map[string]any) + if len(page["records"].([]any))+recordQueryServicePageSize*(queryCalls-1) >= size { + // The live service can retain a continuation after it has + // returned every requested record ID. Exact-ID verification + // must use ID coverage, not this advisory bit. + page["hasMore"] = true + page["nextCursor"] = "ignored-after-complete-id-set" + payload["hasMore"] = true + } + raw, _ := json.Marshal(payload) + return string(raw), nil + } + + var out string + var err error + if operation == "delete" { + out, err = runRecordDeleteCLI(t, caller, recordIDs(records)) + } else { + out, err = runRecordBatchCLI(t, caller, "+record-"+operation, records) + } + if err != nil || out == "" { + t.Fatalf("%s size %d false negative: output=%q err=%v", operation, size, out, err) + } + wantQueries := (size + recordQueryServicePageSize - 1) / recordQueryServicePageSize + if queryCalls != wantQueries { + t.Fatalf("%s size %d query calls=%d, want %d", operation, size, queryCalls, wantQueries) + } + }) + } + } +} + func TestCrossPlatformCoverageRecordUpdateAutoChunksAndVerifiesE2E(t *testing.T) { records := updateFixtureRecords(0, 101, "完成") caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ @@ -395,6 +538,10 @@ func TestCrossPlatformCoverageRecordDeleteAutoChunksAndProvesAbsenceE2E(t *testi 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":[]}}`}, }} @@ -407,7 +554,7 @@ func TestCrossPlatformCoverageRecordDeleteAutoChunksAndProvesAbsenceE2E(t *testi t.Fatalf("delete output missing %s: %s", want, out) } } - if len(caller.calls) != 4 || caller.calls[0].tool != "delete_records" || caller.calls[1].tool != "query_records" { + if len(caller.calls) != 8 || caller.calls[0].tool != "delete_records" || caller.calls[1].tool != "query_records" || caller.calls[6].tool != "delete_records" { t.Fatalf("delete call sequence = %#v", caller.calls) } } @@ -421,6 +568,17 @@ func TestCrossPlatformCoverageRecordDeleteEmptyReplyRecoveredOnlyByAbsenceE2E(t } }) + t.Run("explicit service success with empty data proves deletion", func(t *testing.T) { + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: `{"deletedCount":1}`}, + {text: `{"success":true,"status":"success","error":{},"data":{}}`}, + }} + out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) + if err != nil || !strings.Contains(out, `"status": "verified_absent"`) { + t.Fatalf("delete explicit empty success = output:%q err:%v", out, err) + } + }) + t.Run("missing records contract is unknown", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"deletedCount":1}`}, {text: `{"data":{}}`}}} out, err := runRecordDeleteCLI(t, caller, []string{"r1"}) @@ -450,6 +608,10 @@ func TestCrossPlatformCoverageRecordDeletePartialCheckpointE2E(t *testing.T) { 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":{}}]}`}, }} diff --git a/internal/shortcut/minutes/workflows.go b/internal/shortcut/minutes/workflows.go index 68c11c8e..e9a44456 100644 --- a/internal/shortcut/minutes/workflows.go +++ b/internal/shortcut/minutes/workflows.go @@ -614,6 +614,17 @@ func executeMinutesShare(rt *shortcut.RuntimeContext) error { } func executeMinutesUnshare(rt *shortcut.RuntimeContext) error { + if !rt.DryRun() { + for _, id := range minutesIDs(rt) { + data, err := rt.CallMCPData("minutes", "get_minutes_basic_info", map[string]any{"taskUuid": id}) + if err != nil { + return fmt.Errorf("minutes unshare preflight for %s: %w", id, err) + } + if _, err := minutesdata.Basic(id, data); err != nil { + return fmt.Errorf("minutes unshare preflight for %s: %w", id, err) + } + } + } return executeMinutesPermissionLedger(rt, "unshare", "remove_member_permission", func(member string) map[string]any { return map[string]any{"uuids": minutesIDs(rt), "memberUids": []string{member}} }) @@ -631,7 +642,11 @@ func executeMinutesPermissionLedger(rt *shortcut.RuntimeContext, operation, tool for index, member := range members { data, err := rt.CallMCPWriteDataStrict("minutes", tool, params(member)) if err == nil { - err = minutesdata.RequireWriteAcknowledgement(operation, data) + if operation == "unshare" { + err = minutesdata.RequirePermissionMutationAcknowledgement(operation, minutesIDs(rt), []string{member}, data) + } else { + err = minutesdata.RequireWriteAcknowledgement(operation, data) + } } if err != nil { failures = append(failures, map[string]any{"memberUid": member, "error": err.Error()}) diff --git a/internal/shortcut/minutes/workflows_coverage_test.go b/internal/shortcut/minutes/workflows_coverage_test.go index aa05eb6d..f94f5e1a 100644 --- a/internal/shortcut/minutes/workflows_coverage_test.go +++ b/internal/shortcut/minutes/workflows_coverage_test.go @@ -323,7 +323,10 @@ func TestCrossPlatformCoverageMinutesPermissionLedgerBranchesE2E(t *testing.T) { if err := runMinutesAlignmentCLIWithWriter(t, &minutesE2ECaller{}, minutesFailWriter{}, "minutes", "+unshare", "--id", "u1", "--member-uids", "m1", "--dry-run"); err == nil { t.Fatal("unshare dry-run output failure accepted") } - unshare := &minutesE2ECaller{responses: map[string][]string{"minutes/remove_member_permission": {`{"success":true,"result":{}}`}}} + unshare := &minutesE2ECaller{responses: map[string][]string{ + "minutes/get_minutes_basic_info": {`{"success":true,"result":{"taskUuid":"u1"}}`}, + "minutes/remove_member_permission": {`{"success":true,"result":{"resultMap":{"u1":["m1"]}}}`}, + }} payload, _, err := runMinutesAlignmentCLI(t, unshare, "minutes", "+unshare", "--id", "u1", "--member-uids", "m1", "--yes") if err != nil || payload["complete"] != true || payload["succeeded"] != float64(1) { t.Fatalf("unshare payload=%#v err=%v", payload, err) @@ -337,11 +340,25 @@ func TestCrossPlatformCoverageMinutesPermissionLedgerBranchesE2E(t *testing.T) { if args["coverPermission"] != "true" || len(args["roleSubResourceIds"].([]string)) != 2 { t.Fatalf("share args=%#v", args) } - continueFailure := &minutesE2ECaller{responses: map[string][]string{"minutes/remove_member_permission": {`{"result":{}}`, `{"success":true,"result":{}}`}}} + continueFailure := &minutesE2ECaller{responses: map[string][]string{ + "minutes/get_minutes_basic_info": {`{"success":true,"result":{"taskUuid":"u1"}}`}, + "minutes/remove_member_permission": { + `{"result":{}}`, + `{"success":true,"result":{"resultMap":{"u1":["m2"]}}}`, + }, + }} payload, output, err := runMinutesAlignmentCLI(t, continueFailure, "minutes", "+unshare", "--id", "u1", "--member-uids", "m1,m2", "--failure-policy", "continue", "--yes") if err == nil || output == "" || payload["failed"] != float64(1) || payload["succeeded"] != float64(1) { t.Fatalf("continue ledger payload=%#v err=%v", payload, err) } + + missing := &minutesE2ECaller{responses: map[string][]string{ + "minutes/get_minutes_basic_info": {`{"success":true,"result":{}}`}, + }} + payload, output, err = runMinutesAlignmentCLI(t, missing, "minutes", "+unshare", "--id", "missing", "--member-uids", "m1", "--yes") + if err == nil || payload != nil || output != "" || missing.counts["minutes/remove_member_permission"] != 0 { + t.Fatalf("missing minutes reached unshare: payload=%#v output=%q err=%v calls=%#v", payload, output, err, missing.counts) + } } func TestCrossPlatformCoverageMinutesArtifactCollectorBranches(t *testing.T) { diff --git a/internal/shortcut/minutesdata/coverage_completion_test.go b/internal/shortcut/minutesdata/coverage_completion_test.go index e4d53737..d8b85dcb 100644 --- a/internal/shortcut/minutesdata/coverage_completion_test.go +++ b/internal/shortcut/minutesdata/coverage_completion_test.go @@ -224,6 +224,22 @@ func TestCrossPlatformCoverageMinutesWorkflowCompletion(t *testing.T) { if err := RequireWriteAcknowledgement("write", map[string]any{"result": map[string]any{"updated": true}}); err != nil { t.Fatal(err) } + if err := RequirePermissionMutationAcknowledgement("unshare", []string{"u1"}, []string{"202397"}, map[string]any{ + "success": true, + "result": map[string]any{"resultMap": map[string]any{"u1": []any{float64(202397)}}}, + }); err != nil { + t.Fatalf("valid permission acknowledgement rejected: %v", err) + } + 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{"other": []any{"202397"}}}}, + {"success": true, "result": map[string]any{"resultMap": map[string]any{"u1": []any{"other"}}}}, + } { + if err := RequirePermissionMutationAcknowledgement("unshare", []string{"u1"}, []string{"202397"}, data); err == nil { + t.Fatalf("invalid permission acknowledgement accepted: %#v", data) + } + } for _, tc := range []struct { cmd, id string data map[string]any diff --git a/internal/shortcut/minutesdata/workflow.go b/internal/shortcut/minutesdata/workflow.go index 6cfd1347..4ffc9a83 100644 --- a/internal/shortcut/minutesdata/workflow.go +++ b/internal/shortcut/minutesdata/workflow.go @@ -30,6 +30,61 @@ func RequireWriteAcknowledgement(operation string, data map[string]any) error { return fmt.Errorf("minutes %s response has no explicit successful acknowledgement", operation) } +// 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. +func RequirePermissionMutationAcknowledgement(operation string, taskUUIDs, memberUIDs []string, data map[string]any) error { + if err := validateEnvelope(data); err != nil { + return err + } + if success, ok := data["success"].(bool); !ok || !success { + return fmt.Errorf("minutes %s response has no explicit success=true", operation) + } + result, ok := data["result"].(map[string]any) + if !ok { + return fmt.Errorf("minutes %s response has no result object", operation) + } + resultMap, ok := result["resultMap"].(map[string]any) + if !ok { + return fmt.Errorf("minutes %s response has no result.resultMap object", operation) + } + if len(resultMap) != len(taskUUIDs) { + return fmt.Errorf("minutes %s resultMap covers %d task UUIDs, want %d", operation, len(resultMap), len(taskUUIDs)) + } + wantMembers := make(map[string]bool, len(memberUIDs)) + for _, member := range memberUIDs { + wantMembers[strings.TrimSpace(member)] = true + } + for _, taskUUID := range taskUUIDs { + rawMembers, exists := resultMap[taskUUID] + if !exists { + return fmt.Errorf("minutes %s resultMap is missing task UUID %s", operation, taskUUID) + } + members, ok := rawMembers.([]any) + if !ok { + return fmt.Errorf("minutes %s resultMap[%s] has type %T, want array", operation, taskUUID, rawMembers) + } + gotMembers := make(map[string]bool, len(members)) + for index, member := range members { + value := stringField(map[string]any{"member": member}, "member") + if value == "" { + return fmt.Errorf("minutes %s resultMap[%s][%d] is not a member UID", operation, taskUUID, index) + } + gotMembers[value] = true + } + if len(gotMembers) != len(wantMembers) { + return fmt.Errorf("minutes %s resultMap[%s] member coverage mismatch", operation, taskUUID) + } + for member := range wantMembers { + if !gotMembers[member] { + return fmt.Errorf("minutes %s resultMap[%s] is missing member UID %s", operation, taskUUID, member) + } + } + } + return nil +} + // RecordResult validates the gateway's observed listening-note command result. func RecordResult(expectedCmd, taskUUID string, data map[string]any) (map[string]any, error) { if err := RequireWriteAcknowledgement("record "+expectedCmd, data); err != nil {