Terminal chat fix

[klshuster]

Patch description
Fix for terminal chat, as raised in #3451. (Fixes #3451)

Basically, the terminal chat client would close a connection when a user sent the "[DONE]" message; however, this did not cleanup the executing future in the background. Thus, if one reached greater than max_workers connections, the terminal chat server would ignore further incoming connections.

Thus, we now have the terminal chat client send the exit message to close the overworld accordingly (as is handled here).

Testing steps
I changed the chatbot config to max_workers: 1, and verified that, after the fix, new incoming connections are allowed.

Logs

[JackUrb]

I had wondered why this was happening back when I was using this for LIGHT debug, thanks for taking this on! I'll admit though I'm not 100% on the details of how the solution works (or if you need the global RUNNING) - you're adding an early return condition to the on_message function, but this function doesn't seem to be a blocking or a looping function like _run is. Ultimately _run is the one that contains the loop, so is it not sufficient to just send the EXIT command you've added there?

[klshuster]

It's a bit nuanced... I'll outline what's going on in this comment (and, if you feel it's useful, I can add a comment in the code as well):

1. when a user is in the chatbot world, the only way to exit is by sending "[DONE]". That part is straightforward.
2. Upon messaging "[DONE]", the user is kicked back out to the overworld, and is sent the "Welcome to the overworld..." message.
3. We then send the "EXIT" string to the overworld, telling it to remove the agent. Only then is the agent finally removed.

Between steps 2 and 3, asynchronously (and nearly instantaneously), the websocket receives the "Welcome to the overworld..." message and displays it to the agent. I considered attempting to send "[DONE"] and "EXIT" back to the back and immediately closing the websocket, but I am not sure what the latency is there. So, I use the RUNNING variable to tell on_message to not bother showing the message if we're no longer going to serve messages to the user.

Does that make sense?

[stephenroller]

Lol amazing. Not in love with that global but let's move on

Add a comment at its defn saying what and why please

[klshuster]

i'm not exactly proud of it either...

as I've mentioned quite a bit, this whole setup needs a refactor.

in the meantime, i'll add a comment