Model Runner (local LLMs) + review remediation#346
Open
mariotoffia wants to merge 161 commits into
Open
Conversation
- Introduced real DMR captures for model management and inference, including JSON outputs for model listing, inspection, and error handling. - Added detailed documentation for Docker Model Runner (DMR), covering management, inference, capabilities, and integration with Docker Compose. - Included examples for quick start, managing models, and wiring models into containers.
… support with local LLMs, including management, chat, and embeddings functionalities.
…d add model listing functionality
feat: Introduce new error code for command execution failures refactor: Remove ModelBackend struct as it is no longer needed fix: Update ModelPruneResult to include raw output for unparsed results fix: Adjust ModelRunnerEndpoint to preserve path-bearing URLs refactor: Change Backend property in ModelConfigureOptions to string for flexibility refactor: Simplify ModelPackageRequest by removing unnecessary Labels property refactor: Clean up ModelRunOptions by removing Backend and IgnoreRuntimeMemoryCheck properties test: Add security tests for ComposeModelBuilder to prevent YAML injection test: Ensure compatibility of WithExtraHost as a default interface method test: Verify cancellation propagation during Docker API model inference test: Validate Docker CLI streaming command execution and exit handling test: Implement tests for ModelApiPaths to ensure safe URL path generation test: Add tests for ModelTlsValidation to enforce strict hostname verification feat: Implement ModelApiPaths for safe URL path construction in Docker API feat: Create ModelTlsValidation for custom root CA validation in TLS connections
…er and API connection
…stances; introduce shallow copy constructors for ChatCompletionRequest and CompletionRequest. Enhance DockerApiConnection and ModelApiConnection to manage X509 certificates properly, ensuring they are disposed correctly. Update Dispose methods across services to avoid sync-over-async deadlocks by dispatching to the thread pool. Improve documentation for Docker Model Runner, marking it as a preview feature and clarifying usage patterns. Adjust README and getting-started documentation to reflect changes in async method usage and model runner integration.
- Updated README.md to clarify Docker Model Runner availability and versioning. - Modified getting-started.md to include context size requirement for DMR v1.2.1. - Updated model-runner.md to reflect async usage and correct example links. - Added unit tests for Docker CLI argument quoting to ensure proper argument handling. - Expanded coverage tests for ModelRunnerService, including store and inference methods. - Introduced MockDriverPack enhancements for better test coverage of model management. - Added validation for environment variable names used in model-runner builders.
- Implemented `ModelApiTlsHandshakeTests` to validate TLS handshake behavior with self-signed certificates and custom CA trust. - Created `InferenceModelIdTests` to ensure correct handling of inference model IDs without injecting default tags. - Introduced `ModelEnvVarsCollection` to manage environment variable tests sequentially, preventing race conditions. - Added command execution logic in `DockerCliDriverBase` for improved process management and output handling. - Developed `InferenceDtoCopy` utility for deep copying inference request DTOs via JSON serialization. - Established `InferenceModelId` struct to represent raw model IDs accurately without automatic tag injection. - Added a script for enforcing coverage thresholds post-test execution to maintain code quality.
… sub-plans) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n on push Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…meMismatch (A3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…not corrupted (B7) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h (B8) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eld) (B10) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…body instead of throwing (C11) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (C12) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mbiguous rows (C14) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s (D19) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ode (D20) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… format Pre-existing tests in DockerCliComposeDriverTests and DockerCliContainerDriverTests expected the old (incorrect) double-backslash behavior for interior path backslashes. The B7 CommandLineToArgvW fix correctly preserves single interior backslashes; update assertions to match the correct output. Also apply `dotnet format whitespace` fixes to four source files flagged by make lint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Finding 1 (C12): Wire StreamReadIdleTimeout into the PRODUCTION SSE read loop in DockerApiModelInferenceDriver.ReadBoundedLineAsync — previously the timeout was stored in ModelApiConnection but never consulted during streaming. Now ReadBoundedLineAsync accepts TimeSpan? idleTimeout and wraps each per-character read with a linked CTS when set. Behavior is opt-in: null = wait indefinitely (caller cancellation only), matching prior behavior. - Finding 4 (C12): Change idle-timeout exception from StreamParseError to EndpointUnreachable — a stalled remote is an unreachable endpoint, not a parse error. - Finding 2 (B8): Add NonStreaming_OversizedOutput_KillsChildProcessTree_ ReturnsFailed to DockerCliProgressStreamingTests: creates a sentinel file before overflowing stdout; asserts Success=false, ExitCode=-1, and that the sentinel was never cleaned up (proving Kill(entireProcessTree:true) fired before the cleanup step ran). - Finding 3 (B7): Add 4th InlineData to QuotesWindowsPathsWithoutCorruption: input "a ends\" (space triggers quoting, trailing backslash) must produce "a ends\\" — trailing backslash doubled before the closing quote. - Remove dead ReadWithIdleTimeoutAsync from ModelApiConnection (superseded by the driver-level wiring); add StreamReadIdleTimeout property to IModelApiConnection + ModelApiConnection + MockModelApiConnection; add StallingStream and SetupStreamStalling to MockModelApiConnection for the new idle-timeout tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…in FixedStreamConnection Interface gained StreamReadIdleTimeout (C12); the benchmark stub must implement it or FluentDocker.Benchmarks fails to compile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…IComposeModelBuilder (A2,D17) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ve Docker coupling from Services (D15,D16) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t throw TrySysCtl's doc claimed it could throw InterfaceNotSupportedException from a "fallback resolver" that no longer exists; TrySysCtl only ever returns false for unsupported interfaces (pinned by Chunk2KernelProdReadinessTests.TrySysCtl_WhenPackSysCtlThrows_WrapsFaultAsDriverException). Removed the stale tag. Also documented that GetCapabilitiesAsync/IsHealthyAsync throw InvalidOperationException when no default driver is configured and no driverId is passed (confirmed via ResolveDriverIdOrDefault). Doc-only.
…tion A pack or plain driver whose TryResolve reports success but hands back an instance that isn't assignable to the requested interface was silently swallowed: the pack path logged at Debug only, the plain-driver path logged nothing at all, and both surfaced as a cause-less InterfaceNotSupportedException claiming the driver "does not implement" the interface even though it's in GetSupportedInterfaces(). That hides a mis-mapped registration (Drivers[typeof(IContainerDriver)] = imageDriver) from on-call. Both paths now log at Warning (parity with LogPackFallbackFailure), and TryResolveCore reports the mismatched actual type back to SysCtl, which throws InterfaceNotSupportedException via a new constructor overload whose message names both the actual resolved type and the requested interface. TrySysCtl's false-for-unsupported contract is unchanged; only SysCtl's thrown message differs. Adds a new InterfaceNotSupportedException(driverId, interfaceName, actualType) overload. FluentDockerKernel.cs was at the 500-line limit, so the resolution-only helpers (TryResolveCore, LogTypeMismatch, LogPackFallbackFailure, IsResolutionContractException, CreateResolutionFailureException) move to a new FluentDockerKernel.Resolution.cs partial, mirroring the existing DriverRegistry / DriverRegistry.Helpers.cs split. TDD coverage added for both the pack path (reusing the existing WrongTypeDriverPack fixture) and the plain-driver path (new WrongTypeResolverDriver fixture), asserting both the exception message and the Warning-level log line via a small in-file recording ILoggerFactory.
…ption The K-M4 change rewrote LogTypeMismatch to dereference resolved.GetType() without the null guard the pre-existing code had. A pack/driver TryResolve returning true with a null implementation (IsInstanceOfType(null) is false) then hit LogTypeMismatch(..., null) -> NullReferenceException, which TryResolveCore's catch-all promoted to DriverException -- breaking TrySysCtl's never-throws-for-unsupported contract and worsening diagnostics vs before. Guard resolved != null at both the pack and plain-driver call sites so a null "success" falls through to the generic InterfaceNotSupportedException (soft: TrySysCtl -> false), never NRE/DriverException. Adds TDD coverage for both paths (pack: new NullResolvingDriverPack; driver: WrongTypeResolverDriver parametrized with the resolved instance). Also (MINOR consistency): give the kernel-level FluentDockerKernel.IsDisposeComplete the same "completed pass != nothing abandoned; also check AbandonedDriverCount" caveat the K-M2 pass added to IDriverRegistry.IsDisposeComplete one hop away. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… growth ModelOperationGate held one SemaphoreSlim per distinct model key forever (GetOrAdd, never removed) — a long-lived process churning through many distinct model references (e.g. per-tenant fine-tunes) leaked memory slowly and unboundedly. Replace the static ConcurrentDictionary with a lock-guarded, ref-counted Dictionary<string, Gate>: the ref is reserved under the lock before WaitAsync, so a gate is evicted (and its semaphore disposed) only when RefCount hits zero, which is only reachable once no caller holds or is queued on it. This sidesteps the naive "CurrentCount==1 => remove" fix, which races a releasing caller against a new waiter and can hand two callers the same logical gate via two different SemaphoreSlim instances. Public API (KeyFor, both AcquireAsync overloads) is unchanged. Adds one read-only diagnostic, TrackedGateCount, so the bound is observable/testable without reflection. TDD: 4 new tests written first against a TrackedGateCount stub on the old implementation (confirmed genuinely RED - real assertion failures, not a compile error), then GREEN after the fix. Assertions are baseline-relative rather than absolute-zero since this static gate is legitimately exercised by other test classes running concurrently under xUnit's default parallelism. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Streaming façade methods (GenericOpenAiModelRunner + ModelRunnerService, chat + completion) called ThrowIfDisposed() once at call time then handed back the driver's lazy IAsyncEnumerable. Disposing the runner (or its owned connection) mid-iteration surfaced a raw ObjectDisposedException from inside HttpClient, bypassing the typed ModelRunnerException taxonomy. - Add terminal ErrorCodes.ModelInference.Disposed = "MIN_008" (NOT transient — a disposed runner must not trigger a retry loop; EndpointUnreachable is transient so was not reused). - Add one shared ModelRunnerInferenceHelpers.GuardDisposalAsync<T> iterator: checks disposed before each advance and translates an ObjectDisposedException from MoveNextAsync into ModelRunnerException(Disposed, inner: ode); yield outside the try/catch; disposes the source enumerator in finally (ConfigureAwait(false)). - Wire all 4 façade stream call sites through it; eager call-time ThrowIfDisposed() preserved (direct misuse still throws ODE synchronously). - TDD: ModelRunnerStreamDisposalTests (gated fake driver) covers mid-stream disposal, ODE translation w/ inner, eager-check-unchanged, happy path; ModelErrorCodesTests asserts MIN_008 and that it is not transient. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…AddCommand backslash normalization (B-M2) Rooted ADD sources skipped staging whenever anything already sat at the destination, and root-level relative COPY/ADD sources were never registered in the collision guard - together this let a rooted ADD silently ship a same-named COPY's bytes (or stale bytes on a reused working folder) under a green build. Root-level relative sources now claim their name via a shared ClaimRootedName helper (also de-duplicating the two pre-existing inline collision guards), and the exists-skip is replaced with an unconditional copy so re-staging always refreshes. AddCommand.ToString() now normalizes backslashes via the same shared DockerfileJson.NormalizePath CopyCommand already used, so Windows-authored ADD paths render with forward slashes instead of producing a tar-entry mismatch on the Linux daemon.
ComposeBuilder.ResetForRetry wiped BorrowedProject/_composeCleanupFailed with no persisted "I created this" marker, so a retried BuildAsync() that found its OWN half-created project (up failed, cleanup down also failed) misclassified it as an external borrow. Dispose then skipped `compose down` for it, leaking the stack on flaky-CI retry loops. Add _priorAttemptCreatedProject, deliberately excluded from ResetForRetry, and re-own by project name when the probe finds the project on a later attempt and this builder set that marker. Mirrors the Network/Volume re-own-by-id pattern; compose projects have no stable id, so re-own is guarded by the persisted marker instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… claim-after-exists IMPORTANT: rooted COPY/ADD staging hit File.Copy into a path already occupied by a directory (created by an earlier nested relative source of the same basename), throwing a raw UnauthorizedAccessException/IOException with no FluentDocker context. New shared GuardNoDirectoryAt helper now throws a typed, actionable NotSupportedException before File.Copy in both rooted branches, matching the method's other guarded failures. MINOR (regression from the B-H1 commit): ClaimRootedName ran before the File.Exists check at all four sites, so a missing relative source skipped under lenient string-gen still reserved its basename - a following rooted source with that basename then threw a spurious collision. Moved each claim to after the existence check so only actually-staged sources register a name; the directory-source and not-found guards keep their prior order. Two new unit tests in BuilderProductionReadinessBugTests (DockerfileBuilderTests was at the 500-line ceiling): directory-collision -> typed error, and missing-lenient-relative + same-basename rooted -> stages without spurious throw. RED verified against the prior commit.
…M3, B-M4) - WithContextSize(tokens) on ModelRunnerBuilder, ModelServiceBuilder, and ModelRunOptionsBuilder now rejects tokens <= 0 with ArgumentOutOfRangeException, matching the guard ComposeModelBuilder already had, instead of silently storing garbage that only surfaces as a raw CLI error deep inside `docker model configure`. - Builder.BuildAsync now collects BuildScopes into an explicit List instead of a Dictionary keyed by (kernel, driverId), so BuildResults' reverse-disposal order (dependents before dependencies) is guaranteed by insertion order rather than relying on Dictionary enumeration order, an unspecified CLR detail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…slabel (C-M2), RequestTimeout doc (C-M3) C-M1: uniform ArgumentNullException.ThrowIfNull(model/source/target/request) across every CLI model-runner op that dereferences a required reference — replaces raw NREs (Load/Unload/ Configure/Tag/Push) and NRE-swallowed-into-misleading-Fail (Pull/Inspect/Remove/Package) with one consistent, documented failure mode. C-M2: SupportsConfigureBackendAsync's backend-probe internal timeout (or seam fault) no longer surfaces as OperationCanceledException to a caller who never cancelled their own token — it now throws a typed, non-transient DriverException(ConfigureFailed) that ConfigureAsync maps to CommandResponse.Fail, so a retry loop can no longer mistake a wedged Docker model plugin for a transient cancellation. Genuine caller cancellation still throws OCE. Also fixed a second pre-existing test that encoded the same bug via a different (seam-fault) trigger. C-M3: DriverContext.RequestTimeout doc now states it bounds both API HTTP requests and CLI buffered command execution, not just the API driver (doc-only, no behavior change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
S-H1 (HIGH): WaitForPortAsync/WaitForHttpAsync/WaitForLogMessageAsync now inspect container state each poll and throw FluentDockerException (exit code + log tail) on a terminal exited/dead state, instead of burning the full timeout and returning a bare false. Extracted the start gate's diagnostic helpers (HasReachedTerminalState/ReadLogTailAsync/AppendLogTail) into a shared internal WaitDiagnostics helper so the gate and the wait extensions share one implementation (no copy-paste); added a service-level log-tail overload and a ThrowIfTerminalAsync that positively confirms death (inspect failures are swallowed so timeout/retry behaviour is unchanged when state is unknown). Added LogWaitFailure + lastException tracking to WaitForPortAsync for parity with the HTTP variant. S-M4 (MEDIUM): inverted IsRetriableWaitException from a blacklist to a whitelist of genuinely transient failures (SocketException, HttpRequestException, transient DriverException, per-attempt TaskCanceledException). Non-transient exceptions (e.g. InvalidOperationException) now surface instead of being silently retried to a timeout; dropped the NRE special-case. Moved GetDockerHost host extensions to ServiceExtensions.Host.cs to keep ServiceExtensions.cs under the 500-line cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
StartAsync/StopAsync fired lifecycle hooks and published StateChange right after the optimistic UpdateState, before reconciling against `compose ps` — so Running/Stopped hooks could run against a dead or still-running stack since `docker compose up/start/stop` reports success even when a service crashes on boot. Mirror the already-correct RestartAsync (SVC-MAJ-3): reconcile first, then only fire hooks if the reconciled state still matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sync tar (S-M1/M2/M3) S-M1: ServiceEndpointResolver returned a literal loopback binding IP verbatim even for a remote daemon (tcp/ssh, non-loopback host), making WaitForPortAsync probe the client's own loopback instead of the daemon's -> false-positive readiness. Added IsRemoteDaemon() and resolve loopback bindings to the daemon host for remote daemons, mirroring the existing wildcard-binding branch; local daemons (unix/npipe, tcp://localhost, tcp loopback host) keep returning the loopback binding unchanged. S-M2: named-volume dispose cleanup caught OperationCanceledException alongside every other exception, logged a warning, and kept calling the driver for the remaining volumes with an already-canceled token. Narrowed the catch to exclude OCE so cancellation stops the loop instead of issuing dead calls. S-M3: dispose-time export "explode" used synchronous TarFile.ExtractToDirectory, blocking a threadpool thread and ignoring cancellation once started. Switched to TarFile.ExtractToDirectoryAsync with the dispose-scoped token and made the source stream an await-using IAsyncDisposable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-ups on the wait fail-fast change: - IMPORTANT-2: WaitForLogMessageAsync now checks log content BEFORE the terminal-state fail-fast, so a short-lived container that logs the awaited message and then exits still returns true (was regressing to a fail-fast throw). WaitForPortAsync/WaitForHttpAsync keep the top-of-loop fail-fast (a gone container cannot serve a port/HTTP). - IMPORTANT-1: documented the new terminal-state throw in docs/containers.md and the XML <remarks> of all three wait helpers (they now throw FluentDockerException with exit code + log tail on exited/dead, in addition to cancellation and non-transient driver errors). - MINOR: AppendLogTail is now idempotent (skips re-append when the message already carries the "Container log tail:" marker) — one guard in the shared helper fixes the double-tail from the builder catch re-wrapping a ThrowIfTerminalAsync / start-gate exception that already has a tail. - Tests: added WaitForLogMessageAsync_MessagePresentButContainerTerminal_ReturnsTrue (RED→GREEN for the reorder) and WaitForPortAsync_RestartingContainer_DoesNotFailFast_TimesOutToFalse (restarting is not terminal). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reset + parser XML docs (P-M1/M2/M3/M4) P-M1: IsMachineManagedContext now gates on a remote DriverContext.Host first, so a connection-refused error against a remote rootful daemon (ssh://) classifies as Api.ConnectionFailed instead of the misleading, transient-retried Machine.NotRunning. P-M2: AutoStartMachineCoreAsync no longer guesses an arbitrary machine when none is flagged default and more than one exists; it throws PodmanMachineNotRunningException naming AutoStartMachineConfig.MachineName instead of starting a possibly-wrong VM and hanging for the full readiness timeout. P-M3: buffered StatsAsync now passes --no-reset, symmetric with the streaming stats path, so goterm's raw \033[2J reset sequence never reaches ParseStatsOutput regardless of Flush()'s non-TTY detection. P-M4: XML-documented all 21 public parser methods (10 delegating methods in PodmanCliContainerDriver.Parsing.cs, 11 in PodmanContainerParser.cs) with accurate null/throw semantics read from each method body. PodmanContainerParser.cs and PodmanCliDriverPack.cs split via partial class (PodmanContainerParser.Network.cs, PodmanCliDriverPack.AutoStart.cs) to stay under the 500-line file cap after the doc/fix additions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ulti-dot filenames (Co-H1/Co-M1) ResourceExtract(type, target, files) queried non-recursively, so ExtractFile's lossy namespace/filename split dropped any multi-dot resource (Dockerfile.template, settings.production, archive.tar.gz, app.properties) before Include's suffix-match ever ran (Co-H1, PROVEN LIVE: empty dir, no exception). Even when matched via a recursive query, the writer scattered it into a mangled subfolder + fragment, e.g. dir/Dockerfile/template instead of dir/Dockerfile.template (Co-M1). Fix: query recursively in the files-branch, and rewrite Include's suffix-matched ResourceInfo onto the REQUESTED name (reconstructing Namespace/RelativeRootNamespace from the fully-qualified manifest name), so the requested-files path is fully deterministic regardless of ExtractFile's guess. Also add a best-effort refinement in QueryCore for the no-files recursive path (Query()/ToFile(), which has no requested name to disambiguate): a dotless ExtractFile guess beyond the query root now re-anchors to the full remainder, landing common multi-dot shapes as one file at the target root instead of a directory+fragment. Corrected the MDL-MAJ-2 comment that overclaimed the non-recursive path already worked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xed docker volume specs (Co-M2)
TemplateString's opt-in handleWindowsPathIfNeeded constructor flag drove ToTargetOs,
which converted every '/' to '\' (outside quoted http(s) URLs) when both
FdOs.IsWindows() and the flag were true. A mixed docker volume spec like
c:/data:/data (Windows host : Unix container) would become c:\data:\data,
corrupting the container-side path. The flag had zero callers inside the library
(grep-verified across FluentDocker/, FluentDocker.Tests/, FluentDocker.Benchmarks/,
and Examples/) - dead flexibility exposed as a public footgun. v3.2.0-preview.2 is
not yet on NuGet, so removing the parameter is a clean pre-release change.
Deleted the flag, the constructor param, ToTargetOs, and the now-orphaned
UrlDetector/MyRegex() regex machinery that existed solely to support it. Render
now runs directly on the input string on every call, on every OS. Removed the one
test that exercised the dead flag (its assertion was the corruption itself) and
added two regression guards: a literal c:/data:/data round-trip, and a
${TMP}/data:/data host-mount spec whose container side keeps forward slashes.
No runtime RED was possible on this non-Windows machine (the flag defaulted to
false and every real call site used the 1-arg constructor already), so the proof
is structural: the whole solution + ~5.4k tests still build and pass with the
2-arg constructor gone, confirming no caller relied on it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c aggregate + TTY log-sniff fallback (A-H1/A-M1/A-M2/A-M3)
A-H1 (HIGH): PullAsync("nginx:1.25", tag: null) sent fromImage=nginx%3A1.25&tag=latest -
the daemon's reference.WithTag discards the embedded tag and silently pulls
nginx:latest instead of nginx:1.25. Added TrySplitEmbeddedTag, mirroring the CLI
driver's ShouldAppendTag detection rule exactly, to split an embedded ":tag" into
fromImage/tag; a conflicting explicit tag now rejects with InvalidArgument instead of
being silently discarded.
A-M1 (MEDIUM): DockerApiServiceDriver.RemoveAsync aborted a multi-id remove on the
first failed DELETE, leaving every id after it (including live services) untouched.
Now attempts every id and aggregates failures, mirroring DockerApiSystemDriver.
PruneAsync's collect-then-fail pattern and the CLI driver's docker service rm
semantics. Split the now-524-line file into DockerApiServiceDriver.Parsing.cs
(spec builders + JSON parsing) to stay under the 500-line cap.
A-M2 (MEDIUM): below API 1.42, GetLogsAsync byte-sniffed the first frame to decide
multiplexed-vs-raw; a TTY container's binary output can coincidentally start with
stdcopy-header-shaped bytes and get misparsed as multiplexed, silently stripping fake
"headers" throughout the log. ReadDockerLogTailAsync now takes a containerId and, on
content-type gate failure, falls back to a TTY inspect (DetectTtyAsync, promoted from
DockerApiStreamDriver to the shared DockerApiDriverBase.Logs.cs) before falling back
further to the byte-sniff only when TTY can't be determined either.
A-M3 (doc): documented StreamIdleTimeout's full blast radius (applies to every
streamed response incl. /events/stats/attach, not just logs) and the per-timeout
pooled-buffer leak trade-off on both DockerApiConnectionConfig.StreamIdleTimeout and
IDockerApiDriverBuilder.WithStreamIdleTimeout. No behavior change.
make lint clean; make test: 5437/5437 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ore non-recursive exactness) Review found the QueryCore dotless-guess re-anchoring ran unconditionally, before the _recursive gate, so on the public recursive:false path a genuinely-nested dotless file (real folder Res/Sub/ with a file literally named "README" -> "Res.Sub.README") was wrongly re-anchored to the root and included, instead of correctly excluded — a new wrong-location silent write, the same class Co-H1 closes. Added `_recursive &&` to the re-anchoring condition: preserves the tested (always-recursive) Co-H1/Co-M1 behavior and restores non-recursive root-level-only exactness. The Include path is unaffected (it reconstructs the true manifest name independently of re-anchoring). Tests: new embedded fixture Res/Sub/README + regression test asserting a non-recursive query EXCLUDES it (RED pre-gate: wrongly re-anchored to root as "Sub.README"; GREEN after). Also pinned the documented no-files nested case (Res.Sub.Dockerfile.template collapses to a flat "Sub.Dockerfile.template" at the root). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…M1) + 602-line trims Doc-H1: swept all 8 docs/README references (plus the MsTestClassContainerFixtureBase.cs XML cross-ref) from the deprecated MsTestContainerFixtureBase to the new MsTestPerTestContainerFixtureBase, leaving exactly one deprecation footnote in docs/testing/mstest.md. Following the old docs previously yielded CS0618 on first build. Doc-M1: amended the [3.2.0-preview.2] CHANGELOG entry in place for post-dated API changes (MsTest rename/Deprecated section, ISysCtl.GetCapabilitiesAsync/IsHealthyAsync/ DefaultDriverId, IDriverRegistry.IsDisposeComplete/AbandonedDriverCount) and the full production-readiness remediation since 0902baf, grouped under Breaking/Added/Fixed/ Deprecated with scannable single-topic bullets — including the breaking TemplateString 2-arg constructor removal and the new ErrorCodes.ModelInference.Disposed (MIN_008) / ModelOperationGate.TrackedGateCount surface. Structural: trimmed docs/utilities.md, docs/testing/xunit.md, and docs/migrate-v2-to-v3/test-migration.md from 602 to 598-599 lines by reflowing hard-wrapped prose paragraphs (pure whitespace change, no content removed). make lint clean; make test: 5438/5438 passed (foreground, blocking run). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ADMEs (Doc-H1) Two package READMEs still taught the deprecated MsTestContainerFixtureBase as the primary name in their cross-package lifetime tables (the exact pattern Doc-H1 targets). Swept both the table cell and the prose sentence to MsTestPerTestContainerFixtureBase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tials Document all 181 undocumented public members in FluentDocker/Model/Drivers/ (all ErrorCodes.cs error-code constants; every other file in the directory was already fully documented). ErrorCodes.cs exceeded 500 lines after docs, so it is split into 18 partial files, one per existing nested static class (General/Driver/Container/Image/Network/Volume/Compose/Pod/Kubernetes/ Machine/Manifest/Api/Config/Auth/Stack/Service/Model/ModelInference), with the main file retaining only the class doc and IsTransientCode. Mechanical split verified byte-identical (constant name, group, and string value) via a diff against the original file; no logic or values changed. CS1591 stays suppressed in the csproj (untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document all 148 undocumented public members in FluentDocker/Services/Impl/ (HostService + .Operations, ImageService, NetworkService, ComposeService + .Lifecycle/.Unpause, VolumeService, ContainerService + .Operations/.Lifecycle/ .Export, PodService, EngineScope). Members implementing a documented interface method use <inheritdoc />, matching the file's own existing convention; the handful of Pause/Stop/Remove/Hook overrides that unconditionally throw FluentDockerNotSupportedException got a real one-line summary plus an <exception> tag instead, since inheriting the base "removes the service" wording would misdescribe them. ContainerService.cs exceeded 500 lines after docs, so the inspect-cache subsystem (InspectAsync + its cache fields/record/ helper) was split into a new ContainerService.Inspect.cs partial, mirroring the file's existing .Export.cs/.Lifecycle.cs/.Operations.cs convention. No behavior change; CS1591 stays suppressed in the csproj (untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Document all 123 undocumented public/protected members under FluentDocker/Drivers/ (Docker CLI driver base + components, Podman CLI driver base + components, the ISystemDriver result types, IPodmanManifestDriver's OCI-shaped result types, and the few remaining gaps in the otherwise-documented Docker API/Models areas). Most flagged members were base-class helpers (error/failure-code classification, argument quoting, process execution overloads), DTO properties mirroring Docker/ Podman CLI JSON output, or driver constructors rather than port-interface implementations, so real behavioral docs were written throughout; <inheritdoc /> was used only for the 2 PopulateMeta() overrides whose base contract is unchanged. DockerCliComposeDriver.Info.cs, ModelJsonParser.cs, and PodmanCliDriverBase.cs exceeded 500 lines after docs, so each is split via an additional partial file following the existing convention (Create.cs, Tables.cs, Execution.cs — the latter mirrors the Docker-side driver base's existing Execution.cs). No behavior change; CS1591 stays suppressed in the csproj (untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s DTOs Task 16 batch 16d: document all 108 previously-undocumented public members across Model/Containers, Model/Events, Model/Networks, Model/Images, and Model/Stacks (CS1591 stays suppressed at the csproj level; enforcement is a separate final step). Docs name the Docker/Podman CLI or JSON concept each member maps to; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task 16e: documents 67 CS1591-flagged public members across Model/Compose, Model/Common, Model/Builders, Model/Models, and top-level Model/*.cs. Docs name the compose YAML key or Docker concept each member maps to; also relocates a doc comment on ModelRunOptions.ContextSize that had drifted onto its private backing field. No behavior change; CS1591 suppression untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pass
Documents the last 77 undocumented public members of Task 16's full XML-doc
pass, across FluentDocker/{Resources,Testing,Common,Builders,Extensions}/:
resource query/reader/writer types, OrphanCleanup/SessionLabel test-cleanup
API, lenient/tolerant JSON converters, lifecycle-hook/network-alias builder
models, and extension-class summaries. A fresh whole-project CS1591 probe
now shows zero warnings anywhere in FluentDocker/, confirming this is the
last outstanding batch.
Testing/Core/OrphanCleanup.cs (499 lines pre-docs) is split into
OrphanCleanup.cs (public surface), OrphanCleanup.Resources.cs (per-kind
sweep logic), OrphanCleanup.Filters.cs (shared predicates), and a new
SessionLabel.cs (its own top-level type, extracted rather than left
crammed into the same file) — all four ≤ 500 lines, verified byte-for-byte
equivalent to the original via a code-line diff.
Also fixes a pre-existing broken XML doc cref in Builders/Builder.Models.cs
(missing `using FluentDocker.Kernel;`) found while probing the same
directory.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task 16 final step: remove the blanket CS1591 suppression in FluentDocker.csproj and promote it to a build error via a scoped WarningsAsErrors (no repo-wide TreatWarningsAsErrors exists to piggyback on). A clean rebuild surfaced zero straggler undocumented members — docs batches A-F already fully covered the public API. Added the T7-deferred <exception cref="ArgumentNullException"> tags to IModelManagementDriver / IModelRuntimeDriver, verified 1:1 against the sole CLI adapter's actual ArgumentNullException.ThrowIfNull calls. Smoke-tested the gate (delete a doc -> build fails with CS1591 as an error -> restore -> build succeeds) and added a CHANGELOG line. make check: lint clean, 5438/5438 unit tests, 13+4 runner tests, coverage 87.53%/79.1% (floor 78%/71%) - all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- PodmanMachineNotRunningException: add a (message, isTransient) overload so the P-M2 ambiguous-machine auto-start guard (multiple machines, none default) can report itself as a permanent config error instead of inheriting the hardcoded IsTransient=true meant for stopped-machine retries. Only that one throw site now passes isTransient: false; all other throw sites are unchanged. - LenientInt32Converter: reword the class summary, which claimed 0 was returned for "unparsable runtime drift" while Read() actually throws JsonException for any unrecognized token (0 is only for JSON null), matching the honest style of the sibling LenientBoolConverter. - ErrorCodes.Machine.NotRunning: note the one non-transient exception (the P-M2 ambiguous-machine case) alongside the existing transient guidance. - Pin the P-M2 test with Assert.False(ex.IsTransient). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ents - Improve kernel registration-rollback dispose to be budget-bounded. - Add IsTracked method to ModelOperationGate for per-key diagnostics. - Update Docker CLI command timeout messages for clarity. - Refactor tests to ensure proper handling of consecutive failed retries and named volume cleanup. - Ensure ModelOperationGate correctly evicts idle gates and prevents memory leaks. - Adjust assertions in tests for better accuracy and reliability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First-class Docker Model Runner support (local LLMs) — and any OpenAI-compatible runner — behind the existing
Builder → WithinDriver → UseModelRunner()pattern, plus the full review-remediation pass.See CHANGELOG.md (
[3.2.0] - Unreleased) and docs/model-runner.md for the complete feature, docs/model-runner-plugins.md for non-Docker runner plugins, and docs/model-runner-compose.md for Composemodels:.Highlights
IModelRunnerfaçade over three hexagonal ports (management/runtime viadocker modelCLI; inference via OpenAI-compatible HTTP on :12434 with SSE streaming), ergonomicChatAsync/ChatStreamAsync/EmbedAsync,ModelReference/LlamaCppRuntimeFlagsvalue objects, managedIModelService,WithModel(...)container wiring, Composemodels:overlay, andGenericOpenAiModelRunnerfor any OpenAI endpoint.Review remediation (latest)
Correctness
EndpointUnreachable(MIN_003) on transport failure (was downgraded to RequestFailed).EmbeddingsRequestdeep-copied before send.ModelReferenceregistry host lowercased at parse so equal refs serialize identically.Extensibility
DockerApiModelInferenceDriver→OpenAiModelInferenceDriver, relocated toFluentDocker.Drivers.Models(generic OpenAI-over-HTTP, not Docker-specific).IModelBackendInfo(Docker reportsllama.cpp; custom packs report own/none;GenericOpenAiModelRunnerreports none) instead of a hardcoded literal.NotSupportedExceptionfor partial packs instead ofInterfaceNotSupportedException.Tests — driver transport-failure mapping, SSE split-frame reassembly, completion mid-stream fault, concurrent
ChatAsync, inference-only pack capabilities + NotSupported, mTLS handshake, CLI-injection quoting.Docs — split
model-runner.mdinto Compose + plugin pages, plugin-authoring guide, README "What's New" with a multi-turn chat demo.Breaking changes
DockerApiModelInferenceDriver→OpenAiModelInferenceDriver(namespace…Docker.Api.Components→…Drivers.Models). Source-breaking only; done while the subsystem is preview/unreleased.Verification
dotnet format whitespace+style) — clean.