From c5078198594a0c5cd09dfe2565076c8f67073b97 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 14:28:54 +0200 Subject: [PATCH 1/9] feat(rust): forward decisionContext on permission replies The runtime emits `auto_approval_decision` telemetry only when a client supplies an explicit `decisionContext` alongside its permission reply. The generated wire types already carry the optional field, but the hand-written reply path built a fixed three-key JSON literal and had no way for a PermissionHandler to attribute its decision. Add `PermissionResult::AttributedDecision` plus a `with_context` builder, and forward the context as a top-level sibling of `result`. When no context is supplied the emitted params are byte-identical to before, so legacy behavior is preserved exactly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- rust/src/handler.rs | 37 +++++++++++- rust/src/session.rs | 143 +++++++++++++++++++++++++++++++++++++++++--- rust/src/types.rs | 4 +- 3 files changed, 174 insertions(+), 10 deletions(-) diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 77edf919c9..808b2ca4df 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, @@ -42,6 +42,13 @@ use crate::types::{ 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(PermissionDecision, PermissionDecisionContext), /// Decline to respond to this request, allowing another connected /// client to answer instead. The SDK suppresses the response. NoResult, @@ -75,6 +82,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..c41d235213 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. From c162ce7aa5e812fcfdebf01f8d48f2cce3c23456 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 15:46:55 +0200 Subject: [PATCH 2/9] sdk: Forward decisionContext on permission replies across languages Permission handlers can now attach optional provenance describing how and where a decision was reached. The SDK forwards it to the runtime as a sibling of `result` -- never nested inside it -- so auto-approval decisions made programmatically can be attributed. The wire schema and every language's generated types already accepted the field; only the hand-written reply paths never populated it. No schema, codegen, or protocol version change is required. Fully additive: handlers returning a plain decision emit a payload byte-identical to before, with no `decisionContext` key at all. No-result suppression is preserved in every language. Per CONTRIBUTING.md, the feature is implemented in sync across all six SDKs: - Rust: PermissionResult::AttributedDecision + with_context() - Node: AttributedPermissionResult + withDecisionContext() - Python: AttributedPermissionResult + with_decision_context() - Go: AttributedPermissionResult + WithDecisionContext() - .NET: PermissionDecision.WithContext() - Java: PermissionRequestResult.withContext() Each language gains focused unit tests asserting the sibling placement, the byte-identical legacy payload, replace-not-nest on re-application, and preserved no-result suppression. Node and Rust add end-to-end coverage against a CLI carrying the runtime-side support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- docs/troubleshooting/compatibility.md | 2 +- dotnet/src/PermissionDecision.cs | 26 +++ dotnet/src/Session.cs | 2 +- .../test/Unit/ClientSessionLifetimeTests.cs | 191 ++++++++++++++++ go/permission_context_test.go | 210 ++++++++++++++++++ go/permissions.go | 40 ++++ go/session.go | 13 +- go/types.go | 41 ++++ .../com/github/copilot/CopilotSession.java | 2 +- .../copilot/rpc/PermissionRequestResult.java | 42 ++++ ...ssionRequestResultDecisionContextTest.java | 88 ++++++++ nodejs/src/index.ts | 6 + nodejs/src/session.ts | 15 +- nodejs/src/types.ts | 54 ++++- nodejs/test/client.test.ts | 82 +++++++ nodejs/test/e2e/permissions.e2e.test.ts | 59 ++++- python/copilot/__init__.py | 12 + python/copilot/session.py | 49 +++- python/test_permission_decision_context.py | 99 +++++++++ rust/tests/e2e/permissions.rs | 67 +++++- ...cision_annotated_with_decisioncontext.yaml | 24 ++ 21 files changed, 1110 insertions(+), 14 deletions(-) create mode 100644 go/permission_context_test.go create mode 100644 java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java create mode 100644 python/test_permission_decision_context.py create mode 100644 test/snapshots/permissions/should_honor_a_decision_annotated_with_decisioncontext.yaml 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..237e9a9b2b 100644 --- a/dotnet/src/PermissionDecision.cs +++ b/dotnet/src/PermissionDecision.cs @@ -43,4 +43,30 @@ 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; } + + /// + /// Attaches provenance to this decision so the runtime can attribute + /// auto-approval telemetry. Returns the same instance mutated in place; + /// because the static factories (, , + /// etc.) return a fresh instance on every call, mutating is safe and keeps + /// the fluent call site concise. Calling this more than once replaces the + /// previously attached context rather than nesting it. + /// + /// The provenance to attach. + /// This decision, for fluent chaining. + public PermissionDecision WithContext(PermissionDecisionContext context) + { + ArgumentNullException.ThrowIfNull(context); + DecisionContext = context; + return this; + } } 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..68989de126 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -564,6 +564,193 @@ 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( + PermissionDecision.ApproveOnce().WithContext(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_WithContext_Called_Twice() + { + 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() + .WithContext(new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.PromptedUser, + Source = PermissionDecisionSource.HumanResponse, + Surface = PermissionDecisionSurface.Tui + }) + .WithContext(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-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( + PermissionDecision.NoResult().WithContext(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( + PermissionDecision.Reject("denied by policy").WithContext(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 +1009,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..b267e787b0 --- /dev/null +++ b/go/permission_context_test.go @@ -0,0 +1,210 @@ +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 WithDecisionContext(&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 TestWithDecisionContextReplacesRatherThanNests(t *testing.T) { + first := &rpc.PermissionDecisionContext{ + Outcome: PermissionDecisionOutcomePromptedUser, + Source: PermissionDecisionSourceHumanResponse, + Surface: PermissionDecisionSurfaceTui, + } + second := sampleDecisionContext() + + wrapped := WithDecisionContext(WithDecisionContext(&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 WithDecisionContext(&rpc.PermissionDecisionNoResult{}, sampleDecisionContext()), nil + }) + if sent { + t.Fatalf("expected no response to be sent for an attributed no-result decision, got frame: %s", frame) + } +} diff --git a/go/permissions.go b/go/permissions.go index 24b9cc7f13..95f3973f6f 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -6,6 +6,46 @@ 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 [WithDecisionContext] 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 +} + +// WithDecisionContext attaches provenance to a permission decision 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. Applying WithDecisionContext +// to 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: WithDecisionContext is part of an experimental API and may +// change or be removed. +func WithDecisionContext(result rpc.PermissionDecision, decisionContext *rpc.PermissionDecisionContext) *AttributedPermissionResult { + if attributed, ok := result.(*AttributedPermissionResult); ok { + result = attributed.PermissionDecision + } + return &AttributedPermissionResult{ + PermissionDecision: result, + DecisionContext: decisionContext, + } +} + // 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..42fd42941e 100644 --- a/go/session.go +++ b/go/session.go @@ -1646,6 +1646,14 @@ 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. + var decisionContext *rpc.PermissionDecisionContext + if attributed, ok := decision.(*AttributedPermissionResult); ok { + decisionContext = attributed.DecisionContext + decision = attributed.PermissionDecision + } if _, ok := decision.(*rpc.PermissionDecisionNoResult); ok { return } @@ -1654,8 +1662,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..b10e92716b 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 [WithDecisionContext] 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..da914f225e 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; + } + + /** + * Attaches 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 attached context. + * The context is never serialized inside the result; the SDK forwards it as a + * sibling of {@code result}. + * + * @param context + * the decision context, or {@code null} to clear it + * @return this result for method chaining + * @since 1.3.0 + */ + public PermissionRequestResult withContext(PermissionDecisionContext context) { + this.decisionContext = context; + 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..33497de625 --- /dev/null +++ b/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java @@ -0,0 +1,88 @@ +/*--------------------------------------------------------------------------------------------- + * 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 withContextForwardsDecisionContextAsSiblingOfResult() throws Exception { + var result = PermissionRequestResult.approveOnce().withContext(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 withContextTwiceReplacesRatherThanNests() { + var first = sampleContext(); + var second = new PermissionDecisionContext(PermissionDecisionOutcome.PROMPTED_USER, + PermissionDecisionSource.HUMAN_RESPONSE, PermissionDecisionSurface.TUI); + + var result = PermissionRequestResult.approveOnce().withContext(first).withContext(second); + + assertSame(second, result.getDecisionContext(), "second withContext must replace the first, not nest"); + } + + @Test + void serializingResultWithContextDoesNotEmitContextInsideResult() throws Exception { + var result = PermissionRequestResult.approveOnce().withContext(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()); + } +} diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index 5ab53471a6..e08abfe593 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -27,6 +27,7 @@ export { export { defineTool, approveAll, + withDecisionContext, 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..f6be35146e 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,54 @@ 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..3c4be64540 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, + withDecisionContext, CopilotClient, createCanvas, RuntimeConnection, @@ -83,6 +84,87 @@ 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(() => + withDecisionContext({ 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(() => + withDecisionContext({ 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 withDecisionContext is 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 = withDecisionContext({ kind: "approve-once" }, first); + const twice = withDecisionContext(once, second); + + expect(twice).toEqual({ 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..7fbf482ac1 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, withDecisionContext } from "../../src/index.js"; import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; import { getFinalAssistantMessage, getNextEventOfType } from "./harness/sdkTestHelper.js"; @@ -90,6 +91,60 @@ 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: () => withDecisionContext({ 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..38bef5a43c 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, + with_decision_context, ) 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", @@ -372,6 +383,7 @@ "UserPromptTransformedHandler", "UserPromptTransformedHookInput", "UserPromptTransformedHookOutput", + "with_decision_context", "convert_mcp_call_tool_result", "create_session_fs_adapter", "define_tool", diff --git a/python/copilot/session.py b/python/copilot/session.py index 92c24bdd84..b15ccba3bc 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,43 @@ 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:`with_decision_context` 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 with_decision_context( + 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 +412,9 @@ class PermissionInvocation(TypedDict, total=False): _PermissionHandlerFn = Callable[ [PermissionRequest, PermissionInvocation], - PermissionRequestResult | Awaitable[PermissionRequestResult], + PermissionRequestResult + | AttributedPermissionResult + | Awaitable[PermissionRequestResult | AttributedPermissionResult], ] @@ -2174,7 +2214,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 +2227,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..0c1a9cb463 --- /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, + with_decision_context, +) +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 with_decision_context(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_with_decision_context_replaces_rather_than_nests() -> None: + first = PermissionDecisionContext( + outcome=PermissionDecisionOutcome.PROMPTED_USER, + source=PermissionDecisionSource.HUMAN_RESPONSE, + surface=PermissionDecisionSurface.TUI, + ) + second = _context() + + once_wrapped = with_decision_context(PermissionDecisionApproveOnce(), first) + twice_wrapped = with_decision_context(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 with_decision_context(PermissionNoResult(), _context()) + + await session._execute_permission_and_respond("permission-1", request, handler) + + handle.assert_not_awaited() 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"}' From c1a784fb01e09aac0a502415315e4b7f604d58c7 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 17:20:17 +0200 Subject: [PATCH 3/9] sdk(java): Reject null in withContext to match the other SDKs Java accepted null as a "clear" operation while .NET rejects it and the other SDKs disallow it at the type level. Since null is Java's default, an uninitialized variable would have silently dropped the context -- producing exactly the unattributed telemetry this feature removes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- .../com/github/copilot/rpc/PermissionRequestResult.java | 7 +++++-- .../rpc/PermissionRequestResultDecisionContextTest.java | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) 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 da914f225e..651f89cc94 100644 --- a/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java +++ b/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java @@ -5,6 +5,7 @@ package com.github.copilot.rpc; import java.util.List; +import java.util.Objects; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; @@ -202,12 +203,14 @@ public PermissionDecisionContext getDecisionContext() { * sibling of {@code result}. * * @param context - * the decision context, or {@code null} to clear it + * the decision context; must not be {@code null} * @return this result for method chaining + * @throws NullPointerException + * if {@code context} is {@code null} * @since 1.3.0 */ public PermissionRequestResult withContext(PermissionDecisionContext context) { - this.decisionContext = context; + this.decisionContext = Objects.requireNonNull(context, "context must not be null"); 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 index 33497de625..1db345a19c 100644 --- a/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java +++ b/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java @@ -8,6 +8,7 @@ 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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.fasterxml.jackson.databind.JsonNode; @@ -85,4 +86,12 @@ void serializingResultWithContextDoesNotEmitContextInsideResult() throws Excepti "@JsonIgnore must keep decisionContext out of the serialized result"); assertEquals(PermissionRequestResultKind.APPROVED.getValue(), resultJson.get("kind").asText()); } + + @Test + void withContextRejectsNull() { + var result = PermissionRequestResult.approveOnce(); + + assertThrows(NullPointerException.class, () -> result.withContext(null), + "withContext must reject null rather than silently dropping the context"); + } } From 5d74762dec3a5cfa216529d04fcfaac6875a0810 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 17:32:48 +0200 Subject: [PATCH 4/9] sdk: Unwrap value-form attribution in Go and seal the Rust enum Go embedded the decision interface in AttributedPermissionResult, which promotes the interface methods to the value type. A handler returning `*WithDecisionContext(...)` therefore satisfied rpc.PermissionDecision but slipped past the pointer-only type assertion: the wrapper itself was sent as `result` and the context was silently dropped. Both the unwrap in the session and the replace-not-nest check now accept either form, with regression tests that fail against the pointer-only code. Rust PermissionResult gains #[non_exhaustive], matching the convention used throughout this crate, so downstream exhaustive matches keep compiling as variants are added. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- go/permission_context_test.go | 50 +++++++++++++++++++++++++++++++++++ go/permissions.go | 5 +++- go/session.go | 6 ++++- rust/src/handler.rs | 1 + 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/go/permission_context_test.go b/go/permission_context_test.go index b267e787b0..fbbf0e28bc 100644 --- a/go/permission_context_test.go +++ b/go/permission_context_test.go @@ -208,3 +208,53 @@ func TestAttributedNoResultStillSuppressesResponse(t *testing.T) { 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 *WithDecisionContext(&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 TestWithDecisionContextReplacesContextOnValueForm(t *testing.T) { + first := sampleDecisionContext() + second := &rpc.PermissionDecisionContext{ + Outcome: PermissionDecisionOutcomePromptedUser, + Source: PermissionDecisionSourceHumanResponse, + Surface: PermissionDecisionSurfaceTui, + } + + valueForm := *WithDecisionContext(&rpc.PermissionDecisionApproveOnce{}, first) + replaced := WithDecisionContext(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 95f3973f6f..6573bb823e 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -37,7 +37,10 @@ type AttributedPermissionResult struct { // Experimental: WithDecisionContext is part of an experimental API and may // change or be removed. func WithDecisionContext(result rpc.PermissionDecision, decisionContext *rpc.PermissionDecisionContext) *AttributedPermissionResult { - if attributed, ok := result.(*AttributedPermissionResult); ok { + switch attributed := result.(type) { + case *AttributedPermissionResult: + result = attributed.PermissionDecision + case AttributedPermissionResult: result = attributed.PermissionDecision } return &AttributedPermissionResult{ diff --git a/go/session.go b/go/session.go index 42fd42941e..7975665815 100644 --- a/go/session.go +++ b/go/session.go @@ -1650,7 +1650,11 @@ func (s *Session) executePermissionAndRespond(requestID string, permissionReques // not nested inside it. The suppression and send logic below operates on the // underlying decision. var decisionContext *rpc.PermissionDecisionContext - if attributed, ok := decision.(*AttributedPermissionResult); ok { + switch attributed := decision.(type) { + case *AttributedPermissionResult: + decisionContext = attributed.DecisionContext + decision = attributed.PermissionDecision + case AttributedPermissionResult: decisionContext = attributed.DecisionContext decision = attributed.PermissionDecision } diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 808b2ca4df..585799b4ed 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -38,6 +38,7 @@ 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. From 7ed8892430431d28be4d61aaf4ebf8b98e3f4c4b Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 19:02:09 +0200 Subject: [PATCH 5/9] sdk(dotnet): Drop fluent WithContext in favor of the settable property The hand-written .NET SDK has no other fluent `With*` builders, so adding one here introduced a pattern that exists nowhere else in the surface. Java and Rust keep their fluent forms because those match long-standing convention in each of those SDKs. Callers now set the public `DecisionContext` property through an object initializer, which is what the class documentation already recommends for richer decisions. The wire format is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- dotnet/src/PermissionDecision.cs | 17 ----- .../test/Unit/ClientSessionLifetimeTests.cs | 73 +++++++++++-------- 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/dotnet/src/PermissionDecision.cs b/dotnet/src/PermissionDecision.cs index 237e9a9b2b..3eb1d0e08a 100644 --- a/dotnet/src/PermissionDecision.cs +++ b/dotnet/src/PermissionDecision.cs @@ -52,21 +52,4 @@ public static PermissionDecision Reject(string? feedback = null) => /// [JsonIgnore] public PermissionDecisionContext? DecisionContext { get; set; } - - ///

- /// Attaches provenance to this decision so the runtime can attribute - /// auto-approval telemetry. Returns the same instance mutated in place; - /// because the static factories (, , - /// etc.) return a fresh instance on every call, mutating is safe and keeps - /// the fluent call site concise. Calling this more than once replaces the - /// previously attached context rather than nesting it. - /// - /// The provenance to attach. - /// This decision, for fluent chaining. - public PermissionDecision WithContext(PermissionDecisionContext context) - { - ArgumentNullException.ThrowIfNull(context); - DecisionContext = context; - return this; - } } diff --git a/dotnet/test/Unit/ClientSessionLifetimeTests.cs b/dotnet/test/Unit/ClientSessionLifetimeTests.cs index 68989de126..edc2fa8b16 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -573,13 +573,16 @@ public async Task PermissionResponse_Forwards_DecisionContext_As_Sibling_Of_Resu await using var session = await client.CreateSessionAsync(new SessionConfig { - OnPermissionRequest = (_, _) => Task.FromResult( - PermissionDecision.ApproveOnce().WithContext(new PermissionDecisionContext + OnPermissionRequest = (_, _) => Task.FromResult( + new PermissionDecisionApproveOnce { - Outcome = PermissionDecisionOutcome.AutoApproved, - Source = PermissionDecisionSource.HostPolicy, - Surface = PermissionDecisionSurface.Sdk - })) + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutoApproved, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + } + }) }); DispatchEvent(session, new PermissionRequestedEvent @@ -633,7 +636,7 @@ public async Task PermissionResponse_Omits_DecisionContext_When_Not_Supplied() } [Fact] - public async Task PermissionResponse_Uses_Latest_Context_When_WithContext_Called_Twice() + 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) }); @@ -641,20 +644,25 @@ public async Task PermissionResponse_Uses_Latest_Context_When_WithContext_Called await using var session = await client.CreateSessionAsync(new SessionConfig { - OnPermissionRequest = (_, _) => Task.FromResult( - PermissionDecision.ApproveOnce() - .WithContext(new PermissionDecisionContext + OnPermissionRequest = (_, _) => + { + var decision = new PermissionDecisionApproveOnce + { + DecisionContext = new PermissionDecisionContext { Outcome = PermissionDecisionOutcome.PromptedUser, Source = PermissionDecisionSource.HumanResponse, Surface = PermissionDecisionSurface.Tui - }) - .WithContext(new PermissionDecisionContext - { - Outcome = PermissionDecisionOutcome.AutoApproved, - Source = PermissionDecisionSource.HostPolicy, - Surface = PermissionDecisionSurface.Sdk - })) + } + }; + decision.DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutoApproved, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + }; + return Task.FromResult(decision); + } }); DispatchEvent(session, new PermissionRequestedEvent @@ -687,13 +695,16 @@ public async Task PermissionResponse_Is_Suppressed_For_NoResult_Even_With_Contex OnPermissionRequest = (_, _) => { handlerInvoked.TrySetResult(); - return Task.FromResult( - PermissionDecision.NoResult().WithContext(new PermissionDecisionContext + return Task.FromResult( + new PermissionDecisionNoResult { - Outcome = PermissionDecisionOutcome.PromptedUser, - Source = PermissionDecisionSource.HumanResponse, - Surface = PermissionDecisionSurface.Sdk - })); + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.PromptedUser, + Source = PermissionDecisionSource.HumanResponse, + Surface = PermissionDecisionSurface.Sdk + } + }); } }); @@ -722,13 +733,17 @@ public async Task PermissionResponse_Never_Nests_DecisionContext_Inside_Result() await using var session = await client.CreateSessionAsync(new SessionConfig { - OnPermissionRequest = (_, _) => Task.FromResult( - PermissionDecision.Reject("denied by policy").WithContext(new PermissionDecisionContext + OnPermissionRequest = (_, _) => Task.FromResult( + new PermissionDecisionReject { - Outcome = PermissionDecisionOutcome.AutopilotDenied, - Source = PermissionDecisionSource.HostPolicy, - Surface = PermissionDecisionSurface.Sdk - })) + Feedback = "denied by policy", + DecisionContext = new PermissionDecisionContext + { + Outcome = PermissionDecisionOutcome.AutopilotDenied, + Source = PermissionDecisionSource.HostPolicy, + Surface = PermissionDecisionSurface.Sdk + } + }) }); DispatchEvent(session, new PermissionRequestedEvent From 8df71544c3f5032f99c364cb1cfbc65d288771f7 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 19:26:16 +0200 Subject: [PATCH 6/9] sdk: Name attribution helpers after each SDK's own conventions The Go, Node, and Python helpers were named `WithDecisionContext` and friends, a shape none of those SDKs use. In Go a `WithX` function conventionally builds a functional option rather than decorating a value, and there were no `With` functions in the package at all. Node and Python had no `with`-prefixed helper either. Each now follows the constructor naming its own SDK already uses: `NewAttributedPermissionResult` alongside `NewCanvasError`, `createAttributedPermissionResult` alongside `createCanvas`, and `create_attributed_permission_result` alongside `create_session_fs_adapter`. The Node wrapper also gains a `kind: "attributed"` discriminant so it is narrowed the same way as every other union in that SDK, instead of by testing for the presence of a property. Java and Rust keep their fluent methods, which match long-standing convention in each of those SDKs. Behavior and wire format are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- go/permission_context_test.go | 16 ++++++++-------- go/permissions.go | 21 +++++++++++---------- go/types.go | 2 +- nodejs/src/index.ts | 2 +- nodejs/src/types.ts | 19 ++++++++----------- nodejs/test/client.test.ts | 18 +++++++++++------- nodejs/test/e2e/permissions.e2e.test.ts | 5 +++-- python/copilot/__init__.py | 4 ++-- python/copilot/session.py | 7 ++++--- python/test_permission_decision_context.py | 12 ++++++------ 10 files changed, 55 insertions(+), 51 deletions(-) diff --git a/go/permission_context_test.go b/go/permission_context_test.go index fbbf0e28bc..16c6d2d592 100644 --- a/go/permission_context_test.go +++ b/go/permission_context_test.go @@ -108,7 +108,7 @@ func sampleDecisionContext() *rpc.PermissionDecisionContext { func TestPermissionDecisionContextForwardedAsSiblingOfResult(t *testing.T) { frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { - return WithDecisionContext(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil + return NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil }) if !sent { t.Fatal("expected a permission response to be sent") @@ -162,7 +162,7 @@ func TestPermissionDecisionContextOmittedWithoutAttribution(t *testing.T) { } } -func TestWithDecisionContextReplacesRatherThanNests(t *testing.T) { +func TestAttributedResultReplacesRatherThanNests(t *testing.T) { first := &rpc.PermissionDecisionContext{ Outcome: PermissionDecisionOutcomePromptedUser, Source: PermissionDecisionSourceHumanResponse, @@ -170,7 +170,7 @@ func TestWithDecisionContextReplacesRatherThanNests(t *testing.T) { } second := sampleDecisionContext() - wrapped := WithDecisionContext(WithDecisionContext(&rpc.PermissionDecisionApproveOnce{}, first), second) + 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) @@ -202,7 +202,7 @@ func TestWithDecisionContextReplacesRatherThanNests(t *testing.T) { func TestAttributedNoResultStillSuppressesResponse(t *testing.T) { frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { - return WithDecisionContext(&rpc.PermissionDecisionNoResult{}, sampleDecisionContext()), nil + 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) @@ -216,7 +216,7 @@ func TestAttributedNoResultStillSuppressesResponse(t *testing.T) { // context is silently dropped. func TestValueFormAttributedResultIsUnwrapped(t *testing.T) { frame, sent := runPermissionExchange(t, func(PermissionRequest, PermissionInvocation) (rpc.PermissionDecision, error) { - return *WithDecisionContext(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil + return *NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, sampleDecisionContext()), nil }) if !sent { t.Fatal("expected a permission response to be sent") @@ -240,7 +240,7 @@ func TestValueFormAttributedResultIsUnwrapped(t *testing.T) { } } -func TestWithDecisionContextReplacesContextOnValueForm(t *testing.T) { +func TestAttributedResultReplacesContextOnValueForm(t *testing.T) { first := sampleDecisionContext() second := &rpc.PermissionDecisionContext{ Outcome: PermissionDecisionOutcomePromptedUser, @@ -248,8 +248,8 @@ func TestWithDecisionContextReplacesContextOnValueForm(t *testing.T) { Surface: PermissionDecisionSurfaceTui, } - valueForm := *WithDecisionContext(&rpc.PermissionDecisionApproveOnce{}, first) - replaced := WithDecisionContext(valueForm, second) + valueForm := *NewAttributedPermissionResult(&rpc.PermissionDecisionApproveOnce{}, first) + replaced := NewAttributedPermissionResult(valueForm, second) if replaced.DecisionContext != second { t.Fatal("expected the second context to replace the first") diff --git a/go/permissions.go b/go/permissions.go index 6573bb823e..8aa97c0dea 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -14,7 +14,7 @@ import ( // 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 [WithDecisionContext] rather than by hand. +// through [NewAttributedPermissionResult] rather than by hand. // // Experimental: AttributedPermissionResult is part of an experimental API and // may change or be removed. @@ -25,18 +25,19 @@ type AttributedPermissionResult struct { DecisionContext *rpc.PermissionDecisionContext } -// WithDecisionContext attaches provenance to a permission decision so the -// runtime can attribute auto-approval telemetry to the responding surface. +// 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. Applying WithDecisionContext -// to 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. +// [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: WithDecisionContext is part of an experimental API and may -// change or be removed. -func WithDecisionContext(result rpc.PermissionDecision, decisionContext *rpc.PermissionDecisionContext) *AttributedPermissionResult { +// Experimental: NewAttributedPermissionResult is part of an experimental API +// and may change or be removed. +func NewAttributedPermissionResult(result rpc.PermissionDecision, decisionContext *rpc.PermissionDecisionContext) *AttributedPermissionResult { switch attributed := result.(type) { case *AttributedPermissionResult: result = attributed.PermissionDecision diff --git a/go/types.go b/go/types.go index b10e92716b..23959aa03b 100644 --- a/go/types.go +++ b/go/types.go @@ -380,7 +380,7 @@ type PermissionInvocation struct { } // PermissionDecisionContext describes how and where a permission decision was -// reached. Attach it to a decision with [WithDecisionContext] so the runtime +// 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. // diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index e08abfe593..0dfbb5ff72 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -27,7 +27,7 @@ export { export { defineTool, approveAll, - withDecisionContext, + createAttributedPermissionResult, convertMcpCallToolResult, createSessionFsAdapter, CopilotRequestHandler, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index f6be35146e..5ec355340e 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -1162,37 +1162,34 @@ export type PermissionRequestResult = PermissionDecisionRequest["result"] | { ki * the responding surface. */ export interface AttributedPermissionResult { + kind: "attributed"; result: PermissionRequestResult; decisionContext: PermissionDecisionContext; } /** * Narrows a {@link PermissionHandler} return value to an attributed result. - * - * Every {@link PermissionRequestResult} is a `kind`-discriminated decision and - * never carries `decisionContext`, so its presence unambiguously identifies the - * attributed wrapper. */ export function isAttributedPermissionResult( result: PermissionRequestResult | AttributedPermissionResult ): result is AttributedPermissionResult { - return "decisionContext" in result; + return result.kind === "attributed"; } /** - * Attach provenance describing how and where a permission decision was made, so - * the runtime can attribute auto-approval telemetry. + * Pair a permission decision with the context describing how and where it was + * made, so the runtime can attribute auto-approval telemetry. * - * Applying this to an already-attributed result replaces the previous context - * rather than nesting it. The context is informational only and never changes + * Passing an already-attributed result replaces the previous context rather + * than nesting it. The context is informational only and never changes * permission behavior. */ -export function withDecisionContext( +export function createAttributedPermissionResult( result: PermissionRequestResult | AttributedPermissionResult, decisionContext: PermissionDecisionContext ): AttributedPermissionResult { const inner = isAttributedPermissionResult(result) ? result.result : result; - return { result: inner, decisionContext }; + return { kind: "attributed", result: inner, decisionContext }; } export type PermissionHandler = ( diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 3c4be64540..e048218b92 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -7,7 +7,7 @@ import { join } from "node:path"; import { describe, expect, it, onTestFinished, vi } from "vitest"; import { approveAll, - withDecisionContext, + createAttributedPermissionResult, CopilotClient, createCanvas, RuntimeConnection, @@ -92,7 +92,7 @@ describe("CopilotClient", () => { surface: "sdk" as const, }; session.registerPermissionHandler(() => - withDecisionContext({ kind: "approve-once" }, decisionContext) + createAttributedPermissionResult({ kind: "approve-once" }, decisionContext) ); const spy = vi .spyOn(session.rpc.permissions, "handlePendingPermissionRequest") @@ -135,7 +135,7 @@ describe("CopilotClient", () => { surface: "sdk" as const, }; session.registerPermissionHandler(() => - withDecisionContext({ kind: "no-result" }, decisionContext) + createAttributedPermissionResult({ kind: "no-result" }, decisionContext) ); const spy = vi.spyOn(session.rpc.permissions, "handlePendingPermissionRequest"); @@ -144,7 +144,7 @@ describe("CopilotClient", () => { expect(spy).not.toHaveBeenCalled(); }); - it("replaces the context when withDecisionContext is applied twice", () => { + it("replaces the context when applied twice", () => { const first = { outcome: "auto_approved" as const, source: "judge_recommendation" as const, @@ -156,10 +156,14 @@ describe("CopilotClient", () => { surface: "tui" as const, }; - const once = withDecisionContext({ kind: "approve-once" }, first); - const twice = withDecisionContext(once, second); + const once = createAttributedPermissionResult({ kind: "approve-once" }, first); + const twice = createAttributedPermissionResult(once, second); - expect(twice).toEqual({ result: { kind: "approve-once" }, decisionContext: 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(); diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index 7fbf482ac1..b7fa6087a3 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -13,7 +13,7 @@ import type { PermissionRequestResult, ToolResultObject, } from "../../src/index.js"; -import { approveAll, defineTool, withDecisionContext } 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"; @@ -106,7 +106,8 @@ describe("Permission callbacks", async () => { }; const session = await client.createSession({ - onPermissionRequest: () => withDecisionContext({ kind: "reject" }, decisionContext), + onPermissionRequest: () => + createAttributedPermissionResult({ kind: "reject" }, decisionContext), }); // Spies preserve the original implementation, so the decision still reaches diff --git a/python/copilot/__init__.py b/python/copilot/__init__.py index 38bef5a43c..f7a71ebe91 100644 --- a/python/copilot/__init__.py +++ b/python/copilot/__init__.py @@ -183,7 +183,7 @@ UserPromptTransformedHandler, UserPromptTransformedHookInput, UserPromptTransformedHookOutput, - with_decision_context, + create_attributed_permission_result, ) from .session_fs_provider import ( SessionFsFileInfo, @@ -383,8 +383,8 @@ "UserPromptTransformedHandler", "UserPromptTransformedHookInput", "UserPromptTransformedHookOutput", - "with_decision_context", "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 b15ccba3bc..2399ab36ef 100644 --- a/python/copilot/session.py +++ b/python/copilot/session.py @@ -378,8 +378,9 @@ class AttributedPermissionResult: 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:`with_decision_context` rather than constructing directly, so - re-attributing an already-wrapped result replaces the context instead of nesting. + 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 @@ -389,7 +390,7 @@ class AttributedPermissionResult: """Context describing how and where the decision was reached.""" -def with_decision_context( +def create_attributed_permission_result( result: PermissionRequestResult | AttributedPermissionResult, decision_context: PermissionDecisionContext, ) -> AttributedPermissionResult: diff --git a/python/test_permission_decision_context.py b/python/test_permission_decision_context.py index 0c1a9cb463..2b013942d7 100644 --- a/python/test_permission_decision_context.py +++ b/python/test_permission_decision_context.py @@ -11,7 +11,7 @@ AttributedPermissionResult, CopilotSession, PermissionNoResult, - with_decision_context, + create_attributed_permission_result, ) from copilot.session_events import PermissionRequestRead @@ -38,7 +38,7 @@ async def test_decision_context_serialized_as_sibling_of_result() -> None: request = PermissionRequestRead(intention="Read", path="/workspace/file.txt") def handler(_request, _invocation): - return with_decision_context(PermissionDecisionApproveOnce(), _context()) + return create_attributed_permission_result(PermissionDecisionApproveOnce(), _context()) await session._execute_permission_and_respond("permission-1", request, handler) @@ -71,7 +71,7 @@ def handler(_request, _invocation): assert params["result"]["kind"] == "approve-once" -def test_with_decision_context_replaces_rather_than_nests() -> None: +def test_attributed_result_replaces_rather_than_nests() -> None: first = PermissionDecisionContext( outcome=PermissionDecisionOutcome.PROMPTED_USER, source=PermissionDecisionSource.HUMAN_RESPONSE, @@ -79,8 +79,8 @@ def test_with_decision_context_replaces_rather_than_nests() -> None: ) second = _context() - once_wrapped = with_decision_context(PermissionDecisionApproveOnce(), first) - twice_wrapped = with_decision_context(once_wrapped, second) + 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) @@ -92,7 +92,7 @@ async def test_no_result_with_context_still_suppresses_response() -> None: request = PermissionRequestRead(intention="Read", path="/workspace/file.txt") def handler(_request, _invocation): - return with_decision_context(PermissionNoResult(), _context()) + return create_attributed_permission_result(PermissionNoResult(), _context()) await session._execute_permission_and_respond("permission-1", request, handler) From 575f6aaa4303e5aaf057028c77303aaa2df3c4c3 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 19:34:11 +0200 Subject: [PATCH 7/9] sdk(go): Centralize attribution unwrapping in one helper The pointer/value type switch was duplicated verbatim in NewAttributedPermissionResult and the session permission dispatch. Embedding an interface promotes its methods to the value type too, so both forms satisfy rpc.PermissionDecision and both must be unwrapped -- missing the value case is what produced the bug caught in review. Fold both copies into splitAttribution so that hazard is stated and handled in exactly one place. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- go/permissions.go | 24 ++++++++++++++++++------ go/session.go | 10 +--------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/go/permissions.go b/go/permissions.go index 8aa97c0dea..f27f9b6e62 100644 --- a/go/permissions.go +++ b/go/permissions.go @@ -38,16 +38,28 @@ type AttributedPermissionResult struct { // 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: - result = attributed.PermissionDecision + return attributed.PermissionDecision, attributed.DecisionContext case AttributedPermissionResult: - result = attributed.PermissionDecision - } - return &AttributedPermissionResult{ - PermissionDecision: result, - DecisionContext: decisionContext, + return attributed.PermissionDecision, attributed.DecisionContext } + return result, nil } // PermissionHandler provides pre-built OnPermissionRequest implementations. diff --git a/go/session.go b/go/session.go index 7975665815..600a4bbebc 100644 --- a/go/session.go +++ b/go/session.go @@ -1649,15 +1649,7 @@ func (s *Session) executePermissionAndRespond(requestID string, permissionReques // 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. - var decisionContext *rpc.PermissionDecisionContext - switch attributed := decision.(type) { - case *AttributedPermissionResult: - decisionContext = attributed.DecisionContext - decision = attributed.PermissionDecision - case AttributedPermissionResult: - decisionContext = attributed.DecisionContext - decision = attributed.PermissionDecision - } + decision, decisionContext := splitAttribution(decision) if _, ok := decision.(*rpc.PermissionDecisionNoResult); ok { return } From 549c30e507f4d6a2df3d5d2a4d16f81e2b309555 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Sat, 8 Aug 2026 10:31:56 +0200 Subject: [PATCH 8/9] sdk(java): Rename withContext to setDecisionContext The Java SDK uses setX for mutators (589 of them); withX appears twice and both return a copy rather than mutating in place. withContext was the odd one out on both counts, and did not match its own getter or the sibling setKind/setRules/setFeedback on this class. Also drop the requireNonNull. The other setters here do not null-check, and null now means "no context" in every other SDK, so throwing made Java the outlier rather than the consistent one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- .../copilot/rpc/PermissionRequestResult.java | 17 ++++++-------- ...ssionRequestResultDecisionContextTest.java | 22 +++++++++---------- 2 files changed, 18 insertions(+), 21 deletions(-) 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 651f89cc94..6546291cf7 100644 --- a/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java +++ b/java/src/main/java/com/github/copilot/rpc/PermissionRequestResult.java @@ -5,7 +5,6 @@ package com.github.copilot.rpc; import java.util.List; -import java.util.Objects; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; @@ -195,22 +194,20 @@ public PermissionDecisionContext getDecisionContext() { } /** - * Attaches provenance describing how and where this decision was made, so the + * 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 attached context. - * The context is never serialized inside the result; the SDK forwards it as a + * 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 context - * the decision context; must not be {@code null} + * @param decisionContext + * the decision context, or {@code null} to attach none * @return this result for method chaining - * @throws NullPointerException - * if {@code context} is {@code null} * @since 1.3.0 */ - public PermissionRequestResult withContext(PermissionDecisionContext context) { - this.decisionContext = Objects.requireNonNull(context, "context must not be null"); + 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 index 1db345a19c..395ad50ad6 100644 --- a/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java +++ b/java/src/test/java/com/github/copilot/rpc/PermissionRequestResultDecisionContextTest.java @@ -8,7 +8,6 @@ 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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.fasterxml.jackson.databind.JsonNode; @@ -36,8 +35,8 @@ private static PermissionDecisionContext sampleContext() { } @Test - void withContextForwardsDecisionContextAsSiblingOfResult() throws Exception { - var result = PermissionRequestResult.approveOnce().withContext(sampleContext()); + void setDecisionContextForwardsContextAsSiblingOfResult() throws Exception { + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); var params = new SessionPermissionsHandlePendingPermissionRequestParams("session-1", "req-1", result, result.getDecisionContext()); @@ -66,19 +65,19 @@ void withoutContextOmitsDecisionContextKey() throws Exception { } @Test - void withContextTwiceReplacesRatherThanNests() { + void setDecisionContextTwiceReplacesRatherThanNests() { var first = sampleContext(); var second = new PermissionDecisionContext(PermissionDecisionOutcome.PROMPTED_USER, PermissionDecisionSource.HUMAN_RESPONSE, PermissionDecisionSurface.TUI); - var result = PermissionRequestResult.approveOnce().withContext(first).withContext(second); + var result = PermissionRequestResult.approveOnce().setDecisionContext(first).setDecisionContext(second); - assertSame(second, result.getDecisionContext(), "second withContext must replace the first, not nest"); + assertSame(second, result.getDecisionContext(), "second setDecisionContext must replace the first, not nest"); } @Test void serializingResultWithContextDoesNotEmitContextInsideResult() throws Exception { - var result = PermissionRequestResult.approveOnce().withContext(sampleContext()); + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); JsonNode resultJson = MAPPER.valueToTree(result); @@ -88,10 +87,11 @@ void serializingResultWithContextDoesNotEmitContextInsideResult() throws Excepti } @Test - void withContextRejectsNull() { - var result = PermissionRequestResult.approveOnce(); + void setDecisionContextAcceptsNullAsNoContext() { + var result = PermissionRequestResult.approveOnce().setDecisionContext(sampleContext()); + + result.setDecisionContext(null); - assertThrows(NullPointerException.class, () -> result.withContext(null), - "withContext must reject null rather than silently dropping the context"); + assertNull(result.getDecisionContext(), "null must clear the context rather than throwing"); } } From 277bb6c226c7a3a9a65f00082de86ec1438da4c5 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Sat, 8 Aug 2026 10:53:24 +0200 Subject: [PATCH 9/9] sdk(rust): Make AttributedDecision a struct-style variant Hand-written Rust here has 157 enum variants: 89 unit, 35 tuple with exactly one payload, and 33 struct-style. Every variant carrying two or more values uses the struct form, so a two-payload tuple was the only one of its kind. Name the payloads instead. Construction and both read sites now say which value they mean rather than relying on position. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- rust/src/handler.rs | 11 ++++++++--- rust/src/session.rs | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 585799b4ed..3745c3dd71 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -49,7 +49,12 @@ pub enum PermissionResult { /// /// The context is informational only — it never changes permission /// behavior. - AttributedDecision(PermissionDecision, PermissionDecisionContext), + 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, @@ -105,8 +110,8 @@ impl PermissionResult { /// ``` pub fn with_context(self, context: PermissionDecisionContext) -> Self { match self { - Self::Decision(decision) | Self::AttributedDecision(decision, _) => { - Self::AttributedDecision(decision, context) + Self::Decision(decision) | Self::AttributedDecision { decision, .. } => { + Self::AttributedDecision { decision, context } } Self::NoResult => Self::NoResult, } diff --git a/rust/src/session.rs b/rust/src/session.rs index c41d235213..f096a2ab8f 100644 --- a/rust/src/session.rs +++ b/rust/src/session.rs @@ -1581,7 +1581,7 @@ fn notification_permission_payload(result: &PermissionResult) -> Option { match result { PermissionResult::NoResult => None, PermissionResult::Decision(decision) - | PermissionResult::AttributedDecision(decision, _) => Some( + | PermissionResult::AttributedDecision { decision, .. } => Some( serde_json::to_value(decision).expect("serializing permission decision should succeed"), ), } @@ -1605,7 +1605,7 @@ fn permission_response_params( "requestId": request_id, "result": result_value, }); - if let PermissionResult::AttributedDecision(_, context) = result { + if let PermissionResult::AttributedDecision { context, .. } = result { params["decisionContext"] = serde_json::to_value(context).expect("serializing decision context should succeed"); }