Commit Graph
7 Commits
Author SHA1 Message Date
Paul HittandClaude Fable 5.1 2fa430f21c API-style comments and ktlint formatting
Every comment is reduced to its first sentence plus @param, @return
and @throws tags. Comment lines: 20040 to 8526. ktlint 1.8 applied
with its default (official Kotlin) style across all modules; 262 files
reformatted. The 264 remaining ktlint findings are naming and filename
rules, which are code changes, and are left alone.

Comments and formatting only. 1208 tests pass, counted from the XML.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-08 20:02:07 -04:00
Paul HittandClaude Fable 5.1 8a7f40fb13 Second pass with the avoid-ai-writing detector
The skill's detector scored the README, PARITY.md and the extracted
comment prose. Fixed what it flagged: hollow intensifiers, "in order
to", "additionally", "the whole point", bold sentences in docs, and
bold that had wrapped across comment lines. Comment prose scores a
maximum of 4 out of 100 across the repo.

One sweep rule mangled four comments ("which is the entire point of X"
lost its clause); repaired, and the rule now keeps the clause.

Comments and strings only. 1208 tests pass, counted from the XML.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-08 19:20:07 -04:00
Paul HittandClaude Fable 5.1 4dd544083c Remove machine-writing tells from comments and docs
Emoji markers, markdown bold inside code comments, and em-dashes are
gone from source comments, string literals, scripts, Gradle files and
the markdown docs. Counted before and after:

  emoji markers   2680 -> 1 (the emoji picker's own list, which is the feature)
  em-dashes       2490 -> 9 (table placeholders and fenced code)

Two warning glyphs in user-facing strings in ChatScreens.kt are removed
as well; the sentences say the same thing without them. Two backtick
test names carried an em-dash and now carry a comma.

Nothing else changed. 1208 tests pass, counted from the result XML.
The three obfs4 interop tests are opted out with OBFS4_INTEROP_OPTOUT=1
for this run, as their own message instructs when no bridge is set up.

CONTRIBUTING gets a four-line rule so it does not grow back.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-08 18:52:12 -04:00
Paul HittandClaude Opus 5 2d05bbe7c4 Fix: a contact with no address was pinned as the string "null"
The same platform difference as the vault index, in shipped code, with a
worse consequence.

🔴 `saveTrust` wrote `"fnid": null` explicitly when a contact's global
address was not yet known. Android's bundled `org.json` reads a JSON null
back as the four-character string `"null"` — a non-null value that is not
an address. What that caused on a phone:

- `Trust.pin` compares a newly offered fnid against the pinned one and
  treats a difference as a different person. `"null" != "fnid:…"`, so the
  FIRST TIME a real address arrived the contact was marked **CHANGED** —
  the "security keys changed" warning, which is supposed to mean an
  attack, firing on an ordinary first sighting. The vault's own comment
  about minting a fresh device warns about exactly this shape of false
  alarm.
- `rememberIdentity` returns early once `fnid != null`, so the real
  address was never resolved and the record stayed wrong permanently.
- `pinnedFnid` fed `"null"` into key-log verification, which recomputes a
  leaf against the address a contact was accepted under — every check
  against the wrong subject.

Two halves, because records written the old way are already on phones:
the writer now omits the key entirely (an absent key reads as `""` on both
platforms, which is what the reader always expected), and the reader
rejects the literal `"null"` so existing records repair themselves.

⚠️ The test that matters is the one feeding the reader the literal string.
On the JVM a real JSON null reads back as `""` whether the key is omitted
or not, so the plain round-trip passes with the bug intact — which is why
this survived every suite. Confirmed by restoring the old reader and
watching it go red.

Found by the sweep flagged in android!54. `JSONObject.NULL` is written in
exactly two places in this app, and both were wrong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-04 16:10:47 -04:00
Paul HittandClaude Opus 5 02e5c45cd4 A test that could actually have caught it
`a retired session is kept and still opens its chain` passed while the fix was
broken, because it retires a PRISTINE session — which is not what the receive
path holds. By the time that path decides to retire anything it has just proved
the peer is on another chain by handing this session an envelope it could not
open, and E2ESession is a class, so that attempt consumed skipped keys and
stepped the chain.

The new test reproduces that state and asserts both halves: the damaged instance
must NOT still open its chain (or the test proves nothing), and the copy re-read
from the vault must. It fails against the first version of the fix and passes
against this one.

13 VaultTest cases, 112 engine tests, 0 failures. Both platforms compile.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 17:13:59 -04:00
Paul HittandClaude Opus 5 999ab2bd26 Keep the chain a re-establishment replaces
When both sides open the same chat before either has processed the other's
prekey — two people tapping a conversation at the same moment — each runs X3DH
and each ends up adopting the other's chain to read what arrived. From then on
the peer sends on the chain being replaced. Overwriting it without a copy does
not lose one message; it makes that direction undecryptable for the rest of the
conversation, silently, because an envelope that will not open is skipped rather
than reported.

**iOS.** `E2EInbox` had the fallback already — `loadArchivedRatchet` is tried
before building from a prekey, and its comment describes exactly this situation.
It was unreachable: `saveArchivedRatchet` appeared once in the file, and that
call only re-saves a ratchet that was already archived. Nothing ever put a live
session there. It does now, immediately before the session it replaces is
overwritten.

**Android was worse, and had no fallback at all.** `Conversations.poll` read

    val session = existingSession(fromDevice) ?: E2ESession.respond(identity, envelope)

which looks like a fallback and is not one: with a stored session present that
was the ONLY thing tried, `decrypt` threw for a peer on another chain, and the
catch swallowed it. `respond` was never reached. Now three chains are tried in
order — the current one, the retired one, then a fresh one built from the prekey
block — and the replaced chain is kept.

⚠️ The watermark still advances on a total failure, deliberately. An envelope
none of the three chains could open will not open next poll either, and holding
the cursor there would wedge the inbox behind one bad message. What made this
lossy was never the watermark; it was that only one chain was ever tried.

`Vault` grows a retired-session slot under its own prefix — deliberately not
`SESSION_PREFIX`, or `knownPeers()` would list every retired chain as a peer —
and `forgetAllSessions` clears it too, since a sign-out that left chain keys
behind would keep exactly what it claims to erase.

Tests, and the one that matters is the storage-level one:

- `VaultTest.a retired session is kept and still opens its chain` — asserts the
  ADOPTED session cannot open the retired chain before asserting the retired one
  can, so it cannot pass by accident. **Verified load-bearing**: neutering
  `saveRetiredSession` fails it, restoring it passes.
- `VaultTest.forgetting sessions forgets the retired ones as well`.
- `E2ESessionTest.simultaneous initiation does not strand the conversation` —
  covers the engine primitives and asserts the SECOND exchange, the one that
  used to die. ⚠️ It holds both sessions in local variables, so it passes with
  or without the storage fix; it is not evidence for it.

111 engine tests, 0 failures. Both platforms compile.

⚠️ This is a real correctness defect and is fixed on its merits. Its
relationship to the one-way loss seen between two simulators is NOT established
— that symptom is the first message, and both first messages open fine either
way. Do not close that investigation on this commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 16:53:19 -04:00
Paul HittandClaude Opus 5 5f983c1455 Android: the app, and the two things only a phone could tell us
There is now a Flamenet Messenger app that installs, launches, creates an
account on a self-hosted relay, registers a device, publishes prekeys, and
stores all of it in an Android Keystore-backed vault. Verified on an emulator
through the actual screens, not by unit test.

92 JVM tests, 37 cross-engine, 9 on-device, 3 against live relays.

Two findings, both of which the JVM suite was structurally incapable of
producing.

**Android has no ordinary Ed25519 provider.** `KeyPairGenerator.Ed25519` and
`KeyFactory.Ed25519` come from `AndroidKeyStore` and from nowhere else -- probed
on API 35 rather than assumed -- and Keystore only makes keys it keeps. Every
Ed25519 key in this engine is one we must hold: a device identity in our own
vault, an account key derived from a passphrase. So the JCA route could never
have worked. It compiled, passed 81 tests on a laptop, and threw "Not
initialized" on the first launch on a phone. Worse, it was LATENT: a wrong
password failed before any key was minted, so the error path worked perfectly
while the happy path was broken.

Ed25519 now uses BouncyCastle's `math.ec.rfc8032` core directly -- the engine's
one real dependency, present because the platform leaves a hole, and a strictly
better answer than the non-constant-time keygen it replaces. ProviderProbeTest
records the measurement so nobody guesses again.

**The prekey field is `prekeys`, not `opks`.** I guessed. The relay accepted the
registration, returned 200, and stored nothing -- so the device was published
and could not be started with, a failure with no error anywhere. Caught by a
live test against the throwaway fleet, which exists because the same path
through the emulator takes four minutes to say "timed out" and nothing about
where.

Two testing traps, recorded where they bite:

An indeterminate progress spinner animates for ever, so Compose's test clock
never goes idle and `waitUntil` never evaluates its condition -- it fails on the
full timeout, reading as "sign-up is broken" when sign-up works. Auto-advance is
now disabled around the wait only; disabling it for the whole test is the next
mistake, because the app opens on a spinner while it reads the vault.

Wiping the Keystore in `@Before` deletes the key the already-launched activity
is holding, and every later write fails against a key that no longer exists. The
test signs out through the UI instead, which is both safe and what a person does.

Failure messages now include what the screen was showing. A bare timeout sends
you to the network, the relay and the crypto in turn while the app has been
displaying the reason the whole time.

Also corrects a note that said this Mac had no Android SDK. It does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-23 17:02:16 -04:00