fix: harden aitable pagination and minutes unshare

This commit is contained in:
Dennis committed 2026-08-17 14:13:33 +08:00
1 parent 8e4519cacd
commit fe2f3ca92f
9 files changed
+432 -17

No files matched your search

+2
View File
@@ -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":
+1 -1
View File
@@ -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)
},
}
+34 -11
View File
@@ -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 {
+126 -1
View File
@@ -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
}
@@ -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":{}}]}`},
}}
+16 -1
View File
@@ -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()})
@@ -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) {
@@ -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
+55
View File
@@ -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 {