diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt index 85789d7..7105f46 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt @@ -5,9 +5,14 @@ package de.jeanlucmakiola.agendula.data.tasks * * Only two values, though the document describes three modes. **Synced is not a * third store**: it is [LOCAL] with an account attached, so it is derived state - * (does an account of ours exist?) rather than something the user picks. That - * also means switching sync on is never a migration. Adding a `SYNCED` constant - * here would imply otherwise. + * (does an account of ours exist?) rather than something the user picks. Adding + * a `SYNCED` constant here would imply otherwise. + * + * ⚠️ This used to add "so switching sync on is never a migration". That is wrong. + * `TaskLists.ACCOUNT_TYPE` is write-once in the provider — `processors/lists/ + * Validating.java:68-76` throws `IllegalArgumentException` on any attempt to + * change it — so attaching an account to an existing local list means recreating + * every list and every task under the new account. See `docs/SYNC.md`. * * Nothing above the data layer reads this; it selects an authority for * [ProviderResolver] and stops there. diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt index e310bdf..520fa7a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/export/ICalendarWriter.kt @@ -57,12 +57,23 @@ object ICalendarWriter { /** * The UID to write for [task]. * - * Local tasks have none: the dmfs provider only permits a sync adapter to - * assign `_uid`, so in Local mode every task arrives here with `uid == null`. - * A VTODO without a UID is invalid and, worse, un-mergeable — re-importing a - * backup would duplicate every task instead of matching it. So we synthesise - * one from the row id, which is stable for as long as the row is, and tag it - * with our own domain so a synthesised UID is recognisable as such. + * Local tasks have none, because nothing assigns one: the provider never + * generates a `_uid` itself, and our write path does not set it either, so in + * Local mode every task arrives here with `uid == null`. + * + * Note this is a gap we leave open, not one the provider imposes. + * `processors/tasks/Validating.java:92-96` restricts `_uid` to sync adapters + * on *update* only; `insert` does not check it, so any caller may assign a UID + * at creation. Doing that would be strictly better than synthesising here — + * see `docs/SYNC.md`, where it is a phase-1 item, because a real UID minted at + * creation is what lets a local task later be pushed to CalDAV without + * duplicating. + * + * Until then: a VTODO without a UID is invalid and, worse, un-mergeable — + * re-importing a backup would duplicate every task instead of matching it. So + * we synthesise one from the row id, which is stable for as long as the row + * is, and tag it with our own domain so a synthesised UID is recognisable as + * such. */ fun uidFor(task: ExportTask): String = task.uid?.takeIf { it.isNotBlank() } ?: "agendula-${task.taskId}@jeanlucmakiola.de" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4fadb62..36d125e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -22,7 +22,7 @@ the original "owns no database" thesis, settled in installed made someone else's roadmap a gate on the app working at all. The database is the vendored dmfs provider under *our* authority — our namespace, not a schema written from scratch — so every CalDAV engine still understands it. A -sync adapter of our own is the 1.x arc. +sync adapter of our own is the 1.x arc, designed in [`SYNC.md`](SYNC.md). The whole design hangs off one rule: @@ -143,9 +143,15 @@ The platform calls sit behind `ProviderEnvironment` so this decision is unit tested on the JVM (`ProviderResolverTest`) rather than only on a device. **Storage modes** are `LOCAL` and `EXTERNAL` only. `STORAGE-AND-SYNC.md` -describes three, but *Synced* is not a third store — it is Local with an account -attached, so it is derived state, and modelling it as a separate mode would imply -that turning sync on is a migration. It isn't. +describes three, but *Synced* is not a third store — it is the same store with an +account attached, so it stays derived state. + +⚠️ **What this used to say — "so turning sync on is not a migration" — is wrong.** +A task list's `ACCOUNT_NAME`/`ACCOUNT_TYPE` are write-once in the provider +(`processors/lists/Validating.java:68-76`), so local lists cannot be re-pointed at +an account; tasks have to be moved into new lists. Not modelling `SYNCED` as a +mode is still right, but the migration it implied away is real. See +[`SYNC.md`](SYNC.md). ### 4.2 `TasksContract` @@ -261,7 +267,9 @@ resolver, so vendoring an entire content provider **changed no UI, no ViewModel, no domain type, and not one line of `TasksRepository`.** Bundling the provider bundles **storage, not sync**. Our own sync adapter is a -separate, later piece of work — see `STORAGE-AND-SYNC.md`. +separate, later piece of work — designed in [`SYNC.md`](SYNC.md), and it lands +*underneath* this same seam: it writes through `TaskContract` with +`CALLER_IS_SYNCADAPTER`, so the layers above it stay untouched a second time. --- diff --git a/docs/OWN-STORE.md b/docs/OWN-STORE.md new file mode 100644 index 0000000..00ab459 --- /dev/null +++ b/docs/OWN-STORE.md @@ -0,0 +1,439 @@ +# Agendula's own task store — architecture and plan + +**Branch:** `feat/own-store` +**Decision:** taken. `docs/STORAGE-DECISION.md` costed it; this is the build. +**Supersedes:** the "keep `:provider`" position in `STORAGE-AND-SYNC.md` and the +"Settled" section of `SYNC.md`. + +--- + +## The decision, in one paragraph + +Agendula stops vendoring the dmfs OpenTasks provider. The `:provider` module — +14,555 lines of Java, 1.66× the size of the app itself — is **deleted**. In its +place the app gets its own Room database, designed for the two things Agendula +actually does: show tasks, and sync them over CalDAV. Support for *external* +providers (OpenTasks, tasks.org) **stays**, unchanged, as a user choice — so +anyone already syncing through DAVx5 keeps working exactly as they do today. + +Agendula becomes a normal Android app with a normal database, plus an optional +compatibility path into somebody else's ContentProvider. + +--- + +## What changes and what does not + +``` +BEFORE AFTER + + UI / ViewModels UI / ViewModels + │ │ + TasksRepository TasksRepository ← unchanged + │ │ + TasksDataSource (interface) TasksDataSource ← unchanged + │ ╱ ╲ + AndroidTasksDataSource RoomTasksDataSource AndroidTasksDataSource + │ │ │ + ContentResolver Room / SQLite ContentResolver + │ │ │ + ┌────┴─────┐ our tables ┌──────┴──────┐ + │ │ │ │ +:provider OpenTasks OpenTasks tasks.org +(deleted) tasks.org (external, unchanged) +``` + +**Unchanged above the data layer.** `TasksRepository`, every ViewModel, every +screen. Verified: exactly one file outside `data/tasks` references +`TasksContract` (`domain/Models.kt`, for five constants), and it stops doing so +in phase 0. + +**Deleted.** The `:provider` Gradle module, its manifest ``, its two +custom permissions, its 84 Java files, its 13 translated string resources, and +its three dmfs runtime dependencies from `:provider`'s own build file. + +**Kept for External mode.** `TasksContract.kt`, `ColumnReader.kt`, +`TaskMapper.kt`, `TaskWriteMapper.kt`, `AndroidTasksDataSource.kt`, +`ProviderResolver.kt`, `ProviderEnvironment.kt`, `TaskProjections.kt`. These +describe *somebody else's* schema and are exactly right for that job. + +--- + +## Storage modes after the change + +```kotlin +enum class StorageMode { + /** Agendula's own Room database. The default; always available. */ + OWN, + /** A tasks provider app already installed — OpenTasks, tasks.org. */ + EXTERNAL, +} +``` + +`LOCAL` (meaning "our bundled dmfs provider") is gone. `ProviderResolver.own` +and the `TaskProvider(isOwn = true)` case go with it: in `OWN` mode there is no +authority, no ContentResolver and no permission to grant. + +`ProviderResolver` narrows to what it was always really for — **discovering +external providers** — and `ProviderStatus.READY` becomes unconditional in `OWN` +mode. + +### Consequences worth stating plainly + +- **No runtime permission is needed for the default path.** Today's permission + prompt only ever applied to External mode; now that is visibly true. +- **Third-party apps can no longer read Agendula's tasks.** We publish no + ContentProvider. Users who need interop pick External mode, or wait for a + possible read-only facade (explicitly out of scope — see *Deliberately not + doing*). +- **Auto Backup gets simpler and safer.** One Room file we control, with a + documented restore path, instead of a provider database whose `cleanUpLists` + routine could delete restored lists whose accounts no longer exist. + +--- + +## The schema + +Four tables. Designed from Agendula's actual reads and writes plus RFC 5545's +`VTODO`, not inherited from a 2013 schema. + +### `task_lists` + +| Column | Type | Notes | +|---|---|---| +| `id` | INTEGER PK | | +| `name` | TEXT NOT NULL | | +| `color` | INTEGER NOT NULL | ARGB | +| `account_id` | INTEGER NULL | FK → `accounts`, NULL = device-only | +| `is_visible` | INTEGER NOT NULL | default 1 | +| `is_synced` | INTEGER NOT NULL | default 1 | +| `owner` | TEXT NULL | CalDAV owner display name | +| `is_read_only` | INTEGER NOT NULL | **new** — the provider could not express this at all | +| `sort_order` | INTEGER NOT NULL | user ordering, which the provider also lacked | +| `href` | TEXT NULL | collection URL, relative to the account root | +| `ctag` | TEXT NULL | | +| `sync_token` | TEXT NULL | RFC 6578, **per collection** — not squatted into a shared slot | +| `is_dirty` | INTEGER NOT NULL | a real boolean, not dmfs's monotonic counter | + +> `account_id` being nullable is the single most important schema change. In the +> dmfs provider `ACCOUNT_TYPE` is write-once and throws on change, which made +> "turn sync on" a full data migration. Here, attaching a local list to an +> account is `UPDATE task_lists SET account_id = ?`. + +### `tasks` + +Master rows *and* recurrence overrides live here; an override is a row with +`recurrence_id` set and `parent_task_id` pointing at its master. + +| Group | Columns | +|---|---| +| identity | `id`, `list_id`, `uid` (NOT NULL, minted at creation), `href`, `etag` | +| content | `title`, `description`, `location`, `url`, `color` | +| state | `status`, `percent_complete`, `completed_at`, `priority`, `classification` | +| time | `dtstart`, `due`, `duration`, `is_all_day`, `timezone` | +| recurrence | `rrule`, `rdate`, `exdate`, `recurrence_id`, `master_id` | +| hierarchy | `parent_id`, `sort_order` | +| audit | `created_at`, `last_modified`, `sequence` | +| sync | `is_dirty`, `is_deleted`, `unknown_properties` | + +Two entries deserve explanation. + +**`uid` is NOT NULL and assigned at creation.** Every task gets a real +RFC 4122 UUID the moment it is inserted, in every mode, synced or not. This +closes the gap `ICalendarWriter.uidFor` currently papers over by synthesising +`agendula-@…`, and it means any local task can later be pushed to a +server without duplicating. The provider never assigned one. + +**`unknown_properties`** holds the raw unfolded iCalendar lines of every +property we do not model — `ATTENDEE`, `CATEGORIES`, `X-*`, `GEO`, and anything +a future RFC adds. On write we re-emit them verbatim after the properties we do +own. This is what makes an honest round-trip possible, and it replaces the +provider's `data0`–`data15` bag with something that cannot silently lose a field +it has no column for. + +Indices: `(list_id, is_deleted)`, `(parent_id)`, `(uid)` unique per list, +`(master_id, recurrence_id)`, `(is_dirty)`. + +### `task_alarms` + +| Column | Notes | +|---|---| +| `id`, `task_id` | FK, `ON DELETE CASCADE` | +| `minutes_before` | positive = before the reference | +| `reference` | `DUE` or `START` | +| `message` | optional | + +Replaces `AlarmHandler` (133 lines of Java) and the `dataN` slot convention. +Delete-and-reinsert stops being necessary — the provider's re-validate-everything +behaviour was the only reason `setAlarm` worked that way. + +### `accounts` + +| Column | Notes | +|---|---| +| `id`, `display_name`, `principal_url`, `home_set_url` | +| `username` | the app password is **not** here — Keystore only, per `SYNC.md` | +| `last_sync_at`, `last_sync_error` | + +Not populated until `SYNC.md` phase 2, but the FK exists from v1 so enabling +sync never requires a schema migration. + +--- + +## Recurrence: expand at read, not on write + +The provider maintained a materialised `instances` table, recomputed by +`Instantiating.java` on every write — and still only ever materialised **one** +upcoming occurrence. + +Agendula expands lazily instead: + +``` +tasks (masters + overrides) ──► RecurrenceExpander ──► List + rrule/rdate/exdate (lib-recur, in memory) occurrences +``` + +This is the right call here because **the repository already filters and sorts +in Kotlin, not SQL**. `TasksRepositoryImpl.loadTasks` reads the whole set, +applies `TaskFiltering.matches`, then `TaskSorting.DEFAULT`. Nothing depends on +the database being able to order by instance time, so nothing is lost — and a +materialised table's entire class of staleness bugs never exists. + +- Bounded window: expansion is capped (default: 1 year back, 2 years forward, + hard ceiling of N occurrences per series) so an unbounded `RRULE` cannot hang + the UI. +- `lib-recur` **pinned at 0.12.2** — 0.16.0 removed `RecurrenceSet`. We pin + because we chose to, and the pin is now ours to lift on our own schedule. +- Client-side expansion is required for CalDAV regardless: server-side + `CALDAV:expand` on `VTODO` is broken on every server `SYNC.md` targets. + +### Completing one occurrence of a recurring task + +The provider's `Detaching.java` implemented **model (d): detach the occurrence +as a brand-new task with its own UID**. We inherited that without ever choosing +it, and it is the model least compatible with CalDAV. + +**We implement model (a): a `RECURRENCE-ID` override.** Completing one +occurrence writes a second `tasks` row with the same `uid`, a `recurrence_id` +naming the occurrence, `master_id` pointing at the series, and the completed +state. This is what RFC 5545 specifies and what every other CalDAV client +expects to receive. + +This decision is now made explicitly, recorded here, and testable. + +--- + +## Reactivity + +`TasksDataSource.registerObserver(onChange: () -> Unit): AutoCloseable` **stays +as-is**. The Room implementation backs it with `InvalidationTracker.Observer` +over the four tables; the External implementation keeps its `ContentObserver`. +One interface, two mechanisms, `TasksRepositoryImpl.observing()` untouched. + +Going Flow-native in the DAOs is a later, optional refinement. Doing it now +would change the interface and therefore the External path, for no user-visible +gain. + +--- + +## Migrating existing users + +Anyone on v0.3.x has their tasks inside the bundled provider's SQLite file at +`/data/data/de.jeanlucmakiola.agendula/databases/tasks.db` (dmfs schema +version 23). Removing the Gradle module does **not** remove that file — an app +update leaves the data directory intact. + +So the migration reads the file directly, with no provider and no +ContentResolver involved: + +``` +OneShotImport + 1. does databases/tasks.db exist? no → nothing to do, mark done + 2. open SQLiteDatabase.OPEN_READONLY + 3. read tasklists → task_lists (account_type LOCAL → account_id NULL) + 4. read tasks → tasks (skip _deleted = 1; mint uid where NULL) + 5. read properties → task_alarms (mimetype = …/alarm only) + 6. verify counts, inside one Room transaction + 7. record completion in DataStore + 8. rename tasks.db → tasks.db.imported (kept one release, then deleted) +``` + +Rules that make this safe: + +- **Read-only, single transaction, verified counts.** Either the whole import + lands or none of it does. +- **The source file is renamed, never deleted**, for one release. If the import + is wrong we can still recover from a user's device. +- **Idempotent.** Guarded by a DataStore flag *and* by the rename, so a crash + mid-import cannot double-import. +- **Runs before first UI read**, gated the same way `StorageModeHolder.awaitReady()` + already gates the launch reminder re-sync. +- Tasks that were in an *external* account inside our bundled provider (only + possible if the user had pointed DAVx5 at our authority) are imported as + local lists, with their `uid` preserved. Rare, but preserving the UID is what + lets them be re-attached to an account later. + +`tasks.db.imported` is excluded from Auto Backup; the new Room database is +included, which is the whole point of owning it. + +--- + +## Effects on the sync plan + +`SYNC.md`'s phase list was written against the provider. Owning the store +deletes work from it outright: + +| `SYNC.md` item | Fate | +|---|---| +| "Assign UIDs at creation" (phase 0) | **gone** — `uid` is NOT NULL from v1 | +| Auto Backup / `cleanUpLists` data-loss guard (phase 0) | **gone** — no `cleanUpLists` | +| `lib-recur` pin rationale (phase 0) | reduced to a normal version choice | +| Local → Synced migration (phase 3) | **gone** — `account_id` is a nullable FK | +| ETag / href / CTag squats into `SYNC1`–`SYNC8` | **gone** — real columns | +| Per-collection sync token (phase 3) | **gone** — real column | +| `_DIRTY` set-on-delete workaround (phase 3) | **gone** — tombstones are ours | +| `CALLER_IS_SYNCADAPTER` ignored by instances URI | **gone** — no URIs | +| `Moving` dual-UID collision (phase 4) | **gone** | +| Read-only collections (phase 4) | now *possible* — `is_read_only` exists | +| Recurring-completion model | **decided here** — RECURRENCE-ID override | +| Byte-stable round-trip | improved — `unknown_properties` preserves the rest | + +Everything platform-level and protocol-level in `SYNC.md` is untouched: the +`targetSdk 34` sync-framework gate, the stub sync-adapter pattern, credential +storage, Play compliance, discovery, conditional `PUT`, conflict policy, and +every per-server quirk in the server-reality table. + +--- + +## Plan + +### Phase 0 — Untangle (0.5 wk) + +- `domain/Models.kt` stops importing `TasksContract`; the status and priority + constants move into `domain`. This is the last contract reference above the + data layer. +- `StorageMode`: `LOCAL` → `OWN`; `ProviderResolver` loses `own` / `isOwn`. +- `ProviderChangeReceiver`'s manifest filter drops our own authority. +- Add Room + KSP to the version catalog (KSP is already applied to `:app`). + +**Done when:** the app still builds and behaves identically, with the provider +still present and still default. + +### Phase 1 — Schema and DAOs (1 wk) + +- The four entities above, plus DAOs, plus `schemas/` exported for migration + testing (`room.schemaLocation`, committed). +- `RoomTasksDataSource` implementing all 14 `TasksDataSource` methods except the + recurrence-dependent ones, which throw until phase 2. +- `DataModule` binds by `StorageMode`. + +**Done when:** a JVM test creates lists and non-recurring tasks through +`TasksDataSource` against an in-memory Room database and reads them back. + +### Phase 2 — Recurrence (1.5–2 wk) + +The hard phase. Budget accordingly. + +- `RecurrenceExpander` over `lib-recur`: `RRULE`, `RDATE`, `EXDATE`, overrides, + all-day handling, bounded window, `distanceFromCurrent`. +- `RECURRENCE-ID` override creation on single-occurrence edit and completion. +- A test suite that is the deliverable, not an afterthought: daily/weekly/ + monthly-by-day/yearly, `COUNT` and `UNTIL`, DST boundaries, all-day series, + a series with an override, a series with an exception, and an unbounded rule + hitting the window ceiling. + +**Done when:** `updateInstance` and recurring reads pass parity tests written +against the current provider's observed behaviour, *except* where model (a) +deliberately differs from model (d) — those differences enumerated as tests. + +### Phase 3 — Semantics parity (1 wk) + +- Completion coherence: `status` ↔ `percent_complete` ↔ `completed_at` ↔ closed, + replacing `AutoCompleting.java` and — importantly — the reopen asymmetry that + `TaskWriteMapper` currently works around in the app. +- Parent/child integrity, orphan handling on delete. +- Validation: `DUE` xor `DURATION`, `due >= dtstart`, all-day pinned to UTC + midnight, list must exist. +- Delete semantics: hard delete when `account_id IS NULL`, tombstone when set. + +**Done when:** `TaskWriteMapper`'s provider-quirk workarounds are demonstrably +unnecessary on the Room path (they stay for External). + +### Phase 4 — Import and cutover (1 wk) + +- `OneShotImport` per the rules above, with tests over a fixture `tasks.db` + captured from a real v0.3.x install. +- `OWN` becomes the default for new installs and for upgraders after import. +- Backup rules updated: include the Room database, exclude `tasks.db.imported` + and the Keystore blob. + +**Done when:** an upgrade from a v0.3.2 APK with seeded data lands every task, +list and reminder in Room, verified by count and by content. + +### Phase 5 — Delete `:provider` (0.5 wk) + +- Remove the module, its `settings.gradle.kts` include, its `:app` dependency, + the three dmfs deps it pulled in, `provider/PROVENANCE.md`. +- Add `lib-recur` (and `rfc5545-datetime`) directly to `:app`. +- Attribution screen: dmfs code is gone, but `lib-recur` stays and is + Apache-2.0. `PROVENANCE.md` is replaced by a short note in + `STORAGE-DECISION.md` recording that the fork existed and why it ended. + +**Done when:** `./gradlew build` is green with `:provider` absent, and the APK +declares no ContentProvider and no custom permissions. + +### Phase 6 — Harden (1 wk) + +- Room migration test infrastructure (`MigrationTestHelper`) wired up, so v1 → + v2 is cheap when sync adds columns. +- Restore-path test: Auto Backup restore into a fresh install. +- Performance check at 5,000 tasks with 20 recurring series. + +**Total: 6–6.5 weeks** to a shipping app with its own store, before any CalDAV +work begins. `SYNC.md`'s own estimate drops by 2.5–4 weeks in exchange. + +--- + +## Testing posture + +| Layer | How | +|---|---| +| Entities, DAOs, migrations | Room in-memory + `MigrationTestHelper`, JVM | +| `RecurrenceExpander` | pure JVM, no Android — the largest suite | +| Semantics (completion, hierarchy, validation) | JVM through `TasksDataSource` | +| `OneShotImport` | fixture `tasks.db` committed as a test resource | +| External mode | unchanged; existing `TaskMapper` / `TaskWriteMapper` tests stay | + +The 93 existing app tests must stay green throughout. The 56 provider tests +leave with the module in phase 5 — replaced, not abandoned: phases 2 and 3 owe +equivalent coverage of the behaviour those tests protected, and phase 2's +parity suite is written against them. + +--- + +## Risks + +| Risk | Mitigation | +|---|---| +| **Recurrence is subtler than estimated** | Phase 2 is isolated and pure-JVM; it can overrun without blocking phases 3–4. The provider stays in-tree until phase 5, so we can always compare against it. | +| **Import loses a user's data** | Read-only source, single transaction, count verification, source file renamed not deleted, fixture-based tests. | +| **Regression in a behaviour nobody documented** | Phase 2's parity tests are written *against the provider while it is still present*. That is why deletion is phase 5, not phase 0. | +| **Losing third-party interop** | External mode covers users who need it. A read-only facade stays possible later; nothing in this design forecloses it. | +| **Room + KSP build cost** | KSP is already in the build for Hilt; Room adds one processor. | + +--- + +## Deliberately not doing + +- **An exported ContentProvider facade over Room.** Possible later (~1–1.5 wk), + not now. Shipping one would recreate the public-API surface whose validation + and URI plumbing is most of what we are deleting. +- **A domain-native schema.** The table shapes above stay recognisably close to + `TaskContract` where `TaskContract` was right, because it is a proven design + for `VTODO` and because it keeps a future facade cheap. +- **Flow-native DAOs.** Later refinement; changes the interface for no + user-visible gain today. +- **FTS / search.** The provider carried 798 lines of it. The app has never + called it. If search is wanted it is a feature request, designed on its own + terms. +- **Categories and attendees as first-class tables.** They round-trip through + `unknown_properties` until a feature actually needs them. diff --git a/docs/README.md b/docs/README.md index eb146c7..b672819 100644 --- a/docs/README.md +++ b/docs/README.md @@ -1,8 +1,10 @@ # Agendula — documentation -Agendula is a Material 3 Expressive **task** app for Android: a pure front-end over -the OpenTasks `TaskContract` provider (synced by DAVx5 / SmoothSync / DecSync -over CalDAV), with no own database or sync stack. Sibling to +Agendula is a Material 3 Expressive **task** app for Android. It **carries its +own task store** — the dmfs task provider vendored under our own authority — so +it is complete and local-first with nothing else installed; an external provider +(OpenTasks / tasks.org, synced by DAVx5 / SmoothSync / DecSync) is a user choice +rather than a requirement, and our own CalDAV sync is the 1.x arc. Sibling to [Calendula](https://codeberg.org/jlmakiola/calendula). See the top-level [`../README.md`](../README.md) for the project pitch. @@ -12,15 +14,24 @@ top-level [`../README.md`](../README.md) for the project pitch. |---|---| | [`ARCHITECTURE.md`](ARCHITECTURE.md) | How Agendula is built **today** — layers, the data seam, provider resolution, the reminder engine, DI, build/tooling, manifest. Start here to work on the code. | | [`ROADMAP.md`](ROADMAP.md) | **Status** and what's next — milestones (M0–M6 + Posture B), what's done, open decisions, how to build/verify. | +| [`STORAGE-AND-SYNC.md`](STORAGE-AND-SYNC.md) | **Where task data lives** — the decision to ship our own provider, the storage modes, permissions, distribution, and the dead ends. Supersedes `PLAN.md` on storage. | +| [`SYNC.md`](SYNC.md) | **How data reaches a server** — the CalDAV sync adapter: the VTODO ↔ `TaskContract` mapper, Nextcloud sign-in, the engine, libraries and their licenses. Step 5 of `STORAGE-AND-SYNC.md`. | +| [`STORAGE-DECISION.md`](STORAGE-DECISION.md) | **Keep the vendored provider, or build our own?** The measured cost of both. **Decided: build our own.** | +| [`OWN-STORE.md`](OWN-STORE.md) | **Agendula's own Room store** — the schema, recurrence design, migration off the vendored provider, and the six-phase plan that deletes `:provider`. Supersedes the "keep the provider" position in `STORAGE-AND-SYNC.md`. | | [`PLAN.md`](PLAN.md) | The original implementation plan and **design rationale** — the A-now-B-later thesis, what transfers from Calendula, the locked decisions. The "why". | | [`RELEASING.md`](RELEASING.md) | How to cut a release — the git-tag-as-source-of-truth flow, CI jobs, F-Droid repo, required secrets. | +| [`../provider/PROVENANCE.md`](../provider/PROVENANCE.md) | What the vendored `:provider` module is, where it came from, and **every** deviation from upstream dmfs. | Also: [`../CHANGELOG.md`](../CHANGELOG.md) (Keep a Changelog format; tag sections feed the release notes). ## How the docs relate -- **PLAN** is the design decisions (mostly stable; the "why"). +- **PLAN** is the original design decisions (the "why"), left as the historical + record. On storage it is **superseded by STORAGE-AND-SYNC**. +- **STORAGE-AND-SYNC** and **SYNC** are the standing decision documents: the + first settles where data lives, the second how it syncs. Both record rejected + alternatives on purpose, so decisions don't get relitigated. - **ARCHITECTURE** is the current shape of the code (kept in sync with the source as it grows). - **ROADMAP** is the moving status layer (update as milestones land). diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 81839d0..564f999 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -142,7 +142,7 @@ squatting `org.dmfs.tasks`, which is a dead end). prune account types we authenticate ourselves — the deletion is unsafe without that rework. Modernized to minSdk 29 / targetSdk 36 / Java 17. [`provider/PROVENANCE.md`](../provider/PROVENANCE.md) records every deviation, - each also marked `AGENDULA CHANGE` at the site. Upstream's 51 JVM tests pass. + each also marked `AGENDULA CHANGE` at the site. Upstream's 56 JVM tests pass. - ✅ Storage modes + the permission-gate bypass — `ProviderStatus.NEEDS_PERMISSION` can no longer fire in Local mode, and an upgrading Posture A user stays on the provider that holds their data (`ProviderResolver.autoMode`). @@ -152,9 +152,12 @@ squatting `org.dmfs.tasks`, which is a dead end). - ⬜ **Frontend surfaces for the above** — a storage-mode picker in Settings and an export screen. The backend is done and unused until these exist. - ⬜ File the DAVx5 issue (step 4) — non-blocking, cheap, serves F-Droid users. -- ⬜ Sync adapter (step 5) — the 1.x arc. Design discussion still open: protocol - coverage, account model, conflict resolution, and `ical4android`'s licence - against our MIT. +- ⬜ Sync adapter (step 5) — the 1.x arc. **Designed in [`SYNC.md`](SYNC.md)**, + not started: mapper → auth → engine → hardening, ~8–9 weeks. The account model + is settled (`AccountManager`) and `ical4android` is closed out (superseded by + `synctools`, GPLv3, so we write the mapper in-house); what's still open is + dav4jvm's JitPack-only distribution, conflict policy, and whether External mode + survives the milestone. - ⬜ Verify on a device: the local path with no account, and the vendored provider's timezone-change behaviour (change 3 in `PROVENANCE.md`). @@ -168,7 +171,9 @@ These carry over from [`PLAN.md`](PLAN.md) §9; resolved ones are struck through 2. ~~**tasks.org provider authority**~~ verified on device: `org.tasks.opentasks` + `org.tasks.permission.*`. 3. **jtx Board** — support its richer contract later, or stay OpenTasks-only? - (Not in the candidate list today.) + (Not in the candidate list today.) Note this is now downstream of + [`SYNC.md`](SYNC.md) open question 3: if External mode is retired once we sync + ourselves, the question disappears with it. 4. ~~**Posture B authority choice**~~ resolved: **our own** `de.jeanlucmakiola.agendula.tasks`. Squatting `org.dmfs.tasks` is a dead end, not merely a trade-off — two apps cannot declare the same authority or @@ -181,8 +186,9 @@ These carry over from [`PLAN.md`](PLAN.md) §9; resolved ones are struck through 6. **Resolver ordering / mode-selection UX** — `autoMode()` picks a sane default today (see [`ARCHITECTURE.md`](ARCHITECTURE.md) §4.1); the Settings override it assumes is not built yet. -7. **Sync protocol coverage**, account model, conflict resolution — the next - design discussion. +7. ~~**Sync protocol coverage**, account model, conflict resolution — the next + design discussion.~~ Taken up in [`SYNC.md`](SYNC.md); the remaining opens + live on that document's list. --- diff --git a/docs/STORAGE-AND-SYNC.md b/docs/STORAGE-AND-SYNC.md index aba0a6f..4cfb065 100644 --- a/docs/STORAGE-AND-SYNC.md +++ b/docs/STORAGE-AND-SYNC.md @@ -1,5 +1,15 @@ # Agendula — storage and sync +> ⚠️ **Partly superseded, 2026-08-13.** The core decision below — *vendor the +> dmfs provider in-tree as `:provider`* — has been **reversed**. Agendula builds +> its own Room store and deletes the vendored provider; External mode (OpenTasks, +> tasks.org) is unaffected and everything this document says about it still +> stands. See [`STORAGE-DECISION.md`](STORAGE-DECISION.md) for why and +> [`OWN-STORE.md`](OWN-STORE.md) for what replaces it. The permissions, +> distribution, storage-mode and dead-end sections below remain accurate; treat +> the "our own provider" sections as the historical record of a decision that was +> made, shipped, and then costed properly. + > Decided direction, captured 2026-08-01. Supersedes the earlier "Posture B = > bundle OpenTasks" working notes, which are withdrawn (see > [Dead ends](#dead-ends--do-not-revisit)). This is the detailed companion to @@ -34,7 +44,7 @@ | 2 | Vendor `:provider` under our own authority | the identity, done once — and it ships a complete local-first app | ✅ done | | 3 | Export / backup | our data now lives only in our app's private storage | ✅ backend done; no UI yet | | 4 | File the DAVx5 issue | cheap, non-blocking, serves F-Droid users | ⬜ | -| 5 | Sync adapter | the 1.x arc; design discussion pending | ⬜ | +| 5 | Sync adapter | the 1.x arc; designed in [`SYNC.md`](SYNC.md), not yet built | ⬜ | Everything below is the reasoning behind those choices, the alternatives that were rejected, and the constraints they have to survive. @@ -174,8 +184,13 @@ feature — see [Storage modes](#storage-modes--the-users-choice). | **Synced** | our bundled provider | our sync adapter | network + an account the user configures | | **External** | OpenTasks / tasks.org | whatever that provider's engine does (DAVx5 …) | that provider's `READ`/`WRITE_TASKS`, granted at runtime | -Local and Synced are the same store — Synced is Local with an account attached, -so switching on sync is not a migration. +Local and Synced are the same store — but ⚠️ **switching on sync *is* a +migration, contrary to what this document said until 2026-08-13.** The provider +enforces `ACCOUNT_NAME` and `ACCOUNT_TYPE` as **write-once** on a task list +(`processors/lists/Validating.java:68-76`, which throws), so a list created under +`org.dmfs.account.LOCAL` can never be re-pointed at a real account. Enabling sync +means creating new lists under the account and moving tasks into them. See +[`SYNC.md`](SYNC.md) — it is a costed deliverable there, not a free consequence. **Resolver ordering — decided, and it went both ways as expected.** `ProviderResolver` now takes an explicit `StorageMode` from Settings when there @@ -235,10 +250,17 @@ compliance and a small enum-shaped addition with near-zero ongoing maintenance for them. Say that explicitly. "Here's a change that can't break anything" lands very differently from "please support my app." -**Open — the next discussion.** Protocol coverage ("support as much as -possible"), the account model, conflict resolution, and where the DAV/iCalendar -work lives. One constraint to settle early: we're MIT; `dav4jvm` is Apache-2.0 -and fine, but **verify `ical4android`'s license** before assuming it's usable. +**The design is now written out in [`SYNC.md`](SYNC.md)** — protocol coverage, +the account model, conflict resolution, the VTODO ↔ `TaskContract` mapper, and +where the DAV/iCalendar work lives. Two corrections to what this section +originally said, both verified 2026-08-13: + +- `dav4jvm` is **MPL-2.0**, not Apache-2.0. Still fine against our MIT (file-level + copyleft), but it is ⚠️ **JitPack-only**, which collides with our + `FAIL_ON_PROJECT_REPOS` + `google()`/`mavenCentral()` policy and with the + JitPack dead end below. `SYNC.md` open question 1. +- `ical4android` is **superseded by `synctools`, which is GPLv3** — so it is + unusable, and the open question below is closed. We write the mapper in-house. --- @@ -336,6 +358,13 @@ Not on either yet; both are targets, so build *for* them rather than retrofittin throwaway spike* (a library string resource can be overridden from the app module, so the authority rename works), but not for anything we ship. + ⚠️ **This one comes back.** It is a dead end *for the provider*, where + vendoring was mandatory anyway. `dav4jvm` is JitPack-only, so the sync adapter + has to answer the same question on its own terms — and F-Droid turns out not to + be the obstacle (its inclusion policy trusts jitpack.io for freely-licensed + artifacts); our own trust-surface policy is. See [`SYNC.md`](SYNC.md) open + question 1. + --- ## Sequencing @@ -358,8 +387,11 @@ the roadmap should say so rather than inheriting the old estimate. ## Open questions -1. **Sync protocol coverage**, account model, conflict resolution — the next - discussion. Still open. +1. **Sync protocol coverage**, account model, conflict resolution — ✅ taken up in + [`SYNC.md`](SYNC.md). The account model is answered there (`AccountManager`, + which `PROVENANCE.md` change 1 already assumes); what stays open moves to that + document's own list — dav4jvm's distribution, conflict policy, External mode's + future, and recurring-task completion. 2. **Resolver ordering** — ✅ decided, see [Storage modes](#storage-modes--the-users-choice). The **mode-selection UX** is still open: `autoMode()` picks a default, but the Settings override it @@ -376,9 +408,11 @@ the roadmap should say so rather than inheriting the old estimate. ⚠️ **But that test is Robolectric, and it skips on ARM64**, where Robolectric has no SQLite backend in either mode. It runs on x86_64 CI. It is not a substitute for a device, and this remains on the device-verification list. -4. **`ical4android` licensing** vs our MIT. Still open — and note the export path - does *not* depend on it: `ICalendarWriter` is our own ~200 lines, no library. - The question is really about the sync adapter's iCalendar *parsing*. +4. ~~**`ical4android` licensing** vs our MIT.~~ ✅ Closed: `ical4android` is + superseded by `synctools`, which is **GPLv3**, so it is out — as is + `cert4android`. The export path never depended on it anyway (`ICalendarWriter` + is our own ~200 lines). The sync adapter's iCalendar parsing goes through an + in-house mapper over `ical4j`/`biweekly`; see [`SYNC.md`](SYNC.md). 5. **jtx Board** as an additional External-mode candidate — richer contract, later. (`PLAN.md` decision #3, still open.) 6. **The vendored provider's timezone-change behaviour** — upstream's receiver diff --git a/docs/STORAGE-DECISION.md b/docs/STORAGE-DECISION.md new file mode 100644 index 0000000..bf21c4d --- /dev/null +++ b/docs/STORAGE-DECISION.md @@ -0,0 +1,246 @@ +# Storage: keep the vendored provider, or build our own? + +**Status:** **decided — build our own.** See [`OWN-STORE.md`](OWN-STORE.md) for +the architecture and plan; this document is the reasoning that got there. + +The decision went further than the recommendation below: `:provider` is not kept +alongside a Room store, it is **deleted**. External mode (OpenTasks, tasks.org) +stays. The staged sequencing survives in a different form — the provider remains +in-tree until `OWN-STORE.md` phase 5 so recurrence parity can be tested against +it, then goes. + +This reopens a question `SYNC.md` marked settled. It is reopened on purpose: the +argument that settled it was *"the provider hands us the sync bookkeeping for +free"*, and the phase-1 audit found most of that bookkeeping broken, absent, or +unusable for our purposes. A conclusion is only as good as its premise. + +--- + +## The three options + +| | What it means | Store | Exported provider | +|---|---|---|---| +| **A** | Keep `:provider` as-is | dmfs `TaskProvider` | yes, ours today | +| **B** | Room, same `TaskContract` shape | our Room DB | dropped, or a later facade | +| **C** | Room, clean domain schema, `TaskContract` only as an export format | our Room DB | no | + +External mode (talking to OpenTasks / tasks.org) is orthogonal and survives all +three. It is the reason nothing below gets deleted. + +--- + +## What the swap actually touches — measured, not estimated + +### The seam is already there, and it is clean + +`TasksDataSource` (`data/tasks/TasksDataSource.kt`, 58 lines) is a **14-method, +domain-shaped interface**. It takes and returns `Task`, `TaskList`, `TaskForm` — +no `Cursor`, no `Uri`, no `ContentValues`. + +Above it, **17 files** import from `data.tasks`. What they import: + +``` +8 × TasksRepository 4 × TasksDataSource 3 × ProviderResolver +4 × recoveringFromProviderFailure 2 × ProviderStatus +1 each: StorageMode, StorageModeHolder, ProviderEnvironment, TaskQuery, … +``` + +**Exactly one file outside the data package touches `TasksContract` at all** — +`domain/Models.kt`, and only for four status integers, one priority constant and +one account-type string. Ten lines. Nothing else above the data layer knows a +ContentProvider exists. + +> The whole UI, all five milestones of it, is untouched by a storage swap. +> That is not luck — `AndroidTasksDataSource`'s own KDoc says the seam exists so +> that *"swapping the provider never reaches above this file."* It holds. + +### Nothing gets deleted + +| File | Lines | Under a Room store | +|---|---|---| +| `AndroidTasksDataSource.kt` | 220 | **kept** — External mode still needs it | +| `TasksContract.kt` | 178 | **kept** — External mode speaks it | +| `TasksRepositoryImpl.kt` | 151 | unchanged | +| `ProviderResolver.kt` | 143 | unchanged | +| `TaskWriteMapper.kt` | 118 | **kept** for External | +| `TaskMapper.kt` | 102 | **kept** for External | +| `TasksDataSource.kt` | 58 | unchanged — it is the interface | +| `TasksRepository.kt` | 53 | unchanged | +| `ProviderEnvironment.kt` | 51 | unchanged | +| `StorageModeHolder.kt` | 47 | unchanged | +| `ColumnReader.kt` | 40 | **kept** for External | +| `ProviderFlow.kt` | 31 | unchanged | +| `StorageMode.kt` | 30 | one new constant | +| `TaskProjections.kt` | 24 | **kept** for External | +| `Failures.kt` | 16 | unchanged | +| | **1,262** | **0 removed** | + +The work is **additive**: a second `TasksDataSource` implementation, a third +`StorageMode`, and one `@Binds` becoming a dispatcher. `DataModule.kt` has a +single binding to change. + +This reframes the question. It is not *rewrite vs. keep*. It is **write a second +backend behind an interface that exists for exactly this purpose, and run both +until one wins.** + +--- + +## What the new backend has to do + +Room entities and DAOs for the ~50 columns the app actually uses across four +tables are mechanical. The real work is the behaviour the provider's processors +perform. Measured against `:provider`'s Java: + +| Behaviour | Provider | Notes | +|---|---:|---| +| Instance expansion | ~1,070 | `Instantiating` + `instancedata` + iterables. **The hard one.** | +| Recurring-instance edit | 337 | `Detaching` — this *is* the recurrence-model decision | +| Completion coherence | 210 | `AutoCompleting`: status ↔ percent ↔ completed ↔ is_closed | +| Validation | 601 | three processors, mostly defending a *public* API | +| Parent / child | 269 | we use `parent_id` only | +| Alarm property rows | 133 | one Room entity | +| | **~2,620** | | + +And what we would **not** write, of the 14,555 vendored lines: + +| Not needed | Lines | Why | +|---|---:|---| +| `TaskDatabaseHelper` | 895 | 23 migrations from a 2013 schema. We start at v1. | +| `FTSDatabaseHelper` + ngrams | 798 | **the app never searches the provider** — verified, zero call sites | +| `model/adapters` | 1,581 | a type-safe layer over `ContentValues`. Room entities delete the problem. | +| `model` | 1,811 | cursor ↔ entity adaptation. Room's job. | +| `TaskProvider` + `SQLiteContentProvider` | 1,772 | URI matching, permissions, batch ops — for a public API | +| `CategoryHandler` + `RelationHandler` | 553 | unused | +| `utils` (most) | ~800 | dmfs jems idiom → Kotlin stdlib | +| **≈ 8,200 lines we would simply not have** | | | + +Two things make instance expansion less frightening than its line count: + +1. **We need client-side recurrence expansion regardless.** Server-side + `CALDAV:expand` on `VTODO` is broken on every server we target (`SYNC.md`), + so `lib-recur` is in the build either way. +2. **We would use the same eight `lib-recur` classes the provider does** — + `RecurrenceRule`, `RecurrenceSet`, `RecurrenceSetIterator`, `RecurrenceList`, + `RecurrenceRuleAdapter`, `DateTime`, `Duration`, + `InvalidRecurrenceRuleException`. The algorithm is in the library, not in the + provider. +3. And the provider's expansion **materialises only one upcoming occurrence + anyway** — it is not the complete implementation its size suggests. + +--- + +## The cost, both directions + +### Building it + +| | | +|---|---:| +| Schema, entities, DAOs | 1 wk | +| Instance expansion on `lib-recur`, with a real test suite | 1.5–2 wk | +| Completion / parent / validation semantics | 1 wk | +| Recurring-edit model — *shared cost, phase 1 either way* | (0.5–1 wk) | +| Migrating existing users' local data out of the provider | 0.5 wk | +| Tests to parity with the current 93 + 56 | 1 wk | +| **Net additional** | **4.5–6 wk** | + +### What it removes from the sync plan + +Roughly sixteen of the phase-1 audit's storage findings are **provider-imposed** +— they exist only because we run dmfs's implementation, and vanish when we own +the store: + +- `_DIRTY` not set on delete, and defaulting to `1` +- `TaskLists._DIRTY` as a monotonic counter, not a flag +- the instances URI ignoring `CALLER_IS_SYNCADAPTER` +- no home for a per-collection sync token, href, ETag or CTag — all four squat + into generic `SYNC1`–`SYNC8` slots +- **read-only collections cannot be represented at all** (`ACCESS_LEVEL` inert) +- sync-adapter delete ignoring the account parameters it forces you to supply +- `Moving` leaving a dual-UID collision +- `ACCOUNT_TYPE` write-once → enabling sync is a full data migration +- Auto Backup restore arming `cleanUpLists` → silent task loss +- `Detaching` deciding the recurring-completion model for us +- the `lib-recur` version trap (0.16.0 removed `RecurrenceSet`) — we pin because + the provider does, not because we want to + +Conservatively that is **2.5–4 weeks** off phases 0, 3 and 4 of the 11.5–15 week +sync plan, plus a class of bug that is currently *unfixable without patching +vendored Java*. + +### Net + +**≈ +1 to +3.5 weeks**, for a store we control, in exchange for two real losses. + +--- + +## The honest case for keeping it (Option A) + +Not nothing, and it should not be waved away: + +- **It works, and it has 56 passing JVM tests** over recurrence, reparenting, + instances and observers. A Room reimplementation is *new code with new bugs*, + in the layer that holds the user's only copy of their data. That risk is real + and it points at A. +- **Tombstones actually work.** Soft delete for account rows, hard delete for + sync adapters, hidden from normal queries, undelete refused. Of all the sync + bookkeeping, this is the piece that held up under audit. +- **The exported provider under our own authority** — third-party apps can read + Agendula's tasks, and asking DAVx5 to sync us stays possible. +- Eleven local modification sites, all marked `AGENDULA CHANGE`, all documented + in `provider/PROVENANCE.md`. The fork is under control today. + +And the case against keeping it: + +- **14,555 lines of Java — 1.66× the entire app** (8,783 lines of Kotlin). We + carry, build, lint, translate and ship all of it to use maybe a third. +- Upstream is effectively dormant; every future `targetSdk` bump and every + Android SQLite behaviour change lands on us, in someone else's code, in a + language the rest of the app does not use. +- It makes behavioural decisions on our behalf (`Detaching`, `AutoCompleting`) + that we then have to reverse-engineer before we can honour them over CalDAV. + +--- + +## Recommendation + +**Option B — build our own store on Room, keeping the `TaskContract` *shape* as +the internal model — and keep `:provider` in-tree while we do.** + +Three reasons, in order of weight: + +1. **The seam already exists and the work is additive.** Nothing is deleted, + nothing above `data/tasks` changes, and both backends can ship side by side + behind `StorageMode`. The "big rewrite" this decision was originally weighed + against does not exist. +2. **The premise that settled it is gone.** The provider was kept for sync + bookkeeping we have since measured as broken. Sixteen findings deep, keeping + it is now a *cost* to the sync plan, not a saving. +3. **The window is now.** After phase 1 the mapper and engine are written against + whichever store won, and this stops being a two-file change. + +Keeping the `TaskContract` *shape* rather than going domain-native (Option C) is +deliberate: it is a proven schema for exactly this problem, other engines +understand it, and it keeps a future exported facade cheap — without obliging us +to run a 2015 Java implementation of it. + +### Sequencing that keeps the risk low + +1. Add `StorageMode.OWN` and a Room `TasksDataSource`. Both backends live. +2. Ship it behind a setting; the vendored provider stays the default. +3. Run the sync engine against Room only. +4. Once Room has real production mileage, decide whether the exported + ContentProvider is worth re-implementing as a thin facade (~1–1.5 wk) or + whether External mode already covers everyone who wanted it. + +Step 4 is a genuinely open question and does not need answering now. That is the +point of sequencing it last. + +--- + +## Open + +- **Is an exported provider worth keeping at all?** It matters only if third + parties should read our tasks, or if we want DAVx5 to sync our store. External + mode arguably already serves the second. Undecided. +- **The Room estimate is mine, not measured.** Instance expansion is the item + that could overrun; everything else is well-bounded. diff --git a/docs/SYNC.md b/docs/SYNC.md new file mode 100644 index 0000000..4938dd8 --- /dev/null +++ b/docs/SYNC.md @@ -0,0 +1,1185 @@ +# Agendula — CalDAV sync + +> Design notes for Agendula's own sync adapter. Drafted 2026-08-13; **audited the +> same day** against the platform, the vendored provider source, and the current +> state of every library named. The audit refuted or corrected a substantial part +> of the first draft — the corrections are marked ⚠️ **inline and kept visible** +> rather than quietly rewritten, because most of them are things the next person +> would otherwise assume again. +> +> This is step 5 of [`STORAGE-AND-SYNC.md`](STORAGE-AND-SYNC.md)'s sequencing. +> That document decided **where task data lives**; this one decides **how it gets +> to a server**. +> +> Status: **draft / decision document.** Nothing here is built. Where a question +> is already answered by shipped code, it is marked ✅ and the code is named. +> +> ⚠️ **The storage question this document declared settled was reopened, and the +> answer changed.** Agendula is building its own Room store and deleting the +> vendored provider — see [`STORAGE-DECISION.md`](STORAGE-DECISION.md) and +> [`OWN-STORE.md`](OWN-STORE.md). Roughly sixteen of the findings below are +> **provider-imposed** and disappear with it; `OWN-STORE.md` § *Effects on the +> sync plan* lists them item by item. Everything platform-level (the targetSdk 34 +> sync gate, the stub adapter, credential storage, Play compliance) and everything +> protocol-level (discovery, RFC 6578, conditional PUT, conflict policy, the +> server-reality table) is unaffected and remains the plan. + +Scope: self-hosted CalDAV first (Nextcloud), F-Droid and Play, MIT license. + +--- + +## The plan, in short + +| Question | Direction | +|---|---| +| Where does task data live? | Our vendored `:provider` — **settled, shipped** | +| Who syncs it? | Agendula, via its own sync adapter | +| Account model | `AccountManager` **+ a real (stub) sync adapter** — ⚠️ the hybrid without one does not work | +| Scheduling | WorkManager, triggered *through* the sync framework | +| Protocol library | `dav4jvm` (MPL-2.0) — ⚠️ costs more than the first draft assumed | +| Self-signed certs | `cert4android` — ⚠️ **MPL-2.0, not GPLv3.** The first draft rejected it on a false premise | +| iCalendar | In-house mapper over `ical4j`; **no `synctools`** (GPLv3) | +| Recurrence | Read/write `RRULE`/`RDATE`/`EXDATE` directly — ⚠️ **not** the Instances table | +| Primary read path | ⚠️ `REPORT calendar-query` (VTODO filter, no time-range); `sync-collection` is the optimisation | +| Recurring completion | ⚠️ Accept all four models; `:provider` has **already chosen model (d)** for us | +| Sign-in | Nextcloud Login Flow v2 + generic CalDAV discovery + Digest | +| Conflict policy | `If-Match`; on 412 the server wins, local copy preserved | +| Reference implementations | jtx Board and DAVx5 — read, never link against (GPLv3) | + +| # | Phase | Deliverable | Effort | +|---|---|---|---| +| 0 | **Groundwork** | UIDs at creation, backup/prune safety, licence-attribution screen, Java-21 decision | 1 week | +| 1 | Mapper | VTODO ↔ `TaskContract`, unknown-property round-trip, fixture corpus | 2–3 weeks | +| 2 | Auth | Discovery, Login Flow v2, Digest, credential storage, cert trust | 1.5–2 weeks | +| 3 | Engine | `calendar-query` baseline, `sync-collection` optimisation, full reconciliation, conflicts, scheduling, **Local→Synced migration** | 5–6 weeks | +| 4 | Hardening | Per-server trap matrix, error UX, re-auth, Play compliance | 2–3 weeks | + +⚠️ **Revised upward from the first draft's 8–9 weeks to 11.5–15.** Phase 0 is new; +the migration in phase 3 was previously believed not to exist at all; and the +engine grew a second sync path plus a permanent reconciliation pass. + +**Calibration, for sanity:** Evolution shipped RFC 6578 in **June 2026** against +a request open since 2019. vdirsyncer has declined to implement it for twelve +years. Thunderbird still carries unlanded patches for one of its error paths. +This is not a phase that gets shortened by trying harder. + +--- + +## Settled — the storage question is not reopened here + +A later working draft re-argued storage as a fresh choice between *bundle the +dmfs provider* and *a Room-native store with a `TaskContract` facade*, and +recommended Room. Not adopted; this is the audit trail so it does not come back. + +The provider shipped in `f978c37`. Three of the arguments against it do not +survive contact with the repository: + +1. **"`synctools` is GPLv3 and propagates to the app."** Misattributed. + `:provider` depends on `jems`, `rfc5545-datetime` and `lib-recur` — all + Apache-2.0. `synctools` is a *mapper* choice, equally avoidable either way. +2. **"Cursor access everywhere; the widget pays the Calendula cost."** Calendula + queries `CalendarProvider` **cross-process**; ours is same-package, same-uid, + so `ContentResolver` returns the local provider instance — no Binder hop, no + `CursorWindow` marshalling. The observer→Flow seam already exists. +3. **"The dmfs schema can't hold what we need."** The `Properties` table takes + arbitrary mimetypes — `PropertyHandlerFactory.get` falls through to + `DEFAULT_PROPERTY_HANDLER`, and `PropertyHandler.insert` is a bare + `db.insert` with no validation, over sixteen real `data0`–`data15` TEXT + columns. That is the mechanism for unknown-property round-tripping, this + document's single most important correctness requirement. + +**But the audit also weakened the positive case.** The first draft claimed the +provider hands us the sync bookkeeping for free. It hands us *some* of it, with +sharp edges — see the next section. Room would have been the wrong trade anyway +(it buys Flow ergonomics we already have and costs the tombstone/instance +machinery we already run), but "for free" was too generous. + +--- + +## What the provider actually gives us + +⚠️ **This section is almost entirely rewritten.** Every line is verified against +`provider/src/main/java/`. + +| Mechanism | Reality | +|---|---| +| `CALLER_IS_SYNCADAPTER` | Works on `tasks` and `tasklists`. ⚠️ **Ignored on the `instances` URI** — `processors/instances/TaskValueDelegate` hardcodes `false` on every delegation | +| `_DIRTY` on user writes | ⚠️ **Set on insert/update only, never on delete.** `AutoCompleting.delete` skips `updateFields`; `TaskCommitProcessor.delete` sets only `_DELETED` | +| `_DIRTY` default | ⚠️ **Defaults to `1`** (`TaskDatabaseHelper:456`). Every downstream insert must write `_dirty=0` explicitly or it uploads straight back | +| `TaskLists._DIRTY` | ⚠️ **A monotonic counter, not a flag** — a trigger does `_dirty = _dirty + new._dirty + new._deleted` and nothing ever decrements it | +| `_DELETED` tombstones | ✅ Real. Soft-delete for account-backed tasks, hard delete for sync adapters *and* for the local account. Hidden from non-sync queries | +| Instances | ⚠️ **Materialises exactly one upcoming occurrence** (`UPCOMING_INSTANCE_COUNT_LIMIT = 1`), on write only, never re-expanded as time passes. Useless as a recurrence representation for sync | +| `_UID` | ⚠️ **Settable by anyone on insert** — restricted to sync adapters on *update* only. Never generated by the provider | +| `SYNC1`–`SYNC8`, `_SYNC_ID`, `SYNC_VERSION` | Exist and are free — but ⚠️ **`Moving` nulls all of them on a list move while keeping `_UID`**, leaving a live row and a tombstone sharing one UID | +| Per-collection sync state | ⚠️ **Does not exist.** The `SyncState` table is one blob **per account**, `db.replace`d | +| Read-only collections | ⚠️ **Cannot be represented.** `ACCESS_LEVEL` is inert — the contract says "not used yet", and `Validating.java:60` still carries upstream's `// TODO: ensure that the list is writable` | +| Account scoping on delete | ⚠️ The provider **requires** account params on a sync-adapter task delete and then **ignores them** (`TaskProvider.java:793` — upstream `// TODO`) | + +### The consequences, as rules + +1. **The upload query is `_dirty = 1 OR _deleted = 1`.** Deleting is not dirtying. +2. **Every downstream insert sets `_dirty = 0` explicitly.** +3. **Name the squats now**, because none of these columns exist: CTag → + `TaskLists.SYNC_VERSION`; per-task ETag → `Tasks.SYNC_VERSION`; href → + `Tasks._SYNC_ID`; per-collection sync-token → a `TaskLists.SYNC*` slot (the + `SyncState` blob is per-account and cannot hold it). +4. **Never write through the `instances` URI.** Read and write `RRULE`, `RDATE` + and `EXDATE` on `tasks` — they are raw TEXT and round-trip cleanly. +5. **Scope every adapter delete by `list_id` yourself.** +6. **Sequence DELETE before PUT on a list move**, or the two rows sharing a + `_UID` collide on the server. +7. **Read-only collections are enforced in the app layer**, not the provider. +8. **Soft-deleted subtasks lose their `Relation` rows** before the adapter sees + the tombstone (`Reparenting.unlinkParent` runs regardless of `isSyncAdapter`). + Either cache the relation or accept that `RELATED-TO` is unrecoverable on + delete. + +### Two collisions with the shipped app layer + +- **`AndroidTasksDataSource.setAlarm` deletes every alarm property row on the + task** before re-inserting. Once the adapter round-trips `VALARM`s, one local + reminder edit destroys all server-side alarms on that task. Decide ownership: + either the adapter owns `VALARM`s and the app stops bulk-deleting, or reminders + stay local-only and are never serialised. +- **`updateInstance` routes through the instances URI**, which per the table + above is permanently non-sync-adapter and forks override rows. Those overrides + arrive `_dirty=1` with no `_uid`/`_sync_id` and must be uploaded as + `RECURRENCE-ID` components. + +--- + +## ⚠️ The migration that was believed not to exist + +The first draft, `STORAGE-AND-SYNC.md` and `ARCHITECTURE.md` all asserted: +*"Synced is Local with an account attached, so switching on sync is not a +migration."* **That is false.** + +`processors/lists/Validating.java:68-76` throws on any attempt to change a task +list's `ACCOUNT_NAME` or `ACCOUNT_TYPE` — both are write-once, and the contract +documents them as such. Local lists live under `org.dmfs.account.LOCAL` and can +never be re-pointed at a real account. + +Turning on sync therefore means, as a real deliverable with its own tests: + +1. Create new lists under the account (sync-adapter insert; account params come + from the **URI**, never the values, and are frozen thereafter). +2. `UPDATE list_id` on every task — this *is* permitted, and it is the one path + the provider gives us. +3. Assign `_UID`s (or better: have them already, see phase 0). +4. Delete the old local lists (sync-adapter only). +5. Re-point `DEFAULT_LIST_ID`, per-list reminder overrides in DataStore, and + every scheduled alarm. + +Not modelling `SYNCED` as a third `StorageMode` is still right — it is derived +state. But the migration it implied away is real, and step 2's `Moving` processor +nulls `_sync_id`/`sync_version`/`SYNC1`–`SYNC8` and clones a tombstone as it goes. + +**Alternative worth considering:** don't migrate. Synced lists are always *new* +lists, and moving local tasks into them is an explicit user action with a visible +UI. Cheaper, more honest, and it never silently rewrites the user's data. + +--- + +## Account model — and the trap under it + +**Decision: `AccountManager` with our own account type, plus a registered +`` service whose entire job is to enqueue a WorkManager job.** + +⚠️ **The first draft's justification was circular** and its architecture did not +work. Both corrected: + +### The justification + +The draft argued AccountManager was *required* because the provider prunes lists +whose `ACCOUNT_TYPE` has no authenticator in this package. That is backwards — +`PROVENANCE.md` change 1 made the prunable set **empty by construction** +precisely so that shipping no authenticator prunes nothing. The provider imposes +no requirement at all. tasks.org proves the alternative: no AccountManager, Room +accounts, pure WorkManager, `GET_ACCOUNTS` stripped with `tools:node="remove"`. + +The real reasons, which are still good ones: a stable account identity that +third-party engines can address (the DAVx5 ask, open question 5), presence in +system Settings, and the sync framework as a change-trigger. + +### The trap ⚠️ + +`ContentService.hasAuthorityAccess()` gates `requestSync`, `setSyncAutomatically`, +`addPeriodicSync`, `setIsSyncable`, `getSyncStatus` and seven more behind a +compat change `@EnabledAfter(TIRAMISU)` — **on for targetSdk ≥ 34**, which we +are. With no package registering a sync adapter for our authority, every one of +those calls **returns silently**: no exception, no log. It is documented on no +Android behaviour-changes page. + +So "AccountManager accounts for visibility + WorkManager for scheduling + no sync +adapter" — exactly what the first draft described — yields: + +- every `ContentResolver` sync API a no-op that passes on a Robolectric shadow, +- an account in system Settings permanently reading **"Sync off for all items"**, +- a **greyed-out "Sync now"**, because `enabledSyncNowMenu()` needs at least one + checked authority switch. + +**Fix:** register a real `AbstractThreadedSyncAdapter` whose `onPerformSync` +enqueues a WorkManager job and waits — DAVx5's own comment: *"We use the sync +adapter framework only for the trigger, actual syncing is implemented with +WorkManager."* Declare `READ_SYNC_SETTINGS` / `WRITE_SYNC_SETTINGS`. Ship an +in-app sync button too, since "Sync now" stays greyed out under +`userVisible="false"`. + +**Free trigger already running:** `TaskProvider.syncToNetwork()` returns `true` +unconditionally, so `SQLiteContentProvider` already fires +`notifyChange(uri, null, syncToNetwork=true)` on every user write. The system is +*already* requesting a sync for our authority on each edit — there is simply +nothing registered to receive it. + +### ⚠️ Auto Backup will arm `cleanUpLists` into a data-loss path + +`backup_rules.xml` and `data_extraction_rules.xml` are both **empty rule sets**, +and `allowBackup="true"`. An empty set means Auto Backup's default: databases +included. So the provider's `tasks.db` is backed up and restored — while +AccountManager accounts, which live in `/data/system_ce/`, are not. + +`TaskProvider.onCreate` registers `addOnAccountsUpdatedListener(…, updateImmediately=true)`. +On the first callback after a restore, `cleanUpLists` sees lists carrying our +`ACCOUNT_TYPE` with no matching account and **deletes them, and their tasks, +silently** — cascading through `task_list_cleanup_trigger`. Exactly the failure +`PROVENANCE.md` change 1 exists to prevent, reintroduced from behind. + +Latent today (no authenticator ⇒ nothing prunable); live the day phase 3 ships. + +**Do not fix this by excluding the database from backup.** That was the audit's +suggestion and it is wrong for us: Local-mode data lives in exactly one place, +and Auto Backup is currently its only automatic safety net — removing it to +protect *synced* lists would trade a latent bug for a live one. Fix it at the +cause instead: + +1. Make pruning **event-driven** — react to `AccountManager`'s account-removed + broadcast, not to "absent from the visible set". +2. Add a post-restore reconciliation that offers to **re-attach the account** + rather than deleting. +3. Exclude only the **Keystore-encrypted credential blob** from backup — a + restored ciphertext is permanently undecryptable, since Keystore keys are + non-exportable. +4. Device-verify: restore a backup onto a fresh device, confirm nothing is pruned. + +--- + +## VTODO ↔ `TaskContract` + +In-house, behind an interface, so the iCalendar library stays swappable. + +### Unknown properties: the one non-negotiable + +Any `X-` property, unrecognised component or parameter written by another client +**must survive a read-modify-write cycle unchanged.** Failing this silently +destroys other people's data and is invisible in our own UI. + +Storage exists: a `Properties` row with our own `unknown-property` mimetype and +the serialised property in `DATA0`. Verified: no validation rejects an unknown +mimetype, the only index is non-unique so repeats are fine, and there is no FTS +interaction (`updateFTSEntry` is called only from `CategoryHandler`). Note +`MIMETYPE` is *declared* `INTEGER` while holding strings — harmless under SQLite +affinity, but don't be alarmed by it. `Tasks.HAS_PROPERTIES` is never set by +anything; do not filter on it. + +⚠️ **But "byte-stable" is unachievable as the draft stated it, and stating it that +way is dangerous** — the corpus would fail on day one and then be normalised +until the only thing that matters, *no unknown property is dropped*, is no longer +tested. Six independent reasons a faithful implementation cannot be byte-identical: +`PRODID` **must** change (emitting another product's is a lie); fold position +carries no information and a line may split between any two characters; parameter +quoting is optional (`TZID=Europe/Berlin` ≡ `TZID="Europe/Berlin"`); property +order within a component is unconstrained, and storing known fields as columns +destroys the original interleaving **by construction**; `DTSTAMP` is regenerated +and `VTIMEZONE` re-emitted; and the server will not return what we sent anyway. + +**Restate it as a semantic round-trip with byte-stable property values.** Re-parse +both sides into a canonical multiset of +`(component path, property name, params as a sorted map, unfolded unescaped value)` +and assert equality **modulo an explicitly enumerated allowlist** — `PRODID`, +`DTSTAMP`, `LAST-MODIFIED`, `SEQUENCE`, VTIMEZONE bodies, fold positions, +parameter quoting. Nothing else may differ. Byte-equality is then asserted where +it means something: the **unfolded, unescaped value octets** of every untouched +property, plus full parameter preservation including unknown parameters. RFC 5545 +§3.1 is the requirement being encoded: *"Applications MUST preserve the value data +for x-name and iana-token values that they don't recognize."* + +⚠️ **And the requirement is necessary but not sufficient.** A client that +round-trips perfectly, passing every fixture, still destroys the owner's data if +it PUTs back a body Nextcloud filtered on the way out (see +[Server reality](#️-server-reality--the-matrix-audited)). Never write back a body +whose ETag does not match the hash of what we downloaded. + +**Three gaps in the storage sketch.** A flat property-per-row model does not +represent unknown properties **nested inside a known sub-component** (an `X-` prop +on a `VALARM`), or entirely unknown components, or RFC 9074's alarm properties +(`ACKNOWLEDGED`, `PROXIMITY`) which are what other clients now write. Decide +between an opaque sub-component blob and reconstructing nesting from +`DATA0`–`DATA15`. And **set a size cap** — DAVx5 drops unknown properties above +~25 kB — for two reasons: Android's `CursorWindow` row limit, and +`CALDAV:max-resource-size`, whose violation is a failed PUT. + +**Test-first.** The corpus comes before the mapper, and the fixtures that catch +bugs are the adversarial ones, not a clean server-generated VTODO: a +`CLASS:CONFIDENTIAL` task fetched from a **shared** Nextcloud calendar; a resource +carrying a master plus `RECURRENCE-ID` overrides; unknown properties nested inside +a `VALARM`; a `TZID` the device's tzdb does not know; a UID containing `/` and +`@`; and one exceeding `max-resource-size`. + +### The rest of the minefield + +- `DUE` vs `DTSTART`; `VALUE=DATE` vs `DATE-TIME`; floating times and `TZID`. + Match the all-day/UTC convention `fix/provider-interaction-review` established. +- **`STATUS` / `PERCENT-COMPLETE` / `COMPLETED` disagree across clients.** Pick a + canonical reading, normalise on write only. Local convention to reconcile + against: the edit form writes `PERCENT_COMPLETE` clamped 0–100 and leaves + `STATUS` to the complete toggle. +- `RELATED-TO` for subtask trees, including orphans in another collection. The UI + nests one level; the *data* must not assume it. +- `VALARM` — see the `setAlarm` collision above before writing a line of this. +- `CATEGORIES`, `PRIORITY` (**0 = undefined, 1 = highest**). +- ⚠️ **`SEQUENCE` is preserve-verbatim, not ours to bump.** It is the *Organizer's* + revision counter (§3.8.7.4); a client that increments it on every save confuses + scheduling-aware peers. Related: **`DTSTAMP` is regenerated per serialisation, + `LAST-MODIFIED` changes only when the data actually did.** Conflating them makes + every sync look like an edit. +- ⚠️ **`COMPLETED` MUST be UTC** (§3.8.2.1) — no TZID, no floating, no DATE. +- ⚠️ **A `VALARM` with `TRIGGER;RELATED=END` requires `DUE`, or `DTSTART` plus + `DURATION`** (§3.8.6.3). A user clearing the due date on a task that has an + end-relative reminder produces an invalid resource, permanently rejected. Validate + before PUT — this is reachable from ordinary UI actions. +- ⚠️ **`RELATED-TO;RELTYPE` reads backwards to most implementers.** §3.8.4.5: + `PARENT` means *the referencing component is subordinate to the referenced + one*. Both Nextcloud Tasks and tasks.org put `RELTYPE=PARENT` on the child + pointing up — that is the correct reading. Also: the RFC explicitly disclaims + cascade semantics, so "completing a parent completes its subtasks" is a local + UI convention peers will not reproduce. +- ⚠️ **Round-trip `X-MOZ-LASTACK` / `X-MOZ-SNOOZE-TIME` unmodified.** Dropping + them causes documented **alarm storms** on Thunderbird. Emit RFC 9074 + `ACKNOWLEDGED` for our own writes rather than minting `X-MOZ-*`. Preserve + `X-APPLE-SORT-ORDER`; never interpret it. + +### ⚠️ Recurring completion — and the model our provider already chose + +The draft said "no standard; choose one". That is **confirmed and understated**, +and the choice is less free than it looked. + +**How settled the non-standard is.** `draft-ietf-calext-ical-tasks-17` — the +active IETF work item whose entire purpose is extending VTODO, `Updates: RFC5545` +— contains the substring **"recur" zero times** in 1,904 lines. It adds +`SUBSTATE`, `REASON`, `TASK-MODE` and a `VSTATUS` component and leaves this +untouched. RFC 8984 §5.2.6 (JSCalendar) is the only RFC that addresses it at all, +and it **blesses two mutually incompatible approaches and declines to pick**. +RFC 5545 permits `COMPLETED` on a recurring master with no interaction rule — +undefined, which is worse than forbidden, because every client picks differently +and all stay conformant. + +The four models in the wild: + +| | Model | Who | +|---|---|---| +| **a** | Write a `RECURRENCE-ID` override; master stays open | jtx Board, Thunderbird, eM Client | +| **b** | Advance the master's `DUE`/`DTSTART` in place, clear completion | tasks.org, Evolution, Nextcloud Tasks *in practice* | +| **c** | `STATUS:COMPLETED` on the master — kills the series | Nextcloud Tasks ≤ 0.17. **Always a bug** | +| **d** | Detach the completed occurrence as a **new task with a new UID**, and advance the master | **OpenTasks — i.e. our `:provider`** | + +**The finding that matters to us: `:provider` has already chosen model (d).** +`processors/instances/Detaching.java` nulls `_UID`, `_SYNC_ID` and every +`ORIGINAL_INSTANCE_*` on the detached row, and `detachAll` advances the master +and decrements `RRULE;COUNT`. dmfs did this deliberately — on the record, *"the +primary reason is to support Apple clients; they don't support overrides"*. So +our storage layer emits a UID-less orphan plus a moved master, and our sync +adapter has to either honour that, bypass the processor, or reconcile after it. +**That is a design constraint we inherited without deciding it**, and it is the +strongest single argument for treating this as a phase-1 decision rather than a +phase-4 detail. + +**Rules the audit establishes, regardless of which model we write:** + +- **Never write model (c).** Every instance found was filed as a defect; + Thunderbird fixed it fifteen years ago. +- **Never do (a) and (b) together.** `RECURRENCE-ID` is *defined* as the + instance's original `DTSTART`, so advancing the master orphans your own + override. Nextcloud Tasks 0.18 attempts exactly this — and is saved only by an + accident: its override write dispatches a Vuex action that **does not exist**, + which Vuex 4 swallows without throwing. Shipped behaviour is therefore silent + model (b) with no record the instance was ever completed. +- **Accept all four models on read, unconditionally**, including an inbound + master whose `DUE` moved and whose `COMPLETED` vanished. That is not corruption. +- **Never abort a sync batch on a multi-VTODO resource.** tasks.org returns from + its whole sync function on one — so a single Thunderbird-completed recurring + task **stops that entire collection from syncing**, ctag never advances, and + every other change in the batch is silently lost. Degrade to the master and + continue. +- **Keep overrides in the same calendar object resource** (RFC 4791 §4.1). +- ⚠️ **`RRULE` + `DUE` with no `DTSTART` has no well-defined `RECURRENCE-ID` + value** — undefined in RFC 5545, ubiquitous in the wild. Synthesising + `DTSTART := DUE` is the common workaround and is itself the source of visible + DTSTART/DUE desync between clients. Handle it explicitly. +- ⚠️ **Repeat-from-completion has no interoperable encoding at all.** Either drop + it or document that peers read it as repeat-from-due. +- Consider tasks.org's escape hatch: a per-account **"let the server schedule + recurring tasks"** switch. + +Open question 4 — but now with a default: **write (a) if we own the whole path; +accept that `:provider`'s `Detaching` pushes us toward (d) unless we bypass it.** + +> **Process note.** During this research a summarising fetch **fabricated a +> verbatim RFC 5545 sentence** ("A 'to-do' calendar component without the +> 'dtstart' property MUST NOT be part of a recurring set") that appears nowhere +> in the RFC — grep confirms zero hits — along with an invented DTSTART/DUE +> exclusivity rule. Normative text gets read from the raw RFC, never from a +> summary. Both fabrications would have inverted a design decision here. + +--- + +## Libraries + +⚠️ **The first draft's table had one outright licence error and understated two +libraries by roughly an order of magnitude.** Re-verified 2026-08-13 against +published POMs, Gradle module metadata and extracted jars. + +### Take + +| Library | Licence | Real cost | +|---|---|---| +| **dav4jvm** 4.0.1 | MPL-2.0 | ⚠️ **Ktor-only** (the OkHttp package was deleted in 3.0.0); ⚠️ **requires Java 21 bytecode** — we target 17 everywhere, including all seven floret-kit modules; pulls Ktor (~2.45 MB), `guava-jre` (wrong flavour — force `-android`), and `xpp3` (371 KB, duplicates framework `org.xmlpull.v1`). The library itself is only 433 KB / 246 classes | +| **cert4android** | ⚠️ **MPL-2.0 — not GPLv3** | Same org, same licence, same JitPack question as dav4jvm. See below | +| **ical4j** 4.3.0 | BSD-3-Clause | ⚠️ Not "needs desugaring" — `java.time` is native at minSdk 26 and we are 29. Real costs: **2.2 MB of duplicated tz data** in the jar (`zoneinfo/` *and* `zoneinfo-global/`, ~596 `.ics` each), a `ZoneRulesProvider` that pre-allocates 1500 synthetic zone IDs **and exhausts in production**, and a mandatory `ical4j.properties` + `MapTimeZoneCache` + registry shim | +| **lib-recur** | Apache-2.0 | ⚠️ **Version trap, see below** | + +### ⚠️ cert4android was rejected on a false premise + +The first draft listed it as GPLv3 and budgeted a hand-rolled trust-on-first-use +dialog instead. **It is MPL-2.0** — verbatim MPL text in `LICENSE`, SPDX +boilerplate in the README, GitHub agrees. The same licence as dav4jvm, which the +same document accepts two rows above. Two independent audit tracks caught this. + +That matters because the hand-rolled version is not a dialog: + +- **A background sync has no UI to show a dialog in.** cert4android's bound + service + notification approval *is* the library, not an accessory to it. +- **Network Security Config cannot express runtime trust** — it is a static + manifest resource. And since API 24, user-installed CAs aren't trusted without + an NSC entry, so "tell the user to install their CA" fails too. +- **The 3-arg `checkServerTrusted(chain, authType, host)` is mandatory**; + 2-arg-only TrustManagers have repeatedly broken on OkHttp. +- **Hostname verification is a second override** and the other thing Play flags. +- **Play has blocked publishing on unsafe `X509TrustManager` since 2016**, and + the guidance explicitly names "buggy or incomplete custom verification". + +This roughly dissolves the self-signed-cert line item in phase 4. + +### ⚠️ lib-recur is a version trap, not a free dependency + +The first draft said "already in the build at 0.12.2 — no new dependency". Both +halves are wrong. `provider/build.gradle.kts:56` declares it `implementation`, not +`api`, so it is **not** on `:app`'s compile classpath. And lib-recur **0.16.0 +removed `RecurrenceSet`**, which the vendored provider uses in +`TaskInstanceIterable`/`TaskInstanceIterator`. The moment `:app` adds a current +lib-recur, Gradle's highest-wins resolution upgrades the graph and **the provider +stops compiling.** + +Decide explicitly: pin `strictly = "0.12.2"` and accept the known fixes we forgo +(0.15.1 `FastForwarded`, 0.15.2 empty-`ByDay`), or budget the iterator rewrite +onto the post-0.16 API. Note upstream is dormant — last release 0.17.1, last +commit 2024-04. + +### Consider + +**biweekly** (BSD-2) is stronger than the first draft credited: 639 KB / 432 +classes, **no bundled tz database at all**, legacy `java.util.Date` so no +`ZoneRulesProvider` hazard — against ical4j's 2.2 MB of zone data and its +registry shim. Caveats: last release 0.6.8 (2024-01), still 0.x, mandatory +`jackson-core` for jCal (excludable), and the missing tz database means it relies +on `VTIMEZONE` components being present rather than resolving `TZID`s itself — +a real gap for CalDAV round-tripping. + +### Do not take + +| Library | Why not | +|---|---| +| **synctools** | **GPLv3.** Does exactly the mapping we need against exactly the schema we run, which makes it the sharpest temptation here. Still a one-way door. (Repo now archived and folded into `davx5-ose` as a module — the standalone coordinate is stale.) | +| **Android-SingleSignOn** | ⚠️ **GPL-3.0.** The first draft carried it as a harmless optional extra; it is the actual one-way door. It also proxies through the Files app and supports only OCS plus a few WebDAV verbs — not a CalDAV transport | +| **caldav4j** | Apache-2.0, but server-oriented, last release 2022-01 | +| **sardine** | Needs JAXB — a non-starter on Android | + +**Still true:** there is no mature Kotlin-native iCalendar library, and no +Android-suitable CalDAV client on Maven Central at all. That is *why* the JitPack +question is unavoidable rather than optional. + +### On GPL and Play + +The rejection reasoning is right in effect but was imprecise. MIT **is** +GPL-compatible; the constraint is on the terms of the distributed binary, not on +our source headers. And ⚠️ **GPLv3 is not a Play problem** — DAVx5 is GPLv3 and +ships on Play with 287k+ installs; §6 Installation Information is a hardware +provision. If the real reason is wanting Agendula to stay permissively +relicensable, say that, because that is the reason that holds. + +### ⚠️ Licence obligations we cannot currently discharge + +Settings exposes only our own MIT `LICENSE`. There is no third-party attribution +surface and no AboutLibraries in the build. But MPL-2.0 §3.2(a) requires telling +recipients how to obtain source; §3.4 requires retaining file headers; **BSD-3 +requires reproducing the copyright notice in binary distributions** (that is +ical4j, and it is not optional); Apache-2.0 §4(d) propagates NOTICE. Shipping any +of these without an attribution screen is a plain violation, independent of +copyleft. **Phase 0 work item.** Bonus trap: ical4j's POM declares a non-SPDX +licence name and a `LICENSE` URL that 404s, so generators produce empty output. + +### The JitPack question, restated + +`dav4jvm` and `cert4android` are both `com.github.bitfireAT:*` — JitPack only. +F-Droid's inclusion policy does trust jitpack.io for freely-licensed artifacts, +so **F-Droid is not the obstacle; our own `FAIL_ON_PROJECT_REPOS` policy is.** +But F-Droid's own writing is lukewarm — JitPack "hosts whatever is built from +GitHub, without checking the license" — and concretely: JitPack **does not sign +artifacts** (`.asc` 404s; Maven Central's does not), and rebuilds on demand, so a +coordinate is not immutable. We have no `verification-metadata.xml` today. + +Weigh against that: **dav4jvm shipped two breaking majors nineteen days apart** +(3.0.0 OkHttp→Ktor 2026-07-08; 4.0.0 callbacks→coroutines 2026-07-27), which +argues for vendoring a known-good tree rather than a floating pin — and at 433 KB +vendoring is far cheaper than depending, once the Ktor/guava/xpp3 tail is counted. +Open question 1. + +--- + +## Authentication and discovery + +### Nextcloud Login Flow v2 + +The protocol description survives audit against the server source: the endpoint, +the `{poll:{token,endpoint},login}` shape, 404-until-approval, the 20-minute +lifetime (`lifetime = 1200` in `LoginFlowV2Mapper.php`), and "the 200 is returned +exactly once" (the mapper deletes the row inside `poll()` before returning). It is +not deprecated, there is no v3, and OAuth2 is a worse fit. Corrections: + +- ⚠️ **Poll with `POST`, form-encoded.** A `GET` gets 405. The draft didn't say. +- ⚠️ **Set an explicit `User-Agent`.** `init()` passes it to `createTokens()`, + where it becomes the app password's **name** in Settings → Security → Devices & + sessions. With OkHttp's default the user sees `okhttp/4.12.0` and cannot tell + what to revoke — defeating the entire point of the flow. (`OCS-APIRequest` is + *not* needed here; v2 is a Frontpage route.) +- ⚠️ **404 only means pending.** "Treat anything that isn't a 200 as pending" + swallows 429 (brute-force protection), 503 (maintenance), Cloudflare challenge + pages (200 with HTML), and DNS/TLS failure — turning a diagnosable error into a + 20-minute spinner. Require `Content-Type: application/json` before parsing. + Stop polling on anything that is neither 404 nor 200. Note 404 is *also* + returned for expired/consumed, so keep tracking the deadline locally. +- ⚠️ **Validate the `endpoint` origin.** Verbatim is right for the *path*, wrong + as a blanket rule: it is generated from `overwrite.cli.url` / `overwriteprotocol` + / `trusted_proxies`, misconfigured on a large fraction of self-hosted installs. + Refuse a scheme downgrade to `http` outright — the poll token is exchanged for a + long-lived app password, so this is a credential-grade secret. If the host + differs from the one the user typed, confirm explicitly and say *"your server's + `overwrite.cli.url` is wrong"*, which saves a support round-trip. (The draft's + "some deployments return 302" is a proxy symptom, not a Nextcloud variant.) +- ⚠️ **`loginName` is not the uid.** It is what the user typed — possibly an + email, an LDAP-derived value, or the right name in the wrong case. Use it + **only** as the Basic auth username; never interpolate + `remote.php/dav/calendars//`. Discover via `current-user-principal` + → `calendar-home-set`, exactly as the generic path already does. This is the + classic "logged in but no calendars" bug. +- ⚠️ **Custom Tabs needs four things the draft omitted:** a `` entry for + `android.support.customtabs.action.CustomTabsService` (or provider detection + silently fails on API 30+); a try/catch with an `ACTION_VIEW` fallback + (`launchUrl` throws `ActivityNotFoundException` with no Custom Tabs browser — + realistic on GrapheneOS/CalyxOS/AOSP, i.e. disproportionately our users); + persistence of `{token, endpoint, deadline}` to disk immediately, so process + death mid-flow is resumable; and an explicit "I finished / Cancel" affordance, + since Custom Tabs return **no result** when dismissed and Nextcloud's flow ends + on a "you can close this window" page that never returns to the app. + +### Generic CalDAV discovery + +⚠️ **The draft's five steps were the happy path of a much longer pipeline, and +one of its two filters was inverted.** Corrected version, with live probes run +2026-08-13: + +``` +1. Input: email / mailto: / http(s) URL +2. Base URL typed → PROPFIND Depth:0 on it first (principal, home-set and + collection can all come back in one response) +3. Else: + a. SRV _caldavs._tcp. — honour RFC 2782 priority/weight, + honour non-443 ports, target "." = none + b. TXT _caldavs._tcp. — parse path= ⚠️ MISSING FROM DRAFT + c. ladder: [TXT path] → /.well-known/caldav → / ⚠️ "/" MISSING +4. PROPFIND Depth:0 for DAV:current-user-principal + - follow 301/302/303/307/308, re-sending PROPFIND and its body + - relative Location; reject HTTPS→HTTP; cap at 5; PERSIST 301/308 + - 401 → authenticate and retry. NOT a failure ⚠️ MISSING + - reject ⚠️ MISSING + - OPTIONS gate: DAV: header must contain calendar-access ⚠️ MISSING +5. PROPFIND Depth:0 on the principal for calendar-home-set + → iterate ALL hrefs (0..n); cross-host is normative ⚠️ DRAFT ASSUMED ONE +6. PROPFIND Depth:1 per home set, requesting properties BY NAME + → optionally recurse one level into plain {DAV:}collection members +7. Classify: + a. resourcetype as a SET; require CALDAV:calendar ⚠️ MISSING + b. VTODO test: property ABSENT ⇒ INCLUDE ⚠️ DRAFT INVERTED IT + c. privilege-set absent ⇒ assume writable; handle 403 on write +8. sync path: supported-report-set → RFC 6578 keyed on DAV:sync-token +9. Creation: OPTIONS feature-detect → MKCALENDAR / extended MKCOL / disable UI +``` + +**The two filter corrections, which are the important part:** + +- ⚠️ **`supported-calendar-component-set` absent means "supports everything", + not "supports nothing".** The draft kept only collections whose set *includes* + VTODO, which silently drops every server that doesn't advertise it. RFC 4791 + §5.2.3 also says the property SHOULD NOT come back from an allprop request — + so **request properties by name**, or you get none of them. Its grammar is + `(comp+)`; an empty element is non-conformant, and `dav4jvm`'s parser starts + all-`false` and would classify it as supporting nothing. Treat empty as all. +- ⚠️ **Classify on `resourcetype`, with a positive test.** The draft had no + resourcetype check at all, so a Depth:1 listing on Nextcloud yields inboxes, + outboxes, notification collections, trash bins and subscriptions as "task + lists". The test must be **`CALDAV:calendar` is present in the set** — *not* + exclusion by `schedule-outbox`, because **SOGo's main personal calendar reports + `collection` + `calendar` + `schedule-outbox` simultaneously** for every + non-Apple client, which is exactly what we are. The positive rule also keeps + shared calendars (which add `CS:shared` alongside `CALDAV:calendar`) and drops + Nextcloud's `nc:deleted-calendar`, which deliberately strips `caldav:calendar`. + Treat resourcetype as an **unordered set**, never by position. + +**Discovery traps, live-probed:** + +| Trap | Detail | +|---|---| +| SRV TXT `path=` | Live at **Posteo** (`path=/`), **GMX** and **Web.de** (`path=/begenda/dav/users/`). Skip it and GMX/Web.de land on the wrong path | +| Posteo | **SRV-only**, on port **8443**; its `/.well-known/caldav` 404s. Hardcoding 443 fails | +| Null SRV target | `_caldav._tcp.fastmail.com` and `runbox.com` return `0 0 0 .` — "explicitly unavailable" | +| Google SRV | Returns a valid record pointing at `calendar.google.com`, which **is not a DAV server** (PROPFIND → 405). A strict RFC 6764 client follows it into a dead end for every `@gmail.com` | +| well-known 401 | **iCloud and Zoho** answer 401 — the endpoint *is* the DAV root and wants auth. RFC-legal; must not be read as failure | +| Redirect downgrade | `dav.runbox.com` redirects **HTTPS→HTTP** in production today | +| Method preservation | A generic HTTP stack may legally downgrade 301/302 to GET, silently breaking PROPFIND | +| `` | RFC 5397 §3 — a **200** whose body means auth failed. Without this check a failed login looks like a successful discovery that found nothing | +| Cross-host home set | Normative (RFC 4791 §6.2.1's own example), and iCloud depends on it: principal on `caldav.icloud.com`, home set on `pNN-caldav.icloud.com`. Allow it, require HTTPS, surface the host change, never send credentials into an unvalidated redirect chain | +| `caldav.fastmail.com` | `d.fastmail.com` is dead — cert mismatch | + +⚠️ **Two `dav4jvm` defects we would inherit:** it handles 301/302/307/308 but +**not 303**, which RFC 6764 §5 names explicitly; and issue #209 — `location` is +mutated in place, so **permanent redirects never reach the caller**. DAVx5 never +rewrites its stored collection URL after a 301 and re-follows on every sync. +Persist the new URL ourselves on 301/308. + +**Read-only detection is softer than the draft assumed.** RFC 3744 §3.7 defines +a `DAV:read-current-user-privilege-set` privilege, so a server may legally return +`current-user-privilege-set` in a **403 propstat**. Default to writable when it is +absent (as DAVx5 does), expand aggregates yourself (`DAV:write` and `DAV:all` +imply content-write; some servers don't expand), and prefer sabre's +`{DAV:}share-access` where offered as the cleanest signal. + +**Creating lists is not always possible.** MKCALENDAR is only *RECOMMENDED* by +RFC 4791 §5.3.1. **iCloud is MKCOL-only** (and MKCOL under `…/calendars//` +returns 412 while `…///` returns 201); **Google has neither**; +**Posteo disables it** despite running sabre. Feature-detect via OPTIONS and +**disable the "new task list" UI** where neither is available. Set +`supported-calendar-component-set` **at creation — it is protected afterwards**, +and follow up with an explicit PROPPATCH for `displayname`, which most servers +ignore in the MKCALENDAR body. + +**Two more VTODO viability landmines:** Zimbra and OX/mailbox.org restrict tasks +to dedicated task lists (MKCALENDAR with `[VTODO]`), and **OX rejects recurring +VTODOs with 400**. And **server-side `CALDAV:expand` on VTODO is broken on +Nextcloud, Baïkal, SOGo, Radicale and Posteo alike** — expand client-side, which +`lib-recur` already gives us. Request `max-resource-size` too: violating it is a +failed PUT, and long `DESCRIPTION`/`ATTACH` payloads reach it. + +⚠️ **`getctag` is not a cheap pre-check** — `caldav-ctag-03` deprecated it in +2015 in favour of RFC 6578, and `DAV:sync-token` is itself PROPFIND-able, so the +same pre-check comes back in the Depth:1 listing we already make. On Nextcloud +they are literally the same value. Keep `getctag` only as a legacy fallback where +`supported-report-set` omits `sync-collection`. + +⚠️ **Auth is not just Basic:** + +- **Baïkal defaults to Digest** (`dav_auth_type`), and **OkHttp has no Digest + support** — square/okhttp#205 has been open for years. DAVx5 carries a + hand-written `BasicDigestAuthHandler` in dav4jvm precisely for this. Take it + (MPL-2.0, same decision as above) or detect the `WWW-Authenticate: Digest` + challenge and emit a real error instead of "wrong password". Baïkal is squarely + in our target audience. +- **Send Basic preemptively via an `Interceptor`**, gated to HTTPS and the + account's own origin. OkHttp's `Authenticator` is reactive-only — an extra round + trip on every request of a PROPFIND-heavy sync, and it never fires at all on + servers that answer 403/404 without a challenge. Note OkHttp strips + `Authorization` on cross-host redirects (correct, but it breaks `.well-known` + discovery across hosts — re-attach only after validating the target). +- **Fastmail requires an app password** and its Basic plan has no CalDAV at all. + **iCloud requires an app-specific password** and 2FA to mint one. **Google is + OAuth2-only** — refuse it with an explanation rather than a 401. Detect these + by domain at account-add time; "wrong password" that is actually "you used your + account password" is the single most common support ticket any CalDAV client + inherits. + +### Credential storage, rotation, revocation + +⚠️ `androidx.security:security-crypto` is not "effectively stalled" — it is +**formally deprecated and terminal**: deprecated at 1.1.0-alpha07 (2025-04), +shipped deprecated in stable 1.1.0 (2025-07), with release notes saying there +will be no subsequent releases. Its successor `datastore-tink` is alpha only. + +⚠️ And the alternative is weaker than implied: **AccountManager stores passwords +as plain `TEXT`** — no encryption or hashing anywhere in AOSP. FBE plus a +same-signature check is the whole boundary. That is DAVx5's actual posture and is +defensible, but state it rather than implying it is secure storage. + +**Decision:** Keystore `AES/GCM/NoPadding`, blob in DataStore. +`setUserAuthenticationRequired(false)` is the default — don't call it. Do **not** +set `setUnlockedDeviceRequired` (breaks background sync). Handle +`AEADBadTagException` / `KeyPermanentlyInvalidatedException` as *re-authenticate*, +not as a crash. Note `getUserData` returns null while the device is locked, so a +boot-triggered sync must wait for unlock. + +⚠️ **Revocation is bidirectional and the draft had neither direction:** + +- **On 401: stop syncing that account immediately**, mark `NEEDS_REAUTH`, notify + with a deep link into the login flow, and **do not retry on a timer**. + Nextcloud's brute-force protection throttles then 429s **per source IP** — a + retry loop on a dead app password takes down the user's *other* Nextcloud + clients on that network and looks like we broke their server. App passwords do + die in the wild (password change, admin revocation, server bug #39615). + Distinguish 401 (re-auth) from 403 (forbidden, do not re-auth) from 429/503 + (back off, honour `Retry-After`). Nextcloud returns 401 with + `PasswordLoginForbidden` when 2FA is on and a real password was used — worth + detecting for a precise message. +- **On account removal: call `DELETE /ocs/v2.php/core/apppassword`** + (this one *does* need `OCS-APIRequest: true`), best-effort. Otherwise + uninstalling never revokes access, and orphaned entries accumulate that the user + cannot identify — see the User-Agent point above. + +--- + +## The sync engine + +- Collection discovery and refresh; per-collection sync state (in a `TaskLists` + `SYNC*` slot, per the squat table). +- ⚠️ **Baseline is `REPORT calendar-query` with a VTODO comp-filter and *no* + time-range**, matching DAVx5 — which deliberately does not use RFC 6578 for + tasks, and omits the time-range *"because some servers don't return tasks + without time at all"*. `sync-collection` is the **optimisation on top**, not + the primary path. The draft had this the wrong way round. +- CTag / sync-token loop (RFC 6578 `sync-collection`) where it works. ⚠️ **The + full-reconciliation path is not a fallback for weak servers — it is a permanent + safety net on every server**, because a pruned change log behind a still-valid + token is undetectable (below). +- Local change detection: `_dirty = 1 OR _deleted = 1`, scoped by `list_id`. +- `If-Match` conditional PUT. +- Backoff and partial-failure recovery. A failed collection must not fail the + account. + +### ⚠️ RFC 6578, and where every shipping client has bugs + +The audit read RFC 6578 in full (no errata, fourteen years on) plus the +w3c-dist-auth threads that are its only authoritative gloss. Calibration first: +**Evolution shipped `sync-collection` in June 2026** against a request open since +2019; **vdirsyncer has declined it for twelve years**; **Thunderbird still has +unlanded patches** for one of the cases below. The library covers about a third. + +**⚠️ Note DAVx5 does not use RFC 6578 for tasks at all** — its own documentation +says *CalDAV tasks: use `REPORT calendar-query`*, because a collection +advertising both VEVENT and VTODO would stream every event change and force a +fetch to discover it isn't a task. That is a real argument for making +`calendar-query` our primary path and `sync-collection` the optimisation. + +1. **Invalidation has no status code.** §3.2 defines the `DAV:valid-sync-token` + precondition and never assigns an HTTP status. Observed: **403** (sabre ⇒ + Nextcloud, ownCloud, Baïkal, and Radicale 3.1.8), **400** (Google, CalDAV and + CardDAV), **409** (Radicale, per its maintainer), **412** (accepted by + Evolution). One server family, two codes across versions. + **Rule: ignore the status; match `` anywhere in the body + on any 4xx.** Thunderbird's CardDAV code accepts only 400 and therefore never + recovers from the 403 that most of the self-hosted world emits. +2. **Initial sync must not report deletions** (§3.4), so a forced full resync + cannot learn what was deleted. **Mark-and-sweep is mandatory** — and the + `initialIncomplete` flag must be persisted *alongside* the token, or a resumed + partial sync sweeps against an incomplete "present remotely" set and **deletes + live data**. +3. **⚠️ Persist the token only after the bodies are applied.** The RFC's own + Appendix B gets this backwards — it associates the new token with the + collection *first*, then fetches. Death in between loses those changes + permanently. Under WorkManager, process death mid-sync is routine, not + exotic. Persist per page, after step 5, atomically with `initialIncomplete`. +4. **Worse than invalidation: a token the server still accepts over a change log + it already pruned.** Returns 207, zero changes, "you're current" — no error, + no recovery, and **RFC 6578 provides no signal for it.** Nextcloud's + `totalNumberOfSyncTokensToKeep` defaults to 10,000 and its own admin manual + warns this "will lead to premature data deletion and synchronization + problems"; Baïkal #1140 has shipped an empty change set *forever* since 2022. + **The only mitigation is periodic full reconciliation** (PROPFIND `Depth: 1` + + ETag diff) on a slow cadence regardless of the token. +5. **Truncation: detect the 507 on the SELF href, not the error element.** + §3.6's `DAV:number-of-matches-within-limits` is a SHOULD and sabre omits it + entirely. Distinguish it from a **507 as the outer HTTP status**, which means + your `DAV:limit` could not be honoured — retry without the limit, don't page. + iCloud emits a SELF response with status **200**; ignore that one. + **Do not send `DAV:limit`** — Nextcloud regressed it to a localised HTML error + page in 28.0.10/29.0.7/30.0.0. Cap by bytes client-side instead, and still + implement 507 handling. Add an iteration cap **and** a no-progress guard: the + RFC never requires the token to advance, and an unchanged token spins forever. +6. **`supported-report-set` is a hint, not a contract.** Radicale advertised + `sync-collection` for years without implementing it; Cyrus 3.8 advertises it + and rejects the mandated empty token. A 207 with no `` must + **degrade to PROPFIND, not throw**. +7. **Tokens are opaque.** §3.2 says they MUST be URIs; Google, iCloud, fruux + (`0`), and grommunio all violate it. Never parse or validate. And **never + reuse a token across collections** — sabre validates only the prefix, then + returns a wrong-but-plausible delta with no error. Key by + `(accountId, collectionUrl)`. +8. **Deletion is `404` at *``* level.** A 404 + inside a `` is a missing *property* on a resource that exists. + Confusing the two nesting levels deletes live data. +9. **Three membership edge cases** (§3.5): create-then-delete between syncs is + reported as removed, so the delete handler must no-op on an href it has never + seen; delete-then-recreate at the same URI is reported as **changed**, so href + identity is not UID identity — re-read the UID from the body; and **ACL churn + may be reported as removal**, so toggling a share can look like mass deletion. + Apply a sanity threshold before acting on a large delete batch. +10. **`sync-collection` never reports collection property changes.** §3.5.1 keys + "changed" on an entity tag, and a calendar collection has no entity body. So + displayname, colour and **read-only status can only be refreshed by PROPFIND + on the home set** — and on sabre, `CalendarHome` does not implement + `ISyncCollection` at all, so there is no sync-collection there to use. + DAVx5 has this exact gap open as its own bug. +11. **`calendar-data` inside `sync-collection` is sanctioned by neither RFC.** + RFC 4791 §9.6 says it "is not a WebDAV property"; it works on sabre only + because that codebase exposes it as one by explicit accident. Request + `getetag` + `resourcetype`, then batch `calendar-multiget` — **and match the + returned hrefs against what you asked for**, because real servers reply with + responses for unrelated URLs. +12. **`getctag` is formally deprecated** by `caldav-ctag-03` in favour of this + REPORT — and every shipping client still keeps it as a fallback. Do the same, + but never compare a ctag to a sync-token. + +**What `dav4jvm` actually gives us:** spec-correct serialisation, `Depth: 0`, +`"infinite"` spelled right, a streaming `Flow` with the token arriving as an +`ExtraProperty`, and typed exceptions. **What it does not:** the truncation loop, +507 detection, any `valid-sync-token` handling (its `Error.kt` says outright +*"there is no logic for subclassing errors"*), mark-and-sweep, `initialIncomplete` +persistence, or multiget orchestration. And its error extraction only parses XML +content-types within a 20 KB excerpt at depth 1 — so a `` served as +`text/html`, or buried behind a PHP stack trace (exactly what ownCloud and Baïkal +emit), yields no recovery. Add a raw-body substring fallback, as Evolution does. + +### ⚠️ Scheduling has a ceiling the draft didn't price + +"WorkManager with network constraints" was the entire treatment. Reality: + +- An ordinary worker is documented for **< 10 minutes**. An initial full sync of a + large collection over a slow homelab link will exceed it. DAVx5's own + `workerWaitTimeout` is 10 minutes. +- Escalating to `setForeground` pulls in `FOREGROUND_SERVICE` + + `FOREGROUND_SERVICE_DATA_SYNC` (missing ⇒ `SecurityException` at targetSdk 34+), + a `tools:node="merge"` override on WorkManager's own service, and the + **Android 15 six-hours-per-24 `dataSync` budget** whose failure mode is a fatal + `RemoteServiceException`. Android 15 also **forbids starting a `dataSync` FGS + from `BOOT_COMPLETED`** — and we have a boot receiver. +- **Android 16 removed the shield**: jobs running alongside a foreground service + now obey the job runtime quota, and the `active` bucket is capped at 20 min / + rolling 60 min. + +**Therefore:** make sync **chunked and resumable** — persist the sync-token/ETag +cursor per collection so a killed worker resumes rather than restarts. Hard socket +and wall-clock timeouts. Periodic sync is a plain `PeriodicWorkRequest`, no FGS. +"Sync now" from a visible screen uses `setExpedited(RUN_AS_NON_EXPEDITED_WORK_REQUEST)` +— and implement `getForegroundInfo` unconditionally, since omitting it crashes +below API 31 and we support 29. FGS only for user-initiated full syncs, with +`Service.onTimeout → stopSelf()` as a backstop. Play requires a video demo per +declared FGS type. + +⚠️ **Be honest about cadence.** `PeriodicWorkRequest`'s 15-minute floor is +nominal. In the `rare` and `restricted` buckets network access is disabled +outright; Doze allows idle apps network roughly **once a day**. Combined with +unmetered-only, worst case is genuinely "once overnight". Promise eventual +consistency, and sync hard on app open and on connectivity-regained. + +### ⚠️ Server reality — the matrix, audited + +The draft listed six servers as a test matrix. What they actually do: + +| Server | VTODO | The thing that will bite | +|---|---|---| +| **Nextcloud** (sabre) | ✅ | ⚠️ **Never round-trip a body fetched from a shared calendar** — see below. Never send ``. Per-calendar UID uniqueness ⇒ **409 `no-uid-conflict`**. Trashbin renames the href to `-deleted.ics` ⇒ 403 on delete-then-recreate. MKCALENDAR is rate-limited 10/hour ⇒ 429, max 30 calendars ⇒ 403 | +| **Baïkal** (sabre) | ✅ | Handles `` and 507 **correctly** — the reference implementation for that path. Never prunes its change log, so tokens stay valid forever. Defaults to **Digest** auth | +| **Radicale** | ✅ | `supported-calendar-component-set` is **never enforced** — a VEVENT PUT into a VTODO-only collection is accepted. Advertises three reports it does not implement. Its VTODO time-range now implements all eight RFC 4791 §9.9 rows — **the widespread "Radicale doesn't do time ranges" claim is stale** | +| **SOGo** | ✅ | ⚠️ **Never invalidates a sync token** (`valid |= …` makes the check always pass) and tokens are **second-granularity**, so you re-receive up to a second of changes every sync. ⚠️ **The ETag is a row-version counter, and the body is regenerated per-principal** — same ETag, different bytes. Cannot create a VTODO-only collection at all | +| **Fastmail** | ✅ | ⚠️ **Reframe as supported.** The backend does VTODO fine; their own UI hides task-only calendars by design. Create collections as **mixed `VEVENT,VTODO`** so the list doesn't vanish from Fastmail's UI. Requires an app password; Basic plan has no CalDAV | +| **iCloud** | ⚠️ | **Reminders left CalDAV at iOS 13.** A new VTODO collection syncs bidirectionally but is **invisible in Reminders.app forever**. Market it as "store tasks in iCloud", never as "sync with Apple Reminders". Cheap detection: no VTODO-capable collection in the home set ⇒ upgraded account | +| **Google** | ❌ | ⚠️ **Drop it.** First-party docs: *"Doesn't support VTODO or VJOURNAL data"* and no MKCALENDAR. Refuse with an explanation, don't fail with a 401 | + +**Two data-destruction landmines, both confirmed from server source:** + +1. **Nextcloud rewrites task bodies on GET from a shared calendar.** + `CalendarObject::get()` strips `VALARM` on read-only shares, and for + `CLASS:CONFIDENTIAL` reduces the object to a VEVENT-shaped whitelist that + **deletes `DUE`, `STATUS`, `COMPLETED`, `PERCENT-COMPLETE`, `PRIORITY` and + `RELATED-TO`** — every property that makes it a task. **The ETag is left + untouched**, so `ETag ≠ md5(body)` and re-PUTting what you downloaded destroys + the task. Baïkal never does this. +2. **sabre runs vobject `REPAIR` on every PUT** unless you send + `Prefer: handling=strict` — adding UID/DTSTAMP/PRODID/VERSION — and when it + modifies the object it **suppresses the ETag response header**, so you must + re-GET rather than assume. It also 415s on **`DUE` < `DTSTART`**, value-type + mismatch between them, multiple UIDs, mixed component types in one resource, + and a present `METHOD`. + +**Three consequences for the design:** + +- **`supported-calendar-component-set` is not an invariant.** Unenforced on + Radicale, discarded by SOGo, immutable on sabre (403 if you try to change it). + Filter on it, but never rely on it. +- **VTODO scheduling exists nowhere.** sabre's own docs: *"We don't do VTODO + scheduling yet, and only support VEVENT."* Treat `ORGANIZER`/`ATTENDEE` on a + task as inert text to round-trip — which is a mercy, since it also means + scheduling never rewrites our objects or suppresses our ETags. +- **Never trust an ETag as a content hash** (SOGo, and Nextcloud shares). + +### ⚠️ Writing: conditional PUT, and the conflict policy that had to change + +**`If-None-Match: *` on create. `If-Match` on update and DELETE.** The draft said +"`If-Match` on every PUT", which omits the creation case entirely — RFC 4791 +§5.3.2 asks for `If-None-Match: *` there, and all three reference clients send it. +Without it, a filename collision (two devices minting the same UID, or a sanitiser +folding two UIDs onto one name) makes the second PUT **silently destroy the +first**, with no ETag to protect it because we have never seen the resource. + +⚠️ **412 means three different things** and the draft's single rule conflated them: + +| On | Means | Do | +|---|---|---| +| create (`If-None-Match`) | the filename is taken | re-fetch that href; adopt if the UID matches, else regenerate the name as a UUID | +| update (`If-Match`) | the server has a newer version | conflict resolution, below | +| update, resource gone | no selected representation, so the condition is false — **spec-correct** (Radicale and DAViCal do this) | `HEAD` to disambiguate; 404 ⇒ delete-vs-edit, not a conflict | + +⚠️ **The proposed conflict policy is unimplementable and is withdrawn.** The draft +said the local version would be *"preserved rather than discarded — a duplicate +task, marked, in the same list."* RFC 4791 §4.1 requires a **UID to be unique +within a collection**, and every target server enforces it: Nextcloud, Radicale +and SOGo all answer **409 `CALDAV:no-uid-conflict`**. So the preserved duplicate +can never be uploaded — same UID fails forever, a new UID forks a task that never +reconciles. The "visible clutter" the design wanted is either a permanently +failing row or a permanent fork. + +**Pick one and write the consequence down** (open question 2, now with real +options): **server-wins and discard the local edit** — DAVx5's stated policy — +or **server-wins and fork under a new UID**, marked in the UI, with the new UID +persisted so the fork is first-class from that moment. Prompting is unavailable; +a background sync has nobody to ask. + +⚠️ **The ETag may be weak or absent, and then `If-Match` can never succeed.** +RFC 4791 §5.3.4: when the server does not store your bytes verbatim, *"a strong +entity tag MUST NOT be returned"*. RFC 9110 §13.1.1: *"A weak entity-tag cannot be +used with If-Match."* On sabre this is the **default path** — `validateICalendar` +runs vobject `REPAIR` unless you send `Prefer: handling=strict`, and +`Server::createFile` then deliberately withholds the ETag. Worse, **weak ETags +also arrive from the user's reverse proxy**: any gzip-compressing nginx, +Cloudflare or Traefik in front of Nextcloud produces them, so the risk tracks the +user's deployment rather than their server software. + +Therefore: send **`Prefer: handling=strict`** to sabre-based servers — the +cheapest single fix in this whole audit, since it preserves both our bytes and +the ETag. Request `Accept-Encoding: identity`. Strip `W/` and keep a weak flag. +**If a PUT returns no ETag or a weak one, discard it and re-fetch** for the strong +validator *and* the server's canonical body. Bound every 412 retry loop. + +⚠️ **Errors are not one status.** RFC 4791 §5.3.2.1 defines **eleven** +preconditions, and the one we will hit most is not in the draft at all: **sabre +returns 415** for a VTODO whose `DUE` precedes `DTSTART`, whose `DUE`/`DTSTART` +value types disagree, or which carries a `METHOD`. The first two are reachable +from ordinary UI actions and must be validated client-side. Also: **507 MUST NOT +be auto-retried** (RFC 4918 §11.5 — quota exhaustion is common on hosted +Nextcloud, and a generic backoff loop violates the spec), and **5xx is not safely +retryable either** — a contradictory `RRULE`/`EXDATE` pair returns 500 from +Nextcloud and will do so forever. + +So line-for-line with "a failed collection must not fail the account", add its +twin: ⚠️ **a failed resource must not fail the collection.** Per-resource +quarantine with a failure counter, not backoff. A single HTTP 400 has halted all +calendar sync in DAVx5 for weeks. + +**DELETE needs the same care:** conditional on `If-Match`; **404/410 count as +success**; a resource deleted locally that was never uploaded is never DELETEd. +And Nextcloud's trashbin renames the href to `-deleted.ics`, so +delete → recreate → delete the same href returns **403** — which task apps hit +constantly, because they reuse hrefs. + +**href and UID are unrelated.** RFC 4791 §5.3.2 opens by saying the URL *"is +entirely arbitrary and does not need to bear a specific relationship"* to the +content, and `.ics` is MAY. Sanitise filenames following vdirsyncer's rule — +`a–zA–Z0–9_.-+`, **excluding `@`**, because some servers percent-encode it in the +path and then reject or "repair" the URL, and RFC 4791's own example UID is +`…@example.com`. Cap the basename around 200 bytes; fall back to a UUID. + +### Manifest and permissions for phase 3 + +⚠️ The draft named only `INTERNET`. Actually needed: `INTERNET`, +`READ_SYNC_SETTINGS`, `WRITE_SYNC_SETTINGS`, `ACCESS_NETWORK_STATE` (merged in by +`work-runtime`, but it shows in F-Droid's permission diff), plus +`FOREGROUND_SERVICE` + `FOREGROUND_SERVICE_DATA_SYNC` if the FGS route is taken. +The authenticator `` must be `android:exported="true"` guarded by +`android:permission="android.permission.ACCOUNT_MANAGER"` — note +`android.permission.ACCOUNT_AUTHENTICATOR` **does not exist**. + +Also missing from `libs.versions.toml` entirely: `androidx.work`, +`androidx.hilt:hilt-work`, `androidx.browser`. With Hilt that means a +`HiltWorkerFactory`, removing the default `WorkManagerInitializer`, and an +`@EarlyEntryPoint` for the authenticator service. + +### Two network facts for homelab users + +- ⚠️ **Ship a `network-security-config` with ``.** Since + Android 7, a user who correctly installs their private CA into Android's store + is *still* not trusted by apps. Cleartext `http://` is blocked by default since + API 28; any escape hatch must be a narrow, warned, per-account opt-in — Play's + User Data policy requires modern cryptography in transit. +- ⚠️ **Android 17 / targetSdk 37 breaks LAN CalDAV.** Local network protections + become mandatory: TCP to a local address and `.local` resolution require the + runtime `ACCESS_LOCAL_NETWORK` permission. The failure mode is a **connection + timeout, not a `SecurityException`** — "my Nextcloud at 192.168.1.50 just + hangs", the worst bug-report shape there is. We are safe at targetSdk 36 (which + gets an implicit grant) and must **not** request it before targeting 37 — but + `compileSdk` is already 37 and Play's floor rises annually, so this is a + scheduled break aimed precisely at the self-hosting demographic. + +--- + +## External mode's future + +`STORAGE-AND-SYNC.md` keeps Posture A as a user choice; a later draft proposed +replacing it with a one-time importer. Premature — it argues against something +shipped and working — but it is the right question one phase early. + +Once we sync ourselves, External mode's only job is reading tasks in someone +else's app. Costs are real and permanent: capability divergence (tasks.org's fork +is DB 22 and lacks `is_recurring`, which is why `TaskMapper.task` derives +recurrence from `rrule`/`rdate`), per-backend UI degradation forever, two +dangerous permissions in a static manifest, a doubled device matrix. + +**Decide before phase 1** — it determines whether the mapper and UI stay +dual-capable. Open question 3. + +If retired, the importer spec is sound: read-only, one-time, idempotent; identify +local lists by `ACCOUNT_TYPE`, treating unknown types as synced; **import the +`Properties` table, not just `Tasks`** (categories, alarms, `RELATED-TO`, `X-` +props — the commonly forgotten half); preserve `_UID`s; persist +`(authority, _ID, uid)` for idempotency; and while scanning, read the *names* of +synced lists so the UI can say *"these 3 lists come from cloud.example.de — add +that account to bring them back"*. + +--- + +## Play compliance + +⚠️ Absent from the first draft entirely. + +- **A privacy policy is mandatory regardless of collection**, linked both in Play + Console and **inside the app**. +- **"Not collected" is not defensible.** Play defines collection as transmitting + data off the device *irrespective of recipient*. Neither the on-device nor the + ephemeral exemption applies, and the E2EE exemption doesn't survive TLS to a + server that reads plaintext. File **Collected, not Shared**, encrypted in + transit. There is no credentials category, but "authentication information" is + explicitly named as personal and sensitive data. +- **Account Deletion policy does not apply** (offline-created accounts are out of + scope), but ship a "Remove account and delete local data" action anyway — + cheap insurance against a reviewer pattern-matching. +- **Do not ship `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` in the Play build.** + Generic server sync is not on the acceptable-use list. Use + `ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS`. DAVx5 declares the former and is + F-Droid-safe on it; we would not be. + +--- + +## What lands in floret-kit + +| Candidate | Kit module | Note | +|---|---|---| +| DAV client + iCalendar parse/serialise | new, e.g. `core-dav` | **the big one.** Calendula needs the same primitives. Design for two consumers from the start | +| Sync-adapter + authenticator scaffolding | new, e.g. `core-sync` | including the stub-adapter→WorkManager bridge, which is pure mechanics | +| Nextcloud Login Flow v2 | with `core-dav` | pure protocol, zero task knowledge | +| Credential storage | kit | Keystore mechanics, not domain | +| Third-party licence screen | kit | Calendula needs it the moment it takes any of the above | + +**Stays app-local:** the VTODO ↔ `TaskContract` mapper (domain, and where all the +judgement calls live), `ICalendarWriter`, conflict policy and its UI, and +everything about storage modes. + +--- + +## Test strategy + +- **Round-trip corpus first** — VTODO fixtures from each target server; assert + `parse → store → serialise` is byte-stable for untouched properties. The + mapper's specification, not its regression net. +- **Server matrix:** Nextcloud, Radicale, Baïkal (**on Digest**), SOGo, Fastmail, + iCloud — with the per-server traps above as named test cases. Google is out. + Two that must be explicit tests, because both silently destroy data: + **round-tripping a task from a shared Nextcloud calendar**, and **an + ETag-unchanged body change on SOGo**. +- **Conflict scenarios:** concurrent edit, delete-vs-edit, collection removed + server-side, credentials revoked mid-sync. +- **Interop:** edit the same task from Nextcloud web and from tasks.org + DAVx5. +- **Device, not Robolectric,** for account paths — `ProviderAccountCleanupTest` + skips on ARM64. Two specific device cases the audit added: **restore a cloud + backup onto a fresh device** and confirm nothing is pruned; **remove the + account** and confirm only our lists go. +- ⚠️ **Minified-build tests.** `:app` runs `isMinifyEnabled` + `isShrinkResources` + in release, and `proguard-rules.pro` already documents two R8-pruning outages. + ical4j resolves through seven `META-INF/services` files and instantiates its + cache from a class-name string — it needs `-keep class net.fortuna.ical4j.** { *; }` + plus ~8 `-dontwarn` lines, and is effectively unshrinkable. Without them this + fails in `release` only. + +--- + +## Open questions + +1. **dav4jvm + cert4android distribution *and* Java target.** Scoped JitPack, + vendor, or in-house verbs? Now compounded: 4.x requires **Java 21** and we + target 17 across `:app` and all of floret-kit. Vendoring recompiles at our own + target and freezes the API churn — it looks better than it did. Before phase 2. +2. **Conflict policy** — preserve-local-on-412, or documented LWW? +3. **External mode** — survives, or becomes an importer? Before phase 1. +4. **Canonical recurring-completion behaviour** — and specifically, ⚠️ **do we + honour `:provider`'s `Detaching` processor (model d), or bypass it and write + `RECURRENCE-ID` overrides (model a)?** No longer an open-ended taste question: + our storage layer already answered it and we have to ratify or override that. + Moved up to **phase 1**. +5. **The DAVx5 enum ask** — worth filing, and what compatibility we owe if it + lands. +6. ⚠️ **New: does Local→Synced migrate, or do synced lists start empty?** See + [the migration section](#-the-migration-that-was-believed-not-to-exist). +7. ⚠️ **New: lib-recur — pin at 0.12.2, or rewrite the provider's iterators?** + +Answered elsewhere and **not** open: the account model (`AccountManager` **plus a +stub sync adapter**), `ical4android` (superseded by `synctools`, GPLv3), and the +storage question. + +--- + +## Dead ends — do not revisit + +- **Depending on DAVx5 for sync.** Settled in `STORAGE-AND-SYNC.md`. +- **`synctools` / `ical4android`.** GPLv3. The temptation recurs because it does + exactly the right mapping against exactly our schema. +- **Rewriting storage to Room before sync exists.** [See above](#settled--the-storage-question-is-not-reopened-here). +- ⚠️ **AccountManager + WorkManager with no registered sync adapter.** Not a + design choice — a silent no-op at targetSdk ≥ 34. +- ⚠️ **Writing through the `instances` URI as a sync adapter.** The flag is + ignored there; every such write dirties the row and forks an override. +- ⚠️ **Hand-rolled TrustManager to avoid a GPL licence cert4android does not + have.** + +--- + +## Related + +- [`STORAGE-AND-SYNC.md`](STORAGE-AND-SYNC.md) — where task data lives. This is + its step 5. +- [`../provider/PROVENANCE.md`](../provider/PROVENANCE.md) — every deviation from + upstream dmfs. Change 1 is load-bearing for the account model here. +- [`ARCHITECTURE.md`](ARCHITECTURE.md) §4 — the data seam the adapter writes + underneath. diff --git a/provider/PROVENANCE.md b/provider/PROVENANCE.md index f911df4..6cfcefe 100644 --- a/provider/PROVENANCE.md +++ b/provider/PROVENANCE.md @@ -159,7 +159,7 @@ drift apart. Keep that convention. `contenttestpal`, which are JitPack-only; adding JitPack would widen the dependency trust surface for test-only code. ⚠️ This is the one place vendoring lost coverage — those were the provider's *integration* tests - (recurrence, reparenting, instances, observers). The 51 JVM tests in + (recurrence, reparenting, instances, observers). The 56 JVM tests in `src/test` all pass and are retained. 15. **`agendula_provider_changed_receivers` emptied.** Upstream notifies `org.andstatus.todoagenda`, which listens for changes to the *dmfs* authority @@ -175,7 +175,7 @@ drift apart. Keep that convention. Unlikely to ever be worth it — upstream 1.4.2 is from 2021 — but if it is: the `AGENDULA CHANGE` markers are the complete list of what to reapply, `git log` on -this directory is the audit trail, and the 51 JVM tests are the safety net. +this directory is the audit trail, and the 56 JVM tests are the safety net. Re-read change 1 before touching anything account-related. ## Known-unverified