From 96acac5c5fe466e8caf0f0c2ebdd604a9127b40e Mon Sep 17 00:00:00 2001 From: kimi Date: Thu, 19 Mar 2026 21:03:33 -0400 Subject: [PATCH] refactor: break up shell.py::run() into helpers Extract _build_run_env() and _exec_subprocess() from the 89-line run() method, reducing it to ~30 lines that validate and delegate. Fixes #539 Co-Authored-By: Claude Opus 4.6 --- src/infrastructure/hands/shell.py | 110 ++++++++++++++++-------------- 1 file changed, 60 insertions(+), 50 deletions(-) diff --git a/src/infrastructure/hands/shell.py b/src/infrastructure/hands/shell.py index c549377..0b28319 100644 --- a/src/infrastructure/hands/shell.py +++ b/src/infrastructure/hands/shell.py @@ -144,6 +144,60 @@ class ShellHand: return None + @staticmethod + def _build_run_env(env: dict | None) -> dict: + """Merge *env* overrides into the current process environment.""" + import os + + run_env = os.environ.copy() + if env: + run_env.update(env) + return run_env + + async def _exec_subprocess( + self, + command: str, + effective_timeout: int, + cwd: str | None, + run_env: dict, + start: float, + ) -> ShellResult: + """Launch *command*, enforce timeout, and return the result.""" + proc = await asyncio.create_subprocess_shell( + command, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + cwd=cwd, + env=run_env, + ) + + try: + stdout_bytes, stderr_bytes = await asyncio.wait_for( + proc.communicate(), timeout=effective_timeout + ) + except TimeoutError: + proc.kill() + await proc.wait() + logger.warning("Shell command timed out after %ds: %s", effective_timeout, command) + return ShellResult( + command=command, + success=False, + exit_code=-1, + error=f"Command timed out after {effective_timeout}s", + latency_ms=(time.time() - start) * 1000, + timed_out=True, + ) + + exit_code = proc.returncode if proc.returncode is not None else -1 + return ShellResult( + command=command, + success=exit_code == 0, + exit_code=exit_code, + stdout=stdout_bytes.decode("utf-8", errors="replace").strip(), + stderr=stderr_bytes.decode("utf-8", errors="replace").strip(), + latency_ms=(time.time() - start) * 1000, + ) + async def run( self, command: str, @@ -164,7 +218,6 @@ class ShellHand: """ start = time.time() - # Validate validation_error = self._validate_command(command) if validation_error: return ShellResult( @@ -174,64 +227,21 @@ class ShellHand: latency_ms=(time.time() - start) * 1000, ) - effective_timeout = timeout or self._default_timeout - cwd = working_dir or self._working_dir - try: - import os - - run_env = os.environ.copy() - if env: - run_env.update(env) - - proc = await asyncio.create_subprocess_shell( + return await self._exec_subprocess( command, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - cwd=cwd, - env=run_env, + effective_timeout=timeout or self._default_timeout, + cwd=working_dir or self._working_dir, + run_env=self._build_run_env(env), + start=start, ) - - try: - stdout_bytes, stderr_bytes = await asyncio.wait_for( - proc.communicate(), timeout=effective_timeout - ) - except TimeoutError: - proc.kill() - await proc.wait() - latency = (time.time() - start) * 1000 - logger.warning("Shell command timed out after %ds: %s", effective_timeout, command) - return ShellResult( - command=command, - success=False, - exit_code=-1, - error=f"Command timed out after {effective_timeout}s", - latency_ms=latency, - timed_out=True, - ) - - latency = (time.time() - start) * 1000 - exit_code = proc.returncode if proc.returncode is not None else -1 - stdout = stdout_bytes.decode("utf-8", errors="replace").strip() - stderr = stderr_bytes.decode("utf-8", errors="replace").strip() - - return ShellResult( - command=command, - success=exit_code == 0, - exit_code=exit_code, - stdout=stdout, - stderr=stderr, - latency_ms=latency, - ) - except Exception as exc: - latency = (time.time() - start) * 1000 logger.warning("Shell command failed: %s — %s", command, exc) return ShellResult( command=command, success=False, error=str(exc), - latency_ms=latency, + latency_ms=(time.time() - start) * 1000, ) def status(self) -> dict: