fix(compliance): add /get_feature_flag endpoint to the test adapter#542
fix(compliance): add /get_feature_flag endpoint to the test adapter#542turnipdabeets wants to merge 3 commits into
Conversation
The harness was getting a 404 on /get_feature_flag, failing all 16 feature-flag tests. Implements the endpoint (mirrors posthog-ios): set person/group properties, reload flags, resolve the value, flush. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Satisfies the semgrep package-manager rules: add a 7-day cooldown to the Dependabot github-actions ecosystem and minimumReleaseAge: 10080 to the pnpm workspace, so newly published packages wait before being adopted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
posthog-android Compliance ReportDate: 2026-05-29 16:40:51 UTC
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 2309ms |
| Format Validation.Event Has Uuid | ❌ | 2026ms |
| Format Validation.Event Has Lib Properties | ❌ | 2022ms |
| Format Validation.Distinct Id Is String | ❌ | 2019ms |
| Format Validation.Token Is Present | ✅ | 2021ms |
| Format Validation.Custom Properties Preserved | ❌ | 2018ms |
| Format Validation.Event Has Timestamp | ❌ | 2024ms |
| Retry Behavior.Retries On 503 | ✅ | 7022ms |
| Retry Behavior.Does Not Retry On 400 | ❌ | 4020ms |
| Retry Behavior.Does Not Retry On 401 | ❌ | 4023ms |
| Retry Behavior.Respects Retry After Header | ❌ | 7023ms |
| Retry Behavior.Implements Backoff | ❌ | 17033ms |
| Retry Behavior.Retries On 500 | ✅ | 7024ms |
| Retry Behavior.Retries On 502 | ✅ | 7022ms |
| Retry Behavior.Retries On 504 | ✅ | 7023ms |
| Retry Behavior.Max Retries Respected | ❌ | 17033ms |
| Deduplication.Generates Unique Uuids | ✅ | 2039ms |
| Deduplication.Preserves Uuid On Retry | ❌ | 7020ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ❌ | 12017ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ❌ | 7027ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 2033ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 2023ms |
| Compression.Sends Gzip When Enabled | ✅ | 2015ms |
| Batch Format.Uses Proper Batch Structure | ❌ | 2013ms |
| Batch Format.Flush With No Events Sends Nothing | ❌ | 2012ms |
| Batch Format.Multiple Events Batched Together | ❌ | 2024ms |
| Error Handling.Does Not Retry On 403 | ❌ | 4016ms |
| Error Handling.Does Not Retry On 413 | ❌ | 4017ms |
| Error Handling.Retries On 408 | ✅ | 7022ms |
Failures
format_validation.event_has_uuid
Event missing 'uuid' field
format_validation.event_has_lib_properties
Event missing '$lib' property. Available properties: []
format_validation.distinct_id_is_string
Expected distinct_id='test_user_123', got '019e7499-7bfd-7f80-b446-a21fb50df081'
format_validation.custom_properties_preserved
Expected custom_string='hello', got 'None'. Available properties: []
format_validation.event_has_timestamp
Event missing 'timestamp' field
retry_behavior.does_not_retry_on_400
Expected 1 requests, got 2
retry_behavior.does_not_retry_on_401
Expected 1 requests, got 2
retry_behavior.respects_retry_after_header
Retry delay too short: 11ms < 2500ms
retry_behavior.implements_backoff
First retry delay too short: 12ms < 100ms
retry_behavior.max_retries_respected
Expected 4 requests, got 5
deduplication.preserves_uuid_on_retry
UUIDs changed on retry: [] != ['019e749a-d135-736e-a2e2-6a83abf8b61c']
deduplication.preserves_uuid_and_timestamp_on_retry
UUIDs changed on retry: [] != ['019e749a-eca4-787d-9ce6-10d5a74e551d']
deduplication.preserves_uuid_and_timestamp_on_batch_retry
UUIDs changed on retry: [] != ['019e749b-1b94-758b-992c-1d136823e95b', '019e749b-1b98-7576-89f9-7333814e842f', '019e749b-1b9b-720d-90f7-b927a918abff']
batch_format.uses_proper_batch_structure
Batch format missing 'batch' array field
batch_format.flush_with_no_events_sends_nothing
Expected 0 requests, got 1
batch_format.multiple_events_batched_together
Expected 1 requests, got 2
error_handling.does_not_retry_on_403
Expected 1 requests, got 2
error_handling.does_not_retry_on_413
Expected 1 requests, got 2
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ❌ | 2022ms |
| Request Payload.Flags Request Uses V2 Query Param | ❌ | 2014ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ❌ | 2017ms |
| Request Payload.Flags Request Omits Authorization Header | ❌ | 2014ms |
| Request Payload.Token In Flags Body Matches Init | ❌ | 2014ms |
| Request Payload.Groups Round Trip | ❌ | 2018ms |
| Request Payload.Groups Default To Empty Object | ❌ | 2017ms |
| Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It | ❌ | 2014ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ❌ | 2015ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 2015ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ❌ | 2015ms |
| Request Lifecycle.No Flags Request On Init Alone | ❌ | 9ms |
| Request Lifecycle.No Flags Request On Normal Capture | ❌ | 2012ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ❌ | 4021ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ❌ | 2013ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ❌ | 4016ms |
Failures
request_payload.request_with_person_properties_device_id
Expected 1 /flags requests, got 2
request_payload.flags_request_uses_v2_query_param
Expected 1 /flags requests, got 2
request_payload.flags_request_hits_flags_path_not_decide
Expected 1 /flags requests, got 2
request_payload.flags_request_omits_authorization_header
Expected 1 /flags requests, got 2
request_payload.token_in_flags_body_matches_init
Expected token='phc_specific_key_12345', got 'phc_test_key'
request_payload.groups_round_trip
Field 'groups' not found in /flags request body at path 'groups.company'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.groups_default_to_empty_object
Field 'groups' not found in /flags request body at path 'groups'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.person_properties_distinct_id_auto_populated_when_caller_omits_it
Field 'distinct_id' not found in /flags request body at path 'person_properties.distinct_id'. Available keys: ['$lib', '$lib_version']
request_payload.disable_geoip_false_propagates_as_geoip_disable_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_payload.flag_keys_to_evaluate_contains_only_requested_key
Field 'flag_keys_to_evaluate' not found in /flags request body at path 'flag_keys_to_evaluate'. Available keys: ['$anon_distinct_id', '$device_id', 'api_key', 'distinct_id', 'timezone', 'person_properties']
request_lifecycle.no_flags_request_on_init_alone
Expected 0 /flags requests, got 1
request_lifecycle.no_flags_request_on_normal_capture
Expected 0 /flags requests, got 1
request_lifecycle.two_flag_calls_produce_two_remote_requests
Expected 2 /flags requests, got 3
request_lifecycle.mock_response_value_is_returned_to_caller
Last action result missing field 'value'. Keys: ['success']
side_effect_events.get_feature_flag_captures_feature_flag_called_event
Event '$feature_flag_called' property '$feature_flag_response': expected True, got ''
…n init Declare capabilities ["capture_v0", "encoding_gzip"] in /health so the harness runs the capture suites (android posts events to /batch and gzips them). Also set remoteConfig = false in /init so tests don't depend on the remote config load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
sdk_compliance_adapter/src/main/kotlin/com/posthog/compliance/ComplianceAdapter.kt:352-354
**`ph.group()` fires an extra `$groupidentify` event and may trigger a second `/flags` request**
`PostHog.group()` calls `groupStateless()` internally, which calls `captureStateless(GROUP_IDENTIFY, ...)`, queuing a `$groupidentify` event. More critically, because `PostHog.reloadFeatureFlags: Boolean = true` by default, it also calls `reloadFeatureFlags(config?.onFeatureFlags)` whenever the group key is new. Since `/reset` clears persisted groups between tests, every test run sees a fresh group — making `reloadFeatureFlagsIfNewGroup` true every time. This means two `/flags` requests get made (one from `group()`, one from the explicit `reloadFeatureFlags { latch.countDown() }` below), and one `$groupidentify` batch request is created before the latch is even started. If the harness asserts exact batch or `/flags` request counts for group flag tests, those tests will remain red despite this fix.
Reviews (1): Last reviewed commit: "feat(compliance): declare capture capabi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| req.groups?.forEach { (type, key) -> | ||
| ph.group(type, key, req.group_properties?.get(type)) | ||
| } |
There was a problem hiding this comment.
ph.group() fires an extra $groupidentify event and may trigger a second /flags request
PostHog.group() calls groupStateless() internally, which calls captureStateless(GROUP_IDENTIFY, ...), queuing a $groupidentify event. More critically, because PostHog.reloadFeatureFlags: Boolean = true by default, it also calls reloadFeatureFlags(config?.onFeatureFlags) whenever the group key is new. Since /reset clears persisted groups between tests, every test run sees a fresh group — making reloadFeatureFlagsIfNewGroup true every time. This means two /flags requests get made (one from group(), one from the explicit reloadFeatureFlags { latch.countDown() } below), and one $groupidentify batch request is created before the latch is even started. If the harness asserts exact batch or /flags request counts for group flag tests, those tests will remain red despite this fix.
Prompt To Fix With AI
This is a comment left during a code review.
Path: sdk_compliance_adapter/src/main/kotlin/com/posthog/compliance/ComplianceAdapter.kt
Line: 352-354
Comment:
**`ph.group()` fires an extra `$groupidentify` event and may trigger a second `/flags` request**
`PostHog.group()` calls `groupStateless()` internally, which calls `captureStateless(GROUP_IDENTIFY, ...)`, queuing a `$groupidentify` event. More critically, because `PostHog.reloadFeatureFlags: Boolean = true` by default, it also calls `reloadFeatureFlags(config?.onFeatureFlags)` whenever the group key is new. Since `/reset` clears persisted groups between tests, every test run sees a fresh group — making `reloadFeatureFlagsIfNewGroup` true every time. This means two `/flags` requests get made (one from `group()`, one from the explicit `reloadFeatureFlags { latch.countDown() }` below), and one `$groupidentify` batch request is created before the latch is even started. If the harness asserts exact batch or `/flags` request counts for group flag tests, those tests will remain red despite this fix.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Accurate, thanks. There's no way around this with the public android API: group() is the only way to put a group into the /flags body, and it unconditionally emits $groupidentify and reloads flags on a new group — there's no suppression parameter (unlike setPersonPropertiesForFlags(reloadFeatureFlags = false), which the adapter does use for person properties). So the extra /flags + $groupidentify are inherent to using the public API for groups. It only affects the group-bearing flag tests (groups_round_trip), which are already red because /init fires a /flags regardless on the SDK side (every flag count is +1), so suppressing this wouldn't flip the test. Leaving the adapter on the public group() API.
|
|
||
| // Flush that event and wait so it can't spill into the next test's mock window. | ||
| ph.flush() | ||
| Thread.sleep(2000) |
There was a problem hiding this comment.
do we need this? Is there a better approach, like using a latch, waiting for the flush, or something?
Sleep slows down tests
|
bsed on #542 (comment) that didnt work? |
2 participants
TL;DR: the compliance adapter was missing
/get_feature_flag→ harness 404s → 0/16 flag tests. This adds the endpoint (mirrors posthog-ios).Expect the universal flag tests to pass; the genuinely server-only ones (caller
distinct_id,disable_geoip,flag_keys) stay red by design.