auto-fix batch claude/friendly-maxwell-3QJhB 2026-05-05#631
Merged
Conversation
- icons: purpose "any maskable" so Android adaptive containers crop with safe-zone awareness - top-level scope + id "/" so browsers identify install by id, not URL — prevents collisions between dev and prod builds at same origin Refs #605
- Replace `let _ = h.create_ephemeral_channel(...).await` in the member-list "start temp channel…" button click handler with an explicit `if let Err(e)` branch emitting `tracing::warn!(?e, "create_ephemeral_channel failed")`. - Mirrors the #585 / 41736c0 ICE-candidate pattern: silently dropping the Result hid network/permission/name-collision failures from both the user and the logs. - Log only, no toast: surfacing UI feedback would require touching the failure-UI helper convention, out of scope for an error-logging parity fix. Audit raised both options; narrowed to logging. Refs #620
- PLAN.md opened with "Built with Rust, libp2p, and Leptos" and named GossipSub/Kademlia/mDNS in its architecture box. Project migrated to iroh; see docs/specs/2026-03-29-iroh-migration-design.md. - README.md (overview + setup) + CLAUDE.md (architecture, crate layout, state-management, test-tier tree) cover everything PLAN duplicated, with current terminology. - Phase checklist (lines 66-157) was a historical artifact; every phase except Phase 9 read COMPLETE but doc didn't track code evolution. Active voice/video state lives in crates/web/src/voice.rs + specs. - "Deployed" section's port mentions (9090/9091) were a third drift site already flagged by audit #627 / PR #630. - Audit #607 explicitly accepted deletion as the principled fix vs rewrite-and-let-it-drift-again. - No references: grep -rn "PLAN.md" returned zero hits across repo. No include_str! / build.rs / test reads PLAN.md, so cargo test gate doesn't apply per the no-asset-coverage rule. Ran cargo fmt --check and cargo check --workspace; both green. Refs #607
Audits sometimes prescribe a fix predicated on a stated mechanism — log via tracing::warn!, the rebuild Effect picks it up, the existing test would catch this. Pre-flight verify the mechanism actually exists at HEAD AND doesn't violate a module-local constraint (privacy contracts, observability forbids, etc.). Surfaced this run by #622: search/handle.rs module-doc forbids all tracing::* (per local-search privacy spec) AND the rebuild Effect that insert's own doc-comment cites doesn't exist as described — the real recovery is an event-loop subscription with the same do_send fragility. Audit's both options were non-viable as-stated; correct call was ambiguous-fix-path skip without close. Refs #622
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-04 general-audit (master ticket #600). 4/4 dispatches landed clean. picks chosen to NOT overlap with PR #630's in-flight files (docker-compose.yml+ relay/worker entrypoints + state tests +crates/messaging/lib.rs+crates/web/tests/static_assets.rs+crates/network/src/traits.rs+README.md+CLAUDE.md+justfile), PR #599's (.github/workflows/*+e2e/permissions.spec.ts+crates/web/tests/browser.rs+ state-mgmt spec doc), and PR #566's (crates/{client,messaging,state,storage,web,worker}/src/...deep churn). all 4 picks landed on web public assets / web src non-overlapping files / project root.Fixes
Fixes audit F5 [tech-debt]: PWA manifest icons missing
purpose: maskable; manifest missingscopeandid#605 —fix(web): add maskable purpose, scope, id to manifest(d3543cc). PWA manifest now has"id": "/","scope": "/"top-level +"purpose": "any maskable"on both icon entries. Android adaptive-icon containers get a safe-zone-aware variant; same-origin install identity is no longer URL-derived. Coordinator pre-decided narrowing: simpler-form (audit's own accepted fallback — both icons gain"any maskable"purpose; existing SVGs already have ample safe-zone padding) over the aspirational separate-maskable-icon ship.Fixes audit F6 [quality]: service-worker registration swallows all errors silently #606 —
fix(web): port sw register to web_sys + log err(791faa5).crates/web/src/main.rs:17swappedjs_sys::eval(...)string forweb_sys::window().navigator().service_worker().register("/sw.js")driven bywasm_bindgen_futures::spawn_local. OnErr(JsValue), formats viaas_string().unwrap_or_else(|| format!("{e:?}"))and logs throughtracing::warn!. Drops onejs_sys::evaluser (still ~6 elsewhere in the web crate per [security] XSS-prone js_sys::eval() in pinned message jump #171/[WS-1] Web: js_sys::eval(format!()) for pinned-message scroll uses band-aid sanitization #425 tracker) AND surfaces SW registration failures (HTTPS misconfig, MIME mismatch, parse, scope) that previously vanished. Coordinator pre-decided narrowing: Option B (port toweb_sys) over Option A (just log the eval result), justified by audit's explicit endorsement + concreteunsafe-evalremoval progress.web_sys::ServiceWorkerContainerwas already in the Cargo.toml feature list — no new flag added.Fixes audit F20 [robustness]: swallowed
create_ephemeral_channelResult in member-list temp-channel button #620 —fix(web): log create_ephemeral_channel error(22969f5).crates/web/src/components/member_list.rs:383-390swappedlet _ = h.create_ephemeral_channel(...).await;forif let Err(e) = ... { tracing::warn!(?e, "create_ephemeral_channel failed"); }. Mirrors the audit F18 [quality]: ICE candidate handle_ice_candidate result swallowed #585 /41736c0ICE-candidate-handle-error pattern verbatim. Coordinator pre-decided narrowing: log-only over toast-surface (toast wiring is a separate scope decision; the audit's primary concern was diagnostic visibility).Fixes audit F7 [docs]: PLAN.md is pre-iroh-migration (still says libp2p, GossipSub, Kademlia, mDNS) #607 —
docs: delete stale PLAN.md (duplicates README + CLAUDE.md)(e290875). 209-line doc deleted. Pre-iroh-migration content (libp2p, GossipSub, Kademlia, mDNS, port 9090/9091) had drifted on three top-level docs — README + CLAUDE were already canonical post-iroh, PLAN was the only stale source. Phase-status checklist was decorative, drifted independently of the codebase. Coordinator pre-flightgrep -rn "PLAN.md"returned zero hits — no broken references anywhere in the repo. Coordinator pre-decided narrowing: Option A (delete) over Option B (rewrite), per the rationale thatdocs/specs/2026-03-29-iroh-migration-design.mdis the canonical historical record for the architecture transition; PLAN.md adds no information not better captured elsewhere AND drifts faster than CLAUDE.md.Already-Fixed
None. Audit @
88498a5= master-branch base (88498a5); same-day audit-to-fix gap. Per the skill's "When same-day audit-to-fix gap, expect ~zero already-fixed hits" clause, the sweep was a one-linegit log origin/main..HEADcheck that confirmed empty. No time invested.Parked
audit F22 [quality]: search index
remove_*mutations silently dropped on mailbox full #622 (audit F22 [quality]: search index remove_* mutations silently dropped on mailbox full). Ambiguous-fix-path skip without close per skill rule. Audit's option (b) "log viatracing::warn!" violates the explicit privacy contract incrates/client/src/search/handle.rs:3-8("Alltracing::*macros in this module are forbidden" perdocs/specs/2026-04-19-ui-design/local-search.md§Privacy). Audit's option (a) "document the same recovery story" was based on a misreading —insert's own doc-comment cites a "rebuild Effect incrates/web/src/app.rsre-runs on everymessages_sigchange and reseeds the index from scratch" but no such Effect exists: the actual recovery path is the event-loop subscription atapp.rs:393-459which itself usesdo_send(same fragility as theremove_*calls the audit flags). Both audit options non-viable as-stated. A real fix requires design work (bounded-mailbox + Result return?ask().awaitwith sync→async ripple? privacy-conscious in-memory metric?). Skipped per skill; future session decides. This run's skill edit (18d5564) codifies the lesson so the next run pre-flight catches the same shape.audit F2 [tech-debt]: docker-compose.yml has zero healthchecks; depends_on waits only for container start #602 (
audit F2 [tech-debt]: docker-compose healthchecks). Cross-PR coupling — audit's prescribed healthcheck command targets port3340, but the docker-compose.yml onmain(and on this run's master branch) still has port9090until PR auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630 lands. A healthcheck against 3340 would fail on this PR's tree pre-auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630-merge. Conversely, a healthcheck against 9090 would fail post-auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630-merge. Park until auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630 lands; not a coordinator-skip, just an order-dependency. The skill's "Cargo.lock conflicts are additive" clause covers most shared-file cases but a port-renumber dependency is one degree stricter — re-trigger on next scheduled run.Skill Evolution
One skill edit this run, committed on master in
18d5564 docs(skill): pre-flight verify audit's claimed mechanism:remove_*mutations silently dropped on mailbox full #622 as the surfacing example so the next session has full provenance.Lessons Learned
4/4 implementer dispatches landed cleanly. No mid-flight aborts. No finalize-implementer rescues. Sequential pattern + pre-decided narrowing held. All four picks resolved without brainstorm gate triggering on the implementer side — the coordinator pre-resolved the design calls in each brief (simpler-form vs aspirational for audit F5 [tech-debt]: PWA manifest icons missing
purpose: maskable; manifest missingscopeandid#605; port vs log for audit F6 [quality]: service-worker registration swallows all errors silently #606; log vs toast for audit F20 [robustness]: swallowedcreate_ephemeral_channelResult in member-list temp-channel button #620; delete vs rewrite for audit F7 [docs]: PLAN.md is pre-iroh-migration (still says libp2p, GossipSub, Kademlia, mDNS) #607). This same pattern is reproducing across the last 5+ scheduled runs (PR auto-fix batch claude/friendly-maxwell-M5xB6 2026-05-03 #566 11/11, auto-fix batch claude/friendly-maxwell-m1efN 2026-05-03 #596 9/9, auto-fix batch claude/friendly-maxwell-cCGXk 2026-05-04 #598 5/5, auto-fix batch claude/friendly-maxwell-iEXwu 2026-05-04 #599 4/4, auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630 10/10, this run 4/4). Fully reliable now.Coordinator pre-flight read on audit F22 [quality]: search index
remove_*mutations silently dropped on mailbox full #622 prevented a wasted dispatch. Readingsearch/handle.rslines 1-15 surfaced the explicit privacy contract before dispatching — without that read, the implementer would have followed the audit's "log via tracing::warn!" prescription, hit the contract, and returned with a brainstorm note (probably 5-10 min wasted + a stop-hook noise spike). Pre-flight read cost was 1 file read + 2 minutes of mechanism-verification grepping (hydrate_index,rebuild,messages_sig). This is the second run where coordinator pre-flight saved a wasted dispatch (auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630's audit F26 [quality]: Docker relay entrypoint passes--tcp-port/--ws-portflags the binary does not accept (HARD RUNTIME BREAK) #626 was the first — discovered worker entrypoints also stale). Pre-flight read is now load-bearing for any audit issue with non-trivial mechanism prescription. Codified into the skill as18d5564.Audit narrative can misrepresent code mechanism. audit F22 [quality]: search index
remove_*mutations silently dropped on mailbox full #622's audit citedinsert's doc-comment as ground truth ("the rebuild Effect incrates/web/src/app.rsre-runs on everymessages_sigchange and reseeds the index from scratch"). The doc-comment IS in the file at HEAD. But verifying that mechanism inapp.rs— searching for anyEffect::*watchingmessages_sigand callingsearch.rebuild(...)— produced zero hits. The cited rebuild path doesn't exist; only the event-loop subscription exists, sharing the samedo_sendfragility. Lesson: the audit may be correct about the symptom (mutations silently dropped) but wrong about the recovery story; pre-flight verify the recovery code, not just the call site of the bug.PLAN.md deletion (audit F7 [docs]: PLAN.md is pre-iroh-migration (still says libp2p, GossipSub, Kademlia, mDNS) #607) was the right principled call. Doc duplicated README + CLAUDE.md but with stale terminology. Two reasonable approaches (delete vs search-and-replace rewrite); coordinator chose delete per the rationale that (a) the historical roadmap is captured better in
docs/specs/2026-03-29-iroh-migration-design.md, (b) phase-status checklists drift faster than canonical docs, and (c) the "Deployed" section was a third site for stale port info that audit F27 [docs]: README + CLAUDE.md "Local Development" tables reference removed relay ports 9090/9091 #627/auto-fix batch claude/friendly-maxwell-s1tqN 2026-05-04 #630 already flagged twice elsewhere. Future-proofing > preservation of decorative content.Sandbox quirk on audit F5 [tech-debt]: PWA manifest icons missing
purpose: maskable; manifest missingscopeandid#605. Implementer reported pre-existing rust toolchain damage (missinglibrustc_driver*.so) and stalewasm32-unknown-unknowndirectory blockingrustup target add. Self-repaired viarustup toolchain install stable --force+ manual orphan cleanup, then ran the gate clean. Worth flagging — this is the first observed case of an actively-broken toolchain on dispatch entry. Future runs may want a brief pre-flightcargo --version && rustup target list --installedsanity step on the implementer side, OR the sandbox provisioning to lock the toolchain version. NOT codified yet — single observation; revisit if it reproduces.grep -rn "PLAN.md"zero-hit pre-flight on audit F7 [docs]: PLAN.md is pre-iroh-migration (still says libp2p, GossipSub, Kademlia, mDNS) #607 saved a deletion-with-broken-references hazard. The implementer's brief explicitly required them to redo the grep + report back if any reference was found. They did, returned zero, deleted clean. Pattern is worth keeping for any future "delete a top-level doc" pick.Test plan
master-PR CI is the load-bearing gate. each implementer ran the scoped subset locally (fmt + native clippy + cargo test on touched crates + wasm32 check + wasm32 clippy on touched crates).
CI gates to verify on this PR:
cargo fmtcargo clippyworkspace (native + wasm32)cargo testworkspace — no new tests added this run (4/4 picks were doc / asset / single-callsite-log / file-delete fixes); existing 252-test willow-state suite + 53-test willow-messaging suite + willow-web wasm-pack browser tests should run unchanged.cargo test -p willow-web --test static_assets(7/7 — confirmed locally green by audit F5 [tech-debt]: PWA manifest icons missingpurpose: maskable; manifest missingscopeandid#605 implementer; manifest still references both bundled SVG icons after the field additions)wasm-packbrowser tests (Firefox + geckodriver — observable on CI only) — service-worker registration viaweb_sysruns at app boot in the browser-test harness; verify no new console errors there.cargo audit(no advisory changes this run).Cargo.lock churn: none this run. No new deps added.
Mergeable_state expected: clean against current
main. Possible textual conflicts if PR #630 / PR #599 / PR #566 land first — none of them touch any of this PR's 4 file paths (crates/web/manifest.json,crates/web/src/main.rs,crates/web/src/components/member_list.rs,PLAN.md), so any conflict would be entirely on.claude/skills/resolving-issues/SKILL.md(this run's skill edit + previous runs' skill edits in the in-flight PRs). All such conflicts are textual and resolvable in seconds; no logic-fix conflicts expected.Generated by Claude Code