dilim/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-08 11:02:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(copilot): drop unused permission policy helpers

Browse Source
This commit is contained in:
Vincent Koc
2026-06-19 13:43:26 +08:00
parent 6370f2023a
commit 2db37c2cd0
2 changed files with 4 additions and 285 deletions
Download Patch File Download Diff File Expand all files Collapse all files

168
extensions/copilot/src/permission-bridge.test.ts
View File

@@ -1,15 +1,10 @@
// Copilot tests cover permission bridge plugin behavior.
import type {
PermissionRequest as SdkPermissionRequest,
PermissionRequestResult as SdkPermissionRequestResult,
} from "@github/copilot-sdk";
import { describe, expect, it, vi } from "vitest";
import {
allowListPolicy,
allowOncePolicy,
composePolicies,
createPermissionBridge,
delegatingPolicy,
rejectAllPolicy,
REJECT_ALL_FEEDBACK,
type CopilotPermissionContext,
@@ -52,168 +47,9 @@ describe("rejectAllPolicy", () => {
});
});
describe("allowOncePolicy", () => {
it("returns approve-once for every request kind", async () => {
for (const kind of [
"shell",
"write",
"mcp",
"read",
"url",
"custom-tool",
"memory",
"hook",
] as const) {
const result = await allowOncePolicy(makeCtx({ request: makeRequest({ kind }) }));
expect(result).toEqual({ kind: "approve-once" });
}
});
});
describe("allowListPolicy", () => {
it("approves listed kinds and rejects others with default feedback", async () => {
const policy = allowListPolicy({ kinds: ["read"] });
const approved = await policy(makeCtx({ request: makeRequest({ kind: "read" }) }));
expect(approved).toEqual({ kind: "approve-once" });
const rejected = await policy(makeCtx({ request: makeRequest({ kind: "shell" }) }));
expect(rejected).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
});
it("uses custom rejectFeedback when provided", async () => {
const policy = allowListPolicy({
kinds: ["read"],
rejectFeedback: "only reads allowed",
});
const result = await policy(makeCtx({ request: makeRequest({ kind: "write" }) }));
expect(result).toEqual({ kind: "reject", feedback: "only reads allowed" });
});
it("supports multiple kinds in the allow-list", async () => {
const policy = allowListPolicy({ kinds: ["read", "write"] });
expect(await policy(makeCtx({ request: makeRequest({ kind: "read" }) }))).toEqual({
kind: "approve-once",
});
expect(await policy(makeCtx({ request: makeRequest({ kind: "write" }) }))).toEqual({
kind: "approve-once",
});
expect((await policy(makeCtx({ request: makeRequest({ kind: "mcp" }) })))?.kind).toBe("reject");
});
it("rejects all when given an empty allow-list", async () => {
const policy = allowListPolicy({ kinds: [] });
for (const kind of ["shell", "read", "write"] as const) {
const result = await policy(makeCtx({ request: makeRequest({ kind }) }));
expect(result?.kind).toBe("reject");
}
});
});
describe("delegatingPolicy", () => {
it("forwards the request to the host callback and returns its decision", async () => {
const onRequest = vi.fn<CopilotPermissionPolicy>().mockResolvedValue({
kind: "approve-for-session",
} satisfies SdkPermissionRequestResult);
const policy = delegatingPolicy({ onRequest });
const ctx = makeCtx({ sessionId: "sess-xyz", request: makeRequest({ kind: "write" }) });
const result = await policy(ctx);
expect(result).toEqual({ kind: "approve-for-session" });
expect(onRequest).toHaveBeenCalledTimes(1);
expect(onRequest).toHaveBeenCalledWith(ctx);
});
it("returns the rejectAll default when host callback returns undefined", async () => {
const onRequest = vi.fn<CopilotPermissionPolicy>().mockResolvedValue(undefined);
const policy = delegatingPolicy({ onRequest });
const result = await policy(makeCtx());
expect(result).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
});
it("rejects with the error message when host callback throws", async () => {
const onRequest = vi
.fn<CopilotPermissionPolicy>()
.mockRejectedValue(new Error("host policy boom"));
const policy = delegatingPolicy({ onRequest });
const result = await policy(makeCtx());
expect(result?.kind).toBe("reject");
expect((result as { feedback?: string }).feedback).toContain("host policy boom");
});
it("falls back to onError policy when host callback throws", async () => {
const onError = vi.fn<CopilotPermissionPolicy>().mockResolvedValue({ kind: "approve-once" });
const policy = delegatingPolicy({
onRequest: () => {
throw new Error("host policy boom");
},
onError,
});
const result = await policy(makeCtx());
expect(result).toEqual({ kind: "approve-once" });
expect(onError).toHaveBeenCalledTimes(1);
});
it("falls through to a hard-coded reject if onError also throws", async () => {
const policy = delegatingPolicy({
onRequest: () => {
throw new Error("host boom");
},
onError: () => {
throw new Error("fallback boom");
},
});
const result = await policy(makeCtx());
expect(result?.kind).toBe("reject");
expect((result as { feedback?: string }).feedback).toContain("host boom");
});
it("formats non-Error throws via JSON.stringify", async () => {
const policy = delegatingPolicy({
onRequest: () => {
throw { code: 42, msg: "weird" } as unknown as Error;
},
});
const result = await policy(makeCtx());
expect((result as { feedback?: string }).feedback).toContain('"code":42');
});
});
describe("composePolicies", () => {
it("returns the first non-undefined result and skips subsequent policies", async () => {
const a: CopilotPermissionPolicy = () => undefined;
const b: CopilotPermissionPolicy = () => ({ kind: "approve-once" });
const c = vi.fn<CopilotPermissionPolicy>(() => ({
kind: "reject",
feedback: "should never run",
}));
const policy = composePolicies(a, b, c);
const result = await policy(makeCtx());
expect(result).toEqual({ kind: "approve-once" });
expect(c).not.toHaveBeenCalled();
});
it("falls through to fail-closed reject when all policies return undefined", async () => {
const policy = composePolicies(
() => undefined,
() => undefined,
);
const result = await policy(makeCtx());
expect(result).toEqual({ kind: "reject", feedback: REJECT_ALL_FEEDBACK });
});
it("short-circuits to reject if any policy throws (does not consult later policies)", async () => {
const later = vi.fn<CopilotPermissionPolicy>(() => ({ kind: "approve-once" }));
const policy = composePolicies(() => {
throw new Error("nope");
}, later);
const result = await policy(makeCtx());
expect(result?.kind).toBe("reject");
expect((result as { feedback?: string }).feedback).toContain("nope");
expect(later).not.toHaveBeenCalled();
});
});
describe("createPermissionBridge", () => {
it("adapts a policy to the SDK PermissionHandler shape", async () => {
const handler = createPermissionBridge(allowOncePolicy);
const handler = createPermissionBridge(() => ({ kind: "approve-once" }));
const result = await handler(makeRequest(), { sessionId: "sess-1" });
expect(result).toEqual({ kind: "approve-once" });
});
@@ -251,7 +87,7 @@ describe("createPermissionBridge", () => {
});
it("handles all SDK permission kinds without throwing", async () => {
const handler = createPermissionBridge(allowOncePolicy);
const handler = createPermissionBridge(() => ({ kind: "approve-once" }));
for (const kind of [
"shell",
"write",

121
extensions/copilot/src/permission-bridge.ts
View File

@@ -10,19 +10,13 @@
* 1. Defines a small `CopilotPermissionPolicy` contract that the
* host can implement to mirror PI's policy decisions for the
* copilot agent runtime.
* 2. Provides built-in policies for common defaults (fail-closed,
* approve-all-for-test, allow-list-by-kind).
* 3. Provides a `delegatingPolicy({ onRequest })` so the core layer
* can plug in a host-side callback that calls into
* `runBeforeToolCallHook` / `effective-tool-policy` and returns
* the SDK-shaped decision.
* 4. Adapts the resulting policy into the SDK's
* 2. Adapts the resulting policy into the SDK's
* `PermissionHandler` shape via `createPermissionBridge(policy)`.
*
* Cross-package boundary note: the heavy `pi-tools.before-tool-call`
* surface cannot be imported here (`tsconfig.package-boundary.base.json`).
* The host bridges core PI logic into this module by injecting a
* `delegatingPolicy` from the core wiring layer that constructs
* `CopilotPermissionPolicy` from the core wiring layer that constructs
* `AgentHarnessAttemptParams` for the copilot agent runtime.
*
* If PI's permission semantics change materially, the contract here
@@ -67,117 +61,6 @@ export const rejectAllPolicy: CopilotPermissionPolicy = () => ({
feedback: REJECT_ALL_FEEDBACK,
});
/**
* Approve every request as "approve-once". Use only in tests / live
* smoke runs where the operator has accepted the risk. This is the
* SDK-bundled `approveAll` behavior re-exported as an explicit named
* policy so test sites can opt in without `@github/copilot-sdk`
* imports leaking into call sites.
*/
export const allowOncePolicy: CopilotPermissionPolicy = () => ({
kind: "approve-once",
});
export interface AllowListPolicyOptions {
/** Permission kinds that should be approved once. */
kinds: ReadonlyArray<SdkPermissionRequest["kind"]>;
/** Optional feedback text attached to rejections. */
rejectFeedback?: string;
}
/**
* Approve requests whose `kind` is in the allow-list; reject everything
* else with `rejectFeedback` (defaulting to `REJECT_ALL_FEEDBACK`).
*/
export function allowListPolicy(options: AllowListPolicyOptions): CopilotPermissionPolicy {
const allowed = new Set<SdkPermissionRequest["kind"]>(options.kinds);
const feedback = options.rejectFeedback ?? REJECT_ALL_FEEDBACK;
return ({ request }) => {
if (allowed.has(request.kind)) {
return { kind: "approve-once" };
}
return { kind: "reject", feedback };
};
}
export interface DelegatingPolicyOptions {
/**
* Host-supplied callback. Returning `undefined` falls through to the
* fail-closed default. Throwing falls back to the configured
* `onError` policy if provided; otherwise the throw is converted to a
* reject with the error message embedded in `feedback` (so the model
* sees the diagnostic instead of a generic RPC failure).
*/
onRequest: CopilotPermissionPolicy;
/**
* Optional fallback when `onRequest` throws. If omitted, throws are
* reflected back as `reject` with the error message in `feedback`.
* If supplied and `onError` also throws, fall through to the
* error-message reject.
*/
onError?: CopilotPermissionPolicy;
}
/**
* Wrap a host callback into a policy, catching synchronous throws and
* async rejections so the SDK never sees an exception (which would
* surface as a generic RPC failure to the model).
*/
export function delegatingPolicy(options: DelegatingPolicyOptions): CopilotPermissionPolicy {
const { onRequest, onError } = options;
return async (ctx) => {
try {
const result = await onRequest(ctx);
if (result !== undefined) {
return result;
}
return { kind: "reject", feedback: REJECT_ALL_FEEDBACK };
} catch (error) {
if (onError) {
try {
const fallback = await onError(ctx);
if (fallback !== undefined) {
return fallback;
}
} catch {
// fall through to error-message reject
}
}
return {
kind: "reject",
feedback: `copilot permission policy threw: ${formatError(error)}`,
};
}
};
}
/**
* Compose policies in order. The first policy to return a non-undefined
* result wins. If all return undefined, a fail-closed `reject` is
* produced. Throws inside any policy short-circuit to `reject` with the
* error message; downstream policies are not consulted after a throw
* (so a misbehaving host policy cannot mask itself by being followed by
* an allow-policy).
*/
export function composePolicies(...policies: CopilotPermissionPolicy[]): CopilotPermissionPolicy {
return async (ctx) => {
for (const policy of policies) {
try {
const result = await policy(ctx);
if (result !== undefined) {
return result;
}
} catch (error) {
return {
kind: "reject",
feedback: `copilot permission policy threw: ${formatError(error)}`,
};
}
}
return { kind: "reject", feedback: REJECT_ALL_FEEDBACK };
};
}
/**
* Adapt a `CopilotPermissionPolicy` to the SDK's
* `PermissionHandler` shape. The returned handler always resolves