Skip to content

feat(remote-comms): Add Node.js e2e tests and fix shutdown handling#692

Merged
sirtimid merged 1 commit intomainfrom
sirtimid/remote-comms-e2e-test
Nov 4, 2025
Merged

feat(remote-comms): Add Node.js e2e tests and fix shutdown handling#692
sirtimid merged 1 commit intomainfrom
sirtimid/remote-comms-e2e-test

Conversation

@sirtimid
Copy link
Copy Markdown
Contributor

@sirtimid sirtimid commented Nov 4, 2025

This PR adds comprehensive end-to-end tests for remote kernel communications in Node.js and addresses critical shutdown issues discovered during testing.

Changes

Testing Infrastructure:

  • Added Node.js e2e tests for basic remote kernel connectivity, message passing, and OCAP URL handling
  • Tests cover basic kernel-to-kernel communication through a libp2p relay server

Remote Communications Fixes:

  • Fixed hanging libp2p.stop() issue by switching to connection abort strategy instead of full stop
  • Made stopRemoteComms() safe to call when remote comms not initialized (returns early instead of throwing)
  • Improved relay server configuration for better connection management (reduced hop timeout, disabled default limits)

Database Management:

  • Added close() method to KernelDatabase interface for proper resource cleanup
  • Implemented database close functionality for both Node.js and WASM SQLite implementations
  • Tests now use in-memory databases (:memory:) to avoid file locking issues

Kernel Shutdown:

  • Refactored kernel stop sequence to properly stop remote comms through platform services
  • Fixed test cleanup to ensure kernels and databases are properly closed after each test
  • Resolved hanging process issues during test suite execution

Technical Details

The main issue addressed was that libp2p.stop() could hang indefinitely, preventing clean kernel shutdown. The solution involves:

  1. Aborting all active connections instead of calling libp2p.stop()
  2. Clearing the libp2p instance reference to allow garbage collection
  3. Ensuring proper cleanup order: queue → command stream → remote comms → platform services

Testing

  • All existing tests pass
  • New e2e tests verify basic remote kernel communication functionality
  • Tests run sequentially to prevent resource conflicts

Note

Adds Node.js e2e remote-comms tests, shifts libp2p shutdown to connection aborts, makes stopRemoteComms safe if uninitialized, refactors kernel stop order, and introduces KernelDatabase.close().

  • Remote communications & shutdown:
    • ConnectionFactory.stop() now aborts all connections (no libp2p.stop()), clears inflight dials, and logs hangups; tests updated accordingly.
    • Kernel stop() reordered: wait for queue → end command stream → stop remote comms via platform services → terminate workers.
    • Platform services (browser/node): stopRemoteComms returns early if not initialized; related tests updated.
    • Relay config tweaks in packages/cli/src/relay.ts: disable default reservation limits and reduce hopTimeout; removed unused protocol handler; added connection/discovery logging.
  • Node.js E2E tests:
    • New end-to-end tests for remote kernel connectivity and OCAP URL messaging via a local relay; adds test/vats/remote-vat.js and test/make-test-kernel.ts.
    • CI script passes through args to e2e runner.
  • Kernel store:
    • Add close() to KernelDatabase; implement in SQLite (Node/WASM) and add tests.
  • Misc:
    • Add dependencies for tests/runtime (@endo/eventual-send, @endo/marshal, @libp2p/interface); adjust coverage thresholds.

Written by Cursor Bugbot for commit 400678c. This will update automatically on new commits. Configure here.

@sirtimid sirtimid requested a review from a team as a code owner November 4, 2025 21:23
Copy link
Copy Markdown
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This looks like it potentially might address one of the things I'm trying to puzzle out!

@sirtimid sirtimid enabled auto-merge (squash) November 4, 2025 21:58
@sirtimid sirtimid merged commit 1f133c3 into main Nov 4, 2025
58 of 64 checks passed
@sirtimid sirtimid deleted the sirtimid/remote-comms-e2e-test branch November 4, 2025 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants