refactor(net): encapsulate Group.state behind non-blocking read methods

[kixelated]

Summary

Makes @moq/net's Group.state private and routes the one external reader through new public methods, then uses those methods to give @moq/json's consumer the same backlog-collapse the Rust consumer already has.

Follow-up to #1905 (which extracted moq-flate/@moq/flate and added the Rust-side backlog-collapse). This brings the JS side to parity and cleans up the leaky Group surface that made it possible.

What changed

@moq/net — Group encapsulation

• Group.state is now #state (private); GroupState is no longer exported. Callers can't poke the reactive signals directly, and the internal representation can change freely.
• New public methods (the non-blocking equivalents of Rust's poll_read_frame):
  • tryReadFrame() / tryReadFrameSequence() — read an already-buffered frame without blocking; return undefined when nothing is buffered.
  • get done() — true once the group has closed and every buffered frame is read (no more frames will ever be read). Disambiguates tryReadFrame()'s undefined: "nothing buffered yet" vs "finished".
  • readable() — resolves once readFrame would not block (a frame is buffered, or the group closed). Always settles, so pair it with done to avoid re-waiting on a finished group.
• readFrame/readFrameSequence are reimplemented on top of these (behavior unchanged).
• Track.readFrameSequence (the only external reader of a Group's state) now uses tryReadFrameSequence() + done + races group.readable() instead of reaching into group.state.frames.

@moq/json — backlog-collapse parity

• Consumer.next() drains every buffered frame in the group but yields only the latest reconstructed value, so a late joiner (or any consumer that fell behind) catches up to the head in one step instead of replaying every superseded state. Live consumers still see each update.

Livelock fix (found during review)

The first cut returned undefined from tryReadFrame() for both "nothing buffered yet" and "finished", and readable() resolves immediately on a closed group. Track.readFrameSequence therefore busy-looped on a sole finished group: Promise.race([…, group.readable()]) resolved on every microtask, starving the macrotask queue so a newly-arriving group was never observed (an event-loop livelock, not just a spin). The done getter fixes it: Track only folds the current group's readable() into its wait while the group can still deliver frames. Covered by a new regression test.

Public API changes (@moq/net)

• Added: Group.tryReadFrame, Group.tryReadFrameSequence, Group.readable, Group.done.
• Removed (breaking, but incidental): the public Group.state field and the GroupState export. Both were unintentionally-public internal reactive state; the only reader anywhere in the repo was Track itself, now migrated. No published consumer is known to use them.
• @moq/json's exported surface is unchanged.

Branch targeting

Targeting main: it builds directly on #1905 (already on main), and the removed surface was incidental/unused. The Group.state/GroupState removal is technically breaking per the js/net rules, so if you'd rather this go to dev, say the word and I'll retarget (it would need rebasing off dev, which doesn't yet have #1905's @moq/flate consumer changes).

Test plan

• bun test — full suite 406 pass (incl. new js/net/src/group.test.ts, the Track livelock regression, and the json collapse test)
• tsc + Biome clean across all JS packages
• New tests: Group tryReadFrame/tryReadFrameSequence/readable/done semantics (buffered drain, sequence numbers, resolve-on-frame, resolve-on-close, drain-after-close, finished-vs-empty); Track "does not livelock when a sole group finishes before the next arrives"; json "late joiner collapses a buffered backlog to the latest value"

(Written by Claude)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c84f7545-cce7-4d33-93a3-83ed38feea35

📥 Commits

Reviewing files that changed from the base of the PR and between fd94ee9 and b1efe77.

📒 Files selected for processing (1)

• js/net/src/group.test.ts

✅ Files skipped from review due to trivial changes (1)

• js/net/src/group.test.ts

---

Walkthrough

Consumer next() now drains buffered frames before waiting and returns the latest reconstructed value for late joins. In the net package, Group state is private, non-blocking read helpers and readable() were added, Track.readFrameSequence() now uses them, and tests cover buffered reads, closure, sequence values, and read liveness.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main net refactor: encapsulating Group.state behind new non-blocking read methods.
Description check	✅ Passed	The description clearly matches the refactor and backlog-collapse changes described in the diff.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/net-group-encapsulation

---

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: 1

🧹 Nitpick comments (1)

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

27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert readable() is pending before writing.

This test would still pass if readable() incorrectly resolved immediately for an empty, open group. Add a microtask check before writeString() so the new wait contract is actually covered.

Test tightening

 test("readable resolves once a frame is buffered", async () => {
 	const group = new Group(0);
 	// No frame yet: readable() is pending. Resolve it by writing on the next tick.
 	const readable = group.readable();
+	let settled = false;
+	readable.then(() => {
+		settled = true;
+	});
+	await Promise.resolve();
+	expect(settled).toBe(false);
 	group.writeString("hi");
 	await readable; // must not hang
 	expect(dec.decode(group.tryReadFrame())).toBe("hi");
 });

As per coding guidelines, "Write unit tests for critical functionality".

🤖 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/group.test.ts` around lines 27 - 33, The test in Group.readable()
is too weak because it only verifies eventual resolution after writeString(), so
it would miss a bug where readable() resolves immediately on an empty open
group. Update the readable() test to explicitly assert that the promise is still
pending before any write by checking it across a microtask boundary, then
perform group.writeString() and verify it resolves and tryReadFrame() returns
the frame. Use the Group.readable, Group.writeString, and Group.tryReadFrame
symbols to keep the test focused on the wait contract.

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/track.ts`:
- Around line 186-198: In readFrameSequence in track.ts, the current group wake
path can loop forever when group.readable() resolves because the group has
closed and no frames remain. Update the await Promise.race handling so that when
the current group wins the wake, you consume the group result, detect
exhaustion/closure, and remove or advance past that group before re-checking for
the next frame. Keep the logic aligned with the existing group,
this.state.groups, and this.state.closed flow so the reader waits for a new
group instead of spinning on a closed empty one.

---

Nitpick comments:
In `@js/net/src/group.test.ts`:
- Around line 27-33: The test in Group.readable() is too weak because it only
verifies eventual resolution after writeString(), so it would miss a bug where
readable() resolves immediately on an empty open group. Update the readable()
test to explicitly assert that the promise is still pending before any write by
checking it across a microtask boundary, then perform group.writeString() and
verify it resolves and tryReadFrame() returns the frame. Use the Group.readable,
Group.writeString, and Group.tryReadFrame symbols to keep the test focused on
the wait contract.

🪄 Autofix (Beta)

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

Run ID: ec5fe270-cac0-4071-b1e0-5627b67d99c8

📥 Commits

Reviewing files that changed from the base of the PR and between bdad42c and 1251f48.

📒 Files selected for processing (5)

• js/json/src/consumer.ts
• js/json/src/json.test.ts
• js/net/src/group.test.ts
• js/net/src/group.ts
• js/net/src/track.ts

[coderabbitai]

Actionable comments posted: 1

🤖 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/group.test.ts`:
- Around line 49-51: The Promise created in the group test is intentionally not
awaited, but it is currently a floating Promise that can trigger lint/CI
failures and hide async errors. Update the then-call on readable in the test to
be explicitly marked as intentional with void, and keep the settled flag logic
in place so the intent is clear in group.test.ts.

🪄 Autofix (Beta)

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

Run ID: 6a92aa1a-6ca3-4c93-843e-91f3a925202b

📥 Commits

Reviewing files that changed from the base of the PR and between cd11485 and fd94ee9.

📒 Files selected for processing (1)

• js/net/src/group.test.ts