Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .claude/skills/resolving-issues/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ Fresh agent per issue, scoped to one issue + master branch ref. Steps:

**Sandbox `git reset --hard origin/<branch>` interference (known hazard).** Some sandboxed environments run a periodic `git reset --hard origin/<branch>` 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 <pre-dispatch-SHA> && git commit -m "<final message>" && 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 <master-branch>` + `git reset --hard origin/<master-branch>` 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`.
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)` \
Expand Down
2 changes: 1 addition & 1 deletion crates/client/src/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ pub(crate) fn derive_client_events(event: &willow_state::Event) -> Vec<ClientEve
EventKind::Propose { action } => {
out.push(ClientEvent::ProposalCreated {
proposal_hash: event.hash.to_string(),
action_description: format!("{action:?}"),
action_description: format!("{action}"),
});
}
EventKind::Vote { proposal, accept } => {
Expand Down
35 changes: 35 additions & 0 deletions crates/state/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
82 changes: 82 additions & 0 deletions crates/state/src/tests/voting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
3 changes: 2 additions & 1 deletion crates/web/src/components/file_share.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ pub fn FileShareButton(channel: ReadSignal<String>) -> 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;
}
};
Expand Down
4 changes: 3 additions & 1 deletion crates/web/src/event_processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
}
}
Expand Down
40 changes: 33 additions & 7 deletions crates/web/src/util.rs
Original file line number Diff line number Diff line change
@@ -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;
};
Expand Down
2 changes: 1 addition & 1 deletion e2e/.wait-timeout-baseline
Original file line number Diff line number Diff line change
@@ -1 +1 @@
39
10
44 changes: 40 additions & 4 deletions e2e/helpers/touch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -65,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).
}

/**
Expand Down Expand Up @@ -137,8 +161,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 });
Expand All @@ -147,7 +183,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();
}

Expand Down
Loading