From 52f1a333285328816cc6b0ebc5061f05a8775ab4 Mon Sep 17 00:00:00 2001 From: sarahxsanders Date: Fri, 4 Sep 2026 11:32:07 -0400 Subject: [PATCH 1/4] fix(integration): capture completion events on success, not on submit intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capture step told the agent to instrument "the real user action — the click or submit handler". That draws the line at handler-vs-render and never at intent-vs-outcome, so a completion event landed at the top of the submit handler and a form the server rejected still counted as a success. Inflated event counts, and the inflation lands on the metrics people steer by. Completion events now belong in the branch that runs after the awaited response confirms success. Capturing the attempt as well is still fine where the drop-off matters, under its own name — one name must never mean both. Also closes an adjacent gap on the server side. Backend SDKs process a person profile on every capture carrying a distinct id, and the runtime metadata they attach overwrites what the browser set on that person, so an event captured from a Linux host rewrites a macOS user's profile. Identified events also cost more than anonymous ones. Server-side business events now pass $process_person_profile false unless they deliberately update the person. audit-identify already stated both rules, but only as after-the-fact checks — the write-time prompts never got told, so the wizard could generate code its own audit would flag. The wording here is lifted from that audit reference so the two surfaces agree. Generated-By: PostHog Desktop Task-Id: 4fd61d6e-eb4e-498e-abfd-1d0b60d95a63 --- context/agents/integration-v2/capture.md | 5 ++++- context/commandments.yaml | 5 +++-- .../integration-v2/capture/description.md | 18 +++++++++++++++--- .../references/product-analytics.md | 2 ++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/context/agents/integration-v2/capture.md b/context/agents/integration-v2/capture.md index cfc4ef9d..0e187e7f 100644 --- a/context/agents/integration-v2/capture.md +++ b/context/agents/integration-v2/capture.md @@ -45,7 +45,10 @@ user the way the identify docs describe, not the event. ## How you know you succeeded The meaningful user actions across the app have capture calls that fire on the -real action, not on page load, each one attributable to the user who took it, and +real action, not on page load — and where the action waits on a server, in the +branch that runs once the response confirms success, never before it. Each one is +attributable to the user who took it, server-side business events that are not +updating the person pass `$process_person_profile: false`, and `.posthog-wizard-cache/.posthog-events.json` lists the events you instrumented. You do not run builds, linters, or tests — the review task verifies the whole integration after you; your edits just need to be right by reading. diff --git a/context/commandments.yaml b/context/commandments.yaml index ff5ca2d0..e59f7d55 100644 --- a/context/commandments.yaml +++ b/context/commandments.yaml @@ -17,7 +17,7 @@ commandments: react: # PostHog-specific - For feature flags, use useFeatureFlagEnabled() or useFeatureFlagPayload() hooks - they handle loading states and external sync automatically - - Add analytics capture in event handlers where user actions occur, NOT in useEffect reacting to state changes + - Add analytics capture in event handlers where user actions occur, NOT in useEffect reacting to state changes - and when the handler awaits a server call, capture in the success branch after it resolves, never at the top of the handler where a rejected request still counts as a success # General React patterns (react.dev/learn/you-might-not-need-an-effect) - Do NOT use useEffect for data transformation - calculate derived values during render instead - Do NOT use useEffect to respond to user events - put that logic in the event handler itself @@ -60,7 +60,8 @@ commandments: javascript_node: - posthog-node is the Node.js server-side SDK package name; posthog-js is browser-only, so use posthog-node on the server instead - 'Include enableExceptionAutocapture: true in the PostHog constructor options' - - Add posthog.capture() calls in route handlers for meaningful user actions – every route that creates, updates, or deletes data should track an event with contextual properties + - Add posthog.capture() calls in route handlers for meaningful user actions – every route that creates, updates, or deletes data should track an event with contextual properties, captured after the write succeeds rather than on entry to the handler + - 'Backend SDKs process a person profile on every capture carrying a distinct id, and the runtime metadata they attach ($os, $lib) overwrites what the browser set on that person. Pass `$process_person_profile: false` in the properties of server-side business events that are not deliberately updating person properties.' - Add posthog.captureException(err, distinctId) in the application's error handler (e.g., Express error middleware, Fastify setErrorHandler, Koa app.on('error')) - The SDK batches events and flushes asynchronously. await flush() or await shutdown() before letting that process exit. If unsure, set flushAt 1 and flushInterval 0. - '`posthog.capture()` enqueues synchronously and returns; the batched HTTP send happens afterwards. Treat every per-request handler as short-lived even when the framework feels like a server: Next.js / Nuxt / SvelteKit / Remix route handlers, serverless and edge functions, and Lambda are torn down per invocation before the send runs. Create the client with flushAt 1 and flushInterval 0, then await the send before returning. Always use `await posthog.flush()` for a shared/singleton client, `await posthog.shutdown()` for a per-request client. Never skip the awaited flush or risk the enqueued event being silently dropped.' diff --git a/context/skills/integration-v2/capture/description.md b/context/skills/integration-v2/capture/description.md index 1b392c65..0010a8bf 100644 --- a/context/skills/integration-v2/capture/description.md +++ b/context/skills/integration-v2/capture/description.md @@ -37,15 +37,27 @@ and it is the source the report reads later. ## Instrument For each event call the SDK's capture method on the real user action — the click -or submit handler, the server action — not on render or page load. Use clear -`lower_snake_case` names and useful properties. Edit each file while it is already -open. +or submit handler, the server action — not on render or page load. Where that +action waits on a server, capture the outcome rather than the attempt: the call +belongs in the branch that runs after the awaited response confirms success, so a +submission the server rejects never counts as one that worked. Capture the attempt +as well where the drop-off between the two is worth measuring, under its own +name — one name must never mean both. Use clear `lower_snake_case` names and +useful properties. Edit each file while it is already open. Server-side, use the authenticated user's id as the distinct id. For a genuinely unauthenticated action, emit a personless event — never fabricate a placeholder id like `'anonymous'`, which collapses every anonymous user into one person and corrupts the data. +Backend SDKs process a person profile on every capture that carries a distinct id, +and the runtime metadata they attach — `$os`, `$lib`, and the rest — overwrites +whatever the browser set on that same person, so an event captured from a Linux +host rewrites a macOS user's profile. Pass `$process_person_profile: false` on +server-side business events that are not meant to update person properties, which +is most of them. Leave it off only where the capture deliberately updates the +person, paired with an `identify()` or `$set` in the same flow. + Leave `.posthog-wizard-cache/.posthog-events.json` in place for the report. ## Reference diff --git a/context/skills/posthog-best-practices/references/product-analytics.md b/context/skills/posthog-best-practices/references/product-analytics.md index 7c895728..5bd05a44 100644 --- a/context/skills/posthog-best-practices/references/product-analytics.md +++ b/context/skills/posthog-best-practices/references/product-analytics.md @@ -8,12 +8,14 @@ Do not read this page unless a rule below is violated and you need further expli - `suggestion`: Track **growth events first** using `posthog.capture()`: signups, activations, purchases, subscriptions, invites, core feature adoption. Do not rely on autocapture for key business milestones. - `warning`: If signup or activation is missing, add that before lower-value clicks or page interactions. +- `error`: Do not capture a success or completion event before the server confirms it. Put the call in the branch that runs after the awaited response resolves, so a submission the server rejects is never counted as a success. Where the attempt is worth measuring too, capture it as a separate, distinctly named event. - `suggestion`: Keep names **static, lowercase, and consistent**. Prefer present-tense verbs and snake case. - `suggestion`: Prefer `category:object_action` for events, e.g. `signup_flow:pricing_page_view`. - `suggestion`: Prefer descriptive, bounded properties like `signup_method`, `plan_name`, `is_test_user`, `last_login_timestamp`. - `error`: Never generate event names or property keys dynamically. Put variable data in property values. - `error`: Use one stable `distinct_id` across frontend and backend. Do not collapse user-scoped server events onto IDs like `system` or `backend`. - `error`: If an event is truly anonymous or system-level, disable person processing instead of sharing one identifier. Consult [Capture anonymous events / disable person processing](https://posthog.com/docs/product-analytics/capture-events#how-to-capture-anonymous-events) for the exact SDK pattern. +- `warning`: Backend SDKs process a person profile on every capture that carries a `distinct_id`, and the runtime metadata they attach (`$os`, `$lib`) overwrites what the browser set on that person. Pass `$process_person_profile: false` on server-side business events that are not deliberately updating person properties. - `warning`: Capture critical business events on the backend when accuracy matters. Use frontend tracking for journeys and UI interactions where some loss is acceptable. - `warning`: Do not duplicate the same milestone in frontend and backend unless each event serves a distinct analytical purpose. - `warning`: Do not assume cross-client event ordering. Use timestamps for analysis, not ingestion order. From c484db06402402d8b587d1c57eb09bf2abf8b90b Mon Sep 17 00:00:00 2001 From: sarahxsanders Date: Fri, 4 Sep 2026 11:32:27 -0400 Subject: [PATCH 2/4] fix(audit-events): seed the ledger, and detect events that fire on intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the same skill. The ledger was never seeded. audit-events' overview claimed the wizard pre-seeded it, but only `audit all` gets that treatment — `wizard audit events` resolves through the generic skill program, which seeds nothing. With no ledger, audit_resolve_checks rejects every id as unknown, so no check in the skill could report anything. Step 1 now seeds it itself, the way audit-attribution already does, and the overview says so instead of the opposite. Adds capture-fires-on-success to audit-events and to the comprehensive audit. It flags completion-named events that fire before the awaited response resolves, or outside the success branch, and names the branch the call belongs in. This is the detection side of the capture-timing rule the integration skills now write against — it matters because fixing the write-time prompt only helps future runs, and anyone who already ran the wizard has no other way to find the over-reporting sitting in their data. Scoped to completion-named events on purpose. A blanket "not in a success branch" rule would flag every legitimate intent event and make the check noise. Generated-By: PostHog Desktop Task-Id: 4fd61d6e-eb4e-498e-abfd-1d0b60d95a63 --- context/skills/audit-events/description.md | 3 +- .../audit-events/references/1-presence.md | 29 +++++++++- .../audit-events/references/2-events-fix.md | 53 +++++++++++++++++-- .../audit/references/4-event-capture.md | 29 ++++++++-- 4 files changed, 103 insertions(+), 11 deletions(-) diff --git a/context/skills/audit-events/description.md b/context/skills/audit-events/description.md index 25731cd7..ff6c0ca7 100644 --- a/context/skills/audit-events/description.md +++ b/context/skills/audit-events/description.md @@ -11,7 +11,7 @@ The audit covers two lenses: The audit runs as a step chain. **The exact step list lives in the reference files themselves, not in this overview.** Step 1 lives at `references/1-presence.md`; each step file ends with a `next_step:` frontmatter pointer to the next, and the final step has `next_step: null`. Follow them in the order they point. You must resolve each step in order before any source-tree exploration. -The audit ledger is seeded by the wizard with one pending check per event check. **Each step gracefully handles a missing check id**: if a step's expected id is not in the ledger, it skips its `audit_resolve_checks` call for that id and continues. Use `mcp__wizard-tools__audit_resolve_checks` to patch each check as you finish it. +The audit ledger is seeded by Step 1, not by the wizard — `wizard audit events` runs through the generic skill program, which pre-seeds nothing. Step 1 calls `mcp__wizard-tools__audit_seed_checks` with one pending entry per check; skip it and every later `audit_resolve_checks` call is rejected as an unknown id. **Each later step still handles a missing check id gracefully**: if a step's expected id is not in the ledger, it skips its `audit_resolve_checks` call for that id and continues. Use `mcp__wizard-tools__audit_resolve_checks` to patch each check as you finish it. **Start by reading the path relative to this file at `references/1-presence.md`.** Do not Glob, ls, or find the skill directory. Do not preload future steps. Do not re-read a step file once you've moved past it. Do not re-read SKILL.md. @@ -33,6 +33,7 @@ The wizard intercepts these and updates the spinner. Use them freely — they ar The ledger lives at `.posthog-audit-checks.json` and is rendered live in the "Audit plan" tab. It is owned by MCP tools — **never `Write` this file directly**: +- `mcp__wizard-tools__audit_seed_checks({ checks })` — create the ledger. Called once, by Step 1, with the exact payload that step lists. - `mcp__wizard-tools__audit_resolve_checks({ updates })` — patch one or more checks by `id`. Each `update` is `{ id, status, file?, details? }`. Batch updates from the same step into a single call. All audit ledger calls are atomic and serialize internally — **concurrent calls from parallel subagents cannot lose updates**, so feel free to fan out runtime checks across `Agent` subagents when a step says so. diff --git a/context/skills/audit-events/references/1-presence.md b/context/skills/audit-events/references/1-presence.md index 0447db17..e5311d97 100644 --- a/context/skills/audit-events/references/1-presence.md +++ b/context/skills/audit-events/references/1-presence.md @@ -4,7 +4,7 @@ next_step: 2-events-fix.md # Step 1 — Presence detector -This step decides whether the rest of the audit has anything to look at. Run it **before** any other work. Resolve zero ledger checks here — this step is gating only. +This step decides whether the rest of the audit has anything to look at, then seeds the ledger the later steps resolve against. Run it **before** any other work. Resolve zero ledger checks here — this step gates and seeds only. ## Status @@ -14,6 +14,10 @@ Emit: [STATUS] Detecting PostHog event capture usage ``` +## Load the tools + +Load via `ToolSearch select:Grep,mcp__wizard-tools__audit_seed_checks,mcp__wizard-tools__audit_resolve_checks` once at the start of this step. Later steps reuse `audit_resolve_checks` to patch each check as it resolves, so it stays loaded. + ## Action Run **two `Grep` calls in parallel**, both with `output_mode: "files_with_matches"`: @@ -24,9 +28,30 @@ Run **two `Grep` calls in parallel**, both with `output_mode: "files_with_matche ## Decision - **Both greps return zero hits anywhere in the project:** emit `[ABORT] PostHog SDK initialization not found` and stop. The wizard catches `[ABORT]` and terminates the run. -- **Init found, capture not found:** continue. Step 2 (fix) will detect this and resolve its four ledger checks with skip details. Step 3 (optimize) still has work to do because pageview defaults and downstream usage may still matter. +- **Init found, capture not found:** continue. Step 2 (fix) will detect this and resolve its five ledger checks with skip details. Step 3 (optimize) still has work to do because pageview defaults and downstream usage may still matter. - **Both found:** continue normally. +## Seed the audit ledger + +Do this only on the continue paths above — never after an `[ABORT]`. + +The ledger lives at `.posthog-audit-checks.json` and renders live in the wizard sidebar / "Audit plan" tab. **The runtime does not pre-seed this skill's ledger** — `wizard audit events` runs through the generic skill program, which seeds nothing, so `audit_resolve_checks` rejects every id until the ledger exists. Call `mcp__wizard-tools__audit_seed_checks` directly here with the exact payload below. The tool replaces the file atomically, so calling it once at the start of every run is safe. + +```json +{ + "checks": [ + { "id": "capture-event-names-static", "area": "Event Capture", "label": "Event names are static string literals", "status": "pending" }, + { "id": "event-naming-standardization", "area": "Event Capture", "label": "Event names follow one consistent convention", "status": "pending" }, + { "id": "event-duplicates-and-bloat", "area": "Event Capture", "label": "No duplicate or kitchen-sink events", "status": "pending" }, + { "id": "event-quality-context-review", "area": "Event Capture", "label": "Capture calls are free of PII and hot-path issues", "status": "pending" }, + { "id": "capture-fires-on-success", "area": "Event Capture", "label": "Completion events fire on success, not on intent", "status": "pending" }, + { "id": "event-usage-coverage", "area": "Event Capture — Optimize", "label": "Captured events are used downstream", "status": "pending" }, + { "id": "events-pageview-defaults", "area": "Event Capture — Optimize", "label": "Pageview / pageleave defaults are sized right", "status": "pending" }, + { "id": "events-env-pollution", "area": "Event Capture — Optimize", "label": "Dev / staging events are not leaking into production", "status": "pending" } + ] +} +``` + Do not read any files in this step. Do not call `audit_resolve_checks`. Do not preload future steps. Continue to **`2-events-fix.md`**. diff --git a/context/skills/audit-events/references/2-events-fix.md b/context/skills/audit-events/references/2-events-fix.md index 44ea0d66..a3ba30cc 100644 --- a/context/skills/audit-events/references/2-events-fix.md +++ b/context/skills/audit-events/references/2-events-fix.md @@ -4,16 +4,17 @@ next_step: 3-events-optimize.md # Step 2 — Event capture (fix) -This step resolves four event-capture quality checks **in parallel**, one subagent per check. These are the same ids the broader PostHog audit seeds — `audit-events` reuses them so a fix once made here is observable from either entry point. +This step resolves five event-capture quality checks **in parallel**, one subagent per check. These are the same ids the broader PostHog audit seeds — `audit-events` reuses them so a fix once made here is observable from either entry point. - `capture-event-names-static` - `event-naming-standardization` - `event-duplicates-and-bloat` - `event-quality-context-review` +- `capture-fires-on-success` ## Skip case — no `posthog.capture` calls found -If Step 1's capture grep returned **zero** hits, resolve all four checks in a single `audit_resolve_checks` call with `status: "pass"` and `details: "skip: no posthog.capture call sites detected"`. Then continue to **`3-events-optimize.md`**. Do not dispatch subagents. +If Step 1's capture grep returned **zero** hits, resolve all five checks in a single `audit_resolve_checks` call with `status: "pass"` and `details: "skip: no posthog.capture call sites detected"`. Then continue to **`3-events-optimize.md`**. Do not dispatch subagents. ## Status @@ -23,9 +24,9 @@ Emit before dispatching: [STATUS] Auditing event capture quality ``` -## Action — dispatch four subagents in one message +## Action — dispatch five subagents in one message -Make **four `Agent` tool calls in a single message** so they run concurrently. Wait for all four to return, then continue to `3-events-optimize.md`. Do not run any other tools between dispatch and the next step. +Make **five `Agent` tool calls in a single message** so they run concurrently. Wait for all five to return, then continue to `3-events-optimize.md`. Do not run any other tools between dispatch and the next step. The bundled `best-practices.md` reference holds PostHog's authoritative guidance on event-name shape, naming consistency, duplication, and event-quality patterns. It's typically at `.claude/skills/audit-events/references/best-practices.md`; if that path doesn't exist, discover it with `Glob` `**/skills/audit-events/references/best-practices.md`. Each subagent reads it once before judging. @@ -175,6 +176,48 @@ Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for Return when the call completes. Do not write the audit report. ``` -## After all four return +### Task E — `capture-fires-on-success` + +`description`: `Audit capture-fires-on-success` + +`prompt`: +``` +You are an audit subagent. Resolve exactly one rule and return: capture-fires-on-success. + +Background: a capture call placed at the top of a submit handler fires on the user's *intent*, before the server has accepted anything. A form that fails server-side validation then counts as a success, so the event over-reports the thing it is supposed to measure and inflates the event bill. The correct placement for a completion event is the branch that runs after the awaited call resolves successfully — after `if (response.ok)`, inside the `.then()`, after the `await` that would have thrown. + +Run **one** Grep: `posthog\.capture\(`. Read each file that contains a hit, once. + +Consider only captures whose event name reads as a **completed outcome** — names ending in or containing `_saved`, `_created`, `_updated`, `_deleted`, `_completed`, `_purchased`, `_submitted`, `_signed_up`, `_subscribed`, `_upgraded`, `_paid`, `_sent`, `_published`, or an equivalent past-tense completion in this project's own naming convention. Ignore intent-stage names (`_clicked`, `_started`, `_viewed`, `_opened`, `_attempted`) — those are *supposed* to fire on the action itself, and flagging them is a false positive. + +For each completion-named capture, determine whether it sits inside an `async` function, a promise chain, or a callback that receives a server result. If it does not (pure client-side state change, no network call), it passes — there is no outcome to wait for. + +Where it does, decide whether the capture executes: +- AFTER the awaited call resolved AND inside the branch taken on success (after an `if (response.ok)` / `if (!error)` guard, inside `.then()`, or after an `await` that would throw on failure and is not wrapped in a `try` that swallows the error) — correct. +- BEFORE the `await`, or after it but OUTSIDE any success guard so it also runs on a rejected/failed response (for example after a `try/catch` that swallows, or before `if (!response.ok) return`) — a violation. + +Rule: +- pass: no completion-named captures wait on a server, OR every one of them fires only on the success path. +- suggestion: 1–2 completion-named captures fire before the awaited response resolves or outside the success branch. +- warning: 3+ such captures, OR any one of them on a payment, checkout, subscription, or billing path — those inflate the metrics the business is steered by. + +Report the fix concretely: name the guard or branch the call should move inside, at that `path:line`. Do not rewrite any code — this is an audit. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-fires-on-success`, with `file` set to the most material violation's path:line if any, and `details` as compact JSON: + +``` +{ + "completion_event_count": , + "fires_on_intent_count": , + "examples": [ + {"event": "", "file": "", "issue": "before-await | outside-success-branch", "fix": ""} + ] +} +``` + +Return when the call completes. Do not write the audit report. +``` + +## After all five return Continue to **`3-events-optimize.md`**. Do not write the report yet — that's Step 4's job after Step 3 finishes. diff --git a/context/skills/audit/references/4-event-capture.md b/context/skills/audit/references/4-event-capture.md index dce21142..e018dc65 100644 --- a/context/skills/audit/references/4-event-capture.md +++ b/context/skills/audit/references/4-event-capture.md @@ -4,11 +4,12 @@ next_step: 5-live-data.md # Step 4 — Event capture -This step resolves three event-capture checks **in parallel**, one subagent per check: +This step resolves four event-capture checks **in parallel**, one subagent per check: - `capture-event-names-static` - `capture-uses-proxy` - `capture-growth-events` +- `capture-fires-on-success` Each subagent owns its own grep, reads, evaluates its single rule, and emits one `audit_resolve_checks` call with one update. The ledger's mutex serializes concurrent writes. @@ -20,9 +21,9 @@ Emit before dispatching: [STATUS] Auditing event capture ``` -## Action — dispatch three subagents in one message +## Action — dispatch four subagents in one message -Make **three `Agent` tool calls in a single message** so they run concurrently. Wait for all three to return, then continue to `5-live-data.md`. Do not run any other tools between dispatch and the next step. +Make **four `Agent` tool calls in a single message** so they run concurrently. Wait for all four to return, then continue to `5-live-data.md`. Do not run any other tools between dispatch and the next step. The bundled `best-practices.md` reference holds PostHog's authoritative guidance on event-name shape, reverse-proxy setup, and growth-event coverage. It's typically at `.claude/skills/audit/references/best-practices.md`; if that path doesn't exist, discover it with `Glob` `**/skills/audit/references/best-practices.md`. Each subagent reads it once before judging. @@ -91,3 +92,25 @@ Rule: Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-growth-events`, including `file` (path:line of the most relevant capture or growth-surface site) and `details` (one-line explanation, listing missing growth events when applicable). Return when the call completes. Do not write the audit report. ``` + +### Task D — `capture-fires-on-success` + +`description`: `Audit capture-fires-on-success` + +`prompt`: +``` +You are an audit subagent. Resolve exactly one rule and return: capture-fires-on-success. + +Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit/references/best-practices.md`). + +Run **one** Grep: `posthog\.capture\(`. Read each file that contains a hit, once. + +Consider only captures whose event name reads as a completed outcome — `_saved`, `_created`, `_updated`, `_deleted`, `_completed`, `_purchased`, `_submitted`, `_signed_up`, `_subscribed`, `_upgraded`, `_paid`, `_published`, or the project's own past-tense equivalent. Ignore intent-stage names (`_clicked`, `_started`, `_viewed`, `_opened`) — those are supposed to fire on the action itself, and flagging them is a false positive. + +Rule: +- A completion event must fire only after the server confirms the action: inside the success branch, after the awaited call resolves. Fired at the top of a submit handler instead, a submission the server rejects still counts as a success, which over-reports the metric and inflates the event bill. +- pass: no completion-named capture waits on a server, OR every one of them sits in a success branch (after `if (response.ok)` / `if (!error)`, inside `.then()`, or after an `await` that throws on failure and is not swallowed by a `try`). +- warning: any completion-named capture runs before its `await`, or outside the success guard so it also runs on a failed response. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-fires-on-success`, including `file` (path:line of the most material violation, otherwise a representative completion capture) and `details` (one-line explanation naming the branch the call belongs in). Return when the call completes. Do not write the audit report. +``` From 6f0c6fb271c436f15b2b4db6f658ac97691dd704 Mon Sep 17 00:00:00 2001 From: sarahxsanders Date: Fri, 4 Sep 2026 11:48:11 -0400 Subject: [PATCH 3/4] fix(integration): give Server Action forms a legal place to capture from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The success-branch rule assumed there is always an awaited response to branch on. With a Server Action driven by useActionState, a form action prop, or a mutation hook, the client never gets one — the outcome arrives as state. The existing React rule forbids reacting to state in an effect, so between the two rules the agent had no legal placement left, in the framework the wizard is used with most. Capture belongs inside the action itself, on the server, right after the mutation succeeds: that is where the outcome is known, and it needs no client state. Staying in the browser is the fallback, and then firing off the returned success state is correct rather than the effect anti-pattern — called out explicitly so the two rules stop contradicting each other. Splits the React commandment back into two entries instead of one overloaded line, so the original useEffect rule reads unchanged and the timing rule can carry its own exception. The audit checks pick up the same shape. A capture in the submit path of an action-driven form has no await beside it to notice, so it would otherwise slip past a reviewer looking only for misplaced awaits. Generated-By: PostHog Desktop Task-Id: 4fd61d6e-eb4e-498e-abfd-1d0b60d95a63 --- context/commandments.yaml | 3 ++- context/skills/audit-events/references/2-events-fix.md | 2 ++ context/skills/audit/references/4-event-capture.md | 2 ++ context/skills/integration-v2/capture/description.md | 10 ++++++++++ 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/context/commandments.yaml b/context/commandments.yaml index e59f7d55..7c4580b7 100644 --- a/context/commandments.yaml +++ b/context/commandments.yaml @@ -17,7 +17,8 @@ commandments: react: # PostHog-specific - For feature flags, use useFeatureFlagEnabled() or useFeatureFlagPayload() hooks - they handle loading states and external sync automatically - - Add analytics capture in event handlers where user actions occur, NOT in useEffect reacting to state changes - and when the handler awaits a server call, capture in the success branch after it resolves, never at the top of the handler where a rejected request still counts as a success + - Add analytics capture in event handlers where user actions occur, NOT in useEffect reacting to state changes + - When the handler awaits a server call, capture in the success branch after it resolves, never at the top of the handler - a request the server rejects must not count as a success. When the outcome only ever arrives as state (useActionState, a mutation hook's isSuccess), capture inside the server action instead, or off that success state - the one legitimate exception to the useEffect rule above. # General React patterns (react.dev/learn/you-might-not-need-an-effect) - Do NOT use useEffect for data transformation - calculate derived values during render instead - Do NOT use useEffect to respond to user events - put that logic in the event handler itself diff --git a/context/skills/audit-events/references/2-events-fix.md b/context/skills/audit-events/references/2-events-fix.md index a3ba30cc..234aa25a 100644 --- a/context/skills/audit-events/references/2-events-fix.md +++ b/context/skills/audit-events/references/2-events-fix.md @@ -192,6 +192,8 @@ Consider only captures whose event name reads as a **completed outcome** — nam For each completion-named capture, determine whether it sits inside an `async` function, a promise chain, or a callback that receives a server result. If it does not (pure client-side state change, no network call), it passes — there is no outcome to wait for. +Watch for the case that has no awaited response at all: a component whose submission runs through a Next.js Server Action, `useActionState`, a form `action` prop, or a mutation hook, with the capture sitting in the submit path rather than gated on the returned success state. That is the same defect and it is easy to miss, because there is no `await` next to the capture to notice. The fix to report is to move the capture into the action itself, on the server, after the mutation succeeds — or, where it must stay in the browser, to fire it off the returned success state. + Where it does, decide whether the capture executes: - AFTER the awaited call resolved AND inside the branch taken on success (after an `if (response.ok)` / `if (!error)` guard, inside `.then()`, or after an `await` that would throw on failure and is not wrapped in a `try` that swallows the error) — correct. - BEFORE the `await`, or after it but OUTSIDE any success guard so it also runs on a rejected/failed response (for example after a `try/catch` that swallows, or before `if (!response.ok) return`) — a violation. diff --git a/context/skills/audit/references/4-event-capture.md b/context/skills/audit/references/4-event-capture.md index e018dc65..726f7c33 100644 --- a/context/skills/audit/references/4-event-capture.md +++ b/context/skills/audit/references/4-event-capture.md @@ -112,5 +112,7 @@ Rule: - pass: no completion-named capture waits on a server, OR every one of them sits in a success branch (after `if (response.ok)` / `if (!error)`, inside `.then()`, or after an `await` that throws on failure and is not swallowed by a `try`). - warning: any completion-named capture runs before its `await`, or outside the success guard so it also runs on a failed response. +Include the case with no `await` beside the capture at all: a submission running through a Next.js Server Action, `useActionState`, a form `action` prop, or a mutation hook, with the capture in the submit path rather than gated on the returned success state. Same defect, easier to miss. The fix is to capture inside the action on the server after the mutation succeeds, or off the returned success state. + Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `capture-fires-on-success`, including `file` (path:line of the most material violation, otherwise a representative completion capture) and `details` (one-line explanation naming the branch the call belongs in). Return when the call completes. Do not write the audit report. ``` diff --git a/context/skills/integration-v2/capture/description.md b/context/skills/integration-v2/capture/description.md index 0010a8bf..16acd1fd 100644 --- a/context/skills/integration-v2/capture/description.md +++ b/context/skills/integration-v2/capture/description.md @@ -45,6 +45,16 @@ as well where the drop-off between the two is worth measuring, under its own name — one name must never mean both. Use clear `lower_snake_case` names and useful properties. Edit each file while it is already open. +Some frameworks never hand the client a response to branch on — a Next.js Server +Action driven by `useActionState`, a form action, a mutation hook that only +exposes `isSuccess`. Do not settle for firing on submit there. Put the capture +inside the action itself, on the server, immediately after the mutation succeeds: +that is where the outcome is actually known, and it needs no client state at all. +Keep it in the browser only when the action cannot take the server SDK, and then +fire it off the returned success state — reacting to state is the correct answer +in this one case, not the effect-driven anti-pattern the framework rules warn +about. + Server-side, use the authenticated user's id as the distinct id. For a genuinely unauthenticated action, emit a personless event — never fabricate a placeholder id like `'anonymous'`, which collapses every anonymous user into one person and From 94cf96487358ef9a99a69d94361420f3bda1bcab Mon Sep 17 00:00:00 2001 From: sarahxsanders Date: Fri, 4 Sep 2026 16:52:17 -0400 Subject: [PATCH 4/4] fix(integration): stop the person-processing flag from silencing signup and billing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live run against a Django SaaS app put `$process_person_profile: false` on all 13 server captures, including user_registered, subscription_started, subscription_plan_changed and subscription_canceled, with no identify() call anywhere. The project would create no person profiles at all — no cohorts, no person-property filters, no lifecycle insights, no funnel from first visit to paying. Cheaper events, and nothing left to analyse. The wording invited it. "Server-side business events that are not meant to update person properties, which is most of them" reads as a default, and "business event" covers a subscription as readily as a cron job. The rule now splits on what the event is for. Events that only record that something happened — a row written, a job run, a webhook received — take the flag. Events that decide who the person is or what they are worth do not, and are paired with the identify()/$set that records the change. Blanketing every capture is called out as its own mistake. audit-identify graded the same way in one direction only: it penalised a missing flag and never a flag on everything, and its own worked example was subscription_upgraded — an event that should reach the person. That check now reads both ways. Generated-By: PostHog Desktop Task-Id: 4fd61d6e-eb4e-498e-abfd-1d0b60d95a63 --- context/commandments.yaml | 2 +- .../audit-identify/references/5-server-sdk.md | 13 ++++++++----- .../skills/integration-v2/capture/description.md | 13 ++++++++++--- .../references/product-analytics.md | 3 ++- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/context/commandments.yaml b/context/commandments.yaml index 7c4580b7..d457df04 100644 --- a/context/commandments.yaml +++ b/context/commandments.yaml @@ -62,7 +62,7 @@ commandments: - posthog-node is the Node.js server-side SDK package name; posthog-js is browser-only, so use posthog-node on the server instead - 'Include enableExceptionAutocapture: true in the PostHog constructor options' - Add posthog.capture() calls in route handlers for meaningful user actions – every route that creates, updates, or deletes data should track an event with contextual properties, captured after the write succeeds rather than on entry to the handler - - 'Backend SDKs process a person profile on every capture carrying a distinct id, and the runtime metadata they attach ($os, $lib) overwrites what the browser set on that person. Pass `$process_person_profile: false` in the properties of server-side business events that are not deliberately updating person properties.' + - 'Backend SDKs process a person profile on every capture carrying a distinct id, and the runtime metadata they attach ($os, $lib) overwrites what the browser set on that person. Pass `$process_person_profile: false` on server events that only record that something happened - a row written, a job run, a webhook received. NOT on signup, subscription, plan-change or churn events: those must reach the person, paired with identify()/$set. Never put the flag on every capture - that leaves the project with no person profiles at all.' - Add posthog.captureException(err, distinctId) in the application's error handler (e.g., Express error middleware, Fastify setErrorHandler, Koa app.on('error')) - The SDK batches events and flushes asynchronously. await flush() or await shutdown() before letting that process exit. If unsure, set flushAt 1 and flushInterval 0. - '`posthog.capture()` enqueues synchronously and returns; the batched HTTP send happens afterwards. Treat every per-request handler as short-lived even when the framework feels like a server: Next.js / Nuxt / SvelteKit / Remix route handlers, serverless and edge functions, and Lambda are torn down per invocation before the send runs. Create the client with flushAt 1 and flushInterval 0, then await the send before returning. Always use `await posthog.flush()` for a shared/singleton client, `await posthog.shutdown()` for a per-request client. Never skip the awaited flush or risk the enqueued event being silently dropped.' diff --git a/context/skills/audit-identify/references/5-server-sdk.md b/context/skills/audit-identify/references/5-server-sdk.md index aacd1117..1f4b49f8 100644 --- a/context/skills/audit-identify/references/5-server-sdk.md +++ b/context/skills/audit-identify/references/5-server-sdk.md @@ -47,15 +47,18 @@ Run **two** Greps in parallel: - `\$process_person_profile|process_person_profile|processPersonProfile` — explicit usage of the property anywhere. Read each file that contains a server-side `capture(` hit, once. For each server-side capture, determine whether: -- The capture is intended to update person properties (it should follow a corresponding `identify()` and pass `$set` / `$set_once`), OR -- The capture is a transactional/business event that shouldn't touch person properties (most cases — `subscription_upgraded`, `webhook_received`, `cron_job_completed`). +- The capture defines the person or their account state — signup/registration, subscription started, plan changed, churn, or anything that should follow a corresponding `identify()` and pass `$set` / `$set_once`. These are **supposed** to reach the person, OR +- The capture only records that something happened — a row written, a job run, a webhook received (`webhook_received`, `cron_job_completed`). For the second category, check whether `$process_person_profile: false` is set in the properties object. +This check reads in **both** directions. A missing flag corrupts person properties; a flag on everything is worse, because the project then has no person profiles at all and loses cohorts, person-property filters, lifecycle insights, and its acquisition funnel. Judge each capture on which category it falls in, not on a global preference. + Rule: -- pass: every server-side capture either passes `$process_person_profile: false` OR is paired with an explicit identify()/$set in the same flow (intentional person-property update). -- suggestion: 1–3 server-side captures lack `$process_person_profile: false` and don't appear to update person properties intentionally — recommend adding the flag to prevent silent property corruption. -- warning: 4+ server-side captures or any high-frequency server-side capture (cron, webhook, polling loop) without `$process_person_profile: false` — high blast radius for property corruption. +- pass: every server-side capture that only records an activity passes `$process_person_profile: false`, AND every person-defining capture (signup, subscription, plan change, churn) does **not** — each paired with an explicit identify()/$set in the same flow. +- suggestion: 1–3 activity-only captures lack `$process_person_profile: false` and don't appear to update person properties intentionally — recommend adding the flag to prevent silent property corruption. +- warning: 4+ activity-only captures or any high-frequency server-side capture (cron, webhook, polling loop) without `$process_person_profile: false` — high blast radius for property corruption. +- warning: `$process_person_profile: false` is set on a person-defining event (signup, subscription started, plan changed, churn), or on **every** server capture in the project with no `identify()` anywhere — the project creates no person profiles at all. Name the events that should be let through. Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `server-process-person-profile`, including `file` (path:line of the most representative offending capture) and `details` as compact JSON: diff --git a/context/skills/integration-v2/capture/description.md b/context/skills/integration-v2/capture/description.md index 16acd1fd..4367b76e 100644 --- a/context/skills/integration-v2/capture/description.md +++ b/context/skills/integration-v2/capture/description.md @@ -64,9 +64,16 @@ Backend SDKs process a person profile on every capture that carries a distinct i and the runtime metadata they attach — `$os`, `$lib`, and the rest — overwrites whatever the browser set on that same person, so an event captured from a Linux host rewrites a macOS user's profile. Pass `$process_person_profile: false` on -server-side business events that are not meant to update person properties, which -is most of them. Leave it off only where the capture deliberately updates the -person, paired with an `identify()` or `$set` in the same flow. +server-side events that only record that something happened — a row was written, a +job ran, a webhook arrived. + +Not on the events that decide who the person is or what they are worth. Signup, +subscription started, plan changed, churn: those are supposed to reach the person, +and silencing them is worse than the metadata they overwrite, because it costs the +project cohorts, person-property filters, lifecycle insights, and the funnel from +first visit to paying. Let those through, and pair them with the `identify()` or +`$set` that records what changed. Never blanket the flag across every server +capture — a project with the flag everywhere has no person profiles at all. Leave `.posthog-wizard-cache/.posthog-events.json` in place for the report. diff --git a/context/skills/posthog-best-practices/references/product-analytics.md b/context/skills/posthog-best-practices/references/product-analytics.md index 5bd05a44..d2d49244 100644 --- a/context/skills/posthog-best-practices/references/product-analytics.md +++ b/context/skills/posthog-best-practices/references/product-analytics.md @@ -15,7 +15,8 @@ Do not read this page unless a rule below is violated and you need further expli - `error`: Never generate event names or property keys dynamically. Put variable data in property values. - `error`: Use one stable `distinct_id` across frontend and backend. Do not collapse user-scoped server events onto IDs like `system` or `backend`. - `error`: If an event is truly anonymous or system-level, disable person processing instead of sharing one identifier. Consult [Capture anonymous events / disable person processing](https://posthog.com/docs/product-analytics/capture-events#how-to-capture-anonymous-events) for the exact SDK pattern. -- `warning`: Backend SDKs process a person profile on every capture that carries a `distinct_id`, and the runtime metadata they attach (`$os`, `$lib`) overwrites what the browser set on that person. Pass `$process_person_profile: false` on server-side business events that are not deliberately updating person properties. +- `warning`: Backend SDKs process a person profile on every capture that carries a `distinct_id`, and the runtime metadata they attach (`$os`, `$lib`) overwrites what the browser set on that person. Pass `$process_person_profile: false` on server-side events that only record that something happened — a row written, a job run, a webhook received. +- `error`: Do not put `$process_person_profile: false` on the events that define the person or their account state — signup, subscription started, plan changed, churn. Those must reach the person, paired with `identify()` / `$set`. A project carrying the flag on every server capture has no person profiles at all, and loses cohorts, person-property filters, lifecycle insights, and its acquisition funnel. - `warning`: Capture critical business events on the backend when accuracy matters. Use frontend tracking for journeys and UI interactions where some loss is acceptable. - `warning`: Do not duplicate the same milestone in frontend and backend unless each event serves a distinct analytical purpose. - `warning`: Do not assume cross-client event ordering. Use timestamps for analysis, not ingestion order.