mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-07 02:22:46 +00:00
fix(codex): preserve requester across approval bridge (#116152)
This commit is contained in:
committed by
GitHub
parent
2e1bf01f51
commit
ec46d30fbf
@@ -232,6 +232,48 @@ describe("Codex app-server approval bridge", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves the initiating requester when re-running policy for a promoted file approval", async () => {
|
||||
const params = createParams();
|
||||
params.senderId = "owner-1";
|
||||
params.senderIsOwner = true;
|
||||
params.memberRoleIds = ["role-a", "role-b"];
|
||||
mockRunBeforeToolCallHook.mockImplementation(async ({ params: hookParams, ctx }) =>
|
||||
ctx?.requester?.senderIsOwner === true
|
||||
? { blocked: false, params: hookParams }
|
||||
: { blocked: true, reason: "owner required", kind: "veto" },
|
||||
);
|
||||
|
||||
const result = await handleCodexAppServerApprovalRequest({
|
||||
method: "item/fileChange/requestApproval",
|
||||
requestParams: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
itemId: "patch-owner-policy",
|
||||
},
|
||||
paramsForRun: params,
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
autoApproveOpenClawToolPolicy: true,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ decision: "accept" });
|
||||
expect(mockRunBeforeToolCallHook).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
toolName: "apply_patch",
|
||||
ctx: expect.objectContaining({
|
||||
requester: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
senderId: "owner-1",
|
||||
senderIsOwner: true,
|
||||
roleIds: ["role-a", "role-b"],
|
||||
},
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["promoted tool policy", { autoApproveOpenClawToolPolicy: true }],
|
||||
["full-auto runtime policy", { autoApprove: true }],
|
||||
@@ -328,6 +370,10 @@ describe("Codex app-server approval bridge", () => {
|
||||
agentId: "main",
|
||||
sessionKey: "agent:main:session-1",
|
||||
channelId: "chat-1",
|
||||
requester: {
|
||||
channel: "telegram",
|
||||
accountId: "default",
|
||||
},
|
||||
workspaceDir: undefined,
|
||||
turnSourceChannel: "telegram",
|
||||
turnSourceTo: "chat-1",
|
||||
|
||||
@@ -19,6 +19,7 @@ import { normalizeTrimmedStringList } from "openclaw/plugin-sdk/string-coerce-ru
|
||||
import { sliceUtf16Safe, truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";
|
||||
import { formatCodexDisplayText } from "../command-formatters.js";
|
||||
import { resolveCodexToolAbortTerminalReason } from "./dynamic-tool-execution.js";
|
||||
import { buildCodexHookRequester } from "./hook-requester.js";
|
||||
import {
|
||||
approvalRequestExplicitlyUnavailable,
|
||||
mapExecDecisionToOutcome,
|
||||
@@ -470,6 +471,7 @@ async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
currentChannelId: params.paramsForRun.currentChannelId,
|
||||
messageTo: params.paramsForRun.messageTo,
|
||||
}).channelId;
|
||||
const requester = buildCodexHookRequester(params.paramsForRun);
|
||||
const outcome = await runBeforeToolCallHook({
|
||||
toolName: policyRequest.toolName,
|
||||
params: policyRequest.params,
|
||||
@@ -485,6 +487,9 @@ async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
...(params.paramsForRun.sessionId ? { sessionId: params.paramsForRun.sessionId } : {}),
|
||||
...(params.paramsForRun.runId ? { runId: params.paramsForRun.runId } : {}),
|
||||
...(hookChannelId ? { channelId: hookChannelId } : {}),
|
||||
// This is the same concrete call already seen by native PreToolUse. Preserve
|
||||
// its host-proven actor so sender-aware policy cannot authorize two identities.
|
||||
...(requester ? { requester } : {}),
|
||||
trigger: params.paramsForRun.trigger,
|
||||
approvalReviewerDeviceId: params.paramsForRun.approvalReviewerDeviceId,
|
||||
turnSourceChannel: params.paramsForRun.messageChannel ?? params.paramsForRun.messageProvider,
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import type { EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime";
|
||||
import {
|
||||
initializeGlobalHookRunner,
|
||||
resetGlobalHookRunner,
|
||||
} from "openclaw/plugin-sdk/hook-runtime";
|
||||
import { createMockPluginRegistry } from "openclaw/plugin-sdk/plugin-test-runtime";
|
||||
import { withTempDir } from "openclaw/plugin-sdk/test-env";
|
||||
import type { PluginHookToolContext } from "openclaw/plugin-sdk/types";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { resolveCodexAppServerRuntimeOptions } from "./config.js";
|
||||
import type { CodexModelListResponse } from "./protocol.js";
|
||||
import { runCodexAppServerAttempt } from "./run-attempt.js";
|
||||
import { createCodexTestBindingStore } from "./session-binding.test-helpers.js";
|
||||
import { createIsolatedCodexAppServerClient } from "./shared-client.js";
|
||||
|
||||
const LIVE =
|
||||
process.env.OPENCLAW_LIVE_TEST === "1" &&
|
||||
process.env.OPENCLAW_LIVE_CODEX_APPROVAL_REQUESTER === "1";
|
||||
const describeLive = LIVE ? describe : describe.skip;
|
||||
|
||||
afterEach(() => {
|
||||
resetGlobalHookRunner();
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
describeLive("Codex app-server approval requester real-binary bridge", () => {
|
||||
it("preserves an owner requester through promoted apply_patch approval", async () => {
|
||||
await withTempDir("openclaw-codex-approval-requester-", async (root) => {
|
||||
const workspace = path.join(root, "workspace");
|
||||
const agentDir = path.join(root, "agent");
|
||||
const target = path.join(workspace, "memory", "real-binary-owner.md");
|
||||
await fs.mkdir(path.dirname(target), { recursive: true });
|
||||
vi.stubEnv("OPENCLAW_STATE_DIR", path.join(root, "state"));
|
||||
|
||||
const runtime = resolveCodexAppServerRuntimeOptions({
|
||||
pluginConfig: { appServer: { homeScope: "user" } },
|
||||
env: {},
|
||||
});
|
||||
const client = await createIsolatedCodexAppServerClient({
|
||||
startOptions: runtime.start,
|
||||
agentDir,
|
||||
authProfileId: null,
|
||||
timeoutMs: 120_000,
|
||||
});
|
||||
try {
|
||||
const listed = await client.request<CodexModelListResponse>(
|
||||
"model/list",
|
||||
{ limit: 100, cursor: null, includeHidden: false },
|
||||
{ timeoutMs: 60_000 },
|
||||
);
|
||||
const modelId =
|
||||
listed.data.find((model) => model.isDefault)?.model ?? listed.data[0]?.model;
|
||||
if (!modelId) {
|
||||
throw new Error("Codex model/list returned no models");
|
||||
}
|
||||
|
||||
const hookRequesters: unknown[] = [];
|
||||
initializeGlobalHookRunner(
|
||||
createMockPluginRegistry([
|
||||
{
|
||||
hookName: "before_tool_call",
|
||||
handler: async (_event, ctx) => {
|
||||
const hookContext = ctx as PluginHookToolContext;
|
||||
hookRequesters.push(hookContext.requester);
|
||||
return hookContext.requester?.senderIsOwner === true
|
||||
? undefined
|
||||
: { block: true, blockReason: "owner required" };
|
||||
},
|
||||
},
|
||||
]),
|
||||
);
|
||||
const serverRequestMethods: string[] = [];
|
||||
client.addRequestHandler((request) => {
|
||||
serverRequestMethods.push(request.method);
|
||||
return undefined;
|
||||
});
|
||||
const agentEvents: Array<{ stream: string; data: Record<string, unknown> }> = [];
|
||||
const params = {
|
||||
sessionId: "approval-requester-session",
|
||||
sessionKey: "agent:approval-requester:main",
|
||||
sessionFile: path.join(root, "session.jsonl"),
|
||||
workspaceDir: workspace,
|
||||
cwd: workspace,
|
||||
agentDir,
|
||||
provider: "codex",
|
||||
modelId,
|
||||
model: {
|
||||
id: modelId,
|
||||
name: modelId,
|
||||
provider: "codex",
|
||||
api: "openai-chatgpt-responses",
|
||||
reasoning: true,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 200_000,
|
||||
maxTokens: 8_000,
|
||||
compat: { supportsTools: false },
|
||||
},
|
||||
prompt:
|
||||
"Use the exec tool exactly once. In its JavaScript call tools.apply_patch to create memory/real-binary-owner.md containing exactly REAL_BINARY_OWNER_OK, then reply done. Do not call apply_patch directly.",
|
||||
runId: "approval-requester-run",
|
||||
contextTokenBudget: 150_000,
|
||||
contextWindowInfo: {
|
||||
tokens: 150_000,
|
||||
referenceTokens: 200_000,
|
||||
source: "agentContextTokens",
|
||||
},
|
||||
thinkLevel: "medium",
|
||||
disableTools: false,
|
||||
config: { tools: { web: { search: { enabled: false } } } },
|
||||
timeoutMs: 180_000,
|
||||
trigger: "user",
|
||||
oneShotCliRun: true,
|
||||
senderIsOwner: true,
|
||||
authStorage: {},
|
||||
authProfileStore: { version: 1, profiles: {} },
|
||||
modelRegistry: {},
|
||||
onAgentEvent: (event: { stream: string; data: Record<string, unknown> }) => {
|
||||
agentEvents.push(event);
|
||||
},
|
||||
} as unknown as EmbeddedRunAttemptParams;
|
||||
|
||||
const result = await runCodexAppServerAttempt(params, {
|
||||
bindingStore: createCodexTestBindingStore(),
|
||||
pluginConfig: { appServer: { homeScope: "user" } },
|
||||
nativeHookRelay: { enabled: true, events: ["pre_tool_use"] },
|
||||
clientFactory: async () => client,
|
||||
});
|
||||
|
||||
expect(result.terminal.kind, JSON.stringify(result.terminal)).toBe("ok");
|
||||
expect(await fs.readFile(target, "utf8")).toBe("REAL_BINARY_OWNER_OK\n");
|
||||
expect(serverRequestMethods).toContain("item/fileChange/requestApproval");
|
||||
expect(hookRequesters).toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ senderIsOwner: true })]),
|
||||
);
|
||||
expect(agentEvents).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
stream: "approval",
|
||||
data: expect.objectContaining({
|
||||
status: "approved",
|
||||
message: "Codex app-server approval accepted by OpenClaw tool policy.",
|
||||
}),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
} finally {
|
||||
await client.closeAndWait();
|
||||
}
|
||||
});
|
||||
}, 240_000);
|
||||
});
|
||||
27
extensions/codex/src/app-server/hook-requester.ts
Normal file
27
extensions/codex/src/app-server/hook-requester.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
import type { EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime";
|
||||
import type { PluginHookToolContext } from "openclaw/plugin-sdk/types";
|
||||
|
||||
type CodexHookRequester = NonNullable<PluginHookToolContext["requester"]>;
|
||||
|
||||
/** Rebuilds the host-proven requester identity shared by native and bridged tool hooks. */
|
||||
export function buildCodexHookRequester(
|
||||
params: Pick<
|
||||
EmbeddedRunAttemptParams,
|
||||
| "messageChannel"
|
||||
| "messageProvider"
|
||||
| "agentAccountId"
|
||||
| "senderId"
|
||||
| "senderIsOwner"
|
||||
| "memberRoleIds"
|
||||
>,
|
||||
): CodexHookRequester | undefined {
|
||||
const channel = params.messageChannel ?? params.messageProvider;
|
||||
const requester: CodexHookRequester = {
|
||||
...(channel ? { channel } : {}),
|
||||
...(params.agentAccountId ? { accountId: params.agentAccountId } : {}),
|
||||
...(params.senderId ? { senderId: params.senderId } : {}),
|
||||
...(params.senderIsOwner !== undefined ? { senderIsOwner: params.senderIsOwner } : {}),
|
||||
...(params.memberRoleIds?.length ? { roleIds: [...params.memberRoleIds] } : {}),
|
||||
};
|
||||
return Object.keys(requester).length > 0 ? requester : undefined;
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { resolveCodexStartupTimeoutMs } from "./attempt-timeouts.js";
|
||||
import type { CodexAppServerClient } from "./client.js";
|
||||
import { resolveCodexToolAbortTerminalReason } from "./dynamic-tool-execution.js";
|
||||
import { CodexAppServerEventProjector } from "./event-projector.js";
|
||||
import { buildCodexHookRequester } from "./hook-requester.js";
|
||||
import {
|
||||
buildCodexNativeHookRelayDisabledConfig,
|
||||
buildCodexNativeHookRelayConfig,
|
||||
@@ -183,14 +184,7 @@ export function prepareCodexAttemptResources(prompt: CodexAttemptPrompt) {
|
||||
timeoutFloorMs: options.startupTimeoutFloorMs,
|
||||
});
|
||||
const requesterChannel = params.messageChannel ?? params.messageProvider;
|
||||
const requester = {
|
||||
...(requesterChannel ? { channel: requesterChannel } : {}),
|
||||
...(params.agentAccountId ? { accountId: params.agentAccountId } : {}),
|
||||
...(params.senderId ? { senderId: params.senderId } : {}),
|
||||
...(params.senderIsOwner !== undefined ? { senderIsOwner: params.senderIsOwner } : {}),
|
||||
...(params.memberRoleIds?.length ? { roleIds: [...params.memberRoleIds] } : {}),
|
||||
};
|
||||
const hasRequester = Object.keys(requester).length > 0;
|
||||
const requester = buildCodexHookRequester(params);
|
||||
const buildNativeHookRelayFinalConfigPatch = (
|
||||
decision: { action: "resume"; binding: CodexAppServerThreadBinding } | { action: "start" },
|
||||
) => {
|
||||
@@ -210,7 +204,7 @@ export function prepareCodexAttemptResources(prompt: CodexAttemptPrompt) {
|
||||
config: params.config,
|
||||
runId: params.runId,
|
||||
channelId: hookChannelId,
|
||||
...(hasRequester ? { requester } : {}),
|
||||
...(requester ? { requester } : {}),
|
||||
approvalContext: {
|
||||
trigger: params.trigger,
|
||||
approvalReviewerDeviceId: params.approvalReviewerDeviceId,
|
||||
|
||||
Reference in New Issue
Block a user