Commit Graph

7 Commits

Author SHA1 Message Date
Claude fc99bed55a fix(cli): default amy launcher to C.UTF-8 so emoji argv survives
`marmot message react "$gid" "$id" "🍕"` was producing a kind:7 whose
inner content was four `U+FFFD` replacement characters instead of the
emoji. Whitenoise's `message_aggregator::emoji_utils` rejected it with
`Invalid reaction content: ����`, and the openmls receiver retried the
event ten times before giving up — by which point the secret tree had
already consumed that generation, so the retry surfaced as a confusing
`Ciphertext generation out of bounds 1 / SecretReuseError`. That last
error was a downstream symptom; the root cause is argv decoding.

Why it happens: JEP 400 (Java 18+) pins `file.encoding` to UTF-8 but
deliberately leaves `sun.jnu.encoding` — the encoding the JVM uses to
decode argv, filenames, and native interop strings — tied to the OS
locale, and `sun.jnu.encoding` is read **before** the JVM parses any
`-D` flag, so it can't be overridden via `applicationDefaultJvmArgs`.
Under POSIX/C locales (no `LANG`/`LC_ALL` set, common on CI runners
and stripped-down containers) `sun.jnu.encoding` ends up at
`ANSI_X3.4-1968` — i.e. ASCII — and every byte > 0x7F in argv lands
as `U+FFFD` before our code ever sees it. The four UTF-8 bytes of 🍕
(F0 9F 8D 95) become four replacement chars, get signed into the kind:7
content, and the test fails on receipt.

The only place we can set the encoding from is the launching shell.
Patch the gradle-generated start scripts (POSIX `amy`, Windows
`amy.bat`) right after `:cli:startScripts`:

* POSIX: if neither `LANG` nor `LC_ALL` is set, `export LANG=C.UTF-8`
  before invoking Java. Existing locale settings are respected.
* Windows: `chcp 65001` to switch the console to UTF-8 before Java
  runs.

Tested manually: with `LANG=` (sandbox default) `amy` previously sent
`argv[0]=????` for an emoji argument; with the patch it sends `🍕` and
`sun.jnu.encoding=UTF-8`, and whitenoise accepts the kind:7 reaction.

The interop test 09 polling also needed a fix unrelated to the
encoding bug: wn aggregates kind:7 reactions onto the anchor message
under `.reactions.by_emoji.<emoji>` instead of surfacing them as
standalone messages whose `.content` is the emoji, so the previous
`wait_for_message B "$mls_gid" "🍕"` would have failed even with a
correctly-decoded reaction. The new poll inspects each message's
`.reactions.by_emoji` keys for the emoji literal.

Marmot interop score: 15/16 → **16/16** 🎉

https://claude.ai/code/session_013VYkpz8P1mPh9Ejxy9anhJ
2026-04-25 02:53:38 +00:00
Claude 359b6069f1 fix(marmot): handle being removed mid-commit without OOM (test 14)
When wn admin-removes A in interop test 14, A processed the proposal
locally — `tree.removeLeaf(A.leafIndex)` blanked her leaf and shrank
`tree.leafCount` past `myLeafIndex` — and then immediately walked into

    BinaryTree.directPath(myLeafIndex, tree.leafCount)

at MlsGroup.processCommitInner. With `myLeafIndex >= leafCount`, the
left-balanced parent walk had no valid stopping point: each recursion
step doubled the candidate parent index, integer-overflowed past 2^31,
and either returned garbage or kept appending to the result list until
the JVM OOM'd. The OOM bubbled up as a `runBlocking` failure that
rolled back the whole commit — so A also stayed locally convinced she
was still a member, and the test timed out waiting for `not_member`.

Three layered fixes so the failure mode can't recur:

* `BinaryTree.parent`/`directPath`/`copath` now `require` an in-range
  input. The root and any node ≥ nodeCount used to silently loop or
  return garbage; now they throw `IllegalArgumentException` immediately.
  This is defense-in-depth — a future caller passing an invalid index
  gets a stack trace at the boundary instead of an OOM five frames
  deep.

* `MlsGroup.processCommitInner` short-circuits the path-decrypt + epoch
  advancement when the proposals in the commit just removed *us*. We
  preserve the proposal-side tree mutations so the caller (and the
  outer state machine) can observe that we're out, clear pending
  proposals + sent keys, and return. Without this short-circuit the
  function would derive a bogus all-zero `commit_secret`, fail
  `confirmation_tag` verification, throw, and roll the snapshot back —
  leaving us still convinced we were a live member.

* `MlsGroupManager.isMember` and a new `MlsGroup.isLocalMember()`
  helper now return false once our leaf is null or past `leafCount`.
  The post-Remove group entry still lives in `groups` so callers can
  inspect the final tree, but every cli command (`group show`,
  `message send`, etc.) sees `not_member` and returns the right error.

Also add a comprehensive `BinaryTreeTest` covering non-power-of-2
trees (3, 5, …, 32 leaves) and the boundary cases (root has no
parent, leafIndex ≥ leafCount must throw). The pre-existing tests
only exercised the 4-leaf example from RFC 9420 Appendix C; nothing
hit the `parentInRange` branch, which is exactly where the OOM lived.

Test 14's bash polling needed a small companion fix: it captured
amy's stderr through `2>&1` and fed the result to `jq`, but the
captured stream is interleaved with quartz `Log.d(…)` debug lines,
so the JSON parse always failed. Switch to a `grep` for the
`"error":"not_member"` literal — that signature only appears in
the JSON payload and survives the debug noise. Also tee the
captured output into `$LOG_FILE` so post-mortem logs include each
polling iteration's `[cli] ingest …` traces.

Marmot interop score: 11/16 → 14/16 (tests 9, 15 are unrelated MLS
issues — see follow-up).

https://claude.ai/code/session_013VYkpz8P1mPh9Ejxy9anhJ
2026-04-25 01:32:08 +00:00
Claude c74bbb8510 test(marmot-interop): unwrap newer whitenoise-rs groups show shape
Whitenoise-rs ≥ v0.2.x nests the group object one level deeper —
`{"result": {"group": {...}}}` instead of `{"result": {...}}`. The two
metadata-name pollers in tests 07 and 10 still asked for `.result.name`
and silently saw the empty string, so even when wn-side `groups show`
returned the correct new name the polling loops timed out:

    07 metadata rename       fail   B saw name="" not "Interop-02-renamed"
    10 concurrent commits    fail   diverged: A sees "race-from-amethyst", B sees ""

Both queries now also peel a `.group` wrapper when present, leaving the
older bare-`result` shape working too. Re-running the headless harness
flips 07 + 10 from fail → pass; the remaining failures (09, 14, 15) are
unrelated MLS-encryption / Remove-commit issues that need their own fix.

https://claude.ai/code/session_013VYkpz8P1mPh9Ejxy9anhJ
2026-04-24 23:56:47 +00:00
Claude 655e56791b Merge branch 'main' into claude/review-marmot-implementation-BSrzC
# Conflicts:
#	amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AllSettingsScreen.kt
#	cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/Commands.kt
2026-04-23 22:36:29 +00:00
Claude 43257cb895 Merge remote-tracking branch 'origin/main' into claude/nip17-dm-cli-plan-vvtb5 2026-04-23 21:40:24 +00:00
Claude b8a642b406 fix(cli/tests/dm): bind relay to 127.0.0.2 + dm-06 → --since filter
Two fixes needed to make the DM harness actually pass end-to-end:

1. Bind the loopback relay to 127.0.0.2 instead of 127.0.0.1.

   Quartz's `RelayTag.parse` (called from `ChatMessageRelayListEvent.
   relays()`) rejects any URL for which `isLocalHost()` returns true —
   the substring check matches `"127.0.0.1"` among others. That means a
   kind:10050 event whose `relay` tag reads `ws://127.0.0.1:8090/`
   silently parses to an empty relay list, and `RecipientRelayFetcher`
   reports `dmInbox == []`. Under strict mode `dm send` then correctly
   refuses with `no_dm_relays` — hiding an otherwise-valid test setup.

   `127.0.0.2` is the cleanest fix: still pure loopback (no network
   traffic, no config needed), not matched by `isLocalHost()`, and
   Linux binds any IP in 127.0.0.0/8 without setup. The Marmot harness
   is unaffected because its tests never depend on parsing their own
   kind:10051/10002 relay tags back out — they hit the bootstrap
   fallback path.

   marmot/setup.sh's `start_local_relay` now reads `${RELAY_HOST:-
   127.0.0.1}` so both harnesses can share it; the DM harness sets
   `RELAY_HOST=127.0.0.2` by default and exposes `--host` as an escape
   hatch.

2. Replace dm-06 cursor-advance check with `--since` filter check.

   The original assertion ("second no-flag `dm list` returns 0") was
   wrong for the actual design: `filterGiftWrapsToPubkey` always
   subtracts a 2-day lookback from `since` (required to catch NIP-59's
   randomised `created_at`), so a repeat no-flag query legitimately
   re-pulls everything in the last two days. The cursor advances the
   scan window for efficiency, not for idempotency. dm-06 now verifies
   `--since` actually filters: query with `--since <newest + 2 days +
   1 hour>` — after the filter's lookback subtraction, the effective
   floor lands past every message, and the list comes back empty.

Also: added `protoc` to the DM preflight checks so the missing-dep
error comes before 4 failed cargo retries.

Result on this machine: 6/6 pass, two clean runs in a row.
2026-04-23 21:32:39 +00:00
Claude b0698e0a66 test(cli): move tests/ out of tools/, separate marmot vs dm
`tools/` is for libraries (arti-build); shell-based interop harnesses
that drive the `amy` binary belong with the CLI module. New layout:

  cli/tests/
  ├── lib.sh                       # shared logging + result tracking
  ├── headless/helpers.sh          # shared amy_a / amy_json / assertions
  ├── marmot/                      # MLS group-messaging interop (vs whitenoise-rs)
  │   ├── marmot-interop.sh
  │   ├── marmot-interop-headless.sh
  │   ├── setup.sh
  │   ├── tests-create.sh
  │   ├── tests-manage.sh
  │   ├── tests-extras.sh
  │   └── patches/
  └── dm/                          # NIP-17 DM interop (amy ↔ amy)
      ├── dm-interop-headless.sh
      ├── setup.sh
      └── tests-dm.sh

Per-suite changes:
- Each suite owns its own setup.sh; previously the Marmot setup was at
  headless/setup.sh and the DM setup at headless/setup-dm.sh, which
  implied shared infra they don't actually share.
- `cli/tests/lib.sh` and `cli/tests/headless/helpers.sh` are the only
  shared bits across suites.
- The DM harness still reuses Marmot's `start_local_relay` /
  `stop_local_relay` (sourced via `../marmot/setup.sh`) — same relay
  binary, no duplication.
- All sourcing paths updated; REPO_ROOT now climbs three levels (was
  two when the harness lived under `tools/`); patches/ is now a sibling
  of marmot/setup.sh rather than under marmot/headless/.
- `.gitignore` updated to cover both suites' state dirs.

cli/ROADMAP.md and cli/tests/README.md updated to point at the new paths.
No behavioural change — every test runs the same code; only the file
layout and source paths moved.
2026-04-23 20:37:22 +00:00