diff --git a/docs/troubleshooting/compatibility.md b/docs/troubleshooting/compatibility.md index 3238c59d91..da8bf0daae 100644 --- a/docs/troubleshooting/compatibility.md +++ b/docs/troubleshooting/compatibility.md @@ -77,7 +77,7 @@ The Copilot SDK communicates with the CLI via JSON-RPC protocol. Features must b | System message | `systemMessage` config | Append or replace | | Custom provider | `provider` config | BYOK support | | Infinite sessions | `infiniteSessions` config | Auto-compaction | -| Permission handler | `onPermissionRequest` | Approve/deny requests | +| Permission handler | `onPermissionRequest` | Approve/deny requests; optionally attach a `decisionContext` for auto-approval telemetry | | User input handler | `onUserInputRequest` | Handle ask_user | | Skills | `skillDirectories` config | Custom skills | | Disabled skills | `disabledSkills` config | Disable specific skills | diff --git a/dotnet/src/PermissionDecision.cs b/dotnet/src/PermissionDecision.cs index 54e1237917..3eb1d0e08a 100644 --- a/dotnet/src/PermissionDecision.cs +++ b/dotnet/src/PermissionDecision.cs @@ -43,4 +43,13 @@ public static PermissionDecision Reject(string? feedback = null) => /// connected client to answer instead. /// public static PermissionDecision NoResult() => new PermissionDecisionNoResult(); + + /// + /// Optional provenance describing how and where this decision was made. + /// This is never serialized as part of the decision itself: the SDK forwards + /// it to the runtime as a sibling of result so that auto-approval + /// telemetry can be attributed correctly. + /// + [JsonIgnore] + public PermissionDecisionContext? DecisionContext { get; set; } } diff --git a/dotnet/src/Session.cs b/dotnet/src/Session.cs index 7c34ded166..0ce10c2909 100644 --- a/dotnet/src/Session.cs +++ b/dotnet/src/Session.cs @@ -954,7 +954,7 @@ private async Task ExecutePermissionAndRespondAsync(string requestId, Permission return; } var responseRpcTimestamp = Stopwatch.GetTimestamp(); - await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, decision); + await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, decision, decision.DecisionContext); LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null, "CopilotSession.ExecutePermissionAndRespondAsync response sent successfully. Elapsed={Elapsed}, SessionId={SessionId}, RequestId={RequestId}", responseRpcTimestamp, diff --git a/dotnet/test/Unit/ClientSessionLifetimeTests.cs b/dotnet/test/Unit/ClientSessionLifetimeTests.cs index d4b4100b4c..edc2fa8b16 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -564,6 +564,208 @@ public async Task CreateSessionAsync_Serializes_ManagedSettings_Permissions() Assert.True(invocation.ManagedSettingsEnabled); } + [Fact] + public async Task PermissionResponse_Forwards_DecisionContext_As_Sibling_Of_Result() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await client.StartAsync(); + + await using var session = await client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = (_, _) => Task.FromResult( + new PermissionDecisionApproveOnce + { + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutoApproved, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + } + }) + }); + + DispatchEvent(session, new PermissionRequestedEvent + { + Data = new PermissionRequestedData + { + PermissionRequest = new PermissionRequest { Kind = "read" }, + RequestId = "req-with-context" + } + }); + + var request = await WaitForRequestAsync(server, "session.permissions.handlePendingPermissionRequest"); + + Assert.True(request.Params.TryGetProperty("decisionContext", out var decisionContext)); + Assert.Equal("auto_approved", decisionContext.GetProperty("outcome").GetString()); + Assert.Equal("host_policy", decisionContext.GetProperty("source").GetString()); + Assert.Equal("sdk", decisionContext.GetProperty("surface").GetString()); + + var result = request.Params.GetProperty("result"); + Assert.Equal("approve-once", result.GetProperty("kind").GetString()); + Assert.False(result.TryGetProperty("decisionContext", out _)); + } + + [Fact] + public async Task PermissionResponse_Omits_DecisionContext_When_Not_Supplied() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await client.StartAsync(); + + await using var session = await client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = (_, _) => Task.FromResult(PermissionDecision.ApproveOnce()) + }); + + DispatchEvent(session, new PermissionRequestedEvent + { + Data = new PermissionRequestedData + { + PermissionRequest = new PermissionRequest { Kind = "read" }, + RequestId = "req-no-context" + } + }); + + var request = await WaitForRequestAsync(server, "session.permissions.handlePendingPermissionRequest"); + + Assert.False(request.Params.TryGetProperty("decisionContext", out _)); + var result = request.Params.GetProperty("result"); + Assert.Equal("approve-once", result.GetProperty("kind").GetString()); + Assert.False(result.TryGetProperty("decisionContext", out _)); + } + + [Fact] + public async Task PermissionResponse_Uses_Latest_Context_When_Reassigned() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await client.StartAsync(); + + await using var session = await client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = (_, _) => + { + var decision = new PermissionDecisionApproveOnce + { + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.PromptedUser, + Source = PermissionDecisionSource.HumanResponse, + Surface = PermissionDecisionSurface.Tui + } + }; + decision.DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutoApproved, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + }; + return Task.FromResult(decision); + } + }); + + DispatchEvent(session, new PermissionRequestedEvent + { + Data = new PermissionRequestedData + { + PermissionRequest = new PermissionRequest { Kind = "read" }, + RequestId = "req-replace-context" + } + }); + + var request = await WaitForRequestAsync(server, "session.permissions.handlePendingPermissionRequest"); + + var decisionContext = request.Params.GetProperty("decisionContext"); + Assert.Equal("auto_approved", decisionContext.GetProperty("outcome").GetString()); + Assert.Equal("host_policy", decisionContext.GetProperty("source").GetString()); + Assert.Equal("sdk", decisionContext.GetProperty("surface").GetString()); + } + + [Fact] + public async Task PermissionResponse_Is_Suppressed_For_NoResult_Even_With_Context() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await client.StartAsync(); + + var handlerInvoked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + await using var session = await client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = (_, _) => + { + handlerInvoked.TrySetResult(); + return Task.FromResult( + new PermissionDecisionNoResult + { + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.PromptedUser, + Source = PermissionDecisionSource.HumanResponse, + Surface = PermissionDecisionSurface.Sdk + } + }); + } + }); + + DispatchEvent(session, new PermissionRequestedEvent + { + Data = new PermissionRequestedData + { + PermissionRequest = new PermissionRequest { Kind = "read" }, + RequestId = "req-no-result" + } + }); + + await handlerInvoked.Task.WaitAsync(TimeSpan.FromSeconds(5)); + // Give the send path a chance to (incorrectly) fire before asserting suppression. + await Task.Delay(200); + + Assert.DoesNotContain(server.Requests, request => request.Method == "session.permissions.handlePendingPermissionRequest"); + } + + [Fact] + public async Task PermissionResponse_Never_Nests_DecisionContext_Inside_Result() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await client.StartAsync(); + + await using var session = await client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = (_, _) => Task.FromResult( + new PermissionDecisionReject + { + Feedback = "denied by policy", + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutopilotDenied, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + } + }) + }); + + DispatchEvent(session, new PermissionRequestedEvent + { + Data = new PermissionRequestedData + { + PermissionRequest = new PermissionRequest { Kind = "read" }, + RequestId = "req-reject-context" + } + }); + + var request = await WaitForRequestAsync(server, "session.permissions.handlePendingPermissionRequest"); + + var result = request.Params.GetProperty("result"); + Assert.Equal("reject", result.GetProperty("kind").GetString()); + Assert.Equal("denied by policy", result.GetProperty("feedback").GetString()); + // The context provenance must never be serialized inside the decision itself. + Assert.False(result.TryGetProperty("decisionContext", out _)); + // It is forwarded as a sibling instead. + Assert.True(request.Params.TryGetProperty("decisionContext", out _)); + } + [Fact] public async Task CreateSessionAsync_Omits_ManagedSettings_When_Unset() { @@ -822,6 +1024,10 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel { ["success"] = true }, + "session.permissions.handlePendingPermissionRequest" => new Dictionary + { + ["success"] = true + }, "session.delete" => new Dictionary { ["success"] = true diff --git a/go/permission_context_test.go b/go/permission_context_test.go new file mode 100644 index 0000000000..16c6d2d592 --- /dev/null +++ b/go/permission_context_test.go @@ -0,0 +1,260 @@ +package copilot + +import ( + "encoding/json" + "fmt" + "io" + "testing" + "time" + + "github.com/github/copilot-sdk/go/internal/jsonrpc2" + "github.com/github/copilot-sdk/go/rpc" +) + +// runPermissionExchange drives executePermissionAndRespond with the supplied +// handler and captures the raw JSON-RPC request frame the SDK emits (if any). +// The second return value reports whether a request was sent at all, so tests +// can assert that no-result decisions suppress the response entirely. +func runPermissionExchange(t *testing.T, handler PermissionHandlerFunc) (frame []byte, sent bool) { + t.Helper() + + stdinR, stdinW := io.Pipe() + stdoutR, stdoutW := io.Pipe() + t.Cleanup(func() { + stdinR.Close() + stdinW.Close() + stdoutR.Close() + stdoutW.Close() + }) + + client := jsonrpc2.NewClient(stdinW, stdoutR) + client.Start() + t.Cleanup(client.Stop) + + session := &Session{ + SessionID: "session-1", + client: client, + RPC: rpc.NewSessionRPC(client, "session-1"), + } + + frameCh := make(chan []byte, 1) + go func() { + captured, err := readTestJSONRPCFrame(stdinR) + if err != nil { + return + } + var request struct { + ID json.RawMessage `json:"id"` + } + _ = json.Unmarshal(captured, &request) + // Publish the captured frame before unblocking the RPC round trip so a + // sent response is always observable before executePermissionAndRespond + // returns. + frameCh <- captured + response := map[string]any{ + "jsonrpc": "2.0", + "id": json.RawMessage(request.ID), + "result": map[string]any{"applied": true}, + } + data, _ := json.Marshal(response) + _, _ = fmt.Fprintf(stdoutW, "Content-Length: %d\r\n\r\n%s", len(data), data) + }() + + done := make(chan struct{}) + go func() { + session.executePermissionAndRespond("permission-1", nil, handler) + close(done) + }() + + select { + case captured := <-frameCh: + return captured, true + case <-done: + select { + case captured := <-frameCh: + return captured, true + default: + return nil, false + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for permission response") + return nil, false + } +} + +// paramsOf extracts the top-level params object from a JSON-RPC request frame. +func paramsOf(t *testing.T, frame []byte) map[string]json.RawMessage { + t.Helper() + var request struct { + Method string `json:"method"` + Params map[string]json.RawMessage `json:"params"` + } + if err := json.Unmarshal(frame, &request); err != nil { + t.Fatalf("failed to unmarshal request frame: %v", err) + } + if request.Method != "session.permissions.handlePendingPermissionRequest" { + t.Fatalf("unexpected method %q", request.Method) + } + return request.Params +} + +func sampleDecisionContext() *rpc.PermissionDecisionContext { + return &rpc.PermissionDecisionContext{ + Outcome: PermissionDecisionOutcomeAutoApproved, + Source: PermissionDecisionSourceHostPolicy, + Surface: PermissionDecisionSurfaceSDK, + } +} + +func TestPermissionDecisionContextForwardedAsSiblingOfResult(t *testing.T) { + frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { + return NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil + }) + if !sent { + t.Fatal("expected a permission response to be sent") + } + + params := paramsOf(t, frame) + + // decisionContext must be a top-level sibling of result. + rawContext, ok := params["decisionContext"] + if !ok { + t.Fatal("expected decisionContext to be present as a top-level sibling of result") + } + var context rpc.PermissionDecisionContext + if err := json.Unmarshal(rawContext, &context); err != nil { + t.Fatalf("failed to unmarshal decisionContext: %v", err) + } + if context.Outcome != PermissionDecisionOutcomeAutoApproved || + context.Source != PermissionDecisionSourceHostPolicy || + context.Surface != PermissionDecisionSurfaceSDK { + t.Fatalf("unexpected decisionContext contents: %#v", context) + } + + // result must exist and must NOT contain a nested decisionContext. + rawResult, ok := params["result"] + if !ok { + t.Fatal("expected result to be present") + } + var result map[string]json.RawMessage + if err := json.Unmarshal(rawResult, &result); err != nil { + t.Fatalf("failed to unmarshal result: %v", err) + } + if _, nested := result["decisionContext"]; nested { + t.Fatal("decisionContext must not be nested inside result") + } +} + +func TestPermissionDecisionContextOmittedWithoutAttribution(t *testing.T) { + frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { + return &rpc.PermissionDecisionApproveOnce{}, nil + }) + if !sent { + t.Fatal("expected a permission response to be sent") + } + + params := paramsOf(t, frame) + if _, ok := params["decisionContext"]; ok { + t.Fatal("expected decisionContext to be absent when no context is supplied") + } + if _, ok := params["result"]; !ok { + t.Fatal("expected result to be present") + } +} + +func TestAttributedResultReplacesRatherThanNests(t *testing.T) { + first := &rpc.PermissionDecisionContext{ + Outcome: PermissionDecisionOutcomePromptedUser, + Source: PermissionDecisionSourceHumanResponse, + Surface: PermissionDecisionSurfaceTui, + } + second := sampleDecisionContext() + + wrapped := NewAttributedPermissionResult(NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, first), second) + + if wrapped.DecisionContext != second { + t.Fatalf("expected the second context to replace the first, got %#v", wrapped.DecisionContext) + } + // The underlying decision must be the plain approve-once, not another wrapper. + if _, ok := wrapped.PermissionDecision.(*rpc.PermissionDecisionApproveOnce); !ok { + t.Fatalf("expected unwrapped decision to be *rpc.PermissionDecisionApproveOnce, got %T", wrapped.PermissionDecision) + } + + frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { + return wrapped, nil + }) + if !sent { + t.Fatal("expected a permission response to be sent") + } + params := paramsOf(t, frame) + rawContext, ok := params["decisionContext"] + if !ok { + t.Fatal("expected decisionContext to be present") + } + var context rpc.PermissionDecisionContext + if err := json.Unmarshal(rawContext, &context); err != nil { + t.Fatalf("failed to unmarshal decisionContext: %v", err) + } + if context.Surface != PermissionDecisionSurfaceSDK { + t.Fatalf("expected replaced surface %q, got %q", PermissionDecisionSurfaceSDK, context.Surface) + } +} + +func TestAttributedNoResultStillSuppressesResponse(t *testing.T) { + frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { + return NewAttributedPermissionResult(&rpc.PermissionDecisionNoResult{}, sampleDecisionContext()), nil + }) + if sent { + t.Fatalf("expected no response to be sent for an attributed no-result decision, got frame: %s", frame) + } +} + +// A handler may dereference the wrapper and return it by value. The embedded +// interface promotes its methods to the value type, so the value form also +// satisfies rpc.PermissionDecision and must be unwrapped identically to the +// pointer form -- otherwise the wrapper itself is sent as result and the +// context is silently dropped. +func TestValueFormAttributedResultIsUnwrapped(t *testing.T) { + frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { + return *NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil + }) + if !sent { + t.Fatal("expected a permission response to be sent") + } + + params := paramsOf(t, frame) + + if _, ok := params["decisionContext"]; !ok { + t.Fatal("expected decisionContext to be forwarded for a value-form attributed result") + } + + var result map[string]json.RawMessage + if err := json.Unmarshal(params["result"], &result); err != nil { + t.Fatalf("failed to unmarshal result: %v", err) + } + if _, nested := result["decisionContext"]; nested { + t.Fatal("decisionContext must not be nested inside result") + } + if _, leaked := result["PermissionDecision"]; leaked { + t.Fatal("the wrapper leaked into result instead of being unwrapped") + } +} + +func TestAttributedResultReplacesContextOnValueForm(t *testing.T) { + first := sampleDecisionContext() + second := &rpc.PermissionDecisionContext{ + Outcome: PermissionDecisionOutcomePromptedUser, + Source: PermissionDecisionSourceHumanResponse, + Surface: PermissionDecisionSurfaceTui, + } + + valueForm := *NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, first) + replaced := NewAttributedPermissionResult(valueForm, second) + + if replaced.DecisionContext != second { + t.Fatal("expected the second context to replace the first") + } + if _, nested := replaced.PermissionDecision.(AttributedPermissionResult); nested { + t.Fatal("value-form attribution must be replaced, not nested") + } +} diff --git a/go/permissions.go b/go/permissions.go index 24b9cc7f13..f27f9b6e62 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -6,6 +6,62 @@ import ( "github.com/github/copilot-sdk/go/rpc" ) +// AttributedPermissionResult pairs a permission decision with the context +// describing how it was reached, so the runtime can attribute auto-approval +// telemetry to the responding surface. +// +// The embedded [rpc.PermissionDecision] carries the actual decision, while +// DecisionContext is informational only and never changes permission behavior. +// It satisfies [rpc.PermissionDecision] itself, so a [PermissionHandlerFunc] +// can return it wherever a plain decision is expected. Prefer constructing it +// through [NewAttributedPermissionResult] rather than by hand. +// +// Experimental: AttributedPermissionResult is part of an experimental API and +// may change or be removed. +type AttributedPermissionResult struct { + rpc.PermissionDecision + // DecisionContext describes how and where the decision was reached. When nil + // the SDK omits it from the wire, preserving legacy behavior. + DecisionContext *rpc.PermissionDecisionContext +} + +// NewAttributedPermissionResult pairs a permission decision with the context +// describing how it was reached, so the runtime can attribute auto-approval +// telemetry to the responding surface. +// +// The returned value satisfies [rpc.PermissionDecision], so a +// [PermissionHandlerFunc] can return it directly. Passing an already-attributed +// result replaces the previous context rather than nesting it. If result is a +// [rpc.PermissionDecisionNoResult] (attributed or not), the SDK still +// suppresses the response. +// +// Experimental: NewAttributedPermissionResult is part of an experimental API +// and may change or be removed. +func NewAttributedPermissionResult(result rpc.PermissionDecision, decisionContext *rpc.PermissionDecisionContext) *AttributedPermissionResult { + decision, _ := splitAttribution(result) + return &AttributedPermissionResult{ + PermissionDecision: decision, + DecisionContext: decisionContext, + } +} + +// splitAttribution separates an optionally attributed result into the bare +// decision and its context, returning a nil context when there is none. +// +// Both the pointer and value forms are matched: embedding an interface promotes +// its methods to the value type too, so an AttributedPermissionResult passed by +// value also satisfies [rpc.PermissionDecision] and must not slip through +// unwrapped. +func splitAttribution(result rpc.PermissionDecision) (rpc.PermissionDecision, *rpc.PermissionDecisionContext) { + switch attributed := result.(type) { + case *AttributedPermissionResult: + return attributed.PermissionDecision, attributed.DecisionContext + case AttributedPermissionResult: + return attributed.PermissionDecision, attributed.DecisionContext + } + return result, nil +} + // PermissionHandler provides pre-built OnPermissionRequest implementations. var PermissionHandler = struct { // ApproveAll approves permission requests when managed settings are disabled. diff --git a/go/session.go b/go/session.go index 99939de4a8..600a4bbebc 100644 --- a/go/session.go +++ b/go/session.go @@ -1646,6 +1646,10 @@ func (s *Session) executePermissionAndRespond(requestID string, permissionReques }) return } + // Unwrap any attribution so decisionContext travels as a sibling of result, + // not nested inside it. The suppression and send logic below operates on the + // underlying decision. + decision, decisionContext := splitAttribution(decision) if _, ok := decision.(*rpc.PermissionDecisionNoResult); ok { return } @@ -1654,8 +1658,9 @@ func (s *Session) executePermissionAndRespond(requestID string, permissionReques } s.RPC.Permissions.HandlePendingPermissionRequest(context.Background(), &rpc.PermissionDecisionRequest{ - RequestID: requestID, - Result: decision, + RequestID: requestID, + Result: decision, + DecisionContext: decisionContext, }) } diff --git a/go/types.go b/go/types.go index 6d6a877d30..23959aa03b 100644 --- a/go/types.go +++ b/go/types.go @@ -379,6 +379,47 @@ type PermissionInvocation struct { ManagedSettingsEnabled bool } +// PermissionDecisionContext describes how and where a permission decision was +// reached. Attach it to a decision with [NewAttributedPermissionResult] so the runtime +// can attribute auto-approval telemetry to the responding surface. It is +// informational only and never changes permission behavior. +// +// Experimental: PermissionDecisionContext is part of an experimental API and +// may change or be removed. +type PermissionDecisionContext = rpc.PermissionDecisionContext + +// PermissionDecisionOutcome describes the disposition of a permission request +// as observed by the responding client. +type PermissionDecisionOutcome = rpc.PermissionDecisionOutcome + +const ( + PermissionDecisionOutcomeAutoApproved = rpc.PermissionDecisionOutcomeAutoApproved + PermissionDecisionOutcomeAutopilotDenied = rpc.PermissionDecisionOutcomeAutopilotDenied + PermissionDecisionOutcomePromptedUser = rpc.PermissionDecisionOutcomePromptedUser +) + +// PermissionDecisionSource identifies the controlled reason or actor +// responsible for a permission response. +type PermissionDecisionSource = rpc.PermissionDecisionSource + +const ( + PermissionDecisionSourceHostPolicy = rpc.PermissionDecisionSourceHostPolicy + PermissionDecisionSourceHumanResponse = rpc.PermissionDecisionSourceHumanResponse + PermissionDecisionSourceJudgeRecommendation = rpc.PermissionDecisionSourceJudgeRecommendation + PermissionDecisionSourceUnattendedFallback = rpc.PermissionDecisionSourceUnattendedFallback +) + +// PermissionDecisionSurface identifies the client surface that submitted a +// permission response. +type PermissionDecisionSurface = rpc.PermissionDecisionSurface + +const ( + PermissionDecisionSurfaceCopilotApp = rpc.PermissionDecisionSurfaceCopilotApp + PermissionDecisionSurfacePromptMode = rpc.PermissionDecisionSurfacePromptMode + PermissionDecisionSurfaceSDK = rpc.PermissionDecisionSurfaceSDK + PermissionDecisionSurfaceTui = rpc.PermissionDecisionSurfaceTui +) + // MCPAuthWwwAuthenticateParams contains parsed parameters from an MCP server's WWW-Authenticate response. type MCPAuthWwwAuthenticateParams struct { ResourceMetadataURL *string `json:"resourceMetadataUrl,omitempty"` diff --git a/java/src/main/java/com/github/copilot/CopilotSession.java b/java/src/main/java/com/github/copilot/CopilotSession.java index 4683fdf015..ca2adf4629 100644 --- a/java/src/main/java/com/github/copilot/CopilotSession.java +++ b/java/src/main/java/com/github/copilot/CopilotSession.java @@ -1026,7 +1026,7 @@ private void executePermissionAndRespondAsync(String requestId, PermissionReques } getRpc().permissions.handlePendingPermissionRequest( new SessionPermissionsHandlePendingPermissionRequestParams(sessionId, requestId, result, - null)); + result.getDecisionContext())); } catch (Exception e) { LOG.log(Level.WARNING, "Error sending permission result for requestId=" + requestId, e); } diff --git a/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java b/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java index 2e5c60100a..6546291cf7 100644 --- a/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java +++ b/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java @@ -6,8 +6,10 @@ import java.util.List; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import com.github.copilot.generated.rpc.PermissionDecisionContext; /** * Result of a permission request decision. @@ -42,6 +44,15 @@ public final class PermissionRequestResult { @JsonProperty("feedback") private String feedback; + /** + * Optional provenance describing how and where this decision was made. Never + * serialized inside the result — the SDK forwards it as a sibling of + * {@code result} so the runtime can attribute {@code auto_approval_decision} + * telemetry. + */ + @JsonIgnore + private PermissionDecisionContext decisionContext; + /** * Creates a result that approves this single request. * @@ -168,4 +179,35 @@ public PermissionRequestResult setFeedback(String feedback) { this.feedback = feedback; return this; } + + /** + * Gets the optional provenance describing how and where this decision was made. + *

+ * This value is never serialized inside the result JSON; the SDK forwards it as + * a sibling of {@code result} when responding to the runtime. + * + * @return the decision context, or {@code null} if none was attached + * @since 1.3.0 + */ + public PermissionDecisionContext getDecisionContext() { + return decisionContext; + } + + /** + * Sets provenance describing how and where this decision was made, so the + * runtime can attribute {@code auto_approval_decision} telemetry. + *

+ * Calling this method more than once replaces any previously set context. The + * context is never serialized inside the result; the SDK forwards it as a + * sibling of {@code result}. + * + * @param decisionContext + * the decision context, or {@code null} to attach none + * @return this result for method chaining + * @since 1.3.0 + */ + public PermissionRequestResult setDecisionContext(PermissionDecisionContext decisionContext) { + this.decisionContext = decisionContext; + return this; + } } diff --git a/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java b/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java new file mode 100644 index 0000000000..395ad50ad6 --- /dev/null +++ b/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java @@ -0,0 +1,97 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.rpc; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.github.copilot.generated.rpc.PermissionDecisionContext; +import com.github.copilot.generated.rpc.PermissionDecisionOutcome; +import com.github.copilot.generated.rpc.PermissionDecisionSource; +import com.github.copilot.generated.rpc.PermissionDecisionSurface; +import com.github.copilot.generated.rpc.SessionPermissionsHandlePendingPermissionRequestParams; +import org.junit.jupiter.api.Test; + +/** + * Verifies that {@link PermissionRequestResult} carries an optional + * {@link PermissionDecisionContext} as a sibling of {@code result} — never + * nested inside the serialized result — when the SDK forwards a permission + * response to the runtime. + */ +class PermissionRequestResultDecisionContextTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private static PermissionDecisionContext sampleContext() { + return new PermissionDecisionContext(PermissionDecisionOutcome.AUTO_APPROVED, + PermissionDecisionSource.HOST_POLICY, PermissionDecisionSurface.SDK); + } + + @Test + void setDecisionContextForwardsContextAsSiblingOfResult() throws Exception { + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); + var params = new SessionPermissionsHandlePendingPermissionRequestParams("session-1", "req-1", result, + result.getDecisionContext()); + + JsonNode json = MAPPER.valueToTree(params); + + assertTrue(json.has("decisionContext"), "decisionContext must be a top-level sibling of result"); + assertEquals("host_policy", json.get("decisionContext").get("source").asText()); + assertEquals("auto_approved", json.get("decisionContext").get("outcome").asText()); + assertEquals("sdk", json.get("decisionContext").get("surface").asText()); + assertFalse(json.get("result").has("decisionContext"), "decisionContext must NOT be nested inside result"); + } + + @Test + void withoutContextOmitsDecisionContextKey() throws Exception { + var result = PermissionRequestResult.approveOnce(); + assertNull(result.getDecisionContext()); + + var params = new SessionPermissionsHandlePendingPermissionRequestParams("session-1", "req-1", result, + result.getDecisionContext()); + + JsonNode json = MAPPER.valueToTree(params); + + // Generated params record is @JsonInclude(NON_NULL), so a null + // decisionContext is omitted entirely — byte-identical to legacy behavior. + assertFalse(json.has("decisionContext"), "decisionContext key must be absent when no context is supplied"); + } + + @Test + void setDecisionContextTwiceReplacesRatherThanNests() { + var first = sampleContext(); + var second = new PermissionDecisionContext(PermissionDecisionOutcome.PROMPTED_USER, + PermissionDecisionSource.HUMAN_RESPONSE, PermissionDecisionSurface.TUI); + + var result = PermissionRequestResult.approveOnce().setDecisionContext(first).setDecisionContext(second); + + assertSame(second, result.getDecisionContext(), "second setDecisionContext must replace the first, not nest"); + } + + @Test + void serializingResultWithContextDoesNotEmitContextInsideResult() throws Exception { + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); + + JsonNode resultJson = MAPPER.valueToTree(result); + + assertFalse(resultJson.has("decisionContext"), + "@JsonIgnore must keep decisionContext out of the serialized result"); + assertEquals(PermissionRequestResultKind.APPROVED.getValue(), resultJson.get("kind").asText()); + } + + @Test + void setDecisionContextAcceptsNullAsNoContext() { + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); + + result.setDecisionContext(null); + + assertNull(result.getDecisionContext(), "null must clear the context rather than throwing"); + } +} diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 5ab53471a6..0dfbb5ff72 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -27,6 +27,7 @@ export { export { defineTool, approveAll, + createAttributedPermissionResult, convertMcpCallToolResult, createSessionFsAdapter, CopilotRequestHandler, @@ -123,6 +124,11 @@ export type { PermissionRequestedData, PermissionRequestedEvent, PermissionRequestResult, + AttributedPermissionResult, + PermissionDecisionContext, + PermissionDecisionOutcome, + PermissionDecisionSource, + PermissionDecisionSurface, ProviderConfig, ProviderModelConfig, ProviderTokenArgs, diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index ed575a5154..996189cf25 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -24,6 +24,7 @@ import type { import { type Canvas, CanvasError } from "./canvas.js"; import type { OpenCanvasInstance } from "./generated/rpc.js"; import { getTraceContext } from "./telemetry.js"; +import { isAttributedPermissionResult } from "./types.js"; import type { CommandHandler, AutoModeSwitchHandler, @@ -43,6 +44,7 @@ import type { McpAuthRequest, PermissionHandler, PermissionRequest, + PermissionRequestResult, ContextTier, ReasoningEffort, ReasoningSummary, @@ -1124,17 +1126,26 @@ export class CopilotSession { permissionRequest: PermissionRequest ): Promise { try { - const result = await this.permissionHandler!(permissionRequest, { + const handlerResult = await this.permissionHandler!(permissionRequest, { sessionId: this.sessionId, managedSettingsEnabled: this.managedSettingsEnabled, }); + const isAttributed = isAttributedPermissionResult(handlerResult); + const result: PermissionRequestResult = isAttributed + ? handlerResult.result + : handlerResult; + const decisionContext = isAttributed ? handlerResult.decisionContext : undefined; if (result.kind === "no-result") { return; } if (this.disconnected) { return; } - await this.rpc.permissions.handlePendingPermissionRequest({ requestId, result }); + await this.rpc.permissions.handlePendingPermissionRequest( + decisionContext === undefined + ? { requestId, result } + : { requestId, result, decisionContext } + ); } catch (error) { if (this.disconnected) { return; diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 4ff2791898..5ec355340e 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -52,6 +52,12 @@ export type { SessionFsSqliteStatement } from "./sessionFsProvider.js"; export type { SessionFsSqliteTransactionErrorClass } from "./sessionFsProvider.js"; export { SessionFsSqliteTransactionFailure } from "./sessionFsProvider.js"; export type { LlmInferenceHeaders } from "./generated/rpc.js"; +export type { + PermissionDecisionContext, + PermissionDecisionOutcome, + PermissionDecisionSource, + PermissionDecisionSurface, +} from "./generated/rpc.js"; export type { CopilotRequestContext } from "./copilotRequestHandler.js"; export { CopilotRequestHandler, @@ -1112,7 +1118,7 @@ export type SystemMessageConfig = | SystemMessageReplaceConfig | SystemMessageCustomizeConfig; -import type { PermissionDecisionRequest } from "./generated/rpc.js"; +import type { PermissionDecisionRequest, PermissionDecisionContext } from "./generated/rpc.js"; /** * Permission request types from the server. This is the generated @@ -1148,10 +1154,51 @@ export type PermissionRequestedEvent = Omit Promise | PermissionRequestResult; +) => + | Promise + | PermissionRequestResult + | AttributedPermissionResult; /** * Approves permission requests when managed settings are disabled. diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 01a97e9800..e048218b92 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -7,6 +7,7 @@ import { join } from "node:path"; import { describe, expect, it, onTestFinished, vi } from "vitest"; import { approveAll, + createAttributedPermissionResult, CopilotClient, createCanvas, RuntimeConnection, @@ -83,6 +84,91 @@ describe("CopilotClient", () => { expect(spy).not.toHaveBeenCalled(); }); + it("forwards decisionContext as a top-level sibling of result", async () => { + const session = new CopilotSession("session-1", {} as any); + const decisionContext = { + outcome: "auto_approved" as const, + source: "host_policy" as const, + surface: "sdk" as const, + }; + session.registerPermissionHandler(() => + createAttributedPermissionResult({ kind: "approve-once" }, decisionContext) + ); + const spy = vi + .spyOn(session.rpc.permissions, "handlePendingPermissionRequest") + .mockResolvedValue({ kind: "approve-once" } as any); + + await (session as any)._executePermissionAndRespond("request-1", { kind: "write" }); + + expect(spy).toHaveBeenCalledOnce(); + const params = spy.mock.calls[0][0] as any; + expect(params).toEqual({ + requestId: "request-1", + result: { kind: "approve-once" }, + decisionContext, + }); + // decisionContext is a sibling of result, never nested inside it. + expect(params.result.decisionContext).toBeUndefined(); + }); + + it("emits exactly requestId and result with no decisionContext key when unattributed", async () => { + const session = new CopilotSession("session-1", {} as any); + session.registerPermissionHandler(() => ({ kind: "approve-once" })); + const spy = vi + .spyOn(session.rpc.permissions, "handlePendingPermissionRequest") + .mockResolvedValue({ kind: "approve-once" } as any); + + await (session as any)._executePermissionAndRespond("request-1", { kind: "write" }); + + expect(spy).toHaveBeenCalledOnce(); + const params = spy.mock.calls[0][0] as any; + expect(params).toEqual({ requestId: "request-1", result: { kind: "approve-once" } }); + expect(Object.keys(params).sort()).toEqual(["requestId", "result"]); + expect("decisionContext" in params).toBe(false); + }); + + it("does not respond when a no-result decision is wrapped with a context", async () => { + const session = new CopilotSession("session-1", {} as any); + const decisionContext = { + outcome: "auto_approved" as const, + source: "host_policy" as const, + surface: "sdk" as const, + }; + session.registerPermissionHandler(() => + createAttributedPermissionResult({ kind: "no-result" }, decisionContext) + ); + const spy = vi.spyOn(session.rpc.permissions, "handlePendingPermissionRequest"); + + await (session as any)._executePermissionAndRespond("request-1", { kind: "write" }); + + expect(spy).not.toHaveBeenCalled(); + }); + + it("replaces the context when applied twice", () => { + const first = { + outcome: "auto_approved" as const, + source: "judge_recommendation" as const, + surface: "sdk" as const, + }; + const second = { + outcome: "prompted_user" as const, + source: "human_response" as const, + surface: "tui" as const, + }; + + const once = createAttributedPermissionResult({ kind: "approve-once" }, first); + const twice = createAttributedPermissionResult(once, second); + + expect(twice).toEqual({ + kind: "attributed", + result: { kind: "approve-once" }, + decisionContext: second, + }); + // The result stays unwrapped rather than nesting an AttributedPermissionResult. + expect((twice.result as any).result).toBeUndefined(); + expect((twice.result as any).decisionContext).toBeUndefined(); + }); + it("responds to MCP OAuth requests with host token data", async () => { const sendRequest = vi.fn(async () => ({ success: true })); let observedRequest: any; diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index e7c26a2930..b7fa6087a3 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -5,14 +5,15 @@ import { realpathSync } from "fs"; import { mkdir, readFile, writeFile } from "fs/promises"; import { join } from "path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { z } from "zod"; import type { + PermissionDecisionContext, PermissionRequest, PermissionRequestResult, ToolResultObject, } from "../../src/index.js"; -import { approveAll, defineTool } from "../../src/index.js"; +import { approveAll, defineTool, createAttributedPermissionResult } from "../../src/index.js"; import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; import { getFinalAssistantMessage, getNextEventOfType } from "./harness/sdkTestHelper.js"; @@ -90,6 +91,61 @@ describe("Permission callbacks", async () => { await session.disconnect(); }); + it("should honor a decision annotated with decisionContext", async () => { + // End-to-end proof that decisionContext survives the real permission flow. + // The runtime only emits its auto_approval_decision telemetry when its own + // auto-approval judge metadata is also present (feature-flagged and model + // backed), so that event is not observable here. Instead we assert the exact + // params handed to the CLI: decisionContext must be a top-level sibling of + // `result`, never nested inside it. The CLI tolerates a nested key silently, + // so asserting the params shape is what actually gives this test teeth. + const decisionContext: PermissionDecisionContext = { + outcome: "prompted_user", + source: "human_response", + surface: "sdk", + }; + + const session = await client.createSession({ + onPermissionRequest: () => + createAttributedPermissionResult({ kind: "reject" }, decisionContext), + }); + + // Spies preserve the original implementation, so the decision still reaches + // the CLI and the assertions below observe a real, honored round-trip. + const respondSpy = vi.spyOn(session.rpc.permissions, "handlePendingPermissionRequest"); + + let userRejectedToolCall = false; + session.on((event) => { + if ( + event.type === "tool.execution_complete" && + !event.data.success && + event.data.error?.message.toLowerCase().includes("user rejected") + ) { + userRejectedToolCall = true; + } + }); + + const originalContent = "protected content"; + const testFile = join(workDir, "protected.txt"); + await writeFile(testFile, originalContent); + + await session.sendAndWait({ + prompt: "Edit protected.txt and replace 'protected' with 'hacked'.", + }); + + // The decision was applied by the CLI, not merely sent. + expect(userRejectedToolCall).toBe(true); + expect(await readFile(testFile, "utf-8")).toBe(originalContent); + + expect(respondSpy).toHaveBeenCalled(); + const params = respondSpy.mock.calls[0]![0]; + expect(params.decisionContext).toEqual(decisionContext); + expect(params.result).toEqual({ kind: "reject" }); + expect(Object.keys(params).sort()).toEqual(["decisionContext", "requestId", "result"]); + + await session.disconnect(); + }); + it("should deny tool operations when handler explicitly denies", async () => { let permissionDenied = false; diff --git a/python/copilot/__init__.py b/python/copilot/__init__.py index a7366db543..f7a71ebe91 100644 --- a/python/copilot/__init__.py +++ b/python/copilot/__init__.py @@ -87,6 +87,10 @@ GitHubTelemetryNotification, ModelBillingTokenPrices, ModelBillingTokenPricesLongContext, + PermissionDecisionContext, + PermissionDecisionOutcome, + PermissionDecisionSource, + PermissionDecisionSurface, ) from .generated.session_events import ( PermissionRequest, @@ -97,6 +101,7 @@ AgentStopHandler, AgentStopHookInput, AgentStopHookOutput, + AttributedPermissionResult, AutoModeSwitchHandler, AutoModeSwitchRequest, AutoModeSwitchResponse, @@ -178,6 +183,7 @@ UserPromptTransformedHandler, UserPromptTransformedHookInput, UserPromptTransformedHookOutput, + create_attributed_permission_result, ) from .session_fs_provider import ( SessionFsFileInfo, @@ -209,6 +215,7 @@ "AgentStopHandler", "AgentStopHookInput", "AgentStopHookOutput", + "AttributedPermissionResult", "AutoModeSwitchHandler", "AutoModeSwitchRequest", "AutoModeSwitchResponse", @@ -295,6 +302,10 @@ "PermissionNoResult", "PermissionRequest", "PermissionRequestResult", + "PermissionDecisionContext", + "PermissionDecisionOutcome", + "PermissionDecisionSource", + "PermissionDecisionSurface", "PingResponse", "PostToolUseHandler", "PostToolUseFailureHandler", @@ -373,6 +384,7 @@ "UserPromptTransformedHookInput", "UserPromptTransformedHookOutput", "convert_mcp_call_tool_result", + "create_attributed_permission_result", "create_session_fs_adapter", "define_tool", ] diff --git a/python/copilot/session.py b/python/copilot/session.py index 92c24bdd84..2399ab36ef 100644 --- a/python/copilot/session.py +++ b/python/copilot/session.py @@ -44,6 +44,7 @@ ModelSwitchToRequest, PermissionDecision, PermissionDecisionApproveOnce, + PermissionDecisionContext, PermissionDecisionRequest, PermissionDecisionUserNotAvailable, ProviderTokenAcquireRequest, @@ -367,6 +368,44 @@ class PermissionNoResult: PermissionRequestResult = PermissionDecision | PermissionNoResult +@dataclass +class AttributedPermissionResult: + """A permission result annotated with the context describing how it was reached. + + The Copilot runtime emits an ``auto_approval_decision`` telemetry event only + when a client supplies an explicit :class:`PermissionDecisionContext` alongside + its permission reply. Wrapping a :data:`PermissionRequestResult` with this class + forwards that context to the runtime as a sibling of the decision on the wire. + + The context is informational only — it never changes permission behavior. Build + instances via :func:`create_attributed_permission_result` rather than constructing + directly, so re-attributing an already-wrapped result replaces the context instead + of nesting. + """ + + result: PermissionRequestResult + """The underlying permission decision (or :class:`PermissionNoResult`).""" + + decision_context: PermissionDecisionContext + """Context describing how and where the decision was reached.""" + + +def create_attributed_permission_result( + result: PermissionRequestResult | AttributedPermissionResult, + decision_context: PermissionDecisionContext, +) -> AttributedPermissionResult: + """Annotate a permission result with the context describing how it was reached. + + Returns an :class:`AttributedPermissionResult` carrying ``result`` and + ``decision_context`` as siblings. If ``result`` is already an + :class:`AttributedPermissionResult`, its underlying decision is preserved and the + context is *replaced* — attribution never nests. + """ + if isinstance(result, AttributedPermissionResult): + result = result.result + return AttributedPermissionResult(result=result, decision_context=decision_context) + + class PermissionInvocation(TypedDict, total=False): session_id: Required[str] managed_settings_enabled: NotRequired[bool] @@ -374,7 +413,9 @@ class PermissionInvocation(TypedDict, total=False): _PermissionHandlerFn = Callable[ [PermissionRequest, PermissionInvocation], - PermissionRequestResult | Awaitable[PermissionRequestResult], + PermissionRequestResult + | AttributedPermissionResult + | Awaitable[PermissionRequestResult | AttributedPermissionResult], ] @@ -2174,7 +2215,11 @@ async def _execute_permission_and_respond( request_id=request_id, ) - result = cast(PermissionRequestResult, result) + result = cast("PermissionRequestResult | AttributedPermissionResult", result) + decision_context: PermissionDecisionContext | None = None + if isinstance(result, AttributedPermissionResult): + decision_context = result.decision_context + result = result.result if isinstance(result, PermissionNoResult): return @@ -2183,6 +2228,7 @@ async def _execute_permission_and_respond( PermissionDecisionRequest( request_id=request_id, result=result, + decision_context=decision_context, ) ) log_timing( diff --git a/python/test_permission_decision_context.py b/python/test_permission_decision_context.py new file mode 100644 index 0000000000..2b013942d7 --- /dev/null +++ b/python/test_permission_decision_context.py @@ -0,0 +1,99 @@ +from unittest.mock import AsyncMock, MagicMock + +from copilot.rpc import ( + PermissionDecisionApproveOnce, + PermissionDecisionContext, + PermissionDecisionOutcome, + PermissionDecisionSource, + PermissionDecisionSurface, +) +from copilot.session import ( + AttributedPermissionResult, + CopilotSession, + PermissionNoResult, + create_attributed_permission_result, +) +from copilot.session_events import PermissionRequestRead + + +def _context() -> PermissionDecisionContext: + return PermissionDecisionContext( + outcome=PermissionDecisionOutcome.AUTO_APPROVED, + source=PermissionDecisionSource.HOST_POLICY, + surface=PermissionDecisionSurface.SDK, + ) + + +def _session_with_captured_rpc() -> tuple[CopilotSession, AsyncMock]: + session = CopilotSession("session-1", client=None) + handle = AsyncMock() + rpc = MagicMock() + rpc.permissions.handle_pending_permission_request = handle + session._rpc = rpc + return session, handle + + +async def test_decision_context_serialized_as_sibling_of_result() -> None: + session, handle = _session_with_captured_rpc() + request = PermissionRequestRead(intention="Read", path="/workspace/file.txt") + + def handler(_request, _invocation): + return create_attributed_permission_result(PermissionDecisionApproveOnce(), _context()) + + await session._execute_permission_and_respond("permission-1", request, handler) + + handle.assert_awaited_once() + sent = handle.await_args.args[0] + params = sent.to_dict() + + assert params["decisionContext"] == { + "outcome": "auto_approved", + "source": "host_policy", + "surface": "sdk", + } + assert "decisionContext" not in params["result"] + assert params["result"]["kind"] == "approve-once" + + +async def test_no_context_omits_decision_context_key() -> None: + session, handle = _session_with_captured_rpc() + request = PermissionRequestRead(intention="Read", path="/workspace/file.txt") + + def handler(_request, _invocation): + return PermissionDecisionApproveOnce() + + await session._execute_permission_and_respond("permission-1", request, handler) + + handle.assert_awaited_once() + params = handle.await_args.args[0].to_dict() + + assert "decisionContext" not in params + assert params["result"]["kind"] == "approve-once" + + +def test_attributed_result_replaces_rather_than_nests() -> None: + first = PermissionDecisionContext( + outcome=PermissionDecisionOutcome.PROMPTED_USER, + source=PermissionDecisionSource.HUMAN_RESPONSE, + surface=PermissionDecisionSurface.TUI, + ) + second = _context() + + once_wrapped = create_attributed_permission_result(PermissionDecisionApproveOnce(), first) + twice_wrapped = create_attributed_permission_result(once_wrapped, second) + + assert isinstance(twice_wrapped, AttributedPermissionResult) + assert isinstance(twice_wrapped.result, PermissionDecisionApproveOnce) + assert twice_wrapped.decision_context is second + + +async def test_no_result_with_context_still_suppresses_response() -> None: + session, handle = _session_with_captured_rpc() + request = PermissionRequestRead(intention="Read", path="/workspace/file.txt") + + def handler(_request, _invocation): + return create_attributed_permission_result(PermissionNoResult(), _context()) + + await session._execute_permission_and_respond("permission-1", request, handler) + + handle.assert_not_awaited() diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 77edf919c9..3745c3dd71 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -22,7 +22,7 @@ use crate::generated::api_types::{ McpOauthPendingRequestResponse, McpOauthPendingRequestResponseCancelled, McpOauthPendingRequestResponseCancelledKind, McpOauthPendingRequestResponseToken, McpOauthPendingRequestResponseTokenKind, PermissionDecision, PermissionDecisionApproveOnce, - PermissionDecisionReject, PermissionDecisionUserNotAvailable, + PermissionDecisionContext, PermissionDecisionReject, PermissionDecisionUserNotAvailable, }; use crate::session_events::{ McpOauthRequestReason, McpOauthRequiredStaticClientConfig, McpOauthWWWAuthenticateParams, @@ -38,10 +38,23 @@ use crate::types::{ /// approve-for-session, approve-permanently, user-not-available, …) or /// [`PermissionResult::NoResult`], which tells the SDK to suppress its /// response so another connected client can answer instead. +#[non_exhaustive] #[derive(Debug, Clone)] pub enum PermissionResult { /// Send a permission decision on the wire. Decision(PermissionDecision), + /// Send a permission decision annotated with the context describing how + /// and where it was reached, so the runtime can attribute + /// auto-approval telemetry to the responding surface. + /// + /// The context is informational only — it never changes permission + /// behavior. + AttributedDecision { + /// The decision to send on the wire. + decision: PermissionDecision, + /// Context describing how and where the decision was reached. + context: PermissionDecisionContext, + }, /// Decline to respond to this request, allowing another connected /// client to answer instead. The SDK suppresses the response. NoResult, @@ -75,6 +88,34 @@ impl PermissionResult { pub fn no_result() -> Self { Self::NoResult } + + /// Attach provenance describing how and where this decision was made, + /// so the runtime can attribute auto-approval telemetry. + /// + /// Applying this to an already-attributed decision replaces the + /// previous context. It is a no-op on [`PermissionResult::NoResult`]. + /// + /// ```rust,no_run + /// # use github_copilot_sdk::handler::PermissionResult; + /// # use github_copilot_sdk::{ + /// # PermissionDecisionContext, PermissionDecisionOutcome, PermissionDecisionSource, + /// # PermissionDecisionSurface, + /// # }; + /// + /// let result = PermissionResult::approve_once().with_context(PermissionDecisionContext { + /// outcome: PermissionDecisionOutcome::AutoApproved, + /// source: PermissionDecisionSource::HostPolicy, + /// surface: PermissionDecisionSurface::Sdk, + /// }); + /// ``` + pub fn with_context(self, context: PermissionDecisionContext) -> Self { + match self { + Self::Decision(decision) | Self::AttributedDecision { decision, .. } => { + Self::AttributedDecision { decision, context } + } + Self::NoResult => Self::NoResult, + } + } } impl From for PermissionResult { diff --git a/rust/src/session.rs b/rust/src/session.rs index c6c806b1c1..f096a2ab8f 100644 --- a/rust/src/session.rs +++ b/rust/src/session.rs @@ -1580,12 +1580,38 @@ fn permission_request_data( fn notification_permission_payload(result: &PermissionResult) -> Option { match result { PermissionResult::NoResult => None, - PermissionResult::Decision(decision) => Some( + PermissionResult::Decision(decision) + | PermissionResult::AttributedDecision { decision, .. } => Some( serde_json::to_value(decision).expect("serializing permission decision should succeed"), ), } } +/// Build the full `session.permissions.handlePendingPermissionRequest` +/// params for a [`PermissionResult`]. +/// +/// `decisionContext` is a sibling of `result` and is only present when the +/// handler attributed the decision — omitting it preserves legacy behavior. +/// +/// Returns `None` when the SDK must not send a response. +fn permission_response_params( + session_id: &SessionId, + request_id: &RequestId, + result: &PermissionResult, +) -> Option { + let result_value = notification_permission_payload(result)?; + let mut params = serde_json::json!({ + "sessionId": session_id, + "requestId": request_id, + "result": result_value, + }); + if let PermissionResult::AttributedDecision { context, .. } = result { + params["decisionContext"] = + serde_json::to_value(context).expect("serializing decision context should succeed"); + } + Some(params) +} + async fn register_mcp_auth_interest(client: &Client, session_id: &SessionId) -> Result<(), Error> { let mut params = serde_json::to_value(RegisterEventInterestParams { event_type: "mcp.oauth_required".to_string(), @@ -1779,7 +1805,8 @@ async fn handle_notification( request_id = %request_id, "PermissionHandler::handle dispatch" ); - let Some(result_value) = notification_permission_payload(&result) else { + let Some(params) = permission_response_params(&sid, &request_id, &result) + else { // Handler returned Deferred / NoResult — it will // call handlePendingPermissionRequest itself (or // leave the request unanswered). @@ -1789,11 +1816,7 @@ async fn handle_notification( let _ = client .call( "session.permissions.handlePendingPermissionRequest", - Some(serde_json::json!({ - "sessionId": sid, - "requestId": request_id, - "result": result_value, - })), + Some(params), ) .await; tracing::debug!( @@ -2563,8 +2586,15 @@ fn inject_transform_sections_resume( mod tests { use serde_json::json; - use super::{has_managed_settings, notification_permission_payload, permission_request_data}; + use super::{ + has_managed_settings, notification_permission_payload, permission_request_data, + permission_response_params, + }; use crate::handler::PermissionResult; + use crate::types::{ + PermissionDecisionContext, PermissionDecisionOutcome, PermissionDecisionSource, + PermissionDecisionSurface, RequestId, SessionId, + }; #[test] fn direct_injection_enables_managed_safeguards() { @@ -2598,6 +2628,103 @@ mod tests { ); } + fn attribution_context() -> PermissionDecisionContext { + PermissionDecisionContext { + outcome: PermissionDecisionOutcome::AutoApproved, + source: PermissionDecisionSource::JudgeRecommendation, + surface: PermissionDecisionSurface::CopilotApp, + } + } + + #[test] + fn response_params_omit_decision_context_without_attribution() { + let params = permission_response_params( + &SessionId::from("session-1"), + &RequestId::from("permission-1"), + &PermissionResult::approve_once(), + ) + .unwrap(); + assert_eq!( + params, + json!({ + "sessionId": "session-1", + "requestId": "permission-1", + "result": { "kind": "approve-once" }, + }) + ); + assert!(params.get("decisionContext").is_none()); + } + + #[test] + fn response_params_forward_decision_context_alongside_result() { + let params = permission_response_params( + &SessionId::from("session-1"), + &RequestId::from("permission-1"), + &PermissionResult::approve_once().with_context(attribution_context()), + ) + .unwrap(); + assert_eq!( + params, + json!({ + "sessionId": "session-1", + "requestId": "permission-1", + "result": { "kind": "approve-once" }, + "decisionContext": { + "outcome": "auto_approved", + "source": "judge_recommendation", + "surface": "copilot_app", + }, + }) + ); + // The context is a sibling of `result`, never nested inside it. + assert!(params["result"].get("decisionContext").is_none()); + } + + #[test] + fn response_params_suppressed_for_no_result() { + assert!( + permission_response_params( + &SessionId::from("session-1"), + &RequestId::from("permission-1"), + &PermissionResult::NoResult, + ) + .is_none() + ); + } + + #[test] + fn with_context_is_a_no_op_on_no_result() { + assert!(matches!( + PermissionResult::no_result().with_context(attribution_context()), + PermissionResult::NoResult + )); + } + + #[test] + fn with_context_replaces_rather_than_nests() { + let result = PermissionResult::approve_once() + .with_context(attribution_context()) + .with_context(PermissionDecisionContext { + outcome: PermissionDecisionOutcome::PromptedUser, + source: PermissionDecisionSource::HumanResponse, + surface: PermissionDecisionSurface::Sdk, + }); + let params = permission_response_params( + &SessionId::from("session-1"), + &RequestId::from("permission-1"), + &result, + ) + .unwrap(); + assert_eq!( + params["decisionContext"], + json!({ + "outcome": "prompted_user", + "source": "human_response", + "surface": "sdk", + }) + ); + } + #[test] fn permission_request_data_reads_nested_managed_approval_metadata() { let data = permission_request_data( diff --git a/rust/src/types.rs b/rust/src/types.rs index d3c4faa16b..f927a23270 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -5740,7 +5740,9 @@ pub use crate::generated::api_types::{ Model, ModelBilling, ModelBillingTokenPrices, ModelBillingTokenPricesLongContext, ModelCapabilities, ModelCapabilitiesLimits, ModelCapabilitiesLimitsVision, ModelCapabilitiesSupports, ModelList, ModelPolicy, PermissionDecision, - PermissionDecisionApproveOnce, PermissionDecisionReject, PermissionDecisionUserNotAvailable, + PermissionDecisionApproveOnce, PermissionDecisionContext, PermissionDecisionOutcome, + PermissionDecisionReject, PermissionDecisionSource, PermissionDecisionSurface, + PermissionDecisionUserNotAvailable, }; /// Permission categories the CLI may request approval for. diff --git a/rust/tests/e2e/permissions.rs b/rust/tests/e2e/permissions.rs index 8f594841fd..28096a8920 100644 --- a/rust/tests/e2e/permissions.rs +++ b/rust/tests/e2e/permissions.rs @@ -5,7 +5,9 @@ use github_copilot_sdk::handler::{PermissionHandler, PermissionResult}; use github_copilot_sdk::rpc::PermissionsSetApproveAllRequest; use github_copilot_sdk::session_events::{SessionEventType, ToolExecutionCompleteData}; use github_copilot_sdk::{ - PermissionRequestData, RequestId, ResumeSessionConfig, SessionConfig, SessionId, + PermissionDecisionContext, PermissionDecisionOutcome, PermissionDecisionSource, + PermissionDecisionSurface, PermissionRequestData, RequestId, ResumeSessionConfig, + SessionConfig, SessionId, }; use tokio::sync::{mpsc, oneshot}; @@ -120,6 +122,69 @@ async fn should_deny_permission_when_handler_returns_denied() { .await; } +#[tokio::test] +async fn should_honor_a_decision_annotated_with_decisioncontext() { + // End-to-end proof that a decision carrying provenance still round-trips through + // the real CLI and is honored. Shares the Node snapshot of the same name. + // + // Scope note: the runtime only emits its `auto_approval_decision` telemetry when + // its own auto-approval judge metadata is present (feature-flagged and model + // backed), and it otherwise accepts `decisionContext` without validating it — so + // the CLI exposes no observable signal for the field's shape. The exact wire + // shape (top-level sibling of `result`, omitted entirely when absent) is asserted + // by the `permission_response_params` unit tests in `src/session.rs`. What this + // test covers is that attaching context does not disturb the live permission + // round-trip: the reject decision must still be applied by the CLI. + super::support::with_shared_e2e_context( + &E2E, + "permissions", + "should_honor_a_decision_annotated_with_decisioncontext", + |ctx| { + Box::pin(async move { + ctx.set_default_copilot_user(); + let test_file = ctx.work_dir().join("protected.txt"); + std::fs::write(&test_file, "protected content").expect("write protected file"); + let client = ctx.start_client().await; + + let decision_context = PermissionDecisionContext { + outcome: PermissionDecisionOutcome::PromptedUser, + source: PermissionDecisionSource::HumanResponse, + surface: PermissionDecisionSurface::Sdk, + }; + let session = client + .create_session( + SessionConfig::default() + .with_github_token(DEFAULT_TEST_TOKEN) + .with_permission_handler(Arc::new(StaticPermissionHandler::new( + PermissionResult::reject(None).with_context(decision_context), + ))), + ) + .await + .expect("create session"); + + let events = session.subscribe(); + + session + .send_and_wait("Edit protected.txt and replace 'protected' with 'hacked'.") + .await + .expect("send"); + + wait_for_event(events, "user-rejected tool completion", |event| { + is_user_rejected_tool_completion(event) + }) + .await; + + let content = std::fs::read_to_string(&test_file).expect("read protected file"); + assert_eq!(content, "protected content"); + + session.disconnect().await.expect("disconnect session"); + client.stop().await.expect("stop client"); + }) + }, + ) + .await; +} + #[tokio::test] async fn should_deny_tool_operations_when_handler_explicitly_denies() { super::support::with_shared_e2e_context( diff --git a/test/snapshots/permissions/should_honor_a_decision_annotated_with_decisioncontext.yaml b/test/snapshots/permissions/should_honor_a_decision_annotated_with_decisioncontext.yaml new file mode 100644 index 0000000000..ef6f60dbed --- /dev/null +++ b/test/snapshots/permissions/should_honor_a_decision_annotated_with_decisioncontext.yaml @@ -0,0 +1,24 @@ +models: + - claude-sonnet-4.5 +conversations: + - messages: + - role: system + content: ${system} + - role: user + content: Edit protected.txt and replace 'protected' with 'hacked'. + - role: assistant + content: I'll view the file first, then make the edit. + - role: assistant + tool_calls: + - id: toolcall_0 + type: function + function: + name: report_intent + arguments: '{"intent":"Editing protected.txt file"}' + - role: assistant + tool_calls: + - id: toolcall_1 + type: function + function: + name: view + arguments: '{"path":"${workdir}/protected.txt"}'