Skip to content

Commit 27977af

Browse files
committed
fix: detect shell exit via idle timeout instead of Code message
The platform API proxy never forwards ArgoCD Code messages and does not close the WebSocket when the remote shell exits. Replace the Code/reconnect logic with exit echo detection: when the server echoes back "exit\r\n", arm a 2-second idle timeout on ws.recv(). If no more data arrives, the shell has exited -- break out cleanly. Also removes Code handling from _exec_session (markers already work).
1 parent 4265839 commit 27977af

2 files changed

Lines changed: 108 additions & 327 deletions

File tree

‎centml/cli/shell.py‎

Lines changed: 64 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import signal
99
import sys
1010
import termios
11-
import time
1211
import tty
1312
import urllib.parse
1413

@@ -216,72 +215,74 @@ def _resolve_pod(cclient, deployment_id, pod_name=None):
216215
return running_pods[0]
217216

218217

218+
_EXIT_IDLE_TIMEOUT = 2.0
219+
220+
219221
async def _forward_io(ws, screen, stream, shutdown):
220222
"""Bidirectional forwarding between local stdin/stdout and WebSocket.
221223
222224
Output flows through a pyte terminal emulator so that cursor
223225
addressing, line wrapping, and colors are rendered correctly
224226
regardless of the remote PTY dimensions.
225227
228+
The platform API proxy does not close the WebSocket when the remote
229+
shell exits, so we detect the ``exit`` echo and use a short idle
230+
timeout to break out.
231+
226232
Args:
227233
ws: WebSocket connection.
228234
screen: pyte.Screen instance sized to the local terminal.
229235
stream: pyte.Stream attached to *screen*.
230236
shutdown: asyncio.Event set by signal handlers to request exit.
231237
232238
Returns:
233-
Tuple of (exit_code, should_reconnect). ``should_reconnect`` is True
234-
when the server sent a ``{"Code": ...}`` reconnect signal (ArgoCD
235-
token refresh), False on normal exit or connection close.
239+
The exit code (always 0 for interactive sessions).
236240
"""
237241
loop = asyncio.get_running_loop()
238-
exit_code = 0
239-
should_reconnect = False
240242
stdin_fd = sys.stdin.fileno()
241243
stdin_closed = asyncio.Event()
242244

243245
async def _read_ws():
244-
nonlocal exit_code, should_reconnect
245246
_log.debug("[read_ws] started")
246247
msg_count = 0
248+
recv_timeout = None
247249
try:
248-
async for raw_msg in ws:
250+
while True:
251+
try:
252+
raw_msg = await asyncio.wait_for(ws.recv(), timeout=recv_timeout)
253+
except asyncio.TimeoutError:
254+
_log.debug(
255+
"[read_ws] idle timeout (%.1fs) after %d msgs -- shell exited",
256+
recv_timeout, msg_count,
257+
)
258+
return
249259
msg_count += 1
250260
msg = json.loads(raw_msg)
251261
keys = list(msg.keys())
252-
data_snippet = ""
253-
if msg.get("data"):
254-
data_snippet = repr(msg["data"][:120])
262+
data = msg.get("data", "")
263+
data_snippet = repr(data[:120]) if data else ""
255264
_log.debug(
256265
"[read_ws] msg#%d keys=%s data=%s",
257266
msg_count, keys, data_snippet,
258267
)
259-
if msg.get("data"):
260-
# pyte expects \r\n; remote PTY may send bare \n
261-
# (same as xterm.js ``convertEol: true``).
262-
stream.feed(msg["data"].replace("\n", "\r\n"))
268+
if data:
269+
stream.feed(data.replace("\n", "\r\n"))
263270
_render_dirty(screen, sys.stdout.buffer)
271+
# Detect shell exit echo. When the user runs ``exit``,
272+
# the remote PTY echoes ``exit\r\n`` as the final
273+
# output. Switch to a short recv timeout so we exit
274+
# cleanly instead of hanging on the open WebSocket.
275+
if "exit\r\n" in data:
276+
_log.debug("[read_ws] detected exit echo, arming idle timeout")
277+
recv_timeout = _EXIT_IDLE_TIMEOUT
264278
elif msg.get("error"):
265279
_log.debug("[read_ws] error: %s", msg["error"])
266280
stream.feed(f"Error: {msg['error']}\r\n")
267281
_render_dirty(screen, sys.stdout.buffer)
268-
# ArgoCD sends {"Code": ...} as a reconnect signal (token
269-
# refresh), not a shell exit code. Mirror ArgoCD UI behavior:
270-
# disconnect and reconnect with a fresh token.
271-
if "Code" in msg:
272-
_log.debug(
273-
"[read_ws] got Code=%s, setting should_reconnect=True",
274-
msg["Code"],
275-
)
276-
should_reconnect = True
277-
return
278-
_log.debug("[read_ws] ws iterator exhausted after %d msgs", msg_count)
279282
except websockets.ConnectionClosed as exc:
280283
_log.debug(
281284
"[read_ws] ConnectionClosed after %d msgs: %s", msg_count, exc,
282285
)
283-
# Backend proxy may not send a clean close frame when
284-
# ArgoCD disconnects after the remote shell exits.
285286
return
286287

287288
async def _read_stdin():
@@ -352,23 +353,16 @@ async def _watch_shutdown():
352353
if t.exception() is not None:
353354
_log.debug("[forward_io] task exception: %s", t.exception())
354355
raise t.exception()
355-
_log.debug(
356-
"[forward_io] returning exit_code=%d should_reconnect=%s",
357-
exit_code, should_reconnect,
358-
)
359-
return (exit_code, should_reconnect)
356+
_log.debug("[forward_io] returning exit_code=0")
357+
return 0
360358

361359

362-
async def _interactive_session(ws_url, get_token_fn):
360+
async def _interactive_session(ws_url, token):
363361
"""Run an interactive terminal session over WebSocket.
364362
365-
Enters raw mode, forwards I/O bidirectionally, and restores terminal on
366-
exit. Reconnects automatically when the server sends a ``{"Code": ...}``
367-
token-refresh signal (matching ArgoCD UI behavior).
368-
369-
Args:
370-
ws_url: WebSocket URL for the terminal endpoint.
371-
get_token_fn: Callable that returns a fresh bearer token string.
363+
Enters raw mode, forwards I/O bidirectionally, and restores terminal
364+
on exit. SIGTERM and SIGHUP are caught to ensure terminal settings
365+
are always restored.
372366
"""
373367
fd = sys.stdin.fileno()
374368
old_settings = termios.tcgetattr(fd)
@@ -389,71 +383,40 @@ async def _interactive_session(ws_url, get_token_fn):
389383
loop.add_signal_handler(signal.SIGTERM, shutdown.set)
390384
loop.add_signal_handler(signal.SIGHUP, shutdown.set)
391385

392-
# _ws_ref holds the current websocket so SIGWINCH can reach it.
393-
_ws_ref = [None]
394-
395-
def _send_resize():
396-
c, r = shutil.get_terminal_size()
397-
screen.resize(r, c)
398-
screen.dirty.update(range(r))
399-
if _ws_ref[0] is not None:
386+
headers = {"Authorization": f"Bearer {token}"}
387+
_log.debug("[session] connecting to %s", ws_url)
388+
async with websockets.connect(
389+
ws_url, additional_headers=headers, close_timeout=2
390+
) as ws:
391+
_log.debug("[session] connected, sending resize %dx%d", cols, rows)
392+
393+
def _send_resize():
394+
c, r = shutil.get_terminal_size()
395+
screen.resize(r, c)
396+
screen.dirty.update(range(r))
400397
asyncio.ensure_future(
401-
_ws_ref[0].send(
398+
ws.send(
402399
json.dumps({"operation": "resize", "rows": r, "cols": c})
403400
)
404401
)
405402

406-
loop.add_signal_handler(signal.SIGWINCH, _send_resize)
403+
loop.add_signal_handler(signal.SIGWINCH, _send_resize)
407404

408-
try:
409-
last_code_time = -999.0
410-
attempt = 0
411-
while True:
412-
attempt += 1
413-
_log.debug("[session] attempt #%d, fetching token", attempt)
414-
token = get_token_fn()
415-
headers = {"Authorization": f"Bearer {token}"}
416-
_log.debug("[session] connecting to %s", ws_url)
417-
async with websockets.connect(
418-
ws_url, additional_headers=headers, close_timeout=2
419-
) as ws:
420-
_log.debug("[session] connected, sending resize %dx%d", cols, rows)
421-
_ws_ref[0] = ws
422-
await ws.send(
423-
json.dumps(
424-
{"operation": "resize", "rows": rows, "cols": cols}
425-
)
426-
)
427-
exit_code, should_reconnect = await _forward_io(
428-
ws, screen, stream, shutdown
429-
)
430-
_ws_ref[0] = None
431-
_log.debug(
432-
"[session] forward_io returned exit_code=%d reconnect=%s shutdown=%s",
433-
exit_code, should_reconnect, shutdown.is_set(),
434-
)
435-
if not should_reconnect or shutdown.is_set():
436-
_log.debug("[session] exiting with code %d", exit_code)
437-
return exit_code
438-
# If we got two Code signals within 3 seconds the shell
439-
# has genuinely exited (reconnect just opened a new
440-
# session that immediately closed). Stop reconnecting.
441-
now = time.monotonic()
442-
gap = now - last_code_time
443-
_log.debug(
444-
"[session] reconnect: gap=%.2fs last_code_time=%.2f now=%.2f",
445-
gap, last_code_time, now,
446-
)
447-
if gap < 3.0:
448-
_log.debug("[session] rapid Code, exiting with 0")
449-
return 0
450-
last_code_time = now
451-
_log.debug("[session] will reconnect")
452-
finally:
453-
loop.remove_signal_handler(signal.SIGWINCH)
454-
loop.remove_signal_handler(signal.SIGTERM)
455-
loop.remove_signal_handler(signal.SIGHUP)
405+
await ws.send(
406+
json.dumps(
407+
{"operation": "resize", "rows": rows, "cols": cols}
408+
)
409+
)
410+
try:
411+
exit_code = await _forward_io(ws, screen, stream, shutdown)
412+
finally:
413+
loop.remove_signal_handler(signal.SIGWINCH)
414+
415+
_log.debug("[session] exiting with code %d", exit_code)
416+
return exit_code
456417
finally:
418+
loop.remove_signal_handler(signal.SIGTERM)
419+
loop.remove_signal_handler(signal.SIGHUP)
457420
termios.tcsetattr(fd, termios.TCSADRAIN, old_settings)
458421
# Leave alternate screen buffer, restore cursor and attributes.
459422
sys.stdout.buffer.write(b"\033[?1049l\033[?25h\033[0m")
@@ -556,10 +519,6 @@ async def _exec_session(ws_url, token, command):
556519
_log.debug("[exec] error: %s", msg["error"])
557520
sys.stderr.write(f"Error: {msg['error']}\n")
558521
return 1
559-
if "Code" in msg and not is_done:
560-
_log.debug("[exec] Code=%s before done, returning 1", msg["Code"])
561-
sys.stderr.write("Connection interrupted, please retry.\n")
562-
return 1
563522
if is_done:
564523
_log.debug("[exec] done, breaking")
565524
break
@@ -593,7 +552,8 @@ def shell(deployment_id, pod, shell_type):
593552
ws_url = _build_ws_url(
594553
settings.CENTML_PLATFORM_API_URL, deployment_id, pod_name, shell_type
595554
)
596-
exit_code = asyncio.run(_interactive_session(ws_url, auth.get_centml_token))
555+
token = auth.get_centml_token()
556+
exit_code = asyncio.run(_interactive_session(ws_url, token))
597557
sys.exit(exit_code)
598558

599559

0 commit comments

Comments
 (0)