fix/docker-review-issues

[PolarBearEs]

No description provided.

[greptile-apps]

Greptile Summary

This PR addresses several correctness and performance issues in the Docker transport and image-handling stack. It introduces a shared DockerDaemon instance via tokio::sync::OnceCell, replaces manual raw-string reference parsing with ImageReference::parse, and moves blocking archive I/O onto spawn_blocking.

• Shared daemon + OnceCell: DockerDaemon::shared() replaces per-call connect() everywhere in mod.rs, layers.rs, and daemon.rs, removing redundant setup on every operation.
• ChunkReadBuffer: The Windows transport's chunked HTTP parser replaces a Vec<u8> with a cursor-based buffer type, eliminating O(n²) drain calls on large payloads; find_bytes is replaced by find_crlf that operates on absolute indices, and parse_chunk_size is extracted as a shared helper used by both the streaming and in-memory paths.
• Algorithm-agnostic digest copy: copy_reader_with_digest in digest.rs now derives the hash algorithm from the expected-digest string, so layer verification works correctly for SHA-384/512 manifests and removes the SHA-256 hard-coding in layers.rs.

Confidence Score: 5/5

Safe to merge; all changed paths are well-reasoned refactors with no dropped logic.

The shared daemon uses OnceCell correctly (failed init retries, successful init is reused), the ChunkReadBuffer cursor arithmetic is sound, the absolute-index find_crlf is consistent with how its callers slice the buffer, and tagged_repository_name correctly handles Docker Hub library vs user images. The new copy_reader_with_digest test covers the copy-and-hash path end-to-end. No logic was silently dropped during refactoring.

No files require special attention.

Important Files Changed

Filename	Overview
src/docker/daemon.rs	Adds SHARED_DAEMON OnceCell and shared() initializer; changes pull_image/tag_image to use &repository/&tag now that split_tagged_reference returns owned Strings. Clean change.
src/docker/layers.rs	Moves archive extraction to spawn_blocking; switches to shared daemon; replaces inline SHA-256 loop with copy_reader_with_digest; splits function to return HashMap instead of mutating a ref. Correct refactoring.
src/docker/mod.rs	Replaces manual colon-split reference parsing with ImageReference::parse; adds tagged_repository_name that correctly strips "library/" prefix for Docker Hub official images. All callers updated to use shared daemon.
src/docker/transport_windows.rs	Introduces ChunkReadBuffer (cursor-based, avoids O(n²) drain), build_request_head helper, find_crlf (absolute-index CRLF search), and parse_chunk_size. Adds checked_add for chunk_end overflow. Clean and correct.
src/docker/transport.rs	Extracts build_failure_error from ensure_success_status so reqwest and windows transports can share the same error-formatting logic. Straightforward refactor with no behavioral change.
src/docker/transport_reqwest.rs	ensure_success now delegates to build_failure_error; removes the duplicate trim/format logic. Equivalent behavior.
src/digest.rs	Adds copy_reader_with_digest / copy_and_hash_reader that stream data through a writer while computing the digest, delegating algorithm selection to the parsed digest prefix. New unit test verifies the copy + hash behavior.
ci/smoke-docker.sh	Adds a smoke test that exercises the parallel cached-load path: pulls two images with --no-load, removes them, then re-pulls with --max-parallel-images 2 and verifies both appear in the Docker daemon.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant mod as docker/mod.rs
    participant OnceCell as SHARED_DAEMON (OnceCell)
    participant Daemon as DockerDaemon
    participant Transport as DockerTransport
    participant Digest as digest.rs

    Caller->>mod: load_archive / pull_image / tag_image / ...
    mod->>OnceCell: shared().await
    alt first call
        OnceCell->>Daemon: connect()
        Daemon-->>OnceCell: "&'static DockerDaemon"
    else already initialized
        OnceCell-->>mod: "&'static DockerDaemon (cached)"
    end
    mod->>Daemon: operation (load_archive / pull_image / ...)

    Note over Daemon,Transport: pull_image uses split_tagged_reference
    Daemon->>mod: split_tagged_reference(reference)
    mod->>mod: ImageReference::parse(reference)
    mod->>mod: tagged_repository_name(ref)
    mod-->>Daemon: (String repository, String tag)
    Daemon->>Transport: "POST /images/create?fromImage=...&tag=..."

    Note over Transport: Windows chunked response → ChunkReadBuffer
    Transport->>Transport: read_chunk_size(ChunkReadBuffer)
    Transport->>Transport: write_chunk_data(ChunkReadBuffer)

    Note over Daemon,Digest: Layer materialization (spawn_blocking)
    Daemon->>Digest: copy_reader_with_digest(expected, reader, writer)
    Digest->>Digest: parse_digest → select SHA algorithm
    Digest->>Digest: copy_and_hash_reader (stream + hash)
    Digest-->>Daemon: actual_digest string
    Daemon->>Daemon: "verify actual == expected"

Loading

_{Reviews (2): Last reviewed commit: "Address Greptile Docker review comments" | Re-trigger Greptile}