From 375740fdd5bd125eb6a1fd639c7a8640f4b93cef Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2026 03:45:39 +0000 Subject: [PATCH 1/3] provider/copilotprovider: gate approval-required tools Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- provider/copilotprovider/copilot.go | 48 ++++++- .../copilotprovider/copilot_internal_test.go | 126 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 provider/copilotprovider/copilot_internal_test.go diff --git a/provider/copilotprovider/copilot.go b/provider/copilotprovider/copilot.go index 1f6d9ee8..3f86af33 100644 --- a/provider/copilotprovider/copilot.go +++ b/provider/copilotprovider/copilot.go @@ -236,6 +236,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 +245,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 +254,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 +266,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 +293,49 @@ func copyBoolDefaultTrue(source *bool) *bool { return &value } +func sessionHooksWithApprovalRequests(tools []copilot.Tool, hooks *copilot.SessionHooks) *copilot.SessionHooks { + if hooks != nil && hooks.OnPreToolUse != nil { + return hooks + } + approvalRequiredToolNames := approvalRequiredToolNames(tools) + if len(approvalRequiredToolNames) == 0 { + return hooks + } + clone := cloneSessionHooks(hooks) + clone.OnPreToolUse = func(input copilot.PreToolUseHookInput, _ copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) { + if _, ok := approvalRequiredToolNames[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 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 { diff --git a/provider/copilotprovider/copilot_internal_test.go b/provider/copilotprovider/copilot_internal_test.go new file mode 100644 index 00000000..0f750488 --- /dev/null +++ b/provider/copilotprovider/copilot_internal_test.go @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft. All rights reserved. + +package copilotprovider + +import ( + "context" + "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 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 }, + ) +} From 53fbe7dc3f44e7d009cd570fbbdbe7b282224920 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:36:20 +0000 Subject: [PATCH 2/3] copilotprovider: warn when custom pre-tool hook skips auto-gating Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com> --- provider/copilotprovider/copilot.go | 20 +++++++++- .../copilotprovider/copilot_internal_test.go | 37 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/provider/copilotprovider/copilot.go b/provider/copilotprovider/copilot.go index 3f86af33..1d934a22 100644 --- a/provider/copilotprovider/copilot.go +++ b/provider/copilotprovider/copilot.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "iter" + "log/slog" "os" "path/filepath" "slices" @@ -294,10 +295,11 @@ func copyBoolDefaultTrue(source *bool) *bool { } func sessionHooksWithApprovalRequests(tools []copilot.Tool, hooks *copilot.SessionHooks) *copilot.SessionHooks { + approvalRequiredToolNames := approvalRequiredToolNames(tools) if hooks != nil && hooks.OnPreToolUse != nil { + logApprovalGatingSkipped(approvalRequiredToolNames) return hooks } - approvalRequiredToolNames := approvalRequiredToolNames(tools) if len(approvalRequiredToolNames) == 0 { return hooks } @@ -328,6 +330,22 @@ func approvalRequiredToolNames(tools []copilot.Tool) map[string]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{} diff --git a/provider/copilotprovider/copilot_internal_test.go b/provider/copilotprovider/copilot_internal_test.go index 0f750488..22056a67 100644 --- a/provider/copilotprovider/copilot_internal_test.go +++ b/provider/copilotprovider/copilot_internal_test.go @@ -3,7 +3,10 @@ package copilotprovider import ( + "bytes" "context" + "log/slog" + "strings" "testing" copilot "github.com/github/copilot-sdk/go" @@ -98,6 +101,40 @@ func TestSessionConfig_WithExistingPreToolUseHook_PreservesCallerHook(t *testing } } +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{} From 4468762c1d987f2a88118bdaf2670360e8af048e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:40:04 +0000 Subject: [PATCH 3/3] copilotprovider: cache approval-required tool names per hook setup Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com> --- provider/copilotprovider/copilot.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/provider/copilotprovider/copilot.go b/provider/copilotprovider/copilot.go index 1d934a22..96b4b989 100644 --- a/provider/copilotprovider/copilot.go +++ b/provider/copilotprovider/copilot.go @@ -295,17 +295,24 @@ func copyBoolDefaultTrue(source *bool) *bool { } func sessionHooksWithApprovalRequests(tools []copilot.Tool, hooks *copilot.SessionHooks) *copilot.SessionHooks { - approvalRequiredToolNames := approvalRequiredToolNames(tools) + 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(approvalRequiredToolNames) + logApprovalGatingSkipped(approvalRequiredNames()) return hooks } - if len(approvalRequiredToolNames) == 0 { + names = approvalRequiredNames() + if len(names) == 0 { return hooks } clone := cloneSessionHooks(hooks) clone.OnPreToolUse = func(input copilot.PreToolUseHookInput, _ copilot.HookInvocation) (*copilot.PreToolUseHookOutput, error) { - if _, ok := approvalRequiredToolNames[input.ToolName]; !ok { + if _, ok := names[input.ToolName]; !ok { return nil, nil } return &copilot.PreToolUseHookOutput{