From b8e11582f57386a0963568322fe4d99fe6c85da7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:15:39 +0000 Subject: [PATCH 01/11] chore: open auto-fix batch claude/friendly-maxwell-m1efN From 232b296ee515e77b49ba960202d8f1a892a24123 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:16:56 +0000 Subject: [PATCH 02/11] chore(ci): prune 3 stale RUSTSEC ignores from cargo-audit The cargo-audit --ignore list contained three IDs that no longer match any active advisory and were silently dead-weight: - RUSTSEC-2026-0098 (rustls-webpki name-constraint) - RUSTSEC-2026-0099 (rustls-webpki URI-constraint) - RUSTSEC-2026-0104 (rustls-webpki CRL panic) Retains the 7 still-active entries flagged by `cargo audit` at HEAD 6404719: - RUSTSEC-2023-0089 (atomic-polyfill unmaintained, #318) - RUSTSEC-2024-0370 (proc-macro-error unmaintained, #317) - RUSTSEC-2024-0436 (paste unmaintained, #316) - RUSTSEC-2025-0141 (bincode 1.x unmaintained, #247) - RUSTSEC-2026-0097 (rand unsoundness, #246) - RUSTSEC-2026-0119 (hickory-proto O(n^2) name compression, #508) - RUSTSEC-2026-0120 (hickory-net NSEC3 unbounded loop, #509) Verification: - YAML still parses (python3 yaml.safe_load). - Extracted run script passes `bash -n` and expands to exactly 7 --ignore flags when the cargo invocation is dry-run. - `cargo audit` itself unavailable in the sandbox; the CI cargo-audit job remains the regression test. Refs #592 --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0bf9c8b8..38220fa3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -183,9 +183,6 @@ jobs: - name: Run cargo audit run: | cargo audit \ - --ignore RUSTSEC-2026-0098 `# rustls-webpki name-constraint (#223)` \ - --ignore RUSTSEC-2026-0099 `# rustls-webpki URI-constraint (#223)` \ - --ignore RUSTSEC-2026-0104 `# rustls-webpki CRL panic (#223)` \ --ignore RUSTSEC-2026-0097 `# rand unsoundness (#246)` \ --ignore RUSTSEC-2025-0141 `# bincode 1.x unmaintained (#247)` \ --ignore RUSTSEC-2024-0436 `# paste unmaintained, via leptos+iroh (#316)` \ From 127af73758d436867732dee08326c408143196c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:20:53 +0000 Subject: [PATCH 03/11] test(e2e): parameterise longPressAvatar over LONG_PRESS_MS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `longPressAvatar` previously held the mouse down for a bare `500` ms literal, with no link to the product-side threshold. If `HOLD_MS` ever shifts above 500 ms — or the runner is under enough load that the 500 ms wall-clock wait elapses before the product timer fires — the helper produces false negatives (issue #591). Mirror the product threshold as `LONG_PRESS_MS` (350 ms — source: `HOLD_MS` in `crates/web/src/components/long_press.rs:17`) and add `LONG_PRESS_BUFFER_MS` (250 ms) to absorb scheduler jitter under CI load. The default duration is now `LONG_PRESS_MS + LONG_PRESS_BUFFER_MS` = 600 ms, with `durationMs` exposed as an optional override. Considered option (b) from the audit — `page.clock.runFor(...)` like `longPressWithClock` already does — but rejected as too invasive: it would require installing `page.clock` in every caller. The constants mirror co-locates the timing knob with the product threshold and is a one-line change at the call site. Refs #591 --- e2e/helpers/touch.ts | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/e2e/helpers/touch.ts b/e2e/helpers/touch.ts index 71e3c4f0..250648a4 100644 --- a/e2e/helpers/touch.ts +++ b/e2e/helpers/touch.ts @@ -7,6 +7,27 @@ import { Page, Locator } from '@playwright/test'; import { isMobile, visibleShell, openMemberList } from './ui'; +/** + * Mirrors the product-side long-press hold threshold. + * + * Source of truth: `HOLD_MS` in `crates/web/src/components/long_press.rs` + * (currently 350 ms; spec: `docs/specs/2026-04-19-ui-design/trust-verification.md` + * §Long-press SAS on mobile). + * + * Keep this in sync with the product constant. If `HOLD_MS` moves, this + * must move too — otherwise `longPressAvatar` will arm-or-not based on + * whatever buffer is left, producing flaky false negatives (issue #591). + */ +export const LONG_PRESS_MS = 350; + +/** + * Buffer added on top of {@link LONG_PRESS_MS} to absorb scheduler jitter + * under CI load. The total wait is `LONG_PRESS_MS + LONG_PRESS_BUFFER_MS`, + * which crosses the product threshold deterministically without coupling + * tests to the exact threshold value. + */ +export const LONG_PRESS_BUFFER_MS = 250; + /** Simulate a long-press on an element to open the mobile action sheet. * Prefixes the selector with the visible-shell scope so a raw `.message` * picks the mobile copy, not the hidden desktop one. */ @@ -137,8 +158,20 @@ export async function longPressWithClock(page: Page, selector: string, durationM await page.clock.runFor(300); } -/** Long-press a peer avatar by name in the member list (mobile only). */ -export async function longPressAvatar(page: Page, peerName: string) { +/** Long-press a peer avatar by name in the member list (mobile only). + * + * The hold duration is parameterised over {@link LONG_PRESS_MS} (mirror + * of the product's `HOLD_MS`) plus {@link LONG_PRESS_BUFFER_MS}. Callers + * may override `durationMs` if they need a different timing, but the + * default crosses the product threshold deterministically — no longer + * coupled to a bare `500` literal that could fall under the threshold + * if `HOLD_MS` ever shifts up (issue #591). + */ +export async function longPressAvatar( + page: Page, + peerName: string, + durationMs: number = LONG_PRESS_MS + LONG_PRESS_BUFFER_MS, +) { await openMemberList(page); const member = page.locator(`${visibleShell(page)} .member-item`, { hasText: peerName }); await member.waitFor({ timeout: 10_000 }); @@ -147,7 +180,7 @@ export async function longPressAvatar(page: Page, peerName: string) { if (!box) throw new Error('avatar not measurable'); await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); await page.mouse.down(); - await page.waitForTimeout(500); + await page.waitForTimeout(durationMs); await page.mouse.up(); } From d0120c04ab789dbf44740478291e760e535dd700 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:23:03 +0000 Subject: [PATCH 04/11] test(e2e): drop bare waitForTimeout from longPress post-touchend The trailing `await page.waitForTimeout(300)` after dispatching `touchend` was an unobservable settle. Every real caller already waits on the gesture's outcome (action-sheet visibility), so the 300 ms tax was pure dead weight. Brainstorm (Option A vs B): - Option A (chosen): drop the wait entirely; require callers to `waitFor` whatever the press should produce. Survey of all 6 call sites (mobile-actions.spec.ts x4, helpers/ui.ts messageAction + reactToMessage) confirmed each one already does `expect(.shell-mobile .mobile-action-sheet.open).toBeVisible({...})` or `.waitFor({...})` immediately after the longPress call. No caller needs a new waitFor added. helpers.barrel.test.ts only `void`s the symbol for type-only smoke testing. - Option B (rejected): bake `await page.locator('.shell-mobile .mobile-action-sheet.open').waitFor(...)` into the helper. Self- contained for the common case, but couples a generic gesture helper to one specific outcome, breaking any future caller that long-presses for a non-sheet flow (peer-card SAS, custom context menu, etc.). longPress is a gesture primitive, not an action-sheet opener. Files modified: - e2e/helpers/touch.ts: remove `waitForTimeout(300)` after touchend in the real-time `longPress`. `longPressWithClock` and `longPressAvatar` untouched (their waits are correct / already fixed under #591). Refs #590 https://claude.ai/code/session_01V5SXuK7UbHJJEbkhwLR65B --- e2e/helpers/touch.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/e2e/helpers/touch.ts b/e2e/helpers/touch.ts index 250648a4..900e3206 100644 --- a/e2e/helpers/touch.ts +++ b/e2e/helpers/touch.ts @@ -86,7 +86,10 @@ export async function longPress(page: Page, selector: string, durationMs = 600) })); }, { x, y }); - await page.waitForTimeout(300); + // No post-touchend settle: callers must `waitFor` whatever the press + // produces (typically `.shell-mobile .mobile-action-sheet.open`). A + // bare `waitForTimeout` here was unobservable dead weight and coupled + // the gesture helper to a single outcome (issue #590). } /** From 076b848b28576529b8a62bd7f568a1caf0b71b5f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:30:56 +0000 Subject: [PATCH 05/11] test(e2e): replace bare waitForTimeout with waitFor in helpers/ui.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrated 14 of 16 bare `page.waitForTimeout` calls in e2e/helpers/ui.ts to selector-based `waitFor` per docs/specs/2026-04-27-event-based-waits-design.md. Pre-edit count was 16 sites; post-edit 2 remain with `TODO(#589)` markers where no observable destination selector exists. Migration recipes applied: - switchTab (1 site): wait for `[aria-selected="true"]` on the target tab button after click. - messageAction mobile (1 site): wait for `.mobile-action-sheet.open` to be hidden after sheet-item click. - messageAction desktop (3 sites): post-hover wait for `.action-trigger` visible; post-trigger-click wait for `.dropdown-item` visible; post-item-click wait for `.dropdown-item` hidden. - editMessage (1 site): post-Enter wait for `.edit-bar` to unmount. - deleteMessage (1 site): post-confirm wait for `.confirm-dialog` to unmount. - reactToMessage mobile (1 site): wait for `.mobile-action-sheet.open` hidden after emoji tap. - reactToMessage desktop (4 sites): same hover→trigger→dropdown→ emoji-row pattern as messageAction; post-emoji wait for `.dropdown-item` hidden. - kickPeer (2 sites): dropped redundant 500ms before `confirmBtn.waitFor`; post-confirm wait for `.confirm-dialog` unmount. Sites left with TODO(#589) — no-condition wait, needs design: - trustPeer / untrustPeer (2 sites): `propose_grant_admin` / `propose_revoke_admin` go through proposed-action voting; the local DOM does not change synchronously after the click, so there is no immediate destination selector to wait on. Callers verify trust state separately. Migrating these requires either a state-event hook (Peer.nextEvent) or a sync-aware helper, both out of scope for this mechanical migration. Wait-timeout ratchet baseline updated from 39 → 10 to lock in the improvement. Refs #589 --- e2e/.wait-timeout-baseline | 2 +- e2e/helpers/ui.ts | 71 ++++++++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/e2e/.wait-timeout-baseline b/e2e/.wait-timeout-baseline index a2720097..f599e28b 100644 --- a/e2e/.wait-timeout-baseline +++ b/e2e/.wait-timeout-baseline @@ -1 +1 @@ -39 +10 diff --git a/e2e/helpers/ui.ts b/e2e/helpers/ui.ts index 767f12e7..ec0fd33f 100644 --- a/e2e/helpers/ui.ts +++ b/e2e/helpers/ui.ts @@ -158,7 +158,12 @@ export async function switchTab( ) { if (!isMobile(page)) return; await page.locator(`.mobile-tab-bar .tab[data-tab="${tabId}"]`).click(); - await page.waitForTimeout(200); + // Tab activation flips `aria-selected` on the target button — wait for + // that rather than a fixed sleep so the transition settles before the + // caller starts interacting with the new tab's panel. + await page + .locator(`.mobile-tab-bar .tab[data-tab="${tabId}"][aria-selected="true"]`) + .waitFor({ state: 'visible', timeout: 3_000 }); } /** Opens the member list in the right rail. On desktop clicks the @@ -256,16 +261,24 @@ export async function messageAction(page: Page, messageText: string, actionName: await page .locator('.shell-mobile .mobile-action-sheet.open .sheet-item', { hasText: actionRe }) .click(); - await page.waitForTimeout(300); + // Sheet item drops the `.open` class — wait for the open-sheet + // selector to disappear instead of a fixed sleep. + await page + .locator('.shell-mobile .mobile-action-sheet.open') + .waitFor({ state: 'hidden', timeout: 3_000 }); } else { const msg = page.locator('.shell-desktop .message', { hasText: messageText }).last(); // Desktop: hover to reveal action trigger, click dropdown item. await msg.hover(); - await page.waitForTimeout(200); + await msg.locator('.action-trigger').waitFor({ state: 'visible', timeout: 3_000 }); await msg.locator('.action-trigger').click(); - await page.waitForTimeout(200); + await page + .locator('.dropdown-item', { hasText: actionName }) + .waitFor({ state: 'visible', timeout: 3_000 }); await page.locator('.dropdown-item', { hasText: actionName }).click(); - await page.waitForTimeout(200); + // Clicking a dropdown-item closes the dropdown (set_show_dropdown + // → false in message.rs); gate on the items unmounting. + await page.locator('.dropdown-item').first().waitFor({ state: 'hidden', timeout: 3_000 }); } } @@ -275,7 +288,10 @@ export async function editMessage(page: Page, originalText: string, newText: str const input = page.locator('.input-area input, .input-area textarea').first(); await input.fill(newText); await input.press('Enter'); - await page.waitForTimeout(500); + // Submitting clears the editing signal — the `.edit-bar` (rendered + // only while editing in input.rs) unmounts. Gate on that instead of + // a fixed sleep. + await page.locator(`${visibleShell(page)} .edit-bar`).waitFor({ state: 'hidden', timeout: 5_000 }); } /** Deletes a message (desktop or mobile). */ @@ -285,7 +301,9 @@ export async function deleteMessage(page: Page, text: string) { const confirmBtn = page.locator('.confirm-dialog .btn-danger', { hasText: 'Delete' }); await confirmBtn.waitFor({ timeout: 3000 }); await confirmBtn.click(); - await page.waitForTimeout(500); + // Confirmation closes the dialog — wait for the overlay to unmount + // rather than a fixed sleep. + await page.locator('.confirm-dialog').waitFor({ state: 'hidden', timeout: 5_000 }); } /** Reacts to a message with an emoji (desktop or mobile). */ @@ -296,17 +314,30 @@ export async function reactToMessage(page: Page, messageText: string, emojiIndex .waitFor({ timeout: 3000 }); await page.locator('.shell-mobile .mobile-action-sheet.open .sheet-emoji-row button') .nth(emojiIndex).click(); - await page.waitForTimeout(500); + // Tapping a quick-emoji button closes the sheet — gate on the + // open-class disappearing instead of a fixed sleep. + await page + .locator('.shell-mobile .mobile-action-sheet.open') + .waitFor({ state: 'hidden', timeout: 3_000 }); } else { const msg = page.locator('.shell-desktop .message', { hasText: messageText }).last(); await msg.hover(); - await page.waitForTimeout(200); + await msg.locator('.action-trigger').waitFor({ state: 'visible', timeout: 3_000 }); await msg.locator('.action-trigger').click(); - await page.waitForTimeout(200); + await page + .locator('.dropdown-item', { hasText: 'React' }) + .waitFor({ state: 'visible', timeout: 3_000 }); await page.locator('.dropdown-item', { hasText: 'React' }).click(); - await page.waitForTimeout(200); + // Clicking React reveals the dropdown emoji row — wait for the + // first emoji button to mount before clicking by index. + await page + .locator('.dropdown-emoji-row button') + .first() + .waitFor({ state: 'visible', timeout: 3_000 }); await page.locator('.dropdown-emoji-row button').nth(emojiIndex).click(); - await page.waitForTimeout(500); + // Selecting an emoji closes the dropdown — gate on the items + // unmounting rather than a fixed sleep. + await page.locator('.dropdown-item').first().waitFor({ state: 'hidden', timeout: 3_000 }); } } @@ -319,6 +350,12 @@ export async function trustPeer(page: Page, peerName: string) { await member.hover(); // Use a regex to avoid matching "Untrust" when looking for "Trust". await member.locator('button').filter({ hasText: /^Trust$/ }).click(); + // TODO(#589): no-condition wait — needs design. Trust dispatches a + // `propose_grant_admin` event whose effect (target peer's row picking + // up the "Trusted" badge) only materialises once the proposed-action + // vote applies, which is async / multi-peer dependent. The button + // itself doesn't change locally on click. Callers verify post-trust + // behaviour separately, so the prior 500ms sleep was a fudge factor. await page.waitForTimeout(500); await closeMemberList(page); } @@ -331,6 +368,9 @@ export async function untrustPeer(page: Page, peerName: string) { // Hover to reveal action buttons (desktop hides them until hover). await member.hover(); await member.locator('button', { hasText: 'Untrust' }).click(); + // TODO(#589): no-condition wait — needs design. Same reasoning as + // `trustPeer` above: `propose_revoke_admin` is async and the local + // row doesn't flip until the proposal vote applies. await page.waitForTimeout(500); await closeMemberList(page); } @@ -377,11 +417,14 @@ export async function kickPeer(page: Page, peerName: string) { // Hover to reveal action buttons (desktop hides them until hover). await member.hover(); await member.locator('.btn-danger', { hasText: 'Kick' }).click(); - await page.waitForTimeout(500); + // The Kick click opens the confirm dialog; the next `confirmBtn.waitFor` + // already gates on it appearing, so the prior 500ms sleep was redundant. // Confirm the kick dialog. const confirmBtn = page.locator('.confirm-dialog .btn-danger', { hasText: 'Kick' }); await confirmBtn.waitFor({ timeout: 5_000 }); await confirmBtn.click(); - await page.waitForTimeout(500); + // Confirmation closes the dialog — gate on the overlay unmounting + // rather than a fixed sleep before closing the member list. + await page.locator('.confirm-dialog').waitFor({ state: 'hidden', timeout: 5_000 }); await closeMemberList(page); } From 84d4a71705d04ed3be9a6554f75124f8e0005fdc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:33:06 +0000 Subject: [PATCH 06/11] test(e2e): drop permanent skip on mobile compare-sheet long-press MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the `test('mobile long-press opens the compare sheet', ...)` block from `e2e/permissions.spec.ts` (previously around lines 239-253), which carried an unconditional `test.skip(true, 'mobile member-list surface deferred')` after a mobile-project gate. The body still ran `setupTwoPeers` + `longPressAvatar` setup but the assertion never fired, masquerading as coverage that did not exist. Also drops the now-unused `longPressAvatar` import and refreshes the stale top-of-describe comment that claimed "long-press / compare-sheet tests below opt back in explicitly" — the remaining compare-sheet tests are desktop-only. Mobile compare-sheet long-press regression coverage will be restored once the mobile member-list surface ships. Tracked at #595. Refs #588 --- e2e/permissions.spec.ts | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/e2e/permissions.spec.ts b/e2e/permissions.spec.ts index 7c8906c2..64a0d79b 100644 --- a/e2e/permissions.spec.ts +++ b/e2e/permissions.spec.ts @@ -9,7 +9,6 @@ import { openCompareFingerprints, markFingerprintsMatch, markFingerprintsMismatch, - longPressAvatar, visibleShell, } from './helpers'; @@ -29,10 +28,9 @@ test.describe('Permissions and trust', () => { // Mobile member-list surface is deferred to a later phase (Phase 1b // shipped the mobile shell without the right-rail members pane). - // Kick tests go through `.member-item`, which only renders on desktop - // today. Long-press / compare-sheet tests below opt back in explicitly - // since they drive trust via the compare-fingerprints sheet, not the - // member list. + // Kick + compare-sheet tests go through `.member-item`, which only + // renders on desktop today, so they're skipped on mobile projects. + // Mobile long-press coverage tracked in #595. // // Trust / untrust tests that used to live here (Unknown → Verified // and badge-render contracts) moved to: @@ -235,20 +233,4 @@ test.describe('Permissions and trust', () => { await ctx2.close(); } }); - - test('mobile long-press opens the compare sheet', async ({ browser }, testInfo) => { - test.skip(!testInfo.project.name.startsWith('mobile'), 'mobile-chrome path'); - // Long-press targets a member avatar; the mobile member surface - // lands in a follow-up phase. Skip until that ships. - test.skip(true, 'mobile member-list surface deferred'); - const { ctx1, ctx2, page1 } = await setupTwoPeers(browser, 'LongPress', 'Alice', 'Bob'); - try { - await longPressAvatar(page1, 'Bob'); - await expect(page1.locator('.add-friend__card[role="dialog"]')) - .toBeVisible({ timeout: 10_000 }); - } finally { - await ctx1.close(); - await ctx2.close(); - } - }); }); From 41736c0ba81e1e473e4c72417ae645b9ac354100 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:39:27 +0000 Subject: [PATCH 07/11] fix(web): log ICE candidate handle errors Replace `let _ = vm.borrow().handle_ice_candidate(&from, json);` at crates/web/src/event_processing.rs:128 with an explicit `if let Err(e)` branch that emits `tracing::warn!(?e, "handle_ice_candidate failed")`. `VoiceManager::handle_ice_candidate` returns `Result<(), String>`, so silently dropping its error meant any failure during the WebRTC voice- call signaling path (missing connection for peer, malformed JSON, or the underlying `RTCPeerConnection.addIceCandidate` rejecting) vanished without a trace. The `handlers.rs` module-doc explicitly calls out `let _ = ...` over `Result` as an antipattern; this brings the ICE branch in line with that guidance. No test added: triggering the failure path requires a JS-runtime mock that makes `addIceCandidate` reject deterministically, which is out of scope for an error-logging fix. Refs #585 --- crates/web/src/event_processing.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/web/src/event_processing.rs b/crates/web/src/event_processing.rs index 77b97ba6..86f02348 100644 --- a/crates/web/src/event_processing.rs +++ b/crates/web/src/event_processing.rs @@ -125,7 +125,9 @@ pub fn process_event_batch( wasm_bindgen_futures::spawn_local(handle_voice_answer(vm, from, s)); } VoiceSignalPayload::IceCandidate(json) => { - let _ = vm.borrow().handle_ice_candidate(&from, json); + if let Err(e) = vm.borrow().handle_ice_candidate(&from, json) { + tracing::warn!(?e, "handle_ice_candidate failed"); + } } } } From 7a975fabf1b3f333e91f9d3592cf58d45218377b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:41:12 +0000 Subject: [PATCH 08/11] fix(web): prefer string extraction for FileReader JsValue errors The FileReader result-error closure at crates/web/src/components/ file_share.rs:69 logged the JsValue with Debug formatting, which renders opaquely as `JsValue(Object)`. Switch to the established pattern: try `as_string()` first (handles string-typed JS errors cleanly) and fall back to `{e:?}` only when the value is not a string. The log now uses a structured `error` field so the extracted message is searchable. Sites migrated: 1 (the FileReader::result Err arm). The other `tracing::error!` calls in this file do not capture a JsValue, so they are left untouched. Refs #587 --- crates/web/src/components/file_share.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/web/src/components/file_share.rs b/crates/web/src/components/file_share.rs index 55b6d59f..9f4b7dd2 100644 --- a/crates/web/src/components/file_share.rs +++ b/crates/web/src/components/file_share.rs @@ -66,7 +66,8 @@ pub fn FileShareButton(channel: ReadSignal) -> impl IntoView { let result = match reader_clone.result() { Ok(r) => r, Err(e) => { - tracing::error!("FileReader result error: {e:?}"); + let msg = e.as_string().unwrap_or_else(|| format!("{e:?}")); + tracing::error!(error = %msg, "FileReader result failure"); return; } }; From bca6b7a3cb1d5e4b7fbdc14f3d5e66f3609f14f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:51:28 +0000 Subject: [PATCH 09/11] feat(state): add Display for ProposedAction and VoteThreshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously crates/client/src/mutations.rs:948 built the `ClientEvent::ProposalCreated { action_description, .. }` payload via `format!("{action:?}")`, leaking Rust Debug rendering (e.g. `KickMember { peer_id: EndpointId(...) }`) directly into UI display strings. Add structural `Display` impls for `ProposedAction` and `VoteThreshold` in `crates/state/src/event.rs`, then swap the consumer at `crates/client/src/mutations.rs:948` from `{:?}` to `{}`. Rendered strings: - ProposedAction::GrantAdmin -> "Grant admin to {peer_id}" - ProposedAction::RevokeAdmin -> "Revoke admin from {peer_id}" - ProposedAction::KickMember -> "Kick {peer_id}" - ProposedAction::SetVoteThreshold -> "Set vote threshold to {threshold}" - VoteThreshold::Majority -> "majority" - VoteThreshold::Unanimous -> "unanimous" - VoteThreshold::Count(n) -> "{n} admins" Peer ids render via `EndpointId`'s own Display (64-char hex). Display-name resolution lives in willow-web's state, not willow-state — UI layers that want richer rendering should consume the typed `ProposedAction` directly rather than substring-matching this string. That UI follow-up is out of scope for this fix. Tests: 7 new display tests in crates/state/src/tests/voting.rs covering each ProposedAction variant, each VoteThreshold variant, and the threshold-embedded-in-action case. Tests use `EndpointId::from_bytes(&[0u8; 32])` for a deterministic peer id and dynamically format the expected string with `{peer}` to avoid hard-coding iroh's hex encoding. Refs #571 --- crates/client/src/mutations.rs | 2 +- crates/state/src/event.rs | 35 ++++++++++++++ crates/state/src/tests/voting.rs | 82 ++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/crates/client/src/mutations.rs b/crates/client/src/mutations.rs index 7cdf2c8e..3099cb7e 100644 --- a/crates/client/src/mutations.rs +++ b/crates/client/src/mutations.rs @@ -945,7 +945,7 @@ pub(crate) fn derive_client_events(event: &willow_state::Event) -> Vec { out.push(ClientEvent::ProposalCreated { proposal_hash: event.hash.to_string(), - action_description: format!("{action:?}"), + action_description: format!("{action}"), }); } EventKind::Vote { proposal, accept } => { diff --git a/crates/state/src/event.rs b/crates/state/src/event.rs index 653dc9f1..cf168ebe 100644 --- a/crates/state/src/event.rs +++ b/crates/state/src/event.rs @@ -206,6 +206,41 @@ pub enum VoteThreshold { Count(u32), } +impl std::fmt::Display for ProposedAction { + /// Render a structural, human-readable description of the action. + /// + /// Peer ids are rendered via [`EndpointId`]'s own `Display` (64-char + /// hex). UI layers that want richer rendering (e.g. resolving a peer + /// id to a display name) should consume the typed [`ProposedAction`] + /// directly instead of substring-matching on this string. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProposedAction::GrantAdmin { peer_id } => { + write!(f, "Grant admin to {peer_id}") + } + ProposedAction::RevokeAdmin { peer_id } => { + write!(f, "Revoke admin from {peer_id}") + } + ProposedAction::KickMember { peer_id } => { + write!(f, "Kick {peer_id}") + } + ProposedAction::SetVoteThreshold { threshold } => { + write!(f, "Set vote threshold to {threshold}") + } + } + } +} + +impl std::fmt::Display for VoteThreshold { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + VoteThreshold::Majority => f.write_str("majority"), + VoteThreshold::Unanimous => f.write_str("unanimous"), + VoteThreshold::Count(n) => write!(f, "{n} admins"), + } + } +} + // ───── EventKind ─────────────────────────────────────────────────────────── /// All possible state mutations — 22 variants. diff --git a/crates/state/src/tests/voting.rs b/crates/state/src/tests/voting.rs index 641de106..84b9f305 100644 --- a/crates/state/src/tests/voting.rs +++ b/crates/state/src/tests/voting.rs @@ -455,3 +455,85 @@ fn no_vote_proposal_does_not_auto_apply_with_two_admins() { "target should have been kicked" ); } + +// ───── Display impls (issue #571) ────────────────────────────────────────── +// +// These tests pin the exact rendered text used by the client's +// `ClientEvent::ProposalCreated { action_description, .. }` payload (see +// `crates/client/src/mutations.rs` — the `Propose` arm formats with `{}`). +// Previously that site used `{:?}`, leaking Rust Debug rendering into UI +// strings (e.g. "KickMember { peer_id: EndpointId(...) }"). The structural +// peer-id rendering uses `EndpointId`'s own `Display` (64-char hex). UI +// layers that want display-name resolution should consume the typed +// `ProposedAction` directly rather than substring-matching this string. + +#[cfg(test)] +mod display_tests { + use crate::event::{ProposedAction, VoteThreshold}; + use willow_identity::EndpointId; + + /// Construct a deterministic peer id from fixed bytes. `[0u8; 32]` is + /// accepted by `EndpointId::from_bytes` (it's the curve identity), so + /// the output is stable across test runs. + fn fixed_peer() -> EndpointId { + EndpointId::from_bytes(&[0u8; 32]).unwrap() + } + + #[test] + fn display_grant_admin() { + let peer = fixed_peer(); + let action = ProposedAction::GrantAdmin { peer_id: peer }; + // Use peer.to_string() to avoid hard-coding iroh's hex encoding. + let expected = format!("Grant admin to {peer}"); + assert_eq!(format!("{action}"), expected); + } + + #[test] + fn display_revoke_admin() { + let peer = fixed_peer(); + let action = ProposedAction::RevokeAdmin { peer_id: peer }; + let expected = format!("Revoke admin from {peer}"); + assert_eq!(format!("{action}"), expected); + } + + #[test] + fn display_kick_member() { + let peer = fixed_peer(); + let action = ProposedAction::KickMember { peer_id: peer }; + let expected = format!("Kick {peer}"); + assert_eq!(format!("{action}"), expected); + } + + #[test] + fn display_set_vote_threshold_majority() { + let action = ProposedAction::SetVoteThreshold { + threshold: VoteThreshold::Majority, + }; + assert_eq!(format!("{action}"), "Set vote threshold to majority"); + } + + #[test] + fn display_set_vote_threshold_unanimous() { + let action = ProposedAction::SetVoteThreshold { + threshold: VoteThreshold::Unanimous, + }; + assert_eq!(format!("{action}"), "Set vote threshold to unanimous"); + } + + #[test] + fn display_set_vote_threshold_count() { + let action = ProposedAction::SetVoteThreshold { + threshold: VoteThreshold::Count(3), + }; + assert_eq!(format!("{action}"), "Set vote threshold to 3 admins"); + } + + #[test] + fn display_vote_threshold_variants() { + assert_eq!(format!("{}", VoteThreshold::Majority), "majority"); + assert_eq!(format!("{}", VoteThreshold::Unanimous), "unanimous"); + assert_eq!(format!("{}", VoteThreshold::Count(0)), "0 admins"); + assert_eq!(format!("{}", VoteThreshold::Count(1)), "1 admins"); + assert_eq!(format!("{}", VoteThreshold::Count(42)), "42 admins"); + } +} From 252456841a8ce0aae4268a1aaad7cc462a5e205d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:55:08 +0000 Subject: [PATCH 10/11] fix(web): textarea fallback only on clipboard write rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `copy_to_clipboard` previously discarded the Promise returned by `navigator.clipboard.writeText` and unconditionally ran the textarea + `execCommand('copy')` legacy path. Two side effects: a hidden textarea was momentarily appended/removed even when the modern API succeeded, and a second copy could overwrite the first by ordering. Keep the sync `pub fn copy_to_clipboard(text: &str)` signature — every caller (`components/message.rs`, `components/chat.rs`, `components/settings.rs`, `components/add_server.rs`) invokes it from a non-async Leptos `on:click` closure. Internally, build the Promise from `clipboard.write_text(text)`, await it inside `wasm_bindgen_futures:: spawn_local`, and only run the textarea fallback on `Err`. Happy path is now silent and leaves the DOM untouched. Runner-up considered: make `copy_to_clipboard` async and await each caller. Rejected because every caller is already inside a non-async `on:click` closure — that approach would push `spawn_local` to every call site instead of centralising it once in the helper, and the success/failure state isn't surfaced to callers today. No tests added: a browser-tier integration test would have to mock clipboard rejection (and the underlying behaviour is logging + ordering of an existing fallback), not worth the harness complexity. Refs #572 --- crates/web/src/util.rs | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/crates/web/src/util.rs b/crates/web/src/util.rs index 6689e0bb..81d6b23d 100644 --- a/crates/web/src/util.rs +++ b/crates/web/src/util.rs @@ -1,20 +1,46 @@ /// Copy text to the clipboard. /// /// Tries `navigator.clipboard.writeText` first (modern API, requires HTTPS). -/// Falls back to creating a temporary textarea and using `execCommand('copy')`. +/// Falls back to creating a temporary textarea and using `execCommand('copy')` +/// only if the modern API rejects (non-HTTPS, no user gesture, browser +/// restrictions). The signature stays sync so Leptos `on:click` handlers can +/// call it directly; the Promise returned by `writeText` is awaited inside a +/// `spawn_local` task so the fallback never runs on the happy path. pub fn copy_to_clipboard(text: &str) { - use wasm_bindgen::JsCast; - let Some(window) = web_sys::window() else { return; }; - // Try modern clipboard API first. + // Kick off the modern clipboard API and await its Promise. The textarea + // fallback only runs if the Promise rejects. let clipboard = window.navigator().clipboard(); - let _ = clipboard.write_text(text); + let promise = clipboard.write_text(text); + let owned = text.to_owned(); + wasm_bindgen_futures::spawn_local(async move { + match wasm_bindgen_futures::JsFuture::from(promise).await { + Ok(_) => { + tracing::debug!("clipboard.writeText succeeded"); + } + Err(err) => { + tracing::debug!( + ?err, + "clipboard.writeText rejected; using textarea fallback" + ); + exec_command_copy_fallback(&owned); + } + } + }); +} + +/// Legacy clipboard path: append a hidden textarea, select its contents, +/// invoke `document.execCommand('copy')`, and remove the textarea. Only +/// invoked when the modern `navigator.clipboard.writeText` Promise rejects. +fn exec_command_copy_fallback(text: &str) { + use wasm_bindgen::JsCast; - // Also do the textarea fallback in case clipboard API fails silently - // (e.g. non-HTTPS, no user gesture, browser restrictions). + let Some(window) = web_sys::window() else { + return; + }; let Some(document) = window.document() else { return; }; From 4702486777c1ab3a4f9c61c54607db4a9b1d97c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 16:56:31 +0000 Subject: [PATCH 11/11] docs(skill): codify pre-staged working tree as expected pattern Adds sibling paragraph to the Implementer Agent step 8 sandbox-interference note, capturing the inverse phenomenon: pre-staged uncommitted edits at session start. Observed in ~40-50% of dispatches across PR #566 + #567 runs. Brief implementers to verify + gate + commit (don't redo). Refs #567 --- .claude/skills/resolving-issues/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/resolving-issues/SKILL.md b/.claude/skills/resolving-issues/SKILL.md index 88aa0df2..605016be 100644 --- a/.claude/skills/resolving-issues/SKILL.md +++ b/.claude/skills/resolving-issues/SKILL.md @@ -119,6 +119,8 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps: **Sandbox `git reset --hard origin/` interference (known hazard).** Some sandboxed environments run a periodic `git reset --hard origin/` between tool invocations that silently rolls back uncommitted edits — visible as `Edit`/`Write` results vanishing between cargo commands, or as the working tree being clean when you expected staged changes. Detection: run `git status` after a tool call you expected to leave changes; if it's clean and the file content matches origin, the sandbox wiped it. Recovery: apply edits and `git add -A && git commit` in a tight single-shell pipeline (one `bash -c` invocation) before the next tool call lands. If you accumulate commits-as-checkpoints this way, follow the no-wip-commits rule above by squashing at the end via `git reset --soft && git commit -m "" && git push --force-with-lease`. Note the sandbox-interference workaround in the commit body so the human can audit. + **Pre-staged working tree at session start (expected, NOT anomalous).** The opposite of the sandbox-reset case: implementers commonly find their prescribed diff already applied as uncommitted edits in the working tree the moment they start. Observed across multiple runs (~40-50% of dispatches in PR #566's 10-dispatch run, #567's 9-dispatch run). Mechanism is unclear — possibly the harness caches edits between implementer invocations, or prior session's uncommitted work survives in the sandbox. **Action:** when you see a dirty `git status` matching the brief's prescribed diff at session start, verify the content matches the design (read the file, diff against the brief), run the local merge gate, and commit. Do NOT redo the work — that wastes cycles and risks divergence from the staged version. Do NOT panic — this is now an expected pattern, not a bug. Note the pre-staged finding in the commit body briefly ("working tree had the prescribed diff at session start; verified diff and committed") so the human has the audit trail. Distinguish from the sandbox-reset case above: pre-staged means edits *exist that you didn't make yet* (apply gate + commit); sandbox-reset means edits *vanish that you did make* (re-apply in tight pipeline). + 9. **Mid-fix block** (CI red on the local gate that won't resolve, brainstorm reveals deeper structural issue, fix demands cross-cutting refactor): **abort the dispatch.** `git checkout ` + `git reset --hard origin/` to drop any local work. File a follow-up GH issue (caveman body, link original + cite the blocker). Return to coordinator. The follow-up issue is the durable handoff for the next scheduled run. 10. **Already-fixed-upstream path:** if pre-flight investigation (e.g. `cargo audit`, file-state grep, `cargo tree`) shows the issue was resolved by a recently-merged upstream PR, do NOT make a no-op commit. Leave a caveman comment on the original issue naming the upstream PR + the fix location, close the issue (`completed` if the audit's intent now holds — the upstream fix solved it for us; `not_planned` if the audit's premise is moot — e.g. the targeted code was deleted), report back. Coordinator records under `## Already-Fixed` in the master PR — NOT under `Fixes`.