Repository navigation
Conversation
Add two protected hooks to PostHogCoreStateless: getFlagsResponseVersion(), which sets the `v` query parameter and returns 2, and readFlagsResponse(), which reads the body with the shared normalizeFlagsResponse as before. Clients that override neither, including the React Native and web SDKs, keep requesting v=2 and reading it exactly as before; tests pin both.
posthog-node and posthog-edge now request /flags/?v=4 and read each flag's typed value, reason and rule metadata. The legacy rendering every existing getter and event reads is derived from `value` in one place: a boolean as is, a string as the variant, null as false, and a number or object as true with the value as its payload. A record without `value` is an older-server record, read as before with config version 1; each client logs that once. A known field with the wrong JSON type makes only that record unreadable. It reads as a disabled flag, and a single-flag read of it reports what a failed /flags request reports: unknown_error and no request details. The parsed record is kept internally for the typed flag API: beside each legacy detail, and on each remotely evaluated flag of an evaluateFlags() snapshot. No public API, type or option is added. Tests vendor the reader, response, legacy projection and value corpora from posthog-sdk-test-harness 1.27.0 with a digest check, and pin every legacy getter and event to results recorded with posthog-node 5.61.2, from both response shapes. The compliance adapter declares flags_response_v4.
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
Coherence📄 0 spec file errors · ⛔ 0 broken rules · 🚪 2 unchecked inputs · 🐛 0 bugs that came back What is Coherence?Coherence compares the code with its spec files ( This check read 44 parts of the code and 7 rules. Note These findings do not make this check fail. None of these findings are in files that this pull request changes. You do not need to fix them in this pull request. 🚪 Unchecked inputs (2)
What this means and how to fix itInput from outside this code comes in at these places. The spec does not say what checks that input. 10 more places were already like this when Coherence was added. This list does not show them. To close one of these, find the place by its name in the
To get the line to add, run |
Contributor
Contributor
Node SDK acceptance v2Commit: ec7a277 Capture v0✅ 233 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:135 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1✅ 298 passed / ❌ 4 non-passing / Drill-down: run artifacts → ❌ server_snapshot_payload_false — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_zero — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ server_snapshot_payload_null — failed_assertionExpected native data result Code: Failed step: acceptance/public/evaluate-flags.feature:868 the public snapshot outcomes should have these semantics: ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:135 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
Contributor
|
Size Change: -3.67 kB (-0.01%) Total Size: 26.5 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
posthog-js Compliance ReportDate: 2026-11-09 09:52:49 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
7 of 24 tasks
The adapter declares the capability when the installed client requests /flags?v=4, read from the client's own request version. The shared scenarios then hold posthog-node to v=4 instead of v=2.
/get_feature_flag_payload takes the /get_feature_flag arguments and calls getFeatureFlagPayload with matchValue unset, so the SDK evaluates the flag. The adapter declares feature_flag_payloads: the payload getter returns the decoded JSON payload. With flags_response_v4, the shared Gherkin scenarios then check the legacy getter and payload getter results for each /flags?v=4 contract fixture.
Contributor
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
Move the v2 compliance job to harness 1.29.0, which runs the Gherkin /flags?v=4 read suite and replaces the v2 query scenario with its v4 counterpart for SDKs that declare flags_response_v4. Copy the feature flag rules v2 fixtures from harness 1.29.0 (contract 9.0.0). Reader cases now declare legacy_payload; check it through getFeatureFlagPayload and the snapshot's getFlagPayload. A failed v4 record reads as disabled, so its payload is now null, even when it carries metadata.payload. Before, a snapshot read returned that payload.
Contributor
SDK performance: posthog-nodeBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
6 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
The flags service can answer with a typed flag record (PostHog/posthog#107732), selected by
/flags?v=4(PostHog/posthog#123366). A v4 record carries one typedvalue(boolean, string, number, object or null) instead ofenabledandvariant, plus a structuredreasonand rule metadata.This PR moves posthog-node (and posthog-edge, which shares its client) to that response and keeps the parsed record for each flag, while every existing getter and event returns the same values as before for valid flag records. It adds no public methods, options, types or exports, and does not change event emission. The browser, React Native and lightweight web SDKs keep requesting
v=2. It ports the behaviour of PostHog/posthog-python#1031, which shipped in posthog 7.83.0.The reader follows the wire contract in posthog-sdk-test-harness 1.29.0 (
contracts/feature_flag_rules_v2, contract 9.0.0):/flags/?v=4. A record withvalueis a v4 record. A record without it comes from a server that does not serve v4: its value isvariant, orenabledwhen there is no variant, its config version is 1, and any rule context in it is ignored. There is no probing and no retry withv=2: the flags service answers avit does not serve with the older record rather than an error, so a fallback request would only repeat requests that failed for other reasons. Each client logs one diagnostic (at info level, so only withdebug) the first time a response has a record without a typed value.keymember of the wrong type is such a field; a stringkeythat differs from the map key is ignored, and the map key stays the flag key. An unreadable record reads as a disabled flag without reason or metadata. posthog-node has no cache of remote results, so there is no cached value to fall back to. A single-flag getter (getFeatureFlag,isFeatureEnabled,getFeatureFlagResult) reports what 5.61.2 reports when the/flagsrequest fails:$feature_flag_error: "unknown_error", with no request ID or evaluated-at.valuein one place, when the record is read: a boolean as is, a string as the variant,nullasfalse, and a number or object astruewith the value as its payload. v4 sendsmetadata.payload: nullfor number and object values, so the SDK supplies the payload from the value, and ignores a payload a record sends with a number or object value. Payloads for boolean, string and null values are kept as sent. A failed record reads as disabled, so its payload isnull, even when it carriesmetadata.payload.getFeatureFlag,isFeatureEnabled,getFeatureFlagPayload,getFeatureFlagResult,getAllFlags(AndPayloads), theevaluateFlags()snapshot getters,$feature/<key>,$active_feature_flagsand$feature_flag_calledreturn the same values from both responses.evaluateFlags()snapshot keeps its parsed record internally (typed value, failure state, reason, and metadata such as config version, rule type, rule ID and variant key), for the typed accessors and exposure events that follow.Notes for reviewers
The seam.
PostHogCoreStatelessgets two protected methods:getFlagsResponseVersion(), which sets thevquery parameter and returns2, andreadFlagsResponse(body), which returnsnormalizeFlagsResponse(body), exactly whatgetFlagsdid before.PostHogBackendClient(posthog-node and posthog-edge) overrides both. Neither is a user option. The shared reader,normalizeFlagsResponse, is unchanged: posthog-node first reads each record into its legacy rendering (src/flags-response.ts) and then hands the body to the shared reader, so the legacy maps come from the same code as before.Why other SDKs are untouched. The React Native and lightweight web SDKs extend
PostHogCoreand override neither method, so they keep sendingv=2and reading with the shared reader.posthog-jsand@posthog/browserdo not use core'sgetFlagsat all; they request and read/flagsin@posthog/browser-common, which this PR does not change. New core tests pin thev=2URL (with and withoutconfig=true) and that a v1, v2 or v4-shaped body reads exactly asnormalizeFlagsResponsereads it, and the existing React Native and web tests that assert/flags/?v=2still pass.Where the parsed record lives.
FeatureFlagDetail,PostHogFeatureFlagsResponseand the other public types are unchanged. The parsed record of each legacy rendering is kept in a module-levelWeakMapkeyed by that detail object, and the snapshot stores it on the internalEvaluatedFlagRecord(parsed). A record that is not read by the v4 reader, such as one from a v1 response, reads as a config version 1 record.Compatibility differences, beyond the reason text below:
reason, an empty-stringreasonor a non-booleanhas_experiment, now reads as described in point 2. 5.61.2 read such a record anyway (a stringreasonread as no reason), and a record that is not an object (null) failed the whole response. For the unreadable flag itself, a single-flag getter now returnsfalseinstead of the value 5.61.2 read, and a snapshot read sends its event as for a disabled flag, withoutflag_missing. Its sibling flags read as before.reason.description("Matched rule 2" instead of "Matched condition set 2") andreason.code(no_rule_matchinstead ofno_condition_match).FeatureFlagResult.reason,FeatureFlagResult.reasonCodeand$feature_flag_reasonshow the v4 text once a server sends v4. Values do not change, and v1 flags are identical in every field.JSON.stringify). Getters return decoded payloads, which are the same values as atv=2; posthog-node does not expose payload text.flags_response_v4. The v2 compliance adapter now declares it too, when the installed client requests/flags?v=4(read from the client's own request version, not hardcoded). With harness 1.29.0, the shared migration scenario "Remote flag requests use the v2 query parameter" (remote-flags-v1.feature) is not selected for SDKs that declareflags_response_v4(test(flags): check the v4 /flags query parameter for SDKs that request it sdk-specs#173, feat(v2): skip a Gherkin case when the adapter declares a capability it excludes posthog-sdk-test-harness#102), and its v4 counterpart runs and passes./get_feature_flag_payload, which takes the/get_feature_flagarguments and callsgetFeatureFlagPayload(key, distinctId, undefined, options), and declaresfeature_flag_payloads. Withflags_response_v4, that selects the Gherkin form of the harness's/flags?v=4read suite, test(flags): read /flags?v=4 records through the legacy flag and payload getters sdk-specs#174 and feat(v2): run the /flags?v=4 read cases in the Gherkin runner posthog-sdk-test-harness#103: 150 scenarios that serve each reader fixture and value corpus case of the contract and check the legacy getter (75) and payload getter (75) results. The 75th payload case,readers.v1_failed, expects no payload for a failed record. This workflow'sHARNESS_IMAGEis harness 1.29.0, which includes them./flags/definitions) is untouched.Changesets:
posthog-nodeminor,@posthog/corepatch.💚 How did you test it?
src/__tests__/released-flags.spec.tscompares this SDK with results recorded by posthog-node 5.61.2.released-flags-scenarios.tsuses only the public client API, so the same scenarios ran on 5.61.2 from npm (which recordedfixtures/released-flags-5.61.2.json; the module says how to regenerate it) and run here from both thev2and thev4response. They cover every legacy getter, the snapshot getters,capture({ flags }),capture({ sendFeatureFlags: true }), every$feature_flag_calledevent, a failed flag, and a response with an unreadable record. The only expected difference is the unreadable record, which reads as 5.61.2 read a disabled flag without reason or metadata, plus theunknown_errorof its single-flag reads.src/__tests__/flags-v4-response.spec.ts(192 tests), built on fixtures copied unchanged from harness release 1.29.0 (contract 9.0.0).fixtures/feature_flag_rules_v2/SOURCE.jsonrecords the release, revision and file digests, and a test checks them. It covers:readers.json), projected from the parsed record onto each case's expected value, config version, exposure eligibility and error; for cases with alegacy_payload,getFeatureFlagPayloadand the snapshot'sgetFlagPayloadreturn that payload;v=2tov=4). Its mocks send v1 and v2 responses, so every existing flag test now runs through the older-record path.main: posthog-node (1,783 tests) and core (2,362), with lint andcheck-typesfor node. Before the merge, also browser, browser-common, React Native and web,pnpm format,pnpm lint:oxlint, andcheck-typesfor core, React Native and web.$feature_flag_calledevents. They were identical for every v1 flag. For config version 2 flags the only difference was the reason text ofgetFeatureFlagResult()(reasonandreasonCode), as described above. The same scenarios on posthog-python with the same change gave the same results apart from representation (payload text, null against absent).compliance/node/v2/run-compliance.shand the released harness 1.29.0 image, as the workflow runs it. Capture v0: 233 passed, 4 failed, 1,196 not selected. Capture v1: 298 passed, 4 failed, 1,131 not selected. In both modes all 150/flags?v=4read scenarios pass, the v4 query scenario passes, and the v2 query scenario is not selected. The four failures also fail onmain: the three snapshot payload cases (server_snapshot_payload_false,_zero,_null) and "Omitted GeoIP settings default to enabled in flag requests". The read results agree withflags-v4-response.spec.ts, which reads the same fixtures.flags_request_uses_v4_query_param; capture v1 115/117, where the two failures (retry_behavior.retries_on_404andretries_on_429) fail the same way on a build from before this change.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Generated with Claude Code