[ucguy4u]

Addresses Principal Architect review 5747379532 on top of 5746993924.

CI: run 41580399358 on e4195a0 — flutter, rust, worker success.

Backfill (BF1–BF6, BF1b)

Mutant / item	Test
BF1 / BF1b	Z13/BF1: member push…, BF1b: member revoke… (real members: {user-2})
BF2	BF2: revoke backfills owner id before DELETE runs (onBeforeDelete asserts user-1)
BF3	BF3: legacy push backfills stored cloud profile owner id
BF4	BF4: backfill does not overwrite an existing cloud profile owner id
BF5	BF5/BF6: owner select 500 leaves owner id unchanged
BF6	BF6: backfill uses identity_id filter when multiple rows visible
Swap	BF3 swap: backfill does not write after session swap
Non-JSON 200	backfill ignores non-JSON owner select body

Other blockers

Item	Test
X2b	X2b: revoke with null identityId…
X11 / X11b	profile_secrets_store_test.dart — migrate + clearCloudBackupCredentials
X12	identity_edit_revoke_widget_test.dart — X12: successful revoke clears recovery code…
Z4	Z4: refresh auth failure reads error_description without error_code
BF7	_strandedInstallRevokeMessage literal in Z5/Z6 tests
Fake hardening	fake POST profiles rejects owner_id mismatch with 403, fake profile_members rejects non-owner join with 403

O1/O2 unchanged.

  

[ucguy4u]

Changes requested and blocking — head baa8a0f. This closes out my block 5746993924 on #47 (merged); it does not clear it. Flagged for owner (human) review; this is a COMMENT review, not an approval.

Verdict. The production change is correct and safe. The tests do not pin it. The backfill, which is the headline change, has no effective test, and three of the items Amiro lists as closed (X2, X11, X12) still survive my mutants. I would not let this land as "closes 5746993924" until the fixes below are in. They are tests only; I found no production change to ask for.

1. Main vs #47 head. Main 3643ac0 and #47's head 4e00853 have the identical tree (0aa107b0c878beaac5bc1c0390510207c9859c78). The squash differs from the head in history only. #48 is one commit on 3643ac0 and touches 3 files: profile_secrets_store.dart (+6), supabase_profile_cloud.dart (+37), supabase_profile_cloud_http_test.dart (+390). There are no UI test, worker or migration changes.

2. Mutants against baa8a0f, full suite per mutant. I ran the suite on a stubbed avatar_renderer harness (no C toolchain on my box). The baseline is 240/240 and the worker is 21/21 with tsc --noEmit clean.

• Executed: 98 mutants. 79 killed, 19 survived.
• Not run: 3 stale patterns (R3, Y8, Y2 — see below). My earlier "89" counted R3 once as a stand-in for X13, so the distinct executed set is 88 plus the new ones.
• App set: 85 listed. 3 patterns no longer apply and were not run: R3 (stand-in for X13, which ran and was killed), Y8 (Y8b ran and was killed) and X2 (the revoke code moved; I re-expressed it as X2b). That leaves 82 run, 66 killed and 16 survived. That includes 8 new backfill/copy mutants (BF1, BF1b, BF2–BF7), all 8 of which survive.
• Extra app mutants: X2b and X11b, both survived.
• Worker: W1–W12, W2b and W7b — 14 run, 13 killed. W2 survives and is equivalent (W2b is the real form and is killed).

Survivors (19):

• Real, from the backfill, and not covered by any test: BF1 (write without the owner_id == session.userId check), BF1b (write session.userId whenever any row is visible), BF2 (backfill removed from revoke), BF3 (removed from push), BF4 (overwrites an existing owner id), BF5 (HTTP status ignored), BF6 (select not scoped to identity_id).
• Real, claimed closed but not: X2b (null id skips DELETE and select), X11 and X11b (legacy plaintext code file), X12 (recovery code stays on screen after revoke).
• Minor: Z4 (msg / error_description not read; the body test carries error_code, so the description path is unpinned) and BF7 (stranded copy reverted to the old text — the tests compare against the constant itself, so the wording is not pinned).
• Redundant or equivalent, as I classified them in earlier rounds: N9, N10, X14, Z10, Z11 (Z10 and Z11 are each redundant with the other, and the combined Z14 is killed), W2.
• Killed this time: Z2, Z3, Z5, Z6, Z7, Z8, Z9, Z12, Z13, Z14 (Z12/Z13 are Amiro's test names; as mutants Z12/Z13 are the member-join and claim writes).

3. Backfill safety, probed directly. I used a per-user-visibility fake and probed it separately from the suite. The code behaves as required:

• K1: a legacy owner device (no owner id stored) pushes and the owner id is written (u0).
• K2: a member device (row visible, owner is someone else) pushes with no write, and its revoke makes no write (it gets the "Only the device that created the backup…" message).
• K3: after a swap to a new anonymous user, push and revoke write nothing and the row is untouched.
• K4: with the select returning 500, state is unchanged; push is true and an owner revoke still deletes.
• With BF1b applied, the K2 probe writes the wrong id (owner), so a member test would kill BF1/BF1b. The suite just has no such test.
• Nit, non-blocking: a 2xx non-JSON body from the backfill select (K5, a captive portal or proxy page) throws out of the unguarded jsonDecode. Push then returns false after the update succeeded, and revoke fails before the DELETE is ever sent. It fails safe and a retry works, but a best-effort backfill should not be able to fail the operation it supports. Wrap it in a try/catch.

What the existing tests do and don't pin:

• legacy owner id backfills from server before revoke DELETE only asserts that revoke succeeds. An owner can delete under RLS without any backfill, so it passes with the backfill removed (BF2). It never asserts the stored owner id.
• Z13: member push … does not use a member. user-2 is seeded as a non-member, so the push goes through insert → 409 → join, not through the update and backfill path. It does kill the member-join mutant (Z12), but it never reaches the backfill.
• X11: the test pushes after writing the legacy file, and the push's own recoveryCode() migration deletes the file before revoke runs. It would pass whether or not clearCloudBackupCredentials removed it (X11b). The stale-file branch in migrateFromLegacyPlaintext (X11) is not pinned either.
• X12: no UI test changed in this PR, and identity_edit_revoke_widget_test does not assert that the recovery code is cleared.
• X2: the test passes a non-null identityId, so the null-id branch is untested.

Tests I need before this closes (no production change):

1. Owner device, no owner id → push → assert the stored owner id equals the session user (kills BF3/BF5). Same via revoke, asserting before the DELETE (kills BF2).
2. A real member (seeded in members) → push and revoke → owner id stays null (kills BF1/BF1b).
3. Swap to a different user → no write.
4. Select returns 500 → state unchanged.
5. A second identity row visible → backfill reads the right one (BF6).
6. A pre-set owner id is not overwritten (BF4).
7. X11: pre-seed the secure code and the legacy file, then run the migration. X12: a widget assertion. X2: revoke with a null identityId.
8. Optionally pin the stranded copy in a literal; the tests currently compare it to its own constant.

4. RLS-scoped fake vs the real migrations. It now models the real behaviour where the tests need it:

• GET returns only rows the caller owns or is a member of (matches profiles_select_member after 20261016182000).
• PATCH applies to owner or member (matches profiles_update_member).
• DELETE is owner-only (matches 20261102100000), with Prefer: return=representation.
• The owner row selected by select=owner_id is available through the table-wide select grant in 20261016181000.
• A duplicate identity insert returns 409.

The gaps, none of which block:

• POST /profiles does not enforce owner_id = auth.uid() (profiles_insert_owner).
• POST /profile_members accepts any caller, but the real members_insert_owner needs user_id = auth.uid() and the profile owner. The fake lets a non-owner join; production returns 403.
• members_select_self is not modelled.
• Column grants and the join_profile minimum code length are not modelled.

5. Stranded-install copy. "This device cannot remove the cloud backup copy. Use the phone that turned backup on, or request deletion by email as described in the privacy policy." The email route exists (privacy-policy.md and .html both say to email coffee.devloper@gmail.com to remove a cloud profile backup), and a member or legacy device really cannot delete, so the claim is accurate. It no longer promises a restore path that does not exist. One nit: after a swap on the original phone, "the phone that turned backup on" is this phone, so only the email route helps. The tests only compare the string with its own constant, so the wording is unpinned (BF7).

6. CI, checked by me on the exact SHA. Run 41575978471 on baa8a0f, event pull_request, completed with success. All three jobs have every step success and none is skipped, queued or cancelled:

• flutter: checkout, native deps for thermion_dart, flutter-action, melos bootstrap, melos run analyze and melos run test.
• worker: npm ci, npm test and npx --no-install tsc --noEmit.
• rust: cargo test and cargo clippy -- -D warnings.
  CI cannot catch the test gaps above, because those tests pass.

O1 (a recovery-code member device cannot turn backup off) and O2 (a delete_profile(code) RPC) stay with the owner and do not block this.