Skip to content

fix: honor selected Docker context for local API clients - #915

Merged
alongubkin merged 8 commits into
mainfrom
alon/alien-1296-docker-context
Oct 6, 2026
Merged

alongubkin merged 8 commits into
mainfrom
alon/alien-1296-docker-context

Conversation

@islo-labs

@islo-labs islo-labs Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Local Docker API clients used Bollard's local defaults, ignoring the persisted Docker context. With a nondefault Unix socket, API operations could reach a different daemon from Docker CLI operations.

The shared connector now resolves context and host settings for container, sandbox, and bridge clients. Image loading is pinned to the captured API endpoint. Invalid selected configuration and unexpected bridge API failures preserve their source errors. Unix sockets, platform-supported named pipes, and plaintext TCP are supported; TLS and SSH return actionable unsupported errors without downgrading security. Socket contexts ignore TLS verification flags as Docker CLI does, while TCP TLS remains rejected.

Selection matches Docker CLI source and installed behavior: DOCKER_HOST > DOCKER_CONTEXT > persisted currentContext. The synthetic default context honors DOCKER_HOST. Named contexts are inspected through Docker CLI rather than duplicating its metadata/TLS store. Native services using the default endpoint need no Docker CLI executable. No new dependencies are added.

Validation:

  • All 48 local library tests passed with nextest, including real Unix and TCP API requests, endpoint precedence, invalid metadata, TLS rejection, and explicit TCP port preservation.
  • A dedicated empty Docker engine on a nondefault Unix socket proved the old connector selected a different engine. The shared connector passed ping, version, empty-container listing, container-manager runtime status, sandbox-manager session listing, and optional bridge discovery through an isolated Docker config.
  • All 14 isolated fixture cases passed: persisted/explicit contexts, conflicting context/host variables with host-first precedence, host and synthetic-default overrides, malformed config, a missing context/socket, SSH, TLS, CLI pinning, baseline engine mismatch, and a cleared-environment host proof. Container, sandbox, and bridge consumers reject invalid selection consistently.
  • All 48 ordinary library tests also passed with Docker absent from PATH, using injected context-inspection fixtures. The five dedicated-fixture tests are explicitly ignored; real named-context proofs require Docker CLI.
  • With a cleared child environment, no HOME/USERPROFILE/DOCKER_CONFIG, a conflicting context, and no Docker executable, the valid explicit host reaches the dedicated engine through both managers and bridge discovery. The pre-fix implementation rejected this environment before connecting. Native optional default selection also passes without home/config/CLI.
  • A real Docker CLI subprocess reached the captured engine after the persisted context was changed to a missing context and the environment host conflicted.
  • Cargo checks passed for local tests and the audited manager test consumer. Changed Rust files pass formatting checks; workspace-wide formatting reports pre-existing differences in untouched files.

No application containers or builds are needed for the proof. The Docker registry integration test is explicitly ignored when its external prerequisites are absent and fails strictly when invoked.

References: Docker contexts, CLI context selection, context store layout, Bollard local connection.

@enclave-ai

enclave-ai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Enclave skipped this draft pull request. It will run automatically when you mark the PR as Ready for review.

@alongubkin
alongubkin marked this pull request as ready for review October 6, 2026 22:20
@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High risk] Refactors Docker connection logic across local platform.

Fix the unnecessary home-directory requirement before merging so stripped-down service environments can use valid endpoints.

Fix All in CodexFindings

  1. P1 Explicit hosts require a home ▶
  2. P2 Library tests require Docker CLI ▶
Fix with agent prompt
### Issue 1
crates/alien-local/src/docker_connection.rs:79-85
If a service has neither `HOME` (or `USERPROFILE`) nor `DOCKER_CONFIG`, `capture()` rejects even a valid `DOCKER_HOST` before checking it. Container and sandbox clients cannot start, and native Postgres also fails during optional bridge discovery even when Docker is not needed.

Read the host first and require a config directory only when selection needs it.

### Issue 2
crates/alien-local/src/docker_connection.rs:331-341
If `docker` is absent, ordinary library tests such as `selection_precedence_and_synthetic_default` fail because they unwrap named-context resolution that runs the real CLI. Default-endpoint users do not otherwise need that executable. These tests also depend on the installed CLI version.

Supply fixture inspect output for ordinary tests, and keep real CLI checks behind an explicit integration-test prerequisite.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR shares Docker endpoint selection across container, sandbox, and bridge clients. Image loading uses the endpoint captured by the container client.

  • Explicit hosts unnecessarily require a home-directory variable.
  • Ordinary library tests now require Docker CLI.
  • islo-labs[bot] states that invalid selected configuration and unexpected bridge API failures should propagate; those intentional changes were not reported.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  E[Capture environment] --> H{DOCKER_HOST set?}
  H -->|Yes| D[Default context with explicit host]
  H -->|No| C{DOCKER_CONTEXT set?}
  C -->|Yes| S[Selected context]
  C -->|No| P[Read persisted currentContext]
  P --> S
  S --> N{Named context?}
  N -->|Yes| I[Docker CLI context inspect]
  N -->|No| D
  I --> V[Check endpoint and TLS settings]
  D --> V
  V --> A[Create API client]
  A --> L[Pin docker load to captured host]
Loading

Reviews (1) · Last reviewed commit: "fix: match Docker CLI precedence and ins..."

Comment thread crates/alien-local/src/docker_connection.rs Outdated
Comment thread crates/alien-local/src/docker_connection.rs Outdated
@alongubkin
alongubkin merged commit ab2651e into main Oct 6, 2026
23 checks passed
@alongubkin
alongubkin deleted the alon/alien-1296-docker-context branch October 6, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant