fix(posthog-ai): stop /ticket creating duplicate and blank tickets - #72932
Conversation
The /ticket flow could file blank tickets whose body was only the conversation and trace IDs, and sometimes a duplicate alongside the real one. Two entry points hit the same defects: sending /ticket as the first message then pressing enter on an empty field, and a double submit in the summary flow after the first success cleared the form. - appendTicketMetadata returns an empty string for an empty body, so the ID footer can no longer stand in as a fake message that slips past the "please enter a message" validation - handleSupportFormSubmit guards the composed body and blocks re-entry with a synchronous ref, and the create and submit controls lock once a ticket exists for the conversation - pressing enter on an empty field no longer opens the ticket modal - customers get an optional "anything to add" note that leads the ticket with the AI summary attached as context Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.24 MiB · 22 files | no change | ███░░░░░░░ 27.6% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.07 MiB · 3,011 files | 🔺 +78 B (+0.0%) | ████████░░ 83.1% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 236.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.1 KiB | ../node_modules/.pnpm/posthog-js@1.407.2/node_modules/posthog-js/dist/module.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 106.2 KiB | src/lib/api.ts |
| 94.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.18 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.18 MiB · 17 files | no change | ████░░░░░░ 38.1% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 716.0 KiB | dist/toolbar/toolbar-app-UT2TAV4U.css |
| 545.1 KiB | dist/toolbar/chunk-chunk-R67YSTOI.js |
| 484.3 KiB | dist/toolbar/chunk-chunk-PY4X2WR2.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-P36RTWFE.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-KIASA47E.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-NZ4IQ3HA.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-DKDYNNXI.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +8.6 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1353.98 MiB · 🔺 +8.6 KiB (+0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
Writing the composed message (summary + ID footer) back into the form before submitting dumped the whole ticket body into the visible "anything to add" textarea, left it polluted on failure, and a retry would have composed the summary and footer in twice. Success was also detected via the kea-forms submit promise plus an effect watching lastSubmittedTicketId, which resolves before the ticket request finishes, so the modal never closed. Compose the final message locally and pass it straight to submitSupportTicket (replicating the form wrapper's name/email defaulting), then close the modal synchronously once the awaited listener has set the new ticket id. The pending-submission state and effect are no longer needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
frontend/src/scenes/max/TicketPrompt.tsx:5
**Toast Import Breaks Compilation**
`lemonToast` is provided by the frontend LemonToast module, not the `@posthog/lemon-ui` package barrel. This named import makes the changed module fail TypeScript compilation with a missing-export error.
```suggestion
import { LemonButton, LemonInput, LemonModal } from '@posthog/lemon-ui'
import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast'
```
### Issue 2 of 2
frontend/src/scenes/max/TicketPrompt.tsx:110
**Retry Reprocesses Final Payload**
When submission fails, this stores the composed body and metadata in the shared message field before the retry lock is released. The next attempt treats that stored payload as the user's note and composes it again, producing duplicate metadata without a summary or duplicate summary and metadata with one.
Reviews (1): Last reviewed commit: "fix(posthog-ai): submit ticket directly ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }) | ||
| } catch { | ||
| // Failure is detected below via the unchanged ticket id | ||
| } |
There was a problem hiding this comment.
When submission fails, this stores the composed body and metadata in the shared message field before the retry lock is released. The next attempt treats that stored payload as the user's note and composes it again, producing duplicate metadata without a summary or duplicate summary and metadata with one.
Rule Used: If data has already been processed, avoid re-proce... (source)
Learned From
PostHog/posthog#32520
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/scenes/max/TicketPrompt.tsx
Line: 110
Comment:
**Retry Reprocesses Final Payload**
When submission fails, this stores the composed body and metadata in the shared message field before the retry lock is released. The next attempt treats that stored payload as the user's note and composes it again, producing duplicate metadata without a summary or duplicate summary and metadata with one.
**Rule Used:** If data has already been processed, avoid re-proce... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=951766f2-db79-4e5b-b8ee-ae0fa0c13119))
**Learned From**
[PostHog/posthog#32520](https://github.com/PostHog/posthog/pull/32520)
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Checked both findings against the PR head (e1f31a5):
- Retry reprocessing: this described the previous revision, which wrote the composed message back into the form via setSendSupportRequestValue before releasing the retry lock. The current code composes the final message locally and passes it straight to submitSupportTicket, so the shared message field only ever holds the user's note. Nothing is stored on failure, and a retry recomposes from the untouched note.
- lemonToast import (from the summary comment): the @posthog/lemon-ui barrel does export lemonToast. There are 146 existing barrel imports of it on master, including SupportForm.tsx in this same diff, and the Frontend typechecking CI job passes on this commit.
slshults
added
team/conversations
🦔 ReviewHog reviewed this pull requestFound 1 must fix, 0 should fix, 3 consider. Published 4 findings (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
| onPressEnter={() => { | ||
| if (issueText.trim()) { | ||
| openSupportModal() | ||
| } | ||
| }} |
There was a problem hiding this comment.
Enter key on the issue-text input doesn't check hasSubmitted, unlike the adjacent button
Why we think it's a valid issue
- Checked: Working tree is at base commit
527d71b6, not the PR head, so I pulled the real PR-head content viagh pr diff 72932. Confirmed the final render block (PATCH 2/2 stops before it, so PATCH 1/2's version stands) and traced the downstream submit guards. - Found: At the flagged lines the Enter handler is
onPressEnter={() => { if (issueText.trim()) { openSupportModal() } }}— onlyissueText.trim(). The sibling button in the same block usesdisabledReason={hasSubmitted ? 'Ticket already created' : !issueText.trim() ? 'Describe your issue first' : undefined}.hasSubmittedis set true inhandleTicketCreatedafter a successful ticket, andissueTextis never cleared — so once a ticket exists the button is disabled but pressing Enter in the still-populated input still callsopenSupportModal()and reopens the modal. The guard mismatch is genuine and sits on lines this PR authored. - Impact: Real but bounded. Resubmission is fully blocked —
handleSupportFormSubmitearly-returns onif (submitInFlightRef.current || hasSubmitted)and the modal's Submit button is disabled viadisabledReason={hasSubmitted ? ...}— so no duplicate or blank ticket can result. The only observable effect is a confusing dead-end modal reopening after a ticket already exists, which contradicts the PR's own stated goal of locking the controls once a ticket exists. Minor cosmetic UX inconsistency, not a data/correctness defect, fixed by adding!hasSubmitted &&to the existing condition. The reviewer'sconsider(lowest) priority is the right bucket, so no adjustment.
Issue description
The 'Create support ticket' button in the no-summary flow disables via disabledReason={hasSubmitted ? 'Ticket already created' : !issueText.trim() ? 'Describe your issue first' : undefined}, but the input's onPressEnter handler only checks issueText.trim() and omits the hasSubmitted check. issueText is never cleared after a successful submission, so once a ticket has been created for this TicketPrompt instance, pressing Enter in the still-populated input reopens the create-ticket modal even though the button right below it is disabled — an inconsistent guard between the two entry points for the same action (actual resubmission is still blocked by handleSupportFormSubmit's own hasSubmitted check and the Submit button's disabledReason, but the modal can confusingly reopen after a ticket already exists).
Suggested fix
Mirror the button's guard in the Enter handler: onPressEnter={() => { if (!hasSubmitted && issueText.trim()) { openSupportModal() } }}.
Prompt to fix with AI (copy-paste)
## Context
@frontend/src/scenes/max/TicketPrompt.tsx#L196-200
<issue_description>
The 'Create support ticket' button in the no-summary flow disables via `disabledReason={hasSubmitted ? 'Ticket already created' : !issueText.trim() ? 'Describe your issue first' : undefined}`, but the input's `onPressEnter` handler only checks `issueText.trim()` and omits the `hasSubmitted` check. `issueText` is never cleared after a successful submission, so once a ticket has been created for this `TicketPrompt` instance, pressing Enter in the still-populated input reopens the create-ticket modal even though the button right below it is disabled — an inconsistent guard between the two entry points for the same action (actual resubmission is still blocked by `handleSupportFormSubmit`'s own `hasSubmitted` check and the Submit button's `disabledReason`, but the modal can confusingly reopen after a ticket already exists).
</issue_description>
<issue_validation>
- **Checked:** Working tree is at base commit `527d71b6`, not the PR head, so I pulled the real PR-head content via `gh pr diff 72932`. Confirmed the final render block (PATCH 2/2 stops before it, so PATCH 1/2's version stands) and traced the downstream submit guards.
- **Found:** At the flagged lines the Enter handler is `onPressEnter={() => { if (issueText.trim()) { openSupportModal() } }}` — only `issueText.trim()`. The sibling button in the same block uses `disabledReason={hasSubmitted ? 'Ticket already created' : !issueText.trim() ? 'Describe your issue first' : undefined}`. `hasSubmitted` is set true in `handleTicketCreated` after a successful ticket, and `issueText` is never cleared — so once a ticket exists the button is disabled but pressing Enter in the still-populated input still calls `openSupportModal()` and reopens the modal. The guard mismatch is genuine and sits on lines this PR authored.
- **Impact:** Real but bounded. Resubmission is fully blocked — `handleSupportFormSubmit` early-returns on `if (submitInFlightRef.current || hasSubmitted)` and the modal's Submit button is disabled via `disabledReason={hasSubmitted ? ...}` — so no duplicate or blank ticket can result. The only observable effect is a confusing dead-end modal reopening after a ticket already exists, which contradicts the PR's own stated goal of locking the controls once a ticket exists. Minor cosmetic UX inconsistency, not a data/correctness defect, fixed by adding `!hasSubmitted &&` to the existing condition. The reviewer's `consider` (lowest) priority is the right bucket, so no adjustment.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Mirror the button's guard in the Enter handler: `onPressEnter={() => { if (!hasSubmitted && issueText.trim()) { openSupportModal() } }}`.
</potential_solution>
There was a problem hiding this comment.
Fixed in 04fa82e. openSupportModal now early-returns when hasSubmitted, which covers the Enter handler and any other caller, so the dead modal cannot be reopened after a ticket is created.
| } | ||
|
|
||
| const finalMessage = appendMetadataToMessage(sendSupportRequest.message) | ||
| setSendSupportRequestValue('message', finalMessage) | ||
| submitInFlightRef.current = true | ||
| setIsSubmitting(true) | ||
| const ticketIdBefore = supportLogic.values.lastSubmittedTicketId | ||
| setTicketIdBeforeSubmission(ticketIdBefore) | ||
| setPendingTicketSubmission(true) | ||
| try { | ||
| await supportLogic.asyncActions.submitSendSupportRequest() | ||
| await supportLogic.asyncActions.submitSupportTicket({ | ||
| ...sendSupportRequest, | ||
| name: user?.first_name ?? sendSupportRequest.name ?? 'name not set', | ||
| email: user?.email ?? sendSupportRequest.email ?? '', | ||
| message: finalMessage, | ||
| }) |
There was a problem hiding this comment.
Direct call to submitSupportTicket bypasses the form's required-field validation for the legacy (non-conversations) ticket path
Why we think it's a valid issue
- Checked: PR-head
handleSupportFormSubmit(viagh pr diff), the kea-formsforms()block andsubmitSupportTicketlistener insupportLogic.ts, thetarget_areafield inSupportForm.tsx,openSupportModal's pre-fill inTicketPrompt.tsx, and thefeaturePreviewsLogic.tsxprecedent. - Found: PATCH 2/2 replaced
submitSendSupportRequest()with a directsubmitSupportTicket({...sendSupportRequest, ...})call. The validator lives on the kea-forms wrapper (supportLogic.ts:684-696, requiringkind/severity_level/target_areawhen!conversationsFlagEnabled);submitSupportTicket(supportLogic.ts:780) is thesubmittarget and runs no validation.SupportForm.tsx:145-168renders a clearabletarget_areaselect (onChange(newValue ?? null)at :163) only on the!conversationsFlagEnabledbranch, and the Zendesk fallback coerces null viatarget_area ?? 'General'(supportLogic.ts:953). So clearing the topic on the legacy path yields a ticket routed to 'General' with no client-side block — a genuine regression, since the base code'ssubmitSendSupportRequest()ran the validator. Diagnosis and thefeaturePreviewsLogic.tsx:193-199hardcoded-fields precedent are accurate. - Impact: Real but narrow. Trigger requires the deprecated Zendesk path (
conversationsFlagEnabledfalse — code labels it the temporary fallback being retired) AND a user deliberately clearing the pre-filled Topic (openSupportModalseedstarget_area: targetArea ?? 'analytics', always non-null at open;kind/severity_levelare pre-set and not user-nullable). Consequence is a soft routing downgrade to 'General' — the ticket is still created and delivered; no data loss, no duplicate/blank ticket, no security dimension. - Priority: Lowering
should_fix→consider: the finding is accurate and a legitimate completeness gap (this PR guardedmessagebut not the other required fields), but the deprecated path plus deliberate-clear trigger and the benign 'General'-bucket outcome put it below the should-fix bar.
Issue description
handleSupportFormSubmit now calls supportLogic.asyncActions.submitSupportTicket({...sendSupportRequest, name, email, message: finalMessage}) directly instead of the kea-forms-generated submitSendSupportRequest(). That generated action runs the errors validator declared in supportLogic.ts (forms(...), requiring kind, severity_level, and target_area whenever !conversationsFlagEnabled) before calling submit; calling submitSupportTicket directly skips that validator entirely. SupportForm renders those triage fields (including a LemonInputSelect for target_area that can be cleared to null via its onChange={([newValue]) => onChange(newValue ?? null)}) whenever conversationsFlagEnabled is false — the exact 'legacy triage-field variant' the PR description says was manually tested. A user who clears the topic can now submit a ticket with no client-side error and no target_area, silently degrading to the 'General' bucket server-side, whereas the same action on the standard support modal (via submitSendSupportRequest) would have blocked the submission with a 'Please choose' validation error. The codebase's existing precedent for calling submitSupportTicket directly (featurePreviewsLogic.tsx) works around this by hardcoding non-null kind/severity_level/target_area values itself — this call site doesn't provide the same guarantee since it spreads live, user-editable sendSupportRequest state.
Suggested fix
Either route back through the validated submitSendSupportRequest() action (letting the form report the missing-field errors), or, if bypassing kea-forms validation is intentional for this flow, replicate the same required-field guarantees this PR already added for message — e.g. also guard on sendSupportRequest.target_area/kind/severity_level before calling submitSupportTicket, matching the pattern featurePreviewsLogic.tsx uses of always supplying non-null values for the legacy triage fields.
Prompt to fix with AI (copy-paste)
## Context
@frontend/src/scenes/max/TicketPrompt.tsx#L96-107
<issue_description>
`handleSupportFormSubmit` now calls `supportLogic.asyncActions.submitSupportTicket({...sendSupportRequest, name, email, message: finalMessage})` directly instead of the kea-forms-generated `submitSendSupportRequest()`. That generated action runs the `errors` validator declared in `supportLogic.ts` (`forms(...)`, requiring `kind`, `severity_level`, and `target_area` whenever `!conversationsFlagEnabled`) before calling `submit`; calling `submitSupportTicket` directly skips that validator entirely. `SupportForm` renders those triage fields (including a `LemonInputSelect` for `target_area` that can be cleared to `null` via its `onChange={([newValue]) => onChange(newValue ?? null)}`) whenever `conversationsFlagEnabled` is false — the exact 'legacy triage-field variant' the PR description says was manually tested. A user who clears the topic can now submit a ticket with no client-side error and no `target_area`, silently degrading to the 'General' bucket server-side, whereas the same action on the standard support modal (via `submitSendSupportRequest`) would have blocked the submission with a 'Please choose' validation error. The codebase's existing precedent for calling `submitSupportTicket` directly (`featurePreviewsLogic.tsx`) works around this by hardcoding non-null `kind`/`severity_level`/`target_area` values itself — this call site doesn't provide the same guarantee since it spreads live, user-editable `sendSupportRequest` state.
</issue_description>
<issue_validation>
- **Checked:** PR-head `handleSupportFormSubmit` (via `gh pr diff`), the kea-forms `forms()` block and `submitSupportTicket` listener in `supportLogic.ts`, the `target_area` field in `SupportForm.tsx`, `openSupportModal`'s pre-fill in `TicketPrompt.tsx`, and the `featurePreviewsLogic.tsx` precedent.
- **Found:** PATCH 2/2 replaced `submitSendSupportRequest()` with a direct `submitSupportTicket({...sendSupportRequest, ...})` call. The validator lives on the kea-forms wrapper (`supportLogic.ts:684-696`, requiring `kind`/`severity_level`/`target_area` when `!conversationsFlagEnabled`); `submitSupportTicket` (`supportLogic.ts:780`) is the `submit` target and runs no validation. `SupportForm.tsx:145-168` renders a clearable `target_area` select (`onChange(newValue ?? null)` at :163) only on the `!conversationsFlagEnabled` branch, and the Zendesk fallback coerces null via `target_area ?? 'General'` (`supportLogic.ts:953`). So clearing the topic on the legacy path yields a ticket routed to 'General' with no client-side block — a genuine regression, since the base code's `submitSendSupportRequest()` ran the validator. Diagnosis and the `featurePreviewsLogic.tsx:193-199` hardcoded-fields precedent are accurate.
- **Impact:** Real but narrow. Trigger requires the deprecated Zendesk path (`conversationsFlagEnabled` false — code labels it the temporary fallback being retired) AND a user deliberately clearing the pre-filled Topic (`openSupportModal` seeds `target_area: targetArea ?? 'analytics'`, always non-null at open; `kind`/`severity_level` are pre-set and not user-nullable). Consequence is a soft routing downgrade to 'General' — the ticket is still created and delivered; no data loss, no duplicate/blank ticket, no security dimension.
- **Priority:** Lowering `should_fix` → `consider`: the finding is accurate and a legitimate completeness gap (this PR guarded `message` but not the other required fields), but the deprecated path plus deliberate-clear trigger and the benign 'General'-bucket outcome put it below the should-fix bar.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Either route back through the validated `submitSendSupportRequest()` action (letting the form report the missing-field errors), or, if bypassing kea-forms validation is intentional for this flow, replicate the same required-field guarantees this PR already added for `message` — e.g. also guard on `sendSupportRequest.target_area`/`kind`/`severity_level` before calling `submitSupportTicket`, matching the pattern `featurePreviewsLogic.tsx` uses of always supplying non-null values for the legacy triage fields.
</potential_solution>
There was a problem hiding this comment.
Not addressing this one: the clearable triage fields (message type, topic, severity) only render on the legacy Zendesk form variant, which is being retired now that support runs on conversations. When the conversations extension falls back to Zendesk at submit time, the rendered form was the conversations variant, where these fields are prefilled by openSupportModal and not clearable, so the cleared-topic scenario cannot occur there. Accepting the target_area ?? 'General' coercion for the remaining lifetime of the fallback.
There was a problem hiding this comment.
Update: the Zendesk fallback is staying in service for now, so this is fixed in 9961928 after all. The triage fields (kind, topic, severity) are validated before the direct submit on the non-conversations variant, restoring the block the kea-forms validator used to provide.
|
|
||
| function handleSupportModalCancel(): void { | ||
| setIsSupportModalOpen(false) | ||
| setPendingTicketSubmission(false) | ||
| submitInFlightRef.current = false | ||
| setIsSubmitting(false) | ||
| closeSupportForm() | ||
| } |
There was a problem hiding this comment.
Cancelling the modal during an in-flight submission resets the duplicate-submit guard, enabling a real double ticket
Why we think it's a valid issue
- Checked: Final PR-head
handleSupportModalCancelandhandleSupportFormSubmit(viagh pr diff), thesubmitInFlightRef/hasSubmittedguard,LemonModal's close defaults,submitSupportTicket's listener insupportLogic.ts, and whatcloseSupportFormresets. - Found:
handleSupportModalCancelclearssubmitInFlightRef.current = falseunconditionally, with noisSubmittingguard.LemonModaldefaultsclosable = true(LemonModal.tsx:87) and routes the Cancel button, X (:136), Escape (shouldCloseOnEsc,:192) and overlay click (:186-188) toonClose→handleSupportModalCancel, so the modal can close whileawait submitSupportTicket(...)is still pending.submitSupportTicket(supportLogic.ts:780) has noAbortController; the conversations path awaitswaitForConversations()+sendMessage, a real multi-second window.closeSupportFormis only a reducer flag (supportLogic.ts:525) — it does not reset the localhasSubmitted, which stays false until the first request resolves. After cancel, the outer 'Create support ticket' button (gated only onhasSubmitted) re-enables, so reopening and re-submitting passes thesubmitInFlightRef.current || hasSubmittedguard and fires a secondsubmitSupportTicket. - Impact: Two tickets from one /ticket invocation if the first (cancelled) request still succeeds — precisely the duplicate-ticket defect this PR exists to eliminate, produced by clearing the PR's own synchronous lock from the cancel path. Trigger is a plausible real action: cancelling a submit that feels slow and retrying (single-click double-submit is already blocked by the
loadingSubmit button + ref, so this is the remaining hole). The suggested fix — don't clear the ref on cancel, and/orclosable={!isSubmitting}— closes it without side effects.
Issue description
submitInFlightRef is the PR's new synchronous lock against double submission, but handleSupportModalCancel unconditionally sets submitInFlightRef.current = false even while a submission is genuinely in flight. LemonModal defaults closable to true, and onClose is wired straight to handleSupportModalCancel with no isSubmitting guard, so the user can hit Cancel, click the X, click outside the modal, or press Escape at any point while await supportLogic.asyncActions.submitSupportTicket(...) is still pending. submitSupportTicket's listener is fire-and-forget (there is no AbortController), so the original request keeps running server-side. Once the ref is cleared, the outer 'Create support ticket' button (gated only on hasSubmitted, still false) is clickable again, and a fresh handleSupportFormSubmit() call passes the submitInFlightRef.current || hasSubmitted guard and fires a second submitSupportTicket. If the first ("cancelled") request still succeeds, two tickets get filed from one /ticket invocation — the exact defect this PR is meant to fix.
Suggested fix
Don't clear submitInFlightRef.current from the cancel path. Only reset isSupportModalOpen/isSubmitting/closeSupportForm() there, and let the ref get cleared solely by the in-flight handleSupportFormSubmit once it observes the real outcome (success or failure). That way a stale in-flight request still blocks a resubmission even if the user reopens the modal before it settles. Alternatively (or additionally), disable the modal's close affordances (closable={!isSubmitting} on LemonModal) while isSubmitting is true.
Prompt to fix with AI (copy-paste)
## Context
@frontend/src/scenes/max/TicketPrompt.tsx#L120-126
@frontend/src/scenes/max/TicketPrompt.tsx#L86-100
<issue_description>
`submitInFlightRef` is the PR's new synchronous lock against double submission, but `handleSupportModalCancel` unconditionally sets `submitInFlightRef.current = false` even while a submission is genuinely in flight. `LemonModal` defaults `closable` to `true`, and `onClose` is wired straight to `handleSupportModalCancel` with no `isSubmitting` guard, so the user can hit Cancel, click the X, click outside the modal, or press Escape at any point while `await supportLogic.asyncActions.submitSupportTicket(...)` is still pending. `submitSupportTicket`'s listener is fire-and-forget (there is no `AbortController`), so the original request keeps running server-side. Once the ref is cleared, the outer 'Create support ticket' button (gated only on `hasSubmitted`, still false) is clickable again, and a fresh `handleSupportFormSubmit()` call passes the `submitInFlightRef.current || hasSubmitted` guard and fires a second `submitSupportTicket`. If the first ("cancelled") request still succeeds, two tickets get filed from one /ticket invocation — the exact defect this PR is meant to fix.
</issue_description>
<issue_validation>
- **Checked:** Final PR-head `handleSupportModalCancel` and `handleSupportFormSubmit` (via `gh pr diff`), the `submitInFlightRef`/`hasSubmitted` guard, `LemonModal`'s close defaults, `submitSupportTicket`'s listener in `supportLogic.ts`, and what `closeSupportForm` resets.
- **Found:** `handleSupportModalCancel` clears `submitInFlightRef.current = false` unconditionally, with no `isSubmitting` guard. `LemonModal` defaults `closable = true` (`LemonModal.tsx:87`) and routes the Cancel button, X (`:136`), Escape (`shouldCloseOnEsc`, `:192`) and overlay click (`:186-188`) to `onClose` → `handleSupportModalCancel`, so the modal can close while `await submitSupportTicket(...)` is still pending. `submitSupportTicket` (`supportLogic.ts:780`) has no `AbortController`; the conversations path awaits `waitForConversations()` + `sendMessage`, a real multi-second window. `closeSupportForm` is only a reducer flag (`supportLogic.ts:525`) — it does not reset the local `hasSubmitted`, which stays false until the first request resolves. After cancel, the outer 'Create support ticket' button (gated only on `hasSubmitted`) re-enables, so reopening and re-submitting passes the `submitInFlightRef.current || hasSubmitted` guard and fires a second `submitSupportTicket`.
- **Impact:** Two tickets from one /ticket invocation if the first (cancelled) request still succeeds — precisely the duplicate-ticket defect this PR exists to eliminate, produced by clearing the PR's own synchronous lock from the cancel path. Trigger is a plausible real action: cancelling a submit that feels slow and retrying (single-click double-submit is already blocked by the `loading` Submit button + ref, so this is the remaining hole). The suggested fix — don't clear the ref on cancel, and/or `closable={!isSubmitting}` — closes it without side effects.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Don't clear `submitInFlightRef.current` from the cancel path. Only reset `isSupportModalOpen`/`isSubmitting`/`closeSupportForm()` there, and let the ref get cleared solely by the in-flight `handleSupportFormSubmit` once it observes the real outcome (success or failure). That way a stale in-flight request still blocks a resubmission even if the user reopens the modal before it settles. Alternatively (or additionally), disable the modal's close affordances (`closable={!isSubmitting}` on `LemonModal`) while `isSubmitting` is true.
</potential_solution>
There was a problem hiding this comment.
Fixed in 04fa82e. Cancelling (esc, X, overlay, or the button) is now a no-op while the submit is in flight, since the request cannot be aborted, and the Cancel button shows a disabled reason while submitting. If the submit fails, the guard clears and cancel works again, so the modal cannot get stuck.
| const ticketIdBefore = supportLogic.values.lastSubmittedTicketId | ||
| setTicketIdBeforeSubmission(ticketIdBefore) | ||
| setPendingTicketSubmission(true) | ||
| try { | ||
| await supportLogic.asyncActions.submitSendSupportRequest() | ||
| await supportLogic.asyncActions.submitSupportTicket({ | ||
| ...sendSupportRequest, | ||
| name: user?.first_name ?? sendSupportRequest.name ?? 'name not set', | ||
| email: user?.email ?? sendSupportRequest.email ?? '', | ||
| message: finalMessage, | ||
| }) | ||
| } catch { | ||
| // Failure is detected below via the unchanged ticket id | ||
| } | ||
| // Success is handled by the effect watching lastSubmittedTicketId. If no ticket was created, | ||
| // the submit failed — stop the spinner so the user can retry (the error toast already showed). | ||
| if (supportLogic.values.lastSubmittedTicketId === ticketIdBefore) { | ||
| const ticketIdAfter = supportLogic.values.lastSubmittedTicketId | ||
| if (ticketIdAfter && ticketIdAfter !== ticketIdBefore) { | ||
| handleTicketCreated(ticketIdAfter) | ||
| } else { | ||
| // Submit failed (the error toast already showed) — allow a retry with the note untouched | ||
| submitInFlightRef.current = false | ||
| setIsSubmitting(false) | ||
| setPendingTicketSubmission(false) | ||
| } |
There was a problem hiding this comment.
Success detection depends on lastSubmittedTicketId changing, but the Zendesk sendBeacon fallback never sets it
Why we think it's a valid issue
- Checked: All three success branches of
submitSupportTicketinsupportLogic.ts, the PR-head success-detection inhandleSupportFormSubmit, and the base (pre-PR) success-detection to determine regression status. - Found: The Beacon-success branch (
supportLogic.ts:1000-1018) callscapture/lemonToast.success/closeSupportForm()/resetSendSupportRequest()and returns but never callssetLastSubmittedTicketId, whereas the fetch-success (:1062) and conversations-success (:795) branches do. PR-headhandleSupportFormSubmitinfers success solely fromticketIdAfter !== ticketIdBefore, so a beacon-success reads as failure (hasSubmittedstays false, modal stays open, no confirmation appended, retry invited) → possible duplicate. Diagnosis confirmed accurate. - Impact: Real but a rare edge within a deprecated path — reached only on the Zendesk fallback (conversations flag off or extension not loaded) AND when
fetchreturns non-ok butnavigator.sendBeaconsucceeds (the Firefox strict-privacy case). The duplicate is also probabilistic:sendBeacontrue only means the request was queued, not accepted, so a duplicate only results when the beacon ticket actually lands and the user resubmits. - Priority: Lowering
should_fix→consider: (1) not a regression — the beacon branch has never set the id and the baseTicketPromptalready detected success via the samelastSubmittedTicketId-diff, so the PR inherits a pre-existing latent gap rather than introducing one; (2) the trigger is a rare browser/network edge inside a Zendesk fallback the codebase describes as being retired; (3) the fix lands in unchangedsupportLogic.ts, beyond this diff's scope. Real and in the PR's domain, so kept on record, but below the should-fix bar.
Issue description
handleSupportFormSubmit treats the ticket as created only when lastSubmittedTicketId changes (ticketIdAfter !== ticketIdBefore); otherwise it falls into the 'submit failed, allow a retry' branch. In supportLogic.ts's submitSupportTicket listener, the Zendesk fallback's sendBeacon success branch (hit when fetch resolves with a non-ok response but navigator.sendBeacon still succeeds — the code's own comment references Firefox's strict privacy mode) shows a success toast and calls closeSupportForm()/resetSendSupportRequest(), but never calls actions.setLastSubmittedTicketId(...) (unlike the fetch-success and conversations-success branches). From TicketPrompt's point of view this looks identical to a real failure: hasSubmitted stays false, the modal stays open, no 'I've created a support ticket for you' confirmation is appended to the thread (so ticketUtils.getTicketPromptData/getTicketSummaryData will still say a ticket is needed on a later /ticket invocation too), and the user is invited to resubmit — filing a genuine duplicate ticket for a submission that already succeeded via Beacon.
Suggested fix
Have the Beacon-success branch in supportLogic.ts also call actions.setLastSubmittedTicketId(...) (a synthetic/local id works since the real Zendesk id isn't available from a fire-and-forget beacon), or replace the ticket-id diff with an explicit success boolean returned/set by submitSupportTicket so TicketPrompt doesn't have to infer success from an id it can't rely on for every code path.
Prompt to fix with AI (copy-paste)
## Context
@frontend/src/scenes/max/TicketPrompt.tsx#L100-118
<issue_description>
`handleSupportFormSubmit` treats the ticket as created only when `lastSubmittedTicketId` changes (`ticketIdAfter !== ticketIdBefore`); otherwise it falls into the 'submit failed, allow a retry' branch. In `supportLogic.ts`'s `submitSupportTicket` listener, the Zendesk fallback's `sendBeacon` success branch (hit when `fetch` resolves with a non-ok response but `navigator.sendBeacon` still succeeds — the code's own comment references Firefox's strict privacy mode) shows a success toast and calls `closeSupportForm()`/`resetSendSupportRequest()`, but never calls `actions.setLastSubmittedTicketId(...)` (unlike the `fetch`-success and conversations-success branches). From `TicketPrompt`'s point of view this looks identical to a real failure: `hasSubmitted` stays `false`, the modal stays open, no 'I've created a support ticket for you' confirmation is appended to the thread (so `ticketUtils.getTicketPromptData`/`getTicketSummaryData` will still say a ticket is needed on a later /ticket invocation too), and the user is invited to resubmit — filing a genuine duplicate ticket for a submission that already succeeded via Beacon.
</issue_description>
<issue_validation>
- **Checked:** All three success branches of `submitSupportTicket` in `supportLogic.ts`, the PR-head success-detection in `handleSupportFormSubmit`, and the base (pre-PR) success-detection to determine regression status.
- **Found:** The Beacon-success branch (`supportLogic.ts:1000-1018`) calls `capture`/`lemonToast.success`/`closeSupportForm()`/`resetSendSupportRequest()` and returns but never calls `setLastSubmittedTicketId`, whereas the fetch-success (`:1062`) and conversations-success (`:795`) branches do. PR-head `handleSupportFormSubmit` infers success solely from `ticketIdAfter !== ticketIdBefore`, so a beacon-success reads as failure (`hasSubmitted` stays false, modal stays open, no confirmation appended, retry invited) → possible duplicate. Diagnosis confirmed accurate.
- **Impact:** Real but a rare edge within a deprecated path — reached only on the Zendesk fallback (conversations flag off or extension not loaded) AND when `fetch` returns non-ok but `navigator.sendBeacon` succeeds (the Firefox strict-privacy case). The duplicate is also probabilistic: `sendBeacon` true only means the request was queued, not accepted, so a duplicate only results when the beacon ticket actually lands and the user resubmits.
- **Priority:** Lowering `should_fix` → `consider`: (1) not a regression — the beacon branch has never set the id and the base `TicketPrompt` already detected success via the same `lastSubmittedTicketId`-diff, so the PR inherits a pre-existing latent gap rather than introducing one; (2) the trigger is a rare browser/network edge inside a Zendesk fallback the codebase describes as being retired; (3) the fix lands in unchanged `supportLogic.ts`, beyond this diff's scope. Real and in the PR's domain, so kept on record, but below the should-fix bar.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Have the Beacon-success branch in `supportLogic.ts` also call `actions.setLastSubmittedTicketId(...)` (a synthetic/local id works since the real Zendesk id isn't available from a fire-and-forget beacon), or replace the ticket-id diff with an explicit success boolean returned/set by `submitSupportTicket` so `TicketPrompt` doesn't have to infer success from an id it can't rely on for every code path.
</potential_solution>
There was a problem hiding this comment.
Not addressing this one: the sendBeacon branch exists only inside the retired Zendesk fallback, and only fires when fetch returns non-ok and the beacon is queued (the Firefox strict-privacy case). Support now runs on conversations, so this edge is confined to a path that is going away. Behavior matches the base branch here (the pre-PR success detection also keyed off lastSubmittedTicketId, which this branch never set).
There was a problem hiding this comment.
Update: the Zendesk fallback is staying in service for now, so this is fixed in 9961928 after all. The beacon-success branch now calls setLastSubmittedTicketId with the client-generated uuid (the same id already embedded in the ticket subject), so a beacon-delivered ticket reads as success, the modal closes, and no retry is invited.
slshults
left a comment
slshults
left a comment
There was a problem hiding this comment.
@christiaan-ph A couple of the things spotted by ReviewHog look like they're worth addressing (at this point, I guess the Zendesk fallbacks are becoming less important though.)
Address review findings on the /ticket modal: - cancelling the modal (esc, X, overlay, or the button) while a submit was in flight cleared the re-entry guard, so reopening and submitting again could file a duplicate ticket; cancel is now a no-op until the request settles and the button shows why - pressing enter on the issue input after a ticket was created reopened the dead modal; openSupportModal now respects hasSubmitted Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Zendesk fallback is still in service, so address the two findings previously deferred as legacy-only: - the direct submitSupportTicket call skipped the kea-forms validator, so clearing the topic on the Zendesk form variant silently routed the ticket to General; the triage fields are now checked before submit - the sendBeacon success branch never set lastSubmittedTicketId, so a beacon-delivered ticket read as a failure to the /ticket modal, leaving it open and inviting a duplicate; the client-generated uuid now marks success, matching the id already embedded in the subject Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@slshults it turns out that actually we are keeping Zendesk for now as a fallback, so I've made these fixes too! |
|
@slshults do you mind re-reviewing? I can't merge until you approve! |
|
🔀 Tried to auto-resolve conflicts with I won't retry until the branch or master moves. |
Resolve conflict in TicketPrompt.tsx: keep this branch's rewritten submit flow (submitInFlightRef/hasSubmitted guards, synchronous handleTicketCreated, ticketUtils) and drop the local posthog.capture for posthog_ai_support_ticket_created. Master's #73525 moved that capture into supportLogic.submitSupportTicket, which this flow already calls, so keeping the local capture would double-fire the event. The ai_conversation_id and ai_trace_id fields master added to resetSendSupportRequest auto-merged and feed the supportLogic capture.
3 participants
Problem
The
/ticketcommand in PostHog AI could create bad tickets in the support inbox:Conversation ID/Trace IDfooter, with no actual message.Two entry points hit the same underlying defects:
/ticketas the first message, then pressing enter on the empty "describe your issue" field./ticketmid-conversation (summary flow), then submitting twice. The first submit files the real ticket and resets the form, so the second submit sends an empty message and produces a footer-only blank.Root causes:
appendMetadataToMessagereturned the ID footer on its own when the message was empty, turning "no content" into a non-empty-looking body that slipped past the form's own "please enter a message" validation.sendMessage(..., true), which starts a fresh ticket every time.Changes
appendTicketMetadata(extracted intoticketUtils.ts) returns an empty string for an empty body, so the ID footer can no longer stand in as a fake message.handleSupportFormSubmitcomposes the body first and refuses to submit when it is empty (toast: "Please add a description before creating a ticket"), guards re-entry with a synchronous ref, and the create and submit controls lock viahasSubmittedonce a ticket exists for the conversation.SupportForm(messageLabel/messagePlaceholder); no other caller changes behavior.submitSupportTicketinstead of being written back into the form first. Previously the full body was dumped into the visible note field during submission, a failed submit left it polluted (and a retry would have appended the summary and footer twice), and success was detected via the kea-forms submit promise, which resolves before the ticket request finishes, so the modal never closed. The modal now closes synchronously once the awaited submit sets the new ticket id.How did you test this code?
hogli test frontend/src/scenes/max/ticketUtils.test.ts: 21 passing. I added parameterized cases forcomposeTicketBodyandappendTicketMetadata. The empty-body case is the direct regression guard for the footer-only blank ticket that no existing test covered.tsgofailures are pre-existing stale-typegen andposthog-jsresolution issues unrelated to this diff./ticketinput flow, the mid-conversation summary flow with topic inference, and the create-ticket modal in both form variants (the conversations variant withproduct-support-side-panelenabled, and the legacy triage-field variant with it disabled), including ticket creation end to end and the modal closing with a thread confirmation and locked controls afterwards.product-support-side-panelat 100%, conversations extension loaded, tickets landing in the local conversations inbox withchannel_detail: widget_api, matching production). Ticket and comment rows were checked in the database after every submission:Screenshots
1.
/ticketas the first message: the describe-your-issue input (empty Enter and empty submit are now guarded)2.
/ticketafter describing an issue: AI summary with inferred topic, then the create button3. The create-ticket modal: read-only "PostHog AI's analysis" block plus the optional "Anything to add?" note
4. After a successful submit, the ticket is created and we see a confirmation
Automatic notifications
Docs update
No docs change. This is internal support UX, not a documented workflow.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
I (Claude, Opus 4.8, via Claude Code) investigated and wrote this at Christiaan's direction. I confirmed the failure mode against the support inbox with the PostHog MCP (
conversations-tickets-*): the offending tickets aremessage_count: 1with a body of only the ID footer, and at least one pair was an exact duplicate created under a second apart. Two entry points produce the footer-only body: the empty-field enter on the first-message path, and a double submit in the summary path after the first success resets the form.Skills invoked:
/writing-tests(before adding theticketUtilscases).Decisions: I chose an empty-body guard plus a synchronous submit lock over a debounce, since the lock also covers the summary-path double submit, not just fast double clicks. I moved body and metadata composition into pure helpers in
ticketUtils.tsso the regression is covered by cheap unit tests rather than a component render. I kept theSupportFormchange to two optional props to avoid touching its other callers. Manual testing caught that my first version wrote the composed body into the visible note field and never closed the modal (the kea-forms submit promise resolves too early), so the final version submits throughsubmitSupportTicketdirectly. The work was built on a dedicated git worktree offorigin/masterbecause a separate branch was concurrently editing the same support files.