Skip to content

Commit f56828a

Browse files
committed
Handle Qwen CLI refresh process cleanup
1 parent 7357848 commit f56828a

File tree

2 files changed

+92
-0
lines changed

2 files changed

+92
-0
lines changed

src/connectors/qwen_oauth.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,5 +905,21 @@ def __del__(self) -> None:
905905
with contextlib.suppress(Exception):
906906
self._stop_file_watching()
907907

908+
process = self._cli_refresh_process
909+
if process is not None:
910+
try:
911+
if process.poll() is None:
912+
process.terminate()
913+
try:
914+
process.wait(timeout=5)
915+
except subprocess.TimeoutExpired:
916+
process.kill()
917+
process.wait(timeout=5)
918+
except Exception:
919+
# Suppress all errors during cleanup to avoid issues at interpreter shutdown
920+
pass
921+
finally:
922+
self._cli_refresh_process = None
923+
908924

909925
backend_registry.register_backend("qwen-oauth", QwenOAuthConnector)
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""Unit tests for Qwen OAuth connector cleanup behavior."""
2+
3+
from __future__ import annotations
4+
5+
import subprocess
6+
from unittest.mock import AsyncMock, Mock, patch
7+
8+
import httpx
9+
import pytest
10+
11+
from src.connectors.qwen_oauth import QwenOAuthConnector
12+
from src.core.config.app_config import AppConfig
13+
14+
15+
@pytest.fixture
16+
def connector() -> QwenOAuthConnector:
17+
"""Create a QwenOAuthConnector instance for testing cleanup logic."""
18+
19+
mock_client = AsyncMock(spec=httpx.AsyncClient)
20+
return QwenOAuthConnector(mock_client, config=AppConfig())
21+
22+
23+
def test_cleanup_stops_file_watching_and_terminates_process(
24+
connector: QwenOAuthConnector,
25+
) -> None:
26+
"""Connector cleanup should stop file watching and terminate CLI refresh process."""
27+
28+
mock_process = Mock()
29+
mock_process.poll.return_value = None
30+
connector._cli_refresh_process = mock_process
31+
32+
with patch.object(connector, "_stop_file_watching") as mock_stop:
33+
connector.__del__()
34+
mock_stop.assert_called_once()
35+
36+
mock_process.terminate.assert_called_once()
37+
assert mock_process.wait.call_count >= 1
38+
assert connector._cli_refresh_process is None
39+
40+
41+
def test_cleanup_kills_hung_cli_refresh_process(
42+
connector: QwenOAuthConnector,
43+
) -> None:
44+
"""Connector cleanup should kill CLI process if terminate does not finish it."""
45+
46+
mock_process = Mock()
47+
mock_process.poll.return_value = None
48+
mock_process.wait.side_effect = [
49+
subprocess.TimeoutExpired(cmd="qwen", timeout=5),
50+
None,
51+
]
52+
connector._cli_refresh_process = mock_process
53+
54+
connector.__del__()
55+
56+
mock_process.terminate.assert_called_once()
57+
mock_process.kill.assert_called_once()
58+
assert mock_process.wait.call_count == 2
59+
assert connector._cli_refresh_process is None
60+
61+
62+
def test_cleanup_ignores_errors_during_shutdown(
63+
connector: QwenOAuthConnector,
64+
) -> None:
65+
"""Exceptions while stopping watchers or processes should be suppressed."""
66+
67+
connector._cli_refresh_process = Mock()
68+
connector._cli_refresh_process.poll.side_effect = RuntimeError("boom")
69+
70+
with patch.object(
71+
connector, "_stop_file_watching", side_effect=Exception("watch error")
72+
):
73+
connector.__del__()
74+
75+
# Process attribute should be cleared even when errors occur
76+
assert connector._cli_refresh_process is None

0 commit comments

Comments
 (0)