From 6959ea7479eb8604ebac24b72ff0dd10bf5bb2e2 Mon Sep 17 00:00:00 2001
From: "Kai (Tam Nhu) Tran" <61256810+kaitranntt@users.noreply.github.com>
Date: Thu, 26 Mar 2026 01:39:50 -0400
Subject: [PATCH] feat(agent): exclude MCP bridge tools from read-only streak
 detector (#400)

MCP tools (mcp_*) are user-defined external integrations where
read-heavy workflows are expected and legitimate. The read-only
streak detector was designed for filesystem tool loops but caught
MCP tools in the default fallback, triggering false positives on
workflows like "query inbox + read 10 emails + summarize."

Treat mcp_* tools as neutral (same as exec/bash) since GoClaw
cannot determine whether an MCP tool is read or write.

Closes #399
---
 internal/agent/toolloop.go      |  9 +++++---
 internal/agent/toolloop_test.go | 39 +++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/internal/agent/toolloop.go b/internal/agent/toolloop.go
index a8c6f62e..f86e8add 100644
--- a/internal/agent/toolloop.go
+++ b/internal/agent/toolloop.go
@@ -118,14 +118,17 @@ func (s *toolLoopState) detect(toolName string, argsHash string) (level, message
 }
 
 // recordMutation updates the read-only streak based on tool type.
-// Mutating tools reset the streak; exec is neutral (ambiguous); all others increment.
+// Mutating tools reset the streak; exec/bash/mcp are neutral (ambiguous); all others increment.
 func (s *toolLoopState) recordMutation(toolName string) {
 	if mutatingTools[toolName] {
 		s.readOnlyStreak = 0
 		return
 	}
-	if toolName == "exec" || toolName == "bash" {
-		return // ambiguous — neither reset nor increment
+	// exec/bash: ambiguous (could be ls or rm).
+	// mcp_*: user-defined external tools — GoClaw cannot determine read vs write.
+	// Neither reset nor increment the read-only streak.
+	if toolName == "exec" || toolName == "bash" || strings.HasPrefix(toolName, "mcp_") {
+		return
 	}
 	s.readOnlyStreak++
 }
diff --git a/internal/agent/toolloop_test.go b/internal/agent/toolloop_test.go
index 3eca5758..630510aa 100644
--- a/internal/agent/toolloop_test.go
+++ b/internal/agent/toolloop_test.go
@@ -171,6 +171,45 @@ func TestReadOnlyStreak_ExecNeutral(t *testing.T) {
 	}
 }
 
+func TestReadOnlyStreak_MCPNeutral(t *testing.T) {
+	var s toolLoopState
+	// 5 reads → streak = 5
+	for range 5 {
+		s.recordMutation("read_file")
+	}
+	// MCP tools should not reset or increment (same as exec)
+	s.recordMutation("mcp_gmail__query_gmail_emails")
+	if s.readOnlyStreak != 5 {
+		t.Fatalf("expected streak 5 after mcp tool, got %d", s.readOnlyStreak)
+	}
+	s.recordMutation("mcp_gmail__get_gmail_email")
+	if s.readOnlyStreak != 5 {
+		t.Fatalf("expected streak 5 after second mcp tool, got %d", s.readOnlyStreak)
+	}
+	// 7 more reads → streak = 12, should hit critical
+	for range 7 {
+		s.recordMutation("list_files")
+	}
+	if s.readOnlyStreak != 12 {
+		t.Fatalf("expected streak 12, got %d", s.readOnlyStreak)
+	}
+}
+
+func TestReadOnlyStreak_MCPOnlyNeverTriggers(t *testing.T) {
+	var s toolLoopState
+	// 20 consecutive MCP tool calls → streak should stay 0
+	for range 20 {
+		s.recordMutation("mcp_gmail__query_gmail_emails")
+	}
+	if s.readOnlyStreak != 0 {
+		t.Fatalf("expected streak 0 after 20 mcp-only calls, got %d", s.readOnlyStreak)
+	}
+	level, _ := s.detectReadOnlyStreak()
+	if level != "" {
+		t.Fatalf("expected no detection for mcp-only calls, got %q", level)
+	}
+}
+
 // --- Same-result cross-args detection ---
 
 func TestSameResult_Warning(t *testing.T) {