auto-fix batch claude/friendly-maxwell-UlJEd 2026-05-03#560
Merged
Conversation
`npm install` can mutate package-lock.json; `npm ci` installs exactly the lockfile + refuses to mutate it. Deterministic E2E setup demands the strict variant. No Rust changes — only fmt run as smoke check (clippy/test skipped). Refs #530
Bare `cargo install` ignores each tool's Cargo.lock, so any compromised transitive dep on crates.io silently lands in the E2E env. `--locked --version X.Y.Z` deterministic. - trunk 0.21.14 matches deploy.yml workflow pin - just 1.50.0 latest stable (no existing pin in workflows) Refs #529
Mirror the storage cap added by PR #507 / b075140 (MAX_AUTHORS_PER_SYNC = 256) on the replay path. Without the guard, ReplayRole::handle_request(WorkerRequest::Sync) iterates a peer-supplied HeadsSummary into a BTreeMap and walks the in-memory DAG once per author — same DoS shape as storage's sync_since/history before #507. Approach A (centralize the const in willow-common alongside SYNC_BATCH_LIMIT) chosen over B (define a local const in replay): the cap is a wire-protocol invariant that BOTH workers must agree on. A single source of truth in willow-common — already a dep of both crates — guarantees they cannot drift. Cost is one extra crate dep edge for willow-replay (already had willow-state, willow-identity, willow-worker, willow-network). Storage's local pub const is removed; it now imports from willow-common. No behavioural change to storage — the value and the bail! sites are byte-identical. Replay uses WorkerResponse::Denied { reason } (sync handler, not anyhow::Result) mirroring the existing "unknown server" branch and the storage error message text. Tests: - sync_request_rejects_oversize_heads (MAX+1 → Denied) - sync_request_accepts_exact_cap_heads (MAX → not Denied) Refs #514 https://claude.ai/code/session_019HhgeDZ5HCbEUygRRLCjde
is_zero_duration only matched "", "0s", "0ms" — but the global prefers-reduced-motion rule in style.css forces transition-duration to 0.01ms !important on every element, which engines serialise as either "0.01ms" or "0.0001s". Both slipped past the strict matcher, so mobile_shell, confirm_dialog, bottom_sheet, grove_drawer, and message reactions all sat waiting for a transitionend that never fires under reduced-motion — UI hang. Replace the string-equality check with parse_duration_seconds (parses both s and ms suffixes) and accept anything ≤ 1ms (epsilon) as zero. Unparseable input stays conservative (not zero). Sibling-of-closed audit follow-up to #496 (8d89f18). Approach A (parse-and-compare) chosen over B (hardcode the two known strings) because reduced-motion is the authoritative contract — any sub-millisecond duration is indistinguishable from "no transition" for transitionend purposes, so a numeric threshold is the durable fix. Tests added (native, no DOM): parse_duration_seconds_handles_units, parse_duration_seconds_rejects_malformed, is_zero_duration_str_recognises_explicit_zero, is_zero_duration_str_recognises_reduced_motion_override, is_zero_duration_str_treats_sub_millisecond_as_zero, is_zero_duration_str_rejects_real_durations, is_zero_duration_str_multi_value_all_zero, is_zero_duration_str_multi_value_mixed_is_not_zero, is_zero_duration_str_unparseable_is_not_zero. Gates: fmt clean, clippy native + wasm32 clean (-D warnings), 86 willow-web lib tests pass, wasm32 --tests check clean (wasm-pack / geckodriver not available in env — used cargo check --target wasm32-unknown-unknown --tests as fallback gate per CLAUDE.md). Refs #515 https://claude.ai/code/session_019HhgeDZ5HCbEUygRRLCjde
Audit F47 (#547): `evict_stale_removes_expired` slept 10ms after a 1ms TTL — flake risk on slow CI per `condition-based-waiting`. Fix per audit suggestion: inject a clock. Added `pub(crate) evict_stale_at(&mut self, now: Instant)`; production `evict_stale()` delegates with `Instant::now()`. Test now feeds a synthetic future `now`, removing the timing dependency entirely. Picked clock-injection (A) over a clock trait (B, too heavy for one test) and over cutoff-injection (C, equivalent but maps less directly to "inject a clock" in the audit). #545 (Instant::now wasm linkage) is a separate ticket — not addressed here. Refs #547
Match surrounding let-Ok-else style: log + return instead of unwrap, so non-window contexts (e.g. worker harness) do not panic. Refs #543
No production or test code in willow-identity uses tokio. `grep -rn "tokio::\|#\[tokio::" crates/identity/src` returns only a doc-comment reference at lib.rs:99. Lib crates must stay tokio-free per CLAUDE.md; dev-dep was dead weight. Refs #537
Replaces a blind 3s sleep before curl with a 15-attempt loop (2s backoff). The relay can take longer than 3s to bind after a restart, and combined with StrictHostKeyChecking=no a slow bind could mask a silent deploy failure as a passing job. Refs #531
Permission enum (crates/state/src/event.rs:46-69) defines exactly:
SyncProvider, ManageChannels, ManageRoles, SendMessages, CreateInvite
(plus __UnknownLegacy sentinel). Admin status and member kicks live on
the ProposedAction + vote path by design — see
docs/specs/2026-04-01-per-author-merkle-dag-state-design.md:295 ("the
Permission enum does not contain Administrator"). The type system
enforces the governance path; granting admin via GrantPermission is
structurally impossible.
CLAUDE.md and the agentic-peer-api spec listed KickMembers and
Administrator as valid permission values — pure doc drift. Realign
both with the enum and clarify trust_peer/untrust_peer route through
the vote path, not GrantPermission.
Refs #521
Same-day audit runs yield ~zero already-fixed sweep hits — note explicitly so future runs skip the deeper search. Document the ambiguous-fix-path pattern (audit premise real, fix is a design call w/ ≥2 valid approaches): skip from dispatch queue without closing, surface in Lessons Learned. Surfaced this run by #535 (Arc<Mutex> in fsm.rs is already in mod tests; audit's "move to mod tests" prescription doesn't apply — fix is either external test file or glob update, design call).
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. Eight small-scope fixes from the 2026-05-02 general-audit (master ticket #513) landed sequentially. One audit child (#535) coordinator-skipped as ambiguous-fix-path (design call needed); skill edit captures the new pattern.Fixes
fix(scripts): use npm ci in setup-e2e(8089a62). One-line lockfile-respect swap;package-lock.jsonprecondition verified.build(setup-e2e): pin trunk + just installs with --locked(a5e2ad0).trunk@0.21.14mirrorsdeploy.yml's pin;just@1.50.0is latest stable.fix(replay): cap peer HeadsSummary in sync(c65ffab). Sibling-of-closed (auto-fix batch claude/friendly-maxwell-BjjKA (2026-05-02) #507 b075140 storage cap). Approach A: centralizedMAX_AUTHORS_PER_SYNCinwillow-commonalongsideSYNC_BATCH_LIMITso storage + replay can't drift; replay returnsWorkerResponse::Deniedmirroring storage'sbail!text. Testssync_request_rejects_oversize_heads+sync_request_accepts_exact_cap_heads. (5 files, +111/-17.)fix(web): treat sub-1ms transition as zero-duration(d23f3f8). Sibling-of-closed (feat(web): event-based waits PR-3 — data-state lifecycle + page.clock helper #496 8d89f18). Addedparse_duration_secondsparser +is_zero_duration_strpredicate w/ 1ms epsilon — handles the0.01ms !importantreduced-motion override + future near-zero serializations. 9 unit tests (testable string predicate, no DOM). Browser run deferred to CI (no wasm-pack/Firefox in sandbox;cargo check --target wasm32fallback gate green).test(client): inject now into worker_cache eviction(b20b8ec). Approach A: addedpub(crate) fn evict_stale_at(&mut self, now: Instant); existingevict_stale()delegates. Test rewritten to use syntheticnow = Instant::now() + 60s— no morestd::thread::sleep. (audit F45 [robustness]: WorkerCache uses Instant::now() (native-only) in willow-client lib crate #545 Instant-on-wasm gating left to its own ticket.)fix(web): handle missing window in getUserMedia path(4a19bc6).let Some(window) = web_sys::window() else { tracing::error!(...); return; }matches surroundinglet Ok(...) elsestyle. No other unwraps in the gesture block.chore(identity): drop unused tokio dev-dep(2363f99).grepconfirmed only a doc-comment hit, no#[tokio::test]use.tempfiledev-dep stays (used by tempdir tests).ci(deploy): poll verify endpoint up to ~30s(baa4484). Replaced blindsleep 3w/ 15-attempt × 2s curl poll loop, exit 1 on final failure with stderr diagnostic.systemctl is-activecheck unchanged.docs: drop fictional KickMembers/Administrator perms(64ca482). Dropped both from CLAUDE.md's permission list + the agentic-peer-api spec's "Valid permission values" line. Rewordedtrust_peer/untrust_peertable entries to "Grant/Revoke admin status (via vote)". Aligns docs w/ canonicalPermissionenum atcrates/state/src/event.rs:46-69and the design rationale atdocs/specs/2026-04-01-per-author-merkle-dag-state-design.md:295,623,634,1565. Did NOT add the variant — would require state-machine + spec change, out of scope.Already-Fixed
None this run. The general-audit at #513 was filed earlier the same day against the same
main @ b901575head this run started from, so by definition no fixes had landed between audit and dispatch (skill now notes this is the expected zero-yield case for same-day audit-to-fix gaps).Parked
None. One audit child (#535 —
Arc<Mutex>inwillow-actorfsm.rs:415) was coordinator-skipped during step 6 picks, NOT parked: the test code at the cited line is already inside#[cfg(test)] mod tests(lines 175-460), so the audit's literal "move into mod tests" prescription is satisfied at HEAD. The audit's underlying concern is real (the!**/tests*exclusion glob doesn't catch in-file test modules) but the fix is a design call — either move the test to an externalcrates/actor/tests/fsm_tests.rsso the path-based glob catches it, or update the audit glob. Skill edit5320c8edocuments this new "ambiguous-fix-path" coordinator-skip pattern; left open for the next run with fresh eyes / accumulated context.Skill Evolution
5320c8e docs(skill): coordinator-skip for ambiguous-fix-path issues— adds two notes to step 6 of the Core Loop:git log <audit-ref>..origin/mainper pick is enough.Lessons Learned
8/8 implementer dispatches landed cleanly, no blockers. No mid-flight aborts, no finalize-implementer rescues, no scope-creep guards tripped. Sequential pattern + Monitor SHA-watch pattern both worked smoothly. Two-signal convergence (SHA-advance via Monitor + agent-completion notification) behaved per the existing skill guidance — both arrived per dispatch in mostly the same order,
git statusafter each signal disambiguated cleanly.Coordinator-side pre-flight
grep/sedverification at HEAD was load-bearing. Step 6 spot-checks of every cited file/line confirmed all 9 picks were real before dispatch — caught one skip-worthy pick (audit F35 [debt]: Arc<Mutex> in willow-actor fsm.rs (move into mod tests if test-only) #535) and saved its dispatch slot. No false-premise audit findings reached an implementer this run. Worth keeping the discipline; cheap up front, expensive when wasted.Brainstorm gate threshold worked correctly across the picks. Six issues skipped brainstorm (one-liner shell / config / docs / single-let-else / dev-dep removal / yaml shell loop). Two triggered automated brainstorm (audit F1 [robustness]: replay/role.rs missing storage's heads-cap rejection #514 cross-crate const placement → A vs B; audit F3 [robustness]: is_zero_duration misses 0.01ms!important reduced-motion override #515 parser approach → A vs B vs C; audit F47 [quality]: worker_cache test uses std::thread::sleep — flake risk #547 clock injection → A vs B vs C). Implementer briefs included pre-decided narrowing context where the coordinator could (e.g. for audit F1 [robustness]: replay/role.rs missing storage's heads-cap rejection #514, named the
WorkerResponse::Deniedpattern to mirror storage'sbail!); each implementer's brainstorm landed within the cap. None expanded to plan-file territory.Mechanical scope expansion ("call-site migration is part of the fix") triggered three times, all cleanly justified.
willow-commondep edge towillow-replayto centralize the cap. Caught two prod sites (storage + replay) that share the invariant.matches!predicate for a parser + epsilon predicate. The string-predicate refactor was the load-bearing site, the parser is its testable foundation.KickMembers/Administratordrift in two doc files (CLAUDE.md + the agentic-peer-api spec) — single coherent fix, not creep.All three flagged the deviation in implementer reports + commit bodies. Skill's existing wording covered each; no edit needed.
#506territory (pre-existing wasm-clippy--all-targetsfailures in willow-client tests) surfaced again from the audit F47 [quality]: worker_cache test uses std::thread::sleep — flake risk #547 implementer. PR auto-fix batch claude/friendly-maxwell-f34GI 2026-05-02 #511 fixed this on its branch but is unmerged, so each implementer touching willow-client still hits it onclippy --target wasm32 -p willow-client --all-targets. Mitigation already known — implementer scoped to--liband noted the gap. Once auto-fix batch claude/friendly-maxwell-f34GI 2026-05-02 #511 merges, the friction disappears.No false-alarm finalize-implementer dispatches this run. Full-SHA capture (skill
d5d6457) + Monitor wait pattern held; no preemptive crash conclusions on lagging completion notifications.Test plan
Master-PR CI is the load-bearing gate. Locally, each implementer ran the scoped subset (fmt, native + wasm clippy on touched crates, native test, wasm32 check). No
just checkworkspace-wide run sincejustis unavailable in some sandboxes; rawcargoequivalents per the skill's fallback.CI gates to verify on this PR:
is_zero_durationparser tests run as native unit tests)Generated by Claude Code