SDK v1.17.12: session.permission — programmatic create/fetch, more reliable auto-accept (#1982)
* docs: add SDK v1.17.12 migration plan — phase 4 (session.permission) * feat(permissions): verify pending permission before auto-accept via SDK v1.17.12 Adds createPermission() and fetchPermission() wrappers on OpencodeService for the new v2.session.permission endpoints (OpenCode SDK 1.17.12). fetchPermission() is used by the auto-accept sweep in resyncBlockingRequestsForDirectory to verify a permission is still pending before replying. The auto-accept flow now skips permissions that are already resolved, returning a null from fetchPermission() rather than blindly calling respondToPermission on a stale entry. createPermission() is exposed for future programmatic permission creation; the V1 list/reply path used by the UI is unchanged. The plan doc at plans/opencode-v1.17.12-sdk/ was rebased onto origin/main in the prior commit to keep the PR diff focused on this change. Closes #1972 * fix(permissions): drop confirmed-resolved permissions from auto-accept resync fetchPermission() now returns a tagged FetchPermissionResult so the auto-accept loop can distinguish a server-confirmed 404 (the permission is no longer pending) from a fetch failure (network error or pre-v1.17.12 server). Previously both cases collapsed to null, so a permission the server had already answered would still appear in the resync output and trigger a spurious 'Permission needed' toast. The auto-accept loop in resyncBlockingRequestsForDirectory now tracks both accepted and resolved permissions, then drops both from the 'grouped' map before it falls through to the toast path. On a pre-v1.17.12 server (no V2 endpoint) the call still returns 'unknown' and the permission stays in the resync output so the user can answer manually — fail-closed, no false-resolved signals. Adds a focused unit test for fetchPermission (4 cases: 200 ok, 404 resolved, 500 unknown, network throw) mocking the V2 SDK client shape. --------- Co-authored-by: bashrusakh <bashrusakh@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
co-authored by
bashrusakh
parent
6d7ea82d86
commit
01a52eccab
@@ -0,0 +1,154 @@
|
||||
import { describe, expect, mock, test } from 'bun:test';
|
||||
|
||||
type PermissionV2Fixture = {
|
||||
id: string;
|
||||
sessionID: string;
|
||||
action: string;
|
||||
resources: string[];
|
||||
};
|
||||
|
||||
const permissionGetMock = mock((args: { sessionID: string; requestID: string }) => {
|
||||
return new Promise<unknown>((resolve, reject) => {
|
||||
pendingResolutions.push((r: TestResponse) => {
|
||||
if (r.kind === 'throw') {
|
||||
reject(new Error('network down'));
|
||||
} else if (r.kind === 'ok') {
|
||||
resolve(makeSuccessResult(r.permission));
|
||||
} else {
|
||||
const status = r.kind === 'not-found' ? 404 : 500;
|
||||
resolve(makeErrorResult(status));
|
||||
}
|
||||
});
|
||||
pendingArgs.push(args);
|
||||
});
|
||||
});
|
||||
|
||||
type TestResponse =
|
||||
| { kind: 'ok'; permission: PermissionV2Fixture }
|
||||
| { kind: 'not-found' }
|
||||
| { kind: 'server-error' }
|
||||
| { kind: 'throw' };
|
||||
|
||||
const pendingResolutions: Array<(r: TestResponse) => void> = [];
|
||||
const pendingArgs: Array<{ sessionID: string; requestID: string }> = [];
|
||||
|
||||
/**
|
||||
* Build a HeyApi success result that matches the wrapper's expectations:
|
||||
* - error === undefined (success branch)
|
||||
* - data.data === the permission (200 status payload)
|
||||
* - response.status === 200
|
||||
*/
|
||||
const makeSuccessResult = (permission: PermissionV2Fixture) => ({
|
||||
data: { data: permission },
|
||||
error: undefined,
|
||||
request: new Request('http://test/'),
|
||||
response: new Response(null, { status: 200 }),
|
||||
});
|
||||
|
||||
/**
|
||||
* Build a HeyApi error result with the given status code.
|
||||
*/
|
||||
const makeErrorResult = (status: number) => ({
|
||||
data: undefined,
|
||||
error: {
|
||||
name: status === 404 ? 'PermissionNotFoundError' : 'ServerError',
|
||||
data: { message: 'err' },
|
||||
},
|
||||
request: new Request('http://test/'),
|
||||
response: new Response(null, { status }),
|
||||
});
|
||||
|
||||
const createOpencodeClientMock = mock(() => ({
|
||||
v2: {
|
||||
session: {
|
||||
permission: {
|
||||
get: permissionGetMock,
|
||||
},
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
(mock as unknown as { restore?: () => void }).restore?.();
|
||||
|
||||
mock.module('@opencode-ai/sdk/v2', () => ({
|
||||
createOpencodeClient: createOpencodeClientMock,
|
||||
}));
|
||||
|
||||
mock.module('@/contexts/runtimeAPIRegistry', () => ({
|
||||
getRegisteredRuntimeAPIs: mock(() => null),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-url', () => ({
|
||||
getRuntimeUrlResolver: mock(() => ({
|
||||
api: (path: string) => path,
|
||||
})),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-switch', () => ({
|
||||
getRuntimeApiBaseUrl: mock(() => ''),
|
||||
getRuntimeKey: mock(() => 'test-runtime'),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/runtime-fetch', () => ({
|
||||
runtimeFetch: mock(async () => new Response(JSON.stringify([]), {
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})),
|
||||
}));
|
||||
|
||||
mock.module('@/lib/startupTrace', () => ({
|
||||
markStartupTrace: mock(() => undefined),
|
||||
}));
|
||||
|
||||
const { opencodeClient } = await import(`./client?cache-test-permission=${Date.now()}`);
|
||||
|
||||
/**
|
||||
* Drive the in-flight mocked `get()` call to the next resolver with the
|
||||
* given response shape. Each test owns exactly one queued call, so this
|
||||
* is unambiguous as long as tests do not overlap.
|
||||
*/
|
||||
const resolveNext = (response: TestResponse) => {
|
||||
queueMicrotask(() => {
|
||||
const resolver = pendingResolutions.shift();
|
||||
if (resolver) resolver(response);
|
||||
});
|
||||
};
|
||||
|
||||
describe('opencodeClient.fetchPermission', () => {
|
||||
test('returns state="ok" with the permission when the server returns 200', async () => {
|
||||
const permission: PermissionV2Fixture = {
|
||||
id: 'perm_1',
|
||||
sessionID: 'ses_1',
|
||||
action: 'bash',
|
||||
resources: ['*'],
|
||||
};
|
||||
const promise = opencodeClient.fetchPermission('ses_1', 'perm_1');
|
||||
resolveNext({ kind: 'ok', permission });
|
||||
const result = await promise;
|
||||
expect(result.state).toBe('ok');
|
||||
if (result.state === 'ok') {
|
||||
expect(result.permission).toEqual(permission);
|
||||
}
|
||||
expect(pendingArgs[0]).toEqual({ sessionID: 'ses_1', requestID: 'perm_1' });
|
||||
});
|
||||
|
||||
test('returns state="resolved" when the server returns 404', async () => {
|
||||
const promise = opencodeClient.fetchPermission('ses_1', 'perm_gone');
|
||||
resolveNext({ kind: 'not-found' });
|
||||
const result = await promise;
|
||||
expect(result).toEqual({ state: 'resolved' });
|
||||
});
|
||||
|
||||
test('returns state="unknown" on non-404 error responses (e.g. 500)', async () => {
|
||||
const promise = opencodeClient.fetchPermission('ses_1', 'perm_1');
|
||||
resolveNext({ kind: 'server-error' });
|
||||
const result = await promise;
|
||||
expect(result).toEqual({ state: 'unknown' });
|
||||
});
|
||||
|
||||
test('returns state="unknown" when the SDK throws (network failure)', async () => {
|
||||
const promise = opencodeClient.fetchPermission('ses_1', 'perm_1');
|
||||
resolveNext({ kind: 'throw' });
|
||||
const result = await promise;
|
||||
expect(result).toEqual({ state: 'unknown' });
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import { createOpencodeClient, OpencodeClient } from "@opencode-ai/sdk/v2";
|
||||
import type { PermissionV2Request, PermissionV2Effect, PermissionV2Source } from "@opencode-ai/sdk/v2/client";
|
||||
import type { FilesAPI } from "../api/types";
|
||||
import { getDesktopHomeDirectory } from "../desktop";
|
||||
import type {
|
||||
@@ -13,6 +14,17 @@ import type {
|
||||
} from "@opencode-ai/sdk/v2";
|
||||
import type { PermissionRequest } from "@/types/permission";
|
||||
import type { QuestionRequest } from "@/types/question";
|
||||
|
||||
/**
|
||||
* Tagged result of `OpencodeService.fetchPermission()`. The caller can
|
||||
* distinguish a server-confirmed "no longer pending" permission (HTTP
|
||||
* 404) from a fetch failure (network error, malformed response, or a
|
||||
* pre-v1.17.12 server without the V2 endpoint).
|
||||
*/
|
||||
export type FetchPermissionResult =
|
||||
| { state: "ok"; permission: PermissionV2Request }
|
||||
| { state: "resolved" }
|
||||
| { state: "unknown" };
|
||||
import { getRuntimeUrlResolver } from "@/lib/runtime-url";
|
||||
import { runtimeFetch } from "@/lib/runtime-fetch";
|
||||
import { getRuntimeKey } from "@/lib/runtime-switch";
|
||||
@@ -1108,6 +1120,106 @@ class OpencodeService {
|
||||
return unwrapSdkOptional(response, 'permission.reply') === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Programmatically evaluate and (when approval is required) create a
|
||||
* permission request for a session via the V2 endpoint introduced in
|
||||
* OpenCode SDK v1.17.12. Wraps `session.permission.create`.
|
||||
*
|
||||
* Returns `{ id, effect }` on success, or `null` on any failure
|
||||
* (network error, 4xx/5xx response, malformed payload, or pre-v1.17.12
|
||||
* server without the V2 endpoint). Callers driving authoritative state
|
||||
* must treat `null` as "unknown — do not act" rather than "permission
|
||||
* allowed."
|
||||
*
|
||||
* Thin wrapper for future programmatic permission creation. The V1
|
||||
* `permission.list` / `permission.reply` flow used by the auto-accept
|
||||
* path is unchanged.
|
||||
*/
|
||||
async createPermission(
|
||||
sessionID: string,
|
||||
action: string,
|
||||
resources: string[],
|
||||
options?: {
|
||||
id?: string;
|
||||
save?: string[];
|
||||
metadata?: Record<string, unknown>;
|
||||
source?: PermissionV2Source;
|
||||
agent?: string;
|
||||
}
|
||||
): Promise<{ id: string; effect: PermissionV2Effect } | null> {
|
||||
try {
|
||||
const response = await this.client.v2.session.permission.create({
|
||||
sessionID,
|
||||
action,
|
||||
resources,
|
||||
...(options?.id ? { id: options.id } : {}),
|
||||
...(options?.save ? { save: options.save } : {}),
|
||||
...(options?.metadata ? { metadata: options.metadata } : {}),
|
||||
...(options?.source ? { source: options.source } : {}),
|
||||
...(options?.agent ? { agent: options.agent } : {}),
|
||||
});
|
||||
// Discriminated union narrowing on `error` (see fetchPermission).
|
||||
if (response.error !== undefined) return null;
|
||||
const payload = response.data?.data;
|
||||
if (payload === undefined) return null;
|
||||
return { id: payload.id, effect: payload.effect };
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a pending permission request owned by a session via the V2
|
||||
* endpoint introduced in OpenCode SDK v1.17.12. Wraps
|
||||
* `session.permission.get`.
|
||||
*
|
||||
* Returns a tagged `FetchPermissionResult` so the caller can distinguish
|
||||
* a confirmed-resolved permission (HTTP 404) from a fetch failure
|
||||
* (network error, malformed response, or pre-v1.17.12 server without
|
||||
* the V2 endpoint). The auto-accept flow uses this distinction to drop
|
||||
* resolved permissions from the resync output, preventing stale
|
||||
* `permission.list` entries from sticking around in the UI.
|
||||
*/
|
||||
async fetchPermission(
|
||||
sessionID: string,
|
||||
requestID: string,
|
||||
): Promise<FetchPermissionResult> {
|
||||
try {
|
||||
// The V2 path is session-scoped and does not require a `directory`
|
||||
// parameter. The client-scoped directory (set via setDirectory) is
|
||||
// honored by the underlying SDK client when the call is routed.
|
||||
const response = await this.client.v2.session.permission.get({
|
||||
sessionID,
|
||||
requestID,
|
||||
});
|
||||
// The SDK returns a discriminated union on `error`/`data` (HeyApi
|
||||
// `RequestResult` with `ThrowOnError = false`). The error branch
|
||||
// collapses `data` to `undefined`; the data branch returns the
|
||||
// 200-response payload as `{ data: PermissionV2Request }`. Narrow
|
||||
// via `error` first, then unwrap the inner `data` field.
|
||||
if (response.error === undefined) {
|
||||
const payload = response.data?.data;
|
||||
if (payload !== undefined) {
|
||||
return { state: "ok", permission: payload };
|
||||
}
|
||||
}
|
||||
// On the error branch the server has answered but the request was
|
||||
// not found. V2SessionPermissionGetErrors maps 404 to
|
||||
// `PermissionNotFoundError`, so the only server-confirmed
|
||||
// "no longer pending" signal we have is HTTP 404.
|
||||
if (response.response?.status === 404) {
|
||||
return { state: "resolved" };
|
||||
}
|
||||
return { state: "unknown" };
|
||||
} catch {
|
||||
// Network failure, pre-v1.17.12 server, or runtimeFetch throwing.
|
||||
// Treat as "unknown" — caller must decide what to do (auto-accept
|
||||
// fails closed, but the permission stays in the resync output so
|
||||
// the user can still act on it).
|
||||
return { state: "unknown" };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Throws on fetch/SDK failure. Callers that drive authoritative state from
|
||||
* the result (e.g. reconnect resync) must let the throw propagate so they
|
||||
|
||||
@@ -1139,13 +1139,41 @@ export async function resyncBlockingRequestsForDirectory(
|
||||
|
||||
if (autoAcceptingSessionIds.length > 0) {
|
||||
const acceptedIdsBySession = new Map<string, Set<string>>()
|
||||
// Track server-confirmed resolved permissions separately so we can
|
||||
// remove them from `grouped` below — the V1 listPendingPermissions
|
||||
// snapshot can still contain entries the server has already answered,
|
||||
// and leaving them in place produces a spurious "Permission needed"
|
||||
// toast for a permission the user has already resolved.
|
||||
const resolvedIdsBySession = new Map<string, Set<string>>()
|
||||
await Promise.all(autoAcceptingSessionIds.flatMap((sessionId) =>
|
||||
(grouped[sessionId] ?? []).map(async (permission) => {
|
||||
try {
|
||||
await sessionActions.respondToPermission(permission.sessionID, permission.id, "once")
|
||||
const accepted = acceptedIdsBySession.get(sessionId) ?? new Set<string>()
|
||||
accepted.add(permission.id)
|
||||
acceptedIdsBySession.set(sessionId, accepted)
|
||||
// Verify the permission is still pending before auto-accepting.
|
||||
// - state: "ok" → still pending, safe to auto-accept
|
||||
// - state: "resolved" → server returned 404, drop from grouped
|
||||
// - state: "unknown" → network error / pre-1.17.12 server,
|
||||
// keep in grouped for the user to act on
|
||||
//
|
||||
// On a pre-v1.17.12 server without the V2 endpoint, every call
|
||||
// returns "unknown". This permanently disables auto-accept
|
||||
// (acknowledged scope tradeoff — project requires SDK 1.17.12)
|
||||
// but does not falsely report permissions as resolved.
|
||||
const outcome = await opencodeClient.fetchPermission(
|
||||
permission.sessionID,
|
||||
permission.id,
|
||||
)
|
||||
if (outcome.state === "ok") {
|
||||
await sessionActions.respondToPermission(permission.sessionID, permission.id, "once")
|
||||
const accepted = acceptedIdsBySession.get(sessionId) ?? new Set<string>()
|
||||
accepted.add(permission.id)
|
||||
acceptedIdsBySession.set(sessionId, accepted)
|
||||
} else if (outcome.state === "resolved") {
|
||||
const resolved = resolvedIdsBySession.get(sessionId) ?? new Set<string>()
|
||||
resolved.add(permission.id)
|
||||
resolvedIdsBySession.set(sessionId, resolved)
|
||||
}
|
||||
// state: "unknown" → keep the permission in grouped; user can
|
||||
// answer manually.
|
||||
} catch {
|
||||
// Keep failed auto-accept permissions in UI state so the user can act.
|
||||
}
|
||||
@@ -1154,8 +1182,11 @@ export async function resyncBlockingRequestsForDirectory(
|
||||
|
||||
for (const sessionId of autoAcceptingSessionIds) {
|
||||
const acceptedIds = acceptedIdsBySession.get(sessionId)
|
||||
if (!acceptedIds) continue
|
||||
const remaining = (grouped[sessionId] ?? []).filter((permission) => !acceptedIds.has(permission.id))
|
||||
const resolvedIds = resolvedIdsBySession.get(sessionId)
|
||||
if (!acceptedIds && !resolvedIds) continue
|
||||
const drop = (id: string) =>
|
||||
acceptedIds?.has(id) || resolvedIds?.has(id) || false
|
||||
const remaining = (grouped[sessionId] ?? []).filter((permission) => !drop(permission.id))
|
||||
if (remaining.length > 0) grouped[sessionId] = remaining
|
||||
else delete grouped[sessionId]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user