Skip to content

Enhance Docker support in NOIR with detection and containerization - #20

Merged
Verlintas merged 9 commits into
Verlintas:mainfrom
du5:claude/affectionate-archimedes-srnbiu
Nov 12, 2026
Merged

Verlintas merged 9 commits into
Verlintas:mainfrom
du5:claude/affectionate-archimedes-srnbiu

Conversation

@du5

@du5 du5 commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

This pull request adds comprehensive Docker support and container awareness to NOIR, enhancing both functionality and user guidance. It introduces a new Docker image for simplified deployment, updates the doctor command to detect Docker and container environments, and adjusts device troubleshooting advice when running inside containers. Documentation and tests are expanded to cover these new features.

Docker and Container Support:

  • Added a Dockerfile to build a NOIR image with JRE and adb, enabling easy deployment on Linux hosts with Docker only. The README now documents usage, USB passthrough, persistent volumes, and host adb reuse. (Dockerfile, README.md) [1] [2]
  • Updated .dockerignore to minimize build context and prevent unwanted files from entering the image. (.dockerignore)

Doctor Command Enhancements:

  • noir doctor now reports Docker installation and daemon status, with clear, actionable guidance for all scenarios (not installed, stopped, permission denied, etc.). It also detects when NOIR is running inside Docker, Podman, Kubernetes, or LXC, and displays a container row. (cli/src/main/kotlin/app/noir/cli/Main.kt, cli/src/main/kotlin/app/noir/cli/Guidance.kt, README.md, CHANGELOG.md) [1] [2] [3] [4]

Device Troubleshooting Improvements:

  • When running inside a container, missing device errors now provide USB passthrough instructions instead of generic cable advice, improving clarity for container users. (cli/src/main/kotlin/app/noir/cli/Main.kt, cli/src/main/kotlin/app/noir/cli/Guidance.kt) [1] [2] [3] [4] [5]

Testing and Documentation:

  • Added DockerGuidanceTest to ensure Docker-related guidance is accurate, localized, and fits terminal constraints. (cli/src/test/kotlin/app/noir/cli/DockerGuidanceTest.kt)
  • Expanded the README with detailed Docker usage instructions and explanations for all new features. (README.md) [1] [2] [3]

These changes make NOIR more robust and user-friendly in both standard and containerized environments, with clear guidance and improved deployment options.## What changed

Why

How to test

./gradlew :core:jvmTest :cli:test :cli:installDist
./cli/build/install/noir/bin/noir scan --only 'com\.example\.'

Checklist

  • :core:jvmTest passes
  • :cli:test passes
  • :cli:installDist passes
  • dashboard changes survive COLUMNS=50
  • no new network calls
  • CHANGELOG.md updated under Unreleased
  • no new destructive action in auto without an explicit flag

`noir doctor` gains a `docker` row that answers two questions: is Docker
installed, and is its daemon running. A stopped daemon and a refused
socket each end with the fix for the current platform; a daemon endpoint
that does not answer within 10 s is reported as unknown instead of
hanging the check. Docker is optional for NOIR, so the row never changes
the exit code, and the probe only runs the local `docker --version` and
`docker version`.

NOIR also recognises when it runs inside Docker, Podman, Kubernetes or
LXC. `doctor` then shows a `container` row, and the "no device" guidance
explains USB passthrough, since the cable is rarely the problem there.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
The image carries a JRE, adb and NOIR, so a Linux host needs nothing but
Docker. Build and runtime stages share one Java release, because Gradle
targets the JDK it runs on. adb comes from the distribution package,
which also exists for arm64 (Google's platform-tools zip is x86_64
only); jadx stays out and is fetched on demand by `noir setup`, as with
the portable builds.

.dockerignore is an allowlist, which keeps local scan reports
(noir-out/) and tool caches out of the build context.

The README documents the USB passthrough flags, the volume that keeps
the adb authorisation between runs, and how to reuse a host adb server
where USB cannot be passed through.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
.gitattributes marks gradlew.bat as `text eol=crlf`, so git expects the
stored blob to be LF and converts to CRLF on checkout. The blob was
committed with CRLF instead, which made every fresh clone report the
file as modified.

Only the stored blob changes. The checked-out file is byte-identical
and still has CRLF, as Windows batch files need.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
Copilot AI balanced review requested due to automatic review settings November 11, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Generic Docker connection errors can be misreported as a stopped daemon, hiding certificate failures, while the ADB guidance lacks a container-reachable listener setup and access restrictions for unauthenticated port 5037.

2 open issues
What changed in this PR

This PR adds container deployment support to NOIR and extends its diagnostics with Docker status, container detection, and container-specific device troubleshooting.

Changes:

  • Adds a multi-stage Docker image with JRE and adb, plus a restricted build context.
  • Extends doctor and missing-device guidance with Docker and container awareness.
  • Adds detection and guidance tests, deployment instructions, and changelog entries.
File Description
README.md Documents Docker diagnostics, deployment, USB passthrough, and host adb reuse.
gradlew.bat Formatting-only changes; script logic is unchanged.
Dockerfile Builds NOIR and packages it with JRE and adb.
core/​src/​jvmTest/​kotlin/​app/​noir/​core/​platform/​DockerTest.kt Tests Docker probing, error parsing, timeouts, and container detection.
core/​src/​jvmMain/​kotlin/​app/​noir/​core/​platform/​Platform.kt Makes executable validation available within the module.
core/​src/​jvmMain/​kotlin/​app/​noir/​core/​platform/​Docker.kt Implements Docker discovery, daemon probing, and container detection.
cli/​src/​test/​kotlin/​app/​noir/​cli/​DockerGuidanceTest.kt Tests Docker guidance, localization, ASCII output, and terminal width.
cli/​src/​main/​kotlin/​app/​noir/​cli/​Main.kt Integrates Docker diagnostics and container-aware device guidance.
cli/​src/​main/​kotlin/​app/​noir/​cli/​Guidance.kt Adds localized Docker status and container troubleshooting messages.
CHANGELOG.md Records the new Docker and container features.
.dockerignore Limits build inputs and excludes generated build caches.

Review effort: Balanced

"",
"2. macOS / Windows:Docker Desktop 不能透传 USB",
" 改用便携包,或让容器复用宿主机的 adb server:",
" -e ADB_SERVER_SOCKET=tcp:host.docker.internal:5037",
Comment on lines +82 to +85
"failed to connect to the docker api",
"is the docker daemon running",
"docker daemon is not running",
"error during connect",

@Verlintas Verlintas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The Docker feature and tests look good overall, and CI is green on Linux, macOS, and Windows. A few items need fixing before merge:

  1. Rebase: the PR now conflicts with main in CHANGELOG.md (only that file).
  2. ADB server reuse is not workable as documented. adb start-server listens on 127.0.0.1:5037, so ADB_SERVER_SOCKET=tcp:host.docker.internal:5037 usually cannot connect. Using adb -a exposes the unauthenticated 5037 port; document the required binding, firewall/network restrictions, or recommend a tunnel/USB instead.
  3. Docker error classification: "error during connect" is too generic. TLS/certificate failures contain this phrase and are currently misclassified as STOPPED. Remove it and add a certificate-error regression test. The other generic markers ("no such file or directory", "cannot find the file specified") should also be narrowed.
  4. Drop the gradlew.bat line-ending commit. It changes the blob from CRLF to LF, conflicts with .gitattributes, and adds 224 lines of unrelated noise.
  5. Add a docker build CI smoke test (or provide a manual build result). The Dockerfile is currently unverified: adb package availability/version, arm64 support, and the .dockerignore context are untested.

Minor: consider pinning the floating base-image tags and documenting that the container runs as root with USB access.

This reverts commit c81285b.

Requested in review: the change is unrelated to Docker support and put
224 lines of line-ending noise into the pull request. gradlew.bat is
byte-identical to main again.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
Brings in "fix(scan): tolerate OEM dumpsys formats" from Verlintas/NOIR.

CHANGELOG.md was the only conflict: both sides added a section under
Unreleased. Resolved in Keep a Changelog order, Added before Fixed, as
the 0.4.1 entry already does.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
"error during connect" is the prefix the Docker CLI puts in front of
every transport failure, so matching it reported a rejected TLS
certificate or a protocol mismatch as "daemon not running" while a
daemon was in fact listening. The marker is gone; such failures are now
unknown and `doctor` shows Docker's own message.

The operating system's wording ("no such file or directory", "cannot
find the file specified", "permission denied", ...) was too broad as
well: a missing ca.pem or an unreadable key matched it. It now only
counts on a line that names the daemon's own socket or pipe.

The new fixtures are the messages Docker 29 printed for an untrusted
certificate, missing certificate files and HTTP/TLS mismatches.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
The Docker Desktop recipe pointed the container at the host's adb
server through ADB_SERVER_SOCKET=tcp:host.docker.internal:5037. A host
adb server listens on 127.0.0.1:5037 only, so that connection usually
fails, and the usual way around it, `adb -a`, exposes an
unauthenticated port to the whole network.

The README and the in-container "no device" guidance now send macOS and
Windows users to the portable build, where USB works, and the README
says why the shared-server route is not offered. A test keeps the
guidance from pointing at a host adb server again.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
The floating eclipse-temurin tags are replaced by the exact release they
resolve to today (17.0.21_9), each with its multi-arch index digest, so
a rebuild cannot silently pick up a different base. Dependabot now also
watches the Dockerfile, otherwise the pins would only go stale.

The README and the Dockerfile state that the container runs as root so
adb can open the passed-through USB nodes, and that the documented
flags reach every USB device on the host.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
The Dockerfile was not exercised anywhere. The new job builds the image
on amd64 and on a native arm64 runner, then checks what the build could
not show on its own:

- the distribution's adb package installs and runs,
- NOIR starts on the image's JRE, notices the container and explains
  USB passthrough when no device is attached,
- the build context holds the .dockerignore allowlist and nothing else;
  a planted noir-out/report.json must not reach the build stage.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSsPaaLQJLx6wnsTuHo5hD
@du5

du5 commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Everything is addressed in the new commits.

  1. Conflict with main: merged main. CHANGELOG.md keeps Added before Fixed.
  2. ADB server reuse: removed. macOS and Windows are pointed to the portable build, and the README says why a shared host adb server is not offered. A test keeps the in-tool guidance from suggesting it again.
  3. Error classification: "error during connect" is gone. OS wording such as no such file or directory or cannot find the file specified only counts on a line that names the daemon socket or pipe. The new fixtures are real Docker 29 output for an untrusted certificate, a missing ca.pem and HTTP/TLS mismatches.
  4. gradlew.bat: reverted. The file is byte-identical to main and no longer in the diff. History was not rewritten, so the commit and its revert are still listed. I can force-push a linear branch if you prefer.
  5. Docker build: a new docker CI job builds the image on amd64 and arm64, runs noir --version, adb version and noir doctor, and checks the build context against the allowlist. Manual amd64 result: adb 34.0.4-debian, build context about 145 kB, image 390 MB.

Minor: both base images are pinned to 17.0.21_9 with digests, and Dependabot now watches the Dockerfile. The README and the Dockerfile state that the container runs as root and can reach every USB device on the host.

Not verified yet: USB passthrough with a physical phone, and the arm64 build, which the new CI job will run for the first time.

@Verlintas Verlintas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

All review points are addressed, including the ADB-server guidance, TLS error handling, gradlew.bat revert, and Docker build tests on amd64/arm64. All five CI jobs pass.

@Verlintas
Verlintas merged commit 1f1c262 into Verlintas:main Nov 12, 2026
5 checks passed
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.

4 participants