fix(discord): keep exec approval fallbacks reachable

This commit is contained in:
Peter Steinberger
2026-04-29 06:29:36 +01:00
parent 66b4324d41
commit 548c280eff
14 changed files with 270 additions and 10 deletions

View File

@@ -92,6 +92,7 @@ Docs: https://docs.openclaw.ai
- OpenAI-compatible providers: drop malformed event-only or blank-data SSE frames before the OpenAI SDK stream parser sees them, so proxies that split `event:` from `data:` no longer crash streaming runs with `Unexpected end of JSON input`. Fixes #52802. Thanks @LyHug.
- Gateway/OpenAI-compatible streaming: strip `<final>` tags split across streamed model deltas before they reach SSE clients, so `/v1/chat/completions` no longer emits tag remnants or drops content when final-answer wrappers cross chunk boundaries. Fixes #63325. Thanks @tzwickl.
- Ollama: resolve explicitly selected signed-in `:cloud` models through `/api/show` when `/api/tags` omits them, so working models such as `gemini-3-flash-preview:cloud` and `deepseek-v4-pro:cloud` do not fail dynamic model resolution before the native `/api/chat` transport runs. Fixes #73909. Thanks @chtse53.
- Discord/exec approvals: keep the local `/approve` prompt when no native Discord approval runtime is active, and send a manual fallback notice when native approval delivery reaches no targets, so failed DM cards no longer leave approval turns silent or dependent on model-written shell commands. Fixes #73954; carries forward #74027. Thanks @guarismo and @brokemac79.
- Local model prompt caching: keep stable Project Context above volatile channel/session prompt guidance and stop embedding current channel names in the message tool description, so Ollama, MLX, llama.cpp, and other prefix-cache backends avoid avoidable full prompt reprocessing across channel turns. Fixes #40256; supersedes #40296. Thanks @rhclaw and @sriram369.
- Gateway/OpenAI-compatible API: guard provider policy lookup against runtime providers with non-array `models` values, so `/v1/chat/completions` no longer fails with `provider?.models?.some is not a function`. Fixes #66744; carries forward #66761. Thanks @MightyMoud, @MukundaKatta.
- WhatsApp/Web: pass explicit Baileys socket timings into every WhatsApp Web socket and expose `web.whatsapp.*` keepalive, connect, and query timeout settings so unstable networks can avoid repeated 408 disconnect and opening-handshake timeout loops. Fixes #56365. (#73580) Thanks @velvet-shark.

View File

@@ -912,6 +912,11 @@ Default slash command settings:
When those buttons are present, they are the primary approval UX; OpenClaw
should only include a manual `/approve` command when the tool result says
chat approvals are unavailable or manual approval is the only path.
If the Discord native approval runtime is not active, OpenClaw keeps the
local deterministic `/approve <id> <decision>` prompt visible. If the
runtime is active but a native card cannot be delivered to any target,
OpenClaw sends a same-chat fallback notice with the exact `/approve`
command from the pending approval.
Gateway auth and approval resolution follow the shared Gateway client contract (`plugin:` IDs resolve through `plugin.approval.resolve`; other IDs through `exec.approval.resolve`). Approvals expire after 30 minutes by default.

View File

@@ -262,6 +262,12 @@ agent-facing path. The agent should not also echo a duplicate plain chat
`/approve` command unless the tool result says chat approvals are unavailable or
manual approval is the only remaining path.
If a native approval client is configured but no native runtime is active for
the originating channel, OpenClaw keeps the local deterministic `/approve`
prompt visible. If the native runtime is active and attempts delivery but no
target receives the card, OpenClaw sends a same-chat fallback notice with the
exact `/approve <id> <decision>` command so the request can still be resolved.
Generic model:
- host exec policy still decides whether exec approval is required

View File

@@ -791,11 +791,12 @@ export const discordPlugin: ChannelPlugin<ResolvedDiscordAccount, DiscordProbe>
...discordOutbound,
preferFinalAssistantVisibleText: true,
shouldTreatDeliveredTextAsVisible: shouldTreatDiscordDeliveredTextAsVisible,
shouldSuppressLocalPayloadPrompt: ({ cfg, accountId, payload }) =>
shouldSuppressLocalPayloadPrompt: ({ cfg, accountId, payload, hint }) =>
shouldSuppressLocalDiscordExecApprovalPrompt({
cfg,
accountId,
payload,
hint,
}),
},
});

View File

@@ -1,3 +1,4 @@
import type { ChannelOutboundPayloadHint } from "openclaw/plugin-sdk/channel-contract";
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-types";
import type { DiscordExecApprovalConfig } from "openclaw/plugin-sdk/config-types";
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-dispatch-runtime";
@@ -5,6 +6,7 @@ import { resolveDiscordAccount } from "./accounts.js";
import {
getExecApprovalReplyMetadata,
isChannelExecApprovalClientEnabledFromConfig,
matchesApprovalRequestFilters,
resolveApprovalApprovers,
} from "./approval-runtime.js";
import { parseDiscordTarget } from "./target-parsing.js";
@@ -87,9 +89,22 @@ export function shouldSuppressLocalDiscordExecApprovalPrompt(params: {
cfg: OpenClawConfig;
accountId?: string | null;
payload: ReplyPayload;
hint?: ChannelOutboundPayloadHint;
}): boolean {
const metadata = getExecApprovalReplyMetadata(params.payload);
const config = resolveDiscordAccount(params).config.execApprovals;
return (
params.hint?.kind === "approval-pending" &&
params.hint.nativeRouteActive === true &&
isDiscordExecApprovalClientEnabled(params) &&
getExecApprovalReplyMetadata(params.payload) !== null
metadata !== null &&
matchesApprovalRequestFilters({
request: {
agentId: metadata.agentId,
sessionKey: metadata.sessionKey,
},
agentFilter: config?.agentFilter,
sessionFilter: config?.sessionFilter,
})
);
}

View File

@@ -792,6 +792,23 @@ describe("buildAgentSystemPrompt", () => {
expect(prompt).toContain("do not also send plain chat /approve instructions");
});
it("keeps approval slug guidance separate from command previews", () => {
const prompt = buildAgentSystemPrompt({
workspaceDir: "/tmp/openclaw",
runtimeInfo: {
channel: "discord",
},
});
expect(prompt).toContain(
'copy the exact /approve command from the tool output\'s "Reply with:" line',
);
expect(prompt).toContain("keep command/script previews separate from the /approve command");
expect(prompt).toContain(
"never substitute the shell command/script for the approval id or slug",
);
});
it("includes runtime provider capabilities when present", () => {
const prompt = buildAgentSystemPrompt({
workspaceDir: "/tmp/openclaw",

View File

@@ -149,9 +149,9 @@ function buildExecApprovalPromptGuidance(params: {
? Boolean(resolveChannelApprovalCapability(getChannelPlugin(runtimeChannel))?.native)
: false);
if (usesNativeApprovalUi) {
return "When exec returns approval-pending on this channel, rely on native approval card/buttons when they appear and do not also send plain chat /approve instructions. Only include the concrete /approve command if the tool result says chat approvals are unavailable or only manual approval is possible.";
return 'When exec returns approval-pending on this channel, rely on native approval card/buttons when they appear and do not also send plain chat /approve instructions. Only include the concrete /approve command if the tool result says chat approvals are unavailable or only manual approval is possible; when needed, copy the exact /approve command from the tool output\'s "Reply with:" line.';
}
return "When exec returns approval-pending, include the concrete /approve command from tool output as plain chat text for the user, and do not ask for a different or rotated code.";
return 'When exec returns approval-pending, include the concrete /approve command from the tool output\'s "Reply with:" line as plain chat text for the user, and do not ask for a different or rotated code.';
}
function buildSkillsSection(params: { skillsPrompt?: string; readToolName: string }) {
@@ -769,7 +769,7 @@ export function buildAgentSystemPrompt(params: {
}),
"Never execute /approve through exec or any other shell/tool path; /approve is a user-facing approval command, not a shell command.",
"Treat allow-once as single-command only: if another elevated command needs approval, request a fresh /approve and do not claim prior approval covered it.",
"When approvals are required, preserve and show the full command/script exactly as provided (including chained operators like &&, ||, |, ;, or multiline shells) so the user can approve what will actually run.",
"When approvals are required, preserve and show the full command/script exactly as provided (including chained operators like &&, ||, |, ;, or multiline shells) so the user can approve what will actually run, but keep command/script previews separate from the /approve command and never substitute the shell command/script for the approval id or slug.",
"",
],
}),

View File

@@ -1,5 +1,9 @@
import { beforeAll, beforeEach, describe, expect, it, vi, type Mock } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import {
clearApprovalNativeRouteStateForTest,
createApprovalNativeRouteReporter,
} from "../../infra/approval-native-route-coordinator.js";
import type { SessionBindingRecord } from "../../infra/outbound/session-binding-service.js";
import type {
AcpRuntime,
@@ -701,7 +705,14 @@ describe("dispatchReplyFromConfig", () => {
}),
outbound: {
deliveryMode: "direct",
shouldSuppressLocalPayloadPrompt: ({ payload }: { payload: ReplyPayload }) =>
shouldSuppressLocalPayloadPrompt: ({
payload,
hint,
}: {
payload: ReplyPayload;
hint?: { nativeRouteActive?: boolean };
}) =>
hint?.nativeRouteActive === true &&
Boolean(
payload.channelData &&
typeof payload.channelData === "object" &&
@@ -719,6 +730,7 @@ describe("dispatchReplyFromConfig", () => {
},
]),
);
clearApprovalNativeRouteStateForTest();
acpManagerRuntimeMocks.getAcpSessionManager.mockReset();
acpManagerRuntimeMocks.getAcpSessionManager.mockReturnValue(createMockAcpSessionManager());
resetInboundDedupe();
@@ -2581,7 +2593,7 @@ describe("dispatchReplyFromConfig", () => {
expect(replyResolver).toHaveBeenCalledTimes(1);
});
it("suppresses local discord exec approval tool prompts when discord approvals are enabled", async () => {
it("keeps local discord exec approval tool prompts when the native runtime is inactive", async () => {
setNoAbort();
const cfg = {
channels: {
@@ -2616,12 +2628,67 @@ describe("dispatchReplyFromConfig", () => {
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
expect(dispatcher.sendToolResult).not.toHaveBeenCalled();
expect(dispatcher.sendToolResult).toHaveBeenCalledWith(
expect.objectContaining({ text: "Approval required." }),
);
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(
expect.objectContaining({ text: "done" }),
);
});
it("suppresses local discord exec approval tool prompts when the native runtime is active", async () => {
setNoAbort();
const cfg = {
channels: {
discord: {
enabled: true,
execApprovals: {
enabled: true,
approvers: ["123"],
},
},
},
} as OpenClawConfig;
const reporter = createApprovalNativeRouteReporter({
handledKinds: new Set(["exec"]),
channel: "discord",
channelLabel: "Discord",
accountId: "default",
requestGateway: async <T>() => ({ ok: true }) as T,
});
reporter.start();
try {
const dispatcher = createDispatcher();
const ctx = buildTestCtx({
Provider: "discord",
Surface: "discord",
AccountId: "default",
});
const replyResolver = vi.fn(async (_ctx: MsgContext, options?: GetReplyOptions) => {
await options?.onToolResult?.({
text: "Approval required.",
channelData: {
execApproval: {
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
allowedDecisions: ["allow-once", "allow-always", "deny"],
},
},
});
return { text: "done" } as ReplyPayload;
});
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
expect(dispatcher.sendToolResult).not.toHaveBeenCalled();
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith(
expect.objectContaining({ text: "done" }),
);
} finally {
await reporter.stop();
}
});
it("deduplicates same-agent inbound replies across main and direct session keys", async () => {
setNoAbort();
const cfg = emptyConfig;

View File

@@ -1,5 +1,6 @@
import type { ReplyPayload } from "../../auto-reply/reply-payload.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { hasActiveApprovalNativeRouteRuntime } from "../../infra/approval-native-route-coordinator.js";
import { getChannelPlugin, normalizeChannelId } from "./registry.js";
export function shouldSuppressLocalExecApprovalPrompt(params: {
@@ -17,7 +18,15 @@ export function shouldSuppressLocalExecApprovalPrompt(params: {
cfg: params.cfg,
accountId: params.accountId,
payload: params.payload,
hint: { kind: "approval-pending", approvalKind: "exec" },
hint: {
kind: "approval-pending",
approvalKind: "exec",
nativeRouteActive: hasActiveApprovalNativeRouteRuntime({
channel,
accountId: params.accountId,
approvalKind: "exec",
}),
},
}) ?? false
);
}

View File

@@ -54,7 +54,11 @@ export type ChannelDeliveryCapabilities = {
};
export type ChannelOutboundPayloadHint =
| { kind: "approval-pending"; approvalKind: "exec" | "plugin" }
| {
kind: "approval-pending";
approvalKind: "exec" | "plugin";
nativeRouteActive?: boolean;
}
| { kind: "approval-resolved"; approvalKind: "exec" | "plugin" };
export type ChannelOutboundTargetRef = {

View File

@@ -212,4 +212,66 @@ describe("createApprovalNativeRouteReporter", () => {
});
expect(otherGateway).not.toHaveBeenCalled();
});
it("sends a manual fallback notice when native delivery reaches no targets", async () => {
const requestGateway = createGatewayRequestMock();
const request = {
id: "deadbeef-1234-4567-89ab-cdef01234567",
request: {
command: "echo hi",
allowedDecisions: ["allow-once", "deny"],
turnSourceChannel: "discord",
turnSourceTo: "channel:C123",
turnSourceAccountId: "default",
},
createdAtMs: 0,
expiresAtMs: Date.now() + 60_000,
} as const;
const reporter = createApprovalNativeRouteReporter({
handledKinds: new Set(["exec"]),
channel: "discord",
channelLabel: "Discord",
accountId: "default",
requestGateway,
});
reporter.start();
reporter.observeRequest({
approvalKind: "exec",
request,
});
await reporter.reportDelivery({
approvalKind: "exec",
request,
deliveryPlan: {
targets: [
{
surface: "approver-dm",
target: {
to: "user:owner",
},
reason: "preferred",
},
],
originTarget: {
to: "channel:C123",
},
notifyOriginWhenDmOnly: true,
},
deliveredTargets: [],
});
expect(requestGateway).toHaveBeenCalledWith("send", {
channel: "discord",
to: "channel:C123",
accountId: "default",
threadId: undefined,
message:
"Approval required. I could not deliver the native approval request.\n" +
"Reply with: /approve deadbeef allow-once|deny\n" +
"If the short code is ambiguous, use the full id in /approve.",
idempotencyKey: "approval-route-notice:deadbeef-1234-4567-89ab-cdef01234567",
});
});
});

View File

@@ -8,6 +8,7 @@ import type {
} from "./approval-native-delivery.js";
import {
describeApprovalDeliveryDestination,
resolveApprovalDeliveryFailedNoticeText,
resolveApprovalRoutedElsewhereNoticeText,
} from "./approval-native-route-notice.js";
import { buildChannelApprovalNativeTargetKey } from "./approval-native-target-key.js";
@@ -151,7 +152,21 @@ function didReportDeliverToOrigin(report: ApprovalRouteReport, originAccountId?:
);
}
function hasPlannedNativeTargets(report: ApprovalRouteReport): boolean {
return report.deliveryPlan.targets.length > 0;
}
function readAllowedDecisionStrings(request: ApprovalRequest): string[] | undefined {
const allowedDecisions =
"allowedDecisions" in request.request ? request.request.allowedDecisions : undefined;
if (!Array.isArray(allowedDecisions)) {
return undefined;
}
return allowedDecisions.filter((value): value is string => typeof value === "string");
}
function resolveApprovalRouteNotice(params: {
approvalKind: ChannelApprovalKind;
request: ApprovalRequest;
reports: readonly ApprovalRouteReport[];
}): { requestGateway: GatewayRequestFn; target: RouteNoticeTarget; text: string } | null {
@@ -176,6 +191,20 @@ function resolveApprovalRouteNotice(params: {
return null;
}
const originAccountId = normalizeOptionalString(target.accountId);
const deliveredAnyTarget = params.reports.some((report) => report.deliveredTargets.length > 0);
if (!deliveredAnyTarget && params.reports.some(hasPlannedNativeTargets)) {
return {
requestGateway:
params.reports.find((report) => activeApprovalRouteRuntimes.has(report.runtimeId))
?.requestGateway ?? params.reports[0].requestGateway,
target,
text: resolveApprovalDeliveryFailedNoticeText({
approvalId: params.request.id,
approvalKind: params.approvalKind,
allowedDecisions: readAllowedDecisionStrings(params.request),
}),
};
}
// If any same-channel runtime already delivered into the origin chat, every
// other fallback delivery becomes supplemental and should not trigger a notice.
@@ -237,6 +266,27 @@ function resolveApprovalRouteNotice(params: {
};
}
export function hasActiveApprovalNativeRouteRuntime(params: {
approvalKind: ChannelApprovalKind;
channel?: string | null;
accountId?: string | null;
}): boolean {
const channel = normalizeChannel(params.channel);
const accountId = normalizeOptionalString(params.accountId);
return Array.from(activeApprovalRouteRuntimes.values()).some((runtime) => {
if (!runtime.handledKinds.has(params.approvalKind)) {
return false;
}
if (channel && normalizeChannel(runtime.channel) !== channel) {
return false;
}
const runtimeAccountId = normalizeOptionalString(runtime.accountId);
return (
accountId === undefined || runtimeAccountId === undefined || runtimeAccountId === accountId
);
});
}
async function maybeFinalizeApprovalRouteNotice(approvalId: string): Promise<void> {
const entry = pendingApprovalRouteNotices.get(approvalId);
if (!entry || entry.finalized) {
@@ -251,6 +301,7 @@ async function maybeFinalizeApprovalRouteNotice(approvalId: string): Promise<voi
entry.finalized = true;
const reports = Array.from(entry.reports.values());
const notice = resolveApprovalRouteNotice({
approvalKind: entry.approvalKind,
request: entry.request,
reports,
});

View File

@@ -24,3 +24,24 @@ export function resolveApprovalRoutedElsewhereNoticeText(
uniqueDestinations.toSorted((a, b) => a.localeCompare(b)),
)}, not this chat.`;
}
export function resolveApprovalDeliveryFailedNoticeText(params: {
approvalId: string;
approvalKind: "exec" | "plugin";
allowedDecisions?: readonly string[];
}): string {
const commandId =
params.approvalKind === "exec" && params.approvalId.length > 8
? params.approvalId.slice(0, 8)
: params.approvalId;
const decisions = (
params.allowedDecisions?.length
? params.allowedDecisions
: ["allow-once", "allow-always", "deny"]
).join("|");
return [
"Approval required. I could not deliver the native approval request.",
`Reply with: /approve ${commandId} ${decisions}`,
"If the short code is ambiguous, use the full id in /approve.",
].join("\n");
}

View File

@@ -38,6 +38,7 @@ export type {
ChannelGatewayContext,
ChannelOutboundAdapter,
ChannelOutboundContext,
ChannelOutboundPayloadHint,
ChannelStatusAdapter,
} from "../channels/plugins/types.adapters.js";
export type { ChannelRuntimeSurface } from "../channels/plugins/channel-runtime-surface.types.js";