promptStop should cancel already executing tool call futures

[bombaywalla]

To upvote this issue, give it a thumbs up. See this list for the most upvoted issues.

When a chat/promptStop is executed while a tool call is executing in its own future, we currently get an "Invalid state transition" (by design). ECA then waits for the future to complete. It would be good if the future could be cancelled - especially if the tool call is long-lived. However, it may be challenging since the future thread has to cooperate with the cancellation process. And we don't always have control over the implementation.

Steps to reproduce the behavior (in eca-emacs):

1. Send a prompt like Please run the bash tool with a sleep of 10 secs followed by a 'ls'..
2. Click on accept to allow the tool.
3. Click on stop, to stop the prompt, while the tool is still running.
4. Check the <eca:stderr:1> buffer. You should see an "Invalid state transition" exception.

What we'd like in this issue is just to be able to cancel the future - ideally in a clean way, but okay if not clean.
Secondarily, we want to allow the user to interrupt ECA from hanging on a long-lived call, but see #77 on background tasks and long-lived processes.
Minimally, get rid of the invalid state transition exception.

[bombaywalla]

Are you okay with storing the futures in the tool call state?

[ericdallo]

@bombaywalla I guess so, we should just:

• make sure we clean it to avoid memory leak after the tool was executed
• make sure we don't serialize it to a file, but I believe we already do this, check here

[bombaywalla]

I checked the place you pointed me at and yes, we are dissocing all the tool calls before serializing to a file.

Do you want me to remove the whole tool call state after the tool finishes executing or is stopped? Or just the future?
Should the cleanup be done as soon at the tool call completes, or after the chat ends?

What are your thoughts about supporting "background" tool calls, as suggested in #77 ?
If we do that, we may need to keep some state around to note that the tool is running?
I'd prefer to come up with a solution to this issue that will be compatible with a "background" tool call if/when it is implemented.

Right now, I'm in the "hammock" phase :-) so just thinking about potential pros/cons of different approaches.

[ericdallo]

Do you want me to remove the whole tool call state after the tool finishes executing or is stopped? Or just the future?

I believe it's a good idea clean the whole toolCall atom.

Should the cleanup be done as soon at the tool call completes, or after the chat ends?

Cleaning after toolCall reaches its final state sounds good to me, but whatever is easy to evolve/maintain, cleaning on finish-chat sounds good too.

What are your thoughts about supporting "background" tool calls,

That's something I want to add to ECA carefully, background agents/subagents is a feature I wanna support but in a way easy to evolve/maintain, I agree we should solve this issue in a way easy to support #77 later, but I haven't spent too much thought on that.
Hammock is definitely a good idea for this one

[bombaywalla]

Is each ECA message (request or notification) run in its own thread asynchronously?
Or does the ECA server loop take each message, run it to completion, and then run the next one?

In my investigations, I came across the pending-request function in the lsp4clj.server namespace.
It has an interesting docstring.
I was wondering whether we need to use something like what is described there?

[ericdallo]

@bombaywalla yeah, lsp4clj should help with that, It's been years since I touched that code so I don't recall exactly, but not sure the thread that runs the request/notification is the issue here, but the future we spawn for tool calling here, WDYT?

[bombaywalla]

Thanks.

As of now, I see three main areas of concern:

1. How best to save the future in tool call, so that we have a handle on it. For example, we can always do it directly in code, but it would be more consistent if we made it an action in the state machine framework, since currently all the changes to the tool call state are managed through actions.
2. How best to clean up the future in the calling thread. And where. We have touched on this before.
3. How best to clean up the underlying tool call inside the future, when there is an interrupt. This is what I am currently investigating.

[ericdallo]

Sounds good @bombaywalla , I agree all three are valid concerns

[bombaywalla]

From a protocol and UX perspective, what do you think about a protocol message, toolCallCancel and a corresponding toolCallCancelled or toolCallStopped? That would allow some flexibility in the UX. For example, if a tool call takes more than (say) 20 secs, the editor client could ask the user "This call is taking too long. Would you like to abort the call?" and if the users says "yes" it could send a toolCallCancel message rather than a promptStop message. The former would just interrupt the one call, whereas the latter would kill the whole prompt, which may have multiple tool calls.