feat: harden send preview idempotency

2026-07-10 19:24:53 +08:00
parent 138615a29c
commit 93112d6d99
8 changed files with 482 additions and 22 deletions
--- a/internal/tools/isphere_send_message_test.go
+++ b/internal/tools/isphere_send_message_test.go
@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"strings"
 	"testing"

 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -12,8 +13,9 @@ import (

 func TestISphereSendMessagePreviewPlansAndAuditsWithoutRawContent(t *testing.T) {
 	audit := &fakeSendMessageAuditSink{}
+	idempotency := newFakeSendMessageIdempotencyStore()
 	session, cleanup := connectToolsTestSession(t, func(server *mcp.Server) {
-		RegisterISphereSendMessageTool(server, audit)
+		RegisterISphereSendMessageToolWithState(server, audit, idempotency)
 	})
 	defer cleanup()

@@ -90,8 +92,9 @@ func TestISphereSendMessagePreviewPlansAndAuditsWithoutRawContent(t *testing.T)

 func TestISphereSendMessageProductionIsBlockedWithoutSideEffects(t *testing.T) {
 	audit := &fakeSendMessageAuditSink{}
+	idempotency := newFakeSendMessageIdempotencyStore()
 	session, cleanup := connectToolsTestSession(t, func(server *mcp.Server) {
-		RegisterISphereSendMessageTool(server, audit)
+		RegisterISphereSendMessageToolWithState(server, audit, idempotency)
 	})
 	defer cleanup()

@@ -145,7 +148,7 @@ func TestISphereSendMessageProductionIsBlockedWithoutSideEffects(t *testing.T) {

 func TestISphereSendMessageRejectsContentHashMismatch(t *testing.T) {
 	session, cleanup := connectToolsTestSession(t, func(server *mcp.Server) {
-		RegisterISphereSendMessageTool(server, &fakeSendMessageAuditSink{})
+		RegisterISphereSendMessageToolWithState(server, &fakeSendMessageAuditSink{}, newFakeSendMessageIdempotencyStore())
 	})
 	defer cleanup()

@@ -165,6 +168,171 @@ func TestISphereSendMessageRejectsContentHashMismatch(t *testing.T) {
 	}
 }

+func TestISphereSendMessageDetectsDuplicateIdempotencyKey(t *testing.T) {
+	audit := &fakeSendMessageAuditSink{}
+	idempotency := newFakeSendMessageIdempotencyStore()
+	session, cleanup := connectToolsTestSession(t, func(server *mcp.Server) {
+		RegisterISphereSendMessageToolWithState(server, audit, idempotency)
+	})
+	defer cleanup()
+
+	content := "duplicate preview"
+	contentHash := sha256HexForSendMessageTest(content)
+	args := map[string]any{
+		"target_type":     "direct",
+		"target_id":       "alice@imopenfire1-lanzhou",
+		"content_text":    content,
+		"content_sha256":  contentHash,
+		"idempotency_key": "idem-duplicate-1",
+		"execution_mode":  "preview",
+	}
+	if _, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: ToolNameSendMessage, Arguments: args}); err != nil {
+		t.Fatalf("first call: %v", err)
+	}
+	second, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: ToolNameSendMessage, Arguments: args})
+	if err != nil {
+		t.Fatalf("second call: %v", err)
+	}
+	if second.IsError {
+		t.Fatalf("duplicate should be structured preview metadata, got error: %+v", second)
+	}
+
+	var decoded struct {
+		OK          bool           `json:"ok"`
+		SendStatus  string         `json:"send_status"`
+		Idempotency map[string]any `json:"idempotency"`
+		SideEffects map[string]any `json:"side_effects"`
+	}
+	payload, _ := json.Marshal(second.StructuredContent)
+	if err := json.Unmarshal(payload, &decoded); err != nil {
+		t.Fatalf("decode duplicate payload %s: %v", payload, err)
+	}
+	if !decoded.OK || decoded.SendStatus != "planned" {
+		t.Fatalf("duplicate preview should remain planned/no-send: %s", payload)
+	}
+	if decoded.Idempotency["duplicate_detected"] != true || decoded.Idempotency["conflict_detected"] != false {
+		t.Fatalf("duplicate idempotency fields mismatch: %#v", decoded.Idempotency)
+	}
+	if decoded.Idempotency["first_audit_ref"] == "" {
+		t.Fatalf("duplicate response missing first_audit_ref: %#v", decoded.Idempotency)
+	}
+	if decoded.SideEffects["sent_message"] != false || decoded.SideEffects["clicked_ui"] != false {
+		t.Fatalf("duplicate preview should not have side effects: %#v", decoded.SideEffects)
+	}
+	if len(audit.events) != 2 || audit.events[1].Result != "planned" {
+		t.Fatalf("audit events = %+v, want two planned events", audit.events)
+	}
+}
+
+func TestISphereSendMessageBlocksIdempotencyKeyConflict(t *testing.T) {
+	audit := &fakeSendMessageAuditSink{}
+	idempotency := newFakeSendMessageIdempotencyStore()
+	session, cleanup := connectToolsTestSession(t, func(server *mcp.Server) {
+		RegisterISphereSendMessageToolWithState(server, audit, idempotency)
+	})
+	defer cleanup()
+
+	firstContent := "first body"
+	firstHash := sha256HexForSendMessageTest(firstContent)
+	_, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: ToolNameSendMessage, Arguments: map[string]any{
+		"target_type":     "direct",
+		"target_id":       "alice@imopenfire1-lanzhou",
+		"content_text":    firstContent,
+		"content_sha256":  firstHash,
+		"idempotency_key": "idem-conflict-1",
+		"execution_mode":  "preview",
+	}})
+	if err != nil {
+		t.Fatalf("first call: %v", err)
+	}
+
+	secondContent := "different body"
+	secondHash := sha256HexForSendMessageTest(secondContent)
+	second, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: ToolNameSendMessage, Arguments: map[string]any{
+		"target_type":     "direct",
+		"target_id":       "alice@imopenfire1-lanzhou",
+		"content_text":    secondContent,
+		"content_sha256":  secondHash,
+		"idempotency_key": "idem-conflict-1",
+		"execution_mode":  "preview",
+	}})
+	if err != nil {
+		t.Fatalf("conflict call: %v", err)
+	}
+	if second.IsError {
+		t.Fatalf("idempotency conflict should be structured blocked metadata, got error: %+v", second)
+	}
+
+	var decoded struct {
+		OK             bool           `json:"ok"`
+		SendStatus     string         `json:"send_status"`
+		BlockedReason  string         `json:"blocked_reason"`
+		ConnectorStage string         `json:"connector_stage"`
+		Idempotency    map[string]any `json:"idempotency"`
+		SideEffects    map[string]any `json:"side_effects"`
+	}
+	payload, _ := json.Marshal(second.StructuredContent)
+	if err := json.Unmarshal(payload, &decoded); err != nil {
+		t.Fatalf("decode conflict payload %s: %v", payload, err)
+	}
+	if decoded.OK || decoded.SendStatus != "blocked" || decoded.ConnectorStage != "blocked" {
+		t.Fatalf("conflict should be blocked: %s", payload)
+	}
+	if !strings.Contains(strings.ToLower(decoded.BlockedReason), "idempotency") {
+		t.Fatalf("blocked reason should explain idempotency conflict: %q", decoded.BlockedReason)
+	}
+	if decoded.Idempotency["duplicate_detected"] != false || decoded.Idempotency["conflict_detected"] != true {
+		t.Fatalf("conflict idempotency fields mismatch: %#v", decoded.Idempotency)
+	}
+	if decoded.SideEffects["sent_message"] != false || decoded.SideEffects["typed_text"] != false {
+		t.Fatalf("conflict should not have side effects: %#v", decoded.SideEffects)
+	}
+	if len(audit.events) != 2 || audit.events[1].Result != "blocked" {
+		t.Fatalf("audit events = %+v, want second blocked event", audit.events)
+	}
+}
+
+func TestFileSendMessageIdempotencyStoreDetectsDuplicateAndConflict(t *testing.T) {
+	store := fileSendMessageIdempotencyStore{path: t.TempDir() + "\\send-idempotency.jsonl"}
+	first := SendMessageIdempotencyRecord{
+		Tool:                 ToolNameSendMessage,
+		TargetRef:            "contact:alice@imopenfire1-lanzhou",
+		ContentSHA256:        sha256HexForSendMessageTest("first body"),
+		IdempotencyKeySHA256: sha256HexForSendMessageTest("idem-file-store"),
+		AuditRef:             "send-message:first",
+		StartedAt:            "2026-07-10T00:00:00Z",
+	}
+	firstDecision, err := store.ReserveSendMessageIdempotency(context.Background(), first)
+	if err != nil {
+		t.Fatalf("reserve first: %v", err)
+	}
+	if firstDecision.Duplicate || firstDecision.Conflict {
+		t.Fatalf("first decision = %+v, want fresh", firstDecision)
+	}
+
+	duplicateDecision, err := store.ReserveSendMessageIdempotency(context.Background(), first)
+	if err != nil {
+		t.Fatalf("reserve duplicate: %v", err)
+	}
+	if !duplicateDecision.Duplicate || duplicateDecision.Conflict || duplicateDecision.FirstAuditRef != first.AuditRef {
+		t.Fatalf("duplicate decision = %+v", duplicateDecision)
+	}
+
+	conflict := first
+	conflict.ContentSHA256 = sha256HexForSendMessageTest("changed body")
+	conflict.AuditRef = "send-message:conflict"
+	conflictDecision, err := store.ReserveSendMessageIdempotency(context.Background(), conflict)
+	if err != nil {
+		t.Fatalf("reserve conflict: %v", err)
+	}
+	if !conflictDecision.Conflict || conflictDecision.Duplicate {
+		t.Fatalf("conflict decision = %+v", conflictDecision)
+	}
+	if conflictDecision.FirstAuditRef != first.AuditRef {
+		t.Fatalf("conflict first audit ref = %q, want %q", conflictDecision.FirstAuditRef, first.AuditRef)
+	}
+}
+
 type fakeSendMessageAuditSink struct {
 	events []SendMessageAuditEvent
 }
@@ -174,6 +342,34 @@ func (f *fakeSendMessageAuditSink) AppendSendMessageAudit(_ context.Context, eve
 	return nil
 }

+type fakeSendMessageIdempotencyStore struct {
+	records []SendMessageIdempotencyRecord
+}
+
+func newFakeSendMessageIdempotencyStore() *fakeSendMessageIdempotencyStore {
+	return &fakeSendMessageIdempotencyStore{}
+}
+
+func (f *fakeSendMessageIdempotencyStore) ReserveSendMessageIdempotency(_ context.Context, record SendMessageIdempotencyRecord) (SendMessageIdempotencyDecision, error) {
+	for _, existing := range f.records {
+		if existing.IdempotencyKeySHA256 != record.IdempotencyKeySHA256 {
+			continue
+		}
+		decision := SendMessageIdempotencyDecision{
+			FirstAuditRef:  existing.AuditRef,
+			FirstStartedAt: existing.StartedAt,
+		}
+		if existing.TargetRef == record.TargetRef && existing.ContentSHA256 == record.ContentSHA256 {
+			decision.Duplicate = true
+			return decision, nil
+		}
+		decision.Conflict = true
+		return decision, nil
+	}
+	f.records = append(f.records, record)
+	return SendMessageIdempotencyDecision{}, nil
+}
+
 func sha256HexForSendMessageTest(value string) string {
 	sum := sha256.Sum256([]byte(value))
 	return hex.EncodeToString(sum[:])