[dotnet-port-fixes] Gate approval-required Copilot tools

provider/copilotprovider/copilot.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"iter"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"slices"
@@ -236,6 +237,7 @@ func (p *provider) sessionConfig(streaming bool, options []agent.Option) copilot
 	cfg.Streaming = copilot.Bool(streaming)
 	cfg.SystemMessage = systemMessageWithInstructions(cfg.SystemMessage, slices.Collect(agent.AllOptions(options, agent.WithInstructions)))
 	cfg.Tools = append(cfg.Tools, copilotTools(options)...)
+	cfg.Hooks = sessionHooksWithApprovalRequests(cfg.Tools, cfg.Hooks)
 	return cfg
 }
 
@@ -244,6 +246,7 @@ func (p *provider) resumeSessionConfig(streaming bool, options []agent.Option) c
 	cfg.Streaming = copilot.Bool(streaming)
 	cfg.SystemMessage = systemMessageWithInstructions(cfg.SystemMessage, slices.Collect(agent.AllOptions(options, agent.WithInstructions)))
 	cfg.Tools = append(cfg.Tools, copilotTools(options)...)
+	cfg.Hooks = sessionHooksWithApprovalRequests(cfg.Tools, cfg.Hooks)
 	return cfg
 }
 
@@ -252,6 +255,7 @@ func copySessionConfig(source *copilot.SessionConfig) copilot.SessionConfig {
 		return copilot.SessionConfig{Streaming: copilot.Bool(true)}
 	}
 	clone := *source
+	clone.Tools = slices.Clone(source.Tools)
 	clone.Streaming = copyBoolDefaultTrue(source.Streaming)
 	return clone
 }
@@ -263,7 +267,7 @@ func copyResumeSessionConfig(source *copilot.SessionConfig) copilot.ResumeSessio
 	return copilot.ResumeSessionConfig{
 		Model:               source.Model,
 		ReasoningEffort:     source.ReasoningEffort,
-		Tools:               source.Tools,
+		Tools:               slices.Clone(source.Tools),
 		SystemMessage:       source.SystemMessage,
 		AvailableTools:      source.AvailableTools,
 		ExcludedTools:       source.ExcludedTools,
@@ -290,6 +294,73 @@ func copyBoolDefaultTrue(source *bool) *bool {
 	return &value
 }
 
+func sessionHooksWithApprovalRequests(tools []copilot.Tool, hooks *copilot.SessionHooks) *copilot.SessionHooks {
+	var names map[string]struct{}
+	approvalRequiredNames := func() map[string]struct{} {
+		if names == nil {
+			names = approvalRequiredToolNames(tools)
+		}
+		return names
+	}
+	if hooks != nil && hooks.OnPreToolUse != nil {
+		logApprovalGatingSkipped(approvalRequiredNames())
+		return hooks
+	}
+	names = approvalRequiredNames()
+	if len(names) == 0 {
+		return hooks
+	}
+	clone := cloneSessionHooks(hooks)
+	clone.OnPreToolUse = func(input copilot.PreToolUseHookInput, _ copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) {
+		if _, ok := names[input.ToolName]; !ok {
+			return nil, nil
+		}
+		return &copilot.PreToolUseHookOutput{
+			PermissionDecision:       "ask",
+			PermissionDecisionReason: fmt.Sprintf("Tool %q requires approval before it can run.", input.ToolName),
+		}, nil
+	}
+	return clone
+}
+
+func approvalRequiredToolNames(tools []copilot.Tool) map[string]struct{} {
+	var names map[string]struct{}
+	for _, tl := range tools {
+		if tl.Name == "" || tl.SkipPermission {
+			continue
+		}
+		if names == nil {
+			names = make(map[string]struct{})
+		}
+		names[tl.Name] = struct{}{}
+	}
+	return names
+}
+
+func logApprovalGatingSkipped(names map[string]struct{}) {
+	if len(names) == 0 {
+		return
+	}
+	toolNames := make([]string, 0, len(names))
+	for name := range names {
+		toolNames = append(toolNames, name)
+	}
+	slices.Sort(toolNames)
+	slog.Warn(
+		"A custom OnPreToolUse hook is configured, so approval-required tools will not be automatically gated.",
+		"approvalRequiredToolCount", len(toolNames),
+		"approvalRequiredTools", strings.Join(toolNames, ", "),
+	)
+}
+
+func cloneSessionHooks(source *copilot.SessionHooks) *copilot.SessionHooks {
+	if source == nil {
+		return &copilot.SessionHooks{}
+	}
+	clone := *source
+	return &clone
+}
+
 func systemMessageWithInstructions(base *copilot.SystemMessageConfig, instructions []string) *copilot.SystemMessageConfig {
 	instructions = compactInstructions(instructions)
 	if len(instructions) == 0 {

provider/copilotprovider/copilot_internal_test.go

@@ -0,0 +1,163 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+package copilotprovider
+
+import (
+	"bytes"
+	"context"
+	"log/slog"
+	"strings"
+	"testing"
+
+	copilot "github.com/github/copilot-sdk/go"
+	"github.com/microsoft/agent-framework-go/agent"
+	"github.com/microsoft/agent-framework-go/tool"
+	"github.com/microsoft/agent-framework-go/tool/functool"
+)
+
+func TestSessionConfig_WithApprovalRequiredTool_InstallsAskPreToolUseHook(t *testing.T) {
+	dangerousTool := tool.ApprovalRequiredFunc(testFuncTool(t, "dangerous"))
+	plainTool := testFuncTool(t, "plain")
+	p := &provider{}
+
+	cfg := p.sessionConfig(true, []agent.Option{
+		agent.WithTool(dangerousTool),
+		agent.WithTool(plainTool),
+	})
+
+	if cfg.Hooks == nil || cfg.Hooks.OnPreToolUse == nil {
+		t.Fatal("OnPreToolUse hook was not installed")
+	}
+
+	dangerousDecision, err := cfg.Hooks.OnPreToolUse(copilot.PreToolUseHookInput{ToolName: "dangerous"}, copilot.HookInvocation{})
+	if err != nil {
+		t.Fatalf("OnPreToolUse(dangerous): %v", err)
+	}
+	if dangerousDecision == nil || dangerousDecision.PermissionDecision != "ask" {
+		t.Fatalf("dangerous permission decision = %#v, want ask", dangerousDecision)
+	}
+
+	plainDecision, err := cfg.Hooks.OnPreToolUse(copilot.PreToolUseHookInput{ToolName: "plain"}, copilot.HookInvocation{})
+	if err != nil {
+		t.Fatalf("OnPreToolUse(plain): %v", err)
+	}
+	if plainDecision != nil {
+		t.Fatalf("plain permission decision = %#v, want nil", plainDecision)
+	}
+}
+
+func TestSessionConfig_WithSessionConfigToolRequiringApproval_InstallsHookWithoutMutatingSource(t *testing.T) {
+	source := &copilot.SessionConfig{
+		Tools: []copilot.Tool{{Name: "dangerous"}},
+		Hooks: &copilot.SessionHooks{},
+	}
+	p := &provider{cfg: AgentConfig{SessionConfig: source}}
+
+	cfg := p.sessionConfig(true, []agent.Option{agent.WithTool(testFuncTool(t, "plain"))})
+
+	if source.Hooks.OnPreToolUse != nil {
+		t.Fatal("source hooks were mutated")
+	}
+	if len(source.Tools) != 1 {
+		t.Fatalf("source tools length = %d, want 1", len(source.Tools))
+	}
+	if len(cfg.Tools) != 2 {
+		t.Fatalf("session tools length = %d, want 2", len(cfg.Tools))
+	}
+	if cfg.Hooks == nil || cfg.Hooks == source.Hooks {
+		t.Fatal("session hooks were not cloned")
+	}
+	decision, err := cfg.Hooks.OnPreToolUse(copilot.PreToolUseHookInput{ToolName: "dangerous"}, copilot.HookInvocation{})
+	if err != nil {
+		t.Fatalf("OnPreToolUse(dangerous): %v", err)
+	}
+	if decision == nil || decision.PermissionDecision != "ask" {
+		t.Fatalf("dangerous permission decision = %#v, want ask", decision)
+	}
+}
+
+func TestSessionConfig_WithExistingPreToolUseHook_PreservesCallerHook(t *testing.T) {
+	expected := &copilot.PreToolUseHookOutput{PermissionDecision: "allow"}
+	source := &copilot.SessionConfig{
+		Hooks: &copilot.SessionHooks{
+			OnPreToolUse: func(copilot.PreToolUseHookInput, copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) {
+				return expected, nil
+			},
+		},
+	}
+	p := &provider{cfg: AgentConfig{SessionConfig: source}}
+
+	cfg := p.sessionConfig(true, []agent.Option{agent.WithTool(tool.ApprovalRequiredFunc(testFuncTool(t, "dangerous")))})
+
+	if cfg.Hooks == nil || cfg.Hooks.OnPreToolUse == nil {
+		t.Fatal("OnPreToolUse hook is nil")
+	}
+	got, err := cfg.Hooks.OnPreToolUse(copilot.PreToolUseHookInput{ToolName: "dangerous"}, copilot.HookInvocation{})
+	if err != nil {
+		t.Fatalf("OnPreToolUse: %v", err)
+	}
+	if got != expected {
+		t.Fatalf("permission decision = %#v, want %#v", got, expected)
+	}
+}
+
+func TestSessionConfig_WithExistingPreToolUseHookAndApprovalTools_LogsWarning(t *testing.T) {
+	var logs bytes.Buffer
+	defaultLogger := slog.Default()
+	slog.SetDefault(slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelWarn})))
+	t.Cleanup(func() {
+		slog.SetDefault(defaultLogger)
+	})
+
+	source := &copilot.SessionConfig{
+		Hooks: &copilot.SessionHooks{
+			OnPreToolUse: func(copilot.PreToolUseHookInput, copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) {
+				return nil, nil
+			},
+		},
+	}
+	p := &provider{cfg: AgentConfig{SessionConfig: source}}
+
+	_ = p.sessionConfig(true, []agent.Option{
+		agent.WithTool(tool.ApprovalRequiredFunc(testFuncTool(t, "dangerous-a"))),
+		agent.WithTool(tool.ApprovalRequiredFunc(testFuncTool(t, "dangerous-b"))),
+	})
+
+	logOutput := logs.String()
+	if !strings.Contains(logOutput, "not be automatically gated") {
+		t.Fatalf("expected warning log for skipped approval gating, got %q", logOutput)
+	}
+	if !strings.Contains(logOutput, "approvalRequiredToolCount=2") {
+		t.Fatalf("expected tool count in warning log, got %q", logOutput)
+	}
+	if !strings.Contains(logOutput, "dangerous-a, dangerous-b") {
+		t.Fatalf("expected tool names in warning log, got %q", logOutput)
+	}
+}
+
+func TestResumeSessionConfig_WithApprovalRequiredTool_InstallsAskPreToolUseHook(t *testing.T) {
+	p := &provider{}
+
+	cfg := p.resumeSessionConfig(true, []agent.Option{
+		agent.WithTool(tool.ApprovalRequiredFunc(testFuncTool(t, "dangerous"))),
+	})
+
+	if cfg.Hooks == nil || cfg.Hooks.OnPreToolUse == nil {
+		t.Fatal("OnPreToolUse hook was not installed")
+	}
+	decision, err := cfg.Hooks.OnPreToolUse(copilot.PreToolUseHookInput{ToolName: "dangerous"}, copilot.HookInvocation{})
+	if err != nil {
+		t.Fatalf("OnPreToolUse: %v", err)
+	}
+	if decision == nil || decision.PermissionDecision != "ask" {
+		t.Fatalf("dangerous permission decision = %#v, want ask", decision)
+	}
+}
+
+func testFuncTool(t *testing.T, name string) tool.FuncTool {
+	t.Helper()
+	return functool.MustNew(
+		functool.Config{Name: name, Description: name + " description"},
+		func(context.Context, struct{}) (string, error) { return "ok", nil },
+	)
+}