ci(docker): dual-publish nir-rs image to GHCR and Docker Hub - #38
Conversation
Add root Dockerfile (Rust 1.97 + libhdf5, example binary) and docker.yml with PR verify-only vs main/tag dual push. Document pull paths in README. Part of #26 / LIM-996. Co-authored-by: Grok <noreply@x.ai>
|
Skipping CodeAnt AI review — this PR is a back-merge between long-lived branches ( If you want to analyze this anyway (e.g. you resolved conflicts with new logic), comment |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded a multi-stage Docker image for ChangesDocker delivery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant DockerBuild
participant DockerHub
participant GHCR
GitHubActions->>DockerBuild: Build and validate the image
GitHubActions->>GitHubActions: Generate tags from Cargo.toml and the Git ref
GitHubActions->>DockerHub: Authenticate and publish generated tags
GitHubActions->>GHCR: Authenticate and publish generated tags
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Review Summary
This PR adds Docker image publishing to GHCR and Docker Hub with appropriate CI verification workflows. The implementation follows security best practices with pinned action SHAs, minimal permissions, and proper secret handling.
Critical Issue Found
Security Configuration: The GHCR registry path is hardcoded while Docker Hub correctly uses a variable, creating a maintenance and security risk if the repository is transferred or the organization changes.
Overall Assessment
The Docker workflow structure is well-designed with proper separation between PR verification (read-only) and publishing (write permissions only when needed). The Dockerfile follows multi-stage build best practices. Once the hardcoded registry path is addressed, this will be ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Qodana for RustIt seems all right 👌 No new problems were found according to the checks applied ☁️ View the detailed Qodana report Contact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker.yml:
- Around line 113-121: Update the version-validation block in the Docker
workflow to validate tag events against the extracted VER from Cargo.toml. When
the event is a tag push, require the triggering tag name to equal v${VER}
exactly and fail before constructing TAGS or publishing if it does not; preserve
the existing semver validation and non-tag behavior.
- Around line 82-85: Update the publish job condition in the workflow to require
that github.repository matches the upstream repository, in addition to the
existing push-to-main or version-tag checks. Preserve the current branch and tag
restrictions while preventing fork workflows from attempting to publish.
In `@Dockerfile`:
- Around line 43-48: Update the runtime stage around WORKDIR, the /workspace
COPY, and the cargo fetch RUN instruction to create an unprivileged user, assign
ownership of the copied workspace to that user, switch to USER before
pre-warming the cache, and preserve the existing failure-tolerant cargo fetch
behavior.
In `@README.md`:
- Line 81: Remove the `|| true` failure suppression from the documented `docker
run` verification command so missing binaries, unsupported arguments, and
runtime failures propagate as command failures; if the invocation is intended to
be informational, explicitly label it as such instead.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c769334d-4228-4c38-9766-cb85fec020ee
📒 Files selected for processing (6)
.dockerignore.github/workflows/docker.ymlAGENTS.mdCHANGELOG.mdDockerfileREADME.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5475fd9. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5475fd98ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Publish :X.Y.Z only on matching v* tags; main gets SHA + latest - GHCR owner from github.repository_owner (lowercased) - Restrict publish to Limen-Neural/nir-rs - Runtime WORKDIR /src so example CARGO_MANIFEST_DIR fixtures work - Non-root user; drop target/ from published image - README smoke uses load_inspect_lif without || true - CI smoke runs default load_inspect_lif Co-authored-by: Grok <noreply@x.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dfe67a020
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- chown /src so non-root can recreate target/; PATH includes CARGO_HOME/bin - Drop Docker Hub login from PR verify (no DOCKER_PAT on PR workflows) - concurrency cancel-in-progress on docker jobs - Remove redundant builder-target load from verify - Clarify Hub image uses vars.DOCKER_USER Co-authored-by: Grok <noreply@x.ai>
|
Babysit check cycle 2 (push
Local smoke: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19f33544c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Avoid shell-placeholder Hub pulls that can target an unrelated namespace. Co-authored-by: Grok <noreply@x.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 207d72e122
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ublic - Target main and Main (REVIEW.md CI checklist) - Publish job concurrency shared across branch/tag so :latest cannot race - Best-effort set GHCR package visibility public after first push - Document GHCR public requirement in README Co-authored-by: Grok <noreply@x.ai>
|
Codex pass (
CI re-running. Not merging (babysit policy). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3d720add4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker.yml:
- Around line 77-80: Update the workflow concurrency configuration for the
Docker publication group to set cancel-in-progress to false and queue to max,
ensuring version-tag publications are never canceled or replaced. Preserve the
shared group behavior for main and tag refs, and use a release queue if the
platform requires preserving more than 100 pending runs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ca70c63-a3ae-43b1-91bc-4584f0f29f8d
📒 Files selected for processing (2)
.github/workflows/docker.ymlREADME.md
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Tag script treats refs/heads/Main like main (SHA + latest) - Publish concurrency still shared across branch/tag but cancel-in-progress: false so v* :X.Y.Z jobs are not aborted Co-authored-by: Grok <noreply@x.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker.yml (1)
149-167: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse a package-management credential for GHCR visibility.
GITHUB_TOKENcannot update package visibility. If anonymous pulls are required, use an authorized PAT and fail unless visibility ispublic; otherwise document that GHCR pulls require authentication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docker.yml around lines 149 - 167, The “Ensure GHCR package is public” step must not use GITHUB_TOKEN for visibility changes. Replace GH_TOKEN with an authorized package-management PAT secret, verify the package visibility is public after the org or user API update, and fail the workflow if it cannot be set or confirmed; if anonymous pulls are not intended, remove this automation and document the authentication requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker.yml:
- Around line 77-81: Update the docker-publish concurrency group to configure
queueing with queue: max alongside cancel-in-progress: false, so pending main
and version-tag publishes are retained rather than replaced; leave the existing
group key and active-run cancellation behavior unchanged.
---
Outside diff comments:
In @.github/workflows/docker.yml:
- Around line 149-167: The “Ensure GHCR package is public” step must not use
GITHUB_TOKEN for visibility changes. Replace GH_TOKEN with an authorized
package-management PAT secret, verify the package visibility is public after the
org or user API update, and fail the workflow if it cannot be set or confirmed;
if anonymous pulls are not intended, remove this automation and document the
authentication requirement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d5210c5-1d07-4c7c-9d3e-19c161f1d41a
📒 Files selected for processing (1)
.github/workflows/docker.yml
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Tag publishes never cancel and are not replaced by main queue slots; branch publishes still cancel superseded main runs. Co-authored-by: Grok <noreply@x.ai>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Shared concurrency group for main and v* tags (cancel-in-progress: false) so branch/tag jobs cannot race on :latest and in-flight version-tag publishes are not aborted. Co-authored-by: Grok <noreply@x.ai>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Default concurrency keeps only one pending run; a later main push could replace a queued v* publish and skip the immutable :X.Y.Z image. queue: max retains pending tag jobs while still serializing all :latest writers in one group with cancel-in-progress: false. Co-authored-by: Grok <noreply@x.ai>
Move dual GHCR/Hub publish from Unreleased into the 0.4.1 section so the crates.io + image release story matches one version. Co-authored-by: Grok <noreply@x.ai>

Summary
Part of #26 / #26: Docker package for GitHub Container Registry and Docker Hub.
Dockerfile— multi-stage: builder runscargo test --all-features+ releaseload_inspect_lif; runtime image is Rust 1.97 + libhdf5 + workspace + binary.github/workflows/docker.yml— neuromod-style:contents: read, nopackages: write)v*tags: dual push Hub + GHCRlatest, Cargo.toml semver, commit SHARegistries
ghcr.io/limen-neural/nir-rs${{ vars.DOCKER_USER }}/nir-rsSecrets
vars.DOCKER_USER,secrets.DOCKER_PAT,GITHUB_TOKEN(packages:write on publish job only)Test plan
docker build --target buildersucceedsCloses nothing alone — remaining #26 work:
cargo publish+ GitHub Release notes.Co-authored-by: Grok noreply@x.ai
Summary by cubic
Dual-publish
nir-rsDocker image to GHCR and Docker Hub with a verify-on-PR, publish-on-main/tags CI. Part of #26 / #26 to ship a stable Rust 1.97 +libhdf5toolchain image with automated releases.Dockerfile: multi-stage; builder runscargo test --all-featuresand buildsload_inspect_lif; runtime includes Rust 1.97 +libhdf5..github/workflows/docker.yml): PRs verify build;main/v*tags dual-publish toghcr.io/limen-neural/nir-rsand${{ vars.DOCKER_USER }}/nir-rs.latest, semver fromCargo.toml, and commit SHA.vars.DOCKER_USER,secrets.DOCKER_PAT,GITHUB_TOKEN(packages:write on publish).Written for commit 5475fd9. Summary will update on new commits.