From 394e3fc9203384131203f72f12edda37e8e00179 Mon Sep 17 00:00:00 2001 From: Robert Helewka Date: Tue, 28 Jul 2026 21:46:16 -0400 Subject: [PATCH] docs: add deployment validation plan and expand env config Add comprehensive deployment and validation plan documenting a staged bring-up approach that gates each layer on its predecessor and defers PSTN testing until everything else is proven. Update .env.example to reflect current configuration: - Replace static API_TOKEN with Casdoor SSO + owner-minted PAT auth - Add Rhema TTS settings with port-collision warning - Add AI Receptionist settings for inbound calls - Reconcile GATEWAY_SIP_PORT to 5060 and default SIP domain --- .env.example | 47 ++++- docs/deployment-validation-plan.md | 315 +++++++++++++++++++++++++++++ services/audio_classifier.py | 6 +- tests/test_hold_slayer.py | 7 +- tests/test_oauth_metadata.py | 7 +- 5 files changed, 372 insertions(+), 10 deletions(-) create mode 100644 docs/deployment-validation-plan.md diff --git a/.env.example b/.env.example index 3cb31b6..bb91307 100644 --- a/.env.example +++ b/.env.example @@ -1,15 +1,32 @@ # ============================================================ # Hold Slayer Gateway Configuration # ============================================================ -# Copy to .env and fill in your values +# Copy to .env and fill in your values. This is the app's own .env for a bare +# `uvicorn main:app` run — see .env.compose.example for the Docker stack. # --- Database (required) --- DATABASE_URL=postgresql+asyncpg://holdslayer:@localhost:5432/holdslayer -# --- API auth (required unless HOST=127.0.0.1) --- -# One static bearer token shared by REST, WebSocket (?token=...), and MCP. -# Generate with: openssl rand -hex 32 -API_TOKEN= +# --- Auth: Casdoor SSO + owner-minted PATs (owner-only) --- +# The browser signs in via Casdoor (short-lived JWT); MCP/CLI clients use +# owner-minted PATs (hs_pat_…). Both resolve to a User gated to OWNER_NAME. +# +# Two supported configurations, enforced at startup: +# 1. CASDOOR_ENABLED=true + endpoint/client_id/client_secret/OWNER_NAME set +# 2. CASDOOR_ENABLED=false + HOST=127.0.0.1 (dev-owner mode, loopback ONLY) +# SSO-off with an off-loopback HOST is refused — it would resolve every request +# to the dev owner, open to the network. +CASDOOR_ENABLED=true +CASDOOR_ENDPOINT=https://id.example.com +CASDOOR_CLIENT_ID= +CASDOOR_CLIENT_SECRET= +CASDOOR_ORG_NAME= +CASDOOR_APP_NAME=hold-slayer +# The owner's Casdoor username — the only identity allowed on any surface. +OWNER_NAME= +# Public base URL the browser reaches (drives OAuth discovery + the Casdoor +# redirect_uri). Blank derives it from the request headers. +PUBLIC_BASE_URL= # --- SIP Trunk --- # The mock engine must be requested explicitly; an unconfigured trunk @@ -26,13 +43,23 @@ SIP_TRUNK_DID=+15551234567 # --- Gateway SIP Listener --- # Port for devices (softphones/hardphones) to register to GATEWAY_SIP_HOST=0.0.0.0 -GATEWAY_SIP_PORT=5080 -GATEWAY_SIP_DOMAIN=gateway.helu.ca +GATEWAY_SIP_PORT=5060 +GATEWAY_SIP_DOMAIN=gateway.local # --- Speaches STT --- SPEACHES_URL=http://localhost:22070 SPEACHES_MODEL=whisper-large-v3 +# --- Rhema TTS (OpenAI-compatible /v1/audio/speech) --- +# Must NOT point at this app's own port (default PORT=8000) — set a real +# endpoint or TTS requests loop back into the gateway. +TTS_BASE_URL=http://localhost:8001 +TTS_MODEL=speaches-ai/Kokoro-82M-v1.0-ONNX +TTS_VOICE=af_heart +TTS_API_KEY= +TTS_TIMEOUT=30.0 +TTS_SAMPLE_RATE=16000 + # --- Audio Classifier --- # Thresholds for hold music detection (0.0 - 1.0) CLASSIFIER_MUSIC_THRESHOLD=0.7 @@ -50,6 +77,12 @@ LLM_TIMEOUT=30.0 LLM_MAX_TOKENS=1024 LLM_TEMPERATURE=0.3 +# --- AI Receptionist (inbound calls) --- +RECEPTIONIST_ENABLED=true +RECEPTIONIST_LISTEN_TIMEOUT_S=15.0 +RECEPTIONIST_END_OF_UTTERANCE_SILENCE_S=1.2 +RECEPTIONIST_MESSAGE_MAX_SECONDS=90 + # --- Hold Slayer --- # Default device to transfer to when human detected DEFAULT_TRANSFER_DEVICE=sip_phone diff --git a/docs/deployment-validation-plan.md b/docs/deployment-validation-plan.md new file mode 100644 index 0000000..0f961b1 --- /dev/null +++ b/docs/deployment-validation-plan.md @@ -0,0 +1,315 @@ +# Hold Slayer — Deployment & Validation Plan + +Bring the gateway up in stages, proving each layer before the one above it can +lie about working. The ordering principle: **nothing touches the PSTN until +everything that can be validated without it has been.** A bug found on the trunk +costs money and rings a stranger's phone; the same bug found internally costs +nothing. + +Phases gate on each other. Do not start a phase until its predecessor's exit +criteria are all green. + +--- + +## Environment as found (2026-07-28) + +Verified on `caliban`, not assumed: + +| Fact | State | Consequence | +|---|---|---| +| `sippy` 2.3.0 | installed | SIP signaling is real | +| `pjsua2` | **not installed** | media pipeline is a **stub** — signaling works, audio does not | +| `USE_MOCK_SIP` | `true` | engine is `MockSIPEngine`; `/health` can never be `healthy` | +| `SIP_TRUNK_*` | placeholders (`sip.yourprovider.com`) | no trunk configured | +| `SIP_TRUNK_DID` | `+16472474242` | real DID already allocated | +| `GATEWAY_SIP_PORT` | `5060` | `.env.example` says 5080 — reconcile | +| `DATABASE_URL` | `portia.incus:5432/hold_slayer` | remote Postgres, reachable? unverified | +| Casdoor | enabled, `hold-slayer` app, `OWNER_NAME=r@helu.ca` | SSO path is configured | +| `SPEACHES_URL` | `pan.helu.ca:22070` | not reachable from this host as tested | +| `LLM_BASE_URL` | `nyx.helu.ca:29000` | not reachable from this host as tested | +| `TTS_*` | unset → defaults to `localhost:8000` | **collides with the app's own port** — must be set | +| `pytest` | 159 pass, 6 fail | 4 are `.env` bleed, 2 are real (see Phase 0) | + +Two config landmines worth fixing before anything else: `TTS_BASE_URL` is unset +and defaults to `http://localhost:8000`, which is Hold Slayer's own port — TTS +calls would loop back into the gateway. And `API_TOKEN` still sits in `.env` +though the code now uses Casdoor + PATs; it is dead weight that suggests the old +auth path still exists. + +--- + +## Phase 0 — Baseline: make the test suite an honest gate ✅ COMPLETE + +**Done 2026-07-28.** Suite is 165 passed / 0 failed from the repo root with the +real `.env` present. Outcomes differed from the plan's guesses in one important +way: the classifier failure was a **real bug in `classify_chunk`**, not a +threshold or fixture problem — see item 3. One item is deferred with rationale +(`ruff`, below). + +You cannot use a suite with known failures as a regression gate; every later +phase's "did I break something" check depends on a clean baseline. + +**Do:** +1. Fix the `.env` bleed in `tests/test_oauth_metadata.py`. The 4 failures are + entirely because pydantic reads the real `.env` and `PUBLIC_BASE_URL=http://localhost:21081` + overrides the test's expected `http://test`. Confirmed: the same tests pass + when run from a directory without `.env`. Fix by monkeypatching + `get_settings().public_base_url = ""` in the `client` fixture, so tests derive + the base URL from request headers as intended. +2. Resolve `tests/test_hold_slayer.py::TestMockSIPEngine::test_trunk_status`. + **The test is wrong, not the code.** It asserts the mock trunk reports + `registered is True` after `start()`, but `MockSIPEngine.get_trunk_status()` + deliberately returns `registered: False` with + `reason: "No SIP trunk configured (mock mode)"` — which is exactly what the + `/health`-tells-the-truth invariant requires. Update the test to assert + `False` + the reason. +3. Triage `test_audio_classifier.py::test_complex_tone_as_music`. **Outcome: the + test was right and the code had a real bug.** Measured feature values for the + C-major chord: `music_score` = **0.850**, comfortably above the 0.7 + `music_threshold` — music was detected confidently. But step 5's + `if speech_score > music_score` ran first and short-circuited, because + `speech_score` = **1.000**: the four speech bands are wide and overlapping + (flatness 0.223 ∈ (0.1,0.5), centroid 852 ∈ (500,4000), ZCR 0.039 ∈ + (0.02,0.2), RMS 0.183 ∈ (0.01,0.5)) and a sustained musical chord satisfies + all four trivially. Verified structural, not a one-off — C major, A minor and + a bright chord *all* classified `live_human` despite scoring 0.85/0.65 music. + + Fixed in `services/audio_classifier.py` by making the speech branch yield to a + confident music score: + `if speech_score > music_score and music_score < self.settings.music_threshold:` + One line, both thresholds stay meaningful, scorers untouched. Chords now + classify `music` (0.85) and all 18 classifier tests pass, so speech-like audio + still classifies as speech. + + **This was the plan's highest-value find:** the bug is exactly the operational + failure that rings your desk for a hold queue. Worth re-validating against + *real* hold music in Phase 2 — the synthetic "bright chord" case scores 0.65, + below threshold, so it still reads as speech. + +**Exit criteria:** `pytest tests/ -v` green from the repo root with the real +`.env` present ✅ (165 passed). `ruff check .` clean — **deferred, see below**. + +**Deferred: `ruff check .` reports 217 errors — all pre-existing.** Verified +identical (217) with my changes stashed, so nothing here was introduced by +Phase 0, and the three files I touched add none. The backlog is stylistic and +repo-wide: 157 × `UP045` (`Optional[X]` → `X | None`), 18 × `F401` unused +imports, 14 × `E501`, spread across `services/` (77), `models/` (75) and +`core/` (49). Auto-fixing would rewrite annotations throughout `sippy_engine.py` +and `media_pipeline.py` — the thread-boundary and media code that currently +**cannot be exercised** (no PJSUA2, no trunk). That is a large unverifiable diff +in the highest-risk files, unrelated to making the suite an honest gate. Recommend +its own commit, ideally after Phase 1b restores the ability to actually run that +code. + +--- + +## Phase 1a — Internal signaling with a registered softphone (no audio) + +Prove the SIP stack, registration, dial plan, and call lifecycle with zero +media and zero PSTN. This is the phase that de-risks the most for the least. + +**Config:** +``` +USE_MOCK_SIP=false # real Sippy engine +SIP_TRUNK_HOST= # blank — engine skips _register_trunk() +GATEWAY_SIP_HOST=0.0.0.0 +GATEWAY_SIP_PORT=5060 +GATEWAY_SIP_DOMAIN=voip.helu.ca +``` +Leaving `SIP_TRUNK_HOST` blank is deliberate and load-bearing: `SippyEngine.start()` +only calls `_register_trunk()` `if self._trunk_host`, so the gateway comes up +listening for devices with **no possible path to the PSTN**. That is the +strongest safety guarantee available in this phase — not policy, but absence of +a route. + +**Setup:** +1. Register a softphone (Linphone or Zoiper on a laptop; Groundwire on mobile) + to `:5060`, any username, domain `voip.helu.ca`. + **Expect registration to succeed with any credentials** — see the security + finding below. +2. Confirm the device appears: `GET /api/v1/devices` and the `list_devices` MCP + tool, plus the `📱 SIP REGISTER` line in the log. +3. Create a `Device` row for it (`type: sip_phone`, `sip_uri` matching the + registered contact) so `call_device()` can target it. + +**Validate:** +- REGISTER lands, appears in `_registered_devices`, refreshes on re-REGISTER, + and disappears on `expires=0` (hang up / unregister in the softphone). +- Inbound INVITE from the softphone to the gateway surfaces an `incoming_invite` + event and routes through the receptionist path. +- `gateway.call_device()` toward the softphone makes it **ring** — this + exercises `_call_sip_device`, SDP generation, and the loop→Sippy funnel. +- Answer, then hang up from each end in turn; confirm `terminated` propagates + and the leg is cleaned from `_legs`. +- DTMF from the softphone appears in the log (note: received DTMF currently has + no consumer — it logs and stops). +- `/health` reports `degraded` with `sip_trunk.registered: false`. **This is + correct** — do not chase a green light here. + +**Security finding to address in this phase:** `_handle_incoming_register()` +replies `200 OK` to any REGISTER with no authentication challenge — no 401 with +a nonce, no digest verification. Any host that can reach port 5060 can register +as any AOR and become a transfer target. On a LAN-only bind for this phase +that's tolerable; **before Phase 3 exposes the gateway to a trunk it is not.** +Either add digest auth to the REGISTER path or firewall 5060 to known device +IPs. Flagging rather than folding in — it's a real change to the SIP path and +your call how to scope it. + +**Exit criteria:** softphone registers, rings on `call_device`, both-direction +teardown is clean, no leaked legs after 10 call cycles, suite still green. + +--- + +## Phase 1b — Gate: build PJSUA2 + +**Everything audio-shaped is blocked until this is done.** No TTS into a call, +no STT out of one, no recording, no hold-music classification on live audio. +`MediaPipeline` logs `⚠️ PJSUA2 not installed — media pipeline running in stub +mode` and every media call becomes a no-op that *returns successfully* — which +is the dangerous part, since it looks like it worked. + +Build pjproject with Python bindings (`--enable-shared`, swig python2/3 +bindings) into the `/home/robert/env/hold-slayer` venv. Budget real time for +this; it is a from-source build, not a pip install. + +**Exit criteria:** `python -c "import pjsua2"` succeeds in the venv, and +`/health` / `media_pipeline.get_status()` reports `pjsua2_available: true`. + +--- + +## Phase 2 — Audio, TTS, STT, and the classifier on internal calls + +Now the softphone is a full test instrument: you can speak into the gateway and +hear it speak back, with no telephony charges. + +**Config to fix first:** +``` +TTS_BASE_URL= # NOT localhost:8000 — that's this app +TTS_MODEL=speaches-ai/Kokoro-82M-v1.0-ONNX +SPEACHES_URL=http://pan.helu.ca:22070 +``` + +**Validate leaf services standalone before wiring them into a call** — a failure +here is much easier to read outside the call path: +- `TTSService.synthesize()` returns PCM directly (a short script against the + service). Confirm `service.available` flips to `True` and `/health` reports + `tts: ok`. +- `TranscriptionService.transcribe()` against a known WAV returns expected text; + `/health` reports `stt: ok`. +- Both are reachable **from wherever the gateway actually runs** — neither + `pan.helu.ca:22070` nor `nyx.helu.ca:29000` answered from this host during + assessment. Resolve that before blaming the call path. +- LLM: `nyx.helu.ca:29000` with `Qwen3.6-35B-A3B-UD-Q4_K_XL`. Note the URL has + no `/v1` suffix while the default does — verify which the client expects. + +**Then in-call, softphone ↔ gateway only:** +- Receptionist answers an inbound call from the softphone: you hear the TTS + greeting, speak, and your speech is transcribed. This single test exercises + TTS + STT + LLM + media in the real path. +- Recording writes a real file to `recordings/` with actual audio in it (check + the file plays, not just that it exists — the stub path created files too). +- Classifier: play hold music from a phone/laptop into the call and confirm + `MUSIC`; speak and confirm `LIVE_HUMAN`. This is the ground truth that + Phase 0's failing classifier test was gesturing at. +- Transcript persists to the DB on hangup and is readable via + `get_call_transcript`. + +**Exit criteria:** a full receptionist conversation over the softphone with +audible TTS, accurate STT, correct classification, a playable recording, and a +persisted transcript. + +--- + +## Phase 3 — MCP integration + +Independent of audio, so it can run in parallel with 1b/2 if convenient. + +- Mint a PAT via `api/tokens.py`, confirm `hs_pat_…` format. +- Connect an MCP client to `https:///mcp/` and confirm OAuth discovery + (`/.well-known/oauth-protected-resource`) resolves against the real + `PUBLIC_BASE_URL`. +- Read-only tools first: `gateway_status`, `list_active_calls`, `list_devices`, + `search_call_history`, and the three resources. +- **Verify the auth boundary negatively**: a non-owner identity and a bogus PAT + both get 403/401 on `/mcp/`, REST, and WS. Owner-only is an invariant; prove + it rather than assuming it. +- `make_call` against the softphone (internal, no trunk) — confirms the MCP + path reaches the gateway. +- **Test the emergency guard through MCP specifically**: `make_call("911")`, + `"9911"`, `"112"`, `"+1911"`, and with whitespace/dashes must every one raise + `ToolError` before any SIP action. Do this while `SIP_TRUNK_HOST` is still + blank, so a guard failure cannot become an actual emergency call. **This is + the single most important test in the plan** — run it before Phase 4, never + after. + +**Exit criteria:** all read tools return sane data, `make_call` rings the +softphone, every emergency variant is refused, non-owner is refused. + +--- + +## Phase 4 — SIP trunk (first PSTN contact) + +Only now does real money and a real network get involved. + +1. Set `SIP_TRUNK_HOST/PORT/USERNAME/PASSWORD/TRANSPORT` for the provider; keep + `SIP_TRUNK_DID=+16472474242`. +2. Restart and watch registration: `_register_trunk()` posts `trunk_registered`, + `/health` should flip to **`healthy`** — real engine + registered trunk + + reachable DB. This is the first moment a green `/health` is meaningful. +3. **Re-run the entire emergency-guard suite from Phase 3** now that a real + route exists. The guard is what stands between an AI agent and a 911 + dispatcher with no E911 location attached. +4. First outbound call: dial **your own mobile**, nothing else. Confirm ring, + answer, two-way audio, clean teardown, DB persistence. +5. First inbound: call the DID from your mobile, receptionist answers, routing + and transfer-to-softphone work. +6. Then a real hold scenario against a known IVR with a long queue. + +**Safety rails while validating:** +- Drop `MAX_CONCURRENT_CALLS` to `1` for first calls; restore to 4 after. +- Keep `MAX_HOLD_TIME` low initially — 7200s is two hours of trunk time if + something wedges. +- Watch the provider's billing/CDR page live during the first calls. +- Have the provider portal open to kill calls out-of-band. + +**Exit criteria:** `/health` genuinely `healthy`, one successful outbound to a +known number, one inbound handled, CDRs match expectations, no orphaned legs. + +--- + +## Cross-cutting: `.env.example` drift ✅ FIXED (Phase 0) + +`.env.example` had documented the removed `API_TOKEN` as the auth mechanism and +omitted every `CASDOOR_*` var, `OWNER_NAME`, `PUBLIC_BASE_URL`, and the whole +`TTS_*` block. Rewritten to match `config.py`: dropped `API_TOKEN`, added the +Casdoor/auth block (documenting both supported configurations and why SSO-off +off-loopback is refused), the full `TTS_*` and `RECEPTIONIST_*` blocks, and +reconciled `GATEWAY_SIP_PORT` to 5060 to match `.env`. + +`TTS_BASE_URL` now defaults to `localhost:8001` in the template with a comment +warning that it must not equal the app's own `PORT` — this was blocker #3, and +the template previously would have reproduced it for any fresh checkout. + +Validated by copying the template to a clean directory and parsing it through +`Settings()`: every sub-config loads, and `tts.base_url != localhost:{port}` is +asserted. `grep` confirms no `API_TOKEN` references remain in code, templates or +docs. The README config table was already current — no drift there. + +--- + +## Summary of blockers found + +| # | Blocker | Blocks | Severity | +|---|---|---|---| +| 1 | PJSUA2 not installed | all audio: TTS/STT/recording/classifier in-call | **hard gate** | +| 2 | REGISTER accepts any credentials, no digest auth | safe exposure of port 5060 | **security** | +| 3 | `TTS_BASE_URL` unset → defaults to app's own port | TTS entirely | config — ✅ fixed in template; **still set it in your real `.env`** | +| 4 | STT/LLM endpoints unreachable as tested | Phase 2 | environment | +| 5 | 6 failing tests (2 real, 4 `.env` bleed) | honest regression gate | ✅ **fixed** — 165 pass | +| 6 | `.env.example` documents removed `API_TOKEN` auth | fresh-environment test | ✅ **fixed** | +| 7 | Hold music classified as `LIVE_HUMAN` (found during Phase 0) | correct hold detection | ✅ **fixed** — real bug, see Phase 0 item 3 | + +**Note on #3:** the fix landed in `.env.example` (the template). Your live +`.env` still has no `TTS_*` block at all, so TTS resolves to the +`localhost:8000` default and collides with the app. Set `TTS_BASE_URL` before +Phase 2. diff --git a/services/audio_classifier.py b/services/audio_classifier.py index 7b440ca..5a2082b 100644 --- a/services/audio_classifier.py +++ b/services/audio_classifier.py @@ -144,7 +144,11 @@ class AudioClassifier: ) # 5. If it's speech-like, is it live or automated? - if speech_score > music_score: + # A confident music score wins outright: the speech bands are wide + # enough that sustained hold music satisfies all four and scores 1.0, + # which would otherwise mask music as LIVE_HUMAN and ring the owner + # for a hold queue. + if speech_score > music_score and music_score < self.settings.music_threshold: # Use history to distinguish live human from IVR # IVR: repetitive patterns, synthetic prosody # Human: natural variation, conversational rhythm diff --git a/tests/test_hold_slayer.py b/tests/test_hold_slayer.py index b75ed25..85c5526 100644 --- a/tests/test_hold_slayer.py +++ b/tests/test_hold_slayer.py @@ -260,6 +260,11 @@ class TestMockSIPEngine: status = await engine.get_trunk_status() assert status["registered"] is False + # Starting the mock engine must NOT report a registered trunk: /health + # requires a registered trunk to be "healthy", so a mock that claimed + # registration would let a gateway that cannot place calls go green. await engine.start() status = await engine.get_trunk_status() - assert status["registered"] is True + assert status["registered"] is False + assert status["mock"] is True + assert status["reason"] == "No SIP trunk configured (mock mode)" diff --git a/tests/test_oauth_metadata.py b/tests/test_oauth_metadata.py index 266032f..02f3f49 100644 --- a/tests/test_oauth_metadata.py +++ b/tests/test_oauth_metadata.py @@ -13,7 +13,12 @@ from config import get_settings @pytest.fixture -async def client(): +async def client(monkeypatch): + # These endpoints derive their URLs from PUBLIC_BASE_URL when it is set, + # falling back to the request's Host header. Pin it empty so the assertions + # below exercise the header path and can't be overridden by a developer's + # real .env. + monkeypatch.setattr(get_settings(), "public_base_url", "") transport = httpx.ASGITransport(app=main.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: yield c