feat: duplicate-pair detection + logout-before-delete + ordering tests
Three connected bits of paired-account hygiene:
1. Duplicate-pair guard (apps/bot/src/ipc/pair-handler.ts)
Operator scans the QR with a phone that's already linked to
another account row → both rows would fight over the same
WhatsApp device and sends become a coin flip. After Baileys'
`open` event the bot now queries siblings of the same operator,
passes them through findDuplicateExistingAccount() (a pure
helper extracted to pair-state.ts), and on a hit:
- stops the new session (intentional; keeps the original's
session intact)
- scrubs the partial auth blob from disk
- resets the row's status to unpaired and clears phone_number
- emits a new session.duplicate event with the existing row's
label so PairLive can render a clear message
New PairLive 'duplicate' phase: amber icon + "Phone already
linked, unpair the existing account first or scan with a
different phone".
2. Logout-before-delete (apps/bot/src/ipc/unpair-handler.ts +
apps/bot/src/whatsapp/session-manager.ts)
Delete used to call account.unpair which only closes the local
socket — the operator's phone kept showing a phantom "linked
device" pointing at a row that no longer exists. Added:
- new account.delete command type (web side and bot side)
- sessionManager.logoutAndStop(): calls socket.logout() so
WhatsApp drops the device on the server side, THEN closes
the local socket. Best-effort; logout RPC failure doesn't
strand the delete.
- new handleDelete() handler that calls logoutAndStop, removes
session files, audits, and notifies.
- deleteAccountAction now sends account.delete instead of
account.unpair.
Unpair stays unchanged — re-pair-friendly, no logout.
3. Tests (bot 77 → 88, web 477 → 480)
- findDuplicateExistingAccount: 6 cases covering match, no-match,
self-exclusion, null/empty/whitespace handling, whitespace
normalisation, deterministic-pick when (defensively) two
siblings share a phone.
- handleUnpair / handleDelete: handleDelete calls logoutAndStop
BEFORE rm; handleUnpair never touches logoutAndStop (regression
guard for a refactor that swaps them); audit log payload
includes the row's label; audit lookup throwing doesn't strand
the delete.
- listAccounts ordering: static guard against the rename-
reshuffles-list regression. Pins `asc(a.createdAt)` + `asc(a.id)`
and rejects `asc(a.label)` in the function body.
Bot restarted with the new flow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f566e4683a
commit
2fe8459d25
@ -3,7 +3,7 @@ import type { Notification } from "pg";
|
|||||||
import { logger } from "../logger.js";
|
import { logger } from "../logger.js";
|
||||||
import { env } from "../env.js";
|
import { env } from "../env.js";
|
||||||
import { handleStartPairing } from "./pair-handler.js";
|
import { handleStartPairing } from "./pair-handler.js";
|
||||||
import { handleUnpair } from "./unpair-handler.js";
|
import { handleUnpair, handleDelete } from "./unpair-handler.js";
|
||||||
import { handleSyncGroups } from "./sync-groups-handler.js";
|
import { handleSyncGroups } from "./sync-groups-handler.js";
|
||||||
import { handleSendTest } from "./send-test-handler.js";
|
import { handleSendTest } from "./send-test-handler.js";
|
||||||
import {
|
import {
|
||||||
@ -14,6 +14,11 @@ import {
|
|||||||
export type BotCommand =
|
export type BotCommand =
|
||||||
| { type: "account.start_pairing"; accountId: string }
|
| { type: "account.start_pairing"; accountId: string }
|
||||||
| { type: "account.unpair"; accountId: string }
|
| { type: "account.unpair"; accountId: string }
|
||||||
|
// Like unpair, but tells WhatsApp to drop this device from the
|
||||||
|
// user's linked-devices list first via socket.logout(). The web
|
||||||
|
// action calls this immediately before deleting the row so the
|
||||||
|
// operator's phone doesn't keep showing a phantom linked device.
|
||||||
|
| { type: "account.delete"; accountId: string }
|
||||||
| { type: "account.sync_groups"; accountId: string }
|
| { type: "account.sync_groups"; accountId: string }
|
||||||
| { type: "group.send_test"; groupId: string; text: string }
|
| { type: "group.send_test"; groupId: string; text: string }
|
||||||
| { type: "reminder.schedule"; reminderId: string; scheduledAtIso: string }
|
| { type: "reminder.schedule"; reminderId: string; scheduledAtIso: string }
|
||||||
@ -74,6 +79,9 @@ export function registerDefaultHandlers(): void {
|
|||||||
registerHandler("account.unpair", async (cmd) => {
|
registerHandler("account.unpair", async (cmd) => {
|
||||||
await handleUnpair(cmd.accountId);
|
await handleUnpair(cmd.accountId);
|
||||||
});
|
});
|
||||||
|
registerHandler("account.delete", async (cmd) => {
|
||||||
|
await handleDelete(cmd.accountId);
|
||||||
|
});
|
||||||
registerHandler("account.sync_groups", async (cmd) => {
|
registerHandler("account.sync_groups", async (cmd) => {
|
||||||
await handleSyncGroups(cmd.accountId);
|
await handleSyncGroups(cmd.accountId);
|
||||||
});
|
});
|
||||||
|
|||||||
@ -10,6 +10,16 @@ export type WebEvent =
|
|||||||
| { type: "session.connected"; accountId: string; phoneNumber: string | null }
|
| { type: "session.connected"; accountId: string; phoneNumber: string | null }
|
||||||
| { type: "session.disconnected"; accountId: string }
|
| { type: "session.disconnected"; accountId: string }
|
||||||
| { type: "session.timeout"; accountId: string }
|
| { type: "session.timeout"; accountId: string }
|
||||||
|
// Operator scanned the QR with a phone that's already linked to another
|
||||||
|
// account row. We park the new pairing instead of letting two account
|
||||||
|
// rows fight over the same WhatsApp device. existingLabel surfaces in
|
||||||
|
// the UI so the operator knows which account already owns the phone.
|
||||||
|
| {
|
||||||
|
type: "session.duplicate";
|
||||||
|
accountId: string;
|
||||||
|
phoneNumber: string;
|
||||||
|
existingLabel: string;
|
||||||
|
}
|
||||||
| { type: "groups.synced"; accountId: string; count: number }
|
| { type: "groups.synced"; accountId: string; count: number }
|
||||||
| {
|
| {
|
||||||
type: "reminder.fired";
|
type: "reminder.fired";
|
||||||
|
|||||||
@ -10,7 +10,11 @@ import { renderQrPng } from "../whatsapp/qr-renderer.js";
|
|||||||
import { syncGroupsForAccount } from "../whatsapp/group-sync.js";
|
import { syncGroupsForAccount } from "../whatsapp/group-sync.js";
|
||||||
import { writeAuditLog } from "../audit.js";
|
import { writeAuditLog } from "../audit.js";
|
||||||
import { pgNotifyWeb } from "./notify.js";
|
import { pgNotifyWeb } from "./notify.js";
|
||||||
import { decidePairListenerOnClose, nextWarmingUpAfterEvent } from "./pair-state.js";
|
import {
|
||||||
|
decidePairListenerOnClose,
|
||||||
|
findDuplicateExistingAccount,
|
||||||
|
nextWarmingUpAfterEvent,
|
||||||
|
} from "./pair-state.js";
|
||||||
|
|
||||||
const PAIR_TIMEOUT_MS = 5 * 60 * 1000;
|
const PAIR_TIMEOUT_MS = 5 * 60 * 1000;
|
||||||
const offByAccount = new Map<string, () => void>();
|
const offByAccount = new Map<string, () => void>();
|
||||||
@ -126,6 +130,53 @@ export async function handleStartPairing(accountId: string): Promise<void> {
|
|||||||
}
|
}
|
||||||
lastQrPayload.delete(id);
|
lastQrPayload.delete(id);
|
||||||
offByAccount.delete(id);
|
offByAccount.delete(id);
|
||||||
|
|
||||||
|
// Duplicate-pair guard. Operator scanned the QR with a phone
|
||||||
|
// that's already linked to another account row. Letting both
|
||||||
|
// rows claim the same WhatsApp device confuses Baileys and
|
||||||
|
// turns sends into a coin flip — abandon this pairing and
|
||||||
|
// surface a clear message to the UI.
|
||||||
|
const siblings = await db.query.whatsappAccounts.findMany({
|
||||||
|
where: (a, { eq: dEq }) => dEq(a.operatorId, account.operatorId),
|
||||||
|
columns: { id: true, phoneNumber: true, label: true },
|
||||||
|
});
|
||||||
|
const dup = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: id,
|
||||||
|
currentPhoneNumber: event.phoneNumber,
|
||||||
|
siblings,
|
||||||
|
});
|
||||||
|
if (dup) {
|
||||||
|
logger.warn(
|
||||||
|
{
|
||||||
|
accountId: id,
|
||||||
|
phoneNumber: event.phoneNumber,
|
||||||
|
existingAccountId: dup.existingAccountId,
|
||||||
|
existingLabel: dup.existingLabel,
|
||||||
|
},
|
||||||
|
"pair: duplicate phone — abandoning new pairing",
|
||||||
|
);
|
||||||
|
// Stop the duplicate session, scrub the partial auth blob,
|
||||||
|
// and reset the row's status. We DO NOT logout() here — the
|
||||||
|
// original account's session remains valid and the operator
|
||||||
|
// hasn't actually added a new linked device on the phone yet
|
||||||
|
// (it'd just be the freshly-completed scan, which Baileys
|
||||||
|
// hasn't yet committed to the WhatsApp side).
|
||||||
|
await sessionManager.stop(id, { intentional: true });
|
||||||
|
await rm(join(env.SESSIONS_DIR, id), { recursive: true, force: true });
|
||||||
|
await db
|
||||||
|
.update(whatsappAccounts)
|
||||||
|
.set({ status: "unpaired", lastQrPng: null, phoneNumber: null })
|
||||||
|
.where(eq(whatsappAccounts.id, id));
|
||||||
|
await pgNotifyWeb({
|
||||||
|
type: "session.duplicate",
|
||||||
|
accountId: id,
|
||||||
|
phoneNumber: event.phoneNumber!,
|
||||||
|
existingLabel: dup.existingLabel,
|
||||||
|
});
|
||||||
|
off();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const session = sessionManager.getSession(id);
|
const session = sessionManager.getSession(id);
|
||||||
let synced = 0;
|
let synced = 0;
|
||||||
if (session) {
|
if (session) {
|
||||||
|
|||||||
@ -3,6 +3,7 @@ import {
|
|||||||
decideOnPairClose,
|
decideOnPairClose,
|
||||||
decideOnPairTimeout,
|
decideOnPairTimeout,
|
||||||
decidePairListenerOnClose,
|
decidePairListenerOnClose,
|
||||||
|
findDuplicateExistingAccount,
|
||||||
nextWarmingUpAfterEvent,
|
nextWarmingUpAfterEvent,
|
||||||
shouldAutoReconnect,
|
shouldAutoReconnect,
|
||||||
} from "./pair-state.js";
|
} from "./pair-state.js";
|
||||||
@ -204,3 +205,105 @@ describe("nextWarmingUpAfterEvent (pair-listener flag transitions)", () => {
|
|||||||
expect(warming).toBe(false);
|
expect(warming).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("findDuplicateExistingAccount (one-phone-per-account guard)", () => {
|
||||||
|
const sibling = (id: string, phone: string | null, label: string) => ({
|
||||||
|
id,
|
||||||
|
phoneNumber: phone,
|
||||||
|
label,
|
||||||
|
});
|
||||||
|
|
||||||
|
it("flags a sibling that already holds this phone number", () => {
|
||||||
|
const r = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: "60123456789",
|
||||||
|
siblings: [
|
||||||
|
sibling("new", null, "scratch"),
|
||||||
|
sibling("existing", "60123456789", "Yiekheng-my"),
|
||||||
|
sibling("other", "60987654321", "WaBot Test"),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(r).toEqual({
|
||||||
|
existingAccountId: "existing",
|
||||||
|
existingLabel: "Yiekheng-my",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns null when the phone is unique", () => {
|
||||||
|
const r = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: "60123456789",
|
||||||
|
siblings: [
|
||||||
|
sibling("new", null, "scratch"),
|
||||||
|
sibling("other", "60987654321", "WaBot"),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(r).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("excludes the current account from comparison (a row's own phone isn't a 'duplicate')", () => {
|
||||||
|
// After session-manager.handleEvent runs first it has already
|
||||||
|
// written phone_number on the current row. The check must skip
|
||||||
|
// that row, otherwise EVERY successful pair would match itself
|
||||||
|
// and look like a duplicate.
|
||||||
|
const r = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "self",
|
||||||
|
currentPhoneNumber: "60123456789",
|
||||||
|
siblings: [sibling("self", "60123456789", "Self")],
|
||||||
|
});
|
||||||
|
expect(r).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns null for null/empty/whitespace phone numbers (don't false-positive on unset)", () => {
|
||||||
|
const siblings = [
|
||||||
|
sibling("new", null, "scratch"),
|
||||||
|
sibling("a", null, "Old A"),
|
||||||
|
sibling("b", "", "Old B"),
|
||||||
|
sibling("c", " ", "Old C"),
|
||||||
|
];
|
||||||
|
expect(
|
||||||
|
findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: null,
|
||||||
|
siblings,
|
||||||
|
}),
|
||||||
|
).toBeNull();
|
||||||
|
expect(
|
||||||
|
findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: "",
|
||||||
|
siblings,
|
||||||
|
}),
|
||||||
|
).toBeNull();
|
||||||
|
expect(
|
||||||
|
findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: " ",
|
||||||
|
siblings,
|
||||||
|
}),
|
||||||
|
).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("normalises whitespace on both sides before comparing", () => {
|
||||||
|
const r = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: " 60123456789 ",
|
||||||
|
siblings: [sibling("existing", "60123456789", "Existing")],
|
||||||
|
});
|
||||||
|
expect(r?.existingAccountId).toBe("existing");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("picks the FIRST matching sibling when (somehow) two rows share the phone", () => {
|
||||||
|
// Defensive: this state shouldn't exist in production but the helper
|
||||||
|
// should at least be deterministic so the message is consistent.
|
||||||
|
const r = findDuplicateExistingAccount({
|
||||||
|
currentAccountId: "new",
|
||||||
|
currentPhoneNumber: "60123456789",
|
||||||
|
siblings: [
|
||||||
|
sibling("first", "60123456789", "First"),
|
||||||
|
sibling("second", "60123456789", "Second"),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
expect(r?.existingAccountId).toBe("first");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@ -141,3 +141,45 @@ export function nextWarmingUpAfterEvent(input: {
|
|||||||
if (input.event === "close" && input.restartRequired) return true;
|
if (input.event === "close" && input.restartRequired) return true;
|
||||||
return input.warmingUp;
|
return input.warmingUp;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide whether a freshly-paired account is a duplicate of an
|
||||||
|
* existing account row owned by the same operator. The operator
|
||||||
|
* cannot legitimately link the same WhatsApp number to two account
|
||||||
|
* rows — Baileys keeps one auth blob per phone and the second row
|
||||||
|
* would just hijack the first's session.
|
||||||
|
*
|
||||||
|
* Inputs:
|
||||||
|
* - `currentAccountId` the row that just received the open event
|
||||||
|
* - `currentPhoneNumber` the JID-derived phone string (or null)
|
||||||
|
* - `siblings` every other operator-owned account row
|
||||||
|
*
|
||||||
|
* Returns `null` if the phone is unique (proceed normally), or a
|
||||||
|
* descriptor with the existing-row's id+label so the caller can park
|
||||||
|
* the duplicate row and surface a clear "already linked" message to
|
||||||
|
* the UI. A null/empty phone never reports a duplicate (we'd be
|
||||||
|
* comparing apples and we'd block legitimate first pairs that
|
||||||
|
* haven't received the WID yet).
|
||||||
|
*/
|
||||||
|
export interface DuplicatePairInput {
|
||||||
|
currentAccountId: string;
|
||||||
|
currentPhoneNumber: string | null | undefined;
|
||||||
|
siblings: Array<{ id: string; phoneNumber: string | null; label: string }>;
|
||||||
|
}
|
||||||
|
export interface DuplicatePairFinding {
|
||||||
|
existingAccountId: string;
|
||||||
|
existingLabel: string;
|
||||||
|
}
|
||||||
|
export function findDuplicateExistingAccount(
|
||||||
|
input: DuplicatePairInput,
|
||||||
|
): DuplicatePairFinding | null {
|
||||||
|
const phone = (input.currentPhoneNumber ?? "").trim();
|
||||||
|
if (!phone) return null;
|
||||||
|
for (const s of input.siblings) {
|
||||||
|
if (s.id === input.currentAccountId) continue;
|
||||||
|
if ((s.phoneNumber ?? "").trim() === phone) {
|
||||||
|
return { existingAccountId: s.id, existingLabel: s.label };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|||||||
128
apps/bot/src/ipc/unpair-handler.test.ts
Normal file
128
apps/bot/src/ipc/unpair-handler.test.ts
Normal file
@ -0,0 +1,128 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
|
||||||
|
// Hoisted spies so the vi.mock factories can reach them.
|
||||||
|
const {
|
||||||
|
stopMock,
|
||||||
|
logoutAndStopMock,
|
||||||
|
rmMock,
|
||||||
|
findFirstMock,
|
||||||
|
writeAuditLogMock,
|
||||||
|
pgNotifyWebMock,
|
||||||
|
} = vi.hoisted(() => ({
|
||||||
|
stopMock: vi.fn(async () => undefined),
|
||||||
|
logoutAndStopMock: vi.fn(async () => undefined),
|
||||||
|
rmMock: vi.fn(async () => undefined),
|
||||||
|
findFirstMock: vi.fn(async () => ({ operatorId: "op-1", label: "WaBot" })),
|
||||||
|
writeAuditLogMock: vi.fn(async () => undefined),
|
||||||
|
pgNotifyWebMock: vi.fn(async () => undefined),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("node:fs/promises", () => ({
|
||||||
|
rm: (...args: unknown[]) => rmMock(...args),
|
||||||
|
}));
|
||||||
|
vi.mock("../db.js", () => ({
|
||||||
|
db: {
|
||||||
|
query: { whatsappAccounts: { findFirst: (...a: unknown[]) => findFirstMock(...a) } },
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../env.js", () => ({ env: { SESSIONS_DIR: "/data/sessions" } }));
|
||||||
|
vi.mock("../whatsapp/session-manager.js", () => ({
|
||||||
|
sessionManager: {
|
||||||
|
stop: (...a: unknown[]) => stopMock(...a),
|
||||||
|
logoutAndStop: (...a: unknown[]) => logoutAndStopMock(...a),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
vi.mock("../audit.js", () => ({
|
||||||
|
writeAuditLog: (...a: unknown[]) => writeAuditLogMock(...a),
|
||||||
|
}));
|
||||||
|
vi.mock("./notify.js", () => ({
|
||||||
|
pgNotifyWeb: (...a: unknown[]) => pgNotifyWebMock(...a),
|
||||||
|
}));
|
||||||
|
vi.mock("../logger.js", () => ({
|
||||||
|
logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn() },
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { handleUnpair, handleDelete } from "./unpair-handler.js";
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
stopMock.mockReset();
|
||||||
|
stopMock.mockResolvedValue(undefined);
|
||||||
|
logoutAndStopMock.mockReset();
|
||||||
|
logoutAndStopMock.mockResolvedValue(undefined);
|
||||||
|
rmMock.mockReset();
|
||||||
|
rmMock.mockResolvedValue(undefined);
|
||||||
|
findFirstMock.mockReset();
|
||||||
|
findFirstMock.mockResolvedValue({ operatorId: "op-1", label: "WaBot" });
|
||||||
|
writeAuditLogMock.mockReset();
|
||||||
|
writeAuditLogMock.mockResolvedValue(undefined);
|
||||||
|
pgNotifyWebMock.mockReset();
|
||||||
|
pgNotifyWebMock.mockResolvedValue(undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("handleUnpair", () => {
|
||||||
|
it("stops the session WITHOUT logout, removes files, audits, notifies", async () => {
|
||||||
|
await handleUnpair("acct-A");
|
||||||
|
// The unpair flow MUST NOT call logoutAndStop — that would tell
|
||||||
|
// WhatsApp to drop the linked device, which the operator might
|
||||||
|
// re-pair shortly after. logoutAndStop is only for permanent
|
||||||
|
// delete.
|
||||||
|
expect(logoutAndStopMock).not.toHaveBeenCalled();
|
||||||
|
expect(stopMock).toHaveBeenCalledWith("acct-A", { intentional: true });
|
||||||
|
expect(rmMock).toHaveBeenCalled();
|
||||||
|
expect(writeAuditLogMock).toHaveBeenCalledWith(
|
||||||
|
expect.anything(),
|
||||||
|
expect.objectContaining({ action: "account.unpaired", targetId: "acct-A" }),
|
||||||
|
);
|
||||||
|
expect(pgNotifyWebMock).toHaveBeenCalledWith({
|
||||||
|
type: "session.disconnected",
|
||||||
|
accountId: "acct-A",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("handleDelete (logout-before-teardown)", () => {
|
||||||
|
it("calls logoutAndStop BEFORE rm so WhatsApp drops the linked device first", async () => {
|
||||||
|
await handleDelete("acct-A");
|
||||||
|
expect(logoutAndStopMock).toHaveBeenCalledTimes(1);
|
||||||
|
expect(logoutAndStopMock).toHaveBeenCalledWith("acct-A");
|
||||||
|
expect(rmMock).toHaveBeenCalledTimes(1);
|
||||||
|
// Order: logout-and-stop must invoke before rm (otherwise the
|
||||||
|
// socket was torn down on disk before WhatsApp could be told to
|
||||||
|
// drop the linked device).
|
||||||
|
expect(logoutAndStopMock.mock.invocationCallOrder[0]).toBeLessThan(
|
||||||
|
rmMock.mock.invocationCallOrder[0]!,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT call the plain stop() — that's reserved for unpair", async () => {
|
||||||
|
// Sanity guard: a refactor that swaps logoutAndStop for stop()
|
||||||
|
// would silently regress the linked-device cleanup. The test
|
||||||
|
// pins the contract.
|
||||||
|
await handleDelete("acct-A");
|
||||||
|
expect(stopMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("writes an account.deleted audit log carrying the row's label", async () => {
|
||||||
|
findFirstMock.mockResolvedValueOnce({ operatorId: "op-7", label: "Yiekheng-my" });
|
||||||
|
await handleDelete("acct-X");
|
||||||
|
expect(writeAuditLogMock).toHaveBeenCalledWith(
|
||||||
|
expect.anything(),
|
||||||
|
expect.objectContaining({
|
||||||
|
action: "account.deleted",
|
||||||
|
operatorId: "op-7",
|
||||||
|
targetId: "acct-X",
|
||||||
|
payload: { label: "Yiekheng-my" },
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still completes when the audit-log lookup fails (best-effort)", async () => {
|
||||||
|
// The web action runs the cascade DELETE right after; if the row
|
||||||
|
// is gone before this handler reads it, the audit lookup throws.
|
||||||
|
// Delete must not strand on that.
|
||||||
|
findFirstMock.mockRejectedValueOnce(new Error("no such row"));
|
||||||
|
await expect(handleDelete("acct-A")).resolves.toBeUndefined();
|
||||||
|
expect(rmMock).toHaveBeenCalled();
|
||||||
|
expect(pgNotifyWebMock).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -39,3 +39,41 @@ export async function handleUnpair(accountId: string): Promise<void> {
|
|||||||
}
|
}
|
||||||
await pgNotifyWeb({ type: "session.disconnected", accountId });
|
await pgNotifyWeb({ type: "session.disconnected", accountId });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Delete-account flow on the bot side. Distinct from unpair because
|
||||||
|
* we want WhatsApp to drop this device from the user's linked-devices
|
||||||
|
* list — otherwise the phone keeps showing a phantom entry that has
|
||||||
|
* to be manually removed from WhatsApp's UI.
|
||||||
|
*
|
||||||
|
* Order is important:
|
||||||
|
* 1. socket.logout() over the still-connected socket → WhatsApp
|
||||||
|
* removes the linked device on the server side.
|
||||||
|
* 2. close() the local Baileys session.
|
||||||
|
* 3. rm() the on-disk auth blob so the next pairing starts clean.
|
||||||
|
*
|
||||||
|
* Step 1 is best-effort — if the socket is already torn down or the
|
||||||
|
* RPC fails the delete still proceeds. The web action then deletes
|
||||||
|
* the row (cascade FKs handle groups/reminders/runs).
|
||||||
|
*/
|
||||||
|
export async function handleDelete(accountId: string): Promise<void> {
|
||||||
|
await sessionManager.logoutAndStop(accountId);
|
||||||
|
await rm(join(env.SESSIONS_DIR, accountId), { recursive: true, force: true });
|
||||||
|
try {
|
||||||
|
const row = await db.query.whatsappAccounts.findFirst({
|
||||||
|
where: (a, { eq }) => eq(a.id, accountId),
|
||||||
|
columns: { operatorId: true, label: true },
|
||||||
|
});
|
||||||
|
await writeAuditLog(db, {
|
||||||
|
operatorId: row?.operatorId ?? null,
|
||||||
|
source: "web",
|
||||||
|
action: "account.deleted",
|
||||||
|
targetType: "whatsapp_account",
|
||||||
|
targetId: accountId,
|
||||||
|
payload: { label: row?.label ?? null },
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
logger.warn({ err, accountId }, "delete: audit log failed (non-fatal)");
|
||||||
|
}
|
||||||
|
await pgNotifyWeb({ type: "session.disconnected", accountId });
|
||||||
|
}
|
||||||
|
|||||||
@ -120,6 +120,44 @@ class SessionManager {
|
|||||||
this.sessions.delete(accountId);
|
this.sessions.delete(accountId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tell WhatsApp to remove this device from the linked-devices list,
|
||||||
|
* then close the socket. Used by the delete-account flow so the
|
||||||
|
* operator's phone doesn't keep showing a phantom "linked device"
|
||||||
|
* pointing at a row that no longer exists. Best-effort: if the
|
||||||
|
* socket is already torn down or the logout RPC fails (network
|
||||||
|
* blip, already-disconnected, etc.) we still proceed to close +
|
||||||
|
* teardown — no point stranding the delete because WhatsApp didn't
|
||||||
|
* acknowledge.
|
||||||
|
*/
|
||||||
|
async logoutAndStop(accountId: string): Promise<void> {
|
||||||
|
const timer = this.reconnectTimers.get(accountId);
|
||||||
|
if (timer) {
|
||||||
|
clearTimeout(timer);
|
||||||
|
this.reconnectTimers.delete(accountId);
|
||||||
|
}
|
||||||
|
const session = this.sessions.get(accountId);
|
||||||
|
if (!session) return;
|
||||||
|
// Suppress reconnect/handleEvent bookkeeping for the close that
|
||||||
|
// logout() emits — the row is about to be deleted entirely so
|
||||||
|
// status writes are pointless.
|
||||||
|
this.intentionalStops.add(accountId);
|
||||||
|
try {
|
||||||
|
await session.socket.logout();
|
||||||
|
} catch (err) {
|
||||||
|
logger.warn(
|
||||||
|
{ err, accountId },
|
||||||
|
"session-manager: socket.logout() failed (continuing with teardown)",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
await session.close();
|
||||||
|
} catch (err) {
|
||||||
|
logger.warn({ err, accountId }, "session-manager: post-logout close failed");
|
||||||
|
}
|
||||||
|
this.sessions.delete(accountId);
|
||||||
|
}
|
||||||
|
|
||||||
async stopAll(): Promise<void> {
|
async stopAll(): Promise<void> {
|
||||||
await Promise.all([...this.sessions.keys()].map((id) => this.stop(id)));
|
await Promise.all([...this.sessions.keys()].map((id) => this.stop(id)));
|
||||||
}
|
}
|
||||||
|
|||||||
@ -193,8 +193,12 @@ export async function deleteAccountAction(formData: FormData): Promise<void> {
|
|||||||
where: (a, { eq, and }) => and(eq(a.id, accountId), eq(a.operatorId, op.id)),
|
where: (a, { eq, and }) => and(eq(a.id, accountId), eq(a.operatorId, op.id)),
|
||||||
});
|
});
|
||||||
if (!account) return;
|
if (!account) return;
|
||||||
// Stop any live session / clean session files first.
|
// Tell the bot to logout() over the live socket FIRST (so WhatsApp
|
||||||
await pgNotifyBot({ type: "account.unpair", accountId });
|
// drops this device from the operator's linked-devices list), then
|
||||||
|
// close + remove session files. Distinct from account.unpair which
|
||||||
|
// never calls logout — keeping linked-devices clean is specific to
|
||||||
|
// the delete flow.
|
||||||
|
await pgNotifyBot({ type: "account.delete", accountId });
|
||||||
// Cascade FKs handle groups, reminders, runs, run_targets, messages.
|
// Cascade FKs handle groups, reminders, runs, run_targets, messages.
|
||||||
await db.delete(whatsappAccounts).where(eq(whatsappAccounts.id, accountId));
|
await db.delete(whatsappAccounts).where(eq(whatsappAccounts.id, accountId));
|
||||||
revalidatePath("/accounts");
|
revalidatePath("/accounts");
|
||||||
|
|||||||
@ -18,7 +18,8 @@ type PairingState =
|
|||||||
| { phase: "waiting" }
|
| { phase: "waiting" }
|
||||||
| { phase: "qr"; qrUrl: string }
|
| { phase: "qr"; qrUrl: string }
|
||||||
| { phase: "connected"; phoneNumber: string }
|
| { phase: "connected"; phoneNumber: string }
|
||||||
| { phase: "timeout" };
|
| { phase: "timeout" }
|
||||||
|
| { phase: "duplicate"; phoneNumber: string; existingLabel: string };
|
||||||
|
|
||||||
interface PairLiveProps {
|
interface PairLiveProps {
|
||||||
accountId: string;
|
accountId: string;
|
||||||
@ -112,6 +113,15 @@ export function PairLive({ accountId, label }: PairLiveProps) {
|
|||||||
if (timerRef.current) clearInterval(timerRef.current);
|
if (timerRef.current) clearInterval(timerRef.current);
|
||||||
setPairingState({ phase: "timeout" });
|
setPairingState({ phase: "timeout" });
|
||||||
},
|
},
|
||||||
|
"session.duplicate": (data) => {
|
||||||
|
if (data.accountId !== accountId) return;
|
||||||
|
if (timerRef.current) clearInterval(timerRef.current);
|
||||||
|
setPairingState({
|
||||||
|
phase: "duplicate",
|
||||||
|
phoneNumber: data.phoneNumber,
|
||||||
|
existingLabel: data.existingLabel,
|
||||||
|
});
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
// Auto-redirect on connected
|
// Auto-redirect on connected
|
||||||
@ -234,6 +244,35 @@ export function PairLive({ accountId, label }: PairLiveProps) {
|
|||||||
</Button>
|
</Button>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{pairingState.phase === "duplicate" && (
|
||||||
|
<div className="flex flex-col items-center gap-4 text-center">
|
||||||
|
<div className="flex size-16 items-center justify-center rounded-full bg-amber-500/15">
|
||||||
|
<XCircleIcon className="size-8 text-amber-600 dark:text-amber-400" />
|
||||||
|
</div>
|
||||||
|
<div className="space-y-1">
|
||||||
|
<p className="text-base font-semibold">Phone already linked</p>
|
||||||
|
<p className="text-xs text-muted-foreground">
|
||||||
|
<span className="font-mono">
|
||||||
|
+{pairingState.phoneNumber.replace(/^\+/, "")}
|
||||||
|
</span>{" "}
|
||||||
|
is already paired to{" "}
|
||||||
|
<span className="font-medium text-foreground">
|
||||||
|
{pairingState.existingLabel}
|
||||||
|
</span>
|
||||||
|
. Each WhatsApp number can only be linked to one account here.
|
||||||
|
Unpair the existing account first, or scan with a different
|
||||||
|
phone.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
<Button asChild size="sm">
|
||||||
|
{/* eslint-disable-next-line @typescript-eslint/no-explicit-any */}
|
||||||
|
<Link href={`/accounts/${accountId}` as any}>
|
||||||
|
Back to accounts
|
||||||
|
</Link>
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -9,6 +9,11 @@ export type WebEventMap = {
|
|||||||
"session.connected": { accountId: string; phoneNumber: string | null };
|
"session.connected": { accountId: string; phoneNumber: string | null };
|
||||||
"session.disconnected": { accountId: string };
|
"session.disconnected": { accountId: string };
|
||||||
"session.timeout": { accountId: string };
|
"session.timeout": { accountId: string };
|
||||||
|
"session.duplicate": {
|
||||||
|
accountId: string;
|
||||||
|
phoneNumber: string;
|
||||||
|
existingLabel: string;
|
||||||
|
};
|
||||||
"groups.synced": { accountId: string; count: number };
|
"groups.synced": { accountId: string; count: number };
|
||||||
"reminder.fired": {
|
"reminder.fired": {
|
||||||
reminderId: string;
|
reminderId: string;
|
||||||
|
|||||||
49
apps/web/src/lib/list-accounts-order.test.ts
Normal file
49
apps/web/src/lib/list-accounts-order.test.ts
Normal file
@ -0,0 +1,49 @@
|
|||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { readFileSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Static guard: listAccounts() in apps/web/src/lib/queries.ts MUST
|
||||||
|
* order rows by createdAt ascending (with id as a deterministic
|
||||||
|
* tiebreaker) so the operator's earliest-added account stays on top.
|
||||||
|
*
|
||||||
|
* Earlier the orderBy used `asc(a.label)` which silently re-shuffled
|
||||||
|
* the list every time an account was renamed. This test pins the
|
||||||
|
* fix in source so a future refactor can't quietly bring the rename
|
||||||
|
* regression back.
|
||||||
|
*
|
||||||
|
* It's a static (regex) guard rather than an integration test
|
||||||
|
* because the live query needs Postgres + a seeded operator;
|
||||||
|
* pinning the source spelling keeps coverage cheap and CI-friendly.
|
||||||
|
*/
|
||||||
|
describe("listAccounts ordering (regression guard)", () => {
|
||||||
|
const src = readFileSync(
|
||||||
|
join(__dirname, "queries.ts"),
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
|
||||||
|
it("orders by created_at ASC", () => {
|
||||||
|
// Match across whitespace/comments inside listAccounts. Anchors:
|
||||||
|
// function header → orderBy → asc(a.createdAt).
|
||||||
|
const fnStart = src.indexOf("export async function listAccounts(");
|
||||||
|
expect(fnStart).toBeGreaterThan(-1);
|
||||||
|
const fnEnd = src.indexOf("export async function ", fnStart + 1);
|
||||||
|
const fnBody = src.slice(fnStart, fnEnd > -1 ? fnEnd : undefined);
|
||||||
|
expect(fnBody).toMatch(/orderBy:[\s\S]*asc\(a\.createdAt\)/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses id as a deterministic tiebreaker for ties on createdAt", () => {
|
||||||
|
const fnStart = src.indexOf("export async function listAccounts(");
|
||||||
|
const fnEnd = src.indexOf("export async function ", fnStart + 1);
|
||||||
|
const fnBody = src.slice(fnStart, fnEnd > -1 ? fnEnd : undefined);
|
||||||
|
expect(fnBody).toMatch(/asc\(a\.id\)/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT order by label (the regression we're guarding against)", () => {
|
||||||
|
const fnStart = src.indexOf("export async function listAccounts(");
|
||||||
|
const fnEnd = src.indexOf("export async function ", fnStart + 1);
|
||||||
|
const fnBody = src.slice(fnStart, fnEnd > -1 ? fnEnd : undefined);
|
||||||
|
expect(fnBody).not.toMatch(/asc\(a\.label\)/);
|
||||||
|
expect(fnBody).not.toMatch(/desc\(a\.label\)/);
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -5,6 +5,10 @@ import { db } from "./db";
|
|||||||
export type BotCommand =
|
export type BotCommand =
|
||||||
| { type: "account.start_pairing"; accountId: string }
|
| { type: "account.start_pairing"; accountId: string }
|
||||||
| { type: "account.unpair"; accountId: string }
|
| { type: "account.unpair"; accountId: string }
|
||||||
|
// Like account.unpair, but the bot also calls socket.logout() so
|
||||||
|
// WhatsApp drops this device from the operator's linked-devices
|
||||||
|
// list before the row is deleted.
|
||||||
|
| { type: "account.delete"; accountId: string }
|
||||||
| { type: "account.sync_groups"; accountId: string }
|
| { type: "account.sync_groups"; accountId: string }
|
||||||
| { type: "group.send_test"; groupId: string; text: string }
|
| { type: "group.send_test"; groupId: string; text: string }
|
||||||
| { type: "reminder.schedule"; reminderId: string; scheduledAtIso: string }
|
| { type: "reminder.schedule"; reminderId: string; scheduledAtIso: string }
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user