mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-11 08:47:33 +00:00
docs(qa): record v1.13.0 test coverage, open findings and recipes
The 30 fixes from this session each carry their reasoning in their own commit message, and the regression tests are in the repo. What none of that captures is the shape of the testing itself: what was actually exercised, what was not, what we knowingly left broken, and how to reproduce the setups. That lived only in one conversation. Records four things: - **Coverage**, honestly split three ways: exercised on device; fixed but only unit-verified (roughly half the session's work, including the entire Concord authority chain); and never opened at all (Nests, Marmot, the whole Desktop app, payments, notifications). Plus the platform gaps — one tablet, one flavour, one ABI, Android 14 only while targetSdk is 37, and no real `release` build. - **Open findings** we chose not to fix, so they are decisions rather than oversights: the unbumped versionCode, the Firebase scheduling failure that may cost Crashlytics, Tor settings needing a restart, an unreachable relay reported as empty, WebView profiles surviving logout, control-plane edits still dropping unknown keys, and the CORD-05 root-binding gap that Armada shares and therefore needs a spec conversation. - **Setup recipes** that took real time to work out: isolating `amy` by `$HOME`, which relays actually accept a NIP-29 create, deep-linking past a 1000-entry directory, and proving "no network before consent" by running a local relay over `adb reverse` and counting events either side of the tap. - **Patterns** that recurred often enough to be process problems: tests that assert the bug, implemented-but-unreachable capabilities, Gradle serving a stale up-to-date test run and reporting success for a deliberately broken build, and four confident diagnoses that measurement overturned. Written for whoever picks this up next, including the parts that reflect badly on the testing rather than only the parts that reflect well. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
a040aa522c
commit
4ee0416a90
@@ -0,0 +1,154 @@
|
||||
# v1.13.0 release QA — coverage, open findings, and recipes
|
||||
|
||||
One extended testing session against v1.13.0 (~2011 commits since v1.12.6). Fixes landed on
|
||||
`fix/napplet-account-isolation-and-consent` (30 commits); each commit message carries its own
|
||||
root-cause reasoning and is the better reference for *why* a given change looks the way it does.
|
||||
|
||||
This document records what that session **could not** capture in commit messages: what was actually
|
||||
exercised, what was not, what we chose to leave broken, and how to reproduce the setups.
|
||||
|
||||
**Device under test:** Samsung SM-T220 tablet, Android 14, `sw600dp`, `play`/`benchmark`, arm64.
|
||||
Everything below is that one configuration unless stated.
|
||||
|
||||
---
|
||||
|
||||
## 1. Coverage
|
||||
|
||||
### Exercised on device
|
||||
|
||||
| Area | Notes |
|
||||
|---|---|
|
||||
| Upgrade path | install over a month-old build; migrations survived, ~880 ms cold start |
|
||||
| Napplet / web app per-account isolation | leak found and fixed; each account now has its own jar |
|
||||
| Embedded tab rebuild on account switch | blank-tab bug found and fixed; verified both directions + a forced-failure A/B |
|
||||
| Launch-account signing binding | desync reproduced end to end, then verified fixed |
|
||||
| NIP-46 remote signer | 13 checks, all passing (pairing gate, 22242 prompt, decrypt counterparty/plaintext/narrow grant/scoping) |
|
||||
| Concord | invite consent gate, private/voice rename, role rank gate, revoke gate |
|
||||
| NIP-29 relay groups | directory browse (crash found), naddr deep-link join, membership resolution |
|
||||
| Breadth sweep | Messages, NIP-29, Git, Podcasts, Blossom list, Location, Theming |
|
||||
| Location | map picker + teleport, before/after on 4 symptoms, composer path |
|
||||
|
||||
### Fixed but **only unit-verified** — never run on a device
|
||||
|
||||
Concord rollback floor · stranded recovery · member-set gap · invite expiry · moderation head ·
|
||||
**chain-poisoning fix** · community-list unknown-key preservation · V4V fee clamp · Cashu SSRF
|
||||
validator · Blossom 402 (cap / re-prompt / double-spend / `X-Reason`) · amountless-invoice display ·
|
||||
**napplet consent diff dialog** (never seen rendered) · connect-dialog capability disclosure ·
|
||||
`identity.watch` DENY · DM ciphertext previews and the `User` metadata race · chat date separators ·
|
||||
podcast duplicate description · Concord leave affordance · the three revocation wirings.
|
||||
|
||||
### Never opened at all
|
||||
|
||||
Nests / audio rooms (`quic` + MoQ) · Marmot / MLS · **the entire Desktop app** (where Privacy Lock
|
||||
actually ships) · Blossom "Sync all" (skipped deliberately — uploads to real servers) · podcast
|
||||
chapters / transcripts / credits · Git branch switching · Messages live-typing and per-type toggles ·
|
||||
real payments (zaps, V4V streaming, Cashu redeem) · notifications / push · search · Calendar, Chess,
|
||||
Polls, Marketplace, Workouts, Badges, Follow Packs, Emojis, HLS Upload, App Store, Live Streams.
|
||||
|
||||
NIP-29 admin: the menu is reachable and renders, but Edit metadata, invite creation, subgroups and
|
||||
pinned messages were never exercised.
|
||||
|
||||
### Platform gaps
|
||||
|
||||
- **Android 14 only.** `targetSdk` is 37; Android 15+ forces edge-to-edge and that path is untested.
|
||||
An emulator makes this cheap and it is the highest-value remaining gap.
|
||||
- **Tablet only** — no phone layout. **`play` only** — no fdroid. **`benchmark` only** — the real
|
||||
`release` (full R8) has never been built or run. **arm64 only.**
|
||||
- **Amber / NIP-55** external signer never tested, including a known decrypt double-prompt risk.
|
||||
- **Tor-on paths** — Tor was disabled for untrusted relays mid-session and not restored.
|
||||
|
||||
---
|
||||
|
||||
## 2. Open findings (known, deliberately not fixed)
|
||||
|
||||
**Release mechanics**
|
||||
- `appCode` still `454` and `app` still `1.12.6` — Play hard-rejects a duplicate versionCode.
|
||||
- Firebase `TransportRuntime` cannot schedule (`JobInfoSchedulerService` missing from the merged
|
||||
manifest). If this reproduces in `release`, **Crashlytics delivery is broken** and the release
|
||||
ships blind.
|
||||
|
||||
**Correctness / UX**
|
||||
- Tor settings do not take effect until app restart, with no indication.
|
||||
- An unreachable relay is reported as "No groups on this relay yet" — indistinguishable from empty.
|
||||
- NIP-29: a stale "Requested" join state is never reconciled against an arriving 39002 roster.
|
||||
- Concord: leaving does not unpin from the bottom bar, leaving a dead tab.
|
||||
- Read-only accounts render nothing for a kind:4 chatroom body (better than ciphertext, still wrong).
|
||||
- Modal geohash picker header is overdrawn by the MapView (pre-existing).
|
||||
- `amy relaygroup create` reports success on relays that silently reject it — always verify with `info`.
|
||||
- `amy login bunker://…` hangs and never delivers a `connect`.
|
||||
|
||||
**Security / protocol**
|
||||
- NIP-46 "Generate a new address" claims to disconnect every app; it rotates the transport key and
|
||||
revokes nothing.
|
||||
- WebView storage profiles are never deleted on logout — a removed account's cookies persist.
|
||||
Requires a broker message so `:napplet` can call `ProfileStore.deleteProfile`.
|
||||
- Control-plane *edit* paths (`editConcordMetadata`, `grant`, channel edits) still drop unknown JSON
|
||||
keys; only the community list was fixed.
|
||||
- **CORD-05: `community_id` does not commit to `community_root`**, so a crafted invite can carry a
|
||||
real community's identity with an attacker's root. **Armada has the identical gap** — this needs a
|
||||
spec conversation, not a unilateral fix.
|
||||
- `grantConcordRole` is implemented and unreachable; the changelog claims role grants ship. A
|
||||
proportionate UI exists (a picker beside "Make admin"); it was blocked by concurrent work.
|
||||
|
||||
---
|
||||
|
||||
## 3. Setup recipes
|
||||
|
||||
**`amy` with an isolated identity** (never touch the maintainer's real one):
|
||||
|
||||
```
|
||||
AMY=$(pwd)/cli/build/install/amy/bin/amy
|
||||
H=/tmp/qa-home; mkdir -p $H
|
||||
HOME=$H $AMY --account qa login <nsec> --secret-backend plaintext
|
||||
```
|
||||
`amy` scopes by `$HOME`, not a flag. `init` prompts for a passphrase and hangs without a TTY — use
|
||||
`--secret-backend plaintext` for throwaway identities.
|
||||
|
||||
**NIP-29 test group.** `relaygroup create` on `communities.nos.social` and `relay.groups.nip29.com`
|
||||
returned success but published nothing; `groups.0xchat.com` worked. Always confirm with
|
||||
`relaygroup info`. To reach a group in a 1000+ entry directory, skip the UI and deep-link:
|
||||
`adb shell am start -a android.intent.action.VIEW -d "nostr:<naddr>"`, encoded via
|
||||
`amy encode naddr --pubkey <relay-nip11-pubkey> --kind 39000 --identifier <groupId> --relay <url>`.
|
||||
To make a device account an admin, have it join first, read its pubkey from `relaygroup info`, then
|
||||
`put-user … --role admin`.
|
||||
|
||||
**Proving "no network before consent."** Run a local relay (`amy serve`), expose it with
|
||||
`adb reverse tcp:7777`, and make it the *only* relay in the artefact under test. Count events before
|
||||
and after the user action — that turns "I didn't see traffic" into an actual measurement.
|
||||
|
||||
---
|
||||
|
||||
## 4. Patterns worth acting on
|
||||
|
||||
These recurred often enough to be process problems rather than individual bugs.
|
||||
|
||||
**Tests that assert the bug.** At least five encoded the buggy behaviour as intended — a NIP-46 test
|
||||
named `getPublicKeyReturnsUserPubKeyWithoutAuthorization`, a V4V invariant only ever run on
|
||||
well-formed input, a napplet session test that never crossed accounts. *Always verify a new
|
||||
regression test fails without the fix* — and beware that **Gradle will serve a stale up-to-date
|
||||
`jvmTest` and report BUILD SUCCESSFUL**, which makes that check silently lie. Use `--rerun-tasks`.
|
||||
|
||||
**Implemented-but-unreachable capabilities.** Five found: `leaveConcordCommunity`,
|
||||
`ConcordInviteBundle.isExpired`, `NappletPermissionLedger.endSession`, `grantConcordRole`, and
|
||||
`NappletBroker.revokeSessionGrants`. Each made a feature look complete to anyone reading the model
|
||||
while being unreachable to users, and the first one actually invoked turned out to be **broken as
|
||||
written**. A lint for "public capability with no caller outside its declaring file" would catch the
|
||||
whole class cheaply.
|
||||
|
||||
**Hypotheses need measurement, not plausibility.** Four confident diagnoses were wrong: the "npub in
|
||||
title" bug was a `User` lazy-init data race, not a display bug; chat date separators were a
|
||||
`reverseLayout` misconception, not bubble grouping; the map picker had no tile problem at all; and
|
||||
NIP-29 membership was a *relay rejecting the REQ* (`blocked: it's not allowed to mix metadata kinds
|
||||
with others`), not membership modelling. Instrument first.
|
||||
|
||||
**Check the reference implementation.** Reading Armada changed the answer three times out of three —
|
||||
it corrected an owner-rotation rule that would have stranded owners, stopped an invite-binding "fix"
|
||||
that was both interop-breaking and ineffective, and supplied the chain-poisoning design (gate *after*
|
||||
folding, not before). Armada is **AGPLv3** and Amethyst is MIT: read for semantics, copy nothing.
|
||||
|
||||
**Comments encoding constraints are load-bearing.** The synchronous SharedPreferences read looks like
|
||||
an obvious StrictMode fix; its comment records that an async hydrate reopens a settings-clobber race.
|
||||
Removing it would have been a confident, review-passing regression.
|
||||
|
||||
**Beware concurrent agents and `git add -A`.** Two commits were contaminated, and one silently
|
||||
committed another worker's temporary revert. Stage explicit paths, always.
|
||||
Reference in New Issue
Block a user