Skip to content

Conversation

@cristipufu
Copy link
Member

@cristipufu cristipufu commented Jul 3, 2025

Summary

This PR adds robust shutdown handling to the CLI runtime by catching interrupts, cancelling tasks, and ensuring cleanup runs before exit.

@cristipufu cristipufu requested review from Copilot and edis-uipath July 3, 2025 10:39
@cristipufu cristipufu self-assigned this Jul 3, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds robust shutdown handling to the CLI runtime by catching interrupts, cancelling tasks, and ensuring cleanup runs before exit.

  • Wraps execute logic in nested try/except/finally to handle KeyboardInterrupt, cancel tasks, and return None on user abort.
  • Moves cleanup (self.cleanup) and trace_provider.shutdown into the outer finally block.
  • Refactors _keep_alive loop to use asyncio.wait_for for cancellable sleep and logs cancellation.
Comments suppressed due to low confidence (3)

src/uipath_mcp/_cli/_runtime/_runtime.py:104

  • New graceful shutdown paths (interrupt handling, task cancellation, and cleanup) are introduced here but lack corresponding unit or integration tests. Consider adding tests that simulate KeyboardInterrupt and verify that tasks are cancelled and cleanup is called.
                try:

src/uipath_mcp/_cli/_runtime/_runtime.py:460

  • [nitpick] Defining on_keep_alive_response inside the loop causes a new function to be created on each iteration. Consider moving it outside the loop to reduce overhead and simplify the loop body.
                    async def on_keep_alive_response(

src/uipath_mcp/_cli/_runtime/_runtime.py:131

  • [nitpick] There are two separate KeyboardInterrupt handlers with different log messages. Consolidating them into one catch block would simplify the control flow and ensure a single consistent message.
        except KeyboardInterrupt:

done, pending = await asyncio.wait(
[run_task, cancel_task], return_when=asyncio.FIRST_COMPLETED
)
except KeyboardInterrupt:
Copy link

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inner except KeyboardInterrupt catches the interrupt and sets the cancellation event but does not re-raise or return, causing the flow to continue to output_result and potentially return an incomplete result. Consider re-raising the exception after setting the event or returning early to ensure the outer handler is invoked.

Copilot uses AI. Check for mistakes.
@cristipufu cristipufu merged commit 00e4554 into main Jul 3, 2025
7 checks passed
@cristipufu cristipufu deleted the fix/hosted_graceful_shutdown branch July 3, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant