Persist split rules in the app's own local state

[Ryanmello07]

The bug

Split rules do not survive leaving the app. Configure a few, close the app, come back, and the Local statistics card reads "0 split rules" over the never-configured empty state — while the rules are still sitting in the extension's .block_action_overrides.

The rules live in the network extension's container, which the app process cannot read. BlockActionsStore kept no copy of its own: splitRules is written only from device.getBlockActionOverrides(), and with the RPC service down DeviceRemote answers out of its own memory (device_rpc.go:4661-4663), which a fresh process has nothing in. An empty answer is indistinguishable from "the user has no rules", so the app renders zero.

It also destroys rules, which is the worse half

DeviceRemote's offline mutators rebuild an authoritative full list from whatever base they can see (device_rpc.go:4529-4534). With nothing seeded, that base is empty. So a rule added while the tunnel was down does not merely fail to save — the truncated one-element list crosses on the next connect and DeviceLocalRpc.Sync applies it as a wholesale replace (device_rpc.go:9092-9093 → device_local.go:5642), erasing the rules the extension had persisted.

The fix

Give split rules the two halves transport settings already has in this tree:

• The mirror. BlockActionsStore takes a localState: in setup, exactly as TransportSettingsStore.setup(_:localState:) does, and writes the rules to the app's own store — but only from a read it can vouch for as the whole list. Two reads qualify: one taken off a connected device (the extension's own list), and one taken right after an edit made here (that list plus the edit). Mirroring anything else would replace the app's only durable copy with a list it never read.
• The seed. DeviceManager.initDevice seeds the next device from that mirror, next to the transport-settings seed whose comment already names this exact hazard.

A remote-connected listener re-reads once the RPC is actually up, because the override change the extension replays on connect arrives over the reverse sync — before the remote publishes its service — so that read still comes out of the remote's own memory. This is also what backfills the mirror on the first connect after upgrade.

Why app-wins on connect cannot lose data

The patch adds no arbitration, because the wire has none to add it to — deviceRemoteValue carries only {Value, IsSet}. What it changes is what the app is allowed to put in that slot.

These rules have exactly one author. DeviceLocal changes its override list solely through the RPC setters the app drives, plus its own construction seed from its own dotfile (device_local.go:1374); nothing in app/extension touches block actions. So "the app wins" is the single author replaying its own record, not two authors racing. The extension's copy is a replica.

Two distinctions carry the rest, and both are pinned by the new tests:

• A store that was never written reads back nil, so the seed's if let queues nothing and state.BlockActionOverrides.IsSet stays false. Rules left by a pre-fix build survive the first launch after this ships untouched. The unreadable/corrupt case reports the same way and fails toward the extension's copy too.
• An empty list is written as an empty list, never as nil. LocalState.SetBlockActionOverrides(nil) is an os.Remove, and a deleted store reads back as "never edited" — which would resurrect the rules the user just deleted. exportedList.MarshalJSON renders empty as [], so "I removed my last rule offline" round-trips as a real value.

One incidental improvement: because the mirror is written from the read-back rather than from the pushed value, hostedSafeBlockActionOverride stripping a route override converges the app onto the extension's stored form instead of re-pushing the pre-strip version forever.

Known limitation

The seed makes state.BlockActionOverrides.IsSet true on every cold launch, so the first sync of each launch is a full replace. If the app's mirror is ever behind the extension's real list, that difference is lost silently. The mirror can only fall behind in narrow windows — an edit whose RPC succeeded but whose store write then failed, or an app container restored from backup out of step with the extension's — and the next connected read re-mirrors. This is the same contract routeLocal, blockerEnabled, connectLocation and the transport settings already operate under, but it is worth stating rather than discovering.

Not in scope

• DNS resolver settings have the identical gap. DnsSettingsStore takes no localState either, and SdkLocalState already exposes getDnsResolverSettings/setDnsResolverSettings, so it takes the same Swift-only treatment. Left for a follow-up to keep this diff to one subject. Note the two DNS-specific guards: DeviceLocal.SetDnsResolverSettings returns early without persisting on nil settings and when the mux is disabled, so a mirrored write there is not proof of an extension-side write — it would need to mirror from the read-back, as this patch already does.
• The SDK-level footgun is untouched. DeviceRemote's offline mutators still rebuild a full list from a possibly-empty base and Sync still applies it unguarded. This patch stops the iOS app from feeding it a bad list; it does not disarm it. A base-relative apply for list-valued settings is a wire change and belongs in its own SDK PR.
• sdk/device_rpc_test.go's TestDeviceRemoteBlockAndDns shares one network space between the DeviceRemote and the DeviceLocal, so both sides read the same dotfile. That is why the suite is green while iOS loses data. Calling testing_newNetworkSpace twice — as control_ip_family_test.go and log_verbosity_test.go already do for this exact two-process problem — would make this class of bug visible in the repo where the footgun lives.
• The rule editor's "Update" button stays enabled at zero selection (SplitRulesView.swift:721) and updateRule with empty hosts delegates to removeRule, so "Update" with everything deselected deletes the rule. Pre-existing, and there is a separate "Remove rule" button; not touched here.

Tests

BlockActionOverridesMirrorTests pins the store contract with no device and no mocking — SdkAsyncLocalState(localStorageHome) over a per-test temporary directory. Covers the id/hosts/mode round trip, fields the app does not model (blockOverride, a missing routeOverride), empty-round-trips-as-empty, nil-deletes-the-store, and never-written-reads-nil.

No test is proposed for BlockActionsStore itself: SdkDeviceRemote is a concrete gomobile class rather than a protocol, and every existing file in networkTests is a pure-function or value-type test. Inventing a mocking layer for one store would be a larger and riskier change than the fix.

Verification

Builds for iOS and macOS; the new tests pass.

On-device checks this needs before merge, in order — the first launch after upgrade is expected to still show zero, because no mirror exists yet and reading a missing mirror as "no rules" is the wipe the design deliberately refuses:

1. Connect, create two rules, confirm "2 split rules". This first authoritative read is what writes the mirror.
2. Stop the VPN, force-quit, relaunch still disconnected. The rules are there. (Before this patch: "0 split rules".)
3. Still disconnected, edit or delete a rule, force-quit immediately, relaunch. The edit is there; reconnect and it is there too, meaning it crossed to the extension.
4. Disconnected, create a third rule, then connect. All three rules. (Before this patch: only the third — the other two were erased from the extension's store.)
5. Disconnected, delete every rule, then connect. Still zero, rather than the extension's copy resurrecting them.

The VPN must actually stop for 2-5 to be the disconnected case: with providing on or route-local off the extension keeps running and the rules would have rendered anyway.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AWV4MaF9JBcQppSX9UxATa