From c5078198594a0c5cd09dfe2565076c8f67073b97 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 7 Aug 2026 14:28:54 +0200 Subject: [PATCH 01/12] 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 77edf919c..808b2ca4d 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 c6c806b1c..c41d23521 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 d3c4faa16..f927a2327 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 02/12] 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 3238c59d9..da8bf0daa 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 54e123791..237e9a9b2 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 7c34ded16..0ce10c290 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 d4b4100b4..68989de12 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 000000000..b267e787b --- /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 24b9cc7f1..95f3973f6 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 99939de4a..42fd42941 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 6d6a877d3..b10e92716 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 4683fdf01..ca2adf462 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 2e5c60100..da914f225 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 000000000..33497de62 --- /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 5ab53471a..e08abfe59 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 ed575a515..996189cf2 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 4ff279189..f6be35146 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 01a97e980..3c4be6454 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 e7c26a293..7fbf482ac 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 a7366db54..38bef5a43 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 92c24bdd8..b15ccba3b 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 000000000..0c1a9cb46 --- /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 8f594841f..28096a892 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 000000000..ef6f60dbe --- /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 03/12] 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 da914f225..651f89cc9 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 33497de62..1db345a19 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 04/12] 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 b267e787b..fbbf0e28b 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 95f3973f6..6573bb823 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 42fd42941..797566581 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 808b2ca4d..585799b4e 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 05/12] 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 237e9a9b2..3eb1d0e08 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 68989de12..edc2fa8b1 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 06/12] 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 fbbf0e28b..16c6d2d59 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 6573bb823..8aa97c0de 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 b10e92716..23959aa03 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 e08abfe59..0dfbb5ff7 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 f6be35146..5ec355340 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 3c4be6454..e048218b9 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 7fbf482ac..b7fa6087a 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 38bef5a43..f7a71ebe9 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 b15ccba3b..2399ab36e 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 0c1a9cb46..2b013942d 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 07/12] 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 8aa97c0de..f27f9b6e6 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 797566581..600a4bbeb 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 08/12] 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 651f89cc9..6546291cf 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 1db345a19..395ad50ad 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 09/12] 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 585799b4e..3745c3dd7 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 c41d23521..f096a2ab8 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"); } From 7a590cf7036833eaff0e3947482348b3d8dc70e2 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Mon, 10 Aug 2026 14:50:39 +0200 Subject: [PATCH 10/12] Preserve Rust permission handler compatibility Add a separate attributed permission handler path so clients can forward decision context without changing the existing PermissionResult enum or PermissionHandler contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- rust/src/handler.rs | 105 +++++++++++++++++++++++++++------- rust/src/permission.rs | 30 ++++++---- rust/src/session.rs | 30 +++++----- rust/src/types.rs | 68 ++++++++++++++++++---- rust/tests/e2e/permissions.rs | 37 ++++++++++-- 5 files changed, 206 insertions(+), 64 deletions(-) diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 3745c3dd7..5914edfd7 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -15,6 +15,8 @@ //! [`Tool::with_handler`](crate::types::Tool::with_handler) on entries passed to //! [`SessionConfig::with_tools`](crate::types::SessionConfig::with_tools). +use std::sync::Arc; + use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -38,28 +40,35 @@ 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] +/// +/// ``` +/// use github_copilot_sdk::handler::PermissionResult; +/// +/// fn is_decision(result: PermissionResult) -> bool { +/// match result { +/// PermissionResult::Decision(_) => true, +/// PermissionResult::NoResult => false, +/// } +/// } +/// ``` #[derive(Debug, Clone)] pub enum PermissionResult { /// Send a permission decision on the wire. Decision(PermissionDecision), - /// Send a permission decision annotated with the context describing how - /// and where it was reached, so the runtime can attribute - /// auto-approval telemetry to the responding surface. - /// - /// The context is informational only — it never changes permission - /// behavior. - AttributedDecision { - /// The decision to send on the wire. - decision: PermissionDecision, - /// Context describing how and where the decision was reached. - context: PermissionDecisionContext, - }, /// Decline to respond to this request, allowing another connected /// client to answer instead. The SDK suppresses the response. NoResult, } +/// A permission result with optional context describing how it was reached. +#[derive(Debug, Clone)] +pub struct AttributedPermissionResult { + /// The permission result. + pub result: PermissionResult, + /// Context describing how and where the decision was reached. + pub context: Option, +} + impl PermissionResult { /// Approve this single request. pub fn approve_once() -> Self { @@ -92,8 +101,7 @@ impl PermissionResult { /// 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`]. + /// It is a no-op on [`PermissionResult::NoResult`]. /// /// ```rust,no_run /// # use github_copilot_sdk::handler::PermissionResult; @@ -108,16 +116,71 @@ impl PermissionResult { /// 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, + pub fn with_context(self, context: PermissionDecisionContext) -> AttributedPermissionResult { + let context = match self { + Self::Decision(_) => Some(context), + Self::NoResult => None, + }; + AttributedPermissionResult { + result: self, + context, + } + } +} + +impl AttributedPermissionResult { + /// Replace the context describing how this decision was reached. + pub fn with_context(mut self, context: PermissionDecisionContext) -> Self { + if matches!(self.result, PermissionResult::Decision(_)) { + self.context = Some(context); + } + self + } +} + +impl From for AttributedPermissionResult { + fn from(result: PermissionResult) -> Self { + Self { + result, + context: None, } } } +/// Handler for permission requests that also reports how the decision was made. +#[async_trait] +pub trait AttributedPermissionHandler: Send + Sync + 'static { + /// Resolve a permission request and report how it was decided. + async fn handle( + &self, + session_id: SessionId, + request_id: RequestId, + data: PermissionRequestData, + ) -> AttributedPermissionResult; +} + +struct UnattributedHandler(Arc); + +#[async_trait] +impl AttributedPermissionHandler for UnattributedHandler { + async fn handle( + &self, + session_id: SessionId, + request_id: RequestId, + data: PermissionRequestData, + ) -> AttributedPermissionResult { + PermissionHandler::handle(&*self.0, session_id, request_id, data) + .await + .into() + } +} + +pub(crate) fn attributed( + handler: Arc, +) -> Arc { + Arc::new(UnattributedHandler(handler)) +} + impl From for PermissionResult { fn from(value: PermissionDecision) -> Self { Self::Decision(value) diff --git a/rust/src/permission.rs b/rust/src/permission.rs index e353ce315..fe1aab6d7 100644 --- a/rust/src/permission.rs +++ b/rust/src/permission.rs @@ -16,7 +16,9 @@ use std::sync::Arc; use async_trait::async_trait; -use crate::handler::{PermissionHandler, PermissionResult, permission_handler_failure}; +use crate::handler::{ + AttributedPermissionHandler, PermissionHandler, PermissionResult, permission_handler_failure, +}; use crate::types::{PermissionRequestData, RequestId, SessionId}; /// Return a [`PermissionHandler`] that approves requests when managed settings @@ -93,12 +95,16 @@ impl std::fmt::Debug for Policy { /// `requestPermission: false`). pub(crate) fn resolve_handler( handler: Option>, + attributed_handler: Option>, policy: Option, -) -> Option> { - match (handler, policy) { - (_, Some(policy)) => Some(Arc::new(PolicyHandler { policy })), - (Some(h), None) => Some(h), - (None, None) => None, +) -> Option> { + match (handler, attributed_handler, policy) { + (_, _, Some(policy)) => Some(crate::handler::attributed(Arc::new(PolicyHandler { + policy, + }))), + (_, Some(h), None) => Some(h), + (Some(h), None, None) => Some(crate::handler::attributed(h)), + (None, None, None) => None, } } @@ -227,12 +233,13 @@ mod tests { } } let resolved = - resolve_handler(Some(Arc::new(AlwaysApprove)), Some(Policy::DenyAll)).unwrap(); + resolve_handler(Some(Arc::new(AlwaysApprove)), None, Some(Policy::DenyAll)).unwrap(); // Policy wins -- the AlwaysApprove handler is discarded. assert!(matches!( resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) - .await, + .await + .result, PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) )); } @@ -251,17 +258,18 @@ mod tests { PermissionResult::approve_once() } } - let resolved = resolve_handler(Some(Arc::new(H)), None).unwrap(); + let resolved = resolve_handler(Some(Arc::new(H)), None, None).unwrap(); assert!(matches!( resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) - .await, + .await + .result, PermissionResult::Decision(crate::types::PermissionDecision::ApproveOnce(_)) )); } #[test] fn resolve_handler_with_neither_returns_none() { - assert!(resolve_handler(None, None).is_none()); + assert!(resolve_handler(None, None, None).is_none()); } } diff --git a/rust/src/session.rs b/rust/src/session.rs index f096a2ab8..32b77221a 100644 --- a/rust/src/session.rs +++ b/rust/src/session.rs @@ -20,8 +20,8 @@ use crate::generated::session_events::{ SessionCanvasClosedData, SessionErrorData, SessionEventType, }; use crate::handler::{ - AutoModeSwitchHandler, AutoModeSwitchResponse, ElicitationHandler, ExitPlanModeHandler, - McpAuthHandler, McpAuthRequest, McpAuthResult, PermissionHandler, PermissionResult, + AttributedPermissionHandler, AutoModeSwitchHandler, AutoModeSwitchResponse, ElicitationHandler, + ExitPlanModeHandler, McpAuthHandler, McpAuthRequest, McpAuthResult, PermissionResult, UserInputHandler, UserInputResponse, }; use crate::hooks::SessionHooks; @@ -56,7 +56,7 @@ const TOOL_SEARCH_TOOL_NAME: &str = "tool_search_tool"; /// are derived from these fields. #[derive(Clone)] pub(crate) struct SessionHandlers { - pub permission: Option>, + pub permission: Option>, pub managed_settings_enabled: bool, pub elicitation: Option>, pub mcp_auth: Option>, @@ -902,6 +902,7 @@ impl Client { let permission_handler = crate::permission::resolve_handler( runtime.permission_handler.take(), + runtime.attributed_permission_handler.take(), runtime.permission_policy.take(), ); let handlers = SessionHandlers { @@ -1175,6 +1176,7 @@ impl Client { let permission_handler = crate::permission::resolve_handler( runtime.permission_handler.take(), + runtime.attributed_permission_handler.take(), runtime.permission_policy.take(), ); let handlers = SessionHandlers { @@ -1580,15 +1582,14 @@ fn permission_request_data( fn notification_permission_payload(result: &PermissionResult) -> Option { match result { PermissionResult::NoResult => None, - PermissionResult::Decision(decision) - | PermissionResult::AttributedDecision { decision, .. } => Some( + PermissionResult::Decision(decision) => Some( serde_json::to_value(decision).expect("serializing permission decision should succeed"), ), } } /// Build the full `session.permissions.handlePendingPermissionRequest` -/// params for a [`PermissionResult`]. +/// params for an attributed permission result. /// /// `decisionContext` is a sibling of `result` and is only present when the /// handler attributed the decision — omitting it preserves legacy behavior. @@ -1597,15 +1598,15 @@ fn notification_permission_payload(result: &PermissionResult) -> Option { fn permission_response_params( session_id: &SessionId, request_id: &RequestId, - result: &PermissionResult, + result: &crate::handler::AttributedPermissionResult, ) -> Option { - let result_value = notification_permission_payload(result)?; + let result_value = notification_permission_payload(&result.result)?; let mut params = serde_json::json!({ "sessionId": session_id, "requestId": request_id, "result": result_value, }); - if let PermissionResult::AttributedDecision { context, .. } = result { + if let Some(context) = &result.context { params["decisionContext"] = serde_json::to_value(context).expect("serializing decision context should succeed"); } @@ -2641,7 +2642,7 @@ mod tests { let params = permission_response_params( &SessionId::from("session-1"), &RequestId::from("permission-1"), - &PermissionResult::approve_once(), + &PermissionResult::approve_once().into(), ) .unwrap(); assert_eq!( @@ -2686,7 +2687,7 @@ mod tests { permission_response_params( &SessionId::from("session-1"), &RequestId::from("permission-1"), - &PermissionResult::NoResult, + &PermissionResult::NoResult.into(), ) .is_none() ); @@ -2694,10 +2695,9 @@ mod tests { #[test] fn with_context_is_a_no_op_on_no_result() { - assert!(matches!( - PermissionResult::no_result().with_context(attribution_context()), - PermissionResult::NoResult - )); + let result = PermissionResult::no_result().with_context(attribution_context()); + assert!(matches!(result.result, PermissionResult::NoResult)); + assert!(result.context.is_none()); } #[test] diff --git a/rust/src/types.rs b/rust/src/types.rs index f927a2327..6c5a1bfc9 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -25,8 +25,8 @@ use crate::generated::session_events::ReasoningSummary; /// Context window tier for models that support tiered context windows. pub use crate::generated::session_events::{ContextTier, SessionLimitsConfig}; use crate::handler::{ - AutoModeSwitchHandler, ElicitationHandler, ExitPlanModeHandler, McpAuthHandler, - PermissionHandler, UserInputHandler, + AttributedPermissionHandler, AutoModeSwitchHandler, ElicitationHandler, ExitPlanModeHandler, + McpAuthHandler, PermissionHandler, UserInputHandler, }; use crate::hooks::SessionHooks; use crate::provider_token::BearerTokenProvider; @@ -2166,6 +2166,8 @@ pub struct SessionConfig { /// `requestPermission: false` on the wire so the runtime does not /// emit `permission.requested` broadcasts to this client. pub permission_handler: Option>, + /// Optional context-aware permission-request handler. + pub attributed_permission_handler: Option>, /// Optional elicitation-request handler. When `None`, /// `requestElicitation: false` goes on the wire. pub elicitation_handler: Option>, @@ -2418,6 +2420,7 @@ impl Default for SessionConfig { managed_settings: None, session_fs_provider: None, permission_handler: None, + attributed_permission_handler: None, elicitation_handler: None, mcp_auth_handler: None, user_input_handler: None, @@ -2442,6 +2445,7 @@ impl Default for SessionConfig { /// stays a pure data shape. pub(crate) struct SessionConfigRuntime { pub permission_handler: Option>, + pub attributed_permission_handler: Option>, pub permission_policy: Option, pub elicitation_handler: Option>, pub mcp_auth_handler: Option>, @@ -2473,8 +2477,9 @@ impl SessionConfig { mut self, session_id: Option, ) -> Result<(crate::wire::SessionCreateWire, SessionConfigRuntime), crate::Error> { - let permission_active = - self.permission_handler.is_some() || self.permission_policy.is_some(); + let permission_active = self.permission_handler.is_some() + || self.attributed_permission_handler.is_some() + || self.permission_policy.is_some(); let request_user_input = self.user_input_handler.is_some(); let request_exit_plan_mode = self.exit_plan_mode_handler.is_some(); let request_auto_mode_switch = self.auto_mode_switch_handler.is_some(); @@ -2586,6 +2591,7 @@ impl SessionConfig { let runtime = SessionConfigRuntime { permission_handler: self.permission_handler, + attributed_permission_handler: self.attributed_permission_handler, permission_policy: self.permission_policy, elicitation_handler: self.elicitation_handler, mcp_auth_handler: self.mcp_auth_handler, @@ -2609,6 +2615,17 @@ impl SessionConfig { /// short-circuits permission prompts for this client. pub fn with_permission_handler(mut self, handler: Arc) -> Self { self.permission_handler = Some(handler); + self.attributed_permission_handler = None; + self + } + + /// Install a context-aware permission handler for this session. + pub fn with_attributed_permission_handler( + mut self, + handler: Arc, + ) -> Self { + self.attributed_permission_handler = Some(handler); + self.permission_handler = None; self } @@ -3431,6 +3448,8 @@ pub struct ResumeSessionConfig { /// Optional permission-request handler. See /// [`SessionConfig::permission_handler`]. pub permission_handler: Option>, + /// Optional context-aware permission handler. + pub attributed_permission_handler: Option>, /// Optional elicitation handler. See /// [`SessionConfig::elicitation_handler`]. pub elicitation_handler: Option>, @@ -3601,8 +3620,9 @@ impl ResumeSessionConfig { pub(crate) fn into_wire( mut self, ) -> Result<(crate::wire::SessionResumeWire, SessionConfigRuntime), crate::Error> { - let permission_active = - self.permission_handler.is_some() || self.permission_policy.is_some(); + let permission_active = self.permission_handler.is_some() + || self.attributed_permission_handler.is_some() + || self.permission_policy.is_some(); let request_user_input = self.user_input_handler.is_some(); let request_exit_plan_mode = self.exit_plan_mode_handler.is_some(); let request_auto_mode_switch = self.auto_mode_switch_handler.is_some(); @@ -3716,6 +3736,7 @@ impl ResumeSessionConfig { let runtime = SessionConfigRuntime { permission_handler: self.permission_handler, + attributed_permission_handler: self.attributed_permission_handler, permission_policy: self.permission_policy, elicitation_handler: self.elicitation_handler, mcp_auth_handler: self.mcp_auth_handler, @@ -3808,6 +3829,7 @@ impl ResumeSessionConfig { suppress_resume_event: None, continue_pending_work: None, permission_handler: None, + attributed_permission_handler: None, elicitation_handler: None, mcp_auth_handler: None, user_input_handler: None, @@ -3827,6 +3849,17 @@ impl ResumeSessionConfig { /// Install a [`PermissionHandler`] for the resumed session. pub fn with_permission_handler(mut self, handler: Arc) -> Self { self.permission_handler = Some(handler); + self.attributed_permission_handler = None; + self + } + + /// Install a context-aware permission handler for the resumed session. + pub fn with_attributed_permission_handler( + mut self, + handler: Arc, + ) -> Self { + self.attributed_permission_handler = Some(handler); + self.permission_handler = None; self } @@ -7536,7 +7569,7 @@ mod tests { mod permission_builder_tests { use std::sync::Arc; - use crate::handler::{ApproveAllHandler, PermissionHandler, PermissionResult}; + use crate::handler::{ApproveAllHandler, AttributedPermissionHandler, PermissionResult}; use crate::permission; use crate::types::{ PermissionDecision, PermissionRequestData, RequestId, ResumeSessionConfig, SessionConfig, @@ -7552,18 +7585,29 @@ mod permission_builder_tests { /// Apply the same policy-resolution logic that `Client::create_session` /// uses, so tests exercise the effective handler. - fn resolve_create(mut cfg: SessionConfig) -> Option> { - permission::resolve_handler(cfg.permission_handler.take(), cfg.permission_policy.take()) + fn resolve_create(mut cfg: SessionConfig) -> Option> { + permission::resolve_handler( + cfg.permission_handler.take(), + cfg.attributed_permission_handler.take(), + cfg.permission_policy.take(), + ) } - fn resolve_resume(mut cfg: ResumeSessionConfig) -> Option> { - permission::resolve_handler(cfg.permission_handler.take(), cfg.permission_policy.take()) + fn resolve_resume( + mut cfg: ResumeSessionConfig, + ) -> Option> { + permission::resolve_handler( + cfg.permission_handler.take(), + cfg.attributed_permission_handler.take(), + cfg.permission_policy.take(), + ) } - async fn dispatch(handler: &Arc) -> PermissionResult { + async fn dispatch(handler: &Arc) -> PermissionResult { handler .handle(SessionId::from("s1"), RequestId::new("1"), data()) .await + .result } #[tokio::test] diff --git a/rust/tests/e2e/permissions.rs b/rust/tests/e2e/permissions.rs index 28096a892..5e9e67099 100644 --- a/rust/tests/e2e/permissions.rs +++ b/rust/tests/e2e/permissions.rs @@ -1,7 +1,9 @@ use std::sync::Arc; use async_trait::async_trait; -use github_copilot_sdk::handler::{PermissionHandler, PermissionResult}; +use github_copilot_sdk::handler::{ + AttributedPermissionHandler, AttributedPermissionResult, PermissionHandler, PermissionResult, +}; use github_copilot_sdk::rpc::PermissionsSetApproveAllRequest; use github_copilot_sdk::session_events::{SessionEventType, ToolExecutionCompleteData}; use github_copilot_sdk::{ @@ -155,9 +157,10 @@ async fn should_honor_a_decision_annotated_with_decisioncontext() { .create_session( SessionConfig::default() .with_github_token(DEFAULT_TEST_TOKEN) - .with_permission_handler(Arc::new(StaticPermissionHandler::new( - PermissionResult::reject(None).with_context(decision_context), - ))), + .with_attributed_permission_handler(Arc::new( + StaticPermissionHandler::new(PermissionResult::reject(None)) + .with_context(decision_context), + )), ) .await .expect("create session"); @@ -699,11 +702,20 @@ fn permission_request_tool_call_id(request: &PermissionRequestData) -> Option<&s #[derive(Clone)] struct StaticPermissionHandler { result: PermissionResult, + context: Option, } impl StaticPermissionHandler { fn new(result: PermissionResult) -> Self { - Self { result } + Self { + result, + context: None, + } + } + + fn with_context(mut self, context: PermissionDecisionContext) -> Self { + self.context = Some(context); + self } } @@ -719,6 +731,21 @@ impl PermissionHandler for StaticPermissionHandler { } } +#[async_trait] +impl AttributedPermissionHandler for StaticPermissionHandler { + async fn handle( + &self, + _session_id: SessionId, + _request_id: RequestId, + _data: PermissionRequestData, + ) -> AttributedPermissionResult { + match &self.context { + Some(context) => self.result.clone().with_context(context.clone()), + None => self.result.clone().into(), + } + } +} + struct RecordingPermissionHandler { request_tx: mpsc::UnboundedSender, } From a53f6242c8c3162921ea6d13fc2f9ba5b11fc47b Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Thu, 13 Aug 2026 16:57:05 +0200 Subject: [PATCH 11/12] Simplify Rust permission attribution Keep PermissionHandler as the single dispatch API and carry decision context through PermissionResult. Replace the ineffective rejection E2E test with an exact fake-server wire assertion. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- rust/src/handler.rs | 95 +++++------------------- rust/src/permission.rs | 29 +++----- rust/src/session.rs | 136 ++++++++++++++-------------------- rust/src/types.rs | 68 +++-------------- rust/tests/e2e/permissions.rs | 98 +----------------------- rust/tests/session_test.rs | 64 +++++++++++++++- 6 files changed, 161 insertions(+), 329 deletions(-) diff --git a/rust/src/handler.rs b/rust/src/handler.rs index 5914edfd7..d9e84ccc2 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -15,8 +15,6 @@ //! [`Tool::with_handler`](crate::types::Tool::with_handler) on entries passed to //! [`SessionConfig::with_tools`](crate::types::SessionConfig::with_tools). -use std::sync::Arc; - use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -37,16 +35,18 @@ use crate::types::{ /// Decision returned by a [`PermissionHandler`]. /// /// Either a concrete wire-level [`PermissionDecision`] (approve, reject, -/// 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. +/// approve-for-session, approve-permanently, user-not-available, …), an +/// attributed decision carrying telemetry context, or +/// [`PermissionResult::NoResult`], which tells the SDK to suppress its response +/// so another connected client can answer instead. /// /// ``` /// use github_copilot_sdk::handler::PermissionResult; /// /// fn is_decision(result: PermissionResult) -> bool { /// match result { -/// PermissionResult::Decision(_) => true, +/// PermissionResult::Decision(_) +/// | PermissionResult::AttributedDecision { .. } => true, /// PermissionResult::NoResult => false, /// } /// } @@ -55,20 +55,18 @@ use crate::types::{ pub enum PermissionResult { /// Send a permission decision on the wire. Decision(PermissionDecision), + /// Send a permission decision with context describing how it was reached. + AttributedDecision { + /// The permission decision. + 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, } -/// A permission result with optional context describing how it was reached. -#[derive(Debug, Clone)] -pub struct AttributedPermissionResult { - /// The permission result. - pub result: PermissionResult, - /// Context describing how and where the decision was reached. - pub context: Option, -} - impl PermissionResult { /// Approve this single request. pub fn approve_once() -> Self { @@ -116,71 +114,16 @@ impl PermissionResult { /// surface: PermissionDecisionSurface::Sdk, /// }); /// ``` - pub fn with_context(self, context: PermissionDecisionContext) -> AttributedPermissionResult { - let context = match self { - Self::Decision(_) => Some(context), - Self::NoResult => None, - }; - AttributedPermissionResult { - result: self, - context, - } - } -} - -impl AttributedPermissionResult { - /// Replace the context describing how this decision was reached. - pub fn with_context(mut self, context: PermissionDecisionContext) -> Self { - if matches!(self.result, PermissionResult::Decision(_)) { - self.context = Some(context); - } - self - } -} - -impl From for AttributedPermissionResult { - fn from(result: PermissionResult) -> Self { - Self { - result, - context: None, + pub fn with_context(self, context: PermissionDecisionContext) -> Self { + match self { + Self::Decision(decision) | Self::AttributedDecision { decision, .. } => { + Self::AttributedDecision { decision, context } + } + Self::NoResult => Self::NoResult, } } } -/// Handler for permission requests that also reports how the decision was made. -#[async_trait] -pub trait AttributedPermissionHandler: Send + Sync + 'static { - /// Resolve a permission request and report how it was decided. - async fn handle( - &self, - session_id: SessionId, - request_id: RequestId, - data: PermissionRequestData, - ) -> AttributedPermissionResult; -} - -struct UnattributedHandler(Arc); - -#[async_trait] -impl AttributedPermissionHandler for UnattributedHandler { - async fn handle( - &self, - session_id: SessionId, - request_id: RequestId, - data: PermissionRequestData, - ) -> AttributedPermissionResult { - PermissionHandler::handle(&*self.0, session_id, request_id, data) - .await - .into() - } -} - -pub(crate) fn attributed( - handler: Arc, -) -> Arc { - Arc::new(UnattributedHandler(handler)) -} - impl From for PermissionResult { fn from(value: PermissionDecision) -> Self { Self::Decision(value) diff --git a/rust/src/permission.rs b/rust/src/permission.rs index fe1aab6d7..b936d9bae 100644 --- a/rust/src/permission.rs +++ b/rust/src/permission.rs @@ -16,9 +16,7 @@ use std::sync::Arc; use async_trait::async_trait; -use crate::handler::{ - AttributedPermissionHandler, PermissionHandler, PermissionResult, permission_handler_failure, -}; +use crate::handler::{PermissionHandler, PermissionResult, permission_handler_failure}; use crate::types::{PermissionRequestData, RequestId, SessionId}; /// Return a [`PermissionHandler`] that approves requests when managed settings @@ -95,16 +93,11 @@ impl std::fmt::Debug for Policy { /// `requestPermission: false`). pub(crate) fn resolve_handler( handler: Option>, - attributed_handler: Option>, policy: Option, -) -> Option> { - match (handler, attributed_handler, policy) { - (_, _, Some(policy)) => Some(crate::handler::attributed(Arc::new(PolicyHandler { - policy, - }))), - (_, Some(h), None) => Some(h), - (Some(h), None, None) => Some(crate::handler::attributed(h)), - (None, None, None) => None, +) -> Option> { + match (handler, policy) { + (_, Some(policy)) => Some(Arc::new(PolicyHandler { policy })), + (handler, None) => handler, } } @@ -233,13 +226,12 @@ mod tests { } } let resolved = - resolve_handler(Some(Arc::new(AlwaysApprove)), None, Some(Policy::DenyAll)).unwrap(); + resolve_handler(Some(Arc::new(AlwaysApprove)), Some(Policy::DenyAll)).unwrap(); // Policy wins -- the AlwaysApprove handler is discarded. assert!(matches!( resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) - .await - .result, + .await, PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) )); } @@ -258,18 +250,17 @@ mod tests { PermissionResult::approve_once() } } - let resolved = resolve_handler(Some(Arc::new(H)), None, None).unwrap(); + let resolved = resolve_handler(Some(Arc::new(H)), None).unwrap(); assert!(matches!( resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) - .await - .result, + .await, PermissionResult::Decision(crate::types::PermissionDecision::ApproveOnce(_)) )); } #[test] fn resolve_handler_with_neither_returns_none() { - assert!(resolve_handler(None, None, None).is_none()); + assert!(resolve_handler(None, None).is_none()); } } diff --git a/rust/src/session.rs b/rust/src/session.rs index 32b77221a..0854b7785 100644 --- a/rust/src/session.rs +++ b/rust/src/session.rs @@ -12,16 +12,16 @@ use tracing::{Instrument, warn}; use crate::canvas::CanvasHandler; use crate::generated::api_types::{ - LogRequest, ModelSwitchToRequest, OpenCanvasInstance, RegisterEventInterestParams, - ToolsGetCurrentMetadataResult, rpc_methods, + LogRequest, ModelSwitchToRequest, OpenCanvasInstance, PermissionDecisionRequest, + RegisterEventInterestParams, ToolsGetCurrentMetadataResult, rpc_methods, }; use crate::generated::session_events::{ CommandExecuteData, ElicitationRequestedData, ExternalToolRequestedData, McpOauthRequiredData, SessionCanvasClosedData, SessionErrorData, SessionEventType, }; use crate::handler::{ - AttributedPermissionHandler, AutoModeSwitchHandler, AutoModeSwitchResponse, ElicitationHandler, - ExitPlanModeHandler, McpAuthHandler, McpAuthRequest, McpAuthResult, PermissionResult, + AutoModeSwitchHandler, AutoModeSwitchResponse, ElicitationHandler, ExitPlanModeHandler, + McpAuthHandler, McpAuthRequest, McpAuthResult, PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse, }; use crate::hooks::SessionHooks; @@ -56,7 +56,7 @@ const TOOL_SEARCH_TOOL_NAME: &str = "tool_search_tool"; /// are derived from these fields. #[derive(Clone)] pub(crate) struct SessionHandlers { - pub permission: Option>, + pub permission: Option>, pub managed_settings_enabled: bool, pub elicitation: Option>, pub mcp_auth: Option>, @@ -902,7 +902,6 @@ impl Client { let permission_handler = crate::permission::resolve_handler( runtime.permission_handler.take(), - runtime.attributed_permission_handler.take(), runtime.permission_policy.take(), ); let handlers = SessionHandlers { @@ -1176,7 +1175,6 @@ impl Client { let permission_handler = crate::permission::resolve_handler( runtime.permission_handler.take(), - runtime.attributed_permission_handler.take(), runtime.permission_policy.take(), ); let handlers = SessionHandlers { @@ -1575,21 +1573,8 @@ fn permission_request_data( } } -/// Map a [`PermissionResult`] to the `result` payload sent back to the -/// server via `session.permissions.handlePendingPermissionRequest`. -/// -/// Returns `None` when the SDK must not send a response. -fn notification_permission_payload(result: &PermissionResult) -> Option { - match result { - PermissionResult::NoResult => None, - PermissionResult::Decision(decision) => Some( - serde_json::to_value(decision).expect("serializing permission decision should succeed"), - ), - } -} - /// Build the full `session.permissions.handlePendingPermissionRequest` -/// params for an attributed permission result. +/// params for a permission result. /// /// `decisionContext` is a sibling of `result` and is only present when the /// handler attributed the decision — omitting it preserves legacy behavior. @@ -1598,18 +1583,23 @@ fn notification_permission_payload(result: &PermissionResult) -> Option { fn permission_response_params( session_id: &SessionId, request_id: &RequestId, - result: &crate::handler::AttributedPermissionResult, + result: &PermissionResult, ) -> Option { - let result_value = notification_permission_payload(&result.result)?; - let mut params = serde_json::json!({ - "sessionId": session_id, - "requestId": request_id, - "result": result_value, - }); - if let Some(context) = &result.context { - params["decisionContext"] = - serde_json::to_value(context).expect("serializing decision context should succeed"); - } + let (decision, decision_context) = match result { + PermissionResult::Decision(decision) => (decision, None), + PermissionResult::AttributedDecision { decision, context } => { + (decision, Some(context.clone())) + } + PermissionResult::NoResult => return None, + }; + let mut params = serde_json::to_value(PermissionDecisionRequest { + decision_context, + request_id: request_id.clone(), + result: decision.clone(), + }) + .expect("serializing permission response should succeed"); + params["sessionId"] = + serde_json::to_value(session_id).expect("serializing session ID should succeed"); Some(params) } @@ -1816,7 +1806,7 @@ async fn handle_notification( let rpc_start = Instant::now(); let _ = client .call( - "session.permissions.handlePendingPermissionRequest", + rpc_methods::SESSION_PERMISSIONS_HANDLEPENDINGPERMISSIONREQUEST, Some(params), ) .await; @@ -2587,10 +2577,7 @@ fn inject_transform_sections_resume( mod tests { use serde_json::json; - use super::{ - has_managed_settings, notification_permission_payload, permission_request_data, - permission_response_params, - }; + use super::{has_managed_settings, permission_request_data, permission_response_params}; use crate::handler::PermissionResult; use crate::types::{ PermissionDecisionContext, PermissionDecisionOutcome, PermissionDecisionSource, @@ -2604,31 +2591,6 @@ mod tests { assert!(!has_managed_settings(None, None)); } - #[test] - fn notification_payload_suppresses_no_result() { - assert!(notification_permission_payload(&PermissionResult::NoResult).is_none()); - } - - #[test] - fn notification_payload_serializes_decisions() { - assert_eq!( - notification_permission_payload(&PermissionResult::approve_once()), - Some(json!({ "kind": "approve-once" })) - ); - assert_eq!( - notification_permission_payload(&PermissionResult::reject(None)), - Some(json!({ "kind": "reject" })) - ); - assert_eq!( - notification_permission_payload(&PermissionResult::reject(Some("bad".to_string()))), - Some(json!({ "kind": "reject", "feedback": "bad" })) - ); - assert_eq!( - notification_permission_payload(&PermissionResult::user_not_available()), - Some(json!({ "kind": "user-not-available" })) - ); - } - fn attribution_context() -> PermissionDecisionContext { PermissionDecisionContext { outcome: PermissionDecisionOutcome::AutoApproved, @@ -2639,21 +2601,36 @@ mod tests { #[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().into(), - ) - .unwrap(); - assert_eq!( - params, - json!({ - "sessionId": "session-1", - "requestId": "permission-1", - "result": { "kind": "approve-once" }, - }) - ); - assert!(params.get("decisionContext").is_none()); + for (result, expected) in [ + ( + PermissionResult::approve_once(), + json!({ "kind": "approve-once" }), + ), + (PermissionResult::reject(None), json!({ "kind": "reject" })), + ( + PermissionResult::reject(Some("bad".to_string())), + json!({ "kind": "reject", "feedback": "bad" }), + ), + ( + PermissionResult::user_not_available(), + json!({ "kind": "user-not-available" }), + ), + ] { + let params = permission_response_params( + &SessionId::from("session-1"), + &RequestId::from("permission-1"), + &result, + ) + .unwrap(); + assert_eq!( + params, + json!({ + "sessionId": "session-1", + "requestId": "permission-1", + "result": expected, + }) + ); + } } #[test] @@ -2687,7 +2664,7 @@ mod tests { permission_response_params( &SessionId::from("session-1"), &RequestId::from("permission-1"), - &PermissionResult::NoResult.into(), + &PermissionResult::NoResult, ) .is_none() ); @@ -2696,8 +2673,7 @@ mod tests { #[test] fn with_context_is_a_no_op_on_no_result() { let result = PermissionResult::no_result().with_context(attribution_context()); - assert!(matches!(result.result, PermissionResult::NoResult)); - assert!(result.context.is_none()); + assert!(matches!(result, PermissionResult::NoResult)); } #[test] diff --git a/rust/src/types.rs b/rust/src/types.rs index 6c5a1bfc9..f927a2327 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -25,8 +25,8 @@ use crate::generated::session_events::ReasoningSummary; /// Context window tier for models that support tiered context windows. pub use crate::generated::session_events::{ContextTier, SessionLimitsConfig}; use crate::handler::{ - AttributedPermissionHandler, AutoModeSwitchHandler, ElicitationHandler, ExitPlanModeHandler, - McpAuthHandler, PermissionHandler, UserInputHandler, + AutoModeSwitchHandler, ElicitationHandler, ExitPlanModeHandler, McpAuthHandler, + PermissionHandler, UserInputHandler, }; use crate::hooks::SessionHooks; use crate::provider_token::BearerTokenProvider; @@ -2166,8 +2166,6 @@ pub struct SessionConfig { /// `requestPermission: false` on the wire so the runtime does not /// emit `permission.requested` broadcasts to this client. pub permission_handler: Option>, - /// Optional context-aware permission-request handler. - pub attributed_permission_handler: Option>, /// Optional elicitation-request handler. When `None`, /// `requestElicitation: false` goes on the wire. pub elicitation_handler: Option>, @@ -2420,7 +2418,6 @@ impl Default for SessionConfig { managed_settings: None, session_fs_provider: None, permission_handler: None, - attributed_permission_handler: None, elicitation_handler: None, mcp_auth_handler: None, user_input_handler: None, @@ -2445,7 +2442,6 @@ impl Default for SessionConfig { /// stays a pure data shape. pub(crate) struct SessionConfigRuntime { pub permission_handler: Option>, - pub attributed_permission_handler: Option>, pub permission_policy: Option, pub elicitation_handler: Option>, pub mcp_auth_handler: Option>, @@ -2477,9 +2473,8 @@ impl SessionConfig { mut self, session_id: Option, ) -> Result<(crate::wire::SessionCreateWire, SessionConfigRuntime), crate::Error> { - let permission_active = self.permission_handler.is_some() - || self.attributed_permission_handler.is_some() - || self.permission_policy.is_some(); + let permission_active = + self.permission_handler.is_some() || self.permission_policy.is_some(); let request_user_input = self.user_input_handler.is_some(); let request_exit_plan_mode = self.exit_plan_mode_handler.is_some(); let request_auto_mode_switch = self.auto_mode_switch_handler.is_some(); @@ -2591,7 +2586,6 @@ impl SessionConfig { let runtime = SessionConfigRuntime { permission_handler: self.permission_handler, - attributed_permission_handler: self.attributed_permission_handler, permission_policy: self.permission_policy, elicitation_handler: self.elicitation_handler, mcp_auth_handler: self.mcp_auth_handler, @@ -2615,17 +2609,6 @@ impl SessionConfig { /// short-circuits permission prompts for this client. pub fn with_permission_handler(mut self, handler: Arc) -> Self { self.permission_handler = Some(handler); - self.attributed_permission_handler = None; - self - } - - /// Install a context-aware permission handler for this session. - pub fn with_attributed_permission_handler( - mut self, - handler: Arc, - ) -> Self { - self.attributed_permission_handler = Some(handler); - self.permission_handler = None; self } @@ -3448,8 +3431,6 @@ pub struct ResumeSessionConfig { /// Optional permission-request handler. See /// [`SessionConfig::permission_handler`]. pub permission_handler: Option>, - /// Optional context-aware permission handler. - pub attributed_permission_handler: Option>, /// Optional elicitation handler. See /// [`SessionConfig::elicitation_handler`]. pub elicitation_handler: Option>, @@ -3620,9 +3601,8 @@ impl ResumeSessionConfig { pub(crate) fn into_wire( mut self, ) -> Result<(crate::wire::SessionResumeWire, SessionConfigRuntime), crate::Error> { - let permission_active = self.permission_handler.is_some() - || self.attributed_permission_handler.is_some() - || self.permission_policy.is_some(); + let permission_active = + self.permission_handler.is_some() || self.permission_policy.is_some(); let request_user_input = self.user_input_handler.is_some(); let request_exit_plan_mode = self.exit_plan_mode_handler.is_some(); let request_auto_mode_switch = self.auto_mode_switch_handler.is_some(); @@ -3736,7 +3716,6 @@ impl ResumeSessionConfig { let runtime = SessionConfigRuntime { permission_handler: self.permission_handler, - attributed_permission_handler: self.attributed_permission_handler, permission_policy: self.permission_policy, elicitation_handler: self.elicitation_handler, mcp_auth_handler: self.mcp_auth_handler, @@ -3829,7 +3808,6 @@ impl ResumeSessionConfig { suppress_resume_event: None, continue_pending_work: None, permission_handler: None, - attributed_permission_handler: None, elicitation_handler: None, mcp_auth_handler: None, user_input_handler: None, @@ -3849,17 +3827,6 @@ impl ResumeSessionConfig { /// Install a [`PermissionHandler`] for the resumed session. pub fn with_permission_handler(mut self, handler: Arc) -> Self { self.permission_handler = Some(handler); - self.attributed_permission_handler = None; - self - } - - /// Install a context-aware permission handler for the resumed session. - pub fn with_attributed_permission_handler( - mut self, - handler: Arc, - ) -> Self { - self.attributed_permission_handler = Some(handler); - self.permission_handler = None; self } @@ -7569,7 +7536,7 @@ mod tests { mod permission_builder_tests { use std::sync::Arc; - use crate::handler::{ApproveAllHandler, AttributedPermissionHandler, PermissionResult}; + use crate::handler::{ApproveAllHandler, PermissionHandler, PermissionResult}; use crate::permission; use crate::types::{ PermissionDecision, PermissionRequestData, RequestId, ResumeSessionConfig, SessionConfig, @@ -7585,29 +7552,18 @@ mod permission_builder_tests { /// Apply the same policy-resolution logic that `Client::create_session` /// uses, so tests exercise the effective handler. - fn resolve_create(mut cfg: SessionConfig) -> Option> { - permission::resolve_handler( - cfg.permission_handler.take(), - cfg.attributed_permission_handler.take(), - cfg.permission_policy.take(), - ) + fn resolve_create(mut cfg: SessionConfig) -> Option> { + permission::resolve_handler(cfg.permission_handler.take(), cfg.permission_policy.take()) } - fn resolve_resume( - mut cfg: ResumeSessionConfig, - ) -> Option> { - permission::resolve_handler( - cfg.permission_handler.take(), - cfg.attributed_permission_handler.take(), - cfg.permission_policy.take(), - ) + fn resolve_resume(mut cfg: ResumeSessionConfig) -> Option> { + permission::resolve_handler(cfg.permission_handler.take(), cfg.permission_policy.take()) } - async fn dispatch(handler: &Arc) -> PermissionResult { + async fn dispatch(handler: &Arc) -> PermissionResult { handler .handle(SessionId::from("s1"), RequestId::new("1"), data()) .await - .result } #[tokio::test] diff --git a/rust/tests/e2e/permissions.rs b/rust/tests/e2e/permissions.rs index 5e9e67099..8f594841f 100644 --- a/rust/tests/e2e/permissions.rs +++ b/rust/tests/e2e/permissions.rs @@ -1,15 +1,11 @@ use std::sync::Arc; use async_trait::async_trait; -use github_copilot_sdk::handler::{ - AttributedPermissionHandler, AttributedPermissionResult, PermissionHandler, PermissionResult, -}; +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::{ - PermissionDecisionContext, PermissionDecisionOutcome, PermissionDecisionSource, - PermissionDecisionSurface, PermissionRequestData, RequestId, ResumeSessionConfig, - SessionConfig, SessionId, + PermissionRequestData, RequestId, ResumeSessionConfig, SessionConfig, SessionId, }; use tokio::sync::{mpsc, oneshot}; @@ -124,70 +120,6 @@ 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_attributed_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( @@ -702,20 +634,11 @@ fn permission_request_tool_call_id(request: &PermissionRequestData) -> Option<&s #[derive(Clone)] struct StaticPermissionHandler { result: PermissionResult, - context: Option, } impl StaticPermissionHandler { fn new(result: PermissionResult) -> Self { - Self { - result, - context: None, - } - } - - fn with_context(mut self, context: PermissionDecisionContext) -> Self { - self.context = Some(context); - self + Self { result } } } @@ -731,21 +654,6 @@ impl PermissionHandler for StaticPermissionHandler { } } -#[async_trait] -impl AttributedPermissionHandler for StaticPermissionHandler { - async fn handle( - &self, - _session_id: SessionId, - _request_id: RequestId, - _data: PermissionRequestData, - ) -> AttributedPermissionResult { - match &self.context { - Some(context) => self.result.clone().with_context(context.clone()), - None => self.result.clone().into(), - } - } -} - struct RecordingPermissionHandler { request_tx: mpsc::UnboundedSender, } diff --git a/rust/tests/session_test.rs b/rust/tests/session_test.rs index 727911081..c312a6586 100644 --- a/rust/tests/session_test.rs +++ b/rust/tests/session_test.rs @@ -10,7 +10,7 @@ use github_copilot_sdk::canvas::{CanvasDeclaration, CanvasHandler, CanvasResult} use github_copilot_sdk::handler::{ ApproveAllHandler, AutoModeSwitchHandler, AutoModeSwitchResponse, ElicitationHandler, ExitPlanModeHandler, ExitPlanModeResult, McpAuthHandler, McpAuthRequest, McpAuthResult, - UserInputHandler, UserInputResponse, + PermissionHandler, PermissionResult, UserInputHandler, UserInputResponse, }; use github_copilot_sdk::rpc::{ CanvasProviderInvokeActionRequest, CanvasProviderOpenRequest, CanvasProviderOpenResult, @@ -24,8 +24,9 @@ use github_copilot_sdk::types::{ CanvasProviderIdentity, CloudSessionOptions, CloudSessionRepository, CommandContext, CommandDefinition, CommandHandler, DeliveryMode, DisableBypassPermissionsMode, ElicitationRequest, ElicitationResult, ExitPlanModeData, ExtensionInfo, ManagedSettings, - ManagedSettingsPermissions, MessageOptions, RequestId, SessionConfig, SessionId, - SetModelOptions, Tool, ToolInvocation, ToolResult, + ManagedSettingsPermissions, MessageOptions, PermissionDecisionContext, + PermissionDecisionOutcome, PermissionDecisionSource, PermissionDecisionSurface, RequestId, + SessionConfig, SessionId, SetModelOptions, Tool, ToolInvocation, ToolResult, }; use github_copilot_sdk::{Client, ContextTier, ErrorKind, ProtocolErrorKind, tool}; use serde_json::Value; @@ -38,6 +39,24 @@ struct TestCanvasHandler; struct CancelMcpAuthHandler; +struct AttributedApproveHandler; + +#[async_trait] +impl PermissionHandler for AttributedApproveHandler { + async fn handle( + &self, + _session_id: SessionId, + _request_id: RequestId, + _data: github_copilot_sdk::PermissionRequestData, + ) -> PermissionResult { + PermissionResult::approve_once().with_context(PermissionDecisionContext { + outcome: PermissionDecisionOutcome::PromptedUser, + source: PermissionDecisionSource::HumanResponse, + surface: PermissionDecisionSurface::CopilotApp, + }) + } +} + #[async_trait] impl McpAuthHandler for CancelMcpAuthHandler { async fn handle( @@ -2491,6 +2510,45 @@ async fn approve_all_handler_approves_permission() { assert_eq!(request["params"]["result"]["kind"], "approve-once"); } +#[tokio::test] +async fn attributed_permission_result_forwards_context_beside_result() { + let (_session, mut server) = create_session_pair_with_config(|cfg| { + cfg.with_permission_handler(Arc::new(AttributedApproveHandler)) + }) + .await; + + server + .send_event( + "permission.requested", + serde_json::json!({ + "requestId": "perm-attributed", + "sessionId": server.session_id, + "permissionRequest": { "kind": "shell" }, + }), + ) + .await; + + let request = timeout(TIMEOUT, server.read_request()).await.unwrap(); + assert_eq!( + request["method"], + "session.permissions.handlePendingPermissionRequest" + ); + assert_eq!( + request["params"], + serde_json::json!({ + "sessionId": server.session_id, + "requestId": "perm-attributed", + "result": { "kind": "approve-once" }, + "decisionContext": { + "outcome": "prompted_user", + "source": "human_response", + "surface": "copilot_app", + }, + }) + ); + assert!(request["params"]["result"].get("decisionContext").is_none()); +} + #[tokio::test] async fn session_event_notification_reaches_handler() { let (session, mut server) = create_session_pair().await; From 3654e00cbcce2f795de5666afe6f82fd5c8050b9 Mon Sep 17 00:00:00 2001 From: Aymen Furter Date: Fri, 14 Aug 2026 11:13:55 +0200 Subject: [PATCH 12/12] Unify Rust permission decisions Store optional decision context on the existing Decision variant so every decision follows one semantic path and downstream matches receive a compiler-guided migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 79152db2-4cc7-4777-983a-655fae5b68c9 --- rust/src/handler.rs | 75 +++++++++++++++++++++-------------- rust/src/permission.rs | 35 ++++++++++++---- rust/src/session.rs | 5 +-- rust/src/types.rs | 60 ++++++++++++++++++++++------ rust/tests/e2e/permissions.rs | 7 ++-- rust/tests/session_test.rs | 8 ++-- 6 files changed, 131 insertions(+), 59 deletions(-) diff --git a/rust/src/handler.rs b/rust/src/handler.rs index d9e84ccc2..f1f0d9566 100644 --- a/rust/src/handler.rs +++ b/rust/src/handler.rs @@ -35,18 +35,17 @@ use crate::types::{ /// Decision returned by a [`PermissionHandler`]. /// /// Either a concrete wire-level [`PermissionDecision`] (approve, reject, -/// approve-for-session, approve-permanently, user-not-available, …), an -/// attributed decision carrying telemetry context, or -/// [`PermissionResult::NoResult`], which tells the SDK to suppress its response -/// so another connected client can answer instead. +/// approve-for-session, approve-permanently, user-not-available, …) with +/// optional telemetry context, or [`PermissionResult::NoResult`], which tells +/// the SDK to suppress its response so another connected client can answer +/// instead. /// /// ``` /// use github_copilot_sdk::handler::PermissionResult; /// /// fn is_decision(result: PermissionResult) -> bool { /// match result { -/// PermissionResult::Decision(_) -/// | PermissionResult::AttributedDecision { .. } => true, +/// PermissionResult::Decision { .. } => true, /// PermissionResult::NoResult => false, /// } /// } @@ -54,13 +53,11 @@ use crate::types::{ #[derive(Debug, Clone)] pub enum PermissionResult { /// Send a permission decision on the wire. - Decision(PermissionDecision), - /// Send a permission decision with context describing how it was reached. - AttributedDecision { - /// The permission decision. + Decision { + /// The decision to send. decision: PermissionDecision, - /// Context describing how and where the decision was reached. - context: PermissionDecisionContext, + /// Optional context describing how and where the decision was reached. + context: Option, }, /// Decline to respond to this request, allowing another connected /// client to answer instead. The SDK suppresses the response. @@ -70,24 +67,31 @@ pub enum PermissionResult { impl PermissionResult { /// Approve this single request. pub fn approve_once() -> Self { - Self::Decision(PermissionDecision::ApproveOnce( - PermissionDecisionApproveOnce::default(), - )) + Self::Decision { + decision: PermissionDecision::ApproveOnce(PermissionDecisionApproveOnce::default()), + context: None, + } } /// Reject the request, optionally forwarding feedback to the LLM. pub fn reject(feedback: impl Into>) -> Self { - Self::Decision(PermissionDecision::Reject(PermissionDecisionReject { - feedback: feedback.into(), - ..Default::default() - })) + Self::Decision { + decision: PermissionDecision::Reject(PermissionDecisionReject { + feedback: feedback.into(), + ..Default::default() + }), + context: None, + } } /// Deny because no user is available to confirm. pub fn user_not_available() -> Self { - Self::Decision(PermissionDecision::UserNotAvailable( - PermissionDecisionUserNotAvailable::default(), - )) + Self::Decision { + decision: PermissionDecision::UserNotAvailable( + PermissionDecisionUserNotAvailable::default(), + ), + context: None, + } } /// Decline to respond, allowing another connected client to answer @@ -116,9 +120,10 @@ 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::Decision { + decision, + context: Some(context), + }, Self::NoResult => Self::NoResult, } } @@ -126,7 +131,10 @@ impl PermissionResult { impl From for PermissionResult { fn from(value: PermissionDecision) -> Self { - Self::Decision(value) + Self::Decision { + decision: value, + context: None, + } } } @@ -385,7 +393,10 @@ mod tests { .await; assert!(matches!( result, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -403,7 +414,10 @@ mod tests { .await; assert!(matches!( result, - PermissionResult::Decision(PermissionDecision::UserNotAvailable(_)) + PermissionResult::Decision { + decision: PermissionDecision::UserNotAvailable(_), + .. + } )); } @@ -433,7 +447,10 @@ mod tests { .await; assert!(matches!( result, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); } diff --git a/rust/src/permission.rs b/rust/src/permission.rs index b936d9bae..57c078570 100644 --- a/rust/src/permission.rs +++ b/rust/src/permission.rs @@ -151,7 +151,10 @@ mod tests { assert!(matches!( h.handle(SessionId::from("s"), RequestId::new("1"), data()) .await, - PermissionResult::Decision(crate::types::PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -163,7 +166,10 @@ mod tests { assert!(matches!( h.handle(SessionId::from("s"), RequestId::new("1"), request) .await, - PermissionResult::Decision(crate::types::PermissionDecision::UserNotAvailable(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::UserNotAvailable(_), + .. + } )); } @@ -173,7 +179,10 @@ mod tests { assert!(matches!( h.handle(SessionId::from("s"), RequestId::new("1"), data()) .await, - PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::Reject(_), + .. + } )); } @@ -183,7 +192,10 @@ mod tests { assert!(matches!( h.handle(SessionId::from("s"), RequestId::new("1"), data()) .await, - PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::Reject(_), + .. + } )); } @@ -207,7 +219,10 @@ mod tests { assert!(matches!( h.handle(SessionId::from("s"), RequestId::new("1"), request) .await, - PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::Reject(_), + .. + } )); } @@ -232,7 +247,10 @@ mod tests { resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) .await, - PermissionResult::Decision(crate::types::PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::Reject(_), + .. + } )); } @@ -255,7 +273,10 @@ mod tests { resolved .handle(SessionId::from("s"), RequestId::new("1"), data()) .await, - PermissionResult::Decision(crate::types::PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: crate::types::PermissionDecision::ApproveOnce(_), + .. + } )); } diff --git a/rust/src/session.rs b/rust/src/session.rs index 0854b7785..99e793015 100644 --- a/rust/src/session.rs +++ b/rust/src/session.rs @@ -1586,10 +1586,7 @@ fn permission_response_params( result: &PermissionResult, ) -> Option { let (decision, decision_context) = match result { - PermissionResult::Decision(decision) => (decision, None), - PermissionResult::AttributedDecision { decision, context } => { - (decision, Some(context.clone())) - } + PermissionResult::Decision { decision, context } => (decision, context.clone()), PermissionResult::NoResult => return None, }; let mut params = serde_json::to_value(PermissionDecisionRequest { diff --git a/rust/src/types.rs b/rust/src/types.rs index f927a2327..4637c1e0d 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -7574,7 +7574,10 @@ mod permission_builder_tests { let h = resolve_create(cfg).expect("policy + handler yields handler"); assert!(matches!( dispatch(&h).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -7584,7 +7587,10 @@ mod permission_builder_tests { let h = resolve_create(cfg).expect("policy alone yields handler"); assert!(matches!( dispatch(&h).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -7602,11 +7608,17 @@ mod permission_builder_tests { let hb = resolve_create(b).unwrap(); assert!(matches!( dispatch(&ha).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); assert!(matches!( dispatch(&hb).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -7622,11 +7634,17 @@ mod permission_builder_tests { let hb = resolve_create(b).unwrap(); assert!(matches!( dispatch(&ha).await, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); assert!(matches!( dispatch(&hb).await, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); } @@ -7638,7 +7656,10 @@ mod permission_builder_tests { let h = resolve_create(cfg).unwrap(); assert!(matches!( dispatch(&h).await, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); } @@ -7657,11 +7678,17 @@ mod permission_builder_tests { let hb = resolve_create(b).unwrap(); assert!(matches!( dispatch(&ha).await, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); assert!(matches!( dispatch(&hb).await, - PermissionResult::Decision(PermissionDecision::Reject(_)) + PermissionResult::Decision { + decision: PermissionDecision::Reject(_), + .. + } )); } @@ -7673,7 +7700,10 @@ mod permission_builder_tests { let h = resolve_resume(cfg).unwrap(); assert!(matches!( dispatch(&h).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } @@ -7689,11 +7719,17 @@ mod permission_builder_tests { let hb = resolve_resume(b).unwrap(); assert!(matches!( dispatch(&ha).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); assert!(matches!( dispatch(&hb).await, - PermissionResult::Decision(PermissionDecision::ApproveOnce(_)) + PermissionResult::Decision { + decision: PermissionDecision::ApproveOnce(_), + .. + } )); } diff --git a/rust/tests/e2e/permissions.rs b/rust/tests/e2e/permissions.rs index 8f594841f..65b37928d 100644 --- a/rust/tests/e2e/permissions.rs +++ b/rust/tests/e2e/permissions.rs @@ -50,9 +50,10 @@ async fn should_handle_permission_handler_errors_gracefully() { assert!(matches!( result, - PermissionResult::Decision( - github_copilot_sdk::types::PermissionDecision::UserNotAvailable(_) - ) + PermissionResult::Decision { + decision: github_copilot_sdk::types::PermissionDecision::UserNotAvailable(_), + .. + } )); } diff --git a/rust/tests/session_test.rs b/rust/tests/session_test.rs index c312a6586..0a9fc02c3 100644 --- a/rust/tests/session_test.rs +++ b/rust/tests/session_test.rs @@ -39,10 +39,10 @@ struct TestCanvasHandler; struct CancelMcpAuthHandler; -struct AttributedApproveHandler; +struct ContextualApproveHandler; #[async_trait] -impl PermissionHandler for AttributedApproveHandler { +impl PermissionHandler for ContextualApproveHandler { async fn handle( &self, _session_id: SessionId, @@ -2511,9 +2511,9 @@ async fn approve_all_handler_approves_permission() { } #[tokio::test] -async fn attributed_permission_result_forwards_context_beside_result() { +async fn permission_result_forwards_context_beside_result() { let (_session, mut server) = create_session_pair_with_config(|cfg| { - cfg.with_permission_handler(Arc::new(AttributedApproveHandler)) + cfg.with_permission_handler(Arc::new(ContextualApproveHandler)) }) .await;