From 5c0418349b25cccb7d8653376dccbc2d29df0493 Mon Sep 17 00:00:00 2001
From: xj <gh-xj@users.noreply.github.com>
Date: Wed, 25 Feb 2026 22:23:41 -0800
Subject: [PATCH] fix(lint): address golines and vet issues on hook/plugin
 changes

---
 pkg/agent/plugin_test.go             | 83 ++++++++++++++++++++++++++++
 pkg/hooks/hooks.go                   |  8 ++-
 pkg/hooks/hooks_test.go              | 10 +++-
 pkg/plugin/demoplugin/policy_demo.go |  8 +--
 pkg/plugin/manager.go                |  2 +-
 5 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/pkg/agent/plugin_test.go b/pkg/agent/plugin_test.go
index e208282b3..55b374f83 100644
--- a/pkg/agent/plugin_test.go
+++ b/pkg/agent/plugin_test.go
@@ -106,3 +106,86 @@ func TestSetHooksReturnsErrorWhenRunning(t *testing.T) {
 		t.Fatal("expected error when calling SetHooks while running")
 	}
 }
+
+func TestSetPluginManagerDoesNotPartiallyUpdateOnError(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-plugin-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	al := NewAgentLoop(cfg, msgBus, &mockProvider{})
+	al.running.Store(true)
+
+	pm := plugin.NewManager()
+	if err := pm.Register(blockingPlugin{}); err != nil {
+		t.Fatalf("register plugin: %v", err)
+	}
+
+	if err := al.SetPluginManager(pm); err == nil {
+		t.Fatal("expected SetPluginManager to fail while running")
+	}
+	if al.pluginManager != nil {
+		t.Fatal("expected plugin manager to remain unchanged on SetPluginManager failure")
+	}
+}
+
+func TestBeforeToolCallHooksCannotLeaveToolArgsNil(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-plugin-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &nilArgsProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	captureTool := &nilArgsCaptureTool{}
+	al.RegisterTool(captureTool)
+
+	r := hooks.NewHookRegistry()
+	r.OnBeforeToolCall("force-nil-args", 0, func(_ context.Context, e *hooks.BeforeToolCallEvent) error {
+		if e.ToolName == "nil_args_tool" {
+			e.Args = nil
+		}
+		return nil
+	})
+	if setErr := al.SetHooks(r); setErr != nil {
+		t.Fatalf("SetHooks: %v", setErr)
+	}
+
+	resp, err := al.ProcessDirectWithChannel(context.Background(), "run nil args test", "s1", "cli", "direct")
+	if err != nil {
+		t.Fatalf("ProcessDirectWithChannel: %v", err)
+	}
+	if resp != "done" {
+		t.Fatalf("expected final response 'done', got %q", resp)
+	}
+	if captureTool.receivedNil {
+		t.Fatal("expected tool args to be reinitialized to non-nil map")
+	}
+}
diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go
index 24dc8cbff..f1b9959fa 100644
--- a/pkg/hooks/hooks.go
+++ b/pkg/hooks/hooks.go
@@ -161,7 +161,13 @@ func triggerVoid[T any](ctx context.Context, hooks []HookRegistration[T], event
 
 // triggerModifying runs handlers sequentially by priority, stopping if Cancel is set.
 // The cancelCheck function inspects the event to determine if Cancel was set.
-func triggerModifying[T any](ctx context.Context, hooks []HookRegistration[T], event *T, hookName string, cancelCheck func(*T) bool) {
+func triggerModifying[T any](
+	ctx context.Context,
+	hooks []HookRegistration[T],
+	event *T,
+	hookName string,
+	cancelCheck func(*T) bool,
+) {
 	if len(hooks) == 0 {
 		return
 	}
diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go
index 427ec0d07..1f9aee18d 100644
--- a/pkg/hooks/hooks_test.go
+++ b/pkg/hooks/hooks_test.go
@@ -295,9 +295,13 @@ func TestConcurrentRegistrationAndTrigger(t *testing.T) {
 		wg.Add(1)
 		go func(idx int) {
 			defer wg.Done()
-			r.OnMessageReceived(fmt.Sprintf("reg-hook-%d", idx), idx, func(_ context.Context, _ *MessageReceivedEvent) error {
-				return nil
-			})
+			r.OnMessageReceived(
+				fmt.Sprintf("reg-hook-%d", idx),
+				idx,
+				func(_ context.Context, _ *MessageReceivedEvent) error {
+					return nil
+				},
+			)
 		}(i)
 	}
 
diff --git a/pkg/plugin/demoplugin/policy_demo.go b/pkg/plugin/demoplugin/policy_demo.go
index 2f1e4b47d..6e89b1dae 100644
--- a/pkg/plugin/demoplugin/policy_demo.go
+++ b/pkg/plugin/demoplugin/policy_demo.go
@@ -198,8 +198,8 @@ func normalizeLower(s string) string {
 	return strings.ToLower(strings.TrimSpace(s))
 }
 
-func clampArgNumber(args map[string]any, key string, max int) {
-	if args == nil || max <= 0 {
+func clampArgNumber(args map[string]any, key string, limit int) {
+	if args == nil || limit <= 0 {
 		return
 	}
 	v, ok := args[key]
@@ -210,8 +210,8 @@ func clampArgNumber(args map[string]any, key string, max int) {
 	if !ok {
 		return
 	}
-	if n > max {
-		args[key] = max
+	if n > limit {
+		args[key] = limit
 	}
 }
 
diff --git a/pkg/plugin/manager.go b/pkg/plugin/manager.go
index bf671bb20..edc2408ed 100644
--- a/pkg/plugin/manager.go
+++ b/pkg/plugin/manager.go
@@ -23,7 +23,7 @@ const APIVersion = "v1alpha1"
 type Plugin interface {
 	Name() string
 	APIVersion() string
-	Register(*hooks.HookRegistry) error
+	Register(registry *hooks.HookRegistry) error
 }
 
 // Manager owns a shared hook registry and loaded plugin metadata.