# Ticket Category Routing and Background Email Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Route a ticket-notification email to the right team inbox by category, confirm receipt to the customer who raised it, and stop making the chat wait on any outbound email actually finishing before replying. **Architecture:** Extend the existing ticket tool and settings system with a category concept and a per-tenant, per-category business-email list; fire outbound email sends as background `asyncio` tasks instead of `await`ing them inside the chat tool loop. **Tech Stack:** FastAPI, PostgreSQL (per-tenant schema via `psycopg2`), the existing `app/services/chat/tools_settings.py` settings store, `app/services/messaging/email.py` send path, `asyncio`. **Spec:** `docs/superpowers/specs/2026-08-21-ticket-category-routing-and-background-email-design.md` ## Global Constraints - Categories are a fixed tuple: `("General enquiries", "Sales", "Business")` — not tenant-configurable. - `ticket_emails` save semantics are **full replace**, not merge — a `POST /tools-settings` call including `ticket_emails` replaces the tenant's entire stored list. - If a ticket's category has no configured business email, send nothing to the team (no fallback to "notify everyone"). - Category is decided by the model via a new required tool-schema enum param — no backend keyword classification. - A failed background email send is logged server-side only — no dashboard event, no customer-facing signal, in this iteration. - Notification/confirmation emails fire only when a ticket is *created*, never on an update/consolidation. - All new settings-layer code lives in `app/services/chat/tools_settings.py`; all new tool-executor code lives in `app/services/chat/tools.py`; all new persistence code lives in `app/services/infra/database.py` — follow the file each existing piece of related code already lives in. --- ### Task 1: `ticket_emails` setting — storage, validation, category tuple **Files:** - Modify: `app/services/chat/tools_settings.py` - Test: `tests/unit/test_ticket_emails_settings.py` (new) **Interfaces:** - Consumes: `is_valid_email`-equivalent logic (duplicated locally — see step 3 note on why, not imported from `email.py`). - Produces: `TICKET_CATEGORIES: tuple[str, ...]`, `get_tool_settings(tenant_id)["ticket_emails"]: list[dict]`, `save_tool_settings(tenant_id, ticket_emails=[...])` accepting and validating the new list. - [ ] **Step 1: Write the failing tests** ```python # tests/unit/test_ticket_emails_settings.py """ticket_emails: a per-tenant, per-category list of team notification addresses.""" from unittest.mock import MagicMock import pytest from app.services.chat.tools_settings import ( TICKET_CATEGORIES, apply_over_defaults, save_tool_settings, ) def _mock_store(monkeypatch, initial=None): stored = {"value": initial} def fake_update_setting(tenant_id, key, fn): stored["value"] = fn(stored["value"]) return stored["value"] def fake_get_tool_settings(tenant_id): return apply_over_defaults(stored["value"]) monkeypatch.setattr( "app.services.chat.tools_settings.update_setting", fake_update_setting) monkeypatch.setattr( "app.services.chat.tools_settings.read_legacy_blob", lambda t: None) monkeypatch.setattr( "app.services.chat.tools_settings.get_tool_settings", fake_get_tool_settings) return stored def test_a_fresh_tenant_has_no_business_emails(): assert apply_over_defaults(None)["ticket_emails"] == [] def test_the_category_tuple_has_exactly_the_three_dashboard_options(): assert TICKET_CATEGORIES == ("General enquiries", "Sales", "Business") def test_a_legacy_stored_blob_with_no_ticket_emails_key_does_not_crash(): settings = apply_over_defaults({"features": {}, "email": {}}) assert settings["ticket_emails"] == [] def test_save_accepts_a_valid_list(monkeypatch): _mock_store(monkeypatch) saved = save_tool_settings("org_ticket_test", ticket_emails=[ {"email": "sales@acme.com", "category": "Sales"}, {"email": "support@acme.com", "category": "General enquiries"}, ]) assert saved["ticket_emails"] == [ {"email": "sales@acme.com", "category": "Sales"}, {"email": "support@acme.com", "category": "General enquiries"}, ] def test_save_rejects_an_invalid_email(monkeypatch): _mock_store(monkeypatch) with pytest.raises(ValueError, match="not a valid email"): save_tool_settings("org_ticket_test", ticket_emails=[ {"email": "not-an-email", "category": "Sales"}]) def test_save_rejects_an_unknown_category(monkeypatch): _mock_store(monkeypatch) with pytest.raises(ValueError, match="must be one of"): save_tool_settings("org_ticket_test", ticket_emails=[ {"email": "sales@acme.com", "category": "Marketing"}]) def test_save_rejects_an_entry_missing_a_field(monkeypatch): _mock_store(monkeypatch) with pytest.raises(ValueError, match="email.*category"): save_tool_settings("org_ticket_test", ticket_emails=[{"email": "sales@acme.com"}]) def test_save_replaces_the_whole_list_not_merges(monkeypatch): _mock_store(monkeypatch, initial={ "features": {}, "email": {}, "ticket_emails": [{"email": "old@acme.com", "category": "Sales"}]}) saved = save_tool_settings("org_ticket_test", ticket_emails=[ {"email": "new@acme.com", "category": "Business"}]) assert saved["ticket_emails"] == [{"email": "new@acme.com", "category": "Business"}] def test_omitting_ticket_emails_leaves_the_stored_list_untouched(monkeypatch): _mock_store(monkeypatch, initial={ "features": {}, "email": {}, "ticket_emails": [{"email": "old@acme.com", "category": "Sales"}]}) saved = save_tool_settings("org_ticket_test", features={"voice_mode": False}) assert saved["ticket_emails"] == [{"email": "old@acme.com", "category": "Sales"}] def test_save_accepts_an_empty_list_to_clear_all_business_emails(monkeypatch): _mock_store(monkeypatch, initial={ "features": {}, "email": {}, "ticket_emails": [{"email": "old@acme.com", "category": "Sales"}]}) saved = save_tool_settings("org_ticket_test", ticket_emails=[]) assert saved["ticket_emails"] == [] ``` - [ ] **Step 2: Run tests to verify they fail** Run: `.venv/bin/python -m pytest tests/unit/test_ticket_emails_settings.py -v` Expected: FAIL — `TICKET_CATEGORIES` does not exist, `ticket_emails` keyword not accepted. - [ ] **Step 3: Implement** In `app/services/chat/tools_settings.py`, add near `PROVIDER_REQUIRES_FEATURE` (after it, before `EMAIL_COPY_DEFAULTS`): ```python # The three categories the "Business Emails" dashboard screen offers. Fixed, # not tenant-configurable -- adding a fourth means editing this tuple and the # dashboard's dropdown together, not a per-tenant setting. TICKET_CATEGORIES = ("General enquiries", "Sales", "Business") # A lightweight duplicate of email.py's EMAIL_RE: email.py already imports # get_email_settings/get_tool_settings from this module, so importing # is_valid_email back from email.py here would create an import cycle. A # one-line regex is cheaper than restructuring either module for it. import re as _re _TICKET_EMAIL_RE = _re.compile(r"^[^@\s]+@[^@\s]+\.[a-zA-Z]{2,}$") ``` Update `_defaults()`: ```python def _defaults() -> dict: return {"features": dict(FEATURE_DEFAULTS), "email": dict(EMAIL_COPY_DEFAULTS), "providers": dict(PROVIDER_DEFAULTS), "ticket_emails": []} ``` Update `apply_over_defaults()` — add after the `providers` loop, before the legacy-flat-shape block: ```python stored_ticket_emails = saved.get("ticket_emails") if isinstance(stored_ticket_emails, list): settings["ticket_emails"] = stored_ticket_emails ``` Update `_merge_settings()` signature and body: ```python def _merge_settings(tenant_id: str, features: dict, email: dict, providers: dict, ticket_emails: list, current) -> dict: """...""" base = apply_over_defaults(current or read_legacy_blob(tenant_id)) for name, value in (features or {}).items(): base["features"][name] = bool(value) for name, value in (email or {}).items(): base["email"][name] = (value or "").strip() or EMAIL_COPY_DEFAULTS[name] for name, value in (providers or {}).items(): base["providers"][name] = value # Full replace, not merge -- matches the "add rows locally, Save once" # dashboard flow; there is no per-entry identity to merge against anyway. if ticket_emails is not None: base["ticket_emails"] = ticket_emails return base ``` Update `save_tool_settings()` signature, validation, and the `update_setting` call: ```python def save_tool_settings(tenant_id: str, features: dict = None, email: dict = None, providers: dict = None, ticket_emails: list = None) -> dict: """Merge the supplied switches, copy, provider choices and business emails, and persist.""" tenant_id = normalise_tenant_id(tenant_id) unknown = set(features or {}) - set(FEATURE_DEFAULTS) if unknown: raise ValueError(f"Unknown feature(s): {', '.join(sorted(unknown))}") unknown = set(email or {}) - set(EMAIL_COPY_DEFAULTS) if unknown: raise ValueError(f"Unknown email setting(s): {', '.join(sorted(unknown))}") unknown = set(providers or {}) - set(PROVIDER_OPTIONS) if unknown: raise ValueError(f"Unknown provider setting(s): {', '.join(sorted(unknown))}") for name, value in (providers or {}).items(): if value is not None and value not in PROVIDER_OPTIONS[name]: raise ValueError( f"{name} must be one of {PROVIDER_OPTIONS[name]} or null, got {value!r}") if ticket_emails is not None: for entry in ticket_emails: if not isinstance(entry, dict) or "email" not in entry or "category" not in entry: raise ValueError( "Each ticket_emails entry needs both email and category.") if not _TICKET_EMAIL_RE.match((entry.get("email") or "").strip()): raise ValueError(f"{entry.get('email')!r} is not a valid email address.") if entry["category"] not in TICKET_CATEGORIES: raise ValueError( f"category must be one of {TICKET_CATEGORIES}, got {entry['category']!r}") needing_gate = [name for name, value in (providers or {}).items() if value is not None and name in PROVIDER_REQUIRES_FEATURE] if needing_gate: effective_features = dict(get_tool_settings(tenant_id)["features"]) effective_features.update(features or {}) for name in needing_gate: gate = PROVIDER_REQUIRES_FEATURE[name] if not effective_features.get(gate): raise ValueError( f"{name} cannot be set while {gate} is off; enable it first " f"or in the same request") if (providers or {}).get("send_email") == "mailchimp": if not get_mailchimp_connection(tenant_id): raise ValueError( "Connect Mailchimp under Integrations before selecting it as your " "email provider.") merged = update_setting( tenant_id, SETTINGS_KEY, partial(_merge_settings, tenant_id, features, email, providers, ticket_emails)) off = sorted(n for n, on in merged["features"].items() if not on) logger.info(f"Saved tool settings for {tenant_id}; off: {off or 'none'}") return merged ``` (Only the signature, the new `ticket_emails` validation block, and the `partial(...)` call change — the rest of the function body is unchanged from its current state.) - [ ] **Step 4: Run tests to verify they pass** Run: `.venv/bin/python -m pytest tests/unit/test_ticket_emails_settings.py -v` Expected: PASS, all 9 tests. - [ ] **Step 5: Run the full existing provider-settings suite to confirm no regression** Run: `.venv/bin/python -m pytest tests/unit/test_provider_settings.py -v` Expected: PASS, all 24 tests (unchanged — `ticket_emails` is additive). - [ ] **Step 6: Commit** ```bash git add app/services/chat/tools_settings.py tests/unit/test_ticket_emails_settings.py git commit -m "feat: ticket_emails setting -- per-category business email list" ``` --- ### Task 2: `category` column and CRUD support in the ticket table **Files:** - Modify: `app/services/infra/database.py` **Interfaces:** - Consumes: nothing new. - Produces: `create_ticket(tenant_id, ticket_data, thread_id=None)` persists `ticket_data.get("category")`; `get_ticket`, `get_tickets`, `get_ticket_by_thread` return a `"category"` key; `update_ticket(tenant_id, ticket_id, update_data)` persists `update_data.get("category")` when present. No unit tests in this task — `database.py` has no existing unit-test coverage anywhere in this repo (it is a raw `psycopg2` layer, verified only against the real per-tenant schema, same as the Mailchimp DB work earlier this branch). Step 6 below verifies live against the real dev database instead. - [ ] **Step 1: Add the column migration** In `bootstrap_tenant`, immediately after the existing thread_id migration line (currently around line 275): ```python # Migration: Ensure thread_id column exists cur.execute(sql.SQL("ALTER TABLE {}.strategist_tickets ADD COLUMN IF NOT EXISTS thread_id TEXT").format(sql.Identifier(tenant_id))) # Migration: Ensure category column exists cur.execute(sql.SQL("ALTER TABLE {}.strategist_tickets ADD COLUMN IF NOT EXISTS category TEXT").format(sql.Identifier(tenant_id))) ``` Also add `category TEXT` to the `CREATE TABLE IF NOT EXISTS {}.strategist_tickets (...)` DDL block (right after the `priority TEXT NOT NULL,` line) so a brand-new tenant gets the column at creation time, not only via the `ALTER` migration. - [ ] **Step 2: `create_ticket` persists category** Change the `INSERT` in `create_ticket` (around line 726) to: ```python query = sql.SQL(""" INSERT INTO {}.strategist_tickets (ticket_id, user_name, heading, content, priority, status, email, contact_no, contact_medium, thread_id, category) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) RETURNING id """).format(sql.Identifier(tenant_id)) ``` and both `cur.execute(query, (...))` calls in that function (the first attempt and the retry-after-self-heal), adding `ticket_data.get('category') or 'General enquiries'` as the final tuple element in each. - [ ] **Step 3: `get_ticket_by_thread` returns category** Change the `SELECT` (around line 791) to: ```python query = sql.SQL("SELECT id, ticket_id, heading, content, priority, status, category FROM {}.strategist_tickets WHERE thread_id = %s LIMIT 1").format(sql.Identifier(tenant_id)) ``` and the returned dict to add `"category": row[6]`. - [ ] **Step 4: `update_ticket` persists category when supplied** Change `update_ticket` (around line 823) to: ```python def update_ticket(tenant_id: str, ticket_id: str, update_data: dict): """ Updates an existing ticket with new details. """ logger.info(f"Updating ticket {ticket_id} for tenant: {tenant_id}") conn = get_db_connection() try: with conn.cursor() as cur: query = sql.SQL(""" UPDATE {}.strategist_tickets SET heading = %s, content = %s, priority = %s, category = %s WHERE ticket_id = %s """).format(sql.Identifier(tenant_id)) cur.execute(query, ( update_data['heading'], update_data['content'], update_data['priority'], update_data.get('category'), ticket_id )) conn.commit() return True except Exception as e: logger.error(f"Failed to update ticket {ticket_id}: {e}", exc_info=True) conn.rollback() return False finally: conn.close() ``` - [ ] **Step 5: `get_ticket` and `get_tickets` return category** In `get_ticket` (around line 891), change the `SELECT` to add `, category` at the end of the column list, and add `"category": row[13]` to the returned dict (it becomes the 14th column, index 13, after `thread_id` at index 12). In `get_tickets` (around line 933), make the same `SELECT` column-list change, and add `"category": row[13]` to the per-row dict built in the loop. - [ ] **Step 6: Verify live against the real dev database** ```bash .venv/bin/python -c " from app.services.infra.database import bootstrap_tenant, create_ticket, get_ticket, get_tickets tenant = 'org_00000000-0000-0000-0000-000000000098' bootstrap_tenant(tenant) ticket_data = {'ticket_id': 'TICK-PLANTEST', 'user_name': 'Test', 'heading': 'h', 'content': 'c', 'priority': 'Low', 'category': 'Sales', 'email': 'a@b.com'} create_ticket(tenant, ticket_data, thread_id='th-plantest') print(get_ticket(tenant, 'TICK-PLANTEST')) print(get_tickets(tenant)) " ``` Expected: both prints show `'category': 'Sales'`. Then drop the throwaway schema: ```bash .venv/bin/python -c " from app.services.infra.database import get_db_connection conn = get_db_connection() with conn.cursor() as cur: cur.execute('DROP SCHEMA IF EXISTS \"org_00000000-0000-0000-0000-000000000098\" CASCADE') conn.commit() conn.close() " ``` - [ ] **Step 7: Commit** ```bash git add app/services/infra/database.py git commit -m "feat: persist a ticket's category through the full CRUD path" ``` --- ### Task 3: `category` on the ticket tool schema **Files:** - Modify: `app/services/chat/tools.py` - Modify: `tests/unit/test_chat_tools.py` **Interfaces:** - Consumes: `TICKET_CATEGORIES` from Task 1 (`app.services.chat.tools_settings`). - Produces: `TOOL_SCHEMAS`'s `create_or_update_ticket` entry gains a `category` param; `execute_create_or_update_ticket` sets `ticket_data["category"]` on create, and `update_data["category"]` on update. - [ ] **Step 1: Write the failing tests** Add to `tests/unit/test_chat_tools.py`, near `test_ticket_schema_required_fields`: ```python def test_ticket_schema_has_a_category_enum(): ticket = next(t for t in TOOL_SCHEMAS if t["function"]["name"] == "create_or_update_ticket") params = ticket["function"]["parameters"] assert "category" in params["properties"] assert params["properties"]["category"]["enum"] == list(TICKET_CATEGORIES) assert "category" in params["required"] ``` Add `from app.services.chat.tools_settings import TICKET_CATEGORIES` to the file's imports. Change `TICKET_ARGS` to include a category (existing tests reuse this fixture): ```python TICKET_ARGS = { "user_name": "Jo", "heading": "Login broken", "content": "Cannot log in", "priority": "High", "email": "jo@x.com", "contact_no": "123", "category": "General enquiries", } ``` Add a new test confirming the category reaches `create_ticket`. Task 3 does not call `_notify_ticket` yet (that call site is added in Task 4), so these tests do not patch or reference it — they only check what reaches `create_ticket`: ```python @patch("app.services.chat.tools.publish_event", new_callable=AsyncMock) @patch("app.services.chat.tools.get_ticket") @patch("app.services.chat.tools.create_ticket") @patch("app.services.chat.tools.get_ticket_by_thread") def test_a_created_tickets_category_reaches_create_ticket( mock_by_thread, mock_create, mock_get, mock_publish): from app.services.chat.tools import execute_create_or_update_ticket mock_by_thread.return_value = None mock_create.return_value = 1 mock_get.return_value = {"ticket_id": "TICK-X"} _run(execute_create_or_update_ticket(dict(TICKET_ARGS), _ctx())) ticket_data = mock_create.call_args[0][1] assert ticket_data["category"] == "General enquiries" @patch("app.services.chat.tools.publish_event", new_callable=AsyncMock) @patch("app.services.chat.tools.get_ticket") @patch("app.services.chat.tools.create_ticket") @patch("app.services.chat.tools.get_ticket_by_thread") def test_a_ticket_created_with_no_category_defaults_to_general_enquiries( mock_by_thread, mock_create, mock_get, mock_publish): from app.services.chat.tools import execute_create_or_update_ticket mock_by_thread.return_value = None mock_create.return_value = 1 mock_get.return_value = {"ticket_id": "TICK-X"} args = {k: v for k, v in TICKET_ARGS.items() if k != "category"} _run(execute_create_or_update_ticket(dict(args), _ctx())) assert mock_create.call_args[0][1]["category"] == "General enquiries" ``` - [ ] **Step 2: Run tests to verify they fail** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -v` Expected: FAIL — `category` missing from the tool schema, and the two new tests assert a `"category"` key that `create_ticket` is not yet called with. - [ ] **Step 3: Add the schema field** In `app/services/chat/tools.py`, add `from app.services.chat.tools_settings import get_email_settings, TICKET_CATEGORIES, get_tool_settings` (extending the existing `tools_settings` import line rather than duplicating it). In the `create_or_update_ticket` schema's `properties` (around line 92-100), add: ```python "category": { "type": "string", "enum": list(TICKET_CATEGORIES), "description": ( "Which team should handle this: General enquiries, Sales, " "or Business. Pick from the conversation's content." ), }, ``` and add `"category"` to the `"required"` list alongside `"heading", "content", "priority"`. - [ ] **Step 4: Wire category into the executor** In `execute_create_or_update_ticket`, in the update branch (around line 367-378), add `"category"` to both `update_data` dicts (the consolidation-success one and the fallback one): ```python update_data = { "heading": cons.get("heading", args["heading"]), "content": cons.get("content", f"{existing['content']}\n\n---\n{args['content']}"), "priority": args["priority"], "category": args.get("category") or existing.get("category") or "General enquiries", } ``` and the same `"category"` line added to the exception-fallback `update_data` dict a few lines below it. In the create branch (around line 386-389), before `new_id = create_ticket(...)`: ```python ticket_data = dict(args) ticket_data["ticket_id"] = f"TICK-{uuid.uuid4().hex[:8].upper()}" ticket_data["category"] = args.get("category") or "General enquiries" if user_name: ticket_data["user_name"] = user_name if user_id: ticket_data["user_id"] = user_id ``` (Only the new `ticket_data["category"] = ...` line is added; everything else in this block is unchanged.) - [ ] **Step 5: Run tests to verify they pass** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -v` Expected: PASS, all tests in the file. - [ ] **Step 6: Commit** ```bash git add app/services/chat/tools.py tests/unit/test_chat_tools.py git commit -m "feat: the ticket tool classifies each ticket into a category" ``` --- ### Task 4: Background email helper + ticket notification/confirmation emails **Files:** - Modify: `app/services/chat/tools.py` - Modify: `tests/unit/test_chat_tools.py` **Interfaces:** - Consumes: `get_tool_settings(tenant_id)["ticket_emails"]` (Task 1), `ticket_data["category"]` (Task 3), `send_email` and `is_valid_email` (already imported). - Produces: `_fire_and_forget_email(to_email, subject, body, tenant_id, first_name=None, on_sent=None) -> asyncio.Task`; `_notify_ticket(tenant_id, ticket_data)`, called from the create branch (this task adds that call site). - [ ] **Step 1: Write the failing tests** Add to `tests/unit/test_chat_tools.py`: ```python @patch("app.services.chat.tools.asyncio.create_task") def test_fire_and_forget_email_returns_immediately_without_awaiting_send(mock_create_task): from app.services.chat.tools import _fire_and_forget_email mock_create_task.return_value = MagicMock() _fire_and_forget_email("a@b.com", "Subject", "Body", "org_test") mock_create_task.assert_called_once() @patch("app.services.chat.tools.get_tool_settings") @patch("app.services.chat.tools._fire_and_forget_email") def test_notify_ticket_emails_only_matching_category(mock_fire, mock_settings): from app.services.chat.tools import _notify_ticket mock_settings.return_value = {"ticket_emails": [ {"email": "sales@acme.com", "category": "Sales"}, {"email": "support@acme.com", "category": "General enquiries"}, ]} ticket_data = {"ticket_id": "TICK-1", "category": "Sales", "heading": "h", "content": "c", "priority": "High", "user_name": "Jo"} _notify_ticket("org_test", ticket_data) team_calls = [c for c in mock_fire.call_args_list if c.args[0] == "sales@acme.com"] assert len(team_calls) == 1 assert not any(c.args[0] == "support@acme.com" for c in mock_fire.call_args_list) @patch("app.services.chat.tools.get_tool_settings") @patch("app.services.chat.tools._fire_and_forget_email") def test_notify_ticket_sends_nothing_when_no_category_match(mock_fire, mock_settings): from app.services.chat.tools import _notify_ticket mock_settings.return_value = {"ticket_emails": [ {"email": "sales@acme.com", "category": "Sales"}]} ticket_data = {"ticket_id": "TICK-1", "category": "Business", "heading": "h", "content": "c", "priority": "High", "user_name": "Jo"} _notify_ticket("org_test", ticket_data) assert mock_fire.call_count == 0 @patch("app.services.chat.tools.get_tool_settings") @patch("app.services.chat.tools._fire_and_forget_email") def test_notify_ticket_confirms_to_the_customer_when_email_given(mock_fire, mock_settings): from app.services.chat.tools import _notify_ticket mock_settings.return_value = {"ticket_emails": []} ticket_data = {"ticket_id": "TICK-1", "category": "Sales", "heading": "h", "content": "c", "priority": "High", "user_name": "Jo", "email": "jo@x.com"} _notify_ticket("org_test", ticket_data) customer_calls = [c for c in mock_fire.call_args_list if c.args[0] == "jo@x.com"] assert len(customer_calls) == 1 @patch("app.services.chat.tools.get_tool_settings") @patch("app.services.chat.tools._fire_and_forget_email") def test_notify_ticket_skips_customer_email_when_none_given(mock_fire, mock_settings): from app.services.chat.tools import _notify_ticket mock_settings.return_value = {"ticket_emails": []} ticket_data = {"ticket_id": "TICK-1", "category": "Sales", "heading": "h", "content": "c", "priority": "High", "user_name": "Jo"} _notify_ticket("org_test", ticket_data) assert mock_fire.call_count == 0 ``` - [ ] **Step 2: Run tests to verify they fail** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -v` Expected: FAIL — `_fire_and_forget_email` and `_notify_ticket` do not exist. - [ ] **Step 3: Implement** Add `import asyncio` to the top of `app/services/chat/tools.py` (alongside the existing `import json`, `import logging`, `import uuid`, `import anyio`). Add, near `_record_email_sent` (just before `execute_send_email`, around line 495): ```python # Tasks created here are not awaited by anything, so nothing keeps a # reference to them once this module's local variable goes out of scope -- # without this set, asyncio may garbage-collect a task mid-send. See # https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task _background_email_tasks = set() def _fire_and_forget_email(to_email, subject, body, tenant_id, first_name=None, on_sent=None): """Send in the background; the caller never waits on this. on_sent, if given, is an async callable run only after a confirmed successful send -- used to keep execute_send_email's per-thread rate counter accurate without blocking the tool-call round trip on it. """ async def _send(): try: result = await anyio.to_thread.run_sync( lambda: send_email(to_email, subject, body, first_name=first_name, tenant_id=tenant_id)) if result.get("status") == "sent": if on_sent: await on_sent() else: logger.error(f"Background email to {to_email} failed: {result.get('detail')}") except Exception as ex: logger.error(f"Background email to {to_email} raised: {ex}", exc_info=True) task = asyncio.create_task(_send()) _background_email_tasks.add(task) task.add_done_callback(_background_email_tasks.discard) return task def _ticket_team_email_content(ticket_data): ticket_id = ticket_data["ticket_id"] subject = f"New {ticket_data.get('category')} ticket: {ticket_data['heading']}" lines = [ f"Ticket ID: {ticket_id}", f"Category: {ticket_data.get('category')}", f"Priority: {ticket_data.get('priority')}", f"From: {ticket_data.get('user_name') or 'Unknown'}", ] if ticket_data.get("email"): lines.append(f"Email: {ticket_data['email']}") if ticket_data.get("contact_no"): lines.append(f"Phone: {ticket_data['contact_no']}") lines.append("") lines.append(ticket_data.get("content") or "") return subject, "\n".join(lines) def _ticket_customer_email_content(ticket_data): ticket_id = ticket_data["ticket_id"] subject = f"We've received your request — {ticket_id}" body = ( f"Thanks for reaching out. Your ticket has been logged.\n\n" f"Ticket ID: {ticket_id}\n" f"Summary: {ticket_data['heading']}\n\n" f"Our team will follow up with you shortly." ) return subject, body def _notify_ticket(tenant_id, ticket_data): """Email the matching business team, and confirm receipt to the customer. Only called on ticket CREATION, never on update -- see the module's calling code. A category with no configured business email sends nothing to the team; that is intentional, not a bug (see the design spec). """ category = ticket_data.get("category") or "General enquiries" team_emails = [entry["email"] for entry in get_tool_settings(tenant_id).get("ticket_emails", []) if entry.get("category") == category] if team_emails: subject, body = _ticket_team_email_content(ticket_data) for address in team_emails: _fire_and_forget_email(address, subject, body, tenant_id) customer_email = (ticket_data.get("email") or "").strip() if customer_email and is_valid_email(customer_email): subject, body = _ticket_customer_email_content(ticket_data) _fire_and_forget_email(customer_email, subject, body, tenant_id, first_name=ticket_data.get("user_name")) ``` Then in `execute_create_or_update_ticket`'s create branch, call it right after the ticket is confirmed created: ```python new_id = create_ticket(tenant_id, ticket_data, thread_id=thread_id) if new_id: await _publish_ticket_event(tenant_id, ticket_data["ticket_id"], "new_ticket_created", f"A new ticket has been created: {ticket_data['ticket_id']}") _notify_ticket(tenant_id, ticket_data) return {"status": "created", "ticket_id": ticket_data["ticket_id"]} return {"status": "error", "ticket_id": None} ``` (Only the added `_notify_ticket(tenant_id, ticket_data)` line — everything else in this branch is unchanged.) - [ ] **Step 4: Run tests to verify they pass** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -v` Expected: PASS, all tests in the file, including this task's new ones. - [ ] **Step 5: Commit** ```bash git add app/services/chat/tools.py tests/unit/test_chat_tools.py git commit -m "feat: notify the matching team inbox and confirm to the customer on ticket creation" ``` --- ### Task 5: `execute_send_email` becomes fire-and-forget, and tool-description wording **Files:** - Modify: `app/services/chat/tools.py` - Modify: `tests/unit/test_chat_tools.py` **Interfaces:** - Consumes: `_fire_and_forget_email` and `_background_email_tasks` from Task 4. - Produces: `execute_send_email` returns `{"status": "queued", "to": to_email}` instead of waiting for the send; `_record_email_sent` still only increments on a confirmed successful send, just from inside the background task rather than the awaited call. - [ ] **Step 1: Write the failing test** Add to `tests/unit/test_chat_tools.py`: ```python @patch("app.services.chat.tools._fire_and_forget_email") @patch("app.services.chat.tools.get_email_settings") @patch("app.services.chat.tools.email_configured", return_value=True) def test_send_email_returns_queued_without_waiting_on_the_send( mock_configured, mock_email_settings, mock_fire): from app.services.chat.tools import execute_send_email mock_email_settings.return_value = {"enabled": True} args = {"to_email": "user@example.com", "subject": "Hi", "body": "Body text"} result = _run(execute_send_email(args, _ctx())) assert result == {"status": "queued", "to": "user@example.com"} mock_fire.assert_called_once() assert mock_fire.call_args[0][0] == "user@example.com" ``` - [ ] **Step 2: Run test to verify it fails** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -k queued -v` Expected: FAIL — `execute_send_email` still returns `send_email`'s own synchronous result shape (`{"status": "sent", ...}` or an error dict), not `{"status": "queued", ...}`. - [ ] **Step 3: Implement** Replace the tail of `execute_send_email` (from the `EMAIL_MAX_PER_THREAD` block through its `return result`, currently lines ~530-548) with: ```python # EMAIL_MAX_PER_THREAD <= 0 disables the cap entirely. # The counter must survive across turns: tool_ctx is rebuilt on every message, # so an in-memory count would only ever limit a single turn. limit = settings.EMAIL_MAX_PER_THREAD already_sent = await _emails_sent_in_thread(ctx["tenant_id"], ctx["thread_id"]) if limit > 0 else 0 if limit > 0 and already_sent >= limit: logger.warning( f"Email limit ({settings.EMAIL_MAX_PER_THREAD}) reached for thread {ctx.get('thread_id')}" ) return {"status": "error", "detail": "Email limit reached for this conversation. Tell the user you cannot " "send any more emails in this chat."} tenant_id = ctx["tenant_id"] thread_id = ctx["thread_id"] async def _on_sent(): await _record_email_sent(tenant_id, thread_id) logger.info(f"Chat emailed {to_email} for thread {thread_id}") _fire_and_forget_email(to_email, subject, body, tenant_id, first_name=ctx.get("user_name"), on_sent=_on_sent) return {"status": "queued", "to": to_email} ``` Then update the tool's system-facing description (around line 158-173) so the model tells the user it is sending, not that it has already sent, by adding one sentence at the end of the existing description string: ```python "NEVER offer to email, and never call this tool, when your answer is that you " "could not find the information — an email saying you do not know is worse than " "no email. Offer a ticket or a human instead. " "Sending happens in the background after this call returns -- tell the user " "you are sending it now ('I'm sending that over now'), never that it has already " "arrived." ``` Do the same for `create_or_update_ticket`'s description (around line 68-89), adding one sentence at the end: ```python "If the session already has a ticket, do NOT re-ask contact details; just ask what " "the new issue is. " "Any team notification email is also sent in the background -- tell the user " "their ticket has been logged/raised, never that the team has already been " "emailed or has already seen it." ``` - [ ] **Step 4: Run tests to verify they pass** Run: `.venv/bin/python -m pytest tests/unit/test_chat_tools.py -v` Expected: PASS, all tests in the file. - [ ] **Step 5: Run the full unit suite** Run: `.venv/bin/python -m pytest tests/unit -q` Expected: all pass, no regressions (this touches shared tool-schema text other tests may snapshot — check any failure output carefully before assuming it's unrelated). - [ ] **Step 6: Commit** ```bash git add app/services/chat/tools.py tests/unit/test_chat_tools.py git commit -m "feat: send_email no longer blocks the chat reply on the send finishing" ``` --- ### Task 6: `/tools-settings` endpoint accepts and returns `ticket_emails` **Files:** - Modify: `app/api/endpoints.py` - Test: `tests/integration/test_ticket_emails_endpoint.py` (new) **Interfaces:** - Consumes: `save_tool_settings(..., ticket_emails=...)` from Task 1. - Produces: `POST /tools-settings` accepts a top-level `ticket_emails` JSON field and includes it in the response `settings`; `GET /tools-settings` includes it too (automatic, since it reads `get_tool_settings`, already covered by Task 1). `ticket_emails` is JSON-only (no form-field parsing) — the existing form path (`_parse_settings_form`) is the legacy flat-field poster; a list-of-objects field has no natural flat-form encoding, and nothing in this codebase's current form-based caller needs it. Document this rather than inventing a form encoding nothing will use. - [ ] **Step 1: Write the failing tests** ```python # tests/integration/test_ticket_emails_endpoint.py """POST /tools-settings accepts and round-trips ticket_emails via JSON.""" from unittest.mock import patch from fastapi.testclient import TestClient from app.main import app client = TestClient(app) TENANT = "org_8c32bf3e-6a18-4739-9b1c-94c0cf11125f" def test_json_body_passes_ticket_emails_through(): with patch("app.api.endpoints.bootstrap_tenant"), \ patch("app.api.endpoints.save_tool_settings") as save: save.return_value = {"features": {}, "email": {}, "providers": {}, "ticket_emails": []} client.post("/tools-settings", json={ "tenant_id": TENANT, "ticket_emails": [{"email": "sales@acme.com", "category": "Sales"}]}) assert save.call_args.kwargs["ticket_emails"] == [ {"email": "sales@acme.com", "category": "Sales"}] def test_an_empty_ticket_emails_list_is_still_sent_through_to_clear_it(): # Distinct from omitting the key entirely -- an empty list must reach # save_tool_settings so a tenant can remove every business email. with patch("app.api.endpoints.bootstrap_tenant"), \ patch("app.api.endpoints.save_tool_settings") as save: save.return_value = {"features": {}, "email": {}, "providers": {}, "ticket_emails": []} client.post("/tools-settings", json={"tenant_id": TENANT, "ticket_emails": []}) assert save.call_args.kwargs["ticket_emails"] == [] def test_ticket_emails_alone_is_enough_to_pass_validation(): with patch("app.api.endpoints.bootstrap_tenant"), \ patch("app.api.endpoints.save_tool_settings") as save: save.return_value = {"features": {}, "email": {}, "providers": {}, "ticket_emails": []} resp = client.post("/tools-settings", json={ "tenant_id": TENANT, "ticket_emails": [{"email": "sales@acme.com", "category": "Sales"}]}) assert resp.status_code == 200 def test_an_invalid_ticket_email_reaches_the_client_as_a_400(): with patch("app.api.endpoints.bootstrap_tenant"), \ patch("app.api.endpoints.save_tool_settings", side_effect=ValueError("'bad' is not a valid email address.")): resp = client.post("/tools-settings", json={ "tenant_id": TENANT, "ticket_emails": [{"email": "bad", "category": "Sales"}]}) assert resp.status_code == 400 ``` - [ ] **Step 2: Run tests to verify they fail** Run: `.venv/bin/python -m pytest tests/integration/test_ticket_emails_endpoint.py -v` Expected: FAIL — `save_tool_settings` is called without a `ticket_emails` kwarg today. - [ ] **Step 3: Implement** In `save_tools_settings_endpoint` (`app/api/endpoints.py`, around line 523-534), add `ticket_emails` extraction to the JSON branch: ```python if request.headers.get("content-type", "").startswith("application/json"): payload = await request.json() if not isinstance(payload, dict): raise HTTPException(status_code=400, detail={"status": "error", "message": "Body must be an object."}) tenant_id = (payload.get("tenant_id") or "").strip() features, email = payload.get("features") or {}, payload.get("email") or {} providers = payload.get("providers") or {} ticket_emails = payload.get("ticket_emails") else: form = await request.form() tenant_id = (form.get("tenant_id") or "").strip() features, email, providers = _parse_settings_form(form) ticket_emails = None ``` Update the "nothing supplied" guard and the `save_tool_settings` call a few lines below (currently around line 543-555): ```python features = {k: v for k, v in features.items() if v is not None} email = {k: v for k, v in email.items() if v is not None} if not features and not email and not providers and ticket_emails is None: raise HTTPException(status_code=400, detail={"status": "error", "message": "No settings supplied."}) try: bootstrap_tenant(normalise_tenant_id(tenant_id)) settings = save_tool_settings(tenant_id, features=features or None, email=email or None, providers=providers or None, ticket_emails=ticket_emails) return {"status": "success", "settings": settings, "wired": sorted(FEATURE_TOOLS)} except ValueError as ve: raise HTTPException(status_code=400, detail={"status": "error", "message": str(ve)}) except Exception as e: logger.error(f"Error saving tool settings for {tenant_id}: {e}", exc_info=True) ``` Note `ticket_emails` is passed through as-is (`ticket_emails or None` would be wrong here — an empty list `[]` must reach `save_tool_settings` to clear the stored list, and `[] or None` evaluates to `None`, which means "don't touch it." Pass `ticket_emails` directly; it is already `None` when the key was absent from the payload. Also update the module docstring's example just above `_parse_settings_form` (around line 512-521) to mention the new field, and update `get_tools_settings_endpoint`'s docstring (around line 469-477) is unaffected since it already forwards whatever `get_tool_settings` returns. - [ ] **Step 4: Run tests to verify they pass** Run: `.venv/bin/python -m pytest tests/integration/test_ticket_emails_endpoint.py -v` Expected: PASS, all 4 tests. - [ ] **Step 5: Run the existing provider-settings integration suite to confirm no regression** Run: `.venv/bin/python -m pytest tests/integration/test_provider_settings_endpoint.py -v` Expected: PASS, unchanged. - [ ] **Step 6: Commit** ```bash git add app/api/endpoints.py tests/integration/test_ticket_emails_endpoint.py git commit -m "feat: POST /tools-settings accepts and round-trips ticket_emails" ``` --- ### Task 7: Full-suite verification and live smoke test **Files:** none modified — verification only. - [ ] **Step 1: Run the full unit suite** Run: `.venv/bin/python -m pytest tests/unit -q` Expected: all pass (baseline was 752 before this plan; expect that plus the new tests from Tasks 1, 3, 4, 5). - [ ] **Step 2: Run the full integration suite** Run: `.venv/bin/python -m pytest tests/integration -q` Expected: all pass. - [ ] **Step 3: Live smoke test against the real dev database** ```bash .venv/bin/python -c " from app.services.chat.tools_settings import save_tool_settings, get_tool_settings tenant = 'org_00000000-0000-0000-0000-000000000097' saved = save_tool_settings(tenant, ticket_emails=[ {'email': 'sales-smoke-test@example.com', 'category': 'Sales'}]) print('saved:', saved['ticket_emails']) print('read-back:', get_tool_settings(tenant)['ticket_emails']) " ``` Expected: both prints show the one entry. Then clean up: ```bash .venv/bin/python -c " from app.services.infra.database import get_db_connection conn = get_db_connection() with conn.cursor() as cur: cur.execute('DROP SCHEMA IF EXISTS \"org_00000000-0000-0000-0000-000000000097\" CASCADE') conn.commit() conn.close() " ``` - [ ] **Step 4: Push** ```bash git push origin feature/product-catalog-recommendations ``` Verify the pushed commits' author is `krishbhanderi ` before pushing (`git log --format='%an <%ae>' -1`), per this repo's standing convention.