fix(imessage): deduplicate only configured watchers with canonical backend identity (#118974)

This commit is contained in:
Peter Steinberger
2026-08-03 14:37:27 -07:00
committed by GitHub
parent 3ded8fba9b
commit 7fd3363866
4 changed files with 283 additions and 13 deletions
+140 -2
View File
@@ -202,6 +202,144 @@ describe("iMessage duplicate-source watcher ownership", () => {
expect(resolveIMessageDuplicateSourceOwner({ cfg, account: ownerAccount })).toBeUndefined();
});
it.each([
{
name: "the implicit and explicitly configured default database",
first: { cliPath: "imsg" },
second: () => ({
dbPath: path.join(process.env.HOME || os.homedir(), "Library", "Messages", "chat.db"),
}),
},
{
name: "a home-relative default database",
first: { cliPath: "imsg" },
second: () => ({ dbPath: "~/Library/Messages/chat.db" }),
},
{
name: "a lexically equivalent default database",
first: { cliPath: "imsg" },
second: () => ({
dbPath: `${process.env.HOME || os.homedir()}/Library/Messages/../Messages/chat.db`,
}),
},
{
name: "the same absolute executable with implicit and explicit databases",
first: { cliPath: "/usr/local/bin/imsg" },
second: () => ({
cliPath: "/usr/local/bin/imsg",
dbPath: path.join(process.env.HOME || os.homedir(), "Library", "Messages", "chat.db"),
}),
},
])("assigns one watcher and doctor warning for $name", ({ first, second }) => {
const cfg = {
channels: {
imessage: {
accounts: {
primary: first,
secondary: second(),
},
},
},
} as never;
expect(
resolveIMessageDuplicateSourceOwner({
cfg,
account: resolveIMessageAccount({ cfg, accountId: "primary" }),
}),
).toBeUndefined();
expect(
resolveIMessageDuplicateSourceOwner({
cfg,
account: resolveIMessageAccount({ cfg, accountId: "secondary" }),
}),
).toBe("primary");
expect(collectIMessageDuplicateAccountSourceWarnings({ cfg })).toHaveLength(1);
});
it.each([
{
name: "different explicit local databases",
first: { cliPath: "imsg", dbPath: "/tmp/imessage-primary.db" },
second: { cliPath: "imsg", dbPath: "/tmp/imessage-secondary.db" },
},
{
name: "different custom wrappers for the same remote database path",
first: { cliPath: "/usr/local/bin/imsg-primary", dbPath: "/Users/bot/Messages/chat.db" },
second: { cliPath: "/usr/local/bin/imsg-secondary", dbPath: "/Users/bot/Messages/chat.db" },
},
{
name: "different auto-detected remote wrappers both named imsg",
first: { cliPath: "/opt/host-a/imsg", dbPath: "/Users/bot/Library/Messages/chat.db" },
second: { cliPath: "/opt/host-b/imsg", dbPath: "/Users/bot/Library/Messages/chat.db" },
},
{
name: "an unverified bare command and a different absolute executable",
first: { cliPath: "imsg" },
second: { cliPath: "/usr/local/bin/imsg" },
},
{
name: "different remote hosts behind the same wrapper",
first: {
cliPath: "/usr/local/bin/imsg-ssh",
dbPath: "/Users/bot/Messages/chat.db",
remoteHost: "bot@primary.example",
},
second: {
cliPath: "/usr/local/bin/imsg-ssh",
dbPath: "/Users/bot/Messages/chat.db",
remoteHost: "bot@secondary.example",
},
},
{
name: "an explicitly remote and a local default binary",
first: { cliPath: "imsg" },
second: { cliPath: "imsg", remoteHost: "bot@remote.example" },
},
])("preserves independent watchers for $name", ({ first, second }) => {
const cfg = {
channels: {
imessage: {
accounts: {
primary: first,
secondary: second,
},
},
},
} as never;
for (const accountId of ["primary", "secondary"]) {
expect(
resolveIMessageDuplicateSourceOwner({
cfg,
account: resolveIMessageAccount({ cfg, accountId }),
}),
).toBeUndefined();
}
expect(collectIMessageDuplicateAccountSourceWarnings({ cfg })).toEqual([]);
});
it("never lets an unconfigured account own or warn for the only startable watcher", () => {
const cfg = {
channels: {
imessage: {
accounts: {
primary: {},
secondary: { enabled: true, cliPath: "imsg" },
},
},
},
} as never;
const unconfigured = resolveIMessageAccount({ cfg, accountId: "primary" });
const configured = resolveIMessageAccount({ cfg, accountId: "secondary" });
expect(unconfigured).toMatchObject({ enabled: true, configured: false });
expect(configured).toMatchObject({ enabled: true, configured: true });
expect(resolveIMessageDuplicateSourceOwner({ cfg, account: unconfigured })).toBeUndefined();
expect(resolveIMessageDuplicateSourceOwner({ cfg, account: configured })).toBeUndefined();
expect(collectIMessageDuplicateAccountSourceWarnings({ cfg })).toEqual([]);
});
it("reports no duplicate ownership when accounts target different cliPaths", () => {
const cfg = {
channels: {
@@ -246,8 +384,8 @@ describe("iMessage duplicate-source watcher ownership", () => {
channels: {
imessage: {
accounts: {
"swang430-gmail-com": {},
default: {},
"swang430-gmail-com": { enabled: true },
default: { enabled: true },
},
},
},
+24 -8
View File
@@ -1,4 +1,5 @@
import { statSync } from "node:fs";
import path from "node:path";
import { createAccountListHelpers } from "openclaw/plugin-sdk/account-helpers";
import { DEFAULT_ACCOUNT_ID } from "openclaw/plugin-sdk/account-id";
import { normalizeAccountId, type OpenClawConfig } from "openclaw/plugin-sdk/account-resolution";
@@ -7,7 +8,11 @@ import { expectDefined } from "openclaw/plugin-sdk/expect-runtime";
import { resolveAccountEntry } from "openclaw/plugin-sdk/routing";
import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
import type { IMessageAccountConfig } from "./account-types.js";
import { resolveLocalIMessageChatDbPath } from "./cli-path.js";
import {
expandIMessageUserPath,
resolveIMessageHomeDir,
resolveLocalIMessageChatDbPath,
} from "./cli-path.js";
export type ResolvedIMessageAccount = {
accountId: string;
@@ -134,10 +139,21 @@ function normalizeIMessageDbPath(value: string | undefined | null): string {
// Two enabled accounts that share a signature watch the same source, which
// caused duplicate inbound handling in openclaw/openclaw#65141.
function resolveIMessageAccountSourceSignature(account: ResolvedIMessageAccount): string {
return JSON.stringify([
normalizeIMessageCliPath(account.config.cliPath),
normalizeIMessageDbPath(account.config.dbPath),
]);
const cliPath = normalizeIMessageCliPath(account.config.cliPath);
const dbPath = normalizeIMessageDbPath(account.config.dbPath);
const remoteHost = account.config.remoteHost?.trim();
// A remote path belongs to the SSH host and must not expand against the local home.
if (remoteHost) {
return JSON.stringify([cliPath, dbPath, remoteHost]);
}
const home = resolveIMessageHomeDir();
const localDbPath = dbPath
? expandIMessageUserPath(dbPath)
: home
? path.join(home, "Library", "Messages", "chat.db")
: undefined;
// Preserve the exact executable: same-basename SSH wrappers can target different hosts.
return JSON.stringify([cliPath, localDbPath ? path.resolve(localDbPath) : "", ""]);
}
function resolveIMessageAccountSourceOwner(params: {
@@ -152,7 +168,7 @@ function resolveIMessageAccountSourceOwner(params: {
cfg: params.cfg,
accountId: candidateAccountId,
});
if (!candidate.enabled) {
if (!candidate.enabled || !candidate.configured) {
continue;
}
if (resolveIMessageAccountSourceSignature(candidate) !== params.signature) {
@@ -188,7 +204,7 @@ export function resolveIMessageDuplicateSourceOwner(params: {
cfg: OpenClawConfig;
account: ResolvedIMessageAccount;
}): string | undefined {
if (!params.account.enabled) {
if (!params.account.enabled || !params.account.configured) {
return undefined;
}
const owner = resolveIMessageAccountSourceOwner({
@@ -256,7 +272,7 @@ export function collectIMessageDuplicateAccountSourceWarnings(params: {
const groups = new Map<string, ResolvedIMessageAccount[]>();
for (const accountId of listIMessageAccountIds(params.cfg)) {
const account = resolveIMessageAccount({ cfg: params.cfg, accountId });
if (!account.enabled) {
if (!account.enabled || !account.configured) {
continue;
}
const signature = resolveIMessageAccountSourceSignature(account);
+117 -1
View File
@@ -1,4 +1,6 @@
// Imessage tests cover channel plugin behavior.
import os from "node:os";
import path from "node:path";
import { describe, expect, it, vi } from "vitest";
const monitorMock = vi.hoisted(() => vi.fn(async () => undefined));
@@ -47,7 +49,7 @@ describe("startIMessageGatewayAccount duplicate-source handling", () => {
imessage: {
accounts: {
"swang430-gmail-com": { cliPath: "imsg" },
default: {},
default: { enabled: true },
},
},
},
@@ -97,6 +99,120 @@ describe("startIMessageGatewayAccount duplicate-source handling", () => {
);
});
it.each([
{
name: "an implicit and explicit default database",
primary: { cliPath: "imsg" },
secondary: () => ({
cliPath: "imsg",
dbPath: path.join(process.env.HOME || os.homedir(), "Library", "Messages", "chat.db"),
}),
},
{
name: "the same absolute executable with implicit and explicit databases",
primary: { cliPath: "/usr/local/bin/imsg" },
secondary: () => ({
cliPath: "/usr/local/bin/imsg",
dbPath: path.join(process.env.HOME || os.homedir(), "Library", "Messages", "chat.db"),
}),
},
])("starts only one real monitor for $name", async ({ primary, secondary }) => {
monitorMock.mockClear();
monitorMock.mockResolvedValueOnce(undefined);
const cfg = {
channels: {
imessage: {
accounts: { primary, secondary: secondary() },
},
},
} as never;
const owner = makeCtx({ cfg, accountId: "primary" });
const duplicate = makeCtx({ cfg, accountId: "secondary" });
await startIMessageGatewayAccount(owner.ctx);
const duplicateTask = startIMessageGatewayAccount(duplicate.ctx);
try {
await Promise.resolve();
await Promise.resolve();
expect(monitorMock).toHaveBeenCalledTimes(1);
expect(duplicate.logEvents.some((event) => event.line.includes("skipping watcher"))).toBe(
true,
);
expect(
duplicate.statusEvents.every((event) => !(event as Record<string, unknown>).lifecycle),
).toBe(true);
} finally {
duplicate.abort();
await duplicateTask;
}
});
it("starts the only configured watcher instead of parking it under an unconfigured account", async () => {
monitorMock.mockClear();
monitorMock.mockResolvedValueOnce(undefined);
const cfg = {
channels: {
imessage: {
accounts: {
primary: {},
secondary: { enabled: true, cliPath: "imsg" },
},
},
},
} as never;
const configured = makeCtx({ cfg, accountId: "secondary" });
const task = startIMessageGatewayAccount(configured.ctx);
try {
await Promise.resolve();
await Promise.resolve();
expect(monitorMock).toHaveBeenCalledTimes(1);
expect(configured.statusEvents).toContainEqual(
expect.objectContaining({ lifecycle: "starting", accountId: "secondary" }),
);
} finally {
configured.abort();
await task;
}
});
it("starts independent monitors for distinct auto-detected remote wrappers named imsg", async () => {
monitorMock.mockClear();
monitorMock.mockResolvedValue(undefined);
const cfg = {
channels: {
imessage: {
accounts: {
primary: {
enabled: true,
cliPath: "/opt/host-a/imsg",
dbPath: "/Users/bot/Library/Messages/chat.db",
},
secondary: {
enabled: true,
cliPath: "/opt/host-b/imsg",
dbPath: "/Users/bot/Library/Messages/chat.db",
},
},
},
},
} as never;
const first = makeCtx({ cfg, accountId: "primary" });
const second = makeCtx({ cfg, accountId: "secondary" });
await startIMessageGatewayAccount(first.ctx);
const secondTask = startIMessageGatewayAccount(second.ctx);
try {
await Promise.resolve();
await Promise.resolve();
expect(monitorMock).toHaveBeenCalledTimes(2);
expect(second.logEvents.some((event) => event.line.includes("skipping watcher"))).toBe(false);
} finally {
second.abort();
await secondTask;
}
});
it("starts monitorIMessageProvider when an account has no duplicate sibling", async () => {
monitorMock.mockClear();
monitorMock.mockResolvedValueOnce(undefined);
+2 -2
View File
@@ -9,8 +9,8 @@ describe("imessageDoctor.collectPreviewWarnings", () => {
channels: {
imessage: {
accounts: {
"swang430-gmail-com": {},
default: {},
"swang430-gmail-com": { enabled: true },
default: { enabled: true },
},
},
},