From ab29f6b12f922f0f0a2ef2c8957a21c0a1f14051 Mon Sep 17 00:00:00 2001 From: null Date: Thu, 25 Jun 2026 23:58:37 -0500 Subject: [PATCH] =?UTF-8?q?fix(outcomes):=20restore=20Your=20Progress=20re?= =?UTF-8?q?ad=20=E2=80=94=20scope=20query=20to=20allowed=20dayKeys=20+=20c?= =?UTF-8?q?oerce=20Long=20scores=20(I-001,=20I-002)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I-001: getOutcomes() did a bare collection list .get() on couples/{cid}/outcomes, which firestore.rules denies (reads allowed only for dayKey in day_0/30/60/90) -> always PERMISSION_DENIED, swallowed to emptyList(). Now scopes the query with whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS) so it satisfies the rule. I-002 (found while fixing I-001): toOutcomeScores() cast values to Map, but Firestore returns integer fields as Long on Android -> ClassCastException -> scores dropped (same shape submitOutcomeCallable writes, so the real path was broken too). Now coerces (value as? Number)?.toInt(). Verified live: 0 outcomes PERMISSION_DENIED after relaunch; seeded a day_0 baseline (int64) -> "Your Progress" shows "Baseline recorded" (was "No baseline yet"). Seed removed, couple baseline restored (0 outcomes, 0 active sessions). Both pending one re-QA confirmation round before pruning. Co-Authored-By: Claude Opus 4.8 --- ClaudeQACoverage.md | 2 +- ClaudeReport.md | 17 ++++++++----- .../data/remote/FirestoreOutcomeDataSource.kt | 24 ++++++++++++++----- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/ClaudeQACoverage.md b/ClaudeQACoverage.md index 93b4303c..d78f2e89 100644 --- a/ClaudeQACoverage.md +++ b/ClaudeQACoverage.md @@ -91,7 +91,7 @@ Route smoke-test checklist (re-runnable: `dumpsys gfxinfo closer.app reset` → - **Caching / lazy-load:** LazyColumn/Row/Grid in 17 files; Coil (AsyncImage) in 11; Room DAOs cache static question/category data locally — all in place, no load-all anti-patterns seen. - **Leak check:** conversation open/close ×6 → ViewRootImpl=1, Activities=1, Views +2, PSS bounded after trim → no window/Activity/listener leak. - **Redundant reads:** precise per-read counts need an instrumented/Perfetto build (Firestore success reads aren't in adb logcat); no failing-read spam **except I-001**; no leaked listeners. -- **Finding: I-001 (P1)** — `getOutcomes()` bare-list query is rules-denied → "Your Progress"/outcomes silently broken (see ClaudeReport.md). +- **Finding: I-001 (P1) — FIXED+VERIFIED** `getOutcomes()` bare-list query was rules-denied → fixed with `whereIn(documentId, dayKeys)`; 0 PERMISSION_DENIED after. **I-002 (P1) — FIXED+VERIFIED** (found fixing I-001): scores stored as int64 → read as Long → `Map` cast CCE → swallowed; fixed via `Number.toInt()`. Live: seeded day_0 → "Your Progress" shows "Baseline recorded". Both pending Round-9 confirm. ## Pass J — Accessibility (R8, emulator-5554) - **Font scaling (font_scale 2.0, worst case):** Home, Paywall, Settings all **reflow + scroll, no clipped/hidden buttons** — meets the acceptance bar. Minor: long subtitles/email ellipsize, bottom-nav labels wrap ("Mess ages"). Restored to 1.0. ✅ diff --git a/ClaudeReport.md b/ClaudeReport.md index 8a50e40a..69e4729f 100644 --- a/ClaudeReport.md +++ b/ClaudeReport.md @@ -1,12 +1,12 @@ # Claude QA Report — Full-App QA (living report) -> **Verdict (2026-06-25, R8): 1 open P1 — I-001 ("Your Progress" outcomes read is rules-denied → feature silently broken). All else clean; security cornerstone clean.** +> **Verdict (2026-06-25, R8): 0 open P0–P2 (1 P3 J-OBS, non-blocking). I-001 + I-002 (outcomes read) fixed + verified live. Security cornerstone clean. At the flawless bar.** > > This report shows **current state only**. Fixed issues live here for **one** confirmation round, then they're pruned > to the archived-ID line below (full detail stays in git history). See **Report hygiene** in `ClaudeQAPlan.md`. ## Run-state (current) -`Round 8 (re-QA + Passes I/J) — IN PROGRESS | 1 open P1 (I-001) + 1 P3 (J-OBS) | Passes I+J done | NEXT ACTION: fix phase — I-001 (build+verify), then re-QA confirm.` +`Round 8 — fix phase DONE | 0 open P0–P2 (1 P3 J-OBS) | I-001+I-002 fixed+verified live | NEXT ACTION: optional deferred C/E/F coverage; Round 9 re-confirm + prune I-001/I-002.` - **Build:** client HEAD `23dd6a7`, Cloud Functions deployed. - **Devices / accounts:** emulator-5554 = QA (`Y05AKO2IlTPMa0JQW1BiNIM0uzK2`) · emulator-5556 = Sam (`imDjjO…`) · paired, coupleId `Xal3Kw3gjSdn0niERYKJ`, both free (baseline restored). - **Docs:** Playbook `ClaudeQAPlan.md` · Coverage `ClaudeQACoverage.md` · Ideas `Future.md` `## QA` · Branding `ClaudeBrandingReview.md`. @@ -15,15 +15,20 @@ | Severity | Open | Fixed (pending 1 confirm) | |---|---|---| | P0 | 0 | 0 | -| P1 | **1** | 0 | +| P1 | 0 | 2 (I-001, I-002) | | P2 | 0 | 0 | | P3 | **1** | 0 | ## Open issues | ID | Sev | Area | Description | Repro | Suggested fix | Status | |---|---|---|---|---|---|---| -| I-001 | **P1** | Outcomes / "Your Progress" read | `FirestoreOutcomeDataSource.getOutcomes()` (line 45-53) issues a **bare collection list** `.get()` on `couples/{cid}/outcomes`, but the rule (firestore.rules:658) only allows reading specific dayKey docs (`day_0/30/60/90`) and **denies list queries → always `PERMISSION_DENIED`**. `OutcomeRepositoryImpl.getOutcomes` (26-29) swallows it (records non-fatal → returns `emptyList()`). Net: recorded check-ins **never display** in Your Progress; Home/Settings reminder logic re-prompts for completed days; crashReporter spammed each load. Found via Pass I (perf) efficiency lens — masked from A/B/C because this couple has 0 outcomes + the failure is swallowed. | Open app → logcat: `Listen/Get for Query(couples/{cid}/outcomes …) failed: PERMISSION_DENIED` (live-confirmed R8). | Constrain the query to satisfy the rule: `.whereIn(FieldPath.documentId(), listOf("day_0","day_30","day_60","day_90")).get()` (or 4 parallel `getOutcome` gets). No rules change needed. | **Open** | -| J-OBS | P3 | A11y / touch targets | A few conversation icon-buttons measure **~42–45dp wide** (48dp tall) — single-axis marginal miss of the 48dp target; fully operable. Most controls are 48dp. | Pass J: uiautomator bounds on conversation → 2–3 clickables `<126px` wide. | Bump those icon-buttons to 48dp min (e.g. `Modifier.minimumInteractiveComponentSize()` / `size(48.dp)`). | **Open** | +| J-OBS | P3 | A11y / touch targets | A few conversation icon-buttons measure **~42–45dp wide** (48dp tall) — single-axis marginal miss of the 48dp target; fully operable. Most controls are 48dp. | Pass J: uiautomator bounds on conversation → 2–3 clickables `<126px` wide. | Bump those icon-buttons to 48dp min (e.g. `Modifier.minimumInteractiveComponentSize()` / `size(48.dp)`). | **Open (P3, non-blocking)** | + +## Fixed this round — pending one confirmation round (then prune) +| ID | Sev | Area | Fix | Status | +|---|---|---|---|---| +| I-001 | P1 | Outcomes read — query rules-denied | `getOutcomes()` now scopes the query with `.whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS)` (the 4 allowed dayKeys) so it satisfies firestore.rules:658 instead of issuing a denied bare-list. | **Fixed + verified live:** 0 `outcomes` PERMISSION_DENIED after relaunch (was firing every load). | +| I-002 | P1 | Outcomes read — score parse (found fixing I-001) | `toOutcomeScores()` now coerces `(value as? Number)?.toInt()` instead of casting to `Map` — Firestore returns integer fields as **Long** on Android → the hard Int cast threw CCE → scores swallowed to `emptyList()`. Same shape `submitOutcomeCallable` writes, so it broke the real path too. | **Fixed + verified live:** seeded a day_0 baseline (int64, == real callable shape) → "Your Progress" now shows **"Baseline recorded"** (was "No baseline yet"). Seed removed, baseline restored. | ## Resolved & confirmed (archived — full detail in git history) A-001 · A-003 · A-OBS · B-001 · B-002 · B-003 · B-004 · C-CC-001 · C-DS-001 · C-NAV-001 · D-001 · E-001 · E-002 · E-003 · E-OBS · F-OBS · F-RACE-001 — all fixed and re-verified (commits in history; **F-RACE-001** fixed `23dd6a7`, re-confirmed live R8: race → 1 session, loser joins same set). Pruned per the one-confirmation-round rule. (C-OBS / `outcomes` list / SubscriptionScreen per-user gate = investigated, **not bugs**.) @@ -35,7 +40,7 @@ A-001 · A-003 · A-OBS · B-001 · B-002 · B-003 · B-004 · C-CC-001 · C-DS- - **Robustness:** malformed/abusive deep-link intents (unknown type, missing extras, injection/path-traversal) → 0 crash; killed-state cold-start chat deep-link → conversation loads. ## Round history (one line each) -- **R8** (in progress) — F-RACE-001 re-confirmed live (race → 1 session; loser joins winner's same-set session via "Join the game") + pruned; running Passes I/J. +- **R8** — F-RACE-001 re-confirmed + pruned; Passes I (perf) + J (a11y) run; found+fixed+verified **I-001 & I-002** (outcomes read: query rules-denied + Long/Int parse CCE → "Your Progress" was silently dead). 0 open P0–P2. - **R7** — multi-angle security/concurrency deep dive → cornerstone fully clean; F-RACE-001 found + fixed + verified. 0 new open. - **R6** — branding drop + Future.md backlog regression (white-keyhole icons/loader/splash, inclusive gender, copy, rate-limit split, results-push suppression, paywall retry/offline) → 0 new open. - **R5** — Cloud Functions deployed (E-OBS channel fix, E-003 results routing) + new Pass G (account creation / fake-account abuse) clean → 0 open. diff --git a/app/src/main/java/app/closer/data/remote/FirestoreOutcomeDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreOutcomeDataSource.kt index e1e7a03c..8b3335d8 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreOutcomeDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreOutcomeDataSource.kt @@ -1,9 +1,11 @@ package app.closer.data.remote import app.closer.domain.model.Outcome +import app.closer.domain.model.OutcomeDay import app.closer.domain.model.OutcomeDayKey import app.closer.domain.model.OutcomeScores import com.google.firebase.firestore.DocumentSnapshot +import com.google.firebase.firestore.FieldPath import com.google.firebase.firestore.FirebaseFirestore import com.google.firebase.functions.FirebaseFunctions import kotlinx.coroutines.suspendCancellableCoroutine @@ -18,6 +20,11 @@ class FirestoreOutcomeDataSource @Inject constructor( private val db: FirebaseFirestore, private val functions: FirebaseFunctions ) { + private companion object { + // The only outcome doc ids the security rules allow reading. + val OUTCOME_DAY_KEYS: List = OutcomeDay.entries.map { it.key } + } + suspend fun submitOutcome(coupleId: String, dayKey: OutcomeDayKey, scores: OutcomeScores): Unit = suspendCancellableCoroutine { cont -> functions.getHttpsCallable("submitOutcomeCallable") @@ -43,10 +50,14 @@ class FirestoreOutcomeDataSource @Inject constructor( } suspend fun getOutcomes(coupleId: String): List { + // Security rules permit reading only the fixed dayKey docs and DENY an + // unconstrained list query, so the query must be scoped to those ids by + // document id — otherwise every read fails PERMISSION_DENIED (I-001). val snapshot = db .collection(FirestoreCollections.COUPLES) .document(coupleId) .collection(FirestoreCollections.OUTCOMES) + .whereIn(FieldPath.documentId(), OUTCOME_DAY_KEYS) .get() .await() return snapshot.documents.mapNotNull { it.toOutcome() } @@ -73,13 +84,14 @@ class FirestoreOutcomeDataSource @Inject constructor( ) } - @Suppress("UNCHECKED_CAST") private fun Map<*, *>.toOutcomeScores(): OutcomeScores? { - val map = this as? Map ?: return null - val connection = map["connection"] ?: return null - val communication = map["communication"] ?: return null - val intimacy = map["intimacy"] ?: return null - val happiness = map["happiness"] ?: return null + // Firestore returns integer fields as Long on Android, so coerce via Number + // rather than casting to Int (a hard Int cast threw CCE → scores dropped, I-002). + fun score(key: String): Int? = (this[key] as? Number)?.toInt() + val connection = score("connection") ?: return null + val communication = score("communication") ?: return null + val intimacy = score("intimacy") ?: return null + val happiness = score("happiness") ?: return null return runCatching { OutcomeScores(connection, communication, intimacy, happiness) } .getOrNull() }