Skip to content

fix: remove docker info check from container runtime detection#829

Merged
jesseturner21 merged 1 commit intomainfrom
fix/remove-docker-info-check
Apr 14, 2026
Merged

fix: remove docker info check from container runtime detection#829
jesseturner21 merged 1 commit intomainfrom
fix/remove-docker-info-check

Conversation

@jesseturner21
Copy link
Copy Markdown
Contributor

@jesseturner21 jesseturner21 commented Apr 13, 2026

Problem

During agentcore deploy, the "Check dependencies" preflight calls detectContainerRuntime() which runs docker info. This command communicates with the Docker daemon through the Unix socket (/var/run/docker.sock). If the user isn't in the docker group, the OS prompts for a password — a confusing experience since the container runtime check is non-blocking (deploy falls back to CodeBuild for container builds anyway).

The same issue affects agentcore dev with Container agents and users of Colima (which uses the standard docker CLI binary).

Options considered

  1. Remove docker info entirely — detect runtimes with which + --version only. If the daemon isn't running, the user finds out when they actually use it (docker build fails with a clear "Cannot connect to the Docker daemon" error).

  2. Keep docker info but suppress the password prompt — not possible. The OS-level polkit/sudo prompt fires before our code can react to the result.

  3. Keep docker info but catch the failure silently — same problem: the prompt appears before the command returns a result. We can't prevent it.

Solution

Option 1. Remove the docker info call from detectContainerRuntime() and rely on which + --version for binary detection. This matches the approach already used by detectContainerRuntimeSync() in src/lib/packaging/container.ts.

The tradeoff is losing the proactive "start Docker" hint when the daemon is down. But the docker build error message is clear enough, and avoiding a surprise password prompt is worth that tradeoff.

Changes

  • Remove docker info probe from detectContainerRuntime()
  • Remove notReadyRuntimes tracking (only populated by the docker info failure path)
  • Remove getStartHint() helper and START_HINTS constant (only used to format notReadyRuntimes messages)
  • Simplify requireContainerRuntime() to just throw "not found" if no runtime passes which + --version
  • Simplify container-dev-server.ts prepare() to just check if runtime is null

Test plan

  • Unit tests: 35/35 pass across detect.test.ts and container-dev-server.test.ts
  • Full test suite: 3295/3295 pass
  • TypeScript compilation: 0 errors
  • Pre-commit hooks: eslint, prettier, secretlint, typecheck all pass
  • Manual: detectContainerRuntime() returns Docker info via which + --version only (no password prompt)
  • Manual: agentcore dev -l with Container agent — full flow completes (build, start, run) with no password prompt
  • Manual: agentcore deploy with Container agent on a machine without docker group membership — verify no password prompt

detectContainerRuntime() called `docker info` to verify the daemon was
running.  This requires access to the Docker socket and triggers an OS
password prompt on machines where the user is not in the docker group.

The check provided no real value: deploy falls back to CodeBuild anyway,
and dev will fail with a clear error from `docker build` if the daemon
is down.  Remove the `docker info` probe and rely on `which` + `--version`
only, matching the approach already used by detectContainerRuntimeSync().

Also removes the now-unused START_HINTS constant, getStartHint() helper,
and notReadyRuntimes tracking.
@jesseturner21 jesseturner21 requested a review from a team April 13, 2026 19:10
@github-actions github-actions bot added the size/m PR size: M label Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.8.0.tgz

How to install

npm install https://github.com/aws/agentcore-cli/releases/download/pr-829-tarball/aws-agentcore-0.8.0.tgz

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 44.54% 7051 / 15828
🔵 Statements 44.01% 7480 / 16993
🔵 Functions 41.98% 1262 / 3006
🔵 Branches 43.52% 4734 / 10876
Generated in workflow #1736 for commit 4a00013 by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

@aidandaly24 aidandaly24 left a comment

Choose a reason for hiding this comment

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

this lgtm approved

@jesseturner21 jesseturner21 merged commit 6729eb2 into main Apr 14, 2026
26 checks passed
@jesseturner21 jesseturner21 deleted the fix/remove-docker-info-check branch April 14, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants