From 95a14bb8d2b663a4b7a72880e96e5b4ee86a96b2 Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 12 May 2026 10:13:56 -0400 Subject: [PATCH] style: pass explicit check= to every subprocess.run call Silences pylint W1510 / ruff PLW1510 across the codebase. The choice at each site reflects existing intent: - check=True where the caller implicitly trusts success (docker ps / network ls returning stdout, docker build, exec chown/chmod inside provisioners). - check=False where the caller inspects .returncode (race-retry on docker run, pipelock sidecar lifecycle, network plumbing, exec_claude propagating the session's exit code, best-effort cleanup paths). No behavior change; check= defaults to False so the False sites are semantically identical. --- claude_bottle/backend/docker/backend.py | 7 +++++++ claude_bottle/backend/docker/bottle.py | 2 +- claude_bottle/backend/docker/network.py | 5 ++++- claude_bottle/backend/docker/pipelock.py | 13 +++++++++---- claude_bottle/backend/docker/util.py | 3 +++ tests/_docker.py | 1 + tests/canaries/test_pipelock_image.py | 3 ++- tests/integration/test_dry_run_plan.py | 3 +++ tests/integration/test_orphan_cleanup.py | 5 +++-- tests/integration/test_pipelock_sidecar_smoke.py | 2 ++ tests/unit/test_cli_start_format.py | 1 + 11 files changed, 36 insertions(+), 9 deletions(-) diff --git a/claude_bottle/backend/docker/backend.py b/claude_bottle/backend/docker/backend.py index f7fb3f8..4acd985 100644 --- a/claude_bottle/backend/docker/backend.py +++ b/claude_bottle/backend/docker/backend.py @@ -50,6 +50,7 @@ def _force_remove_container(name: str) -> None: ["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) @@ -246,6 +247,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup capture_output=True, text=True, env=child_env, + check=False, ) if run_result.returncode == 0: return candidate @@ -314,6 +316,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ], capture_output=True, text=True, + check=True, ) containers = tuple(sorted( line for line in (cr.stdout or "").splitlines() if line @@ -330,6 +333,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ], capture_output=True, text=True, + check=True, ) networks = tuple(sorted( line for line in (nr.stdout or "").splitlines() if line @@ -347,6 +351,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) for name in plan.networks: info(f"removing network {name}") @@ -354,6 +359,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ["docker", "network", "rm", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) # --- List --- @@ -370,6 +376,7 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup ], capture_output=True, text=True, + check=True, ) containers = (result.stdout or "").strip() if not containers: diff --git a/claude_bottle/backend/docker/bottle.py b/claude_bottle/backend/docker/bottle.py index 2508a5e..d11e46f 100644 --- a/claude_bottle/backend/docker/bottle.py +++ b/claude_bottle/backend/docker/bottle.py @@ -36,7 +36,7 @@ class DockerBottle(Bottle): if tty: cmd.append("-it") cmd.extend([self.name, "claude", *full_argv]) - return subprocess.run(cmd).returncode + return subprocess.run(cmd, check=False).returncode def cp_in(self, host_path: str, container_path: str) -> None: subprocess.run( diff --git a/claude_bottle/backend/docker/network.py b/claude_bottle/backend/docker/network.py index f6d1e68..1d082d8 100644 --- a/claude_bottle/backend/docker/network.py +++ b/claude_bottle/backend/docker/network.py @@ -35,6 +35,7 @@ def network_exists(name: str) -> bool: ["docker", "network", "inspect", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode == 0 ) @@ -61,7 +62,7 @@ def _network_create_with_prefix(base: str, internal: bool) -> str: args.append("--internal") args.append(name) info(f"creating {kind} network {name}") - result = subprocess.run(args, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE) + result = subprocess.run(args, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, check=False) if result.returncode != 0: flag = " --internal" if internal else "" die(f"docker network create{flag} {name} failed") @@ -85,6 +86,7 @@ def network_attach(network: str, container: str) -> None: ["docker", "network", "connect", network, container], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) if result.returncode != 0: die(f"docker network connect {network} {container} failed") @@ -100,6 +102,7 @@ def network_remove(name: str) -> bool: ["docker", "network", "rm", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) if result.returncode != 0: warn(f"failed to remove network {name}; clean up with 'docker network rm {name}'") diff --git a/claude_bottle/backend/docker/pipelock.py b/claude_bottle/backend/docker/pipelock.py index 7b322a8..f2ab4be 100644 --- a/claude_bottle/backend/docker/pipelock.py +++ b/claude_bottle/backend/docker/pipelock.py @@ -65,32 +65,35 @@ class DockerPipelockProxy(PipelockProxy): "run", "--config", "/etc/pipelock.yaml", "--listen", f"0.0.0.0:{PIPELOCK_PORT}", ] - if subprocess.run(create_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL).returncode != 0: + if subprocess.run(create_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False).returncode != 0: die(f"failed to create pipelock sidecar {name}") cp_result = subprocess.run( ["docker", "cp", str(plan.yaml_path), f"{name}:/etc/pipelock.yaml"], capture_output=True, text=True, + check=False, ) if cp_result.returncode != 0: - subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) die(f"failed to copy pipelock yaml into {name}: {cp_result.stderr.strip()}") if subprocess.run( ["docker", "network", "connect", plan.egress_network, name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode != 0: - subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) die(f"failed to attach pipelock sidecar {name} to egress network {plan.egress_network}") if subprocess.run( ["docker", "start", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode != 0: - subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + subprocess.run(["docker", "rm", "-f", name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) die(f"failed to start pipelock sidecar {name}") return name @@ -102,11 +105,13 @@ class DockerPipelockProxy(PipelockProxy): ["docker", "inspect", proxy_target], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode == 0: if subprocess.run( ["docker", "rm", "-f", proxy_target], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode != 0: warn( f"failed to remove pipelock sidecar {proxy_target}; " diff --git a/claude_bottle/backend/docker/util.py b/claude_bottle/backend/docker/util.py index 94f23d4..1b65d46 100644 --- a/claude_bottle/backend/docker/util.py +++ b/claude_bottle/backend/docker/util.py @@ -33,6 +33,7 @@ def runsc_available() -> bool: ["docker", "info", "--format", "{{json .Runtimes}}"], capture_output=True, text=True, + check=False, ) return r.returncode == 0 and "runsc" in r.stdout @@ -58,6 +59,7 @@ def container_exists(name: str) -> bool: ["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"], capture_output=True, text=True, + check=True, ) return bool(result.stdout.strip()) @@ -129,4 +131,5 @@ def _silent_run(cmd: Iterable[str]) -> int: list(cmd), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode diff --git a/tests/_docker.py b/tests/_docker.py index deb0d39..e123ce4 100644 --- a/tests/_docker.py +++ b/tests/_docker.py @@ -15,6 +15,7 @@ def docker_available() -> bool: ["docker", "info"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ).returncode == 0 ) diff --git a/tests/canaries/test_pipelock_image.py b/tests/canaries/test_pipelock_image.py index 72a54cf..82b8e30 100644 --- a/tests/canaries/test_pipelock_image.py +++ b/tests/canaries/test_pipelock_image.py @@ -27,6 +27,7 @@ class TestPipelockImage(unittest.TestCase): ["docker", "pull", PIPELOCK_IMAGE], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) if result.returncode != 0: raise unittest.SkipTest(f"could not pull {PIPELOCK_IMAGE}") @@ -34,7 +35,7 @@ class TestPipelockImage(unittest.TestCase): def test_binary_runs(self): result = subprocess.run( ["docker", "run", "--rm", PIPELOCK_IMAGE, "--version"], - capture_output=True, text=True, + capture_output=True, text=True, check=False, ) out = result.stdout + result.stderr self.assertRegex(out, r"[Pp]ipelock|2\.[0-9]+\.[0-9]+") diff --git a/tests/integration/test_dry_run_plan.py b/tests/integration/test_dry_run_plan.py index 1672faa..fd8014f 100644 --- a/tests/integration/test_dry_run_plan.py +++ b/tests/integration/test_dry_run_plan.py @@ -43,6 +43,7 @@ class TestDryRunPlan(unittest.TestCase): env=env, capture_output=True, text=True, + check=False, ) self.assertEqual( 0, result.returncode, @@ -83,6 +84,7 @@ class TestDryRunPlan(unittest.TestCase): ["docker", "network", "ls", "--format", "{{.Name}}"], capture_output=True, text=True, + check=True, ) return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle")) @@ -91,6 +93,7 @@ class TestDryRunPlan(unittest.TestCase): ["docker", "ps", "-a", "--format", "{{.Names}}"], capture_output=True, text=True, + check=True, ) return sum(1 for n in result.stdout.splitlines() if n.startswith("claude-bottle")) diff --git a/tests/integration/test_orphan_cleanup.py b/tests/integration/test_orphan_cleanup.py index 928fbf3..4aff79b 100644 --- a/tests/integration/test_orphan_cleanup.py +++ b/tests/integration/test_orphan_cleanup.py @@ -34,6 +34,7 @@ class TestOrphanCleanup(unittest.TestCase): ["docker", "network", "rm", n], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) def test_remove_missing_is_noop(self): @@ -51,7 +52,7 @@ class TestOrphanCleanup(unittest.TestCase): nets = subprocess.run( ["docker", "network", "ls", "--format", "{{.Name}}"], - capture_output=True, text=True, + capture_output=True, text=True, check=True, ).stdout.splitlines() self.assertIn(self.internal_name, nets) self.assertIn(self.egress_name, nets) @@ -61,7 +62,7 @@ class TestOrphanCleanup(unittest.TestCase): nets_after = subprocess.run( ["docker", "network", "ls", "--format", "{{.Name}}"], - capture_output=True, text=True, + capture_output=True, text=True, check=True, ).stdout.splitlines() self.assertNotIn(self.internal_name, nets_after) self.assertNotIn(self.egress_name, nets_after) diff --git a/tests/integration/test_pipelock_sidecar_smoke.py b/tests/integration/test_pipelock_sidecar_smoke.py index a501ed9..682e61f 100644 --- a/tests/integration/test_pipelock_sidecar_smoke.py +++ b/tests/integration/test_pipelock_sidecar_smoke.py @@ -46,6 +46,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase): ["docker", "pull", CURL_IMAGE], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) if result.returncode != 0: raise unittest.SkipTest(f"could not pull {CURL_IMAGE}") @@ -104,6 +105,7 @@ class TestPipelockSidecarSmoke(unittest.TestCase): capture_output=True, text=True, timeout=60, + check=False, ) self.assertEqual( 0, probe.returncode, diff --git a/tests/unit/test_cli_start_format.py b/tests/unit/test_cli_start_format.py index e88f8d8..dd73e73 100644 --- a/tests/unit/test_cli_start_format.py +++ b/tests/unit/test_cli_start_format.py @@ -38,6 +38,7 @@ class TestStartFormatFlag(unittest.TestCase): env=env, capture_output=True, text=True, + check=False, ) self.assertNotEqual(0, result.returncode) self.assertIn(