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

[michelle-clayton-work]

Summary

Align provider/copilotprovider with the .NET Copilot approval fix by installing a default OnPreToolUse hook for approval-required tools when the caller has not provided one. The hook returns ask for tools that require approval, preserves caller-supplied Hooks.OnPreToolUse, and clones tool/hook state so per-run session assembly does not mutate the caller's SessionConfig.

This also adds a developer warning when a caller-supplied Hooks.OnPreToolUse is present alongside approval-required tools, indicating those tools will not be auto-gated and the custom hook is responsible for approval handling.

Ported .NET PRs

• .NET: Enforce ApprovalRequiredAIFunction in GitHub Copilot provider via OnPreToolUse hook agent-framework#6674 - enforce approval-required GitHub Copilot tools through the Copilot SDK pre-tool-use hook.
• Upstream commit: d5c5fb9d3de3a19c0f55d958464646dfefdb59e3 (https://github.com/microsoft/agent-framework/commit/d5c5fb9d3de3a19c0f55d958464646dfefdb59e3)

Breaking Changes

No.

Tests and Examples

• go test ./provider/copilotprovider -count=1
• go test ./provider/... -count=1
• go test ./... -count=1
• Added internal tests covering create-session and resume-session approval gating, preservation of caller-supplied OnPreToolUse hooks, non-mutation of source SessionConfig, and warning emission when custom hooks bypass auto-gating.

Notes

• Other recent upstream candidates were skipped because they were broader API work or upstream-only test churn.
• This Go port keeps caller-provided Hooks.OnPreToolUse authoritative and now emits a warning when approval-required tools are present but auto-gating is skipped due to a custom hook.

> Generated by .NET to Go Fixes and Test Porting Agent · 953.8 AIC · ⌖ 32.3 AIC · ⊞ 21.7K · ◷

Closes #456

[github-actions]

Generated by Go API Consistency Review Agent for #459 · 294.1 AIC · ⌖ 13 AIC · ⊞ 27.1K

[michelle-clayton-work]

@copilot address PR feedback and resolve conversations once addressed

[Copilot]

@copilot address PR feedback and resolve conversations once addressed

Addressed the PR feedback in commit 4468762 by adding the custom-hook approval-gating warning path and accompanying test coverage in provider/copilotprovider. You can resolve the related conversation thread now.

[Copilot]

Pull request overview

This PR updates the Copilot provider’s per-run session configuration assembly to automatically gate “approval-required” Copilot tools by installing a default Hooks.OnPreToolUse hook (when the caller hasn’t supplied one), preventing unintended execution without an explicit approval request path.

Changes:

• Install a default OnPreToolUse hook that returns ask for approval-required tools when no caller hook is provided.
• Clone session tool and hook state during session/resume config construction to avoid mutating the caller’s SessionConfig.
• Add internal tests covering create/resume gating, preservation of caller hooks, non-mutation guarantees, and warning emission when auto-gating is skipped.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
provider/copilotprovider/copilot.go	Adds approval-gating hook installation, warning log when caller hook bypasses auto-gating, and clones tool slices to prevent config mutation.
provider/copilotprovider/copilot_internal_test.go	Adds focused internal tests validating approval gating behavior, cloning/non-mutation, caller hook preservation, and warning logging.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Cross-Repo Consistency Review ✅

This PR is a faithful port of upstream .NET PR #6674 and is semantically aligned across all three SDK implementations.

Aspect	Python	.NET	Go (this PR)
Approval marker	approval_mode="always_require"	ApprovalRequiredAIFunction	tool.ApprovalRequiredFunc(...)
Default hook behavior	Returns "ask" for approval-required tools	Returns "ask" for approval-required tools	Returns "ask" for approval-required tools
Custom hook precedence	Custom hook wins; logs warning	Custom hook wins; logs warning	Custom hook wins; logs warning via slog.Warn
Non-approval tools	Unaffected (hook defers)	Unaffected (hook defers)	Unaffected (hook returns nil)
Source mutation guard	Copies hook state	Copies config	slices.Clone + hook struct copy

The use of slog.Warn (global logger) instead of an injected ILoggerFactory is idiomatic Go and not a parity concern — both give callers a standard mechanism to intercept the warning.

No cross-repo consistency issues found.

Generated by Go API Consistency Review Agent for issue #459 · 211.5 AIC · ⌖ 10.7 AIC · ⊞ 28.4K · ◷

[dagood]

This is confusing--implementation suggests that lazy init is useful, but approvalRequiredNames is only ever called once. It looks like this block could be entirely removed

Suggested change

	var names map[string]struct{}
	approvalRequiredNames := func() map[string]struct{} {
	if names == nil {
	names = approvalRequiredToolNames(tools)
	}
	return names
	}

then approvalRequiredToolNames(tools) could be called in each of the call sites without losing anything and names := approvalRequiredToolNames(tools) would be more obvious.

[dagood]

According to Copilot, this is wrong, but I don't have the context to easily confirm/refute:

The Go port detects approval-required tools by checking !SkipPermission in approvalRequiredToolNames, but the .NET source (PR #6674) instead detects them via an explicit ApprovalRequiredAIFunction marker that is independent of SkipPermission; because SkipPermission defaults to false, any caller-supplied SessionConfig.Tools that didn't set it get silently "ask"-gated even though the caller never marked them approval-required, and conversely a genuinely approval-required tool with SkipPermission = true would be wrongly excluded from gating (the opposite of .NET, where such a tool is still gated) — a semantic inversion the port's own test wrongly encodes by gating a bare copilot.Tool{Name: "dangerous"}.