[jacderida]

Linear issue

Closes V2-1871

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Observability only: no cap, policy or admission change. The request worker gains
an Instant::now() and, for GETs, two relaxed fetch_add/fetch_max pairs; the
new summary loop only reads atomics and semaphore permit counts, so it holds no
lock across an await and never blocks the accept loop or a request worker.

Compatibility

• Wire: none. No request, response or framing change.
• Storage: none.
• API: additive and crate-internal. WebRtcServerSnapshot gains nine pub
  fields and WebRtcServerDiagnostics a log_webrtc_listener_summary() method.
  Both live in the private web_rtc module, so no external name is affected;
  the only readers are src/devnet.rs and tests/webrtc_direct_devnet.rs.

Semver impact

• breaking
• feature
• fix

Test evidence

Run locally at rustc 1.99.0, both clean: cargo fmt --all -- --check and
cargo clippy --all-targets --all-features -- -D warnings. The crate denies
panic, unwrap_used and expect_used, so the ms conversion goes through
u64::try_from(..).unwrap_or(u64::MAX). Everything else is left to this PR's
checks, including cargo check --lib --no-default-features and the test matrix.

Security Audit fails on RUSTSEC-2026-0387 in the transitive crypto-bigint.
That is unrelated to this PR, which adds no dependency and does not touch
Cargo.lock.

Tests added or extended in src/web_rtc.rs, exercised by this PR's checks:

• diagnostics_report_capacity_rate_and_byte_pressure_without_leaking_slots
  extended to assert the seven new counters start at 0, bulk_slots_in_use
  is 0, and bulk_slots_max is 1 at that test's 8-byte frame budget.
• latency_counters_accumulate_and_the_summary_logs_the_snapshot — two
  durations through each recorder, asserting count, sum and max, that
  bulk_slots_max is 4 at the default config, and that
  log_webrtc_listener_summary emits both lines without panicking.
• one_get_records_a_bulk_wait_and_a_service_time — drives one GET through
  the real start_request worker against a storage-disabled ServerState,
  asserting bulk_wait_count == 1, get_service_count == 1,
  get_service_failures == 0, and that the bulk slot is released after the
  response is dropped.

Not yet observed on a testnet; the line is INFO on a 300 s cadence, same as the
replication and chunk-RPC summaries.

New dependency

none

ADR

docs/adr/ADR-0015-direct-browser-clients-over-webrtc-direct.md — the
"Internal listener diagnostics" section now documents the line, its cadence and
every field. (n/a as a gate at T1; updated because the ADR owns this surface.)

Mitigation / rollback

Revert the commit, or raise WEBRTC_SUMMARY_INTERVAL_SECS if the cadence proves
noisy. The instrumentation is read-only accounting, so dropping it cannot affect
request admission or responses.

🤖 Generated with Claude Code

[MaidSafe-Hermes]

REQUEST_CHANGES — the emitted metric contract is inaccurate.

At docs/adr/ADR-0015-direct-browser-clients-over-webrtc-direct.md:344-346 and src/web_rtc.rs:707-709, the text says every field except capacity/slot fields is a cumulative counter that can be differenced for rates. That is false for running, active_connections, active_channels, active_requests, and transport active/connecting connection counts (gauges), and for get_service_max_ms / bulk_wait_max_ms (lifetime high-water marks). Differencing these can report meaningless rates and conceal sustained high latency. Please classify additive counters, instantaneous gauges, and lifetime maxima separately in the ADR and rustdoc.

Also align the latency comments in src/web_rtc.rs: get_service_total_ms and record_get_service say timing starts at request admission, but start_request starts the clock when the spawned worker begins. Either document that boundary consistently or move the start time if admission-to-response latency is intended.

Coverage: 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The seats disagreed on verdict; sol-1 required changes and glm-1 approved while flagging the same metric-contract error. I confirmed the error against the source, so it blocks approval.

Checks at this head: Quick checks, Rust 1.95 MSRV, release build, filesystem jobs, ADR validation and PR checks pass; macOS/Ubuntu/Windows tests are still pending. Security Audit fails on crypto-bigint 0.5.5 (RUSTSEC-2026-0387); this PR does not change Cargo.lock, so that failure is inherited, not introduced here.

[jacderida]

Both points are correct — thanks. Fixed in 22502f9.

1. Metric classification

The old sentence ("every other field is a counter accumulated since process
start") was wrong for the gauges and for the two maxima, exactly as described.
The ADR and the rustdoc on log_webrtc_listener_summary now classify every
field into four classes and say which one may be differenced:

• Additive counters — the rejection and handler-error counts,
  get_service_count, get_service_total_ms, get_service_failures,
  bulk_wait_count, bulk_wait_total_ms, and every transport_ field except
  the two active/connecting gauges. The only fields a query may diff. Mean GET
  latency over an interval is the get_service_total_ms diff over the
  get_service_count diff.
• Instantaneous gauges — running, active_connections,
  active_channels, active_requests, in_flight_bytes,
  bulk_slots_in_use, transport_active_connections,
  transport_connecting_connections. Levels, not totals; differencing them
  reports a meaningless rate.
• Lifetime maxima — get_service_max_ms and bulk_wait_max_ms, never
  reset, so they only rise. The docs now say outright that a diff is 0 for any
  window that does not beat the all-time worst case, so differencing them
  conceals sustained high latency, and that the counter-derived mean is what to
  alert on.
• Configured capacities — max_connections, max_channels,
  max_concurrent_requests, max_in_flight_bytes, bulk_slots_max. Constant
  for the process, emitted so a gauge reads as a fraction of its limit.

One thing I did not change: the message string is still
webrtc listener summary (cumulative). That follows the existing house style
(chunk rpc traffic summary (cumulative), replication traffic summary (cumulative)), and the message is the field-contract key a query matches on,
so renaming it has a cost beyond this PR. The per-class detail now lives in the
docs instead. Say the word if you'd rather the message itself stopped claiming
"cumulative".

2. Timing boundary

Also correct — get_service_total_ms and record_get_service claimed
"request admission" while the clock starts when the spawned worker begins.

I fixed the documentation rather than moving the clock, and renamed the
variable admitted_at to worker_started_at so the code stops implying the
wrong thing. Reasoning:

• Admission (try_admit_request) happens in read_webrtc_request, before
  frame assembly and PQ decryption finish. Starting the clock there would fold
  the browser's frame upload and the decrypt into "service time", which is not
  the number this issue is after — V2-1795 was about whether GETs queue on the
  four global bulk slots, and that queueing is entirely inside the worker.
• V2-1871 specifies this placement: "Take let admitted_at = Instant::now();
  as the first statement inside the spawned closure." Only the variable name it
  suggested was misleading.
• Measuring from admission would also mean threading an Instant through
  AdmittedRequest from the read path.

The docs on the field, the recorder and the ADR now all state the same
boundary: from the spawned request worker starting to its response being
prepared, including the bulk-slot wait, excluding admission, frame assembly and
decryption. If you want admission-to-response instead, that is a small change
(an Instant on AdmittedRequest) and I am happy to make it — it just
measures something different from what the issue asked for.

Checks

Agreed on Security Audit: RUSTSEC-2026-0387 in transitive
crypto-bigint 0.5.5, inherited rather than introduced, and this PR does not
touch Cargo.lock.

[MaidSafe-Hermes]

Focused re-review of the follow-up delta from 78202bb to 22502f9; this is not a fresh full-PR panel.

Both seats reviewed and approve the delta (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The ADR and logger rustdoc now distinguish additive counters, instantaneous gauges, lifetime maxima and configured capacities. The timing documentation and variable name now consistently identify the worker-start boundary. I found no remaining blocker in the reviewed changes.

The branch-protected checks (linear-link and pr-template) pass. Ubuntu tests, Quick checks, MSRV, release build, filesystem checks and ADR validation also pass. macOS and Windows tests are still pending; this follow-up changes documentation and renames a local variable, with no runtime behavior change. Security Audit remains failed on the known inherited crypto-bigint 0.5.5 advisory (RUSTSEC-2026-0387); this PR does not change Cargo.lock.