Skip to content

Collect IPv4 and observed IPv6 as device attributes - #524

Open
anglinb wants to merge 9 commits into
developfrom
codex/device-ip-collection
Open

anglinb wants to merge 9 commits into
developfrom
codex/device-ip-collection

Conversation

@anglinb

@anglinb anglinb commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Changes in this pull request

Add a best-effort IPv4 request alongside existing enrichment and expose ipV4, ipV6, ipV4ObservedAt, and ipV6ObservedAt in device attributes. Each family is retained separately; malformed or stale observations are omitted. Collection does not wait on the network during configuration or purchases, attempts are coalesced for 15 minutes, and the public IPv4 request sends no user attributes or API key.

This is off by default. It only runs when the backend turns on attributionOptions.mmp.enabled in the app's config, so the SDK's privacy manifest stays as it is and apps that use the MMP declare the IP collection themselves. The IPv4 request is only made with the release network environments, since the IPv4-only host has no dev version. Any ipAddress the enrichment API returns passes through unchanged.

IPv6 is taken from the existing enrichment response when that connection uses IPv6. Deploy https://github.com/superwall/paywall-next/pull/4160 before releasing this change. It does not guarantee IPv6 on dual-stack devices. Mobile wrappers inherit this when they adopt the native release; no wrapper versions are bumped here.

Validation: 29 tests passed on the iOS 26.4 simulator (DeviceIPCollectorTests and DeviceHelperTests). Full SDK and test targets compiled. Ran scripts/lint.sh; no new-file violations remain (existing repository/configuration warnings remain). Added README contract and changelog entry. UI/demo/Catalyst/visionOS and public online docs remain unchecked.

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs on iOS.
  • Demo project builds and runs on Mac Catalyst.
  • Demo project builds and runs on visionOS.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run swiftlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

RetriggerConfidence Score: 4/5

The PR should not merge until IP collection can run after the MMP flag becomes available on a cold launch.

Findings

  1. P1 Cold launch skips IP collection ▶
  2. P2 Unrelated timestamps can mark IPv6 fresh ▶
Fix with agent prompt
### Issue 1
Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:1090
On a cold launch, `ConfigManager.fetchConfiguration()` reads enrichment and device attributes before publishing the fetched config. This gate sees no config, so it skips the IPv4 request and does not record IP observations from enrichment. Publishing the MMP-enabled config does not guarantee another device-attribute read, leaving the initial session without collected IP attributes.

### Issue 2
Sources/SuperwallKit/Network/Device Helper/DeviceIPCollector.swift:78-79
If enrichment returns `ipV6` without `ipV6ObservedAt`, alongside an IPv4 `ipAddress` and its `ipAddressObservedAt`, `record` pairs the IPv6 address with the IPv4 timestamp. That can make an older IPv6 address appear fresh rather than omitting an observation whose time is unknown. Pair each address only with its own timestamp.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds an MMP-gated, best-effort IPv4 lookup and exposes separately timestamped IPv4 and IPv6 device observations. The initial configuration path reads device attributes before publishing the flag, however, so collection can be missed on a cold launch.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Fetch config] --> B[Read enrichment and device attributes]
  B --> C{MMP flag readable?}
  C -->|No on cold launch| D[Skip IP collection]
  D --> E[Publish fetched config]
  E --> F[No guaranteed subsequent attribute read]
Loading

Reviews (1) · Last reviewed commit: "Wait a minute before retrying a failed I..."

@yusuftor
yusuftor marked this pull request as ready for review September 23, 2026 08:22
…llection

# Conflicts:
#	CHANGELOG.md
#	CLAUDE.md
#	SuperwallKit.xcodeproj/project.pbxproj

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The timestamp parser accepts only fractional-seconds ISO 8601, so a backend that emits 2026-01-01T00:00:00Z silently disables the entire feature. The collection endpoint is also hardcoded outside SuperwallOptions.NetworkEnvironment. Both are worth settling before release.

Reviewed changes — the single commit 4cca2f6 adding session-local device IP collection, across 7 files.

  • New DeviceIPCollector actor — holds timestamped ipV4/ipV6 observations with a 15-minute lifetime, validates addresses with inet_pton, keeps each family independent, and rejects ::ffff:-mapped v4 addresses as IPv6.
  • Best-effort IPv4 fetch — a fire-and-forget request to https://v4.superwall-enrichment.com/api/v1/enrich on an ephemeral URLSession with 3 s timeouts, no API key and no user attributes, coalesced to one attempt per 15 minutes.
  • DeviceHelper wiring — refreshIfNeeded() at the top of getEnrichment(), plus a record / strip / re-merge step in getTemplateDevice() that removes the six IP keys from the enrichment dict and re-adds only still-fresh observations.
  • Docs and tests — README.md contract, a CLAUDE.md invariants section, a CHANGELOG.md entry, and three swift-testing cases covering family separation, inet_pton validation and refresh coalescing.

⚠️ Nothing exercises the DeviceHelper strip-and-merge path, or a timestamp without milliseconds

DeviceIPCollectorTests covers the actor in isolation, but the actual integration — DeviceHelper.swift:1050-1054, where cached enrichment IP fields are stripped and fresh observations merged back — has no test. Every timestamp in the suite is generated with .withFractionalSeconds, so the parser strictness described inline is exactly the case the tests cannot catch.

Technical details
# Cover the DeviceHelper integration and the timestamp format boundary

## Affected sites
- `Tests/SuperwallKitTests/DeviceIPCollectorTests.swift:10` — the only timestamp shape under test is `[.withInternetDateTime, .withFractionalSeconds]`, which is also the only shape the implementation can parse. The test and the bug agree with each other.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:1050-1054` — untested. `DeviceHelperTests.swift` already builds a `DeviceHelper` and calls `getTemplateDevice()` (lines 219, 234), so there is an existing seam to extend.

## Required outcome
- A test that feeds a second-precision timestamp (`2026-01-01T00:00:00Z`) through `record` and asserts the observation survives. This test must fail against the current implementation.
- A test asserting that a stale `ipAddress` present in `enrichment.device` does not appear in the dictionary returned by `getTemplateDevice()`, and that a fresh observation does.

## Open questions for the human
- Is `DeviceHelperTests` the right home for the integration case, or should `DeviceIPCollector` be injectable into `DeviceHelper` so the fetch can be stubbed there too? It is currently a hardcoded `private let` at `DeviceHelper.swift:17`.

ℹ️ PrivacyInfo.xcprivacy is unchanged while the SDK starts retaining and republishing the public IP

The manifest at Sources/SuperwallKit/Resources/PrivacyInfo.xcprivacy declares only NSPrivacyCollectedDataTypePurchaseHistory. This PR is the first point where the SDK's own code issues a request whose sole purpose is learning the IP, holds it for 15 minutes, and republishes it as named device attributes that flow into later requests and into the public getDeviceAttributes() surface. Worth a deliberate call rather than an omission — URLSessionConfiguration.ephemeral and inet_pton are confirmed not required-reason APIs, so only NSPrivacyCollectedDataTypes is in question.

Technical details
# Decide whether the new IP retention changes the privacy-manifest obligation

## Affected sites
- `Sources/SuperwallKit/Resources/PrivacyInfo.xcprivacy` — unchanged by this PR; `NSPrivacyCollectedDataTypes` lists only purchase history.

## Context
Apple's app-privacy guidance (https://developer.apple.com/app-store/app-privacy-details/) draws the line at retention: data "sent on a server call and not retained" needs no disclosure, whereas "you collect and store IP address from your users" does, mapped onto whichever data type matches the use. `DeviceIPCollector` retains for `lifetime = 15 * 60` and `DeviceHelper.getTemplateDevice()` re-emits the value into subsequent enrichment and audience-evaluation payloads.

## Required outcome
- An explicit decision, recorded somewhere durable, on whether `NSPrivacyCollectedDataTypes` needs a new entry.

## Open questions for the human
- The existing enrichment endpoint already returns IP-derived geo that the SDK persists via `LatestEnrichment`, so part of this may be a pre-existing question rather than one this PR creates. Does Superwall already have a position on this?

ℹ️ Nitpicks

  • DeviceIPCollector.swift:21 — lastAttempt is set before the fetch is attempted, so a single DNS blip or timeout burns the whole 15-minute window rather than just a successful attempt. Resetting lastAttempt on failure would let the next getEnrichment() retry.
  • DeviceIPCollector.swift:1 — the file has no header comment. Every neighbour in Network/Device Helper/ opens with the house // File.swift / // Created by … block.
  • DeviceIPCollector.swift:23, 70 — both failure paths (try? swallowing the fetch error, the unreachable URL guard) are silent. The repo convention is to route network failures through Logger, which would make a misconfigured or undeployed endpoint diagnosable in the field.
  • DeviceIPCollectorTests.swift:33, 42 — coalescesRefreshesWithoutWaitingForNetwork gives a 100 ms fetch a 150 ms window, which is a narrow margin on a loaded CI machine. An actor-based signal (e.g. awaiting a continuation the stub resumes) would make it deterministic.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceIPCollector.swift Outdated
Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceIPCollector.swift Outdated
Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift Outdated
Comment thread CHANGELOG.md Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Merge-only update — no new findings. Prior review still applies.

Reviewed changes — the delta since the previous review at 4cca2f6 is a single commit, afd70c9, which merges origin/develop into the branch. It carries no changes of its own.

  • Merge is semantically clean — git diff --merge-base origin/develop on DeviceHelper.swift still resolves to exactly the three hunks this PR introduced (the ipCollector property, refreshIfNeeded() at the top of getEnrichment(), and the strip-and-merge block in getTemplateDevice()). The range-diff shows no change to the PR's own content; the churn is project.pbxproj regeneration and context shift from files that landed on develop.
  • No collision with what develop brought in — ipV4/ipV6/ipAddress appear nowhere in the merged tree outside this PR's own two files, and the new DeviceIdentifiers.swift / AttributionFetcher work operates on user attributes, not the device dictionary this PR edits.

One note on merged-in context rather than a new finding: develop landed SuperwallOptions.devServer with localhost discovery in this same window. That sharpens the hardcoded-host point from the previous review — a superwall dev session now has a first-class local story, but the IPv4 collector will still reach out to production v4.superwall-enrichment.com from it.

Pullfrog  | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Release-plumbing update — no new findings. The prior review's open items still apply.

Reviewed changes — the delta since the previous review at afd70c9 is a single commit, 4aa3b70, which stages the release. It has no behavioral surface.

  • Version bumped to 4.18.0 — Sources/SuperwallKit/Misc/Constants.swift:21 and SuperwallKit.podspec:4.
  • Changelog entry regrouped — the IP-attributes bullet moved out from under ## 4.17.0 into a new ## 4.18.0 → ### Enhancements section, which addresses the changelog nit from the previous review (thread now resolved).
  • Nothing else moved — DeviceIPCollector.swift, DeviceHelper.swift, README.md, CLAUDE.md, DeviceIPCollectorTests.swift and project.pbxproj are byte-identical to afd70c9.

The version choice checks out against the repo's own rule in CLAUDE.md: origin/master and origin/develop were both on 4.17.0 and master's changelog already carries the full ## 4.17.0 section, so starting a new section and bumping all three files is correct rather than appending to a staged release. A minor bump fits an additive device-attribute change, and project.yml carries no hardcoded version, so the three-file set is complete.

The three substantive threads from the initial review are untouched by this commit and remain open — the fractional-seconds-only timestamp parser (DeviceIPCollector.swift:92-97), the hardcoded collection host outside NetworkEnvironment (DeviceIPCollector.swift:70), and the unconditional ipAddress strip (DeviceHelper.swift:1082-1084). Since this commit stages a release, the first of those is worth settling before the tag: if the backend from superwall/paywall-next#4160 ever emits a second-precision timestamp, the shipped feature is a silent no-op with no log to diagnose it.

Pullfrog  | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

yusuftor and others added 2 commits September 29, 2026 15:27
…llection

# Conflicts:
#	CHANGELOG.md
#	SuperwallKit.podspec
- Parse observation timestamps with or without milliseconds, reusing two formatters.
- Take the IPv4 host from the network environment, and skip the request outside release builds since there is no dev host.
- Stop removing `ipAddress` from device attributes, since the enrichment API returns it today.
- Retry after a failed fetch instead of waiting out the 15 minute window, and log the failure.
- Ignore non-string values in the response rather than failing to decode.
- Test the DeviceHelper stale-IP filtering, second-precision timestamps and retry, without timing sleeps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. The three substantive threads from the initial review are addressed, and the new tests exercise the exact cases that were previously uncovered.

Reviewed changes — the delta since the previous review at 4aa3b70: a merge of origin/develop (07bdda1) and the feedback commit 1372760.

  • Parsed timestamps with or without milliseconds — two stored static let formatters sit behind DeviceIPCollector.date(from:), which also removes the per-access formatter allocation on the getTemplateDevice() hot path.
  • Routed the IPv4 host through NetworkEnvironment — ipV4EnrichmentHost resolves only for .release / .releaseCandidate, Api.Enrichment.ipV4Url builds the URL, and a nil URL makes refreshIfNeeded() a no-op. .developer, .local and .custom now send no traffic to the production domain.
  • Stopped stripping ipAddress — only the four new ipV4/ipV6 keys are filtered, so any existing device.ipAddress audience filter keeps working. The README states the pass-through.
  • Retried after a failed fetch — lastAttempt is cleared on failure (guarded so it cannot clobber a newer attempt) and the error goes through Logger.
  • Made DeviceIPCollector injectable into DeviceHelper — added templateDevice_dropsStaleIPObservations, which would fail without the strip, plus collector tests for second-precision timestamps, newer-wins, retry-after-failure, the environment → URL mapping, and tolerant parseDevice. The coalescing test now awaits the returned task instead of racing a sleep.
  • Rebased the changelog onto develop's staged 4.18.0 — develop already carries the bump, so the PR only appends its bullet to the existing ### Enhancements, as CLAUDE.md requires when a release is already staged.

The privacy-manifest question from the initial review is still a decision for the team rather than a code issue, and the online docs checkbox is still open. Neither blocks this change.

Pullfrog  | View workflow run | Using Claude Opus | 𝕏

IP collection is off by default and runs only when config has the
`enable_mmp` toggle on. With it off, no IPv4 request is made and no
`ipV4`/`ipV6` attributes are exposed. The check happens when building
device attributes, so a first launch picks it up once config arrives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The MMP gate is correct, and it answers the earlier privacy-manifest question. But moving refreshIfNeeded() onto the getTemplateDevice() hot path, together with the failure reset added last round, means any persistent fetch failure sends a new IPv4 request on almost every device-attribute read. Details inline.

Reviewed changes — the delta since the previous review at 1372760 is one commit, 363faa3, which puts IP collection behind the backend's enable_mmp toggle.

  • Added FeatureFlags.enableMMP — decoded from the enable_mmp toggle, defaulting to false, encoded back in encode(to:), and true in the stub. Old cached configs without the key decode to off.
  • Gated collection in getTemplateDevice() — the four ipV4/ipV6 keys are still always stripped. refreshIfNeeded(), record and the merge now only run when the flag is on. The refresh moved here from getEnrichment() because the first cold-launch enrichment runs before config is loaded.
  • Widened DeviceHelper's factory to FeatureFlagsFactory — DependencyContainer already conforms, and it is the only production call site.
  • Added flag tests — templateDevice_withoutMMPFlag_skipsIPCollection checks that no attributes are exposed, and that refreshIfNeeded() still returns a task afterwards, which proves no lookup had started. This test can fail. mmpFlagIsOffUnlessTheBackendTurnsItOn covers decoding.
  • Updated docs — the README says collection is off by default, and CLAUDE.md records that apps turning on the MMP declare the IP collection in their own privacy manifest.

ℹ️ Nitpicks

  • README.md:119-120 — "limited to one per 15 minutes when enrichment runs" no longer matches the code. Attempts now start on any device-attribute read while the MMP flag is on, not only during enrichment.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift
Moves the IP collection switch from a general toggle to
`attributionOptions.mmp.enabled`, next to Apple Search Ads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The gate swap is clean, and I found no new code issues. The retry-storm thread on DeviceHelper.swift:1091 is still open, since DeviceIPCollector.swift didn't change in this commit. The README nit from the last review also still applies.

Reviewed changes — the delta since the previous review at 363faa3 is one commit, a5f1a01. It moves the MMP switch from a feature flag to the attribution config options.

  • Reverted FeatureFlags.enableMMP — the enable_mmp toggle is gone from decode, encode, the memberwise init and the stub. FeatureFlags.swift is no longer in the PR.
  • Added Attribution.mmp — a new optional MMPAttribution { enabled: Bool } sits next to appleSearchAds. Its init defaults mmp to nil, so the existing Config.stub() call site still compiles. It decodes from attributionOptions.mmp through the existing Config.CodingKeys.attribution mapping.
  • Changed the gate to read ConfigStateFactory — getTemplateDevice() now checks makeConfigState().value.getConfig()?.attribution?.mmp?.enabled == true, the same shape AttributionPoster uses for appleSearchAds. DependencyContainer already conforms, and it's the only production factory.
  • Updated tests and docs — mmpFlagIsOffUnlessTheBackendTurnsItOn decodes Attribution with and without mmp. setMMPFlag builds the config through the new init. README.md and CLAUDE.md now name attributionOptions.mmp.enabled.

ℹ️ attributionOptions.mmp.enabled gates IP collection but not the MMP install match

The SDK now decodes a switch named for the whole MMP. Only IP collection reads it: MMPAttributionManager still sends the install match to the /mmp endpoint whatever the flag says. By contrast, appleSearchAds.enabled does gate AttributionPoster. The doc comment on the new field, "Superwall's install attribution (MMP). Off unless the backend enables it.", suggests it gates more than it does.

Technical details
# Decide what `attributionOptions.mmp.enabled` is meant to gate

## Affected sites
- `Sources/SuperwallKit/Config/Models/Attribution.swift:12-13` — the doc comment describes `mmp` as the install-attribution switch.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:1090` — the only reader of `attribution?.mmp?.enabled`.
- `Sources/SuperwallKit/Analytics/Attribution/MMPAttributionManager.swift` — the install match doesn't check the flag.

## Required outcome
- Either the doc comment says the field gates device IP collection only, or the install match honors the flag too (that would be a follow-up, not this PR).

## Open questions for the human
- On the backend, does `attributionOptions.mmp.enabled` mean "this app uses the Superwall MMP", with the install match expected to stop when it's off? Or is it only meant to control IP collection?

Pullfrog  | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

The lookup now runs whenever device attributes are read, so clearing the
last attempt on failure meant an outage or offline device retried on
almost every read. Wait 60 seconds after a failure instead; a success
still waits the full 15 minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. The retry backoff fixes the hot-path retry storm, and the new test fails against the old behavior.

Reviewed changes — the delta since the previous review at a5f1a01 is one commit, 168ae34. It adds a failure backoff to the IPv4 lookup.

  • Replaced the window check with nextAttemptAt — starting an attempt sets it to date + lifetime, and a failure pulls it in to date + retryDelay (60 s). The lastAttempt == date guard still stops a stale failure from overwriting a newer attempt. A persistent failure (offline, v4 host down, blocked host) now costs at most one request a minute instead of one on nearly every getTemplateDevice() read.
  • Rewrote the retry test as waitsAMinuteBeforeRetryingAFailedFetch — it uses an injectable Clock to check that there's no retry at +30 s, a retry at +61 s, and the full 15-minute window after a success. The +30 s nil assertion would fail against the previous clear-on-failure code, so this test can catch the regression.
  • Updated the README timing line — it now says a lookup can start on any device-attribute read while the MMP is on, at most every 15 minutes, or a minute after a failure. This fixes the stale-wording nit from the 363faa3 review.

The scope question from the last review is still open for the team: attributionOptions.mmp.enabled gates IP collection but not the MMPAttributionManager install match. It doesn't block this change.

Pullfrog  | View workflow run | Using Claude Opus | 𝕏

@yusuftor

Copy link
Copy Markdown
Collaborator

@greptileai

Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift
Comment thread Sources/SuperwallKit/Network/Device Helper/DeviceIPCollector.swift Outdated
On a cold launch the first enrichment is read before config arrives,
so its IPs were dropped and the IPv4 lookup waited for a later read.
Enrichment IPs are now always kept in memory (only exposing them is
gated), and config arriving with the MMP on starts the lookup.

An ipV6 without its own timestamp could also borrow the IPv4
ipAddressObservedAt and look fresh; each address now only uses its
own timestamp.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. Both cold-launch findings from the last round are fixed, and each fix comes with a test that fails against the old code.

Reviewed changes — the delta since the previous review at 168ae34 is one commit, 6220829. It starts the IPv4 lookup when config is applied and stops one IP family from borrowing the other family's timestamp.

  • Started the lookup when config is applied — ConfigManager.processConfig now calls a new storeAndApply(_:), which contains the same four save/trigger/variant lines as before plus deviceHelper.startIPCollectionIfEnabled(for:). That method reads the flag from the incoming config, not from configState, so it works before config is published. processConfig is the only path that publishes .retrieved, so cold sync, cold cached and refreshConfiguration are all covered. Config.stub() has no mmp, so the existing ConfigManager tests don't start lookups.
  • Recorded enrichment IPs even when the flag is off — record now runs outside the MMP gate in getTemplateDevice(), but attributes() is still only merged when the flag is on. If a later enrichment replaces a pre-config one, its observations are kept, and nothing new is exposed while the MMP is off. These IPs were already in memory through enrichment.
  • Paired each address only with its own timestamp — when ipV{n} is present, record only uses ipV{n}ObservedAt. It falls back to ipAddress + ipAddressObservedAt only when the family key is missing, so an ipV6 without a timestamp is dropped instead of taking the IPv4 time.
  • Added tests — doesNotPairAnAddressWithAnotherFamilysTimestamp and coldLaunch_keepsEnrichmentIPsAndStartsLookupWhenConfigArrives would both fail against 168ae34. configWithMMPOff_doesNotStartLookup covers the off path. templateDevice_withoutMMPFlag_skipsIPCollection lost its attributes().isEmpty check, which is correct because record is no longer gated. It still checks that no attributes are exposed and no lookup starts.

The scope question from earlier rounds is still open for the team: attributionOptions.mmp.enabled gates IP collection but not the MMPAttributionManager install match. It doesn't block this change.

Pullfrog  | View workflow run | Using Claude Opus | 𝕏

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants