Repository navigation
docs: weekly drift pass — sharing, store, and QA docs - #44
Conversation
Co-authored-by: Uday Chauhan <chauhan.s.uday26@gmail.com>
ucguy4u
left a comment
ucguy4u
left a comment
There was a problem hiding this comment.
Architect comments clear on this head — not an approval
Reviewed head 016425e5451f129e70ceb07a3b57f9da98820844 (base main @ 5eeb347c). No earlier architect review exists on this PR, so nothing is superseded. No blocking comments.
1. Scope: docs and comments only. Confirmed.
- 9 files: 8
.mdpluspackages/qr/lib/src/qr_generator.dart. qr_generator.dart: the only change is two///doc-comment lines replacing one.git diff -wshows the same hunk, with no code, signature, import or code-whitespace change.buildQrWidget(String data, {double size = 240})is untouched.
2. Claims checked against main @ 5eeb347c:
- Store.
app/lib/store/store_providers.dartdefaultsentitlementStoreProvidertoRevenueCatEntitlementStore, andmain.dartdeliberately does not override it.grantgoes throughPurchases.purchase, and ownership comes frominfo.entitlements.active.InMemoryEntitlementStoreis only referenced in a test-oriented doc comment, and nothing else grants entitlements. The 11 catalog items and 3 paid Founders items (riviera_optics,long_flow,full_beard) matchcosmetic_listing.dart. The TODOS #6 "see #15" pointer resolves to TODOS §15. "Real Play Billing on Android": iOS is a no-op and refuses purchases (_ensureConfigured); the docs say Android and don't overclaim iOS. - Tabs.
app/lib/app.darthas 5NavigationDestinations: Identity, Avatar, Store, Share, Passport. The README label order matches. - Sharing.
amiro://encounter?d=…[&c=…], with a 2000-character cap, comes fromencodeEncounterUri/decodeEncounterUriinencounter_qr.dart.schemaVersion, equipped cosmetics and collection completion are realAmiroSharingPayloadfields. The legacyamiro://sharecodec isbuildShareUri/parseShareUri, andSharedProfilehas the fields the doc lists. Onlyown_encounter.dart/share_screen.dartbuild links for new shares (encounter), andbuildShareUrihas no app caller.resolveScannedTexthandles encounter and legacy links for QR, NFC and deep link, andscreenForScanroutes legacy links toSharedProfileScreen. - NFC.
NfcReader/ManagerNfcReaderread the firstamiro://link viaamiroLinkTextFromNdef, with strict UTF-8/UTF-16.NfcEmulator/AndroidNfcEmulatoruseMethodChannel('amiro/nfc_hce')→AmiroHceService(Kotlin).NoopNfcEmulatoris used off Android, andshare_screen.dartgates oncanEmulate. "Implemented but not fully device-verified" matches overview §11 and TODOS PR7-9. - QR.
buildQrWidgetis exported fromamiro_qr.dart. The package depends onqr_flutteronly, so "does not import app/ or sharing" is true. Scanning is inapp/lib/sharing/qr_scan_screen.dart(mobile_scanner). - identity-link.md. It matches the code. The Android manifest has the
encounterhost, andid.amiro.appappears nowhere in code, so "not built yet" is accurate. - Discovery / #43.
packages/discoveryis a ledger (EncounterRecord,EncounterProcessor,FileDiscoveryRepository), and the Passport UI is inapp/lib/discovery/. The Supabase profile backup claim matchesSupabaseProfileCloudinmain.dart(#43). - TODOS.md. Nothing undone is marked done. The edits remove the stale "stubbed" claims, and the new PR4 note matches PR7 cutover and
scan_result.dart. - test-plan.md. Every listed suite exists:
packages/{store,sharing,nfc,qr,discovery}/testandapp/test/{sharing,discovery,store,theme}. The RevenueCat tests mock thepurchases_flutterchannel. The "not covered" list (two-phone NFC, live Play, iOS, live Supabase, web fallback) is honest. - DESIGN.md. Untouched by the PR, and nothing contradicts it.
Non-blocking, worth fixing in this PR or a follow-up:
docs/architecture/overview.md:22-23. The snapshot limits "stub" corrections to "store or sharing packages". The same file still says NFC/QR are stubs or non-goals (lines 45, 51, 281), and thatidentity-link.mdandtest-plan.mdare "stub — not started" (lines 79, 83, 235). This PR rewrites those two files, so the contradiction is now inside the PR's own scope. Widen the sentence to cover nfc, qr, identity-link.md and test-plan.md. The "Sections 1–10 are history" banner mostly covers it already.docs/architecture/overview.md:22points to "each package's README.md", butdiscovery(andidentity_core,avatar_core,avatar_renderer) have no README. Say "each package that has one", or add a discovery README later.docs/protocols/identity-link.md:42. Theapp_linksstream lives inapp/lib/sharing/sharing_providers.dart(incomingShareLinkProvider).incoming_share_listener.dartonly consumes it. Cite both.TODOS.md:38still says "7 items don't need it", but the catalog has 11.TODOS.md:144says "The app itself never reads Supabase", which was true for entitlements but is now misleading, becausemain.dartpulls the profile from Supabase (#43). Neither is touched by this PR and both are pre-existing, so this is optional.
Checks: not green. The rust check passes. The flutter check (run 41389095993) fails on 2 tests in app/test/store/store_screen_test.dart ("lists every catalog item with buy affordance or owned state" and "series items render after unseriesed items, under their header"). main @ 5eeb347c fails the same two tests (run 40260955072), so a docs-only diff did not cause it. It still means the PR has no green CI, and test-plan.md says tests run "on CI" while CI is red. Don't merge on a docs-only argument. Someone needs to fix or acknowledge the main failure first.
Split: no. This is one coherent docs-only fix: every file corrects the same stale "stub / stubbed purchases / not started" drift, and the README, package READMEs and identity-link doc cross-reference each other. Splitting would leave inconsistent docs in between. At +137/−65 and no code, it is reviewable as is.
This is a COMMENT review, not an approval. I have not merged, marked ready or pushed anything.
Clarify foundation-era stub wording in overview.md, fix app_links reference to sharing_providers.dart, and align TODOS catalog count and Supabase profile backup wording with #43. Co-authored-by: Uday Chauhan <chauhan.s.uday26@gmail.com>
ucguy4u
left a comment
ucguy4u
left a comment
There was a problem hiding this comment.
Architect comments clear on this head — not an approval
Re-reviewed head 15ebd634581e88b8a2df5d813219ce0607e79049. This supersedes my earlier review 5732139991 at 016425e5 (also a clear COMMENT). No blocking comments.
Delta 016425e5..15ebd634: docs only. Confirmed.
- 3 files, all
.md:TODOS.md,docs/architecture/overview.md,docs/protocols/identity-link.md. - No code and no test files changed.
app/test/store/store_screen_test.dartis untouched, and so isqr_generator.dart(still the same doc-comment-only change as before).
My four nits, each verified against the code:
- overview.md stub wording and tree labels: fixed and accurate.
- §1, §2, §7 and §10 now say "at foundation time" and "were stubs".
- The tree labels read "sharing/nfc/qr/store: foundation: stubs; now implemented", "identity-link.md: encounter + legacy share links (implemented)" and "test-plan.md: CI coverage + manual gaps".
- §10 correctly keeps HTTPS App Links /
id.amiro.appand server-authoritative scarcity as deferred. Noid.amiro.appexists in code. - The snapshot note is now scoped to "§1–§10 describe foundation-era stubs".
grepfinds no remaining unqualified "stub" claim in the docs outsideasset-policy.md, which this PR doesn't touch.
- README pointer: fixed and accurate. It now names
packages/sharing,nfc,qrandstore. Those are exactly the fourpackages/*/README.mdfiles that exist, and the note that the other packages have none is true. - identity-link.md:42: fixed and accurate.
incomingShareLinkProviderexists atapp/lib/sharing/sharing_providers.dart:12and wrapsAppLinks().uriLinkStreamthroughresolveScannedText.incoming_share_listener.dartis the UI consumer. - TODOS.md: fixed and accurate.
- "11 catalog items" matches
cosmeticCatalog. - The §15 wording matches the code. Purchases and entitlements still come only from the RevenueCat SDK, and #43 (
2f9d6c8) addedSupabaseProfileCloud(app/lib/identity/supabase_profile_cloud.dart). It uses an anonymous Supabase session with an RLS-scoped profile row, andmain.dartwires it up. The migration20261016180000_create_profiles.sqlenables RLS onprofilesandprofile_members. The webhook Worker's service-role writes stay server-side.
- "11 catalog items" matches
New overclaims: none found. Nothing else in the delta asserts new behavior.
Checks: not green, and the failures are not from this PR. rust passes. flutter (run 41389778223) fails exactly 2 tests, both in app/test/store/store_screen_test.dart: "lists every catalog item with buy affordance or owned state" and "series items render after unseriesed items, under their header". The same two tests fail on main @ 5eeb347c (CI run 40260955072) and on the previous head 016425e5 (run 41389095993). A docs-only delta cannot cause them. Merge still needs the main failure fixed or explicitly accepted by the owner. I am not treating red CI as green.
Split: still no. This is one coherent docs-only fix.
COMMENT only. I have not merged, marked ready or pushed anything.
Replace references to completedSeries/isSetComplete/collectedCosmeticIds with collectionProgress, CollectionProgress.isComplete, SeriesCompletion.isComplete, and buildShareUri so the doc stays accurate when dead-code PR #45 lands. Co-authored-by: Uday Chauhan <chauhan.s.uday26@gmail.com>
ucguy4u
left a comment
ucguy4u
left a comment
There was a problem hiding this comment.
Changes requested and blocking
Head reviewed: b1754b1 (draft; base 5eeb347, main). Supersedes my review 5732176907 at 15ebd63 ("Architect comments clear on this head"). That earlier clear no longer stands for this head.
Blocking is narrow: one sentence in TODOS.md L53. Everything else checks out and the fix is one line.
What checks out
- 15ebd63..b1754b1 is one commit, docs only: TODOS.md, +2/-2 (L53 and L108). No code or comment change.
- Every symbol and path named in the new text exists on main (5eeb347) and on #45 (e0bf923):
collectionProgress,CollectionProgress,CollectionProgress.isComplete(packages/store/lib/src/collection_progress.dart),SeriesCompletion.isComplete(packages/sharing/lib/src/amiro_sharing_payload.dart),buildShareUriandmaxSharedCosmeticIds(packages/sharing/lib/src/share_payload_codec.dart),collector_frame.dart,buildOwnEncounterPayload(app/lib/sharing/own_encounter.dart),SeriesProgressRow. So the wording holds whichever of #44 / #45 merges first.git merge-treeof the two heads is clean. - L108 is accurate: the completion engine is derived from catalog + entitlements, and the collector's frame is driven by
SeriesCompletion.isComplete(collector_frame.dart), fed bybuildOwnEncounterPayload->collectionProgress(). No removed symbol is left in it. - The "Coordination with #45" note is correct on what #45 removes, that the section numbers are §9 (~L53) and §10 (~L108), and that merge order doesn't matter (#45 does not edit TODOS.md). One nit: "Open PRs ... none" at the top of the body is stale (#45 and #46 are open).
Blocking: L53 is still not true of the app today
New text: "The share URI (QR and NFC) now carries the sender's collected cosmetic ids (owned ...; free catalog items plus purchases use the same ownership rule as collectionProgress, serialized by buildShareUri ...)".
buildShareUrihas no app caller on main. It is only called by tests; the app only usesparseShareUri(scan_result.dart:49), for legacy receive. It was called fromapp/lib/sharing/own_share_uri.dartin #9 (fde5afe), withcollectedCosmeticIdsas input, and that call was removed in the PR7 cutover (#18, 289788b).- Today QR and NFC share
amiro://encounterthroughbuildOwnEncounterUri->encodeEncounterUri, whose payload carries per-seriescollectionCompletioncounts fromcollectionProgress(), not anownedid list. buildShareUriapplies no ownership rule. It sorts, caps at 32 and serializes whatever set the caller passes, and no caller passes one now. So "same ownership rule ... serialized bybuildShareUri" describes a live path that doesn't exist.
This swaps one stale claim for another. A docs-drift PR should not add a claim I can disprove in one grep, so I won't clear it.
Requested change (TODOS.md §9 first sentence), either of:
- Keep it as dated history and say so: "In v1 (2026-09-26) the legacy
amiro://shareURI carried the sender's collected ids (owned, sorted, capped atmaxSharedCosmeticIds= 32). Since the PR7 cutover the app sharesamiro://encounter, whose payload carries per-seriescollectionCompletioncomputed bycollectionProgress(buildOwnEncounterPayload);buildShareUri/parseShareUriremain only for legacy links, and the app no longer callsbuildShareUri." Then leave the SharedProfileScreen sentences as they are. - Or drop the parenthetical's mechanism and just say "(legacy path; see §10 for the current encounter payload)".
CI: flutter on b1754b1 is still running. The previous head's red (15ebd63, run 41389778223) was "160 passed, 2 failed" on the same two store_screen_test.dart tests that fail on main (5eeb347). That red is pre-existing and #46 is fixing it; it is not a reason for this verdict. Rust is green.
Nothing pushed, marked ready or merged by me. After the L53 fix, I'll re-review that head.
Replace incorrect live-path claim (buildShareUri) with dated legacy amiro://share wording and current amiro://encounter payload via buildOwnEncounterUri, per architect review. Co-authored-by: Uday Chauhan <chauhan.s.uday26@gmail.com>
ucguy4u
left a comment
ucguy4u
left a comment
There was a problem hiding this comment.
Architect comments clear on this head — not an approval
Head reviewed: fb7d32d (draft; base 5eeb347, main). Supersedes my block 5732229190 at b1754b1 ("Changes requested and blocking"); that block is resolved on this head.
- Delta b1754b1..fb7d32d is one commit, docs only: TODOS.md, 1 line (§9 L53). L108 is untouched (the diff has a single hunk), and still names
collectionProgress,CollectionProgress.isCompleteandSeriesCompletion.isComplete. - L53 is now accurate, claim by claim, checked on main (5eeb347):
- v1 used the legacy
amiro://shareURI withownedids, sorted and capped atmaxSharedCosmeticIds= 32: true (fde5afe, #9;share_payload_codec.dart). - The #18 cutover is
289788band it deletedapp/lib/sharing/own_share_uri.dart(and its test): true. - The app shares
amiro://encounter, built bybuildOwnEncounterUri/buildOwnEncounterPayload(own_encounter.dart), called from share_screen.dart L180 and L195; the payload's per-seriescollectionCompletioncomes fromcollectionProgress(cosmeticCatalog, purchasedIds): true. buildShareUrihas no app caller on main, only its definition and tests;parseShareUriis called once in the app,resolveScannedTextin scan_result.dart L49, and returnsLegacyProfileLink. That path is reached from QR scan, NFC receive and the incoming deep-link provider, so "remain only for legacy links (parseShareUriinapp/lib/sharing/scan_result.dart)" is exact.
- v1 used the legacy
- Symbols and paths in the sentence exist on main and on #45 (e0bf923), so it holds whichever PR merges first (as in my previous review; #45 does not touch TODOS.md or these files).
- PR body preflight now lists #45, #46 and #44, which matches
gh pr list --state open.
Non-blocking nit: in the same §9, "SharedProfileScreen shows a Collection section" is only true for legacy amiro://share profiles now (it is reached only via LegacyProfileLink). The sentence doesn't claim otherwise, so I'm leaving it, but a later pass could add "for legacy links".
CI: the flutter check on this head was still running when I posted. The red on earlier heads is the same 2 store_screen_test.dart failures that also fail on main (5eeb347), and #46 is fixing it. Not a factor in this verdict. Rust is green.
Nothing pushed, marked ready or merged by me. PR Governor merges only with the owner's yes.
…#43) Co-authored-by: Uday Chauhan <chauhan.s.uday26@gmail.com>
ucguy4u
left a comment
ucguy4u
left a comment
There was a problem hiding this comment.
Post-merge architect review of 434107c — not an approval
Range: fb7d32d..434107c (merged by ucguy4u as 6d9b087). Follows my CLEAR at fb7d32d (review 5732238924); that clear does not cover the 'Pass 2' commit 434107c, which is reviewed here for the first time. Code citations are against main at 6d9b087 (docs in the range are byte-identical on main).
Flag for owner (human) review: these are legal and compliance documents (privacy policy, Play Data Safety draft, store listing). The policy is under docs/, so it is probably live on Pages already. Verdict: do not submit to any store on this text. The privacy-policy .md and .html say the same thing (word-level diff is empty), so the problems below are in both. No secret, key or personal data is committed in the range: the only identifiers are the RC project/app/webhook ids, the public contact email, and a <token> placeholder.
BLOCKING: statements that are false against the code
B1. The profile backup is automatic, not "optional", and it uploads fields the user marked private.
- privacy-policy.md:33,9 / .html:46,24, data-safety.md:22,33-36, store-listing.md:23-25 all call it optional.
- Every autosave pushes to Supabase: app/lib/identity/identity_edit_screen.dart:75-79 (600 ms debounce) -> :138
profileCloud.push(identity). This happens while typing, before Save is tapped. There is no consent step or toggle. main.dart:37-41 always builds the real cloud client. - The upload is
identity.toJson()(packages/identity_core/lib/src/identity.dart:101-115, used at supabase_profile_cloud.dart:47,132-136). That is every field, including email and bio, plus the privacy flags, regardless of whether the user marked them public. - policy .md:39 says "On first launch ... may create an anonymous Supabase session". Not so: main.dart:43
pull()returns null when there is no session file (supabase_profile_cloud.dart:87), so the first network call is the first autosave (:224-243). - policy .md:129-131 says backup is "initiated by whoever uses the App". It is not initiated by the user.
- Fix: either add an in-app opt-in and keep the wording, or reword to "automatic" and say that private-flagged fields are also backed up.
B2. store-listing.md still carries three statements that Pass 2 made false, in the same file where it edited lines 23-25.
- :34 "Nothing you mark private ever leaves your device" is false (see B1).
- :37 "no Amiro server" is false (Supabase project plus the Cloudflare Worker).
- :38 "Uninstalling the app removes them" is false. The cloud rows persist, as privacy-policy.md:122-125 itself says.
B3. "No purchase traffic unless you buy" is false, so the Data Safety "optional / only when buying" answers are wrong.
- privacy-policy.md:103-104 and data-safety.md:57,69-70 ("No — only when user buys a paid Store item").
- The RevenueCat SDK is configured and contacted on every Android launch. app/lib/app.dart:91-96 mounts all five tabs in an IndexedStack. StoreScreen watches
ownedCosmeticsProvider(store_screen.dart:324) -> store_providers.dart:14-16 -> revenuecat_entitlement_store.dart:24-35,43-44, which callsPurchases.configureandgetCustomerInfo. Separately, every profile push callsappUserId()(supabase_profile_cloud.dart:46,199-205 -> :63-66). - RevenueCat's own Data Safety guidance says purchase-history collection is required and cannot be turned off, with purposes App functionality and Analytics. data-safety.md:69 lists only App functionality.
- policy .md:89-91 says the SDK sends only an id and the product. The SDK also sends app and device info and locale/currency.
- TODOS.md:154 says the live Play form already declares this as "optional (only triggered by a paid purchase)". That declaration has the same flaw.
B4. "Only the optional profile backup and purchase paths use the network" is false: the app fetches Google Fonts at runtime.
- policy .md:112-114 and :5-10.
- app/lib/theme/amiro_theme.dart:65 ("loadFonts fetches Instrument Sans/Serif at runtime"), :67 (default true), :82-95; app.dart:25 uses the default. app/pubspec.yaml:35 depends on google_fonts and :52-53 bundles no font assets. Device IP and request metadata go to Google on first use.
- This third party is not disclosed. Fix: bundle the fonts and set
GoogleFonts.config.allowRuntimeFetching = false, or disclose it.
B5. "Only the fields you mark public are included in the QR/NFC payload" is false.
- privacy-policy.md:60-62, .html:72-74; store-listing.md:29-34.
- app/lib/sharing/own_encounter.dart:55-67 always sends the identity id, displayName, the avatar config, equipped cosmetics and per-series collection completion. The legacy URI (packages/sharing/lib/src/share_payload_codec.dart:25-26) always sends displayName and username. displayName has no toggle.
- Only the email field has a public toggle (identity_edit_screen.dart:284-296). Bio and username can never be made public in the UI.
B6. The store listing claims features that don't ship.
- store-listing.md:30 and :33 claim "your name, bio, social handles" and that "Every field — email, phone, social handles, website — has its own privacy toggle". The identity screen has only display name, username, bio and email inputs (identity_edit_screen.dart:249-278) and one toggle (:284). No phone, X, Instagram or website input exists anywhere in app/lib.
- policy .md:25-28 and data-safety.md:35-36 likewise list mobile, social handles and website as collected.
- iOS only (conditional blocker for App Store Connect, which store-listing.md:3 covers): :41-42 and :108-110 say "some purchasable" and "Digital purchases: Yes". Purchases are a no-op on iOS (revenuecat_entitlement_store.dart:26-31,69-75; TODOS.md:153).
B7. Process gate: the live Play Data Safety form predates the backup.
- data-safety.md:16-18 says so itself, and TODOS.md:154 says the form was re-submitted on 2026-09-29. The app source is already at 1.0.6+12 (main.dart:28).
- The live form does not declare Name, Email or User IDs sent to Supabase. It must be updated before any release that contains the backup.
- data-safety.md:84-88 answers deletion with "Partially". Play's question is yes/no and a "yes" needs a URL, so the owner has to decide the answer. Whether an anonymous Supabase user counts as an in-app account under Play's account-deletion policy is also a human call.
Verified accurate (no action)
- No analytics, ads or location SDKs in any pubspec; the permissions match the policy.
- Android: NFC and INTERNET (AndroidManifest.xml:2,5-6) plus the HCE service (:40-50).
- iOS: camera and NFC usage strings (Info.plist:40-43).
- Camera is used only for QR scanning via mobile_scanner; nothing is stored.
- CAMERA and BILLING are not in the app manifest and come from plugin manifest merge. Confirm them in the release AAB's merged manifest.
- HTTPS everywhere: supabase_public.dart:4, RevenueCat, the Worker.
- Webhook README: the path-token check is real (workers/revenuecat-webhook/src/index.ts:99-106). The service-role key is only in the Worker. The migration enables RLS with no policies (20260929060138_create_entitlements.sql:36-40). Product ids, entitlement mapping and the app package id match the code. The event log is idempotent (index.ts:120-122 plus the unique event_id).
- The recovery code is stored on device (supabase_profile_cloud.dart:31,34-38). The passport stays local (main.dart:52-53).
IdentityRepository.clear()has no UI caller, as data-safety.md:87 says. - Discovery README: package API and discovery.json location match (discovery.dart exports, file_discovery_repository.dart, main.dart:53). The app/lib/discovery wiring is real.
Nits (non-blocking)
- policy .md:105-106 says the purchase record holds id, product and timestamp. index.ts:120-133 stores the whole RevenueCat event (
raw_event), including transaction id, price/currency, country and store. Also,revenuecat_app_user_idsits in the profiles table (create_profiles.sql:16), so purchases can be joined to a named profile. Policy :46-48 discloses the link; say so next to :97-98. - Worker README: it omits
CRASH_PATH_TOKENand the/crash/<token>route (index.ts:41-91,96-98; wrangler.toml comment).- "The Flutter app never calls this Worker" (README:8) is conditional. Builds with
--dart-define=AMIRO_CRASH_INGEST_URLpost crashes there (main.dart:25-31, crash_reporter.dart:38-57, migration 20261015100000). - Policy and Data Safety (data-safety.md:50) should be updated before any release sets that define. I found no build script in the repo that sets it, so the owner must confirm the actual AAB build command.
- The path token is compared with
!==(index.ts:104,50), not constant-time, and there is no signature check (TODOS.md:158 acknowledges it). - README:25 calls entitlement_grants "current state", but the grant upsert re-runs on duplicates and stamps a fresh granted_at (index.ts:141-161). Out-of-order events can undo a revoke.
- "The Flutter app never calls this Worker" (README:8) is conditional. Builds with
- Local storage and Android backup: supabase_session.json (access and refresh tokens) and profile_recovery_code.txt are plaintext in the app support dir (supabase_profile_cloud.dart:30-31,324-331). The manifest never sets allowBackup or data-extraction rules (AndroidManifest.xml:7-10), so Android Auto Backup defaults on and copies them to the user's Drive backup. Policy .md:120-121 calls an OS backup something "you created yourself". Set
allowBackup=falseor exclude these files, and reword. - data-safety.md:33 says name is "required at identity creation". Nothing enforces it (identity_edit_screen.dart:124-131).
- data-safety.md:33-36,58 mark "Shared: Yes" for peer-to-peer and Supabase. Under Play's definitions, user-initiated transfer and service providers are usually not "sharing". Over-declaring is safe but should be deliberate.
- policy .md:77-80 says everything public a sender shared is stored on your device. The ledger stores only display name, avatar, cosmetics and collection (encounter_record.dart; contact_card.dart:5-8).
- store-listing.md:108-115 is stale: it says the 3 products "must still be created in Play Console". TODOS.md:148 says they were created on 2026-09-29.
- The policy gives no retention period. There is no cleanup in supabase/migrations, so rows live until an email request. The deletion email should say to include the recovery code, since otherwise the operator can't find the row (policy .md:122-125).
- packages/discovery/README.md:11 says payloads reach the package "only after packages/sharing validates them". EncounterProcessor itself decodes and validates (encounter_processor.dart:92-106). The README also omits InMemoryDiscoveryRepository and newLocalRecordId (discovery.dart).
- The anon key in supabase_public.dart:4-6 is the public anon JWT (role=anon), so it is not a secret. HttpCrashSink does not enforce https on the define (main.dart:31).
Follow-up PR
Urgent, before any store submission: B1-B7 need one docs-and-form fix PR, plus decisions from the owner on opt-in vs. wording (B1), fonts (B4) and the Play deletion answer (B7). The nits can wait for next week's pass, except nit 3 (Auto Backup of tokens) and nit 2 (the crash-ingest disclosure) if a release will set that define. Owner (human) review is required in either case.
I did not review the non-doc files in fb7d32d..434107c beyond what the cited claims touch, and I have not run any app code.
Under the standing rule, nothing here is an approval. Merge and release decisions stay with the owner and PR Governor.
Documentation and comment-only updates. Other open PRs at pass time: #45 (dead-code store helpers), #46 (store screen test fix), and this #44 (
gh pr list --state open).Evidence
README.mdstoreas stubbed purchases andsharing/nfc/qras future stubs; omitteddiscovery.packages/store/lib/src/revenuecat_entitlement_store.dart;app/lib/app.dartL79–85 (five tabs);packages/discovery/in rootpubspec.yamlworkspacepackages/sharing/README.mdhttps://id.amiro.appas current contract.amiro://encounter+ legacyamiro://share, real exports, app wiring.packages/sharing/lib/sharing.dart;packages/sharing/lib/src/encounter_qr.dartL6–7;app/lib/sharing/own_encounter.dartL111–119packages/nfc/README.mdManagerNfcReader,AndroidNfcEmulator, and app entry points.packages/nfc/lib/nfc.dart;app/lib/sharing/nfc_receive_screen.dartpackages/qr/README.mdpackages/qr/lib/src/qr_generator.dart;app/lib/sharing/qr_scan_screen.dartpackages/qr/lib/src/qr_generator.dart(comment)amiro://share.amiro://encounterprimary + legacy share.app/lib/sharing/own_encounter.dartusesencodeEncounterUridocs/protocols/identity-link.mdid.amiro.appshape.docs/protocols/identity-link.md(prior text);app/lib/sharing/scan_result.dartL39–41docs/architecture/overview.md2f9d6c8(#43);e921b1a(#28) RevenueCat; §11 NFC pass already in filedocs/qa/test-plan.mdapp/test/sharing/;packages/sharing/test/;melos run testscripts in rootpubspec.yamlTODOS.md§6packages/store/lib/src/cosmetic_listing.dartL73–157;4d0a1a4(#31)TODOS.md§7 / PR1 notesTODOS.md§15;app/lib/sharing/scan_result.dartNo code behavior changes (one doc comment only).
Architect follow-up (this commit)
docs/architecture/overview.mdidentity-link.md/test-plan.mdstub — not started (~L79, L83, L235); claimed every package has a README (~L22).origin/main@5eeb347chad same stale lines; branch snapshot +docs/protocols/identity-link.md,docs/qa/test-plan.md;packages/{sharing,nfc,qr,store}/README.mdonly (no README indiscovery,identity_core,avatar_core,avatar_renderer)docs/protocols/identity-link.mdincoming_share_listener.dartforapp_links.incomingShareLinkProviderinsharing_providers.dart; listener named for UI only.app/lib/sharing/sharing_providers.dartL12–16;incoming_share_listener.dartwatches that providerTODOS.md§7cosmeticCatalog).packages/store/lib/src/cosmetic_listing.dartL73–157 (11CosmeticListingentries)TODOS.md§152f9d6c8(#43);app/lib/main.dartL37–44;app/lib/identity/supabase_profile_cloud.dartTODOS.md§9 (~L53)ownedids via a livebuildShareUripath.amiro://sharehistory; current share isamiro://encounterwithcollectionCompletionfromcollectionProgress(#18,289788b); legacy codec receive-only.app/lib/sharing/share_screen.dart;app/lib/sharing/own_encounter.dart;app/lib/sharing/scan_result.dart; #18 removedapp/lib/sharing/own_share_uri.dartRebase: onto
origin/main@5eeb347c8fbdeb30253da70a3e1543165ade4f01(already up to date).Skipped:
store_screen_test.dartand other failing tests — pre-existing on main @5eeb347c, out of scope per task (#46 addresses).Coordination with #45
Dead-code PR #45 removes
completedSeries,CollectionProgress.isSetComplete, andcollectedCosmeticIdsfrompackages/store/lib/src/collection_progress.dartandpackages/store/lib/src/collection_highlights.dart. This pass updatesTODOS.mdonly:collectionProgress,CollectionProgress.isComplete, andSeriesCompletion.isComplete(inpackages/sharing/lib/src/amiro_sharing_payload.dart) instead of the removed store helpers; matchesapp/lib/sharing/collector_frame.dartandbuildOwnEncounterPayloadinapp/lib/sharing/own_encounter.dart.amiro://shareownedids vs today'samiro://encountercollectionCompletionpath; does not credit removedcollectedCosmeticIdsor imply the app still callsbuildShareUri.Merge order between #44 and #45 does not matter for these lines (#45 does not edit
TODOS.md).Pass 2 — legal / backup drift (#43)
docs/legal/privacy-policy.md+.html2f9d6c8(#43);app/lib/main.dartL37–44;supabase/migrations/20261016180000_create_profiles.sqldocs/legal/data-safety.mdworkers/revenuecat-webhook/README.mddocs/legal/store-listing.mdworkers/revenuecat-webhook/README.mdSupabaseProfileCloudvs Worker service rolepackages/discovery/README.mdpackages/discovery/lib/discovery.dart;app/lib/discovery/README.mdpackages/discovery/README.mdHead:
434107c22dda1358d089b26acc6bf374911c5207. Skipped:TODOS.mdin pass 2 — open PR #46 also edits it (gender-gating note).