auto-fix batch claude/friendly-maxwell-m1efN 2026-05-03#596
Merged
Conversation
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
`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
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
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
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
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
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
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
`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
7 tasks
This was referenced May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scheduled
/resolving-issuessweep against the 2026-05-03 general-audit (master ticket #567). Nine small-scope fixes landed sequentially; one follow-up issue filed (#595). Picked candidates that did NOT overlap with PR #566's in-flight files (crates/web/src/state.rs,crates/client/src/lib.rs, etc. — see #566 file list).Fixes
chore(ci): prune 3 stale RUSTSEC ignores from cargo-audit(232b296). Drops RUSTSEC-2026-0098/0099/0104 from the--ignorelist. 3-line deletion. YAML + bash syntax verified;cargo auditnot available in sandbox, regression net is CI's audit job.test(e2e): parameterise longPressAvatar over LONG_PRESS_MS(127af73). NewLONG_PRESS_MS = 350(mirrors product-sideHOLD_MSatcrates/web/src/components/long_press.rs:17) +LONG_PRESS_BUFFER_MS = 250. Helper now waitsLONG_PRESS_MS + LONG_PRESS_BUFFER_MSdeterministically.test(e2e): drop bare waitForTimeout from longPress post-touchend(d0120c0). Brainstorm picked Option A (drop the wait, caller-sidewaitForalready covers post-press outcome) over Option B (bake.mobile-action-sheet.openselector into helper). All 6 surveyed callers already had explicit waits, so zero migration needed at call sites.test(e2e): replace bare waitForTimeout with waitFor in helpers/ui.ts(076b848). 14 of 16 audit-listed sites migrated to selector-basedwaitFor(switchTab,messageAction,editMessage,deleteMessage,reactToMessage,kickPeer). 2 sites intrustPeer/untrustPeerleft withTODO(#589)comments — proposed-action voting paths have no synchronous DOM destination. Baseline filee2e/.wait-timeout-baselineratchets 39 → 10.test(e2e): drop permanent skip on mobile compare-sheet long-press(84d4a71). Deleted thetest.skip(true)block (waspermissions.spec.ts:243); follow-up tracker filed at [e2e] mobile compare-sheet long-press regression test (was: e2e/permissions.spec.ts test.skip) #595 to restore the test once the mobile member-list surface ships. Removed now-unusedlongPressAvatarimport; refreshed stale describe-block comment.fix(web): log ICE candidate handle errors(41736c0).let _ = vm.borrow().handle_ice_candidate(...)→if let Err(e) = ... { tracing::warn!(?e, "handle_ice_candidate failed"); }.handle_ice_candidatereturnsResult<(), String>, so?eDebug formatter is correct.fix(web): prefer string extraction for FileReader JsValue errors(7a975fa). One site (file_share.rs:69FileReader::result()Err arm) migrated to the established patterne.as_string().unwrap_or_else(|| format!("{e:?}")). Othertracing::error!calls in the file log static messages — left alone.feat(state): add Display for ProposedAction and VoteThreshold(bca6b7a). Per coordinator pre-decided strings:"Grant admin to {peer_id}"/"Revoke admin from {peer_id}"/"Kick {peer_id}"/"Set vote threshold to {threshold}"forProposedAction;"majority"/"unanimous"/"{n} admins"forVoteThreshold. Consumer atmutations.rs:948swappedformat!("{action:?}")→format!("{action}"). 7 new tests incrates/state/src/tests/voting.rs::display_tests. Display-name resolution at the UI tier is a separate follow-up (out of scope here).fix(web): textarea fallback only on clipboard write rejection(2524568). Synccopy_to_clipboard(text: &str)signature preserved (5+ Leptoson:clickcallers stay sync). Internallywasm_bindgen_futures::spawn_local(async move { ... })awaitsJsFuture::from(promise); onOkhappy path is silent + clean DOM; onErrcalls extractedexec_command_copy_fallback. Runner-up (async signature with callerspawn_local) rejected — every caller is a non-async event handler.Already-Fixed
None this run. The general-audit at #567 was filed 2026-05-03 against the same
main @ 6404719HEAD this run started from; no commits landed in the gap by definition (per the prior PR #566 lessons noting same-day audit-to-fix is the expected zero-yield case).Parked
None. All 9 picks landed cleanly. No mid-flight aborts; no finalize-implementer rescues; no scope-creep guards tripped.
Filed mid-run (follow-ups, NOT closed by this PR)
[e2e] mobile compare-sheet long-press regression test (was: e2e/permissions.spec.ts test.skip). Filed by audit F21 [tech-debt]: permanent test.skip(true) on mobile long-press compare-sheet test #588 implementer after deleting the permanent-skip block. Restore the test once the mobile member-list surface ships.Avoided picks (overlap with PR #566 files in flight)
crates/web/src/state.rswhich PR auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566 already modifies for audit F51 [UX]: state.rs builds permission strings via format!("{p:?}") #550 (Display for Permission). Deferred to next run after auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566 merges.set_join_status"denied:" sentinel → typedJoinStatus::Denied) — touchescrates/client/src/lib.rs(likely event-emit boundary) which PR auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566 also modifies. Deferred.Skill Evolution
4702486 docs(skill): codify pre-staged working tree as expected pattern— adds a sibling paragraph to the Implementer Agent step 8 sandbox-interference note. Captures the inverse phenomenon (pre-staged uncommitted edits at session start) seen in ~40-50% of dispatches across PR auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566 + this run. Implementer brief now expects + accepts this rather than treating it as anomalous; new hint distinguishes pre-staged (apply gate + commit) from sandbox-reset (re-apply in tight pipeline).Lessons Learned
4702486).non_admin_set_profile_is_acceptedregression confirmed AGAIN by audit F4 [UX]: ProposalCreated action_description = format!("{action:?}") #571 implementer'scargo test -p willow-staterun. Already tracked at test(state): non_admin_set_profile_is_accepted regression after #505 membership gate #565 (filed during PR auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566's run). Trail consistent: regression is real, persists on master HEAD, design call still pending. CI may report willow-state test red on this PR — known, not introduced here.Test plan
Master-PR CI is the load-bearing gate. Each implementer ran the scoped subset locally (fmt + native clippy + native test + wasm32 check + wasm32 clippy on touched crates).
CI gates to verify on this PR:
cargo fmtcargo clippyworkspace (native + wasm32)cargo testworkspace (state + client + web especially — Display tests + util.rs tests run as native unit tests)wasm-packbrowser tests (Firefox + geckodriver — observable on CI only)cargo audit(3 stale ignores pruned; 7 active retained)Known pre-existing test failure:
willow-state::tests_materialize::non_admin_set_profile_is_acceptedfails on HEAD (regression from PR #505, tracked at #565). Not introduced by this PR; CI may report it red.Generated by Claude Code