Skip to content

Commit 3a9ea1d

Browse files
colmbroganomarshahine
colmbrogan
and
omarshahine
authored
fix(imessage): skip idle approval discovery scans (#88530)
* fix(imessage): bound idle approval discovery scans * fix(imessage): complete bounded approval discovery --------- Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com>
1 parent a90eb93 commit 3a9ea1d

3 files changed

Lines changed: 251 additions & 7 deletions

File tree

‎extensions/imessage/src/approval-reaction-poller.test.ts‎

Lines changed: 220 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// Imessage tests cover approval reaction poller plugin behavior.
22
import { beforeEach, describe, expect, it, vi } from "vitest";
3-
import { pollPendingIMessageApprovalReactions } from "./approval-reaction-poller.js";
3+
import {
4+
clearIMessageApprovalReactionPollerStateForTest,
5+
pollPendingIMessageApprovalReactions,
6+
} from "./approval-reaction-poller.js";
47
import {
58
clearIMessageApprovalReactionTargetsForTest,
69
registerIMessageApprovalReactionTarget,
@@ -24,6 +27,7 @@ function createClient(request: ReturnType<typeof vi.fn>): IMessageRpcClient {
2427
describe("iMessage approval reaction poller", () => {
2528
beforeEach(() => {
2629
clearIMessageApprovalReactionTargetsForTest();
30+
clearIMessageApprovalReactionPollerStateForTest();
2731
resolverMocks.resolveIMessageApproval.mockReset();
2832
resolverMocks.resolveIMessageApproval.mockResolvedValue(undefined);
2933
resolverMocks.isApprovalNotFoundError.mockReset();
@@ -116,6 +120,221 @@ describe("iMessage approval reaction poller", () => {
116120
});
117121
});
118122

123+
it("bounds no-target recent-chat discovery to one pass per account", async () => {
124+
const request = vi.fn(async (method: string) => {
125+
if (method === "chats.list") {
126+
return { chats: [{ id: 42 }] };
127+
}
128+
if (method === "messages.history") {
129+
return { messages: [] };
130+
}
131+
throw new Error(`unexpected method ${method}`);
132+
});
133+
134+
const pollParams = {
135+
client: createClient(request),
136+
cfg: { channels: { imessage: { allowFrom: ["+15551230000"] } } },
137+
accountId: "default",
138+
allowRecentChatDiscovery: true,
139+
};
140+
141+
await pollPendingIMessageApprovalReactions(pollParams);
142+
await pollPendingIMessageApprovalReactions(pollParams);
143+
144+
expect(request).toHaveBeenCalledTimes(2);
145+
expect(request).toHaveBeenCalledWith("chats.list", { limit: 50 }, { timeoutMs: 10_000 });
146+
expect(request).toHaveBeenCalledWith(
147+
"messages.history",
148+
{ chat_id: 42, limit: 30 },
149+
{ timeoutMs: 10_000 },
150+
);
151+
});
152+
153+
it("bounds no-target discovery after resolving an observed reaction", async () => {
154+
const request = vi.fn(async (method: string, payload?: { chat_id?: number }) => {
155+
if (method === "chats.list") {
156+
return { chats: [{ id: 42 }, { id: 99 }] };
157+
}
158+
if (method === "messages.history" && payload?.chat_id === 42) {
159+
return {
160+
messages: [
161+
{
162+
guid: "msg-1",
163+
chat_id: 42,
164+
chat_guid: "SMS;-;+15551230000",
165+
chat_identifier: "+15551230000",
166+
is_from_me: true,
167+
sender: "+15551230000",
168+
text: [
169+
"Exec approval required",
170+
"ID: exec-1",
171+
"",
172+
"Reply with: /approve exec-1 allow-once|deny",
173+
].join("\n"),
174+
reactions: [
175+
{
176+
id: 7,
177+
sender: "+15551230000",
178+
is_from_me: true,
179+
type: "like",
180+
emoji: "👍",
181+
created_at: "2026-05-27T21:00:00.000Z",
182+
},
183+
],
184+
},
185+
],
186+
};
187+
}
188+
if (method === "messages.history" && payload?.chat_id === 99) {
189+
return { messages: [] };
190+
}
191+
throw new Error(`unexpected request ${method} ${JSON.stringify(payload)}`);
192+
});
193+
194+
const pollParams = {
195+
client: createClient(request),
196+
cfg: { channels: { imessage: { allowFrom: ["+15551230000"] } } },
197+
accountId: "default",
198+
allowRecentChatDiscovery: true,
199+
};
200+
201+
await pollPendingIMessageApprovalReactions(pollParams);
202+
await pollPendingIMessageApprovalReactions(pollParams);
203+
204+
expect(resolverMocks.resolveIMessageApproval).toHaveBeenCalledTimes(1);
205+
expect(request.mock.calls.filter(([method]) => method === "chats.list")).toHaveLength(1);
206+
expect(request.mock.calls.filter(([method]) => method === "messages.history")).toHaveLength(2);
207+
expect(request).toHaveBeenCalledWith(
208+
"messages.history",
209+
{ chat_id: 99, limit: 30 },
210+
{ timeoutMs: 10_000 },
211+
);
212+
});
213+
214+
it("retries no-target discovery after resolver failures expire observed targets", async () => {
215+
resolverMocks.resolveIMessageApproval.mockRejectedValue(new Error("gateway down"));
216+
const request = vi.fn(async (method: string) => {
217+
if (method === "chats.list") {
218+
return { chats: [{ id: 42 }] };
219+
}
220+
if (method === "messages.history") {
221+
return {
222+
messages: [
223+
{
224+
guid: "msg-1",
225+
chat_id: 42,
226+
chat_guid: "SMS;-;+15551230000",
227+
chat_identifier: "+15551230000",
228+
is_from_me: true,
229+
sender: "+15551230000",
230+
text: [
231+
"Exec approval required",
232+
"ID: exec-1",
233+
"",
234+
"Reply with: /approve exec-1 allow-once|deny",
235+
].join("\n"),
236+
reactions: [
237+
{
238+
id: 7,
239+
sender: "+15551230000",
240+
is_from_me: true,
241+
type: "like",
242+
emoji: "👍",
243+
created_at: "2026-05-27T21:00:00.000Z",
244+
},
245+
],
246+
},
247+
],
248+
};
249+
}
250+
throw new Error(`unexpected method ${method}`);
251+
});
252+
const dateNow = vi.spyOn(Date, "now").mockReturnValue(1_800_000_000_000);
253+
254+
try {
255+
const pollParams = {
256+
client: createClient(request),
257+
cfg: { channels: { imessage: { allowFrom: ["+15551230000"] } } },
258+
accountId: "default",
259+
allowRecentChatDiscovery: true,
260+
};
261+
262+
await pollPendingIMessageApprovalReactions(pollParams);
263+
dateNow.mockReturnValue(1_800_000_301_000);
264+
await pollPendingIMessageApprovalReactions(pollParams);
265+
} finally {
266+
dateNow.mockRestore();
267+
}
268+
269+
expect(resolverMocks.resolveIMessageApproval).toHaveBeenCalledTimes(2);
270+
expect(request.mock.calls.filter(([method]) => method === "chats.list")).toHaveLength(2);
271+
});
272+
273+
it("retries no-target recent-chat discovery after the first chat list fails", async () => {
274+
const request = vi.fn(async (method: string) => {
275+
if (method === "chats.list") {
276+
const chatListCalls = request.mock.calls.filter(
277+
([calledMethod]) => calledMethod === "chats.list",
278+
);
279+
if (chatListCalls.length === 1) {
280+
throw new Error("temporary imsg failure");
281+
}
282+
return { chats: [{ id: 42 }] };
283+
}
284+
if (method === "messages.history") {
285+
return { messages: [] };
286+
}
287+
throw new Error(`unexpected method ${method}`);
288+
});
289+
290+
const pollParams = {
291+
client: createClient(request),
292+
cfg: { channels: { imessage: { allowFrom: ["+15551230000"] } } },
293+
accountId: "default",
294+
allowRecentChatDiscovery: true,
295+
};
296+
297+
await expect(pollPendingIMessageApprovalReactions(pollParams)).rejects.toThrow(
298+
"temporary imsg failure",
299+
);
300+
await pollPendingIMessageApprovalReactions(pollParams);
301+
302+
expect(request.mock.calls.filter(([method]) => method === "chats.list")).toHaveLength(2);
303+
expect(request.mock.calls.filter(([method]) => method === "messages.history")).toHaveLength(1);
304+
});
305+
306+
it("retries no-target recent-chat discovery after the first history fetch fails", async () => {
307+
const request = vi.fn(async (method: string) => {
308+
if (method === "chats.list") {
309+
return { chats: [{ id: 42 }] };
310+
}
311+
if (method === "messages.history") {
312+
const historyCalls = request.mock.calls.filter(
313+
([calledMethod]) => calledMethod === "messages.history",
314+
);
315+
if (historyCalls.length === 1) {
316+
throw new Error("temporary history failure");
317+
}
318+
return { messages: [] };
319+
}
320+
throw new Error(`unexpected method ${method}`);
321+
});
322+
323+
const pollParams = {
324+
client: createClient(request),
325+
cfg: { channels: { imessage: { allowFrom: ["+15551230000"] } } },
326+
accountId: "default",
327+
allowRecentChatDiscovery: true,
328+
};
329+
330+
await pollPendingIMessageApprovalReactions(pollParams);
331+
await pollPendingIMessageApprovalReactions(pollParams);
332+
await pollPendingIMessageApprovalReactions(pollParams);
333+
334+
expect(request.mock.calls.filter(([method]) => method === "chats.list")).toHaveLength(2);
335+
expect(request.mock.calls.filter(([method]) => method === "messages.history")).toHaveLength(2);
336+
});
337+
119338
it("does not bind observed approval prompts when the process clock is invalid", async () => {
120339
const request = vi.fn(async (method: string) => {
121340
if (method === "chats.list") {

‎extensions/imessage/src/approval-reaction-poller.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ const RECENT_CHAT_LIMIT = 50;
1919
const PER_CHAT_HISTORY_LIMIT = 30;
2020
const OBSERVED_APPROVAL_PROMPT_TARGET_TTL_MS = 5 * 60 * 1000;
2121

22+
const accountIdsWithCompletedNoTargetDiscovery = new Set<string>();
23+
24+
export function clearIMessageApprovalReactionPollerStateForTest(): void {
25+
accountIdsWithCompletedNoTargetDiscovery.clear();
26+
}
27+
2228
type ChatListEntry = {
2329
id?: number | null;
2430
};
@@ -228,10 +234,13 @@ export async function pollPendingIMessageApprovalReactions(params: {
228234
const targets = listPendingIMessageApprovalReactionPollTargets({
229235
accountId: params.accountId,
230236
});
231-
if (targets.length === 0 && params.allowRecentChatDiscovery !== true) {
237+
const shouldAttemptNoTargetDiscovery =
238+
targets.length === 0 &&
239+
params.allowRecentChatDiscovery === true &&
240+
!accountIdsWithCompletedNoTargetDiscovery.has(params.accountId);
241+
if (targets.length === 0 && !shouldAttemptNoTargetDiscovery) {
232242
return;
233243
}
234-
235244
const pendingByMessageId = buildPendingTargetsByMessageId(targets);
236245
const explicitChatIds = listTargetChatIds(targets);
237246
const shouldDiscoverRecentChats =
@@ -241,13 +250,18 @@ export async function pollPendingIMessageApprovalReactions(params: {
241250
? uniqueChatIds([...explicitChatIds, ...(await listRecentChatIds(params.client))])
242251
: explicitChatIds;
243252
if (chatIds.length === 0) {
253+
if (shouldAttemptNoTargetDiscovery) {
254+
accountIdsWithCompletedNoTargetDiscovery.add(params.accountId);
255+
}
244256
return;
245257
}
258+
let hadHistoryFetchError = false;
246259
for (const chatId of chatIds) {
247260
let messages: HistoryMessage[];
248261
try {
249262
messages = await fetchRecentHistory({ client: params.client, chatId });
250263
} catch (err) {
264+
hadHistoryFetchError = true;
251265
params.logVerboseMessage?.(
252266
`imessage: approval reaction poll skipped chat_id=${chatId}: ${String(err)}`,
253267
);
@@ -282,9 +296,15 @@ export async function pollPendingIMessageApprovalReactions(params: {
282296
logVerboseMessage: params.logVerboseMessage,
283297
});
284298
if (handled.stopPolling) {
299+
if (shouldAttemptNoTargetDiscovery && handled.stopPollingReason !== "resolver-error") {
300+
break;
301+
}
285302
return;
286303
}
287304
}
288305
}
289306
}
307+
if (shouldAttemptNoTargetDiscovery && !hadHistoryFetchError) {
308+
accountIdsWithCompletedNoTargetDiscovery.add(params.accountId);
309+
}
290310
}

‎extensions/imessage/src/approval-reactions.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,12 @@ type IMessageApprovalReactionResolution = {
3232
};
3333
export type IMessageApprovalReactionHandleResult =
3434
| { handled: false; stopPolling: false }
35-
| { handled: true; stopPolling: boolean };
35+
| { handled: true; stopPolling: false }
36+
| {
37+
handled: true;
38+
stopPolling: true;
39+
stopPollingReason: "resolved" | "not-found" | "resolver-error";
40+
};
3641

3742
type IMessageApprovalReactionTarget = ApprovalReactionTargetRecord;
3843

@@ -585,7 +590,7 @@ export async function handleIMessageApprovalReaction(params: {
585590
params.logVerboseMessage?.(
586591
`imessage: approval reaction resolved id=${target.approvalId} sender=${event.actorHandle} decision=${target.decision} via messageId=${matchedMessageId ?? event.messageId}`,
587592
);
588-
return { handled: true, stopPolling: true };
593+
return { handled: true, stopPolling: true, stopPollingReason: "resolved" };
589594
} catch (error) {
590595
if (isApprovalNotFoundError(error)) {
591596
for (const candidate of event.messageIdCandidates) {
@@ -598,7 +603,7 @@ export async function handleIMessageApprovalReaction(params: {
598603
params.logVerboseMessage?.(
599604
`imessage: approval reaction ignored for expired approval id=${target.approvalId} sender=${event.actorHandle}`,
600605
);
601-
return { handled: true, stopPolling: true };
606+
return { handled: true, stopPolling: true, stopPollingReason: "not-found" };
602607
}
603608
// Surface non-NotFound errors at warn level so a gateway 5xx / network
604609
// outage / auth failure is visible without OPENCLAW_LOG_LEVEL=debug.
@@ -616,7 +621,7 @@ export async function handleIMessageApprovalReaction(params: {
616621
params.logVerboseMessage?.(
617622
`imessage: approval reaction failed id=${target.approvalId} sender=${event.actorHandle}: ${String(error)}`,
618623
);
619-
return { handled: true, stopPolling: true };
624+
return { handled: true, stopPolling: true, stopPollingReason: "resolver-error" };
620625
}
621626
}
622627

0 commit comments

Comments
 (0)