feat(runtime): complete ADR-002 bridge migration to BridgeProtocol #153
Conversation
Migrate all three bridges (NodeBridge, HttpBridge, PyodideBridge) to extend the new BridgeProtocol base class, completing the ADR-002 implementation. Key changes: - NodeBridge: Uses PooledTransport for process pool management, simplified from 975+ lines by delegating transport concerns to ProcessIO - HttpBridge: Streamlined to use HttpIO transport with SafeCodec validation - PyodideBridge: Uses PyodideIO transport with unified dispatch handler New components: - PooledTransport: Semaphore-based worker pool for concurrent process management - Enhanced ProcessIO: EPIPE error handling, graceful shutdown, backpressure Test updates: - Updated all Pyodide tests to use new setMockDispatchHandler pattern - Fixed config tests to use resolvedOptions and transport properties - Skipped legacy stats tests (processSpawns, processDeaths) that don't apply to new architecture - Fixed protocol error message patterns Breaking changes (pre-1.0): - getBridgeInfo() method removed (protocol info no longer exposed) - Internal processPool not accessible (managed by PooledTransport) - Stats structure simplified (legacy counters removed) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR implements a protocol-based BridgeProtocol (PROTOCOL_ID, numeric ids, method/params envelope), adds PooledTransport, and migrates NodeBridge, PyodideBridge, and HttpBridge to use BridgeProtocol and pooled/transported execution across the runtime and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Protocol as BridgeProtocol
participant Transport as PooledTransport
participant Pool as WorkerPool
participant Worker as ProcessIO
participant Runtime as Python
Client->>Protocol: call(module, fn, args)
Protocol->>Protocol: generateId() → number
Protocol->>Protocol: build message {id, protocol, method, params}
Protocol->>Transport: send(message)
Transport->>Pool: acquireWorker()
alt existing worker
Pool-->>Transport: worker
else create worker
Pool->>Worker: create/init
Worker->>Runtime: spawn process
Runtime-->>Worker: ready
Pool-->>Transport: worker
end
Transport->>Worker: send(serialized message)
Worker->>Runtime: write stdin (message)
Runtime->>Worker: stdout response
Worker->>Worker: extractMessageId(response)
Worker-->>Transport: response
Transport->>Pool: releaseWorker()
Transport-->>Protocol: response
Protocol->>SafeCodec: decodeResponse(response)
SafeCodec-->>Protocol: inner result
Protocol-->>Client: resolved value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
bot
added
area:runtime-node
- Remove getBridgeInfo check from living-app example (Arrow will fail at decode time if pyarrow is not installed) - Remove BridgeInfo type test (method was removed in bridge migration) - Fix HttpBridge instantiation in type test to provide required baseURL Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![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
tywrap/src/runtime/pyodide-io.ts
Lines 86 to 90 in e00e532
Update Pyodide bootstrap for new protocol
The embedded __tywrap_dispatch still parses msg['type'] and expects top‑level fields like module/functionName, but BridgeProtocol now sends method/params with a protocol header. In a browser/Pyodide runtime this means every call will throw (missing type) and return an error response, effectively breaking PyodideBridge for all operations. The bootstrap needs to accept the new schema (or translate the message before dispatch) to keep Pyodide execution functional.
ℹ️ 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".
src/runtime/node.ts
Outdated
| if ('module' in cmd && 'functionName' in cmd) { | ||
| // New format: { module, functionName, args? } | ||
| await this.call(cmd.module, cmd.functionName, cmd.args ?? []); |
There was a problem hiding this comment.
Warmup commands are executed via this.call(...), which routes through the pooled transport and only touches whichever worker is available (typically the first). With maxProcesses > 1, other Python processes never run the warmup commands, so any required imports or global state setup will be missing when requests are routed to those workers, leading to inconsistent behavior across calls. This is a regression from the previous per‑process warmup semantics and should be restored by iterating workers or pre-creating them and running warmup on each.
Useful? React with 👍 / 👎.
The NodeBridge `cwd` option was being resolved but not passed to Python environment. This caused modules in the working directory to not be importable. Now the `cwd` is prepended to PYTHONPATH so Python can find modules in the specified working directory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/runtime/safe-codec.ts (1)
389-395: Validate special floats on the decoded value, not the pre-decoded result.Line 389 currently checks
result, so NaN/Infinity introduced by Arrow decoding can slip through. Validatedecodedinstead. Based on learnings, post-decode validation is required to catch special floats introduced by Arrow decoders.🛠️ Proposed fix
- if (this.rejectSpecialFloats && containsSpecialFloat(result)) { - const floatPath = findSpecialFloatPath(result); + if (this.rejectSpecialFloats && containsSpecialFloat(decoded)) { + const floatPath = findSpecialFloatPath(decoded); throw new BridgeProtocolError( `Response contains non-finite number (NaN or Infinity) at ${floatPath}` ); }src/runtime/process-io.ts (1)
386-397: Fix environment inheritance logic: envOverrides should be merged with process.env, not replace it.When
inheritProcessEnvis false (the default),buildProcessEnv()deliberately returns a minimal filtered environment containing only PATH, PYTHON*, TYWRAP_*, and user overrides. ProcessIO then receives this minimal environment and incorrectly treats it as a complete base environment, replacingprocess.enventirely.This breaks subprocess execution because critical environment variables (HOME, USER, LD_LIBRARY_PATH, LOCALE, etc.) are missing. The ProcessIO code should merge
envOverrideswithprocess.envrather than using it as a complete replacement:const baseEnv = { ...process.env, ...this.envOverrides };This ensures essential system variables are always available while still allowing
envOverridesto override specific values.src/runtime/pyodide-io.ts (1)
68-133: Critical: Python dispatch code doesn't match the new protocol message shape.The bootstrap Python code expects the old flat message structure with
msg['type'],msg['module'],msg['functionName'], etc. However, the TypeScript side now sends the new protocol envelope withmethodand nestedparams:# Python expects: msg_type = msg['type'] # Line 88 mod = importlib.import_module(msg['module']) # Line 91 fn = getattr(mod, msg['functionName']) # Line 92 # TypeScript sends: { protocol: 'tywrap/1', method: 'call', params: { module: '...', functionName: '...' } }This mismatch will cause all Pyodide calls to fail with
KeyError.🐛 Proposed fix for Python dispatch code
def __tywrap_dispatch(message_json): msg = None try: msg = json.loads(message_json) msg_id = msg['id'] - msg_type = msg['type'] + msg_method = msg['method'] + params = msg.get('params', {}) - if msg_type == 'call': - mod = importlib.import_module(msg['module']) - fn = getattr(mod, msg['functionName']) - result = fn(*msg.get('args', []), **msg.get('kwargs', {})) + if msg_method == 'call': + mod = importlib.import_module(params['module']) + fn = getattr(mod, params['functionName']) + result = fn(*params.get('args', []), **params.get('kwargs', {})) return json.dumps({'id': msg_id, 'result': result}) - elif msg_type == 'instantiate': - mod = importlib.import_module(msg['module']) - cls = getattr(mod, msg['className']) - obj = cls(*msg.get('args', []), **msg.get('kwargs', {})) + elif msg_method == 'instantiate': + mod = importlib.import_module(params['module']) + cls = getattr(mod, params['className']) + obj = cls(*params.get('args', []), **params.get('kwargs', {})) handle = str(id(obj)) __tywrap_instances[handle] = obj return json.dumps({'id': msg_id, 'result': handle}) - elif msg_type == 'call_method': - obj = __tywrap_instances[msg['handle']] - method = getattr(obj, msg['methodName']) - result = method(*msg.get('args', []), **msg.get('kwargs', {})) + elif msg_method == 'call_method': + obj = __tywrap_instances[params['handle']] + method = getattr(obj, params['methodName']) + result = method(*params.get('args', []), **params.get('kwargs', {})) return json.dumps({'id': msg_id, 'result': result}) - elif msg_type == 'dispose_instance': - __tywrap_instances.pop(msg['handle'], None) + elif msg_method == 'dispose_instance': + __tywrap_instances.pop(params['handle'], None) return json.dumps({'id': msg_id, 'result': None}) else: return json.dumps({ 'id': msg_id, 'error': { 'type': 'ValueError', - 'message': f'Unknown message type: {msg_type}' + 'message': f'Unknown message method: {msg_method}' } })test/transport.test.ts (1)
199-202: Test override{ type: 'call' }has no effect.The
createValidMessage()helper creates messages withmethod: 'call'(nottype). The override{ type: 'call' }adds an extraneous field that doesn't affect the test. The test still passes because the base message already hasmethod: 'call', but the override is misleading.💚 Proposed fix
it('returns true for valid call message', () => { - const msg = createValidMessage({ type: 'call' }); + const msg = createValidMessage(); expect(isProtocolMessage(msg)).toBe(true); });
🤖 Fix all issues with AI agents
In `@src/runtime/http.ts`:
- Line 11: Remove the unused type import HttpIOOptions from the import statement
that currently reads "import { HttpIO, type HttpIOOptions } from
'./http-io.js';" so only HttpIO is imported; ensure no other references to
HttpIOOptions exist in this file and run the type checker to confirm the removal
is safe.
In `@src/runtime/node.ts`:
- Around line 73-77: The warmupCommands are currently executed once via
runWarmupCommands and not per worker; change the initialization so each worker
runs them when it is created: invoke the existing runWarmupCommands logic from
the per-worker initialization path (e.g., the worker spawn callback in
PooledTransport / WorkerPool or from ProcessIO construction) instead of only
calling it once from the pooled bridge; ensure the call accepts the same
warmupCommands shape (both {module,functionName,args} and legacy
{method,params}) and preserves error handling/logging so every newly spawned
ProcessIO executes warmupCommands on startup.
- Around line 33-42: The NodeBridgeOptions.minProcesses field is defined but
never propagated into the pool options; update the options plumbing so
minProcesses is mapped into the pool's minWorkers (or minWorkers name expected
by PooledTransportOptions) and used when constructing ResolvedOptions and
PooledTransport; specifically, in the options resolution code that builds
ResolvedOptions and where PooledTransport is instantiated (symbols:
NodeBridgeOptions.minProcesses, ResolvedOptions, PooledTransport,
PooledTransportOptions/minWorkers), pass minProcesses → minWorkers (or the
pool's minWorkers key), defaulting consistently with existing defaults, so the
pool can pre-warm to the requested minimum.
- Around line 20-24: Remove the unused type imports ProcessIOOptions and
PooledTransportOptions from the import list in src/runtime/node.ts (currently
imported alongside ProcessIO and PooledTransport) to satisfy the linter; edit
the import lines for ProcessIO and PooledTransport to only import the runtime
symbols (or alternatively update code to actually reference those types in type
annotations if they are intended to be used).
- Around line 246-250: The ProcessIO is being constructed without the resolved
working directory, so the spawned Python process uses process.cwd() instead of
the user-specified cwd; update the ProcessIO instantiation (the new
ProcessIO(...) call) to include the resolved cwd (e.g., pass cwd:
resolvedOptions.cwd or resolvedCwd) in the options object so ProcessIO uses that
directory when spawning the subprocess.
In `@src/runtime/pooled-transport.ts`:
- Line 14: Remove the unused named import WorkerPoolOptions from the import
specifier that currently imports { WorkerPool, type WorkerPoolOptions } in
pooled-transport.ts; keep only WorkerPool (e.g., change the import to import {
WorkerPool } from './worker-pool.js') so the unused type is no longer imported
and the linter/static analysis error is resolved.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
docs/adr/002-bridge-protocol.mdsrc/index.tssrc/runtime/bridge-protocol.tssrc/runtime/http.tssrc/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/process-io.tssrc/runtime/pyodide-io.tssrc/runtime/pyodide.tssrc/runtime/safe-codec.tssrc/runtime/transport.tstest/arch-stories.test.tstest/bridge-protocol.test.tstest/optimized-node.test.tstest/runtime_bridge_fixtures.test.tstest/runtime_config.test.tstest/runtime_node.test.tstest/runtime_pyodide.test.tstest/transport.test.ts
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:636-637
Timestamp: 2026-01-20T16:01:29.276Z
Learning: In the tywrap BridgeProtocol ProcessIO transport (ADR-002), when implementing `restartAfterRequests` functionality, use drain-before-restart as the default: wait for `pendingRequests.size === 0` before killing and restarting the process. This prevents rejecting in-flight requests during restart. Alternative graceful restart (spawn new process, drain old) adds complexity with two concurrent processes.
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: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
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.
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: 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.
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.
📚 Learning: 2026-01-20T16:01:39.136Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
Applied to files:
docs/adr/002-bridge-protocol.mdsrc/runtime/pooled-transport.tssrc/runtime/pyodide.tssrc/index.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tssrc/runtime/http.tssrc/runtime/pyodide-io.ts
📚 Learning: 2026-01-20T16:00:56.576Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:00:56.576Z
Learning: Before marking the NodeBridge as ready, validate the bridge by sending a test message to the Python subprocess and require a successful response (heartbeat/ACK) to confirm the bridge is responsive. Document this readiness handshake in the ADR and ensure tests cover the handshake scenario.
Applied to files:
docs/adr/002-bridge-protocol.md
📚 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:
docs/adr/002-bridge-protocol.mdtest/runtime_node.test.tstest/optimized-node.test.tssrc/runtime/node.ts
📚 Learning: 2026-01-20T16:00:40.505Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:168-172
Timestamp: 2026-01-20T16:00:40.505Z
Learning: In docs/adr/002-bridge-protocol.md, add a guideline that the SafeCodec path must reject NaN and Infinity before encoding and also validate after Arrow-deserialization. Use explicit checks (e.g., Number.isFinite or equivalent) on all floating-point inputs before encoding and on values produced by decoders after deserialization. Include tests that cover both pre-encode validation and post-decode validation to prevent invalid values from propagating to downstream consumers.
Applied to files:
docs/adr/002-bridge-protocol.md
📚 Learning: 2026-01-20T16:00:49.738Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:168-172
Timestamp: 2026-01-20T16:00:49.738Z
Learning: In the tywrap project's BridgeProtocol SafeCodec implementation, Arrow format decoders can produce NaN/Infinity values from binary representations even when the raw JSON payload doesn't contain them. This is why validation for special floats must occur both before encoding (to reject invalid inputs) and after applying decoders (to catch values introduced during Arrow deserialization), protecting downstream consumers from unexpected NaN/Infinity values.
Applied to files:
src/runtime/safe-codec.tstest/bridge-protocol.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:
src/runtime/safe-codec.tstest/runtime_node.test.tstest/runtime_bridge_fixtures.test.tssrc/runtime/process-io.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tssrc/runtime/pyodide.tssrc/index.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tstest/bridge-protocol.test.tssrc/runtime/http.ts
📚 Learning: 2026-01-19T21:14:35.390Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:35.390Z
Learning: In src/runtime/bridge-core.ts and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (such as runtime, shape, or error-payload checks) inside tight loops unless the protocol boundary is untrusted or there is a concrete bug report. The Python bridge protocol is controlled via tests, so these checks add unnecessary branching overhead without meaningful benefit. Apply this guidance to other hot-path runtime loops in src/runtime/**/*.ts, and re-enable additional validations only when a documented risk or failure scenario is identified. Ensure tests cover protocol validation where applicable.
Applied to files:
src/runtime/safe-codec.tssrc/runtime/process-io.tssrc/runtime/pooled-transport.tssrc/runtime/pyodide.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tssrc/runtime/http.tssrc/runtime/pyodide-io.ts
📚 Learning: 2026-01-19T21:14:29.869Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:29.869Z
Learning: In the runtime env-var parsing (e.g., in src/runtime/bridge-core.ts and similar modules), adopt a tolerant, best-effort policy: numeric env values should parse by taking the leading numeric portion (e.g., TYWRAP_CODEC_MAX_BYTES=1024abc -> 1024). Only reject clearly invalid values (non-numeric start or <= 0). This reduces surprising failures from minor typos. Add tests to cover partial numeric prefixes, and ensure downstream logic documents the trusted range; consider a small helper to extract a positive integer from a string with a safe fallback.
Applied to files:
src/runtime/safe-codec.tssrc/runtime/process-io.tssrc/runtime/pooled-transport.tssrc/runtime/pyodide.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tssrc/runtime/http.tssrc/runtime/pyodide-io.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:
src/runtime/safe-codec.tstest/runtime_node.test.tstest/runtime_bridge_fixtures.test.tssrc/runtime/process-io.tssrc/runtime/pooled-transport.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tstest/arch-stories.test.tssrc/runtime/pyodide.tssrc/index.tstest/transport.test.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tstest/bridge-protocol.test.tssrc/runtime/http.tssrc/runtime/pyodide-io.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:
src/runtime/safe-codec.tstest/runtime_node.test.tstest/runtime_bridge_fixtures.test.tssrc/runtime/process-io.tssrc/runtime/pooled-transport.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tstest/arch-stories.test.tssrc/runtime/pyodide.tssrc/index.tstest/transport.test.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tstest/bridge-protocol.test.tssrc/runtime/http.tssrc/runtime/pyodide-io.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:
src/runtime/safe-codec.tstest/runtime_node.test.tstest/runtime_bridge_fixtures.test.tssrc/runtime/process-io.tssrc/runtime/pooled-transport.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tstest/arch-stories.test.tssrc/runtime/pyodide.tssrc/index.tstest/transport.test.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tssrc/runtime/transport.tstest/bridge-protocol.test.tssrc/runtime/http.tssrc/runtime/pyodide-io.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.tstest/runtime_bridge_fixtures.test.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tssrc/index.tstest/transport.test.tstest/bridge-protocol.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.tstest/runtime_bridge_fixtures.test.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tstest/arch-stories.test.tstest/transport.test.tstest/bridge-protocol.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 `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.tstest/runtime_bridge_fixtures.test.tssrc/runtime/process-io.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tssrc/runtime/pyodide.tssrc/index.tssrc/runtime/node.tssrc/runtime/bridge-protocol.tstest/bridge-protocol.test.tssrc/runtime/http.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.tstest/runtime_bridge_fixtures.test.tstest/runtime_pyodide.test.tstest/optimized-node.test.tstest/runtime_config.test.tstest/bridge-protocol.test.ts
📚 Learning: 2026-01-19T21:00:52.689Z
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.
Applied to files:
test/runtime_node.test.tstest/runtime_bridge_fixtures.test.tstest/optimized-node.test.ts
📚 Learning: 2026-01-19T21:00:52.689Z
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: Policy for adversarial test fixtures: adversarial fixtures must preserve the failure mode they exercise, even if that behavior would be "bad" in real code. When reviewing test fixtures with unusual patterns (e.g., missing error handling, intentional protocol violations, unanswered requests), ask for clarifying comments about the intent rather than suggesting normalization or fixes.
Applied to files:
test/runtime_bridge_fixtures.test.ts
📚 Learning: 2026-01-20T16:01:29.276Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:636-637
Timestamp: 2026-01-20T16:01:29.276Z
Learning: In the tywrap BridgeProtocol ProcessIO transport (ADR-002), when implementing `restartAfterRequests` functionality, use drain-before-restart as the default: wait for `pendingRequests.size === 0` before killing and restarting the process. This prevents rejecting in-flight requests during restart. Alternative graceful restart (spawn new process, drain old) adds complexity with two concurrent processes.
Applied to files:
src/runtime/process-io.tstest/optimized-node.test.tstest/bridge-protocol.test.ts
📚 Learning: 2026-01-20T01:36:14.701Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:743-759
Timestamp: 2026-01-20T01:36:14.701Z
Learning: In the tywrap repository, when reviewing concurrent request handling patterns (especially with BridgeCore request multiplexing), flag uses of boolean `busy` flags that should be request counters instead. A boolean flag creates a race condition: when request A completes and sets `busy = false`, request B (still in flight via multiplexing) is incorrectly treated as idle. The fix is to use an `inFlightRequests: number` counter that increments on request start and decrements on completion, then check `worker.inFlightRequests === 0` before cleanup/termination.
Applied to files:
test/optimized-node.test.tstest/bridge-protocol.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/bridge-protocol.test.ts
🧬 Code graph analysis (11)
src/runtime/pooled-transport.ts (1)
src/runtime/transport.ts (1)
Transport(132-168)
test/runtime_pyodide.test.ts (1)
src/runtime/pyodide.ts (1)
PyodideBridge(66-88)
test/runtime_config.test.ts (1)
src/runtime/node.ts (1)
NodeBridge(208-464)
test/arch-stories.test.ts (2)
src/index.ts (1)
HttpBridge(68-68)src/runtime/http.ts (1)
HttpBridge(63-86)
src/runtime/pyodide.ts (3)
src/runtime/safe-codec.ts (1)
CodecOptions(22-31)src/runtime/bridge-protocol.ts (2)
BridgeProtocol(70-319)BridgeProtocolOptions(30-39)src/runtime/pyodide-io.ts (1)
PyodideIO(164-485)
test/transport.test.ts (1)
src/runtime/transport.ts (4)
PROTOCOL_ID(20-20)isProtocolMessage(229-244)ProtocolResponse(77-96)isProtocolResponse(252-273)
src/runtime/node.ts (10)
src/runtime/optimized-node.ts (2)
NodeBridgeOptions(21-21)NodeBridge(21-21)src/runtime/safe-codec.ts (1)
CodecOptions(22-31)src/runtime/bridge-core.ts (1)
getPathKey(364-372)src/runtime/bridge-protocol.ts (2)
BridgeProtocol(70-319)BridgeProtocolOptions(30-39)src/runtime/pooled-transport.ts (1)
PooledTransport(71-236)src/utils/python.ts (1)
getDefaultPythonPath(43-45)src/runtime/process-io.ts (1)
ProcessIO(134-783)src/utils/codec.ts (1)
autoRegisterArrowDecoder(165-194)src/utils/cache.ts (1)
globalCache(634-634)tywrap_ir/tywrap_ir/optimized_ir.py (1)
duration(44-46)
src/runtime/bridge-protocol.ts (1)
src/runtime/transport.ts (2)
ProtocolMessage(36-69)PROTOCOL_ID(20-20)
src/runtime/transport.ts (1)
src/index.ts (2)
PROTOCOL_ID(20-20)ProtocolMessage(19-19)
test/bridge-protocol.test.ts (3)
src/index.ts (3)
ProtocolResponse(19-19)BridgeProtocolError(58-58)SafeCodec(17-17)src/runtime/transport.ts (1)
ProtocolResponse(77-96)src/runtime/safe-codec.ts (1)
SafeCodec(213-413)
src/runtime/http.ts (2)
src/runtime/safe-codec.ts (1)
CodecOptions(22-31)src/runtime/bridge-protocol.ts (2)
BridgeProtocol(70-319)BridgeProtocolOptions(30-39)
🪛 GitHub Actions: CI
src/index.ts
[error] 127-127: TS2339: Property 'getBridgeInfo' does not exist on type 'NodeBridge'.
🪛 GitHub Check: lint
src/runtime/pooled-transport.ts
[warning] 14-14:
'WorkerPoolOptions' is defined but never used
src/runtime/node.ts
[warning] 23-23:
'PooledTransportOptions' is defined but never used
[warning] 22-22:
'ProcessIOOptions' is defined but never used
[warning] 245-245:
Missing return type on function
[warning] 509-509:
Generic Object Injection Sink
[warning] 508-508:
Generic Object Injection Sink
[warning] 499-499:
Generic Object Injection Sink
[warning] 491-491:
Generic Object Injection Sink
[warning] 482-482:
Generic Object Injection Sink
src/runtime/http.ts
[warning] 11-11:
'HttpIOOptions' is defined but never used
🔇 Additional comments (58)
docs/adr/002-bridge-protocol.md (1)
5-14: Status/progress update looks consistent with the migration completion.test/arch-stories.test.ts (1)
462-464: HttpBridge construction updated correctly for the new options contract.test/runtime_bridge_fixtures.test.ts (2)
49-65: Updated protocol error patterns align with the new error surface.
198-200: Skipping getBridgeInfo parity aligns with the API removal.src/runtime/safe-codec.ts (1)
145-163: Envelope result extraction is clear and keeps validation on the inner payload.Also applies to: 326-337
test/runtime_node.test.ts (2)
133-156: Skipping getBridgeInfo coverage matches the removal in the new architecture.
703-705: Protocol error expectations updated correctly for the new surface.Also applies to: 742-744
test/optimized-node.test.ts (2)
312-315: Skipping legacy pool/stat tests is consistent with the new PooledTransport internals.Also applies to: 413-415, 448-450, 483-485
408-410: Relaxed serialization error expectation is appropriate for BigInt failures.src/index.ts (3)
20-20: LGTM: PROTOCOL_ID export enables external protocol validation.Exporting the protocol identifier allows consumers to validate incoming messages match the expected protocol version.
27-28: LGTM: PooledTransport public export.Exposing
PooledTransportandPooledTransportOptionsenables advanced users to create custom pool configurations for worker management.
65-68: LGTM: Bridge exports with options types.The bridge classes are now properly exported with their corresponding options types, providing a complete public API surface for the new BridgeProtocol architecture.
test/runtime_config.test.ts (4)
42-49: LGTM: Test adapted for resolvedOptions access pattern.The test correctly accesses internal
resolvedOptionsfor verification. UsingtoContain('python_bridge.py')is more robust than exact path matching across different environments.
65-79: LGTM: Custom configuration test updated.The test properly verifies resolved options and correctly documents that
enableJsonFallbackis deprecated and not stored in options.
151-156: LGTM: Custom Pyodide configuration test.The test correctly accesses transport properties to verify custom configuration is properly passed through.
135-141: Default indexURL version is consistent across PyodideIO and tests.The default indexURL
'https://cdn.jsdelivr.net/pyodide/v0.24.1/full/'already matches the DEFAULT_INDEX_URL constant in PyodideIO options (src/runtime/pyodide-io.ts, line 58). No changes needed.src/runtime/process-io.ts (5)
99-103: LGTM: Numeric ID extraction aligns with protocol changes.The regex correctly extracts integer IDs from JSON, supporting both positive and negative values. This matches the new
ProtocolMessage.id: numbertype.
152-152: LGTM: Pending map key type updated to number.Consistent with the numeric ID change in
extractMessageId.
441-461: LGTM: Robust shutdown handling prevents EPIPE crashes.The changes add:
- Proactive error handlers before cleanup to suppress EPIPE during shutdown
- Graceful
stdin.end()wrapped in try-catch- Targeted listener removal for
exit/closewhile preserving other cleanupThis aligns with the PR objective of "graceful stdin.end() on shutdown" and prevents unhandled EPIPE errors. Based on learnings, drain-before-restart is the preferred pattern for restarts.
679-693: LGTM: Synchronous EPIPE handling in writeToStdin.The try-catch around
stdin.write()catches synchronous errors (like EPIPE when the process has died) and converts them toBridgeProtocolError. This prevents unhandled exceptions and provides clear error messages.
716-738: LGTM: flushWriteQueue handles synchronous errors and cascades rejection.On write error, the code properly:
- Rejects the current queued write
- Rejects all remaining queued writes with the same error
- Clears the queue to prevent stale entries
This ensures no writes are left dangling after a process failure.
src/runtime/transport.ts (5)
19-20: LGTM: Protocol identifier constant.Using
'tywrap/1'as a versioned protocol ID enables future protocol evolution while maintaining backwards compatibility detection.
36-69: LGTM: Unified ProtocolMessage structure.The new envelope structure with
methodand nestedparamsis cleaner than flat fields. Key improvements:
- Numeric
idsimplifies correlation and avoids string parsing overheadprotocolfield enables version validation- Union type for
methodprovides compile-time safety- Optional fields in
paramsallow method-specific payloadsThis aligns with ADR-002's protocol specification.
77-96: LGTM: ProtocolResponse with optional protocol echo.The response correctly:
- Uses numeric
idfor correlation- Makes
protocoloptional (graceful handling if Python doesn't echo it)- Maintains existing
result/errorunion semantics
237-243: LGTM: isProtocolMessage type guard updated.The guard correctly validates:
- Numeric
id- Exact
PROTOCOL_IDmatch- Valid
methodstring in the allowed set- Non-null
paramsobject
259-261: LGTM: isProtocolResponse validates numeric id.The check ensures response correlation works with the new numeric ID scheme.
test/runtime_pyodide.test.ts (7)
19-39: LGTM: Centralized dispatch mock pattern.The
setMockDispatchHandlerapproach provides:
- Per-test customization of responses
- Default fallback for tests that don't need custom behavior
- Clean separation between mock setup and test logic
This replaces individual function mocks with a unified JSON dispatch pattern matching the production architecture.
46-50: LGTM: Bootstrap detection via __tywrap_dispatch.The mock correctly detects bootstrap code by checking for
def __tywrap_dispatchand sets up the dispatch function. This mirrors the production Pyodide bootstrap behavior.
78-78: LGTM: Dispatch handler reset in beforeEach.Ensures clean state between tests.
207-222: LGTM: Message structure verification.The test correctly verifies the new protocol envelope:
method: 'call'params.module,params.functionName,params.argsThis ensures the bridge produces correctly structured messages.
254-264: LGTM: Error message updated for new architecture.The error message "Pyodide dispatch function not initialized" correctly reflects the new unified dispatch model instead of referencing individual helper functions.
361-363: LGTM: Bootstrap verification updated.The test verifies that bootstrap code includes
def __tywrap_dispatch, confirming the unified dispatch function is set up correctly.
513-528: LGTM: Error response handling test.The test verifies that dispatch returning an error object with
type,message, andtracebackfields correctly propagates as an exception. This matches theProtocolResponse.errorstructure.src/runtime/bridge-protocol.ts (8)
21-21: LGTM: PROTOCOL_ID import.Centralizing the protocol identifier in
transport.tsand importing it here ensures consistency across all protocol-aware code.
145-153: LGTM: Protocol injection in sendMessage.The implementation correctly:
- Accepts messages without
idorprotocol(viaOmit)- Generates a unique numeric ID
- Injects
protocol: PROTOCOL_IDinto the full messageThis ensures all outgoing messages conform to the protocol specification.
185-193: LGTM: Consistent protocol injection in sendMessageAsync.Same pattern as
sendMessage, maintaining consistency for Arrow-enabled responses.
217-219: LGTM: Numeric ID generation.The monotonically increasing counter is simple and efficient. Pre-increment (
++this.requestId) ensures IDs start at 1, avoiding potential edge cases with ID 0.
240-248: LGTM: call() uses new method/params envelope.The message structure correctly uses:
method: 'call'params: { module, functionName, args, kwargs }This matches the
ProtocolMessagetype definition.
266-274: LGTM: instantiate() message structure.Correctly uses
method: 'instantiate'withparams.className.
292-300: LGTM: callMethod() message structure.Correctly uses
method: 'call_method'withparams.handleandparams.methodName.
312-317: LGTM: disposeInstance() message structure.Correctly uses
method: 'dispose_instance'withparams.handle.src/runtime/http.ts (1)
63-85: LGTM!Clean implementation of
HttpBridgeextendingBridgeProtocol. The constructor properly creates theHttpIOtransport and passes both transport and codec options to the base class. The timeout configuration is consistently applied to both the transport and protocol layers.src/runtime/pyodide-io.ts (2)
315-329: LGTM on protocol message construction.The
call()method correctly constructs the new protocol envelope withprotocol: PROTOCOL_ID,method: 'call', and nestedparamsobject. The same pattern is consistently applied across all convenience methods.
466-468: LGTM on numeric ID generation.The
generateId()method correctly returns sequential numeric IDs, aligning with the protocol change from string to number IDs.test/transport.test.ts (3)
67-81: LGTM on updated test helper.The
createValidMessage()helper correctly produces the new protocol envelope shape with numericid,PROTOCOL_ID,method, and nestedparams.
256-299: LGTM on protocol validation tests.Comprehensive test coverage for the new protocol message validation: numeric ID requirement, protocol field presence and value, method field validation, and params object requirement.
1287-1310: LGTM on numeric ID format test.Test correctly verifies that IDs are sequential integers greater than zero.
src/runtime/pooled-transport.ts (2)
71-155: LGTM!Clean implementation of
PooledTransport. The class properly:
- Validates
createTransportis a function in the constructor- Initializes the
WorkerPoollazily indoInit()- Delegates
send()to the pool's worker transports- Exposes useful pool statistics for monitoring
The disposal check in
send()correctly prevents use after disposal.
189-235: LGTM on runtime execution stubs.The stub methods correctly throw
BridgeExecutionErrorwith a clear message directing users to useBridgeProtocolfor operations. This aligns with the ADR-002 architecture where transports only implementsend()andBridgeProtocolhandles the higher-level operations. Based on learnings, this follows the correct separation of concerns.src/runtime/pyodide.ts (1)
66-88: LGTM!Clean implementation of
PyodideBridgeextendingBridgeProtocol. The constructor correctly:
- Creates
PyodideIOtransport withindexURLandpackagesoptions- Passes transport, codec, and timeout options to the base
BridgeProtocolclass- Accepts a default-empty options object for convenience
The implementation is consistent with
HttpBridgeand follows the ADR-002 architecture.test/bridge-protocol.test.ts (4)
336-351: LGTM on protocol message encoding test.Test correctly verifies the new envelope structure with
method: 'call'and nestedparamscontainingmodule,functionName, andargs.
469-491: LGTM oncall()interface test.Test correctly verifies:
- Result extraction from response envelope (SafeCodec behavior)
- Message structure with
method: 'call'and params fields- Support for kwargs in the params object
705-744: LGTM on WorkerPool integration tests.Tests correctly use the new protocol format with
protocol: 'tywrap/1',method: 'call', andparams: {}in the message payloads.
834-854: LGTM on full stack integration test.Comprehensive test correctly validates the complete request-response cycle through the pool with:
- New protocol envelope encoding
- SafeCodec extracting the result from response
- Correct numeric ID usage
src/runtime/node.ts (4)
279-301: Initialization sequence is well ordered.Script existence check, Arrow decoder registration, and transport init before warmups read cleanly.
310-343: Caching override + helper heuristics look solid.The cache key guard and purity heuristic are contained and easy to reason about.
Also applies to: 424-462
349-397: Pool stats surface looks good for backwards compatibility.Deprecation path is clear and keeps legacy callers functional.
131-165: Utility helpers and env construction are clean and predictable.The venv resolution and filtered env passthrough are straightforward and safe.
Also applies to: 473-518
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
coderabbitai
bot
added
area:codec
- Remove unused type imports (HttpIOOptions, ProcessIOOptions, PooledTransportOptions, WorkerPoolOptions) - Add cwd support to ProcessIO for subprocess working directory - Pass cwd from NodeBridge to ProcessIO for proper subprocess isolation Known limitations (documented for future work): - minProcesses option is not wired through to PooledTransport - Warmup commands run once, not per-worker in multi-process scenarios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CodeRabbit Review ResponseFixed in latest commit:✅ Unused imports - Removed ✅ Custom cwd dropped when spawning ProcessIO - Added Known limitations (deferred to future work):⏸️ ⏸️ Warmup commands run once, not per-worker - In multi-process scenarios (
These are valid improvements for a future PR focused on multi-process optimization. |
Responses to Individual CodeRabbit Comments✅ Fixed
⏸️ Deferred (Known Limitations)
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@examples/living-app/src/index.ts`:
- Around line 126-129: The comment next to the codec check is misleading: update
the comment around the if (codec === 'arrow') block to state that
enableArrowDecoder() will throw immediately if the JS-side apache-arrow package
is missing (so the failure is eager), whereas absence of the Python-side pyarrow
would manifest later at decode time; mention both failure modes and reference
enableArrowDecoder and the apache-arrow / pyarrow dependencies so readers
understand which error occurs when.
In `@src/runtime/node.ts`:
- Around line 366-395: getStats currently sets busyWorkers to
poolStats.totalInFlight which counts in-flight requests and can exceed
workerCount when maxConcurrentPerWorker > 1; update getStats to either (a) clamp
busyWorkers to the number of workers by setting busyWorkers =
Math.min(poolStats.totalInFlight, poolStats.workerCount) so it truly reflects
busy worker slots, or (b) if preserving precise request count is desired, rename
the field to busyRequests (and update any consumers) and leave it as
totalInFlight; change references to busyWorkers in getStats and any callers
accordingly (functions: getStats, getPoolStats; properties: totalInFlight,
workerCount, maxConcurrentPerWorker).
- Around line 407-416: The warmup command loop in runtime
(this.resolvedOptions.warmupCommands inside src/runtime/node.ts) currently
silently ignores legacy { method, params } entries; update the loop to detect
legacy entries and either map them to the new shape (e.g., translate { method,
params } -> call(module=?, functionName=method, args=params) or another explicit
mapping you choose) or emit a clear warning/error via the runtime logger when a
legacy entry is encountered (reference this.resolvedOptions.warmupCommands, the
loop handling code, and the call(...) method) so misconfiguration is not
silently skipped; implement the chosen behavior and ensure tests/log messages
reflect the change.
♻️ Duplicate comments (3)
src/runtime/node.ts (3)
20-23: Unused type-only imports still trigger lint.
ProcessIOOptionsandPooledTransportOptionsare still imported but unused; remove or use them to keep lint clean.
33-41:minProcessesis still a no‑op in pool wiring.
The option is documented but never propagated to the pool config, so it has no effect.Also applies to: 244-253
245-250: Customcwdisn’t forwarded to ProcessIO.
resolvedOptions.cwdis computed but not passed toProcessIO, so the subprocess runs inprocess.cwd()even when users setcwd.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
examples/living-app/src/index.tssrc/runtime/node.tstest-d/types.test-d.ts
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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.
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: 152
File: docs/adr/002-bridge-protocol.md:636-637
Timestamp: 2026-01-20T16:01:29.276Z
Learning: In the tywrap BridgeProtocol ProcessIO transport (ADR-002), when implementing `restartAfterRequests` functionality, use drain-before-restart as the default: wait for `pendingRequests.size === 0` before killing and restarting the process. This prevents rejecting in-flight requests during restart. Alternative graceful restart (spawn new process, drain old) adds complexity with two concurrent processes.
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
📚 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:
examples/living-app/src/index.tssrc/runtime/node.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:
examples/living-app/src/index.ts
📚 Learning: 2026-01-20T16:00:49.738Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:168-172
Timestamp: 2026-01-20T16:00:49.738Z
Learning: In the tywrap project's BridgeProtocol SafeCodec implementation, Arrow format decoders can produce NaN/Infinity values from binary representations even when the raw JSON payload doesn't contain them. This is why validation for special floats must occur both before encoding (to reject invalid inputs) and after applying decoders (to catch values introduced during Arrow deserialization), protecting downstream consumers from unexpected NaN/Infinity values.
Applied to files:
examples/living-app/src/index.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 `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:
examples/living-app/src/index.tstest-d/types.test-d.tssrc/runtime/node.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:
examples/living-app/src/index.tstest-d/types.test-d.tssrc/runtime/node.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:
examples/living-app/src/index.tstest-d/types.test-d.tssrc/runtime/node.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:
examples/living-app/src/index.tstest-d/types.test-d.tssrc/runtime/node.ts
📚 Learning: 2026-01-19T21:14:35.390Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:35.390Z
Learning: In src/runtime/bridge-core.ts and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (such as runtime, shape, or error-payload checks) inside tight loops unless the protocol boundary is untrusted or there is a concrete bug report. The Python bridge protocol is controlled via tests, so these checks add unnecessary branching overhead without meaningful benefit. Apply this guidance to other hot-path runtime loops in src/runtime/**/*.ts, and re-enable additional validations only when a documented risk or failure scenario is identified. Ensure tests cover protocol validation where applicable.
Applied to files:
src/runtime/node.ts
📚 Learning: 2026-01-20T16:01:29.276Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:636-637
Timestamp: 2026-01-20T16:01:29.276Z
Learning: In the tywrap BridgeProtocol ProcessIO transport (ADR-002), when implementing `restartAfterRequests` functionality, use drain-before-restart as the default: wait for `pendingRequests.size === 0` before killing and restarting the process. This prevents rejecting in-flight requests during restart. Alternative graceful restart (spawn new process, drain old) adds complexity with two concurrent processes.
Applied to files:
src/runtime/node.ts
📚 Learning: 2026-01-20T01:36:14.701Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:743-759
Timestamp: 2026-01-20T01:36:14.701Z
Learning: In the tywrap repository, when reviewing concurrent request handling patterns (especially with BridgeCore request multiplexing), flag uses of boolean `busy` flags that should be request counters instead. A boolean flag creates a race condition: when request A completes and sets `busy = false`, request B (still in flight via multiplexing) is incorrectly treated as idle. The fix is to use an `inFlightRequests: number` counter that increments on request start and decrements on completion, then check `worker.inFlightRequests === 0` before cleanup/termination.
Applied to files:
src/runtime/node.ts
📚 Learning: 2026-01-20T01:33:12.841Z
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.
Applied to files:
src/runtime/node.ts
📚 Learning: 2026-01-19T21:14:29.869Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:29.869Z
Learning: In the runtime env-var parsing (e.g., in src/runtime/bridge-core.ts and similar modules), adopt a tolerant, best-effort policy: numeric env values should parse by taking the leading numeric portion (e.g., TYWRAP_CODEC_MAX_BYTES=1024abc -> 1024). Only reject clearly invalid values (non-numeric start or <= 0). This reduces surprising failures from minor typos. Add tests to cover partial numeric prefixes, and ensure downstream logic documents the trusted range; consider a small helper to extract a positive integer from a string with a safe fallback.
Applied to files:
src/runtime/node.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:
src/runtime/node.ts
🧬 Code graph analysis (1)
src/runtime/node.ts (5)
src/runtime/bridge-protocol.ts (2)
BridgeProtocol(70-319)BridgeProtocolOptions(30-39)src/runtime/pooled-transport.ts (1)
PooledTransport(71-236)src/utils/python.ts (1)
getDefaultPythonPath(43-45)src/runtime/process-io.ts (1)
ProcessIO(134-783)src/utils/cache.ts (1)
globalCache(634-634)
🪛 GitHub Check: lint
src/runtime/node.ts
[warning] 23-23:
'PooledTransportOptions' is defined but never used
[warning] 22-22:
'ProcessIOOptions' is defined but never used
[warning] 245-245:
Missing return type on function
[warning] 509-509:
Generic Object Injection Sink
[warning] 508-508:
Generic Object Injection Sink
[warning] 499-499:
Generic Object Injection Sink
[warning] 491-491:
Generic Object Injection Sink
[warning] 482-482:
Generic Object Injection Sink
🔇 Additional comments (9)
test-d/types.test-d.ts (1)
227-227: LGTM for updated HttpBridge constructor usage.examples/living-app/src/index.ts (2)
54-61: LGTM!The
enableArrowDecoderfunction properly checks for the optionalapache-arrowdependency and provides a clear, actionable error message when it's missing. This is a clean pattern for handling optional dependencies.
173-186: LGTM!Good cleanup hygiene: the finally block properly removes the temp directory, disposes the bridge, and clears the global Arrow decoder state. The swallowed exception in the
rmSynccatch is acceptable for cleanup code.src/runtime/node.ts (6)
131-165: Utility path helpers look solid.
No issues withresolveDefaultScriptPath,resolveVirtualEnv, orgetPathKey.
279-300: Init flow is clear and ordered.
Script validation → Arrow decoder registration → transport init → warmup is sensible.
310-343: Caching wrapper is clean and well‑gated.
The cache lookup/write are correctly scoped toenableCache.
352-357: Pool stats passthrough looks good.
Straight mapping fromPooledTransportmetrics.
424-462: Cache key handling and purity heuristics look reasonable.
Safe cache key generation and the purity heuristic are both clear and conservative.
473-526: Env construction and venv wiring look solid.
Whitelisting plus PATH/PYTHONPATH augmentation is consistent and easy to follow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (codec === 'arrow') { | ||
| const info = await bridge.getBridgeInfo(); | ||
| if (!info.arrowAvailable) { | ||
| // Why: fail fast with a clear message; otherwise the bridge will emit Arrow envelopes and the | ||
| // caller will see confusing decode errors. | ||
| throw new Error( | ||
| 'Arrow mode requested but pyarrow is not installed in the Python environment. Install pyarrow or run with --json.' | ||
| ); | ||
| } | ||
| // Enable Arrow decoder - will fail at decode time if pyarrow is not installed | ||
| await enableArrowDecoder(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Comment is slightly misleading regarding failure timing.
The comment states failure occurs "at decode time if pyarrow is not installed," but enableArrowDecoder() throws immediately if the JS-side apache-arrow package is missing (lines 56-59). The pyarrow (Python-side) absence would indeed fail at decode time, but the JS dependency failure happens eagerly.
Consider clarifying:
if (codec === 'arrow') {
- // Enable Arrow decoder - will fail at decode time if pyarrow is not installed
+ // Enable Arrow decoder - throws now if apache-arrow (JS) is missing; pyarrow (Python) absence fails at decode time
await enableArrowDecoder();
}🤖 Prompt for AI Agents
In `@examples/living-app/src/index.ts` around lines 126 - 129, The comment next to
the codec check is misleading: update the comment around the if (codec ===
'arrow') block to state that enableArrowDecoder() will throw immediately if the
JS-side apache-arrow package is missing (so the failure is eager), whereas
absence of the Python-side pyarrow would manifest later at decode time; mention
both failure modes and reference enableArrowDecoder and the apache-arrow /
pyarrow dependencies so readers understand which error occurs when.
| getStats(): { | ||
| totalRequests: number; | ||
| totalTime: number; | ||
| cacheHits: number; | ||
| poolHits: number; | ||
| poolMisses: number; | ||
| processSpawns: number; | ||
| processDeaths: number; | ||
| memoryPeak: number; | ||
| averageTime: number; | ||
| cacheHitRate: number; | ||
| poolSize: number; | ||
| busyWorkers: number; | ||
| } { | ||
| const poolStats = this.getPoolStats(); | ||
| return { | ||
| // Legacy stats (no longer tracked, return 0) | ||
| totalRequests: 0, | ||
| totalTime: 0, | ||
| cacheHits: 0, | ||
| poolHits: 0, | ||
| poolMisses: 0, | ||
| processSpawns: 0, | ||
| processDeaths: 0, | ||
| memoryPeak: 0, | ||
| averageTime: 0, | ||
| cacheHitRate: 0, | ||
| // Current pool stats | ||
| poolSize: poolStats.workerCount, | ||
| busyWorkers: poolStats.totalInFlight, |
There was a problem hiding this comment.
busyWorkers reports requests, not workers.
busyWorkers uses totalInFlight, which can exceed workerCount when maxConcurrentPerWorker > 1. Consider clamping or renaming to avoid misleading legacy consumers.
🧮 Suggested adjustment
- busyWorkers: poolStats.totalInFlight,
+ busyWorkers: Math.min(poolStats.totalInFlight, poolStats.workerCount),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getStats(): { | |
| totalRequests: number; | |
| totalTime: number; | |
| cacheHits: number; | |
| poolHits: number; | |
| poolMisses: number; | |
| processSpawns: number; | |
| processDeaths: number; | |
| memoryPeak: number; | |
| averageTime: number; | |
| cacheHitRate: number; | |
| poolSize: number; | |
| busyWorkers: number; | |
| } { | |
| const poolStats = this.getPoolStats(); | |
| return { | |
| // Legacy stats (no longer tracked, return 0) | |
| totalRequests: 0, | |
| totalTime: 0, | |
| cacheHits: 0, | |
| poolHits: 0, | |
| poolMisses: 0, | |
| processSpawns: 0, | |
| processDeaths: 0, | |
| memoryPeak: 0, | |
| averageTime: 0, | |
| cacheHitRate: 0, | |
| // Current pool stats | |
| poolSize: poolStats.workerCount, | |
| busyWorkers: poolStats.totalInFlight, | |
| getStats(): { | |
| totalRequests: number; | |
| totalTime: number; | |
| cacheHits: number; | |
| poolHits: number; | |
| poolMisses: number; | |
| processSpawns: number; | |
| processDeaths: number; | |
| memoryPeak: number; | |
| averageTime: number; | |
| cacheHitRate: number; | |
| poolSize: number; | |
| busyWorkers: number; | |
| } { | |
| const poolStats = this.getPoolStats(); | |
| return { | |
| // Legacy stats (no longer tracked, return 0) | |
| totalRequests: 0, | |
| totalTime: 0, | |
| cacheHits: 0, | |
| poolHits: 0, | |
| poolMisses: 0, | |
| processSpawns: 0, | |
| processDeaths: 0, | |
| memoryPeak: 0, | |
| averageTime: 0, | |
| cacheHitRate: 0, | |
| // Current pool stats | |
| poolSize: poolStats.workerCount, | |
| busyWorkers: Math.min(poolStats.totalInFlight, poolStats.workerCount), |
🤖 Prompt for AI Agents
In `@src/runtime/node.ts` around lines 366 - 395, getStats currently sets
busyWorkers to poolStats.totalInFlight which counts in-flight requests and can
exceed workerCount when maxConcurrentPerWorker > 1; update getStats to either
(a) clamp busyWorkers to the number of workers by setting busyWorkers =
Math.min(poolStats.totalInFlight, poolStats.workerCount) so it truly reflects
busy worker slots, or (b) if preserving precise request count is desired, rename
the field to busyRequests (and update any consumers) and leave it as
totalInFlight; change references to busyWorkers in getStats and any callers
accordingly (functions: getStats, getPoolStats; properties: totalInFlight,
workerCount, maxConcurrentPerWorker).
src/runtime/node.ts
Outdated
| for (const cmd of this.resolvedOptions.warmupCommands) { | ||
| try { | ||
| // Handle both new and legacy warmup command formats | ||
| if ('module' in cmd && 'functionName' in cmd) { | ||
| // New format: { module, functionName, args? } | ||
| await this.call(cmd.module, cmd.functionName, cmd.args ?? []); | ||
| } else if ('method' in cmd && 'params' in cmd) { | ||
| // Legacy format: { method, params } - skip, not supported in new architecture | ||
| // Legacy warmup commands were for the old BridgeCore protocol | ||
| } |
There was a problem hiding this comment.
Legacy warmup format is silently ignored.
NodeBridgeOptions advertises { method, params } compatibility, but the runtime no‑ops it. That can hide misconfiguration. Either translate it to the new format or surface an explicit warning/error and update the docs.
💡 Example guard to avoid silent skips
- } else if ('method' in cmd && 'params' in cmd) {
- // Legacy format: { method, params } - skip, not supported in new architecture
- // Legacy warmup commands were for the old BridgeCore protocol
- }
+ } else if ('method' in cmd && 'params' in cmd) {
+ throw new BridgeProtocolError(
+ 'Legacy warmupCommands format is not supported with BridgeProtocol. Use {module, functionName, args}.'
+ );
+ }🤖 Prompt for AI Agents
In `@src/runtime/node.ts` around lines 407 - 416, The warmup command loop in
runtime (this.resolvedOptions.warmupCommands inside src/runtime/node.ts)
currently silently ignores legacy { method, params } entries; update the loop to
detect legacy entries and either map them to the new shape (e.g., translate {
method, params } -> call(module=?, functionName=method, args=params) or another
explicit mapping you choose) or emit a clear warning/error via the runtime
logger when a legacy entry is encountered (reference
this.resolvedOptions.warmupCommands, the loop handling code, and the call(...)
method) so misconfiguration is not silently skipped; implement the chosen
behavior and ensure tests/log messages reflect the change.
…worker warmup - Add minWorkers option to WorkerPool for pre-spawning workers during init - Add onWorkerReady callback to WorkerPool for per-worker initialization - Wire minProcesses from NodeBridge through PooledTransport to WorkerPool - Execute warmupCommands on each worker individually via onWorkerReady - Remove global runWarmupCommands() in favor of per-worker warmup - Clean up unused imports in pyodide-io.ts and pyodide.ts This resolves the known limitations from CodeRabbit review: - minProcesses is now properly wired through to pre-spawn workers - warmupCommands now execute on each worker, not just the first one Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Key Changes
Bridge Migrations
__tywrap_dispatchhandlerNew Components
src/runtime/pooled-transport.ts): Semaphore-based worker pool for concurrent process management with configurable min/max processesstdin.end()on shutdown, backpressure-aware writes with try-catch for synchronous errorsTest Updates
setMockDispatchHandlerpatternresolvedOptionsand transport propertiesprocessSpawns,processDeaths) that don't apply to new architectureBreaking Changes (pre-1.0)
getBridgeInfo()method removed (protocol info no longer exposed)processPoolnot accessible (managed by PooledTransport)Test plan
npm run buildpassesnpm run typecheckpassesnpm testpasses (1219 tests pass, 127 skipped)Related
🤖 Generated with Claude Code