Skip to content

Conversation

federicociner
Copy link

@federicociner federicociner commented Aug 27, 2025

We recently had a situation in our agentic CLI tool where we a block of code that had called __aexit__ on an already stopped server and the running count dropped below 0 to -1.

However, when trying to debug and compare active/running vs. stopped servers, we were relying on the is_running property and we were incorrectly seeing stopped servers being classified as running servers. I believe this is because bool(-1) evaluates to True, so this PR simplifies the property condition to only consider a server as running if the _running_count is greater than 0.

@federicociner federicociner changed the title Update is_running property logic so that it never evaluates to True i… Update is_running property logic to prevent _running_count < 0 from evaluating to True Aug 27, 2025
@DouweM
Copy link
Collaborator

DouweM commented Aug 27, 2025

@federicociner What do you think about instead raising an error from __aexit__ if it's called when self._running_count == 0?

@federicociner
Copy link
Author

@DouweM yea that would work too! Would that be your preference here? We could actually do both.

@DouweM
Copy link
Collaborator

DouweM commented Aug 28, 2025

@federicociner Yeah that'd be my preference, so we couldn't get into this invalid state to begin with. Then we shouldn't need the is_running change anymore because it's guaranteed to never be decremented below 0.

@federicociner
Copy link
Author

@DouweM yup makes sense, I'll update the PR.

@federicociner federicociner force-pushed the update-mcp-is-running-logic branch from 374da84 to a0c5bfd Compare August 29, 2025 06:59
@federicociner federicociner changed the title Update is_running property logic to prevent _running_count < 0 from evaluating to True Raise error if MCP server __aexit__ is called when _running_count is already 0 Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants