Feat/jwts for personalized forms#479
Draft
amber-klaviyo wants to merge 23 commits into
Draft
Conversation
Moves the JWT parsing/validation layer (JWTParser, JWTValidationResult, ValidatedToken) to the core module as required by the updated spec. The original PR (#449) placed these in analytics; after discussion with Evan, the spec was updated so auth infrastructure lives in core. No behavioral changes — pure module relocation. Package is now com.klaviyo.core.auth. Downstream MAGE-618 work should import from there. Refs MAGE-617. Supersedes #449. Blocks MAGE-618. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vider (MAGE-627) (#459) * refactor(core): relocate auth package from analytics to core MAGE-627's AuthTokenManager skeleton and registerAuthTokenProvider public API were authored when the spec called for auth code to live in analytics. The spec was subsequently updated to put auth code in core (matching iOS's KlaviyoCore placement and the long-term direction of core as the shared- infrastructure layer); PR #458 (MAGE-617) executed the move for the JWT parser layer. This commit completes the move for the manager, provider, and test, mirroring Evan's PR #450 review note: "I'd prefer to move the entire com.klaviyo.analytics.auth package to com.klaviyo.core.auth." No behavioral changes. The four moved files are byte-identical to their PR #450 counterparts apart from the package declaration. KlaviyoAuthToken- Manager keeps its `internal` modifier — its only consumers (the test and the CoreInitProvider added in the next commit) are both in core. The public facade in Klaviyo.kt (analytics) plus the Java API test and KlaviyoMock additions are kept verbatim, only rewired against the new com.klaviyo.core.auth import path. Refs MAGE-627. Supersedes PR #450 (pending revision against this branch). * feat(core): auto-register AuthTokenManager via CoreInitProvider PR #450 registered the AuthTokenManager service inline in Klaviyo.initialize() (analytics). After moving the manager to core, that registration would cross the module boundary — KlaviyoAuthToken- Manager is `internal` to core. Adopt the established ContentProvider auto-registration pattern (forms-core, location-core), as Evan recommended in PR #450: > this Registry.registerOnce could be moved from Klaviyo.initialize to > a core-level ContentProvider auto-registration, matching the pattern > used in forms-core and location-core. That would be a nice clean way > for core dependencies to be registered from the package that owns them. CoreInitProvider mirrors FormsInitProvider verbatim: an internal ContentProvider whose onCreate registers KlaviyoAuthTokenManager into Registry. The Android manifest declares it with applicationId-scoped authorities; the consumer rule keeps it from being stripped by R8. CoreInitProviderTest mirrors FormsInitProviderTest: instantiate the provider, call onCreate(), assert the service is in Registry. Refs MAGE-627. * refactor(core): address PR review feedback on AuthTokenManager skeleton - Add `lifecycleMonitor` constructor param per MAGE-627 spec, locking the signature for MAGE-629's lifecycle-aware refresh work. - Replace `refreshObservers` placeholder with `CopyOnWriteArrayList<TokenRefreshObserver>` matching the established SDK observer-collection pattern (NetworkObserver, ActivityObserver, etc.) per Evan's PR #450 review. - Introduce `TokenRefreshObserver` typealias in `AuthTokenManager.kt`. - Drop `refreshJob` placeholder — spec type (`Clock.Cancellable?`) and current impl (`Job?`) disagree; let MAGE-629 declare the right shape when it lands. - Demote duplicate eager-fetch warning to debug and drop the exception arg (validateOrThrow already logs at ERROR) per Evan's PR #450 nit. - Add EOF newline to AndroidManifest.xml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): address remaining PR #450 review feedback on AuthTokenManager Three concerns from Evan's and Andrew's reviews, all addressed in this commit: **Typed AuthTokenException (mirrors iOS AuthTokenError)** Introduce `AuthTokenException` sealed class with `NoProviderRegistered` and `ValidationFailed(reason: String)` variants. The forms WebView consumer (MAGE-619) can now distinguish "auth not configured" from "token invalid" when shaping fallback behavior, exactly as the iOS comment anticipates. Replaces bare `IllegalStateException` throws in `invokeProvider` and `validateOrThrow`. Tests updated to assert on typed catches and inspect `ValidationFailed.reason` directly. **Remove coroutineScope from AuthTokenManager interface** Evan flagged exposing the scope on the interface as a structured-concurrency anti-pattern: it invites MAGE-619 consumers to bind their work to the wrong lifecycle. The scope moves to an `internal val scope` on `KlaviyoAuthTokenManager` (still reachable within the core module, e.g. for test cancellation), invisible outside. `registerProvider` becomes non-suspend — the manager fires the eager fetch internally — so `Klaviyo.kt` no longer needs to reach into the scope, reducing the call site to a single line. **@volatile on provider and cachedToken + non-suspend registerProvider** Evan's follow-on: `registerProvider` was async unnecessarily, creating a theoretical race on chained calls (`initialize → registerAuthTokenProvider → …`). Switch `provider`/`cachedToken` to `@Volatile` so the registration path is synchronous and any out-of-lock read in `invokeProvider` sees the latest write. Mutex retained for `currentToken`'s read-validate-write transition; MAGE-628 will replace it with Deferred-based dedup. **validateOrThrow duplicate-message nit (Evan)** `requireNotNull(error.message)` forwards the exception message to the log call rather than duplicating the string literal inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): use ValidationFailed.reason in test instead of Throwable.message Local Kotlin compile treated `e.message` as platform-typed (non-null) and warned about the unnecessary safe call; CI compile treated it as `String?` and refused the direct call. `AuthTokenException.ValidationFailed.reason` is non-null by construction — prefer it over the nullable `Throwable.message` for the reason assertion. The `spyLog.error` first-arg match works directly since `Log.error(message: String, ...)` is non-null too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#460) * feat(forms): inject JWT auth token into IAF WebView at load (MAGE-619 Phase 1) Add data-klaviyo-jwt to the InAppFormsTemplate.html head and wire up KlaviyoWebViewClient.initializeWebView() to fetch the current auth token from AuthTokenManager (via Registry) before calling loadTemplate(). Token fetch runs on the client's own CoroutineScope so the WebView can continue using the main thread for UI work. A stale-ref guard and initJob cancellation in destroyWebView() prevent loading a token onto a destroyed WebView. Blocked portions (best-effort timeout, live refresh observer) wait on MAGE-628 and MAGE-630 respectively, each marked with a TODO comment. Attribute name is data-klaviyo-jwt per Daniel's alignment note (MAGE-553 fender constant). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): address PR #460 review feedback (MAGE-619) (#461) * refactor(forms): tighten IAF auth-token injection per PR #460 review (MAGE-619) Addresses review feedback on the Phase 1 JWT-injection implementation: - HTML-escape the auth token before substituting into the single-quoted `data-klaviyo-jwt` attribute. Defense-in-depth only — KlaviyoAuthTokenManager already JWT-validates, so real tokens cannot contain the escaped characters — but this guards against test doubles or future code paths that bypass validation. Order matters: `&` is escaped first so subsequent entity introductions are not double-encoded. - Align log levels with iOS PR #577: both the success and miss branches now log at info (was debug/warning). The miss case is an expected state for apps without an AuthTokenProvider, not a degraded one; provider-side failures are still logged at error by KlaviyoAuthTokenManager. - Replace the destroy-before-resolve test with a rigorous assertion of initJob cancellation mid-fetch (CompletableDeferred suspends currentToken() so cancellation must propagate from the suspension point; the post-token stale-ref log path is asserted absent). Add a second test for pre-start cancellation, and a third for the HTML-escape behavior. - Hoist the `JWT_TOKEN` placeholder to a companion const, add a MAGE-630 TODO next to the existing MAGE-628 one at the fetch site, document scope retention as intentional across destroyWebView, and rephrase the stale-ref log to cover both destroy and GC paths (the field is held weakly via WeakReferenceDelegate). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): trim non-essential comments in IAF auth-token paths Reduce verbosity per code-review convention: keep TODOs and inline notes where the code is non-obvious, drop explanatory comments that restate self-evident behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Adjust auth token logs to debug and warning (#462) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * refactor: address second-round PR #460 review feedback (MAGE-619) (#463) * refactor(core)!: return ValidatedToken from AuthTokenManager.currentToken (MAGE-619) Per review feedback on PR #460: callers now receive the parsed wrapper rather than the raw String, so: - ValidatedToken's redacted toString() protects against accidental token logging in any consumer. - exp/iat metadata is directly available for MAGE-628 (cache-aware timeout short-circuit) and MAGE-630 (proactive refresh scheduling). - The function signature reads honestly — `currentToken(): ValidatedToken` matches what is actually constructed and cached internally. Callers wanting only the raw JWT now read `.rawToken`. ValidatedToken is promoted from internal to public; documented for cross-module use. Breaking change to a public interface introduced last week in PR #458; no external consumers yet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): extract IAF auth-token coroutine body to its own method (MAGE-619) Per review feedback on PR #460: the `safeLaunch { … }` body in `initializeWebView` is now a private suspend method, `loadTemplateWithAuthToken(webView, partialHtml, nativeBridge)`. The lambda shrinks to a single call, and the named method is easier to scan and reason about than an inline closure with two TODOs, a try/catch, two log branches, and a UI dispatch. Also adopts the new ValidatedToken return type from AuthTokenManager, unwrapping to .rawToken only at the final HTML substitution point. Minor cleanup folded in: - `dispatcher.scheduler.advanceUntilIdle()` added to the `appends asset source` test for consistency with every other initializeWebView test. - Backtick-form `AuthTokenProvider` reference in the escapeForHtmlAttribute KDoc so Dokka resolves it correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): distinguish auth no-provider from fetch failure in IAF log levels (MAGE-619) (#464) Addresses Cursor Bugbot's "Warning log fires on every form load without JWT" finding on PR #460. Previously the catch in `loadTemplateWithAuthToken` collapsed every failure — including `AuthTokenException.NoProviderRegistered`, the expected state for any app not using JWT auth — into a single warning. In debug builds (default threshold is warning+), those apps would see the warning on every IAF form load. This split matches `AuthTokenException.NoProviderRegistered`'s own KDoc intent: "Callers should treat this as 'auth is not enabled' rather than 'auth failed.'" The sealed exception hierarchy was built for this distinction; only the call site needed to use it. - `KlaviyoWebViewClient`: introduces a small file-private `AuthOutcome` sealed interface (`Success` / `NotEnabled` / `FetchFailed`). `NoProviderRegistered` → debug ("Auth not enabled"); any other thrown exception → warning ("Auth token fetch failed"). - Tests: default mock now throws the real `AuthTokenException.NoProviderRegistered` data object instead of a generic `RuntimeException`. The single previous miss-path test is split into two — one for the no-provider debug case, one for a provider-fetch-failed warning case (via `ValidationFailed`). No public API change. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix stale WebView auth-injection logging order (#465) * Fix auth injection log ordering for stale WebView Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * style(forms): wrap long verify-block argument list in WebView client test Fixes ktlint argument-list-wrapping violations on line 557. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com>
Advertises jwtMutation support to the fender JS layer at WebView init time. Without this entry fender does not know the SDK can push token updates and won't wire up that path. Full live-refresh implementation (calling window.jwtMutation on token refresh) tracked in MAGE-630. Requested by Dan Peluso in PR #460 post-merge review. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
) * feat(core): AuthTokenManager deduplication and timeouts (MAGE-628) Concurrent callers that miss the cache now share a single in-flight Deferred<ValidatedToken> rather than each invoking the provider independently. Per-caller timeouts (5 s background / 500 ms interactive) are enforced via withTimeoutOrNull without cancelling the shared task, so a later caller still benefits if the provider eventually responds. Provider swaps cancel any in-flight fetch and use a reference-identity check in invokeOnCompletion to prevent a stale deferred from clearing a freshly-created slot. TimedOut is a new AuthTokenException variant that is logged at ERROR, mirroring iOS AuthTokenError.timedOut. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): mark inFlightFetch @volatile and clarify cancel guards inFlightFetch is read/written both under the mutex (currentToken) and outside it (registerProvider, invokeOnCompletion). Without @volatile those out-of-mutex writes are not guaranteed visible to the mutex- protected read. Marking it @volatile restores the thread-safety contract the rest of the class follows (@volatile cachedToken, @volatile provider). Also expand the registerProvider comment to document both cancellation guards: the isActive check in suspendCancellableCoroutine covers late provider callbacks, and mutex.withLock (a suspension point) catches the case where the callback already fired but the cache write hasn't run yet. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): address review feedback on AuthTokenManager (MAGE-628) - Add ensureActive() before mutex.withLock in doFetch() to guard against stale token writes when a provider swap cancels the deferred after invokeProvider() returns but before the uncontended mutex.withLock runs - Add regression test exercising the uncontended-mutex cancellation path - Add require(timeoutMs > 0L) guard in currentToken() to surface bad callers with a clear IllegalArgumentException - Add provider == null fast-path in currentToken() to avoid creating a Deferred that immediately fails (mirrors Swift's guard provider != nil) - Remove redundant DEBUG log in tryEagerFetch(); failure already logged at ERROR by validateOrThrow / timeout path - Fix ResolvableProvider test helper to queue callbacks so concurrent invocations don't clobber each other - Update Guard #2 comment and KDoc to reflect ensureActive() fix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): address second-pass Bugbot feedback on AuthTokenManager - Wrap deferred.await() in currentToken() with a CancellationException catch that calls currentCoroutineContext().ensureActive() to distinguish "caller's coroutine was cancelled" (rethrow) from "in-flight deferred was cancelled by a provider swap" (retry transparently). Without this, a provider swap while callers are suspended would propagate a spurious CancellationException to them — in KlaviyoWebViewClient this silently cancelled initJob, causing the form to never load. - Downgrade TimedOut log from error() to warning() per AGENTS.md: a timeout with a graceful fallback is degraded-with-fallback, not an unrecoverable failure. Also reduces duplicate logging with the KlaviyoWebViewClient caller that already logs the fallback at warning. - Add regression test covering the provider-swap-while-awaiting path - Update timeout test's verify to expect warning() instead of error() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update sdk/core/src/test/java/com/klaviyo/core/auth/KlaviyoAuthTokenManagerTest.kt Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com> * Update sdk/core/src/main/java/com/klaviyo/core/auth/AuthTokenManager.kt Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com> * Update sdk/core/src/main/java/com/klaviyo/core/auth/AuthTokenException.kt Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com> * Update sdk/core/src/main/java/com/klaviyo/core/auth/AuthTokenManager.kt Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com> * Update sdk/core/src/main/java/com/klaviyo/core/auth/AuthTokenManager.kt Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com> * test(core): add timeout-budget test and clarify retry comment (MAGE-628) Add a test verifying that when a provider swap triggers the CancellationException retry inside currentToken(), the caller's original withTimeoutOrNull deadline still governs the total wait. The retry creates a fresh inner withTimeoutOrNull(timeoutMs) starting from the current time, but the outer one fires first when the budget is nearly exhausted — this is the intended behavior (caller asked for a response within timeoutMs of their call site, not of the swap). Also expand the retry comment to make this budget semantics explicit, addressing Bugbot's "swap retry shares timeout budget" flag and evan's review request for a test covering this path. Also fix trailing whitespace in AuthTokenManager.kt KDoc left by a suggestion commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Evan C Masseau <5167687+evan-masseau@users.noreply.github.com>
…469) * feat(core): AuthTokenManager proactive refresh scheduling (MAGE-629) Schedules a proactive token refresh at 90% of the token's lifetime using Registry.clock (java.util.Timer-backed, wall-clock-aware). A FirstStarted lifecycle observer reconciles cache and refresh state on foreground transitions: clears and re-fetches an expired token, fires a missed scheduled refresh, or no-ops if everything is still valid. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Preserve cached token when proactive refresh fails (#470) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * Fix auth refresh foreground race (#471) * Fix auth refresh foreground race Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * Format auth refresh test assertion Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * Fix auth refresh foreground timer race Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> * review improvements * Update sdk/core/src/main/java/com/klaviyo/core/auth/KlaviyoAuthTokenManager.kt Co-authored-by: Andrew Balmer <ab1470@users.noreply.github.com> * refactor(core): address PR review feedback on AuthTokenManager proactive refresh - Rename currentTokenInternal → getOrFetchToken to better reflect its dual role (return cached token OR force a fresh provider fetch) - Fix dead [mutex] KDoc bracket-link in a plain // comment (drop brackets) - Promote scheduleRefresh / performScheduledRefresh / handleForegroundTransition method-level // blocks to /** */ KDoc so [symbol] links resolve in the IDE - Add 5 tests covering edge cases requested in review: - timeout during scheduled refresh leaves one foreground retry - demand caller joins in-flight fetch when cache expires mid-refresh - second foreground after successful retry is a no-op - computeRefreshTarget degrades gracefully when exp precedes iat - foreground during in-flight refresh that subsequently fails defers retry to the next foreground (BugBot scenario 3 documented) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Amber Wallace <amber-klaviyo@users.noreply.github.com> Co-authored-by: Andrew Balmer <ab1470@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
amber-klaviyo
added
the
do-not-merge
…24) (#475) * refactor(forms): deliver JWT via window.jwtMutation JS bridge (MAGE-724) Previously the JWT was injected as a static data-klaviyo-jwt HTML attribute before the WebView loaded. The onsite personalization module reads that attribute at init and calls setJWT before profile identifiers exist, so the authenticated /api/profiles/ fetch never fired. Deliver the JWT over the same JS bridge mechanism as profile data, at an explicit, ordered trigger point: - window.jwtMutation(token) added to klaviyo-js-bridge.js, mirroring window.profileMutation - JwtObserver injects the token at JsReady (before HandShook) - ProfileMutationObserver moved to HandShook so identifiers arrive after the JWT is set, letting onsite trigger the authenticated fetch - KlaviyoWebViewClient no longer fetches/injects the token at load; template loads synchronously again Live token refresh on rotation remains out of scope (MAGE-630). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): guard null JWT injection and double-start race in JwtObserver - Make JsBridge.jwtMutation(token: String) non-nullable; JwtObserver now passes token ?: "" so a missing token sets data-klaviyo-jwt="" (empty string) instead of the literal "null" that DOM setAttribute would have produced when given a JS null value. - Add fetchJob?.cancel() at the top of JwtObserver.startObserver() to cancel any in-flight fetch if the observer is started again before the previous job completes. - Update KDoc/jsbridge comment and all affected tests accordingly; add a double-start test that confirms only the second fetch's token is injected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): coordinate JWT/profile ordering with a shared CompletableDeferred JwtObserver now holds a CompletableDeferred<Unit> (jwtReady) that it completes after jwtMutation() fires on the UI thread, or cancels in stopObserver() if the WebView is torn down before delivery. ProfileMutationObserver accepts the same deferred and, when provided, awaits it inside a coroutine before its initial injectProfile() call. This eliminates the residual race where a slow async token fetch completes after HandShook fires, causing profile to land with an empty JWT attribute and skipping the authenticated profile fetch. KlaviyoObserverCollection creates the shared deferred and passes it to both observers. The no-arg constructor paths (used by existing unit tests) are unchanged — ProfileMutationObserver defaults to null and stays synchronous when no deferred is provided. New tests in JwtObserverTest cover jwtReady completion/cancellation. ProfileMutationObserverAsyncTest (new, extends BaseTest) covers the await-before-inject and cancelled-deferred-skips-inject paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): harden JwtObserver/ProfileMutationObserver against WebView reinit Three related lifecycle bugs introduced by the JWT coordination pattern: 1. Stale post-suspension injection: after currentToken() returns, cooperative cancellation of fetchJob can no longer prevent the queued runOnUiThread callback from firing. Add a @volatile stopped flag and a deferred identity check (jwtReady === currentJwtReady) inside the runOnUiThread block so a callback from a previous session can never inject into a destroyed or new WebView. 2. jwtReady deferred not reset on reinit: the shared CompletableDeferred was a permanent val that stopObserver cancelled, making complete() a no-op on the next session. JwtObserver now creates a fresh CompletableDeferred in startObserver() and exposes it as a @volatile var; ProfileMutationObserver reads jwtReady from the JwtObserver reference at startObserver time so it always awaits the deferred for the active session. 3. ProfileMutationObserver scope permanently cancelled: stopObserver() called scope.cancel() which permanently destroyed the scope, preventing future startObserver() coroutines from launching. Replaced with per-session job tracking (initJob) — only initJob is cancelled in stopObserver(), leaving the scope alive for reinit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): address CodeRabbit feedback on JwtObserver coordination (#480) * fix(forms): address CodeRabbit feedback on JwtObserver coordination - Reuse jwtReady when still pending so a double-start without an intervening stopObserver does not orphan a ProfileMutationObserver waiter that captured the prior deferred. Swap the identity check inside the UI callback for an isCompleted guard so only the newest fetch's token wins. - Import JwtObserver in KlaviyoWebViewClient and use the short KDoc reference per the repo Kotlin style. - Add existence asserts in the JwtObserver-before-ProfileMutationObserver ordering test to prevent a -1 index from silently satisfying the comparison. - Add a multi-session ProfileMutationObserverAsyncTest that fails if jwtReady is captured at construction instead of read fresh per startObserver call. - Update related KDoc and the KlaviyoObserverCollection comment to reflect the reuse-when-pending invariant. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(forms): trim verbose comments from JwtObserver and its tests Drop narration-style comments that just describe what the adjacent code already says, and remove ticket-ID references that belong in the PR description rather than the source. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(forms): gate JWT injection on the latest fetch identity A reused jwtReady made the !isCompleted guard insufficient: a stale fetchJob whose UI callback was queued before the next startObserver would inject the older token and complete the shared deferred, silencing the newer fetch. Track the active fetch via a sentinel captured at startObserver entry and only inject when the queued callback still matches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * Update sdk/forms/src/main/java/com/klaviyo/forms/webview/KlaviyoWebViewClient.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…E-630) (#477) * refactor(forms): deliver JWT via window.jwtMutation JS bridge (MAGE-724) Previously the JWT was injected as a static data-klaviyo-jwt HTML attribute before the WebView loaded. The onsite personalization module reads that attribute at init and calls setJWT before profile identifiers exist, so the authenticated /api/profiles/ fetch never fired. Deliver the JWT over the same JS bridge mechanism as profile data, at an explicit, ordered trigger point: - window.jwtMutation(token) added to klaviyo-js-bridge.js, mirroring window.profileMutation - JwtObserver injects the token at JsReady (before HandShook) - ProfileMutationObserver moved to HandShook so identifiers arrive after the JWT is set, letting onsite trigger the authenticated fetch - KlaviyoWebViewClient no longer fetches/injects the token at load; template loads synchronously again Live token refresh on rotation remains out of scope (MAGE-630). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): guard null JWT injection and double-start race in JwtObserver - Make JsBridge.jwtMutation(token: String) non-nullable; JwtObserver now passes token ?: "" so a missing token sets data-klaviyo-jwt="" (empty string) instead of the literal "null" that DOM setAttribute would have produced when given a JS null value. - Add fetchJob?.cancel() at the top of JwtObserver.startObserver() to cancel any in-flight fetch if the observer is started again before the previous job completes. - Update KDoc/jsbridge comment and all affected tests accordingly; add a double-start test that confirms only the second fetch's token is injected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): coordinate JWT/profile ordering with a shared CompletableDeferred JwtObserver now holds a CompletableDeferred<Unit> (jwtReady) that it completes after jwtMutation() fires on the UI thread, or cancels in stopObserver() if the WebView is torn down before delivery. ProfileMutationObserver accepts the same deferred and, when provided, awaits it inside a coroutine before its initial injectProfile() call. This eliminates the residual race where a slow async token fetch completes after HandShook fires, causing profile to land with an empty JWT attribute and skipping the authenticated profile fetch. KlaviyoObserverCollection creates the shared deferred and passes it to both observers. The no-arg constructor paths (used by existing unit tests) are unchanged — ProfileMutationObserver defaults to null and stays synchronous when no deferred is provided. New tests in JwtObserverTest cover jwtReady completion/cancellation. ProfileMutationObserverAsyncTest (new, extends BaseTest) covers the await-before-inject and cancelled-deferred-skips-inject paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): harden JwtObserver/ProfileMutationObserver against WebView reinit Three related lifecycle bugs introduced by the JWT coordination pattern: 1. Stale post-suspension injection: after currentToken() returns, cooperative cancellation of fetchJob can no longer prevent the queued runOnUiThread callback from firing. Add a @volatile stopped flag and a deferred identity check (jwtReady === currentJwtReady) inside the runOnUiThread block so a callback from a previous session can never inject into a destroyed or new WebView. 2. jwtReady deferred not reset on reinit: the shared CompletableDeferred was a permanent val that stopObserver cancelled, making complete() a no-op on the next session. JwtObserver now creates a fresh CompletableDeferred in startObserver() and exposes it as a @volatile var; ProfileMutationObserver reads jwtReady from the JwtObserver reference at startObserver time so it always awaits the deferred for the active session. 3. ProfileMutationObserver scope permanently cancelled: stopObserver() called scope.cancel() which permanently destroyed the scope, preventing future startObserver() coroutines from launching. Replaced with per-session job tracking (initJob) — only initJob is cancelled in stopObserver(), leaving the scope alive for reinit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(core): AuthTokenManager refresh observers and profile reset (MAGE-630) Adds the onTokenRefresh/offTokenRefresh observer API so consumers (e.g. the forms WebView) can subscribe to live token updates from proactive refreshes. Adds clearTokenState() which Klaviyo.resetProfile() calls on logout — discards the cached token and cancels in-flight work while retaining the provider (host integration code, not user identity) and registered observers (active form displays survive a profile reset). Observer dispatch is best-effort: exceptions are logged at WARNING and remaining observers are still called. A stale-token guard prevents a refresh that raced with clearTokenState() from broadcasting a stale profile's token to active subscribers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): close two reset-race windows in AuthTokenManager Adds a synchronous invalidate() step to resetProfile() so the profile generation is bumped on the calling thread before the async clearTokenState() is dispatched. Two races are closed: 1. (High) A proactive refresh completing between State.reset() and clearTokenState() could notify TokenRefreshObservers with the logged-out user's JWT. invalidate() advances profileGeneration immediately, causing performScheduledRefresh's new post-fetch guard to skip observer dispatch even before clearTokenState() runs. 2. (Medium) clearTokenState() landing after a rapid registerAuthTokenProvider() call could wipe the new session's token cache and cancel its refresh job. clearTokenState(expectedGeneration) now bails out if registerProvider() has advanced profileGeneration past the captured value, preserving the new session's state. The profileGeneration counter is kept separate from refreshGeneration so that scheduleRefresh()'s routine increments do not produce false negatives in the observer-dispatch guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): replace profileGeneration observer guard with profileResetPending flag The previous fix used a captured-generation approach in performScheduledRefresh to prevent stale observer dispatch after resetProfile(). Bugbot identified a gap: a refresh that starts *after* invalidate() captures the post-invalidate generation and passes the check, since clearTokenState() hasn't run yet to advance the counter. Replace the generation capture with a @volatile boolean profileResetPending: - invalidate() sets it true before returning (synchronous, on the calling thread) - clearTokenState() and registerProvider() reset it to false - performScheduledRefresh reads it at notification time; any pending reset — whether the refresh started before or after invalidate() — causes the dispatch to be skipped Adds Scenario B test covering the specific gap: timer fires after invalidate() and completes before clearTokenState() runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(forms): address CodeRabbit feedback on JwtObserver coordination (#480) * fix(forms): address CodeRabbit feedback on JwtObserver coordination - Reuse jwtReady when still pending so a double-start without an intervening stopObserver does not orphan a ProfileMutationObserver waiter that captured the prior deferred. Swap the identity check inside the UI callback for an isCompleted guard so only the newest fetch's token wins. - Import JwtObserver in KlaviyoWebViewClient and use the short KDoc reference per the repo Kotlin style. - Add existence asserts in the JwtObserver-before-ProfileMutationObserver ordering test to prevent a -1 index from silently satisfying the comparison. - Add a multi-session ProfileMutationObserverAsyncTest that fails if jwtReady is captured at construction instead of read fresh per startObserver call. - Update related KDoc and the KlaviyoObserverCollection comment to reflect the reuse-when-pending invariant. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(forms): trim verbose comments from JwtObserver and its tests Drop narration-style comments that just describe what the adjacent code already says, and remove ticket-ID references that belong in the PR description rather than the source. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(forms): gate JWT injection on the latest fetch identity A reused jwtReady made the !isCompleted guard insufficient: a stale fetchJob whose UI callback was queued before the next startObserver would inject the older token and complete the shared deferred, silencing the newer fetch. Track the active fetch via a sentinel captured at startObserver entry and only inject when the queued callback still matches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * fix(auth): apply CodeRabbit review fixes to AuthTokenManager (#481) - Move auth.invalidate() before State.reset() in resetProfile() to close the race where a proactive refresh completing between the two calls could notify observers with the outgoing profile's JWT - Narrow catch(Throwable) → catch(Exception) in notifyRefreshObservers so JVM-fatal Errors (OOM, StackOverflowError) propagate instead of being swallowed - Update onTokenRefresh KDoc to clarify that CancellationException is rethrown - Assert specific expectedGeneration=1L in KlaviyoTest to pin the invalidate stub value - Replace firstOrNull()+null-guard with first() in retain-observers test since a timer task is guaranteed at that point in the test - Relax warning-log assertion from substring match to any() to avoid brittle pinning Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(core): guard currentToken cache hit against profileResetPending After invalidate() sets profileResetPending=true, currentToken() could still serve the outgoing JWT from cachedToken until the async clearTokenState() ran. Add !profileResetPending to both cache-hit paths in getOrFetchToken() (optimistic read and mutex re-check) so token acquisition falls through to a fresh provider fetch during the reset window. Add a regression test: after invalidate(), a currentToken() call must invoke the provider rather than returning the stale cached value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| private fun injectAndSubscribe() { |
There was a problem hiding this comment.
Logout leaves stale JWT in webview
Medium Severity
After a form closes, the WebView is often detached but not destroyed, so bridge observers (including ProfileMutationObserver) stay subscribed. Klaviyo.resetProfile() clears analytics state and auth token cache, and ProfileMutationObserver pushes an empty profile on ProfileReset, but nothing clears data-klaviyo-jwt in the page. The previous session’s JWT can remain while profile identifiers are cleared or replaced, so onsite personalization can see mismatched auth and profile data.
Reviewed by Cursor Bugbot for commit c608523. Configure here.
…684) (#478) * feat(core): AuthTokenManager connectivity-driven refresh retry (MAGE-684) When the proactive refresh path fails with a network-classified exception (IOException, UnknownHostException, SocketTimeoutException, ConnectException), arm a single connectivityWaitJob on the manager's existing scope that suspends until the next "connectivity available" notification from Registry.networkMonitor, then re-triggers performScheduledRefresh. At most one job is pending at a time — rapid flap is handled by cancelling any existing job before arming a new one, guarded by a monotonic generation counter so the old job's finally block cannot null out the new job. clearTokenState() and registerProvider() cancel and clear the job as part of their existing teardown. Non-network failures (HTTP errors, validation failures) do not arm the retry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): address connectivity retry race conditions and test quality (#482) * fix(auth): address connectivity retry race conditions and test quality - Serialize all connectivityWaitJob/connectivityWaitGeneration transitions under a dedicated connectivityWaitLock so concurrent arm/cancel sequences from rapid flap, registerProvider, and clearTokenState cannot produce multiple active jobs or leave a stale field assignment - Guard armConnectivityWaitJob call site with provider != null && !profileResetPending so a proactive refresh failing after invalidate() does not arm a zombie retry job - Fix observer registration/cancellation race: install invokeOnCancellation after onNetworkChange (prevents removing an unregistered observer), and use AtomicBoolean for one-shot observer semantics instead of a non-atomic isActive check - Add currentCoroutineContext().ensureActive() before the post-connectivity retry so a provider swap that cancels the job after the observer fires does not kick off stale refresh work - Resume connectivity wait immediately when the device is already online (NetworkMonitor does not replay state on registration; fixes SocketTimeoutException hanging indefinitely on a flaky-but-connected network) - Tests: extract executeScheduledRefresh() helper, consolidate the four exception-variant tests into assertConnectivityRetryFires() helper, remove log message substring matching in verifies, replace !! with null-safe calls, remove unused observerSlot field, add tests for profileResetPending guard and already-online path, make FakeNetworkMonitor.isNetworkConnected() controllable Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor(auth): extract usableCachedToken() helper to deduplicate cache eligibility check The condition token != null && isStillValid(token) && !profileResetPending appeared in both the optimistic pre-lock read and the mutex-protected double-check in getOrFetchToken(). Extract it into usableCachedToken() so both sites stay in sync and cyclomatic complexity of getOrFetchToken() is reduced. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(auth): prevent immediate-retry loop and fix FakeNetworkMonitor state sync Two CodeRabbit fixes on the connectivity-driven refresh retry path: 1. **Loop prevention**: When `armConnectivityWaitJob` fires on a device that is already online, the first arm resumes immediately (resumeImmediatelyIfConnected=true). If the retry also fails with a network exception, the re-armed job passes `resumeImmediatelyIfConnected=false` so it waits for a genuine connectivity transition rather than spinning in a tight loop. `performScheduledRefresh` now accepts `allowImmediateConnectivityRetry=true` and threads it through as the flag; the connectivity-retry call path always passes `false`. 2. **FakeNetworkMonitor state sync**: `simulateConnected(isConnected)` now updates `connected = isConnected` before notifying observers, keeping `isNetworkConnected()` consistent with what observers receive. New test: `persistent provider failure on connected device arms a waiting job not a tight loop` — covers the loop-prevention scenario end-to-end. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): guard connectivity retry arm against mid-fetch profile transitions Snapshot profileGeneration at the start of performScheduledRefresh and require it to still match in the catch block before arming a connectivity wait job. Without this guard, a refresh that starts before registerProvider/invalidate/ clearTokenState can fail *after* those transitions complete — at which point profileResetPending has been cleared and provider is non-null for the new profile, so the old failure would incorrectly arm a connectivity retry on behalf of the stale session. New test: mid-fetch profile transition (simulated by calling invalidate() from inside the provider callback) — verifies no connectivity job is armed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(auth): extract shouldArmConnectivityRetry helper Pulls the four-part arming-eligibility guard out of the catch block in performScheduledRefresh into a named private helper for readability. No behavior change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): use resetGeneration to avoid false-positive retry suppression on provider swap Introduce a dedicated resetGeneration counter that is bumped only by invalidate() and clearTokenState(), not by registerProvider(). shouldArmConnectivityRetry now compares against a resetGenerationAtStart snapshot instead of profileGeneration. Previously, a provider swap (registerProvider without a logout) mid-fetch would bump profileGeneration, causing shouldArmConnectivityRetry to return false and silently drop the connectivity retry for the new active session. The new session would eventually retry via its own scheduled timer, but this was an unnecessary blind spot. resetGeneration correctly distinguishes a benign provider swap from a logout-triggered state clear. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> * test(auth): use firstOrNull with explicit AssertionError in executeScheduledRefresh Addresses CodeRabbit's suggestion: first() throws a generic NoSuchElementException when no tasks are scheduled, hiding the test intent. firstOrNull() + AssertionError gives a clear failure message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(core): public unregisterAuthTokenProvider API (MAGE-686) Adds Klaviyo.unregisterAuthTokenProvider() so host apps can detach the provider on logout. Forwards to AuthTokenManager.unregisterProvider(), which mirrors registerProvider()'s synchronous teardown — cancels any in-flight fetch, scheduled refresh, and connectivity-wait job, then clears the cached token and nulls the provider reference. resetGeneration is incremented so any performScheduledRefresh that was in-flight at unregister time does not arm a zombie connectivity retry job on behalf of the now-cleared session. Tests cover the idle, pending-refresh, in-flight-deferred, and connectivity-wait-job scenarios, plus re-register after unregister and Java interop via @JvmStatic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): address review feedback on unregisterProvider - Early-return in unregisterProvider() when provider is already null, making the "has no effect if no provider is registered" doc literally accurate and suppressing the spurious INFO log on repeated calls - Extend inline comment to document the accepted non-mutex teardown trade-off (same rationale as registerProvider) - Clarify the staticClock.scheduledTasks.clear() comment in the re-register test to avoid implying residual tasks are expected Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): null provider before teardown in unregisterProvider Move `provider = null` to the top of `unregisterProvider()` so any concurrent `getOrFetchToken()` path that races past the initial `if (provider == null)` check lands in `invokeProvider()` and receives `NoProviderRegistered` immediately, rather than starting a fresh acquisition against the now-unregistered provider. This matches the spec's stated ordering rationale and the equivalent iOS implementation, where the provider is cleared before token state teardown for the same reason. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(auth): null cachedToken before clearing profileResetPending in unregisterProvider An in-flight performScheduledRefresh that has already fetched its token checks cachedToken and !profileResetPending before notifying observers. If invalidate() was called just before unregisterProvider() (typical logout: resetProfile → unregisterAuthTokenProvider), profileResetPending is true. The previous ordering — clear profileResetPending then null cachedToken — opened a window where the refresh could pass both guards and deliver a now-invalid JWT to observers. Nulling cachedToken first closes the window: the rawToken comparison fails (null != token), so the notify path is skipped regardless of profileResetPending. Matches the ordering in clearTokenState(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Fender's forms webview posts {"type":"BadJWT","data":{}} over the native
bridge when it rejects an injected JWT (e.g. bad signature or a token that
does not resolve to a profile). NativeBridgeMessage had no case for it, so
decodeWebviewMessage threw IllegalStateException, which KlaviyoNativeBridge
caught and logged at error level. A rejected JWT is a normal outcome, so
logging it as an error was misleading.
Add a BadJwt message type and handle it as a graceful no-op logged at
warning, matching iOS's degrade-don't-throw behavior. The wire type is
PascalCase ("BadJWT"), which breaks the lower-camelCase keyName convention,
so it is matched against an explicit literal.
Not added to the handshake: verified against fender that BadJWT delivery is
gated on the SDK advertising jwtMutation, never on BadJWT itself, so a
handshake entry would have no effect.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
KDoc inside a function body documents no declaration and its @see tag is not tooling-navigable, so a plain comment is idiomatic here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JwtObserver only delivered the JWT once at JsReady via a one-shot currentToken() fetch, and never subscribed to AuthTokenManager's refresh stream. When the token was proactively refreshed while a form was displayed, the new JWT was broadcast to zero listeners and the webview kept the stale (eventually expired) token. Subscribe to AuthTokenManager.onTokenRefresh in startObserver() and unsubscribe in stopObserver(). The refresh handler hops to the UI thread and calls jwtMutation with the fresh token, guarded by the existing stopped flag. Registration uses off-then-on to guarantee a single subscription across re-entrant starts. The initial-delivery path and jwtReady completion are unchanged, preserving JWT-before- profile ordering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Claim the initial fetch's injectionSequence in startObserver at request time rather than when currentToken resolves, so a proactive refresh that lands while the fetch is in flight always outranks it. Prevents a slow or failed initial fetch from injecting an empty JWT over a fresher refreshed token. - Add a test covering the failed-fetch-while-refresh-in-flight race. - Extract captureRefreshObserver() test helper to DRY up refresh-observer capture setup. -Claude
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e95e0a3. Configure here.
…GE-875) Observers were only notified from performScheduledRefresh, so a token that resolved after a caller's interactive-timeout fetch gave up was cached silently and never broadcast. A form displayed while the initial fetch was still in flight would keep its empty JWT until a far-future proactive refresh. Move the notification into doFetch, which every acquisition path funnels through (initial demand fetch and proactive refresh alike) and which is deduped, so each fetch broadcasts exactly once under the existing stale guard (cachedToken match + !profileResetPending). Drop the now-redundant notify from performScheduledRefresh, since it always resolves through a doFetch. Updates the onTokenRefresh contract docs accordingly and revises the observer tests that asserted the eager fetch stayed silent, plus adds a regression test for the slow-initial-fetch scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that the auth manager broadcasts on every acquisition, a fast initial
fetch that resolves within the interactive budget delivers the same token
both as its own currentToken() result and via the onTokenRefresh echo.
Track the last injected value in JwtObserver and skip the bridge call when
it is unchanged, so onsite sees each distinct token exactly once. The
empty ("unauthenticated") initial token is still delivered since the
tracked value starts null.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The value-dedup added for the double-injection persisted lastInjectedToken across form sessions on the shared JwtObserver instance. Since each session loads a fresh webview with no JWT, reopening a form while the same token was still valid would skip injection entirely, leaving onsite without credentials until the token string changed. Clear the dedup baseline at the start of each session (consumed on the UI thread so lastInjectedToken stays single-threaded). The monotonic sequence guard already handled restarts correctly; only the value check needed session scoping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A proactive-refresh echo dispatched during one form session could run its UI callback after a stop/start cycle flipped `stopped` back to false, injecting a stale token (and consuming the dedup reset) into the freshly loaded webview. The fetch path already re-checks the per-session `latestFetch` identity token on the UI thread; apply the same guard to the refresh path so a callback from a previous session is dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-875) The notify-time stale guard moved from performScheduledRefresh into doFetch earlier in this branch, but two inline comments (the profileResetPending field and the unregisterProvider cache-ordering note) still named performScheduledRefresh as the check site. Update them so the guard's location is discoverable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
DO NOT MERGE
Just using this to keep the feature branch up-to-date with master during feature development