Repository navigation
fix(node): keep local flag evaluation consistent with the server - #5471
gustavohstrassburger wants to merge 1 commit into
Conversation
Coherence0 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 7 bullets (5 invariants, 2 requirements), 0 problems Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
Node SDK compliance v2Commit: 4c2b4bf Capture v0 — acceptance✅ 28 passed / ❌ 3 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: Harness exit: 1; report validation exit: 1. Capture v1 — acceptance✅ 28 passed / ❌ 3 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: Harness exit: 1; report validation exit: 1. Capture v0 — migration✅ 55 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ 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:134 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 — migration✅ 120 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ 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:134 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. |
Problem
Two cases where local flag evaluation in posthog-node gives a different answer than the server, both from the condition loop change in #5234 (sdk-specs #85). Reviews of the matching PRs in the other server SDKs found them.
depand sets varianta, condition 2 matches everyone with variantb. The server returnsafor a member of the cohort, but the SDK returnedblocally, and with all flags requested the localbwon over the remotea.Changes
unresolved_dependency. The dependent flag falls back to/flags, andunresolvedFlagsreportsunresolved_dependency.unsupported_definitionelsewhere in the flag (cohorts included) still wins. A malformed definition in that lookup does not break the fallback.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset fileHow did you test this code?
New tests in
evaluate-flags.spec.tscover the variant case (flag requested alone and with all flags), local-only reporting, a locally resolvable dependency, the cohort precedence in both leaf orders, and the unexpected-error case. The 4 regression tests fail without the fix. Locally:pnpm test:unitinpackages/node(1298 passed),pnpm lint,pnpm check-types, oxfmt. No manual testing in an app.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
people.set_oncedoesn't seem to be working #85 to the server SDKs. The same two fixes were made in feat: report unresolved local flags on evaluate_flags snapshots posthog-python#1133, feat(flags): report unresolved local flags on EvaluateFlagsAsync snapshots posthog-dotnet#357, feat(flags): report unresolved local flags on evaluate_flags snapshots posthog-ruby#319 and feat(flags): report unresolved local flags on evaluateFlags snapshots posthog-php#281; posthog-go already forced the server fallback.