Fix _is_service_healthy nil-pointer crash for containers without a healthcheck

[Copilot]

Containers without a configured healthcheck have .State.Health == nil. The previous docker inspect template {{.State.Health.Status}} dereferenced this directly, causing Docker to exit 1 with nil pointer evaluating *types.Health.Status. The orchestrator treated every non-zero exit as "container not found" and kept retrying until the 120 s timeout.

Changes

• docker_orchestrator.py — Replace the flat template with a conditional that guards the nil dereference:

  {{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}\t{{.State.Status}}
  

  Update the healthy-status check to accept "none" (emitted by the new template) instead of "<no value>" (the Go template zero-value string, which was never actually reachable given the crash).

• tests/test_docker_orchestrator.py — Update mocked docker inspect output in the no-healthcheck test cases from "<no value>\t..." to "none\t..." to match what the new template actually emits.

Original prompt

The PR #50 is failing CI due to a timeout in the docker_orchestrator.py _is_service_healthy method. The current implementation tries to extract {{.State.Health.Status}} via docker inspect. However, if a container does not have a healthcheck, this results in an error (nil pointer evaluating *types.Health.Status) and the docker inspect command exits with code 1, which the orchestrator incorrectly ignores as "Container not found" and keeps retrying until timeout.

Update _is_service_healthy in shield-claw/src/shieldclaw/sandbox/docker_orchestrator.py to use a safe Go template formatting string that checks for Health before accessing Status:
"{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}\t{{.State.Status}}"
And update the comparison logic below it to recognize "none" as passing, instead of "<no value>".

Review the _is_service_healthy function specifically to ensure the error handling logic is sound.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for failing job 74156731084. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: Based on the logs and the implementation of _is_service_healthy in src/shieldclaw/sandbox/docker_orchestrator.py, the Timed out after 120.0 seconds waiting for compose services to start. error is caused by a template evaluation error in the docker inspect command.

When a Docker container does not have a healthcheck configured, its .State.Health field is nil. Evaluating {{.State.Health.Status}} directly causes a template parsing error (nil pointer evaluating *types.Health.Status), which results in a non-zero exit code. The script mistakenly catches this non-zero exit code as a missing container and moves on, keeping the orchestrator looping until it times out.

To fix this, the format string should gracefully check for the existence of .State.Health before trying to read .Status.

Suggested Fix

Update _is_service_healthy in src/shieldclaw/sandbox/docker_orchestrator.py as follows:

    def _is_service_healthy(self, service_name: str, project: str, cwd: Path) -> bool:
        """Return True when a service is running and its healthcheck is passing or absent."""
        for sep in ("-", "_"):
            container = f"{project}{sep}{service_name}{sep}1"
            cmd = [
                "docker",
                "inspect",
                "--format",
                "{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}\t{{.State.Status}}",
                container,
            ]
            _LOG.debug("Running command: %s", cmd)
            try:
                result = subprocess.run(
                    cmd,
                    capture_output=True,
                    text=True,
                    timeout=_DOCKER_INFO_TIMEOUT,
                    cwd=str(cwd),
                )
            except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
                return False
            if result.returncode != 0:
                continue  # Container not found under this naming form; try the other.
            output = result.stdout.strip()
            if not output:
                continue
            health, _, status = output.partition("\t")
            if status.strip() != "running":
                return False
            # "none" means no healthcheck is configured — treat as passing.
            return health.strip() in ("healthy", "none", "")
        return False

This ensures docker inspect succeeds for standard containers without health checks, emitting none\trunning and correctly recognizing the container's status.

User: Review the complete codebase to make sure that's the optima lfix to be bale to merge the Pr with no checks failing

Assistant: I've taken a deep look at the codebase and reviewed the PR's contents to confirm the issue. The only place this logic is executed is in _is_service_healthy inside docker_orchestrator.py.

The fix is exactly what we identified: updating the Go template inside the docker inspect call to safely check if the container has a healthcheck state before attempting to read its status.

I have launched the Github Coding Agent in the background to automatically create a Pull Request against this repository with the optimal fix. You can view its progress and the created PR shortly, which will resolve the CI pipeline failures so PR #50 can be cleanly merged.

This pull request was created from Copilot chat.

[Copilot]

Pull request overview

Fixes a startup-time timeout in DockerOrchestrator._is_service_healthy when inspecting containers that have no configured Docker healthcheck (where .State.Health is nil), by making the docker inspect --format template nil-safe and updating tests accordingly.

Changes:

• Guard .State.Health.Status access in the docker inspect Go template and emit none when no healthcheck is configured.
• Update the “no healthcheck” healthy-status check from "<no value>" to "none".
• Adjust unit tests’ mocked docker inspect output to match the new template behavior.

Reviewed changes

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

File	Description
shield-claw/src/shieldclaw/sandbox/docker_orchestrator.py	Makes health/status inspection nil-safe and updates the accepted “no healthcheck” sentinel value.
shield-claw/tests/test_docker_orchestrator.py	Updates mocked docker inspect outputs to align with the new template (none\t...).

Comments suppressed due to low confidence (1)

shield-claw/src/shieldclaw/sandbox/docker_orchestrator.py:536

• docker inspect failures are currently treated as “container not found” unconditionally (if result.returncode != 0: continue). This can still mask real Docker errors (e.g., daemon unavailable, permission denied, template/format issues) and cause _wait_for_compose_ready to spin until timeout with no actionable error. Consider checking result.stderr for an actual “no such container/object” message before continuing; otherwise surface the error (raise SandboxStartError or at least log and return False early with the stderr detail).

                "{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}\t{{.State.Status}}",
                container,
            ]
            _LOG.debug("Running command: %s", cmd)
            try:

---

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