Skip to content

Commit acbd319

Browse files
omarshahineomarshahine
committed
fix(imessage): complete bounded approval discovery
1 parent 47b1b8a commit acbd319

3 files changed

Lines changed: 132 additions & 4 deletions

File tree

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

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,126 @@ describe("iMessage approval reaction poller", () => {
150150
);
151151
});
152152

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+
153273
it("retries no-target recent-chat discovery after the first chat list fails", async () => {
154274
const request = vi.fn(async (method: string) => {
155275
if (method === "chats.list") {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ export async function pollPendingIMessageApprovalReactions(params: {
296296
logVerboseMessage: params.logVerboseMessage,
297297
});
298298
if (handled.stopPolling) {
299+
if (shouldAttemptNoTargetDiscovery && handled.stopPollingReason !== "resolver-error") {
300+
break;
301+
}
299302
return;
300303
}
301304
}

‎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)