[containers] Mongo container creation never issues `docker pull`; relies on implicit pull by `docker create`

[JoshuaRowePhantom]

Summary

Mongo container creation never issues an explicit docker pull. MongoDbConnectionBroker.EnsureContainerStartedAsync relies on the implicit pull performed by docker create when an image is absent locally. Because the default image tag is the moving mongodb/mongodb-atlas-local:latest, an already-present-but-stale image is never refreshed, first-run pulls have no observable/diagnosable step, and pull failures surface only as a generic docker create failure.

Root Cause

Phantom.Workspaces.Data.MongoDB\MongoDbConnectionBroker.cs (lines ~153-169)

EnsureContainerStartedAsync tries StartAsync, and on InvalidOperationException calls CreateAsync then StartAsync again. There is NO pull step:

try
{
    await _containerEngine.StartAsync(connectionDefinition.ContainerName, cancellationToken).ConfigureAwait(false);
}
catch (InvalidOperationException)
{
    var containerDefinition = _containerDefinitionGenerator.Generate(connectionDefinition);
    await _containerEngine.CreateAsync(containerDefinition, cancellationToken).ConfigureAwait(false);
    await _containerEngine.StartAsync(connectionDefinition.ContainerName, cancellationToken).ConfigureAwait(false);
}

Phantom.Workspaces.Containers\WindowsDockerDesktopEngine.cs (lines ~42-99)

CreateAsync builds a docker create --name <name> --network <net> [-e ...] [--mount ...] [-p ...] <image> argument list and runs it via _commandRunner.RunAsync(...):

var arguments = new List<string>
{
    "create",
    "--name",
    definition.ContainerName,
    "--network",
    definition.NetworkType.ToString().ToLowerInvariant(),
};
// ... env vars, mounts, port mappings ...
arguments.Add(definition.ImageName);
await RunAndEnsureSuccessAsync(arguments, cancellationToken).ConfigureAwait(false);

StartAsync/StopAsync/DestroyAsync run docker start|stop|rm. No pull command exists anywhere in Phantom.Workspaces.Containers.

Phantom.Workspaces.Data.MongoDB\MongoDBContainerDefinitionGenerator.cs (line ~13)

The default image is a moving :latest tag:

public const string DefaultMongoImageName = "mongodb/mongodb-atlas-local:latest";

Why This Is a Bug (Impact)

1. docker create/docker start only pull an image when it is ABSENT locally. Because the tag is a moving :latest, an already-present-but-stale image is never refreshed -> users silently run an outdated Atlas Local image. An explicit docker pull is the standard way to fetch the newest :latest.
2. The implicit pull that does happen (first run, image absent) occurs inside docker create. Image pulls can take minutes; there is no dedicated, observable pull step, so first-run latency looks like a hang and pull progress/errors are buried. (A sibling bug about docker stdout/stderr not being logged was searched for among open issues but not found at filing time; cross-link if one is later created.)
3. Pull failures (auth, registry down, rate-limit) surface only as a generic docker create failure rather than a clear "could not pull image" diagnostic.

Affected Files

File	Role
Phantom.Workspaces.Data.MongoDB\MongoDbConnectionBroker.cs	EnsureContainerStartedAsync must issue a pull before create
Phantom.Workspaces.Containers\WindowsDockerDesktopEngine.cs	Add docker pull invocation
Phantom.Workspaces.Containers\ContainerEngine.cs / DockerDesktopEngine.cs (base abstraction)	Add PullAsync to the abstraction
Phantom.Workspaces.Data.MongoDB\MongoDBContainerDefinitionGenerator.cs	Supplies the :latest image name that must be pulled

Design / Fix

Recommended

Add an explicit image-pull step to the container engine and invoke it before create in the Mongo broker path.

• Add PullAsync(string imageName, CancellationToken cancellationToken) to the ContainerEngine abstraction (Phantom.Workspaces.Containers\ContainerEngine.cs, inherited by DockerDesktopEngine). Implement it in WindowsDockerDesktopEngine as docker pull <image> via _commandRunner.RunAsync(["pull", imageName], cancellationToken), surfacing failure the same way RunAndEnsureSuccessAsync does (throw InvalidOperationException with the failing command + stderr).
• In MongoDbConnectionBroker.EnsureContainerStartedAsync, before CreateAsync, call PullAsync(containerDefinition.ImageName, cancellationToken) so the newest :latest is fetched and the pull happens as its own observable step. Keep the create-only-on-start-failure structure (pull + create both run inside the catch (InvalidOperationException) block that follows a failed StartAsync).

Design decision: strict vs tolerant pull

Making pull best-effort avoids breaking offline users who already have a usable cached image. Recommended: attempt the pull, and on pull failure fall back to CreateAsync (which uses the cached image) rather than hard-failing. This keeps offline/registry-down scenarios working while still refreshing :latest whenever the registry is reachable. Mark the strict-vs-tolerant choice as an explicit design decision for review; a strict variant (fail loudly on pull error) is the alternative.

Considered / Background

• Do nothing and rely on docker create's implicit pull (current behavior) -- rejected because :latest is never refreshed and there is no observable/diagnosable pull step.

Expected Tests

Test style mirrors WindowsDockerDesktopEngineTests (uses a fake IDockerCommandRunner, RecordingDockerCommandRunner, that captures the argument list per invocation) and MongoDbConnectionBrokerDockerDesktopTests (uses a FakeDockerEngine : ContainerEngine tracking call counts). Naming style: Subject_Scenario_ExpectedOutcome.

Test Name	Class	What It Verifies
WindowsDockerDesktopEngine_PullAsync_RunsDockerPullWithImageName	WindowsDockerDesktopEngineTests	PullAsync("test/sleep:latest") issues a single docker pull test/sleep:latest invocation (Commands[0] equals ["pull", "test/sleep:latest"]).
WindowsDockerDesktopEngine_PullAsync_WhenDockerPullFails_ThrowsInvalidOperationException	WindowsDockerDesktopEngineTests	A non-zero ProcessResult from the runner causes PullAsync to throw InvalidOperationException including the failing command and stderr.
MongoDbConnectionBroker_WhenContainerMissing_PullsImageBeforeCreate	MongoDbConnectionBrokerDockerDesktopTests	When StartAsync first throws InvalidOperationException, the broker calls PullAsync before CreateAsync (assert ordering via the fake engine, e.g. recorded call sequence Pull -> Create -> Start).
MongoDbConnectionBroker_WhenPullFails_FallsBackToCreateUsingCachedImage	MongoDbConnectionBrokerDockerDesktopTests	Tolerant behavior: a failing PullAsync does not abort the flow; CreateAsync and StartAsync are still invoked (matches the recommended tolerant design decision).

If the strict variant is chosen instead, replace the last test with MongoDbConnectionBroker_WhenPullFails_PropagatesError verifying the pull failure surfaces and CreateAsync is not called.

[JoshuaRowePhantom]

Related: #1373 tracks logging docker stdout/stderr. That work makes the pull step added here observable (pull progress/errors surfaced instead of buried).

[JoshuaRowePhantom]

Design

Root cause: MongoDbConnectionBroker.EnsureContainerStartedAsync never issues docker pull; it relies on the implicit pull inside docker create, so a stale :latest is never refreshed and there is no observable pull step.

Approach:

1. Add PullAsync(string imageName, CancellationToken) to the ContainerEngine abstraction.
2. Implement in WindowsDockerDesktopEngine as docker pull <image> via _commandRunner.RunAsync(["pull", imageName], ...), throwing InvalidOperationException (failing command + stderr) on non-zero exit, mirroring RunAndEnsureSuccessAsync.
3. In EnsureContainerStartedAsync, inside the existing catch (InvalidOperationException) (failed StartAsync) block, call PullAsync(containerDefinition.ImageName, ...) before CreateAsync.
4. Design decision — tolerant pull (recommended): wrap PullAsync so a pull failure does not abort; fall back to CreateAsync using the cached image. Keeps offline/registry-down working while refreshing :latest when reachable.

Depends on #1373 for observability (logging makes pull progress/errors visible). Implemented on top of the #1373 commit in the same branch.

Files: ContainerEngine.cs, WindowsDockerDesktopEngine.cs, MongoDbConnectionBroker.cs, plus tests.

Tests: WindowsDockerDesktopEngine_PullAsync_RunsDockerPullWithImageName, WindowsDockerDesktopEngine_PullAsync_WhenDockerPullFails_ThrowsInvalidOperationException, MongoDbConnectionBroker_WhenContainerMissing_PullsImageBeforeCreate, MongoDbConnectionBroker_WhenPullFails_FallsBackToCreateUsingCachedImage.

[JoshuaRowePhantom]

Verification result

Status: ✅ Verified

Scope: Verifying #1374 only (commit 53442cfe). Sibling #1373 commits (30cc02e6, 614409f6, 8e1be665) are handled by a separate verifier.

Commits checked:
8e1be66 Fix #1373: keep gh auth token output at Debug, not Information
614409f Fix #1373: wire real docker logger into production MongoDb broker call sites
53442cf Fix #1374: pull docker image before creating the Mongo container
30cc02e Fix #1373: log docker stdout and stderr through a real logger

Requirements checked

1. ✅ PullAsync(string imageName, CancellationToken) added as an abstract member on the ContainerEngine abstraction (Phantom.Workspaces.Containers/ContainerEngine.cs) and implemented in WindowsDockerDesktopEngine as docker pull <image> via RunAndEnsureSuccessAsync(["pull", imageName], ...), surfacing failure as InvalidOperationException with the failing command + stderr.
2. ✅ MongoDbConnectionBroker.EnsureContainerStartedAsync calls PullAsync(containerDefinition.ImageName, ...) BEFORE CreateAsync, inside the existing catch (InvalidOperationException) block that follows a failed StartAsync — create-only-on-start-failure structure preserved.
3. ✅ TOLERANT variant: the PullAsync call is wrapped in its own try/catch (InvalidOperationException) that swallows the failure and proceeds to CreateAsync + StartAsync using the cached image. Offline/registry-down keeps working. (Not strict.)
4. ✅ All four described tests present and passing:
   • WindowsDockerDesktopEngine_PullAsync_RunsDockerPullWithImageName — single docker pull test/sleep:latest, Commands[0] == ["pull","test/sleep:latest"].
   • WindowsDockerDesktopEngine_PullAsync_WhenDockerPullFails_ThrowsInvalidOperationException — non-zero ProcessResult → InvalidOperationException including command + stderr.
   • MongoDbConnectionBroker_WhenContainerMissing_PullsImageBeforeCreate — call sequence Start → Pull → Create → Start.
   • MongoDbConnectionBroker_WhenPullFails_FallsBackToCreateUsingCachedImage — tolerant; CreateAsync + StartAsync still invoked despite failing pull.

FakeDockerEngine overrides the new abstract PullAsync; all other ContainerEngine subclasses (ContainerDEngine, Windows/Linux/MacOSContainerDEngine, DockerDesktopEngine) are abstract and need not implement it. Solution compiles and tests run clean.

Tests

=== Test Run Summary ===
Executed : 18
Passed   : 18
Failed   : 0

Phantom.Workspaces.Containers.Tests.dll — Failed: 0, Passed: 8
Phantom.Workspaces.Data.MongoDB.Tests.dll — Failed: 0, Passed: 10

Outstanding gaps

None — all criteria satisfied.

Applying needs-slow-tests: the diff touches the container engine and the MongoDB data-access layer, so [Category=SlowDocker] integration tests should run at checkin.

[JoshuaRowePhantom]

Checked in

Commit: 8e1be66
Branch fast-forwarded: features → 8e1be66

Fast suite: 5866 passed, 0 failed. Full suite: 6050 passed, 0 failed. Stability run (ProcessRunnerTests, WindowsDockerDesktopEngineTests, MongoDbConnectionBrokerDockerDesktopTests, DockerCommandRunnerLoggingTests, GitHubAuthTokenResolverTests): 10/10 passed.

Known-flaky #791 (AgentFactoryTests.CreateAgentChat_UsesAgentDefinitionChatHistoryTool, MongoDB server-selection timing hang) recurred in the full run and was confirmed unrelated via hang-dump analysis (block in Cluster.SelectServerAsync under ConnectContainerAsync, not the docker pull/logging changes). Dump archived to dumps\791\. Not blocking.