Service Daemon Application #88

jonah-iden · 2025-01-28T10:47:03Z

This is a small node application, encapsulating the open-collaboration-protocol and yjs libraries and making them available to other non js based environments.
The application communicates through JSON based message on stdout. For the most part it just forwards messages from or to the application/open collaboration protocol library. But for the initial login, room join/create and yjs it has some special messages. A question here is still if they should also use the parameter array patter instead of wrapping the arguments into json data.

There is a test (service.process.test.ts) that shows a little bit how this should be used.
I would love some feedback on this.

Signed-off-by: Jonah Iden <[email protected]>

… fixes for yjs Signed-off-by: Jonah Iden <[email protected]>

jonah-iden · 2025-01-28T14:30:31Z

Current plan for restructuring the message format for custom messages is to move properties to a params array and change the method names to be more method like and more similiar to the format that the oc server uses

Signed-off-by: Jonah Iden <[email protected]>

spoenemann

Thanks!

We could consider using JSON-RPC between the service process and the host IDE.

About the terminology: if we call it a service daemon, it should run as an independent process to which the IDE can connect, right? If the background process lifecycle is fully controlled by the IDE, strictly speaking it's not a daemon.

spoenemann · 2025-01-29T10:39:02Z

packages/open-collaboration-service-daemon/src/process.ts

+const args = parseArgs({options: {
+    'server-address': {type: 'string'},
+    'auth-token': {type: 'string'},
+}});


Shall we use a full CLI library like commander?

I don't think thats neccessary right now. In my opinion we can think about that when we need to add more arguments. Or what benefit would you see in changing this right now?

If you introduce it later, it might break the usage; for example, options might be prefixed like --auth-token

jonah-iden · 2025-01-29T11:12:33Z

Regarding jsonRPC I'm thinking about using the same format as OCP already does with wrapper messages containing the json rpc messages like

{
type: 'request'
id: 1
content: {
  method: 'someMethod:
  params: ['someParam']
  }
}

But i would not go full json rpc so that the compatibility with OCP is still given and the user can send arbitrary messages as if communicating with the open collab server directly

Your right i'll change the name.

Signed-off-by: Jonah Iden <[email protected]>

kaisalmen

Thank you very much. I left a couple of comments. This is a good starting point to start connecting other languages.

kaisalmen · 2025-01-30T13:03:49Z

packages/open-collaboration-service-daemon/package.json

+        "open-collaboration-protocol": "0.2.0",
+        "async-mutex": "^0.5.0"
+    },
+    "devDependencies": {


Because this is a workspace project it makes sense to move devDependencies to the root as they are irrelevant to the individual npm packages. This is true for any other package as well, of course.

kaisalmen · 2025-01-30T13:12:15Z

.vscode/launch.json

+            "program": "${workspaceFolder}/packages/open-collaboration-service-daemon/lib/process.js",
+            "args": [
+                "--server-address=http://localhost:8100",
+                "--auth-token=eyJhbGciOiJIUzI1NiJ9.eyJpZCI6ImZlYTBySW5BU1phWkVDUE9IcXRsZ0lmNSIsIm5hbWUiOiJUZXN0IiwiZW1haWwiOiIiLCJhdXRoUHJvdmlkZXIiOiJVbnZlcmlmaWVkIiwiaWF0IjoxNzMxMzI3MjQzfQ.glt-hv5QtHYzRp2K2uILsNJjPLWJSrWlRVPMOuG8cQI"


Is this token still correct?

kaisalmen · 2025-01-30T13:41:37Z

packages/open-collaboration-service-daemon/src/message-handler.ts

+    protected handlers = new Map<string, (message: unknown) => MaybePromise<void | unknown>>(
+        [
+            ['login', () => this.login()],
+            ['room/joinRoom', message => this.joinRoom(message as JoinRoomRequest)],


Does it make sense to declare those strings as accessible values in the protocol, so they can be referenced in the protocol itself and externally like here? Something like a protocol command index.

kaisalmen · 2025-01-30T13:48:43Z

packages/open-collaboration-service-daemon/test/service.process.test.ts

+        host.process?.kill();
+        guest.process?.kill();
+    });
+    test('test service processes without login', async () => {


Thank you this is very helpful we could introduce a passive containerized Java client for another test (not in the scope of this PR) that answers what a test expects.

kaisalmen · 2025-01-30T13:50:47Z

packages/open-collaboration-service-daemon/src/process.ts

+    userToken: args.values['auth-token']
+});
+
+new MessageHandler(connectionProvider, communicationHandler);


A general idea. Should we have command messages that allow to change behaviour of the process via stdin/out that allows a client like an idea to change the runtime behaviour or trigger a re-connect to another oct server.

Signed-off-by: Jonah Iden <[email protected]>

msujew

As an additional test case for the API: Can we migrate the existing VS Code client to the API exposed by the open-collaboration-service-daemon?

msujew · 2025-02-20T17:12:14Z

packages/open-collaboration-protocol/src/messaging/abstract-connection.ts

+    onUnhandledRequest(handler: UnhandledMessageHandler): void;
+    onUnhandledNotification(handler: UnhandledMessageHandler): void;
+    onUnhandledBroadcast(handler: UnhandledMessageHandler): void;


Suggestion: Instead of having onUnhandled*, we could provide an override for onBroadcast, etc that simply accepts a handler that acts as the generic handler in case no specific handler has been found. The vscode-languageserver library uses a very similar API.

msujew · 2025-02-20T17:12:57Z

packages/open-collaboration-service-daemon/scripts/sea-build.js

+const execSync = require('child_process').execSync;
+const fs = require('fs');
+const inject = require('postject').inject


Suggestion: Can we make this into a .mjs file? Then we can use import instead of require. Alternatively, make this into an .ts file and execute it with tsx.

msujew · 2025-02-20T17:16:35Z

packages/open-collaboration-service-daemon/test/service.process.test.ts

+const authTokenHost: string = 'eyJhbGciOiJIUzI1NiJ9.eyJpZCI6IkF2S2JlczliU1hDZ0FxSHhwa0xqYVRBcyIsIm5hbWUiOiJob3N0IiwiZW1haWwiOiIiLCJhdXRoUHJvdmlkZXIiOiJVbnZlcmlmaWVkIiwiaWF0IjoxNzM4MjQxNTcwfQ.IWetdyBTAo2DswcYKs5Jzxl3AzuKGccFnGsc5A9XE8s';
+const authTokenGuest: string = 'eyJhbGciOiJIUzI1NiJ9.eyJpZCI6IlBrQmN4blBJc1Qzenl5YVlLUEpGWXRxViIsIm5hbWUiOiJndWVzdCIsImVtYWlsIjoiIiwiYXV0aFByb3ZpZGVyIjoiVW52ZXJpZmllZCIsImlhdCI6MTczODI0MTY0Nn0.ABHj54q5u_z1Cd57Mscryp4rMPPiwxLLfSl-anCtD1E';


Suggestion: Can we not hardcode the tokens into the test? :D It shouldn't be too difficult to go through the unverified/anonymous login process.

jonah-iden · 2025-02-21T12:06:06Z

@msujew regarding migrating the vscode client, I would keep typescript/js integrations using the library since that brings typesafety with it which the service process does not have. The service process just wraps the libraries so there is not alot to gain here by migrating.

msujew · 2025-02-21T14:40:31Z

@jonah-iden What I mean is that I would like to abstract away the yjs integration. I feel like I've duplicated a lot of yjs related code between the Theia and VS Code integration and the daemon process is also duplicating a lot of code. Having one source that abstracts away the yjs integration logic would we great :)

jonah-iden · 2025-02-21T14:44:40Z

@msujew ok i see. That makes sense. But maybe there should rather be some helper class in the yjs library instead that can be used by all js integration without the service process. Then the monaco integration could benefit from it as well

jonah-iden added 11 commits October 11, 2024 14:11

service-daemon process basics

1136457

Signed-off-by: Jonah Iden <[email protected]>

added generic message handling

c7f7693

Signed-off-by: Jonah Iden <[email protected]>

added yjs provider

abee367

Signed-off-by: Jonah Iden <[email protected]>

send generic messages to host

51c6ef1

Signed-off-by: Jonah Iden <[email protected]>

restructuring and join handling

4bfc287

Signed-off-by: Jonah Iden <[email protected]>

SEA build scripts

455221f

Signed-off-by: Jonah Iden <[email protected]>

working creating room with join

ae2b8c7

Signed-off-by: Jonah Iden <[email protected]>

refactor messages to be closer to the ocp

0ec2a0d

Signed-off-by: Jonah Iden <[email protected]>

startet on test for service daemon

755f196

Signed-off-by: Jonah Iden <[email protected]>

added test for service daemon and fixed multiple smaller issues

766b7f6

Signed-off-by: Jonah Iden <[email protected]>

extended test with some yjs document messages, and added some smaller…

8815558

… fixes for yjs Signed-off-by: Jonah Iden <[email protected]>

changed method names to be more aligned with ocp

78f0155

Signed-off-by: Jonah Iden <[email protected]>

spoenemann requested review from msujew and kaisalmen January 29, 2025 10:32

spoenemann reviewed Jan 29, 2025

View reviewed changes

jonah-iden added 4 commits January 30, 2025 12:18

fixed test for new messages

b125fd7

Signed-off-by: Jonah Iden <[email protected]>

fixed test for other directories and added to pipeline

79951ce

Signed-off-by: Jonah Iden <[email protected]>

added more server debug info

69705b2

Signed-off-by: Jonah Iden <[email protected]>

custom oct jwt private key for test

a24c32d

Signed-off-by: Jonah Iden <[email protected]>

kaisalmen reviewed Jan 30, 2025

View reviewed changes

changed message types more to the json rpc format used by ocp

4e18902

Signed-off-by: Jonah Iden <[email protected]>

msujew reviewed Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service Daemon Application #88

Service Daemon Application #88

jonah-iden commented Jan 28, 2025

jonah-iden commented Jan 28, 2025

spoenemann left a comment

spoenemann Jan 29, 2025

jonah-iden Jan 29, 2025

spoenemann Feb 5, 2025

jonah-iden commented Jan 29, 2025 •

edited

Loading

kaisalmen left a comment

kaisalmen Jan 30, 2025

kaisalmen Jan 30, 2025

kaisalmen Jan 30, 2025

kaisalmen Jan 30, 2025

kaisalmen Jan 30, 2025

msujew left a comment

msujew Feb 20, 2025

msujew Feb 20, 2025

msujew Feb 20, 2025

jonah-iden commented Feb 21, 2025

msujew commented Feb 21, 2025

jonah-iden commented Feb 21, 2025 •

edited

Loading

		const authTokenHost: string = 'eyJhbGciOiJIUzI1NiJ9.eyJpZCI6IkF2S2JlczliU1hDZ0FxSHhwa0xqYVRBcyIsIm5hbWUiOiJob3N0IiwiZW1haWwiOiIiLCJhdXRoUHJvdmlkZXIiOiJVbnZlcmlmaWVkIiwiaWF0IjoxNzM4MjQxNTcwfQ.IWetdyBTAo2DswcYKs5Jzxl3AzuKGccFnGsc5A9XE8s';
		const authTokenGuest: string = 'eyJhbGciOiJIUzI1NiJ9.eyJpZCI6IlBrQmN4blBJc1Qzenl5YVlLUEpGWXRxViIsIm5hbWUiOiJndWVzdCIsImVtYWlsIjoiIiwiYXV0aFByb3ZpZGVyIjoiVW52ZXJpZmllZCIsImlhdCI6MTczODI0MTY0Nn0.ABHj54q5u_z1Cd57Mscryp4rMPPiwxLLfSl-anCtD1E';

Service Daemon Application #88

Are you sure you want to change the base?

Service Daemon Application #88

Conversation

jonah-iden commented Jan 28, 2025

jonah-iden commented Jan 28, 2025

spoenemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonah-iden commented Jan 29, 2025 • edited Loading

kaisalmen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msujew left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonah-iden commented Feb 21, 2025

msujew commented Feb 21, 2025

jonah-iden commented Feb 21, 2025 • edited Loading

jonah-iden commented Jan 29, 2025 •

edited

Loading

jonah-iden commented Feb 21, 2025 •

edited

Loading