Add gateway WebView and pending connection admission by RyanKung · Pull Request #666 · RingsNetwork/rings

RyanKung · 2026-07-20T19:24:18Z

Summary

Add a reusable rings-webview crate for controlled gateway URLs, HTML/CSS rewriting, cookie/header policy, rendering, browser bootstrap, and gateway tests.
Integrate the WebView entry into the frontend with a dedicated window-style browsing UI, service worker host assets, debug controls, and Rings gateway request flow.
Harden core connection admission with a bounded pending pool so pending handshakes never enter the routable/DHT view before data-channel open.
Make wasm/web-sys cfg target-aware so native --all-features builds do not select wasm-only transport semantics.

Why

The WebView path needs a controlled origin that can render pages through the local Rings gateway without relying on browser extensions. The DHT transport also needed a real pending-handshake model so speculative connections cannot be treated as logical routing entries.

Closes #665

Validation

git diff --check
cargo test -p rings-core --all-targets --all-features -q
cargo test -p rings-transport --all-targets --all-features -q
cargo test -p rings-core --all-targets --features dummy -q
cargo test -p rings-core --lib -q
cargo clippy -p rings-core --all-targets --all-features -- -D warnings
touched-file rustfmt --check

RyanKung

One blocking CORS-isolation finding.

RyanKung · 2026-07-22T08:26:34Z

+}
+
+function requestKind(request) {
+  const taggedKind = request.headers.get("x-rings-webview-kind");


P1: X-Rings-Webview-Kind is page-writable, but this value controls whether the host runs the virtual CORS checks. The bootstrap first adds xhr, then page code can call xhr.setRequestHeader("X-Rings-Webview-Kind", "subresource"); browsers combine duplicate request headers, yielding xhr, subresource. That misses the exact matches below and falls back to subresource, so WebviewHostRequest::subresource skips virtual CORS even though the XHR caller can read the response. Please derive the request kind from trusted host state (or reject malformed/multiple runtime tags) rather than treating an unrecognized tag as a subresource.

RyanKung

Rustacean re-review: the pushed head still has required WASM/clippy failures.

RyanKung · 2026-07-22T09:26:42Z

+        .dht_finger_table_size(TEST_DHT_FINGER_TABLE_SIZE)
+        .build()
+        .map_err(|error| WebviewError::Transport(format!("build processor: {error:?}")))?;
+    let provider = Arc::new(Provider::from_processor(Arc::new(processor)));


P1: this makes the required WASM/clippy job fail. Provider and Processor are explicitly neither Send nor Sync, so wrapping them in Arc triggers clippy::arc_with_non_send_sync (the CI log points here). This browser-only, single-threaded ownership needs Rc throughout its local test boundary, or a justified Send+Sync boundary. The same job also reports the value.as_ref() call below as useless. Please make this test compile cleanly under the repository's -D warnings policy.

RyanKung · 2026-07-22T09:26:42Z

+})();
+"#;
+
+thread_local! {


P1: the required clippy job also rejects this new thread_local! initializer with clippy::missing_const_for_thread_local under -D warnings. Use a const { ... } initializer (or otherwise make the intentional runtime initialization explicit) so the PR's required lint job is green.

RyanKung

The lifecycle model still needs a formal witness.

RyanKung · 2026-07-22T09:26:54Z

+
+/// Bounded, non-routable handshakes owned by the swarm lifecycle.
+///
+/// The pool deliberately has no DHT reference: a peer is visible to Chord


P2: the pending-admission invariant is documented, but it has no executable state-transition model. Pending -> Active is split across this pool, InnerSwarmCallback, and DHT join/leave effects; the new Stateright-style model covers storage CRDT convergence, not this lifecycle. Add a finite model (or a property/state-machine test) over Absent/Pending(generation)/Active with open, close, failed, timeout, replacement, and late-callback actions. It should witness: a pending peer is never routable; only the matching generation may promote; and every terminal/expiry transition removes the DHT membership and transport slot. The current example tests do not cover those event orderings.

RyanKung

Fresh review of the latest feedback commit: required CI still has reproducible blockers.

RyanKung · 2026-07-22T11:03:53Z

+    addEventListener() {},
+  },
+};
+context.globalThis = context;


P1: this currently breaks the required WASM job before the frontend checks run. npm run build:frontend-extension-scripts reports TS4111 here because ServiceWorkerTestContext has an index signature: access this as context["globalThis"] = context (or model globalThis as an explicit property). The local Node execution passes, but the generated-script TypeScript check in CI does not.

RyanKung · 2026-07-22T11:03:53Z

+            peer,
+            generation: self.next_generation,
+        };
+        self.peers.insert(


P1: cargo +nightly fmt --all -- --check still reports this block (and the analogous assertion formatting in transport/tests.rs) as unformatted. QACI runs that exact nightly command, so the required rustfmt job will fail. Please format with the repository's nightly toolchain before pushing.

RyanKung · 2026-07-22T11:03:53Z

+            });
+        }
+
+        self.next_generation = self.next_generation.wrapping_add(1);


P2: the new model is not a refinement of the implementation at the generation-exhaustion boundary. The production counter wraps here, whereas LifecycleModel::replace_pending uses saturating_add; consequently the model never represents the ABA case where an old callback generation becomes equal to a new live generation after wraparound. Either make production generation allocation fallible/non-reusing and model that error, or give the model the same wrap semantics and explicitly state the accepted bound. The claimed generation-safety proof is otherwise incomplete.

RyanKung

Node/wasm follow-up: the wasm CI job is currently blocked by the new registration test.

RyanKung · 2026-07-22T21:20:03Z

+        ["a", "b", "c"]
+            .into_iter()
+            .enumerate()
+            .filter_map(|(bit, value)| (mask & (1 << bit) != 0).then(|| encoded(value)))


P1: This new property test makes the wasm CI job fail. The workflow runs clippy with -D warnings, and this filter_map+bool::then is rejected by clippy::filter_map_bool_then, so Build and test for wasm cannot complete. Please rewrite it as filter(...).map(...) (or otherwise avoid the lint) and rerun the wasm job.

RyanKung

Native follow-up: the native examples/lint workflows are also blocked by the new candidate rewrite.

RyanKung · 2026-07-22T21:20:31Z

+    }
+
+    let fields = line.split_whitespace().collect::<Vec<_>>();
+    if fields.len() < 8 || fields[6] != "typ" || fields[7] != "host" {


P1: The native node CI cannot pass with the repository's panic-free clippy policy. Even though the preceding len() < 8 guard makes these indices logically safe, cargo clippy -p rings-node --features ffi treats clippy::indexing_slicing as an error here (and at the related accesses below), so both native examples and the lint workflow fail. Destructure the required fields with checked access/an iterator before using them.

RyanKung

P1 — The runtime abstraction is not consistent across native and wasm.

The core changes now model browser semantics with all(feature = "wasm", target_family = "wasm"): a native build that enables the wasm feature remains Send-capable and uses native time/storage implementations. rings-node still defines the corresponding runtime distinction with feature = "browser" alone:

extension/ext/mod.rs:58-66 makes MaybeSend empty whenever the browser feature is enabled;
registration.rs:80-95 and processor/mod.rs:98-111 select window_sleep from that feature alone;
RegistrationTask and its async_trait expansion follow the same feature-only predicate.

Thus the same native target has two incompatible models when both capability features are selected: core requires native Send semantics, while node erases them and attempts JS-only adapters. This is not merely an unsupported feature combination: the PR specifically made the core predicate target-aware, but did not refine the node layer to it. cargo check -p rings-node --all-features demonstrates the broken composition (missing js_utils/IndexedDB plus Send and wasm-bindgen errors).

Please define one target-aware runtime predicate (or use cfg(all(feature = "browser", target_family = "wasm")) consistently) for node's MaybeSend, async-trait, timers, browser modules, and exports. Add a native “browser feature enabled” compile check so this refinement cannot drift again.

RyanKung

P2 — The lifecycle capability is implemented below the public browser boundary, so native and wasm expose different abstractions.

Native Provider exposes listen_with(StopToken) in crates/node/src/provider/mod.rs:293-299. The wasm-exported Provider::listen in provider/browser/provider.rs:619-625 still always calls Processor::listen(), which constructs StopToken::never(); JavaScript callers therefore cannot request the lifecycle that the new model introduces. The frontend works around this by retaining a separate Processor and spawning Processor::listen_with directly in frontend/src/node.rs:140-156.

The Provider facade is no longer the runtime-neutral boundary: native callers control the loop through Provider, while browser callers must bypass Provider and know its implementation. Expose an equivalent browser lifecycle handle/stop operation at the Provider boundary (or explicitly make the lifecycle owner a separate cross-runtime type) and add a parity test for start → stop → settled listener in both adapters.

RyanKung

P2 — The public browser boundary is now present, but the webview frontend still bypasses it.

Provider::listen() now returns ProviderListener, yet frontend/src/node.rs:140-163 retains an Arc<Processor>, creates a separate StopSource, and starts Processor::listen_with directly. Consequently the same browser node has two lifecycle abstractions: external callers use ProviderListener, while the in-repo webview owner uses an unrelated source/token pair below the Provider boundary.

The boundary is not yet the single model. Store ProviderListener in DemoNode and implement DemoNode::stop through it (or make a single runtime-neutral listener type used by both adapters). Add a frontend-level test that proves the owned listener task settles after DemoNode::stop(); the current wasm test only observes StopSource::is_stop_requested, not listener completion.

RyanKung added 7 commits July 21, 2026 03:23

Add gateway webview and pending connection admission

6b6c0ae

test browser onion webview flow

aa5d714

Implement webview gateway diagnostics

a3af9de

Fix DHT successor sync for browser joins

225c173

Clean stale DHT topology peers

90ab44f

Document DHT cleanup invariant

a67741c

Fix webview gateway review issues

79b7f36

RyanKung commented Jul 22, 2026

View reviewed changes

Address webview gateway review feedback

559e1d5

RyanKung commented Jul 22, 2026

View reviewed changes

Harden webview cluster lifecycle

4a41f4c

RyanKung commented Jul 22, 2026

View reviewed changes

Fix browser DHT liveness races

ef2302a

RyanKung commented Jul 23, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

RyanKung commented Jul 20, 2026

Summary

Why

Validation

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

RyanKung left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant