From e738011e3afbbcf9fc38f4fdb8af55b87726ebf9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 09:26:11 +0000 Subject: [PATCH] fix(compose): ps_status recognises all daemon-unreachable phrasings & can't hang (#178) ps_status previously (1) detected an unreachable daemon by scraping stderr for only the exact substring "cannot connect to the docker daemon", so the "failed to connect to the docker API at ..." phrasing (custom/rootless DOCKER_HOST socket) fell through and was misreported as "not-created"; and (2) ran `docker compose ps` with no timeout, so a wedged-but-reachable daemon or an unresponsive TCP DOCKER_HOST hung ls/status/doctor forever. Now it matches a family of daemon-unreachable stderr markers and runs the probe under a bounded 20s timeout (mirroring builder._DOCKER_INFO_TIMEOUT), catching subprocess.TimeoutExpired and degrading to "docker-unreachable". Scoped to the read-only ps_status probe; lifecycle/streaming verbs are unchanged. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WmChW67jv2BaPk1a8HreHU --- CHANGELOG.md | 1 + src/beetroot/compose.py | 38 ++++++++++++++++++++++++++++++-------- tests/test_compose.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efc3fe3..b02e3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -630,6 +630,7 @@ are absent), so shell regressions are caught locally before the push. ### Bug fixes +- **`compose.ps_status` now recognises every daemon-unreachable phrasing and can't hang (#178).** It maps both `cannot connect to the docker daemon` and `failed to connect to the docker API at ...` (the custom/rootless `DOCKER_HOST` phrasing) to `docker-unreachable` instead of misreporting a live container as `not-created`, and it runs `docker compose ps` under a bounded 20 s timeout (`subprocess.TimeoutExpired` → `docker-unreachable`) so a wedged daemon or an unresponsive TCP `DOCKER_HOST` degrades gracefully rather than hanging `ls`/`status`/`doctor` forever. Scoped to the read-only `ps_status` probe; lifecycle/streaming verbs are unchanged. - **The Magisk denylist now hides root in the Play-Integrity/DroidGuard process (#170).** Denylist entries take a `package[/process]` shape, and `magisk-config.sh` inserts the real package into `package_name` and the process into `process` — never copying one value into both columns. The default enrolls `com.google.android.gms.unstable` (DroidGuard) as a **process** of the `com.google.android.gms` package instead of a bogus `package_name` that matched no installed app, so vanilla (non-Shamiko) Magisk actually hides root in the attestation process. The grammar validates both halves (SQL-injection prophylaxis), the health check matches enrolment by the package half, and the example configs demonstrate the corrected form. - **`registry._read()` no longer silently drops an instance row that fails row-level validation (#252).** A row with a known backend kind but a rejected payload is now preserved opaquely so its port index stays reserved and it round-trips; a row too broken to salvage (bad `created_at`, missing/non-int index) is surfaced loudly (backed up to `.bak` with a hint) instead of being dropped and having its index silently reused. - **`create`/`register` now refuse a directory that nests inside — or contains — another registered instance (#255).** Previously the overlap guard existed only for `restore`, so a nested `create` could later be wiped out by a `destroy` of the outer instance. The guard runs before any `mkdir`/registry write, so a refused operation is a no-op. diff --git a/src/beetroot/compose.py b/src/beetroot/compose.py index 9388fc2..7f9042e 100644 --- a/src/beetroot/compose.py +++ b/src/beetroot/compose.py @@ -20,11 +20,27 @@ import subprocess from collections.abc import Sequence from pathlib import Path -from typing import Literal +from typing import Final, Literal from . import paths from .settings import settings +# Bounded deadline for the read-only ``docker compose ps`` probe. A wedged +# (reachable-but-unresponsive) daemon or an unresponsive TCP ``DOCKER_HOST`` +# would otherwise make ``ps_status`` — and every verb that reads it (``ls``, +# ``status``, ``doctor``) — hang forever. Mirrors ``builder._DOCKER_INFO_TIMEOUT``. +_PS_STATUS_TIMEOUT: Final[int] = 20 + +# Lowercased stderr markers the Docker CLI emits when the daemon is +# unreachable. The CLI uses more than one phrasing — ``cannot connect to the +# docker daemon`` (default socket) and ``failed to connect to the docker API +# at ...`` (custom/rootless socket via ``DOCKER_HOST``) — so we match a +# family of substrings rather than a single exact string. +_DAEMON_UNREACHABLE_MARKERS: Final[tuple[str, ...]] = ( + "cannot connect to the docker daemon", + "failed to connect to the docker", +) + # Closed enum of the strings ``ps_status`` may return. The compose # subcommand reports a free-form ``State`` field; this enum gates every # string we ever surface to callers so verbs (``doctor``, ``status``) @@ -201,7 +217,9 @@ def ps_status(name: str, instance_root: Path) -> ComposeStatus: Queries ``docker compose ps --format json`` live; never reads from cache. Distinguishes "docker daemon unreachable" from "not-created" so callers (``beetroot doctor``, ``ls --json``) can give a precise - diagnostic. + diagnostic. The probe is bounded by :data:`_PS_STATUS_TIMEOUT`: a + wedged-but-reachable daemon (or an unresponsive TCP ``DOCKER_HOST``) + degrades to ``"docker-unreachable"`` instead of hanging the verb. Args: name: Instance name. @@ -220,17 +238,21 @@ def ps_status(name: str, instance_root: Path) -> ComposeStatus: ["ps", "--format", "json"], capture_output=True, text=True, + timeout=_PS_STATUS_TIMEOUT, ) - except ComposeError: - # ``_ensure_docker`` raised — docker binary not on PATH. + except (ComposeError, subprocess.TimeoutExpired): + # ``ComposeError``: ``_ensure_docker`` raised — docker binary not on + # PATH. ``TimeoutExpired``: a reachable-but-wedged daemon, or a TCP + # ``DOCKER_HOST`` that accepts the connection but never answers. Both + # degrade gracefully rather than block ``ls``/``status``/``doctor``. return "docker-unreachable" if res.returncode != 0: # Non-zero is most commonly "no such project" → not created. - # We could distinguish "daemon not running" via stderr scraping, - # but that's fragile; map to docker-unreachable only when - # compose stderr explicitly reports it. + # A daemon-unreachable failure is distinguished by scraping stderr + # for any of the CLI's connection-failure phrasings; everything + # else is treated as a missing project. stderr = (res.stderr or "").lower() - if "cannot connect to the docker daemon" in stderr: + if any(marker in stderr for marker in _DAEMON_UNREACHABLE_MARKERS): return "docker-unreachable" return "not-created" if not res.stdout.strip(): diff --git a/tests/test_compose.py b/tests/test_compose.py index 81acf6b..fe9b055 100644 --- a/tests/test_compose.py +++ b/tests/test_compose.py @@ -227,6 +227,35 @@ def test_docker_binary_missing_returns_docker_unreachable( monkeypatch.setattr(shutil, "which", lambda name: None) assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + def test_failed_to_connect_api_phrasing_is_docker_unreachable(self, tmp_path: Path) -> None: + # #178: a custom/rootless socket via DOCKER_HOST fails with the + # "failed to connect to the docker API at ..." phrasing, which lacks + # the "cannot connect to the docker daemon" substring. It must still + # map to ``docker-unreachable`` (not the misleading ``not-created``). + stderr = ( + "failed to connect to the docker API at unix:///tmp/missing.sock; " + "check if the path is correct and if the daemon is running" + ) + res = subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr=stderr) + with patch("subprocess.run", return_value=res): + assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + + def test_timeout_returns_docker_unreachable(self, tmp_path: Path) -> None: + # #178: a reachable-but-wedged daemon (or an unresponsive TCP + # DOCKER_HOST) must degrade to ``docker-unreachable`` rather than + # hang the verb forever. + with patch( + "subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd="docker compose ps", timeout=1), + ): + assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + + def test_ps_status_passes_bounded_timeout(self, tmp_path: Path) -> None: + # The read-only probe must be bounded so ls/status/doctor can't hang. + with patch("subprocess.run", return_value=_ok_result("")) as mock_run: + compose.ps_status("alpha", tmp_path) + assert mock_run.call_args.kwargs["timeout"] == compose._PS_STATUS_TIMEOUT + class TestComposeError: def test_is_runtime_error(self) -> None: