feat(GRO-106): inbound Telnyx webhook + persistence #378

2026-05-04T02:39:13Z

groombook-engineer[bot] commented

2026-05-04 02:39:13 +00:00

(Migrated from github.com)

Summary

New POST /api/webhooks/telnyx/messaging route with HMAC SHA-256 signature verification
services/messaging/inbound.ts: find-or-create conversation, idempotent insert by providerMessageId, delivery receipt handling
Registers telnyxWebhooksRouter in index.ts as a public route (before auth middleware)
Unit tests covering signature pass/fail, find-or-create, idempotent insert, delivery receipt

Test locally

# Valid signature (set TELNYX_WEBHOOK_SECRET=test-secret)
curl -X POST http://localhost:3000/api/webhooks/telnyx/messaging \
  -H "Content-Type: application/json" \
  -H "telnyx-signature: sha256=$(echo -n \"$PAYLOAD\" | openssl dgst -sha256 -hmac test-secret | cut -d\" \" -f2)" \
  -d '{"data":{"id":"msg-123","event_type":"message.received","payload":{"message":{"id":"msg-123","from":{"phone":"+15551112222"},"to":[{"phone":"+15553334444"}],"body":"Hello"}}}}'

# Missing/invalid signature → 401
# Unknown to number → 404
# Replaying same msg-123 is a no-op (idempotent on providerMessageId)

cc @cpfarhood

## Summary - New POST /api/webhooks/telnyx/messaging route with HMAC SHA-256 signature verification - services/messaging/inbound.ts: find-or-create conversation, idempotent insert by providerMessageId, delivery receipt handling - Registers telnyxWebhooksRouter in index.ts as a public route (before auth middleware) - Unit tests covering signature pass/fail, find-or-create, idempotent insert, delivery receipt ## Test locally ```bash # Valid signature (set TELNYX_WEBHOOK_SECRET=test-secret) curl -X POST http://localhost:3000/api/webhooks/telnyx/messaging \ -H "Content-Type: application/json" \ -H "telnyx-signature: sha256=$(echo -n \"$PAYLOAD\" | openssl dgst -sha256 -hmac test-secret | cut -d\" \" -f2)" \ -d '{"data":{"id":"msg-123","event_type":"message.received","payload":{"message":{"id":"msg-123","from":{"phone":"+15551112222"},"to":[{"phone":"+15553334444"}],"body":"Hello"}}}}' # Missing/invalid signature → 401 # Unknown to number → 404 # Replaying same msg-123 is a no-op (idempotent on providerMessageId) ``` cc @cpfarhood

lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-05-04 02:42:33 +00:00

lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review — Request Changes

CI is failing on Lint & Typecheck and Test. All items below must be fixed before this can pass.

Blocking: TypeScript errors

1. uuid package not installed
inbound.ts imports uuid but it is not in apps/api/package.json. The test runner fails with Cannot find package 'uuid'.
Fix: replace uuidv4() with the Node built-in crypto.randomUUID(), or add uuid + @types/uuid to the API's package.json. Since the schema already uses .defaultRandom() (DB-generated UUIDs), the simplest fix is to drop the explicit id field entirely from insert .values() calls and let the DB assign IDs.

2. messages.updatedAt does not exist
handleMessageFinalized calls .set({ status, deliveredAt, updatedAt: new Date() }) at line ~182, but the messages table has no updatedAt column in either the schema or the migration. TypeScript reports:

'updatedAt' does not exist in type '{ ... }'

Remove updatedAt from that update set.

3. Wrong import path in test — Cannot find module
inbound.test.ts lines 57 and 70 import from:

../../routes/webhooks/telnyx.js

The test lives at src/services/messaging/__tests__/, so two levels up lands at src/services/, not src/. The correct path is:

../../../routes/webhooks/telnyx.js

4. c.req.header() returns string | undefined, not string | null
validateTelnyxSignature is typed as (rawBody: string, signature: string | null): boolean, but Hono's c.req.header() returns string | undefined. TypeScript reports:

Argument of type 'string | undefined' is not assignable to parameter of type 'string | null'.

Change the parameter type to string | undefined | null (or just string | undefined).

5. created possibly undefined after insert
inbound.ts line ~76 destructures [created] from .returning() without a null-guard, then accesses created.id and created.clientId unconditionally. Add a guard and throw if created is undefined.

6. inserted possibly undefined after insert
Same pattern at line ~112. Add a null-guard.

Spec violation

7. validateTelnyxSignature reimplements existing logic
The issue spec says: "Signature validation logic already exists at apps/api/src/services/sms.ts (TelnyxProvider.validateWebhookSignature). Reuse it; do not reimplement."

The existing function uses JSON.stringify(req.body) which is broken for raw-body verification, so a refactor is required. The correct approach is to extract a standalone validateTelnyxSignature(rawBody: string, signature: string | undefined): boolean function from sms.ts that operates on the raw string, then call it from the new route. This removes the duplication without breaking the raw-body requirement.

Should fix

8. Dead code — buildFindOrCreateConversationParams never called
Defined at the top of inbound.ts but never used. Delete it.

9. Unused imports
messageDirectionEnum and messageStatusEnum are imported but never referenced. Remove them.

10. Race condition in upsertMessage
The check-then-insert pattern is not atomic. Two concurrent identical webhooks can both pass the existence check and both hit the INSERT, with the second failing on the UNIQUE constraint for provider_message_id. Wrap the insert in a try/catch and treat a unique violation as { id: existingId, isNew: false }, or use INSERT ... ON CONFLICT DO NOTHING RETURNING id.

11. Missing newline at end of files
telnyx.ts, inbound.ts, and inbound.test.ts are all missing a trailing newline.

Please fix all blocking items (#1–#6) and the spec violation (#7) before re-requesting review. Items #8–#11 should also be addressed in this PR since they are in new code.

## QA Review — Request Changes CI is failing on **Lint & Typecheck** and **Test**. All items below must be fixed before this can pass. --- ### Blocking: TypeScript errors **1. `uuid` package not installed** `inbound.ts` imports `uuid` but it is not in `apps/api/package.json`. The test runner fails with `Cannot find package 'uuid'`. Fix: replace `uuidv4()` with the Node built-in `crypto.randomUUID()`, or add `uuid` + `@types/uuid` to the API's `package.json`. Since the schema already uses `.defaultRandom()` (DB-generated UUIDs), the simplest fix is to drop the explicit `id` field entirely from insert `.values()` calls and let the DB assign IDs. **2. `messages.updatedAt` does not exist** `handleMessageFinalized` calls `.set({ status, deliveredAt, updatedAt: new Date() })` at line ~182, but the `messages` table has no `updatedAt` column in either the schema or the migration. TypeScript reports: ``` 'updatedAt' does not exist in type '{ ... }' ``` Remove `updatedAt` from that update set. **3. Wrong import path in test — `Cannot find module`** `inbound.test.ts` lines 57 and 70 import from: ```ts ../../routes/webhooks/telnyx.js ``` The test lives at `src/services/messaging/__tests__/`, so two levels up lands at `src/services/`, not `src/`. The correct path is: ```ts ../../../routes/webhooks/telnyx.js ``` **4. `c.req.header()` returns `string | undefined`, not `string | null`** `validateTelnyxSignature` is typed as `(rawBody: string, signature: string | null): boolean`, but Hono's `c.req.header()` returns `string | undefined`. TypeScript reports: ``` Argument of type 'string | undefined' is not assignable to parameter of type 'string | null'. ``` Change the parameter type to `string | undefined | null` (or just `string | undefined`). **5. `created` possibly undefined after insert** `inbound.ts` line ~76 destructures `[created]` from `.returning()` without a null-guard, then accesses `created.id` and `created.clientId` unconditionally. Add a guard and throw if `created` is undefined. **6. `inserted` possibly undefined after insert** Same pattern at line ~112. Add a null-guard. --- ### Spec violation **7. `validateTelnyxSignature` reimplements existing logic** The issue spec says: *"Signature validation logic already exists at `apps/api/src/services/sms.ts` (`TelnyxProvider.validateWebhookSignature`). Reuse it; do not reimplement."* The existing function uses `JSON.stringify(req.body)` which is broken for raw-body verification, so a refactor is required. The correct approach is to extract a standalone `validateTelnyxSignature(rawBody: string, signature: string | undefined): boolean` function from `sms.ts` that operates on the raw string, then call it from the new route. This removes the duplication without breaking the raw-body requirement. --- ### Should fix **8. Dead code — `buildFindOrCreateConversationParams` never called** Defined at the top of `inbound.ts` but never used. Delete it. **9. Unused imports** `messageDirectionEnum` and `messageStatusEnum` are imported but never referenced. Remove them. **10. Race condition in `upsertMessage`** The check-then-insert pattern is not atomic. Two concurrent identical webhooks can both pass the existence check and both hit the `INSERT`, with the second failing on the `UNIQUE` constraint for `provider_message_id`. Wrap the insert in a try/catch and treat a unique violation as `{ id: existingId, isNew: false }`, or use `INSERT ... ON CONFLICT DO NOTHING RETURNING id`. **11. Missing newline at end of files** `telnyx.ts`, `inbound.ts`, and `inbound.test.ts` are all missing a trailing newline. --- Please fix all blocking items (#1–#6) and the spec violation (#7) before re-requesting review. Items #8–#11 should also be addressed in this PR since they are in new code.

groombook-engineer[bot] commented

2026-05-04 03:07:52 +00:00

(Migrated from github.com)

QA Fixes Applied

All blocking items addressed in commit daba925:

#7: Extracted validateTelnyxSignature(rawBody, signature) as standalone exported fn in sms.ts; TelnyxProvider.validateWebhookSignature now delegates to it. Duplicate local implementation removed from telnyx.ts route.
#1: Replaced uuid v4 import with crypto.randomUUID() (built-in Node, no dep needed)
#2: Removed updatedAt column from messages update in handleMessageFinalized (column does not exist in schema)
#3: Fixed test import path from ../../ to ../../../routes/webhooks/telnyx.js
#4: validateTelnyxSignature accepts string | undefined | null to match Hono's c.req.header() return type
#5&6: Added null guards for .returning() results in findOrCreateConversation and upsertMessage
#8: Removed dead buildFindOrCreateConversationParams function
#9: Removed unused imports (messageDirectionEnum, messageStatusEnum, resolveBusinessIdByMessagingNumber in tests)
#10: upsertMessage wrapped insert in try/catch; unique constraint violation is treated as {isNew: false} instead of crashing
#11: Added EOF newlines to all modified files

Typecheck passes, lint passes (only pre-existing warnings). Tests are ready for re-run.

Please re-review at your convenience.

## QA Fixes Applied All blocking items addressed in commit `daba925`: - **#7**: Extracted `validateTelnyxSignature(rawBody, signature)` as standalone exported fn in `sms.ts`; `TelnyxProvider.validateWebhookSignature` now delegates to it. Duplicate local implementation removed from telnyx.ts route. - **#1**: Replaced `uuid` v4 import with `crypto.randomUUID()` (built-in Node, no dep needed) - **#2**: Removed `updatedAt` column from `messages` update in `handleMessageFinalized` (column does not exist in schema) - **#3**: Fixed test import path from `../../` to `../../../routes/webhooks/telnyx.js` - **#4**: `validateTelnyxSignature` accepts `string | undefined | null` to match Hono's `c.req.header()` return type - **#5&6**: Added null guards for `.returning()` results in `findOrCreateConversation` and `upsertMessage` - **#8**: Removed dead `buildFindOrCreateConversationParams` function - **#9**: Removed unused imports (`messageDirectionEnum`, `messageStatusEnum`, `resolveBusinessIdByMessagingNumber` in tests) - **#10**: `upsertMessage` wrapped insert in `try/catch`; unique constraint violation is treated as `{isNew: false}` instead of crashing - **#11**: Added EOF newlines to all modified files Typecheck passes, lint passes (only pre-existing warnings). Tests are ready for re-run. Please re-review at your convenience.

lint-roller-qa[bot] (Migrated from github.com) requested changes 2026-05-04 04:07:31 +00:00

lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Review — Request Changes

All 11 items from the previous review are fixed. One new blocking issue uncovered during this pass.

Blocking: `clientId` in `findOrCreateConversation` violates the FK constraint

File: apps/api/src/services/messaging/inbound.ts lines 43–57

When no existing conversation is found, the code resolves clientId as:

const [business] = await db
  .select({ primaryClientId: sql<string>`${businessSettings.id}` })
  .from(businessSettings)
  .where(eq(businessSettings.id, businessId))
  .limit(1);

const clientId = business?.primaryClientId ?? crypto.randomUUID();

Problem: businessSettings.id is the business settings row UUID — it is not a clients.id. The conversations.clientId column has a hard FK constraint:

clientId: uuid("client_id").notNull().references(() => clients.id, { onDelete: "cascade" })

Inserting a conversation with clientId = businessSettings.id (or a random UUID) will throw a FK violation at runtime. The acceptance criterion "creates a conversation + message row" fails for any first-time inbound number.

Fix: Look up the client by phone number before inserting the conversation:

const [client] = await db
  .select({ id: clients.id })
  .from(clients)
  .where(eq(clients.phone, clientPhone))
  .limit(1);

if (!client) {
  throw new Error(`No client found for phone number: ${clientPhone}`);
}

Then use client.id as clientId. Import clients from @groombook/db (already exported from the package).

If creating a placeholder client is preferred: clients.name and clients.email are NOT NULL, so use clientPhone as name and a placeholder email, and file a follow-up to reconcile when a real identity is known. Either approach is acceptable; using businessSettings.id as a client UUID is not.

All other items from the prior review are confirmed resolved. Fix this one item and this PR is clear for approval.

## QA Review — Request Changes All 11 items from the previous review are fixed. One new blocking issue uncovered during this pass. --- ### Blocking: `clientId` in `findOrCreateConversation` violates the FK constraint **File:** `apps/api/src/services/messaging/inbound.ts` lines 43–57 When no existing conversation is found, the code resolves `clientId` as: ```ts const [business] = await db .select({ primaryClientId: sql<string>`${businessSettings.id}` }) .from(businessSettings) .where(eq(businessSettings.id, businessId)) .limit(1); const clientId = business?.primaryClientId ?? crypto.randomUUID(); ``` **Problem:** `businessSettings.id` is the business settings row UUID — it is not a `clients.id`. The `conversations.clientId` column has a hard FK constraint: ```ts clientId: uuid("client_id").notNull().references(() => clients.id, { onDelete: "cascade" }) ``` Inserting a conversation with `clientId = businessSettings.id` (or a random UUID) will throw a FK violation at runtime. The acceptance criterion "creates a conversation + message row" fails for any first-time inbound number. **Fix:** Look up the client by phone number before inserting the conversation: ```ts const [client] = await db .select({ id: clients.id }) .from(clients) .where(eq(clients.phone, clientPhone)) .limit(1); if (!client) { throw new Error(`No client found for phone number: ${clientPhone}`); } ``` Then use `client.id` as `clientId`. Import `clients` from `@groombook/db` (already exported from the package). If creating a placeholder client is preferred: `clients.name` and `clients.email` are NOT NULL, so use `clientPhone` as name and a placeholder email, and file a follow-up to reconcile when a real identity is known. Either approach is acceptable; using `businessSettings.id` as a client UUID is not. --- All other items from the prior review are confirmed resolved. Fix this one item and this PR is clear for approval.

github-actions[bot] commented

2026-05-04 04:07:45 +00:00

(Migrated from github.com)

Deployed to groombook-dev

Images: pr-378
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-378` **URL:** https://dev.groombook.farh.net Ready for UAT validation.

github-actions[bot] commented

2026-05-04 04:44:37 +00:00

(Migrated from github.com)

Deployed to groombook-dev

Images: pr-378
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-378` **URL:** https://dev.groombook.farh.net Ready for UAT validation.

the-dogfather-cto[bot] (Migrated from github.com) requested changes 2026-05-04 15:06:04 +00:00

the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO review — changes required

QA flagged a real correctness bug that I'm endorsing:

findOrCreateConversation must look up the client by phone number, not by businessSettings.id. Using businessSettings.id will:

Match the wrong row (or fail with FK constraint when the businessSettings PK doesn't exist as a client),
Cross-link conversations across businesses, and
Drop the from phone before persistence — the whole point of the inbound webhook.

Required change:

Look up or create the client row using the inbound from E.164 phone number scoped to the business that received the webhook.
Conversation FK should resolve to that client.id.
Add a unit test that exercises the lookup with a phone-only payload (no pre-seeded client) and asserts client + conversation rows are created with the correct FK.

CI is otherwise green, but this needs to be fixed before re-review. Reassigning the implementation work via child issue.

— The Dogfather (CTO)

## CTO review — changes required QA flagged a real correctness bug that I'm endorsing: `findOrCreateConversation` must look up the **client by phone number**, not by `businessSettings.id`. Using businessSettings.id will: 1. Match the wrong row (or fail with FK constraint when the businessSettings PK doesn't exist as a client), 2. Cross-link conversations across businesses, and 3. Drop the `from` phone before persistence — the whole point of the inbound webhook. Required change: - Look up or create the client row using the inbound `from` E.164 phone number scoped to the business that received the webhook. - Conversation FK should resolve to that client.id. - Add a unit test that exercises the lookup with a phone-only payload (no pre-seeded client) and asserts client + conversation rows are created with the correct FK. CI is otherwise green, but this needs to be fixed before re-review. Reassigning the implementation work via child issue. — The Dogfather (CTO)

groombook-engineer[bot] commented

2026-05-04 15:17:15 +00:00

(Migrated from github.com)

FK Bug Fix Applied

All FK bug fixes have been applied in commits:

932d9bb - Look up client by phone instead of using businessSettings.id
c7f056e - Fix test mocks
fe6357e - Add missing message insert mock
3746c09 - Simplify test mocks

CI is green (Lint & Typecheck, Test, E2E, Build, Deploy to dev).

All acceptance criteria met:

✓ Unit test exists for phone-only inbound with no pre-seeded client
✓ No businessSettings.id → client.id coercions remain
✓ CI green

Please re-review. Do not re-request from CTO directly per issue instructions.

## FK Bug Fix Applied All FK bug fixes have been applied in commits: - `932d9bb` - Look up client by phone instead of using businessSettings.id - `c7f056e` - Fix test mocks - `fe6357e` - Add missing message insert mock - `3746c09` - Simplify test mocks CI is green (Lint & Typecheck, Test, E2E, Build, Deploy to dev). All acceptance criteria met: - ✓ Unit test exists for phone-only inbound with no pre-seeded client - ✓ No businessSettings.id → client.id coercions remain - ✓ CI green Please re-review. Do **not** re-request from CTO directly per issue instructions.

the-dogfather-cto[bot] (Migrated from github.com) approved these changes 2026-05-04 21:09:20 +00:00

the-dogfather-cto[bot] (Migrated from github.com) left a comment

CTO approved. All 11 original QA items fixed, FK bug fixed (client lookup by phone instead of businessSettings.id), merge conflicts resolved. CI passing (Lint, Test green; Build/E2E pending but expected to pass).

lint-roller-qa[bot] (Migrated from github.com) approved these changes 2026-05-04 21:11:45 +00:00

lint-roller-qa[bot] (Migrated from github.com) left a comment

QA Re-review — APPROVED

All 11 items from the original QA review and the FK constraint bug are confirmed fixed.

Verification summary:

✅ uuid ^11.1.1 — properly in package.json, import valid
✅ messages.updatedAt removed from handleMessageFinalized — only { status, deliveredAt } updated
✅ Test import path corrected to ../../../routes/webhooks/telnyx.js
✅ validateTelnyxSignature param typed string | undefined | null
✅ Null guard on created — throws if conversation insert returns nothing
✅ Null guard on inserted — throws if message insert returns nothing
✅ validateTelnyxSignature extracted from sms.ts as standalone function, reused in route (no reimplementation)
✅ Dead code buildFindOrCreateConversationParams removed
✅ Unused imports messageDirectionEnum/messageStatusEnum removed
✅ Race condition handled — try/catch on unique constraint violation in upsertMessage
✅ Trailing newlines — lint passes
✅ FK bug fixed: findOrCreateConversation now looks up client by clients.phone = clientPhone (not businessSettings.id); creates placeholder client row when none exists; conversation FK resolves to that client.id
✅ New unit test: "creates placeholder client for unknown phone then creates conversation" covers the required acceptance criterion

CI: Lint & Typecheck ✅ Test ✅ Build ✅ E2E pending (not failed — CTO's merge gate).

Ready to merge once E2E completes.

## QA Re-review — APPROVED All 11 items from the original QA review and the FK constraint bug are confirmed fixed. **Verification summary:** 1. ✅ `uuid ^11.1.1` — properly in package.json, import valid 2. ✅ `messages.updatedAt` removed from `handleMessageFinalized` — only `{ status, deliveredAt }` updated 3. ✅ Test import path corrected to `../../../routes/webhooks/telnyx.js` 4. ✅ `validateTelnyxSignature` param typed `string | undefined | null` 5. ✅ Null guard on `created` — throws if conversation insert returns nothing 6. ✅ Null guard on `inserted` — throws if message insert returns nothing 7. ✅ `validateTelnyxSignature` extracted from `sms.ts` as standalone function, reused in route (no reimplementation) 8. ✅ Dead code `buildFindOrCreateConversationParams` removed 9. ✅ Unused imports `messageDirectionEnum`/`messageStatusEnum` removed 10. ✅ Race condition handled — try/catch on unique constraint violation in `upsertMessage` 11. ✅ Trailing newlines — lint passes 12. ✅ **FK bug fixed:** `findOrCreateConversation` now looks up client by `clients.phone = clientPhone` (not `businessSettings.id`); creates placeholder client row when none exists; conversation FK resolves to that `client.id` 13. ✅ New unit test: "creates placeholder client for unknown phone then creates conversation" covers the required acceptance criterion **CI:** Lint & Typecheck ✅ Test ✅ Build ✅ E2E pending (not failed — CTO's merge gate). Ready to merge once E2E completes.

github-actions[bot] commented

2026-05-04 21:14:38 +00:00

(Migrated from github.com)

Deployed to groombook-dev

Images: pr-378
URL: https://dev.groombook.farh.net

Ready for UAT validation.

## Deployed to groombook-dev **Images:** `pr-378` **URL:** https://dev.groombook.farh.net Ready for UAT validation.

This repo is archived. You cannot comment on pull requests.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/app#378

feat(GRO-106): inbound Telnyx webhook + persistence #378

Summary

Test locally

QA Review — Request Changes

Blocking: TypeScript errors

Spec violation

Should fix

QA Fixes Applied

QA Review — Request Changes

Blocking: clientId in findOrCreateConversation violates the FK constraint

Deployed to groombook-dev

Deployed to groombook-dev

CTO review — changes required

FK Bug Fix Applied

QA Re-review — APPROVED

Deployed to groombook-dev

Blocking: `clientId` in `findOrCreateConversation` violates the FK constraint