From 9e5220cf2c731b96bf9bb5fba85b29263e2f0c07 Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Mon, 3 Aug 2026 13:57:53 -0700
Subject: [PATCH] fix(signal): normalize accounts during noninteractive setup
 (#118932)

---
 extensions/signal/src/setup-core.test.ts | 72 ++++++++++++++++++++++++
 extensions/signal/src/setup-core.ts      |  6 +-
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/extensions/signal/src/setup-core.test.ts b/extensions/signal/src/setup-core.test.ts
index 4a4f4e1752db..297d60b4ba44 100644
--- a/extensions/signal/src/setup-core.test.ts
+++ b/extensions/signal/src/setup-core.test.ts
@@ -106,6 +106,41 @@ describe("signalSetupAdapter", () => {
     expect(detectSignalTransportMock).not.toHaveBeenCalled();
   });
 
+  it.each([
+    {
+      accountId: "default",
+      signalNumber: " +1 (555) 555-0123 ",
+      expectedAccount: "+15555550123",
+    },
+    {
+      accountId: "work",
+      signalNumber: "signal: +1 (555) 555-0124",
+      expectedAccount: "+15555550124",
+    },
+    { accountId: "default", signalNumber: "15555550125", expectedAccount: "+15555550125" },
+    { accountId: "work", signalNumber: "+12345", expectedAccount: "+12345" },
+    {
+      accountId: "default",
+      signalNumber: "+123456789012345",
+      expectedAccount: "+123456789012345",
+    },
+  ])(
+    "stores the canonical Signal number for the $accountId account",
+    ({ accountId, signalNumber, expectedAccount }) => {
+      const next = signalSetupAdapter.applyAccountConfig?.({
+        cfg: {},
+        accountId,
+        input: { signalNumber },
+      });
+      const account =
+        accountId === "default"
+          ? next?.channels?.signal?.account
+          : next?.channels?.signal?.accounts?.[accountId]?.account;
+
+      expect(account).toBe(expectedAccount);
+    },
+  );
+
   it("restores a generically promoted default account before writing a named account", () => {
     const next = signalSetupAdapter.applyAccountConfig?.({
       cfg: {
@@ -427,6 +462,19 @@ describe("signalSetupAdapter", () => {
     expect(next?.channels?.signal?.accounts?.Default).not.toHaveProperty("transport");
   });
 
+  it.each(["abc", "++12345", "+1+2345", "+1234", "+1234567890123456", "   ", ""])(
+    "rejects invalid Signal account number %s",
+    (signalNumber) => {
+      expect(
+        signalSetupAdapter.validateInput?.({
+          cfg: {},
+          accountId: "work",
+          input: { signalNumber },
+        }),
+      ).toBe("Invalid E.164 phone number (must start with + and country code, e.g. +15555550123)");
+    },
+  );
+
   it.each(["0", "abc", "65536"])("rejects invalid managed HTTP port %s", (httpPort) => {
     expect(
       signalSetupAdapter.validateInput?.({
@@ -473,6 +521,30 @@ describe("signalSetupAdapter", () => {
     ).toBe("Signal container transport requires --signal-number or an existing account.");
   });
 
+  it("rejects an invalid replacement without overwriting an existing container account", () => {
+    const cfg: OpenClawConfig = {
+      channels: {
+        signal: {
+          account: "+15555550123",
+          transport: { kind: "container", url: "http://signal-container:8080" },
+        },
+      },
+    };
+    const input = {
+      signalNumber: "abc",
+      signalTransport: "container" as const,
+      httpUrl: "http://signal-container:8080",
+    };
+
+    expect(signalSetupAdapter.validateInput?.({ cfg, accountId: "default", input })).toBe(
+      "Invalid E.164 phone number (must start with + and country code, e.g. +15555550123)",
+    );
+    expect(
+      signalSetupAdapter.applyAccountConfig?.({ cfg, accountId: "default", input })?.channels
+        ?.signal?.account,
+    ).toBe("+15555550123");
+  });
+
   it("allows a container transport to reuse the configured Signal account", () => {
     expect(
       signalSetupAdapter.validateInput?.({
diff --git a/extensions/signal/src/setup-core.ts b/extensions/signal/src/setup-core.ts
index a09a06a8ad3a..faac51661913 100644
--- a/extensions/signal/src/setup-core.ts
+++ b/extensions/signal/src/setup-core.ts
@@ -134,6 +134,7 @@ function parseSignalAllowFromEntries(raw: string): { entries: string[]; error?:
 }
 
 export function buildSignalSetupPatch(input: SignalSetupInput) {
+  const account = normalizeSignalAccountInput(input.signalNumber);
   const transport = input.httpUrl
     ? {
         // Bare --http-url is classified once by prepareAccountConfigInput. Keep the historical
@@ -150,7 +151,7 @@ export function buildSignalSetupPatch(input: SignalSetupInput) {
         }
       : undefined;
   return {
-    ...(input.signalNumber ? { account: input.signalNumber } : {}),
+    ...(account ? { account } : {}),
     ...(transport ? { transport } : {}),
   };
 }
@@ -332,6 +333,9 @@ const signalSetupAdapterBase = createPatchedAccountSetupAdapter<SignalSetupInput
           return "Signal --http-host must be a hostname or IP address.";
         }
       }
+      if (input.signalNumber !== undefined && !normalizeSignalAccountInput(input.signalNumber)) {
+        return INVALID_SIGNAL_ACCOUNT_ERROR;
+      }
       if (
         input.signalTransport === "container" &&
         !normalizeSignalAccountInput(input.signalNumber) &&