fix(imessage): harden native approval polls

This commit is contained in:
joshavant
2026-07-27 17:30:30 -05:00
committed by Josh Avant
parent 42aeccd149
commit 245dc90543
11 changed files with 355 additions and 22 deletions
@@ -81,10 +81,15 @@ describe("imessage actions runtime", () => {
it("suppresses the imsg poll caption when the caller already rendered context", async () => {
runIMessageCliJsonCommandMock.mockResolvedValue({
guid: "poll-guid",
poll: { options: [] },
poll: {
options: [
{ id: " option-allow ", text: "Allow" },
{ id: "option-deny", text: " Deny " },
],
},
});
await imessageActionsRuntime.sendPoll({
const result = await imessageActionsRuntime.sendPoll({
chatGuid: "iMessage;+;chat0000",
question: "Approval details",
choices: ["Allow", "Deny"],
@@ -114,6 +119,13 @@ describe("imessage actions runtime", () => {
"--no-comment",
],
});
expect(result).toEqual({
messageId: "poll-guid",
pollOptions: [
{ id: "option-allow", text: "Allow" },
{ id: "option-deny", text: "Deny" },
],
});
});
it("drops cached chats.list entries when the current clock is not a valid date timestamp", async () => {
@@ -125,12 +137,14 @@ describe("imessage actions runtime", () => {
imessageActionsRuntime.resolveChatGuidForTarget({
target: { kind: "chat_id", chatId: 1 },
options: { cliPath: "imsg-invalid-clock" },
conversationReadOrigin: "delegated",
}),
).resolves.toBe("iMessage;+;first");
await expect(
imessageActionsRuntime.resolveChatGuidForTarget({
target: { kind: "chat_id", chatId: 2 },
options: { cliPath: "imsg-invalid-clock" },
conversationReadOrigin: "delegated",
}),
).resolves.toBe("iMessage;+;second");
@@ -156,12 +170,14 @@ describe("imessage actions runtime", () => {
imessageActionsRuntime.resolveChatGuidForTarget({
target: { kind: "chat_id", chatId: 1 },
options: { cliPath: "imsg-overflow-clock" },
conversationReadOrigin: "direct-operator",
}),
).resolves.toBe("iMessage;+;first");
await expect(
imessageActionsRuntime.resolveChatGuidForTarget({
target: { kind: "chat_id", chatId: 2 },
options: { cliPath: "imsg-overflow-clock" },
conversationReadOrigin: "direct-operator",
}),
).resolves.toBe("iMessage;+;second");
@@ -1,6 +1,7 @@
// Imessage plugin module implements actions behavior.
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { extname, join } from "node:path";
import type { ChannelMessageActionContext } from "openclaw/plugin-sdk/channel-contract";
import {
asDateTimestampMs,
parseStrictInteger,
@@ -32,6 +33,10 @@ type IMessageBridgeSendResult = {
messageId: string;
};
type IMessageConversationReadOrigin = NonNullable<
ChannelMessageActionContext["conversationReadOrigin"]
>;
/** Option identity assigned by Messages when the poll balloon was created. */
export type IMessagePollSentOption = {
id: string;
@@ -250,7 +255,11 @@ export const imessageActionsRuntime = {
async resolveChatGuidForTarget(params: {
target: Extract<IMessageTarget, { kind: "chat_id" | "chat_identifier" }>;
options: CliRunOptions;
conversationReadOrigin: IMessageConversationReadOrigin;
}): Promise<string | null> {
// Requiring the host-normalized origin at this list-backed read seam keeps
// direct operator lookups distinct from delegated actions, which have
// already passed the core exact-current-conversation gate.
// Each `chats.list` call spawns a fresh imsg rpc subprocess and pulls
// every chat the account knows about. Bursts of agent actions (react
// then reply, reply then add-participant, etc.) all paid that cost
+3
View File
@@ -984,6 +984,7 @@ describe("imessage message actions", () => {
{
target: { kind: "chat_id", chatId: 42 },
options: imsgOptions(),
conversationReadOrigin: "delegated",
},
],
]);
@@ -1111,6 +1112,7 @@ describe("imessage message actions", () => {
{
target: { kind: "chat_identifier", chatIdentifier: "team-thread" },
options: imsgOptions(),
conversationReadOrigin: "delegated",
},
],
]);
@@ -1355,6 +1357,7 @@ describe("imessage message actions", () => {
chatIdentifier: "iMessage;-;+12069106512",
},
options: imsgOptions(),
conversationReadOrigin: "direct-operator",
},
],
]);
+11 -1
View File
@@ -11,6 +11,7 @@ import {
} from "openclaw/plugin-sdk/channel-actions";
import type {
ChannelMessageActionAdapter,
ChannelMessageActionContext,
ChannelMessageActionName,
} from "openclaw/plugin-sdk/channel-contract";
import { createLazyRuntimeNamedExport } from "openclaw/plugin-sdk/lazy-runtime";
@@ -55,6 +56,10 @@ const GROUP_MANAGEMENT_ACTIONS = new Set<ChannelMessageActionName>([
"leaveGroup",
]);
type IMessageConversationReadOrigin = NonNullable<
ChannelMessageActionContext["conversationReadOrigin"]
>;
function readMessageText(params: Record<string, unknown>): string | undefined {
return readStringParam(params, "text") ?? readStringParam(params, "message");
}
@@ -169,6 +174,7 @@ async function resolveChatGuid(params: {
action: ChannelMessageActionName;
actionParams: Record<string, unknown>;
currentChannelId?: string;
conversationReadOrigin: IMessageConversationReadOrigin;
runtime: IMessageActionsRuntime;
options: {
cliPath: string;
@@ -185,6 +191,7 @@ async function resolveChatGuid(params: {
const resolved = await params.runtime.resolveChatGuidForTarget({
target,
options: params.options,
conversationReadOrigin: params.conversationReadOrigin,
});
if (resolved) {
return resolved;
@@ -202,6 +209,7 @@ async function resolveChatGuid(params: {
const resolved = await params.runtime.resolveChatGuidForTarget({
target: { kind: "chat_identifier", chatIdentifier: synthesizedIdentifier },
options: params.options,
conversationReadOrigin: params.conversationReadOrigin,
});
if (resolved) {
return resolved;
@@ -491,11 +499,13 @@ export const imessageMessageActions: ChannelMessageActionAdapter = {
timeoutMs: account.config.probeTimeoutMs,
chatGuid: "",
};
const attestedConversationReadOrigin = conversationReadOrigin ?? "delegated";
const chatGuid = async () =>
await resolveChatGuid({
action,
actionParams: params,
currentChannelId: toolContext?.currentChannelId,
conversationReadOrigin: attestedConversationReadOrigin,
runtime,
options: opts,
});
@@ -525,7 +535,7 @@ export const imessageMessageActions: ChannelMessageActionAdapter = {
dbPath: opts.dbPath,
}),
remoteHost: opts.remoteHost,
conversationReadOrigin,
conversationReadOrigin: attestedConversationReadOrigin,
},
});
};
@@ -681,7 +681,10 @@ describe("imessageApprovalNativeRuntime", () => {
});
expect(actionsMock.resolveChatGuidForTarget).toHaveBeenCalledWith(
expect.objectContaining({ target: { kind: "chat_id", chatId: 42 } }),
expect.objectContaining({
target: { kind: "chat_id", chatId: 42 },
conversationReadOrigin: "direct-operator",
}),
);
expect(actionsMock.sendPoll).toHaveBeenCalled();
expect(entry).toMatchObject({ poll: expect.anything(), reactionFallbackVisible: true });
@@ -208,10 +208,7 @@ function resolveIMessageApprovalCliOptions(params: {
*
* Conversation-read authority: `chatGuid` is resolved from the approval's own
* routing target (origin session or a configured approver), so this read is
* host-originated, not delegated. The bridge runtime seam does not yet accept
* an attested `conversationReadOrigin` the way `sendMessageIMessage` does; the
* safety here rests on the target never being caller-supplied. Route this
* through the attested seam once it exists rather than widening the target.
* host-originated and carries the server-owned direct-operator attestation.
*
*/
async function deliverIMessageApprovalPoll(params: {
@@ -331,7 +328,11 @@ async function resolveIMessageApprovalChatGuid(params: {
}
const runtime = await loadIMessageActionsRuntime();
if (target.kind === "chat_id" || target.kind === "chat_identifier") {
return await runtime.resolveChatGuidForTarget({ target, options: params.cliOptions });
return await runtime.resolveChatGuidForTarget({
target,
options: params.cliOptions,
conversationReadOrigin: "direct-operator",
});
}
if (target.kind !== "handle") {
return null;
@@ -340,6 +341,7 @@ async function resolveIMessageApprovalChatGuid(params: {
return await runtime.resolveChatGuidForTarget({
target: { kind: "chat_identifier", chatIdentifier: `${service};-;${target.to}` },
options: params.cliOptions,
conversationReadOrigin: "direct-operator",
});
}
@@ -292,6 +292,25 @@ describe("maybeResolveIMessageApprovalPollVote", () => {
expect(resolverMocks.resolveIMessageApproval).not.toHaveBeenCalled();
});
it("rejects imsg's local-identity sender fallback on received rows", async () => {
expect(bind()).toBe(true);
await expect(
maybeResolveIMessageApprovalPollVote({
cfg,
accountId: "default",
message: buildVote({
sender: APPROVER,
participant: APPROVER,
isFromMe: false,
destinationCallerId: APPROVER,
}),
}),
).resolves.toBe(false);
expect(resolverMocks.resolveIMessageApproval).not.toHaveBeenCalled();
});
it("uses the option id when imsg reports the prompt GUID instead of the poll GUID", async () => {
registerIMessageApprovalPollTarget({
accountId: "default",
+13 -7
View File
@@ -305,15 +305,21 @@ function readPollVoteEvent(message: IMessagePayload): ApprovalPollVoteEvent | nu
(typeof poll.poll_guid === "string" && poll.poll_guid) ||
"",
);
// chat.db authenticates received rows through sender. For a paired-device
// self-send, sender can be empty; destination_caller_id is the database's
// local-account identity and is accepted only when is_from_me is true.
// chat.db authenticates received rows through sender. Released imsg fills an
// empty sender from destination_caller_id before serialization, so reject a
// received row when those identities are equal: its remote actor is
// indistinguishable from the local-account fallback. Paired-device self-sends
// may use destination_caller_id only when is_from_me is authoritative.
const sender = normalizeIMessageHandle((message.sender ?? "").trim());
const destinationCallerId = normalizeIMessageHandle((message.destination_caller_id ?? "").trim());
const receivedSenderIsLocalFallback =
message.is_from_me !== true &&
Boolean(sender) &&
Boolean(destinationCallerId) &&
sender === destinationCallerId;
const actorHandle =
sender ||
(message.is_from_me === true
? normalizeIMessageHandle((message.destination_caller_id ?? "").trim())
: "");
(receivedSenderIsLocalFallback ? "" : sender) ||
(message.is_from_me === true ? destinationCallerId : "");
if (!pollGuid || !actorHandle) {
return null;
}
@@ -0,0 +1,180 @@
// Imessage tests cover approval poll durable-ingress ownership.
import type { waitForTransportReady } from "openclaw/plugin-sdk/transport-ready-runtime";
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { createIMessageRpcClient } from "./client.js";
import { monitorIMessageProvider } from "./monitor.js";
import { installIMessageStateRuntimeForTest } from "./test-support/runtime.js";
const waitForTransportReadyMock = vi.hoisted(() =>
vi.fn<typeof waitForTransportReady>(async () => {}),
);
const createIMessageRpcClientMock = vi.hoisted(() => vi.fn<typeof createIMessageRpcClient>());
const maybeResolveIMessageApprovalPollVoteMock = vi.hoisted(() => vi.fn());
const ingressLifecycleMocks = vi.hoisted(() => ({
onAdopted: vi.fn(async () => {}),
onDeferred: vi.fn(),
onAdoptionFinalizing: vi.fn(),
onAbandoned: vi.fn(async () => {}),
}));
const ingressHarness = vi.hoisted(() => ({
done: Promise.resolve(),
resolveDone: () => {},
}));
vi.mock("openclaw/plugin-sdk/transport-ready-runtime", () => ({
waitForTransportReady: waitForTransportReadyMock,
}));
vi.mock("openclaw/plugin-sdk/channel-inbound", async (importOriginal) => {
const actual = await importOriginal<typeof import("openclaw/plugin-sdk/channel-inbound")>();
return {
...actual,
createChannelInboundDebouncer: vi.fn(
(opts: { onFlush: (entries: unknown[]) => Promise<void> }) => ({
debouncer: {
enqueue: async (entry: unknown) => {
await opts.onFlush([entry]);
},
},
}),
),
};
});
vi.mock("./approval-polls.js", () => ({
maybeResolveIMessageApprovalPollVote: maybeResolveIMessageApprovalPollVoteMock,
}));
vi.mock("./approval-reaction-poller.js", () => ({
pollPendingIMessageApprovalReactions: vi.fn(async () => {}),
}));
vi.mock("./approval-reactions.js", () => ({
maybeResolveIMessageApprovalReaction: vi.fn(async () => false),
}));
vi.mock("./client.js", () => ({
createIMessageRpcClient: createIMessageRpcClientMock,
}));
vi.mock("./monitor/abort-handler.js", () => ({
attachIMessageMonitorAbortHandler: vi.fn(() => () => {}),
}));
vi.mock("./monitor/ingress.js", () => ({
createIMessageDurableIngress: vi.fn(
(opts: {
dispatch: (
message: unknown,
lifecycle: unknown,
receivedAt: number,
provenance: object,
) => Promise<unknown>;
}) => ({
receive: async (raw: { message: unknown }) => {
try {
await opts.dispatch(
raw.message,
{
abortSignal: new AbortController().signal,
...ingressLifecycleMocks,
},
Date.now(),
{},
);
} finally {
ingressHarness.resolveDone();
}
},
start: vi.fn(),
stop: vi.fn(async () => {}),
}),
),
}));
function createRuntime() {
return {
log: vi.fn(),
error: vi.fn(),
};
}
describe("iMessage approval poll durable ingress", () => {
beforeEach(() => {
installIMessageStateRuntimeForTest();
waitForTransportReadyMock.mockReset().mockResolvedValue(undefined);
createIMessageRpcClientMock.mockReset();
maybeResolveIMessageApprovalPollVoteMock.mockReset();
for (const mock of Object.values(ingressLifecycleMocks)) {
mock.mockClear();
}
ingressHarness.done = new Promise<void>((resolve) => {
ingressHarness.resolveDone = () => resolve();
});
});
function arrangeNotification() {
let onNotification: ((message: { method: string; params: unknown }) => void) | undefined;
createIMessageRpcClientMock.mockImplementation(async (params) => {
onNotification = params?.onNotification;
return {
request: vi.fn(async () => ({ subscription: 1 })),
waitForClose: vi.fn(async () => {
onNotification?.({
method: "message",
params: {
message: {
id: 42,
guid: "approval-poll-vote-guid",
chat_id: 7,
chat_guid: "iMessage;-;redacted-peer",
chat_identifier: "redacted-peer",
sender: "redacted-peer",
is_from_me: false,
is_group: false,
created_at: new Date().toISOString(),
poll: {
kind: "vote",
original_guid: "approval-poll-guid",
vote: { option_id: "approve-once" },
},
},
},
});
await ingressHarness.done;
}),
stop: vi.fn(async () => {}),
} as never;
});
}
it("completes a handled approval vote instead of abandoning it for replay", async () => {
arrangeNotification();
maybeResolveIMessageApprovalPollVoteMock.mockResolvedValue(true);
await monitorIMessageProvider({
config: { channels: { imessage: {} } } as never,
runtime: createRuntime() as never,
});
expect(ingressLifecycleMocks.onAdopted).toHaveBeenCalledTimes(1);
expect(ingressLifecycleMocks.onAbandoned).not.toHaveBeenCalled();
});
it("abandons a claim when approval vote resolution fails transiently", async () => {
arrangeNotification();
maybeResolveIMessageApprovalPollVoteMock.mockRejectedValue(new Error("gateway unavailable"));
const runtime = createRuntime();
await monitorIMessageProvider({
config: { channels: { imessage: {} } } as never,
runtime: runtime as never,
});
expect(ingressLifecycleMocks.onAdopted).not.toHaveBeenCalled();
expect(ingressLifecycleMocks.onAbandoned).toHaveBeenCalledTimes(1);
expect(runtime.error).toHaveBeenCalledWith(
expect.stringContaining("imessage: inbound dispatch failed: Error: gateway unavailable"),
);
});
});
@@ -775,11 +775,8 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P
message,
})
) {
// A vote is a real message row, so the debouncer still owns its ingress
// claim (see dispatch's `deferred` result). Resolving it here means no
// session will ever adopt it; abandon explicitly or the claim stalls
// until the 300s handler-timeout fires.
await ingressLifecycle?.onAbandoned();
// Returning normally lets dispatchUnit settle the durable claim. Only a
// resolver error abandons it for replay; a handled vote must not retry.
return;
}
+89 -1
View File
@@ -720,7 +720,9 @@ describe("probeIMessage", () => {
available: false,
});
expect(runCommand).toHaveBeenCalledTimes(4);
// Each uncached probe runs status plus both side-effect-free CLI capability
// checks (send-rich attachment and poll caption suppression).
expect(runCommand).toHaveBeenCalledTimes(6);
});
it("propagates imsg's status message when advanced features are unavailable", async () => {
@@ -756,6 +758,92 @@ describe("probeIMessage", () => {
});
});
it("detects poll caption suppression from the exact poll send help contract", async () => {
const runCommand = vi
.spyOn(processRuntime, "runCommandWithTimeout")
.mockResolvedValueOnce({
stdout: JSON.stringify({
advanced_features: true,
v2_ready: true,
selectors: { pollPayloadMessage: true },
rpc_methods: ["chats.list"],
}),
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
})
.mockResolvedValueOnce({
stdout: "send-rich --file",
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
})
.mockResolvedValueOnce({
stdout: "poll send --question <text> --option <text> --no-comment",
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
});
await expect(
probeIMessagePrivateApi("imsg-poll-no-comment-supported", 1000),
).resolves.toMatchObject({
cliCapabilities: { pollSendSupportsNoComment: true },
});
expect(runCommand).toHaveBeenNthCalledWith(
3,
["imsg-poll-no-comment-supported", "poll", "send", "--help"],
{ timeoutMs: 1000 },
);
});
it("does not infer poll caption suppression from selectors when the flag is absent", async () => {
vi.spyOn(processRuntime, "runCommandWithTimeout")
.mockResolvedValueOnce({
stdout: JSON.stringify({
advanced_features: true,
v2_ready: true,
selectors: { pollPayloadMessage: true },
rpc_methods: ["chats.list"],
}),
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
})
.mockResolvedValueOnce({
stdout: "send-rich --file",
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
})
.mockResolvedValueOnce({
stdout: "poll send --question <text> --option <text>",
stderr: "",
code: 0,
signal: null,
killed: false,
termination: "exit",
});
await expect(
probeIMessagePrivateApi("imsg-poll-no-comment-absent", 1000),
).resolves.toMatchObject({
available: true,
selectors: { pollPayloadMessage: true },
cliCapabilities: { pollSendSupportsNoComment: false },
});
});
it("fails fast for default local imsg probes on non-mac hosts", async () => {
const createIMessageRpcClientMock = vi
.spyOn(clientModule, "createIMessageRpcClient")