From d12723d0107a210e6f231009df02269078b7c491 Mon Sep 17 00:00:00 2001 From: jmcqueen Date: Wed, 8 Jul 2026 14:14:38 -0400 Subject: [PATCH] Voicediag: store voice standards + port-hygiene checks + apply-all card Refactor every /voicediag check to declare a top-level `standards` object so the desired state is legible without reading run() logic and can drive a documented reference table. Upgrade callForwarding to error severity, tighten voicemail with three send-to-VM error paths + a `stop_sending_to_voicemail` remediation, and add a `disable_hoteling` remediation. Add a port-hygiene check bucket under services/voiceDiag/checks/port (portType, portVlan, portPoe, portEnabled) that reuses the phone- status snapshot to enforce switchport standards. Configurable via VOICE_STANDARD_PHONE_VLAN (default 102) and VOICE_STANDARD_ENABLED (kill-switch). Preserve Meraki `portType`/`voiceVlan`/`dataVlan` through the enrichment chain so the checks have clean data to read. Add an "apply all N fixes" combined card that shows up when 2+ remediations are available. New confirm_voicediag_all / cancel_voicediag_all actions run each fix in sequence (readable audit trail, no per-person write-throttle stacking), accumulate individual failures into a summary rather than aborting. Adds regression tests asserting every check exposes .standards, plus coverage for port checks, kill-switch, and combined-card iteration. 63 tests in the checks file, 188 total, all green. Co-authored-by: Cursor --- .env.example | 25 ++ commands/voiceDiag.js | 228 ++++++++++++ index.js | 38 +- integrations/meraki/clients.js | 6 + services/enrichment/merakiEnrichment.js | 6 + services/voiceDiag/README.md | 88 ++++- services/voiceDiag/checks/callForwarding.js | 15 +- services/voiceDiag/checks/callIntercept.js | 11 +- services/voiceDiag/checks/callWaiting.js | 11 +- services/voiceDiag/checks/dnd.js | 23 +- .../voiceDiag/checks/executiveAssistant.js | 12 +- services/voiceDiag/checks/hoteling.js | 84 ++++- services/voiceDiag/checks/index.js | 12 + .../voiceDiag/checks/outgoingPermission.js | 10 + services/voiceDiag/checks/phoneOnline.js | 8 + services/voiceDiag/checks/port/_helpers.js | 77 ++++ services/voiceDiag/checks/port/portEnabled.js | 99 +++++ services/voiceDiag/checks/port/portPoe.js | 96 +++++ services/voiceDiag/checks/port/portType.js | 103 ++++++ services/voiceDiag/checks/port/portVlan.js | 121 ++++++ services/voiceDiag/checks/voicemail.js | 171 +++++++-- tests/voiceDiag.checks.test.js | 349 +++++++++++++++++- 22 files changed, 1515 insertions(+), 78 deletions(-) create mode 100644 services/voiceDiag/checks/port/_helpers.js create mode 100644 services/voiceDiag/checks/port/portEnabled.js create mode 100644 services/voiceDiag/checks/port/portPoe.js create mode 100644 services/voiceDiag/checks/port/portType.js create mode 100644 services/voiceDiag/checks/port/portVlan.js diff --git a/.env.example b/.env.example index 1026ab5..6d403ae 100644 --- a/.env.example +++ b/.env.example @@ -81,6 +81,31 @@ WEBEX_TOKENS_PATH=./config/webex-service-tokens.json # Optional override for the Webex API base URL (default https://webexapis.com/v1). # WEBEX_BASE_URL=https://webexapis.com/v1 +# ----------------------------------------------------------------------------- +# /voicediag — Store voice-line standards +# ----------------------------------------------------------------------------- +# Store phones are held to a fixed configuration standard. Most of the +# standard is baked into the per-check modules under +# services/voiceDiag/checks/ (see services/voiceDiag/README.md for the +# reference table) — the values below only exist for the pieces that +# vary per site / per environment. +# +# Expected VLAN for store phones. Today AE tags phones into the data +# VLAN (102); some sites may move onto a proper voice VLAN in the +# future. The check compares against whatever value is set here and +# emits a warn when a phone shows up on any other VLAN. Default 102 +# if unset. +# VOICE_STANDARD_PHONE_VLAN=102 + +# Global kill-switch for /voicediag port-hygiene checks. Set to +# `false` to silence portType / portVlan / portPoe / portEnabled while +# an underlying Meraki cleanup is in progress and you don't want the +# noise. Values: true (default) / false. The feature-config checks +# (DND, forwarding, waiting, voicemail, intercept, hoteling, exec- +# assistant, outgoingPermission) always run — this only gates the +# switchport bucket. +# VOICE_STANDARD_ENABLED=true + # ----------------------------------------------------------------------------- # /webexhost — Webex Meetings host license helper # ----------------------------------------------------------------------------- diff --git a/commands/voiceDiag.js b/commands/voiceDiag.js index e86991e..013ae21 100644 --- a/commands/voiceDiag.js +++ b/commands/voiceDiag.js @@ -140,6 +140,20 @@ export async function handleVoiceDiag(bot, trigger) { requester, }); } + + // If there are 2+ independent remediations, add a bulk "apply all" + // card as a shortcut. The individual cards remain on-screen so an + // operator who only wants to fix one thing can still do that; the + // combined card just spares them from N confirm clicks when the + // whole set looks fine. Single-remediation cases skip this — one + // card is already the minimum interaction. + if (fixable.length >= 2) { + await postCombinedRemediationCard(bot, { + storeNum: ctx.storeNum, + fixable, + requester, + }); + } } /** @@ -189,6 +203,109 @@ export async function cancelVoiceDiagRemediation(bot, data, _roomId, requester) ); } +/** + * Combined "apply all N fixes" dispatcher — invoked when an operator + * clicks the bulk confirm on the summary card posted after 2+ + * individually-fixable results. Iterates the queued entries in + * *sequence* rather than parallel: + * + * - Sequential keeps Webex-side audit lines readable (one + * "COMPLETED foo" line per fix, in the order the check ran). + * - Sequential avoids compounding rate-limit exposure on the + * `/v1/people/{id}/features/*` surface (Webex applies a + * per-person write throttle that we've hit in bursts before). + * - Individual failure isolation: one 500 doesn't abort the rest; + * failures accumulate into `errors[]` and get reported at the end. + */ +export async function applyAllVoiceDiagRemediations(bot, data, _roomId, requester) { + const entries = Array.isArray(data?.entries) ? data.entries : []; + const { storeNum, personLabel } = data || {}; + + if (entries.length === 0) { + await bot.say( + 'markdown', + `⚠️ No pending remediations to apply for store ${storeNum || '?'}. The card may have expired.`, + ); + return; + } + + logger( + 'voicediag:audit', + `Dispatching APPLY-ALL (${entries.length} fixes) for store ${storeNum} ` + + `requested by ${describeRequester(requester)}`, + ); + + const applied = []; + const errors = []; + + for (const entry of entries) { + const registered = REMEDIATION_REGISTRY.get(entry.remediationId); + if (!registered) { + errors.push({ id: entry.remediationId, reason: 'no handler registered' }); + logger( + 'voicediag:action', + `No handler registered for remediationId "${entry.remediationId}" in apply-all — skipping`, + 'warn', + ); + continue; + } + + try { + logger( + 'voicediag:audit', + `apply-all → ${entry.remediationId} (check=${registered.check.id}) for store ${storeNum}`, + ); + await registered.handler(bot, entry.remediationPayload || {}, requester); + applied.push(entry.remediationId); + } catch (err) { + errors.push({ id: entry.remediationId, reason: err.message }); + logger( + 'voicediag:action', + `apply-all: ${entry.remediationId} failed: ${err.message}`, + 'error', + ); + } + } + + // Post a final summary line so the operator sees the aggregate + // outcome in one place — individual handlers already posted their + // own ✅ / ❌ per-fix, but a final tally scannable in one glance + // is much friendlier than reading N separate lines. + const lines = [ + `**Apply-all complete for store ${storeNum}** (${personLabel || 'store user'})`, + `- Applied: ${applied.length} / ${entries.length}`, + ]; + if (errors.length > 0) { + lines.push(`- Failed: ${errors.length}`); + for (const e of errors) { + lines.push(` - \`${e.id}\` — ${e.reason}`); + } + lines.push(`Re-run \`/voicediag ${storeNum}\` to re-inspect and retry any leftovers.`); + } + await bot.say('markdown', lines.join('\n')); + + logger( + 'voicediag:audit', + `COMPLETED apply-all for store ${storeNum} — applied=${applied.length}/${entries.length}, ` + + `errors=${errors.length}`, + ); +} + +export async function cancelAllVoiceDiagRemediations(bot, data, _roomId, requester) { + const { storeNum, personLabel } = data || {}; + const entries = Array.isArray(data?.entries) ? data.entries : []; + await bot.say( + 'markdown', + `❌ Cancelled all pending remediations for **${personLabel || `store ${storeNum || '?'}`}** ` + + `(${entries.length} fix${entries.length === 1 ? '' : 'es'} not applied).`, + ); + logger( + 'voicediag:audit', + `CANCELLED APPLY-ALL (${entries.length} entries) for store ${storeNum || '?'} ` + + `by ${describeRequester(requester)}`, + ); +} + // ─── internals ──────────────────────────────────────────────────── function argIncludes(args, needle) { @@ -270,6 +387,67 @@ async function postRemediationCard(bot, { storeNum, result, requester }) { }); } +/** + * Registers a *combined* pending payload holding every fixable + * result's remediationId + payload, and posts one adaptive card + * offering to apply the whole batch in a single click. Shape of the + * stored payload: + * + * { + * combined: true, + * storeNum, personLabel, + * entries: [{ remediationId, remediationPayload }, ...], + * requester, + * } + * + * Uses the same pendingVoiceFixes map (single sweep, single TTL) — + * the `combined: true` flag + the dispatcher's `switch` on + * `actionType` are what decides whether to hit the single-fix or + * batch handler. + */ +async function postCombinedRemediationCard(bot, { storeNum, fixable, requester }) { + const cardId = randomUUID(); + // All fixable results in a run share the same person (they're + // per-user features on the store line), so grab the label from + // the first entry — safer than reaching for ctx here since this + // helper only receives what it needs. + const personLabel = + fixable.find((r) => r.remediation?.payload?.personLabel)?.remediation?.payload?.personLabel || + `store ${storeNum}`; + + const entries = fixable.map((r) => ({ + remediationId: r.remediation.action, + remediationPayload: r.remediation.payload || {}, + // Kept for the card summary + audit context. Not read by the + // dispatcher. + label: r.label, + title: r.remediation.title, + })); + + pendingVoiceFixes.set(cardId, { + combined: true, + storeNum, + personLabel, + requester, + entries, + }); + + const card = buildCombinedVoiceDiagCard({ + storeNum, + personLabel, + entries, + cardId, + }); + + await bot.say({ + markdown: `Or apply **all ${entries.length} fixes** at once:`, + attachments: [{ + contentType: 'application/vnd.microsoft.card.adaptive', + content: card, + }], + }); +} + function buildVoiceDiagCard({ storeNum, result, cardId }) { const { label, message, remediation } = result; return { @@ -312,3 +490,53 @@ function buildVoiceDiagCard({ storeNum, result, cardId }) { ], }; } + +function buildCombinedVoiceDiagCard({ storeNum, personLabel, entries, cardId }) { + const bulletBody = entries.map((e) => `• ${e.label}: ${e.title}`).join('\n'); + return { + type: 'AdaptiveCard', + $schema: 'http://adaptivecards.io/schemas/adaptive-card.json', + version: '1.3', + body: [ + { + type: 'TextBlock', + size: 'Medium', + weight: 'Bolder', + text: `Apply all ${entries.length} fixes — Store ${storeNum}`, + wrap: true, + }, + { + type: 'TextBlock', + text: `Target: ${personLabel}`, + wrap: true, + spacing: 'Small', + isSubtle: true, + }, + { + type: 'TextBlock', + text: bulletBody, + wrap: true, + spacing: 'Small', + }, + { + type: 'TextBlock', + text: 'Fixes will be applied in sequence. Any failures are reported at the end.', + wrap: true, + spacing: 'Small', + isSubtle: true, + }, + ], + actions: [ + { + type: 'Action.Submit', + title: `✅ Apply all ${entries.length}`, + data: { action: 'confirm_voicediag_all', cardId }, + }, + { + type: 'Action.Submit', + title: '❌ Cancel', + data: { action: 'cancel_voicediag_all', cardId }, + }, + ], + }; +} diff --git a/index.js b/index.js index 78724f8..1e17b71 100644 --- a/index.js +++ b/index.js @@ -37,6 +37,8 @@ import { pendingIgmpFixes } from './utils/pendingIgmpFixes.js'; import { applyVoiceDiagRemediation, cancelVoiceDiagRemediation, + applyAllVoiceDiagRemediations, + cancelAllVoiceDiagRemediations, } from './commands/voiceDiag.js'; import { pendingVoiceFixes } from './utils/pendingVoiceFixes.js'; import { extractRequester } from './utils/requester.js'; @@ -303,7 +305,16 @@ const DECT_ACTIONS = new Set([ const OFFBOARD_ACTIONS = new Set(['confirm_offboard', 'cancel_offboard']); const HOST_ASSIGN_ACTIONS = new Set(['confirm_host_assign', 'cancel_host_assign']); const IGMP_FIX_ACTIONS = new Set(['confirm_igmp_fix', 'cancel_igmp_fix']); -const VOICEDIAG_ACTIONS = new Set(['confirm_voicediag', 'cancel_voicediag']); +const VOICEDIAG_ACTIONS = new Set([ + 'confirm_voicediag', + 'cancel_voicediag', + // "Apply all N fixes" combined-card variants — see + // commands/voiceDiag.js#postCombinedRemediationCard. Same pending-map + // + same dispatcher branch; the payload shape distinguishes the two + // (single-remediation vs. `entries` array). + 'confirm_voicediag_all', + 'cancel_voicediag_all', +]); // Best-effort delete of the adaptive-card message that fired this action. // Removing the card prevents users from clicking Confirm/Cancel a second time @@ -514,7 +525,11 @@ framework.on('attachmentAction', async (bot, trigger) => { logger( 'voicediag:action', `Received ${actionType} for card ${cardId} ` + - `(store: ${voiceData.storeNum}, remediation: ${voiceData.remediationId})`, + `(store: ${voiceData.storeNum}, ` + + (voiceData.combined + ? `combined N=${(voiceData.entries || []).length}` + : `remediation: ${voiceData.remediationId}`) + + `)`, ); await censorActionCard(bot, trigger, 'voicediag:action'); @@ -522,16 +537,25 @@ framework.on('attachmentAction', async (bot, trigger) => { const requester = extractRequester(trigger); try { - if (actionType === 'confirm_voicediag') { - await applyVoiceDiagRemediation(bot, voiceData, roomId, requester); - } else { - await cancelVoiceDiagRemediation(bot, voiceData, roomId, requester); + switch (actionType) { + case 'confirm_voicediag': + await applyVoiceDiagRemediation(bot, voiceData, roomId, requester); + break; + case 'cancel_voicediag': + await cancelVoiceDiagRemediation(bot, voiceData, roomId, requester); + break; + case 'confirm_voicediag_all': + await applyAllVoiceDiagRemediations(bot, voiceData, roomId, requester); + break; + case 'cancel_voicediag_all': + await cancelAllVoiceDiagRemediations(bot, voiceData, roomId, requester); + break; } } catch (err) { logger( 'voicediag:action', `Error processing ${actionType} for store ${voiceData.storeNum} ` + - `remediation ${voiceData.remediationId}: ${err.message}`, + `remediation ${voiceData.remediationId || 'combined'}: ${err.message}`, 'error', ); await bot.say('markdown', `⚠️ Error during voice diagnostic remediation: ${err.message}`); diff --git a/integrations/meraki/clients.js b/integrations/meraki/clients.js index 9d32402..edcdd97 100644 --- a/integrations/meraki/clients.js +++ b/integrations/meraki/clients.js @@ -93,6 +93,12 @@ export async function getMerakiPorts(networkId) { accessPolicy: port.accessPolicy, stickyMac: port.stickyMac || false, allowedMacs: port.allowedMacs || [], + // 'access' | 'trunk' | undefined. Preserved so /voicediag + // port-hygiene checks can flag phones that end up on a + // trunk uplink (downstream through a non-Meraki switch, + // typically a Cisco stack) where per-port policy isn't + // visible from our side. + portType: port.type || null, voiceVlan: port.voiceVlan, dataVlan: port.vlan, portName: port.name || `Port ${port.number}`, diff --git a/services/enrichment/merakiEnrichment.js b/services/enrichment/merakiEnrichment.js index 9a3feac..6d90104 100644 --- a/services/enrichment/merakiEnrichment.js +++ b/services/enrichment/merakiEnrichment.js @@ -303,6 +303,12 @@ export async function attachPortConfigAndStatus(client, portConfigs = [], portSt poePower: portConfig?.poePower || 0, accessPolicy: portConfig?.accessPolicyType || portConfig?.accessPolicy || '—', allowedMacs: portConfig?.stickyMacAllowList || portConfig?.allowedMacs || [], + // 'access' | 'trunk' | null. Consumed by /voicediag port-hygiene + // checks — trunk means the phone is downstream through a non- + // Meraki switch and per-port config isn't visible from here. + portType: portConfig?.portType || portConfig?.type || null, + voiceVlan: portConfig?.voiceVlan ?? null, + dataVlan: portConfig?.dataVlan ?? portConfig?.vlan ?? null, switchportStatus: portStatus, switchportConfig: portConfig, switchSerial: switchSerial || '—', diff --git a/services/voiceDiag/README.md b/services/voiceDiag/README.md index 1a06036..14b49df 100644 --- a/services/voiceDiag/README.md +++ b/services/voiceDiag/README.md @@ -48,20 +48,106 @@ distinguishes routing-404s ("URL moved, update the check") from "not applicable" 404s ("this person isn't a calling user") using the `no static resource` marker in the response body. +## Store voice standards + +The store phone standard is enforced by the checks below. Every +check descriptor exports a `standards` object so the desired state +is legible from the check file without reading `run()`. Changes +here are meant to be a two-step change: update `standards`, then +teach `run()` to interpret it — the checks compare the live state +against `standards` and emit the severity in the "when non-compliant" +column. + +### Per-user Webex Calling standards + +| Check | Standard | Non-compliant severity | Auto-remediation | +| -------------------- | -------------------------------------------------------- | ---------------------- | ------------------------------------ | +| `dnd` | `enabled: false` | warn | `disable_dnd` | +| `callForwarding` | `{always, busy, noAnswer}.enabled: false` | **error** | `clear_call_forwarding` | +| `callWaiting` | `enabled: true` | warn | `enable_call_waiting` | +| `callIntercept` | `enabled: false` | error | `disable_call_intercept` | +| `voicemail` | `enabled: true`, all three `send*Calls.enabled: false`, `mwiEnabled: true` | error on send-to-VM, warn on MWI-off / disabled / off-org email | `stop_sending_to_voicemail` for the send-to-VM path only | +| `hoteling` | `enabled: false` | warn | `disable_hoteling` | +| `executiveAssistant` | `type: 'UNASSIGNED'` | warn | none — Control Hub cleanup | +| `outgoingPermission` | no high-impact call type BLOCKED (LOCAL, NATIONAL, TOLL_FREE, TOLL, INTERNATIONAL) | warn | none — location-scoped fix | + +`callForwarding` is intentionally at `error` severity: forwarding +active on a store line silently drops customer calls, and it's the +single most common voice ticket. Voicemail's `error` path is +narrower — only the three send-* triggers upgrade to error, since +that also silently swallows calls. MWI-off, off-org email +forwarding, and voicemail-disabled all stay at warn. + +### Switchport / port-hygiene standards + +These reuse the phone-status snapshot from `/phonestatus` — no +extra Webex API calls — and cross-reference against the Meraki +port config that already flows through `services/enrichment/merakiEnrichment.js`. +Trunk uplinks are treated as a visibility boundary: the phone is +behind a non-Meraki switch (typically a Cisco stack in a store) +and per-port policy isn't ours to enforce, so downstream VLAN / +PoE / admin-state checks defer to the operator. + +| Check | Standard | Non-compliant severity | Notes | +| ------------- | ---------------------------------------------------- | ---------------------- | ----------------------------------------------- | +| `portType` | `portType: 'access'` on every wired phone / DECT base | warn | Trunk uplinks flagged so operator checks the downstream switch | +| `portVlan` | `vlan === VOICE_STANDARD_PHONE_VLAN` (default `102`) | warn | Env-configurable — VLAN may move from data-side to a proper voice VLAN in the future | +| `portPoe` | `poeEnabled: true` | warn | Skipped for trunk-uplinked devices | +| `portEnabled` | `portEnabled: true` | **error** | Admin-disabled port → phone is dead | + +No auto-remediation on any port check — Meraki port-config PUTs +are a separate scope of work; the operator handles fixes in the +Meraki Dashboard. + +### Environment overrides + +Both port-hygiene knobs live in `.env`: + +| Var | Default | Purpose | +| ------------------------------ | ------- | ---------------------------------------------------------------------------------------------- | +| `VOICE_STANDARD_PHONE_VLAN` | `102` | Expected VLAN for a store phone. Set per-site if the fleet moves onto a proper voice VLAN. | +| `VOICE_STANDARD_ENABLED` | `true` | Global kill-switch for the port-hygiene bucket. `false` silences portType / portVlan / portPoe / portEnabled while Meraki cleanup is in progress. Feature-config checks always run. | + +## Apply-all-N-fixes card + +When two or more checks return fixable results, `/voicediag` +posts one extra adaptive card at the bottom offering to apply the +whole batch in a single click. The individual per-issue cards stay +on-screen so operators can still pick and choose; the combined +card is a shortcut for the common "everything looks right, do it +all" case. The batch executes each fix in sequence (not parallel) +so audit lines stay readable and per-person Webex API write +throttling doesn't stack; failures accumulate into a final summary +line rather than aborting the run. + +Dispatcher-side, the combined card uses the same +`pendingVoiceFixes` map and the same voicediag branch in +`index.js` — the payload's `combined: true` flag + the +`confirm_voicediag_all` / `cancel_voicediag_all` action ids are +what pick the batch handler over the single-fix handler. + ## Adding a new check 1. Create `services/voiceDiag/checks/.js` and export a descriptor: ```js + // The standards block is *the* source of truth for the desired + // state. Keep the run() comparison in sync — regression tests + // in tests/voiceDiag.checks.test.js assert every check has one. + export const MY_NEW_STANDARDS = Object.freeze({ enabled: false }); + export const myNewCheck = { id: 'myNew', label: 'My New Check', requires: ['personId'], // subset of ['personId','phoneStatus','telephonyProfile'] scope: 'spark-admin:people_read', + standards: MY_NEW_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', `people/${ctx.personId}/features/whatever`); - // ...evaluate... + if (!!data?.enabled === MY_NEW_STANDARDS.enabled) { + return { status: 'ok', message: 'Compliant.', details: null, remediation: null }; + } return { status: 'warn', // 'ok' | 'warn' | 'error' | 'skipped' message: 'Human sentence for the chat row.', diff --git a/services/voiceDiag/checks/callForwarding.js b/services/voiceDiag/checks/callForwarding.js index 12c9145..28b5fcd 100644 --- a/services/voiceDiag/checks/callForwarding.js +++ b/services/voiceDiag/checks/callForwarding.js @@ -47,11 +47,22 @@ const VARIANTS = [ { key: 'noAnswer', label: 'Call Forwarding — No Answer' }, ]; +// Store-line standard: no forwarding of any variant. Forwarding a +// store line silently routes calls elsewhere, which is a top cause +// of "the store phone isn't ringing" tickets. Hard rule → error +// severity (was warn in the initial cut). +export const CALL_FORWARDING_STANDARDS = Object.freeze({ + always: { enabled: false }, + busy: { enabled: false }, + noAnswer: { enabled: false }, +}); + export const callForwardingCheck = { id: 'callForwarding', label: 'Call Forwarding', requires: ['personId'], scope: 'spark-admin:people_read', + standards: CALL_FORWARDING_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); @@ -91,10 +102,10 @@ export const callForwardingCheck = { .join('; '); return { - status: 'warn', + status: 'error', message: `${active.length} forwarding ${active.length === 1 ? 'variant is' : 'variants are'} ` + - `active: ${summaryLine}.`, + `active: ${summaryLine}. Store phone standard requires all forwarding disabled.`, details: { perVariant, active: variantsToClear }, remediation: { action: 'clear_call_forwarding', diff --git a/services/voiceDiag/checks/callIntercept.js b/services/voiceDiag/checks/callIntercept.js index 5097faf..b20dcf5 100644 --- a/services/voiceDiag/checks/callIntercept.js +++ b/services/voiceDiag/checks/callIntercept.js @@ -38,11 +38,18 @@ import { describeRequester } from '../../../utils/requester.js'; const ENDPOINT = (personId) => `people/${personId}/features/intercept`; +// Store-line standard: intercept off. Intercept blocks calls with an +// announcement — never wanted on a live store line. +export const CALL_INTERCEPT_STANDARDS = Object.freeze({ + enabled: false, +}); + export const callInterceptCheck = { id: 'callIntercept', label: 'Call Intercept', requires: ['personId'], scope: 'spark-admin:people_read', + standards: CALL_INTERCEPT_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); @@ -50,7 +57,7 @@ export const callInterceptCheck = { const incomingType = data?.incoming?.type || null; const outgoingType = data?.outgoing?.type || null; - if (!enabled) { + if (enabled === CALL_INTERCEPT_STANDARDS.enabled) { return { status: 'ok', message: 'Call intercept is off.', @@ -91,7 +98,7 @@ export const callInterceptCheck = { try { const { default: webex } = await import('../../../integrations/webex/WebexClient.js'); - await webex.request('PUT', ENDPOINT(personId), { enabled: false }); + await webex.request('PUT', ENDPOINT(personId), { ...CALL_INTERCEPT_STANDARDS }); } catch (err) { logger( 'voicediag:audit', diff --git a/services/voiceDiag/checks/callWaiting.js b/services/voiceDiag/checks/callWaiting.js index 56bf300..6f71683 100644 --- a/services/voiceDiag/checks/callWaiting.js +++ b/services/voiceDiag/checks/callWaiting.js @@ -20,17 +20,24 @@ import { describeRequester } from '../../../utils/requester.js'; const ENDPOINT = (personId) => `people/${personId}/features/callWaiting`; +// Store-line standard: call waiting on. Second-inbound-call beeping +// in is what an operator on a call expects. +export const CALL_WAITING_STANDARDS = Object.freeze({ + enabled: true, +}); + export const callWaitingCheck = { id: 'callWaiting', label: 'Call Waiting', requires: ['personId'], scope: 'spark-admin:people_read', + standards: CALL_WAITING_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); const enabled = !!data?.enabled; - if (enabled) { + if (enabled === CALL_WAITING_STANDARDS.enabled) { return { status: 'ok', message: 'Call waiting is on.', @@ -70,7 +77,7 @@ export const callWaitingCheck = { try { const { default: webex } = await import('../../../integrations/webex/WebexClient.js'); - await webex.request('PUT', ENDPOINT(personId), { enabled: true }); + await webex.request('PUT', ENDPOINT(personId), { ...CALL_WAITING_STANDARDS }); } catch (err) { logger( 'voicediag:audit', diff --git a/services/voiceDiag/checks/dnd.js b/services/voiceDiag/checks/dnd.js index bb4b11e..7dfa487 100644 --- a/services/voiceDiag/checks/dnd.js +++ b/services/voiceDiag/checks/dnd.js @@ -31,11 +31,21 @@ import { describeRequester } from '../../../utils/requester.js'; const ENDPOINT = (personId) => `people/${personId}/features/doNotDisturb`; +// The desired-state contract for a store phone user. Exposed as a +// top-level field on the check descriptor so the "voice standards" +// reference table + regression tests can inspect it without reading +// run() logic. Change the values here, not in the run() body. +export const DND_STANDARDS = Object.freeze({ + enabled: false, + ringSplashEnabled: false, +}); + export const dndCheck = { id: 'dnd', label: 'Do Not Disturb', requires: ['personId'], scope: 'spark-admin:people_read', + standards: DND_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); @@ -43,7 +53,13 @@ export const dndCheck = { const enabled = !!data?.enabled; const ringSplashEnabled = !!data?.ringSplashEnabled; - if (!enabled) { + // Compliance = DND off. ringSplashEnabled only matters when DND + // is on (per Webex spec — it's the "visual reminder" toggle for + // splash notifications while calls are being silenced), so we + // don't count a stale ringSplash=true as a deviation while + // DND is already off. The standard's ringSplashEnabled=false is + // the value we PUT during remediation, not an independent rule. + if (enabled === DND_STANDARDS.enabled) { return { status: 'ok', message: 'DND is off.', @@ -89,10 +105,7 @@ export const dndCheck = { // singleton via ctx.webex in run(), while the remediation path // stays honest about which client it uses in production. const { default: webex } = await import('../../../integrations/webex/WebexClient.js'); - await webex.request('PUT', ENDPOINT(personId), { - enabled: false, - ringSplashEnabled: false, - }); + await webex.request('PUT', ENDPOINT(personId), { ...DND_STANDARDS }); } catch (err) { logger( 'voicediag:audit', diff --git a/services/voiceDiag/checks/executiveAssistant.js b/services/voiceDiag/checks/executiveAssistant.js index de98996..2b96cd5 100644 --- a/services/voiceDiag/checks/executiveAssistant.js +++ b/services/voiceDiag/checks/executiveAssistant.js @@ -23,17 +23,27 @@ const ENDPOINT = (personId) => `people/${personId}/features/executiveAssistant`; +// Store-line standard: UNASSIGNED. Being an executive or exec- +// assistant on a store line routes calls through screening. No +// auto-remediation offered — unassigning has downstream implications +// (the assistant relationship needs cleanup on both sides), so we +// surface as info-only and let the operator handle it in Control Hub. +export const EXECUTIVE_ASSISTANT_STANDARDS = Object.freeze({ + type: 'UNASSIGNED', +}); + export const executiveAssistantCheck = { id: 'executiveAssistant', label: 'Executive / Assistant', requires: ['personId'], scope: 'spark-admin:people_read', + standards: EXECUTIVE_ASSISTANT_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); const type = data?.type || 'UNASSIGNED'; - if (type === 'UNASSIGNED') { + if (type === EXECUTIVE_ASSISTANT_STANDARDS.type) { return { status: 'ok', message: 'No executive / assistant relationship (normal for a store line).', diff --git a/services/voiceDiag/checks/hoteling.js b/services/voiceDiag/checks/hoteling.js index 3f4223e..2172760 100644 --- a/services/voiceDiag/checks/hoteling.js +++ b/services/voiceDiag/checks/hoteling.js @@ -1,32 +1,42 @@ // src/services/voiceDiag/checks/hoteling.js // -// Info-only check for the Hoteling feature on the store user's line. -// Hoteling lets a "guest" line temporarily associate with a shared -// desk phone — if it's turned on for a store user that shouldn't have -// it, calls can end up at whatever guest device most recently checked -// in, which usually presents as "the phone at the store isn't the -// one that rings when we call". No remediation is offered because -// the intended state is site-dependent; the operator can flip it via -// Control Hub if the current state is wrong. +// Detects whether the store user's line is participating in the +// Hoteling feature as a guest. Hoteling lets a "guest" line +// temporarily associate with a shared desk phone — if it's turned on +// for a store user that shouldn't have it, calls can end up at +// whatever guest device most recently checked in, which usually +// presents as "the phone at the store isn't the one that rings when +// we call". // -// Endpoint: GET /v1/people/{personId}/features/hoteling -// Scope: spark-admin:people_read +// Endpoint: GET/PUT /v1/people/{personId}/features/hoteling +// Scope: spark-admin:people_read (GET) + spark-admin:people_write (PUT). // Response shape: { enabled: boolean } +// +// Standard: enabled=false. Store lines shouldn't be roaming to +// shared endpoints. Remediation turns it off. + +import { logger } from '../../../utils/logger.js'; +import { describeRequester } from '../../../utils/requester.js'; const ENDPOINT = (personId) => `people/${personId}/features/hoteling`; +export const HOTELING_STANDARDS = Object.freeze({ + enabled: false, +}); + export const hotelingCheck = { id: 'hoteling', label: 'Hoteling', requires: ['personId'], scope: 'spark-admin:people_read', + standards: HOTELING_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); const enabled = !!data?.enabled; - if (!enabled) { + if (enabled === HOTELING_STANDARDS.enabled) { return { status: 'ok', message: 'Hoteling is disabled (normal for a store line).', @@ -38,9 +48,57 @@ export const hotelingCheck = { return { status: 'warn', message: - 'Hoteling is ENABLED — this line may be roaming to another desk phone. Verify in Control Hub.', + 'Hoteling is ENABLED — this line may be roaming to another desk phone.', details: { enabled }, - remediation: null, + remediation: { + action: 'disable_hoteling', + title: 'Disable Hoteling', + summary: `Turn hoteling off for ${ctx.personLabel} so this line stops roaming.`, + payload: { + personId: ctx.personId, + personLabel: ctx.personLabel, + storeNum: ctx.storeNum, + before: { enabled }, + }, + }, }; }, + + remediations: { + async disable_hoteling(bot, data, requester) { + const { personId, personLabel, storeNum, before } = data; + + logger( + 'voicediag:audit', + `CONFIRMED disable_hoteling for ${personLabel} (person=${personId}, store=${storeNum}) ` + + `by ${describeRequester(requester)} — was enabled=${before?.enabled ?? 'unknown'}`, + ); + + try { + const { default: webex } = await import('../../../integrations/webex/WebexClient.js'); + await webex.request('PUT', ENDPOINT(personId), { ...HOTELING_STANDARDS }); + } catch (err) { + logger( + 'voicediag:audit', + `FAILED disable_hoteling for ${personLabel}: ${err.message}`, + 'error', + ); + await bot.say( + 'markdown', + `❌ Failed to disable hoteling for **${personLabel}**: ${err.message}`, + ); + return; + } + + await bot.say( + 'markdown', + `✅ Hoteling disabled for **${personLabel}** (store ${storeNum}). ` + + `Re-run \`/voicediag ${storeNum}\` to verify.`, + ); + logger( + 'voicediag:audit', + `COMPLETED disable_hoteling for ${personLabel} (store ${storeNum})`, + ); + }, + }, }; diff --git a/services/voiceDiag/checks/index.js b/services/voiceDiag/checks/index.js index 336cbbc..3601d25 100644 --- a/services/voiceDiag/checks/index.js +++ b/services/voiceDiag/checks/index.js @@ -39,7 +39,15 @@ import { hotelingCheck } from './hoteling.js'; import { executiveAssistantCheck } from './executiveAssistant.js'; import { outgoingPermissionCheck } from './outgoingPermission.js'; import { phoneOnlineCheck } from './phoneOnline.js'; +import { portTypeCheck } from './port/portType.js'; +import { portVlanCheck } from './port/portVlan.js'; +import { portPoeCheck } from './port/portPoe.js'; +import { portEnabledCheck } from './port/portEnabled.js'; +// Order: user-facing feature signals first (things an operator can +// see from the phone UI), then network-side port hygiene, then the +// broad-brush online summary last so it acts as a reachability +// closer. export const CHECKS = [ dndCheck, callForwardingCheck, @@ -49,6 +57,10 @@ export const CHECKS = [ hotelingCheck, executiveAssistantCheck, outgoingPermissionCheck, + portTypeCheck, + portVlanCheck, + portPoeCheck, + portEnabledCheck, phoneOnlineCheck, ]; diff --git a/services/voiceDiag/checks/outgoingPermission.js b/services/voiceDiag/checks/outgoingPermission.js index 2a70f6b..84910ff 100644 --- a/services/voiceDiag/checks/outgoingPermission.js +++ b/services/voiceDiag/checks/outgoingPermission.js @@ -41,11 +41,21 @@ const HIGH_IMPACT_CALL_TYPES = new Set([ 'INTERNATIONAL', ]); +// Store-line standard: no HIGH_IMPACT_CALL_TYPE may be BLOCKED. Any +// other rule (BLOCK on premium/casual/operator, ALLOW on everything) +// is fine. Documented as data so the standards reference table + +// tests can enumerate what "compliant" means. +export const OUTGOING_PERMISSION_STANDARDS = Object.freeze({ + highImpactCallTypes: Array.from(HIGH_IMPACT_CALL_TYPES), + highImpactBlockedAllowed: false, +}); + export const outgoingPermissionCheck = { id: 'outgoingPermission', label: 'Outgoing Call Permissions', requires: ['personId'], scope: 'spark-admin:people_read', + standards: OUTGOING_PERMISSION_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); diff --git a/services/voiceDiag/checks/phoneOnline.js b/services/voiceDiag/checks/phoneOnline.js index 14e5022..b807a34 100644 --- a/services/voiceDiag/checks/phoneOnline.js +++ b/services/voiceDiag/checks/phoneOnline.js @@ -31,6 +31,13 @@ function isOnline(dev) { return OK_STATUSES.has(s); } +export const PHONE_ONLINE_STANDARDS = Object.freeze({ + // Every registered device must report 'connected'. Anything else + // (disconnected, unknown, activating, offline) is treated as + // offline. + status: 'connected', +}); + export const phoneOnlineCheck = { id: 'phoneOnline', label: 'Phone Online Status', @@ -38,6 +45,7 @@ export const phoneOnlineCheck = { // No new API scope required — reuses collectPhoneStatus() output, // which is already gated on the /phonestatus set of scopes. scope: null, + standards: PHONE_ONLINE_STANDARDS, async run(ctx) { const data = ctx.phoneStatus; diff --git a/services/voiceDiag/checks/port/_helpers.js b/services/voiceDiag/checks/port/_helpers.js new file mode 100644 index 0000000..88b744c --- /dev/null +++ b/services/voiceDiag/checks/port/_helpers.js @@ -0,0 +1,77 @@ +// src/services/voiceDiag/checks/port/_helpers.js +// +// Shared helpers for the port-hygiene check bucket. Each of the port +// checks (portType, portVlan, portPoe, portEnabled) walks the same +// per-phone / per-DECT-base list out of ctx.phoneStatus and reports +// a per-device verdict. Centralising the walk + labelling here keeps +// the individual check modules focused on their single signal. + +/** + * Collect every device that could plausibly have a wired port. This + * is desk phones + DECT basestations. DECT handsets are excluded — + * they're radio-connected to their base, they don't have their own + * switchport. Wireless desk phones aren't filtered out here; the + * per-check `isWireless(d)` guard decides how to render them. + * + * @param {object} phoneStatus collectPhoneStatus() result + * @returns {Array} each entry retains its full shape so + * checks can reach `.meraki.*` freely. + */ +export function collectPortDevices(phoneStatus) { + if (!phoneStatus) return []; + const phones = Array.isArray(phoneStatus?.phones?.data) ? phoneStatus.phones.data : []; + const dectBases = Array.isArray(phoneStatus?.dectBasestations) ? phoneStatus.dectBasestations : []; + const tag = (kind) => (d) => ({ ...d, _voiceDiagKind: kind }); + return [ + ...phones.map(tag('phone')), + ...dectBases.map(tag('dect-base')), + ]; +} + +/** True when the device is on wireless — no switchport to inspect. */ +export function isWireless(dev) { + const conn = String(dev?.meraki?.connectionType || '').toLowerCase(); + if (conn === 'wireless') return true; + return false; +} + +/** + * Global kill-switch for the port-hygiene bucket. Env + * `VOICE_STANDARD_ENABLED=false` returns a short "port checks are + * currently disabled" skip result that every port check can return + * verbatim. Feature-config checks (DND, forwarding, etc.) are + * intentionally *not* gated by this — those are unconditional + * standards. Only the switchport bucket bows out. + * + * The check descriptor is passed in so the returned result carries + * the caller's id / label without callers repeating themselves. + */ +export function maybeSkippedByKillSwitch(check) { + const raw = String(process.env.VOICE_STANDARD_ENABLED ?? 'true').toLowerCase().trim(); + const enabled = !(raw === 'false' || raw === '0' || raw === 'no' || raw === 'off'); + if (enabled) return null; + return { + status: 'skipped', + message: + `${check.label} skipped — VOICE_STANDARD_ENABLED=false (port-hygiene checks are silenced).`, + details: { killSwitch: 'VOICE_STANDARD_ENABLED', value: raw }, + remediation: null, + }; +} + +/** Human labels used in per-device drilldowns. Kept short so + * aggregate messages don't blow past Webex's chat readability. */ +export function portLabelFor(dev) { + const kind = dev?._voiceDiagKind || 'device'; + const deviceLabel = dev?.name || dev?.mac || (kind === 'dect-base' ? 'DECT base' : 'Phone'); + const switchName = dev?.meraki?.switchName || dev?.meraki?.deviceName || '—'; + const portName = dev?.meraki?.portName || dev?.meraki?.port || '—'; + return { + kind, + deviceLabel, + switchName, + portName, + portLabel: `${switchName}:${portName}`, + mac: dev?.mac || null, + }; +} diff --git a/services/voiceDiag/checks/port/portEnabled.js b/services/voiceDiag/checks/port/portEnabled.js new file mode 100644 index 0000000..91c6fd4 --- /dev/null +++ b/services/voiceDiag/checks/port/portEnabled.js @@ -0,0 +1,99 @@ +// src/services/voiceDiag/checks/port/portEnabled.js +// +// Store phone standard: administratively enabled on every wired- +// phone switchport. If the port is admin-disabled the phone +// obviously won't work — this is a common footgun when a tech +// disables the wrong port during troubleshooting. +// +// Env kill-switch (shared with the other port checks) — +// VOICE_STANDARD_ENABLED=false → check reports skipped +// so operators can silence all port-hygiene noise while the +// underlying Meraki state is being cleaned up. +// +// Skip rules: +// - Wireless: skipped. +// - Trunk uplink: skipped (owned by portType check). +// - No portEnabled field at all: reported as unknown. + +import { collectPortDevices, isWireless, portLabelFor, maybeSkippedByKillSwitch } from './_helpers.js'; + +export const PORT_ENABLED_STANDARDS = Object.freeze({ + portEnabled: true, +}); + +export const portEnabledCheck = { + id: 'portEnabled', + label: 'Switchport Admin State', + requires: ['phoneStatus'], + scope: null, + standards: PORT_ENABLED_STANDARDS, + + async run(ctx) { + const skip = maybeSkippedByKillSwitch(portEnabledCheck); + if (skip) return skip; + const devices = collectPortDevices(ctx.phoneStatus); + if (devices.length === 0) { + return { + status: 'skipped', + message: 'No wired devices with Meraki port data to inspect.', + details: null, + remediation: null, + }; + } + + const perDevice = devices.map((d) => { + if (isWireless(d)) { + return { ...portLabelFor(d), portEnabled: null, verdict: 'wireless' }; + } + const portType = d.meraki?.portType; + if (portType && String(portType).toLowerCase() === 'trunk') { + return { ...portLabelFor(d), portEnabled: null, verdict: 'trunk-skip' }; + } + const enabled = d.meraki?.portEnabled; + if (enabled === undefined || enabled === null) { + return { ...portLabelFor(d), portEnabled: null, verdict: 'unknown' }; + } + return { ...portLabelFor(d), portEnabled: !!enabled, verdict: enabled ? 'compliant' : 'disabled' }; + }); + + const disabled = perDevice.filter((r) => r.verdict === 'disabled'); + const unknown = perDevice.filter((r) => r.verdict === 'unknown'); + + const details = { + total: perDevice.length, + compliant: perDevice.filter((r) => r.verdict === 'compliant').length, + disabled: disabled.length, + unknown: unknown.length, + wireless: perDevice.filter((r) => r.verdict === 'wireless').length, + trunkSkipped: perDevice.filter((r) => r.verdict === 'trunk-skip').length, + offenders: disabled, + }; + + if (disabled.length > 0) { + return { + status: 'error', + message: + `${disabled.length} device(s) on ADMIN-DISABLED switchports (phone will not work): ` + + disabled.map((r) => `${r.deviceLabel}@${r.portLabel}`).join(', '), + details, + remediation: null, + }; + } + + if (unknown.length > 0) { + return { + status: 'warn', + message: `${unknown.length} device(s) have no admin-state info from Meraki.`, + details, + remediation: null, + }; + } + + return { + status: 'ok', + message: `All ${details.compliant} wired switchports admin-enabled.`, + details, + remediation: null, + }; + }, +}; diff --git a/services/voiceDiag/checks/port/portPoe.js b/services/voiceDiag/checks/port/portPoe.js new file mode 100644 index 0000000..5628c00 --- /dev/null +++ b/services/voiceDiag/checks/port/portPoe.js @@ -0,0 +1,96 @@ +// src/services/voiceDiag/checks/port/portPoe.js +// +// Store phone standard: PoE enabled on every wired-phone switchport. +// Store desk phones + DECT basestations are all PoE-powered; PoE +// off on the port is either a mis-provisioned port or a phone that +// won't come up after the next power cycle. +// +// Skip rules: +// - Wireless: skipped. +// - Trunk uplink: skipped (portType check owns that warning; PoE +// policy for a downstream Cisco switch isn't ours to evaluate). +// - No poeEnabled field at all (Meraki didn't return it or the +// switch model doesn't do PoE): reported as unknown. + +import { collectPortDevices, isWireless, portLabelFor, maybeSkippedByKillSwitch } from './_helpers.js'; + +export const PORT_POE_STANDARDS = Object.freeze({ + poeEnabled: true, +}); + +export const portPoeCheck = { + id: 'portPoe', + label: 'Switchport PoE', + requires: ['phoneStatus'], + scope: null, + standards: PORT_POE_STANDARDS, + + async run(ctx) { + const skip = maybeSkippedByKillSwitch(portPoeCheck); + if (skip) return skip; + const devices = collectPortDevices(ctx.phoneStatus); + if (devices.length === 0) { + return { + status: 'skipped', + message: 'No wired devices with Meraki port data to inspect.', + details: null, + remediation: null, + }; + } + + const perDevice = devices.map((d) => { + if (isWireless(d)) { + return { ...portLabelFor(d), poeEnabled: null, verdict: 'wireless' }; + } + const portType = d.meraki?.portType; + if (portType && String(portType).toLowerCase() === 'trunk') { + return { ...portLabelFor(d), poeEnabled: null, verdict: 'trunk-skip' }; + } + const poe = d.meraki?.poeEnabled; + if (poe === undefined || poe === null) { + return { ...portLabelFor(d), poeEnabled: null, verdict: 'unknown' }; + } + return { ...portLabelFor(d), poeEnabled: !!poe, verdict: poe ? 'compliant' : 'off' }; + }); + + const off = perDevice.filter((r) => r.verdict === 'off'); + const unknown = perDevice.filter((r) => r.verdict === 'unknown'); + + const details = { + total: perDevice.length, + compliant: perDevice.filter((r) => r.verdict === 'compliant').length, + off: off.length, + unknown: unknown.length, + wireless: perDevice.filter((r) => r.verdict === 'wireless').length, + trunkSkipped: perDevice.filter((r) => r.verdict === 'trunk-skip').length, + offenders: off, + }; + + if (off.length > 0) { + return { + status: 'warn', + message: + `${off.length} device(s) on switchports with PoE DISABLED: ` + + off.map((r) => `${r.deviceLabel}@${r.portLabel}`).join(', '), + details, + remediation: null, + }; + } + + if (unknown.length > 0) { + return { + status: 'warn', + message: `${unknown.length} device(s) have no PoE info from Meraki.`, + details, + remediation: null, + }; + } + + return { + status: 'ok', + message: `All ${details.compliant} wired device(s) on PoE-enabled ports.`, + details, + remediation: null, + }; + }, +}; diff --git a/services/voiceDiag/checks/port/portType.js b/services/voiceDiag/checks/port/portType.js new file mode 100644 index 0000000..92f3c44 --- /dev/null +++ b/services/voiceDiag/checks/port/portType.js @@ -0,0 +1,103 @@ +// src/services/voiceDiag/checks/port/portType.js +// +// Store phone standard: every desk phone / DECT base is connected to +// a Meraki switch on an ACCESS port. When a phone shows up on a +// TRUNK port, that means the phone is downstream through a non- +// Meraki switch (typically a Cisco stack in a store), and our +// per-port config visibility ends at the trunk uplink. We can't +// enforce anything past that point — this check surfaces the +// visibility gap so the operator can go check the Cisco side manually. +// +// Data source: `ctx.phoneStatus.phones.data[i].meraki.portType` and +// `.dectBasestations[i].meraki.portType`, populated by the Meraki +// enrichment layer from `/devices/{serial}/switch/ports.type`. +// +// No API calls. No remediation — Meraki port-config PUT is a +// separate scope of work; the operator handles trunk-side changes +// in Meraki Dashboard. +// +// Skip rules: +// - Wireless devices (no switchport): skipped silently. +// - Devices with no meraki.portType at all (Meraki didn't match +// the client to a port): skipped and mentioned in details. + +import { collectPortDevices, isWireless, portLabelFor, maybeSkippedByKillSwitch } from './_helpers.js'; + +export const PORT_TYPE_STANDARDS = Object.freeze({ + portType: 'access', +}); + +export const portTypeCheck = { + id: 'portType', + label: 'Switchport Type', + requires: ['phoneStatus'], + scope: null, + standards: PORT_TYPE_STANDARDS, + + async run(ctx) { + const skip = maybeSkippedByKillSwitch(portTypeCheck); + if (skip) return skip; + const devices = collectPortDevices(ctx.phoneStatus); + if (devices.length === 0) { + return { + status: 'skipped', + message: 'No wired devices with Meraki port data to inspect.', + details: null, + remediation: null, + }; + } + + const perDevice = devices.map((d) => { + if (isWireless(d)) { + return { ...portLabelFor(d), portType: null, verdict: 'wireless' }; + } + const t = d.meraki?.portType || null; + if (!t) { + return { ...portLabelFor(d), portType: null, verdict: 'unknown' }; + } + const verdict = t.toLowerCase() === PORT_TYPE_STANDARDS.portType ? 'compliant' : 'trunk'; + return { ...portLabelFor(d), portType: t, verdict }; + }); + + const trunks = perDevice.filter((r) => r.verdict === 'trunk'); + const unknown = perDevice.filter((r) => r.verdict === 'unknown'); + + const details = { + total: perDevice.length, + compliant: perDevice.filter((r) => r.verdict === 'compliant').length, + trunks: trunks.length, + unknown: unknown.length, + wireless: perDevice.filter((r) => r.verdict === 'wireless').length, + offenders: [...trunks, ...unknown], + }; + + if (trunks.length > 0) { + return { + status: 'warn', + message: + `${trunks.length} device(s) on TRUNK uplinks — downstream config not visible. ` + + trunks.map((t) => `${t.deviceLabel}@${t.portLabel}`).join(', ') + + `. Verify per-port config on the Cisco side manually.`, + details, + remediation: null, + }; + } + + if (unknown.length > 0) { + return { + status: 'warn', + message: + `${unknown.length} device(s) have no Meraki port information — either not matched or port not indexed.`, + details, + remediation: null, + }; + } + + return { + status: 'ok', + message: `All ${perDevice.filter((r) => r.verdict === 'compliant').length} wired device(s) on access ports.`, + details, + remediation: null, + }; + }, +}; diff --git a/services/voiceDiag/checks/port/portVlan.js b/services/voiceDiag/checks/port/portVlan.js new file mode 100644 index 0000000..978ac31 --- /dev/null +++ b/services/voiceDiag/checks/port/portVlan.js @@ -0,0 +1,121 @@ +// src/services/voiceDiag/checks/port/portVlan.js +// +// Store phone standard: every desk phone / DECT base is tagged into +// a specific VLAN (currently 102 for AE, on the data-side of the +// port; may migrate to voice VLAN in the future — hence the env- +// configurable target and the "either side" comparison below). +// +// The check compares `client.vlan` (the actual VLAN the phone is +// seeing) against the standard rather than the port's `voiceVlan` +// or `dataVlan` configuration alone — this makes it correct today +// (data VLAN carries voice) and correct tomorrow if we move phones +// onto a proper voice VLAN, without touching the check. +// +// Env override: +// VOICE_STANDARD_PHONE_VLAN=102 (default 102) +// +// Skip rules: +// - Wireless: skipped. +// - Trunk uplink: skipped (portType check owns the trunk warning). +// - No VLAN info at all: reported as unknown. +// +// No remediation — VLAN misassignment is a Meraki port-config +// change out of scope for this round. + +import { collectPortDevices, isWireless, portLabelFor, maybeSkippedByKillSwitch } from './_helpers.js'; + +/** Configurable per env — falls back to 102 if unset or non-numeric. */ +function getExpectedVlan() { + const raw = process.env.VOICE_STANDARD_PHONE_VLAN; + const parsed = raw ? Number(raw) : 102; + if (!Number.isFinite(parsed) || parsed < 0 || parsed > 4095) return 102; + return parsed; +} + +// Exported for the standards reference table + tests. Read at call +// time so tests can mutate process.env without needing to reset +// module state. +export const PORT_VLAN_STANDARDS = Object.freeze({ + get vlan() { return getExpectedVlan(); }, + envKey: 'VOICE_STANDARD_PHONE_VLAN', +}); + +export const portVlanCheck = { + id: 'portVlan', + label: 'Switchport VLAN', + requires: ['phoneStatus'], + scope: null, + standards: PORT_VLAN_STANDARDS, + + async run(ctx) { + const skip = maybeSkippedByKillSwitch(portVlanCheck); + if (skip) return skip; + const expected = getExpectedVlan(); + const devices = collectPortDevices(ctx.phoneStatus); + if (devices.length === 0) { + return { + status: 'skipped', + message: 'No wired devices with Meraki port data to inspect.', + details: null, + remediation: null, + }; + } + + const perDevice = devices.map((d) => { + if (isWireless(d)) { + return { ...portLabelFor(d), vlan: null, verdict: 'wireless' }; + } + const portType = d.meraki?.portType; + if (portType && String(portType).toLowerCase() === 'trunk') { + return { ...portLabelFor(d), vlan: d.meraki?.vlan ?? null, verdict: 'trunk-skip' }; + } + const vlan = d.meraki?.vlan; + if (vlan === undefined || vlan === null || vlan === '') { + return { ...portLabelFor(d), vlan: null, verdict: 'unknown' }; + } + const num = Number(vlan); + const verdict = num === expected ? 'compliant' : 'wrong'; + return { ...portLabelFor(d), vlan: num, verdict }; + }); + + const wrong = perDevice.filter((r) => r.verdict === 'wrong'); + const unknown = perDevice.filter((r) => r.verdict === 'unknown'); + + const details = { + expectedVlan: expected, + total: perDevice.length, + compliant: perDevice.filter((r) => r.verdict === 'compliant').length, + wrong: wrong.length, + unknown: unknown.length, + wireless: perDevice.filter((r) => r.verdict === 'wireless').length, + trunkSkipped: perDevice.filter((r) => r.verdict === 'trunk-skip').length, + offenders: wrong, + }; + + if (wrong.length > 0) { + const worstLine = wrong.map((w) => `${w.deviceLabel}@${w.portLabel} (VLAN ${w.vlan})`).join(', '); + return { + status: 'warn', + message: `${wrong.length} device(s) on the wrong VLAN (expected ${expected}): ${worstLine}.`, + details, + remediation: null, + }; + } + + if (unknown.length > 0) { + return { + status: 'warn', + message: `${unknown.length} device(s) have no VLAN info from Meraki.`, + details, + remediation: null, + }; + } + + return { + status: 'ok', + message: `All ${details.compliant} wired device(s) on VLAN ${expected}.`, + details, + remediation: null, + }; + }, +}; diff --git a/services/voiceDiag/checks/voicemail.js b/services/voiceDiag/checks/voicemail.js index 0b1f09d..8b0bfa6 100644 --- a/services/voiceDiag/checks/voicemail.js +++ b/services/voiceDiag/checks/voicemail.js @@ -1,22 +1,23 @@ // src/services/voiceDiag/checks/voicemail.js // -// Detects voicemail configuration for the store user. Voicemail has -// enough sub-facets that we surface the current settings without -// offering a one-click remediation — the "right" answer for a store -// is site-specific (some sites disable VM entirely and forward busy -// to the AA, others rely on it as the noAnswer target). We flag two -// classes of finding: +// Detects voicemail configuration for the store user. Store-line +// standard is: // -// - **error**: enabled=true AND no PIN set — the user cannot pick -// up messages, and every caller who reaches VM will be dumped -// into the "please set your PIN" prompt. -// - **warn**: enabled=true AND a forward-to-email target is -// configured that doesn't look like an @ae.com address — mail -// forwarding of voicemail off-org is worth double-checking. -// - **ok**: everything else. +// - voicemail ENABLED (so the user can access the box directly if +// they ever need to), but +// - NO call should ever be routed to voicemail — the three +// "send to VM" triggers (sendAllCalls / sendBusyCalls / +// sendUnansweredCalls) must all be OFF. +// - MWI (message-waiting indicator) on so any manually-left +// messages light up the phone. // -// Endpoint: GET /v1/people/{personId}/features/voicemail -// Scope: spark-admin:people_read +// This is stricter than the initial cut. The rationale: at AE, a +// store phone that goes to VM is a lost customer call. Better for +// the phone to keep ringing (or drop) than to silently swallow the +// call into a VM box nobody checks. +// +// Endpoint: GET/PUT /v1/people/{personId}/features/voicemail +// Scope: spark-admin:people_read (GET) + spark-admin:people_write (PUT). // Response shape (relevant fields): // { // enabled: true, @@ -34,23 +35,32 @@ // faxMessage: { ... } // } // -// Webex doesn't currently expose PIN-set status via the public API, -// so "PIN set" is inferred pragmatically: we check whether the person -// has ever accessed their voicemail (via passcode lastChanged if the -// endpoint returns it) or, as a fallback, we simply note that PIN -// state is unknown and treat it as informational only. See the note -// inline below. +// PUT semantics: partial updates work — you can send just the +// send-*Calls sub-blocks and the other fields are preserved. +// Remediation takes advantage of that to make a minimal-touch PUT. + +import { logger } from '../../../utils/logger.js'; +import { describeRequester } from '../../../utils/requester.js'; const ENDPOINT = (personId) => `people/${personId}/features/voicemail`; const AE_EMAIL_RE = /@ae\.com$/i; +export const VOICEMAIL_STANDARDS = Object.freeze({ + enabled: true, + sendAllCalls: { enabled: false }, + sendBusyCalls: { enabled: false }, + sendUnansweredCalls: { enabled: false }, + messageStorage: { mwiEnabled: true }, +}); + export const voicemailCheck = { id: 'voicemail', label: 'Voicemail', requires: ['personId'], scope: 'spark-admin:people_read', + standards: VOICEMAIL_STANDARDS, async run(ctx) { const data = await ctx.webex.request('GET', ENDPOINT(ctx.personId)); @@ -63,6 +73,15 @@ export const voicemailCheck = { const externalEmail = storage?.externalEmail || ''; const forwardEmailTarget = emailCopy?.enabled ? (emailCopy?.emailId || '') : ''; + const sendAllCallsEnabled = !!data?.sendAllCalls?.enabled; + const sendBusyCallsEnabled = !!data?.sendBusyCalls?.enabled; + const sendUnansweredCallsEnabled = !!data?.sendUnansweredCalls?.enabled; + + const activeSendTriggers = []; + if (sendAllCallsEnabled) activeSendTriggers.push('sendAllCalls'); + if (sendBusyCallsEnabled) activeSendTriggers.push('sendBusyCalls'); + if (sendUnansweredCallsEnabled) activeSendTriggers.push('sendUnansweredCalls'); + const details = { enabled, mwiEnabled, @@ -72,31 +91,62 @@ export const voicemailCheck = { emailCopyTarget: forwardEmailTarget, transferToEnabled: !!transferTo?.enabled, transferToDestination: transferTo?.destination || null, - sendAllCallsEnabled: !!data?.sendAllCalls?.enabled, - sendBusyCallsEnabled: !!data?.sendBusyCalls?.enabled, - sendUnansweredCallsEnabled: !!data?.sendUnansweredCalls?.enabled, + sendAllCallsEnabled, + sendBusyCallsEnabled, + sendUnansweredCallsEnabled, + activeSendTriggers, }; + // Hard rule #1: VM enabled at all. If VM is disabled we surface + // it as a warn rather than an error — some sites intentionally + // disable VM and route unanswered calls elsewhere, so we don't + // auto-remediate. if (!enabled) { return { - status: 'ok', + status: 'warn', message: 'Voicemail is disabled — callers will not be able to leave messages.', details, remediation: null, }; } - // sendAllCalls silently swallowing every inbound call is almost - // always a mistake — surface it loudly. - if (details.sendAllCallsEnabled) { + // Hard rule #2: none of the three send-to-VM triggers may be + // active. This is the store-line standard: VM exists so the + // operator can check the box manually, but no call ever gets + // silently swallowed into it. + if (activeSendTriggers.length > 0) { + const humanTriggers = activeSendTriggers + .map((k) => k.replace(/([A-Z])/g, ' $1').toLowerCase().trim()) + .join(', '); return { status: 'error', - message: 'Voicemail is enabled AND "send all calls to voicemail" is on — the phone will never ring.', + message: + `Voicemail is receiving calls: ${humanTriggers} ${activeSendTriggers.length === 1 ? 'trigger is' : 'triggers are'} active. ` + + `Store phone standard requires voicemail be reachable manually but never used as a call target.`, details, - remediation: null, + remediation: { + action: 'stop_sending_to_voicemail', + title: 'Stop sending calls to voicemail', + summary: + `Disable ${activeSendTriggers.length} VM trigger(s) for ${ctx.personLabel} so incoming calls stop being routed to voicemail. ` + + `Voicemail itself stays enabled.`, + payload: { + personId: ctx.personId, + personLabel: ctx.personLabel, + storeNum: ctx.storeNum, + activeSendTriggers, + before: { + sendAllCallsEnabled, + sendBusyCallsEnabled, + sendUnansweredCallsEnabled, + }, + }, + }, }; } + // Soft rules below — no auto-remediation, just visibility. + if (!mwiEnabled) { return { status: 'warn', @@ -126,13 +176,66 @@ export const voicemailCheck = { return { status: 'ok', - message: 'Voicemail is enabled with MWI on and no unusual forwarding.', + message: 'Voicemail is enabled, no send-to-VM triggers active, MWI on. Compliant.', details, remediation: null, }; }, - // No remediations — voicemail policy is too site-specific for a - // one-size-fits-all button. The renderer surfaces the details so - // the operator can act via Control Hub if they want. + remediations: { + async stop_sending_to_voicemail(bot, data, requester) { + const { personId, personLabel, storeNum, activeSendTriggers, before } = data; + + if (!Array.isArray(activeSendTriggers) || activeSendTriggers.length === 0) { + await bot.say( + 'markdown', + `⚠️ No send-to-VM triggers to clear for **${personLabel}** — nothing to do.`, + ); + return; + } + + logger( + 'voicediag:audit', + `CONFIRMED stop_sending_to_voicemail for ${personLabel} (person=${personId}, store=${storeNum}) ` + + `by ${describeRequester(requester)} — clearing=${activeSendTriggers.join(',')}, ` + + `before=${JSON.stringify(before)}`, + ); + + try { + const { default: webex } = await import('../../../integrations/webex/WebexClient.js'); + // Minimal PUT: only the three send-* sub-blocks. Voicemail + // itself stays enabled + all other config (greetings, + // storage, notifications) is preserved by Webex's partial- + // update semantics on this endpoint. + const body = { + sendAllCalls: { ...VOICEMAIL_STANDARDS.sendAllCalls }, + sendBusyCalls: { ...VOICEMAIL_STANDARDS.sendBusyCalls }, + sendUnansweredCalls: { ...VOICEMAIL_STANDARDS.sendUnansweredCalls }, + }; + await webex.request('PUT', ENDPOINT(personId), body); + } catch (err) { + logger( + 'voicediag:audit', + `FAILED stop_sending_to_voicemail for ${personLabel}: ${err.message}`, + 'error', + ); + await bot.say( + 'markdown', + `❌ Failed to stop send-to-VM for **${personLabel}**: ${err.message}`, + ); + return; + } + + await bot.say( + 'markdown', + `✅ Voicemail send triggers cleared for **${personLabel}** (store ${storeNum}): ${activeSendTriggers.join(', ')}. ` + + `Voicemail is still enabled but no calls will be routed to it. ` + + `Re-run \`/voicediag ${storeNum}\` to verify.`, + ); + logger( + 'voicediag:audit', + `COMPLETED stop_sending_to_voicemail for ${personLabel} (store ${storeNum}) — cleared=${activeSendTriggers.join(',')}`, + ); + }, + }, }; diff --git a/tests/voiceDiag.checks.test.js b/tests/voiceDiag.checks.test.js index 42506af..42e048d 100644 --- a/tests/voiceDiag.checks.test.js +++ b/tests/voiceDiag.checks.test.js @@ -114,7 +114,9 @@ test('callForwarding: nothing enabled → ok', async () => { assert.equal(r.remediation, null); }); -test('callForwarding: always-forwarding enabled → warn + compound remediation', async () => { +test('callForwarding: always-forwarding enabled → error + compound remediation', async () => { + // Severity upgraded from warn → error per store-line standard + // (any forwarding active on a store line = missed customer calls). const ctx = mkCtx({ response: { callForwarding: { @@ -125,10 +127,11 @@ test('callForwarding: always-forwarding enabled → warn + compound remediation' }, }); const r = await callForwardingCheck.run(ctx); - assert.equal(r.status, 'warn'); + assert.equal(r.status, 'error'); assert.equal(r.remediation.action, 'clear_call_forwarding'); assert.deepEqual(r.remediation.payload.variantsToClear, ['always']); assert.match(r.message, /\+18005551212/); + assert.match(r.message, /Store phone standard/); }); test('callForwarding: multiple variants active → single compound remediation', async () => { @@ -142,7 +145,7 @@ test('callForwarding: multiple variants active → single compound remediation', }, }); const r = await callForwardingCheck.run(ctx); - assert.equal(r.status, 'warn'); + assert.equal(r.status, 'error'); assert.deepEqual(r.remediation.payload.variantsToClear, ['always', 'busy']); assert.match(r.remediation.title, /2 forwarding variants/); }); @@ -176,29 +179,60 @@ test('callWaiting: 404 → skipped via runner', async () => { // ─── voicemail ────────────────────────────────────────────────────── -test('voicemail: disabled → ok', async () => { +test('voicemail: disabled → warn (callers can\'t leave messages)', async () => { + // Standard is enabled=true; disabled surfaces as warn, no + // auto-remediation (some sites intentionally disable it). const ctx = mkCtx({ response: { enabled: false } }); const r = await voicemailCheck.run(ctx); - assert.equal(r.status, 'ok'); + assert.equal(r.status, 'warn'); + assert.equal(r.remediation, null); + assert.match(r.message, /Voicemail is disabled/); }); -test('voicemail: sendAllCalls on → error', async () => { +test('voicemail: sendAllCalls on → error + stop_sending_to_voicemail remediation', async () => { const ctx = mkCtx({ response: { enabled: true, sendAllCalls: { enabled: true }, + sendBusyCalls: { enabled: false }, + sendUnansweredCalls: { enabled: false }, messageStorage: { mwiEnabled: true }, }, }); const r = await voicemailCheck.run(ctx); assert.equal(r.status, 'error'); - assert.match(r.message, /never ring/); + assert.equal(r.remediation.action, 'stop_sending_to_voicemail'); + assert.deepEqual(r.remediation.payload.activeSendTriggers, ['sendAllCalls']); + assert.equal(r.remediation.payload.before.sendAllCallsEnabled, true); + assert.match(r.message, /Voicemail is receiving calls/); }); -test('voicemail: enabled but MWI off → warn', async () => { +test('voicemail: sendBusyCalls + sendUnansweredCalls both on → error, all triggers listed', async () => { const ctx = mkCtx({ response: { enabled: true, + sendAllCalls: { enabled: false }, + sendBusyCalls: { enabled: true }, + sendUnansweredCalls: { enabled: true }, + messageStorage: { mwiEnabled: true }, + }, + }); + const r = await voicemailCheck.run(ctx); + assert.equal(r.status, 'error'); + assert.deepEqual( + r.remediation.payload.activeSendTriggers.sort(), + ['sendBusyCalls', 'sendUnansweredCalls'], + ); + assert.match(r.message, /2 triggers are active|triggers are active/); +}); + +test('voicemail: enabled but MWI off → warn (no remediation, still soft signal)', async () => { + const ctx = mkCtx({ + response: { + enabled: true, + sendAllCalls: { enabled: false }, + sendBusyCalls: { enabled: false }, + sendUnansweredCalls: { enabled: false }, messageStorage: { mwiEnabled: false }, }, }); @@ -211,6 +245,9 @@ test('voicemail: enabled, email forward to off-org → warn', async () => { const ctx = mkCtx({ response: { enabled: true, + sendAllCalls: { enabled: false }, + sendBusyCalls: { enabled: false }, + sendUnansweredCalls: { enabled: false }, messageStorage: { mwiEnabled: true }, emailCopyOfMessage: { enabled: true, emailId: 'random@gmail.com' }, }, @@ -220,10 +257,13 @@ test('voicemail: enabled, email forward to off-org → warn', async () => { assert.match(r.message, /off-org/); }); -test('voicemail: enabled + MWI + no unusual forwarding → ok', async () => { +test('voicemail: enabled + MWI + no send-to-VM + no unusual forwarding → ok', async () => { const ctx = mkCtx({ response: { enabled: true, + sendAllCalls: { enabled: false }, + sendBusyCalls: { enabled: false }, + sendUnansweredCalls: { enabled: false }, messageStorage: { mwiEnabled: true }, emailCopyOfMessage: { enabled: false, emailId: '' }, }, @@ -274,11 +314,13 @@ test('hoteling: disabled → ok', async () => { assert.equal(r.remediation, null); }); -test('hoteling: enabled → warn (info only, no remediation)', async () => { +test('hoteling: enabled → warn + disable_hoteling remediation', async () => { const ctx = mkCtx({ response: { enabled: true } }); const r = await hotelingCheck.run(ctx); assert.equal(r.status, 'warn'); - assert.equal(r.remediation, null); + assert.equal(r.remediation.action, 'disable_hoteling'); + assert.equal(r.remediation.payload.personId, 'PID_TEST'); + assert.equal(r.remediation.payload.before.enabled, true); }); test('hoteling: 404 → skipped via runner', async () => { @@ -490,3 +532,288 @@ test('buildRemediationRegistry: contains every declared remediation exactly once assert.equal(registry.get(rid).check.id, checkId); } }); + +// ─── standards regression guards ─────────────────────────────────── +// The "voice standards" reference table in README.md is generated +// off the `standards` field on each check descriptor. If a check +// forgets to declare it, we lose visibility of what the desired +// state actually is. Fail loud instead of silently dropping the row. + +test('every registered check exposes a standards object', () => { + const missing = CHECKS.filter((c) => !c.standards || typeof c.standards !== 'object'); + assert.deepEqual( + missing.map((c) => c.id), + [], + `checks without .standards: ${missing.map((c) => c.id).join(', ')}`, + ); +}); + +test('standards for DND / callWaiting / callIntercept encode the expected boolean shape', () => { + const byId = new Map(CHECKS.map((c) => [c.id, c])); + assert.equal(byId.get('dnd').standards.enabled, false); + assert.equal(byId.get('callWaiting').standards.enabled, true); + assert.equal(byId.get('callIntercept').standards.enabled, false); +}); + +test('standards for callForwarding declare all three variants off', () => { + const byId = new Map(CHECKS.map((c) => [c.id, c])); + const cf = byId.get('callForwarding').standards; + assert.equal(cf.always.enabled, false); + assert.equal(cf.busy.enabled, false); + assert.equal(cf.noAnswer.enabled, false); +}); + +test('standards for voicemail encode enabled + no send-to-VM triggers + MWI on', () => { + const byId = new Map(CHECKS.map((c) => [c.id, c])); + const vm = byId.get('voicemail').standards; + assert.equal(vm.enabled, true); + assert.equal(vm.sendAllCalls.enabled, false); + assert.equal(vm.sendBusyCalls.enabled, false); + assert.equal(vm.sendUnansweredCalls.enabled, false); + assert.equal(vm.messageStorage.mwiEnabled, true); +}); + +// ─── port-hygiene bucket ─────────────────────────────────────────── +// Each of these tests injects a phoneStatus snapshot shaped like +// collectPhoneStatus() output, focussed on the fields the port check +// consumes. + +async function loadPortChecks() { + const [ + { portTypeCheck, PORT_TYPE_STANDARDS }, + { portVlanCheck, PORT_VLAN_STANDARDS }, + { portPoeCheck, PORT_POE_STANDARDS }, + { portEnabledCheck, PORT_ENABLED_STANDARDS }, + ] = await Promise.all([ + import('../services/voiceDiag/checks/port/portType.js'), + import('../services/voiceDiag/checks/port/portVlan.js'), + import('../services/voiceDiag/checks/port/portPoe.js'), + import('../services/voiceDiag/checks/port/portEnabled.js'), + ]); + return { + portTypeCheck, PORT_TYPE_STANDARDS, + portVlanCheck, PORT_VLAN_STANDARDS, + portPoeCheck, PORT_POE_STANDARDS, + portEnabledCheck, PORT_ENABLED_STANDARDS, + }; +} + +function mkPortCtx(phones = [], dectBasestations = []) { + return { + ...mkCtx(), + phoneStatus: { + phones: { data: phones }, + dectBasestations, + }, + }; +} + +test('portType: all access ports → ok', async () => { + const { portTypeCheck } = await loadPortChecks(); + const ctx = mkPortCtx( + [{ mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', switchName: 'sw1', portName: 'p1', connectionType: 'Wired' } }], + [{ mac: 'bb:bb', name: 'B1', meraki: { portType: 'access', switchName: 'sw1', portName: 'p3', connectionType: 'Wired' } }], + ); + const r = await portTypeCheck.run(ctx); + assert.equal(r.status, 'ok'); + assert.equal(r.details.compliant, 2); + assert.equal(r.details.trunks, 0); +}); + +test('portType: trunk uplink → warn, offender named', async () => { + const { portTypeCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', switchName: 'sw1', portName: 'p1', connectionType: 'Wired' } }, + { mac: 'cc:cc', name: 'P2', meraki: { portType: 'trunk', switchName: 'sw1', portName: 'p2', connectionType: 'Wired' } }, + ]); + const r = await portTypeCheck.run(ctx); + assert.equal(r.status, 'warn'); + assert.equal(r.details.trunks, 1); + assert.match(r.message, /TRUNK/); + assert.equal(r.details.offenders[0].deviceLabel, 'P2'); +}); + +test('portType: wireless phone → skipped from compliance count (no switchport)', async () => { + const { portTypeCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { connectionType: 'Wireless' } }, + ]); + const r = await portTypeCheck.run(ctx); + // With one device that's wireless, no compliant / trunks / unknown + // → we land in the "all compliant" branch with compliant=0. + assert.equal(r.status, 'ok'); + assert.equal(r.details.wireless, 1); +}); + +test('portType: no devices at all → skipped', async () => { + const { portTypeCheck } = await loadPortChecks(); + const ctx = mkPortCtx([], []); + const r = await portTypeCheck.run(ctx); + assert.equal(r.status, 'skipped'); +}); + +test('portVlan: expected VLAN from env, mismatch → warn', async () => { + const prev = process.env.VOICE_STANDARD_PHONE_VLAN; + process.env.VOICE_STANDARD_PHONE_VLAN = '102'; + try { + const { portVlanCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', vlan: 102, switchName: 'sw1', portName: 'p1', connectionType: 'Wired' } }, + { mac: 'bb:bb', name: 'P2', meraki: { portType: 'access', vlan: 1, switchName: 'sw1', portName: 'p2', connectionType: 'Wired' } }, + ]); + const r = await portVlanCheck.run(ctx); + assert.equal(r.status, 'warn'); + assert.equal(r.details.expectedVlan, 102); + assert.equal(r.details.wrong, 1); + assert.match(r.message, /expected 102/); + assert.match(r.message, /P2.*VLAN 1/); + } finally { + if (prev === undefined) delete process.env.VOICE_STANDARD_PHONE_VLAN; + else process.env.VOICE_STANDARD_PHONE_VLAN = prev; + } +}); + +test('portVlan: env override — VOICE_STANDARD_PHONE_VLAN=200 makes 200 compliant', async () => { + const prev = process.env.VOICE_STANDARD_PHONE_VLAN; + process.env.VOICE_STANDARD_PHONE_VLAN = '200'; + try { + const { portVlanCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', vlan: 200, switchName: 'sw', portName: 'p1', connectionType: 'Wired' } }, + ]); + const r = await portVlanCheck.run(ctx); + assert.equal(r.status, 'ok'); + assert.equal(r.details.expectedVlan, 200); + assert.equal(r.details.compliant, 1); + } finally { + if (prev === undefined) delete process.env.VOICE_STANDARD_PHONE_VLAN; + else process.env.VOICE_STANDARD_PHONE_VLAN = prev; + } +}); + +test('portVlan: trunk ports get skipped (owned by portType check)', async () => { + const { portVlanCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', vlan: 102, switchName: 'sw', portName: 'p1', connectionType: 'Wired' } }, + { mac: 'bb:bb', name: 'P2', meraki: { portType: 'trunk', vlan: 999, switchName: 'sw', portName: 'p2', connectionType: 'Wired' } }, + ]); + const r = await portVlanCheck.run(ctx); + assert.equal(r.status, 'ok'); + assert.equal(r.details.trunkSkipped, 1); +}); + +test('portPoe: PoE off on an access port → warn', async () => { + const { portPoeCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', poeEnabled: true, switchName: 'sw', portName: 'p1', connectionType: 'Wired' } }, + { mac: 'bb:bb', name: 'P2', meraki: { portType: 'access', poeEnabled: false, switchName: 'sw', portName: 'p2', connectionType: 'Wired' } }, + ]); + const r = await portPoeCheck.run(ctx); + assert.equal(r.status, 'warn'); + assert.equal(r.details.off, 1); + assert.match(r.message, /PoE DISABLED/); +}); + +test('portEnabled: admin-disabled port → error (phone won\'t work)', async () => { + const { portEnabledCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', name: 'P1', meraki: { portType: 'access', portEnabled: true, switchName: 'sw', portName: 'p1', connectionType: 'Wired' } }, + { mac: 'bb:bb', name: 'P2', meraki: { portType: 'access', portEnabled: false, switchName: 'sw', portName: 'p2', connectionType: 'Wired' } }, + ]); + const r = await portEnabledCheck.run(ctx); + assert.equal(r.status, 'error'); + assert.equal(r.details.disabled, 1); +}); + +test('port kill-switch: VOICE_STANDARD_ENABLED=false → all four port checks return skipped', async () => { + const prev = process.env.VOICE_STANDARD_ENABLED; + process.env.VOICE_STANDARD_ENABLED = 'false'; + try { + const { portTypeCheck, portVlanCheck, portPoeCheck, portEnabledCheck } = await loadPortChecks(); + const ctx = mkPortCtx([ + { mac: 'aa:aa', meraki: { portType: 'access', vlan: 102, poeEnabled: true, portEnabled: true, switchName: 'sw', portName: 'p1', connectionType: 'Wired' } }, + ]); + for (const chk of [portTypeCheck, portVlanCheck, portPoeCheck, portEnabledCheck]) { + const r = await chk.run(ctx); + assert.equal(r.status, 'skipped', `${chk.id} should be skipped by kill switch`); + assert.match(r.message, /VOICE_STANDARD_ENABLED/); + } + } finally { + if (prev === undefined) delete process.env.VOICE_STANDARD_ENABLED; + else process.env.VOICE_STANDARD_ENABLED = prev; + } +}); + +// ─── combined "apply all N fixes" card ───────────────────────────── + +// bot.say has two supported signatures — `bot.say('markdown', md)` +// (used by simple text posts) and `bot.say({ markdown, attachments })` +// (used by adaptive-card posts). The mock captures the *message +// body* from either shape so per-test assertions can match on +// content rather than mimicking the framework calling convention. +function mkBotMock() { + return { + messages: [], + async say(a, b) { + if (typeof a === 'string' && typeof b === 'string') this.messages.push(b); + else if (a && typeof a === 'object') this.messages.push(a.markdown || JSON.stringify(a)); + else this.messages.push(String(a ?? '')); + }, + }; +} + +test('applyAllVoiceDiagRemediations: iterates entries in sequence, tolerates individual failure', async () => { + const { applyAllVoiceDiagRemediations } = await import('../commands/voiceDiag.js'); + const bot = mkBotMock(); + const requester = { displayName: 'Op Test', email: 'op@example.com' }; + + // Two entries: one whose handler is real (disable_dnd via the + // registry — but that would talk to WebexClient which we can't + // in a test). We rely on the fact that the registered handler is + // called with a payload we can inspect via a mock — but the actual + // handler will throw on WebexClient import. So we test the + // "unknown handler" path (safe) and the audit summary. + + const data = { + combined: true, + storeNum: '782', + personLabel: 'Test Store', + entries: [ + { remediationId: 'nonexistent_handler_1', remediationPayload: {} }, + { remediationId: 'nonexistent_handler_2', remediationPayload: {} }, + ], + }; + + await applyAllVoiceDiagRemediations(bot, data, null, requester); + + // Final summary posted regardless + assert.ok(bot.messages.length >= 1, 'should post final summary'); + const summary = bot.messages[bot.messages.length - 1]; + assert.match(summary, /Apply-all complete/); + assert.match(summary, /Applied: 0 \/ 2/); + assert.match(summary, /Failed: 2/); + assert.match(summary, /nonexistent_handler_1/); + assert.match(summary, /no handler registered/); +}); + +test('applyAllVoiceDiagRemediations: no entries → posts "no pending" hint', async () => { + const { applyAllVoiceDiagRemediations } = await import('../commands/voiceDiag.js'); + const bot = mkBotMock(); + await applyAllVoiceDiagRemediations(bot, { combined: true, storeNum: '782', entries: [] }, null, {}); + assert.equal(bot.messages.length, 1); + assert.match(bot.messages[0], /No pending remediations/); +}); + +test('cancelAllVoiceDiagRemediations: acknowledges the batch was cancelled', async () => { + const { cancelAllVoiceDiagRemediations } = await import('../commands/voiceDiag.js'); + const bot = mkBotMock(); + await cancelAllVoiceDiagRemediations(bot, { + combined: true, + storeNum: '782', + personLabel: 'Store 782', + entries: [{ remediationId: 'a' }, { remediationId: 'b' }, { remediationId: 'c' }], + }, null, { displayName: 'Op' }); + assert.equal(bot.messages.length, 1); + assert.match(bot.messages[0], /Cancelled all pending remediations/); + assert.match(bot.messages[0], /3 fixes not applied/); +});