feat(experiment-tag): Dual-write RTBT events to relay#334
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unload relay flush skips active writes
- Fixed by always queuing events in pendingWrites first and only removing them after async write confirmation, ensuring flush() during unload will resend in-flight events.
Or push these changes by commenting:
@cursor push beba896619
Preview (beba896619)
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
@@ -189,14 +189,24 @@
if (this.destroyed) {
return;
}
+
+ // Always queue the event in case flush is needed before async write completes
+ this.pendingWrites.push(event);
+
if (!this.available || !this.iframeWindow) {
- this.pendingWrites.push(event);
return;
}
+
+ // Attempt async write; on success, remove from pending queue
void this.sendRequest(
this.createRelayRequest('WRITE_EVENT', { event }),
- ).catch(() => {
- // fire-and-forget
+ ).then(() => {
+ const idx = this.pendingWrites.indexOf(event);
+ if (idx !== -1) {
+ this.pendingWrites.splice(idx, 1);
+ }
+ }).catch(() => {
+ // Keep event in pendingWrites for flush
});
}
diff --git a/packages/experiment-tag/test/behavioral-targeting/relay-client.test.ts b/packages/experiment-tag/test/behavioral-targeting/relay-client.test.ts
--- a/packages/experiment-tag/test/behavioral-targeting/relay-client.test.ts
+++ b/packages/experiment-tag/test/behavioral-targeting/relay-client.test.ts
@@ -202,6 +202,7 @@
await initReady(client, iframeWindow);
client.writeEvent(sampleEvent(1, { page: 'home' }));
+ await Promise.resolve(); // Allow async write completion to remove from pendingWrites
client.flush();
const writeCalls = postMessage.mock.calls.filter(
@@ -210,6 +211,21 @@
expect(writeCalls).toHaveLength(1);
});
+ test('flush includes in-flight writes not yet confirmed', async () => {
+ const { client, iframeWindow, postMessage } = setupClient();
+ await initReady(client, iframeWindow);
+
+ client.writeEvent(sampleEvent(1, { page: 'home' }));
+ // Immediately flush without awaiting - simulates unload scenario
+ client.flush();
+
+ const writeCalls = postMessage.mock.calls.filter(
+ ([payload]) => payload.type === 'WRITE_EVENT',
+ );
+ // Event sent twice: once via sendRequest, once via flush (safety for unload)
+ expect(writeCalls).toHaveLength(2);
+ });
+
test('concurrent init creates only one iframe', async () => {
const { client, iframeWindow } = setupClient();
const first = client.init();You can send follow-ups to the cloud agent here.
6361af4 to
ba2f2e4
Compare
ba2f2e4 to
5a11588
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Migrated origins skip relay upload
- Added logic in syncFromRelay to upload local events not present in relay when the origin is already migrated, ensuring events recorded before setRelayClient runs are synced.
- ✅ Fixed: Failed writes dropped from queue
- Added response.ok check in writeEvent's .then() handler so failed writes remain in pendingWrites for retry by flush().
Or push these changes by commenting:
@cursor push 5222e2519b
Preview (5222e2519b)
diff --git a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
--- a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
@@ -173,6 +173,17 @@
}
const relayStore = await relay.readEvents();
+
+ // Upload local events that aren't in relay (e.g., recorded before setRelayClient)
+ if (migrated) {
+ const relayKeys = new Set(relayStore.events.map(eventDedupKey));
+ for (const event of this.memoryCache.events) {
+ if (!relayKeys.has(eventDedupKey(event))) {
+ relay.writeEvent(event);
+ }
+ }
+ }
+
this.mergeFromRelay(relayStore);
return true;
} catch {
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
@@ -198,7 +198,10 @@
}
void this.sendRequest(this.createRelayRequest('WRITE_EVENT', { event }))
- .then(() => {
+ .then((response) => {
+ if (!response.ok) {
+ return; // Keep in pendingWrites for flush() retry
+ }
const idx = this.pendingWrites.indexOf(event);
if (idx !== -1) {
this.pendingWrites.splice(idx, 1);You can send follow-ups to the cloud agent here.
5a11588 to
5ad9c96
Compare
Inject RelayClient from experiment.ts when RTBT rules are present: non-blocking beginRelaySync after Pass 1 applyVariants, Pass 2 re-apply when behaviors change (storage sync activates with #334). Co-authored-by: Cursor <cursoragent@cursor.com>
c091003 to
c13dbe1
Compare
EventStorageManager writes to relay on addEvent, merges relay store on syncFromRelay (migrate if needed, relay wins on dedup), and flushes relay on unload/visibility. BehavioralTargetingManager exposes setRelayClient and syncFromRelay for Pass 2 evaluation. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Queue writeEvent in pendingWrites until RPC confirms, so flushRelay on tab close can resend events still in flight to the relay store. Co-authored-by: Cursor <cursoragent@cursor.com>
Upload local-only events when origin is already migrated, and only dequeue pendingWrites after a successful WRITE_EVENT RPC response. Co-authored-by: Cursor <cursoragent@cursor.com>
5ad9c96 to
c60770e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Stale relay client after destroy
- Added setRelayClient(null) at the start of beginRelaySync to detach any existing destroyed relay client before init, preventing writes to stale clients when re-init fails.
- ✅ Fixed: Sync flush duplicates relay writes
- Removed the redundant flush() call in syncFromRelay since writeEvent already initiates async RPC calls, eliminating duplicate writes.
Or push these changes by commenting:
@cursor push 8677484f5e
Preview (8677484f5e)
diff --git a/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts b/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
--- a/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
@@ -77,6 +77,8 @@
*/
public async beginRelaySync(relayClient: RelayClient): Promise<boolean> {
const behaviorsBefore = this.serializeMatchedBehaviors();
+ // Detach any existing (potentially destroyed) relay client before init
+ this.setRelayClient(null);
await relayClient.init();
if (!relayClient.relayAvailable) {
return false;
diff --git a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
--- a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
@@ -180,7 +180,6 @@
relay.writeEvent(event);
}
}
- relay.flush();
}
const relayStore = await relay.readEvents();You can send follow-ups to the cloud agent here.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Local FIFO drops relay events
- Removed immediate relay write from addEvent() so relay sync only happens via syncFromRelay() which respects the local FIFO limit.
Or push these changes by commenting:
@cursor push 4979fe42fc
Preview (4979fe42fc)
diff --git a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
--- a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
@@ -101,9 +101,6 @@
// Trigger debounced write to localStorage
this.scheduleDebouncedWrite();
-
- // Fire-and-forget relay write when cross-subdomain sync is enabled
- this.relayClient?.writeEvent(event);
}
/**
diff --git a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
--- a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
+++ b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
@@ -434,23 +434,11 @@
migrateEvents: jest.fn().mockResolvedValue(undefined),
});
- test('writes events to relay on addEvent when relay is attached', () => {
+ test('does not write events to relay on addEvent (relay sync happens via syncFromRelay)', () => {
const relay = createMockRelay();
eventStorage.setRelayClient(relay as unknown as RelayClient);
eventStorage.addEvent('click', { page: 'home' });
- expect(relay.writeEvent).toHaveBeenCalledWith(
- expect.objectContaining({
- event_type: 'click',
- properties: { page: 'home' },
- id: 1,
- }),
- );
- });
-
- test('does not write to relay when relay is not attached', () => {
- const relay = createMockRelay();
- eventStorage.addEvent('click');
expect(relay.writeEvent).not.toHaveBeenCalled();
});You can send follow-ups to the cloud agent here.
Wait for late relay ready before Pass 2 sync, attach relay client before init, remove duplicate flush on migrated upload, and re-migrate relay cache when local FIFO trim evicts events. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Sync failure disables dual-write
- Removed the setRelayClient(null) call when syncFromRelay returns false, so the relay client remains attached for future dual-writes even after transient RPC failures.
- ✅ Fixed: FIFO trim bypasses retry queue
- Changed the code to always call writeEvent(event) regardless of FIFO trimming, ensuring the new event is added to pendingWrites for retry protection.
- ✅ Fixed: Same-millisecond events collapse
- Added event id to the eventDedupKey function so same-type events with the same millisecond timestamp are distinguished by their unique id during relay merge.
- ✅ Fixed: Visibility flush drops retries
- Removed flushRelay() call from handleVisibilityChange since visibility changes are non-terminal and pendingWrites should remain for retry.
Or push these changes by commenting:
@cursor push 2c9badb003
Preview (2c9badb003)
diff --git a/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts b/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
--- a/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts
@@ -89,7 +89,6 @@
}
const synced = await this.syncFromRelay();
if (!synced) {
- this.setRelayClient(null);
return false;
}
return behaviorsBefore !== this.serializeMatchedBehaviors();
diff --git a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
--- a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
@@ -8,8 +8,9 @@
export function eventDedupKey(event: {
event_type: string;
timestamp: number;
+ id: number;
}): string {
- return `${event.event_type}:${event.timestamp}`;
+ return `${event.event_type}:${event.timestamp}:${event.id}`;
}
/**
@@ -105,10 +106,9 @@
this.scheduleDebouncedWrite();
// Fire-and-forget relay write when cross-subdomain sync is enabled
+ this.relayClient?.writeEvent(event);
if (fifoTrimmed) {
this.syncRelayCacheAfterFifoTrim();
- } else {
- this.relayClient?.writeEvent(event);
}
}
@@ -383,11 +383,12 @@
/**
* Handler for visibilitychange event.
+ * Only flushes to localStorage; relay writes remain in pendingWrites
+ * for retry since visibility changes are non-terminal.
*/
private handleVisibilityChange = (): void => {
if (document.visibilityState === 'hidden') {
this.flushToLocalStorage();
- this.flushRelay();
}
};
diff --git a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
--- a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
+++ b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
@@ -417,9 +417,9 @@
});
describe('eventDedupKey', () => {
- test('uses event_type and timestamp', () => {
- expect(eventDedupKey({ event_type: 'click', timestamp: 1000 })).toBe(
- 'click:1000',
+ test('uses event_type, timestamp, and id', () => {
+ expect(eventDedupKey({ event_type: 'click', timestamp: 1000, id: 42 })).toBe(
+ 'click:1000:42',
);
});
});You can send follow-ups to the cloud agent here.
Keep pending writes until RPC confirms, include event id in merge dedup key, use writeEvent retry queue on FIFO trim, and retain relay client on sync failure. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Pass 2 duplicates relay writes
- Added dedup check in writeEvent to skip events with the same dedup key already in pendingWrites, preventing duplicate entries when syncFromRelay calls writeEvent for events already queued during relay initialization.
Or push these changes by commenting:
@cursor push 7f9dd2872d
Preview (7f9dd2872d)
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
@@ -232,6 +232,16 @@
return;
}
+ // Skip if an event with the same dedup key is already pending
+ const dedupKey = `${event.event_type}:${event.timestamp}:${event.id}`;
+ if (
+ this.pendingWrites.some(
+ (e) => `${e.event_type}:${e.timestamp}:${e.id}` === dedupKey,
+ )
+ ) {
+ return;
+ }
+
// Queue until async write confirms — flush() can resend in-flight events on unload.
this.pendingWrites.push(event);
this.sendPendingWrite(event);You can send follow-ups to the cloud agent here.
Defer storage attachment until after init/sync, return typed RelaySyncResult, centralize relay teardown, dedupe pending writes, and use migrateEvents for FIFO reconciliation with writeEvent fallback. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Confirmed write removal uses reference
- Changed removeConfirmedWrite to use findIndex with id, event_type, and timestamp matching instead of indexOf (reference equality), ensuring confirmed events are properly removed from pendingWrites regardless of object reference.
Or push these changes by commenting:
@cursor push d3f6f654cd
Preview (d3f6f654cd)
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
@@ -245,7 +245,12 @@
}
private removeConfirmedWrite(event: RelayEventRecord): void {
- const idx = this.pendingWrites.indexOf(event);
+ const idx = this.pendingWrites.findIndex(
+ (queued) =>
+ queued.id === event.id &&
+ queued.event_type === event.event_type &&
+ queued.timestamp === event.timestamp,
+ );
if (idx !== -1) {
this.pendingWrites.splice(idx, 1);
}You can send follow-ups to the cloud agent here.
Match pending write removal by event id, settle waitForAvailable on destroy, and clear storage when replacing relay via teardownRelay. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: FIFO trim leaves stale relay events
- Replaced migrateEvents with replaceEvents in reconcileRelayAfterFifoTrim and removed the writeEvent fallback that caused stale events to remain in relay storage.
- ✅ Fixed: Sync errors skip relay merge
- Separated try/catch blocks in syncFromRelay so mergeFromRelay is attempted even if migration/upload fails, ensuring relay events are merged regardless of earlier step failures.
Or push these changes by commenting:
@cursor push 55c4b8c1f1
Preview (55c4b8c1f1)
diff --git a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
--- a/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/event-storage.ts
@@ -167,6 +167,8 @@
return false;
}
+ let migrationSucceeded = true;
+
try {
const origin = window.location.origin;
const migrated = await relay.checkMigrated(origin);
@@ -187,13 +189,18 @@
}
}
}
+ } catch {
+ migrationSucceeded = false;
+ }
+ try {
const relayStore = await relay.readEvents();
this.mergeFromRelay(relayStore);
- return true;
} catch {
return false;
}
+
+ return migrationSucceeded;
}
/**
@@ -316,14 +323,12 @@
return;
}
void relay
- .migrateEvents(window.location.origin, {
+ .replaceEvents({
events: [...this.memoryCache.events],
nextId: this.memoryCache.nextId,
})
.catch(() => {
- for (const cached of this.memoryCache.events) {
- relay.writeEvent(cached);
- }
+ // replaceEvents failed; no fallback to avoid stale events
});
}
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-client.ts
@@ -305,6 +305,15 @@
}
}
+ async replaceEvents(store: RelayEventStorage): Promise<void> {
+ const response = await this.sendRequest(
+ this.createRelayRequest('REPLACE_EVENTS', { store }),
+ );
+ if (!response.ok) {
+ throw new Error(response.error ?? 'replace events failed');
+ }
+ }
+
destroy(): void {
this.destroyed = true;
this.cancelBodyReadyPoll?.();
diff --git a/packages/experiment-tag/src/behavioral-targeting/relay-protocol.ts b/packages/experiment-tag/src/behavioral-targeting/relay-protocol.ts
--- a/packages/experiment-tag/src/behavioral-targeting/relay-protocol.ts
+++ b/packages/experiment-tag/src/behavioral-targeting/relay-protocol.ts
@@ -5,6 +5,7 @@
| 'WRITE_EVENT'
| 'READ_EVENTS'
| 'MIGRATE_EVENTS'
+ | 'REPLACE_EVENTS'
| 'CHECK_MIGRATED'
| 'MIGRATE_ACK';
diff --git a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
--- a/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
+++ b/packages/experiment-tag/test/behavioral-targeting/event-storage.test.ts
@@ -432,6 +432,7 @@
readEvents: jest.fn().mockResolvedValue({ events: [], nextId: 1 }),
checkMigrated: jest.fn().mockResolvedValue(true),
migrateEvents: jest.fn().mockResolvedValue(undefined),
+ replaceEvents: jest.fn().mockResolvedValue(undefined),
});
test('writes events to relay on addEvent when relay is attached', () => {
@@ -474,8 +475,7 @@
expect(relay.writeEvent).toHaveBeenCalledWith(
expect.objectContaining({ properties: { index: 500 } }),
);
- expect(relay.migrateEvents).toHaveBeenCalledWith(
- window.location.origin,
+ expect(relay.replaceEvents).toHaveBeenCalledWith(
expect.objectContaining({
events: expect.arrayContaining([
expect.objectContaining({ properties: { index: 500 } }),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit b1b5044. Configure here.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/experiment-tag/src/behavioral-targeting/behavioral-targeting-manager.ts # packages/experiment-tag/src/behavioral-targeting/event-storage.ts # packages/experiment-tag/src/experiment.ts # packages/experiment-tag/test/behavioral-targeting/behavioral-targeting-manager-relay.test.ts


Summary
Stacks on #335 (
web/rtbt-relay-iframe→ #333). Implements RTBT event storage dual-write and Pass 2 merge.EventStorageManager: dual-write onaddEvent,mergeFromRelay,syncFromRelay,flushRelayBehavioralTargetingManager: directsetRelayClient/syncFromRelay(used bybeginRelaySyncfrom feat(experiment-tag): Wire relay iframe on start #335)Completes the relay path started in #335: iframe init → attach client → merge relay store → re-evaluate.
Test plan
npx jest test/behavioral-targeting/event-storage.test.tsnpx jest test/behavioral-targeting/behavioral-targeting-manager-relay.test.tsnpx jest test/experiment-relay-iframe.test.tsnpx jest test/behavioral-targeting/relay-client.test.tsMerge order
web/rtbt-relay-client— RelayClient foundation)web/rtbt-relay-iframe— iframe wiring)web/rtbt-use-relay— event storage)Note
Medium Risk
Changes behavioral-targeting event persistence and cross-subdomain sync, which can affect flag evaluation after relay merge; failures are handled by returning false from sync and leaving local-only behavior intact.
Overview
EventStorageManagernow keeps RTBT events in local storage and mirrors them to the cross-subdomain relay, replacing the previous no-op hooks.New events are dual-written on
addEventwhen a relay client is attached.eventDedupKey(event_type:timestamp:id) drives merge deduplication so same-millisecond events stay distinct.mergeFromRelayunions relay and local caches with relay winning conflicts;syncFromRelayruns Pass 2 (bulk migrate on first origin visit, otherwise push local-only events, then read and merge).flush, unload/hidden handlers, andcleanupalso callflushRelayso pending relay writes are not dropped.Local FIFO eviction at 500 events is unchanged and does not trigger relay backfill—only the new event is sent. Tests cover dedup keys, dual-write, merge, migrate vs incremental sync, and unavailable relay.
Reviewed by Cursor Bugbot for commit cd8a1ce. Bugbot is set up for automated code reviews on this repo. Configure here.