sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Create a DTD service for managing connected apps

Open kenzieschmoll opened this issue 7 months ago • 19 comments

Background

The Dart Tooling Daemon (DTD) is either started by DDS / DevTools server, in the case where an app is started from the CLI, or an IDE plugin / extension (VS Code, Android Studio, or IntelliJ), in the case where an app is started from the IDE. This means that DTD may be connected to a development workflow that has one or many running apps.

For DTD clients like DevTools and the new Dart Tooling MCP Server, a client may need to fetch the list of active debug sessions (the running Dart / Flutter apps). In the case where DTD is started from a 1P supported IDE (VS Code or Android Studio / IntelliJ), the IDE provides an EditorService from which a client can get the list of active debug sessions, and therefore the list of the active VM Service URIs. However, in the case where a 1P supported IDE did NOT start DTD, this service will be unavailable and there is no way to get the VM service URI of the running app(s).

The following is a proposed solution to this problem, allowing DTD to have awareness of the connected Dart & Flutter applications that the DTD clients are interacting with.

Proposed solution

Create a new service ConnectedAppService directly in DTD that provides the methods:

  • [Protected] void registerVmService(String uri)
  • List<String> getVmServiceUris()

[Protected] here means that the only DTD client that started DTD (either the IDE or DDS / DevTools server) should be able to call this method. We can use the same client secret that we require for the setIDEWorkspaceRoots method.

The ConnectedAppService can maintain a set of VmService objects for the VM service URIs that have been registered, which will allow the service to automatically clean up VmService instances when they are shutdown.

[OPTIONAL] Instead of (or in addition to) getVmServiceUris(), we could implement a stream for the VM Service URIs that DTD clients can subscribe to. Upon a client subscribing to this stream, all the current VM Service URIs should be sent. The stream should be updated with vmServiceRegistered and vmServiceShutdown events so that the client can update its set of URIs. This may not be necessary since the client can just request getVmServiceUris() whenever an operation needs to know about the current set of VM service instances.

Implementation plan

  1. Build the ConnectedAppService in DTD. CC @bkonyi
    • [x] Done - https://dart-review.googlesource.com/c/sdk/+/427568
  2. [DDS] For the case of an app started from the CLI (dart or flutter), call the new method ConnectedAppService.registerVmService. This should be called whenever DDS (or DevTools server inside DDS) is responsible for starting DTD.
    • [x] DDS - https://dart-review.googlesource.com/c/sdk/+/429760
    • [x] DevTools server - https://github.com/flutter/devtools/pull/9211
  3. [Dart-Code extension] Call the new method ConnectedAppService.registerVmService when a new debug session is started. CC @DanTup
    • [x] https://github.com/Dart-Code/Dart-Code/issues/5526
  4. [IntelliJ Plugin] Call the new method ConnectedAppService.registerVmService when a new debug session is started. CC @jwren @pq
    • [ ] https://github.com/flutter/flutter-intellij/issues/8261
  5. In the Dart Tooling MCP Server (the first DTD client that will be using this new service), initially call ConnectedAppService.getVmServiceUris only when the EditorService.getDebugSessions method is not available. This will ensure that the Dart Tooling MCP Server is backwards compatible with older IDE plugin / extension versions. After some safe time period (12-18 months?), switch over to using ConnectedAppService.getVmServiceUris exclusively. CC @jakemac53

kenzieschmoll avatar Apr 15 '25 18:04 kenzieschmoll

Which thing will be in charge of calling registerVmService for the standalone flutter/dart run case?

jakemac53 avatar Apr 15 '25 20:04 jakemac53

Which thing will be in charge of calling registerVmService for the standalone flutter/dart run case?

Probably DDS, since it's responsible for starting DTD when we're running from the CLI.

bkonyi avatar Apr 15 '25 20:04 bkonyi

@kenzieschmoll what's the priority of this work? The services team is already committed to a bunch of ongoing efforts so I'd like to figure out if we need to reprioritize anything.

bkonyi avatar Apr 15 '25 20:04 bkonyi

Probably DDS, since it's responsible for starting DTD when we're running from the CLI.

SGTM, @kenzieschmoll can you add that to the implementation plan?

jakemac53 avatar Apr 15 '25 20:04 jakemac53

@kenzieschmoll what's the priority of this work? The services team is already committed to a bunch of ongoing efforts so I'd like to figure out if we need to reprioritize anything.

We are still determining the priority here. I would consider this P2 for now, but we will know more over the coming weeks WRT whether the priority should be raised.

Probably DDS, since it's responsible for starting DTD when we're running from the CLI.

SGTM, [@kenzieschmoll](https://github.com/kenzieschmoll) can you add that to the implementation plan?

Done.

kenzieschmoll avatar Apr 15 '25 20:04 kenzieschmoll

In the Dart Tooling MCP Server (the first DTD client that will be using this new service), initially call ConnectedAppService.getVmServiceUris only when the EditorService.getDebugSessions method is not available.

Should this be the other way? When IDEs provide both services, wouldn't it be better to always use the new one (so that the behaviour doesn't (potentially) change at the time that use of getDebugSessions is removed?).

btw - who is starting the MCP server and when? (I'm wondering if there may be any race here because the getDebugSessions service is registered by the IDE - is it possible the MCP server checks for it just before it is registered?)

DanTup avatar Apr 16 '25 14:04 DanTup

Should this be the other way? When IDEs provide both services, wouldn't it be better to always use the new one (so that the behaviour doesn't (potentially) change at the time that use of getDebugSessions is removed?).

The problem I think is that ConnectedAppService.getVmServiceUris may exist, but the IDE might not be supporting it yet. We could do something where we only try Editor.getDebugSessions if there are no VM service uris registered though.

who is starting the MCP server and when

The clients (cursor, Gemini code assist, copilot, etc) will be starting it - so this is dependent on the situation. But the MCP server doesn't connect to anything until the user actually tries to take an action that requires it, at which point they are prompted for a DTD Uri, which should have been alive for some time already. We can also just listen for it to get registered.

jakemac53 avatar Apr 16 '25 14:04 jakemac53

The problem I think is that ConnectedAppService.getVmServiceUris may exist, but the IDE might not be supporting it yet.

Oh yeah, I was thinking about it being registered by the IDE.

Maybe one option would be to only register getVmServiceUris when the first VM Service is recorded (and then the server can just use whichever shows up first, or if both are available, always prefer the new one). I don't know if it's worth the extra complexity though.

But the MCP server doesn't connect to anything until the user actually tries to take an action that requires it, at which point they are prompted for a DTD Uri, which should have been alive for some time already.

Thanks - sounds like that's a non-issue then :-)

DanTup avatar Apr 16 '25 14:04 DanTup

An initial implementation of this service is here: https://dart-review.googlesource.com/c/sdk/+/427568. The service registers a single method registerVmService and it posts VmServiceRegistered and VmServiceUnregistered events on the ConnectedApp stream. VmServiceRegistered events are sent when the registerVmService method is called and VmServiceUnregistered events are sent when the VmService.onDone handler is called.

@DanTup are there edge cases here that this logic doesn't cover? One I thought of was if the IDE loses connection to the connected app. In that case the VM service would still be alive, and so the object on DTD would remain alive even though the IDE wouldn't have a connection to it.

Another case to think about is when DTD needs to restart but the apps in the IDE remain alive. In that case, the IDE should probably call the registerVmService methods again on the new instance of DTD.

kenzieschmoll avatar May 09 '25 01:05 kenzieschmoll

@DanTup are there edge cases here that this logic doesn't cover?

Apologies - I only just spotted this notification.

One I thought of was if the IDE loses connection to the connected app. In that case the VM service would still be alive, and so the object on DTD would remain alive even though the IDE wouldn't have a connection to it.

I think generally there shouldn't be any cases where VS Code/DAP loses a connection to the VM Service without it shutting down. However, there is one way I could imagine this could be triggered fairly easily, and that's by hitting the "Stop" button on the debug toolbar twice quickly. The first click of Stop will send a request for the debug adapter to shut down gracefully, but a second click will tell it to immediately exit. That second click can easily leave orphaned processes and running apps - but generally I don't think users would trigger this unless we're not responding quickly for some reason (and while it might not be obvious to them, it always has the possibility of leaving orphaned processes, not only for Dart/Flutter).

I'm not completely familiar with who the users of this API are, so I guess one questions is what would we want here. Do we want DTD to have "the set of VM Services the IDE is connected to", or "the set of VM Services that are known about and are still running, regardless of whether the IDE is still connected"?

Another case to think about is when DTD needs to restart but the apps in the IDE remain alive

What to you mean by "DTD needs to restart"? Right now in VS Code we don't support restarting individual services (or letting them restart themselves!) so mostly if you want to restart something (like by using the "Restart Analysis Server" command), we actually restart everything. This way we don't need to ensure that every service/dependency can handle it's other dependencies restarting (for example DevTools server takes a DTD URI on the CLI, so restarting DTD would require the DevTools server be restarted anyway).

DanTup avatar May 13 '25 16:05 DanTup

if you want to restart something (like by using the "Restart Analysis Server" command), we actually restart everything

... and while I haven't test this, I expect it would terminate any in-progress debug sessions. In general, I hope it's very rare we have a need to restart in this way.

DanTup avatar May 13 '25 16:05 DanTup

In general, I hope it's very rare we have a need to restart in this way.

Fwiw, I do end up having to use this command more than I would like, at least today. Not every day but maybe weekly or so. I do think we should make sure we can handle it reasonably.

jakemac53 avatar May 13 '25 18:05 jakemac53

Fwiw, I do end up having to use this command more than I would like, at least today. Not every day but maybe weekly or so. I do think we should make sure we can handle it reasonably.

I think it could be a lot of effort across all tools to support individually restaring other components. I would really prefer we track down why you need to keep using it (or at least try and understand why we don't think we can fix it). If you use the command a few times in the same session, it pops up asking you to file a bug - but if you're only using it occasionally you probably don't see that.

I think it would be good to file a Dart-Code or SDK issue about exactly what you see that leads you to use the command and see if we can figure out what we can do to help track down the problem (for example when you invoke that command, maybe we could dump the recent analyzer traffic and other relevant info to a log file automatically).

DanTup avatar May 13 '25 19:05 DanTup

(for example when you invoke that command, maybe we could dump the recent analyzer traffic and other relevant info to a log file automatically).

I think something like this is a requirement for us to get any actual bugs or anything filed - it's not something reproducible. We have to make it be a lot lower friction and something you can do after the fact (or that is automatic).

jakemac53 avatar May 13 '25 20:05 jakemac53

@jakemac53 makes sense - I've filed https://github.com/Dart-Code/Dart-Code/issues/5513 to start recording something, and if it's not enough we can try to iterate on it until it is.

DanTup avatar May 14 '25 10:05 DanTup

@kenzieschmoll I saw your CL for this landed - is this complete enough that the IDE work should be started?

DanTup avatar May 22 '25 09:05 DanTup

I just found this thread.

For DTD clients like DevTools and the new Dart Tooling MCP Server

The link above is invalid: https://github.com/dart-lang/ai/tree/main/pkgs/dart_tooling_mcp_server

Did you mean https://github.com/dart-lang/ai/tree/main/pkgs/dart_mcp_server?

FMorschel avatar May 22 '25 17:05 FMorschel

[@kenzieschmoll](https://github.com/kenzieschmoll) I saw your CL for this landed - is this complete enough that the IDE work should be started?

I have one more change to make that will change the response type of the ConnectedApp.getVmServiceUris method, so as long as you are just using the register / unregister methods, yes you can add the VS code changes. Here's an in-progress example of using this API from DDS: https://dart-review.googlesource.com/c/sdk/+/429760. I also have a change on a local branch to use this API from DevTools server. dtd 3.0.0 is not yet published but will be soon.

kenzieschmoll avatar May 22 '25 18:05 kenzieschmoll

Did you mean https://github.com/dart-lang/ai/tree/main/pkgs/dart_mcp_server?

Yes, the package has since been renamed. I edited the original issue description. Thanks!

kenzieschmoll avatar May 22 '25 18:05 kenzieschmoll

Closing this issue as complete since the only remaining work is a DTD client implementation tracked separately at https://github.com/flutter/flutter-intellij/issues/8261.

kenzieschmoll avatar Jul 07 '25 21:07 kenzieschmoll