Verify (don't order) phone number; fix DECT location lookup endpoint

Two bugs surfaced by the same failed finalize run for store 7311.

1. addPhoneNumbersToLocation was POSTing to /locations/{id}/numbers,
   which Webex treats as "provision a NEW number" — that triggered
   duplicate PSTN orders against numbers AE had already manually
   ordered through Control Hub, and the provider rejected them with
   ORDER_CREATION_FAILED. AE's workflow is manual ordering, so this
   step now VERIFIES the number's current state in the org via
   GET /telephony/config/numbers?phoneNumber=..., classifies the
   outcome (already here / pending / wrong location / not in org),
   and either succeeds silently or throws with a specific
   Control-Hub fix-it instruction. The bot never triggers a PSTN
   order now.

2. findDectNetworkInLocation was hitting
   GET /telephony/config/locations/{id}/dectNetworks, which Webex
   does not implement (returns HTTP 404 "No static resource ..."
   unconditionally). That silently broke both the finalize
   idempotency pre-check and the /provisionDect fallback, so
   /provisionDect kept showing the "create network" card for stores
   that already had one, then 409'd on the create attempt. Switched
   to the real endpoint GET /telephony/config/dectNetworks with a
   locationId filter. Also added translateCreateDectError to
   rewrite the deeply-nested 27453 "access code in use" 409 blob
   into an actionable "already exists, re-run /provisionDect"
   message.

Extracted both error/state translators as pure exported functions
(classifyNumberAssignment, translateCreateDectError) with unit-test
coverage locking down the exact Webex payload shapes.

README: updated /finalizeStore to reflect the manual-order
expectation and specific fix-it instructions on mismatch.

Co-authored-by: Cursor <cursoragent@cursor.com>

README.md

@@ -132,11 +132,15 @@ Registered in [src/commands](src/commands):
   and attaches the Webex Calling license to the store user. Everything
   except the phone number, which has to be purchased separately in Control Hub.
 - `/finalizeStore <storeNumber>` — cut-over for a previously staged store:
-  attaches the purchased phone number, sets caller ID, creates the
-  `Store XXXX` DECT network (DBS-210) with the per-store default access
-  code, creates the auto-attendant, and finalizes user licensing.
-  Preflights that the store was actually staged first, and is idempotent
-  on the DECT network (skips if one already exists).
+  verifies the store's phone number is already assigned to the location
+  (it must be manually ordered in Control Hub first — this step never
+  triggers a new PSTN order), sets caller ID, creates the `Store XXXX`
+  DECT network (DBS-210) with the per-store default access code, creates
+  the auto-attendant, and finalizes user licensing. Preflights that the
+  store was actually staged first, and is idempotent on the DECT network
+  (skips if one already exists). On phone-number mismatches (pending
+  order, wrong location, or number missing from the org) finalize aborts
+  early with a specific Control-Hub fix-it instruction.
 - `/provisionDect <storeNumber>` — manage DECT phones for a store: card
   for adding basestations by MAC and adding/removing handsets. Every
   removal goes through an explicit confirmation card. New stores have

src/flows/finalizeStore.js

@@ -44,12 +44,15 @@ export async function finalizeStoreLocation(bot, locationInfo, userInfo) {
 
     // Critical: every downstream step (calling identity, per-user
     // caller ID = LOCATION_NUMBER, auto attendant on this DID) needs
-    // the number attached to the location. If this fails there is no
-    // point cascading the rest and burying the root cause in noise —
-    // bail early so the actionable Webex message reaches the operator.
+    // the number attached to the location. The AE workflow expects
+    // this to already be true — the number is manually ordered in
+    // Control Hub before finalize runs — so this step just VERIFIES
+    // the assignment (never re-orders). On mismatch it errors with a
+    // specific Control-Hub instruction rather than cascading the
+    // failure into three unrelated 400s downstream.
     await runStep(
         bot,
-        'Added phone numbers to location',
+        'Verified phone number is assigned to location',
         () => addPhoneNumbersToLocation(location, locationInfo.phoneNumber),
         { critical: true },
     );
@@ -113,10 +116,12 @@ async function ensureDectNetwork(bot, location, storeNumber) {
             return existing;
         }
     } catch (error) {
-        // 404s are handled inside findDectNetworkInLocation (return
-        // null), so anything reaching us here is a real problem worth
-        // flagging in the log. We still fall through to try creating,
-        // since a transient read failure shouldn't block finalize.
+        // findDectNetworkInLocation talks to the org-wide DECT list
+        // endpoint, so any error here is a real problem (auth, 5xx,
+        // etc.) worth flagging in the log. We still fall through to
+        // try creating: createDectNetwork translates the 409 "access
+        // code already in use" case into a clear "already exists"
+        // outcome, so a transient read failure never blocks finalize.
         logger.warn(
             `DECT network pre-check failed for ${netName} (status=${error?.status ?? '?'}): ${error?.message}`,
         );

src/webex/dect.js

@@ -107,25 +107,25 @@ export async function findDectNetworkForStore(storeNumber) {
 
 /**
  * Location-scoped DECT network lookup. Unlike findDectNetworkForStore
- * this doesn't rely on the store user having lines on the network, so it
- * safely finds freshly-created empty networks (e.g. right after finalize
- * creates one and before any handsets are added). Returns the raw match
- * or null.
+ * this doesn't rely on the store user having lines on the network, so
+ * it safely finds freshly-created empty networks (e.g. right after
+ * finalize creates one and before any handsets are added). Returns
+ * the raw match or null.
+ *
+ * We use the org-wide GET /telephony/config/dectNetworks endpoint
+ * with a locationId filter — NOT the per-location
+ * GET /telephony/config/locations/{id}/dectNetworks path. The latter
+ * looks natural but Webex has no such route: it returns HTTP 404
+ * "No static resource ..." unconditionally, whether the location has
+ * DECT networks or not. That silently broke the finalize
+ * idempotency pre-check AND the /provisionDect fallback, causing
+ * /provisionDect to show the "create network" card for stores that
+ * already had one and then 409 on the create attempt.
  */
 export async function findDectNetworkInLocation(locationId, storeNumber) {
     if (!locationId) return null;
-    let data;
-    try {
-        data = await webexJson('GET', `/telephony/config/locations/${locationId}/dectNetworks`);
-    } catch (err) {
-        // Webex returns 404 ("No static resource ... /dectNetworks")
-        // when a location has never had a DECT network — as opposed
-        // to returning an empty list. Treat that as "no networks
-        // yet" so the caller can proceed to create one without a
-        // spurious warning on the finalize output.
-        if (err?.status === 404) return null;
-        throw err;
-    }
+    const encoded = encodeURIComponent(locationId);
+    const data = await webexJson('GET', `/telephony/config/dectNetworks?locationId=${encoded}`);
     const items = data?.dectNetworks ?? data?.items ?? [];
     const target = dectNetworkName(storeNumber).toLowerCase();
     return items.find((n) => (n.name ?? '').trim().toLowerCase() === target) ?? null;
@@ -233,13 +233,43 @@ export async function getDectProvisioningStatus(storeNumber) {
 // Mutating helpers
 
 /**
- * Create a multi-cell DECT network for this store's location. Uses the
- * "Store <4-digit>" name convention. Access code defaults to the
- * generateDectAccessCode(storeNumber) rule but can be overridden by the
- * caller (the /provisionDect create card lets the operator edit it).
+ * Rewrite the raw Webex error from POST /locations/.../dectNetworks
+ * into something an operator can act on for the known "already
+ * exists" failure. Pure + exported so it can be unit-tested.
  *
- * Idempotency is the caller's responsibility — Webex will 409 if a network
- * with the same name already exists in this location.
+ * Webex code 27453 ("Default access code is in use by another DECT
+ * network: <code>") is the concrete symptom: our access code is
+ * derived from the store number, so a duplicate almost always means
+ * the store's own DECT network already exists. The right operator
+ * action is to re-run /provisionDect (which will now find it via
+ * the org-wide lookup) — not to pick a different access code.
+ */
+export function translateCreateDectError(err, storeNumber) {
+    if (typeof err?.body === 'string' && err.body.includes("'27453'")) {
+        const clearer = new Error(
+            `DECT network for Store ${String(storeNumber).padStart(4, '0')} ` +
+                'already exists (default access code is in use). Re-run ' +
+                '`/provisionDect ' +
+                storeNumber +
+                '` to open the management card.',
+        );
+        clearer.cause = err;
+        clearer.status = err.status;
+        return clearer;
+    }
+    return err;
+}
+
+/**
+ * Create a multi-cell DECT network for this store's location. Uses
+ * the "Store <4-digit>" name convention. Access code defaults to the
+ * generateDectAccessCode(storeNumber) rule but can be overridden by
+ * the caller (the /provisionDect create card lets the operator edit
+ * it).
+ *
+ * Idempotency is the caller's responsibility, but we do translate the
+ * 409 "access code in use" error into a clearer message pointing at
+ * the fix (re-run /provisionDect to see the management card).
  */
 export async function createDectNetwork(locationId, storeNumber, { defaultAccessCode } = {}) {
     if (!locationId) throw new Error('locationId required');
@@ -253,11 +283,16 @@ export async function createDectNetwork(locationId, storeNumber, { defaultAccess
         defaultAccessCodeEnabled: true,
         defaultAccessCode: code,
     };
-    const res = await webexJson(
+    let res;
+    try {
+        res = await webexJson(
             'POST',
             `/telephony/config/locations/${locationId}/dectNetworks`,
             body,
         );
+    } catch (err) {
+        throw translateCreateDectError(err, storeNumber);
+    }
     logger.info(`Created DECT network ${name} (id=${res?.dectNetworkId})`);
     return { id: res?.dectNetworkId, name, locationId };
 }

src/webex/locations.js

@@ -1,4 +1,5 @@
 import { webexJson } from './client.js';
+import { logger } from '../logger.js';
 import {
     CALLER_ID,
     MUSIC_ON_HOLD,
@@ -84,16 +85,109 @@ export function translateAddNumberError(err, phoneNumber) {
     return err;
 }
 
+/**
+ * Look up a single phone number in the org's inventory. Returns the
+ * first matching record from Webex's /telephony/config/numbers list
+ * (which includes assignment / state / location info), or null when
+ * the number is not in the org at all.
+ *
+ * We compare against the org-wide listing rather than a
+ * per-location endpoint because we need to distinguish between
+ *   (a) already in this location   → skip, already done
+ *   (b) in a different location     → operator needs to move it
+ *   (c) not in the org yet          → operator needs to order it
+ * and only (a) is safe to proceed from silently.
+ */
+export async function findPhoneNumber(phoneNumber) {
+    const encoded = encodeURIComponent(phoneNumber);
+    const data = await webexJson('GET', `/telephony/config/numbers?phoneNumber=${encoded}`);
+    const items = data?.phoneNumbers ?? data?.items ?? [];
+    return items[0] ?? null;
+}
+
+/**
+ * Classify what we found in the org against the location we're
+ * finalizing, and return a { kind, message?, ... } discriminator the
+ * caller can act on. Exported as a pure function so the branching
+ * logic is unit-testable without any HTTP.
+ *
+ * kinds:
+ *   'already-here'  - number is ACTIVE in this location; nothing to do.
+ *   'pending'       - number is in the org but not yet ACTIVE. Wait.
+ *   'wrong-location'- number is ACTIVE in a different location; move it.
+ *   'not-in-org'    - number isn't in the org at all; order it first.
+ */
+export function classifyNumberAssignment(existing, phoneNumber, location) {
+    if (!existing) {
+        return {
+            kind: 'not-in-org',
+            message:
+                `Phone number ${phoneNumber} is not in the Webex org's number inventory. ` +
+                'Order it in Control Hub (Calling → Numbers → Add), wait for the order ' +
+                'to complete against location "' +
+                location.name +
+                '", then re-run /finalizeStore.',
+        };
+    }
+    const state = String(existing.state ?? '').toUpperCase();
+    const existingLocationId = existing.location?.id;
+    if (existingLocationId === location.id && state === 'ACTIVE') {
+        return { kind: 'already-here', existing };
+    }
+    if (state && state !== 'ACTIVE') {
+        return {
+            kind: 'pending',
+            message:
+                `Phone number ${phoneNumber} is in the org but not yet ACTIVE ` +
+                `(state=${state}). The carrier order is still processing — wait a ` +
+                'few minutes for it to complete, then re-run /finalizeStore.',
+        };
+    }
+    // ACTIVE but in a different (or missing) location.
+    const currentLoc = existing.location?.name ?? '(unassigned)';
+    return {
+        kind: 'wrong-location',
+        message:
+            `Phone number ${phoneNumber} is ACTIVE in the Webex org but assigned to ` +
+            `"${currentLoc}", not "${location.name}". Move it in Control Hub ` +
+            '(Calling → Numbers → the number → Actions → Move) so it lives on the ' +
+            'store location, then re-run /finalizeStore.',
+    };
+}
+
+/**
+ * Ensure the given phone number is ACTIVE on the given location. This
+ * used to POST to /locations/{id}/numbers, which the Webex API treats
+ * as "provision a NEW number" — that triggered duplicate PSTN orders
+ * against numbers AE had already manually ordered through Control
+ * Hub, and the provider rejected them with ORDER_CREATION_FAILED.
+ *
+ * The AE workflow is: manually order the number in Control Hub
+ * (assigning it to the store's location during the wizard), then run
+ * /finalizeStore. All this helper needs to do here is verify that the
+ * number is already assigned to the location — if so, the very next
+ * step (updateLocationCallingIdentity) will select it as the main
+ * number. If not, we give the operator a concrete instruction for
+ * what to fix in Control Hub instead of triggering a bad order.
+ */
 export async function addPhoneNumbersToLocation(location, phoneNumber) {
-    const body = { phoneNumbers: [phoneNumber], state: 'ACTIVE' };
-    try {
-        return await webexJson(
-            'POST',
-            `/telephony/config/locations/${encodeURIComponent(location.id)}/numbers`,
-            body,
+    const existing = await findPhoneNumber(phoneNumber);
+    const outcome = classifyNumberAssignment(existing, phoneNumber, location);
+    switch (outcome.kind) {
+        case 'already-here':
+            logger.info(
+                `${phoneNumber} already ACTIVE on location ${location.name} — skipping add`,
             );
-    } catch (err) {
-        throw translateAddNumberError(err, phoneNumber);
+            return { alreadyAssigned: true, existing: outcome.existing };
+        case 'pending':
+        case 'wrong-location':
+        case 'not-in-org':
+            throw new Error(outcome.message);
+        default:
+            // Defensive: classifyNumberAssignment should be total, but
+            // if a new kind is added and forgotten here, don't silently
+            // pretend everything's fine.
+            throw new Error(`Unhandled number-assignment outcome: ${outcome.kind}`);
     }
 }
 

test/dect.test.js

@@ -1,7 +1,11 @@
 import { describe, it } from 'node:test';
 import assert from 'node:assert/strict';
 
-import { dectNetworkName, generateDectAccessCode } from '../src/webex/dect.js';
+import {
+    dectNetworkName,
+    generateDectAccessCode,
+    translateCreateDectError,
+} from '../src/webex/dect.js';
 
 // AE's access code convention: 4-digit stores use their own digits; anything
 // shorter prefixes '8' and pads the rest to 3. Ports collabFinder's rule
@@ -36,3 +40,44 @@ describe('dectNetworkName', () => {
         assert.equal(dectNetworkName('67'), 'Store 0067');
     });
 });
+
+// Regression: when /provisionDect's location lookup was broken (see
+// findDectNetworkInLocation notes) the create card would appear even
+// though a DECT network already existed, and the resulting POST
+// 409'd with a deeply-nested Webex error blob. The translator
+// surfaces the actionable "re-run /provisionDect" instruction.
+describe('translateCreateDectError', () => {
+    function fakeErr(status, body) {
+        const err = new Error(`HTTP ${status} for POST /whatever\n${body}`);
+        err.status = status;
+        err.body = body;
+        return err;
+    }
+
+    const CONFLICT_BODY =
+        '{"error":[{"key":"409","message":[{"description":"POST failed: HTTP/1.1 409 Conflict (url = https://cpapi-a.wbx2.com/api/v1/customers/DUMMY/locations/DUMMY/dects, request/response TrackingId = ROUTERGW_xxx, errorCode = \'27453\', error = \'[Error 27453] Default access code is in use by another DECT network: 7311\')","code":"27453"}]}],"trackingId":"ROUTERGW_xxx"}';
+
+    it('rewrites Webex error 27453 into an actionable message', () => {
+        const raw = fakeErr(409, CONFLICT_BODY);
+        const translated = translateCreateDectError(raw, 7311);
+        assert.notEqual(translated, raw, 'should be a new Error instance');
+        assert.match(translated.message, /already exists/i);
+        assert.match(translated.message, /\/provisionDect 7311/);
+        assert.match(translated.message, /Store 7311/);
+        assert.equal(translated.status, 409);
+        assert.equal(translated.cause, raw);
+    });
+
+    it('pads short store numbers in the error text (matches dectNetworkName convention)', () => {
+        const raw = fakeErr(409, CONFLICT_BODY);
+        const translated = translateCreateDectError(raw, 499);
+        assert.match(translated.message, /Store 0499/);
+    });
+
+    it('returns unrelated errors unchanged', () => {
+        const raw = fakeErr(400, '{"message":"something else","errorCode":9999}');
+        const result = translateCreateDectError(raw, 7311);
+        assert.equal(result, raw);
+        assert.doesNotMatch(result.message, /already exists/i);
+    });
+});

test/locations.test.js

@@ -1,7 +1,9 @@
 import { describe, it } from 'node:test';
 import assert from 'node:assert/strict';
 
-import { translateAddNumberError } from '../src/webex/locations.js';
+import { classifyNumberAssignment, translateAddNumberError } from '../src/webex/locations.js';
+
+const LOC = { id: 'LOC-STORE-7311', name: 'Store 7311' };
 
 // The exact JSON Webex returns when a freshly-purchased number is
 // still being provisioned by the carrier. Kept verbatim from a real
@@ -18,11 +20,6 @@ function fakeHttpError(status, body) {
 }
 
 describe('translateAddNumberError', () => {
-    // Regression: without the translation, the raw Webex error was
-    // propagated through runStep, cascaded into a Caller-ID failure
-    // (LOCATION_NUMBER unavailable) and an auto-attendant failure
-    // (number "used in another location"), and the actionable root
-    // cause was buried under three stack traces.
     it('rewrites NUMBER_HAS_PENDING_ORDERS into an actionable operator message', () => {
         const raw = fakeHttpError(400, PENDING_ORDERS_BODY);
         const translated = translateAddNumberError(raw, '+14045551212');
@@ -34,26 +31,76 @@ describe('translateAddNumberError', () => {
         assert.equal(translated.cause, raw, 'raw error should be preserved as .cause');
     });
 
-    it('returns unrelated errors unchanged (same instance, no message rewrite)', () => {
+    it('returns unrelated errors unchanged (same instance)', () => {
         const raw = fakeHttpError(400, '{"message":"Nope","errorCode":9999}');
         const result = translateAddNumberError(raw, '+14045551212');
-        assert.equal(result, raw, 'must be same instance so callers can rethrow');
+        assert.equal(result, raw);
         assert.doesNotMatch(result.message, /pending order/i);
     });
 
-    it('tolerates errors with no body property (e.g. transport failures)', () => {
+    it('tolerates errors with no body property', () => {
         const raw = new Error('ENOTFOUND webexapis.com');
-        const result = translateAddNumberError(raw, '+14045551212');
-        assert.equal(result, raw);
-    });
-
-    it('does not misfire on other 400 payloads that happen to be arrays', () => {
-        // Same array-of-errors shape but a different error code.
-        const raw = fakeHttpError(
-            400,
-            '[{"errors":[{"errorCode":"ERR.OTHER","errorMessage":"unrelated"}]}]',
-        );
-        const result = translateAddNumberError(raw, '+14045551212');
-        assert.equal(result, raw);
+        assert.equal(translateAddNumberError(raw, '+14045551212'), raw);
+    });
+});
+
+// classifyNumberAssignment decides what to do about the number's
+// current state in the org. The whole point of extracting it is so
+// this branching is unit-testable without any HTTP; the four kinds
+// here map 1:1 to the four operator-visible outcomes.
+describe('classifyNumberAssignment', () => {
+    // Regression: the endpoint that used to run in this step
+    // (POST /telephony/config/locations/{id}/numbers) triggers a new
+    // PSTN order — which is exactly wrong for AE's manual-order
+    // workflow. The "already-here" path is the common case now and
+    // must be a clean no-op.
+    it("returns 'already-here' when the number is ACTIVE in this location", () => {
+        const existing = {
+            phoneNumber: '+14045551212',
+            state: 'ACTIVE',
+            location: { id: LOC.id, name: LOC.name },
+        };
+        const out = classifyNumberAssignment(existing, '+14045551212', LOC);
+        assert.equal(out.kind, 'already-here');
+        assert.equal(out.existing, existing);
+    });
+
+    it("returns 'pending' when the number is in the org but not ACTIVE yet", () => {
+        const existing = {
+            phoneNumber: '+14045551212',
+            state: 'PENDING',
+            location: { id: LOC.id, name: LOC.name },
+        };
+        const out = classifyNumberAssignment(existing, '+14045551212', LOC);
+        assert.equal(out.kind, 'pending');
+        assert.match(out.message, /still processing/i);
+        assert.match(out.message, /re-run \/finalizeStore/);
+    });
+
+    it("returns 'wrong-location' when the number is ACTIVE in a different location", () => {
+        const existing = {
+            phoneNumber: '+14045551212',
+            state: 'ACTIVE',
+            location: { id: 'LOC-OTHER', name: 'Store 0499' },
+        };
+        const out = classifyNumberAssignment(existing, '+14045551212', LOC);
+        assert.equal(out.kind, 'wrong-location');
+        assert.match(out.message, /Store 0499/); // current location shown
+        assert.match(out.message, /Store 7311/); // target location shown
+        assert.match(out.message, /Move it in Control Hub/);
+    });
+
+    it('treats a missing location on the record as wrong-location (unassigned)', () => {
+        const existing = { phoneNumber: '+14045551212', state: 'ACTIVE' };
+        const out = classifyNumberAssignment(existing, '+14045551212', LOC);
+        assert.equal(out.kind, 'wrong-location');
+        assert.match(out.message, /\(unassigned\)/);
+    });
+
+    it("returns 'not-in-org' when the number isn't in the inventory at all", () => {
+        const out = classifyNumberAssignment(null, '+14045551212', LOC);
+        assert.equal(out.kind, 'not-in-org');
+        assert.match(out.message, /Order it in Control Hub/);
+        assert.match(out.message, /Store 7311/);
     });
 });