feat: パケットフィールド拡充・バグ修正・テスト強化 - #86
Merged
Merged
Conversation
BREAKING CHANGE: errorDataプロパティを廃止し、dataType/layerId/code/messageTypeに分割 closes #77
closes #78
- parsePacketのnullチェックを!= nullに統一 (#73と同根のバグ) - changeset本文にBREAKING CHANGE注記を追加 - auth.test.tsのテスト名を実態に合わせて修正
- TCNetTimecode.modeをsmpteModeにリネーム - TCNetTimePacketTimecode型を削除しTCNetTimecodeクラスで置換 - TimePacket.read()でTCNetTimecode.read()を活用 - StatusPacket.appSpecificの防御的ガードに意図コメント追加
マスター種類によるフィールド取得可否の差異を表で記載 - masterAudioLevel: 常に0 (マスター種類に依らない) - チャンネルaudioLevel: SKマスターのみ取得可能 - EQ/Filter/Trim等: Bridgeマスターのみ取得可能 - フェーダー位置: 両方で取得可能
BREAKING CHANGE: MixerChannel.crossfaderAssignをcrossFaderAssignに変更 - MixerData側のcrossFader/crossFaderCurveと命名規則を統一 - Implementation-Status.mdのSmallWaveForm備考を他行と統一 - Bridge WikiにTCNetマスター種類の説明を追加
- readMultiPacketHeaderに42バイト未満ガードを追加 (短バッファでRangeError防止) - receiveUnicastに24バイト未満の早期リターンを追加 - マルチパケットアセンブラ分岐に42バイト未満ガードを追加 - テスト: 短バッファ/未定義dataType/requestDataバリデーションを網羅 closes #85
🦋 Changeset detectedLatest commit: ae48cd9 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
Walkthrough複数のTCNetパケット解析と受信フローを拡張・堅牢化します。Mixer ID/フィールド拡張、Timecode/APP SPECIFIC/マルチパケットヘッダー公開、Errorパケットの構造化、短バッファ・未定義 dataType のクラッシュ防止、認証判定ロジックの更新を行います。 Changes
Sequence Diagram(s)sequenceDiagram
participant Socket as UDP Socket
participant Receiver as receiveUnicast
participant Parser as Packet Parser
participant Assembler as MultiPacketAssembler
participant Auth as Auth Handler
Socket->>Receiver: datagram(msg)
Receiver->>Receiver: validate length (>=24, multi-header >=42)
Receiver->>Parser: parse management header -> base Packet
alt packet.type == Data
Parser->>Parser: lookup dataPacketClass (nw.TCNetDataPackets[dataType])
alt dataPacketClass != null
Parser->>Parser: new dataPacketClass()
Parser->>Parser: assign buffer/header
Parser->>Assembler: append chunk / try assemble
Assembler->>Parser: assembled finalPacket
Parser->>Parser: if "multiPacketHeader" in finalPacket -> readMultiPacketHeader(msg)
Parser->>Parser: packet.read()
else
Parser->>Receiver: ignore / do not construct
end
end
alt packet is Error
Parser->>Auth: provide structured fields (dataType, layerId, code, messageType)
Auth->>Auth: evaluate auth success/fail -> update state
end
Parser->>Receiver: emit parsed packet (if applicable)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plans used: ✨ 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 significantly expands the TCNet protocol implementation by structuring packet fields for Error, Status, and Time packets, and adding comprehensive fields to the Mixer data structure. It also exposes multi-packet headers for various data types and includes documentation updates and bug fixes for Mixer ID conversion and crash prevention on undefined data types. The review feedback identifies a potential regression in authentication logic where stricter code checks might fail against certain hardware responses, an inconsistency between test data and implementation expectations, and a redundant buffer read in the artwork packet parser.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d6be46d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- readAssembledパスでmultiPacketHeaderを設定 (CodeRabbit P2) - Artwork.read()のreadUInt32LE(38)をmultiPacketHeader経由に変更 - readMultiPacketHeaderをexportしてtcnet.tsから利用可能に - ErrorPacketパーサーテストのデータを非認証値(0x1234)に変更
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af849b9a3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tcnet.ts (1)
714-727:⚠️ Potential issue | 🟠 Major短バッファ防御が Data/File の実読込まで届いていません。
msg.length < 24ではTCNetDataPacket.read()が参照する byte 24/25 を守れないので、24B/25B の Data/File でまだRangeErrorになります。さらに既知dataTypeでもTCNetDataPacketMetrics/TCNetDataPacketMetadata/TCNetDataPacketCUEはdataPacket.read()内で固定オフセットを読むため、26B 以上でも短いパケットで同様に落ちます。parsePacket()前に Data/File の 26B ガードを入れ、具象パケット生成後はlength()を使って最小長を満たす場合だけread()してください。💡 修正案
- if (msg.length < 24) return; + if (msg.length < 24) return; const mgmtHeader = new nw.TCNetManagementHeader(msg); mgmtHeader.read(); + if ( + (mgmtHeader.messageType === nw.TCNetMessageType.Data || + mgmtHeader.messageType === nw.TCNetMessageType.File) && + msg.length < 26 + ) { + return; + } const packet = this.parsePacket(mgmtHeader); @@ const dataPacket: nw.TCNetDataPacket = new dataPacketClass(); dataPacket.buffer = msg; dataPacket.header = mgmtHeader; dataPacket.dataType = packet.dataType; dataPacket.layer = packet.layer; + const expectedLength = dataPacket.length(); + if (expectedLength !== -1 && msg.length < expectedLength) { + return; + } dataPacket.read();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tcnet.ts` around lines 714 - 727, 現在の msg.length < 24 ガードだと Data/File 系の実読込で byte 24/25 や固定オフセット参照により RangeError が発生するので、parsePacket() を呼ぶ前に packet.dataType が Data/File 系の場合は msg.length >= 26 をチェックして早期リターンするロジックを追加し、既知の dataType から具象クラスを生成した後は必ず dataPacket.length() を用いて最小バッファ長を満たしている場合のみ dataPacket.read() を呼ぶように変更してください（参照シンボル: parsePacket, nw.TCNetDataPacket, nw.TCNetDataPackets, TCNetDataPacket.read, TCNetManagementHeader）。
🤖 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 749-751: finalPacket の multiPacketHeader が、assembler 経路では設定されるが
totalPackets===0 の File 経路（handleFileChunkPacket が finalPacket を組み立てるケース）では設定されず
null が返る不整合があるため、handleFileChunkPacket 関数内で finalPacket に multiPacketHeader
を引き継ぐ処理を追加してください；具体的には handleFileChunkPacket 内で受信データから
nw.readMultiPacketHeader(...) を使ってヘッダーを取得するか、呼び元から渡された既存ヘッダー値を
finalPacket.multiPacketHeader に代入して、assembler 完了分岐と同じように
finalPacket.multiPacketHeader が常に設定されるようにします（参照シンボル: finalPacket,
multiPacketHeader, handleFileChunkPacket, nw.readMultiPacketHeader,
totalPackets）。
---
Outside diff comments:
In `@src/tcnet.ts`:
- Around line 714-727: 現在の msg.length < 24 ガードだと Data/File 系の実読込で byte 24/25
や固定オフセット参照により RangeError が発生するので、parsePacket() を呼ぶ前に packet.dataType が Data/File
系の場合は msg.length >= 26 をチェックして早期リターンするロジックを追加し、既知の dataType から具象クラスを生成した後は必ず
dataPacket.length() を用いて最小バッファ長を満たしている場合のみ dataPacket.read()
を呼ぶように変更してください（参照シンボル: parsePacket, nw.TCNetDataPacket, nw.TCNetDataPackets,
TCNetDataPacket.read, TCNetManagementHeader）。
🪄 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: ac5d449e-d81f-4824-8c2e-c55af8459139
📒 Files selected for processing (3)
src/network.tssrc/tcnet.tstests/auth.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
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.
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.
📚 Learning: 2026-04-09T23:09:19.524Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 65
File: src/tcnet.ts:1158-1167
Timestamp: 2026-04-09T23:09:19.524Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the race condition where a stale `sendAuthSequence` from an old auth generation could call `resetAuthSession` and destroy a newly-started pending auth state is resolved by: (1) adding `expectedToken: number` parameter to `sendAuthSequence` and checking `this.sessionToken !== expectedToken` at every async boundary (entry, after cmd=0 send, after 50ms wait, after prepareAuthPayload) — silently returning without calling `resetAuthSession` when stale; (2) capturing `tokenForThisAttempt` in `handleInitialAuthRequest` and only calling `resetAuthSession` in the catch block when `this.sessionToken === tokenForThisAttempt`; (3) keeping the `tokenBeforePing` guard inside `prepareAuthPayload` as a secondary defense. The maintainer (9c5s) intentionally uses this two-layer guard design.
Applied to files:
src/tcnet.tstests/auth.test.tssrc/network.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/auth.test.tssrc/network.ts
📚 Learning: 2026-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In node-tcnet, the changeset bump type is kept as `patch` even for breaking changes (removal of `autoReauth`/`reauthInterval`, `reauthenticated`/`reauthFailed` events, `AuthState` `"refreshing"`, addition of `TCNetLogger.warn`) because the library is pre-release (0.x semver), and the maintainer (9c5s) considers strict semver minor/major bumps unnecessary at this stage.
Applied to files:
src/tcnet.tstests/auth.test.tssrc/network.ts
📚 Learning: 2026-04-09T17:09:02.148Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:961-966
Timestamp: 2026-04-09T17:09:02.148Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the TOCTOU race condition in `performBridgeOsDetection` (where an in-flight ping could overwrite `bridgeIsWindows` with a stale result after a Bridge switch) is resolved by capturing `bridgeIp = this.server?.address` at detection start and checking `this.server?.address === bridgeIp` immediately before writing to `this.bridgeIsWindows`. The maintainer (9c5s) deliberately rejected clearing `bridgeOsDetectionPromise` in `resetAuthSession` (doesn't stop internal execution) and a generation-counter approach (triggers on same-Bridge resets too) in favor of this simpler address-comparison guard.
Applied to files:
src/tcnet.tstests/auth.test.tssrc/network.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.tstests/auth.test.tssrc/network.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.tstests/auth.test.tssrc/network.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.tstests/auth.test.tssrc/network.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.tstests/auth.test.tssrc/network.ts
📚 Learning: 2026-04-08T12:28:37.297Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 63
File: src/tcnet.ts:960-1010
Timestamp: 2026-04-08T12:28:37.297Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the `cleanup?.()` call in the catch block of `executeReauth` is intentionally kept as defensive programming. The maintainer (9c5s) prefers this pattern because `removeListener`/`clearTimeout` are idempotent, so the redundant call costs nothing and provides safety against future changes to the control flow.
Applied to files:
src/tcnet.tstests/auth.test.ts
📚 Learning: 2026-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), `TCNetLogger.warn` is intentionally a required method (not optional). The maintainer (9c5s) made it required so that TypeScript catches missing logger implementations at compile time; making it optional would silently drop warn-level messages (e.g., unexpected token changes during reauth). The library is pre-release, so this breaking change is acceptable and is documented in the PR and changeset.
Applied to files:
src/tcnet.tstests/auth.test.tssrc/network.ts
📚 Learning: 2026-04-12T15:27:49.300Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Applied to files:
src/tcnet.tstests/auth.test.tssrc/network.ts
📚 Learning: 2026-04-12T15:27:25.846Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
Applied to files:
src/tcnet.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d835956c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 717-720: The current short-buffer guard (if (msg.length < 24)
return) still allows 25-byte messages to reach TCNetDataPacket.read() and throw
a RangeError; after constructing and calling read() on TCNetManagementHeader,
check mgmtHeader.messageType for the Data case and reject packets with
msg.length < 26 (e.g., return early) before calling this.parsePacket or
instantiating TCNetDataPacket so Data packets have the correct minimum length;
refer to TCNetManagementHeader, messageType (Data), TCNetDataPacket.read(), and
parsePacket when making the change.
🪄 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: 25b90c64-a78e-49fd-918d-d48bf87d4cca
📒 Files selected for processing (1)
src/tcnet.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
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.
📚 Learning: 2026-04-09T23:09:19.524Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 65
File: src/tcnet.ts:1158-1167
Timestamp: 2026-04-09T23:09:19.524Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the race condition where a stale `sendAuthSequence` from an old auth generation could call `resetAuthSession` and destroy a newly-started pending auth state is resolved by: (1) adding `expectedToken: number` parameter to `sendAuthSequence` and checking `this.sessionToken !== expectedToken` at every async boundary (entry, after cmd=0 send, after 50ms wait, after prepareAuthPayload) — silently returning without calling `resetAuthSession` when stale; (2) capturing `tokenForThisAttempt` in `handleInitialAuthRequest` and only calling `resetAuthSession` in the catch block when `this.sessionToken === tokenForThisAttempt`; (3) keeping the `tokenBeforePing` guard inside `prepareAuthPayload` as a secondary defense. The maintainer (9c5s) intentionally uses this two-layer guard design.
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-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In node-tcnet, the changeset bump type is kept as `patch` even for breaking changes (removal of `autoReauth`/`reauthInterval`, `reauthenticated`/`reauthFailed` events, `AuthState` `"refreshing"`, addition of `TCNetLogger.warn`) because the library is pre-release (0.x semver), and the maintainer (9c5s) considers strict semver minor/major bumps unnecessary at this stage.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-09T17:09:02.148Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:961-966
Timestamp: 2026-04-09T17:09:02.148Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the TOCTOU race condition in `performBridgeOsDetection` (where an in-flight ping could overwrite `bridgeIsWindows` with a stale result after a Bridge switch) is resolved by capturing `bridgeIp = this.server?.address` at detection start and checking `this.server?.address === bridgeIp` immediately before writing to `this.bridgeIsWindows`. The maintainer (9c5s) deliberately rejected clearing `bridgeOsDetectionPromise` in `resetAuthSession` (doesn't stop internal execution) and a generation-counter approach (triggers on same-Bridge resets too) in favor of this simpler address-comparison guard.
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-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-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), `TCNetLogger.warn` is intentionally a required method (not optional). The maintainer (9c5s) made it required so that TypeScript catches missing logger implementations at compile time; making it optional would silently drop warn-level messages (e.g., unexpected token changes during reauth). The library is pre-release, so this breaking change is acceptable and is documented in the PR and changeset.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-08T12:28:37.297Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 63
File: src/tcnet.ts:960-1010
Timestamp: 2026-04-08T12:28:37.297Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the `cleanup?.()` call in the catch block of `executeReauth` is intentionally kept as defensive programming. The maintainer (9c5s) prefers this pattern because `removeListener`/`clearTimeout` are idempotent, so the redundant call costs nothing and provides safety against future changes to the control flow.
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
📚 Learning: 2026-04-12T15:27:49.300Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-12T15:27:25.846Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
Applied to files:
src/tcnet.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tcnet.ts (1)
1431-1449:⚠️ Potential issue | 🟠 Major認証成否の判定に
messageTypeが抜けています。この PR で
TCNetErrorPacketを構造化したのに、ここではdataType/layerId/codeだけで pending session を成功・失敗へ遷移させています。別用途の Error パケットが同じ値を持つと誤認証になるので、auth 応答で期待するpacket.messageTypeも条件に含めるべきです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tcnet.ts` around lines 1431 - 1449, The auth success/failure checks currently only test packet.dataType/layerId/code and can misinterpret unrelated error packets; update the conditional branches that set this._authState and emit("authenticated") / emit("authFailed") to also require packet.messageType to equal the auth-response message type used for TCNetErrorPacket (i.e. add && packet.messageType === <auth-response-messageType-constant-or-enum> to both the success and failure conditions), and keep the existing cleanup (clearing this.authTimeoutId, setting this.sessionToken/null, and logging) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/receive-unicast.test.ts`:
- Around line 56-74: The test doesn't actually assert that requestData() settles
after receiving the short packet because it only calls requestPromise.catch(()
=> {}), leaving timers running; modify the test to either use fake timers
(advance timers until the requestPromise settles) or pass a short requestTimeout
to the TestTCNetClient/requestData invocation, then await the returned promise
and assert its final state (resolve or reject) and clear timers; target symbols:
requestData(), requestPromise, simulateUnicast, requestTimeout (or the test
harness' fake timer utilities) to ensure the promise is awaited and timers are
cleaned up.
---
Outside diff comments:
In `@src/tcnet.ts`:
- Around line 1431-1449: The auth success/failure checks currently only test
packet.dataType/layerId/code and can misinterpret unrelated error packets;
update the conditional branches that set this._authState and
emit("authenticated") / emit("authFailed") to also require packet.messageType to
equal the auth-response message type used for TCNetErrorPacket (i.e. add &&
packet.messageType === <auth-response-messageType-constant-or-enum> to both the
success and failure conditions), and keep the existing cleanup (clearing
this.authTimeoutId, setting this.sessionToken/null, and logging) intact.
🪄 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: 6d655541-1a8b-4d82-9937-664bac0fc9c0
📒 Files selected for processing (2)
src/tcnet.tstests/receive-unicast.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
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.
📚 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:
tests/receive-unicast.test.tssrc/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:
tests/receive-unicast.test.tssrc/tcnet.ts
📚 Learning: 2026-04-09T23:09:19.524Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 65
File: src/tcnet.ts:1158-1167
Timestamp: 2026-04-09T23:09:19.524Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the race condition where a stale `sendAuthSequence` from an old auth generation could call `resetAuthSession` and destroy a newly-started pending auth state is resolved by: (1) adding `expectedToken: number` parameter to `sendAuthSequence` and checking `this.sessionToken !== expectedToken` at every async boundary (entry, after cmd=0 send, after 50ms wait, after prepareAuthPayload) — silently returning without calling `resetAuthSession` when stale; (2) capturing `tokenForThisAttempt` in `handleInitialAuthRequest` and only calling `resetAuthSession` in the catch block when `this.sessionToken === tokenForThisAttempt`; (3) keeping the `tokenBeforePing` guard inside `prepareAuthPayload` as a secondary defense. The maintainer (9c5s) intentionally uses this two-layer guard design.
Applied to files:
tests/receive-unicast.test.tssrc/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:
tests/receive-unicast.test.tssrc/tcnet.ts
📚 Learning: 2026-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In node-tcnet, the changeset bump type is kept as `patch` even for breaking changes (removal of `autoReauth`/`reauthInterval`, `reauthenticated`/`reauthFailed` events, `AuthState` `"refreshing"`, addition of `TCNetLogger.warn`) because the library is pre-release (0.x semver), and the maintainer (9c5s) considers strict semver minor/major bumps unnecessary at this stage.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-09T17:09:02.148Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:961-966
Timestamp: 2026-04-09T17:09:02.148Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the TOCTOU race condition in `performBridgeOsDetection` (where an in-flight ping could overwrite `bridgeIsWindows` with a stale result after a Bridge switch) is resolved by capturing `bridgeIp = this.server?.address` at detection start and checking `this.server?.address === bridgeIp` immediately before writing to `this.bridgeIsWindows`. The maintainer (9c5s) deliberately rejected clearing `bridgeOsDetectionPromise` in `resetAuthSession` (doesn't stop internal execution) and a generation-counter approach (triggers on same-Bridge resets too) in favor of this simpler address-comparison guard.
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
📚 Learning: 2026-04-09T16:28:34.323Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 64
File: src/tcnet.ts:31-35
Timestamp: 2026-04-09T16:28:34.323Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), `TCNetLogger.warn` is intentionally a required method (not optional). The maintainer (9c5s) made it required so that TypeScript catches missing logger implementations at compile time; making it optional would silently drop warn-level messages (e.g., unexpected token changes during reauth). The library is pre-release, so this breaking change is acceptable and is documented in the PR and changeset.
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-04-08T12:28:37.297Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 63
File: src/tcnet.ts:960-1010
Timestamp: 2026-04-08T12:28:37.297Z
Learning: In `src/tcnet.ts` (node-tcnet, TypeScript), the `cleanup?.()` call in the catch block of `executeReauth` is intentionally kept as defensive programming. The maintainer (9c5s) prefers this pattern because `removeListener`/`clearTimeout` are idempotent, so the redundant call costs nothing and provides safety against future changes to the control flow.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-12T15:27:49.300Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:70-70
Timestamp: 2026-04-12T15:27:49.300Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the "未再検証" (unre-verified) annotation in the Time section ("以下のフィールドが0で返される (未再検証)。") is intentional: no Time packets were captured during the PR#83 verification session (0 packets), so re-verification was not possible. The maintainer (9c5s) plans to update this notation once verification becomes possible. Do not flag this wording as needing immediate resolution in future reviews.
Applied to files:
src/tcnet.ts
📚 Learning: 2026-04-12T15:27:25.846Z
Learnt from: 9c5s
Repo: 9c5s/node-tcnet PR: 83
File: docs/wiki/PRO-DJ-LINK-Bridge.md:30-32
Timestamp: 2026-04-12T15:27:25.846Z
Learning: In `docs/wiki/PRO-DJ-LINK-Bridge.md` (node-tcnet, documentation in Japanese), the asymmetry in certainty wording for the Metrics section is intentional: "別マシンのBridgeでは全フィールドが正常に取得可能であることを実機で確認済み" uses definitive language because it is a confirmed fact from actual hardware testing in the PR; "同一マシン上のBridgeではこれらのフィールドが0で返されることがある" uses hedged language ("ことがある") because it is based on a past observation whose reproduction conditions are unknown. Do not flag this wording inconsistency in future reviews.
Applied to files:
src/tcnet.ts
🔇 Additional comments (3)
src/tcnet.ts (3)
537-538: lookup miss をundefinedまで含めて安全に弾けています。
TCNetPackets/TCNetDataPacketsの参照結果がundefinedのケースでもnewに進まなくなるので、unknown type のクラッシュ経路をきちんと塞げています。Also applies to: 724-725
692-694:multiPacketHeaderの引き継ぎが経路間で揃っています。file chunk 経路と assembler 完了経路の両方で
finalPacket.multiPacketHeaderを設定しているので、assembled 応答の戻り値 shape が一貫しました。Also applies to: 752-754
717-720: 短バッファを header parse 前に落とすのは良いです。
TCNetDataPacket.read()に到達する前に 26 バイト未満を除外できていて、防御として十分に直接的です。
This was referenced Apr 12, 2026
This was referenced Apr 12, 2026
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.
概要
TCNetパケットの7つのissueを一括対応し、レビュー指摘・実機検証結果を反映した。
バグ修正
!= null)parsePacketにも同一パターンのクラッシュバグが残存していたため併せて修正機能追加
破壊的変更
TCNetErrorPacket.errorData: BufferをdataType/layerId/code/messageTypeに構造化TCNetTimecode.modeをsmpteModeにリネームMixerChannel.crossfaderAssignをcrossFaderAssignにリネームテスト・品質改善
readMultiPacketHeaderに42バイト未満のバッファ長ガードを追加 (短パケットでのクラッシュ防止)receiveUnicastに24バイト未満の早期リターンを追加ドキュメント
実機検証結果
Bridge (BRIDGE22) + ShowKontrol (TCS-SK8) + CDJ-3000 環境で全issueを検証済み。
テスト
300テスト全通過、typecheck/lint/build全通過。
closes #72, closes #73, closes #75, closes #76, closes #77, closes #78, closes #80, closes #84, closes #85