Merged
Conversation
Windows BridgeはTCNASDP認証のXTEA暗号文(auth[4:12])をバイトリバースして 読み取る。pingのTTL値からBridge OSを自動検出し、Windows判定時に暗号文を 事前リバースして送信することで、Mac/Win両Bridgeに対応する。 - detectBridgeIsWindows: ローカルはos.platform()、リモートはping TTLで判定 - sendAuthSequence: Windows Bridge時にciphertext.reverse()を適用 - execFileベースの非同期実装でイベントループブロッキングを回避 - IPアドレスバリデーション追加でコマンドインジェクション防止 - server未設定時のキャッシュ回避で再検出を保証
🦋 Changeset detectedLatest commit: 0b7f96c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary by CodeRabbit
WalkthroughTCNetClientがブリッジOSを検出し、認証送信時にXTEA暗号文をOS判定で条件付きバイト反転するようになりました。判定結果は Changes
Sequence Diagram(s)sequenceDiagram
participant Client as TCNetClient
participant OS as Local OS / execFile
participant Bridge as Remote Bridge (server)
Client->>Client: detectBridgeIsWindows()
alt server IP equals local adapter IP
Client->>OS: os.platform()
OS-->>Client: platform (win32 / other)
else remote server
Client->>OS: execFile("ping", args..., {timeout:3000})
OS-->>Client: stdout (may contain TTL) / error
Client->>Client: parse TTL -> ttl > 64 ?
end
Client-->>Client: set bridgeIsWindows cache (true/false/null)
Client->>Client: sendAuthSequence()
alt bridgeIsWindows == true
Client->>Client: reverse XTEA ciphertext bytes
else
Client->>Client: use ciphertext as-is
end
Client->>Bridge: send auth payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to detect if the target Bridge is running on Windows by analyzing the TTL value from a ping command. This detection is necessary because the Windows Bridge implementation requires XTEA ciphertext to be byte-reversed before transmission. The changes include the detectBridgeIsWindows method in TCNetClient, logic to reverse the ciphertext in the authentication sequence, and extensive new tests to verify the detection logic and its impact on the payload. I have no feedback to provide as there are no review comments.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tcnet.ts`:
- Around line 939-947: After awaiting this.detectBridgeIsWindows(), re-check
that the auth state didn't change (same guards used after the 50ms wait
elsewhere): wait the short 50ms (or reuse the existing wait function), then
verify this.sessionToken is still valid/non-null, this.broadcastSocket and
this.server are still present and not changed; if any are invalid, abort sending
(return early) to avoid calling
generateAuthPayload/createAppDataPacket/sendPacket with stale/null sessionToken
or socket. Implement this check immediately after the await
this.detectBridgeIsWindows() call so the subsequent calls to
generateAuthPayload, createAppDataPacket and sendPacket are only executed when
state is confirmed stable.
- Around line 849-856: The code currently assigns this.bridgeIsWindows = false
when TTL is not found or ping throws, which caches an uncertain non-Windows
result; instead, remove the assignments to the instance field in those ambiguous
branches and only return/use a local false for this probe (e.g., use a local
variable like isWindowsProbe = false or simply return false from the detection
method), leaving this.bridgeIsWindows unchanged until you have a definitive
positive detection; keep the debug logs as-is but ensure only the
definitive-true branch sets this.bridgeIsWindows = true (or a definitive-false
branch sets it when you have conclusive evidence).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 2d7a9adb-ea38-4ef7-b6fe-faec6b336cf9
📒 Files selected for processing (3)
src/tcnet.tstests/auth.test.tstests/bridge-os-detection.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-21T05:30:31.035Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:249-255
Timestamp: 2026-03-21T05:30:31.035Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacket.read()` (class defined at line 321, read() at line 330) converts the wire-format 1-based layer to 0-based by doing `this.layer = this.buffer.readUInt8(25) - 1`. In contrast, `TCNetRequestPacket.read()` (class defined at line 225, read() at line 231) does NOT subtract 1: `this.layer = this.buffer.readUInt8(25)`. Therefore, `dataPacket.layer` from a received `TCNetDataPacket` is already 0-based and correctly matches the 0-based key stored in `requestData()`. Flagging a missing `-1` in `TCNetDataPacket.read()` is a false positive.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-24T19:15:40.455Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 41
File: src/utils.ts:94-94
Timestamp: 2026-03-24T19:15:40.455Z
Learning: In the `node-tcnet` repository (`src/utils.ts`), the project targets ES2022 + NodeNext and does not need to support Node.js 18.0.0. The temporary numeric `family` field (4/6) in `os.networkInterfaces()` introduced in Node.js 18.0.0 and fixed in 18.1.0 does not need to be handled. Suggesting numeric `family` handling for defensive programming is unnecessary for this project.
Applied to files:
src/tcnet.ts
🔇 Additional comments (2)
tests/auth.test.ts (1)
463-474: この回帰テスト追加は良いです。
bridgeIsWindowsキャッシュのセット/クリアとauth[4:12]の送信順を直接固定できていて、今回のバイトリバース修正の退行を防げています。Also applies to: 719-805
tests/bridge-os-detection.test.ts (1)
1-229: 検出ロジックのテスト構成は堅いです。
os.platform()とexecFile()を切り離した上で、ローカル/リモート、TTL境界、失敗フォールバック、キャッシュ、OS別引数まで押さえられており、この分岐追加をかなり安全にできています。
- ping失敗/TTL未検出時にbridgeIsWindowsをキャッシュしない (次回再検出を許可) - detectBridgeIsWindows() await後に状態再確認ガードを追加 - patch changeset追加
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tcnet.ts`:
- Around line 822-826: The code sets this.bridgeIsWindows = false when bridgeIp
fails the /^\d{1,3}(\.\d{1,3}){3}$/ check, which caches a determination for
invalid inputs; instead, treat an invalid IP as "undetermined" and do not cache
the result—remove the assignment to bridgeIsWindows in the IP-format check
inside the function that validates/pings the bridge (the block that references
bridgeIp and this.bridgeIsWindows) so the function returns false for this call
but leaves this.bridgeIsWindows unchanged.
In `@tests/bridge-os-detection.test.ts`:
- Around line 192-198: Add an assertion that the detection result was cached as
false: after calling BridgeOsTestClient.callDetectBridgeIsWindows(), verify the
client's cached state (e.g., BridgeOsTestClient.bridgeIsWindows or a getter like
getCachedBridgeIsWindows()) is false; if no public accessor exists, add a small
test-only accessor on BridgeOsTestClient to read the cached value and assert it
equals false so the test verifies both return value and caching behavior.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 547b0574-c6cb-41e5-83aa-d551273c3d38
📒 Files selected for processing (3)
.changeset/blue-emus-doubt.mdsrc/tcnet.tstests/bridge-os-detection.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-03-24T19:15:40.455Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 41
File: src/utils.ts:94-94
Timestamp: 2026-03-24T19:15:40.455Z
Learning: In the `node-tcnet` repository (`src/utils.ts`), the project targets ES2022 + NodeNext and does not need to support Node.js 18.0.0. The temporary numeric `family` field (4/6) in `os.networkInterfaces()` introduced in Node.js 18.0.0 and fixed in 18.1.0 does not need to be handled. Suggesting numeric `family` handling for defensive programming is unnecessary for this project.
Applied to files:
.changeset/blue-emus-doubt.mdsrc/tcnet.tstests/bridge-os-detection.test.ts
📚 Learning: 2026-03-21T05:30:31.035Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:249-255
Timestamp: 2026-03-21T05:30:31.035Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacket.read()` (class defined at line 321, read() at line 330) converts the wire-format 1-based layer to 0-based by doing `this.layer = this.buffer.readUInt8(25) - 1`. In contrast, `TCNetRequestPacket.read()` (class defined at line 225, read() at line 231) does NOT subtract 1: `this.layer = this.buffer.readUInt8(25)`. Therefore, `dataPacket.layer` from a received `TCNetDataPacket` is already 0-based and correctly matches the 0-based key stored in `requestData()`. Flagging a missing `-1` in `TCNetDataPacket.read()` is a false positive.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-27T08:24:18.571Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 50
File: src/network.ts:1010-1024
Timestamp: 2026-03-27T08:24:18.571Z
Learning: In the `node-tcnet` repository (`src/network.ts`), `Buffer.allocUnsafe()` is never used — only `Buffer.alloc()` is used throughout the codebase. Reserved fields in packet `write()` methods do not need explicit zero-initialization because `Buffer.alloc()` always zero-initializes. Suggesting defensive zero-writes for reserved packet offsets is unnecessary (YAGNI) for this codebase.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T05:02:27.762Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:397-400
Timestamp: 2026-03-21T05:02:27.762Z
Learning: In the `node-tcnet` repository (`src/network.ts`, `src/tcnet.ts`), `TCNetDataPacket.write()` has no callers. `TCNetDataPacket` objects are only ever deserialized on receive paths (`read()`), never serialized and sent. The only outbound packet carrying a layer field is `TCNetRequestPacket`, constructed inside `requestData()` in `src/tcnet.ts`. Suggesting to move layer conversion logic into `TCNetDataPacket.write()` is inappropriate because that method is dead code.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T11:31:23.742Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 8
File: README.MD:31-33
Timestamp: 2026-03-21T11:31:23.742Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacketMetadata` (line 389) has an `info` property (not `data`). `data` property belongs to the sibling class `TCNetDataPacketMetrics` (line 352). Flagging `meta.info` in README/docs examples as wrong and suggesting `meta.data` is a false positive.
Applied to files:
src/tcnet.ts
🔇 Additional comments (10)
src/tcnet.ts (5)
3-4: LGTM!必要なNode.js標準モジュールのインポートが追加されています。
execFileはexecよりも安全で、シェルインジェクションのリスクがありません。
96-96: LGTM!
null初期値により「未検出」と「非Windows検出済み」を区別できる適切な設計です。
844-853: LGTM!TTL判定ロジックは同一LAN環境において正確です。
ttl > 64の閾値は、Windows (TTL=128)とmacOS/Linux (TTL=64)のデフォルト値の中間点として適切です。ping失敗時およびTTL未検出時にキャッシュしない修正が適用されており、一時的なネットワーク問題で誤判定が固定されるリスクが解消されています。
894-903: LGTM!
resetAuthSession()でbridgeIsWindowsをクリアすることで、再接続時にOS再検出が行われます。これはBridgeの変更やネットワーク構成の変更に対応するための適切な設計です。
941-956: LGTM!Windows Bridge検出時のXTEA暗号文バイトリバース処理が正しく実装されています。
Buffer.from(ct, "hex")で毎回新しいバッファを生成しているため、reverse()のin-place変更は安全ですdetectBridgeIsWindows()のawait後に状態ガードが追加されており、非同期処理中の状態変更に対応していますtests/bridge-os-detection.test.ts (4)
1-22: LGTM!モックのセットアップが適切に構成されています。
vi.mockでモジュール全体をモックしつつ元の実装を保持し、テスト対象のplatformとexecFileのみをスタブ化しています。isolateXteaEnv()で環境変数を分離しているのも良いプラクティスです。
26-91: LGTM!テストヘルパークラスと関数が適切に設計されています。
BridgeOsTestClientはプライベートメソッドへのアクセスを提供し、mockExecFileSuccess/mockExecFileErrorはexecFileのコールバックパターンを正しくシミュレートしています。
145-163: LGTM!ping失敗時およびTTL未検出時のテストケースが、キャッシュされないこと（
getBridgeIsWindows() === null）を明示的に検証しています。これにより、一時的なネットワーク問題後の再検出が正しく動作することが保証されます。
200-230: LGTM!プラットフォーム別のping引数テストが網羅的です。Windows（
-n 1 -w 1000）とmacOS/Linux（-c 1 -W 1）の両方の引数形式を検証し、タイムアウトオプションも確認しています。.changeset/blue-emus-doubt.md (1)
1-5: LGTM!Changesetエントリが適切に作成されています。バグ修正に対するpatchバージョンの指定と、変更内容を正確に説明する日本語の説明文が含まれています。
9c5s
commented
Apr 3, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/tcnet.ts (1)
935-956:⚠️ Potential issue | 🟠 Major
await後は非nullではなく同一セッションを確認してください。この
awaitの間に再接続やアダプタ切替が完了すると、旧 Bridge 向けに作ったclientIp/ OS 判定結果を、新しいsessionTokenとbroadcastSocketで送れてしまいます。今のガードは「値があるか」しか見ていないので、await 前の状態を保持して一致確認し、不一致ならこの古い task だけ中断する必要があります。🛠️ 修正案
- const clientIp = this.getClientIp(); - if (!clientIp) { + const clientIp = this.getClientIp(); + const sessionToken = this.sessionToken; + const broadcastSocket = this.broadcastSocket; + const serverAddress = this.server.address; + const serverPort = this.server.port; + const broadcastAddress = this.config.broadcastAddress; + if (!clientIp || sessionToken === null) { this.resetAuthSession(); return; } const ciphertext = Buffer.from(ct, "hex"); // Windows BridgeはXTEA暗号文をバイトリバースして読み取るため、事前にリバースして送信する // 初回呼び出し時のみpingが発生し最大3秒かかる (AUTH_RESPONSE_TIMEOUT=5秒内に収まる想定) if (await this.detectBridgeIsWindows()) { ciphertext.reverse(); } - // detectBridgeIsWindowsのawait中に状態が変わった場合のガード - if (!this.server || !this.broadcastSocket || this.sessionToken === null) { - this.resetAuthSession(); + if ( + !this.server || + this.broadcastSocket !== broadcastSocket || + this.sessionToken !== sessionToken || + this.server.address !== serverAddress || + this.server.port !== serverPort || + this.config.broadcastAddress !== broadcastAddress || + this.getClientIp() !== clientIp + ) { return; } - const payload = generateAuthPayload(this.sessionToken, clientIp, ciphertext); - const auth = this.createAppDataPacket(2, this.sessionToken, payload); - await this.sendPacket(auth, this.broadcastSocket, TCNET_BROADCAST_PORT, this.config.broadcastAddress); + const payload = generateAuthPayload(sessionToken, clientIp, ciphertext); + const auth = this.createAppDataPacket(2, sessionToken, payload); + await this.sendPacket(auth, broadcastSocket, TCNET_BROADCAST_PORT, broadcastAddress);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tcnet.ts` around lines 935 - 956, Before awaiting detectBridgeIsWindows(), capture the current session identity (e.g., store this.sessionToken and this.broadcastSocket into local variables like preSessionToken and preBroadcastSocket) and after the await verify those stored values still match this.sessionToken and this.broadcastSocket; if they differ, abort this task (call resetAuthSession() or return) so you don't send the clientIp/OS result using a new session. Apply this check around the await in the flow that uses getClientIp(), detectBridgeIsWindows(), generateAuthPayload(), createAppDataPacket(), and sendPacket() with TCNET_BROADCAST_PORT and this.config.broadcastAddress to ensure the same session is used end-to-end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tcnet.ts`:
- Line 96: The field bridgeIsWindows is being cached globally and can return
stale results when this.server (and thus the remote IP) changes; update the
implementation that reads/returns bridgeIsWindows to key the cache by the
current bridge endpoint (use this.server.address()/this.server.address().address
or host:port string) or reset the cached value whenever this.server changes so
the platform decision is based on the current server.address; apply the same
per-endpoint cache or reset logic to the other affected locations (the other
checks around the bridge/platform detection referenced in the review) and ensure
all uses consult the current this.server.address before returning a cached
boolean.
---
Duplicate comments:
In `@src/tcnet.ts`:
- Around line 935-956: Before awaiting detectBridgeIsWindows(), capture the
current session identity (e.g., store this.sessionToken and this.broadcastSocket
into local variables like preSessionToken and preBroadcastSocket) and after the
await verify those stored values still match this.sessionToken and
this.broadcastSocket; if they differ, abort this task (call resetAuthSession()
or return) so you don't send the clientIp/OS result using a new session. Apply
this check around the await in the flow that uses getClientIp(),
detectBridgeIsWindows(), generateAuthPayload(), createAppDataPacket(), and
sendPacket() with TCNET_BROADCAST_PORT and this.config.broadcastAddress to
ensure the same session is used end-to-end.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dd1bd057-4c11-45ee-8d80-b8fa23110ba0
📒 Files selected for processing (2)
src/tcnet.tstests/bridge-os-detection.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-03-24T19:15:40.455Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 41
File: src/utils.ts:94-94
Timestamp: 2026-03-24T19:15:40.455Z
Learning: In the `node-tcnet` repository (`src/utils.ts`), the project targets ES2022 + NodeNext and does not need to support Node.js 18.0.0. The temporary numeric `family` field (4/6) in `os.networkInterfaces()` introduced in Node.js 18.0.0 and fixed in 18.1.0 does not need to be handled. Suggesting numeric `family` handling for defensive programming is unnecessary for this project.
Applied to files:
src/tcnet.tstests/bridge-os-detection.test.ts
📚 Learning: 2026-03-21T05:30:31.035Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:249-255
Timestamp: 2026-03-21T05:30:31.035Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacket.read()` (class defined at line 321, read() at line 330) converts the wire-format 1-based layer to 0-based by doing `this.layer = this.buffer.readUInt8(25) - 1`. In contrast, `TCNetRequestPacket.read()` (class defined at line 225, read() at line 231) does NOT subtract 1: `this.layer = this.buffer.readUInt8(25)`. Therefore, `dataPacket.layer` from a received `TCNetDataPacket` is already 0-based and correctly matches the 0-based key stored in `requestData()`. Flagging a missing `-1` in `TCNetDataPacket.read()` is a false positive.
Applied to files:
src/tcnet.tstests/bridge-os-detection.test.ts
📚 Learning: 2026-03-27T08:24:18.571Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 50
File: src/network.ts:1010-1024
Timestamp: 2026-03-27T08:24:18.571Z
Learning: In the `node-tcnet` repository (`src/network.ts`), `Buffer.allocUnsafe()` is never used — only `Buffer.alloc()` is used throughout the codebase. Reserved fields in packet `write()` methods do not need explicit zero-initialization because `Buffer.alloc()` always zero-initializes. Suggesting defensive zero-writes for reserved packet offsets is unnecessary (YAGNI) for this codebase.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T11:31:23.742Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 8
File: README.MD:31-33
Timestamp: 2026-03-21T11:31:23.742Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacketMetadata` (line 389) has an `info` property (not `data`). `data` property belongs to the sibling class `TCNetDataPacketMetrics` (line 352). Flagging `meta.info` in README/docs examples as wrong and suggesting `meta.data` is a false positive.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T05:02:27.762Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:397-400
Timestamp: 2026-03-21T05:02:27.762Z
Learning: In the `node-tcnet` repository (`src/network.ts`, `src/tcnet.ts`), `TCNetDataPacket.write()` has no callers. `TCNetDataPacket` objects are only ever deserialized on receive paths (`read()`), never serialized and sent. The only outbound packet carrying a layer field is `TCNetRequestPacket`, constructed inside `requestData()` in `src/tcnet.ts`. Suggesting to move layer conversion logic into `TCNetDataPacket.write()` is inappropriate because that method is dead code.
Applied to files:
src/tcnet.ts
🔇 Additional comments (1)
tests/bridge-os-detection.test.ts (1)
93-232: テスト観点は十分です。ローカル/リモート、TTL境界、失敗時の非キャッシュ、プラットフォーム別引数まで押さえていて、この変更の回帰をかなり防げています。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tcnet.ts`:
- Around line 825-829: The current IPv4 format check using
/^\d{1,3}(\.\d{1,3}){3}$/ on bridgeIp only enforces dot-decimal shape and allows
invalid octets like 999; update the validation in the pre-execFile guard around
bridgeIp to enforce each octet is 0–255—either replace the regex with one that
encodes 0-255 ranges or split bridgeIp by '.' and parseInt each octet and return
false if any octet <0 or >255; keep the behavior that invalid IPs are not cached
and ensure the same variable name bridgeIp is used in the new check.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 83630b40-a432-4ec6-9847-783b6cf6a525
📒 Files selected for processing (1)
src/tcnet.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-03-24T19:15:40.455Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 41
File: src/utils.ts:94-94
Timestamp: 2026-03-24T19:15:40.455Z
Learning: In the `node-tcnet` repository (`src/utils.ts`), the project targets ES2022 + NodeNext and does not need to support Node.js 18.0.0. The temporary numeric `family` field (4/6) in `os.networkInterfaces()` introduced in Node.js 18.0.0 and fixed in 18.1.0 does not need to be handled. Suggesting numeric `family` handling for defensive programming is unnecessary for this project.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T05:30:31.035Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:249-255
Timestamp: 2026-03-21T05:30:31.035Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacket.read()` (class defined at line 321, read() at line 330) converts the wire-format 1-based layer to 0-based by doing `this.layer = this.buffer.readUInt8(25) - 1`. In contrast, `TCNetRequestPacket.read()` (class defined at line 225, read() at line 231) does NOT subtract 1: `this.layer = this.buffer.readUInt8(25)`. Therefore, `dataPacket.layer` from a received `TCNetDataPacket` is already 0-based and correctly matches the 0-based key stored in `requestData()`. Flagging a missing `-1` in `TCNetDataPacket.read()` is a false positive.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-27T08:24:18.571Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 50
File: src/network.ts:1010-1024
Timestamp: 2026-03-27T08:24:18.571Z
Learning: In the `node-tcnet` repository (`src/network.ts`), `Buffer.allocUnsafe()` is never used — only `Buffer.alloc()` is used throughout the codebase. Reserved fields in packet `write()` methods do not need explicit zero-initialization because `Buffer.alloc()` always zero-initializes. Suggesting defensive zero-writes for reserved packet offsets is unnecessary (YAGNI) for this codebase.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T05:02:27.762Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 2
File: src/tcnet.ts:397-400
Timestamp: 2026-03-21T05:02:27.762Z
Learning: In the `node-tcnet` repository (`src/network.ts`, `src/tcnet.ts`), `TCNetDataPacket.write()` has no callers. `TCNetDataPacket` objects are only ever deserialized on receive paths (`read()`), never serialized and sent. The only outbound packet carrying a layer field is `TCNetRequestPacket`, constructed inside `requestData()` in `src/tcnet.ts`. Suggesting to move layer conversion logic into `TCNetDataPacket.write()` is inappropriate because that method is dead code.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-03-21T11:31:23.742Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 8
File: README.MD:31-33
Timestamp: 2026-03-21T11:31:23.742Z
Learning: In `src/network.ts` of the `node-tcnet` repository: `TCNetDataPacketMetadata` (line 389) has an `info` property (not `data`). `data` property belongs to the sibling class `TCNetDataPacketMetrics` (line 352). Flagging `meta.info` in README/docs examples as wrong and suggesting `meta.data` is a false positive.
Applied to files:
src/tcnet.ts
🔇 Additional comments (6)
src/tcnet.ts (6)
3-4: OS判定に必要な依存追加は妥当です。
execFile/platformの追加で、ローカル判定とリモート判定の両方を実装する前提が揃っています。
96-96: Bridge OS判定キャッシュの導入は適切です。
boolean | nullで未判定状態を表現できており、セッション内再利用の意図が明確です。
476-478: Master IP変更時のキャッシュ無効化は正しいです。接続先アドレス変更時に
bridgeIsWindowsをクリアしており、誤ったOS判定の使い回しを防げています。
816-865:detectBridgeIsWindows()の判定・キャッシュ方針は良いです。「確定時のみキャッシュ、判定不能時は非キャッシュ」の挙動が保たれており、再試行可能性と誤固定回避のバランスが取れています。
901-901: 認証セッションリセット時のOS判定キャッシュクリアは適切です。再接続・再認証時に再検出されるため、セッション境界の整合性が保てています。
945-956: 暗号文リバース適用とawait後ガードの組み合わせは適切です。Windows向け変換を適用しつつ、
await中の状態変化をsessionToken/socket/server で再検証できており、誤送信リスクを抑えられています。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
背景
Win Bridge環境でCUE/BeatGrid/BigWaveFormが配信されない問題を調査した結果、2つの根本原因を特定した:
本PRは原因1 (バイトリバース) を修正する。原因2はユーザーが
.envで適切な暗号文を設定することで対処する。変更内容
src/tcnet.tsdetectBridgeIsWindows(): Bridge OSの自動検出メソッド (async)os.platform()で判定execFile("ping", ...)でTTL取得、TTL > 64 → WindowssendAuthSequence(): Windows判定時にciphertext.reverse()を適用resetAuthSession(): キャッシュクリア (再接続時に再検出)tests/bridge-os-detection.test.ts(新規, 13テスト)tests/auth.test.ts(+3テスト)Test plan
npx vitest run全204ユニットテストパス