mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(exec): return approved WebChat gateway exec output inline (#85239)
Summary: - The PR changes gateway exec approval handling so native WebChat approvals wait for the decision and return a ... al as the exec tool result, while preserving async follow-ups for diagnostics-direct and non-WebChat paths. - Reproducibility: yes. Current-main source and tests show approval-required gateway exec returns approval-pen ... linked source PR provides live WebChat canary output showing the fixed inline result after native approval. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(exec): return approved WebChat gateway exec output inline Validation: - ClawSweeper review passed for head7182322015. - Required merge gates passed before the squash merge. Prepared head SHA:7182322015Review: https://github.com/openclaw/openclaw/pull/85239#issuecomment-4515339946 Co-authored-by: Zac-W <wangzhifengzac@gmail.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
This commit is contained in:
co-authored by
Zac-W
clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com>
takhoffman
parent
d0a74dbfbe
commit
17e2ccf179
@@ -241,6 +241,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Agents/exec approvals: return approved WebChat gateway exec output inline after native approval instead of leaving the model waiting for an async follow-up. (#82019) Thanks @Zac-W.
|
||||
- CLI/node: reject invalid explicit `node run --port` values instead of silently falling back to the configured or default port. Fixes #83923. Thanks @davinci282828.
|
||||
- CLI: reject explicit port numbers above 65535 before they reach Gateway or Node bind paths. Fixes #83900. (#84008) Thanks @hclsys.
|
||||
- Codex app-server: preserve plugin tool auth profiles when Codex owns model transport so OpenClaw dynamic tools can resolve their provider credentials. (#83603) Thanks @rubencu.
|
||||
|
||||
@@ -576,7 +576,7 @@ EOF`,
|
||||
expect(requireBuildFollowupTargetInput(0).sessionKey).toBe("agent:main:telegram:direct:123");
|
||||
});
|
||||
|
||||
it("formats diagnostics approvals as direct pasteable followups", async () => {
|
||||
it("keeps webchat diagnostics approvals as direct pasteable followups", async () => {
|
||||
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue("allow-once");
|
||||
createExecApprovalDecisionStateMock.mockReturnValue({
|
||||
baseDecision: { timedOut: false },
|
||||
@@ -624,13 +624,15 @@ EOF`,
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
await runGatewayAllowlist({
|
||||
const result = await runGatewayAllowlist({
|
||||
command: "openclaw gateway diagnostics export --json",
|
||||
trigger: "diagnostics",
|
||||
approvalFollowupMode: "direct",
|
||||
approvalFollowup,
|
||||
turnSourceChannel: "webchat",
|
||||
});
|
||||
|
||||
expect(result.pendingResult?.details.status).toBe("approval-pending");
|
||||
await vi.waitFor(() => {
|
||||
expect(sendExecApprovalFollowupResultMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -653,6 +655,51 @@ EOF`,
|
||||
expect(approvalInput?.outcome?.exitCode).toBe(0);
|
||||
});
|
||||
|
||||
it("waits inline for webchat approval so the exec tool can return real output to the model", async () => {
|
||||
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue("allow-once");
|
||||
createExecApprovalDecisionStateMock.mockReturnValue({
|
||||
baseDecision: { timedOut: false },
|
||||
approvedByAsk: true,
|
||||
deniedReason: null,
|
||||
});
|
||||
|
||||
const result = await runGatewayAllowlist({
|
||||
command: "pwd && df -h",
|
||||
turnSourceChannel: "webchat",
|
||||
});
|
||||
|
||||
expect(result.pendingResult).toBeUndefined();
|
||||
expect(result.deniedResult).toBeUndefined();
|
||||
expect(result.allowWithoutEnforcedCommand).toBe(true);
|
||||
expect(runExecProcessMock).not.toHaveBeenCalled();
|
||||
expect(buildExecApprovalFollowupTargetMock).not.toHaveBeenCalled();
|
||||
expect(sendExecApprovalFollowupResultMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns webchat approval denials as the foreground tool result", async () => {
|
||||
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue("deny");
|
||||
createExecApprovalDecisionStateMock.mockReturnValue({
|
||||
baseDecision: { timedOut: false },
|
||||
approvedByAsk: false,
|
||||
deniedReason: "user-denied",
|
||||
});
|
||||
|
||||
const result = await runGatewayAllowlist({
|
||||
command: "pwd && df -h",
|
||||
turnSourceChannel: "webchat",
|
||||
});
|
||||
|
||||
expect(result.pendingResult).toBeUndefined();
|
||||
expect(result.deniedResult?.details.status).toBe("failed");
|
||||
expect(result.deniedResult?.content[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
text: "Exec denied (gateway id=req-1, user-denied): pwd && df -h",
|
||||
}),
|
||||
);
|
||||
expect(runExecProcessMock).not.toHaveBeenCalled();
|
||||
expect(sendExecApprovalFollowupResultMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("denies timed-out inline-eval requests instead of auto-running them", async () => {
|
||||
const result = await runTimedOutStrictInlineEval({
|
||||
security: "full",
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
requiresExecApproval,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js";
|
||||
import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../utils/message-channel.js";
|
||||
import { markBackgrounded, tail } from "./bash-process-registry.js";
|
||||
import {
|
||||
buildExecApprovalRequesterContext,
|
||||
@@ -87,6 +88,7 @@ export type ProcessGatewayAllowlistResult = {
|
||||
execCommandOverride?: string;
|
||||
allowWithoutEnforcedCommand?: boolean;
|
||||
pendingResult?: AgentToolResult<ExecToolDetails>;
|
||||
deniedResult?: AgentToolResult<ExecToolDetails>;
|
||||
};
|
||||
|
||||
function hasGatewayAllowlistMiss(params: {
|
||||
@@ -347,6 +349,36 @@ function buildGatewayExecApprovalFollowupSummary(params: {
|
||||
: `Exec finished (gateway id=${params.approvalId}, session=${params.sessionId}, ${exitLabel})`;
|
||||
}
|
||||
|
||||
function shouldAwaitGatewayApprovalInline(params: {
|
||||
turnSourceChannel?: string;
|
||||
approvalFollowupMode?: "agent" | "direct";
|
||||
}): boolean {
|
||||
if (params.approvalFollowupMode === "direct") {
|
||||
return false;
|
||||
}
|
||||
return normalizeMessageChannel(params.turnSourceChannel) === INTERNAL_MESSAGE_CHANNEL;
|
||||
}
|
||||
|
||||
function buildGatewayExecApprovalDeniedToolResult(params: {
|
||||
approvalId: string;
|
||||
deniedReason: string;
|
||||
command: string;
|
||||
cwd: string;
|
||||
}): AgentToolResult<ExecToolDetails> {
|
||||
const text = `Exec denied (gateway id=${params.approvalId}, ${params.deniedReason}): ${params.command}`;
|
||||
return {
|
||||
content: [{ type: "text", text }],
|
||||
details: {
|
||||
status: "failed",
|
||||
exitCode: null,
|
||||
durationMs: 0,
|
||||
aggregated: text,
|
||||
timedOut: params.deniedReason.includes("timeout"),
|
||||
cwd: params.cwd,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function resolveGatewayExecApprovalFollowupText(params: {
|
||||
approvalFollowup?: ExecApprovalFollowupFactory;
|
||||
approvalId: string;
|
||||
@@ -564,31 +596,14 @@ export async function processGatewayAllowlist(
|
||||
allowlistEval.segments[0]?.resolution ?? null,
|
||||
params.workdir,
|
||||
);
|
||||
const effectiveTimeout =
|
||||
typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec;
|
||||
const followupTarget = buildExecApprovalFollowupTarget({
|
||||
approvalId,
|
||||
sessionKey: params.notifySessionKey ?? params.sessionKey,
|
||||
bashElevated: params.bashElevated,
|
||||
turnSourceChannel: params.turnSourceChannel,
|
||||
turnSourceTo: params.turnSourceTo,
|
||||
turnSourceAccountId: params.turnSourceAccountId,
|
||||
turnSourceThreadId: params.turnSourceThreadId,
|
||||
direct: params.approvalFollowupMode === "direct",
|
||||
});
|
||||
|
||||
void (async () => {
|
||||
const resolveApprovalForExecution = async (onFailure: () => void) => {
|
||||
const decision = await resolveApprovalDecisionOrUndefined({
|
||||
approvalId,
|
||||
preResolvedDecision,
|
||||
onFailure: () =>
|
||||
void sendExecApprovalFollowupResult(
|
||||
followupTarget,
|
||||
`Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`,
|
||||
),
|
||||
onFailure,
|
||||
});
|
||||
if (decision === undefined) {
|
||||
return;
|
||||
return { deniedReason: "approval-request-failed", requestFailed: true };
|
||||
}
|
||||
|
||||
const {
|
||||
@@ -647,10 +662,58 @@ export async function processGatewayAllowlist(
|
||||
deniedReason = deniedReason ?? "allowlist-miss";
|
||||
}
|
||||
|
||||
if (deniedReason) {
|
||||
return { deniedReason, requestFailed: false };
|
||||
};
|
||||
|
||||
if (unavailableReason === null && shouldAwaitGatewayApprovalInline(params)) {
|
||||
const approvalDecision = await resolveApprovalForExecution(() => undefined);
|
||||
if (approvalDecision.deniedReason) {
|
||||
return {
|
||||
deniedResult: buildGatewayExecApprovalDeniedToolResult({
|
||||
approvalId,
|
||||
deniedReason: approvalDecision.deniedReason,
|
||||
command: params.command,
|
||||
cwd: params.workdir,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
recordMatchedAllowlistUse(resolvedPath ?? undefined);
|
||||
return {
|
||||
execCommandOverride: enforcedCommand,
|
||||
allowWithoutEnforcedCommand: enforcedCommand === undefined,
|
||||
};
|
||||
}
|
||||
|
||||
const effectiveTimeout =
|
||||
typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec;
|
||||
const followupTarget = buildExecApprovalFollowupTarget({
|
||||
approvalId,
|
||||
sessionKey: params.notifySessionKey ?? params.sessionKey,
|
||||
bashElevated: params.bashElevated,
|
||||
turnSourceChannel: params.turnSourceChannel,
|
||||
turnSourceTo: params.turnSourceTo,
|
||||
turnSourceAccountId: params.turnSourceAccountId,
|
||||
turnSourceThreadId: params.turnSourceThreadId,
|
||||
direct: params.approvalFollowupMode === "direct",
|
||||
});
|
||||
|
||||
void (async () => {
|
||||
const approvalDecision = await resolveApprovalForExecution(
|
||||
() =>
|
||||
void sendExecApprovalFollowupResult(
|
||||
followupTarget,
|
||||
`Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`,
|
||||
),
|
||||
);
|
||||
if (approvalDecision.requestFailed) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (approvalDecision.deniedReason) {
|
||||
await sendExecApprovalFollowupResult(
|
||||
followupTarget,
|
||||
`Exec denied (gateway id=${approvalId}, ${deniedReason}): ${params.command}`,
|
||||
`Exec denied (gateway id=${approvalId}, ${approvalDecision.deniedReason}): ${params.command}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1081,7 +1081,35 @@ describe("exec approvals", () => {
|
||||
expect(sendMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("executes approved commands and emits a session-only followup in webchat-only mode", async () => {
|
||||
it("waits for native webchat approval and returns approved gateway output as a foreground result", async () => {
|
||||
const agentCalls: Array<Record<string, unknown>> = [];
|
||||
|
||||
mockAcceptedApprovalFlow({
|
||||
onAgent: (params) => {
|
||||
agentCalls.push(params);
|
||||
},
|
||||
});
|
||||
|
||||
const tool = createExecTool({
|
||||
host: "gateway",
|
||||
ask: "always",
|
||||
approvalRunningNoticeMs: 0,
|
||||
sessionKey: "agent:main:main",
|
||||
elevated: { enabled: true, allowed: true, defaultLevel: "ask" },
|
||||
messageProvider: "webchat",
|
||||
});
|
||||
|
||||
const result = await tool.execute("call-gw-native-webchat", {
|
||||
command: "printf webchat-ok",
|
||||
workdir: process.cwd(),
|
||||
});
|
||||
|
||||
expect(result.details.status).toBe("completed");
|
||||
expect(getResultText(result)).toContain("webchat-ok");
|
||||
expect(agentCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("keeps approved internal commands asynchronous without a webchat turn source", async () => {
|
||||
const agentCalls: Array<Record<string, unknown>> = [];
|
||||
|
||||
mockAcceptedApprovalFlow({
|
||||
|
||||
@@ -1593,6 +1593,9 @@ export function createExecTool(
|
||||
if (gatewayResult.pendingResult) {
|
||||
return gatewayResult.pendingResult;
|
||||
}
|
||||
if (gatewayResult.deniedResult) {
|
||||
return gatewayResult.deniedResult;
|
||||
}
|
||||
execCommandOverride = gatewayResult.execCommandOverride;
|
||||
if (gatewayResult.allowWithoutEnforcedCommand) {
|
||||
execCommandOverride = undefined;
|
||||
|
||||
Reference in New Issue
Block a user