# Follow-Ups — Known Issues to Fix Later

> Standing list of non-blocking improvements, tech debt, and bugs discovered
> during feature work. Not the active sprint plan (that's `todo.md`). Move
> items to `todo.md` when you pick them up.
>
> **How to use:** Each item has a **Why / Where / Effort** block so the next
> person (or future-you) can jump straight to the file and understand why it
> matters. Tick `- [x]` when shipped and move to the Done section at the
> bottom.

Last updated: 2026-04-11 (after WhatsApp amend + cancel-with-refund feature work)

---

## 🔴 Production Safety (do before next deploy)

- [ ] **Rotate `CRON_SECRET` before production deploy**
  - **Why:** The dev value `gi4PzKM6bChalAobfA933tnmeH-5lbLB` is committed into `mawidi-site/.env.local` for local testing. It must NOT ship to prod. The cron endpoint is the only gate between the public internet and the reminder-sending service, so a leaked secret = anyone can flood customers with WhatsApp messages or consume Twilio credit.
  - **Where:** `mawidi-site/.env.local` line ~end, `mawidi-site/app/api/cron/appointment-reminders/route.ts`, production env (cPanel / Docker secrets).
  - **Effort:** 5 min. Generate new value (`node -e "console.log(require('crypto').randomBytes(24).toString('base64url'))"`), set in production env, paste in cPanel cron command, delete from `.env.local` (or replace with a new dev-only value).

- [ ] **Ensure `ENABLE_DEV_ROUTES` is NOT set in production**
  - **Why:** Three dev-only routes (`/api/dev/seed-kb`, `/api/dev/ai-ask`, `/api/dev/run-reminders`) are gated on this flag. If accidentally set to `1` in prod, anyone can (a) wipe + reseed the knowledge base, (b) call the AI engine free of charge, (c) manually fire reminders bypassing the cron secret.
  - **Where:** `.env.local` has `ENABLE_DEV_ROUTES=1` today; cPanel/production env must omit it.
  - **Effort:** Verification only (2 min): `curl https://app.mawidi.com/api/dev/seed-kb` should return `403 {"error":"Dev routes disabled"}`.

- [ ] **Delete / disable the test booking used for reminder e2e**
  - **Why:** I created a real booking `kh7b5f9k6395mjj0sg1xmtxnq984n3r1` for "Asim Reminder Test" on 2026-04-12 15:00 under the al Macron city clinic. It still exists in Convex and will show up in the clinic dashboard's appointments tab. The scheduled reminder already fired, so no further harm, but it's junk data.
  - **Where:** Convex table `bookings`, id `kh7b5f9k6395mjj0sg1xmtxnq984n3r1`.
  - **Effort:** 1 min — run a delete mutation, or mark status `cancelled`, or just leave it since tomorrow passes quickly. Note: there is no `bookings/mutations:delete` mutation yet — see separate follow-up below.

---

## 🟠 Bugs Found During Amend/Cancel Implementation (2026-04-11)

- [ ] **Booking timezone inconsistency between creation paths**
  - **Why:** The AI engine's `executeReschedule` (and likely the original `executeCreateBooking`) parses the new `appointmentDate` with `new Date("YYYY-MM-DDTHH:MM:00").getTime()` — which is interpreted in the Docker container's local tz (UTC in production). This means a Qatari customer saying "book me at 16:00 Thursday" gets stored as **16:00 UTC = 19:00 Doha** in the database, while the display field `appointmentTime` says "16:00". Compare with the reminder service which correctly applies a `DOHA_OFFSET_MS` constant. This is a pre-existing bug, not introduced by the amend feature — but it masks real-world conflicts and makes reminders fire at the wrong wall-clock time.
  - **Where:** `lib/services/whatsapp-ai-engine.service.ts` inside `executeReschedule` (where the new Date is constructed); same pattern in `handleBookingRequest`'s booking creation; `lib/services/appointment-reminder.service.ts` already has the correct pattern via `DOHA_OFFSET_MS`.
  - **Effort:** 1 hour for the Doha-hard-coded fix; 1 day if done properly per-org via `organizations.timezone`. Until fixed, all new bookings are offset by 3 hours in the database.
  - **Discovered:** Test 3 (reschedule with conflict) failed in two of three setups until I stopped using manually-Doha-offset timestamps in test data.

- [ ] **`isSlotAvailable` used wrong field names before 2026-04-11 rewrite**
  - **Why:** The pre-existing version of `whatsapp-availability.service.ts` queried `startTime` and `endTime` fields on bookings, but the Convex schema only has `appointmentDate` (full ms) + `appointmentTime` (HH:MM) + `duration` (minutes). Combined with the Prisma-Convex adapter silently returning `[]` for org-scoped queries, this meant `isSlotAvailable` was ALWAYS returning `true` for any slot on any clinic — i.e., **double-booking was impossible to detect**. Shipped in the amend work by replacing the whole function with a direct `convexQuery('bookings/queries:getConflictingBookings', ...)` call and a dedicated time-range query. **Pre-existing callers of `isSlotAvailable` (e.g. `app/api/whatsapp/bookings/route.ts`) should be audited to confirm they now behave correctly** — they may have been relying on the always-true fallback and could start rejecting legitimate slots.
  - **Where:** `lib/services/whatsapp-availability.service.ts`, `convex/bookings/queries.ts:getConflictingBookings`, `app/api/whatsapp/bookings/route.ts:12`
  - **Effort:** 30 min audit + any follow-up fixes.

- [ ] **Prisma-Convex adapter silently returns `[]` for org-scoped booking queries**
  - **Why:** Generic bookings listFn is `bookings/queries:getBookings` which requires `userId`. When the adapter tries to translate a Prisma-style `findMany({ where: { organizationId, ... } })` it passes `organizationId` to the query; the query requires `userId`; Convex throws `ArgumentValidationError`; the adapter's try/catch swallows it and returns `[]`. This bug exists for **all** org-scoped booking reads (not just the ones we hit). We worked around it in the amend flow by adding dedicated queries (`getUpcomingByOrgPhone`, `getConflictingBookings`) and calling them directly via `convexQuery`, but a proper fix would be to either (a) add a generic org-scoped listFn to the adapter, or (b) refactor all org-scoped booking reads to bypass the adapter.
  - **Where:** `lib/prisma-convex-adapter.ts` bookings proxy, `lib/adapters/whatsapp.ts` fetchAll pattern, `convex/bookings/queries.ts`
  - **Effort:** 2-3 hours for a proper fix. Alternative: add more dedicated queries as needed.

---

## 🟡 Known Bugs

- [ ] **Onboarding form conflates `address` and country code**
  - **Why:** `app/[lang]/signup/company-info/page.tsx` line ~197 writes the GCC country value into the `address` field on the user record (so `users.address === "QA"` for Qatar-based orgs). This breaks the appointment reminder's address-link feature — `buildAddressBlock` now rejects bare 2-char country codes and omits the Location line silently, but the real fix is to add a distinct "Street Address" input to the form.
  - **Where:** `mawidi-site/app/[lang]/signup/company-info/page.tsx` (the form) and `mawidi-site/app/api/onboarding/company-info/route.ts` lines ~127-290 (the handler). Convex users schema at `convex/schema/auth.ts:37` already has the right field, just not wired to a UI input.
  - **Effort:** 1-2 hours. Add input, wire to handler, migrate existing records (SQL-style backfill via a one-off script: any user whose `address` equals their `country` gets `address` cleared).
  - **Evidence:** Test clinic owner (`h97ht1nxhkgzbt9e09k8kxfyr9826jsm`) had `address: "QA"` before I manually overrode it to `"3rd Floor, Burj al Mana Tower, Al Sadd Street"` for e2e testing.

- [ ] **Dashboard Knowledge Base tab: summary tile shows `FAQS 0` while inline tab counter shows `FAQs 10`**
  - **Why:** Two different API calls feed two different counters on the same page. The big summary tiles at the top call `/api/dashboard/knowledge-base` (overview endpoint, which doesn't count FAQs from `knowledge_base_faqs`), while the inline tab row below calls `/api/dashboard/knowledge-base/faqs` directly. After seeding 10 FAQs, the bottom tab updates but the top tile stays at 0 — confusing for users.
  - **Where:** Dashboard component at `mawidi-site/app/[lang]/dashboard/components/KnowledgeBase*.tsx` (find via `grep -r "TOTAL MEDIA" app/\[lang\]/dashboard/`) and the API route at `mawidi-site/app/api/dashboard/knowledge-base/route.ts`.
  - **Effort:** 30 min. Either recount FAQs in the overview endpoint, or have the client refetch both on mount.
  - **Evidence:** Screenshots from 2026-04-11 session — tile says `FAQS 0`, inline tab says `FAQs 10`, content list shows all 10.

- [ ] **Two parallel knowledge-base tables: dashboard edits never reach the AI**
  - **Why:** The WhatsApp AI engine's keyword-match fallback (`lib/services/whatsapp-ai-engine.service.ts` line ~1023) reads from `company_knowledge_base` (keyed by `organizationId`). The dashboard Knowledge Base FAQs UI writes to `knowledge_base_faqs` (keyed by `userId`). They're completely separate tables with different schemas (single-language rows vs bilingual rows). If a clinic owner edits a FAQ in the dashboard, the AI never sees the change.
  - **Where:** `convex/schema/whatsapp.ts:115` (`company_knowledge_base`), `convex/schema/voice_agents.ts` (`knowledge_base_faqs`), `convex/whatsapp/queries.ts` (`getKnowledgeBaseEntries`), `convex/voice_agents/mutations.ts:487` (`createKBFAQ`).
  - **Effort:** 1 day. Two possible fixes:
    - **Option A** (simpler): on every `createKBFAQ` / `updateKBFAQ` / `deleteKBFAQ`, mirror the write into `company_knowledge_base` (one row per language per FAQ).
    - **Option B** (cleaner): teach the AI engine to also query `knowledge_base_faqs` via a new `getKBFAQsByOrgId` query (needs an org→owner lookup) and merge results. Then eventually deprecate `company_knowledge_base`.
  - **Current workaround:** I seeded both tables with the same 10-entry mock content on 2026-04-11 so demos look consistent. Any new edits in the dashboard won't affect AI answers.

- [ ] **WhatsApp conversation language is sticky — no way to switch after first selection**
  - **Why:** When a user first messages the clinic, they pick English/Arabic and the choice is stored in `org_whatsapp_conversations.language`. Every subsequent reply — including reminders — honors that choice. If the customer was tested in Arabic once and then wants English, there's no way to reset. For the test phone `+447519699230` all AI replies (even to English questions) come back in Arabic because the conversation is locked.
  - **Where:** `lib/services/whatsapp-language.service.ts`, `convex/schema/whatsapp.ts` (`user_language_preferences` table), webhook at `app/api/webhooks/twilio-whatsapp/route.ts`.
  - **Effort:** 2 hours. Add a command detector ("switch to english" / "انجليزي") that bypasses the lock, or a `/lang` slash command, or surface the toggle in the dashboard WhatsApp inbox.

- [ ] **`convex/bookings/mutations.ts` has no delete mutation**
  - **Why:** There's no way to delete a booking from the adapter or from a test script. For cleanups (like the test booking noted above) the only option is `markBookingReminderSent` with status tweaks or manually marking `status: cancelled` — neither frees the row.
  - **Where:** `mawidi-site/convex/bookings/mutations.ts` (only has `create`, `updateStatus`, `addNote`, `cancel`, `reschedule`, `updateDepositInfo`, `markBookingReminderSent`).
  - **Effort:** 10 min. Mirror `deleteOrgConversation` pattern from `convex/whatsapp/mutations.ts`. Also consider cascade — `booking_notes` references `bookingId`.

---

## 🟢 Reminder Feature Enhancements

- [ ] **Hard-coded Doha timezone in appointment reminder service**
  - **Why:** `lib/services/appointment-reminder.service.ts` `getTomorrowWindowDoha()` bakes in `DOHA_OFFSET_MS = 3 * 60 * 60 * 1000`. If you onboard an org in Riyadh (UTC+3, same), Dubai (UTC+4), or Kuwait (UTC+3) they work by coincidence; a Beirut or Istanbul org would see reminders sent 1 hour late. And if Qatar ever shifts to DST (unlikely but possible) everything breaks.
  - **Where:** `mawidi-site/lib/services/appointment-reminder.service.ts` lines ~60-90.
  - **Effort:** 1-2 hours. Add `organizations.timezone` (IANA string like `Asia/Qatar`), default to `Asia/Qatar` on create, replace the hard-coded offset with a proper tz-aware calculation using `Intl.DateTimeFormat` or `@date-fns/tz`.

- [ ] **Per-org / per-customer reminder opt-out**
  - **Why:** Some businesses may not want reminders (e.g. walk-in only). Some customers may opt out for privacy. Currently everyone in the candidate window gets a message.
  - **Where:** Add `organizations.appointmentRemindersEnabled: boolean` (default true) and check it in `appointmentReminderService.sendReminderForBooking`. Customer-level opt-out would need a new table (`customer_preferences` keyed by org + phone) or a flag on `org_whatsapp_conversations`.
  - **Effort:** 30 min for org-level; 2 hours including a dashboard toggle.

- [ ] **500-booking cap in `getBookingsDueForReminder`**
  - **Why:** The query calls `.take(500)` per status. For the clinic that's fine forever, but if a multi-location chain ever has >500 bookings on a single day the tail will be dropped silently.
  - **Where:** `mawidi-site/convex/bookings/queries.ts` `getBookingsDueForReminder` handler.
  - **Effort:** 30 min. Switch to pagination via `.paginate()`, or chunk the window by hour (call 24× with 1h windows).

- [ ] **Second reminder option ("morning-of")**
  - **Why:** 24h is the industry default but clinics with frequent no-shows often want a second reminder 2-4h before the appointment. We don't track a second send separately — `reminderSentAt` would skip it.
  - **Where:** `convex/schema/bookings.ts` add `secondReminderSentAt`, `reminderSentCount`. Extend `appointmentReminderService` with a second mode or a separate service.
  - **Effort:** 2-3 hours.

- [ ] **`reminderStatus === "failed"` retry cooldown not implemented**
  - **Why:** The commit message + plan mentioned "skip reminderStatus='failed' AND updated > 3h ago" to avoid spamming retries for permanently-broken bookings. The current code leaves `reminderSentAt` null on failure, so the next cron run will retry forever.
  - **Where:** `mawidi-site/convex/bookings/queries.ts` `getBookingsDueForReminder` — add a filter that skips `reminderStatus === 'failed'` within N hours of the last attempt.
  - **Effort:** 20 min.

- [ ] **No reminder for walk-in / manual bookings with no phone**
  - **Why:** The service skips bookings with no `customerPhone`, but also marks them `reminderStatus: "skipped"` which means the query will keep picking them up on every run (since I also skip `reminderStatus === "skipped"` now — actually I don't, I only skip rows where `reminderSentAt` is set). Review the skip logic so no-phone bookings aren't re-processed forever.
  - **Where:** `lib/services/appointment-reminder.service.ts` `sendReminderForBooking` early-return branches.
  - **Effort:** 10 min. After a permanent skip, also clear the row from the candidate set (e.g. set `reminderSentAt` to a sentinel like 0 or `reminderStatus: 'skipped'` and exclude it from the query).

---

## 🔵 Dashboard / UX

- [ ] **No dashboard UI for editing `company_knowledge_base`**
  - **Why:** Related to the "two KB tables" item above. Even after unifying, the WhatsApp-specific entries I seeded (16 rows across 6 categories × 2 languages) are invisible in the dashboard because the UI only reads `knowledge_base_faqs`. A clinic owner has no way to edit what their WhatsApp AI actually answers with unless we fix the unification.

- [ ] **`userName` shows "…." for new WhatsApp customers until name is collected**
  - **Why:** The Twilio webhook stores the customer's WhatsApp profile name in `org_whatsapp_conversations.userName`. For customers who hide their profile name (common) it arrives as "…." and gets saved verbatim. I added logic earlier in the session that overwrites `userName` from `result.newState.collected.name` when the AI collects a real name during booking — but if they never book, the placeholder sticks.
  - **Where:** `mawidi-site/app/api/webhooks/twilio-whatsapp/route.ts` around the conversation upsert.
  - **Effort:** 15 min. Sanitize "…." / empty strings to `null` on insert, display "Unknown" in the inbox UI when null.

- [ ] **Clerk webpack warning: "Module parse failed" on dev server**
  - **Why:** (Not observed in this session, but a known sticky issue — only include if you see it spam the logs.)
  - **Where:** N/A — remove this item if it never shows up.

---

## 🟣 Security Review Items

- [ ] **`/api/dev/*` routes exist in the production Docker image**
  - **Why:** Even though they're gated on `ENABLE_DEV_ROUTES=1`, having any code path that can wipe data, invoke LLMs, or trigger outbound Twilio sends in a production artifact is a weak defense-in-depth. A future config mistake could enable them.
  - **Where:** `mawidi-site/app/api/dev/seed-kb/route.ts`, `mawidi-site/app/api/dev/ai-ask/route.ts`, `mawidi-site/app/api/dev/run-reminders/route.ts`, `mawidi-site/app/api/dev/clerk-debug/`, `mawidi-site/app/api/dev/get-otp/`.
  - **Effort:** 2 options. (a) Gate at build time — wrap the handlers in `if (process.env.NODE_ENV === 'production') { notFound() }` so the route returns 404 in prod builds. (b) Add a `next.config.js` rewrite/exclude rule to not bundle them in prod. Option (a) is simpler.

- [ ] **No rate limit on `/api/dev/ai-ask`**
  - **Why:** Even with the env flag check, if someone flips the flag to debug, the route can be called unlimited times and will spend LLM tokens on every request. The other dev routes aren't expensive, but this one is.
  - **Where:** `mawidi-site/app/api/dev/ai-ask/route.ts`.
  - **Effort:** 15 min. Add the same Upstash ratelimit pattern used in `app/api/webhooks/twilio-whatsapp/route.ts` (10 req/min per IP).

- [ ] **`getCompanyNameByAppUserId` leaks companyName without auth**
  - **Why:** `convex/auth/queries.ts:93` is a public query — anyone with a valid `appUserId` can fetch any user's `companyName`. That ID is a cuid and not easily guessable, but it's still PII and the query has no auth check. Low severity, but note it.
  - **Where:** `convex/auth/queries.ts:93`.
  - **Effort:** 10 min. Convert to `internalQuery` if only called server-side, or add an `auth.check()` guard.

---

## 📝 Documentation

- [ ] **No README section for the reminder feature**
  - **Why:** New engineers coming to the codebase won't know the cron URL, where to set `CRON_SECRET`, or that the cPanel command shape matters (`Authorization: Bearer …` header, not a query param).
  - **Where:** Add a section to `mawidi-site/README.md` or `mawidi-site/claudedocs/features.md`.
  - **Effort:** 15 min. Copy the relevant bits from the feature commit message.

- [ ] **Cron + dev env vars missing from `.env.example`**
  - **Why:** `.env.example` is the template new developers copy. It doesn't mention `CRON_SECRET` or `ENABLE_DEV_ROUTES`, so new devs either won't enable them or will copy the committed dev value.
  - **Where:** `mawidi-site/.env.example`.
  - **Effort:** 2 min.

---

## ✅ Done

_(Move items here when shipped; keep a one-line summary + commit hash for traceability.)_

- _(nothing yet — this file is new)_