From e763feb16f1d4ee6ad8d929bc33a6b9709cafbb9 Mon Sep 17 00:00:00 2001 From: Alexandre Balmes Date: Mon, 11 May 2026 22:43:23 +0200 Subject: [PATCH] fix(copilot): add `dangerously_skip_permissions` alias for `--allow-all` - `internal/infrastructure/agents/copilot_provider.go`: Map `dangerously_skip_permissions` option to `--allow-all` flag for cross-provider compatibility - `internal/infrastructure/agents/copilot_provider_unit_test.go`: Add tests for alias behavior, precedence when both options set, and false value omits flag --- .../infrastructure/agents/copilot_provider.go | 3 ++ .../agents/copilot_provider_unit_test.go | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/internal/infrastructure/agents/copilot_provider.go b/internal/infrastructure/agents/copilot_provider.go index 009a4cd6..2f138339 100644 --- a/internal/infrastructure/agents/copilot_provider.go +++ b/internal/infrastructure/agents/copilot_provider.go @@ -136,6 +136,9 @@ func appendCopilotOptions(args []string, options map[string]any) []string { } if allow, ok := getBoolOption(options, "allow_all"); ok && allow { args = append(args, "--allow-all") + } else if skip, ok := getBoolOption(options, "dangerously_skip_permissions"); ok && skip { + // Alias for cross-provider compatibility: dangerously_skip_permissions maps to --allow-all + args = append(args, "--allow-all") } return args } diff --git a/internal/infrastructure/agents/copilot_provider_unit_test.go b/internal/infrastructure/agents/copilot_provider_unit_test.go index be06e090..539a4c56 100644 --- a/internal/infrastructure/agents/copilot_provider_unit_test.go +++ b/internal/infrastructure/agents/copilot_provider_unit_test.go @@ -90,6 +90,13 @@ func TestCopilotProvider_Execute_WithOptions(t *testing.T) { mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--allow-all"}, }, + { + name: "dangerously_skip_permissions alias", + prompt: "test", + options: map[string]any{"dangerously_skip_permissions": true}, + mockStdout: `{"type":"assistant.message","data":{"content":"code","messageId":"m1"}}` + "\n" + `{"type":"result","sessionId":"s1","exitCode":0}`, + wantCLIArgs: []string{"-p", "test", "--output-format=json", "--silent", "--allow-all"}, + }, { name: "unknown options silently ignored", prompt: "test", @@ -477,6 +484,42 @@ func TestCopilotProvider_DeniedToolsMultiple(t *testing.T) { assert.Contains(t, args, "--deny-tool=python") } +func TestCopilotProvider_DangerouslySkipPermissions_AllowAllTakesPrecedence(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + options := map[string]any{"allow_all": true, "dangerously_skip_permissions": true} + _, err := provider.Execute(context.Background(), "test", options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + args := calls[0].Args + // allow_all takes precedence; --allow-all should appear exactly once + count := 0 + for _, a := range args { + if a == "--allow-all" { + count++ + } + } + assert.Equal(t, 1, count, "--allow-all should appear exactly once when both options are set") +} + +func TestCopilotProvider_DangerouslySkipPermissions_FalseOmitsFlag(t *testing.T) { + mockExec := mocks.NewMockCLIExecutor() + mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil) + provider := NewCopilotProviderWithOptions(WithCopilotExecutor(mockExec)) + + options := map[string]any{"dangerously_skip_permissions": false} + _, err := provider.Execute(context.Background(), "test", options, nil, nil) + + require.NoError(t, err) + calls := mockExec.GetCalls() + require.Len(t, calls, 1) + assert.NotContains(t, calls[0].Args, "--allow-all") +} + func TestCopilotProvider_AllowAllFalseOmitsFlag(t *testing.T) { mockExec := mocks.NewMockCLIExecutor() mockExec.SetOutput([]byte("{\"type\":\"assistant.message\",\"data\":{\"content\":\"code\",\"messageId\":\"m1\"}}\n{\"type\":\"result\",\"sessionId\":\"s1\",\"exitCode\":0}"), nil)