test: harden BridgeInfo assertions

[bbopen]

Follow-up to #188.

• Add explicit assertions for BridgeInfo fields (codecFallback, arrowAvailable, pid, instances).
• Make the timeout recovery test less timing-sensitive by allowing enough time for worker quarantine/replacement after a timeout.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds validations for runtime bridge info fields (codecFallback, arrowAvailable, pid, instances) in tests and adjusts the timeout-handling test to properly trigger timeout behavior with a longer initial timeout and extended sleep duration.

Changes

Cohort / File(s)	Summary
Runtime Bridge Info Validations test/runtime_node.test.ts	Added assertions validating bridge info fields: codecFallback is 'json' or 'none', arrowAvailable is boolean, pid is positive integer, instances is non-negative integer. Adjusted timeout test to initialize NodeBridge with 1000 ms timeout and extend sleep duration to 1.5 seconds to properly trigger timeout behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor: unify NodeBridge and OptimizedNodeBridge (ADR-0001) #136: Tests the NodeBridge bridge-info validation and timeout/quarantine behavior introduced in the refactor
• feat(runtime): add getBridgeInfo() meta call #188: Overlaps with getBridgeInfo() payload validation and its test coverage
• Unify Node bridge core and harden guardrails #127: Related to BridgeCore's validateBridgeInfo and bridge-info/runtime protocol guardrails

Suggested labels

enhancement, area:runtime-node, area:codec, priority:p2

Poem

🐰 Bridge info checks, all in a row,
codecFallback, pid must flow,
Timeouts now trigger with grace,
Validation in its proper place! ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'test: harden BridgeInfo assertions' clearly and specifically describes the main change: adding stricter validations to BridgeInfo field assertions in the test file.
Description check	✅ Passed	The description is directly related to the changeset, explaining both the BridgeInfo assertion additions and the timeout test adjustments mentioned in the raw summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/followup-bridgeinfo-test-assertions

---

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dce1292 and 75a6b5e.

📒 Files selected for processing (1)

• test/runtime_node.test.ts

🧰 Additional context used

🧠 Learnings (11)

📓 Common learnings

Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:238-249
Timestamp: 2026-01-20T01:33:12.841Z
Learning: In the tywrap repository, when reviewing init/startup patterns, prioritize cases that break core functionality over edge cases with diagnostic-only impact (e.g., missing bridgeInfo metadata). If workers are healthy and operational, defer low-impact defensive checks to follow-up PRs.

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/adversarial_playground.test.ts:339-360
Timestamp: 2026-01-19T21:00:30.825Z
Learning: In adversarial tests for the bridge (test/adversarial_playground.test.ts), recovery assertions should be kept in the finally block adjacent to cleanup to verify resilience under stress, even if that means post-timeout checks might mask the original error. The goal is to surface recovery failures as legitimate test failures.

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:40.872Z
Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-298
Timestamp: 2026-01-19T21:48:45.693Z
Learning: In `src/runtime/bridge-core.ts`, keep `normalizeErrorPayload` to validate error payloads from the Python subprocess. The subprocess boundary is effectively untrusted, and normalizing error responses prevents `undefined: undefined` errors on malformed payloads. Error responses are not the hot path, so the small validation overhead is acceptable for the added resilience.

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/fixtures/out_of_order_bridge.py:29-48
Timestamp: 2026-01-19T21:00:52.689Z
Learning: In `test/fixtures/out_of_order_bridge.py`, the fixture intentionally leaves a pending request unanswered at EOF to simulate missing/out-of-order responses and validate bridge behavior when requests never complete; this is the exact failure mode being tested and must be preserved.

📚 Learning: 2026-01-20T16:01:14.323Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-19T21:48:45.693Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-298
Timestamp: 2026-01-19T21:48:45.693Z
Learning: In `src/runtime/bridge-core.ts`, keep `normalizeErrorPayload` to validate error payloads from the Python subprocess. The subprocess boundary is effectively untrusted, and normalizing error responses prevents `undefined: undefined` errors on malformed payloads. Error responses are not the hot path, so the small validation overhead is acceptable for the added resilience.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-19T21:14:40.872Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:40.872Z
Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-19T21:00:30.825Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/adversarial_playground.test.ts:339-360
Timestamp: 2026-01-19T21:00:30.825Z
Learning: In adversarial tests for the bridge (test/adversarial_playground.test.ts), recovery assertions should be kept in the finally block adjacent to cleanup to verify resilience under stress, even if that means post-timeout checks might mask the original error. The goal is to surface recovery failures as legitimate test failures.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-19T21:14:37.032Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:37.032Z
Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-20T16:01:14.323Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-19T21:48:27.823Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/runtime_bridge_fixtures.test.ts:59-66
Timestamp: 2026-01-19T21:48:27.823Z
Learning: In fixture-based tests (e.g., test/runtime_bridge_fixtures.test.ts) and similar tests in the tywrap repository, prefer early returns when Python or fixture files are unavailable. Do not rely on Vitest dynamic skip APIs; a silent pass is intentional for environments lacking Python/fixtures. Treat missing fixtures as optional soft-guards and ensure the test remains non-disruptive in non-availability scenarios.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-20T01:34:07.064Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:444-458
Timestamp: 2026-01-20T01:34:07.064Z
Learning: When reviewing promise-based polling patterns (e.g., recursive setTimeout in waitForAvailableWorker functions), ensure that any recursive timer is tracked and cleared on timeout or promise resolution to prevent timer leaks. Do not rely on no-op checks after settlement; verify clearTimeout is invoked in all paths and consider using an explicit cancellation (e.g., AbortController) or a guard to cancel pending timers when the promise settles.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-20T18:37:05.670Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: src/runtime/process-io.ts:37-40
Timestamp: 2026-01-20T18:37:05.670Z
Learning: In the tywrap repo, ESLint is used for linting (not Biome). Do not flag regex literals that contain control characters (e.g., \u001b, \u0000-\u001F) as lint errors, since the current ESLint configuration allows them. When reviewing TypeScript files (e.g., src/**/*.ts), rely on ESLint results and avoid raising issues about these specific control-character regex literals unless there is a competing config change or a policy requiring explicit escaping.

Applied to files:

• test/runtime_node.test.ts

📚 Learning: 2026-01-20T16:00:49.829Z

Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:203-211
Timestamp: 2026-01-20T16:00:49.829Z
Learning: In the BridgeProtocol implementation (tywrap), reject Map and Set explicitly before serialization (e.g., in safeStringify or the serialization entrypoint) with a clear error like "Bridge protocol does not support Map/Set values". Do not rely on post-hoc checks of non-string keys at the point of JSON.stringify, since Maps/Sets cannot round-trip. This should be enforced at the boundary where data is prepared for serialization to ensure deterministic errors and prevent invalid data from propagating.

Applied to files:

• test/runtime_node.test.ts

🧬 Code graph analysis (1)

test/runtime_node.test.ts (3)

src/utils/logger.ts (1)

• info (166-168)

src/runtime/node.ts (1)

• NodeBridge (238-481)

src/runtime/optimized-node.ts (1)

• NodeBridge (21-21)

🔇 Additional comments (2)

test/runtime_node.test.ts (2)

146-154: LGTM — solid field-level assertions for BridgeInfo.

These assertions properly validate runtime metadata: codecFallback against known enum values, arrowAvailable as boolean, pid as a positive integer, and instances as a non-negative integer. Good coverage of the bridge info contract.

---

295-298: Good timing adjustment to reduce flakiness.

The 1000ms timeout with a 1.5s sleep gives a comfortable 500ms margin to reliably trigger the timeout, while the existing 800ms post-timeout wait is sufficient for the late response to arrive and the worker quarantine/replacement to complete. This should make the test less timing-sensitive as intended.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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 and usage tips.}