Add signal handling for graceful Docker shutdown

[Copilot]

• Analyze CI failure logs
• Identify root cause: artifact name contains forward slash from github.ref_name (2/merge)
• Fix artifact name to use github.sha instead of github.ref_name
• Update both upload and download artifact steps
• Reply to user comment with fix details (commit 1c93bc0)
• CI validation passing with artifact fix

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Docker container is very slow to stop</issue_title>
<issue_description>Stopping the docker container takes a long time - seems like there is something going wrong in the shutdown sequence.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Docker container is very slow to stop #1

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[rgregg]

@copilot can you look at why the pull_request validation failed.

[Copilot]

Pull request overview

This pull request adds graceful signal handling for Docker container shutdown to resolve slow container termination issues. Previously, the Docker container took 10+ seconds to stop because the application didn't handle SIGTERM signals, forcing Docker to wait for the timeout before sending SIGKILL.

Changes:

• Added run_with_shutdown() function in cli.py that registers SIGTERM/SIGINT signal handlers using asyncio's event loop
• Added comprehensive test coverage for signal handling in test_cli.py
• Replaced direct asyncio.run(proxy.run()) call with asyncio.run(run_with_shutdown(proxy)) and removed obsolete KeyboardInterrupt handling

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
src/meshcore_proxy/cli.py	Implements graceful shutdown mechanism with SIGTERM/SIGINT signal handlers and updates main() to use the new wrapper
tests/test_cli.py	Adds new test file with tests for SIGTERM and SIGINT signal handling with 5-second shutdown timeout verification

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Signal handlers are registered but never removed. Consider adding a finally block to remove the signal handlers using loop.remove_signal_handler() to ensure proper cleanup, especially if this function is called multiple times or if the function exits early due to an exception.

Suggested change

	for sig in (signal.SIGTERM, signal.SIGINT):
	loop.add_signal_handler(sig, functools.partial(signal_handler, sig))
	# Create the proxy task
	proxy_task = asyncio.create_task(proxy.run())
	# Wait for either the proxy to complete or a shutdown signal
	shutdown_task = asyncio.create_task(shutdown_event.wait())
	done, pending = await asyncio.wait(
	[proxy_task, shutdown_task],
	return_when=asyncio.FIRST_COMPLETED,
	)
	# If shutdown was signaled, cancel the proxy task
	if shutdown_task in done:
	proxy_task.cancel()
	try:
	await proxy_task
	except asyncio.CancelledError:
	pass
	# Cancel any remaining tasks
	for task in pending:
	task.cancel()
	try:
	await task
	except asyncio.CancelledError:
	pass
	signals = (signal.SIGTERM, signal.SIGINT)
	for sig in signals:
	loop.add_signal_handler(sig, functools.partial(signal_handler, sig))
	try:
	# Create the proxy task
	proxy_task = asyncio.create_task(proxy.run())
	# Wait for either the proxy to complete or a shutdown signal
	shutdown_task = asyncio.create_task(shutdown_event.wait())
	done, pending = await asyncio.wait(
	[proxy_task, shutdown_task],
	return_when=asyncio.FIRST_COMPLETED,
	)
	# If shutdown was signaled, cancel the proxy task
	if shutdown_task in done:
	proxy_task.cancel()
	try:
	await proxy_task
	except asyncio.CancelledError:
	pass
	# Cancel any remaining tasks
	for task in pending:
	task.cancel()
	try:
	await task
	except asyncio.CancelledError:
	pass
	finally:
	# Ensure signal handlers are removed even if an error occurs
	for sig in signals:
	try:
	loop.remove_signal_handler(sig)
	except RuntimeError:
	# Event loop may be closing; ignore cleanup errors
	pass

[Copilot]

The code at lines 142-148 that cancels pending tasks is unnecessary because when shutdown_task is done, the only pending task is shutdown_task itself (since proxy_task is already being awaited), and when proxy_task completes naturally, shutdown_task would be in pending and will never complete. This creates confusing logic. The pending task cleanup can be removed or simplified.

[Copilot]

The signal_task is created but the test doesn't verify that it actually completes before sending the signal. If run_with_shutdown() exits before the signal is sent (due to other issues), the test might pass incorrectly. Consider using asyncio.gather() or ensure both tasks complete as expected.

[Copilot]

Same issue as in the SIGTERM test - the signal_task should be properly awaited or gathered to ensure both tasks complete as expected.

[Copilot]

'except' clause does nothing but pass and there is no explanatory comment.

[Copilot]

'except' clause does nothing but pass and there is no explanatory comment.

[Copilot]

@copilot can you look at why the pull_request validation failed.

The PR validation failed because the artifact name contained a forward slash (meshcore-proxy-2/merge) which is not allowed in GitHub artifact names.

Fixed in commit 1c93bc0 by replacing github.ref_name with github.sha for artifact naming, which avoids the forward slash in PR merge refs.