amadan
Revise the offline kit spec for the #5 review
Durability watermark and replica-plus-lane reopen (I1a/I1b), first-400 retry and a 503 for a full Docs update log, unavailable-versus-lost store handling, never-renamed sealed paths, the existing snapshot op is already pending-inclusive, offline-on-satisfied-path backs off, FIFO-preserving compaction, frozen in-flight Sheets entries, mandatory 180-day session ceiling, a subject guard without a first-seen bypass, Sheets stamps and replay order, sealing details, a narrower v1 API, and a test strategy. Calendar device reminders move to their own spec; Docs maxUpdateBytes rises to 8 MiB.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+392
−148
docs/superpowers/specs/2026-09-23-offline-kit.md+392 −148
CI failed — run details
diff --git a/docs/superpowers/specs/2026-09-23-offline-kit.md b/docs/superpowers/specs/2026-09-23-offline-kit.md| index 9bd9f80..338e54b 100644 |
| --- a/docs/superpowers/specs/2026-09-23-offline-kit.md |
| +++ b/docs/superpowers/specs/2026-09-23-offline-kit.md |
| @@ -1,6 +1,9 @@ |
| # Native offline mode: the shared kit, and how Docs, Sheets, Memoria, Calendar and Calls adopt it |
| -Date: 2026-09-23. Status: **design spec, nothing implemented.** |
| +Date: 2026-09-23. Status: **design spec, nothing implemented.** Revision 2 |
| +addresses the review on rastrillo/native #5 (findings H1–H5, M1–M7, L1–L4, |
| +the API-minimalism list and the test strategy). Each finding is marked where |
| +it changed the text, for example *(H1)*. |
| Paul, 2026-09-23: "make sure that all the native apps as appropriate have |
| offline mode with changes synced back when the connection is restored." |
| @@ -17,7 +20,7 @@ migration code is proposed anywhere in this document. |
| | Repo | Ref read | What changed since the audits | |
| |---|---|---| |
| -| rastrillo/native | `origin/main` e51949f; `origin/merge/native-apple-chain` eacc073 | Main still holds only `RastrilloNative/CoalescedRunner.swift`. The chain merge `3851559` (33ffe99..2295259) is approved on discussion #4 (Fable: APPROVE-WITH-NITS) and waits on CI infrastructure. Kit citations below are to **eacc073**, which is what main will contain once the merge lands. | |
| +| rastrillo/native | `origin/main` **2a5d07b** | The chain merge (33ffe99..2295259, discussion #4) is on main: `Sources/RastrilloApple/*` and `server/` are there. The tree is identical to the merge branch's eacc073, so line numbers are unchanged. *(L4)* | |
| | oficina/docs | `origin/main` 7543cf5 | The `native/sync-batching` line is on main: merged batching (op 253), `creation_token` drafts (migration 0008), rename. Pin is `2295259` (`native/apple/Package.swift:6`, `native/ios/project.yml:11`). | |
| | oficina/sheets | `origin/main` b9a92e3 | `native/document-windows` is on main. **B12 is fixed.** Write responses now carry `stamps`, and `SheetModel.accept` refetches only when `version > held + max(stamps, 1)` (`SheetModel.swift:115-124`, `grid.go` `gridResponse.Stamps`). Pin is `2295259`. | |
| | oficina/calendar | `origin/main` f6aa41c | The native API and clients are on main (cd3478c). **B11 is fixed**: a failed load keeps the last good events (`CalendarModel.swift:75-76`). There is still no revision, uid, delete or reminder data in `internal/calendar/native.go` (grep finds none), and no delete route (`routes.go:112-117`). Pin is `2295259`. | |
| @@ -25,17 +28,17 @@ migration code is proposed anywhere in this document. |
| | oficina/meet (Calls) | `origin/main` 3846ca4 | Unchanged for this purpose. It pins `4c61d62` for `RastrilloMacUpdates` only (`native/apple/Package.swift:6`). Cookies are still held in memory only (`CallsModel.swift:34`). | |
| So the four-way pin split in the shared audit has collapsed to one. The four |
| -data apps pin `2295259`, and Calls pins `4c61d62`. Step 0 of PLAN.md §3 |
| -reduces to three things: |
| +data apps pin `2295259`, and Calls pins `4c61d62`. With #4 landed, Step 0 of |
| +PLAN.md §3 reduces to two things: |
| -1. Land #4. |
| -2. Re-pin every app to the merge commit on rastrillo/native main. |
| -3. Bump each app server's `amadan.net/rastrillo/native/server` module off |
| +1. Re-pin every app to rastrillo/native main: the four data apps off |
| + `2295259`, and Calls off `4c61d62`. |
| +2. Bump each data app server's `amadan.net/rastrillo/native/server` module off |
| `33ffe99` (docs `go.mod:9`, and likewise in the others). #4 already notes |
| that this is needed for the browser callback. §3.5 needs it too. |
| Citation shorthand used below: |
| -- `kit:` means rastrillo/native@eacc073 `Sources/RastrilloApple/`. |
| +- `kit:` means rastrillo/native@2a5d07b `Sources/RastrilloApple/`. |
| - `DocsKit/` means docs@7543cf5 `native/apple/Sources/DocsKit/`. |
| - `SheetsKit/` means sheets@b9a92e3 `native/apple/Sources/SheetsKit/`. |
| @@ -173,8 +176,9 @@ public protocol NativeTransport: Sendable { |
| - Anything that can outlive a session takes |
| `@MainActor () -> NativeTransport?` instead. Re-sign-in (§3.5) swaps the |
| transport under it. |
| - - `NativeConnection` gains `transport(for accountID:) -> NativeTransport?`. |
| - Today only the active account's `client` exists (`kit:NativeConnection.swift:8, 55`). |
| + - v1 needs no per-account transport lookup: only the active account drains |
| + (§3.3). Today only the active account's `client` exists |
| + (`kit:NativeConnection.swift:8, 55`), and that stays true. |
| ### 3.2 Connectivity monitor: `NativeConnectivity` |
| @@ -188,6 +192,8 @@ This is `@MainActor @Observable`, with one instance per process. |
| - A path change resets it to unknown. |
| - Nothing pings. The Docs and Sheets SSE streams act as the heartbeat. The |
| other apps learn on their next real request. |
| + - **A change to `true` also fires `changes`**, so a stream that opens kicks |
| + the drainers at once *(M2)*. |
| - **`isOffline`** means `path != .satisfied`, or the path is satisfied and |
| `serverReachable == false`. |
| - **`changes: AsyncStream<Void>`** fires on a path change and on app |
| @@ -196,7 +202,7 @@ This is `@MainActor @Observable`, with one instance per process. |
| after the current sleep. |
| - This replaces the fixed 2 s sleeps in Docs `listen` (`DocsKit/DocsModel.swift:323`) |
| and Sheets `watch` (`SheetsKit/SheetLive.swift:28`). |
| -- **Fail fast.** While `path == .unsatisfied`, drainers do not attempt sends. |
| +- **Fail fast.** While `path != .satisfied`, drainers do not attempt sends. |
| Apps may skip user-initiated requests and show the offline state at once. |
| Memoria search should, instead of waiting out the 30 s timeout |
| (`kit:NativeHTTP.swift:32`). |
| @@ -214,6 +220,10 @@ Application Support/<bundle id>/accounts/<account.id>/ |
| - Everything is written by `NativeSealedStore` (§3.6). On iOS the file |
| protection is `.complete`, and files are `isExcludedFromBackup`: the key |
| cannot leave the device, so a backed-up copy could never be read anyway. |
| +- **"Durable on return" means it survives a process kill** *(L2)*: the file is |
| + written with `Data.write(options: .atomic)`, a temporary file renamed into |
| + place. It does not promise survival of a power loss, which would need |
| + `F_FULLFSYNC` on every write. The kill case is what the tests prove. |
| - SQLite is unnecessary. The volumes are small, and one atomic file per entry |
| is enough. Sheets proposed SQLite in its audit. With one entry per *cell* |
| (§5), a long offline session is at most thousands of small files, and that |
| @@ -221,16 +231,24 @@ Application Support/<bundle id>/accounts/<account.id>/ |
| - **Keying.** The key is `NativeAccount.id`, the UUID that |
| `credentialKey` already binds to the origin (`kit:NativeAccount.swift:9`). |
| §3.5 makes that id survive a re-sign-in. The audits proposed keying by |
| - (origin, subject) instead. The subject is added as a *guard* in §3.5, not |
| - as the key, so an account that has not yet been given a subject keeps |
| - working. |
| + (origin, subject) instead. The subject is a *guard* in §3.5, not the key. |
| +- **Paths never move** *(H5)*. The sealing binds each file to its path |
| + (§3.6), so the kit has no rename or retarget operation. An app that learns |
| + a better name for something later, as Docs does when a draft gets its sqid, |
| + records the mapping in its own index (§4.5). |
| #### Types (RastrilloNative, Foundation only; the store is behind a protocol) |
| ```swift |
| -public protocol BlobStore: Sendable { // implemented by NativeSealedStore; tests use an in-memory one |
| - func write(_ data: Data, to path: String) throws // atomic |
| - func read(_ path: String) throws -> Data? |
| +public enum BlobStoreError: Error, Sendable { |
| + case unavailable // locked device, file protection, Keychain interaction not allowed: retry later, never delete |
| + case keyLost // errSecItemNotFound while the directory holds sealed files |
| + case corrupt(String) // one file failed its GCM tag: that path is removed alone |
| +} |
| + |
| +public protocol BlobStore: Sendable { // implemented by NativeSealedStore; tests use in-memory and faulting ones (§3.8) |
| + func write(_ data: Data, to path: String) throws // atomic, see "durable on return" |
| + func read(_ path: String) throws -> Data? // nil = no such file |
| func list(_ directory: String) throws -> [String] |
| func remove(_ path: String) throws |
| } |
| @@ -238,9 +256,9 @@ public protocol BlobStore: Sendable { // implemented by NativeSealedS |
| public struct OutboxEntry: Codable, Sendable, Identifiable { |
| public let id: UUID // stable; apps may send it as an idempotency key (Memoria client_id) |
| public let seq: UInt64 // per account, monotonic, recovered as max+1 at load |
| - public var lane: String // FIFO scope: "doc:<sqid|draft-token>", "tab:<id>", "notes", "event:<id|temp>" |
| + public let lane: String // FIFO scope: "doc:<key>", "tab:<id>", "notes", "event:<id|temp>"; never changes |
| public let kind: String // app-defined |
| - public var payload: Data // opaque app bytes |
| + public let payload: Data // opaque app bytes |
| public let createdAt: Date |
| public var attempts: Int |
| public var hold: Hold? // nil = sendable |
| @@ -249,16 +267,14 @@ public struct OutboxEntry: Codable, Sendable, Identifiable { |
| public struct Hold: Codable, Sendable { |
| public enum Reason: Codable, Sendable { case conflict(Data), rejected(String), accessLost, tooLarge } |
| public let reason: Reason |
| - public let blocksLane: Bool // Docs: true (CRDT order); Sheets conflict on one cell: false |
| + public var blocksLane: Bool = true // v1 (Docs) only ever blocks; `false` arrives with the Sheets slice |
| } |
| public actor Outbox { |
| public init(store: BlobStore, account: String) |
| public func enqueue(lane: String, kind: String, payload: Data, id: UUID = UUID()) throws -> OutboxEntry // durable on return |
| - public func replace(_ ids: [UUID], with entry: (lane: String, kind: String, payload: Data)) throws // write-new-then-delete-old |
| - public func update(_ id: UUID, payload: Data) throws // coalescing (Sheets cell, Calendar event, Memoria edit) |
| - public func retarget(lane from: String, to: String) throws // Docs draft -> sqid |
| - public func sendable(lane: String, limit: Int) -> [OutboxEntry] |
| + public func replace(_ ids: [UUID], with payload: Data) throws // compaction; keeps the oldest seq (M3) |
| + public func sendable(lane: String, maxBytes: Int?) -> [OutboxEntry] // by bytes, not count: Docs batches 512 KiB |
| public func ack(_ ids: [UUID]) throws |
| public func hold(_ ids: [UUID], _ hold: Hold) throws |
| public func release(_ id: UUID) throws // user resolved: back to sendable |
| @@ -269,23 +285,44 @@ public actor Outbox { |
| } |
| ``` |
| +**Kept out of v1 on purpose** (the review's API-minimalism list). Docs has to |
| +exercise every method that ships, which is the "one builds, one adopts" bar: |
| +- `retarget` is gone *(H5)*. Lanes and paths never move. |
| +- `update(id, payload:)` (coalescing in place) is deferred to the Sheets |
| + slice. Only Sheets needs it, and it must refuse an entry that is in flight |
| + (§5, *M4*). Calendar and Memoria adopt it after Sheets. |
| +- `Hold.blocksLane` defaults to `true`. The non-blocking hold is added and |
| + tested with Sheets. |
| +- Draining accounts that are not the active one is left out (below). |
| + |
| **Rules** |
| -- **FIFO within a lane; lanes are independent.** A held entry with |
| - `blocksLane` stops its own lane only. Other documents or sheets keep |
| - draining. |
| -- **`replace` is not atomic.** It writes the new entry, then deletes the old |
| - ones. A crash between the two leaves both, so it must only be used where a |
| - duplicate is harmless. That holds for Docs (Yjs integrates duplicates |
| - idempotently: docs `internal/docs/handlers.go:879-880`). For Sheets and |
| - Calendar, coalescing uses `update` on one entry instead. |
| +- **FIFO within a lane; lanes are independent.** A held entry stops its own |
| + lane only. Other documents or sheets keep draining. |
| +- **`replace` keeps FIFO order** *(M3)*. The merged entry takes the seq of the |
| + **oldest** entry it replaces: |
| + 1. overwrite `<oldest seq>.entry` with the merged entry, in one atomic |
| + `write` sealed for that same path. The atomic write is itself a |
| + temporary file renamed into place, and it carries the final path as its |
| + associated data, so a sealed file is never renamed after sealing (§3.6); |
| + 2. delete the other replaced entries. |
| + |
| + A crash after step 1 leaves duplicates only, never a gap and never |
| + reordering. Only a lane whose payloads tolerate duplicates may use |
| + `replace`. Docs qualifies (Yjs integrates duplicates idempotently: docs |
| + `internal/docs/handlers.go:879-880`). |
| - **No silent caps.** An app sets a per-lane byte ceiling. Exceeding it is a |
| visible `failed` state (§3.4). It never drops input. |
| +- **An unavailable store is never an empty store** *(H4)*. `.unavailable` |
| + from any store call means: don't read, don't delete, keep the current |
| + status, and retry on `.active` or unlock. |
| #### `OutboxDrainer` (RastrilloApple) |
| -There is one drainer per account with sendable entries and a credential. |
| -This includes accounts that are not the active one, so a switched-away |
| -account still drains. |
| +There is one drainer, for the **active** account. A switched-away account |
| +keeps its outbox untouched and drains when it is selected again. Nothing is |
| +deleted, which is what principle 4 requires. Draining several accounts at |
| +once is machinery Docs would never exercise, so it waits until an app needs |
| +it. |
| - It takes the account's `Outbox`, a `() -> NativeTransport?` supplier and |
| `NativeConnectivity`. |
| @@ -299,21 +336,25 @@ account still drains. |
| | Typed error | Drainer reaction | |
| |---|---| |
| -| `.offline` | Mark offline; stop all lanes; wait for `connectivity.changes`. No timed retries while the path is down. | |
| +| `.offline`, path not satisfied | Mark offline; stop all lanes; wait for `connectivity.changes`. No timed attempts while the path is down. | |
| +| `.offline`, path satisfied (for example `timedOut`, `cannotConnectToHost`, or a lost response) | **Back off (§3.4) and retry the same batch** *(M2)*. Waiting for a path change here would strand the drainer until the next foreground. | |
| | `.unauthorized` | Mark the account **session ended** (§3.5); stop all lanes; wait for re-sign-in. Never hold or drop entries. | |
| | `.rateLimited(after)` | Back off at least `after`. | |
| | `.server`, `.transient`, `.invalidResponse` | Back off (§3.4) and retry the same batch. | |
| -| `.forbidden`, `.notFound`, `.conflict`, `.tooLarge`, `.rejected` | The app's `send` is expected to catch these and return `.hold`. If it does not, the drainer holds the batch with `blocksLane: true` rather than spinning. | |
| +| `BlobStoreError.unavailable` while acking or holding | `.retry` *(H4)*. The send may already have been accepted; the server-side idempotency of each app (Yjs, base value, `client_id`) makes the repeat harmless. | |
| +| `.forbidden`, `.notFound`, `.conflict`, `.tooLarge`, `.rejected` | The app's `send` is expected to catch these and return `.hold` (or `.retry`, see Docs' first-400 rule, §4.4). If it does not, the drainer holds the batch rather than spinning. | |
| -- **Triggers:** enqueue, connectivity change, foreground, re-sign-in, and an |
| - explicit "Try again". At most one batch per account is in flight, which is |
| - the simplest correct ordering against one server. |
| +- **Triggers:** enqueue, connectivity change (including a stream opening), |
| + foreground, re-sign-in, account selection, and an explicit "Try again". |
| + At most one batch is in flight, which is the simplest correct ordering |
| + against one server. |
| #### iOS lifecycle |
| - On `scenePhase == .background`: |
| - apps flush their cache writes (for example the Docs replica, §4.3); |
| - the drainer wraps any in-flight send in `beginBackgroundTask` so it can |
| - finish. |
| + finish. If the device locks first, the store becomes unavailable, and a |
| + failed ack or cache write is a `.retry`, not a loss *(H4)*. |
| - Nothing is scheduled with `BGTaskScheduler` (D3). On `.active`, the |
| drainers wake. |
| - Today there is no `scenePhase` or `beginBackgroundTask` anywhere in Docs |
| @@ -392,15 +433,34 @@ account still drains. |
| - `selectAccount`'s "This account needs to sign in again." (`:46`) routes |
| to `reauthenticate` as well. |
| - **Subject guard** (server + client). `/native/exchange` (rastrillo/native |
| - `server/auth.go:190-250`) adds an opaque, stable `subject` to its JSON. This |
| - is an HMAC of the person id under the app's key, so the value is not |
| - portable across apps. |
| + `server/auth.go:190-250`) adds an opaque, stable `subject` to its JSON: |
| + `HMAC-SHA256(subjectKey, person id)`. |
| + - **The key is named and stable** *(M6)*. The server module's `Auth` config |
| + gains `SubjectKey []byte`. Each app supplies it from a persistent |
| + instance secret that already survives deploys (the secret class its |
| + sessions or sealing already depend on), under a distinct label. It is |
| + never a per-process random value, which would change the subject at every |
| + restart and lock every account out. It is per app, so subjects are not |
| + portable across apps. |
| - `NativeAccount` gains `subject: String?`, an optional Codable field that |
| old records decode without. |
| - `reauthenticate` refuses a session whose subject differs from the stored |
| one: "You signed in as someone else. The pending changes belong to |
| ‹account›." It then offers *Add as a new account*. |
| - - An account with no stored subject yet accepts the first one it sees. |
| + - **Accounts without a subject** *(M6)*. "Accept the first subject seen" |
| + would disarm the guard for every account that exists today. Instead, |
| + the first launch of a build that carries the kit marks every subject-less |
| + account session-ended. That happens before any outbox exists, so nothing |
| + is at risk, and the one re-sign-in records the subject. Apps hold no real |
| + data and Paul is the only user, so one sign-in per app is the whole cost. |
| + (A `/native/whoami` endpoint read on the first online request would avoid |
| + that sign-in. It is not proposed, because it is another endpoint for a |
| + one-time migration.) |
| + - **The guard is client-enforced**, and the spec says so plainly. The server |
| + cannot tell whose outbox a cookie is draining. The guard stops the app |
| + from sending account A's queued changes under person B's session. It is |
| + not a server-side authorisation control, and the server still authorises |
| + every write as the person who owns the cookie. |
| - This needs the app servers to leave `33ffe99` of the server module (§0). |
| - **Sign-out with pending items: warn, then wipe (D4).** |
| - `NativeConnection` gains `signOutImpact: (accountID) async -> Int`, fed by |
| @@ -422,8 +482,8 @@ account still drains. |
| 4. Remove the account record. |
| 5. POST `/signout` as today. |
| - "Remove account" in `NativeAccountsView` follows the same path. |
| - - **Switching accounts is not signing out.** It keeps the outbox, and the |
| - switched-away account keeps draining (§3.3). Docs currently disables |
| + - **Switching accounts is not signing out.** It keeps the outbox untouched, |
| + and the account drains when it is selected again (§3.3). Docs currently disables |
| Accounts and Sign out while anything is unsaved (`DocsKit/DocsWorkspace.swift:49, 56`). |
| Once the outbox is durable, that gate only needs to cover unpersisted |
| input. |
| @@ -434,23 +494,52 @@ This covers every byte of cached content and every outbox entry, in every |
| app. Cached content today includes Docs replicas and lists, Sheets snapshots, |
| the Calendar window, and Memoria's pending notes. |
| -- **Key.** One 256-bit `SymmetricKey` per account, stored as a generic |
| +- **Key** *(L1)*. One AES-256 `SymmetricKey` per account, stored as a generic |
| password. |
| - Service `<bundle id>.store`, account `store:<account.id>|<origin>`. |
| - - `kSecAttrAccessibleWhenUnlockedThisDeviceOnly`, not synchronizable. That |
| + - `kSecAttrAccessibleWhenUnlockedThisDeviceOnly`, never synchronizable. That |
| is the same class as the session (`kit:NativeCredentials.swift:26`). |
| - It is created on first write and never leaves the device. |
| -- **Sealing.** Each file is `AES.GCM.seal(plaintext, using: key, authenticating: relativePath)`, |
| - stored as `.combined`. The relative path is the associated data, so a file |
| - copied or renamed onto another path fails to open. For example, Document |
| - A's replica cannot be swapped in as Document B's. |
| + - It is reached through a `KeyProvider` protocol, so tests can script every |
| + Keychain status (§3.8). |
| +- **Sealing** *(L1, H5)*. Each file is |
| + `AES.GCM.seal(plaintext, using: key, authenticating: aad)`, stored as |
| + `.combined`. |
| + - `.combined` is the 96-bit nonce, then the ciphertext, then the 128-bit tag. |
| + - No nonce is passed, so CryptoKit draws a fresh random nonce per call. |
| + Random-nonce GCM is safe below 2^32 seals per key (NIST SP 800-38D). The |
| + Docs writer seals about once per 2 s of typing, which is nowhere near |
| + that. The store still counts seals per key and rotates the key at 2^31: |
| + it re-seals under a new key and then deletes the old one. |
| + - The associated data is explicit: `accountID | format version | relative path`. |
| + A file copied or renamed onto another path, or into another account, |
| + fails to open. For example, Document A's replica cannot be swapped in as |
| + Document B's. **Because of this, nothing sealed is ever renamed** (§3.3, |
| + §4.5). |
| +- **File protection.** On iOS, files use `.complete` (§3.3). On Mac the |
| + sandbox container is the only file protection. That is accepted, because |
| + the content is sealed anyway. |
| - **Wipe.** At sign-out or "Remove account", delete the key first, then the |
| directory. A file that survives the delete (for example in a crash, or in a |
| snapshot) is unreadable. |
| -- **Unreadable store.** If the key is missing or a file fails to open, treat |
| - the store as empty and remove it. If outbox entries were among the loss, |
| - say so once: "Unsent changes on this device could not be read and were |
| - removed." Never crash, and never show a partial state. |
| +- **Unavailable is not lost** *(H4)*. With `WhenUnlockedThisDeviceOnly`, |
| + `SecItemCopyMatching` answers `errSecInteractionNotAllowed` while the device |
| + is locked: for example, an app resumed in the background, or launched |
| + before first unlock. `.complete` files refuse I/O after lock. Today |
| + `NativeCredentials.read` folds every Keychain status into `nil` |
| + (`kit:NativeCredentials.swift:15-16`), and the store must not copy that. |
| + - **Key lost** means `errSecItemNotFound` **and** a populated |
| + `accounts/<id>/` directory. Only then is the store treated as empty and |
| + removed, with one notice if outbox entries were among the loss: "Unsent |
| + changes on this device could not be read and were removed." |
| + - **Every other Keychain status, and any file I/O error**, means |
| + *unavailable*. Don't read, don't delete, keep the current status, and |
| + retry on `.active` or unlock. |
| + - **One file failing its GCM tag** is removed alone and reported once. The |
| + directory is never removed for it. |
| + - Never crash, and never show a partial state. |
| + - `NativeCredentials.read` gets the same split, so a locked device reads as |
| + "unavailable", not "this account needs to sign in again". |
| - **Why uniform.** |
| - Memoria's rule is that content is sealed at rest |
| (memoria `internal/memoria/seal/seal.go`). The Memoria spec also says the |
| @@ -468,8 +557,6 @@ the Calendar window, and Memoria's pending notes. |
| D3. |
| - The Calendar menu bar is Mac-only, and the Mac login keychain is unlocked |
| while the user is logged in, so the menu bar is unaffected. |
| - - Calendar local notifications (D6) are scheduled into the system while the |
| - app runs, so they fire without reading the store. |
| - **"Remove offline copies"** is a per-account setting. It deletes the |
| `cache/` directory and keeps the outbox. This is the "forget this device" |
| control the Docs spec calls for. |
| @@ -482,20 +569,76 @@ renewal** (rastrillo v0.23.0 `sessions/sessions.go:56-57, 90-91`; |
| `auth/auth.go:68`). No app overrides it. So a laptop left idle for a month, |
| with queued edits, always comes back to `needsSignIn`. |
| -**Proposed: sliding expiry for native sessions only.** |
| +**Proposed: sliding expiry for native sessions only, with a mandatory |
| +ceiling** *(M5)*. |
| - Mark sessions minted by `/native/exchange` as native. |
| - An authenticated request on a native session extends its expiry to |
| now + 30 days. The row is written at most once per 24 h, so a busy client |
| does not write a session row on every request. |
| +- **The expiry never passes `auth_time` + 180 days.** Without a ceiling, a |
| + stolen native cookie that keeps being used would never expire except by |
| + explicit revocation, and nobody would see it, because the client stores no |
| + expiry. After 180 days the person signs in again (§3.5 keeps the outbox). |
| - The idle limit stays 30 days. Browser sessions are unchanged. |
| - The cookie value does not change, so the client needs no refresh protocol. |
| The stored credential simply stays valid while it is used. |
| - Revocation still works, because renewal happens server-side on a row that |
| can be deleted. |
| -- An absolute ceiling (for example 1 year) is optional, and listed under D2. |
| -- **Where it goes:** the rastrillo `sessions` package, plus the native server |
| - module, plus a module bump in each app. This is shared rastrillo work. It is |
| - not a kit change. |
| +- **Where it goes.** This is shared rastrillo work, not kit work: |
| + - `sessions` has no native flag and no renewal path today. `lookup` is |
| + read-only (rastrillo v0.23.0 `sessions/sessions.go:162`), and the table |
| + has no column for either (`sessions/migrations`: `token_hash`, |
| + `subject`, `method`, `auth_time`, `created_at`, `expires_at`). |
| + - So it is a `carlosframework/rastrillo` change: add `kind` and |
| + `renewed_at` columns, and a best-effort renewal in the resolve path that |
| + never fails a request. |
| + - Then a module bump in the four data apps (docs, sheets, calendar and |
| + memoria all require v0.23.0), plus the native server module, which marks |
| + the sessions it mints. Calls uses `amadan.net/rastrillo/rastrillo` |
| + instead and needs this only once it adopts `NativeConnection` (§8). |
| + |
| +### 3.8 Test strategy |
| + |
| +The fault model has to reach the *store* and the *timing of the writer*, not |
| +only the transport. Every durability bug in the review (H1, H2, H4) lives |
| +there. `RastrilloTestSupport` (§3.1) ships four primitives: |
| + |
| +- **`ScriptedTransport`** answers each call from a script: a status and body, |
| + `.offline`, a hang until cancelled, or "commit, then lose the response". |
| +- **`FaultingBlobStore`** wraps an in-memory `BlobStore`. It follows a crash |
| + schedule: "throw `SimulatedCrash` at store operation N, and freeze the |
| + committed view". The test then rebuilds `Outbox` and the app model from |
| + the frozen view, which models kill -9 at every possible point. It can also |
| + answer `.unavailable` on demand (a locked device). |
| +- **`KeyProvider`** is a protocol with a scriptable fake. It returns a key, |
| + `errSecItemNotFound`, `errSecInteractionNotAllowed` or any other status. The |
| + H4 paths (unavailable versus lost) are then plain unit tests. |
| +- **An integrating fake server.** "Commit, then lose the response" is only |
| + meaningful if the fake actually applies the write. For Docs, the fake |
| + integrates each update into its own yrs document and serves `?since=` |
| + and the stream. For Sheets, it keeps raws and versions with the `stamps` |
| + rule. Convergence can then be asserted in-process. |
| + |
| +**The Docs property test (K3).** |
| +- **Event alphabet:** edit; writer tick, *including between `command` and |
| + `enqueue`*; ack; lose-response; crash at any store operation; reopen and |
| + apply the lane; reconnect. |
| +- **Assertions, after every event and at the end:** |
| + - I1 (§4.3); |
| + - after a full drain, server text equals local text; |
| + - no entry is acknowledged that the server lacks; |
| + - no text appears twice after a reopen (H2). |
| +- A generator that cannot place a writer tick between `command` and `enqueue` |
| + cannot fail on H1, so that placement is required, not incidental. |
| + |
| +**Kept for the real thing.** `native/scripts/test-sync-latency.py` and |
| +`DocumentSyncIntegrationTests` stay for real kill -9 runs against a real |
| +server. Network Link Conditioner and airplane mode stay manual checks. |
| + |
| +**Sheets.** Extend `internal/sheets/version_contract_test.go` with partial |
| +batches (§5), and add M4's sequence: a batch commits, the response is lost, |
| +the same cell is edited offline, reconnect, and the result must contain no |
| +conflict. |
| ## 4. Docs: first adopter, in detail |
| @@ -537,27 +680,34 @@ with queued edits, always comes back to `needsSignIn`. |
| | Path | Content | |
| |---|---| |
| | `cache/docs/list` | Last successful unfiltered `DocumentList` + `fetchedAt` | |
| -| `cache/docs/<key>/replica` | Header `{format: 1, sequence: Int64, guid, savedAt}` + **pending-inclusive** Yjs v1 state | |
| +| `cache/docs/index` | sqid → `<key>`, for documents born on this device as drafts (§4.5) | |
| +| `cache/docs/<key>/replica` | Header `{format: 1, sequence: Int64, guid, savedAt}` + Yjs v1 state from the existing `snapshot` op (pending-inclusive, §4.3) | |
| | `cache/docs/<key>/meta` | `sqid?`, `draftToken?`, `title`, `pendingTitle?`, `can_write`, `can_share`, `lastOpened`, `lastSyncedAt` | |
| | outbox lane `doc:<key>` | Unacknowledged deltas, FIFO (kit outbox, not a Docs file) | |
| -| `cache/docs/images/<digest>` | Image bytes already shown. Slice D6 (§4.10); `DocsImageStore` is memory-only today (`DocsAttachments.swift:25`) | |
| +| `cache/docs/images/<digest>` | Image bytes already shown. Slice K7 (§4.10); `DocsImageStore` is memory-only today (`DocsAttachments.swift:25`) | |
| -- `<key>` is the sqid, or `draft-<creation token>` for a draft (§4.5). |
| +- `<key>` is the sqid for a document first opened from the server. It is |
| + `draft-<creation token>` **for life** for a document created on this device |
| + *(H5)*. It never changes, because sealed paths never move (§3.6). |
| - `sequence` lives in the same file as the state it describes. A sequence can |
| then never be newer than its state, so `?since=` can never skip rows. |
| ### 4.3 Persisted Y.Doc state |
| -**Use a pending-inclusive encoding.** |
| -- The native `snapshot` op is `encode_state_as_update_v1(&StateVector::default())` |
| - (`native/crdt/src/lib.rs:43-44`). That drops yrs's *pending* structs, the |
| - ones whose dependencies have not arrived. |
| -- The server core adds them back through `merge_pending_v1` for exactly this |
| - reason (`core/src/snapshot.rs:7-21, 59`). |
| -- Add a `snapshot_pending` op to native/crdt. It calls the core function if |
| - the crate graph allows, or otherwise mirrors it, with a golden test that |
| - both produce identical bytes for a document holding pending structs. |
| -- Persist only that encoding. `CRDTReplica` gains `persistentState()`. |
| +**The existing `snapshot` op is already pending-inclusive** *(M1)*. |
| +- Revision 1 of this spec, following the Docs audit, claimed the `snapshot` |
| + op drops yrs's pending structs. That is wrong. |
| + - The op is `encode_state_as_update_v1(&StateVector::default())` |
| + (`native/crdt/src/lib.rs:43-44`). In the vendored yrs that native/crdt |
| + builds against (`native/crdt/Cargo.toml:17`), that function *ends* with |
| + `merge_pending_v1` (`native/vendor/yrs-0.23.5/src/transaction.rs:138-143`). |
| + - It is `encode_diff_v1` that omits the pending queue. The server core's |
| + `core/src/snapshot.rs` header says the same. |
| +- So no new op is needed. `CRDTReplica.persistentState()` is `snapshot`. |
| +- **Pin the property.** Port |
| + `a_snapshot_with_a_parked_update_heals_once_its_dependency_arrives` |
| + (`core/tests/snapshot.rs:121`) into `native/crdt/tests`. It then runs |
| + against the vendored crate and its backports, not only the server's copy. |
| **When the replica is written** |
| - The writer is a `CoalescedRunner` (`RastrilloNative/CoalescedRunner.swift`). |
| @@ -565,27 +715,52 @@ with queued edits, always comes back to `needsSignIn`. |
| `refresh` (`DocsModel.swift:326-339`), on window close, and on |
| `scenePhase == .background`. |
| -**Invariant I1 (load-bearing).** *Every local change contained in a persisted |
| -replica is either acknowledged by the server or present in the outbox.* |
| -- The server exposes no state vector, and a whole-document POST is capped at |
| - 1 MiB (`handlers.go:31`). A replica that holds an edit the outbox has lost |
| - can therefore never be reconciled. That edit would stay on this device |
| +**Invariant I1 (load-bearing), in two halves.** |
| + |
| +*I1a. Every local change contained in a persisted replica is either |
| +acknowledged by the server or present in the outbox.* |
| +- The server exposes no state vector, so a replica holding an edit the outbox |
| + has lost can never be reconciled. That edit would stay on this device |
| forever. |
| -- I1 holds by construction: |
| - - `drainEdits` awaits a durable `enqueue` before it takes the next command |
| - (replacing `pending.append`, `DocsModel.swift:185`); |
| - - the replica writer only snapshots on the main actor after that. |
| -- **Discard must delete the replica too.** `discardAll()` |
| - (`DocsModel.swift:242-254`) becomes: discard the lane, delete |
| - `<key>/replica`, and refetch. Otherwise discarded text would reappear |
| - locally without ever being sent. |
| - |
| -**Reopening** |
| -- `CRDTReplica.open(update:)` from the stored state. `fresh` already reserves |
| - every client id found in the state and never reuses one |
| +- **It does not hold by construction** *(H1)*. `CRDTReplica` is an actor |
| + (`CRDTReplica.swift:5`). `command` updates the actor's `lastGood` before |
| + returning (`:149`). `DocsModel.drainEdits` gets control back only after that, |
| + and only then calls the durable `enqueue`. Meanwhile the writer and |
| + `refresh → replica.apply` (`:121-127`) both read state on the replica actor. |
| + Either can capture a `lastGood` that contains a command whose entry is not |
| + yet on disk. A crash at that point strands the edit. |
| +- **Fix: a durability watermark.** |
| + - `CRDTReplica` counts `commandsApplied`. `persistentState()` returns |
| + `(state, sequence, commandsApplied)` captured together on the actor. |
| + - `DocsModel` counts `commandsDurable`, advanced only after `enqueue` |
| + returns. |
| + - The writer persists a capture only if its `commandsApplied <= commandsDurable` |
| + at the moment of writing. Otherwise it drops the capture and re-arms. |
| + Durability only grows, so a capture that passes the check is safe. |
| + - Remote state applied by `refresh` needs no watermark. It is already on |
| + the server. |
| + |
| +*I1b. A reopened document is the persisted replica **plus** its lane* |
| +*(H2).* |
| +- The writer runs 2 s after the last publish. After a kill the replica is |
| + therefore legitimately older than the lane. |
| +- Reopening from the replica alone would show the last edits missing while |
| + offline. The person would retype them, and after the drain the document |
| + would hold that text twice. That is the one user-visible duplication path |
| + in the design. |
| +- **So reopening is:** `CRDTReplica.open(update: replica)`, then apply every |
| + entry of lane `doc:<key>` in seq order, then publish. Yjs integration is |
| + idempotent, so entries already in the state are no-ops. Drafts reopen the |
| + same way. |
| +- `fresh` reserves every client id found in the state and never reuses one |
| (`CRDTReplica.swift:39-50`), so restarts are safe. |
| - Undo history does not survive a relaunch. That is accepted. |
| +**Discard must delete the replica too.** `discardAll()` |
| +(`DocsModel.swift:242-254`) becomes: discard the lane, delete `<key>/replica`, |
| +and refetch. Otherwise discarded text would reappear from the replica without |
| +ever being sent. |
| + |
| ### 4.4 The unsent-update queue |
| - **`pending` becomes a view of lane `doc:<key>`.** |
| @@ -597,8 +772,9 @@ replica is either acknowledged by the server or present in the outbox.* |
| rename input. Only this blocks close. |
| - `hasUnsentChanges` means the lane is non-empty. This is status only. |
| - **Compaction.** When a lane passes 256 entries or 1 MiB, merge its head |
| - into ≤512 KiB entries with `outbox.replace`. A duplicate left by a crash is |
| - harmless for Yjs (§3.3). |
| + into ≤512 KiB entries with `outbox.replace`. The merged entry keeps the |
| + oldest seq, so lane order is unchanged *(M3)*. A duplicate left by a crash |
| + is harmless for Yjs (§3.3). |
| - **The cap becomes a disk cap and is visible.** Replace the 2 MiB in-memory |
| refusal (`:166-167`) with 32 MiB of unsent updates per document. The |
| command-queue cap stays. |
| @@ -611,16 +787,33 @@ replica is either acknowledged by the server or present in the outbox.* |
| - **Title.** `pendingTitle` is persisted in `meta` and sent before updates, |
| as `flush` does today (`:267-275`). Last write wins, as on the server. |
| +**Why a first 400 is not terminal** *(H3)*. `updateIsClientFault` counts |
| +`core.ErrUpdateLogFull` as a client fault (`internal/docs/handlers.go:928-933`). |
| +It is therefore answered 400 with the same `docs.error.badupdate` key as a |
| +malformed update. But for a document whose handle has reached |
| +`core.MaxRetainedUpdates` (4096, `internal/core/core.go:141-146`), |
| +`Store.ApplyUpdate` writes the snapshot first and *expects* that POST to fail |
| +and the client's retry to succeed. The comment above `h.Apply` in |
| +`store.go` (about lines 801-816) says so. A long offline replay into a busy |
| +document will meet exactly this case. |
| + |
| +**Server change (small):** give `ErrUpdateLogFull` its own answer, 503 with |
| +`Retry-After: 1`, so the kit simply backs off. Test: a log at the cap, then a |
| +POST, then a retry, which must return 204. The client-side first-400 rule |
| +above stays as a defence while older servers are deployed. |
| + |
| **How each server response is handled** (`send` for lane `doc:*`): |
| | Server answer | Today | Proposed | |
| |---|---|---| |
| | 204 | `pending.removeFirst(count)` | `.sent` for the merged ids | |
| -| offline | `native.pending`, retry every 2 s | kit: `.offline`, wait for connectivity | |
| +| offline | `native.pending`, retry every 2 s | kit: `.offline`. Wait while the path is down; back off while it is up (§3.3, *M2*) | |
| | 401 / 303 | `refreshAccess` → `inputPaused`, `can_write=false` (`:298-299, 354-358`) | kit: **session ended** (§4.6). Input is *not* paused and `can_write` is untouched | |
| | 403, or 404 confirmed by `refreshAccess` | `inputPaused` | `.hold(.accessLost, blocksLane: true)` → §4.7 | |
| -| 400 (`docs.error.badupdate`) | retried forever | `.hold(.rejected, blocksLane: true)` | |
| -| 413 (a single update >1 MiB) | retried forever | `.hold(.tooLarge, blocksLane: true)` | |
| +| 400 (`docs.error.badupdate`), first time for these bytes | retried forever | **`.retry` after a `?since=` catch-up** *(H3)*, not a hold | |
| +| 400 again for the same bytes after a successful catch-up | retried forever | `.hold(.rejected)` | |
| +| 503 + `Retry-After` for a full update log (after the server change below) | (400 today) | kit backoff | |
| +| 413 (a single update over the cap, 8 MiB after d3) | retried forever | `.hold(.tooLarge)`. It is practically unreachable, because the whole document GET is capped at the same 8 MiB (`handlers.go:638`) | |
| | 5xx, 503 `errTeamUnavailable` (`store.go`) | retried | kit backoff | |
| ### 4.5 The list cache and launching offline |
| @@ -667,9 +860,13 @@ outbox lane is keyed by document, not by window. |
| - After a relaunch, `startCreation` resumes. `POST /documents` with the same |
| `creation_token` returns the same document (`internal/docs/store.go:287`, |
| migration 0008). |
| -- On attach, `outbox.retarget` moves the lane to `doc:<sqid>`, and the |
| - directory is renamed. |
| -- A crash between attach and rename is safe. The next launch re-POSTs the |
| +- **On attach, nothing moves** *(H5)*. The key stays `draft-<token>`, and so |
| + do the lane `doc:draft-<token>` and the directory. `meta.sqid` records the |
| + server identity, and `cache/docs/index` maps sqid → key, so opening that |
| + sqid from the list finds the local copy. Renaming the directory would break |
| + every sealed file in it, because the path is part of the associated data |
| + (§3.6). |
| +- A crash before `meta.sqid` is written is safe. The next launch re-POSTs the |
| same token and receives the same sqid. |
| ### 4.6 The dead end: an expired session with unsent edits |
| @@ -707,14 +904,18 @@ outbox lane is keyed by document, not by window. |
| A held lane shows `failed`, which is sticky. It offers: |
| - **Save as new document.** Create with a new `creation_token`, POST the |
| - local replica state as one update, and move to the new document. This is |
| - offered only when the state fits the 1 MiB update cap (`handlers.go:31`). |
| + local replica state as one update, and move to the new document. After d3 |
| + (authenticated `maxUpdateBytes` raised to 8 MiB) this always fits: any |
| + document the client could GET, capped at `maxLogBytes` (8 MiB, |
| + `handlers.go:638`), can be POSTed. |
| - **Copy text**, from the projection, which is always available. |
| - **Discard local changes**, which removes the lane and the replica, then |
| refetches. |
| -Splitting a large state across several updates is left out of v1. Raising |
| -the cap for native writers is small decision d3 (§10). |
| +**Server change for d3:** raise `maxUpdateBytes` (`handlers.go:31`, 1 MiB) |
| +to `maxLogBytes` (8 MiB) for authenticated writers. It is one constant, and it |
| +needs no client-side splitting code. It also removes the 413 hold for a big |
| +paste. |
| ### 4.8 Close, quit, and the iPhone |
| @@ -739,13 +940,13 @@ the cap for native writers is small decision d3 (§10). |
| | # | Slice | Size | Test | |
| |---|---|---|---| |
| -| K0 | Land #4; re-pin the apps to main; bump the server module | S | existing `make ci` in each repo | |
| +| K0 | Re-pin the apps to rastrillo/native main (#4 has landed); bump the server module | S | existing `make ci` in each repo | |
| | K1 | Typed errors + `NativeTransport` + `RastrilloTestSupport` (§3.1). Docs takes a transport supplier | S | `ScriptedTransport` for each case; a real `NativeHTTP` pointed at a closed port → `.offline` | |
| | K2 | `NativeConnectivity` + `Backoff`; Docs `listen` and Sheets `watch` wait on them; offline banner | S | injected path monitor unit tests; Network Link Conditioner "100% Loss"; Wi-Fi off; airplane mode on a device | |
| -| K3 | `NativeSealedStore` + `Outbox` + `OutboxDrainer` + `SyncStatus` and views; Docs lane, replica persistence (§4.3–4.4), open from replica | M | kill -9 mid-typing, relaunch offline, reconnect: server text equals local text. A property test of I1 over random edit/ack/crash sequences. Tampered or renamed files fail to open | |
| +| K3 | `NativeSealedStore` + `KeyProvider` + `Outbox` + `OutboxDrainer` + `SyncStatus` and views; Docs lane, durability watermark, replica persistence, reopen as replica + lane (§4.3–4.4); port the parked-update test to native/crdt | M | the §3.8 property test (I1a, I1b, convergence) with `FaultingBlobStore` and the integrating fake; `KeyProvider` unavailable-versus-lost cases; tampered, renamed or cross-account files fail to open; then a real kill -9 mid-typing | |
| | K4 | List cache, offline launch, drafts across relaunch (§4.5) | S–M | open, stop the server, relaunch, reopen; create offline → exactly one server document | |
| | K5 | `reauthenticate`, subject guard, warn-then-wipe sign-out; §4.6 dead end removed | S (+ server module) | delete the session row server-side, keep editing, see `needsSignIn(n)`, sign in, see the drain, same account id. Sign in as another person → refused | |
| -| K6 | Holds and Save as new; relaxed close guard; iOS background task | M | force 400, 413 and a revoked grant with `ScriptedTransport`, then against a real server | |
| +| K6 | Holds and Save as new; first-400 retry rule; relaxed close guard; iOS background task. Server: `ErrUpdateLogFull` → 503, `maxUpdateBytes` → 8 MiB (d3) | M (+ S server) | force 400 twice, a log at the cap, and a revoked grant with `ScriptedTransport`, then against a real server. Go test: log at cap → POST → retry → 204 | |
| | K7 | Image disk tier; LRU eviction of `cache/` (never evicting a document with a non-empty lane); "Remove offline copies" | S | eviction unit tests | |
| Optional, not needed for correctness: |
| @@ -774,10 +975,22 @@ way CRDT deltas do. |
| - Sealed: the list, plus one tab snapshot per tab (the whole grid window |
| ≤500×200, its version, formats and sizes). Reconnect refetches whole, as |
| today. |
| -- Lane `tab:<id>`, with **one entry per cell**: `{r, c, raw, base, fromR?, fromC?}`. |
| - - Coalescing keeps the *first* base and the *last* raw. |
| - - This is required, not an optimisation: `SetBatch` rejects a batch that |
| - names a position twice (`store/handle_write_batch.go:23-29`). |
| +- Lane `tab:<id>`, with **one *sendable* entry per cell**: `{r, c, raw, base, fromR?, fromC?}`. |
| + - This slice adds `Outbox.update(id, payload:)` to the kit (§3.3). |
| + - A later edit to a cell whose entry is still sendable updates that entry: |
| + it keeps the *first* base and takes the *last* raw. |
| + - **An entry in flight is frozen** *(M4)*. Otherwise: a batch commits, its |
| + response is lost (`.offline`), and the person edits B7 again offline. The |
| + coalesced entry would be `{base: old, raw: newer}`, while the server holds |
| + the committed middle value, which equals neither. The person's own write |
| + would come back as a conflict. |
| + - So a later edit to a cell with an in-flight entry makes a **new** entry |
| + whose `base` is the in-flight entry's `raw`. |
| + - `update` refuses an in-flight id. |
| + - `send` packs at most one entry per position into a batch. That is |
| + required, not an optimisation: `SetBatch` rejects a batch that names a |
| + position twice (`store/handle_write_batch.go:23-29`). The second entry |
| + goes in the next batch, after the first is acknowledged. |
| - Format and size entries are last-writer-wins and carry no base. |
| - A pending overlay paints queued raws over the server cells. Dependents are |
| marked stale, because formulas evaluate only on the server. |
| @@ -786,11 +999,15 @@ way CRDT deltas do. |
| **Base-value rule (server, about 30 lines in `SetBatch`)** |
| - Each cell write gains an optional `base`: the stored raw the client last |
| saw. |
| -- Under `h.mu`, `current := h.rawText[key]`. That value is already read as |
| - `prevRaw` (`store/handle_report.go:116`). Then: |
| +- Compare inside the transaction, against the **staged** handle |
| + (`staged.rawText`, `store/handle_write_batch.go:39-74`), not `h.rawText` |
| + before it *(M7)*. The staged copy is what the batch is actually writing |
| + over. That value is the one `SetReport` reads as `prevRaw` |
| + (`store/handle_report.go:116`). Then: |
| - `current == base`: apply. |
| - - `current == raw`: already applied, so succeed without stamping. This |
| - makes replay idempotent with no key. |
| + - `current == raw`: already applied, so succeed without stamping and |
| + **without calling `SetReport`**. This makes replay idempotent with no |
| + key. |
| - Otherwise: conflict. |
| - Compare the *translated* raw (`translateCopied`, |
| `handlers_cell_batch.go:48`) so paste provenance replays correctly. |
| @@ -805,12 +1022,24 @@ way CRDT deltas do. |
| `conflicts: [{r, c, base, current, display, mine}]`. |
| - **Non-conflicting cells are applied** (proposed; small decision d2). One |
| conflicted cell must not hold 499 good ones hostage. |
| -- `stamps` counts applied cells only. The b9a92e3 contract |
| - (`version > held + max(stamps, 1)` means a missed edit) then holds |
| - unchanged for partial batches. |
| -- A batch in which every cell conflicts is answered like a stampless |
| - response. |
| -- The applied subset is published on SSE as today. |
| +- `stamps` counts cells actually stamped. Today it is `len(writes)` |
| + (`store/handle_write_batch.go:95`), so it must become a count of |
| + `SetReport` calls, or no-ops and conflicts would over-count *(M7)*. With |
| + that change the b9a92e3 contract (`version > held + max(stamps, 1)` means a |
| + missed edit) holds unchanged for partial batches. |
| +- A batch in which nothing was stamped is answered like a format response: |
| + the version is the next stamp, and `stamps` is absent, so the slack is 1. |
| +- The applied subset is published on SSE as today. `handleCellBatchPost` |
| + publishes the same `gridResponse` it returns (`handlers_cell_batch.go:61-64`), |
| + so the rule holds for the SSE copy too. |
| + |
| +**Replay order** *(M7)*. After an offline period, other devices have written. |
| +Every drained batch's `version` would then exceed `held + stamps`, and each |
| +response would trigger the 25×50 chunked refetch. So on reconnect: |
| +1. refetch once first (the SSE `onOpen` snapshot already does this, |
| + `SheetLive.swift:13-16, 31-36`); |
| +2. keep the local raws painted through the pending overlay; |
| +3. then drain. |
| - Extend `internal/sheets/version_contract_test.go` with partial batches. |
| - **Client handling.** Applied cells return `.sent`. Conflicted cells return |
| `.hold(.conflict(…), blocksLane: false)`, so the rest of the lane keeps |
| @@ -861,9 +1090,10 @@ Memoria captures offline and reads online. |
| `id = client_id` and payload `{title, body}`, then closes the sheet: "Saved |
| on this device. Will be sent when you're online." |
| - Replay is idempotent by construction. |
| -- **Editing a pending note** updates the same entry. On the server, a repeated |
| - `client_id` revises the note, so even an edit made after a lost response is |
| - correct. |
| +- **Editing a pending note** updates the same entry, using the `update` that |
| + Sheets adds to the kit, so Memoria adopts after Sheets. On the server, a |
| + repeated `client_id` revises the note, so even an edit made after a lost |
| + response is correct. |
| - **Server limits are checked before enqueueing** (title ≤512 B, body ≤2 MiB, |
| non-empty: `native.go:106`), so a 400 never becomes a poison entry. |
| - `ErrIndexLag` (`:123`) counts as `.sent` with a "not searchable yet" |
| @@ -894,7 +1124,6 @@ Size S. |
| - Accept a client `uid` on create, idempotent through `uid_hash`. |
| - `DELETE /api/native/events/{id}`, idempotent and 409 on a newer sequence. |
| There is no route today (`routes.go:112-117`). |
| -- Expose reminder offsets. |
| **Client** |
| - A sealed snapshot of −30 to +90 days of *expanded* occurrences. Recurrence |
| @@ -917,10 +1146,12 @@ Size S. |
| calendar". |
| - **Online only:** quick add (the parser is on the server), series edits, |
| booked events, attendees, sharing and settings. |
| -- **Reminders** are server-side email only today (`models_event.go:165-167`). |
| - The device model is D6. |
| +- **Reminders stay server-side email** (`models_event.go:165-167`), which an |
| + offline device does not affect. Device notifications are cut from this spec |
| + into their own (D6). They are not offline work, they are L-sized, and they |
| + are the only new product surface in this document. |
| -Size M, or L with device notifications. |
| +Size M. |
| ## 8. Calls: graceful offline states |
| @@ -947,13 +1178,22 @@ quickly. |
| of a bridge error. |
| - **Separate from offline, but an offline launch cannot get past the lobby |
| without it.** Calls keeps cookies in memory only (`CallsModel.swift:34`), |
| - so every launch needs a browser sign-in. Adopting `NativeConnection` needs |
| - two things: |
| - - `/native/exchange` on the Calls server; |
| - - "Calls" added to the server module's callback allowlist, which names |
| - "Meet" today (#4). |
| - |
| - Re-pin Calls from `4c61d62` at the same time. |
| + so every launch needs a browser sign-in. *(L3)* |
| + - The Calls server already has its own `/native/request` and |
| + `/native/exchange` (`internal/calls/app.go:437-438`, |
| + `internal/calls/native_auth.go`). The exchange reply is |
| + `{name, value, target}` (`native_auth.go:173`), which |
| + `NativeSession` decodes, because `JSONDecoder` ignores `target`. |
| + - **The request does not match, though.** `/native/request` requires a |
| + `target` that passes `nativeTarget`: "/", `/m/{sqid}` or `/j/{token}` |
| + (`native_auth.go:67-80, 110`). `NativeConnection.signIn` sends |
| + `{challenge, callback}` and no target |
| + (`kit:NativeConnection.swift:135-136`), so today it would get a 400. |
| + - Two ways to adopt: `NativeConnection` gains an optional extra request |
| + field (`target: "/"`), or Calls moves to the shared server module. |
| + Either way, "Calls" goes into the module's callback allowlist, which |
| + still names "Meet" (`server/auth.go:254` on main). |
| + - Re-pin Calls from `4c61d62` at the same time. |
| Size S. |
| @@ -965,30 +1205,34 @@ Calendar → Calls → K7. |
| - The session-lifetime server change (§3.7) runs in parallel once D2 is |
| ruled. |
| +- Calendar device notifications are not in this order. They get their own |
| + spec (D6). |
| - None of these slices touches another app's branch. |
| ## 10. Decisions for Paul |
| -Each decision has a proposed default. The spec above is written to that |
| -default. |
| +These are all Paul's calls. Each one has a default, and **the spec above is |
| +written to that default**. Revision 2 adopts the reviewer's position on D2, D6 |
| +and d3. The rest are as first proposed, and the reviewer agreed with them. |
| -| # | Decision | Proposed default | |
| +| # | Decision | Default the spec is written to | |
| |---|---|---| |
| -| D1 | **On-device encryption of cached content** | **Encrypt**, with a per-account AES-GCM key in the Keychain (`WhenUnlockedThisDeviceOnly`, never synced). The key is wiped at sign-out, which crypto-shreds the cache. This applies uniformly to every app's cache and outbox (§3.6). | |
| -| D2 | **Session lifetime** | **Sliding on native use.** Sessions minted by `/native/exchange` renew to now + 30 days on use, with the row written at most daily. Browser sessions stay fixed at 30 days. Optional absolute ceiling: 1 year (§3.7). | |
| +| D1 | **On-device encryption of cached content** | **Encrypt**, with a per-account AES-256-GCM key in the Keychain (`WhenUnlockedThisDeviceOnly`, never synced). The key is wiped at sign-out, which crypto-shreds the cache. This applies uniformly to every app's cache and outbox. A locked device reads as *unavailable*, never as lost (§3.6). | |
| +| D2 | **Session lifetime** | **Sliding on native use, with a mandatory ceiling.** Sessions minted by `/native/exchange` renew to now + 30 days on use, with the row written at most daily, and never beyond `auth_time` + 180 days. Browser sessions stay fixed at 30 days. This is rastrillo work (§3.7). *(Revised: the ceiling was optional.)* | |
| | D3 | **Background sync on a locked iPhone** | **No.** No `BGTaskScheduler`, and the key and session stay `WhenUnlocked`. Queued changes send on the next unlock and open. `beginBackgroundTask` only finishes a send already in flight (§3.3). | |
| -| D4 | **Unsent items at sign-out** | **Warn, then wipe.** Try to send for 10 s. If anything is still unsent, confirm with "Delete N unsent changes?", offering Sign In Again when the session has ended. Then shred the key and delete. Switching accounts keeps everything (§3.5). | |
| +| D4 | **Unsent items at sign-out** | **Warn, then wipe.** Try to send for 10 s, skipped when offline or when the session has ended. If anything is still unsent, confirm with "Delete N unsent changes?", offering Sign In Again when the session has ended. Then shred the key and delete. Switching accounts keeps everything (§3.5). | |
| | D5 | **Sheets conflicts** | **Surface them.** Check the base value, answer 409 per cell, and show "theirs vs yours" with Keep mine or Keep theirs. Conflicts never block other edits (§5). | |
| -| D6 | **Calendar reminders model** | **Device notifications in addition to email.** Schedule local notifications from the cached window, nearest first within iOS's 64-notification limit, re-planned on each refresh and outbox change. Server email continues unchanged. Lock-screen text follows the system "Show Previews" setting. This needs the server to expose reminder offsets (§7). | |
| +| D6 | **Calendar reminders model** | **Email only in this spec.** Server email reminders continue unchanged. Device notifications (the 64-slot planning, re-planning on every change, the preview policy) move to a spec of their own (§7). *(Revised: they were in scope.)* | |
| | D7 | **Prefetch** | **None in v1.** Offline copies are only what this device has opened: Docs documents and Sheets tabs. Calendar's −30/+90 window is its normal fetch, not prefetch. Memoria has no read cache. Revisit with "keep the N most recent available offline" once the cache and eviction (K7) exist. | |
| -Smaller calls, where the spec also proposes a default: |
| +Smaller calls, also written to their defaults: |
| - **d1. Where the kit lives.** In rastrillo/native from the first cut, |
| marked in-progress until Sheets adopts it. The alternative, a literal |
| reading of "add a component only after proving adoption", is to build it |
| inside DocsKit and extract it at Sheets. |
| - **d2. Sheets mixed batches.** Apply the non-conflicting cells and return |
| 409 for the rest, rather than rejecting the whole batch. |
| -- **d3. Docs large updates.** Keep the 1 MiB update cap, and offer Save as |
| - new document only when the state fits. Raising the cap for native writers |
| - would remove that limit. |
| +- **d3. Docs large updates.** Raise `maxUpdateBytes` for authenticated writers |
| + from 1 MiB to `maxLogBytes` (8 MiB). A document the client may GET can then |
| + be POSTed, so Save as new document always fits, and a big paste never holds |
| + a lane (§4.7). *(Revised: the 1 MiB cap was kept.)* |