Skip to content

Content controller hardening: dispose guard, concurrent-evaluate store race, session-exemption TTL/scoping #52

Description

@i8ramin

Umbrella for content-controller robustness gaps surfaced while dogfooding the Dupe extension migration onto standdown and by a high-effort review of the SPA re-evaluation example (#51). Grouping them so the tradeoffs are considered together.

1. Public evaluate() ignored the disposed flag — being fixed in #51

The internal scheduleEvaluation guards on disposed (src/content.ts), but the public evaluate() (src/content.ts:118) did not. Once we recommend adopters drive evaluate() from their own navigation signal (a timer or router hook), a caller that forgets to tear that signal down keeps re-running a disposed controller and re-firing onDecision after dispose().

Fix (in #51): evaluate() now returns a fail-closed controller-disposed decision and does not fire onDecision once disposed. Small guard + test. This is the sharpest edge and is handled.

2. Concurrent evaluate() calls can lose-update the session store

session.ingest -> #withState does an unlocked async load -> modify -> save (src/session.ts:221-268). Through the adapter's own hooks this never overlaps, because scheduleEvaluation coalesces via a pending flag. But external evaluate() callers are not coalesced, so two overlapping evaluations (e.g. an adopter's nav hook firing while a popstate evaluation is mid-flight) can interleave: the second load reads pre-write state and its save drops the first evaluation's record. Worst case a stand-down record is lost and the extension activates on an already-attributed page.

Options:

  • Serialize #withState with a small promise-chain / mutex so writes never interleave (fixes it for all callers, including future adapters).
  • Or route public evaluate() through the same pending-style coalescing the internal path uses.

Serializing #withState is the more general fix and keeps the public API a plain evaluate().

3. Session self-exemptions have no TTL and no value/click-id scoping

(From the extension review, Rusty's comment on selfExemptionScope: 'session'.) Session exemptions are keyed by host + policyId/networkId with a grantedAt but no expiry (recordSessionExemptions/applySessionExemptions, src/session.ts:402-476). So after landing via our own CJ link, a later click on a competitor's CJ link back to the same host in the same tab (same cj policyId) gets its match stripped and we skip standing down.

This is intentional ignore_param-parity behavior today (documented at src/types.ts:214-217), and ADOPTING.md already warns to use 'session' scope only when the homegrown code actually persisted the exemption. So this is a design question, not a bug:

  • Do we want an optional TTL on session exemptions?
  • Or per-value / per-click-id scoping so an exemption only clears the same attribution it was granted for, not any later same-network click?

Lower priority than 1 and 2, and would change documented behavior, so wants a deliberate decision.

Suggested sequencing

1 lands in #51. 2 is a self-contained follow-up (serialize #withState + a concurrency test). 3 is a design discussion to schedule separately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions