collabSupport/services/voiceDiag/voiceDiagService.js
jmcqueen 2eb31a2ddc Add /voicediag rules-engine command with 9 per-user calling checks
Introduces a new diagnostic command that walks a registry of check
modules against a store user's Webex Calling configuration and
surfaces per-issue adaptive-card remediation for the fixable ones.

Checks (services/voiceDiag/checks/): dnd, callForwarding, callWaiting,
callIntercept, voicemail, hoteling, executiveAssistant,
outgoingPermission, phoneOnline. Remediations offered for DND,
forwarding, waiting, and intercept.

Uses the /v1/people/{id}/features/* admin surface (spark-admin:people_read
+ spark-admin:people_write scopes we already hold) — the earlier
telephony/config/people/*/callSettings/* path scheme returns 404 from
the Webex gateway and is not a live surface. Runner distinguishes
routing-404s ("URL moved") from "not applicable" 404s ("no calling
license") via the response body.

Arg parser accepts detail/detailed/--detail/--detailed and normalises
macOS smart-dashes so --detailed doesn't die when auto-correct
turns it into an em-dash.

Wires a VOICEDIAG_ACTIONS dispatcher in index.js mirroring the IGMP
branch, and registers /voicediag in commands/registry.js. 170 tests
pass (52 new: 39 check + 12 renderer + 5 arg-normalization).

Docs updated in .env.example, services/phoneService.js:467, and a new
services/voiceDiag/README.md that includes a "how to add a check"
recipe plus a note on the earlier wrong URL scheme.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-07 18:51:31 -04:00

318 lines
11 KiB
JavaScript

// src/services/voiceDiag/voiceDiagService.js
//
// Orchestrator for the /voicediag command. Two responsibilities:
//
// 1. buildContext(storeNum) — resolves the store's canonical Webex
// user (currently 1:1 with a store number via the ae<pad5>@ae.com
// convention) and pre-fetches shared data that most checks want
// (device list, telephony profile, phone status). Doing this
// once here means individual check modules don't each re-do the
// lookup.
//
// 2. runVoiceDiag(storeNum, opts) — iterates the check registry,
// filters by `requires` capabilities present in the context and
// an optional `only` allow-list, executes each check in parallel
// via Promise.allSettled so one 500 doesn't take the run down,
// and normalises any thrown error into a `status: 'skipped'` or
// `status: 'error'` CheckResult. The returned array is in
// registry order — the renderer relies on that for stable output.
//
// Design principles:
// - Checks never see raw axios errors — the runner turns 401/403/404
// into `skipped: scope missing` and everything else into
// `error: <message>`. Individual checks decide their own OK/WARN
// when their fetch succeeds.
// - Context is intentionally minimal. If a future check needs an
// extra pre-fetch, add it here rather than duplicating the fetch
// per check.
// - Never throws — every failure surfaces as a CheckResult so the
// command handler can render *something* to the user.
import { logger } from '../../utils/logger.js';
import { CHECKS, getCheckById } from './checks/index.js';
// The WebexClient singleton + phoneService helpers are imported LAZILY
// inside buildContext(). Eager top-level imports would run the
// WebexClient constructor at module load time, which requires
// WEBEX_CLIENT_ID / WEBEX_CLIENT_SECRET in the environment. That's
// fine at runtime but breaks unit tests that only exercise
// runVoiceDiag / buildRemediationRegistry against a stub ctx (see
// tests/voiceDiag.checks.test.js).
/**
* Build the shared execution context for a /voicediag run. Pre-fetches
* the person + telephony profile + phone-status snapshot up-front so
* each check has cheap random access to them. Missing pieces are set
* to null so `requires` gating in runVoiceDiag can short-circuit.
*
* @param {string} storeNum 2-4 digit store id
* @returns {Promise<{
* storeNum: string,
* email: string,
* personId: string | null,
* person: object | null,
* telephonyProfile: object,
* phoneStatus: object | null,
* personLabel: string,
* }>}
*/
export async function buildContext(storeNum) {
const email = `ae${String(storeNum).padStart(5, '0')}@ae.com`;
logger('voicediag', `Building context for store ${storeNum}${email}`, 'debug');
const { default: webex } = await import('../../integrations/webex/WebexClient.js');
const {
getPersonIdByEmail,
getPersonDetails,
getTelephonyProfile,
collectPhoneStatus,
} = await import('../phoneService.js');
const personId = await getPersonIdByEmail(email);
// If we have no person, skip the expensive phoneStatus fetch — the
// renderer will show every check as `skipped: no user`. This keeps
// the "unknown store" case cheap.
const [personRes, telProfRes, phoneStatusRes] = personId
? await Promise.allSettled([
getPersonDetails(personId),
getTelephonyProfile(personId),
collectPhoneStatus(storeNum).catch((err) => {
// collectPhoneStatus can throw noisily on network hiccups.
// We tolerate this: the phoneOnline check will read
// ctx.phoneStatus and show 'skipped' if it's null.
logger('voicediag', `collectPhoneStatus soft-failed: ${err.message}`, 'warn');
return null;
}),
])
: [
{ status: 'fulfilled', value: null },
{ status: 'fulfilled', value: {} },
{ status: 'fulfilled', value: null },
];
const person = personRes.status === 'fulfilled' ? personRes.value : null;
const telephonyProfile = telProfRes.status === 'fulfilled' ? (telProfRes.value || {}) : {};
const phoneStatus = phoneStatusRes.status === 'fulfilled' ? phoneStatusRes.value : null;
const personLabel = person?.displayName || email;
return {
storeNum: String(storeNum),
email,
personId,
person,
telephonyProfile,
phoneStatus,
personLabel,
// Bare reference to the WebexClient singleton (lazy-loaded above)
// so checks can make their own targeted calls
// (`people/{id}/features/*`) without re-importing from every
// check module. Keeps mocking easy in tests — a test can pass a
// stub context with a fake webex; no ctx passes through the real
// client unless it came from buildContext().
webex,
};
}
/**
* Options for runVoiceDiag.
*
* @typedef {Object} RunVoiceDiagOptions
* @property {string[]} [only]
* Restrict execution to the given check ids. Unknown ids are logged
* at 'warn' and skipped silently in the results (no CheckResult for
* them). Empty array runs everything (same as omitted).
*/
/**
* Execute all registered checks against the given context. Never
* throws. Returns one CheckResult per check that actually ran; checks
* whose `requires` aren't satisfied by the context appear as
* `skipped: missing <cap>` so the renderer can still show them.
*
* @param {ReturnType<typeof buildContext> extends Promise<infer T> ? T : never} ctx
* @param {RunVoiceDiagOptions} [opts]
* @returns {Promise<CheckResult[]>}
*/
export async function runVoiceDiag(ctx, opts = {}) {
const onlyIds = Array.isArray(opts.only) && opts.only.length > 0
? new Set(opts.only.map((s) => String(s).toLowerCase().trim()))
: null;
if (onlyIds) {
for (const id of onlyIds) {
if (!getCheckById(id)) {
logger('voicediag', `--only referenced unknown check id: ${id}`, 'warn');
}
}
}
// Both sides lowercased so `--only DND` matches check.id === 'dnd'
// and `--only callforwarding` matches check.id === 'callForwarding'.
const toRun = CHECKS.filter((c) => !onlyIds || onlyIds.has(String(c.id).toLowerCase()));
const results = await Promise.all(
toRun.map(async (check) => {
const missing = missingRequirements(check, ctx);
if (missing.length > 0) {
return skipped(
check,
`not applicable — missing ${missing.join(', ')} for this store`,
);
}
try {
const result = await check.run(ctx);
return normalizeResult(check, result);
} catch (err) {
return errorToResult(check, err);
}
}),
);
return results;
}
function missingRequirements(check, ctx) {
const reqs = Array.isArray(check.requires) ? check.requires : [];
const missing = [];
for (const req of reqs) {
if (req === 'personId' && !ctx.personId) missing.push('personId');
else if (req === 'phoneStatus' && !ctx.phoneStatus) missing.push('phoneStatus');
else if (req === 'telephonyProfile' && (!ctx.telephonyProfile || Object.keys(ctx.telephonyProfile).length === 0)) {
missing.push('telephonyProfile');
}
}
return missing;
}
function normalizeResult(check, result) {
if (!result || typeof result !== 'object') {
return {
id: check.id,
label: check.label,
status: 'error',
message: `Check "${check.id}" returned no result`,
details: null,
remediation: null,
};
}
return {
id: check.id,
label: check.label,
status: result.status || 'ok',
message: result.message || '',
details: result.details ?? null,
remediation: result.remediation ?? null,
};
}
function errorToResult(check, err) {
const status = err?.response?.status;
const apiMsg =
err?.response?.data?.message ||
err?.response?.data?.errors?.[0]?.description ||
err?.message ||
String(err);
// 401/403 → the service-app token doesn't have the scope. Surface
// as `skipped: scope missing` and keep the run going against the
// other checks. The scope is uniformly `spark-admin:people_read`
// for the `/v1/people/{id}/features/*` surface, so if this fires
// on ANY check it will fire on all of them — but we still emit
// one skipped row per check so the operator can see the pattern.
if (status === 401 || status === 403) {
return {
id: check.id,
label: check.label,
status: 'skipped',
message:
`${status} on ${check.label} — the service app token is missing ` +
`\`${check.scope || 'spark-admin:people_read'}\`. ` +
`Re-authorize the app and re-bootstrap ` +
`\`tokens/webex-service-tokens.json\`.`,
details: { httpStatus: status },
remediation: null,
};
}
// 404 on `/v1/people/{id}/features/*` most commonly means the
// person isn't a Webex Calling user (no calling license, no
// location assigned, or a service line) — the same admin URL
// returns 200 for a properly-provisioned calling user. If the
// path itself moved (see Webex changelog for scheduled migrations
// to `/v1/telephony/config/people/{id}/...`), you'll see the
// "no static resource" message quoted below, which is a routing
// 404 rather than a "not applicable" 404 — the response body
// distinguishes them.
if (status === 404) {
const routingMiss = /no static resource/i.test(apiMsg);
return {
id: check.id,
label: check.label,
status: 'skipped',
message: routingMiss
? `404 (routing) on ${check.label} — the URL for this feature has moved. ` +
`Update the ENDPOINT constant in the check module. Details: ${apiMsg}`
: `404 on ${check.label} — this user has no Webex Calling license or the ` +
`feature isn't applicable to their line type.`,
details: { httpStatus: 404, routingMiss, apiMsg },
remediation: null,
};
}
logger('voicediag', `Check ${check.id} failed: ${apiMsg}`, 'warn');
return {
id: check.id,
label: check.label,
status: 'error',
message: `Error running ${check.label}: ${apiMsg}${status ? ` (HTTP ${status})` : ''}`,
details: { httpStatus: status || null },
remediation: null,
};
}
function skipped(check, message) {
return {
id: check.id,
label: check.label,
status: 'skipped',
message,
details: null,
remediation: null,
};
}
/**
* Build a lookup { remediationId -> handler } across every check.
* Used by commands/voiceDiag.js's `applyVoiceDiagRemediation` to
* dispatch a `confirm_voicediag` attachmentAction to the right
* check's remediation function.
*
* @returns {Map<string, {check: object, handler: Function}>}
*/
export function buildRemediationRegistry() {
const map = new Map();
for (const check of CHECKS) {
const rems = check.remediations;
if (!rems || typeof rems !== 'object') continue;
for (const [actionId, handler] of Object.entries(rems)) {
if (typeof handler !== 'function') continue;
if (map.has(actionId)) {
// Duplicate remediation ids across two checks would silently
// swallow one. Fail loud so the mistake is obvious in tests.
throw new Error(
`Duplicate voicediag remediation id "${actionId}" — check "${check.id}" ` +
`collides with an already-registered handler`,
);
}
map.set(actionId, { check, handler });
}
}
return map;
}
// Re-export CHECKS so tests / list-checks handlers don't need a
// second import from ./checks/index.js.
export { CHECKS };