Skip to content

Commit 2db37c2

Browse files
vincentkocvincentkoc
committed
refactor(copilot): drop unused permission policy helpers
1 parent 6370f20 commit 2db37c2

2 files changed

Lines changed: 4 additions & 285 deletions

File tree

‎extensions/copilot/src/permission-bridge.test.ts‎

Lines changed: 2 additions & 166 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
11
// Copilot tests cover permission bridge plugin behavior.
22
import type {
33
PermissionRequest as SdkPermissionRequest,
4-
PermissionRequestResult as SdkPermissionRequestResult,
54
} from "@github/copilot-sdk";
65
import { describe, expect, it, vi } from "vitest";
76
import {
8-
allowListPolicy,
9-
allowOncePolicy,
10-
composePolicies,
117
createPermissionBridge,
12-
delegatingPolicy,
138
rejectAllPolicy,
149
REJECT_ALL_FEEDBACK,
1510
type CopilotPermissionContext,
@@ -52,168 +47,9 @@ describe("rejectAllPolicy", () => {
5247
});
5348
});
5449

55-
describe("allowOncePolicy", () => {
56-
it("returns approve-once for every request kind", async () => {
57-
for (const kind of [
58-
"shell",
59-
"write",
60-
"mcp",
61-
"read",
62-
"url",
63-
"custom-tool",
64-
"memory",
65-
"hook",
66-
] as const) {
67-
const result = await allowOncePolicy(makeCtx({ request: makeRequest({ kind }) }));
68-
expect(result).toEqual({ kind: "approve-once" });
69-
}
70-
});
71-
});
72-
73-
describe("allowListPolicy", () => {
74-
it("approves listed kinds and rejects others with default feedback", async () => {
75-
const policy = allowListPolicy({ kinds: ["read"] });
76-
const approved = await policy(makeCtx({ request: makeRequest({ kind: "read" }) }));
77-
expect(approved).toEqual({ kind: "approve-once" });
78-
const rejected = await policy(makeCtx({ request: makeRequest({ kind: "shell" }) }));
79-
expect(rejected).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
80-
});
81-
82-
it("uses custom rejectFeedback when provided", async () => {
83-
const policy = allowListPolicy({
84-
kinds: ["read"],
85-
rejectFeedback: "only reads allowed",
86-
});
87-
const result = await policy(makeCtx({ request: makeRequest({ kind: "write" }) }));
88-
expect(result).toEqual({ kind: "reject", feedback: "only reads allowed" });
89-
});
90-
91-
it("supports multiple kinds in the allow-list", async () => {
92-
const policy = allowListPolicy({ kinds: ["read", "write"] });
93-
expect(await policy(makeCtx({ request: makeRequest({ kind: "read" }) }))).toEqual({
94-
kind: "approve-once",
95-
});
96-
expect(await policy(makeCtx({ request: makeRequest({ kind: "write" }) }))).toEqual({
97-
kind: "approve-once",
98-
});
99-
expect((await policy(makeCtx({ request: makeRequest({ kind: "mcp" }) })))?.kind).toBe("reject");
100-
});
101-
102-
it("rejects all when given an empty allow-list", async () => {
103-
const policy = allowListPolicy({ kinds: [] });
104-
for (const kind of ["shell", "read", "write"] as const) {
105-
const result = await policy(makeCtx({ request: makeRequest({ kind }) }));
106-
expect(result?.kind).toBe("reject");
107-
}
108-
});
109-
});
110-
111-
describe("delegatingPolicy", () => {
112-
it("forwards the request to the host callback and returns its decision", async () => {
113-
const onRequest = vi.fn<CopilotPermissionPolicy>().mockResolvedValue({
114-
kind: "approve-for-session",
115-
} satisfies SdkPermissionRequestResult);
116-
const policy = delegatingPolicy({ onRequest });
117-
const ctx = makeCtx({ sessionId: "sess-xyz", request: makeRequest({ kind: "write" }) });
118-
const result = await policy(ctx);
119-
expect(result).toEqual({ kind: "approve-for-session" });
120-
expect(onRequest).toHaveBeenCalledTimes(1);
121-
expect(onRequest).toHaveBeenCalledWith(ctx);
122-
});
123-
124-
it("returns the rejectAll default when host callback returns undefined", async () => {
125-
const onRequest = vi.fn<CopilotPermissionPolicy>().mockResolvedValue(undefined);
126-
const policy = delegatingPolicy({ onRequest });
127-
const result = await policy(makeCtx());
128-
expect(result).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
129-
});
130-
131-
it("rejects with the error message when host callback throws", async () => {
132-
const onRequest = vi
133-
.fn<CopilotPermissionPolicy>()
134-
.mockRejectedValue(new Error("host policy boom"));
135-
const policy = delegatingPolicy({ onRequest });
136-
const result = await policy(makeCtx());
137-
expect(result?.kind).toBe("reject");
138-
expect((result as { feedback?: string }).feedback).toContain("host policy boom");
139-
});
140-
141-
it("falls back to onError policy when host callback throws", async () => {
142-
const onError = vi.fn<CopilotPermissionPolicy>().mockResolvedValue({ kind: "approve-once" });
143-
const policy = delegatingPolicy({
144-
onRequest: () => {
145-
throw new Error("host policy boom");
146-
},
147-
onError,
148-
});
149-
const result = await policy(makeCtx());
150-
expect(result).toEqual({ kind: "approve-once" });
151-
expect(onError).toHaveBeenCalledTimes(1);
152-
});
153-
154-
it("falls through to a hard-coded reject if onError also throws", async () => {
155-
const policy = delegatingPolicy({
156-
onRequest: () => {
157-
throw new Error("host boom");
158-
},
159-
onError: () => {
160-
throw new Error("fallback boom");
161-
},
162-
});
163-
const result = await policy(makeCtx());
164-
expect(result?.kind).toBe("reject");
165-
expect((result as { feedback?: string }).feedback).toContain("host boom");
166-
});
167-
168-
it("formats non-Error throws via JSON.stringify", async () => {
169-
const policy = delegatingPolicy({
170-
onRequest: () => {
171-
throw { code: 42, msg: "weird" } as unknown as Error;
172-
},
173-
});
174-
const result = await policy(makeCtx());
175-
expect((result as { feedback?: string }).feedback).toContain('"code":42');
176-
});
177-
});
178-
179-
describe("composePolicies", () => {
180-
it("returns the first non-undefined result and skips subsequent policies", async () => {
181-
const a: CopilotPermissionPolicy = () => undefined;
182-
const b: CopilotPermissionPolicy = () => ({ kind: "approve-once" });
183-
const c = vi.fn<CopilotPermissionPolicy>(() => ({
184-
kind: "reject",
185-
feedback: "should never run",
186-
}));
187-
const policy = composePolicies(a, b, c);
188-
const result = await policy(makeCtx());
189-
expect(result).toEqual({ kind: "approve-once" });
190-
expect(c).not.toHaveBeenCalled();
191-
});
192-
193-
it("falls through to fail-closed reject when all policies return undefined", async () => {
194-
const policy = composePolicies(
195-
() => undefined,
196-
() => undefined,
197-
);
198-
const result = await policy(makeCtx());
199-
expect(result).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
200-
});
201-
202-
it("short-circuits to reject if any policy throws (does not consult later policies)", async () => {
203-
const later = vi.fn<CopilotPermissionPolicy>(() => ({ kind: "approve-once" }));
204-
const policy = composePolicies(() => {
205-
throw new Error("nope");
206-
}, later);
207-
const result = await policy(makeCtx());
208-
expect(result?.kind).toBe("reject");
209-
expect((result as { feedback?: string }).feedback).toContain("nope");
210-
expect(later).not.toHaveBeenCalled();
211-
});
212-
});
213-
21450
describe("createPermissionBridge", () => {
21551
it("adapts a policy to the SDK PermissionHandler shape", async () => {
216-
const handler = createPermissionBridge(allowOncePolicy);
52+
const handler = createPermissionBridge(() => ({ kind: "approve-once" }));
21753
const result = await handler(makeRequest(), { sessionId: "sess-1" });
21854
expect(result).toEqual({ kind: "approve-once" });
21955
});
@@ -251,7 +87,7 @@ describe("createPermissionBridge", () => {
25187
});
25288

25389
it("handles all SDK permission kinds without throwing", async () => {
254-
const handler = createPermissionBridge(allowOncePolicy);
90+
const handler = createPermissionBridge(() => ({ kind: "approve-once" }));
25591
for (const kind of [
25692
"shell",
25793
"write",

‎extensions/copilot/src/permission-bridge.ts‎

Lines changed: 2 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,13 @@
1010
* 1. Defines a small `CopilotPermissionPolicy` contract that the
1111
* host can implement to mirror PI's policy decisions for the
1212
* copilot agent runtime.
13-
* 2. Provides built-in policies for common defaults (fail-closed,
14-
* approve-all-for-test, allow-list-by-kind).
15-
* 3. Provides a `delegatingPolicy({ onRequest })` so the core layer
16-
* can plug in a host-side callback that calls into
17-
* `runBeforeToolCallHook` / `effective-tool-policy` and returns
18-
* the SDK-shaped decision.
19-
* 4. Adapts the resulting policy into the SDK's
13+
* 2. Adapts the resulting policy into the SDK's
2014
* `PermissionHandler` shape via `createPermissionBridge(policy)`.
2115
*
2216
* Cross-package boundary note: the heavy `pi-tools.before-tool-call`
2317
* surface cannot be imported here (`tsconfig.package-boundary.base.json`).
2418
* The host bridges core PI logic into this module by injecting a
25-
* `delegatingPolicy` from the core wiring layer that constructs
19+
* `CopilotPermissionPolicy` from the core wiring layer that constructs
2620
* `AgentHarnessAttemptParams` for the copilot agent runtime.
2721
*
2822
* If PI's permission semantics change materially, the contract here
@@ -67,117 +61,6 @@ export const rejectAllPolicy: CopilotPermissionPolicy = () => ({
6761
feedback: REJECT_ALL_FEEDBACK,
6862
});
6963

70-
/**
71-
* Approve every request as "approve-once". Use only in tests / live
72-
* smoke runs where the operator has accepted the risk. This is the
73-
* SDK-bundled `approveAll` behavior re-exported as an explicit named
74-
* policy so test sites can opt in without `@github/copilot-sdk`
75-
* imports leaking into call sites.
76-
*/
77-
export const allowOncePolicy: CopilotPermissionPolicy = () => ({
78-
kind: "approve-once",
79-
});
80-
81-
export interface AllowListPolicyOptions {
82-
/** Permission kinds that should be approved once. */
83-
kinds: ReadonlyArray<SdkPermissionRequest["kind"]>;
84-
/** Optional feedback text attached to rejections. */
85-
rejectFeedback?: string;
86-
}
87-
88-
/**
89-
* Approve requests whose `kind` is in the allow-list; reject everything
90-
* else with `rejectFeedback` (defaulting to `REJECT_ALL_FEEDBACK`).
91-
*/
92-
export function allowListPolicy(options: AllowListPolicyOptions): CopilotPermissionPolicy {
93-
const allowed = new Set<SdkPermissionRequest["kind"]>(options.kinds);
94-
const feedback = options.rejectFeedback ?? REJECT_ALL_FEEDBACK;
95-
return ({ request }) => {
96-
if (allowed.has(request.kind)) {
97-
return { kind: "approve-once" };
98-
}
99-
return { kind: "reject", feedback };
100-
};
101-
}
102-
103-
export interface DelegatingPolicyOptions {
104-
/**
105-
* Host-supplied callback. Returning `undefined` falls through to the
106-
* fail-closed default. Throwing falls back to the configured
107-
* `onError` policy if provided; otherwise the throw is converted to a
108-
* reject with the error message embedded in `feedback` (so the model
109-
* sees the diagnostic instead of a generic RPC failure).
110-
*/
111-
onRequest: CopilotPermissionPolicy;
112-
/**
113-
* Optional fallback when `onRequest` throws. If omitted, throws are
114-
* reflected back as `reject` with the error message in `feedback`.
115-
* If supplied and `onError` also throws, fall through to the
116-
* error-message reject.
117-
*/
118-
onError?: CopilotPermissionPolicy;
119-
}
120-
121-
/**
122-
* Wrap a host callback into a policy, catching synchronous throws and
123-
* async rejections so the SDK never sees an exception (which would
124-
* surface as a generic RPC failure to the model).
125-
*/
126-
export function delegatingPolicy(options: DelegatingPolicyOptions): CopilotPermissionPolicy {
127-
const { onRequest, onError } = options;
128-
return async (ctx) => {
129-
try {
130-
const result = await onRequest(ctx);
131-
if (result !== undefined) {
132-
return result;
133-
}
134-
return { kind: "reject", feedback: REJECT_ALL_FEEDBACK };
135-
} catch (error) {
136-
if (onError) {
137-
try {
138-
const fallback = await onError(ctx);
139-
if (fallback !== undefined) {
140-
return fallback;
141-
}
142-
} catch {
143-
// fall through to error-message reject
144-
}
145-
}
146-
return {
147-
kind: "reject",
148-
feedback: `copilot permission policy threw: ${formatError(error)}`,
149-
};
150-
}
151-
};
152-
}
153-
154-
/**
155-
* Compose policies in order. The first policy to return a non-undefined
156-
* result wins. If all return undefined, a fail-closed `reject` is
157-
* produced. Throws inside any policy short-circuit to `reject` with the
158-
* error message; downstream policies are not consulted after a throw
159-
* (so a misbehaving host policy cannot mask itself by being followed by
160-
* an allow-policy).
161-
*/
162-
export function composePolicies(...policies: CopilotPermissionPolicy[]): CopilotPermissionPolicy {
163-
return async (ctx) => {
164-
for (const policy of policies) {
165-
try {
166-
const result = await policy(ctx);
167-
if (result !== undefined) {
168-
return result;
169-
}
170-
} catch (error) {
171-
return {
172-
kind: "reject",
173-
feedback: `copilot permission policy threw: ${formatError(error)}`,
174-
};
175-
}
176-
}
177-
return { kind: "reject", feedback: REJECT_ALL_FEEDBACK };
178-
};
179-
}
180-
18164
/**
18265
* Adapt a `CopilotPermissionPolicy` to the SDK's
18366
* `PermissionHandler` shape. The returned handler always resolves

0 commit comments

Comments
 (0)