Repository navigation
Add a support intake bot that files Zendesk tickets - #98
Merged
Merged
Conversation
This was referenced Oct 7, 2026
zamanafzal
force-pushed
the
zafzal/13712-support-intake
branch
2 times, most recently
from
October 14, 2026 11:32
820874a to
dd9e06b
Compare
zamanafzal
marked this pull request as ready for review
October 14, 2026 11:33
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cross-site write protection and support-flow correctness issues remain unresolved.
4 open findings
What changed in this PR
Adds the backend for conversational support intake, connecting learner chat threads to Zendesk tickets.
Changes:
- Adds a support bot, SSE endpoint, request validation, and persistent ticket references.
- Implements Zendesk OAuth authentication, ticket creation, and local stub mode.
- Adds throttling, cross-site cookie settings, origin checks, and regression tests.
| File | Description |
|---|---|
| pytest.ini | Sets Zendesk test defaults. |
| main/settings.py | Adds support, Zendesk, and cookie settings. |
| main/migrations/0004_support_rate_limit.py | Seeds support throttling limits. |
| main/consumer_throttles_test.py | Checks throttle migration coverage. |
| env/backend.local.example.env | Documents local support configuration. |
| ai_chatbots/zendesk.py | Implements OAuth and ticket creation. |
| ai_chatbots/zendesk_test.py | Tests Zendesk integration behavior. |
| ai_chatbots/serializers.py | Validates support requests. |
| ai_chatbots/serializers_test.py | Tests support request fields. |
| ai_chatbots/routing.py | Registers the support SSE endpoint. |
| ai_chatbots/prompts.py | Defines the intake-only prompt. |
| ai_chatbots/models.py | Stores ticket reference and email. |
| ai_chatbots/migrations/0015_support_ticket_email.py | Adds the ticket email column. |
| ai_chatbots/migrations/0014_support_ticket_reference.py | Adds the ticket reference column. |
| ai_chatbots/consumers.py | Adds support handling and origin validation. |
| ai_chatbots/consumers_test.py | Tests identity handling and origin checks. |
| ai_chatbots/constants.py | Adds Zendesk constants and cookie attributes. |
| ai_chatbots/constants_test.py | Tests cookie configuration. |
| ai_chatbots/checkpointers.py | Handles null updated channels. |
| ai_chatbots/checkpointers_test.py | Covers null-channel checkpoints. |
| ai_chatbots/chatbots.py | Implements intake, review, and filing flow. |
| ai_chatbots/chatbots_test.py | Tests support flow and persistence. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mbertrand
requested changes
Oct 15, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Intake parsing, filing retry safety, and sensitive-data redaction still need correction.
6 open issues
Address extraction corrupts requester and edit details · New Unicode normalization triggers unintended confirmation · New Uncertain filings can be retried concurrently · New Filing lock expires during long-running requests · New Post-submit transport errors are misreported as definite failures · New Dependency locals leak learner input to Sentry · New
4 resolved since last review
Review effort: Balanced
mbertrand
requested changes
Oct 20, 2026
mbertrand
left a comment
Member
There was a problem hiding this comment.
Looking great, just found a few more things.
Adds a SupportBot that asks a learner what went wrong, collects a reply-to address, and files one Zendesk ticket per chat thread, so a problem can be reported from the page it happened on. The bot deliberately answers nothing: it is an intake form with a conversational front end, not a tutor. A new http/support_agent/ route and SupportChatRequestSerializer take the single-message request shape the widget sends, and the ticket reference and reply-to address are stored on UserChatSession so a reconnecting learner is not asked twice or filed twice. Ticket creation is stubbed by default. AI_ZENDESK_STUB_MODE defaults to True independently of whether credentials are set, so local dev and CI never need a Zendesk account, and a deployed environment has to turn it off explicitly.
UserChatSession.support_ticket_email is varchar(254). A longer address passed validation, so the bot would file the Zendesk ticket and then fail on save, recording no reference and filing again on every retry.
The bot filed silently on the turn the email address arrived, so the first the learner heard of a ticket was its reference number. Now that turn replies with what will be sent and asks if anything is missing, and the turn after files whatever they add. The gate is a state channel, not the model deciding it has heard enough: the turn after the review always files, so there is no way to get stuck waiting for a confirmation this version has no mechanism to collect.
Accept url and name parameters in file_support_ticket() and include them in the ticket payload when provided. Verified both fields are sent to Zendesk.
Zendesk silently drops ticket keys it doesn't recognize, so the previous top-level url/name keys never reached a ticket. Route page_url, name, and a new username through the custom_fields array with their field ids instead, and stop the requester name falling back to the raw email when a name is known. The username rides separately from the display name so Zendesk's username field always holds the literal account handle.
AI_ZENDESK_STUB_MODE defaulted to True, so an environment that forgot to set it would tell every learner their ticket was filed while quietly never calling Zendesk. Default it off instead; local dev and CI now set it explicitly.
…eld ids Nothing in file_support_ticket is a LangGraph tool: SupportBot files deterministically from get_completion rather than waiting for the model to emit a tool call, and it returns a dict where every tool in tools.py returns a JSON string. Give it its own module and its own test module. The custom ticket field ids were hardcoded from the test Zendesk account. They are minted per instance, so they resolve to nothing on MIT's production Zendesk, and Zendesk rejects an entire ticket for a field id it does not recognise. Read them from settings instead and leave a value off the ticket when its id is unset.
Added to exercise the support intake flow without provider credentials, and always meant to come out before merge. It was injected in BaseChatbot.get_llm, so a value set in a deployed environment would silence every bot, not just the support one. The filing path no longer depends on it: SupportBot builds the review and files the ticket in code rather than through a tool call.
check_origin() sits on BaseBotHttpConsumer, so it gates every bot, not just support intake. CORS_ALLOW_ORIGINS is not set in every deployed environment - an environment that never needed it would have started returning 403 for the chatbots that work there today. An empty allow-list now skips the check, which leaves those environments exactly where they are on main.
…ling Replace the static AI_ZENDESK_OAUTH_TOKEN with an OAuth client id/secret and mint a short-lived access token on demand, caching it until just before expiry. Re-mint and retry once on a 401 so a token revoked before its TTL does not drop a ticket. API tokens stop working 2027-04-30, so client_credentials is the supported path for a server-to-server client.
Every failure that was not a lost reply got the same answer: try "send" again. A ticket Zendesk rejects on its own merits is rejected the same way every time, so the learner was walked around a loop with no exit, and a bad address said nothing about the address. file_support_ticket now reports why it failed. A 400, 413 or 422 means the ticket itself is the problem, so the thread stops and points the learner at email. An address Zendesk will not take drops back into collection to ask for another one. Everything else stays retryable - a 401, 403 or 404 is our credentials or our API URL, and a thread blocked on one would still refuse to file after someone had fixed it.
_normalised stripped every digit and non-ASCII character, so "yes, 18.01" reduced to a bare "yes". Every predicate built on it then read that as permission to send: the draft was filed and the course number went nowhere. _is_filler did the same at collection time, deleting the whole message from the ticket body.
_reply_to_email accepts any single address in a short message, so "the page says contact admin@example.org" replaced an address the learner had already given - and _collect_the_request wrote it back to user_email, so it survived the following turns and Zendesk raised the ticket in the stranger's name. Replacing a known address now takes a message that is nothing but the new one; retyping it still works.
_reply_without_the_model yielded first and wrote the thread state after, so a learner whose connection dropped on that yield lost the turn entirely. Nothing recorded that Zendesk may already hold their ticket, and the retry after it filed a second one - or, inside the two hours a Zendesk idempotency key lives, got the 400 that key returns for an amended body and told them their request had been refused.
The TTL assumed REQUESTS_TIMEOUT bounds a filing call, but httpx bounds each connect, write and read separately - four requests of four phases each can outrun it. The lock then lapsed mid-call, the next turn took it legitimately, and the first call's finally deleted that one on its way out. It now holds a token and deletes only its own.
Two ways a filed ticket was reported as not filed. A dropped connection raises ReadError or RemoteProtocolError rather than a timeout, and the old test only recognised timeouts. And the ticket id was parsed inside the same try as the request, so a 2xx whose body we could not read - a proxy answering with its own HTML, say - came back as a plain failure. Both now report uncertain, because the retry they used to invite is what files the duplicate.
A DRF ValidationError is raised from rest_framework's own frame, and the serializer in that frame's self renders the entire request body through BaseSerializer.__repr__ - the learner's message, their address, the page they were on. Matching on the module name missed it, and there is no list of library frames that could hold a reference. An event with a chatbot frame anywhere in it now loses locals everywhere; events that never touch the package still keep theirs.
The lock TTL is built from max_filing_seconds, which multiplied REQUESTS_TIMEOUT by the four requests a filing can make. But the call passes a scalar timeout, and httpx hands that to connect, write, read and pool separately, so one request can spend four of them. The budget was 120 seconds against a worst case of 480, and the lock could lapse while the call it guards was still in flight - the one moment a second turn must not get through. The token added alongside it kept that turn from deleting a lock it no longer owned, but nothing stopped it filing.
Writing the reference onto the session could raise - a dropped database connection is enough - and that came back up through get_completion as a plain error. The learner was told nothing had been filed, awaiting_send was still set, and their next message sent the same request again. The reply now reports the reference it got, and the failure to record it sets filing_uncertain so no later turn files against this thread. That downgrades every reply after it to "may have been filed", which is the safe direction: the row that normally answers this never got written. The success path also clears awaiting_send, as every other branch here already does. Nothing reads it today, because the session row shadows it, but that is exactly the row this path can no longer rely on.
The first message is kept whatever it looks like, because nothing has been asked at that point and dropping it once filed an empty ticket. That reasoning is about the thread's opening message, but the check was on the position within the current request, so it fired again on the first message after a cancellation - where the learner has already been asked for an address and answered. A bare "ok" there led the ticket. Also corrects the comment on cancelled_after, which still described the exchange as written after the reply. It has been written ahead of the yield since the reply had to stop being the thing that got checkpointed first; the +2 is right either way, the reason given for it was not.
"wait" was a cancellation trigger, so a learner typing it on its own had every word of their request discarded and was told so. It far more often means give me a moment, which is the one reading that cannot be undone if we get it wrong - cancelling moves cancelled_after past the text. It now reaches _is_unclear_go_ahead, which asks. "wait, don't send it" still cancels, on the "send" and the negation rather than the "wait", and "stop" stays a trigger: under a question about whether to send, it has no second reading. PAUSE_PHRASES is filler too, so a pause typed during intake - where no review has been shown and nothing asks about it - does not end up in what support reads.
Zendesk documents expires_in as not mandatory in the token response, so reading it as a required key raised KeyError on a mint that had actually succeeded, and null raised TypeError. The token was never cached and the learner was told their request could not be filed. No fixture omitted it, so nothing caught it. The mint request now asks for a lifetime, which Zendesk accepts between 300 and 172800 seconds, and the response falls back to it when the field is absent or null. The TTL and token reads moved inside the try, so an unexpected shape there becomes a ZendeskTokenError on purpose rather than by accident - nothing was posted, so it is a definite failure.
CORS_ALLOW_ORIGINS lists the other sites allowed to embed a bot, so nobody adding one would think to list learn-ai's own origin, and a browser sends Origin on a same-origin POST too. frontend-demo fetches relative URLs, so its requests were arriving with one and being refused. check_origin now accepts a same-origin request before it consults the allow-list, the way CsrfViewMiddleware does. The scheme comes from SECURE_PROXY_SSL_HEADER rather than the ASGI scope, since APISIX terminates TLS and the raw scheme would refuse every deployed environment while passing locally, and the Host is validated against ALLOWED_HOSTS before it counts as ours.
The comment said the token stops a worker that wakes up late deleting the lock its successor holds. Read-then-delete is not atomic, so it narrows that window without closing it. Says so now, and names what actually enforces one ticket per thread: the already-filed check inside the lock, and Zendesk's Idempotency-Key.
A host that puts user_email in every request body accumulated it after the address the learner typed to correct it, so _latest picked the stale one and the bare "send" turn had no typed address to prefer instead. The body value is now only a fallback for when the thread has none saved.
Every branch of its get_completion answers without the model, so loading PROMPT_SUPPORT only bought a LangSmith lookup on a cache miss. PROMPT_TEMPLATE is None, which the base class already handles.
zamanafzal
force-pushed
the
zafzal/13712-support-intake
branch
from
October 21, 2026 10:45
8b4a0c7 to
5c0070a
Compare
The ownership check this PR adds makes an anonymous thread resume only while its session key still matches, but the cookie carrying that key expired with the browser while the thread cookie it is checked against lasts a week. The learner came back to a new thread, and on the support bot a repeat of the same problem would have filed a second ticket.
5 of 6 tasks
This was referenced Oct 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


What are the relevant tickets?
Part of https://github.com/mitodl/hq/issues/13712. Frontend half is mitodl/smoot-design#274.
Description (What does it do?)
A SupportBot that asks a learner what went wrong, collects a reply-to address, shows the request back for review, and files one Zendesk ticket per chat thread. It answers nothing on purpose — it's an intake form with a conversational front end, not a tutor, and both the conversation and the filing run in code rather than through the model.
http/support_agent/SSE route,SupportBotHttpConsumerandSupportChatRequestSerializer, taking the single-message request shape the widget posts.ai_chatbots/zendesk.py, called fromget_completion.UserChatSession.support_ticket_referenceandsupport_ticket_email, so a reconnecting learner isn't asked twice or filed twice.main/migrations/0004_support_rate_limit.py).AI_CHATBOTS_COOKIE_CROSS_SITE, so the thread cookie carriesSameSite=None; Securewhen the widget is embedded on another origin, plus an origin check onBaseBotHttpConsumer— these routes sit ahead ofdjango_asgi_appinmain/asgi.py, soCsrfViewMiddlewarenever runs for them.may_continue_thread()applies one ownership rule however the thread id arrived, body or cookie. That bug predates this PR, but the cross-site cookie above is what makes it reachable from another origin, so it's fixed here rather than left as a merge order someone has to remember.ai_chatbots(main/sentry.py). The reply-to address and the learner's own words travel as plain arguments right through that package and the SDK ships locals by default, so a traceback anywhere in the intake would have carried them. It is the whole event rather than the chatbot frames alone because the data does not stay inside the package - a DRFValidationErroris raised from rest_framework's own frame, whoseselfrenders the entire request body. Scoped to the event rather than disabling locals globally, so every other app keeps the locals that make a traceback readable.The three custom ticket field ids have to be set in each deployed environment (Admin Center > Objects and rules > Tickets > Fields), or tickets arrive without the name and username, and with the page URL only in the comment body rather than in its own field. Full config checklist is under
Additional Context.
Implementation details
Request shape.
message, plus optionalpage_url,user_email,user_name,user_username,confirm, and the usualthread_id/clear_history. An authenticated learn-ai session always beats the body, so arequest can't name somebody else as the requester for a signed-in learner;
hosts learn-ai holds no session for (the edX MFE) fall back to the body and get
the
tim_unverified_requestertag. Whether the address is verified is decidedfrom the request doing the filing, never inherited from an earlier turn of the
thread.
confirmis the widget's Send button, so a learner's approval doesn'tdepend on the bot recognising the word they typed.
user_usernameis keptdistinct from
user_namebecause Zendesk's username field should only evercarry the literal handle.
Why nothing here is a LangGraph tool.
file_support_ticketisn't intools.pyand isn't registered as a tool. A model that declines to emit the call leaves a learner who was told their request was filed with no way to findout it wasn't. Same reasoning for the conversation: it's a fixed form (collect
the problem and a reply-to, show it back, file), so a model only adds a
dependency and a way for every intake to fail.
SupportBot.CALLS_THE_MODEL = Falseskipscreate_proxy_user, a blocking HTTP call that would otherwise putthe intake form — the thing a learner reaches when something else is already
broken — behind the LiteLLM proxy being up.
get_llmstill runs;create_react_agentneeds it to build the graph the bot reads and writes threadstate through.
Send, edit or cancel. A go-ahead files, a plain no cancels, a sentence of
detail is folded in and re-reviewed, and a short reply that opens with "yes" but
carries other words gets asked about — "yes please send it to support asap" adds
nothing to the ticket while "yes, and the course is 18.01" does, and nothing
here can tell them apart. A wrong cancel costs a turn to undo; a wrong send
cannot be undone. Cancelling withdraws the request rather than just the send:
the next thing the learner types starts a fresh one, so withdrawn words can't
reappear in a later ticket — which on a shared device would be someone else's.
What reaches support. Every message the learner typed about the problem
since the last cancellation, with the bare reply-to address and bare
confirmations dropped. The opening
message is kept whatever it looks like: nobody has been asked for an address at
that point, so "I can't log in as x@y.com" is the problem rather than an answer,
and dropping it filed an empty ticket.
One ticket per thread. A
cache.aaddlock keyed on the thread, anarefresh_from_dbinside the lock (the lock alone leaves the read-then-writewindow open), and an
Idempotency-Keyheader plusexternal_idboth keyed onthe thread id. The lock's TTL is sized off the worst-case filing path — mint a
token, POST, re-mint on a 401, POST again, and a scalar httpx timeout is a budget
per phase rather than per request, so each of those four can spend four
REQUESTS_TIMEOUTs — rather than a round number, since a lock that lapses whilethe first turn is still in flight lets a second one through at the one moment
it must not get through.
A lost reply isn't a failure. A 5xx or a read timeout after the request went
out may still have created the ticket, so the bot says it may have gone through
and stops rather than inviting the retry that duplicates it.
ConnectTimeout/PoolTimeoutand any failure minting the OAuth token aredefinite failures, since nothing was posted.
Zendesk configuration.
AI_ZENDESK_API_URLis deliberately separate fromthe existing
AI_ZENDESK_URL: that one is the branded help center used forarticle search, while ticket API calls have to go to the canonical
{subdomain}.zendesk.comhost. Auth is OAuthclient_credentials(
AI_ZENDESK_OAUTH_CLIENT_ID/AI_ZENDESK_OAUTH_CLIENT_SECRET), minting ashort-lived access token, caching it until just before expiry and re-minting
once on a 401 — API tokens stop working 2027-04-30, so this is the supported
server-to-server path.
AI_ZENDESK_STUB_MODEdefaults toFalse: anenvironment that forgets to configure Zendesk should fail loudly rather than
tell every learner their request was filed and drop it;
env/backend.local.example.envturns it on for local work. The stub stillvalidates the address first, so a malformed one can't pass locally and fail only
where Zendesk is configured. The custom field ids (
AI_ZENDESK_URL_FIELD_ID,AI_ZENDESK_NAME_FIELD_ID,AI_ZENDESK_USERNAME_FIELD_ID) are configuredrather than hardcoded because they're minted per Zendesk instance and Zendesk
rejects a whole ticket for an id it doesn't recognise; unset means the value is
left off. Every ticket carries
tim_support_intake, and one the caller assertedthe address for also carries
tim_unverified_requester. On those, the firstcomment is filed private and the subject is replaced with a fixed one: this call
authenticates as an agent, so nothing has proved the requester owns the address
the caller named, and a public first comment plus a caller-chosen subject is a
way to send chosen text to a chosen address over MIT's domain through whichever
received-request trigger the instance has turned on. Agents still read the
learner's own subject and words on the ticket itself.
Cross-site cookies. Carry
SameSite=None;SecurewheneverAI_CHATBOTS_COOKIE_CROSS_SITEis on; what the allow-list gates is whichcross-origin callers get a response at all. The
"null"origin is never accepted. The origin checkallows a same-origin request before it consults the allow-list at all, the
way
CsrfViewMiddlewaredoes, so no environment has to list its own origin.It also allows an absent origin (server-to-server callers send none, and a
Referrer-Policy can strip both headers) and an empty allow-list, the latter
only while cookies stay same-site — the pair of cross-site cookies and no allow-list
fails closed.
How can this be tested?
docker compose run --rm web pytest: 860 passed, 1 skipped, 5 failed. The 5 arepre-existing — I re-ran the same tests on
origin/mainin a separate worktreeand got the identical 5. Four are tutor tests that need network for LiteLLM's
model-cost map (the run logs a
ConnectTimeoutfetching it); the fifth ismain/opik_keycloak_auth_test.py.To walk the flow, with
AI_ZENDESK_STUB_MODE=Trueinenv/backend.local.envsono real ticket is filed:
docker compose upbefore filing.
and confirm the review shows only the new one. Reply "yes please send it
over" and confirm it asks rather than sending. Reply with extra detail and
confirm the review comes back with it included.
STUB-A1B2C3D4, so it can't be mistaken for areal ticket number.
Additional Context
Before this does anything in a deployed environment:
AI_ZENDESK_API_URL, theOAuth client id and secret, the three custom field ids,
AI_ZENDESK_STUB_MODEoff, and
CORS_ALLOW_ORIGINSplusAI_CHATBOTS_COOKIE_CROSS_SITEfor theembedding origins.
CORS_ALLOW_ORIGINScomes from Vault and doesn't appear inol-infrastructure, so I couldn't confirm its current value anywhere, andcross-site cookies have to stay off for plain-HTTP local dev since browsers
reject
Securecookies over HTTP. Nothing mounts the widget yet either — thisand smoot-design#274 give the launcher and the backend; a host still has to
render it.
Known gaps, each tracked separately:
learners can share a thread identity (mitodl/hq#13990). Both come from
main/consumers.pykeying anonymous callers on a Django session key that,under
signed_cookieswith an always-empty session, is a wall-clock-secondtimestamp. Every anonymous chat route, not just this one — so the throttle
this PR seeds does not currently limit anonymous callers.
aupdate_state(..., as_node="agent")yieldsupdated_channels=NoneandChatMessageViewSetselects onmetadata.writes. The review, the filingresult and the already-filed reply are invisible to it, so a learner who
reconnects sees a thread with holes. The fix is in the checkpointer every bot
shares.
(mitodl/hq#13985) — only the tag says otherwise.
(mitodl/hq#13987), on the login path every ASGI request takes.
chat naming a stranger as the requester. A learner signed into the wrong
account can sign out and use the form anonymously, which gets them the
unverified-requester path and the tag.
route accepts an unverified address from an anonymous submitter exactly as the
help center form there does; the production account still has to be checked
before launch. The private comment and fixed subject above mean a
received-request trigger has nothing of the caller's to put in that email
through the usual comment placeholders, but which placeholders the triggers on
each account actually use hasn't been checked, and
{{ticket.description}}isthe one to look at.
Idempotency-Keytests only assert that the same key andexternal_idgoout, including on the re-POST after a
401- against a mocked httpx client, notthe stub, which returns before any request is sent. Zendesk documents a 2-hour
replay window; that hasn't been confirmed against a live instance.