fix(exec): resolve no-route approvals immediately (#108935)

Fixes #104413. Supersedes the implementation direction in #104923 after preserving its contributor work and extending route detection across approval clients.

Co-authored-by: RickLin <83101411+ObliviateRickLin@users.noreply.github.com>
This commit is contained in:
Peter Steinberger
2026-07-16 04:26:38 -07:00
committed by GitHub
co-authored by RickLin
parent 037ef6695a
commit 68694599af
27 changed files with 403 additions and 44 deletions
@@ -76,7 +76,10 @@ export type GatewayClientInfo = {
/** Capability flags a client may advertise during the gateway handshake. */
export const GATEWAY_CLIENT_CAPS = {
APPROVALS: "approvals",
EXEC_APPROVALS: "exec-approvals",
INLINE_WIDGETS: "inline-widgets",
PLUGIN_APPROVALS: "plugin-approvals",
TASK_SUGGESTIONS: "task-suggestions",
TERMINAL_OFFSET_SEQ: "terminal-offset-seq",
TOOL_EVENTS: "tool-events",
+2 -2
View File
@@ -368,14 +368,14 @@ describe("serveAcpGateway startup", () => {
}
});
it("subscribes the Gateway client to run-scoped tool events", async () => {
it("advertises approval handling and subscribes to run-scoped tool events", async () => {
const { signalHandlers, onceSpy } = captureProcessSignalHandlers();
try {
const servePromise = serveAcpGateway({});
await emitHelloAndWaitForAgentSideConnection();
expect(mockState.gatewayOptions[0]?.caps).toEqual(["tool-events"]);
expect(mockState.gatewayOptions[0]?.caps).toEqual(["exec-approvals", "tool-events"]);
await stopServeWithSigint(signalHandlers, servePromise);
} finally {
+1 -1
View File
@@ -93,7 +93,7 @@ export async function serveAcpGateway(opts: AcpServerOptions = {}): Promise<void
clientDisplayName: "ACP",
clientVersion: "acp",
mode: GATEWAY_CLIENT_MODES.CLI,
caps: [GATEWAY_CLIENT_CAPS.TOOL_EVENTS],
caps: [GATEWAY_CLIENT_CAPS.EXEC_APPROVALS, GATEWAY_CLIENT_CAPS.TOOL_EVENTS],
onEvent: (evt) => {
if (stopped) {
return;
@@ -48,6 +48,8 @@ type SendExecApprovalFollowupResult =
type BuildExecApprovalFollowupTarget =
typeof import("./bash-tools.exec-host-shared.js").buildExecApprovalFollowupTarget;
type ExecApprovalFollowupTarget = Parameters<BuildExecApprovalFollowupTarget>[0];
type ShouldResolveExecApprovalUnavailableInline =
typeof import("./bash-tools.exec-host-shared.js").shouldResolveExecApprovalUnavailableInline;
type ExecAutoReviewer = typeof import("../infra/exec-auto-review.js").defaultExecAutoReviewer;
type BuildExecApprovalFollowupTargetMock = (
value: ExecApprovalFollowupTarget,
@@ -171,7 +173,9 @@ const markBackgroundedMock = vi.hoisted(() => vi.fn());
const sendExecApprovalFollowupResultMock = vi.hoisted(() =>
vi.fn<SendExecApprovalFollowupResult>(async () => undefined),
);
const shouldResolveExecApprovalUnavailableInlineMock = vi.hoisted(() => vi.fn(() => false));
const shouldResolveExecApprovalUnavailableInlineMock = vi.hoisted(() =>
vi.fn<ShouldResolveExecApprovalUnavailableInline>(() => false),
);
const enforceStrictInlineEvalApprovalBoundaryMock = vi.hoisted(() =>
vi.fn<StrictInlineEvalBoundary>((value) => ({
approvedByAsk: value.approvedByAsk,
@@ -409,6 +413,15 @@ describe("processGatewayAllowlist", () => {
});
}
async function useRealUnavailableApprovalGate() {
const actualShared = await vi.importActual<typeof import("./bash-tools.exec-host-shared.js")>(
"./bash-tools.exec-host-shared.js",
);
shouldResolveExecApprovalUnavailableInlineMock.mockImplementation(
actualShared.shouldResolveExecApprovalUnavailableInline,
);
}
async function planAllowlistedNodeVersion() {
const command = "node --version";
const authorizationPlan = await planShellAuthorization({ command, env: process.env });
@@ -614,6 +627,84 @@ describe("processGatewayAllowlist", () => {
expect(serialized).not.toContain("agent-1");
});
it("resolves a triggerless CLI no-route approval through the real gate", async () => {
await useRealUnavailableApprovalGate();
createAndRegisterDefaultExecApprovalRequestMock.mockResolvedValue({
approvalId: "approval-cli-no-route",
approvalSlug: "slug",
warningText: "",
expiresAtMs: 0,
preResolvedDecision: null,
initiatingSurface: { kind: "unsupported" },
sentApproverDms: false,
unavailableReason: "no-approval-route",
});
createExecApprovalDecisionStateMock.mockReturnValue({
baseDecision: { timedOut: true },
approvedByAsk: false,
deniedReason: "approval-timeout",
});
enforceStrictInlineEvalApprovalBoundaryMock.mockReturnValue({
approvedByAsk: false,
deniedReason: "approval-timeout",
});
const captured = captureSecurityEvents();
try {
await expect(
runGatewayAllowlist({
command: "echo askfallback-proof",
agentId: "agent-1",
ask: "on-miss",
}),
).rejects.toThrow("denied");
} finally {
captured.stop();
}
expect(shouldResolveExecApprovalUnavailableInlineMock).toHaveBeenCalledWith({
unavailableReason: "no-approval-route",
preResolvedDecision: null,
});
expect(shouldResolveExecApprovalUnavailableInlineMock).toHaveReturnedWith(true);
expect(resolveApprovalDecisionOrUndefinedMock).not.toHaveBeenCalled();
expect(captured.events.at(-1)).toMatchObject({
action: "exec.approval.denied",
outcome: "denied",
});
});
it("preserves a routed approval through the real gate", async () => {
await useRealUnavailableApprovalGate();
createAndRegisterDefaultExecApprovalRequestMock.mockResolvedValue({
approvalId: "approval-routed",
approvalSlug: "slug",
warningText: "",
expiresAtMs: Date.now() + 60_000,
preResolvedDecision: undefined,
initiatingSurface: { kind: "channel" },
sentApproverDms: true,
unavailableReason: null,
});
resolveApprovalDecisionOrUndefinedMock.mockImplementation(() => new Promise(() => {}));
const result = await runGatewayAllowlist({
command: "echo routed-approval-proof",
agentId: "agent-1",
ask: "on-miss",
});
expect(result.pendingResult?.details.status).toBe("approval-pending");
expect(shouldResolveExecApprovalUnavailableInlineMock).toHaveBeenCalledWith({
unavailableReason: null,
preResolvedDecision: undefined,
});
expect(shouldResolveExecApprovalUnavailableInlineMock).toHaveReturnedWith(false);
expect(resolveApprovalDecisionOrUndefinedMock).toHaveBeenCalledWith(
expect.objectContaining({ approvalId: "approval-routed" }),
);
});
it("emits an approved security event for inline unavailable approval approvals", async () => {
shouldResolveExecApprovalUnavailableInlineMock.mockReturnValue(true);
createExecApprovalDecisionStateMock.mockReturnValue({
@@ -859,7 +859,6 @@ export async function processGatewayAllowlist(
});
if (
shouldResolveExecApprovalUnavailableInline({
trigger: params.trigger,
unavailableReason,
preResolvedDecision,
})
-1
View File
@@ -435,7 +435,6 @@ export async function executeNodeHostCommand(
});
if (
execHostShared.shouldResolveExecApprovalUnavailableInline({
trigger: params.trigger,
unavailableReason,
preResolvedDecision,
})
@@ -16,6 +16,7 @@ import {
enforceStrictInlineEvalApprovalBoundary,
resolveExecHostApprovalContext,
sendExecApprovalFollowupResult,
shouldResolveExecApprovalUnavailableInline,
} from "./bash-tools.exec-host-shared.js";
const mocks = vi.hoisted(() => ({
@@ -531,6 +532,42 @@ describe("buildExecApprovalPendingToolResult", () => {
expect(state.unavailableReason).toBe("no-approval-route");
});
it("resolves terminal no-route approvals inline", () => {
expect(
shouldResolveExecApprovalUnavailableInline({
unavailableReason: "no-approval-route",
preResolvedDecision: null,
}),
).toBe(true);
});
it("keeps waiting when a route exists or a decision arrived", () => {
expect(
shouldResolveExecApprovalUnavailableInline({
unavailableReason: null,
preResolvedDecision: null,
}),
).toBe(false);
expect(
shouldResolveExecApprovalUnavailableInline({
unavailableReason: "no-approval-route",
preResolvedDecision: "allow-once",
}),
).toBe(false);
expect(
shouldResolveExecApprovalUnavailableInline({
unavailableReason: "no-approval-route",
preResolvedDecision: undefined,
}),
).toBe(false);
expect(
shouldResolveExecApprovalUnavailableInline({
unavailableReason: "initiating-platform-disabled",
preResolvedDecision: null,
}),
).toBe(false);
});
it("keeps a local /approve prompt when the initiating Discord surface is disabled", () => {
const result = buildDisabledSurfaceApprovalResult({
channel: "discord",
+4 -11
View File
@@ -83,10 +83,6 @@ type ExecApprovalUnavailableReason =
| "initiating-platform-disabled"
| "initiating-platform-unsupported";
function isHeadlessExecTrigger(trigger?: string): boolean {
return trigger === "cron";
}
/** Context returned after a default approval request is registered. */
type RegisteredExecApprovalRequestContext = {
approvalId: string;
@@ -414,17 +410,14 @@ export function enforceStrictInlineEvalApprovalBoundary(params: {
};
}
/** Returns true when a headless run should resolve an unavailable approval inline. */
/** Returns true when registration proved no approval decision can arrive later. */
export function shouldResolveExecApprovalUnavailableInline(params: {
trigger?: string;
unavailableReason: ExecApprovalUnavailableReason | null;
preResolvedDecision: string | null | undefined;
}): boolean {
return (
isHeadlessExecTrigger(params.trigger) &&
params.unavailableReason === "no-approval-route" &&
params.preResolvedDecision === null
);
// finalDecision:null is emitted only after the gateway expires a no-route record.
// Resolve fallback inline; an async wait can never observe a later decision.
return params.unavailableReason === "no-approval-route" && params.preResolvedDecision === null;
}
/** Builds the denial copy for headless runs that cannot wait for approval. */
@@ -113,6 +113,7 @@ describe("withOperatorApprovalsGatewayClient", () => {
});
expect(clientState.options?.scopes).toEqual(["operator.approvals"]);
expect(clientState.options?.caps).toEqual(["approvals"]);
expect(typeof clientState.options?.approvalRuntimeToken).toBe("string");
expect(clientState.options?.deviceIdentity).toBeNull();
expect(clientState.requestSpy).toHaveBeenCalledWith("exec.approval.resolve", {
+2
View File
@@ -1,6 +1,7 @@
// Gateway operator-approvals client helper.
// Connects a backend Gateway client scoped to operator approval events.
import {
GATEWAY_CLIENT_CAPS,
GATEWAY_CLIENT_MODES,
GATEWAY_CLIENT_NAMES,
} from "../../packages/gateway-protocol/src/client-info.js";
@@ -56,6 +57,7 @@ export async function createOperatorApprovalsGatewayClient(
clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
clientDisplayName: params.clientDisplayName,
mode: GATEWAY_CLIENT_MODES.BACKEND,
caps: [GATEWAY_CLIENT_CAPS.APPROVALS],
scopes: ["operator.approvals"],
deviceIdentity: shouldOmitApprovalRuntimeDeviceIdentity({
sendsApprovalRuntimeToken,
@@ -37,12 +37,14 @@ export type PluginApprovalIosPushDelivery = {
};
function broadcastResolvedEvent(params: {
approvalKind: "exec" | "plugin" | "system-agent";
context: GatewayRequestContext;
eventName: "exec.approval.resolved" | "plugin.approval.resolved" | "openclaw.approval.resolved";
event: ExecApprovalResolved | PluginApprovalResolved | SystemAgentApprovalResolved;
liveRecord: ExecApprovalRecord<ApprovalRequest>;
}): void {
const recipientConnIds = resolveApprovalRequestRecipientConnIds({
approvalKind: params.approvalKind,
context: params.context,
record: {
id: params.liveRecord.id,
@@ -124,6 +126,7 @@ export async function publishAppliedApprovalResolution(params: {
effect: "broadcast",
run: () =>
broadcastResolvedEvent({
approvalKind: params.record.kind,
context: params.context,
eventName,
event,
@@ -434,6 +434,7 @@ describe("handlePendingApprovalRequest", () => {
);
const decisionPromise = manager.register(record, 60_000);
const respond = vi.fn();
const getApprovalClientConnIds = vi.fn(() => new Set<string>());
const requestPromise = handlePendingApprovalRequest({
manager,
record,
@@ -441,6 +442,8 @@ describe("handlePendingApprovalRequest", () => {
respond,
context: {
broadcast: vi.fn(),
broadcastToConnIds: vi.fn(),
getApprovalClientConnIds,
hasExecApprovalClients: () => false,
} as unknown as GatewayRequestContext,
requestEventName: "plugin.approval.requested",
@@ -456,6 +459,9 @@ describe("handlePendingApprovalRequest", () => {
});
await Promise.resolve();
expect(getApprovalClientConnIds).toHaveBeenCalledWith(
expect.objectContaining({ approvalKind: "plugin" }),
);
expect(hasApprovalTurnSourceRouteMock).toHaveBeenCalledWith({
turnSourceChannel: "whatsapp",
turnSourceAccountId: "default",
@@ -1145,6 +1151,7 @@ describe("handlePendingApprovalRequest", () => {
const respond = vi.fn();
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1270,6 +1277,7 @@ describe("handlePendingApprovalRequest", () => {
const broadcastToConnIds = vi.fn();
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1328,6 +1336,7 @@ describe("handlePendingApprovalRequest", () => {
const broadcastToConnIds = vi.fn();
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1387,6 +1396,7 @@ describe("handlePendingApprovalRequest", () => {
const broadcastToConnIds = vi.fn();
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1448,6 +1458,7 @@ describe("handlePendingApprovalRequest", () => {
const broadcastToConnIds = vi.fn();
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1501,6 +1512,7 @@ describe("handlePendingApprovalRequest", () => {
const visibleConnIds = new Set(["conn-owner-approval"]);
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "allow-once",
@@ -1659,6 +1671,7 @@ describe("handlePendingApprovalRequest", () => {
try {
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId: record.id,
decision: "deny",
@@ -293,12 +293,14 @@ export function resolveApprovalDecisionParams<TParams extends ApprovalResolvePar
/** Resolves the approval clients that should receive request or resolution events. */
export function resolveApprovalRequestRecipientConnIds<TPayload>(params: {
approvalKind: "exec" | "plugin" | "system-agent";
context: GatewayRequestContext;
record: ExecApprovalRecord<TPayload>;
excludeConnId?: string;
}): ReadonlySet<string> | null {
return (
params.context.getApprovalClientConnIds?.({
approvalKind: params.approvalKind,
excludeConnId: params.excludeConnId,
record: params.record,
filter: (client) =>
@@ -456,6 +458,7 @@ export async function handlePendingApprovalRequest<
const approvalClientConnIds = suppressDelivery
? null
: resolveApprovalRequestRecipientConnIds({
approvalKind: params.approvalKind ?? "exec",
context: params.context,
record: params.record,
excludeConnId: params.clientConnId,
@@ -588,6 +591,7 @@ export async function handlePendingApprovalRequest<
/** Resolves a pending approval and broadcasts the final decision exactly once. */
export async function handleApprovalResolve<TPayload, TResolvedEvent extends object>(params: {
approvalKind: "exec" | "plugin";
manager: ExecApprovalManager<TPayload>;
inputId: string;
decision: ExecApprovalDecision;
@@ -609,7 +613,6 @@ export async function handleApprovalResolve<TPayload, TResolvedEvent extends obj
snapshot: ExecApprovalRecord<TPayload>;
}) => boolean;
resolvedEventName: string;
approvalKind?: "exec" | "plugin";
buildResolvedEvent: (params: {
approvalId: string;
decision: ExecApprovalDecision;
@@ -729,6 +732,7 @@ export async function handleApprovalResolve<TPayload, TResolvedEvent extends obj
nowMs: Date.now(),
});
const resolvedEventConnIds = resolveApprovalRequestRecipientConnIds({
approvalKind: params.approvalKind,
context: params.context,
record: resolved.snapshot,
});
@@ -282,12 +282,14 @@ describe("unified approval handlers", () => {
systemAgentApprovalManager: managers.systemAgent,
databaseOptions,
});
const context = createContext();
const response = await invoke({
handlers,
method: "approval.resolve",
body: { id: pending.record.id, kind: "system-agent", decision: "allow-once" },
client: createClient({ deviceId: "reviewer" }),
context,
});
expect(response.result).toMatchObject({
@@ -303,6 +305,9 @@ describe("unified approval handlers", () => {
},
});
await expect(pending.decision).resolves.toBe("allow-once");
expect(context.getApprovalClientConnIds).toHaveBeenCalledWith(
expect.objectContaining({ approvalKind: "system-agent" }),
);
});
it("returns an exact-id, deep-linkable exec projection without execution bindings", async () => {
@@ -900,6 +905,7 @@ describe("unified approval handlers", () => {
const recipientLookup = context.getApprovalClientConnIds as ReturnType<typeof vi.fn>;
const recipientOptions = recipientLookup.mock.calls[0]?.[0] as
| {
approvalKind?: string;
filter?: (
client: GatewayRequestHandlerOptions["client"],
record?: { id: string },
@@ -907,6 +913,7 @@ describe("unified approval handlers", () => {
record?: { id: string };
}
| undefined;
expect(recipientOptions?.approvalKind).toBe("plugin");
expect(
recipientOptions?.filter?.(createClient({ deviceId: "unrelated" }), recipientOptions.record),
).toBe(false);
@@ -1121,6 +1128,7 @@ describe("unified approval handlers", () => {
const recipientLookup = context.getApprovalClientConnIds as ReturnType<typeof vi.fn>;
const recipientOptions = recipientLookup.mock.calls[0]?.[0] as
| {
approvalKind?: string;
filter?: (
client: GatewayRequestHandlerOptions["client"],
record?: { requestedByConnId?: string | null },
@@ -1128,6 +1136,7 @@ describe("unified approval handlers", () => {
record?: { requestedByConnId?: string | null };
}
| undefined;
expect(recipientOptions?.approvalKind).toBe("exec");
expect(recipientOptions?.record?.requestedByConnId).toBe("requester-connection");
expect(
recipientOptions?.filter?.(
+1 -1
View File
@@ -449,6 +449,7 @@ export function createExecApprovalHandlers(
const { inputId, decision } = resolveParams;
let autoReviewResolution = false;
await handleApprovalResolve({
approvalKind: "exec",
manager,
inputId,
decision,
@@ -490,7 +491,6 @@ export function createExecApprovalHandlers(
? manager.resolveAutoReview(approvalId, resolvedBy)
: manager.resolve(approvalId, decisionLocal, resolvedBy),
resolvedEventName: "exec.approval.resolved",
approvalKind: "exec",
buildResolvedEvent: ({
approvalId,
decision: decisionLocal,
@@ -1,17 +1,17 @@
// Gateway e2e proof: turn-source routing fields change gateway response behavior.
// Gateway e2e proof: real delivery routes change approval response behavior.
//
// Without turn-source fields: plugin.approval.request expires immediately with
// {decision: null} because there is no approval client and no turn-source route.
//
// With turn-source fields for a routable channel ("tui" is always routable):
// the approval stays alive and returns {status: "accepted"} because
// hasApprovalTurnSourceRoute returns true.
// With a connected approval-capable client: the approval stays alive and
// returns {status: "accepted"} because it has a real delivery route.
//
// This test runs against a real gateway server with no Telegram required.
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { GATEWAY_CLIENT_CAPS } from "../../../packages/gateway-protocol/src/client-info.js";
import { clearConfigCache, clearRuntimeConfigSnapshot } from "../../config/config.js";
import { clearSessionStoreCacheForTest } from "../../config/sessions/store.js";
import { captureEnv, deleteTestEnvValue, setTestEnvValue } from "../../test-utils/env.js";
@@ -33,11 +33,13 @@ const TEST_ENV_KEYS = [
"OPENCLAW_GATEWAY_PORT",
];
describe("plugin.approval.request turn-source routing (real gateway)", () => {
describe("plugin.approval.request delivery routing (real gateway)", () => {
let envSnapshot: ReturnType<typeof captureEnv>;
let tempHome: string;
let server: Awaited<ReturnType<typeof startGatewayServer>>;
let requester: Awaited<ReturnType<typeof connectGatewayClient>>;
let approvalClient: Awaited<ReturnType<typeof connectGatewayClient>> | undefined;
let connectApprovalClient: () => Promise<Awaited<ReturnType<typeof connectGatewayClient>>>;
beforeAll(async () => {
envSnapshot = captureEnv(TEST_ENV_KEYS);
@@ -73,11 +75,24 @@ describe("plugin.approval.request turn-source routing (real gateway)", () => {
token,
clientDisplayName: "plugin-approval requester",
scopes: [APPROVALS_SCOPE],
requestTimeoutMs: 5_000,
timeoutMs: 60_000,
});
connectApprovalClient = () =>
connectGatewayClient({
url,
token,
clientDisplayName: "plugin approval client",
scopes: [APPROVALS_SCOPE],
caps: [GATEWAY_CLIENT_CAPS.APPROVALS],
timeoutMs: 60_000,
});
});
afterAll(async () => {
if (approvalClient) {
await disconnectGatewayClient(approvalClient).catch(() => undefined);
}
await disconnectGatewayClient(requester).catch(() => undefined);
await server?.close();
await fs.rm(tempHome, { recursive: true, force: true, maxRetries: 5 }).catch(() => undefined);
@@ -103,18 +118,15 @@ describe("plugin.approval.request turn-source routing (real gateway)", () => {
expect((result as { id?: string }).id).toMatch(/^plugin:/);
});
it("returns accepted when turn-source route is present (tui channel is always routable)", async () => {
// With the fix: turn-source fields forwarded from HookContext to the gateway
// call. hasApprovalTurnSourceRoute("tui") returns true, so the record stays alive.
it("returns accepted when a real approval client is connected", async () => {
approvalClient = await connectApprovalClient();
const result = await requester.request("plugin.approval.request", {
pluginId: "test-plugin",
title: "Confirm action",
description: "Plugin wants to perform an action",
twoPhase: true,
timeoutMs: 10_000,
turnSourceChannel: "tui",
turnSourceTo: "main",
turnSourceAccountId: "local",
});
expect(result).toMatchObject({ status: "accepted" });
@@ -224,6 +224,7 @@ export function createPluginApprovalHandlers(
}
const { inputId, decision } = resolveParams;
await handleApprovalResolve({
approvalKind: "plugin",
manager,
inputId,
decision,
@@ -243,7 +244,6 @@ export function createPluginApprovalHandlers(
},
},
resolvedEventName: "plugin.approval.resolved",
approvalKind: "plugin",
buildResolvedEvent: ({
approvalId,
decision: decisionLocal,
@@ -164,6 +164,7 @@ export type GatewayRequestContext = {
approvalEvents?: GatewayApprovalEventPublisher;
recoveryRuntime?: GatewayRecoveryRuntime;
getApprovalClientConnIds?: <TPayload>(params?: {
approvalKind?: "exec" | "plugin" | "system-agent";
excludeConnId?: string;
filter?: (client: GatewayClient, record?: ExecApprovalRecord<TPayload>) => boolean;
record?: ExecApprovalRecord<TPayload>;
+131
View File
@@ -2,6 +2,11 @@
* Gateway request context construction tests.
*/
import { describe, expect, it, vi } from "vitest";
import {
GATEWAY_CLIENT_CAPS,
GATEWAY_CLIENT_IDS,
GATEWAY_CLIENT_MODES,
} from "../../packages/gateway-protocol/src/client-info.js";
import type { GatewayServerLiveState } from "./server-live-state.js";
import { createGatewayRequestContext } from "./server-request-context.js";
@@ -87,6 +92,35 @@ function makeContextParams(
};
}
function makeGatewayClient(params: {
connId: string;
clientId: (typeof GATEWAY_CLIENT_IDS)[keyof typeof GATEWAY_CLIENT_IDS];
mode?: (typeof GATEWAY_CLIENT_MODES)[keyof typeof GATEWAY_CLIENT_MODES];
scopes?: string[];
caps?: string[];
approvalRuntime?: boolean;
invalidated?: boolean;
}) {
return {
connId: params.connId,
connect: {
minProtocol: 1,
maxProtocol: 1,
client: {
id: params.clientId,
version: "test",
platform: "test",
mode: params.mode ?? GATEWAY_CLIENT_MODES.CLI,
},
scopes: params.scopes ?? [],
caps: params.caps ?? [],
},
socket: { close: vi.fn() },
...(params.approvalRuntime ? { internal: { approvalRuntime: true } } : {}),
...(params.invalidated ? { invalidated: true } : {}),
};
}
describe("createGatewayRequestContext", () => {
it("reads cron state live from runtime state", () => {
const cronA = { start: vi.fn(), stop: vi.fn() } as never;
@@ -142,6 +176,103 @@ describe("createGatewayRequestContext", () => {
expect(context.getConfigReloaderHotReloadStatus?.()).toBe("disabled");
});
it("does not treat scoped CLI or backend callers as approval delivery routes", () => {
const clients = new Set([
makeGatewayClient({
connId: "cli",
clientId: GATEWAY_CLIENT_IDS.CLI,
scopes: ["operator.admin"],
}),
makeGatewayClient({
connId: "backend",
clientId: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT,
mode: GATEWAY_CLIENT_MODES.BACKEND,
scopes: ["operator.approvals"],
}),
]) as never;
const context = createGatewayRequestContext(makeContextParams({ clients }));
expect(context.hasExecApprovalClients?.()).toBe(false);
expect(context.getApprovalClientConnIds?.()).toEqual(new Set());
expect(context.getApprovalClientConnIds?.({ approvalKind: "plugin" })).toEqual(new Set());
});
it("preserves only clients that handle each approval kind", () => {
const clients = new Set([
makeGatewayClient({
connId: "control-ui",
clientId: GATEWAY_CLIENT_IDS.CONTROL_UI,
mode: GATEWAY_CLIENT_MODES.WEBCHAT,
scopes: ["operator.approvals"],
}),
makeGatewayClient({
connId: "ios",
clientId: GATEWAY_CLIENT_IDS.IOS_APP,
mode: GATEWAY_CLIENT_MODES.UI,
scopes: ["operator.admin"],
}),
makeGatewayClient({
connId: "bridge",
clientId: GATEWAY_CLIENT_IDS.CLI,
scopes: ["operator.approvals"],
caps: [GATEWAY_CLIENT_CAPS.APPROVALS],
}),
makeGatewayClient({
connId: "acp",
clientId: GATEWAY_CLIENT_IDS.CLI,
scopes: ["operator.approvals"],
caps: [GATEWAY_CLIENT_CAPS.EXEC_APPROVALS],
}),
makeGatewayClient({
connId: "tui",
clientId: GATEWAY_CLIENT_IDS.TUI,
scopes: ["operator.approvals"],
}),
makeGatewayClient({
connId: "plugin-bridge",
clientId: GATEWAY_CLIENT_IDS.CLI,
scopes: ["operator.approvals"],
caps: [GATEWAY_CLIENT_CAPS.PLUGIN_APPROVALS],
}),
makeGatewayClient({
connId: "runtime",
clientId: GATEWAY_CLIENT_IDS.GATEWAY_CLIENT,
mode: GATEWAY_CLIENT_MODES.BACKEND,
scopes: ["operator.approvals"],
approvalRuntime: true,
}),
makeGatewayClient({
connId: "invalidated-ui",
clientId: GATEWAY_CLIENT_IDS.CONTROL_UI,
scopes: ["operator.approvals"],
invalidated: true,
}),
makeGatewayClient({
connId: "unscoped-ui",
clientId: GATEWAY_CLIENT_IDS.CONTROL_UI,
}),
]) as never;
const context = createGatewayRequestContext(makeContextParams({ clients }));
expect(context.hasExecApprovalClients?.()).toBe(true);
expect(context.getApprovalClientConnIds?.()).toEqual(
new Set(["control-ui", "ios", "bridge", "acp", "runtime"]),
);
expect(context.getApprovalClientConnIds?.({ approvalKind: "plugin" })).toEqual(
new Set(["control-ui", "bridge", "tui", "plugin-bridge", "runtime"]),
);
expect(context.getApprovalClientConnIds?.({ approvalKind: "system-agent" })).toEqual(
new Set(["control-ui", "bridge", "runtime"]),
);
expect(context.hasExecApprovalClients?.("control-ui")).toBe(true);
expect(
context.getApprovalClientConnIds?.({
excludeConnId: "control-ui",
filter: (client) => client.connect.client.id === GATEWAY_CLIENT_IDS.IOS_APP,
}),
).toEqual(new Set(["ios"]));
});
it("invalidateClientsForDevice sets the flag on matching clients without closing the socket", () => {
const target = {
connId: "conn-target",
+49 -8
View File
@@ -1,8 +1,14 @@
// Gateway request context factory.
// Wires live runtime state into method handlers and client management helpers.
import {
GATEWAY_CLIENT_CAPS,
GATEWAY_CLIENT_IDS,
hasGatewayClientCap,
type GatewayClientId,
} from "../../packages/gateway-protocol/src/client-info.js";
import type { OpenClawConfig } from "../config/types.openclaw.js";
import type { GatewayServerLiveState } from "./server-live-state.js";
import type { GatewayRequestContext, GatewayClient } from "./server-methods/types.js";
import type { GatewayClient, GatewayRequestContext } from "./server-methods/types.js";
import { disconnectAllSharedGatewayAuthClients } from "./server-shared-auth-generation.js";
type GatewayRequestContextClient = GatewayClient & {
@@ -90,14 +96,49 @@ type GatewayRequestContextParams = {
unavailableGatewayMethods: ReadonlySet<string>;
};
const ALL_APPROVAL_CLIENT_IDS: ReadonlySet<GatewayClientId> = new Set([
GATEWAY_CLIENT_IDS.CONTROL_UI,
]);
const EXEC_APPROVAL_CLIENT_IDS: ReadonlySet<GatewayClientId> = new Set([
GATEWAY_CLIENT_IDS.MACOS_APP,
GATEWAY_CLIENT_IDS.IOS_APP,
GATEWAY_CLIENT_IDS.ANDROID_APP,
]);
const PLUGIN_APPROVAL_CLIENT_IDS: ReadonlySet<GatewayClientId> = new Set([GATEWAY_CLIENT_IDS.TUI]);
function canDeliverApprovals(
gatewayClient: GatewayRequestContextClient,
approvalKind: "exec" | "plugin" | "system-agent",
): boolean {
if (gatewayClient.invalidated) {
return false;
}
const scopes = Array.isArray(gatewayClient.connect.scopes) ? gatewayClient.connect.scopes : [];
const hasApprovalScope =
scopes.includes("operator.admin") || scopes.includes("operator.approvals");
if (!hasApprovalScope) {
return false;
}
// Scope grants approval access; it does not prove the client renders this approval kind.
// Stable ids preserve shipped clients while explicit caps describe newer non-UI bridges.
return (
gatewayClient.internal?.approvalRuntime === true ||
ALL_APPROVAL_CLIENT_IDS.has(gatewayClient.connect.client.id) ||
hasGatewayClientCap(gatewayClient.connect.caps, GATEWAY_CLIENT_CAPS.APPROVALS) ||
(approvalKind === "exec" &&
(EXEC_APPROVAL_CLIENT_IDS.has(gatewayClient.connect.client.id) ||
hasGatewayClientCap(gatewayClient.connect.caps, GATEWAY_CLIENT_CAPS.EXEC_APPROVALS))) ||
(approvalKind === "plugin" &&
(PLUGIN_APPROVAL_CLIENT_IDS.has(gatewayClient.connect.client.id) ||
hasGatewayClientCap(gatewayClient.connect.caps, GATEWAY_CLIENT_CAPS.PLUGIN_APPROVALS)))
);
}
export function createGatewayRequestContext(
params: GatewayRequestContextParams,
): GatewayRequestContext {
const hasApprovalScope = (gatewayClient: GatewayClient): boolean => {
const scopes = Array.isArray(gatewayClient.connect.scopes) ? gatewayClient.connect.scopes : [];
return scopes.includes("operator.admin") || scopes.includes("operator.approvals");
};
return {
deps: params.deps,
// Keep cron reads live so config hot reload can swap cron/store state without rebuilding
@@ -141,7 +182,7 @@ export function createGatewayRequestContext(
if (excludeConnId && gatewayClient.connId === excludeConnId) {
continue;
}
if (hasApprovalScope(gatewayClient)) {
if (canDeliverApprovals(gatewayClient, "exec")) {
return true;
}
}
@@ -156,7 +197,7 @@ export function createGatewayRequestContext(
if (opts.excludeConnId && gatewayClient.connId === opts.excludeConnId) {
continue;
}
if (!hasApprovalScope(gatewayClient)) {
if (!canDeliverApprovals(gatewayClient, opts.approvalKind ?? "exec")) {
continue;
}
if (opts.filter && !opts.filter(gatewayClient, opts.record)) {
+9
View File
@@ -69,4 +69,13 @@ describe("hasApprovalTurnSourceRoute", () => {
expect(hasApprovalTurnSourceRoute({ turnSourceChannel: undefined })).toBe(false);
expect(resolveApprovalInitiatingSurfaceStateMock).not.toHaveBeenCalled();
});
it.each(["webchat", "tui"])(
"requires a live approval client for the %s turn source",
(turnSourceChannel) => {
expect(hasApprovalTurnSourceRoute({ turnSourceChannel })).toBe(false);
expect(resolveApprovalInitiatingSurfaceStateMock).not.toHaveBeenCalled();
expect(loadConfigMock).not.toHaveBeenCalled();
},
);
});
+6 -2
View File
@@ -1,5 +1,6 @@
// Checks whether an approval reply can route to the initiating turn source.
import { getRuntimeConfig } from "../config/config.js";
import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../utils/message-channel.js";
import { resolveApprovalInitiatingSurfaceState } from "./exec-approval-surface.js";
/** Returns whether approval replies can route back to the turn's initiating surface. */
@@ -8,12 +9,15 @@ export function hasApprovalTurnSourceRoute(params: {
turnSourceAccountId?: string | null;
approvalKind?: "exec" | "plugin";
}): boolean {
if (!params.turnSourceChannel?.trim()) {
const channel = normalizeMessageChannel(params.turnSourceChannel);
// INTERNAL_MESSAGE_CHANNEL is webchat; web and TUI routes exist only while
// their approval-capable Gateway clients are connected and counted separately.
if (!channel || channel === INTERNAL_MESSAGE_CHANNEL || channel === "tui") {
return false;
}
return (
resolveApprovalInitiatingSurfaceState({
channel: params.turnSourceChannel,
channel,
accountId: params.turnSourceAccountId,
cfg: getRuntimeConfig(),
approvalKind: params.approvalKind ?? "exec",
+2 -1
View File
@@ -114,7 +114,7 @@ export class OpenClawChannelBridge {
{ GatewayClient: GatewayClientCtor },
{ startGatewayClientWhenEventLoopReady },
{ APPROVALS_SCOPE, READ_SCOPE, WRITE_SCOPE },
{ GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES },
{ GATEWAY_CLIENT_CAPS, GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES },
] = await Promise.all([
import("../gateway/client-bootstrap.js"),
import("../gateway/client.js"),
@@ -145,6 +145,7 @@ export class OpenClawChannelBridge {
clientDisplayName: "OpenClaw MCP",
clientVersion: VERSION,
mode: GATEWAY_CLIENT_MODES.CLI,
caps: [GATEWAY_CLIENT_CAPS.APPROVALS],
scopes: [READ_SCOPE, WRITE_SCOPE, APPROVALS_SCOPE],
requestTimeoutMs: 180_000,
onEvent: (event) => {
+1 -1
View File
@@ -686,7 +686,7 @@ describe("GatewayChatClient", () => {
expect(constructedOptions).toHaveLength(1);
expect(constructedOptions[0]).toMatchObject({
clientName: "openclaw-tui",
caps: ["task-suggestions", "tool-events"],
caps: ["plugin-approvals", "task-suggestions", "tool-events"],
mode: "ui",
preauthHandshakeTimeoutMs: 30_000,
tlsFingerprint: "sha256:11:22:33:44",
+5 -1
View File
@@ -152,7 +152,11 @@ export class GatewayChatClient implements TuiBackend {
platform: process.platform,
mode: GATEWAY_CLIENT_MODES.UI,
deviceIdentity: connection.allowInsecureLocalOperatorUi ? null : undefined,
caps: [GATEWAY_CLIENT_CAPS.TASK_SUGGESTIONS, GATEWAY_CLIENT_CAPS.TOOL_EVENTS],
caps: [
GATEWAY_CLIENT_CAPS.PLUGIN_APPROVALS,
GATEWAY_CLIENT_CAPS.TASK_SUGGESTIONS,
GATEWAY_CLIENT_CAPS.TOOL_EVENTS,
],
instanceId: randomUUID(),
minProtocol: MIN_CLIENT_PROTOCOL_VERSION,
maxProtocol: PROTOCOL_VERSION,
+1
View File
@@ -423,6 +423,7 @@ describe("GatewayBrowserClient", () => {
expect(connectFrame.params?.minProtocol).toBe(MIN_CLIENT_PROTOCOL_VERSION);
expect(connectFrame.params?.maxProtocol).toBe(PROTOCOL_VERSION);
expect(connectFrame.params?.caps).toEqual([
GATEWAY_CLIENT_CAPS.APPROVALS,
GATEWAY_CLIENT_CAPS.TASK_SUGGESTIONS,
GATEWAY_CLIENT_CAPS.TERMINAL_OFFSET_SEQ,
GATEWAY_CLIENT_CAPS.TOOL_EVENTS,
+1
View File
@@ -457,6 +457,7 @@ export class GatewayBrowserClient {
scopes,
device,
caps: [
GATEWAY_CLIENT_CAPS.APPROVALS,
GATEWAY_CLIENT_CAPS.TASK_SUGGESTIONS,
GATEWAY_CLIENT_CAPS.TERMINAL_OFFSET_SEQ,
GATEWAY_CLIENT_CAPS.TOOL_EVENTS,