feat(lifecycle): LifecycleEventEmitter actor + unit tests [2/4]#43
Merged
Conversation
This was referenced Apr 27, 2026
Slice 1 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the storage layer and shared types that the upcoming emitter / wiring slices depend on. Pure additions — no behavior change to AnalyticsClient, no events emitted yet. - LifecycleStorage: UserDefaults wrapper for (version, build) under the metarouter:lifecycle:* namespace, separate from identity keys so reset() cannot wipe install/update state. - IdentityStorage.hasAnyValue(): helper for the emitter (slice 2) to snapshot identity presence at construction time, before IdentityManager.initialize() auto-creates an anonymousId. - AppContext: gains Equatable + fromBundle(_:) — single source of truth for app metadata, replacing the per-event Bundle.main.infoDictionary reads that DeviceContextProvider does today (consumed in slice 3). - AppForegroundState enum: platform-neutral active/inactive/background trichotomy used by the emitter and coordinator. Ticket: sc-38228 Parent: sc-36764 Stack: this PR -> sc-38229 -> sc-38230 -> sc-38231
choudlet
force-pushed
the
chrish/sc-38228/lifecycle-storage-and-types
branch
from
ce63c9c to
31063e3
Compare
choudlet
force-pushed
the
chrish/sc-38229/lifecycle-event-emitter
branch
from
e072bd0 to
aaac4fc
Compare
Code review follow-up (sc-38228 M1). The `clear()` method is a test-only seam, not part of the SDK's public contract. Marking it `public` would expose a "wipe install/update state" affordance to consumers that contradicts the entire rationale for the `metarouter:lifecycle:*` namespace separation: nothing — not even `reset()` — should be able to wipe this state. `@testable import MetaRouter` already gives test code access to `internal` symbols, so this is purely a tightening of the public surface with no functional change.
Slice 2 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the lifecycle event emitter — the actor that owns install/update detection, the cold-launch state machine, foreground/background transitions, and the one-shot deep-link buffer. Standalone in this slice; not yet wired into AnalyticsClient. The emitter exposes four entrypoints: - emitColdLaunchSequence(initialAppState:): decides Installed vs Updated vs no-op based on persisted (version, build) and identity-existed- before-init snapshot, then conditionally emits Application Opened with from_background:false (suppressed for background-launched processes; the next true foreground entry emits the deferred Opened via the cold-launch bridge). - emitForegroundFromBackground(): handles bridge case, suppresses the first didBecomeActive during init, filters inactive→active, and emits Application Opened with from_background:true on real background→active transitions. - emitBackgrounded(): updates lastTrackedAppState and emits Application Backgrounded. - openURL(url:sourceApplication:): one-shot buffer (last-write-wins), consumed by the next emitOpened. State machine flags (coldLaunchEmitted, coldLaunchSuppressed, lastTrackedAppState, pendingDeepLink) are serialised through actor isolation. 22 unit tests cover the install/update decision tree, cold-launch sequencing, resume scenarios, deep-link buffer semantics, and double- emit suppression. Ticket: sc-38229 Parent: sc-36764 Stack: sc-38228 -> this PR -> sc-38230 -> sc-38231
Code review follow-ups (sc-38229).
- Guard emitBackgrounded against firing before the cold-launch sequence
runs. Race scenario: process woken in .background, observer already
registered, didEnterBackground arrives before the async
initTask → onReady chain. Without the guard, we'd emit Backgrounded
with no preceding Opened — a spec violation.
- Add log line on the cold-launch bridge emit (suppressed cold launch
→ first true foreground entry). This path is rare and notoriously
hard to diagnose in field reports; one log line earns its keep.
- Add log line when openURL overwrites an existing pending deep link
(last-write-wins), making field debugging of "why did Opened carry
a different URL than I expected" tractable.
- Drop the unnecessary lastTrackedAppState = .active write on the
pre-cold-launch suppressed branch. The cold-launch path is the
canonical source of truth for this flag (set in
emitColdLaunchSequence); writing it during the suppression window
is a no-op at best and hides intent at worst.
- Add 5 missing test branches:
- Cold launch with initialAppState == .inactive (distinct from
.background) suppresses + bridges
- Deep-link buffered before a suppressed cold launch survives the
suppression and attaches to the bridge Opened
- emitBackgrounded does NOT consume the deep-link buffer (one-shot
per Application Opened, not per any-emit)
- Same-bundle no-op cold launch still persists (version, build) to
storage (regression guard against accidental skip)
- Backgrounded before cold launch is suppressed (the new guard)
Test count goes from 438 -> 443. All passing.
choudlet
force-pushed
the
chrish/sc-38229/lifecycle-event-emitter
branch
from
aaac4fc to
7052d4a
Compare
brandon-metarouter
approved these changes
Apr 28, 2026
Collaborator
brandon-metarouter
left a comment
brandon-metarouter
left a comment
There was a problem hiding this comment.
Some more take it or leave its!
Comment on lines
+76
to
+87
| if hadIdentityBeforeInit { | ||
| // Existing user upgrading from a pre-lifecycle SDK build — | ||
| // avoid spurious install spike for the upgraded population. | ||
| await emit( | ||
| LifecycleEventNames.applicationUpdated, | ||
| properties: [ | ||
| LifecycleEventProperties.version: .string(curr.version), | ||
| LifecycleEventProperties.build: .string(curr.build), | ||
| LifecycleEventProperties.previousVersion: .string("unknown"), | ||
| LifecycleEventProperties.previousBuild: .string("unknown"), | ||
| ] | ||
| ) |
Collaborator
There was a problem hiding this comment.
Good catch with this this case of no prev version / build + has anon id. 💪
Comment on lines
+168
to
+171
| let event = await enrichmentService.createTrackEvent( | ||
| event: LifecycleEventNames.applicationBackgrounded, | ||
| properties: nil | ||
| ) |
Collaborator
There was a problem hiding this comment.
Should this also use emit vs calling enrichmentService.createTrackEvent directly?
| /// Buffers a deep-link URL (and optional source application) to be attached | ||
| /// to the next `Application Opened` event. One-shot — cleared on emit. | ||
| /// Last-write-wins if called multiple times before the next Opened. | ||
| func openURL(url: String, sourceApplication: String?) { |
Collaborator
There was a problem hiding this comment.
Just a naming callout, openURL feels like it would do something, but it is just setting (at least currently). No change request, just stood out to me.
Collaborator
Author
There was a problem hiding this comment.
good callout switching this over to recordOpenedUrl
choudlet
changed the base branch from
chrish/sc-38228/lifecycle-storage-and-types
to
main
choudlet
added a commit
that referenced
this pull request
May 11, 2026
#44) * feat(lifecycle): wire emitter into AnalyticsClient + openURL public API Slice 3 of 4 of the iOS lifecycle events feature (parent: sc-36764). Connects the emitter (slice 2) to AnalyticsClient via a new LifecycleCoordinator and ships the public openURL deep-link API. After this PR, the feature works end-to-end behind trackLifecycleEvents=true. Default stays false so no existing customer is impacted on upgrade. LifecycleCoordinator: - Wraps the emitter, owns the cold-launch UIKit probe (single #if canImport(UIKit) block), exposes onForeground / onBackground / onReady / openURL. - Single seam between the platform-notification observer and the emitter; UIApplication access stays out of AnalyticsClient. AnalyticsClient integration: - Snapshots AppContext.fromBundle() once at init; injects into both DeviceContextProvider (replacing per-event Bundle reads) and LifecycleEventEmitter. - Conditionally constructs LifecycleCoordinator when trackLifecycleEvents == true. - Named handleForeground() / handleBackground() async methods replace inline closures so the load-bearing emit-before-flush ordering rule lives next to the code it describes. - coordinator.onReady() fires inside initTask after .ready, before drainDiskStoreToNetwork, so cold-launch events ship in the same drain. Public API: - openURL(_ url: URL, sourceApplication: String?) on AnalyticsInterface, AnalyticsClient, and AnalyticsProxy. First param positional — mirrors UIApplication.application(_:open:) and Segment's API. - Logs Logger.warn when called while feature disabled (silent no-op was bad DX). - AnalyticsProxy buffers openURL identical to other proxied methods. Opt-in default: - InitOptions.trackLifecycleEvents: Bool = false. - InitOptionsTests cover default + explicit override. Test seams: - AnalyticsDependencies gains appContext, lifecycleStorage, identityStorage, initialAppState for integration test injection. - AppLifecycleEventIntegrationTests drives a real AnalyticsClient via DI, posts UIApplication/NSApplication notifications through NotificationCenter, asserts events flow through queue + network. Covers: cold launch end-to-end, flag-disabled path emits zero events, reset() preserves lifecycle storage, re-init same version emits only Opened, background notification triggers Application Backgrounded, foreground after background emits Opened with from_background:true. Ticket: sc-38230 Parent: sc-36764 Stack: sc-38228 -> sc-38229 -> this PR -> sc-38231 * test(lifecycle): proxy openURL coverage + warn-capture + tmpDir cleanup Code review follow-ups (sc-38230). - Add openURL round-trip test through AnalyticsProxy: forward-when-bound and queued-before-bind-replayed-in-order paths. Closes the test gap the reviewer flagged where the new openURL Call enum case had no dedicated coverage even though the dispatch is mechanical. - Add testOpenURLWithFeatureDisabledLogsWarning in AppLifecycleEventIntegrationTests. Verifies the Logger.warn line fires when openURL is called while trackLifecycleEvents=false, so a silent-no-op misconfiguration is diagnosable from logs. - Hoist captureStderrAndStdout from InitOptionsTests (private) to TestHelpers (file-level). Add an async variant with a `settle` parameter for tests where the block under test fires fire-and-forget Tasks (the openURL flow does this — Task { ... } around the actor call). - Track tempDir on Setup and remove it in deinit so per-test integration tmp dirs don't accumulate in /var/folders/.../T. Sleep migration in integration tests deferred — would require adding a non-draining count API to PersistentEventQueue, which is a production surface change that the reviewer flagged as low priority. Tests pass reliably as-is. Test count goes 446 -> 454 (+5 emitter tests from slice 2's amend, +2 proxy tests, +1 warn-capture test). * feat: add MetaRouter.Analytics.shared, deprecate client() (#46) * feat: add MetaRouter.Analytics.shared, deprecate client() Closes sc-38240. Add a property-style accessor `MetaRouter.Analytics.shared` that returns the same buffered proxy as `client()`. Mark `client()` as deprecated with `@available(*, deprecated, renamed: "shared")` so existing callers get a one-line fix-it warning. Why: - Idiomatic Swift convention. Apple's framework singletons are all property accessors: URLSession.shared, UserDefaults.standard, FileManager.default, NotificationCenter.default. iOS engineers reach for `.shared` first by muscle memory. - Surfaced during the lifecycle MR (sc-36764) review when README snippets defaulted to `MetaRouter.Analytics.shared.openURL(...)`, which didn't compile because `shared` didn't exist. The fix landed as `client()` for consistency, but `shared` is the right end state. - Reduces friction for Segment iOS migrators (Segment uses Analytics.shared at the call-site level). Migration: - Soft deprecation in this minor release. `client()` keeps working but emits a yellow warning at every call site, with auto-fix-it pointing at `shared`. - v2.0 will remove `client()` entirely (separate ticket when major version bump lands). Internal call sites migrated in this PR: - Tests/MetaRouterTests/MetaRouterTests.swift (2 sites) - Tests/MetaRouterTests/MetaRouterIntegrationTests.swift (6 sites) No README changes — the lifecycle MR (sc-36764) currently uses `client()` consistently in its in-flight slice 4 README; that PR will update to `.shared` after this lands as a follow-up. Behavior change: none. Both accessors return the same proxy instance. 418 tests pass; no new deprecation warnings (all internal callers migrated). * docs: add MetaRouter.Analytics.shared to API Reference Surface the new property-style accessor in the README's API Reference section so it's discoverable. Adds a short subsection between initialize(with:) and Analytics Interface that: - Explains the convention (matches URLSession.shared / UserDefaults .standard / FileManager.default) - Shows the typical usage pattern (initialize once, call .shared anywhere) - Notes that calls before initialize are buffered identically to the proxy returned from initialize(with:) - Calls out that client() is now deprecated and points users at .shared No new TOC entry — matches the existing convention of listing only top-level sections in the TOC, not sub-sections. * fix: per-key tolderance in CodableValue.from for unsupported value (#47) * feat(lifecycle): storage + bundle metadata foundation [1/4] (#42) * feat(lifecycle): storage + bundle metadata foundation Slice 1 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the storage layer and shared types that the upcoming emitter / wiring slices depend on. Pure additions — no behavior change to AnalyticsClient, no events emitted yet. - LifecycleStorage: UserDefaults wrapper for (version, build) under the metarouter:lifecycle:* namespace, separate from identity keys so reset() cannot wipe install/update state. - IdentityStorage.hasAnyValue(): helper for the emitter (slice 2) to snapshot identity presence at construction time, before IdentityManager.initialize() auto-creates an anonymousId. - AppContext: gains Equatable + fromBundle(_:) — single source of truth for app metadata, replacing the per-event Bundle.main.infoDictionary reads that DeviceContextProvider does today (consumed in slice 3). - AppForegroundState enum: platform-neutral active/inactive/background trichotomy used by the emitter and coordinator. Ticket: sc-38228 Parent: sc-36764 Stack: this PR -> sc-38229 -> sc-38230 -> sc-38231 * refactor(lifecycle): mark LifecycleStorage.clear() as internal Code review follow-up (sc-38228 M1). The `clear()` method is a test-only seam, not part of the SDK's public contract. Marking it `public` would expose a "wipe install/update state" affordance to consumers that contradicts the entire rationale for the `metarouter:lifecycle:*` namespace separation: nothing — not even `reset()` — should be able to wipe this state. `@testable import MetaRouter` already gives test code access to `internal` symbols, so this is purely a tightening of the public surface with no functional change. * fix: fixing review comments * feat(lifecycle): LifecycleEventEmitter actor + unit tests [2/4] (#43) * feat(lifecycle): storage + bundle metadata foundation Slice 1 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the storage layer and shared types that the upcoming emitter / wiring slices depend on. Pure additions — no behavior change to AnalyticsClient, no events emitted yet. - LifecycleStorage: UserDefaults wrapper for (version, build) under the metarouter:lifecycle:* namespace, separate from identity keys so reset() cannot wipe install/update state. - IdentityStorage.hasAnyValue(): helper for the emitter (slice 2) to snapshot identity presence at construction time, before IdentityManager.initialize() auto-creates an anonymousId. - AppContext: gains Equatable + fromBundle(_:) — single source of truth for app metadata, replacing the per-event Bundle.main.infoDictionary reads that DeviceContextProvider does today (consumed in slice 3). - AppForegroundState enum: platform-neutral active/inactive/background trichotomy used by the emitter and coordinator. Ticket: sc-38228 Parent: sc-36764 Stack: this PR -> sc-38229 -> sc-38230 -> sc-38231 * refactor(lifecycle): mark LifecycleStorage.clear() as internal Code review follow-up (sc-38228 M1). The `clear()` method is a test-only seam, not part of the SDK's public contract. Marking it `public` would expose a "wipe install/update state" affordance to consumers that contradicts the entire rationale for the `metarouter:lifecycle:*` namespace separation: nothing — not even `reset()` — should be able to wipe this state. `@testable import MetaRouter` already gives test code access to `internal` symbols, so this is purely a tightening of the public surface with no functional change. * feat(lifecycle): LifecycleEventEmitter actor + unit tests Slice 2 of 4 of the iOS lifecycle events feature (parent: sc-36764). Adds the lifecycle event emitter — the actor that owns install/update detection, the cold-launch state machine, foreground/background transitions, and the one-shot deep-link buffer. Standalone in this slice; not yet wired into AnalyticsClient. The emitter exposes four entrypoints: - emitColdLaunchSequence(initialAppState:): decides Installed vs Updated vs no-op based on persisted (version, build) and identity-existed- before-init snapshot, then conditionally emits Application Opened with from_background:false (suppressed for background-launched processes; the next true foreground entry emits the deferred Opened via the cold-launch bridge). - emitForegroundFromBackground(): handles bridge case, suppresses the first didBecomeActive during init, filters inactive→active, and emits Application Opened with from_background:true on real background→active transitions. - emitBackgrounded(): updates lastTrackedAppState and emits Application Backgrounded. - openURL(url:sourceApplication:): one-shot buffer (last-write-wins), consumed by the next emitOpened. State machine flags (coldLaunchEmitted, coldLaunchSuppressed, lastTrackedAppState, pendingDeepLink) are serialised through actor isolation. 22 unit tests cover the install/update decision tree, cold-launch sequencing, resume scenarios, deep-link buffer semantics, and double- emit suppression. Ticket: sc-38229 Parent: sc-36764 Stack: sc-38228 -> this PR -> sc-38230 -> sc-38231 * fix(lifecycle): emitter race guard + buffer log lines + missing tests Code review follow-ups (sc-38229). - Guard emitBackgrounded against firing before the cold-launch sequence runs. Race scenario: process woken in .background, observer already registered, didEnterBackground arrives before the async initTask → onReady chain. Without the guard, we'd emit Backgrounded with no preceding Opened — a spec violation. - Add log line on the cold-launch bridge emit (suppressed cold launch → first true foreground entry). This path is rare and notoriously hard to diagnose in field reports; one log line earns its keep. - Add log line when openURL overwrites an existing pending deep link (last-write-wins), making field debugging of "why did Opened carry a different URL than I expected" tractable. - Drop the unnecessary lastTrackedAppState = .active write on the pre-cold-launch suppressed branch. The cold-launch path is the canonical source of truth for this flag (set in emitColdLaunchSequence); writing it during the suppression window is a no-op at best and hides intent at worst. - Add 5 missing test branches: - Cold launch with initialAppState == .inactive (distinct from .background) suppresses + bridges - Deep-link buffered before a suppressed cold launch survives the suppression and attaches to the bridge Opened - emitBackgrounded does NOT consume the deep-link buffer (one-shot per Application Opened, not per any-emit) - Same-bundle no-op cold launch still persists (version, build) to storage (regression guard against accidental skip) - Backgrounded before cold launch is suppressed (the new guard) Test count goes from 438 -> 443. All passing. * fix: review comments rename recordUrl * fix: improving tests and build issues
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.
Ticket
sc-38229 — iOS Lifecycle PR 2: LifecycleEventEmitter actor
Parent: sc-36764
Base: #42 (slice 1) — review/merge that first
Summary
Slice 2 of 4 of the iOS lifecycle events feature. Adds the lifecycle event emitter algorithm — the actor that owns install/update detection, cold-launch sequencing, foreground/background transitions, and the deep-link buffer.
The emitter is heavily branchy (cold launch × identity presence × persisted state × initial app state, plus the resume/inactive/bridge state machine). Algorithm + its tests should travel together so reviewers can validate both at once. ~250 LOC implementation + ~345 LOC tests covering 22 distinct branches.
Depends on
LifecycleStorage,IdentityStorage.hasAnyValue,AppContext.fromBundle,AppForegroundState.What's in
Sources/MetaRouter/lifecycle/LifecycleEventEmitter.swift(new) — full actor body:emitColdLaunchSequence(initialAppState:)— decidesApplication InstalledvsApplication Updatedvs no-op based on persisted(version, build)andhadIdentityBeforeInitsnapshot. Persists current state. Conditionally emitsApplication Opened {from_background: false}if process is foreground-active; otherwise setscoldLaunchSuppressed = true.emitForegroundFromBackground()— three branches: cold-launch bridge (coldLaunchSuppressed == true), suppression of firstdidBecomeActiveduring init (coldLaunchEmitted == false), and thebackground → activeresume detection that filters outinactive → activetransitions.emitBackgrounded()— emitsApplication Backgrounded, updateslastTrackedAppState.openURL(url:sourceApplication:)— one-shot buffer, last-write-wins, consumed by nextemitOpened.Tests/MetaRouterTests/LifecycleEventEmitterTests.swift(new) — 22 unit tests covering:background → activeemits,inactive → activedoes not, double-emit suppressionWhat's not in
AnalyticsClient/ lifecycle observer / public API — slice 3.LifecycleCoordinator— slice 3.NotificationCenter— slice 3.Test plan
swift buildclean.swift test— 438 tests pass, 0 failures (was 424 after slice 1; +14 new emitter unit tests bring total to 438. Slice 2 actually contains 22 emitter test methods — some run within combined fixtures so the count is perfunc test*declarations not test cases).swift test --filter LifecycleEventEmitter— all emitter tests pass in isolation.Stack
LifecycleEventEmitteractor + unit testsAnalyticsClientwiring +openURLpublic API + integration tests