fix: cached-render exposure read before identify() (SRM canary) by kilbot · Pull Request #29 · wcpos/wp-admin-landing

kilbot · 2026-06-15T10:24:56Z

Fixes a reviewer-flagged bug in the instant-cache render (PR #27).

Bug: the cache path recorded $feature_flag_called via an async onFeatureFlags subscription, so on a repeat consented visit the read could fire after identifyConsented() reloaded flags under site_uuid — a different experiment bucket than the anon_id the cached assignment was made in. That records exposure for the wrong bucket and breaks the SRM/stickiness canary (spec §3.1.4, §6.1).

Fix: read the flag synchronously in recordCachedExposure(ph) inside prepareVariant, before it returns and therefore before identify() (flag-before-identify, §5.1). Spec §3.1.4 specifies exactly this — getFeatureFlag returns synchronously from PostHog's persisted flags after the first load wrote the cache. No onFeatureFlags subscription on the cache path, so a post-identify reload can never re-record it. Confirmed against the bundled posthog-js source that init() restores persisted flags synchronously.

Cold path and proActive unaffected. Adds tests/exposure-ordering.test.mjs pinning the ordering invariant.

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Added test coverage for variant exposure ordering and bootstrap flow.
Chores
- Updated cached data timestamps.
- Optimized internal variant tracking mechanism.

The instant-cache path (PR #27) recorded $feature_flag_called via an async onFeatureFlags subscription, so on a repeat consented visit the read could land AFTER identifyConsented() reloaded flags under site_uuid (a different bucket) — recording exposure for the wrong bucket and breaking the SRM/stickiness canary. Per spec §3.1.4 the read is synchronous from PostHog's persisted flags, so it now runs inside prepareVariant before the function returns and therefore before identify() (flag-before-identify, §5.1). No onFeatureFlags subscription on the cache path → a post-identify reload can never re-record it. Adds tests/exposure-ordering.test.mjs pinning the invariant.

coderabbitai · 2026-06-15T10:25:10Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: wcpos/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a3ce9ee1-abbb-417a-aed7-ea372e639585

📥 Commits

Reviewing files that changed from the base of the PR and between 993f0d3 and 0e1c0e1.

📒 Files selected for processing (5)

assets/js/welcome.js
src/bootstrap/variant-loader.ts
src/shared/roadmap.json
src/shared/wporg-reviews.json
tests/exposure-ordering.test.mjs

📝 Walkthrough

Walkthrough

Replaces the async watchExposure / onFeatureFlags subscription pattern in variant-loader.ts with a synchronous recordCachedExposure helper that calls getFeatureFlag(FLAG_KEY) directly and reads the kill-switch without emitting an exposure event, clearing the assignment cache when the kill-switch is active. A new test file validates the ordering and implementation constraints. Two shared JSON files have their generated_at timestamps bumped.

Changes

Synchronous Cached Exposure Recording

Layer / File(s)	Summary
`recordCachedExposure` helper and cache branch wiring `src/bootstrap/variant-loader.ts`	Replaces `watchExposure` (async `onFeatureFlags` subscription) with `recordCachedExposure`, which synchronously fires `$feature_flag_called` via `getFeatureFlag(FLAG_KEY)` and reads the kill-switch with `send_event: false`, calling `clearCache()` when the kill-switch is on. The `'cache'` branch in `prepareVariant` is updated to call `recordCachedExposure(ph)`.
Exposure ordering tests `tests/exposure-ordering.test.mjs`	New test file loads `variant-loader.ts` and `bootstrap/index.tsx` as source strings, uses a `slice()` helper to isolate function bodies, and asserts the synchronous exposure path, absence of `onFeatureFlags`/`watchExposure`, kill-switch suppression of `$feature_flag_called`, `clearCache()` call, and that `await prepareVariant` precedes `identifyConsented()` in bootstrap.

Shared JSON timestamp refresh

Layer / File(s)	Summary
Timestamp bump `src/shared/roadmap.json`, `src/shared/wporg-reviews.json`	`generated_at` updated from `2026-06-13` to `2026-06-15` in both files; no other content changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

wcpos/wp-admin-landing#27: Directly related — that PR introduced watchExposure in the cached fast path of variant-loader.ts, which this PR replaces with the synchronous recordCachedExposure approach.

Poem

🔄 No more waiting on a flag to call,
The cache path now reads sync — no async stall.
The kill-switch whispers "clear it, start fresh,"
recordCachedExposure keeps the order enmeshed.
Tests now guard the sequence, line by line,
From variant to identity — all in time! ✅

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cached-exposure-before-identify

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e1c0e1cf1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-15T10:29:05Z

    const killValue = ph.getFeatureFlag(KILL_SWITCH_KEY, { send_event: false });
    if (killValue === true || killValue === 'on') clearCache();


Preserve fresh kill-switch handling on cache hits

When the ops kill-switch is flipped after a visitor's last load, this cache path only reads the synchronously persisted value, so a stale false leaves wcpos_landing_assignment intact and the user gets at least one extra cached variant render after PostHog refreshes flags in the background. The previous cache-path subscription cleared the assignment when the fresh flag response arrived; keeping exposure synchronous is fine, but the kill-switch still needs a fresh, non-exposure (send_event: false) path so the kill takes effect on the next load as documented.

Useful? React with 👍 / 👎.

Right — fixed in #30. Added watchKillSwitch(ph): a background onFeatureFlags watch that re-reads the kill-switch with send_event: false (non-exposure, so it's safe after identify — the ops flag is bucket-agnostic) and clears the assignment cache so a freshly-flipped kill takes effect next load. The experiment-flag exposure read stays synchronous + pre-identify, so #29's invariant is intact.

…30) #29 — restore fresh kill-switch on cache hits. The synchronous exposure read only sees the persisted kill-switch value; a switch flipped after the last load wouldn't clear the cache. Add watchKillSwitch(ph): a background onFeatureFlags watch that re-reads the kill-switch with send_event:false (non-exposure, so no post-identify bucket concern — the ops flag is bucket-agnostic) and clears the assignment cache so the kill takes effect next load (spec §3.1.4). The experiment-flag exposure read stays synchronous + pre-identify. #23 — preview must not resolve production flags. Add advanced_disable_flags in preview so the Pages harness never fetches landing-variant (a live flag could otherwise win over the seeded assignment) and preview iframe loads don't hit the production flag endpoint. Verified both seeded variants render deterministically. Extends tests/exposure-ordering.test.mjs.

kilbot merged commit 423bcfa into main Jun 15, 2026
7 of 8 checks passed

kilbot deleted the fix/cached-exposure-before-identify branch June 15, 2026 10:25

chatgpt-codex-connector Bot reviewed Jun 15, 2026

View reviewed changes

kilbot mentioned this pull request Jun 15, 2026

fix: address Codex review comments (preview flags, fresh kill-switch) #30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: cached-render exposure read before identify() (SRM canary)#29

fix: cached-render exposure read before identify() (SRM canary)#29
kilbot merged 1 commit into
mainfrom
fix/cached-exposure-before-identify

kilbot commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Uh oh!

kilbot Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		const killValue = ph.getFeatureFlag(KILL_SWITCH_KEY, { send_event: false });
		if (killValue === true \|\| killValue === 'on') clearCache();

Uh oh!

Conversation

kilbot commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

kilbot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kilbot commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading