Skip to content

fix(python): honor auto_restart on process exit#797

Closed
haosenwang1018 wants to merge 1 commit intogithub:mainfrom
haosenwang1018:fix/python-auto-restart
Closed

fix(python): honor auto_restart on process exit#797
haosenwang1018 wants to merge 1 commit intogithub:mainfrom
haosenwang1018:fix/python-auto-restart

Conversation

@haosenwang1018
Copy link

@haosenwang1018 haosenwang1018 commented Mar 12, 2026

Summary

  • route Python SDK requests through a stable request proxy so auto_restart is actually honored
  • reconnect once and retry the failed RPC when the underlying CLI process exits unexpectedly
  • keep existing CopilotSession objects wired to the refreshed transport after reconnect
  • add focused unit coverage for both the retrying and non-retrying paths

Testing

  • python -m ruff check python/copilot/client.py python/test_client.py
  • python -m pytest python/test_client.py -q

Closes #789.

@SteveSandersonMS
Copy link
Contributor

Thanks for propsing this.

However, this feature has never actually worked across any of the languages - there's non-implemented options for it on Python and Go, and then on TypeScript it tries to restart in certain cases but there are many cases where it won't work.

The feature dates back to the original spike of this SDK. I think we'd be better off removing it until we can make a more solid implementation, and of course only in reaction to real-world evidence of the need that can provide a clear case for what specifically should be supported.

I appreciate the PR you've added here makes it a little closer to working on Python, but it still won't work in a lot of cases (e.g., the CLI subprocess becomes frozen, or it was launched externally in the first place).

@SteveSandersonMS
Copy link
Contributor

Thanks for proposing this, @haosenwang1018.

On investigation, we realised that the "auto restart" feature was an artifact of the original spike implementation of this SDK and didn't really work in many cases (e.g., if the child process hangs or with cliUrl). Plus there wasn't even any implementation for it in the Go, Python, or .NET SDKs despite the option being present.

To clean this all up we decided to take a different approach, which is to remove the option since it doesn't really work (#803). Since there's no longer a feature here for the PR to be about, I hope you don't mind me closing the PR.

We'd love your contributions in other areas though if you find other things you'd like to fix. Thanks again!

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.

Python has an auto_restart argument for CopilotClient, but it is never used

2 participants