fix(flags): clear the flag-definition ETag when definitions are reset (sdk-specs flag-definition-loader) - #825
Draft
posthog[bot] wants to merge 1 commit into
Draft
Conversation
The sdk-specs flag-definition-loader contract requires a reset to drop the in-memory definitions *and* the associated ETag state. Python cleared the definitions on a 402 (quota limited) or 401 (unauthorized) definitions response but kept `_flags_etag`, so the next poll sent `If-None-Match` for definitions the SDK no longer held. The server answered 304, the loader took its "definitions unchanged" early return, and local evaluation stayed empty until the definitions changed server-side. Fold the reset into a `_reset_flag_definitions()` helper that also clears the ETag, and use it from both error handlers. Generated-By: PostHog Code Task-Id: b2f32a21-8d44-4644-b548-60a2da6d1067
Contributor
posthog-python Compliance ReportDate: 2026-08-03 08:00:21 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
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
Compliance gap against the cross-SDK contract in PostHog/sdk-specs.
openspec/specs/flag-definition-loader/spec.md, Behavior #10 — "Clear definitions on explicit reset/clear":The same spec lists the ETag/cache validator under State written, and Behavior #5 is what makes the omission bite: "A
304keeps current definitions."What was out of compliance.
_fetch_feature_flags_from_api()dropped the definitions on a402(quota limited) or401(unauthorized) definitions response, but never cleared_flags_etag— that attribute was only ever written on a successful fetch, and nothing else in the SDK reset it. So after a reset the next poll still sentIf-None-Match: <stale etag>. Once the block lifted (quota reset, key fixed) the server saw an unchanged definition set, answered304, and the loader took its "flags not modified, using cached data" early return — except there was no cached data left. Local evaluation stayed permanently empty until someone happened to edit a flag server-side.How this fixes it. The reset now goes through a
_reset_flag_definitions()helper that clearsfeature_flags/group_type_mapping/cohortsand_flags_etag, so the recovery poll is unconditional and repopulates the definitions. Both error handlers call it.Behavior change / compatibility risk: minimal. The only observable difference is one unconditional (non-
304-eligible) definitions request after a 401/402 reset, in exchange for local evaluation actually recovering. No public API, config, event, or property changes. Transient failures (503, timeouts) are untouched and still keep the last known good definitions and their ETag, per the spec's "Transient failures keep the last known good definitions" error-handling rule.Scoped deliberately to the loader's reset paths. The
disabled/ missing-secret-key early returns inload_feature_flags()also zerofeature_flags, but they return before the loader ever runs, so a stale ETag can't be acted on there; I left them alone rather than widen the diff.💚 How did you test it?
Added
test_load_feature_flags_clears_etag_when_definitions_are_reset, parameterized over both reset paths (402 and 401). It loads definitions with an ETag, takes the error response, asserts the definitions and the ETag are gone, then asserts the recovery poll sends noetagand repopulates the definitions.Verified the test fails on
main's behavior (AssertionError: '"etag-v1"' is not None) and passes with the fix. Also ranposthog/test/test_feature_flags.py,test_flag_definition_cache.py,test_feature_flag_called_minimization.py(214 passed) andtest_client.py(152 passed), plusruff format --check,ruff check, andmypyonposthog/client.py. No manual/integration testing against a live PostHog instance.📝 Checklist
If releasing new changes
sampo addto generate a changeset file (added by hand at.sampo/changesets/clear-flag-definition-etag-on-reset.md, matching the existing format)🤖 Agent context
Autonomy: Fully autonomous
Opened by the scheduled "SDK Spec Compliance Enforcer" loop for posthog-python, running in PostHog Code (Claude Code harness). The loop reads
PostHog/sdk-specsas the source of truth each run, audits the Python SDK against the contracts in scope for a server SDK, and opens one focused draft PR per confirmed divergence.This run swept 30-ish in-scope specs across six parallel read-only audit agents (capture/identify/alias/groups; flag getters; batcher/flush/http/retry; local evaluation + definition loader; exceptions/before-send/tracing headers; logs/traces). This finding was picked over the alternatives because the spec language is unambiguous ("and associated ETag state"), the failure mode is a reproducible stuck-empty state rather than a stylistic difference, and the fix carries essentially no backward-compatibility risk. Candidates deliberately not acted on this run, for the reviewer's awareness: missing input validation on
alias()/group_identify()(spec has explicit@bothdrop-and-warn scenarios, but fixing it stops emitting events some users may be counting today), inverted frame order in the synthesized traceback for never-raised exceptions (would re-fingerprint existing issues), and payload-type inconsistency between locally- and remotely-evaluated flags (strvs parsed). The module-levelget_feature_flag_payloaddefault was skipped as already-decided — #794 was closedwontfix.Agent-authored, so no human co-author is claimed, and it needs human review before merge.
Created with PostHog Code