Commit Graph

8 Commits

Author SHA1 Message Date
Claude e17ef42e54 fix(cli): rename global --name to --account to free --name for subcommands
The marmot harness surfaced the bug on first run: amy stripped
`marmot group create --name "Interop-02"` thinking the global
account selector ran into the group's display-name flag. Result:
amy resolved the "Interop-02" account, found no identity.json, and
errored — no group ever got created.

Renames the global account-selector flag to `--account`. The per-
subcommand `--name` flags (`marmot group create --name "Demo"`,
`profile edit --name "Alice"`, `marmot await group --name X`) are
untouched — they're free of the global parser now that it doesn't
claim the same name.

Sweep:
- `Main.kt`: GlobalFlag.NAME → ACCOUNT, long "--account".
- `Config.kt`: DataDir.resolve param renamed nameFlag → accountFlag;
  every error message points the user at --account.
- `UseCommand.kt`: error hint says `amy --account NAME init`.
- All test wrappers + direct $AMY_BIN calls under cli/tests/ swap
  the global `--name X` for `--account X` (subcommand --name kept
  exactly where it appeared).
- README + DEVELOPMENT updated.
2026-04-25 15:26:17 +00:00
Claude 99586fbe7e feat(cli): drop --data-dir; tests isolate via $HOME override
There is no installed base to protect, so the self-contained
`--data-dir P` escape hatch goes away entirely. amy now has exactly
one layout — the production multi-account one — and tests exercise
that same code path. Drops one global flag, one DataDir construction
mode, and the "tests use a different code path than users" footgun.

Layout (unchanged from the previous commits — just the only mode now):

    ~/.amy/
    ├── current                      # `amy use NAME` marker
    ├── shared/
    │   └── events-store/            # FsEventStore, one per machine
    ├── alice/
    │   ├── identity.json
    │   ├── state.json
    │   ├── aliases.json             # {"alice": "<own npub>"} after init
    │   └── marmot/
    └── bob/ ...

`DataDir.DEFAULT_ROOT` reads `$HOME` directly (falling back to
`user.home`) because JDK 21 resolves `user.home` via getpwuid and
ignores `$HOME` — which would have broken the standard CLI test
isolation pattern of `HOME=/tmp/foo amy …` (the same convention
git, gpg, npm follow).

Test sweep:

- `cli/tests/headless/helpers.sh`, `cli/tests/dm/setup.sh`,
  `cli/tests/cache/cache-headless.sh` wrappers all switch to
  `HOME=$STATE_DIR amy --name X …`.
- `ensure_identity_for` drops its `dir` parameter; the function and
  every harness call site go through `--name` only.
- `A_DIR`/`B_DIR`/`D_DIR` get repointed at `$STATE_DIR/.amy/X`. The
  one consumer (`cache-headless.sh`'s T6 `relays.json` check) still
  works since it's just a path probe.
- `cli/tests/dm/tests-dm.sh` ghost identities get their own
  short-lived `$HOME` so they don't pollute the main test root.

Cache-test T4 inverts: pre-shared-store, "B has not seen A → first
profile show is a relay miss, second is a cache hit" tested
per-account caching. With one shared events-store, B's first lookup
of A is already a cache hit because A wrote kind:0 there during
bootstrap. T4 now asserts that — drops the stale "second lookup hits
cache" half. T7 (no-identity maintenance verbs) gets a fresh fake
`$HOME` plus a throwaway `--name` so the empty store has no inherited
events.

Docs (README + DEVELOPMENT) rewritten to match — quick-start now uses
`amy --name alice create`, the on-disk-layout section shows the new
tree, and the global-flags table replaces `--data-dir` with `--name`
plus the new `amy use` verb.
2026-04-25 15:07:31 +00:00
Claude 03eb7be509 feat(cli): default amy stdout to human-readable text; --json opts in
amy's default stdout is now a YAML-ish render of the underlying result
map; the previous single-line JSON contract moves behind a global
`--json` flag. Errors mirror the same rule (`error: <code>: <detail>`
on stderr by default, JSON `{"error":...,"detail":...}` under --json).
Exit codes (0/1/2/124) and the --json shape itself are unchanged —
only the default presentation flips.

- Replaces Json.writeLine / Json.error with mode-aware
  Output.emit / Output.error. The same Jackson mapper is reused for
  on-disk JSON via Output.mapper.
- Adds `--json` to the global flag set in Main.kt; honoured even when
  argument parsing fails so error JSON keeps shape under --json.
- Updates the test harness wrappers (amy_a / amy_b / amy_d in
  cli/tests/{headless,dm,cache}/) and the few direct $AMY_BIN call
  sites whose stdout is consumed via $() / 2>&1 — they now pass
  --json so the existing jq pipelines keep working.
- Rewrites the README "Output contract" and DEVELOPMENT design
  principles to describe the new default, and clarifies that only
  --json is the public API; the text shape is allowed to drift.
2026-04-25 12:23:37 +00:00
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 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