docs: plan for portable event identity (restore to new device) - #275
Conversation
Adds a development plan for the first half of #273: making a restored app database re-associate with the right calendars and events on a new phone. Event rows currently store only device-local autoincrement IDs (cid, id), so an auto-backup restore leaves every event orphaned. The plan captures durable identity at write time -- the calendar account tuple plus the event's iCalendar UID_2445 -- in the unused reserved column s2, avoiding any schema migration, and re-resolves both IDs on a detected restore with retry plus a manual trigger. Also notes an existing bug in ApplicationController.restoreToActive(), which looks up the old calendar ID against the new device's provider and so cannot work cross-device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
Clarifies why the calendar's account email alone can't solve the restore problem. There are two stale IDs: cid (which calendar) and id (which event in it). The account tuple resolves the first; every event in a calendar shares that same email, so it can't resolve the second. UID_2445 is the RFC 5545 iCalendar UID -- server-assigned, so it's identical on every device that syncs the event, unlike _ID which is a local SQLite row number. Adds three mermaid diagrams (matching the existing docs/architecture convention): the two-halves split, why local _ID diverges across devices, and the full resolution flow with its failure paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
Code Coverage Report
|
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
Resolution strategy was vague about which write is actually dangerous. Separates them: cid is a plain column (safe in-place UPDATE), while id is half the primary key, so changing it forces delete + re-insert. Sequences the safe write first and commits it, so a failed event lookup still leaves the row strictly improved. Found on review: MonitorStorage and DismissedEventsStorage are keyed on eventId in separate databases, so an id change orphans their rows -- which would break restoreToUpcoming, since clearWasHandled aborts when the alert is missing. Splits Phase 1 into 1a (cid only) and 1b (id re-key across all three DBs with manual rollback). Corrects the non-goal that wrongly excluded the monitor DB entirely. Documents the calendar sync window: Google syncs ~12 months back, and there is no API to widen it (requestSync takes no date range). Bounded impact, since MAX_SCAN_BACKWARD_DAYS is 31 -- a limit on the tail, not the feature. Adds database_schema_reference.md covering all three databases, since this is the first deliberate claim on a reserved column. Sets the rule that claiming one requires renaming and documenting it, and records the existing i1=flags / s1=description precedents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
… voice Three corrections to the portable event identity plan. Room only. Phase 0c previously added identity mapping to EventsStorageImplV9 and DismissedEventsStorageImplV2, which is new code on a path already scheduled for deletion (deprecated_features.md item 5). Removes those from scope and states the consequence: on the legacy fallback, s2 stays empty and the resolver skips those rows -- an already-degraded mode, left no worse than today. Added as a Key Decision and noted in the schema reference. Specifies the s2 payload. The plan described the blob abstractly but never showed it. Adds a concrete JSON example with a field table, explains why origCid/origId are stored despite duplicating the row's columns (they are the staleness check that distinguishes "never resolved" from "already resolved" across retries), gives the per-row size, and names who writes and reads it -- only EventIdentityResolver; nothing in normal runtime touches s2. Also states what it is not: not a content cache, since duplicating title/times would create a second source of truth. Removes conversational references so the plan reads standalone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
Reverses the earlier decision to put identity in eventsV9.s2. The reserved text columns are a finite, effectively irreversible resource and should be spent only on data that must live in the event row -- read on the hot path, needed in the same query, or required to travel through the PowerSync pipeline. Portable identity is none of those: it's read only by the resolver on a restore, joins by key, and should NOT reach Supabase, since it contains account emails. That last point inverts the original rationale, which counted the Supabase table already mirroring s2 as a benefit. A new database is cheap here: no legacy predecessor and no copy migration (unlike the existing three), backup_rules.xml already covers it via domain="database", and PowerSync targets eventsV9 by name so a new table is invisible to sync. Switches the payload from a JSON blob to typed Room columns, which removes the serialization layer entirely and makes resolveAttempts a plain UPDATE rather than decode/mutate/re-encode. Notes the real cost: a fourth store with no cross-database transactions, so an id re-key must now move four rows with manual rollback. Identity rows are derived metadata, so orphans are harmless. Schema reference gains the new database and guidance on when a reserved column is and isn't the right home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
Two readability fixes. Column names are spelled out rather than abbreviated. The new table had been drafted in the eventsV9 house style (acctName, dispName, origEventId), but that style is a 2016 inheritance kept only because changing it now is risky -- there is no reason for a brand-new table to adopt it. The five calendar columns now map one-to-one onto the CalendarContract.Calendars columns they come from (calendarAccountName, calendarOwnerAccount, ...), and resolveAttempts becomes resolutionAttemptCount. Column names cost schema space, not per-row space. Unstacks the slash-combined table rows. Cramming "acctName / acctType" into one row against "will@example.com / com.google" made the reader match up values positionally for no benefit -- the table had no width pressure. One column per row now. Adds a column-naming section to the schema reference so the rule outlives this plan, using attsts/oattsts as the cautionary case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
williscool
left a comment
williscool
left a comment
There was a problem hiding this comment.
Reviewed the plan against the code it cites — every file path, line number, and constant checks out (restoreToActive at ApplicationController.kt:1321, MAX_SCAN_BACKWARD_DAYS at Consts.kt:177, the events_storage_state prefs precedent, MonitorDatabase as the version=1 template, MockCalendarProvider's existing stubs, scripts/test_cloud_backup.sh). The core insight is right and the 1a/1b risk split is the best structural decision in here.
Two things to fix in the doc before implementation, plus one readability nit. Both findings are about the plan's own logic, not about the approach.
|
|
||
| ### Phase 2: Backfill for pre-existing rows | ||
|
|
||
| Events stored before Phase 0 have no identity row. While the app is still on the *original* device, they can be backfilled by reading identity from the live provider (the stale IDs are still valid there). Run this opportunistically on app start while any event lacks an identity row. |
There was a problem hiding this comment.
Phase 2 and Phase 3 must be mutually exclusive — worth stating explicitly.
These two phases make opposite assumptions about which device you're on:
- Phase 2 (backfill) assumes the original device. It manufactures an identity row by looking up the event's current
idin the live provider — which works precisely because nothing is broken yet. The stale IDs aren't stale, they're live. - Phase 3 (restore detection) fires on a new device: fingerprint mismatch means this DB arrived from elsewhere and every
idin it is meaningless.
The trigger condition as written — "opportunistically on app start while any event lacks an identity row" — is exactly the condition that's true right after a restore. So on a restored device Phase 2 will take an id that now points at nothing (or worse, at an unrelated event the new device happened to assign that number), query the provider with it, and write an identity row from whatever comes back.
That's worse than doing nothing, because the event now has an identity row, so the resolver treats it as covered rather than skipping it. Best case it never resolves; worst case it points at an unrelated event and the 1b re-key moves the row onto it.
One sentence fixes it: backfill runs only when the Phase 3 fingerprint matches.
Separately — the sentence "this is what makes the feature useful to the current user rather than only to new installs" is right but slightly undersells the constraint. Backfill isn't a rescue mechanism for already-orphaned data; it's inoculation for data that hasn't been restored yet. Anyone who already restored is past saving regardless of what ships, since the old provider IDs are gone. Might be worth saying so, if only to set expectations on what this feature can and can't recover.
There was a problem hiding this comment.
Good catch — this was the most dangerous of the three, and fixed in 7364e19.
You are right that the trigger as written ("opportunistically on app start while any event lacks an identity row") is precisely the condition that holds right after a restore, so the two phases would have collided exactly when it mattered. And the failure mode is worse than a no-op for the reason you give: the event ends up with an identity row, so the resolver treats it as covered rather than skipping it — best case it never resolves, worst case 1b re-keys the row onto an unrelated event.
Backfill is now gated on the Phase 3 fingerprint matching, stated as a hard precondition rather than an optimization, with a note that Phase 3 is therefore a prerequisite for Phase 2 despite being numbered after it. Added a matching test: on a mismatched fingerprint, backfill writes no identity rows at all.
Took the expectation-setting point too — the plan now says backfill is inoculation for data not yet restored, not a rescue for data already orphaned, and that anyone who has already restored is past saving regardless, since the provider IDs identity would derive from are gone.
| - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. | ||
| - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. | ||
| - **Migrating the existing databases** — no schema change to `eventsV9`, `dismissedEventsV2`, or `manualAlertsV1`. The identity database is new and starts at version 1. | ||
| - **Storing identity for `MonitorStorage` rows** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. |
There was a problem hiding this comment.
Minor readability: this leads with the exemption ("doesn't need its own identity") and then bolds the walk-back ("but its rows … must still be re-keyed"). Both are true, and the cross-database section resolves it correctly — but a reader hits the exemption first and has to un-learn it a clause later.
Reads easier as one claim: no identity row of its own, but its rows still need re-keying on an id change. Same content, no double-take.
There was a problem hiding this comment.
Agreed — fixed in 7364e19. It now leads with the single claim ("no identity row of its own, but its rows still need re-keying when an id changes") and gives the reasoning after, so there is nothing to un-learn mid-sentence.
Addresses three PR review threads. findMatchingCalendarId() returns -1L, not null, when all three fallback tiers miss. Since cid = -1 means "unknown, treated as handled" (fail-open), writing that result unguarded would turn a stale-but-plausible calendar ID into one the app silently treats as handled -- making the row quieter rather than better, which inverts the premise the safe-write-first ordering rests on. Requires an explicit != -1L check before any write. The same trap exists in shipping code: restoreToActive()'s elvis catches a null from getCalendarBackupInfo() but passes -1L straight through, so the guard belongs there too. Phase 2 and Phase 3 assume opposite devices, and "any event lacks an identity row" is exactly the condition holding right after a restore. Unguarded, backfill on a restored device would query the provider with a meaningless id and write an identity row from whatever came back -- worse than nothing, since the resolver would then treat the event as covered instead of skipping it. Backfill now runs only when the fingerprint matches, making Phase 3 a prerequisite for Phase 2. Also states what backfill can recover: it inoculates data not yet restored; already-orphaned data is past saving either way. Adds test cases for both, and rephrases the MonitorStorage non-goal so it no longer leads with an exemption it walks back a clause later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
The plan asserted that anyone who already restored onto a new device is "past saving regardless of what ships here." That was stated without checking, and it is wrong. An orphaned row is not empty: it still holds title, startTime, instanceStartTime, location and isAllDay. instanceStartTime is UTC epoch millis derived from the event's real start, so unlike id and cid it is NOT device-assigned -- it is the same value on the new device for the same event. That makes it a usable matching key. Adds Phase 6: query CalendarContract.Instances over a window bracketing the stored instanceStartTime (same API the existing instance scan already uses at CalendarProvider.kt:1594), filter candidates by title/isAllDay/location, and accept only when exactly one survives. A match yields both a real eventId and calendarId, and lets an identity row be written so the event is protected against the next restore. Marked optional and manual-only, with its limits stated rather than glossed: recurring events with identical titles are the weak case, renamed or moved events will not match, and ambiguity is declined outright. Acceptable because the failure mode is "still unresolved" -- exactly where the row already sits -- so it can only improve matters. The sync-window section notes it bounds Phase 6 too. Also notes the hit rate should be measured with the existing test_cloud_backup.sh harness rather than assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
The plan never said plainly that UID_2445 has to be captured on the OLD device while its IDs are still valid. Read without that, "use the UID to find the event" sounds like a lookup that works from nothing -- it doesn't. The app cannot ask a new device for the UID of event 91427, because 91427 means nothing there. Adds a precondition section to the Goal: an ASCII diagram of the snapshot-then-search flow, the blunt practical consequence (this works properly only if you still have the old device, or a backup taken from it after this ships), and a two-tier table separating the exact path (Phases 0-5, requires that backup) from the best-effort one (Phase 6, requires nothing). Also flags the misreading directly in the UID section. Per review, Phase 6 now skips recurring events outright rather than attempting and usually declining them -- they were the known weak case and are not worth the risk for the fraction they would recover. Reframes Phase 6 as explicitly best-effort rather than partial-but- trying, with a matching test case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
"A backup made before then has no identity rows" was true but framed as terminal, which hid a real case. Backfill does not require the old device as such -- it requires the stored id to still resolve against whatever provider is present. That holds on the original device, and equally for an older backup restored back ONTO that device. So there are three cases, not two, and the middle one is recoverable: restore the old backup onto the original device, let backfill run, and the data becomes identity-bearing and therefore portable from then on. That is a genuine migration path for pre-feature data: restore old -> backfill -> re-backup -> move. This also exposes a bug in the Phase 2 gate. Gating purely on the install fingerprint is too blunt, because restoring an old backup onto the original device ALSO clears the fingerprint -- so the gate would have skipped backfill in exactly the case we want it to run. Replaces it with a cheap validation sample: look up a handful of stored ids and compare returned title/start against the stored rows. Agreement means the ids are live and backfill is safe; disagreement or empty results mean a new provider, so skip to Phase 6. The fingerprint stays as a fast path rather than the sole gate. Adds tests for both directions, including the same-device restore that must not be misread as a new-device restore. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
williscool
left a comment
williscool
left a comment
There was a problem hiding this comment.
Second pass — both earlier findings are addressed well, and the validation-sample rework of the Phase 2 gate is better than the fingerprint-only check I'd suggested (it correctly spots that restoring an old backup onto the original device also clears the fingerprint, which my version would have wrongly skipped).
This round is from researching the Android platform surface rather than the codebase. One blocking issue: Phase 3's exclusion mechanism doesn't work on Android 12+, because the manifest declares only fullBackupContent while targetSdkVersion = 36. Details inline on Phase 3.
Also confirmed two things the plan takes on faith, both of which hold: UID_2445 really is readable by a non-sync-adapter app (AOSP CalendarProvider2 adds it to ALLOWED_IN_EXCEPTION; the sync-adapter write restriction covers DIRTY/_SYNC_ID/SYNC_DATA1-10, not it, and neither list restricts reads), and the _SYNC_ID fallback ordering is right — for a stronger reason than stated, noted inline.
One item I explicitly withdrew: I'd drafted a suggestion to replace the fingerprint with a BackupAgent + onRestoreFinished() hook. That's wrong for this app — it only fires when a backup transport drives the restore, so for a userbase that mostly sideloads and copies databases by hand it would be dead code. The fingerprint + validation-sample design is the right call; disregard if you saw that idea elsewhere.
|
|
||
| The fingerprint check here feeds Phase 2's gate as a fast path, so build it first even though it is numbered later. Note Phase 2 does not rely on it alone — see the validation-sample check there, which is what allows an old backup restored onto the same device to still backfill. | ||
|
|
||
| Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. |
There was a problem hiding this comment.
Blocking: this exclusion has no effect on Android 12+ — backup_rules.xml isn't being read.
The manifest declares only the legacy attribute:
android:allowBackup="true"
android:fullBackupContent="@xml/backup_rules"But targetSdkVersion = 36. Per Google's Auto Backup docs, fullBackupContent applies to API 30 and below; API 31+ reads android:dataExtractionRules. With no such attribute declared, the platform falls back to its default — "that mode is fully enabled for all content except no-backup and cache directories" — i.e. back up everything.
This is currently harmless, because backup_rules.xml is almost all <include> and "include everything" is a superset. It stops being harmless here: Phase 3 is the first thing in the codebase that needs an exclude. As written, on any Android 12+ device the fingerprint would be backed up along with everything else, always match on launch, and a restore would never be detected — so Phases 1, 2 and 5 would sit waiting for a signal that never fires.
The fix, as a prerequisite before Phase 3:
- Add
res/xml/data_extraction_rules.xmland declareandroid:dataExtractionRulesalongside the existingfullBackupContent(keep both — API 24 devices still read the old one). - Exclude the fingerprint prefs from
<cloud-backup>. - Also exclude it from
<device-transfer>— see below.
<data-extraction-rules>
<cloud-backup>
<include domain="database" path="." />
<exclude domain="sharedpref" path="install_fingerprint.xml" />
</cloud-backup>
<device-transfer>
<include domain="database" path="." />
<exclude domain="sharedpref" path="install_fingerprint.xml" />
</device-transfer>
</data-extraction-rules>Why <device-transfer> matters separately. Android 12 split direct phone-to-phone transfer (the setup-wizard cable flow) from cloud backup, with independent rules — allowBackup="false" doesn't even stop D2D anymore. Excluding from <cloud-backup> alone would leave the cable path undetected, and that's a common way to hit the exact scenario this feature exists for.
Also: the EventsStorageState precedent cited here is stale. Its doc comment says "This prefs file is NOT in backup_rules.xml, so it won't be backed up" — no longer true on API 31+, for the same reason. Pre-existing bug and out of scope for this PR, but the citation as "a proven pattern here" should be dropped or qualified, since the pattern is currently broken.
Verification gap worth adding while you're here: scripts/test_cloud_backup.sh drives bmgr over the cloud transport, so it only ever exercised that path. Testing D2D means adb shell bmgr list transports then switching to com.google.android.gms/.backup.migrate.service.D2dTransport — with the documented caveat that you can't restore from D2D via bmgr, so that half stays manual.
There was a problem hiding this comment.
Confirmed and fixed in fd53116. Verified the manifest declares only fullBackupContent with targetSdkVersion = 36, and there is no data_extraction_rules.xml in res/xml/.
This was the right call to block on. The failure mode is the nastiest kind: the fingerprint gets backed up, always matches on launch, restore is never detected, and Phases 1/2/5 sit waiting on a signal that never fires — with nothing anywhere reporting an error. And it is invisible in review precisely because the existing rules file is almost all <include>, so "include everything" has been a working superset right up until the first <exclude>.
Added as Phase 3's first step, with your XML adopted as-is: both attributes kept (API 24–30 still read the old one), and the fingerprint excluded from <cloud-backup> and <device-transfer>. Called out the D2D split explicitly, since the cable flow is a very common way to reach the exact scenario this feature exists for.
Also took the two side points: the EventsStorageState precedent is no longer cited as proven and is filed under a new "Follow-ups outside this plan" section, and verification now specifies running on API 31+ plus the bmgr list transports / D2dTransport route with your caveat that restore-from-D2D stays manual.
| Less than it might appear, because the infrastructure is already in place: | ||
|
|
||
| - **No schema migration.** A brand-new database starts at `version = 1` with no legacy predecessor and no copy-migration step — strictly simpler than the existing three, which all carry legacy baggage. `monitorstorage/MonitorDatabase.kt` is the closest template: single entity, `version = 1`, `exportSchema = false`. | ||
| - **Backed up automatically.** `res/xml/backup_rules.xml` includes `<include domain="database" path="." />`, so any new database file is covered with no config change. This is essential — the identity DB is useless unless it restores alongside the events it describes. |
There was a problem hiding this comment.
Right conclusion, but the reasoning won't survive the dataExtractionRules fix (see the Phase 3 comment).
The identity DB is backed up — but on Android 12+ that's because the platform default includes everything when dataExtractionRules is absent, not because this <include domain="database" path="."/> line is read. It isn't.
Once a data-extraction-rules file exists, the <include> has to be carried into both the <cloud-backup> and <device-transfer> sections, or the identity DB silently stops being backed up on one path — which would be a quiet way to break the whole feature, since the DB is useless unless it restores alongside the events it describes.
Worth rewording so the "no config change needed" claim doesn't outlive the config change that's now required.
There was a problem hiding this comment.
Fixed in fd53116. You are right that the conclusion was accidentally correct — the identity DB does get backed up today, but on API 31+ that is the platform default kicking in because dataExtractionRules is absent, not because the <include domain="database" path="."/> line is read.
Reworded so the claim does not outlive its own premise: it now says the DB is covered by default today, notes why that is not the include doing the work, and states that once Phase 3 adds the rules file the include must be repeated in both <cloud-backup> and <device-transfer>.
Flagged the consequence of getting that wrong, since it is the same silent-failure shape as the fingerprint issue: omitting the include from one section stops backing up the identity DB on that path, and the DB is useless unless it restores alongside the events it describes.
|
|
||
| **Locally-created events that never synced to an account may have a null/empty `UID_2445`.** Those events are also the ones least likely to exist on the new phone at all — a local-only calendar isn't restored by Google. They degrade to unresolved and keep their stale ID: no crash, no data loss. | ||
|
|
||
| `_SYNC_ID` is the fallback when `UID_2445` is empty. It's also server-assigned and stable, but it's the sync adapter's own key rather than the portable iCalendar one, so it's second choice. |
There was a problem hiding this comment.
The fallback ordering is correct, but the reason is stronger than "second choice" suggests — worth stating, because it changes what the fallback actually buys you.
AOSP's CalendarProvider2 notes: "If this event hasn't been sync'ed with the server yet, the _sync_id field will be null."
So _SYNC_ID is null in exactly the case the Caveats section already identifies for UID_2445 — locally-created events that never synced to an account. The fallback doesn't rescue that case; it covers the narrower one where an event did sync but the provider populated _SYNC_ID without UID_2445.
That's still worth having, but it means the two columns don't stack the way a reader might assume. The honest framing: the never-synced case degrades to unresolved regardless of the fallback, which is what the Caveats section already (correctly) concludes.
Related, and worth resolving before building on UID_2445: there's a long-standing Android issue titled "CalendarContract.Events.UID_2445 column is always null" that I couldn't read the resolution of (auth-walled). Since the whole exact-match path depends on this column actually being populated, one throwaway query across a few real calendars — count non-null UID_2445 vs total — would de-risk the core assumption cheaply. If it turns out to be sparsely populated on some providers, Phase 6's heuristic stops being optional and becomes the primary path, which is a materially different plan.
There was a problem hiding this comment.
Both points taken, fixed in fd53116.
On the fallback: you are right that I implied the two columns stack when they do not. Given _sync_id is null for never-synced events, the fallback misses exactly the case the Caveats section already calls out for UID_2445 — it only covers the narrower "synced, but the provider populated _SYNC_ID without UID_2445" situation. Reworded to say that plainly, including that the never-synced case degrades to unresolved regardless.
On the bigger one — that is the right thing to check before building anything, and I had been treating a load-bearing assumption as settled. Added it as an explicit prerequisite ahead of Phase 0: count non-null UID_2445 against total, broken down by account type, on real calendars. Wrote out both branches, since they are genuinely different plans:
- well populated → proceed as written, Phase 6 stays an optional safety net
- sparse on common providers → the exact path is not viable and Phase 6 becomes the primary mechanism
Much cheaper to find that out now than after Phases 0–5 are built on top of it.
Addresses three review threads, one of them blocking. backup_rules.xml is not read on Android 12+. The manifest declares only fullBackupContent (API 30 and below) while targetSdkVersion is 36; API 31+ reads android:dataExtractionRules, which is not declared, so the platform falls back to backing up everything. That has been harmless because backup_rules.xml is almost all <include> -- but Phase 3 is the first thing here needing an <exclude>. As written, the fingerprint would be backed up, always match on launch, and a restore would never be detected, leaving Phases 1/2/5 waiting on a signal that never fires. Adds data_extraction_rules.xml as Phase 3's first step, excluding the fingerprint from both <cloud-backup> and <device-transfer> -- the latter matters separately since Android 12 split cable transfer from cloud backup with independent rules. Corrects the "backed up with no config change" claim: true today only because of the platform default, not because the <include> is read. Once the rules file exists the include must be repeated in both sections or the identity DB silently stops being backed up. _SYNC_ID does not stack with UID_2445 the way the text implied -- AOSP notes _sync_id is null for never-synced events, the same case UID_2445 misses. Adds a de-risking step before Phase 0: verify UID_2445 is actually populated on real calendars, given a long-standing unresolved Android issue claiming it is always null. If sparse, Phase 6's heuristic becomes primary rather than a fallback. Notes EventsStorageState's stale exclusion comment as a separate pre-existing bug, and adds D2D transport testing to verification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z
📊 Code Coverage Summary
|
1 participant
Dev plan for the first half of #273 — restoring a backup on a new phone and having it re-associate with the right calendars.
Problem
Android auto-backup already backs up the app databases (
backup_rules.xmlincludesdomain="database"), so event rows do land on the new phone. ButeventsV9stores only device-local autoincrement IDs —cid(calendar_ID) andid(event_ID). The new device assigns different numbers when it re-syncs, so every restored event is orphaned: wrong calendar attribution, broken filter pills, and "open in calendar" opens the wrong event or nothing.Approach
Capture durable identity at write time — the calendar account tuple plus the event iCalendar
UID_2445— in the unused reserved columns2, so no schema migration or Room version bump is needed. On a detected restore, re-resolve both IDs, retrying until they match (calendars often sync onto the phone after our first launch), plus a manual trigger modeled on the existing pull-to-refresh inCalendarsActivity.Reuses the existing
CalendarBackupInfo+findMatchingCalendarId()matcher, which already does this correctly for the settings JSON export.Also found
ApplicationController.restoreToActive()(line 1321) has a real bug: it callsgetCalendarBackupInfo(context, event.calendarId), looking up the old calendar ID against the new device's provider. That returns null and it silently falls back to the stale ID — it cannot work cross-device by construction.Notes
docs/README.md— per the plan-making skill, that happens when it moves todocs/dev_completed/.🤖 Generated with Claude Code
https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z