test: add edge-case coverage for phase 1 hardening

Add boundary tests for depth limit, self-handoff cycle detection,
provider error propagation, JSON unmarshal invalid data, empty
blackboard list, hook observability on block, and no-hook/not-found
tool guard paths.

pkg/multiagent/blackboard_test.go

@@ -328,6 +328,55 @@ func TestBlackboardTool_SetBoard(t *testing.T) {
 	}
 }
 
+// TestBlackboard_UnmarshalJSON_InvalidData verifies that UnmarshalJSON returns an error
+// for malformed input instead of silently producing a broken blackboard.
+func TestBlackboard_UnmarshalJSON_InvalidData(t *testing.T) {
+	bb := NewBlackboard()
+	err := bb.UnmarshalJSON([]byte("not valid json"))
+	if err == nil {
+		t.Error("expected error for invalid JSON input")
+	}
+	// Board should remain empty after a failed unmarshal
+	if bb.Size() != 0 {
+		t.Errorf("Size() = %d after failed unmarshal, want 0", bb.Size())
+	}
+}
+
+// TestBlackboardTool_ListEmpty verifies the "Blackboard is empty" message path when
+// the board has no entries.
+func TestBlackboardTool_ListEmpty(t *testing.T) {
+	bb := NewBlackboard()
+	tool := NewBlackboardTool(bb, "lister")
+
+	result := tool.Execute(context.Background(), map[string]any{
+		"action": "list",
+	})
+	if result.IsError {
+		t.Fatalf("list on empty board should not error: %s", result.ForLLM)
+	}
+	if !contains(result.ForLLM, "empty") {
+		t.Errorf("expected 'empty' in result for empty board, got %q", result.ForLLM)
+	}
+}
+
+// TestBlackboardTool_DeleteMissing verifies the "not found" path for delete on a
+// key that does not exist.
+func TestBlackboardTool_DeleteMissing(t *testing.T) {
+	bb := NewBlackboard()
+	tool := NewBlackboardTool(bb, "deleter")
+
+	result := tool.Execute(context.Background(), map[string]any{
+		"action": "delete",
+		"key":    "nonexistent_key",
+	})
+	if result.IsError {
+		t.Fatalf("delete of missing key should not be an error: %s", result.ForLLM)
+	}
+	if !contains(result.ForLLM, "not found") {
+		t.Errorf("expected 'not found' in result, got %q", result.ForLLM)
+	}
+}
+
 func TestBoardAware_Interface(t *testing.T) {
 	// Verify both tools implement BoardAware
 	bb := NewBlackboard()

pkg/multiagent/handoff_test.go

@@ -2,6 +2,7 @@ package multiagent
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -600,6 +601,142 @@ func TestAllowlistCheckerFunc(t *testing.T) {
 	}
 }
 
+// TestExecuteHandoff_DepthBoundary verifies that depth == maxDepth - 1 (one below limit) succeeds,
+// while depth == maxDepth fails. This is the exact boundary behaviour of the recursion guard.
+func TestExecuteHandoff_DepthBoundary(t *testing.T) {
+	provider := &mockProvider{response: "done"}
+	resolver := newMockResolver(&AgentInfo{
+		ID: "target", Name: "Target", Model: "test",
+		Provider: provider, Tools: tools.NewToolRegistry(), MaxIter: 5,
+	})
+	bb := NewBlackboard()
+
+	// depth == maxDepth - 1 (2 < 3): must succeed
+	result := ExecuteHandoff(context.Background(), resolver, bb, HandoffRequest{
+		FromAgentID: "main",
+		ToAgentID:   "target",
+		Task:        "do something",
+		Depth:       2,
+		MaxDepth:    3,
+		Visited:     []string{"main", "middle"},
+	}, "cli", "direct")
+	if !result.Success {
+		t.Errorf("depth == maxDepth-1 should succeed, got error: %s", result.Error)
+	}
+
+	// depth == maxDepth (3 >= 3): must fail
+	result = ExecuteHandoff(context.Background(), resolver, bb, HandoffRequest{
+		FromAgentID: "main",
+		ToAgentID:   "target",
+		Task:        "do something",
+		Depth:       3,
+		MaxDepth:    3,
+		Visited:     []string{"main", "a", "b"},
+	}, "cli", "direct")
+	if result.Success {
+		t.Error("depth == maxDepth should fail")
+	}
+	if !strings.Contains(result.Error, "depth limit") {
+		t.Errorf("Error = %q, expected 'depth limit'", result.Error)
+	}
+}
+
+// TestExecuteHandoff_ProviderError verifies that a provider error during RunToolLoop
+// is surfaced as a failed HandoffResult with an error message.
+func TestExecuteHandoff_ProviderError(t *testing.T) {
+	provider := &mockProvider{err: fmt.Errorf("LLM provider unavailable")}
+	resolver := newMockResolver(&AgentInfo{
+		ID: "target", Name: "Target", Model: "test",
+		Provider: provider, Tools: tools.NewToolRegistry(), MaxIter: 5,
+	})
+
+	bb := NewBlackboard()
+	result := ExecuteHandoff(context.Background(), resolver, bb, HandoffRequest{
+		FromAgentID: "main",
+		ToAgentID:   "target",
+		Task:        "failing task",
+	}, "cli", "direct")
+
+	if result.Success {
+		t.Error("expected failure when provider returns error")
+	}
+	if !strings.Contains(result.Error, "provider unavailable") {
+		t.Errorf("Error = %q, expected provider error message", result.Error)
+	}
+	if result.AgentID != "target" {
+		t.Errorf("AgentID = %q, want 'target'", result.AgentID)
+	}
+}
+
+// TestExecuteHandoff_MaxIterDefault verifies that MaxIter == 0 on the target agent
+// is defaulted to 10 inside ExecuteHandoff (not left as 0 which would mean no iterations).
+func TestExecuteHandoff_MaxIterDefault(t *testing.T) {
+	provider := &mockProvider{response: "ran with default iter"}
+	resolver := newMockResolver(&AgentInfo{
+		ID:       "target",
+		Name:     "Target",
+		Model:    "test",
+		Provider: provider,
+		Tools:    tools.NewToolRegistry(),
+		MaxIter:  0, // explicitly zero, should default to 10
+	})
+
+	bb := NewBlackboard()
+	result := ExecuteHandoff(context.Background(), resolver, bb, HandoffRequest{
+		FromAgentID: "main",
+		ToAgentID:   "target",
+		Task:        "task with default iter",
+	}, "cli", "direct")
+
+	if !result.Success {
+		t.Errorf("expected success with default MaxIter, got: %s", result.Error)
+	}
+}
+
+// TestExecuteHandoff_CycleDetectionSingleHop verifies A->A (self-handoff) is caught.
+func TestExecuteHandoff_CycleDetectionSingleHop(t *testing.T) {
+	provider := &mockProvider{response: "done"}
+	resolver := newMockResolver(&AgentInfo{
+		ID: "main", Name: "Main", Model: "test",
+		Provider: provider, Tools: tools.NewToolRegistry(), MaxIter: 5,
+	})
+
+	bb := NewBlackboard()
+	// "main" handing off to itself, already in visited
+	result := ExecuteHandoff(context.Background(), resolver, bb, HandoffRequest{
+		FromAgentID: "main",
+		ToAgentID:   "main",
+		Task:        "self task",
+		Depth:       0,
+		Visited:     []string{"main"},
+	}, "cli", "direct")
+
+	if result.Success {
+		t.Error("expected failure for self-handoff cycle")
+	}
+	if !strings.Contains(result.Error, "cycle detected") {
+		t.Errorf("Error = %q, expected 'cycle detected'", result.Error)
+	}
+}
+
+// TestHandoffTool_SetContext verifies SetContext updates origin channel and chatID.
+func TestHandoffTool_SetContext(t *testing.T) {
+	resolver := newMockResolver()
+	bb := NewBlackboard()
+	tool := NewHandoffTool(resolver, bb, "main")
+
+	tool.SetContext("telegram", "chat-123")
+
+	// Verify fields are updated (access via the exported setter, values verified by ensuring
+	// no panic and the defaults were overwritten — integration confirmed via Execute routing).
+	if tool.originChannel != "telegram" {
+		t.Errorf("originChannel = %q, want %q", tool.originChannel, "telegram")
+	}
+	if tool.originChatID != "chat-123" {
+		t.Errorf("originChatID = %q, want %q", tool.originChatID, "chat-123")
+	}
+}
+
 func TestBuildHandoffSystemPrompt(t *testing.T) {
 	agent := &AgentInfo{
 		Name:         "Code Agent",

pkg/tools/hooks_test.go

@@ -111,3 +111,72 @@ func TestToolHook_FirstBlockStopsChain(t *testing.T) {
 		t.Error("hook2 before should NOT have been called (chain stopped)")
 	}
 }
+
+// TestToolHook_AfterExecuteRunsForAllHooksOnBlock verifies that when a BeforeExecute
+// hook blocks execution, AfterExecute is still invoked on ALL registered hooks
+// (not just the blocking one) for observability purposes.
+func TestToolHook_AfterExecuteRunsForAllHooksOnBlock(t *testing.T) {
+	reg := NewToolRegistry()
+	reg.Register(&dummyTool{name: "observed_tool"})
+
+	hook1 := &testHook{blockTool: "observed_tool"}
+	hook2 := &testHook{} // does not block, but should still get AfterExecute
+	reg.AddHook(hook1)
+	reg.AddHook(hook2)
+
+	result := reg.Execute(context.Background(), "observed_tool", nil)
+
+	if !result.IsError {
+		t.Error("expected error result when hook1 blocks")
+	}
+	// BeforeExecute: hook1 called, hook2 NOT called (chain stopped)
+	if len(hook1.beforeCalls) != 1 {
+		t.Errorf("hook1.beforeCalls = %d, want 1", len(hook1.beforeCalls))
+	}
+	if len(hook2.beforeCalls) != 0 {
+		t.Errorf("hook2.beforeCalls = %d, want 0 (chain stopped)", len(hook2.beforeCalls))
+	}
+	// AfterExecute: BOTH hooks called (inner loop over all hooks for observability)
+	if len(hook1.afterCalls) != 1 {
+		t.Errorf("hook1.afterCalls = %d, want 1", len(hook1.afterCalls))
+	}
+	if len(hook2.afterCalls) != 1 {
+		t.Errorf("hook2.afterCalls = %d, want 1 (AfterExecute runs for all hooks even on block)", len(hook2.afterCalls))
+	}
+}
+
+// TestToolHook_NotFoundToolSkipsHooks verifies that hooks are not called when
+// a tool does not exist in the registry.
+func TestToolHook_NotFoundToolSkipsHooks(t *testing.T) {
+	reg := NewToolRegistry()
+	// Do NOT register the tool
+
+	hook := &testHook{}
+	reg.AddHook(hook)
+
+	result := reg.Execute(context.Background(), "ghost_tool", nil)
+
+	if !result.IsError {
+		t.Error("expected error for unknown tool")
+	}
+	// Hooks should not be called when the tool doesn't exist (early return before hook loop)
+	if len(hook.beforeCalls) != 0 {
+		t.Errorf("hook.beforeCalls = %d, want 0 for missing tool", len(hook.beforeCalls))
+	}
+	if len(hook.afterCalls) != 0 {
+		t.Errorf("hook.afterCalls = %d, want 0 for missing tool", len(hook.afterCalls))
+	}
+}
+
+// TestToolHook_NoHooksSucceeds verifies that a tool executes normally with no hooks registered.
+func TestToolHook_NoHooksSucceeds(t *testing.T) {
+	reg := NewToolRegistry()
+	reg.Register(&dummyTool{name: "plain_tool"})
+	// No hooks added
+
+	result := reg.Execute(context.Background(), "plain_tool", nil)
+
+	if result.IsError {
+		t.Errorf("expected success with no hooks, got error: %s", result.ForLLM)
+	}
+}