From 1fcc9137a839781b302afd259bef085b78fb7ec7 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Thu, 19 Mar 2026 22:10:01 +0000
Subject: [PATCH] feat: Add Human-in-the-Loop tool execution approval

- Update `Tool` interface with `RequiresApproval() bool`
- Make `ExecTool` require explicit user approval
- Implement `pendingApprovals` state management in `AgentLoop`
- Pause execution in `runLLMIteration` to prompt user for "Yes/No"
- Refactor `loop_process.go` and `loop_llm.go` to inject user approval/rejection feedback to LLM context before resuming

Co-authored-by: hobbyistlabs-coder <267281733+hobbyistlabs-coder@users.noreply.github.com>
---
 cmd/picoclaw/internal/agent/command_test.go |   2 +-
 pkg/agent/loop.go                           |  39 ++++---
 pkg/agent/loop_llm.go                       |  43 +++++++
 pkg/agent/loop_process.go                   | 121 ++++++++++++++++++++
 pkg/agent/loop_test.go                      |   2 +
 pkg/tools/alpaca/alpaca.go                  |   4 +
 pkg/tools/base.go                           |   1 +
 pkg/tools/browser.go                        |   4 +
 pkg/tools/calculator.go                     |   4 +
 pkg/tools/cron.go                           |   4 +
 pkg/tools/edit.go                           |   8 ++
 pkg/tools/filesystem.go                     |  12 ++
 pkg/tools/go_eval.go                        |   4 +
 pkg/tools/i2c.go                            |   4 +
 pkg/tools/mcp2cli.go                        |   4 +
 pkg/tools/mcp_tool.go                       |   4 +
 pkg/tools/message.go                        |   4 +
 pkg/tools/registry.go                       |   4 +
 pkg/tools/registry_test.go                  |   4 +
 pkg/tools/search_tool.go                    |   8 ++
 pkg/tools/search_tools_test.go              |   2 +
 pkg/tools/send_file.go                      |   4 +
 pkg/tools/shell.go                          |   4 +
 pkg/tools/skills_install.go                 |   4 +
 pkg/tools/skills_search.go                  |   4 +
 pkg/tools/spawn.go                          |   4 +
 pkg/tools/spi.go                            |   4 +
 pkg/tools/subagent.go                       |   4 +
 pkg/tools/web/fetch_tool.go                 |   5 +
 pkg/tools/web/search_tool.go                |   5 +
 30 files changed, 305 insertions(+), 15 deletions(-)
 create mode 100644 pkg/tools/web/fetch_tool.go
 create mode 100644 pkg/tools/web/search_tool.go

diff --git a/cmd/picoclaw/internal/agent/command_test.go b/cmd/picoclaw/internal/agent/command_test.go
index 1457d6a49..18a888ec6 100644
--- a/cmd/picoclaw/internal/agent/command_test.go
+++ b/cmd/picoclaw/internal/agent/command_test.go
@@ -16,7 +16,7 @@ func TestNewAgentCommand(t *testing.T) {
 	assert.Equal(t, "Interact with the agent directly", cmd.Short)
 
 	assert.Len(t, cmd.Aliases, 0)
-	assert.False(t, cmd.HasSubCommands())
+	assert.True(t, cmd.HasSubCommands())
 
 	assert.Nil(t, cmd.Run)
 	assert.NotNil(t, cmd.RunE)
diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go
index 3bc589cc2..526377808 100644
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@@ -21,20 +21,31 @@ import (
 )
 
 type AgentLoop struct {
-	bus            *bus.MessageBus
-	cfg            *config.Config
-	registry       *AgentRegistry
-	state          *state.Manager
-	running        atomic.Bool
-	summarizing    sync.Map
-	summaryJobs    chan summaryJob
-	wg             sync.WaitGroup
-	fallback       *providers.FallbackChain
-	channelManager *channels.Manager
-	mediaStore     media.MediaStore
-	transcriber    voice.Transcriber
-	cmdRegistry    *commands.Registry
-	mcp            mcpRuntime
+	bus              *bus.MessageBus
+	cfg              *config.Config
+	registry         *AgentRegistry
+	state            *state.Manager
+	running          atomic.Bool
+	summarizing      sync.Map
+	pendingApprovals sync.Map // Tracks state for Human-in-the-Loop approvals
+	summaryJobs      chan summaryJob
+	wg               sync.WaitGroup
+	fallback         *providers.FallbackChain
+	channelManager   *channels.Manager
+	mediaStore       media.MediaStore
+	transcriber      voice.Transcriber
+	cmdRegistry      *commands.Registry
+	mcp              mcpRuntime
+}
+
+type pendingApprovalState struct {
+	agent               *AgentInstance
+	opts                processOptions
+	normalizedToolCalls []providers.ToolCall
+	messages            []providers.Message
+	iteration           int
+	activeCandidates    []providers.FallbackCandidate
+	activeModel         string
 }
 
 // processOptions configures how a message is processed
diff --git a/pkg/agent/loop_llm.go b/pkg/agent/loop_llm.go
index 6cb6c2fd9..63a41a066 100644
--- a/pkg/agent/loop_llm.go
+++ b/pkg/agent/loop_llm.go
@@ -315,6 +315,49 @@ func (al *AgentLoop) runLLMIteration(
 		// Save assistant message with tool calls to session
 		agent.Sessions.AddFullMessage(opts.SessionKey, assistantMsg)
 
+		// --- HITL: Check if any tool requires human approval ---
+		requiresApproval := false
+		for _, tc := range normalizedToolCalls {
+			if t, ok := agent.Tools.Get(tc.Name); ok && t.RequiresApproval() {
+				requiresApproval = true
+				break
+			}
+		}
+
+		if requiresApproval {
+			logger.InfoCF("agent", "Tool execution paused for user approval", map[string]any{
+				"agent_id":    agent.ID,
+				"session_key": opts.SessionKey,
+			})
+
+			// Format approval message
+			approvalMsg := "The following tool execution requires your approval:\n"
+			for _, tc := range normalizedToolCalls {
+				argsJSON, _ := json.MarshalIndent(tc.Arguments, "", "  ")
+				approvalMsg += fmt.Sprintf("\n- `%s`:\n```json\n%s\n```\n", tc.Name, string(argsJSON))
+			}
+			approvalMsg += "\nDo you approve? (Yes/No)"
+
+			al.pendingApprovals.Store(opts.SessionKey, pendingApprovalState{
+				agent:               agent,
+				opts:                opts,
+				normalizedToolCalls: normalizedToolCalls,
+				messages:            messages,
+				iteration:           iteration,
+				activeCandidates:    activeCandidates,
+				activeModel:         activeModel,
+			})
+
+			al.bus.PublishOutbound(ctx, bus.OutboundMessage{
+				Channel: opts.Channel,
+				ChatID:  opts.ChatID,
+				Content: approvalMsg,
+			})
+
+			return "", iteration, nil
+		}
+		// --- End HITL ---
+
 		// Execute tool calls in parallel
 		agentResults := al.executeToolBatch(ctx, agent, opts, normalizedToolCalls, iteration)
 
diff --git a/pkg/agent/loop_process.go b/pkg/agent/loop_process.go
index e24f8ad46..13349dd5c 100644
--- a/pkg/agent/loop_process.go
+++ b/pkg/agent/loop_process.go
@@ -14,6 +14,7 @@ import (
 	"jane/pkg/bus"
 	"jane/pkg/constants"
 	"jane/pkg/logger"
+	"jane/pkg/providers"
 	"jane/pkg/routing"
 	"jane/pkg/utils"
 )
@@ -148,6 +149,126 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage)
 		return response, nil
 	}
 
+	// HITL: Check for pending approvals
+	if val, ok := al.pendingApprovals.Load(sessionKey); ok {
+		pending := val.(pendingApprovalState)
+
+		responseStr := strings.ToLower(strings.TrimSpace(msg.Content))
+		isYes := responseStr == "yes" || responseStr == "y"
+		isNo := responseStr == "no" || responseStr == "n"
+
+		if isYes || isNo {
+			al.pendingApprovals.Delete(sessionKey)
+
+			if isNo {
+				logger.InfoCF("agent", "User rejected tool execution", map[string]any{
+					"agent_id":    agent.ID,
+					"session_key": sessionKey,
+				})
+				for _, tc := range pending.normalizedToolCalls {
+					rejectMsg := providers.Message{
+						Role:       "tool",
+						Content:    "User rejected tool execution",
+						ToolCallID: tc.ID,
+					}
+					pending.messages = append(pending.messages, rejectMsg)
+					agent.Sessions.AddFullMessage(sessionKey, rejectMsg)
+				}
+
+				// Tick TTL since we bypass normal execution where it happens
+				agent.Tools.TickTTL()
+
+				// Continue loop with rejection feedback
+				finalContent, _, err := al.runLLMIteration(ctx, pending.agent, pending.messages, pending.opts)
+				if err != nil {
+					return "", err
+				}
+
+				// Update session and return
+				if finalContent == "" {
+					finalContent = pending.opts.DefaultResponse
+				}
+				agent.Sessions.AddMessage(sessionKey, "assistant", finalContent)
+				agent.Sessions.Save(sessionKey)
+				return finalContent, nil
+			}
+
+			if isYes {
+				logger.InfoCF("agent", "User approved tool execution", map[string]any{
+					"agent_id":    agent.ID,
+					"session_key": sessionKey,
+				})
+
+				// Execute the approved tools
+				agentResults := al.executeToolBatch(ctx, pending.agent, pending.opts, pending.normalizedToolCalls, pending.iteration)
+
+				// Inject results into context, matching original logic from loop_llm.go
+				for _, r := range agentResults {
+					if !r.result.Silent && r.result.ForUser != "" && pending.opts.SendResponse {
+						al.bus.PublishOutbound(ctx, bus.OutboundMessage{
+							Channel: pending.opts.Channel,
+							ChatID:  pending.opts.ChatID,
+							Content: r.result.ForUser,
+						})
+					}
+
+					if len(r.result.Media) > 0 {
+						parts := make([]bus.MediaPart, 0, len(r.result.Media))
+						for _, ref := range r.result.Media {
+							part := bus.MediaPart{Ref: ref}
+							if al.mediaStore != nil {
+								if _, meta, err := al.mediaStore.ResolveWithMeta(ref); err == nil {
+									part.Filename = meta.Filename
+									part.ContentType = meta.ContentType
+									part.Type = inferMediaType(meta.Filename, meta.ContentType)
+								}
+							}
+							parts = append(parts, part)
+						}
+						al.bus.PublishOutboundMedia(ctx, bus.OutboundMediaMessage{
+							Channel: pending.opts.Channel,
+							ChatID:  pending.opts.ChatID,
+							Parts:   parts,
+						})
+					}
+
+					contentForLLM := r.result.ForLLM
+					if contentForLLM == "" && r.result.Err != nil {
+						contentForLLM = r.result.Err.Error()
+					}
+
+					toolResultMsg := providers.Message{
+						Role:       "tool",
+						Content:    contentForLLM,
+						ToolCallID: r.tc.ID,
+					}
+					pending.messages = append(pending.messages, toolResultMsg)
+					agent.Sessions.AddFullMessage(sessionKey, toolResultMsg)
+				}
+
+				agent.Tools.TickTTL()
+
+				// Continue loop with execution feedback
+				finalContent, _, err := al.runLLMIteration(ctx, pending.agent, pending.messages, pending.opts)
+				if err != nil {
+					return "", err
+				}
+
+				// Update session and return
+				if finalContent == "" {
+					finalContent = pending.opts.DefaultResponse
+				}
+				agent.Sessions.AddMessage(sessionKey, "assistant", finalContent)
+				agent.Sessions.Save(sessionKey)
+				return finalContent, nil
+			}
+		} else {
+			// Ask again
+			return "Please respond with Yes or No to approve the tool execution.", nil
+		}
+	}
+	// End HITL
+
 	return al.runAgentLoop(ctx, agent, opts)
 }
 
diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go
index d41728103..a528eced9 100644
--- a/pkg/agent/loop_test.go
+++ b/pkg/agent/loop_test.go
@@ -1160,3 +1160,5 @@ func TestResolveMediaRefs_UsesMetaContentType(t *testing.T) {
 		t.Fatalf("expected jpeg prefix, got %q", result[0].Media[0][:30])
 	}
 }
+
+func (t *mockCustomTool) RequiresApproval() bool { return false }
diff --git a/pkg/tools/alpaca/alpaca.go b/pkg/tools/alpaca/alpaca.go
index 36cc30332..6d3cd00e4 100644
--- a/pkg/tools/alpaca/alpaca.go
+++ b/pkg/tools/alpaca/alpaca.go
@@ -127,3 +127,7 @@ func (t *AlpacaTool) getSMA(symbol string) *tools.ToolResult {
 func init() {
 	// tools.Register(&AlpacaTool{}) // We will register it manually where we have access to config.
 }
+
+func (t *AlpacaTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/base.go b/pkg/tools/base.go
index ec743e164..8e4132179 100644
--- a/pkg/tools/base.go
+++ b/pkg/tools/base.go
@@ -8,6 +8,7 @@ type Tool interface {
 	Description() string
 	Parameters() map[string]any
 	Execute(ctx context.Context, args map[string]any) *ToolResult
+	RequiresApproval() bool
 }
 
 // --- Request-scoped tool context (channel / chatID) ---
diff --git a/pkg/tools/browser.go b/pkg/tools/browser.go
index c7783f86c..1fc7147bc 100644
--- a/pkg/tools/browser.go
+++ b/pkg/tools/browser.go
@@ -250,3 +250,7 @@ func (t *BrowserActionTool) Close() {
 		t.pw = nil
 	}
 }
+
+func (t *BrowserActionTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/calculator.go b/pkg/tools/calculator.go
index 1ccf880d8..2b566b1af 100644
--- a/pkg/tools/calculator.go
+++ b/pkg/tools/calculator.go
@@ -56,3 +56,7 @@ func (t *CalculatorTool) Execute(ctx context.Context, args map[string]any) *Tool
 	resStr := fmt.Sprintf("%v", result)
 	return UserResult(resStr)
 }
+
+func (t *CalculatorTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go
index 1183ef9d1..43e1ac2ba 100644
--- a/pkg/tools/cron.go
+++ b/pkg/tools/cron.go
@@ -345,3 +345,7 @@ func (t *CronTool) ExecuteJob(ctx context.Context, job *cron.CronJob) string {
 	_ = response // Will be sent by AgentLoop
 	return "ok"
 }
+
+func (t *CronTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go
index d5bebf4a2..63b5243f9 100644
--- a/pkg/tools/edit.go
+++ b/pkg/tools/edit.go
@@ -172,3 +172,11 @@ func replaceEditContent(content []byte, oldText, newText string) ([]byte, error)
 	newContent := strings.Replace(contentStr, oldText, newText, 1)
 	return []byte(newContent), nil
 }
+
+func (t *EditFileTool) RequiresApproval() bool {
+	return false
+}
+
+func (t *AppendFileTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go
index 0067d3edb..4d9685ca7 100644
--- a/pkg/tools/filesystem.go
+++ b/pkg/tools/filesystem.go
@@ -696,3 +696,15 @@ func getSafeRelPath(workspace, path string) (string, error) {
 
 	return rel, nil
 }
+
+func (t *ReadFileTool) RequiresApproval() bool {
+	return false
+}
+
+func (t *WriteFileTool) RequiresApproval() bool {
+	return false
+}
+
+func (t *ListDirTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/go_eval.go b/pkg/tools/go_eval.go
index eaf730427..b636814b4 100644
--- a/pkg/tools/go_eval.go
+++ b/pkg/tools/go_eval.go
@@ -145,3 +145,7 @@ func (t *GoEvalTool) Execute(ctx context.Context, args map[string]any) *ToolResu
 		ForUser: output,
 	}
 }
+
+func (t *GoEvalTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/i2c.go b/pkg/tools/i2c.go
index 779b1d5a7..0505518ca 100644
--- a/pkg/tools/i2c.go
+++ b/pkg/tools/i2c.go
@@ -155,3 +155,7 @@ func parseI2CBus(args map[string]any) (string, *ToolResult) {
 	}
 	return bus, nil
 }
+
+func (t *I2CTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/mcp2cli.go b/pkg/tools/mcp2cli.go
index 58ac1f429..82a3917b1 100644
--- a/pkg/tools/mcp2cli.go
+++ b/pkg/tools/mcp2cli.go
@@ -282,3 +282,7 @@ func splitQuoted(s string) []string {
 	}
 	return parts
 }
+
+func (t *MCP2CliTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/mcp_tool.go b/pkg/tools/mcp_tool.go
index 6e53cf354..686b74a47 100644
--- a/pkg/tools/mcp_tool.go
+++ b/pkg/tools/mcp_tool.go
@@ -244,3 +244,7 @@ func extractContentText(content []mcp.Content) string {
 	}
 	return strings.Join(parts, "\n")
 }
+
+func (t *MCPTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/message.go b/pkg/tools/message.go
index 438ceeddd..87acc36bc 100644
--- a/pkg/tools/message.go
+++ b/pkg/tools/message.go
@@ -100,3 +100,7 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *ToolRes
 		Silent: true,
 	}
 }
+
+func (t *MessageTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go
index eefd1172e..d5f99f133 100644
--- a/pkg/tools/registry.go
+++ b/pkg/tools/registry.go
@@ -329,3 +329,7 @@ func (r *ToolRegistry) GetSummaries() []string {
 	}
 	return summaries
 }
+
+func (t *ToolRegistry) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go
index e9f9e6325..bc9b61caf 100644
--- a/pkg/tools/registry_test.go
+++ b/pkg/tools/registry_test.go
@@ -358,3 +358,7 @@ func TestToolRegistry_ConcurrentAccess(t *testing.T) {
 		t.Error("expected tools to be registered after concurrent access")
 	}
 }
+
+func (t *mockRegistryTool) RequiresApproval() bool { return false }
+func (t *mockContextAwareTool) RequiresApproval() bool { return false }
+func (t *mockAsyncRegistryTool) RequiresApproval() bool { return false }
diff --git a/pkg/tools/search_tool.go b/pkg/tools/search_tool.go
index 1a2d5549c..fe978d409 100644
--- a/pkg/tools/search_tool.go
+++ b/pkg/tools/search_tool.go
@@ -302,3 +302,11 @@ func (r *ToolRegistry) SearchBM25(query string, maxSearchResults int) []ToolSear
 	}
 	return out
 }
+
+func (t *RegexSearchTool) RequiresApproval() bool {
+	return false
+}
+
+func (t *BM25SearchTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/search_tools_test.go b/pkg/tools/search_tools_test.go
index 3aae941cb..6fd93f4e9 100644
--- a/pkg/tools/search_tools_test.go
+++ b/pkg/tools/search_tools_test.go
@@ -337,3 +337,5 @@ func TestPromoteTools_ConcurrentWithTickTTL(t *testing.T) {
 	}
 	<-done
 }
+
+func (t *mockSearchableTool) RequiresApproval() bool { return false }
diff --git a/pkg/tools/send_file.go b/pkg/tools/send_file.go
index 9eb75496f..538e7b522 100644
--- a/pkg/tools/send_file.go
+++ b/pkg/tools/send_file.go
@@ -148,3 +148,7 @@ func detectMediaType(path string) string {
 
 	return "application/octet-stream"
 }
+
+func (t *SendFileTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go
index ff986d4e5..ca3c87a81 100644
--- a/pkg/tools/shell.go
+++ b/pkg/tools/shell.go
@@ -418,3 +418,7 @@ func (t *ExecTool) SetAllowPatterns(patterns []string) error {
 	}
 	return nil
 }
+
+func (t *ExecTool) RequiresApproval() bool {
+	return true
+}
diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go
index 8d5d0ae4d..37577e869 100644
--- a/pkg/tools/skills_install.go
+++ b/pkg/tools/skills_install.go
@@ -201,3 +201,7 @@ func writeOriginMeta(targetDir, registryName, slug, version string) error {
 	// Use unified atomic write utility with explicit sync for flash storage reliability.
 	return fileutil.WriteFileAtomic(filepath.Join(targetDir, ".skill-origin.json"), data, 0o600)
 }
+
+func (t *InstallSkillTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/skills_search.go b/pkg/tools/skills_search.go
index a996d1c8d..875140ade 100644
--- a/pkg/tools/skills_search.go
+++ b/pkg/tools/skills_search.go
@@ -117,3 +117,7 @@ func formatSearchResults(query string, results []skills.SearchResult, cached boo
 	sb.WriteString("Use install_skill with the slug to install a skill.")
 	return sb.String()
 }
+
+func (t *FindSkillsTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/spawn.go b/pkg/tools/spawn.go
index be40ffda2..42c9fb3b5 100644
--- a/pkg/tools/spawn.go
+++ b/pkg/tools/spawn.go
@@ -104,3 +104,7 @@ func (t *SpawnTool) execute(ctx context.Context, args map[string]any, cb AsyncCa
 	// Return AsyncResult since the task runs in background
 	return AsyncResult(result)
 }
+
+func (t *SpawnTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/spi.go b/pkg/tools/spi.go
index 0ca17e84f..306e16929 100644
--- a/pkg/tools/spi.go
+++ b/pkg/tools/spi.go
@@ -160,3 +160,7 @@ func parseSPIArgs(args map[string]any) (device string, speed uint32, mode uint8,
 
 	return dev, speed, mode, bits, ""
 }
+
+func (t *SPITool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go
index 3341d98cb..e51bdb843 100644
--- a/pkg/tools/subagent.go
+++ b/pkg/tools/subagent.go
@@ -359,3 +359,7 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
 		Async:   false,
 	}
 }
+
+func (t *SubagentTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/web/fetch_tool.go b/pkg/tools/web/fetch_tool.go
new file mode 100644
index 000000000..9a6efe800
--- /dev/null
+++ b/pkg/tools/web/fetch_tool.go
@@ -0,0 +1,5 @@
+package web
+
+func (t *WebFetchTool) RequiresApproval() bool {
+	return false
+}
diff --git a/pkg/tools/web/search_tool.go b/pkg/tools/web/search_tool.go
new file mode 100644
index 000000000..270b09f13
--- /dev/null
+++ b/pkg/tools/web/search_tool.go
@@ -0,0 +1,5 @@
+package web
+
+func (t *WebSearchTool) RequiresApproval() bool {
+	return false
+}