Skip to content

Commit c942cf1

Browse files
joshspicerCopilot
andcommitted
Fix managed permission approval surfaces
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent e5417bd commit c942cf1

4 files changed

Lines changed: 86 additions & 10 deletions

File tree

nodejs/src/index.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ export {
3939
// consumers can import them directly from "@github/copilot-sdk" instead of
4040
// reaching into the package's internal dist layout. See issue #1156.
4141
//
42-
// Three names from this file are also explicitly exported elsewhere in this
42+
// Five names from this file are also explicitly exported elsewhere in this
4343
// module — `SessionEvent` (re-exported below from `./types.js`),
44-
// `PermissionRequest` (re-exported below from `./types.js`), and
45-
// `AssistantMessageEvent` (re-exported above from `./session.js`). Per the
46-
// ECMAScript module spec, the explicit named re-exports shadow the names
47-
// arriving via `export type *`, so the hand-authored public API surface for
48-
// those three identifiers is preserved unchanged.
44+
// `PermissionRequest` (re-exported below from `./types.js`),
45+
// `PermissionRequestedData`/`PermissionRequestedEvent` (also re-exported below
46+
// from `./types.js`), and `AssistantMessageEvent` (re-exported above from
47+
// `./session.js`). Per the ECMAScript module spec, the explicit named re-exports
48+
// shadow the names arriving via `export type *`, so the hand-authored public API
49+
// surface for those five identifiers is preserved unchanged.
4950
export type * from "./generated/session-events.js";
5051
export type {
5152
CommandContext,
@@ -109,6 +110,8 @@ export type {
109110
NamedProviderConfig,
110111
PermissionHandler,
111112
PermissionRequest,
113+
PermissionRequestedData,
114+
PermissionRequestedEvent,
112115
PermissionRequestResult,
113116
ProviderConfig,
114117
ProviderModelConfig,

nodejs/src/types.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import type { Canvas } from "./canvas.js";
1111
import type { SessionFsProvider } from "./sessionFsProvider.js";
1212
import type { CopilotRequestHandler } from "./copilotRequestHandler.js";
1313
import type {
14+
PermissionRequest as GeneratedPermissionRequest,
15+
PermissionRequestedData as GeneratedPermissionRequestedData,
16+
PermissionRequestedEvent as GeneratedPermissionRequestedEvent,
1417
ReasoningSummary,
1518
SessionLimitsConfig,
1619
SessionEvent as GeneratedSessionEvent,
@@ -35,7 +38,9 @@ export type {
3538
ModelBillingTokenPrices,
3639
ModelBillingTokenPricesLongContext,
3740
} from "./generated/rpc.js";
38-
export type SessionEvent = GeneratedSessionEvent;
41+
export type SessionEvent =
42+
| Exclude<GeneratedSessionEvent, { type: "permission.requested" }>
43+
| PermissionRequestedEvent;
3944
export type { ReasoningSummary } from "./generated/session-events.js";
4045
export type { SessionFsProvider } from "./sessionFsProvider.js";
4146
export { createSessionFsAdapter } from "./sessionFsProvider.js";
@@ -1092,6 +1097,8 @@ export type SystemMessageConfig =
10921097
| SystemMessageReplaceConfig
10931098
| SystemMessageCustomizeConfig;
10941099

1100+
import type { PermissionDecisionRequest } from "./generated/rpc.js";
1101+
10951102
/**
10961103
* Permission request types from the server. This is the generated
10971104
* discriminated union from the runtime schema — switch on `kind` to
@@ -1103,12 +1110,20 @@ export type SystemMessageConfig =
11031110
* normal confirmation UI. The runtime currently emits it for managed Shell,
11041111
* Read, Edit, and Domain selector asks.
11051112
*/
1106-
import type { PermissionRequest as GeneratedPermissionRequest } from "./generated/session-events.js";
11071113
export type PermissionRequest = GeneratedPermissionRequest & {
11081114
readonly managedApprovalRequired?: boolean;
11091115
};
11101116

1111-
import type { PermissionDecisionRequest } from "./generated/rpc.js";
1117+
export type PermissionRequestedData = Omit<
1118+
GeneratedPermissionRequestedData,
1119+
"permissionRequest"
1120+
> & {
1121+
permissionRequest: PermissionRequest;
1122+
};
1123+
1124+
export type PermissionRequestedEvent = Omit<GeneratedPermissionRequestedEvent, "data"> & {
1125+
data: PermissionRequestedData;
1126+
};
11121127

11131128
/**
11141129
* Permission decision result returned from a {@link PermissionHandler}.
@@ -1123,7 +1138,11 @@ export type PermissionHandler = (
11231138
invocation: { sessionId: string }
11241139
) => Promise<PermissionRequestResult> | PermissionRequestResult;
11251140

1126-
export const approveAll: PermissionHandler = () => ({ kind: "approve-once" });
1141+
/**
1142+
* Approves permission requests unless managed policy requires an explicit human decision.
1143+
*/
1144+
export const approveAll: PermissionHandler = (request) =>
1145+
request.managedApprovalRequired ? { kind: "no-result" } : { kind: "approve-once" };
11271146

11281147
export const defaultJoinSessionPermissionHandler: PermissionHandler =
11291148
(): PermissionRequestResult => ({

nodejs/test/client.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,25 @@ async function stopClient(client: CopilotClient): Promise<void> {
1919
await client.stop();
2020
}
2121

22+
describe("approveAll", () => {
23+
const request = {
24+
kind: "url" as const,
25+
url: "https://api.example.com/data",
26+
intention: "Fetch domain data",
27+
};
28+
const invocation = { sessionId: "session-1" };
29+
30+
it("approves ordinary permission requests", () => {
31+
expect(approveAll(request, invocation)).toEqual({ kind: "approve-once" });
32+
});
33+
34+
it("leaves managed permission requests pending for human approval", () => {
35+
expect(approveAll({ ...request, managedApprovalRequired: true }, invocation)).toEqual({
36+
kind: "no-result",
37+
});
38+
});
39+
});
40+
2241
describe("CopilotClient", () => {
2342
it("disposes the stdio connection when child stdin emits an error", async () => {
2443
const client = new CopilotClient();

nodejs/test/session-event-types.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import type {
1919
// The aggregate union; must still resolve via the package root.
2020
SessionEvent,
2121
PermissionRequest,
22+
PermissionRequestedData,
23+
PermissionRequestedEvent,
2224

2325
// *Data payload types from the v0.3.0 generated session-event schema.
2426
AssistantMessageData,
@@ -81,6 +83,11 @@ type _AssistantMessageEventStaysAlignedWithSessionEventUnion = _AssertEqual<
8183
Extract<SessionEvent, { type: "assistant.message" }>
8284
>;
8385
const _assistantMessageEventAlignmentCheck: _AssistantMessageEventStaysAlignedWithSessionEventUnion = true;
86+
type _PermissionRequestedEventStaysAlignedWithSessionEventUnion = _AssertEqual<
87+
PermissionRequestedEvent,
88+
Extract<SessionEvent, { type: "permission.requested" }>
89+
>;
90+
const _permissionRequestedEventAlignmentCheck: _PermissionRequestedEventStaysAlignedWithSessionEventUnion = true;
8491

8592
describe("Session event type exports (#1156)", () => {
8693
it("exposes the headline ToolExecutionStartData type with a usable shape", () => {
@@ -115,6 +122,32 @@ describe("Session event type exports (#1156)", () => {
115122
expect(request.managedApprovalRequired).toBe(true);
116123
});
117124

125+
it("exposes managed approval metadata through permission event types", () => {
126+
const data: PermissionRequestedData = {
127+
permissionRequest: {
128+
kind: "url",
129+
url: "https://api.example.com/data",
130+
intention: "Fetch domain data",
131+
managedApprovalRequired: true,
132+
},
133+
requestId: "permission-1",
134+
};
135+
const event: SessionEvent = {
136+
id: "evt-permission-1",
137+
parentId: null,
138+
timestamp: "2026-01-01T00:00:00.000Z",
139+
type: "permission.requested",
140+
data,
141+
};
142+
143+
if (event.type !== "permission.requested") {
144+
throw new Error("expected permission.requested narrowing");
145+
}
146+
147+
const permissionEvent: PermissionRequestedEvent = event;
148+
expect(permissionEvent.data.permissionRequest.managedApprovalRequired).toBe(true);
149+
});
150+
118151
it("wraps ToolExecutionStartData inside the exported ToolExecutionStartEvent", () => {
119152
const event: ToolExecutionStartEvent = {
120153
id: "evt-1",
@@ -172,6 +205,7 @@ describe("Session event type exports (#1156)", () => {
172205
assertImportable<ToolExecutionProgressData>();
173206
assertImportable<ToolExecutionStartData>();
174207
assertImportable<UserMessageData>();
208+
assertImportable<PermissionRequestedData>();
175209

176210
assertImportable<AssistantMessageEvent>();
177211
assertImportable<ErrorEvent>();
@@ -181,6 +215,7 @@ describe("Session event type exports (#1156)", () => {
181215
assertImportable<ToolExecutionCompleteEvent>();
182216
assertImportable<ToolExecutionStartEvent>();
183217
assertImportable<UserMessageEvent>();
218+
assertImportable<PermissionRequestedEvent>();
184219

185220
// Supporting auxiliary types referenced by the *Data shapes — these
186221
// must round-trip through the package root too, otherwise consumers

0 commit comments

Comments
 (0)