Merge pull request #62 from hobbyistlabs-coder/feat/hitl-tool-approval-13341917865467500972

feat: Human-in-the-Loop (HITL) Tool Approval

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)

pkg/agent/loop.go

@@ -27,6 +27,7 @@ type AgentLoop struct {
 	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
@@ -37,6 +38,16 @@ type AgentLoop struct {
 	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
 type summaryJob struct {
 	agent      *AgentInstance

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)
 

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)
 }
 

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 }

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
+}

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) ---

pkg/tools/browser.go

@@ -250,3 +250,7 @@ func (t *BrowserActionTool) Close() {
 		t.pw = nil
 	}
 }
+
+func (t *BrowserActionTool) RequiresApproval() bool {
+	return false
+}

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
+}

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
+}

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
+}

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
+}

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
+}

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
+}

pkg/tools/mcp2cli.go

@@ -282,3 +282,7 @@ func splitQuoted(s string) []string {
 	}
 	return parts
 }
+
+func (t *MCP2CliTool) RequiresApproval() bool {
+	return false
+}

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
+}

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
+}

pkg/tools/registry.go

@@ -329,3 +329,7 @@ func (r *ToolRegistry) GetSummaries() []string {
 	}
 	return summaries
 }
+
+func (t *ToolRegistry) RequiresApproval() bool {
+	return false
+}

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 }

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
+}

pkg/tools/search_tools_test.go

@@ -337,3 +337,5 @@ func TestPromoteTools_ConcurrentWithTickTTL(t *testing.T) {
 	}
 	<-done
 }
+
+func (t *mockSearchableTool) RequiresApproval() bool { return false }

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
+}

pkg/tools/shell.go

@@ -418,3 +418,7 @@ func (t *ExecTool) SetAllowPatterns(patterns []string) error {
 	}
 	return nil
 }
+
+func (t *ExecTool) RequiresApproval() bool {
+	return true
+}

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
+}

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
+}

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
+}

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
+}

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
+}

pkg/tools/web/fetch_tool.go

@@ -0,0 +1,5 @@
+package web
+
+func (t *WebFetchTool) RequiresApproval() bool {
+	return false
+}

pkg/tools/web/search_tool.go

@@ -0,0 +1,5 @@
+package web
+
+func (t *WebSearchTool) RequiresApproval() bool {
+	return false
+}