Skip to content

feat(docker): close Podman support gaps in doctor, setup, and runtime detection - #10621

Open
zjc-enigma wants to merge 1 commit into
NousResearch:mainfrom
zjc-enigma:fix/podman-support-gaps
Open

feat(docker): close Podman support gaps in doctor, setup, and runtime detection#10621
zjc-enigma wants to merge 1 commit into
NousResearch:mainfrom
zjc-enigma:fix/podman-support-gaps

Conversation

@zjc-enigma

@zjc-enigma zjc-enigma commented Apr 16, 2026

Copy link
Copy Markdown

Summary

Building on PR #10066 which added find_docker() with Podman fallback, this closes the remaining gaps where Podman users see incorrect diagnostics or Docker-only error messages.

  • tools/environments/docker.py: Add is_podman() and runtime_name() helpers; skip --storage-opt size= probe for Podman (unsupported); use runtime-aware error messages
  • hermes_cli/doctor.py: Use find_docker() instead of hardcoded shutil.which("docker") — recognizes Podman as a valid container runtime
  • hermes_cli/setup.py: Detect Podman when configuring the container backend, show install links for both Docker and Podman

Changes

tools/environments/docker.py

  • New is_podman(): cached check if resolved runtime is Podman (checks basename for "podman")
  • New runtime_name(): returns "Podman" or "Docker" for user-facing messages
  • _storage_opt_supported(): early-returns False for Podman (no per-container --storage-opt size=)
  • _ensure_docker_available(): error messages now mention both runtimes when neither found, use runtime_name() for runtime-specific messages

hermes_cli/doctor.py

  • Container runtime check now uses find_docker() to detect either Docker or Podman
  • Displays the correct runtime name in diagnostics
  • "not found" message now says "docker/podman" instead of just "docker"

hermes_cli/setup.py

  • Container backend setup uses find_docker() to detect either runtime
  • Shows Podman install link alongside Docker when neither is found

Tests

  • 6 new tests in test_docker_find.py: TestIsPodman (5 cases) + TestStorageOptPodman (1 case)
  • Updated 2 existing tests for new error message wording

Test plan

  • uv run pytest tests/tools/test_docker_find.py -v — 15/15 pass
  • uv run pytest tests/tools/test_docker_environment.py -v — 18/18 pass
  • uv run pytest tests/hermes_cli/test_doctor.py -v — 19/19 pass
  • Manual: run hermes doctor with only Podman installed — shows "podman (optional)" / "podman (daemon running)"
  • Manual: run hermes setup with only Podman — shows "Podman found: /usr/bin/podman"

🤖 Generated with Claude Code

…essages

Building on PR NousResearch#10066 which added find_docker() with Podman fallback,
this closes the remaining gaps where Podman users see incorrect diagnostics:

- Add is_podman() and runtime_name() helpers to docker.py for consistent
  runtime detection across the codebase
- hermes doctor: use find_docker() instead of hardcoded shutil.which("docker"),
  recognize Podman as a valid container runtime
- hermes setup: detect Podman when configuring the container backend,
  show install links for both Docker and Podman
- _storage_opt_supported(): early-return False for Podman (doesn't support
  per-container --storage-opt size=)
- Error messages: use runtime_name() for context-appropriate "Docker" vs
  "Podman" wording, mention both runtimes when neither is found

Tests: 6 new test cases (is_podman, runtime_name, storage_opt podman skip),
updated 2 existing tests for new error message wording.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have area/docker Docker image, Compose, packaging backend/docker Docker container execution comp/cli CLI entry point, hermes_cli/, setup wizard labels Apr 25, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for closing the remaining Podman diagnostics gap. Current main confirms the central premise: find_docker() already falls back to Podman (tools/environments/docker.py:270-303), while doctor and setup still hard-code Docker (hermes_cli/doctor.py:1462-1465, hermes_cli/setup.py:1238-1243).

Problems

  • The added doctor test covers only the Termux/no-runtime path. It does not exercise the new Podman-only find_docker() path or verify that doctor runs podman info.
  • A sibling diagnostic remains Docker-only in tools/terminal_tool.py:2810-2815: the check uses find_docker() but logs “Docker executable not found.”

Suggested changes

  • Add a mocked Podman-only doctor regression test that asserts both the displayed runtime and the executable used for info.
  • Align the terminal requirement-check error wording with the runtime-agnostic discovery behavior.

This is an automated hermes-sweeper review.

Comment thread hermes_cli/doctor.py
terminal_env = os.getenv("TERMINAL_ENV", "local")
try:
from tools.environments.docker import find_docker, runtime_name
container_exe = find_docker()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a regression test for this Podman-only path: make Docker unavailable, resolve Podman here, and assert doctor reports Podman and invokes the resolved executable for info. The added doctor test currently only covers the Termux case where both runtimes are absent.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging area/install-update Installer, updater, packaging, wheels, doctor backend/docker Docker container execution comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants