feat(test-hooks): PR-1 — event-based-waits foundation#454
Merged
Conversation
Replace time-based waits (53 waitForTimeout, 71 timeout overrides, 3 sleep loops) with event-based instrumentation. Three categories, three tools: state convergence via window.__willow + exposeBinding push; DOM/animation via data-state attribute on transitionend; real durations via page.clock. Test-hooks gated behind cargo feature, off in prod (privacy: third-party JS must not see DAG heads). CI symbol-leak check enforces. Scope = infra + 2 pilots (helpers.ts, multi-peer-sync.spec.ts) + ESLint ratchet + tracking issue for remaining 7 specs.
Round 2 (4 parallel fresh agents): fixed wrong file:line citations (subscribe_events, HeadsSummary, SyncCompleted); rewrote ClientEvent TS shape against real Rust enum (30+ variants, mixed tuple/struct); raised ring buffer cap and made overflow a hard test failure; sorted-keys canonical heads comparison; added transitionend fallbacks for prefers-reduced-motion + unmount + property-specific filtering; corrected page.clock API; fixed app.rs mount sequencing; switched CI symbol-leak grep to dist/*.js (wasm-opt strips); decomposed into 4 sequential PRs; added 2026-09-30 sunset; added rejected-alternatives for "longer waitForTimeout" / "Rust harness" / "iroh mock". Round 3 (architecture pressure-test): fixed Peer.heads return-type drift; closed buffer-drain stale-buffer hazard with three-edge drain; added page.clock vs performance.now caveat + iroh-audit gate in PR 1; scoped justfile FEATURES forwarding into PR 1; bound StoredValue so dispatcher loop survives; added structured author-key diff to waitUntilHeadsEqual error; named transitionend driving property per component; added re-export-shim completeness test; ratchet script enforces sunset cutoff; ESLint rule lands in PR 1 (not PR 4) to close the lint window across PRs 1-3; documented kind PascalCase + fields camelCase wire convention. Round 4: clean.
8 phases, 33 TDD tasks. Lands the test-hooks cargo feature, the WillowTestHooks WASM API (snapshot/heads/event_count/last_event), the ClientEvent wire-shape conversion (10 wire-visible variants, internal variants filtered), the push dispatcher with three-edge buffer drain (init / per-dispatch / read-side), the app.rs mount, the symbol-leak guard, the justfile FEATURES forwarding, and the ESLint rule plus per-spec disable headers tied to the tracking issue. PR 2-4 scoped explicitly out (Peer wrapper, data-state lifecycle, ratchet + flake harness) — separate plans when their predecessors land.
Implements Tasks 2.2 + 2.3 of the PR-1 plan: - `WillowTestHooks::new<N>(handle)` and `from_dag_addr(addr)` constructors in `crates/web/src/test_hooks/mod.rs`; the struct stores the DAG actor address directly, keeping the wasm_bindgen surface monomorphic. - `event_count()` and `last_event()` return `js_sys::Promise` (not `u32`/ `Option<String>`) so the actor ask can complete on the WASM cooperative scheduler without blocking. - Three async accessors on `ClientHandle` in `crates/client/src/accessors.rs`: `dag_event_count()`, `dag_last_event_hash()`, `dag_heads_summary()`. - `dag_addr_clone()` public helper (no feature gate) so `WillowTestHooks` can extract the addr without holding a generic `ClientHandle<N>`. - Async `snapshot::build` + `build_heads` stubs for Phase 2.6 use. - Browser test `crates/web/tests/test_hooks_browser.rs` constructs a `StateActor<DagState>` directly (no `MemNetwork`/tokio needed on WASM), seeds genesis via `ManagedDag::new`, then awaits `event_count` and `last_event` via `JsFuture`. Deviation from plan: `event_count()` / `last_event()` return `Promise` rather than synchronous `u32` / `Option<String>`. The plan assumed a `shared` field on `ClientHandle` that does not exist; the actor system offers no synchronous read path. Returning `Promise` is architecturally correct and matches WASM's cooperative scheduler model. Deviation from plan: browser test uses `from_dag_addr` instead of `ClientConfig::ephemeral_with_network` (does not exist) + `test_client()` (MemNetwork is tokio-only, won't compile on wasm32-unknown-unknown). Note: wasm-pack browser tests cannot be executed in this environment due to a pre-existing duplicate-symbol link error (wasm-streams 0.4.2 vs 0.5.0 diamond from leptos→server_fn and leptos→reqwest). `cargo check --tests --target wasm32-unknown-unknown --features test-hooks` passes cleanly; the link-time issue is unrelated to this PR. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Investigation during PR-1 execution found several mistaken assumptions in the original plan: - ClientHandle has no synchronous DAG read path (all access is async actor-ask). Pull-API methods must return js_sys::Promise. - MemNetwork is native-only (uses tokio::sync::broadcast); browser fixtures must construct StateActor<DagState> directly. - EventHash/EndpointId have no to_hex() method; use Display. - ChannelInfo has no member_count field; use kind instead. - test-hooks feature must span willow-client + willow-web so the new dag_addr_clone() accessor can be gated. Corrects the spec API surface block and adds an errata file documenting the section-by-section plan corrections. The original plan stays in the repo as historical record with a banner pointing at the errata. Investigation findings carry file:line citations against the codebase at b7525c9.
Bring the PR-1 test-hooks scaffolding into compliance with the errata in `docs/plans/2026-04-28-event-based-waits-pr1-errata.md`: - Add a dedicated `test-hooks` feature on `willow-client` (distinct from the existing `test-utils` feature, which transitively pulls in `MemNetwork` and breaks WASM). - Split the `test-hooks` feature on `willow-web` so it forwards to `willow-client/test-hooks`. - Drop the redundant `[dev-dependencies]` block in `crates/web/Cargo.toml` (`willow-actor`, `willow-state`, `willow-identity`, `willow-client`, and `wasm-bindgen-futures` are already in regular `[dependencies]`). - Gate `dag_addr_clone()` on `ClientHandle` behind `#[cfg(feature = "test-hooks")]` and add a matching `event_state_addr_clone()` for the snapshot builder. Drop the bespoke `dag_event_count`, `dag_last_event_hash`, `dag_heads_summary` accessors — `WillowTestHooks` uses `willow_actor::state::select` against the raw actor addresses instead. - Switch `WillowTestHooks::new` to take `&ClientHandle<N>` (borrow form per the errata) and rename `from_dag_addr` to `from_actors(dag_addr, state_addr)` so the wasm32 browser fixture can construct hooks without going through `ClientHandle` (which would require `MemNetwork` — native-only). - Rewrite `snapshot.rs::build` and `build_heads` to take raw actor addresses and read via `state::select`. Replace `ChannelDto.member_count` with `ChannelDto.kind` (matches `state::Channel` — there is no `member_count` field on the source). - Update the browser test to assert empty-DAG invariants (`event_count == 0`, `last_event` is `null`) on a fixture that bypasses `ClientHandle` entirely. Verified: `cargo check` clean (zero warnings) on all four configs (`-p willow-client`, `-p willow-client --features test-hooks`, `-p willow-web`, `-p willow-web --features test-hooks`); `cargo fmt --check` clean; `cargo check --tests --target wasm32-unknown-unknown -p willow-web --features test-hooks` clean; native `cargo test -p willow-web` and `-p willow-client` pass (270 + 0 tests). Browser test not run (`wasm-pack` unavailable in sandbox per errata note). https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Address the code-quality reviewer's Important issue on the
errata-compliance commit: ChannelDto.kind was String filled via
format!("{:?}", ch.kind), so the wire form (`"Text"` / `"Voice"`) was
implicitly contracted by Debug formatting. Switch to embedding
ChannelKind directly — the state crate already derives Serialize on
the enum, so the wire shape is now contracted by the state crate.
Compiles + clippy -D warnings clean under both feature configs.
…s-map> Implements Tasks 2.4 + 2.5 (TDD red-green pair): - Added test in test_hooks_browser.rs asserting heads() returns empty map on empty DAG - Implemented heads() method in WillowTestHooks using state::select pattern - Added Deserialize derive to AuthorHeadDto for test round-trip serialization Follows existing event_count() pattern: clone dag_addr, future_to_promise, state::select, then serde_wasm_bindgen::to_value. Resolves to Promise that yields Record<string, AuthorHead> mapping EndpointId hex strings to head info. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
…napshotDto> TDD red-green for Task 2.6/2.7 of the event-based waits PR-1 plan. Adds failing test first, then implements snapshot() method alongside event_count(), last_event(), and heads(). Required Deserialize derives on SnapshotDto and ChannelDto for test round-trip via serde_wasm_bindgen.
Covers Tasks 3.1-3.4; defines stable wire shape with PascalCase kind discriminator and camelCase fields per spec; 10 wire-visible variants (SyncCompleted, MessageReceived, PeerConnected, PeerDisconnected, ChannelCreated, ChannelDeleted, PeerTrusted, PeerUntrusted, ProfileUpdated, RoleCreated) plus catch-all filter test for internal-only variants. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
…gnalling Implements Tasks 4.1-4.5 of Phase 4 (PR-1 event-based waits). Dispatcher (dispatcher.rs): takes EventReceiver directly (not ClientHandle<N>) so it compiles on wasm32 without MemNetwork. Loops on recv(), converts each ClientEvent to WireEvent via to_wire(), forwards to window.__willowEvent. Three-edge drain: (1) on init — clears prior-dispatcher leftovers; (2) on every dispatch — delivers buffered events once binding appears; (3) read-side handled by Playwright fixture (JS-only). Buffer capacity 65,536; overflow drops oldest and calls window.__willowOverflow(droppedCount). Drop on DispatcherHandle flips the abort Rc<RefCell<bool>> flag. Browser tests (4 new wasm_bindgen_test): build fixture via fresh_dispatcher_setup() — spawns Broker<ClientEvent>, subscribes EventReceiver, installs dispatcher; no ClientHandle or MemNetwork needed. Tests: emit to JS callback, drop aborts loop, buffer drain on first dispatch after binding appears, overflow signals __willowOverflow. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Address Important issues from the Phase 4 code-quality review: - dispatch_or_buffer / drain_buffer_into_callback previously swallowed func.call1 errors via `let _ =`. A buggy test fixture callback that threw would silently lose events and produce the exact class of flake this PR is meant to eliminate. Now log a console.warn so failures surface in the test output. - The Drop-aborts-loop test asserted `count <= count_after_drop + 1` but its message claimed "should not deliver events after handle drop". The +1 tolerance is intentional (the abort flag is checked at the top of the loop after recv().await returns, so one in-flight event can slip through). Update the message to reflect that and add a comment explaining why.
Tasks 5.1–5.3 of the PR-1 event-based waits plan. 5.1: Added `#[cfg(feature = "test-hooks")]` block in `crates/web/src/app.rs` immediately after the `with_trust_store` call and before `provide_context`. The block constructs `WillowTestHooks::new(&inner_for_hooks)` and sets it on `window.__willow` synchronously, then spawns a `wasm_bindgen_futures::spawn_local` task that subscribes to events and installs the push dispatcher; the returned `DispatcherHandle` is leaked via `std::mem::forget` for app-lifetime duration. 5.2: Added `window_willow_is_mounted_under_test_hooks_feature` browser test in `crates/web/tests/browser.rs` (gated `#[cfg(feature = "test-hooks")]`) that mounts `<App/>` and asserts `window.__willow` is non-undefined. The property is set synchronously before any network tasks, so the assertion is stable even in a headless test environment that cannot complete network connections. 5.3: Verified `cargo check -p willow-web` (no feature) passes, confirming `WillowTestHooks` symbols are absent from the default build. All five verification commands (check, check --features, fmt --check, clippy, wasm check) pass with zero warnings. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
All recipes that invoke trunk (dev, setup-e2e, test-e2e-ui, test-e2e-sync, test-e2e-perms, test-e2e-full, check-all) now accept an optional FEATURES parameter. E2e recipes default to FEATURES=test-hooks so Playwright tests always run against a test-hooks build. dev and setup-e2e default to empty so production builds stay clean. WILLOW_FEATURES is forwarded to dev.sh and setup-e2e.sh, which pass it as --features <FEATURES> to trunk build / trunk serve. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Adds scripts/check-no-test-hooks-in-prod.sh which: 1. Runs trunk build --release (no features) and asserts WillowTestHooks is absent from the generated JS shim (and wasm name section if wasm-objdump is available). 2. Runs trunk build --release --features test-hooks and asserts WillowTestHooks IS present — catching cases where the feature gate itself breaks silently. Wires the script as the final step of check-all so every PR gate run verifies the privacy boundary documented in docs/specs/2026-04-27-event-based-waits-design.md. Note: trunk is not present in this sandbox so the script was verified via `bash -n` (syntax check only). Full run-test must happen in CI where trunk + wasm32 target are installed. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Introduces ESLint no-restricted-syntax rule banning *.waitForTimeout(...) in e2e/. This enforces the migration to event-based waits per the design spec. Existing 9 files with waitForTimeout calls (6 with actual usage: cross-browser-sync, helpers, mobile-actions, mobile, multi-peer-mobile, permissions) get per-file eslint-disable headers pointing at issue #458 for migration tracking. The rule fires correctly on new violations (verified with probe test). All e2e files pass linting with headers in place. https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ
Address Minor issues from the holistic PR-1 code review: - snapshot.rs: stale doc comment said build/build_heads "are not yet wired into the JS-exposed methods" — they have been since Tasks 2.5 and 2.7. Update the comment to reflect the current wiring. - dispatcher.rs: Reflect::set for __willowEventBuffer creation now surfaces errors via console.error. Previously a silent failure would drop all subsequent buffered events with no visible signal — exactly the class of failure event-based waits is meant to eliminate. Two other Important items from the review (mount-block early-event race in app.rs and System leak in test fixtures) are sharp-edge documentation matters not landed code defects; they will be tracked as follow-ups against issue #458.
Two follow-ups flagged by the PR-1 final reviewer. 1. Mount-block early-event race The previous mount block spawn_local'd `subscribe_events()` (async), so any ClientEvent published between mount and the BrokerSubscribe confirmation (boot-time hydration, persistence open, initial relay status) was lost. Add `Broker::Handler<BrokerAttach>` — a fire-and-forget twin of `BrokerSubscribe` that returns `()` so callers can `do_send` it from sync contexts. Add `EventReceiver::subscribe_now` which wires it up. Because the broker mailbox is FIFO, any `Publish` enqueued after `subscribe_now` returns is processed after the registration — the race is gone. `app.rs` mount block now subscribes synchronously; `spawn_local` and the dispatcher leak in that path are gone. 2. test_hooks_browser fixtures leaked a System per test `empty_hooks` and `fresh_dispatcher_setup` mem::forget'd `System` to "keep actors alive". Per `System`'s docstring, dropping the system without `shutdown()` does NOT stop tracked actors — they live as long as their `Addr<_>` references. The forget was unnecessary and leaked a mailbox + SystemActor task per test. Return `System` from both helpers and bind it as `_sys` in the test scope; let it drop naturally at test end. Tests added: - `subscribe_now_captures_event_published_synchronously_after_subscribe` proves no-yield publish-after-subscribe lands.
…its-RNFZ9 # Conflicts: # crates/web/tests/browser.rs
intendednull
changed the title
This was referenced Apr 29, 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.
PR-1 of the event-based-waits migration. Lands the cargo-feature-gated
test-hooksscaffolding so subsequent PRs can migrate Playwright e2etests from time-based
waitForTimeoutto event-based waits.Scope: infrastructure only. No spec migrations in this PR; those are
tracked in #458 with a 2026-09-30 sunset.
What this PR ships
test-hooksinwillow-clientandwillow-web,off by default. Symbol-leak guard (
scripts/check-no-test-hooks-in-prod.sh)asserts
WillowTestHooksis absent from releasedist/*.jsand presentwith the feature on; wired into
just check-all.WillowTestHookswasm-bindgen API, mounted atwindow.__willowwhen the feature is on:
event_count() -> Promise<number>— DAG event countlast_event() -> Promise<EventDto | null>— most recent eventheads() -> Promise<Record<string, AuthorHead>>— per-author headssnapshot() -> Promise<SnapshotDto>—event_count+heads+last_event+ materialisedchannels(name +ChannelKind)ClientEventand forwards afiltered wire shape to
window.__willowEvent(PlaywrightexposeBinding). Three-edge buffer drain (init / per-dispatch / JSread-side), 65 536-entry buffer with overflow signalling via
window.__willowOverflow,Drop-based abort.crates/web/src/test_hooks/wire.rs):10 visible variants behind
#[serde(tag = "kind")], internal-onlyvariants (e.g.
RelayStatusChanged) filtered out.eslint.config.js):no-restricted-syntaxforwaitForTimeout. Six pre-existing e2e specs carry aneslint-disableheader pointing at e2e: migrate remaining specs to event-based waits #458.FEATURES=parameter onjust dev/setup-e2e/test-e2e-*/check-all. E2E recipes hardcodeFEATURES=test-hooks.Polish (post-review)
Broker::Handler<BrokerAttach>(fire-and-forget()-result twin ofBrokerSubscribe) andEventReceiver::subscribe_now. The mount blocknow subscribes synchronously; FIFO mailbox ordering guarantees no
events are lost between mount and registration.
mem::forget(System)fromtest_hooks_browser.rsfixtures.Per
System's docstring, dropping withoutshutdown()doesn't stoptracked actors. Helpers now return
Systemto the test scope andlet it drop naturally.
Out of scope (tracked elsewhere)
waitForTimeoutto event-based waits — e2e: migrate remaining specs to event-based waits #458 (sunset 2026-09-30).
Peerwrapper + helpers split + first pilotconversion.
References
docs/specs/2026-04-27-event-based-waits-design.mddocs/plans/2026-04-27-event-based-waits-pr1-test-hooks-foundation.mddocs/plans/2026-04-28-event-based-waits-pr1-errata.mdTest plan
cargo check --workspace --all-targetscargo clippy --workspace --all-targets -- -D warningscargo fmt --all -- --checkcargo check -p willow-web --features test-hooks --target wasm32-unknown-unknown --testscargo test -p willow-actor --lib(94 passed)cargo test -p willow-client --lib(311 passed, includes newsubscribe_now_captures_event_published_synchronously_after_subscribe)cargo test -p willow-state --lib(227 passed)wasm-pack test crates/web --headless --chrome --features test-hooks) — sandbox lackswasm-pack/trunk/just; covered by compile-onlycargo check --target wasm32-unknown-unknownhere. Recommend running locally / in CI before merge.scripts/check-no-test-hooks-in-prod.sh) — same sandbox limitation; run locally before merge.https://claude.ai/code/session_0128gr2kRMyEZAg9sqiNZJcZ