Extract code into separate rpc server

pokey / command-server

Secure VSCode communication channel designed for voice coding

MIT License

16 stars 9 forks source link

Extract code into separate rpc server #27

Open AndreasArvidsson opened 5 months ago

AndreasArvidsson commented 5 months ago

There's also a Talon side of this https://github.com/talonhub/community/pull/1433

The purpose of this pr is two fold

Allow multiple rpc clients at the same time. I'm going to use this to be able to control vscode and my clipboard manager at the same time.
Make nice abstractions separating what's actually needed for the rpc client from file system and node.

Interesting parts

Introduces the RpcServer interface. This isn't file system specific and could be implemented by sockets. Doesn't require file system or node.
RpcServerFs is a file system based implementation of RpcServer, but still not node specific.
FileSystemNode is basically our current implementation of node io. Accepts a path to the communication directory which means you can have multiple instances off different rpc fs clients.

pokey commented 5 months ago

You seem to have a bunch of merge conflicts. Maybe you started from an old version of main?

AndreasArvidsson commented 5 months ago

Yeah I did, but I have fixed it now. I have on purpose reverted the io interface. As we discussed to much of the code and logic is around this being a file based rpc.

AndreasArvidsson commented 5 months ago

to do: understand if it's easy to implement #21 without io.ts

Here is my version of that: https://github.com/AndreasArvidsson/command-server/tree/rpcServer_fallback/src

bjaspan commented 5 months ago

The current Io abstraction and nativeIo implementation ensures that all direct filesystem access and use a Node modules that make filesystem-related assumptions (e.g. fs/promises, path, etc.) are contained within nativeIo.

I've only taken a quick look at this so far, but it looks like PR loses this containment; for example I see an import of fs/promises in rpcServer. Is that correct? If so, this change will make a web version of this extension (and thus of Cursorless) impossible.

AndreasArvidsson commented 5 months ago

The purpose here is that it should not just be a file based rpc. We can definitely make an interface for the file based implementations since it appears that we have multiple of those, but the core abstraction should probably be more flexible se we can have rpc based on sockets and so on. At least that was my assumption.

This needs to split into multiple packages. There should be a common package with just the interfaces and the common logic that is separate from the file based implementation.

@pokey Do you feel ready for a monorepo setup again? :D

bjaspan commented 5 months ago

I note that my internal Google implementation of the Io interface works entirely over stdio to a subprocess that it starts in init, so the existing interface is at least already most of the way towards supporting an RPC based on sockets. Further refactoring it's fine with me though so long as it maintains my primary objective.