Skip to content

feat(workflows): make the first-run template yours before enabling it - #112053

Closed
Silthus wants to merge 7 commits into
PostHog:masterfrom
Silthus:feat/workflows-first-run-make-it-yours
Closed

Silthus wants to merge 7 commits into
PostHog:masterfrom
Silthus:feat/workflows-first-run-make-it-yours

Conversation

@Silthus

@Silthus Silthus commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A new user who picks a card in the first-run gallery lands in the blank workflow editor, with no sender set and nothing tested.
Most gallery templates send two to four emails, so a screen with one email editor would hide the rest.

Stacked on #111816 (tailored gallery) and #112003 (first-run sender seam). Review only the last two commits.
Spec: Silthus#256, section "Make it yours". Ticket: Silthus#260. Behind workflows-first-run.

Changes

  • Picking a gallery card opens Make it yours on the Workflows tab at ?template=<id>. Back and reload return to it.
  • The screen lists every email of the template in flow order with its timing ("Right away", "After 1 day"), marks the edited ones, and says which one is open ("Email 2 of 3"). A template with one email shows no list.
  • Edits to each email survive switching. The editor remounts per email, so the templater's debounced canvas export can never land on the email opened after it. Switching, sending a test and creating the workflow wait for an export the templater still holds, so no action runs on a stale email.
  • The editor shows subject, preheader and content only. The first-run sender is fixed and the recipient comes from the trigger, so the From and To controls stay hidden instead of offering a choice the payload would ignore. The sender shows under the "Enable workflow" switch.
  • "Send me a test" sends the open email, as edited, to the signed-in user from the first-run sender through the test invocation endpoint. A "Skipping send:" reason shows in place, no modal.
  • "Enable and open workflow" creates the workflow in one request with every edited email, the first-run sender on every email step, the matched trigger event, and active or draft per the "Enable workflow" switch. It then opens the editor and remembers the id in the browser. With the switch off the button reads "Open workflow".
  • Without a first-run sender, test and the switch are disabled with a reason, a banner links to Channels, and "Open workflow" still creates a draft. Without Editor access to workflows, test and create are disabled with the access reason.
  • A rejected create shows the server message in place and creates nothing. Both loaders catch their own errors, so no generic failure toast doubles the banner.
  • Events after success: workflows first run test sent (skipped) and workflows first run workflow created (template_id, enabled).
  • Layout: the email list and the actions sit in a side column beside the editor. Below a 56rem container width they stack: list, editor, actions, so the screen holds up at 520px next to a docked side panel.
  • Mechanical: findTestSendSkipReason moves from broadcastTestSendLogic to Workflows/hogflows/, the gallery story fixtures move to firstRunStoryFixtures.ts, and the workflows product manifest gains @testing-library/dom for story play functions.
State Full width 520px scene
Two emails, sandbox sender
Three emails
One email, no list
No sender
Viewer access
Test send skipped
Create rejected

How did you test this code?

Test rationale: firstRunMakeItYoursLogic.test.ts drives the logic with the API mocked at the HTTP layer. Each case names a regression no existing test catches: emails out of flow order or with wrong timing; an edit lost when switching emails; a create payload missing an edited email, the sender on a step, the matched trigger event, or the status per switch; a test invocation that sends the wrong email or recipient; a hidden skip reason; a sender-less state that lets a create go active; a rejected create that still navigates; a viewer who can create; a switch, test send or create that runs while the templater still holds an unexported canvas edit. firstRunGalleryLogic.test.ts now asserts a pick routes to ?template= instead of the blank editor. findTestSendSkipReason.test.ts moved with its helper, unchanged.

  • Red before green for each case, then the local gate on the final tree: typecheck, fix, the first-run, Broadcasts and hogflows Jest suites, and the pre-push preflight.
  • Stories: WorkflowsFirstRunMakeItYours.stories.tsx, one per state above plus a narrow one, with play functions for the skipped send and the rejected create. The screenshots above come from them, rendered headless with the Unlayer editor loaded from its CDN.
  • A scripted browser check typed into email 1's subject, switched to email 2, and back: the edit stayed, the "Edited" tag showed, and the canvas showed each email's own body.
  • Not run: a local stack, so the real test send and an active create against the backend are unverified in this PR.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None. The first run is behind a flag.

🤖 Agent context

Session: a background worker of the wayfinder map Silthus#253, ticket Silthus#260.
Skills invoked: /tdd, /writing-kea-logics, /writing-ui-components, /writing-user-facing-copy, /writing-tests, /writing-code-comments, /adopting-generated-api-types, /writing-pr-descriptions.
CodeRabbit CLI pass: skipped, the CLI is signed out and no person was available.
Codex adversarial review (GPT-6.1 Sol), two findings, both fixed: a canvas edit made in the 500ms before switching, sending or creating was lost or stale, so those actions now wait for the templater's pending export (covered by three logic tests); the editor offered sender controls the payload overrode, so the editor now runs as a template editor without From and To.
Decisions along the way: the create uses the generated hogFlowsCreate, while the test send keeps api.hogFlows.createTestInvocation because the generated invocation function has no response type. The screen reads the template and its matched event from the gallery logic instead of loading the template again. Screenshots live on the fork branch assets/fr-260-make-it-yours because the PR asset uploader refused this account.

🤖 Generated with Claude Code

@trunk-io

trunk-io Bot commented Oct 5, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

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

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change adds a feature-flagged sandbox email sender with backend provisioning, organization-member recipient checks, and dedicated SES delivery behavior. Workflow email editors and test-send interfaces can select the sandbox sender and apply its recipient restrictions. The change also adds start-condition metadata to workflow templates and introduces a first-run gallery that recommends templates based on observed events, supports email editing and test sends, and creates workflows.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to a8c22

The onboarding template can create a workflow that responds to the wrong pageviews. Correct its trigger filters before merging. The other open template and sandbox-configuration concerns also need resolution or explicit acceptance.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to a8c22

The managed sender has strong server-side identity and recipient restrictions. However, it bypasses ordinary project sending budgets, so activation must remain blocked until independent sandbox abuse controls are available. Production configuration and those rollout prerequisites were not established.

Retained concerns

  • Medium · security · inferred: Activating sandbox delivery before its independent limits are deployed would remove project-level volume containment for the new managed sender. Valid organization-member recipients constrain destination reach, but do not bound repeated sends, shared tenant consumption, or account-level reputation exposure. The disabled default and documented activation prerequisite mitigate current exposure; production satisfaction of that prerequisite remains unverified.
Security review details

Security Blast Radius

  • inferred — Authorized workflow writers can cause real sandbox sends to eligible members across their organization using a managed identity. Sender references are project-scoped, but sandbox SES routing is shared; excessive valid traffic can therefore affect shared tenant capacity and the SES account's pooled reputation. No unrestricted external-recipient reach was established.

Security Findings and Attack Paths

  • inferred — The remaining attack path is conditional on premature activation: a workflow writer can repeatedly send to valid organization members without consuming ordinary project hourly or daily budgets. Optional workflow pacing does not replace a mandatory sandbox budget, and test sends bypass that pacing. This is a rollout concern, not evidence that the disabled default is currently exploitable.

Trust Boundaries and Controls

  • observed — The server owns sandbox integration configuration and delivery identity. Normal email update and deletion paths reject managed senders, workflow validation rejects new From and Reply-To overrides, and final delivery independently rejects wrong-project references and unauthorized recipients.

Resilience and Maintainability Implications

  • observed — Existing project suspension and workflow pause checks precede sandbox delivery, including test sends. Sandbox throttling preserves the original email payload for retry, allowing subsequent execution to repeat authorization checks rather than treating throttling as successful delivery.

Hardening Proposals

  • proposed — Make sandbox activation depend on verified independent recipient-volume limits and sandbox-tenant pause handling, rather than documentation alone. Preserve the disabled default until those prerequisites are deployed, and explicitly distinguish project selection flags from the global runtime shutdown control.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, user-visible changes, feature-flag status, test coverage, and unverified backend behavior. The Agent context is incomplete: it omits the required Autonomy…
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch feat/workflows-first-run-make-it-yours
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (3)
products/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.stories.tsx-83-88 (1)

83-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Do not mutate the global app context during render.

This story sets hog_flow = 'viewer' on the shared POSTHOG_APP_CONTEXT and does not restore it. The setting can leak into stories that run later in the same runtime. If resource_access_control is undefined, the assignment throws. Set the context in a decorator that restores the original value, or pass the value as a story parameter.

products/workflows/backend/templates/celebrate-milestone.json-4-8 (1)

4-8: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Keep milestone_reached in the trigger filters.

When a user selects this template before milestone_reached has been seen and enables it, matchedEvent is null. withMatchedEvent then leaves the trigger filters empty, and the backend rejects the active workflow. Add the event filter below. The server compiles bytecode; do not add it by hand.

🐛 Suggested fix
-                "filters": {}
+                "filters": {
+                    "events": [
+                        {
+                            "id": "milestone_reached",
+                            "name": "milestone_reached",
+                            "type": "events",
+                            "order": 0
+                        }
+                    ]
+                }
nodejs/src/cdp/cdp-services.ts-464-472 (1)

464-472: 🔒 Security & Privacy | 🛡️ Detected with Advanced Tier | 🟡 Minor | ⚡ Quick win

Disable the sandbox sender when SES settings are incomplete.

When the feature flag is enabled, empty values are passed to SES for FromEmailAddress, TenantName, or ConfigurationSetName. This can cause sandbox sends to fail. The route does not fall back to the team's SES tenant or configuration set.

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,
🧹 Nitpick comments (2)
products/workflows/frontend/firstRun/firstRunSenderLogic.ts (1)

8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Import IntegrationType from ~/types.

The relative path ../../../../frontend/src/types creates a second import path for the same symbol. Other files use the ~/types alias.

nodejs/src/cdp/services/messaging/email.service.ts (1)

1168-1170: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Fail closed when sandboxSender is missing in the From resolution.

If sandboxSender is null, this returns an empty From address. The earlier gate at Line 570 already skips sandbox sends when sandboxSender?.config.enabled is falsy, so this path cannot run today. sesRouteFor has the same problem: without a sender it falls back to the team tenant. If someone reorders these checks later, a sandbox send can reach SES with an empty From or the wrong tenant. Throw an error instead of returning ''.

Proposed fix
         if (integration.config.provider === 'sandbox') {
-            return { email: this.sandboxSender?.config.fromAddress ?? '', name: integration.config.name }
+            if (!this.sandboxSender) {
+                throw new Error('The sandbox sender is not configured')
+            }
+            return { email: this.sandboxSender.config.fromAddress, name: integration.config.name }
         }

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 68d1eaa6-ec9e-4d3a-a3ed-30b2dc4b9812
📥 Commits

Reviewing files that changed from the base of the PR and between 48b7131 and 9777b8c.

⛔ Files ignored due to path filters (3)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • products/integrations/frontend/generated/api.ts is excluded by !**/generated/**
  • products/workflows/frontend/generated/api.schemas.ts is excluded by !**/generated/**
📒 Files selected for processing (99)
  • docs/internal/workflows-email-sending-tiers.md
  • frontend/src/lib/components/CyclotronJob/CyclotronJobInputs.tsx
  • frontend/src/lib/constants.tsx
  • frontend/src/lib/integrations/IntegrationsList.tsx
  • frontend/src/lib/integrations/integrationsLogic.test.ts
  • frontend/src/lib/integrations/integrationsLogic.ts
  • frontend/src/lib/integrations/utils.ts
  • frontend/src/scenes/hog-functions/email-templater/EmailTemplater.test.tsx
  • frontend/src/scenes/hog-functions/email-templater/EmailTemplater.tsx
  • frontend/src/scenes/hog-functions/email-templater/NativeEmailIntegrationChoice.stories.tsx
  • frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.test.ts
  • frontend/src/scenes/hog-functions/email-templater/emailTemplaterLogic.tsx
  • frontend/src/scenes/hog-functions/email-templater/selectSenders.test.ts
  • frontend/src/scenes/hog-functions/email-templater/selectSenders.ts
  • nodejs/package.json
  • nodejs/src/cdp/cdp-services.ts
  • nodejs/src/cdp/config.ts
  • nodejs/src/cdp/services/managers/organization-members.service.ts
  • nodejs/src/cdp/services/messaging/email.service.test.ts
  • nodejs/src/cdp/services/messaging/email.service.ts
  • nodejs/src/cdp/services/messaging/sandbox-email-sender.ts
  • nodejs/src/cdp/types.ts
  • nodejs/src/cdp/utils.ts
  • posthog/api/integration.py
  • posthog/cdp/test/test_validation.py
  • posthog/cdp/validation.py
  • posthog/models/integration/__init__.py
  • posthog/models/integration/email.py
  • posthog/settings/ses.py
  • products/integrations/mcp/tools.yaml
  • products/workflows/backend/facade/api.py
  • products/workflows/backend/facade/contracts.py
  • products/workflows/backend/presentation/views/hog_flow.py
  • products/workflows/backend/presentation/views/hog_flow_template.py
  • products/workflows/backend/services/sandbox_sender.py
  • products/workflows/backend/templates/announce_a_new_feature_template.json
  • products/workflows/backend/templates/celebrate-milestone.json
  • products/workflows/backend/templates/feature-adoption-tips.json
  • products/workflows/backend/templates/heavy-usage-detected.json
  • products/workflows/backend/templates/high-intent-notification.json
  • products/workflows/backend/templates/negative-survey-response.json
  • products/workflows/backend/templates/onboarding_started_but_not_completed_template.json
  • products/workflows/backend/templates/re-engagement-workflow.json
  • products/workflows/backend/templates/repeated-failure-alert.json
  • products/workflows/backend/templates/support-ticket-notification.json
  • products/workflows/backend/templates/ticket-unresolved-alert.json
  • products/workflows/backend/templates/trial-ending-reminder.json
  • products/workflows/backend/templates/trial_started_upgrade_nudge_template.json
  • products/workflows/backend/templates/unlocking_advanced_features.json
  • products/workflows/backend/templates/unused-features-education.json
  • products/workflows/backend/templates/user-stuck-onboarding.json
  • products/workflows/backend/templates/user-upgrade-webhook.json
  • products/workflows/backend/templates/welcome_email_sequence_template.json
  • products/workflows/backend/test/test_sandbox_sender_lifecycle.py
  • products/workflows/backend/tests/api/test_email_reputation.py
  • products/workflows/backend/tests/api/test_hog_flow_template.py
  • products/workflows/backend/tests/api/test_sandbox_sender.py
  • products/workflows/backend/tests/api/test_sandbox_sender_validation.py
  • products/workflows/frontend/Broadcasts/broadcastTestSendLogic.ts
  • products/workflows/frontend/Broadcasts/steps/BroadcastContentStep.tsx
  • products/workflows/frontend/Channels/MessageChannels.stories.tsx
  • products/workflows/frontend/Channels/MessageChannels.tsx
  • products/workflows/frontend/Channels/SandboxEmailSenderRow.tsx
  • products/workflows/frontend/TemplateLibrary/SendTestEmailModal.stories.tsx
  • products/workflows/frontend/TemplateLibrary/SendTestEmailModal.test.tsx
  • products/workflows/frontend/TemplateLibrary/SendTestEmailModal.tsx
  • products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.test.ts
  • products/workflows/frontend/TemplateLibrary/messageTemplateTestSendLogic.ts
  • products/workflows/frontend/Workflows/hogflows/findTestSendSkipReason.test.ts
  • products/workflows/frontend/Workflows/hogflows/findTestSendSkipReason.ts
  • products/workflows/frontend/Workflows/hogflows/steps/components/HogFlowFunctionConfiguration.tsx
  • products/workflows/frontend/Workflows/templates/WorkflowTemplateCard.tsx
  • products/workflows/frontend/WorkflowsScene.tsx
  • products/workflows/frontend/emptyState/workflowsEmptyState.tsx
  • products/workflows/frontend/firstRun/GalleryTemplateCard.tsx
  • products/workflows/frontend/firstRun/MakeItYoursActions.tsx
  • products/workflows/frontend/firstRun/MakeItYoursEmailList.tsx
  • products/workflows/frontend/firstRun/TemplateStartsOnLine.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.stories.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunTab.test.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunTab.tsx
  • products/workflows/frontend/firstRun/buildFirstRunTestSend.ts
  • products/workflows/frontend/firstRun/buildFirstRunWorkflow.ts
  • products/workflows/frontend/firstRun/firstRunGalleryLogic.test.ts
  • products/workflows/frontend/firstRun/firstRunGalleryLogic.ts
  • products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.test.ts
  • products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.ts
  • products/workflows/frontend/firstRun/firstRunSenderLogic.test.ts
  • products/workflows/frontend/firstRun/firstRunSenderLogic.ts
  • products/workflows/frontend/firstRun/firstRunStoryFixtures.ts
  • products/workflows/frontend/firstRun/firstRunWorkflowStorage.ts
  • products/workflows/frontend/firstRun/templateEmails.ts
  • products/workflows/frontend/firstRun/withFirstRunSender.test.ts
  • products/workflows/frontend/firstRun/withFirstRunSender.ts
  • products/workflows/package.json
  • services/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.

3,
2
]
"actions": []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
rg -nP 'bytecode' products/workflows/backend/presentation/views/hog_flow.py products/workflows/backend -g '*.py' | head -40

Repository: PostHog/posthog

Length of output: 5785


🏁 Script executed:

#!/bin/bash
set -u
file=products/workflows/backend/templates/onboarding_started_but_not_completed_template.json
printf '%s\n' '--- PR diff for template ---'
git diff --no-ext-diff --unified=30 2a289c4dacbcd27e28a0197e3dbdd9f594538fa6 9777b8c10d38c0351957ef5f5a43c2192e53231a -- "$file"
printf '%s\n' '--- Current template, numbered ---'
nl -ba "$file"
printf '%s\n' '--- Template references ---'
rg -n -F -- 'onboarding_started_but_not_completed_template' products/workflows
printf '%s\n' '--- Relevant recompile/save implementation blocks ---'
sed -n '3700,3805p' products/workflows/backend/presentation/views/hog_flow.py
sed -n '5830,5915p' products/workflows/backend/presentation/views/hog_flow.py
printf '%s\n' '--- Template workflow endpoints/references by template loader terms ---'
rg -n 'templates/|template.*content|create.*template|template.*create|_recompil|recompil' products/workflows/backend

Repository: PostHog/posthog

Length of output: 41989


🏁 Script executed:

#!/bin/bash
set -u
printf '%s\n' '--- starts_on consumers and template paths ---'
rg -n 'starts_on|sync_hog_flow_template|sync_template_to_db|global.*template|template.*workflow' products/workflows/frontend products/workflows/backend
printf '%s\n' '--- HogFlow serializer: validation/save and event bytecode handling ---'
sed -n '1360,1455p' products/workflows/backend/presentation/views/hog_flow.py
sed -n '1480,1625p' products/workflows/backend/presentation/views/hog_flow.py
printf '%s\n' '--- Template API creation/detail binding ---'
sed -n '1,130p' products/workflows/backend/presentation/views/hog_flow_template.py
sed -n '300,355p' products/workflows/backend/presentation/views/hog_flow_template.py
printf '%s\n' '--- Template sync/load implementation ---'
rg -n 'def sync_.*template|def get_.*template|templates/.*json|glob\\(' products/workflows/backend
printf '%s\n' '--- workflow create endpoint and serializer save definitions ---'
rg -n 'class HogFlowSerializer|def create\\(|def update\\(|HogFlowSerializer\\(' products/workflows/backend/presentation/views/hog_flow.py | head -60

Repository: PostHog/posthog

Length of output: 42451


🏁 Script executed:

#!/bin/bash
set -u
printf '%s\n' '--- starts_on consumers and template paths ---'
rg -n 'starts_on|sync_hog_flow_template|sync_template_to_db|global.*template|template.*workflow' products/workflows/frontend products/workflows/backend
printf '%s\n' '--- HogFlow serializer: validation/save and event bytecode handling ---'
sed -n '1360,1455p' products/workflows/backend/presentation/views/hog_flow.py
sed -n '1480,1625p' products/workflows/backend/presentation/views/hog_flow.py
printf '%s\n' '--- Template API creation/detail binding ---'
sed -n '1,130p' products/workflows/backend/presentation/views/hog_flow_template.py
sed -n '300,355p' products/workflows/backend/presentation/views/hog_flow_template.py
printf '%s\n' '--- Template sync/load implementation ---'
rg -n 'def sync_.*template|def get_.*template|templates/.*json|glob\(' products/workflows/backend
printf '%s\n' '--- workflow create endpoint and serializer save definitions ---'
rg -n 'class HogFlowSerializer|def create\(|def update\(|HogFlowSerializer\\(' products/workflows/backend/presentation/views/hog_flow.py | head -60

Repository: PostHog/posthog

Length of output: 42759


🏁 Script executed:

#!/bin/bash
set -u
printf '%s\n' '--- Global template service and source loader ---'
nl -ba products/workflows/backend/services/hog_flow_templates.py | sed -n '1,145p'
nl -ba products/workflows/backend/templates/__init__.py | sed -n '120,195p'
printf '%s\n' '--- starts_on gallery presentation/use ---'
nl -ba products/workflows/frontend/firstRun/firstRunGalleryLogic.ts | sed -n '30,60p;150,190p'
rg -n 'startsOn|starts_on|workflowTemplate|template.*actions|createWorkflow|hogFlows' products/workflows/frontend/firstRun products/workflows/frontend/Workflows/NewWorkflowModal.tsx
printf '%s\n' '--- HogFlow trigger validation and event filter serializer ---'
nl -ba products/workflows/backend/presentation/views/hog_flow.py | sed -n '1280,1435p'
nl -ba products/workflows/backend/presentation/views/hog_flow.py | sed -n '1560,1635p'
printf '%s\n' '--- Workflow API create/update methods ---'
rg -n -F 'class HogFlowSerializer' products/workflows/backend/presentation/views/hog_flow.py
rg -n -F 'def create(' products/workflows/backend/presentation/views/hog_flow.py
rg -n -F 'def update(' products/workflows/backend/presentation/views/hog_flow.py

Repository: PostHog/posthog

Length of output: 33159


🏁 Script executed:

#!/bin/bash
set -u
printf '%s\n' '--- First-run workflow creation and build ---'
nl -ba products/workflows/frontend/firstRun/buildFirstRunWorkflow.ts | sed -n '1,130p'
nl -ba products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.ts | sed -n '230,265p;280,315p'
printf '%s\n' '--- starts_on display contract ---'
nl -ba products/workflows/frontend/firstRun/TemplateStartsOnLine.tsx | sed -n '1,40p'
printf '%s\n' '--- HogFunctionFiltersSerializer declaration and bytecode generation ---'
rg -n -F 'class HogFunctionFiltersSerializer' posthog products

Repository: PostHog/posthog

Length of output: 8853


🏁 Script executed:

#!/bin/bash
set -u
printf '%s\n' '--- Filter serializer implementation ---'
nl -ba posthog/cdp/validation.py | sed -n '1070,1185p'
printf '%s\n' '--- HogFlow serializer and create/update implementation ---'
nl -ba products/workflows/backend/presentation/views/hog_flow.py | sed -n '2805,2855p'
nl -ba products/workflows/backend/presentation/views/hog_flow.py | sed -n '3265,3310p'

Repository: PostHog/posthog

Length of output: 11669


🏁 Script executed:

#!/bin/bash
set -u
nl -ba posthog/cdp/validation.py | sed -n '1180,1245p'

Repository: PostHog/posthog

Length of output: 3291


Match both trigger filters to the onboarding URL.

The gallery advertises $pageview on /onboarding, but both trigger filters still match $current_url containing messaging/campaigns. The workflow builder preserves those filters, so the workflow can enroll campaign-page visitors and miss onboarding visits. Update both values:

🐛 Suggested fix
@@ -39 +39 @@
-                            "value": "messaging/campaigns",
+                            "value": "/onboarding",
@@ -133 +133 @@
-                                    "value": "messaging/campaigns",
+                                    "value": "/onboarding",

Comment on lines +57 to +61
for trigger in triggers:
if trigger["type"] == "event":
for event in trigger.get("filters", {}).get("events", []):
if event.get("id"):
assert event["id"] == starts_on["events"][0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail the consistency check when an event trigger has no event filters.

If an event trigger has no filters.events, the loop runs zero times and the test passes. celebrate-milestone.json passes this way even though its trigger has empty filters. For a non-schedule starts_on, assert that each event trigger declares at least one event.

Proposed fix
                     if trigger["type"] == "event":
-                        for event in trigger.get("filters", {}).get("events", []):
+                        events = trigger.get("filters", {}).get("events", [])
+                        if starts_on["kind"] == "event":
+                            assert events, "event trigger must declare its starts_on event"
+                        for event in events:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for trigger in triggers:
if trigger["type"] == "event":
for event in trigger.get("filters", {}).get("events", []):
if event.get("id"):
assert event["id"] == starts_on["events"][0]
for trigger in triggers:
if trigger["type"] == "event":
events = trigger.get("filters", {}).get("events", [])
if starts_on["kind"] == "event":
assert events, "event trigger must declare its starts_on event"
for event in events:
if event.get("id"):
assert event["id"] == starts_on["events"][0]

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Normalize structured create errors before rendering them. · firstRunMakeItYoursLogic.ts:275

products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.ts:275
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Normalize structured create errors before rendering them.

If hogFlowsCreate rejects with a plain-object detail, this stores that object in createError. MakeItYoursActions renders it directly as a React child, which can throw and prevent the error banner from rendering. Convert non-string details to readable text before calling setCreateError.

🟡 Other comments (2)
products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.ts-401-403 (1)

401-403: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep test-send outcomes with the email that was sent.

If the user switches emails after a test send, testSendResult remains visible beside the newly opened email. A request started before the switch can also finish afterward and show a success banner there. Capture the email ID when sending, and show or apply the outcome only while that email remains open. Based on learnings: “capture the entity's id before starting the async work, and re-check that the active entity id still matches before applying the result.”

Source: Learnings

products/workflows/frontend/firstRun/MakeItYoursActions.tsx-78-78 (1)

78-78: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Describe when the workflow sends.

An enabled workflow does not necessarily send right away. The supplied trial template waits for its trigger and schedules emails after 10, 13, and 20 days. Replace this text with wording that says sends follow a matching trigger and the template's schedule.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 8e3735bb-b5dc-4724-af7f-f1106c19ee2d
📥 Commits

Reviewing files that changed from the base of the PR and between 9777b8c and a8c22c7.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • products/workflows/frontend/firstRun/MakeItYoursActions.tsx
  • products/workflows/frontend/firstRun/WorkflowsFirstRunMakeItYours.tsx
  • products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.test.ts
  • products/workflows/frontend/firstRun/firstRunMakeItYoursLogic.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.

@Silthus

Silthus commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Note

🤖 GPT-6.1 Sol responding on behalf of Michael

Replaced by the draft fork PR #332 in its native fork stack. @meikelmosby will start migrating the stack from the fork into PostHog/posthog for upstream review and CI. Closing this master-targeted copy to keep the handoff clear.

@Silthus Silthus closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant