feat(btest): TCP multi-connection fan-out (#87) + UDP client receive CHR gate (#88)

[mobileskyfi]

Targets main (#98, the #84/#85 fixes, is merged). Implements the TCP multi-connection fan-out (#87) and gates the btest client's UDP receive/both path (#88), both validated against real CHR 7.23.1.

#87 — TCP multi-connection fan-out

The centrs btest client now realizes --connection-count > 1: it reads the session token from the primary's OK and opens connection-count − 1 additional TCP data connections, driving them into one shared BandwidthCounters so throughput aggregates. (#98 wired the flag into the command packet; this moves the data.)

Grounded byte-for-byte against RouterOS 7.23.1 (CHR probes, all directions):

• Server OK carries the session token in bytes 1-2 BE; RouterOS generates a non-zero token for connection-count > 1.
• Each secondary sends a 16-byte join [token:u16 BE][0x02][0 …] — byte[2]=0x02 is a constant, direction-independent marker.
• The server sends a 4-byte HELLO per connection, then no ack before data (it waits for all connections to join). The join must not block on a reply read — a bug the CHR gate caught (a blocked secondary stalled the loop, RouterOS timed out and dropped it).

Unauthenticated fan-out works end to end (CHR: activeConnections == 4, data on every connection). Authenticated (EC-SRP5) sessions stay single-stream and warn — the post-auth token isn't captured (follow-up #103). No throughput-rise assertion: the near-zero-latency SLIRP loopback is bandwidth-bound, so that's a WAN/latency property; the per-connection drive is asserted deterministically by the unit test.

#88 — UDP client receive/both CHR gate

The reverse path (server→client UDP) already lands over the guest→host SLIRP gateway (10.0.2.2:clientUdpPort) with only the existing TCP control forward — it works because PR #86 left the client socket unconnected. So no quickchr change was needed (the socket-connect L2-bridge idea from the plan turned out unnecessary). Added UDP receive/both cycles to btest-client.test.ts. The one remaining unproven UDP edge is the server-cell host→guest direction (documented).

Review

Copilot + CodeRabbit comments addressed (secondary-IP pin, try/finally socket cleanup, doc-consistency); the random-data demux guard from #98 is on main. Follow-ups filed: #100 (server-side secondary-accept), #103 (EC-SRP5 multi-conn). quickchr networking-discoverability feedback: tikoci/quickchr#18.

Closes #87
Closes #88

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds TCP multi-connection fan-out for unauthenticated btest client sessions, records realized active connections, changes the connection-count warning to use realized fan-out, and updates validation coverage and documentation.

Changes

btest client fan-out and validation

Layer / File(s)	Summary
TCP fan-out session flow src/protocols/btest-session.ts, src/protocols/btest.ts, test/unit/btest.test.ts	The session layer opens secondary TCP connections from the negotiated token, aggregates their counters into activeConnections, and updates the secondary-join encoding and unit test.
Active-connection warning contract src/btest.ts, src/core/error-catalog.ts, docs/errors/routeros/btest-connection-count-single-stream.md, test/unit/btest-command.test.ts	btestClient now warns after the session when realized fan-out is below the requested connectionCount, and the warning text and command tests reflect the active connection count.
Session and CHR validation test/unit/btest-session.test.ts, test/integration/btest-client.test.ts	New unit and integration tests cover TCP multi-connection fan-out, secondary join payloads, and CHR UDP receive/both runs.
Command docs and MATRIX commands/btest/README.md, commands/btest/examples.md, docs/MATRIX.md	The btest README, examples, MATRIX notes, and related error text describe implemented unauthenticated TCP fan-out, authenticated single-stream behavior, and the updated CHR and SLIRP coverage status.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• btest authenticated (EC-SRP5) multi-connection fan-out — capture the post-auth session token #103: The PR keeps authenticated EC-SRP5 multi-connection sessions single-stream, which overlaps with the issue’s focus on post-auth token capture for fan-out.

Possibly related PRs

• tikoci/centrs#12: This PR builds on the same btest secondary-join and fan-out path by adding realized active-connection tracking and expanded validation.

Poem

A bunny hopped with token light,
and split the streams both left and right.
One join became a fluffle of play,
with 0x02 to mark the way.
🐰 Thump-thump—more bytes took flight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	TCP fan-out and UDP receive/both are implemented, but [#87] still lacks the required CHR throughput-vs-single verification in CI; [#88] appears covered.	Add a real CHR assertion comparing multi-connection throughput against single-connection, or revise #87 acceptance if that metric is intentionally out of scope.
Description check	⚠️ Warning	The author provided no description, so none of the required template sections are filled in.	Add the required Pull Request, Links, Change type, and Notes sections, including linked issue/spec IDs and validation details.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the two main changes: TCP fan-out for #87 and UDP client receive CHR gating for #88.
Out of Scope Changes check	✅ Passed	The diff stays focused on btest fan-out, UDP gating, and related docs/tests without unrelated feature work.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/btest-multiconn-fanout

---

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.}

[mobileskyfi]

@coderabbitai review

Note for reviewers: this PR is stacked on #98 (base is its branch) to keep this diff scoped to the #87 fan-out. CI workflows and CodeRabbit auto-review only run on main-targeted PRs, so this will be retargeted to main (full CI + auto-review) once #98 merges. The #87 work has been locally validated: lint/test/build green and CHR 7.23.1 integration passing (client opens 4 connections against real /tool/bandwidth-server).

[coderabbitai]

@mobileskyfi Understood — I’ll review the changes with the stacked-on-#98 context in mind.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull request overview

Implements TCP multi-connection fan-out for centrs btest client when --connection-count > 1 by opening negotiated secondary data connections and aggregating counters across streams, along with updated reporting (activeConnections), warnings, tests, and docs.

Changes:

• Open connection-count - 1 secondary TCP connections using the negotiated session token and drive bulk loops into shared counters (activeConnections added to outputs).
• Update warning behavior to reflect realized fan-out (post-session) rather than pre-session “single stream” messaging.
• Add/expand unit + CHR integration coverage and refresh btest docs/error-page content for the new behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/unit/btest.test.ts	Updates secondary-join codec expectation to include the 0x02 marker byte.
test/unit/btest-session.test.ts	Adds loopback unit coverage asserting N TCP connections open and carry bulk bytes across all connections.
test/unit/btest-command.test.ts	Updates mocked summaries for activeConnections and adjusts warning tests to use realized fan-out.
test/integration/btest-client.test.ts	Extends CHR integration coverage to assert activeConnections == 4 for TCP fan-out.
src/protocols/btest.ts	Updates encodeSecondaryJoin to include the 0x02 join marker (and related comment text).
src/protocols/btest-session.ts	Implements client-side secondary connection opening and secondary bulk loops; adds activeConnections to summary.
src/core/error-catalog.ts	Refines the catalog summary for the connection-count warning to match new semantics.
src/btest.ts	Plumbs activeConnections into output and changes warning emission to trigger only when realized < requested.
docs/MATRIX.md	Updates btest matrix narrative to reflect fan-out implementation status.
docs/errors/routeros/btest-connection-count-single-stream.md	Updates the warning page to describe “requested vs realized” fan-out and authenticated fallback behavior.
commands/btest/README.md	Updates CLI docs and grounding notes to reflect client fan-out behavior and limitations.
commands/btest/examples.md	Clarifies server-side fan-out is still ungated and adds a new gated client multi-connection example.

[mobileskyfi]

Added: #88 UDP client receive/both CHR gate (commit c0492c2)

The plan flagged #88 (UDP client→server has no CI coverage) for socket-connect L2-bridge research. Outcome: no quickchr change needed. A CHR probe showed the server→client UDP return already lands over the guest→host SLIRP gateway (10.0.2.2:clientUdpPort — the same path the server cell's UDP-transmit uses), with only the existing TCP control forward. It works because PR #86 left the client UDP socket unconnected (a connect() filter previously dropped every datagram); the gap was just that it was never CI-tested.

btest-client.test.ts now gates UDP receive (rx≈474KB) and both (tx>0, rx≈582KB with loss accounting) against real CHR 7.23.1. The one remaining unproven UDP edge is the server cell's host→guest direction (would need a UDP hostfwd) — documented, not part of #88.

This PR now Closes #87 and #88.

[mobileskyfi]

Addressed the review nits (commit 040e6b3):

1. Module header implied a "server OK" on secondary join — reworded: each secondary reads the HELLO, sends the join, then bulk data flows; the server sends no pre-data ack (and the code deliberately doesn't read one).
2. encodeSecondaryJoin comment implied a required 01 …00 reply — same fix: no acknowledgement before data.
3. MATRIX said "CHR-gated for throughput-rises" — corrected to "CHR-gated for the realized fan-out (activeConnections == count); no throughput-rise assertion — the SLIRP loopback is bandwidth-bound," matching what the test actually asserts.

[mobileskyfi]

@coderabbitai help

[coderabbitai]

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai rate limit to show your current review rate limit status.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
• @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
• @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai generate unit tests to generate unit tests for this PR.
• @coderabbitai emit path instructions to emit generated path-specific review instructions for this repository.
• @coderabbitai resolve merge conflict to automatically resolve merge conflicts.
• @coderabbitai autofix to automatically fix issues identified in unresolved review comments.
• @coderabbitai resolve to resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, support, documentation and community

• Visit our status page to check the current availability of CodeRabbit.
• Create a ticket on our support page for assistance with any issues or questions.
• Visit our documentation site for detailed information on how to use CodeRabbit.
• Join our Discord community to connect with other users and get help from the community.
• Follow us on X/Twitter for updates and announcements.

[mobileskyfi]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@commands/btest/README.md`:
- Around line 267-273: The UDP caveat in the README is stale and still implies
client receive/both is unproven through SLIRP, even though that path is already
covered. Update the bullet in the btest UDP section to narrow the remaining open
question to only the server-cell host→guest direction, and keep the wording
aligned with the existing client cell and server cell terminology used in the
surrounding text.

In `@src/protocols/btest-session.ts`:
- Around line 1396-1411: The secondary TCP join path in btest-session is reusing
options.host instead of the primary peer address, which can send follow-up
connections to a different backend. Update the openSecondaryConnection call
inside the TCP secondaryChannels loop to use the established channel’s remote
address fallback (channel.remoteAddress ?? options.host), matching the UDP join
logic and keeping all secondary joins pinned to the primary peer.
- Around line 1433-1445: The cleanup in btest-session’s session shutdown path is
only happening after Promise.all resolves, so any rejection from driveSession or
a secondary loop skips udp.close(), channel.close(), and the secondary channel
closes. Move the socket cleanup in btest-session’s main session flow into a
finally block (or equivalent error-safe cleanup around the Promise.all in the
driveSession/secondaryLoops section) so udp, channel, and every secondaryChannel
are always closed on both success and failure.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 53269341-983f-4831-8d74-e7bc6c1165a9

📥 Commits

Reviewing files that changed from the base of the PR and between 063a64b and 8c21cdc.

📒 Files selected for processing (12)

• commands/btest/README.md
• commands/btest/examples.md
• docs/MATRIX.md
• docs/errors/routeros/btest-connection-count-single-stream.md
• src/btest.ts
• src/core/error-catalog.ts
• src/protocols/btest-session.ts
• src/protocols/btest.ts
• test/integration/btest-client.test.ts
• test/unit/btest-command.test.ts
• test/unit/btest-session.test.ts
• test/unit/btest.test.ts

[mobileskyfi]

Rebased onto the updated #98 and addressed CodeRabbit's review (commit ee6f4b9):

1. Secondary joins re-resolved options.host (Major) — now dial channel.remoteAddress ?? options.host, the IP the primary control socket resolved to, so a load-balancing hostname can't land a secondary on a different RouterOS where the token is unknown.
2. Promise.all cleanup not on the error path (Major) — wrapped the primary+secondary drive in try/finally so a rejected loop still closes the UDP socket and every TCP channel.
3. Stale UDP caveat — narrowed the README Open-questions bullet to the one open edge (server-cell host→guest); the client cell is gated.

Re-validated on CHR 7.23.1: fan-out opens 4 connections, UDP receive/both land throughput, tcp-both RX sustained. (This PR also now carries #98's random-data demux guard via the rebase.)

@coderabbitai review

[coderabbitai]

@mobileskyfi Thanks for the detailed update — I’ll review the rebased changes with the ee6f4b9 fixes in mind, especially the secondary join target resolution, the error-path cleanup, and the narrowed UDP caveat.

---

✏️ Learnings added

Learnt from: mobileskyfi
URL: https://github.com/tikoci/centrs/pull/99

Timestamp: 2026-06-26T01:58:28.156Z
Learning: In `src/protocols/btest-session.ts`, secondary TCP btest joins should dial `channel.remoteAddress ?? options.host` so all joined connections land on the same RouterOS instance as the primary control connection; this avoids load-balanced hostnames sending a secondary join to a different server where the session token is unknown.

---

Learnt from: mobileskyfi
URL: https://github.com/tikoci/centrs/pull/99

Timestamp: 2026-06-26T01:58:28.156Z
Learning: For the btest CHR integration coverage in this repository, UDP client `receive` and `both` modes are now gated; the remaining unproven UDP edge is the server cell's host→guest direction, which would require UDP host forwarding and remains documented as an open caveat.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mobileskyfi]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.