feat(js/net): share one session per relay URL

[kixelated]

Closes #2628.

Summary

• Every connect() dialed its own WebTransport session, so a page showing N broadcasts from one relay opened N sessions (a camera wall pays N handshakes, N congestion controllers competing with each other, and N connection slots on the relay). There was no way to fix it from outside the library: <moq-watch>'s connection field is public but Broadcast/Sync capture the established Signal object, so reassigning it rewires nothing.
• connect() now returns a reference-counted lease on a session shared by URL and options. close() releases your handle; the last one out closes the connection, after a short linger (default 2s) so tearing a component down and rebuilding it costs no handshake. New js/net/src/connection/pool.ts.
• Sharing is skipped where it can't be done honestly: pool: false, a supplied transport, or a pinned server certificate (serverCertificate/serverCertificateHashes are BufferSource | string, which no fingerprint compares safely, and sharing a session pinned to the wrong cert is not a failure worth risking).
• ConnectProps becomes the single options bag for both connect() and Reload, absorbing url, enabled, and a new reload knob. pool, reload, and enabled all default to true, so <moq-watch>, <moq-publish>, moq-boy and demo/web pick this up with no element changes.
• Two adjacent fixes the sharing makes reachable:
  • announced.ts's warnedNoDiscovery was a WeakSet<Established>, so one lease per component meant one warning per component. Rekeyed on relay origin plus path, and capped at 64 entries dropping the oldest. Deliberately not the full href: the query carries the auth token, so keying on it would pin every token an app ever used and mint an entry per rotation. The cap trades exactly-once for a hard bound, which is the right thing to give up for something that exists to keep the log readable.
  • Lite.Publisher.publish / Ietf.Publisher.publish removed a path on producer.closed with no "is this still mine" guard (unlike BroadcastCache.insert). Publishing a path twice on one session and closing the first producer unpublished the live one, and the path stopped answering subscribes. Latent before, reachable once two <moq-publish> elements share a session.
• One pre-existing leak fixed while adding reload: false: Reload settled closed on the retry-timeout path without closing #signals, leaving the pagehide/pageshow/visibilitychange listeners, the probe computed, and the announced() pumps alive on the page. Both terminal paths now run through one #finish() helper that tears the scope down before settling, and close() delegates to it.

Design notes

The reconnect loop stays in Reload, one per consumer; the pool shares only the session. Folding the retry loop into the pool would make delay, webtransport, websocket, discovery, and the retry-timeout rejection on Reload.closed first-acquirer-wins and silently ignored for everyone else. N Reloads that see the same session die all retry in the same turn, so their acquires collapse onto one shared dial anyway.

Two details worth a reviewer's eye:

• The entry reads session.closed once and attaches .then(evict, evict). Established.closed derives a fresh promise per access and WebTransport.closed rejects on abnormal termination, so a one-argument then would both leak an unhandled rejection and leave a dead session in the map for the next caller to lease.
• An acquirer's signal aborts only its own wait, never the shared attempt. When the last acquirer walks away mid-dial the attempt is aborted with no linger, and the entry is evicted synchronously so a caller arriving in the same turn starts fresh rather than joining the doomed one.

Behavior changes

• close() no longer terminates a session by default. It releases your handle; the connection goes when the last one does.
• A shared session never sees its own announcements. We send our origin as exclude_hop on lite-04/05 (the relay filters via select_route) and dropReflected is unconditionally true on lite-06+, so an app that publishes and separately watches the same broadcast over what is now one session stops seeing it and must pass pool: false. Nothing in-repo relies on this: <moq-publish>'s preview renders the local capture or a locally decoded copy of the encoded frames, never a round trip. Documented on ConnectProps.pool and in doc/lib/js/@moq/net.md. Note the AnnouncedOptions.ignoreSelf doc still cites hang.live relying on the old behavior.
• Reload's enabled now defaults to true. It defaulted to false, which made a bare new Reload({ url }) sit there doing nothing. Every in-repo caller passes it explicitly.
• stats() and probe are now session-aggregate. Two <moq-publish> elements sharing a session would each cap their encoder at the full-session estimatedSendRate and together overshoot. Rare (one publish element per page is the norm) and not a regression in kind, since two separate sessions just let congestion control arbitrate. Follow-up if it bites.
• allowPooling: false stays. Browser HTTP/3 pooling would merge the session with unrelated fetch traffic and lose congestionControl: "low-latency"; we share at the moq layer instead.

Public API changes

All additive, so this targets main.

• Added Connection.PoolProps ({ grace?: number }).
• Added ConnectProps.pool, ConnectProps.reload, ConnectProps.url, ConnectProps.enabled. Reload also honors ConnectProps.signal now, closing the whole managed connection.
• Added Reload.reload and Reload.pool fields.
• Added Announce.Broadcast.closed, a promise settling when the handle closes. It was the only closable in the package without one, and the lease needs it to prune handles it handed out.
• Republished ReloadProps (it stays ConnectProps, and remains the constructor's parameter type) so the accepted option shape shows on the Reload page.
• Moved ReloadDelay from reload.ts to connect.ts. Same exported name via Connection.*, no consumer-visible change.
• Hidden, not removed: ConnectProps.delay is @internal, superseded by reload. It still compiles.
• Internal only (not re-exported from connection/index.ts): Pool, Entry, poolKey, sessionPool, resetPool, and createPendingTransports in mock.ts.

Test plan

• bun --cwd js/net test: 348 pass, 0 fail. New pool.test.ts (14 cases) covers sharing, the linger window, eviction when closed rejects, a shared in-flight dial, the three abort cases, every bypass, and lease cleanup. New reload.test.ts cases cover two Reloads sharing one transport, reload: false, the enabled default, and signal. The reload: false case also asserts a later url change does not dial, which only holds if the effect scope really closed. announced.test.ts covers the warning cache deduping per relay across differing tokens and evicting past its cap.
• The publisher fix has a regression test verified to fail without it (a stale producer closing does not unpublish a republished path).
• Existing connect.test.ts / reload.test.ts got beforeEach(resetPool), since they reuse URLs across cases and would otherwise lease each other's sessions.
• just js check clean across all 14 JS packages. moq-doc check fails, but identically on a clean tree (a YAML parse error in doc/.vitepress/drafts.ts on a Windows checkout), so it is pre-existing and unrelated.
• Not run: the browser pass js/CLAUDE.md asks for on playback changes (four tiles on one relay URL showing a single session in devtools, and a DOM move across an await not redialing). No nix in the environment used. Worth exercising before merge.

Cross-package sync

No wire change, so no drafts/ update. rs/moq-net has no equivalent pool and this is JS-only and additive. js/{watch,publish} public API is unchanged, so no demo/web update, though demo/web's own discovery Reload now shares the tiles' session for free.

Not included

Reload still accepts ConnectProps.transport and still ignores it: a supplied session is good for one connection, so a reconnect loop has nothing to reuse after the first drop. That hole predates this PR. It now warns on use rather than failing silently, which is the most that can be done without removing a field from a published type, and removing it is a semver break that belongs in its own dev-targeted PR.

Issue #2627 stays open. The linger keeps the session alive across a detach, so a re-attach skips the handshake entirely, but the element still tears down its broadcast subscription (#enabled gates watch's Broadcast too), so a moved tile re-subscribes and re-renders from the next group. Closing that residual means either an element-level #enabled debounce or the same linger idea applied to BroadcastCache.

(Written by Claude Opus 5)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e52d13bc-de5c-4269-8da0-9497b2b8f1df

📥 Commits

Reviewing files that changed from the base of the PR and between 707da6a and 930e075.

📒 Files selected for processing (5)

• js/net/src/announced.test.ts
• js/net/src/announced.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/reload.ts

🚧 Files skipped from review as they are similar to previous changes (5)

• js/net/src/announced.test.ts
• js/net/src/connection/reload.test.ts
• js/net/src/announced.ts
• js/net/src/connection/reload.ts
• js/net/src/connection/connect.ts

---

Walkthrough

Added URL-keyed connection session pooling with reference-counted leases, grace-period reuse, abort handling, and unsafe-option exclusions. Extended connect and Reload with pooling, reactive options, reload controls, and caller-signal shutdown. Added pool and reload tests. Deduplicated no-discovery warnings per relay URL with bounded cache eviction. Updated publisher cleanup to avoid removing newer registrations when stale producers close.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: sharing one session for each relay URL.
Description check	✅ Passed	The description explains session pooling, related behavior changes, tests, and validation results.
Linked Issues check	✅ Passed	The PR implements URL-keyed session sharing, reference counting, grace-period reuse, and per-element broadcast subscriptions required by issue #2628.
Out of Scope Changes check	✅ Passed	The additional warning, publisher, and reload cleanup changes are connected to pooling behavior and are covered by the stated objectives.

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/plan-issue-2628-2f799d

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (5)

js/net/src/lite/publisher.test.ts (1)

99-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Name the request deadline constant.

100 is a magic timeout value. Define a name such as REQUEST_DEADLINE_MS so the test documents the timing bound.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/lite/publisher.test.ts` around lines 99 - 100, In the test around
live.requested(), replace the magic timeout value 100 with a clearly named
REQUEST_DEADLINE_MS constant, and use that constant when creating the deadline
promise so the request timing bound is documented.

Source: Coding guidelines

js/net/src/connection/reload.ts (1)

16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

@internal on ReloadProps hides the public constructor's option shape.

Reload is public and takes ReloadProps. Marking the whole alias @internal removes it from the generated API documentation, so readers cannot see the accepted options from the Reload page. Only delay needs to be hidden.

Move @internal to the delay member and keep the alias published.

♻️ Proposed doc-tag move

 /**
  * Connection and retry options for {`@link` Reload}.
- *
- * `@internal`
  */
 export type ReloadProps = ConnectProps & {
 	/** `@internal` Superseded by `reload`, which also disables the loop. */
 	delay?: ReloadDelay;
 };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/connection/reload.ts` around lines 16 - 24, Keep the public
ReloadProps type alias published by removing its declaration-level `@internal`
tag, and retain `@internal` only on the delay property. Preserve the existing
deprecation/supersession documentation for delay so the public Reload
constructor option shape remains visible while that member stays hidden.

Source: Coding guidelines

js/net/src/connection/connect.ts (1)

77-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Clarify which options connect() ignores.

Line 80 says "The last three" options apply only to Reload. This depends on declaration order inside the interface, and signal now sits between discovery and url. Name the options (url, enabled, reload) so the sentence stays correct when fields move.

♻️ Proposed doc tweak

- * The last three are about staying connected, so only {`@link` Reload} honors them; {`@link` connect}
- * takes the URL as an argument and connects exactly once.
+ * {`@link` ConnectProps.url}, {`@link` ConnectProps.enabled}, and {`@link` ConnectProps.reload} are about
+ * staying connected, so only {`@link` Reload} honors them; {`@link` connect} takes the URL as an
+ * argument and connects exactly once.

Also applies to: 124-141

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/connection/connect.ts` around lines 77 - 104, Update the
ConnectProps documentation near connect to explicitly name url, enabled, and
reload as the options honored only by Reload, replacing the order-dependent
“last three” wording. Apply the same clarification to the corresponding
documentation block around the additionally referenced section, without changing
behavior or other option descriptions.

js/net/src/connection/pool.test.ts (1)

14-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The linger assertions depend on real timer margins.

expired() waits grace * 3, which is 60 ms against a 20 ms grace. On a loaded CI runner a setTimeout can drift past that margin and the linger assertions can flake. Increase the multiplier, or use Bun's fake timers so the grace expiry is deterministic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/connection/pool.test.ts` around lines 14 - 19, Make the
linger-test expiry deterministic by updating the expired helper near the grace
constant: either increase its wait beyond the current grace * 3 margin enough to
tolerate timer drift, or use Bun fake timers to advance past grace without
relying on wall-clock scheduling. Keep the existing grace-based linger
assertions unchanged.

js/net/src/connection/reload.test.ts (1)

62-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Share one pending-transport stub instead of redeclaring it.

PendingWebTransport now appears twice in this file, once in js/net/src/connection/connect.test.ts at lines 55 to 69, and once in js/net/src/connection/pool.test.ts at lines 54 to 69. The four copies differ only in which counters they increment.

Export a single stub factory from js/net/src/mock.ts, next to createMockTransportPair, and have each test import it. The coding guidelines ask for duplicated helpers to be replaced with shared primitives while the surrounding code is being changed.

Also applies to: 97-108

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/connection/reload.test.ts` around lines 62 - 71, Move the
duplicated PendingWebTransport implementation into a shared factory in mock.ts
alongside createMockTransportPair, parameterizing the close-counter behavior
needed by each test. Update the PendingWebTransport setup in reload.test.ts and
the corresponding implementations in connect.test.ts and pool.test.ts to import
and use that factory, preserving each test’s existing counter updates.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/net/src/announced.ts`:
- Around line 132-135: Bound the module-scoped warnedNoDiscovery cache so
disconnected relay URLs are eventually evicted instead of retained indefinitely.
Update the warning logic that keys entries by conn.url.href to use a bounded or
connection-lifecycle-aware cache while preserving at-most-once warnings per
active relay; add regression coverage proving distinct URLs are handled and old
entries are evicted.

In `@js/net/src/connection/pool.ts`:
- Around line 272-280: The announcedBroadcast method must remove each created
announce.Broadcast from `#broadcasts` when it closes. Add a stored cleanup
callback for the watch that closes its active consumer and deletes the watch
from the tracking set, following the cleanup pattern used by announced, publish,
and consume.
- Around line 286-305: Clarify the contract of the pool handle’s closed getter
and its close method: document that closed returns the shared session lifetime
via session.closed, not when this individual lease releases its resources. Keep
the existing close cleanup and reference-release behavior unchanged.

In `@js/net/src/connection/reload.ts`:
- Around line 213-219: Update the Reload completion handling around
`#closedResolve/`#closedReject to use one shared helper that first closes
`#signals`, then settles closed with the original success or normalized error.
Apply this helper to both the reload: false branch and the retry-timeout branch,
preserving their existing resolution and rejection behavior.

In `@js/net/src/lite/publisher.test.ts`:
- Around line 94-105: Update the test teardown around publisher.runSubscribe so
the subscribe handler is retained and awaited instead of started with void.
After the request is received, complete or close the returned track to release
the handler’s track.info() wait; in finally, close the track, Publisher, and
client as appropriate, then await the runSubscribe promise to ensure all mock
resources terminate.

---

Nitpick comments:
In `@js/net/src/connection/connect.ts`:
- Around line 77-104: Update the ConnectProps documentation near connect to
explicitly name url, enabled, and reload as the options honored only by Reload,
replacing the order-dependent “last three” wording. Apply the same clarification
to the corresponding documentation block around the additionally referenced
section, without changing behavior or other option descriptions.

In `@js/net/src/connection/pool.test.ts`:
- Around line 14-19: Make the linger-test expiry deterministic by updating the
expired helper near the grace constant: either increase its wait beyond the
current grace * 3 margin enough to tolerate timer drift, or use Bun fake timers
to advance past grace without relying on wall-clock scheduling. Keep the
existing grace-based linger assertions unchanged.

In `@js/net/src/connection/reload.test.ts`:
- Around line 62-71: Move the duplicated PendingWebTransport implementation into
a shared factory in mock.ts alongside createMockTransportPair, parameterizing
the close-counter behavior needed by each test. Update the PendingWebTransport
setup in reload.test.ts and the corresponding implementations in connect.test.ts
and pool.test.ts to import and use that factory, preserving each test’s existing
counter updates.

In `@js/net/src/connection/reload.ts`:
- Around line 16-24: Keep the public ReloadProps type alias published by
removing its declaration-level `@internal` tag, and retain `@internal` only on the
delay property. Preserve the existing deprecation/supersession documentation for
delay so the public Reload constructor option shape remains visible while that
member stays hidden.

In `@js/net/src/lite/publisher.test.ts`:
- Around line 99-100: In the test around live.requested(), replace the magic
timeout value 100 with a clearly named REQUEST_DEADLINE_MS constant, and use
that constant when creating the deadline promise so the request timing bound is
documented.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: df7d2433-7b71-46a2-a24f-9c86c4d32d15

📥 Commits

Reviewing files that changed from the base of the PR and between 3c445d5 and 66e88c7.

📒 Files selected for processing (12)

• doc/lib/js/@moq/net.md
• js/net/src/announced.ts
• js/net/src/connection/connect.test.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/index.ts
• js/net/src/connection/pool.test.ts
• js/net/src/connection/pool.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/reload.ts
• js/net/src/ietf/publisher.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/lite/publisher.ts

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

js/net/src/connection/reload.ts (1)

16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Describe ReloadProps by its current behavior.

@internal Superseded by ... describes migration history. The TypeScript guidelines require comments to describe current behavior. Use a current contract description instead.

Proposed wording

-/** `@internal` Superseded by {`@link` ConnectProps}, which {`@link` Reload} takes directly. */
+/** Alias for {`@link` ConnectProps} accepted by {`@link` Reload}. */

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/connection/reload.ts` around lines 16 - 17, Update the JSDoc for
the ReloadProps type alias to describe its current contract and relationship to
ConnectProps, removing the migration-history wording about being superseded.
Keep the alias itself unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/net/src/connection/reload.ts`:
- Around line 16-17: Update the Reload contract around ReloadProps so it does
not accept ConnectProps.transport unless Reload’s connection path explicitly
forwards and consumes that pre-existing transport. Prefer excluding transport
from the Reload input type, while preserving the other ConnectProps fields and
the existing Reload behavior.
- Around line 329-344: Update Reload.#finish to reset the public established
signal and set status to "disconnected" before closing `#signals`, while
preserving the existing closed settlement and cause handling. Add a regression
test that connects successfully, calls Reload.close(), and verifies established
is cleared and status is "disconnected".

---

Nitpick comments:
In `@js/net/src/connection/reload.ts`:
- Around line 16-17: Update the JSDoc for the ReloadProps type alias to describe
its current contract and relationship to ConnectProps, removing the
migration-history wording about being superseded. Keep the alias itself
unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f5572ca4-a727-49e1-8650-b1703ae9bb97

📥 Commits

Reviewing files that changed from the base of the PR and between 66e88c7 and 7898cc5.

📒 Files selected for processing (9)

• js/net/src/announced.ts
• js/net/src/connection/connect.test.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/pool.test.ts
• js/net/src/connection/pool.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/reload.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/mock.ts

🚧 Files skipped from review as they are similar to previous changes (5)

• js/net/src/connection/connect.ts
• js/net/src/announced.ts
• js/net/src/lite/publisher.test.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/pool.ts

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

js/net/src/announced.test.ts (1)

94-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Derive the eviction input from a named cache limit.

The literal 200 does not state the cache-bound contract. Derive the handle count from a named production cache limit through a test hook, then use limit + 1. This keeps the eviction test valid if the production bound changes.

As per coding guidelines, "Avoid using magic numbers; use named constants instead."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/net/src/announced.test.ts` around lines 94 - 100, Update the eviction test
around Announce.Broadcast to obtain the production cache limit through the
existing test hook or an appropriate named symbol, then set the generated handle
count to that limit plus one instead of the literal 200. Preserve the test’s
eviction behavior while keeping it aligned with future production cache-bound
changes.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/net/src/announced.test.ts`:
- Around line 75-85: Extend the test case around countWarnings to include
another connection with the same origin but a different connection.url.pathname,
while keeping the existing token and broadcast-path variations. Update the
expected warning count so distinct relay URL pathnames produce separate
warnings, preserving the one-warning behavior for identical relay pathnames
regardless of auth token.

In `@js/net/src/connection/reload.test.ts`:
- Around line 297-311: Strengthen the “a supplied transport is refused out loud”
test by stubbing globalThis.WebTransport with a construction counter, asserting
it remains zero after creating Reload with enabled: false, and verifying the
warning contains the expected message. Preserve the existing cleanup for Reload,
the transport pair, the warning spy, and the WebTransport stub.
- Around line 271-294: Make the teardown in the affected Reload tests
exception-safe by moving WebTransport, console.warn, and mock-transport setup
into try blocks. Track each constructed Reload and close it, awaiting its closed
promise in finally even when setup, waitUntil, or assertions fail; restore
globals and spies and close supplied transports through nested cleanup,
including constructor-failure paths.

---

Nitpick comments:
In `@js/net/src/announced.test.ts`:
- Around line 94-100: Update the eviction test around Announce.Broadcast to
obtain the production cache limit through the existing test hook or an
appropriate named symbol, then set the generated handle count to that limit plus
one instead of the literal 200. Preserve the test’s eviction behavior while
keeping it aligned with future production cache-bound changes.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b9725309-7627-4dff-8886-74234181ed67

📥 Commits

Reviewing files that changed from the base of the PR and between 7898cc5 and 707da6a.

📒 Files selected for processing (5)

• js/net/src/announced.test.ts
• js/net/src/announced.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/reload.test.ts
• js/net/src/connection/reload.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• js/net/src/announced.ts
• js/net/src/connection/connect.ts
• js/net/src/connection/reload.ts

[kixelated]

Superseded by #2705, which reached the same goal one layer down.

This PR pooled sessions and leased them out, which works but needs three standing rules to stay honest: close() stops meaning close, Established's docs stop being true, and a page that publishes and watches the same broadcast has to pass pool: false because a session never sees its own announcements. The root cause was that publish and consume were owned by sessions, so nothing survived a session and nothing could be shared. #2705 moves ownership to the origin, where rs/moq-net already has it, and pools {origin, reconnect loop} instead. Local and remote routes end up in separate maps, so one origin on both directions of a connection is echo-free by construction and a local publish resolves with no round trip.

Both adjacent fixes from this PR are carried forward there: the publisher stale-close guard lives in origin.publish with a regression test, and the Reload scope leak's paths were restructured out. The warnedNoDiscovery cache is deleted outright rather than rekeyed, since the origin owns discovery state now.

Closing unmerged. Nothing here is lost.

(written by Opus 5)