fix(posthog): validate integration against live API docs, add self-hosted support + CRUD coverage#5476
Conversation
…sted support + CRUD coverage Fix missing response.ok checks across ~36 tools that silently treated error bodies as success. Fix delete_feature_flag ignoring failure responses, evaluate_flags targeting the undocumented /decide endpoint without the required api_key body field, and batch_events reporting a hardcoded events_processed count. Add self-hosted host support (utils.ts) alongside the existing US/EU region selector, and complete CRUD coverage with 5 new tools: update_insight, update_cohort, update_experiment, delete_survey, create_dashboard.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adds update insight/cohort/experiment, create dashboard, and delete survey operations, with matching block fields (e.g. favorited, pinned, archive cohort/experiment). Aligns several tools with documented PostHog behavior: feature flag delete is a soft-delete PATCH; person delete uses bulk_delete; evaluate flags calls Reviewed by Cursor Bugbot for commit 98d6feb. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR hardens the PostHog integration in three areas: fixes ~36 tools that were silently treating error responses as success data, migrates
Confidence Score: 5/5Safe to merge — the fixes are well-scoped and the endpoint migration is validated against live PostHog docs. The core changes (removing incorrect response.ok guards, the evaluate_flags endpoint migration, and the new CRUD tools) are all correct. The SSRF guard for self-hosted hosts is already in place via validateExternalUrl. The two findings are minor: a redundant JSON.parse in batch_events that cannot cause a runtime failure, and a silent-swallow in create_insight that predates this PR but was not corrected alongside its new peer update_insight. Neither affects the correctness of the new tools or the fixes. apps/sim/tools/posthog/create_insight.ts — silent JSON parse fallback to null is inconsistent with every other tool in this PR and could confuse users creating insights with malformed query JSON. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tool invoked] --> B{host param set?}
B -- Yes --> C[normalizeHost]
C --> D[validateExternalUrl / SSRF guard]
D -- Invalid --> E[throw Error]
D -- Valid --> F[Custom host URL]
B -- No --> G{region?}
G -- eu --> H[eu.posthog.com / eu.i.posthog.com]
G -- us/default --> I[us.posthog.com / us.i.posthog.com]
F --> J[HTTP request]
H --> J
I --> J
J --> K{Non-2xx?}
K -- Yes --> L[Framework throws before transformResponse]
K -- No --> M[transformResponse]
M --> N{Ingest tool? capture/batch/flags}
N -- Yes --> O[Check data.status === 1]
N -- No --> P[Parse API response JSON]
O --> Q[Return success/failure]
P --> Q
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Tool invoked] --> B{host param set?}
B -- Yes --> C[normalizeHost]
C --> D[validateExternalUrl / SSRF guard]
D -- Invalid --> E[throw Error]
D -- Valid --> F[Custom host URL]
B -- No --> G{region?}
G -- eu --> H[eu.posthog.com / eu.i.posthog.com]
G -- us/default --> I[us.posthog.com / us.i.posthog.com]
F --> J[HTTP request]
H --> J
I --> J
J --> K{Non-2xx?}
K -- Yes --> L[Framework throws before transformResponse]
K -- No --> M[transformResponse]
M --> N{Ingest tool? capture/batch/flags}
N -- Yes --> O[Check data.status === 1]
N -- No --> P[Parse API response JSON]
O --> Q[Return success/failure]
P --> Q
Reviews (5): Last reviewed commit: "fix(posthog): fix cross-field value leak..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…t bugs
The prior commit added if (!response.ok) branches inside transformResponse
across ~40 tool files. This is dead code — tools/index.ts already throws
on non-2xx (and error-payload) responses before transformResponse is ever
invoked, so transformResponse only ever receives already-successful
responses. Reverted to the idiomatic no-error-branch pattern used
elsewhere in the codebase (e.g. hunter/email_verifier.ts), and added a
posthog-errors errorExtractor so the framework's own error throw surfaces
PostHog's real {type, code, detail, attr} error shape instead of a
generic status message.
Verified against PostHog's live OpenAPI schema, fixed real bugs:
- delete_feature_flag: DELETE always returns 405 (hard delete not
allowed); switched to PATCH with deleted: true
- delete_person: no single-person DELETE endpoint exists; switched to
POST .../persons/bulk_delete/ with ids: [personId]
- create_annotation: insight_short_id is read-only on create; the
writable field is dashboard_id
- removed nonexistent fields (experiment variants, insight
filters/saved) not present in the current schema
- added missing trailing slashes on feature-flag/experiment URLs
The self-hosted host field accepted any string and only stripped trailing slashes before prepending https://, so a workflow could point it at loopback/private/link-local addresses (e.g. cloud instance- metadata endpoints) and the executor would make a real server-side request to it. Reuse the shared validateExternalUrl SSRF guard, same pattern already used for Convex's custom deployment URL (tools/convex/utils.ts); the tool executor separately re-validates with DNS resolution and pins the resolved IP for the actual request. Also drop an unused params arg in evaluate_flags' headers function.
|
@greptile review |
|
@cursor review |
…ON params
Several tools caught JSON.parse failures on user-supplied filter/query/
parameters strings and silently substituted {} or null, which would
wipe the corresponding field on the PATCH/create request instead of
surfacing an error (Cursor Bugbot flagged this for update_cohort,
update_experiment, update_insight; the same pattern existed in their
create_* / update_feature_flag / evaluate_flags counterparts, fixed for
consistency). Now throws a descriptive error, matching the existing
convention in tools/notion/query_database.ts.
Also fixes batch_events: the request body silently sent an empty batch
on invalid JSON, and transformResponse always reported "captured
successfully" even when PostHog's response indicated failure
(data.status !== 1).
|
@greptile review |
|
@cursor review |
posthog_update_experiment was the only experiment operation missing from the region field's required-condition list; the other three (create/get/list) already require it. Region is unconditionally visible on this block (no condition key gates it), so this was a required-ness inconsistency rather than a functional bug, but users updating an EU experiment should still be prompted to select the region explicitly rather than relying on the default.
|
Re: the region/update_experiment finding — clarifying and fixing. |
|
@greptile review |
|
@cursor review |
Merges update_insight's query subblock into the same insightQuery id
already used by create_insight (Cursor flagged: it previously reused
the 'query' subblock id shared by posthog_query's HogQL field and the
cohort query fields, so switching operations could carry a stale
HogQL/cohort-JSON value into an insight PATCH). Matches the existing
merged-condition pattern used elsewhere in this block instead of
duplicating the subblock id.
Also fixes two more false-success reports in the same class as the
batch_events/delete_feature_flag fixes: capture_event now checks the
ingest response's status field (same {"status": 1} contract as
/batch/) instead of unconditionally returning success, and
delete_person now reports failure when persons_deleted is 0 instead of
always success: true.
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 98d6feb. Configure here.
|
Review loop complete — Greptile 5/5, Cursor Bugbot clean, 0 unresolved threads, all on latest commit (98d6feb). Summary of what changed across 6 commits:
Every finding across all rounds was independently verified against PostHog's live docs/OpenAPI schema or the actual framework code before fixing — 2 bot claims were disproven as false positives with doc citations, the rest were real and fixed. |
1 participant
Summary
response.okchecks)delete_feature_flagreturningsuccess: trueregardless of actual API responseevaluate_flagstargeting the undocumented/decideendpoint and missing the requiredapi_keybody field; migrated to the documented/flags/?v=2endpointbatch_eventsreporting a fabricatedevents_processedcount instead of the actual batch sizeutils.ts) alongside the existing US/EU region selectorupdate_insight,update_cohort,update_experiment,delete_survey,create_dashboardType of Change
Testing
Validated every tool against PostHog's live API docs.
tsc --noEmitandbunx biome checkboth clean.Checklist