Repository navigation
feat!: add option defaults; event values always win - #359
Conversation
3078e55 to
9707868
Compare
|
@greptileai review |
cd9063c to
3cf4f49
Compare
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
bad06b9 to
84df98b
Compare
cc10078 to
84395b0
Compare
c36473b to
487b20b
Compare
d0ac310 to
7e1e113
Compare
dustinbyrne
left a comment
There was a problem hiding this comment.
I reproduced the four inline event-composition cases against this head and stack tip #368, using real SDK clients and a local recording server. The inlines include executed results, passing controls, base comparisons and suggested regression coverage.
AI-assisted review.
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
dustinbyrne
left a comment
There was a problem hiding this comment.
The composition fixes and regression coverage look good. Two non-blocking comments inline: preserving the spec/peer-aligned tracing-header default, and correcting the request-option scope in the migration guide.
AI-assisted review.
$mcp_unknown_tool, $mcp_input_required and $mcp_missing_capability now carry the process_person_profile option and RequestContext.Options, like tool calls.
Request-context properties and options, Config.DefaultEventProperties, Config.DefaultEventOptions and the personless process_person_profile option now fill in after BeforeSend, only for keys the event and the hook left unset. An option counts as unset when it is missing or nil; a property only when its key is missing. The request-context distinct ID still resolves before the hook. DefaultEventProperties and DefaultEventOptions now also reach Exception, Identify, Alias and GroupIdentify events, as event properties and options, never in $set or $group_set. DisableGeoIP no longer adds $geoip_disable to DefaultEventProperties. Every message already sets it before BeforeSend, and as a default it would come back after a hook removed it. A default $geoip_disable is still forced to true.
Request-context properties and options, Config.DefaultEventProperties, Config.DefaultEventOptions and the personless process_person_profile option now fill before BeforeSend, still only into keys the event left unset. BeforeSend sees every value and has the final say, including removing a default. $set, $set_once, $groups and $group_set fill one level deep when both values are maps. Capture.Groups merges into a $groups map key by key, and Identify and GroupIdentify Properties merge over a $set or $group_set in the event properties. Identify, Alias and GroupIdentify gain an EventProperties field for the event's own properties, which the defaults fill before the hook. It replaces the unexported default field. Hoisting still runs once, after the hook.
$is_server, $geoip_disable, system context and SendFeatureFlags properties now fill only keys the event, request context and DefaultEventProperties left unset, before BeforeSend. The SDK fills them last, so a DefaultEventProperties $geoip_disable: false wins over Config.DisableGeoIP. posthogmcp merges the sanitized request context under the call for every client and enqueues with an empty request context, so the client does not fill the unsanitized values again.
billing_limit_exceeded is a request-level 402 code. capture never puts it in a per-event drop result; the per-event tags are exceptions_over_quota, survey_responses_over_quota and llm_events_over_quota.
Capture sets both from the PostHog-Sdk-Info header and appends them after the event's keys, so a copy left in properties is a duplicate JSON key.
…racing headers are opt-in Request-context properties and options now fill Identify, Alias and GroupIdentify like Capture and Exception: below the event's own values, above the client defaults. The context session ID fills Identify and Alias but never GroupIdentify, whose distinct ID is not the session's person. Context ids never change an Identify or Alias. NewRequestContextMiddleware no longer reads the X-PostHog-Distinct-Id and X-PostHog-Session-Id headers unless WithCaptureTracingHeaders(true) is set, because any client can send them. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…yed map - posthogmcp: a nil or typed-nil option no longer hides a legacy opt-out or erases a request-context option. - $set, $set_once, $groups and $group_set merge one level deep for any map with string keys, such as map[string]string. - A request-context $session_id set to nil now blocks a default session. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ons inherit Context and default properties now beat SDK-generated feature flag properties, matching the documented event > context > defaults > SDK order, and a nil child request-context option keeps the parent's value. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ab9768c to
c900f09
Compare
💡 Motivation and Context
In 1.x,
DefaultEventPropertiesoverwrote the event's own values, and there was no way to set options for every event or request.This PR makes defaults fill only what the event left unset, on every message type.
Changes
BeforeSendsees every filled value and has the final say.Config.DefaultEventOptionsandRequestContext.Options; aniloption counts as unset.Exception,Identify,AliasandGroupIdentify.Identify,AliasandGroupIdentifygain anEventPropertiesfield.GroupIdentify.NewRequestContextMiddlewarereads theX-PostHog-*headers only withWithCaptureTracingHeaders(true), because any client can set them.$set,$set_once,$groupsand$group_setmerge one level deep.process_person_profileoption that any layer can override.posthogmcpgainsToolCall.OptionsandEventContext.Optionsand takesRequestContext.Options.$libor$lib_versionis removed before send.💚 How did you test it?
BeforeSendview.go vet,go test -raceandmake api-diffpass in all three modules.🤖 Agent context
Autonomy: Human-driven (agent-assisted). Written with Claude Code.
🤖 Generated with Claude Code