Skip to content

fix: defensively filter failed normalized cache records only - #8

Merged
marandaneto merged 1 commit into
mainfrom
fix/spec211-failed-flags
Nov 13, 2026
Merged

marandaneto merged 1 commit into
mainfrom
fix/spec211-failed-flags

Conversation

@marandaneto

Copy link
Copy Markdown
Member

Motivation and context

Related to PostHog/sdk-specs#211.

This is a LIMITED defensive normalized-cache fix only, not end-to-end wire conformance. If a normalized record contains failed: true, the C++ cache currently accepts its placeholder value and payload. Skip that record before reading either field. The shared parser also filters restored records.

Healthy false values remain valid. Complete nonerrored refreshes replace state after filtering. Response-wide errored refreshes keep prior successful values, paired payloads, and source metadata. Existing caller defaults and missing-read behavior are unchanged. There are no breaking changes.

The actual phu_flags bridge serializes flags.export() from an external Rust embedding checkout. The inspected exporter loses failure identity before C++ receives the records, so this guard cannot detect those already-normalized placeholders. The required POSTHOG_RS_PATH checkout is unavailable. An audited failure-aware exporter and an approved immutable dependency pin are still needed before actual ABI, public-getter, and enrichment wire conformance can be established. This PR does not change the ABI or dependency pin.

Release intent is a patch-level defensive fix. This repository has no existing release-intent mechanism, so no changeset or version bump was invented.

How did you test it?

  • Ran real UE 5.8 automation in an isolated scratch module using byte-identical copies of the cache, properties, JSON helper, feature flag headers, and new test. Automation RunTests PostHog. passed PostHog.FailedFlagCache and neighboring PostHog.FlagCache: 2 succeeded, 0 failed, 0 warnings, 0 not run. This is cache-level testing, not a full plugin link or an exporter substitute.
  • The retained pre-fix regression report shows PostHog.FailedFlagCache failing on leaked failed values/payloads, stale-state replacement, and overwritten successful pairs. The current test passes. Cases cover complete and response-wide errored refreshes, healthy false controls, retained source metadata, restore filtering, and all-failed records including a missing value.
  • git diff --check passed. The required branch autoreview against origin/main reported no actionable findings.
  • bash scripts/build-native.sh aarch64-apple-darwin is blocked because POSTHOG_RS_PATH is absent. Full native/plugin, ABI, public-getter wire, and platform-matrix validation were not completed. No shared-harness conformance is claimed.

Checklist

  • I reviewed the submitted code.
  • I added or updated tests where applicable.
  • I updated the documentation where needed.
  • I described any breaking changes and their migration path.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Pi assisted with the implementation and publication using local file and shell tools, Git, GitHub CLI, and the configured autoreview helper. The human-directed scope was limited to the defensive normalized-cache filter, cache tests, and documentation. No public agent-session link is available.

The exporter limitation is documented rather than hidden behind a replacement exporter, speculative ABI, new missing-read tracking, or invented release metadata. Human review is required before merge.

@marandaneto marandaneto self-assigned this Nov 12, 2026
@marandaneto
marandaneto marked this pull request as ready for review November 12, 2026 12:13
@marandaneto
marandaneto requested a review from a team as a code owner November 12, 2026 12:13

@ioannisj ioannisj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 An AI agent wrote this on behalf of @ioannisj.

No findings. The failed: true skip in ParseRecords holds up on the full refresh, the partial merge and the restore path, and healthy false values keep their payloads.

@marandaneto
marandaneto merged commit 7407891 into main Nov 13, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants