Repository navigation
Conversation
…lusions Unfinished: the red/green proof runs and the OpenAPI regeneration were interrupted by a devbox rebalance. Tests are written but not yet run green; generated OpenAPI output is not regenerated. Continue test-first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ender endpoint Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…equire write scope Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…quest_access comment Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…haracters Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…reated Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d modal Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ws tab Behind the workflows-first-run flag, a project with no workflows sees the global email templates on the Workflows tab instead of the empty state. Templates its events can start come first, led by one recommended starter, and "Start playing" opens a blank workflow. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A planned event definition, such as one the Wizard creates before the event arrives, no longer marks a template ready. The recommended reason moves above the cover so the footer shows only what starts the template, a not-ready card refers to the event chip above it, and the subtitle stops promising an order when nothing is ready. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The error banner now covers a failed fit lookup as well as a failed template load, and the subtitle waits for the gallery to load. The template type before fit gets a name that does not clash with the email template type, and the pick event is checked for every kind of card. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The subtitle now matches the picked and all views, and the first run tab reads the setup status without mounting its logic twice. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Covers below the first screen no longer download up front, and a test holds the error banner and its retry for both gallery lookups. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The test send modal passes the member list loading state to the recipient select, so the popover shows skeleton rows instead of the empty state until the suggestions arrive, and the server skip message wraps a long address instead of clipping it. The email sender picker reads the sandbox sender id and its selection from emailTemplaterLogic, which now derives them once with the same rule the picker and chooseSenders already used. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The first run reuses the reader the broadcast test send has, so it moves next to the hog flow test result type it reads. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A gallery card now opens Make it yours on the Workflows tab, addressed by the template search param. The screen lists every email of the template in flow order with its timing, keeps edits across switching, sends the open email to the signed-in user from the first-run sender, and creates the workflow with every edited email, the sender on every email step and the matched trigger event, active or draft per the Enable workflow switch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… make it yours The templater exports a canvas edit 500ms after the last change. Switching emails, sending a test or creating the workflow inside that window acted on the email without the edit, so all three now wait for the export. The editor runs as a template editor, which hides the From and To fields the first run overrides anyway, so the controls no longer promise a sender the payload ignores. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe pull request adds managed sandbox email senders across backend and frontend flows. It provisions senders, validates recipient membership, applies sandbox delivery rules, and updates sender selection interfaces. It also adds a feature-gated workflows first-run gallery and editor with template recommendations, email editing, test sends, workflow creation, and AI context. Workflow templates now expose structured start metadata. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The new sandbox sender and first-run workflow experience are behind feature flags. Three small issues remain. One template tells users it starts on /onboarding but actually fires on a different URL. Turning on sandbox sending before its SES settings are configured causes generic send failures instead of a clean skip. One failed sender setup is not retried until the user switches projects. None of these blocks a flagged merge, but they are cheap to fix. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Recipient identity checks and fixed sender settings constrain sandbox delivery. However, safe enablement depends on independent abuse controls for the shared sending infrastructure. Unsaved email content also becomes AI conversation context, making its trust separation and cleanup important. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Full details: Description checkExplanation The description follows the required sections and clearly explains the first-run AI context change. However, it does not describe many changes listed in the PR summary, including sandbox sender services and validation, email integration updates, and workflow template metadata. Resolution Reconcile the description with the actual PR scope. If those summarized changes are part of this PR, add their user-visible effects, testing, and release status. If they belong to the parent stack, clarify that the description and change counts cover only this increment, and ensure the PR summary reflects that scope.
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (4)
products/workflows/backend/templates/onboarding_started_but_not_completed_template.json-4-8 (1)
4-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe
starts_ondetail does not match the trigger filter.
starts_on.detailsays "on /onboarding". The trigger filter at Lines 37-40 matches$current_urlicontainsmessaging/campaigns. The gallery tells users that the workflow fires on/onboarding, but the created workflow fires on a different URL. Align the detail with the filter value, or change the filter to/onboarding.products/workflows/frontend/firstRun/MakeItYoursEmailList.tsx-14-16 (1)
14-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix pluralization for single-email templates.
When a template has one email, the header reads "1 emails in this workflow". Use the singular noun when the count is 1.
Proposed fix
- {templateEmails.length} emails in this workflow + {templateEmails.length} {templateEmails.length === 1 ? 'email' : 'emails'} in this workflownodejs/src/cdp/cdp-services.ts-464-472 (1)
464-472: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFail closed when the sandbox route is only partly configured.
enabledcomes only fromWORKFLOWS_SANDBOX_SENDER_ENABLED. The tenant, configuration set, and From address all default to''. Suppose the flag is turned on beforeSES_SANDBOX_TENANT_NAME,SES_SANDBOX_CONFIGURATION_SET, andSES_SANDBOX_FROM_ADDRESSare set. Then every sandbox send passes the membership checks and reaches SES withTenantName: '',ConfigurationSetName: '', and an empty From address. The result is a genericFailed to send email via SESerror and anemail_failedmetric. Users do not get the "sandbox sender is unavailable" skip.Derive
enabledfrom all four settings.EmailServicealready handles a disabled sender cleanly.Proposed fix
new SandboxEmailSender( { - enabled: config.WORKFLOWS_SANDBOX_SENDER_ENABLED, + enabled: + config.WORKFLOWS_SANDBOX_SENDER_ENABLED && + !!config.SES_SANDBOX_TENANT_NAME && + !!config.SES_SANDBOX_CONFIGURATION_SET && + !!config.SES_SANDBOX_FROM_ADDRESS, tenantName: config.SES_SANDBOX_TENANT_NAME,frontend/src/lib/integrations/integrationsLogic.ts-1139-1149 (1)
1139-1149: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA
failedresult blocks every later provisioning attempt until the project changes.
provisionMissingSandboxEmailSenderreturns early whensandboxEmailSenderProvisionResultis truthy. A transient error or a 15 s timeout stores'failed'. After that, the sandbox sender is never requested again for the session, and onlyclearIntegrationson a project switch resets it. Users with no own sender then stay on the empty state. Retry on'failed'and keep only'unavailable'sticky.Proposed fix
- values.sandboxEmailSenderProvisionResult || + values.sandboxEmailSenderProvisionResult === 'provisioned' || + values.sandboxEmailSenderProvisionResult === 'unavailable' ||This change can make a hard-failing endpoint retry on every integrations poll. Add a backoff or a retry cap if that is a concern.
🧹 Nitpick comments (1)
products/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.stories.tsx (1)
83-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the mutated app context after the
ViewerAccessstory.The render function writes
'viewer'straight into the globalwindow.POSTHOG_APP_CONTEXT.resource_access_control.hog_flow. Nothing resets it. Stories that render after this one in the same runtime can then inherit viewer access, and their snapshots become wrong. Set the value in a decorator, and restore the previous value when the decorator unmounts.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
fb7f6cb5-9ab4-4fe4-9d89-db4a23c1eb2f
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlproducts/integrations/frontend/generated/api.tsis excluded by!**/generated/**products/workflows/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (102)
docs/internal/workflows-email-sending-tiers.mdfrontend/src/lib/components/CyclotronJob/CyclotronJobInputs.tsxfrontend/src/lib/constants.tsxfrontend/src/lib/integrations/IntegrationsList.tsxfrontend/src/lib/integrations/integrationsLogic.test.tsfrontend/src/lib/integrations/integrationsLogic.tsfrontend/src/lib/integrations/utils.tsfrontend/src/scenes/hog-functions/email-templater/EmailTemplater.test.tsxfrontend/src/scenes/hog-functions/email-templater/EmailTemplater.tsxfrontend/src/scenes/hog-functions/email-templater/NativeEmailIntegrationChoice.stories.tsxfrontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.tsfrontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsxfrontend/src/scenes/hog-functions/email-templater/selectSenders.test.tsfrontend/src/scenes/hog-functions/email-templater/selectSenders.tsnodejs/package.jsonnodejs/src/cdp/cdp-services.tsnodejs/src/cdp/config.tsnodejs/src/cdp/services/managers/organization-members.service.tsnodejs/src/cdp/services/messaging/email.service.test.tsnodejs/src/cdp/services/messaging/email.service.tsnodejs/src/cdp/services/messaging/sandbox-email-sender.tsnodejs/src/cdp/types.tsnodejs/src/cdp/utils.tsposthog/api/integration.pyposthog/cdp/test/test_validation.pyposthog/cdp/validation.pyposthog/models/integration/__init__.pyposthog/models/integration/email.pyposthog/settings/ses.pyproducts/integrations/mcp/tools.yamlproducts/workflows/backend/facade/api.pyproducts/workflows/backend/facade/contracts.pyproducts/workflows/backend/presentation/views/hog_flow.pyproducts/workflows/backend/presentation/views/hog_flow_template.pyproducts/workflows/backend/services/sandbox_sender.pyproducts/workflows/backend/templates/announce_a_new_feature_template.jsonproducts/workflows/backend/templates/celebrate-milestone.jsonproducts/workflows/backend/templates/feature-adoption-tips.jsonproducts/workflows/backend/templates/heavy-usage-detected.jsonproducts/workflows/backend/templates/high-intent-notification.jsonproducts/workflows/backend/templates/negative-survey-response.jsonproducts/workflows/backend/templates/onboarding_started_but_not_completed_template.jsonproducts/workflows/backend/templates/re-engagement-workflow.jsonproducts/workflows/backend/templates/repeated-failure-alert.jsonproducts/workflows/backend/templates/support-ticket-notification.jsonproducts/workflows/backend/templates/ticket-unresolved-alert.jsonproducts/workflows/backend/templates/trial-ending-reminder.jsonproducts/workflows/backend/templates/trial_started_upgrade_nudge_template.jsonproducts/workflows/backend/templates/unlocking_advanced_features.jsonproducts/workflows/backend/templates/unused-features-education.jsonproducts/workflows/backend/templates/user-stuck-onboarding.jsonproducts/workflows/backend/templates/user-upgrade-webhook.jsonproducts/workflows/backend/templates/welcome_email_sequence_template.jsonproducts/workflows/backend/test/test_sandbox_sender_lifecycle.pyproducts/workflows/backend/tests/api/test_email_reputation.pyproducts/workflows/backend/tests/api/test_hog_flow_template.pyproducts/workflows/backend/tests/api/test_sandbox_sender.pyproducts/workflows/backend/tests/api/test_sandbox_sender_validation.pyproducts/workflows/frontend/Broadcasts/broadcastTestSendLogic.tsproducts/workflows/frontend/Broadcasts/steps/BroadcastContentStep.tsxproducts/workflows/frontend/Channels/MessageChannels.stories.tsxproducts/workflows/frontend/Channels/MessageChannels.tsxproducts/workflows/frontend/Channels/SandboxEmailSenderRow.tsxproducts/workflows/frontend/TemplateLibrary/SendTestEmailModal.stories.tsxproducts/workflows/frontend/TemplateLibrary/SendTestEmailModal.test.tsxproducts/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsxproducts/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.tsproducts/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.tsproducts/workflows/frontend/Workflows/hogflows/findTestSendSkipReason.test.tsproducts/workflows/frontend/Workflows/hogflows/findTestSendSkipReason.tsproducts/workflows/frontend/Workflows/hogflows/steps/components/HogFlowFunctionConfiguration.tsxproducts/workflows/frontend/Workflows/templates/WorkflowTemplateCard.tsxproducts/workflows/frontend/WorkflowsScene.tsxproducts/workflows/frontend/emptyState/workflowsEmptyState.tsxproducts/workflows/frontend/firstRun/GalleryTemplateCard.tsxproducts/workflows/frontend/firstRun/MakeItYoursActions.tsxproducts/workflows/frontend/firstRun/MakeItYoursEmailList.tsxproducts/workflows/frontend/firstRun/TemplateStartsOnLine.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.stories.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunTab.test.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunTab.tsxproducts/workflows/frontend/firstRun/buildFirstRunTestSend.tsproducts/workflows/frontend/firstRun/buildFirstRunWorkflow.tsproducts/workflows/frontend/firstRun/firstRunEmailAgentContext.tsproducts/workflows/frontend/firstRun/firstRunEmailAgentLogic.test.tsproducts/workflows/frontend/firstRun/firstRunEmailAgentLogic.tsproducts/workflows/frontend/firstRun/firstRunGalleryLogic.test.tsproducts/workflows/frontend/firstRun/firstRunGalleryLogic.tsproducts/workflows/frontend/firstRun/firstRunMakeItYoursLogic.test.tsproducts/workflows/frontend/firstRun/firstRunMakeItYoursLogic.tsproducts/workflows/frontend/firstRun/firstRunSenderLogic.test.tsproducts/workflows/frontend/firstRun/firstRunSenderLogic.tsproducts/workflows/frontend/firstRun/firstRunStoryFixtures.tsproducts/workflows/frontend/firstRun/firstRunWorkflowStorage.tsproducts/workflows/frontend/firstRun/templateEmails.tsproducts/workflows/frontend/firstRun/withFirstRunSender.test.tsproducts/workflows/frontend/firstRun/withFirstRunSender.tsproducts/workflows/package.jsonservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
Problem
A new Workflows user cannot discuss the selected, unsaved first-run email with PostHog AI without explaining its contents.
Stacked on #112053. Tracks Silthus/posthog#261.
This draft covers opening AI and keeping the email context current. AI does not apply edits to the editor in this draft.
The ticket remains parked for the scope decision.
Note
🔵 Merge risk: low
The increment changes frontend panel state and context behind the first-run and sandbox flags. It adds no writes or sends.
Changes
The existing frontend-applied MaxTool still exists, but only in the frozen LangGraph runtime.
The sandbox tools update saved workflows or library templates; they cannot change this unsaved email.
Saving a record early would break the first-run contract. This draft therefore tells AI to suggest changes for the user to apply.
Before and after interaction
Before:
flowchart LR A[Make it yours] --> B[Edit email manually] classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; class A phYellow; class B phBlue;After:
flowchart LR A[Make it yours] --> B{{AI with selected email}} B --> C[User types a request] C --> D[Suggested changes] D --> E[User edits email manually] classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class A phYellow; class B,C,E phBlue; class D phGray;How did you test this code?
Proof bundle contains red/green logs and the final local gate.
Test rationale: The kea integration seam observes panel state and attached context while driving real email-selection and edit actions.
The closest existing coverage is
firstRunMakeItYoursLogic.test.tsandsceneAgentPanelLogic.test.ts.Those tests do not cover first-run context, dismissal and restoration, cleanup, delayed preflight, body mode, or bounded context.
The HTTP layer supplies synthetic templates; no internal logic is mocked.
The final gate runs the touched test through hogli, frontend typecheck, lint, format checking, and strict preflight against the parent.
Preflight exits successfully with a branch freshness advisory and a complexity warning. Both pushes also pass the strict hook.
The existing first-run Jest suites also pass.
A headless browser renders the production scene and real AI panel in a scratch story using fake data.
The interaction log confirms switching, dismissal, and reopening.
Email 2 shows the updated context.
The scratch story and temporary configuration are removed from the final tree.
A message sent inside the canvas export debounce can attach the previous committed design. The global chat send path is unchanged.
Unchecked: live LLM behavior, backend sends, workflow creation, and AI applying editor changes.
No backend tool changes are included.
Release status
Automatic notifications
Docs update
None. No existing document covers the first-run AI interaction; the scope and limitations are recorded here.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: T3 Code / Codex, GPT-6.1 Sol; Claude Code, Claude Opus 5.5 for the UI changes.
Session: ticket worker for Silthus/posthog#261, under map #253.
Tools: shell, Git, gh, hogli, kea typegen, headless Playwright. The public fixtures use repository templates and
example.com.No matching open PR surfaced in the duplicate search.
CodeRabbit CLI skipped because it is signed out. The PostHog asset uploader refused this account; proof lives on the fork assets branch.
One read-only Claude Opus 5.5 review ran after draft creation against
260cca54a035935295b4c434e87efaf65b34bcd0.The report and all dispositions accompany the final gate.
The fixes ship at
d1e0b0f99fd58241e93a0566b1f55df2195cf880.GitHub denied assigning Silthus as the PR assignee. Silthus remains the ticket assignee and human DRI.
The parent stack has inherited conflicts against upstream master in messaging's email service, its tests, and broadcast test sending.
No file in this increment conflicts. The conductor owns the shared stack refresh.
Skills invoked
/tdd, /unslop, /writing-tests, /writing-ui-components, /writing-kea-logics, /placing-product-frontend-code, /adopting-generated-api-types, /writing-user-facing-copy, /writing-code-comments, /integrating-with-posthog-ai, /using-kea-disposables, /hogli, /run-posthog, /running-ci-preflight, /merge-risk, /writing-pr-descriptions, /reviewing-with-coderabbit, /principle-prove-it-works, /leaving-pr-comments, /writing-voice.
The existing designing-email-templates skill was read and left unchanged.
🤖 Generated with Codex and Claude Code