146 lines
9.1 KiB
Markdown
146 lines
9.1 KiB
Markdown
# Engineering Reference Manual Review Plan — Phase 3 (2026-07-08)
|
|
|
|
**Status: ✅ COMPLETE** — all 13 batches reviewed, 11 issues found and fixed, committed individually, and pushed to `dev`.
|
|
|
|
Triggered by `_null` after the Phase 2 sync (`213cfddb`) + review pass (`d31b58d2`).
|
|
Goal: cross-check every claim in the manual against the live repo + `firestore.rules`
|
|
+ `functions/src/` + iOS source + Android source, batch by batch. No skipped sections.
|
|
|
|
## Approach
|
|
|
|
- **13 batches**, each scoped to one major section of the manual.
|
|
- **Evidence-first.** For every factual claim (file path, function name, collection
|
|
name, cron expression, preference field, threshold, AAD string), verify against
|
|
the live source.
|
|
- **One commit per batch** per the standing git discipline (one batch = one commit = one push).
|
|
- **Log findings to this file** as each batch completes.
|
|
- **Anchors + TOC** verified at the end (Batch 13), not per-batch, to avoid churn.
|
|
|
|
## Batches
|
|
|
|
### Batch 1 — Front matter + System overview + Repository layout (Android + iOS + Cloud Functions + Shared configuration)
|
|
- Read: `## How to use this document` through `### Shared configuration` (lines ~1-260).
|
|
- Verify all directory paths under `app/src/main/java/app/closer/`,
|
|
`functions/src/`, `iphone/Closer/`, `res/drawable-*`, Hilt module list.
|
|
- Cross-check Android package list against the live `find app/src/main/java/app/closer -maxdepth 3 -type d`.
|
|
- Cross-check iOS file list against `ls iphone/Closer/...`.
|
|
- Cross-check Cloud Functions tree against `find functions/src -name "*.ts"`.
|
|
- Flag missing or renamed directories/files.
|
|
|
|
### Batch 2 — Authentication + pairing + couples doc model + rate limit + recovery
|
|
- Read: `## Authentication and pairing flow` (lines ~270-370).
|
|
- Verify auth files in `data/remote/FirebaseAuthDataSource.kt`,
|
|
`data/repository/FirebaseAuthRepositoryImpl.kt`, `domain/repository/AuthRepository.kt`.
|
|
- Verify `couples/{coupleId}` field list against `acceptInviteCallable.ts` + the
|
|
domain `Couple.kt` model.
|
|
- Verify rate-limit env values against `AuthRateLimiter.kt`.
|
|
- Verify recovery phrase flow against `RecoveryKeyManager.kt` + `CoupleEncryptionManager.kt`.
|
|
|
|
### Batch 3 — E2EE model (versions, Argon2id, Tink, sealed-answer, ECIES, single-device, iOS bridge)
|
|
- Read: `## End-to-end encryption model` (lines ~370-475).
|
|
- Verify each `Encryption versions` row against `firestore.rules` regex helpers.
|
|
- Verify the Argon2id v1.3 parameter names + the AAD strings against the actual
|
|
Kotlin/Swift code.
|
|
- Verify the `keybox:v1:` wire format against `Keybox.swift` + `ReleaseKeyEncryptor.kt`.
|
|
- Verify the `wrapReleaseKeyCallable` path against the function source.
|
|
|
|
### Batch 4 — Daily question lifecycle + Reveal flow + Thread questions
|
|
- Read: `## Daily question lifecycle` (lines ~480-575).
|
|
- Verify `assignDailyQuestion` schedule + the deterministic picker
|
|
(DOW → mode map, daily_fun_mc exclusion) against
|
|
`functions/src/questions/assignDailyQuestion.ts` + `DailyModeResolver.kt`.
|
|
- Verify the schemaVersion 2 reveal flow + the read-gated `secure/payload` subdoc
|
|
against `firestore.rules` (the `isCoupleAnswerCreate` + `isEncryptedPayloadUpdate`
|
|
helpers).
|
|
- Verify schemaVersion 3 sealed flow against `SealedAnswerEncryptor.kt` +
|
|
`isSealedAnswerCreate` rules.
|
|
- Verify thread questions path against `isSealedThreadAnswerCreate` + the model.
|
|
|
|
### Batch 5 — Firestore security rules
|
|
- Read: `## Firestore security rules` (lines ~720-810).
|
|
- Line-by-line verify every per-collection note in the manual against `firestore.rules`.
|
|
- Cross-check helper-function names (`coupleEncryptionEnabled`, `isCiphertext`,
|
|
`isUpdatingCoupleRhythm`, `isUpdatingRecoveryWrap`, `isCoupleAnswerCreate`,
|
|
`isEncryptedPayloadUpdate`, `isSealedAnswerCreate`, `isSealedAnswerUpdate`,
|
|
`isSealedThreadAnswerCreate`, `isSealedThreadAnswerUpdate`).
|
|
- Note any rules in the file not documented, or documented rules not in file.
|
|
|
|
### Batch 6 — Cloud Functions
|
|
- Read: `## Cloud Functions` (lines ~815-845).
|
|
- Verify function triggers, handlers, schedules, module responsibilities.
|
|
- Cross-check `functions/src/` tree (already done in Batch 1 but verify exports).
|
|
- Cross-check Schedule table against each function's source.
|
|
|
|
### Batch 7 — Billing
|
|
- Read: `## Billing` (lines ~850-920).
|
|
- Verify RevenueCat integration files, entitlement checkers, webhook handler,
|
|
Purchases.logIn identity link.
|
|
- Cross-check the "Gated features" list against the actual ViewModels that
|
|
inject `CouplePremiumChecker` / `EntitlementChecker`.
|
|
|
|
### Batch 8 — Notifications
|
|
- Read: `## Notifications` (lines ~925-1020).
|
|
- Verify FCM setup + tokenRegistrar + quiet hours.
|
|
- Verify each reminder callable's actual schedule + dedupe.
|
|
- Verify game-push semantics against `onGameSessionUpdate.ts` + the
|
|
part-finished triggers.
|
|
- Verify the foreground banner surface + the deep-link routing table against
|
|
`PartnerNotificationManager.kt`.
|
|
|
|
### Batch 9 — iOS-specific notes
|
|
- Read: `## iOS-specific notes` (lines ~1023-1080).
|
|
- Verify Architecture, CloserTheme tokens, XcodeGen setup, build commands,
|
|
E2EE gap status, CryptoKit guidance.
|
|
- Cross-check `iphone/Closer/Theme/CloserTheme.swift` for the actual color values.
|
|
|
|
### Batch 10 — Build and release
|
|
- Read: `## Build and release` (lines ~1080-1200).
|
|
- Verify Android module/package, biometric recovery phrase reveal, required
|
|
build secrets, Gradle config, ProGuard rules.
|
|
- Cross-check the iOS build commands against the actual XcodeGen spec.
|
|
|
|
### Batch 11 — Engineering conventions
|
|
- Read: `## Engineering conventions` (lines ~1200-1280).
|
|
- Verify the architectural rules (Clean layers, package boundaries, what
|
|
lives where) against the actual code.
|
|
- Verify the "what NOT to do" list (e.g. raw Firestore listeners, no
|
|
`FirebaseAuth.getInstance()` in VMs) is still accurate.
|
|
- Check the version drift (build.gradle vs HISTORY.md) and ProGuard note.
|
|
|
|
### Batch 12 — Known landmines and recent fixes
|
|
- Read: `## Known landmines and recent fixes` (lines ~1280-1450).
|
|
- Verify EVERY landmine still applies:
|
|
- Does the file/component it cites still exist?
|
|
- Is the fix still in place?
|
|
- Is the "Re-introduction risk" description still accurate?
|
|
- Look for any landmine whose fix is now in a different file.
|
|
|
|
### Batch 13 — Where to look first
|
|
- Read: `## Where to look first` (lines ~1450-end).
|
|
- Verify every file path still exists.
|
|
- Verify the "what to read first for a new area" list still applies to the
|
|
current code.
|
|
|
|
### Final pass
|
|
- TOC integrity + every anchor resolves.
|
|
- One final commit if any TOC/anchor cleanup is needed.
|
|
|
|
## Phase 3 results (2026-07-08)
|
|
|
|
| Batch | Section | Issues found | Commit |
|
|
|---|---|---|---|
|
|
| 1 | Front matter + System overview + Repository layout | iOS platform row said "E2EE cross-compatibility not yet implemented" (contradicts the iOS E2EE section below) | `ef5a2331` |
|
|
| 2 | Auth + pairing + couples doc model + rate limit + recovery | Recovery phrase "256-word list" - actual list is 248 entries (`WORDLIST` array has 248; iOS wordlist is 247 lines = 248 entries with no trailing newline) | `1b36eba6` |
|
|
| 3 | E2EE model (versions, Argon2id, Tink, sealed-answer, ECIES) | Argon2id memory "46080 KiB" - actual constant is `46 * 1024 = 47104` KiB; iOS-side docstring was already correct | `0af6f24a` |
|
|
| 4 | Daily question lifecycle + Reveal flow + Thread questions | DOW mode table was partial (missed Saturday `mode_weekend_side_quest` - Side Quest Saturday) | `5e292e66` |
|
|
| 5 | Firestore security rules | Missing 3 helper functions (`otherCoupleMember`, `partnerReflectedDate`, `isPublicKey`); wrong regex for `isPublicKey`; releaseKeys per-collection enforcement said "readable only by recipient" but the rule also allows the sender (for the idempotency existence-check) | `5f7c785f` |
|
|
| 6 | Cloud Functions | Handler types table mixed onDocumentCreated / onDocumentUpdated / onDocumentWritten into one row; onEntitlementChanged was in a separate "Firestore onUpdate" row but is actually onDocumentWritten | `aa356148` |
|
|
| 7 | Billing | iOS "DefaultEntitlementChecker actor" doesn't exist (the real class is `BillingService`); gated-features list was wrong (4 of 7 items were not actually gated; chat-media gate was missing) | `ceb98f4a` |
|
|
| 8 | Notifications | "How to gate a new feature" sub-section got merged into the previous paragraph by Batch 7; game-push flag claim was wrong (no `notificationsSent` map - 4 separate Timestamp fields) | `29cc39aa` |
|
|
| 9 | iOS-specific notes | Tink template name "SHA128" was a typo - actual is `ECIES_P256_HKDF_HMAC_SHA256_AES128_GCM` | `9330c0c4` |
|
|
| 10 | Build and release | RevenueCat version 8.20.0 (stale from v0.2.1) - actual is 10.12.0; iOS purchases-ios 5.x; Glance 1.1.1 missing from the Gradle dependency list | `22c816ad` |
|
|
| 11 | Engineering conventions | (no issues - every claim verified) | `b3725463` |
|
|
| 12 | Known landmines and recent fixes | A-201 landmine heading (`### A-201 - ...`) got dropped during the Phase 2 sync - the body content was there but visually merged with the previous landmine's tail | `80751f82` |
|
|
| 13 | Where to look first | (no issues - all 16 file paths resolve, acceptInviteCallable encryptionVersion=2 verified) | `c5ead4cb` |
|
|
| Final | TOC + anchor integrity | 30/30 anchors resolve, 0 broken TOC links | (this commit) |
|