github/dingtalk-workspace-cli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sheet): classify malformed revision responses

Browse Source
This commit is contained in:
xiatian
2026-08-20 23:06:56 +08:00
parent 57cca7ef71
commit e8ecff586a
2 changed files with 93 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/helpers/sheet_revision.go
+16 -2
View File
@@ -367,7 +367,11 @@ func decodeSheetRevisionResult(tool, raw string) (any, error) {
}
data, err := decodeSheetSingleJSON(raw)
if err != nil {
return nil, apperrors.NewInternal(fmt.Sprintf("解析 %s 返回失败: %v", tool, err))
return nil, invalidSheetRevisionResponse(tool,
fmt.Sprintf("MCP sheet read tool returned invalid JSON: %v", err),
"invalid_tool_response",
false,
)
}
object, ok := data.(map[string]any)
if !ok {
@@ -395,7 +399,12 @@ func decodeSheetRevisionResult(tool, raw string) (any, error) {
}
if tool == sheetChangesetGetRemoteTool {
if err := normalizeSheetChangesetTransport(object); err != nil {
return nil, apperrors.NewInternal(fmt.Sprintf("解析 %s 返回失败: %v%s", tool, err, sheetResultLogIDSuffix(object)))
return nil, invalidSheetRevisionResponse(tool,
fmt.Sprintf("MCP sheet read tool returned invalid changeset data: %v%s",
err, sheetResultLogIDSuffix(object)),
"invalid_tool_response",
false,
)
}
}
if err := validateSheetPublishedResult(tool, object); err != nil {
@@ -493,7 +502,12 @@ func validateSheetPublishedResult(tool string, object map[string]any) error {
default:
return fmt.Errorf("未知工具 %q", tool)
}
return validateSheetPublishedResultWithSchema(tool, object, rawSchema, cache)
}
func validateSheetPublishedResultWithSchema(tool string, object map[string]any,
rawSchema json.RawMessage, cache *sheetPublishedResultSchemaCache,
) error {
cache.once.Do(func() { cache.err = json.Unmarshal(rawSchema, &cache.schema) })
if cache.err != nil {
return fmt.Errorf("读取已发布 Result Schema 失败: %v", cache.err)
internal/helpers/sheet_revision_test.go
+77 -5
View File
@@ -260,11 +260,37 @@ func TestDecodeSheetChangesetTransportRejectsInvalidJSONStrings(t *testing.T) {
}
}
func TestDecodeSheetChangesetTransportPreservesLogIDInParseError(t *testing.T) {
const response = `{"success":true,"logId":"trace-malformed-changeset","changesetsJson":"not-json"}`
_, err := decodeSheetRevisionResult(sheetChangesetGetRemoteTool, response)
if err == nil || !strings.Contains(err.Error(), "logId=trace-malformed-changeset") {
t.Fatalf("parse error = %v, want backend logId", err)
func TestDecodeSheetRevisionProtocolFailuresHaveMCPResponseMetadata(t *testing.T) {
tests := []struct {
name string
tool string
response string
wantLogID string
}{
{
name: "malformed outer json", tool: sheetRevisionGetRemoteTool,
response: `{"success":true`,
},
{
name: "malformed changesets json", tool: sheetChangesetGetRemoteTool,
response: `{"success":true,"logId":"trace-malformed-changeset","changesetsJson":"not-json"}`,
wantLogID: "trace-malformed-changeset",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := decodeSheetRevisionResult(test.tool, test.response)
var apiErr *apperrors.Error
if !errors.As(err, &apiErr) || apiErr.Origin != "mcp" ||
apiErr.FailureStage != "response_validation" || apiErr.Reason != "invalid_tool_response" ||
apiErr.Operation != "sheet/"+test.tool || !apiErr.RetryableSet || apiErr.Retryable {
t.Fatalf("protocol failure error = %#v", err)
}
if test.wantLogID != "" && !strings.Contains(err.Error(), "logId="+test.wantLogID) {
t.Fatalf("protocol failure error = %v, want backend logId", err)
}
})
}
}
@@ -358,6 +384,11 @@ func TestSheetRevisionCommandsRejectPublishedResultContractDrift(t *testing.T) {
response: `{"success":true,"logId":" ","revision":1}`,
build: newSheetRevisionGetCmd, args: []string{"--node", "node-1"},
},
{
name: "revision negative integer", tool: sheetRevisionGetRemoteTool,
response: `{"success":true,"logId":"trace-contract","revision":-1}`,
build: newSheetRevisionGetCmd, args: []string{"--node", "node-1"},
},
{
name: "changeset missing summary", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{`+validChangesetPrefix+validChangesets+`}`),
@@ -383,6 +414,31 @@ func TestSheetRevisionCommandsRejectPublishedResultContractDrift(t *testing.T) {
response: encodeSheetChangesetTransport(t, `{`+validChangesetPrefix+`"summary":{"changeCount":0},`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset negative latest revision", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{"success":true,"logId":"trace-contract","schemaVersion":2,"changeSemantics":"FORWARD_ONLY","latestRevision":-1,"startRevision":0,"endRevision":0,`+validSummary+`,`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset negative start revision", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{"success":true,"logId":"trace-contract","schemaVersion":2,"changeSemantics":"FORWARD_ONLY","latestRevision":1,"startRevision":-1,"endRevision":0,`+validSummary+`,`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset negative end revision", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{"success":true,"logId":"trace-contract","schemaVersion":2,"changeSemantics":"FORWARD_ONLY","latestRevision":1,"startRevision":0,"endRevision":-1,`+validSummary+`,`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset start after end", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{"success":true,"logId":"trace-contract","schemaVersion":2,"changeSemantics":"FORWARD_ONLY","latestRevision":2,"startRevision":2,"endRevision":1,`+validSummary+`,`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset end after latest", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{"success":true,"logId":"trace-contract","schemaVersion":2,"changeSemantics":"FORWARD_ONLY","latestRevision":1,"startRevision":0,"endRevision":2,`+validSummary+`,`+validChangesets+`}`),
build: newSheetChangesetGetCmd, args: []string{"--node", "node-1", "--start-revision", "0"},
},
{
name: "changeset item missing required field", tool: sheetChangesetGetRemoteTool,
response: encodeSheetChangesetTransport(t, `{`+validChangesetPrefix+validSummary+`,"changesets":[{"revision":1,"isSelfEdit":true,"eventType":"EDIT","detailsStatus":"COMPLETE","changes":[]}]}`),
@@ -409,6 +465,22 @@ func TestSheetRevisionCommandsRejectPublishedResultContractDrift(t *testing.T) {
}
}
func TestValidateSheetPublishedResultCoversDefensiveFailures(t *testing.T) {
if err := validateSheetPublishedResult("unknown-tool", map[string]any{}); err == nil {
t.Fatal("unknown tool unexpectedly accepted")
}
cache := &sheetPublishedResultSchemaCache{}
if err := validateSheetPublishedResultWithSchema(sheetRevisionGetRemoteTool, map[string]any{},
json.RawMessage(`{`), cache); err == nil {
t.Fatal("invalid published schema unexpectedly accepted")
}
if _, err := sheetNonNegativeInteger(int64(1)); err == nil {
t.Fatal("non-json number unexpectedly accepted")
}
}
func TestDecodeSheetRevisionEmptyResponseHasStructuredRetryableError(t *testing.T) {
_, err := decodeSheetRevisionResult(sheetRevisionGetRemoteTool, " \n ")
var apiErr *apperrors.Error